fix: restore auth protection for protected routes - #826
Conversation
|
@Ayilojay is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@Ayilojay Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThe backend now supports legacy environment-variable mappings, amount filtering for recipient streams, SEP-10 challenge issuance, body-aware JSON validation, migration execution during database startup, and corrected paused-stream vesting calculations. ChangesBackend runtime and API behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthChallengeEndpoint
participant ChallengeTransactionGenerator
Client->>AuthChallengeEndpoint: GET /api/auth/challenge with accountId
AuthChallengeEndpoint->>ChallengeTransactionGenerator: generate challenge transaction
ChallengeTransactionGenerator-->>AuthChallengeEndpoint: transaction
AuthChallengeEndpoint-->>Client: transaction and network passphrase
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/config/validateEnv.ts`:
- Around line 112-120: Use one canonical value for each Stellar setting: in
backend/src/config/validateEnv.ts lines 112-120, derive contract, RPC, and
network values after parsing, then validate and return only those canonical
values so either legacy or new environment variables work consistently; update
backend/src/index.ts line 1193 to use the canonical configured network for the
challenge response instead of reading only NETWORK_PASSPHRASE.
In `@backend/src/index.ts`:
- Around line 1182-1189: Update the `/api/auth/challenge` handler to validate
the request query through the shared Stellar-account Zod schema from
`validation/schemas.ts` before calling `generateChallenge()`. Use the schema’s
parsed accountId for downstream processing, and return the existing 400
validation response when parsing fails instead of allowing invalid values to
reach the service.
In `@backend/src/services/db.ts`:
- Line 329: Update initDb() to invoke the existing migrate() entry point instead
of calling runMigrations(db) directly. Ensure SQLite initialization and upgrades
remain within migrate(), while the Postgres branch uses a migration path that
explicitly accepts PostgresDatabase.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f2c21fdd-ba6d-4839-ad3a-3d10ad346b83
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
backend/src/config/validateEnv.tsbackend/src/index.tsbackend/src/middleware/contentType.tsbackend/src/services/db.tsbackend/src/services/streamStore.ts
💤 Files with no reviewable changes (1)
- backend/src/services/streamStore.ts
| // Support backwards compatibility: map old variables to new ones if new ones are not set | ||
| if (!process.env.STELLAR_CONTRACT_ID && process.env.CONTRACT_ID) { | ||
| process.env.STELLAR_CONTRACT_ID = process.env.CONTRACT_ID; | ||
| } | ||
| if (!process.env.SOROBAN_RPC_URL && process.env.RPC_URL) { | ||
| process.env.SOROBAN_RPC_URL = process.env.RPC_URL; | ||
| } | ||
| if (!process.env.STELLAR_NETWORK && process.env.NETWORK_PASSPHRASE) { | ||
| process.env.STELLAR_NETWORK = process.env.NETWORK_PASSPHRASE; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use one canonical value for each Stellar setting.
Mapping CONTRACT_ID only into STELLAR_CONTRACT_ID leaves env.CONTRACT_ID unset, so STELLAR_CONTRACT_ID-only deployments still fail the required-config check. The same split causes SOROBAN_RPC_URL/STELLAR_NETWORK to be ignored in validation or returned config, and the challenge response can advertise testnet when only STELLAR_NETWORK is configured.
backend/src/config/validateEnv.ts#L112-L120: derive canonical contract, RPC, and network values after parsing; validate and return those values exclusively.backend/src/index.ts#L1193-L1193: return the canonical configured network rather than reading onlyNETWORK_PASSPHRASE.
📍 Affects 2 files
backend/src/config/validateEnv.ts#L112-L120(this comment)backend/src/index.ts#L1193-L1193
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/config/validateEnv.ts` around lines 112 - 120, Use one canonical
value for each Stellar setting: in backend/src/config/validateEnv.ts lines
112-120, derive contract, RPC, and network values after parsing, then validate
and return only those canonical values so either legacy or new environment
variables work consistently; update backend/src/index.ts line 1193 to use the
canonical configured network for the challenge response instead of reading only
NETWORK_PASSPHRASE.
| app.get("/api/auth/challenge", authChallengeLimiter, (req: Request, res: Response) => { | ||
| const accountId = req.query.accountId; | ||
| if (typeof accountId !== "string" || !accountId.trim()) { | ||
| sendApiError(req, res, 400, "accountId query parameter is required.", { | ||
| code: "VALIDATION_ERROR", | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate accountId with the shared Stellar-account Zod schema.
Any non-empty value reaches generateChallenge(), where an invalid operation source can throw and become a 500. Parse req.query with a schema in backend/src/validation/schemas.ts and return 400 on validation failure.
As per coding guidelines, “validate requests with Zod schemas from backend/src/validation/schemas.ts before passing data to services.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/index.ts` around lines 1182 - 1189, Update the
`/api/auth/challenge` handler to validate the request query through the shared
Stellar-account Zod schema from `validation/schemas.ts` before calling
`generateChallenge()`. Use the schema’s parsed accountId for downstream
processing, and return the existing 400 validation response when parsing fails
instead of allowing invalid values to reach the service.
Source: Coding guidelines
|
|
||
|
|
||
|
|
||
| runMigrations(db); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
ast-grep outline backend/src/services/db.ts --items all --type function
rg -n -C 8 '\bmigrate\s*\(|\baddColumnIfMissing\s*\(|\brunMigrations\s*\(' backend/src/services/db.ts
rg -n -C 8 'PostgresDatabase|runMigrations' backend/srcRepository: ritik4ever/stellar-stream
Length of output: 12242
🏁 Script executed:
#!/bin/bash
set -eu
printf '--- db.ts relevant sections ---\n'
sed -n '1,120p' backend/src/services/db.ts
printf '\n'
sed -n '120,240p' backend/src/services/db.ts
printf '\n'
sed -n '240,340p' backend/src/services/db.ts
printf '\n--- migrations.ts relevant sections ---\n'
sed -n '1,180p' backend/src/services/migrations.ts
printf '\n--- package/imports mentioning postgres driver ---\n'
rg -n '"better-sqlite3"|"pg"|"postgres"|"postgres-js"|postgres|PostgresDatabase|postgres' package.json backend/srcRepository: ritik4ever/stellar-stream
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -eu
echo '--- db.ts relevant sections ---'
sed -n '1,120p' backend/src/services/db.ts
echo
sed -n '120,240p' backend/src/services/db.ts
echo
sed -n '240,340p' backend/src/services/db.ts
echo
echo '--- migrations.ts relevant sections ---'
sed -n '1,180p' backend/src/services/migrations.ts
echo
echo '--- package/imports mentioning postgres driver ---'
rg -n '"better-sqlite3"|"pg"|"postgres"|"postgres-js"|postgres|PostgresDatabase|postgres' package.json backend/srcRepository: ritik4ever/stellar-stream
Length of output: 15024
Keep migration execution within migrate() and the correct backend path.
initDb() now calls runMigrations(db) after both SQLite and Postgres initialization. Route this through the existing migrate() entry point so SQLite schema initialization and upgrades stay in the required backward-compatible path, and keep Postgres migrations on a path that explicitly supports PostgresDatabase; otherwise startup may fail or schema upgrades may bypass the enforced migration boundary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/services/db.ts` at line 329, Update initDb() to invoke the
existing migrate() entry point instead of calling runMigrations(db) directly.
Ensure SQLite initialization and upgrades remain within migrate(), while the
Postgres branch uses a migration path that explicitly accepts PostgresDatabase.
Source: Coding guidelines
2 participants
What changed
Testing done
Related issues
Closes #
Checklist
Closes [FEATURE] Implement wallet-authenticated write endpoints using Stellar challenge-response #597
Summary by CodeRabbit
New Features
Bug Fixes