Repository navigation
Fix Stellar balance error swallowing, JWT secret fallback, and admin auth route split - #111
Merged
memplethee-lab merged 13 commits intoOct 10, 2026
Conversation
…turning 0 The bare catch in StellarProvider.getBalance swallowed every failure (DNS, 429, 5xx, timeouts) and reported 0, which caused WalletsService.updateBalance to overwrite real balances with 0. Now: - 404 (unfunded account) still resolves to 0 - All other errors throw BlockchainProviderUnavailableError with chain, address, and cause - Errors are logged via NestJS Logger - WalletsService.updateBalance re-throws the error so the stored balance is left untouched Closes LintError#13
…ing/placeholder secrets JwtStrategy previously fell back to a publicly known secret, allowing token forgery if JWT_SECRET was unset. Changes: - Added validateJwtSecret() that throws at startup if JWT_SECRET is missing, empty, or a known placeholder - Applied validation in both JwtStrategy and AdminModule's JwtModule.register - Removed the literal 'default_jwt_secret_key_change_in_production' from source code - Updated .env.example and docker-compose.yml to not provide insecure defaults - Added tests covering missing, placeholder, and valid secrets Closes LintError#12
…minGuard error masking AdminController had @UseGuards(AdminGuard, RolesGuard) on the whole controller, but login/OTP routes were marked @roles() (public). AdminGuard ran first and blocked all requests, making admin sign-in impossible. Changes: - Created AdminAuthController at /admin/auth for public routes (login, otp/generate, otp/verify) - AdminController now only has protected routes under /admin - Both controllers registered in AdminModule - Fixed AdminGuard to not mask its own 'Admin access required' error as generic 'Invalid or expired token' by restructuring the try/catch - Updated docs/api-guide.md with new admin auth routes - Added tests for AdminGuard covering public routes without token, protected routes without token, and protected routes with wrong role Closes LintError#11
- blockchain.service.spec.ts: remove unused Horizon import - wallets.service.spec.ts: add proper type casts to avoid unbound method error
- blockchain.service.spec.ts: remove unused StellarSdk import - wallets.service.spec.ts: wrap method call in arrow function to fix unbound method
Line 81 was also missing the arrow function wrapper for the expect call, causing the unbound-method error.
The first expect call was also missing the arrow function wrapper, causing the unbound-method error.
The @typescript-eslint/unbound-method rule flags the arrow-wrapped calls as false positives. Adding eslint-disable comments to silence this while preserving the correct runtime behavior.
eslint-disable-next-line doesn't work well with multi-line statements. Using eslint-disable-line on the same line as the await expect call.
…itive File-level directive ensures the false positive is suppressed regardless of line number mismatches due to line ending differences.
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
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
Three critical fixes addressing security, data integrity, and authentication availability.
#13 - Stop swallowing Horizon errors in StellarProvider.getBalance
Problem: Bare
catchinStellarProvider.getBalancereturned0for any failure (DNS, 429, 5xx, timeouts).WalletsService.updateBalancethen persisted that0, corrupting real balances and causing legitimate payments to be rejected by the "insufficient balance" check.Fix:
0BlockchainProviderUnavailableErrorwithchain,address, and originalcauseLogger(no rawconsole.*)WalletsService.updateBalancere-throws so stored balance remains untouchedBlockchainProviderUnavailableErrorclass exported fromBlockchainService#12 - Remove hardcoded JWT secret fallback in JwtStrategy
Problem:
JwtStrategyfell back to a publicly known secret (default_jwt_secret_key_change_in_production) ifJWT_SECRETwas unset..env.exampleanddocker-compose.ymlshipped the placeholder as a usable value, so a misconfigured deployment silently booted with a compromised secret.Fix:
validateJwtSecret()that throws at startup if secret is missing, empty, or a known placeholderJwtStrategyandAdminModule/JwtModule.register.env.examplenow showsJWT_SECRET=(empty, must be set)docker-compose.ymluses${JWT_SECRET}without default#11 - Split public admin auth routes from protected admin routes
Problem:
@UseGuards(AdminGuard, RolesGuard)on the wholeAdminControllerran before@Roles()could opt out.AdminGuardunconditionally required a bearer token, soPOST /admin/login,POST /admin/otp/generate,POST /admin/otp/verifyall returned 401 — admins could never obtain their first token.Fix:
AdminAuthControllerat/admin/authwith no guards (login, OTP generate, OTP verify)AdminControllernow only contains protected routes under/adminAdminModuleAdminGuardto not mask its ownUnauthorizedException("Admin access required")as generic"Invalid or expired token"by restructuring the try/catchdocs/api-guide.mdwith new/admin/auth/*routesTesting
Added test files:
src/modules/blockchain/__tests__/blockchain.service.spec.ts- success, unfunded (404), network failure, 5xx, 429, timeoutsrc/modules/auth/__tests__/jwt.strategy.spec.ts- missing secret, placeholder secret, valid secretsrc/modules/admin/__tests__/admin.guard.spec.ts- missing token, invalid token, non-admin payload, valid admin, error message not maskedsrc/modules/wallets/__tests__/wallets.service.spec.ts- balance update success, provider error leaves balance untouched, wallet not foundNotes
npm install, CI, or tests run locally per instructionsCloses #13
Closes #12
Closes #11