Implements tasks T026-T032 from specs/002-saas-integration/tasks.md (User Story 3, P3 - the final piece of this feature) plus Polish. - New bespoke Redis fixed-window counter (checkRateLimit, src/infrastructure/cache/rate-limiter.ts) rather than @fastify/rate-limit's default onRequest-stage hook -- that hook runs before this feature's preHandler-based auth resolves the integration/user identity the limit needs to key on. A second preHandler (checkIntegrationRateLimit) runs after authenticateProductIntegration on the inbound route, checking the integration-level limit then the per-user limit independently, each throwing the existing RateLimitError (429 RATE_LIMIT_EXCEEDED) on breach. - New integration test (inbound-rate-limit.test.ts) verifies both limits are enforced independently against a real Postgres/Redis: a throttled user doesn't affect others, and the integration cap throttles even when no individual user has hit their own limit. - Docs: contracts/quickstart updated from the placeholder "RATE_LIMITED" code to the actual reused RATE_LIMIT_EXCEEDED code; cleaned up a duplicated paragraph in the admin endpoints section; added a "SaaS Integration" section to README.md documenting the inbound contract, admin routes (and their known auth-stub limitation), and how rate limits are configured. All 32 tasks in tasks.md are now complete -- all three user stories (P1 trust boundary, P2 admin lifecycle, P3 rate limiting) are implemented and covered by integration tests verified against a live database, in addition to unit tests for the crypto/token primitives. Full quality gate (typecheck/lint/format/architecture/ unit tests) passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6.6 KiB
6.6 KiB
Specification Quality Checklist: SaaS Product Integration & Inbound Request Trust
Purpose: Validate specification completeness and quality before proceeding to planning Created: 2026-08-21 Feature: spec.md
Content Quality
- No implementation details (languages, frameworks, APIs)
- Focused on user value and business needs
- Written for non-technical stakeholders
- All mandatory sections completed
Requirement Completeness
- No [NEEDS CLARIFICATION] markers remain
- Requirements are testable and unambiguous
- Success criteria are measurable
- Success criteria are technology-agnostic (no implementation details)
- All acceptance scenarios are defined
- Edge cases are identified
- Scope is clearly bounded
- Dependencies and assumptions identified
Feature Readiness
- All functional requirements have clear acceptance criteria
- User scenarios cover primary flows
- Feature meets measurable outcomes defined in Success Criteria
- No implementation details leak into specification
Notes
- Auth mechanism choice (signed tokens/OAuth2/mTLS) and rotation-window length are deliberately
left to
/speckit-plan, not decided here — see spec.md Assumptions. - Idempotency-key enforcement is explicitly deferred to the future ticketing feature (FR-012 reserves the field only); this is a scope boundary, not a gap.
- Exact rate-limit values and auth-mechanism-per-integration defaults are
REQUIRES BUSINESS CONFIRMATIONper docs/10-implementation-roadmap.md — not invented here. - All items pass; no revision iterations were needed.
Implementation notes (added during /speckit-implement)
- Found and fixed a codebase-wide bug, not specific to this feature:
src/app.tscalledapp.setErrorHandler(...)afterbootstrapRoutes(app)had already registered every domain module's routes. Fastify resolves each encapsulated child context's error handler at the time that context is registered — a handler set on the parent afterwards does not retroactively apply to already-registered children. Every module registered viaapp.register(someRoutes)(which is every module in this codebase, since none usefastify-plugin) was silently falling back to Fastify's default{statusCode, error, message}error shape instead of this app's{success:false, error:{code,message,details}, requestId}envelope, for any error — not just ones from this feature's plugin. Fixed by movingsetErrorHandler/setNotFoundHandlerbeforebootstrapRoutesinsrc/app.ts. Covered by a new regression test intests/unit/app.test.ts(verified it fails without the fix, passes with it). - Found and fixed a second bug in the same handler: the generic (non-
AppError, non-ZodError) fallback branch always returned500, even for framework-level errors that already carry their own client-facingstatusCode(e.g. Fastify's body-parser rejecting malformed JSON is a400, not a server failure). Fixed to preserve the originalstatusCode/codewhen it's in the 4xx range. - Found and fixed a pre-existing DB/Redis wiring gap that only became harmful because of this
feature:
vitest.config.ts's hardcoded testDATABASE_URL(localhost:5432) and default Redis config don't correspond to any servicedocker-compose.test.ymlactually publishes to the host, sotest:integrationcould never reach a real database under this repo's own tooling. This was harmless while every "integration" test was an instantiation-only check (seespecs/001-ci-pipeline/checklists/requirements.md), but this feature's integration test (tests/integration/product-integration-auth.test.ts) makes real Prisma/Redis calls. Rather than leave a newly-introduced test permanently broken for anyone without a coincidentally matching local Postgres, fixedtest:unit's script to scope totests/unitonly (it was running the entiretests/**glob, including integration/E2E, via no path argument) — matchingtest:integration/test:e2e's existing explicit scoping.test:integrationitself still needs a reachable Postgres/Redis (viadocker-compose.test.ymlin CI, or a local equivalent) and was manually verified end-to-end against a temporary Docker Postgres/Redis (see PR description) — it is not run as part ofnpm test. - Manually verified all of spec.md's User Story 1 acceptance scenarios end-to-end against a live server + Postgres + Redis (via temporary Docker containers), beyond what the automated tests cover: valid in-scope acceptance, indistinguishable invalid-credential/unregistered-product rejection, unknown-field rejection, out-of-scope rejection, and replay rejection.
- User Story 2 (admin onboarding/rotation/revocation/audit-trail) is now implemented and
automatically tested (
tests/integration/product-integrations-admin.test.ts, run against a real Postgres/Redis, verified passing). Known limitation carried over from the existing codebase, not introduced by this feature: the admin routes are gated byfastify.authenticate(src/plugins/auth.plugin.ts), which is currently a no-op stub — it never actually verifies a JWT or rejects an unauthenticated caller. These admin endpoints are therefore not really access-controlled yet. Fixing this requires theidentity/authmodule (itself unimplemented) and is out of scope for this feature — flagged here and incontracts/inbound-request-contract.mdso it isn't mistaken for "done." - User Story 3 (rate limiting) is now implemented and automatically tested
(
tests/integration/inbound-rate-limit.test.ts, run against a real Postgres/Redis, verified passing): a bespoke Redis fixed-window counter (checkRateLimit,src/infrastructure/cache/rate-limiter.ts) rather than@fastify/rate-limit's defaultonRequest-stage hook — that hook runs before this feature's preHandler-based auth resolves the integration/user identity the limit needs to key on, so a second preHandler (checkIntegrationRateLimit) runs afterauthenticateProductIntegrationand checks the integration-level limit, then the per-user limit, independently. Verified both are enforced independently (a single user's own throttling doesn't affect others; the integration cap throttles even when no individual user has hit their own limit). - All three user stories (P1, P2, P3) of this feature are now implemented and covered by
integration tests verified against a live Postgres/Redis, in addition to the unit tests for the
crypto/token primitives.
docs/06-database-schema.mditself is intentionally not modified — it's the source spec this implementation follows, not generated output.