Skip to content

fix(api): improve commitment API route authorization and validation b… - #1865

Open
Gabugo-tech wants to merge 5 commits into
Commitlabs-Org:masterfrom
Gabugo-tech:fix/issue-1758-api-validation
Open

Gabugo-tech wants to merge 5 commits into
Commitlabs-Org:masterfrom
Gabugo-tech:fix/issue-1758-api-validation

Conversation

@Gabugo-tech

Copy link
Copy Markdown

[#1758] Improve commitment API route validation: authorization and hostile-input boundary

Description

This PR addresses #1758 by implementing strict authorization boundaries, input schema validation, network checks, and response structure validation for the commitment API endpoints anchored at src/app/api/commitments/route.ts and src/app/api/commitments/search/route.ts.

Changes Made

  • Authorization & Ownership Cross-Checks:
    • Enforced requireAuth / verifyAuth on GET and POST handlers prior to parsing parameters or making external chain calls.
    • Cross-checked query/body ownerAddress against the verified session address, returning 403 FORBIDDEN on identity mismatch to prevent identity spoofing or tampering.
  • Hostile Input & Schema Validation:
    • Enforced strict 56-character Stellar public key format (/^G[A-Z2-7]{55}$/) for ownerAddress.
    • Added verification for supported asset codes (XLM, USDC) and verified client-supplied network passphrase against server configuration (getBackendConfig()).
    • Enforced positive finite numerical bounds on commitment amounts (amount > 0).
    • Added response shape checks on chain service outputs.
  • Automated Testing:
    • Created unit test suites in src/app/api/commitments/route.test.ts and src/app/api/commitments/search/route.test.ts covering authentication failures (401), authorization mismatches (403), invalid formats (400), and successful execution (200/201).

Acceptance Criteria Verification

  • Establish the feature's explicit state, data, authorization, and failure invariants.
  • Validate route parameters, wallet identity, network, numeric values, and server responses at the boundary.
  • Ensure ownership and authorization assumptions are checked rather than inferred from client state.
  • Cover replay, tampering, wrong-network, disconnected-wallet, and malformed-response scenarios.
  • Keep the change focused on this feature and preserve unrelated public behavior.
  • Automated tests cover success, failure, boundary, retry, and permission behavior.

Validation Results

Executed test suite:

pnpm vitest run --environment node src/app/api/commitments/route.test.ts src/app/api/commitments/search/route.test.ts


closes #1758

@vercel

vercel Bot commented Aug 30, 2026

Copy link
Copy Markdown

@Gabugo-tech is attempting to deploy a commit to the 1nonly's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Aug 30, 2026

Copy link
Copy Markdown

@Gabugo-tech 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! 🚀

Learn more about application limits

…oundaries

- Enforce session authentication and ownership check against session address (Refs Commitlabs-Org#1758)
- Validate Stellar public key regex, supported asset codes, and numeric creation bounds
- Add comprehensive automated unit test suites for route.ts and search/route.ts

Refs Commitlabs-Org#1758
@Gabugo-tech
Gabugo-tech force-pushed the fix/issue-1758-api-validation branch from 959bf51 to 1c112e1 Compare August 30, 2026 20:59
…n and hostile-input boundary

- Enforce server-side session auth via verifyAuth(); callerAddress in body
  is now optional and cross-checked against the session identity rather
  than trusted directly, preventing tampered-body spoofing.
- Validate callerAddress format against the Stellar public-key regex
  (G[A-Z2-7]{55}) before it reaches any business logic.
- Add network passphrase check: optional
etwork field in body must
  match server-configured networkPassphrase; rejects wrong-network wallets.
- Cap idempotency key length at 128 chars to prevent storage inflation.
- Validate commitment.amount from chain is finite and positive; rejects
  zero, negative, and non-numeric values from malformed chain responses.
- Validate chain service response shape: commitmentId echo must match
  requested id, txHash must be a string when present.
- Activate route.test.ts in vitest.config.ts (was excluded as pre-migration
  placeholder); 46/46 tests pass covering: success, idempotency replay,
  disconnected-wallet 401, tampered address 403, wrong-network 400,
  address format 400, key length boundary, numeric amount boundary,
  malformed server response, SETTLED/VIOLATED replay 409, CSRF 403.

Refs Commitlabs-Org#1762
- Hoist idempotencyKey declaration above try block so the catch
  handler can always call idempotencyService.fail() regardless of
  where the error was thrown (was ReferenceError at runtime)
- Switch vi.clearAllMocks() to vi.resetAllMocks() in beforeEach/afterEach
  to wipe mock implementations between tests, preventing state bleed
  that caused order-dependent 403 failures in 409/429/502 tests
- Fix 'special characters' test: mock fundEscrowOnChain to echo the
  requested id so the response shape check (commitmentId echo) passes

All 67 tests pass.
…dation

- Add ForbiddenError import and capture requireAuth() return value;
  cross-check ownerAddress query param against session identity (fixes
  403 for ownership mismatch — was silently passing through)
- Strengthen ownerAddress Zod schema from min(1) to full Stellar public
  key regex G[A-HJ-NP-Z0-9]{55} (fixes 400 for invalid key format)
- Fix test fixtures: replace addresses containing invalid base32 char 'O'
  with valid Stellar keys so regex validation passes for authorized cases

Refs Commitlabs-Org#1758
@Gabugo-tech

Copy link
Copy Markdown
Author

@sasasamaes please review and merge, if there are any issues please call my attention to it

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.

1 participant