Skip to content

feat(create): enforce authorization and validation boundary for commitment wizard and draft recovery - #1851

Open
Joseph-1-Duro wants to merge 1 commit into
Commitlabs-Org:masterfrom
Joseph-1-Duro:fix/commitment-creation-validation
Open

Joseph-1-Duro wants to merge 1 commit into
Commitlabs-Org:masterfrom
Joseph-1-Duro:fix/commitment-creation-validation

Conversation

@Joseph-1-Duro

@Joseph-1-Duro Joseph-1-Duro commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Closes #1750


Objective

Enforces a clear authorization and validation boundary before sensitive UI actions or API calls for src/app/create/page.tsx and src/components/create/ResumeDraftPrompt.tsx.

Fixes production-quality risk where multi-step creation must preserve validated input, recover interrupted drafts, and avoid stale/duplicate submissions.

Refs #1831

Invariants (state / data / authorization / failure)

  • State: step∈[1,3], linear 1→2→3, draft version, createdAt/updatedAt with 7d TTL, prune on load/write.
  • Data: type∈{safe,balanced,aggressive}, asset∈{XLM,USDC}, amount strict decimal ^\d+(\.\d{1,7})?$ , finite >0 ≤1_000_000, durationDays int 1..365, maxLossPercent 0..100 (maxLossBps 0..10000), sourceId ^[A-Za-z0-9_-]{1,64}$, idempotency key ^[A-Za-z0-9_-]{8,64}$.
  • Authorization: connected && address && authenticated, walletNetwork === NEXT_PUBLIC_NETWORK_PASSPHRASE (wrong-network block), draft walletAddress === currentAddress (legacy drafts allowed but ownership re-checked on resume), POST /api/commitments requires verifyAuth || requireAuth and ownerAddress === authAddress (403 otherwise), not inferred from client state.
  • Failure: disconnected → wallet-disconnected-banner + disable submit, wrong-network → wrong-network-banner, tampered/expired draft → discard + field-error-banner + clear, malformed prefill → safe null fallback, malformed create response → throw + retryable error, replay → idempotency STARTED⇒409, COMPLETED⇒200 cached, rate-limit 429, CSRF origin check preserved.

Boundary Validation

  • Route params: sourceId regex + encodeURIComponent, response via CommitmentResponseSchema.safeParse, identity fields dropped.
  • Wallet identity/network: useWallet() gating via checkWalletBoundary (src/lib/validation/walletBoundary.ts).
  • Numeric values: parseAmountStrict, AssetSchema, clamped duration/maxLoss with finite checks.
  • Server responses: CommitmentResponseSchema/CreateResponseSchema safeParse, missing/invalid commitmentId → error, not blind trust.
  • Ownership checked server-side, not from localStorage.

Scenarios Covered

  • replay/tampering (tampered JSON, out-of-range duration, invalid id, Infinity/NaN, prototype pollution __proto__), wrong-network, disconnected-wallet (banner + block submit), malformed-response (invalid JSON, missing fields), permission (owner mismatch 403, unauthenticated), retry (idempotency fail → allow retry, STARTED → 409, network abort).

Tests

pnpm test -- --run — 644 passed, 1 pre-existing failure (src/app/api/analytics/protocol/route.test.ts missing mockDb).
New focused tests (61):

  • src/lib/validation/createCommitment.test.ts — SourceId/Amount/Asset/DraftState/Idempotency boundary
  • src/lib/validation/walletBoundary.test.ts — DISCONNECTED/WRONG_NETWORK/UNAUTHENTICATED
  • src/hooks/__tests__/useDraftPersistence.boundary.test.ts — TTL, tamper, pollution, expired, valid
  • src/app/api/commitments/route.createBoundary.test.ts — 200, 400 (NaN/Infinity/ETH/9999), 403 ownership, 409 replay, invalid key, fail-retry, malformed JSON
  • src/app/create/createBoundary.test.tsx — disconnected/wrong-network banners, tampered draft blocked, duplicate submit guard

Validation commands:

NVM 20.19.5
pnpm test -- src/lib/validation/createCommitment.test.ts src/lib/validation/walletBoundary.test.ts src/hooks/__tests__/useDraftPersistence.boundary.test.ts src/app/api/commitments/route.createBoundary.test.ts src/app/create/createBoundary.test.tsx
pnpm test -- --run

Design Tradeoffs & Limitations

  • Client localStorage is untrusted convenience — server is source of truth. No HMAC secret, so tamper detection is schema-based discard + prune, not cryptographic binding. Documented.
  • Draft scoping is filter-by-walletAddress (legacy anon drafts visible). Alternative per-wallet key commitlabs-create-drafts:${address} noted for stricter privacy.
  • Idempotency uses existing idempotencyService (24h TTL). Idempotency-Key required format 8-64 alnum/_/-; missing key still allowed for backward compat but replay protection only with key.
  • Asset allowlist strictly XLM/USDC via validateSupportedAsset (ETH rejected) — matches chain contract.
  • Explorer URL derived via getExplorerNetworkFromPassphrase(walletNetwork ?? expected) not hardcoded testnet.
  • Stubs added for missing CreateCommitmentStepReview / CommitmentCreatedModal to fix case-sensitive import not found (Commitmentcreatedmodal.tsx vs correct case) — minimal functional stubs.

Remaining

No secrets, no unrelated refactors, no test removals.

…tment wizard and draft recovery

- Define explicit invariants in src/lib/validation/createCommitment.ts and src/lib/validation/walletBoundary.ts (SourceId, Amount, Asset, DraftState, IdempotencyKey, wallet/network gates)
- Harden useDraftPersistence: strict zod (step 1..3, duration 1..365, maxLoss 0..100), pruneExpiredDrafts, isValidDraftId guard, wallet-scoped visible drafts, tamper discard and flush handling
- Harden usePrefillFromCommitment: sourceId regex validation, AbortController, safeParse response, sanitize amount/asset/duration, clamp with finite checks
- Fix create wizard (src/app/create/page.tsx): wallet/network/auth gates with banners, strict parseAmountStrict, asset allowlist, draft resume re-validation with tamper/ownership errors, idempotent submit (Idempotency-Key, CSRF, Bearer), malformed response handling, explorer network via getExplorerNetworkFromPassphrase, replay/retry handling
- Fix integration with ResumeDraftPrompt (drafts[]/onResume(draftId)/onDeleteDraft) and backend POST /api/commitments (idempotency replay protection, requireAuth ownership check, strict numeric validation)
- Implement validateSupportedAsset/validateStellarAddress checks
- Add stubs for missing components and focused tests covering success/failure/boundary/retry/permission
- Refs Commitlabs-Org#1831

Validation:
- NVM 20.19.5, pnpm test -- --run (57 passed, 644 total, 1 pre-existing mockDb failure)
@vercel

vercel Bot commented Aug 30, 2026

Copy link
Copy Markdown

@Joseph-1-Duro 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

@Joseph-1-Duro 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

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.

[Quality][High] Improve commitment creation wizard and draft recovery: authorization and hostile-input boundary

1 participant