fix(api): validate userId is a Stellar public key on notification routes

[xeladev4]

fix(api): validate userId is a Stellar public key on notification routes

Closes #7

Problem

POST /api/notifications/subscribe, GET /api/notifications/preferences, and
DELETE /api/notifications/unsubscribe accepted any non-empty string as
userId with no check that it is a valid Stellar public key. Invalid IDs were
persisted permanently in notification_preferences, breaking downstream
functionality that assumes userId is a valid Stellar address:

• Auto-rebalancer — iterating subscribers and looking up a bogus userId
  on Stellar Horizon errors out, potentially halting a notification pass.
• Stellar signature verification — Keypair.fromPublicKey(userId) throws on
  a malformed address.
• Database integrity — orphaned rows accumulate with no corresponding
  Stellar account.

Fix

• Added a reusable isValidStellarPublicKey() helper and stellarAddressSchema
  in backend/src/api/validation.ts, backed by
  the Stellar SDK's StrKey.isValidEd25519PublicKey().
• Reject malformed userId with HTTP 400 on all three notification routes in
  backend/src/api/routes.ts.

Why StrKey instead of a regex

The originally proposed ^[GT][A-Z0-9]{54}$ regex is not correct for Stellar
addresses:

• Stellar StrKeys are RFC 4648 base32 (A–Z, 2–7) — the regex's 0,1,8,9
  are never valid, while 2–7 are missing.
• The T prefix is pre-auth transaction keys, not "testnet"; account public
  keys always start with G regardless of network.
• A regex cannot verify the trailing CRC16 checksum, so typo'd/truncated
  keys would still pass.

StrKey.isValidEd25519PublicKey() validates length, alphabet, version byte and
checksum, so it correctly rejects malformed keys, secret seeds (S…), and
muxed accounts (M…).

Note: the optional CHECK (user_id ~ '...') constraint from the issue was
intentionally not added — the backend uses SQLite (better-sqlite3),
which does not support the PostgreSQL ~ regex operator. Application-layer
validation is the correct enforcement point here.

Tests

Added backend/src/test/stellarAddressValidation.test.ts
covering valid keys, arbitrary strings, bad checksums, secret seeds, and
non-string inputs.

✓ src/test/stellarAddressValidation.test.ts (7 tests)

npm run build (tsc) passes clean.

Steps to verify

# Invalid userId is now rejected
curl -X POST http://localhost:3001/api/notifications/subscribe \
  -H "Content-Type: application/json" \
  -d '{"userId":"not-a-stellar-address","emailEnabled":true,"emailAddress":"user@example.com","webhookEnabled":false,"events":{"rebalance":true,"circuitBreaker":true,"priceMovement":true,"riskChange":true}}'
# -> 400 { "success": false, "error": "userId must be a valid Stellar public key (G... address)" }

[Uchechukwu-Ekezie]

The StrKey explanation is spot on — the regex approach from the issue would have passed 0, 1, 8, 9 and missed 2–7, and there's no way to verify the checksum without the SDK anyway. Good call using isValidEd25519PublicKey directly.

The type guard (value is string) is correctly typed and the null/undefined handling before calling into the SDK is the right move since some SDK versions throw on non-string input.

Two things worth a look before merge:

The stellarAddressSchema is exported from validation.ts but the routes use isValidStellarPublicKey directly rather than going through Zod. That's fine for now, but if someone adds a new route and reaches for the schema they'll get a different code path from the existing three. Worth either wiring the routes through the schema or noting the split intentionally.

The tests cover the helper and schema in isolation which is thorough, but there's no HTTP-level test confirming the routes actually return 400 for a bad userId. A quick integration test hitting /api/notifications/preferences?userId=not-a-key would close that loop.

Overall this is clean — the core fix is correct and the route coverage is consistent across all three endpoints.

[xeladev4]

The StrKey explanation is spot on — the regex approach from the issue would have passed 0, 1, 8, 9 and missed 2–7, and there's no way to verify the checksum without the SDK anyway. Good call using isValidEd25519PublicKey directly.

The type guard (value is string) is correctly typed and the null/undefined handling before calling into the SDK is the right move since some SDK versions throw on non-string input.

Two things worth a look before merge:

The stellarAddressSchema is exported from validation.ts but the routes use isValidStellarPublicKey directly rather than going through Zod. That's fine for now, but if someone adds a new route and reaches for the schema they'll get a different code path from the existing three. Worth either wiring the routes through the schema or noting the split intentionally.

The tests cover the helper and schema in isolation which is thorough, but there's no HTTP-level test confirming the routes actually return 400 for a bad userId. A quick integration test hitting /api/notifications/preferences?userId=not-a-key would close that loop.

Overall this is clean — the core fix is correct and the route coverage is consistent across all three endpoints.

Thanks for the thorough review — appreciate the detailed read.

On the schema/helper split: the two aren't actually different validation
code paths. stellarAddressSchema is defined as
z.string().refine(isValidStellarPublicKey, …), so anyone reaching for the
schema runs the exact same check as the three routes — the only difference is
error shape (a ZodError vs. the manual 400 JSON). I used the helper directly in
the routes to stay consistent with the surrounding handlers, which all do manual
field validation rather than running through Zod. Happy to add a short comment
above the schema making that intent explicit so the next person isn't surprised
by the split. If you'd rather standardize the notification routes onto Zod, I can
do that too, but it felt out of scope to convert just these three while the rest
of the file stays manual.

On the integration test: agreed, that closes the loop. I'll add an HTTP-level
test asserting a 400 for a bad userId on the notification routes (e.g.
GET /api/notifications/preferences?userId=not-a-key) and push it to the branch.

[Uchechukwu-Ekezie]

nice job @xeladev4

merging soon