Skip to content

fix: harden commitment API route validation and tests - #1875

Closed
Cole-dev-cyber wants to merge 19 commits into
Commitlabs-Org:masterfrom
Cole-dev-cyber:security/issue-1760-quality-medium-improve-commitment-api-route
Closed

Cole-dev-cyber wants to merge 19 commits into
Commitlabs-Org:masterfrom
Cole-dev-cyber:security/issue-1760-quality-medium-improve-commitment-api-route

Conversation

@Cole-dev-cyber

Copy link
Copy Markdown

Overview

This PR hardens the commitments API route validation path by establishing a durable regression contract for the implementation anchored at src/app/api/commitments/search/route.ts. It introduces a strict validation boundary before service/database work: query parameters are schema-checked, authorization is enforced, errors are mapped to standardized API responses, and query results are bounded. Focused tests cover success, failure, empty, retry, permission, and boundary states, and the supported API contract is documented for existing consumers.

Related Issue

Refs #

Changes

🛡️ Validation and error contract

  • [MODIFY] src/lib/api/errors.ts

    • Adds standard API error subclasses: BadRequestError, UnauthorizedError, ForbiddenError, NotFoundError, RateLimitError, and InternalError.
    • Adds a shared error mapper that produces stable JSON error bodies and HTTP status codes.
  • [MODIFY] src/lib/commitments/service.ts

    • Adds input guards for commitment search queries and commitment IDs.
    • Enforces bounded query behavior: limit defaults to 20, max 100; offset max 10,000.
    • Normalizes empty, not-found, and retryable failures into typed errors.
  • [MODIFY] src/app/api/commitments/search/route.ts

    • Validates q, status, limit, and offset with a reusable schema.
    • Rejects unknown query parameters to prevent ambiguous consumer behavior.
    • Enforces session/permission checks before querying and returns consistent 401/403 responses.
    • Adds Cache-Control: no-store and Retry-After on retryable 503 responses.

🧪 Test coverage

  • [ADD] src/app/api/commitments/search/__tests__/search.test.ts
    • Covers success, empty, malformed input, unknown params, permission denied, not found, and retryable upstream failure.
    • Includes boundary cases: limit=0, limit=101, offset=10001, negative values, and non-integer values.
    • Covers async loading/retry behavior: transient rejection maps to 503 with Retry-After, and a subsequent retry succeeds.

📚 API contract and compatibility

  • [MODIFY] docs/api/commitments.md
    • Documents supported query parameters, response shapes, error codes, pagination bounds, and consumer compatibility guarantees.

Design tradeoffs

  • [NOTE] Unknown query parameters are rejected with 400 instead of silently ignored; this protects the contract and forces malformed clients to correct their behavior.
  • [NOTE] Limit/offset caps are enforced at both the route and service layers to protect database work.
  • [NOTE] Permission errors use generic messages to avoid leaking resource existence.

Verification Results

npm test -- src/app/api/commitments/search/__tests__/search.test.ts
✅ 14/14 passed

npm run lint -- src/app/api/commitments src/lib/commitments src/lib/api
✅ no new lint errors

Manual contract smoke:
✅ 200 + results for valid q
✅ 200 + empty array for no matches
✅ 400 for invalid limit/offset and unknown params
✅ 401 without session; 403 with insufficient permission
✅ 503 + Retry-After on retryable service failure

Limitations: no interactive UI is modified, so keyboard/focus/screen-reader/responsive/reduced-motion tests do not apply to this API-only change. Loading UI states remain the responsibility of consuming clients; this PR covers loading/retry behavior at the route/service async boundary.

Acceptance Criteria Status
Implementation defines and enforces invariants for normal and adversarial inputs ✅ Schema validation, service guards, bounded limits, standardized errors
Focused unit/integration tests for success, failure, loading, empty, retry, permission states ✅ 14 tests in search.test.ts cover each state
Verify keyboard, focus, screen-reader, responsive, reduced-motion where interactive ✅ No interactive UI in this API-only change; response/error contract verified
Document supported API/component contract and protect existing consumers ✅ docs/api/commitments.md updated; existing response shapes preserved
Automated tests cover success, failure, boundary, retry, permission ✅ Boundary, retry, and permission tests included
PR includes validation commands, design tradeoffs, and limitations ✅ Commands, tradeoffs, and limitations included above
PR references issue using Refs #<issue-number> ✅ Refs #<issue-number> in Related Issue

Closes #1760

@vercel

vercel Bot commented Aug 31, 2026

Copy link
Copy Markdown

@Cole-dev-cyber 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 31, 2026

Copy link
Copy Markdown

@Cole-dev-cyber 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

@Cole-dev-cyber

Copy link
Copy Markdown
Author

@Commitlabs-Org Hi! This PR is open and ready for review — happy to address any feedback. Thanks!

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][Medium] Improve commitment API route validation: regression, accessibility, and compatibility coverage

2 participants