Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an AuditService and integrates non-blocking audit logs across hosting API routes and payment processing; introduces a HostingSignup React flow with Hive Keychain payment and route; adds Vitest configs and multiple unit tests; switches docker-compose to image-based deploys and adds a multi-stage GitHub Actions workflow and deployment docs updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Browser
participant HSC as HostingSignup (UI)
participant API as Hosting API
participant HK as Hive Keychain
User->>HSC: Enter username
HSC->>API: GET /v1/tenants/{username}/status
API-->>HSC: availability / status
User->>HSC: Submit config
HSC->>API: POST /v1/tenants (username, config)
API-->>HSC: Created + PaymentInstructions
User->>HSC: Click "Pay with Keychain"
HSC->>HK: Initiate transfer (to, amount, memo)
HK-->>HSC: Transfer success
HSC->>API: Poll GET /v1/tenants/{username}/status
API-->>HSC: subscriptionStatus = "active"
HSC->>User: Show success (onSuccess)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/self-hosted/src/features/hosting/components/hosting-signup.tsx (1)
225-226: Move domain and price text out of hardcoded defaults.Line 226 and Line 434-Line 435 hardcode
blogs.ecency.com,1 HBD/month, and3 HBD/month, but the backend already makesBASE_DOMAIN,MONTHLY_PRICE_HBD, andPRO_UPGRADE_PRICE_HBDenvironment-driven in apps/self-hosted/hosting/api/src/routes/tenants.ts. That will drift on staging/self-hosted deployments.Also applies to: 433-436
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/self-hosted/src/features/hosting/components/hosting-signup.tsx` around lines 225 - 226, The UI hardcodes the domain and price strings; update the HostingSignup component to stop using literal "blogs.ecency.com", "1 HBD/month", and "3 HBD/month" and instead consume the environment-driven values the backend exposes (BASE_DOMAIN, MONTHLY_PRICE_HBD, PRO_UPGRADE_PRICE_HBD). Locate the JSX that renders the blog URL (the line showing "Your blog will be at: {username || 'username'}.blogs.ecency.com") and the price lines (the monthly and pro upgrade price text in the same component), and replace those string literals with values provided by a prop or by fetching the tenant/settings endpoint used by tenants.ts (or by importing a shared config if available) so the frontend displays domain and prices from the backend-driven config rather than hardcoded defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/self-hosted/hosting/api/src/routes/tenants.ts`:
- Around line 108-114: Audit entries currently pass
c.req.header('x-forwarded-for') directly into parseClientIp (seen in the
AuditService.log calls), which allows client-controlled spoofing; update those
AuditService.log calls (at the spots using parseClientIp and
c.req.header('x-forwarded-for')) to derive the client IP from a trusted proxy
boundary instead of the raw header—for example, use the framework/request object
method that returns the remote peer IP (e.g., c.req.ip or
connection.remoteAddress) or a central trusted-proxy extractor configured once
and referenced here, then pass that trusted value into parseClientIp before
logging.
In `@apps/self-hosted/src/features/hosting/components/hosting-signup.tsx`:
- Around line 85-92: The check that only blocks active tenants is incorrect —
treat any existing tenant as unavailable. In the block that inspects the tenant
status (where you currently reference data.exists and data.subscriptionStatus),
change the logic to reject when data.exists is true (e.g., if (data.exists) {
setError('This username is already taken'); return; }) so unpaid/expired tenants
are blocked up front instead of letting setConfig/setStep advance to the
configure step and failing later on create.
---
Nitpick comments:
In `@apps/self-hosted/src/features/hosting/components/hosting-signup.tsx`:
- Around line 225-226: The UI hardcodes the domain and price strings; update the
HostingSignup component to stop using literal "blogs.ecency.com", "1 HBD/month",
and "3 HBD/month" and instead consume the environment-driven values the backend
exposes (BASE_DOMAIN, MONTHLY_PRICE_HBD, PRO_UPGRADE_PRICE_HBD). Locate the JSX
that renders the blog URL (the line showing "Your blog will be at: {username ||
'username'}.blogs.ecency.com") and the price lines (the monthly and pro upgrade
price text in the same component), and replace those string literals with values
provided by a prop or by fetching the tenant/settings endpoint used by
tenants.ts (or by importing a shared config if available) so the frontend
displays domain and prices from the backend-driven config rather than hardcoded
defaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8291141-7b17-4943-93a1-d8ae15523c18
📒 Files selected for processing (3)
.github/workflows/self-hosted.ymlapps/self-hosted/hosting/api/src/routes/tenants.tsapps/self-hosted/src/features/hosting/components/hosting-signup.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/self-hosted.yml
Comment on lines
+108
to
+114
| void AuditService.log({ | ||
| tenantId: tenant.id, | ||
| eventType: 'tenant.created', | ||
| eventData: { username: body.username }, | ||
| ipAddress: parseClientIp(c.req.header('x-forwarded-for')), | ||
| userAgent: c.req.header('user-agent'), | ||
| }); |
Contributor
There was a problem hiding this comment.
Don't trust raw X-Forwarded-For for audit IPs.
Line 112 records c.req.header('x-forwarded-for') directly. Unless your ingress strips and rebuilds that header, it's client-controlled, so these new audit entries can be spoofed. The same pattern repeats at Line 237, Line 361, Line 402, and Line 464; please derive the IP at a trusted proxy boundary instead of trusting the raw header here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/self-hosted/hosting/api/src/routes/tenants.ts` around lines 108 - 114,
Audit entries currently pass c.req.header('x-forwarded-for') directly into
parseClientIp (seen in the AuditService.log calls), which allows
client-controlled spoofing; update those AuditService.log calls (at the spots
using parseClientIp and c.req.header('x-forwarded-for')) to derive the client IP
from a trusted proxy boundary instead of the raw header—for example, use the
framework/request object method that returns the remote peer IP (e.g., c.req.ip
or connection.remoteAddress) or a central trusted-proxy extractor configured
once and referenced here, then pass that trusted value into parseClientIp before
logging.
apps/self-hosted/src/features/hosting/components/hosting-signup.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/self-hosted/hosting/docker-compose.yml (1)
77-77: Use bash parameter expansion to fail fast whenTAGis missing.The deployment workflow already exports
TAGbefore runningdocker compose up -d, so the:-latestfallback only masks configuration errors in ad-hoc or miswired runs. Instead, use${TAG:?TAG must be set}to fail immediately ifTAGis unset or empty, surfacing wiring bugs rather than silently pulling the mutablelatestimage.♻️ Suggested change
- image: ecency/self-hosted:${TAG:-latest} + image: ecency/self-hosted:${TAG:?TAG must be set} - image: ecency/hosting-api:${TAG:-latest} + image: ecency/hosting-api:${TAG:?TAG must be set} - image: ecency/hosting-api:${TAG:-latest} + image: ecency/hosting-api:${TAG:?TAG must be set}Also applies to: 125-125, 161-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/self-hosted/hosting/docker-compose.yml` at line 77, The compose file currently uses the fallback expansion "image: ecency/self-hosted:${TAG:-latest}" which hides missing TAG values; update this to use the failing expansion "${TAG:?TAG must be set}" so docker compose will fail fast when TAG is unset or empty (replace each occurrence of "image: ecency/self-hosted:${TAG:-latest}" with "image: ecency/self-hosted:${TAG:?TAG must be set}" and apply the same change for the other occurrences in the file).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/self-hosted/hosting/docker-compose.yml`:
- Line 77: The compose file currently uses the fallback expansion "image:
ecency/self-hosted:${TAG:-latest}" which hides missing TAG values; update this
to use the failing expansion "${TAG:?TAG must be set}" so docker compose will
fail fast when TAG is unset or empty (replace each occurrence of "image:
ecency/self-hosted:${TAG:-latest}" with "image: ecency/self-hosted:${TAG:?TAG
must be set}" and apply the same change for the other occurrences in the file).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b4190c89-e99b-45ac-8e16-05115e3b7e91
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/self-hosted/hosting/api/src/index.tsapps/self-hosted/hosting/api/src/services/audit-service.tsapps/self-hosted/hosting/docker-compose.ymlapps/self-hosted/src/features/hosting/components/hosting-signup.tsxapps/self-hosted/src/routeTree.gen.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Backend
Tests
Chores