Revert "fix: Enforce authenticated user ID in savings controller"#62
Revert "fix: Enforce authenticated user ID in savings controller"#62
Conversation
📝 WalkthroughWalkthroughESLint configuration removed entirely. String literals converted from double to single quotes across multiple controllers and services. Function signatures reformatted from multi-line to single-line. Auth extraction in savingsController changed from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
cl failed @Uchechukwu-Ekezie |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/controllers/savingsController.ts`:
- Around line 18-26: The handlers in savingsController.ts are trusting
req.body.user / req.query.user and passing it to acbuSavingsVaultService.deposit
(and similar calls) which allows one caller to act as another; instead, resolve
the caller's Stellar address from the authenticated principal (AuthRequest) or
verify the supplied address belongs to that principal before calling
acbuSavingsVaultService methods (e.g., acbuSavingsVaultService.deposit), and
throw an AppError(403) if the address does not match or cannot be resolved;
update all savings-related handlers in this file to use the principal-derived
address (or ownership check) instead of blindly forwarding the request-supplied
user.
In `@src/jobs/usdcConvertAndMintJob.ts`:
- Line 8: The import at the top of this file uses the wrong named export;
replace the current import of "db" from '../config/database' with the named
export "prisma" so the module matches the exported identifier used throughout
(references to prisma at lines where functions like the usdc conversion/mint
logic call prisma). Ensure the import reads the named export prisma so all
subsequent uses of prisma in this file resolve correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 56ef5959-ede7-4025-aff4-cb304a0494f5
📒 Files selected for processing (6)
.eslintrc.jsonsrc/controllers/investmentController.tssrc/controllers/savingsController.tssrc/jobs/usdcConvertAndMintJob.tssrc/services/auth/authService.tssrc/services/stellar/contractClient.ts
💤 Files with no reviewable changes (1)
- .eslintrc.json
| const { user, amount, term_seconds } = (req as AuthRequest).body || {}; | ||
| if (!user || !amount || term_seconds == null) { | ||
| throw new AppError('user, amount, and term_seconds required', 400); | ||
| } | ||
| if (!contractAddresses.savingsVault) { | ||
| throw new AppError("Savings vault contract not configured", 503); | ||
| throw new AppError('Savings vault contract not configured', 503); | ||
| } | ||
| const result = await acbuSavingsVaultService.deposit({ | ||
| user: userId, | ||
| user, |
There was a problem hiding this comment.
Don't trust user from the request on savings endpoints.
These handlers now take req.body.user / req.query.user and forward it straight into acbuSavingsVaultService. The savings routes only enforce API key + scope, and the contract service passes user directly to the contract, so any authenticated caller can target another Stellar address by swapping that field. Resolve the caller's Stellar address from the authenticated principal, or verify the supplied address belongs to that principal before invoking the contract.
Also applies to: 55-63, 79-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/controllers/savingsController.ts` around lines 18 - 26, The handlers in
savingsController.ts are trusting req.body.user / req.query.user and passing it
to acbuSavingsVaultService.deposit (and similar calls) which allows one caller
to act as another; instead, resolve the caller's Stellar address from the
authenticated principal (AuthRequest) or verify the supplied address belongs to
that principal before calling acbuSavingsVaultService methods (e.g.,
acbuSavingsVaultService.deposit), and throw an AppError(403) if the address does
not match or cannot be resolved; update all savings-related handlers in this
file to use the principal-derived address (or ownership check) instead of
blindly forwarding the request-supplied user.
| import type { ConsumeMessage } from 'amqplib'; | ||
| import { connectRabbitMQ, QUEUES } from '../config/rabbitmq'; | ||
| import { logger } from '../config/logger'; | ||
| import { db } from '../config/database'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the import statement at line 8 of the job file
sed -n '1,15p' src/jobs/usdcConvertAndMintJob.tsRepository: Pi-Defi-world/acbu-backend
Length of output: 638
🏁 Script executed:
# Next, check what src/config/database.ts exports
cat src/config/database.tsRepository: Pi-Defi-world/acbu-backend
Length of output: 1159
🏁 Script executed:
# Check the references to prisma in the job file
grep -n "prisma" src/jobs/usdcConvertAndMintJob.tsRepository: Pi-Defi-world/acbu-backend
Length of output: 278
Fix import to match the exported identifier.
src/config/database.ts exports prisma as a named export, not db. The current import will fail to type-check, and all references in this file (lines 51, 64, 76, 92) use prisma, not db.
🐛 Fix
-import { db } from '../config/database';
+import { prisma } from '../config/database';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { db } from '../config/database'; | |
| import { prisma } from '../config/database'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/jobs/usdcConvertAndMintJob.ts` at line 8, The import at the top of this
file uses the wrong named export; replace the current import of "db" from
'../config/database' with the named export "prisma" so the module matches the
exported identifier used throughout (references to prisma at lines where
functions like the usdc conversion/mint logic call prisma). Ensure the import
reads the named export prisma so all subsequent uses of prisma in this file
resolve correctly.
1 participant
Reverts #58
Summary by CodeRabbit
Style
Refactor
Chores