Skip to content

feat(config): validate secret format and length at startup#37

Open
MarvyNwaokobia wants to merge 3 commits into
LabsCrypt:mainfrom
MarvyNwaokobia:feat/issue-14-startup-secret-validation
Open

feat(config): validate secret format and length at startup#37
MarvyNwaokobia wants to merge 3 commits into
LabsCrypt:mainfrom
MarvyNwaokobia:feat/issue-14-startup-secret-validation

Conversation

@MarvyNwaokobia

Copy link
Copy Markdown

Summary

  • Enforce minimum 32-character length for JWT_SECRET and INTERNAL_API_KEY at startup
  • Validate LOAN_MANAGER_ADMIN_SECRET parses as a valid Stellar secret key (Ed25519 seed) at startup
  • Fail fast with a clear error message naming the offending variable

Changes

  • src/config/env.ts — Added validateSecretFormat() that runs after presence checks, using StrKey.isValidEd25519SecretSeed from @stellar/stellar-sdk for Stellar key validation and length checks for JWT/API key secrets
  • src/tests/envValidation.test.ts — Added test cases covering all new validation rules (short secrets, invalid Stellar key, valid inputs)

Test plan

  • All 9 env validation tests pass
  • TypeScript compiles without errors
  • Verify startup fails with clear message when secrets are too short
  • Verify startup fails when LOAN_MANAGER_ADMIN_SECRET is not a valid Stellar key

Closes #14

Enforce minimum 32-char length for JWT_SECRET and INTERNAL_API_KEY,
and validate LOAN_MANAGER_ADMIN_SECRET as a valid Stellar secret key.
Fails fast with a clear message naming the offending variable.

Closes LabsCrypt#14

@ogazboiz ogazboiz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice idea and the validation itself is sound: it runs at startup (index.ts) with a 32-char min for JWT/INTERNAL_API_KEY and StrKey.isValidEd25519SecretSeed for the admin secret, only checks when present, and the 9 tests are well structured. two things before it can land:

  1. it rejects the repo's own documented dev creds without updating them. .env.example has INTERNAL_API_KEY=change-me (8 chars, under the new 32 min) and an empty LOAN_MANAGER_ADMIN_SECRET=, so node dist/index.js (the Dockerfile CMD) now process.exit(1) on a fresh checkout. please update .env.example to satisfy the new rules (a 32+ char INTERNAL_API_KEY and a real testnet S... seed) and document the length/format requirements in the README.

  2. prettier fails on src/config/env.ts:40 (CI's lint step runs eslint+prettier, so this fails the job). run: npx prettier --write src/config/env.ts

heads up: since this is your first PR here the CI run is gated (action_required) and shows no checks until i approve it. i'll approve it so you get the signal. fix the two items and it's good.

if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0

…rettier

.env.example INTERNAL_API_KEY was 8 chars (under the new 32-char min)
and LOAN_MANAGER_ADMIN_SECRET was empty. Replace with valid dev defaults
so a fresh checkout passes startup validation. Document the secret
format requirements in the README.

@ogazboiz ogazboiz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both items are fixed, merging once ci is green. .env.example now ships a 32+ char INTERNAL_API_KEY and a valid S... admin seed so a fresh checkout passes the new startup validation, and the README documents the length/format requirements. env.ts is formatted. the validation only fires when a value is present and uses StrKey.isValidEd25519SecretSeed, so it's sane. good.

if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0

ps: CI is red right now on the migrate step, but that is a pre-existing main-branch issue (a commonjs migration file under our ESM package), nothing to do with your change. once that is sorted this lands.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Backend] Startup validation only checks presence of secrets, not their format or length

2 participants