Skip to content

hardening: Prisma error handling, rate limiting, request tracing, input normalization, and infra improvements#1

Merged
lafronzt merged 13 commits into
mainfrom
feat/standards-api-hardening
May 20, 2026
Merged

hardening: Prisma error handling, rate limiting, request tracing, input normalization, and infra improvements#1
lafronzt merged 13 commits into
mainfrom
feat/standards-api-hardening

Conversation

@lafronzt
Copy link
Copy Markdown
Owner

Summary

This PR applies 10 targeted hardening improvements across the repository, HTTP, validation/service, and infrastructure layers. Changes were developed in 4 parallel worktrees and merged with no conflicts.


Changes by Group

Repository Layer (src/repositories/prisma-standards-repository.ts, src/domain/errors.ts)

Explicit Prisma error handling
Previously, any Prisma constraint violation or missing-record error would bubble up as an unhandled exception and surface as a generic 500. All repository methods now catch Prisma.PrismaClientKnownRequestError and translate:

  • P2002 (unique constraint violation) → ConflictError("A standard with this rule_key and version already exists") → 409
  • P2025 (record not found) → NotFoundError → 404

This means concurrent writes to the same (rule_key, version) pair return a clean 409 instead of leaking a Prisma stack trace.

Stable pagination sort
findLatestByRuleKey and findActiveByRuleKey lacked an explicit orderBy, relying on undefined Postgres row ordering. Both now use orderBy: [{ ruleKey: 'asc' }, { version: 'desc' }], consistent with the list() method and safe under concurrent inserts.


HTTP / Middleware Layer (src/app.ts, src/http/routes.ts, package.json)

Request ID tracing
Fastify's genReqId is now configured to read x-request-id from the incoming request header (for upstream proxy continuity) or generate a randomUUID() if absent. The request.id is included in all error response bodies as error.request_id, so clients can correlate errors with server logs without grepping timestamps.

Rate limiting
@fastify/rate-limit is registered globally at 100 req/min per IP. Write endpoints (POST /api/v1/standards and PUT /api/v1/standards/:ruleKey) have a tighter per-route override of 20 req/min. Rate limit errors return a structured JSON body:

{
  "error": {
    "code": "rate_limit_exceeded",
    "message": "Too many requests",
    "details": { "limit": 20, "after": "1 minute" }
  }
}

Auth failure logging
requireWriteApiKey previously threw silently. It now accepts the full FastifyRequest and logs a structured warning before throwing:

{ ip, path, reason: "missing_key" | "invalid_key" } auth failure

This enables alerting on brute-force attempts without exposing the key value in logs.


Validation / Service Layer (src/validation/standards.ts, src/services/standards-service.ts)

Lowercase normalization for appliesTo fields
compactAppliesTo in the service now lowercases values for languages, frameworks, runtimes, teams, repos, and environments before storage. file_patterns is intentionally excluded since globs are case-sensitive. This prevents the database accumulating "TypeScript" and "typescript" as distinct entries, and ensures applicability matching works regardless of caller casing.

Reject empty appliesTo arrays
The appliesToSchema now has a .refine() that rejects any field present with a zero-length array. appliesTo: { languages: [] } previously silently matched no rules — now it fails validation with:

"appliesTo fields must be non-empty arrays if provided; omit the field instead of passing an empty array"

Omitting a field entirely (i.e., not sending the key) continues to mean "match all."


Infrastructure (Dockerfile, .dockerignore, src/server.ts)

Pinned Node base image
All three Dockerfile stages (deps, build, runtime) were updated from the floating node:22-slim to the pinned node:22.3-alpine. This eliminates surprise Node minor-version upgrades in CI and reduces the image footprint (alpine vs. slim).

.dockerignore
A .dockerignore was added to exclude node_modules, dist, .git, *.md, k8s/, test/, docker-compose.yml, .env*, and .claude/ from the build context. Without it, the full repo (including test files, markdown, and local k8s configs) was copied into every build stage unnecessarily.

Graceful Prisma disconnect (src/server.ts)
Confirmed already present in the shutdown handler — no change required.


Test Coverage

  • Added concurrent creation test: fires two simultaneous POST requests for the same (rule_key, version) using Promise.allSettled. Asserts exactly one 201 and one 409, exercising the new P2002 error path end-to-end through the in-memory repository.

Files Changed

File Change
src/repositories/prisma-standards-repository.ts Prisma error handling + stable sort on all findMany calls
src/domain/errors.ts Added ConflictError and NotFoundError if not present
src/app.ts genReqId, rate-limit plugin, request_id in error responses
src/http/routes.ts Rate-limit config on write routes, full-request auth hook, warn logging
src/services/standards-service.ts Lowercase normalization for appliesTo metadata fields
src/validation/standards.ts .refine() rejecting empty array fields in appliesTo
test/standards.test.ts Concurrent version creation test
Dockerfile node:22-slimnode:22.3-alpine across all 3 stages
.dockerignore New file, excludes dev artifacts from build context
package.json / package-lock.json Added @fastify/rate-limit

Test Plan

  • npx tsc --noEmit passes (confirmed clean before merge)
  • npm test — all existing tests pass; concurrent creation test passes
  • POST to a duplicate (rule_key, version) returns 409 with error.code: "conflict"
  • POST/PUT beyond 20 req/min returns 429 with error.code: "rate_limit_exceeded"
  • Error responses include error.request_id matching the x-request-id response header
  • Auth failure with wrong key logs a structured warn line (check server output)
  • appliesTo: { languages: [] } in a create body returns 400
  • appliesTo: { languages: ["TypeScript"] } stored as "typescript" in DB
  • Docker image builds cleanly: docker build .

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings May 20, 2026 00:49
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Standards API across the repository, HTTP/middleware, validation/service, and container build layers—adding Prisma error translation, request tracing, rate limiting, input normalization, and smaller Docker build context.

Changes:

  • Translate key Prisma errors into domain errors (409 conflict / 404 not found) and stabilize ordering for rule-key lookups.
  • Add request ID generation, structured rate-limit responses, and improved auth-failure logging for write endpoints.
  • Normalize and validate appliesTo inputs (lowercasing selected fields; rejecting empty arrays), plus Docker build/pinning improvements.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/repositories/prisma-standards-repository.ts Adds Prisma error translation and explicit ordering to reduce undefined behavior.
src/domain/errors.ts Introduces/ensures domain error types for conflict and not-found cases.
src/app.ts Registers global rate limiting, configures request IDs, and adds request_id to most error responses.
src/http/routes.ts Applies per-route rate limits to write endpoints and logs structured auth failures.
src/services/standards-service.ts Normalizes appliesTo values by lowercasing selected metadata arrays.
src/validation/standards.ts Rejects empty appliesTo arrays when provided to avoid “matches nothing” surprises.
test/standards.test.ts Adds a concurrency test intended to validate duplicate-create behavior.
Dockerfile Pins Node image tag and switches to Alpine across stages.
.dockerignore Excludes dev/test artifacts to reduce build context and image bloat.
package.json Adds @fastify/rate-limit.
package-lock.json Locks @fastify/rate-limit dependency resolution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/app.ts
Comment thread src/app.ts Outdated
Comment thread src/app.ts Outdated
Comment thread src/repositories/prisma-standards-repository.ts Outdated
Comment thread src/repositories/prisma-standards-repository.ts Outdated
Comment thread src/repositories/prisma-standards-repository.ts Outdated
Comment thread src/repositories/prisma-standards-repository.ts Outdated
Comment thread test/standards.test.ts
- Remove dead P2025 catch from read methods (findMany/findFirst/findUnique
  return null/[] on miss, never throw P2025); keep P2025 only on update()
  and updateReplacingActive() where it can actually fire
- Collapse P2002+P2025 to single-condition checks in create/createReplacingActive
- Fix genReqId to normalize x-request-id header safely (handle string[] and
  reject values over 255 chars) instead of unsafe `as string` cast
- Add request_id to rate-limit (429) error responses so clients can correlate
  them the same way as other errors
- Add request_id to setNotFoundHandler (404) response
- Fix MemoryStandardsRepository.create() to enforce (ruleKey, version) uniqueness
  atomically, making the concurrent-creation test deterministic

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.

Comment thread src/repositories/memory-standards-repository.ts
Comment thread src/app.ts
Comment thread src/validation/standards.ts
Comment thread src/services/standards-service.ts
Comment thread src/app.ts
- Move duplicate check before mutation in MemoryStandardsRepository.createReplacingActive()
  so a ConflictError never leaves actives deprecated with no replacement created
- Add onSend hook to echo request.id back as x-request-id response header, fulfilling
  the documented contract that clients can correlate error.request_id with the header
- Accept rateLimitMax override in buildApp() to allow tests to trigger 429 without
  exhausting the real 100/min or 20/min limits
- Add test: applies_to with empty array field returns 400 validation_error
- Add test: appliesTo metadata normalized to lowercase, file_patterns casing preserved
- Add test: 429 response has structured error.code, details.limit, request_id, and
  x-request-id header

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Comment thread src/validation/standards.ts Outdated
Comment thread src/repositories/prisma-standards-repository.ts
- Fix error message in appliesToSchema refine to use the API field name
  "applies_to" instead of the internal camelCase "appliesTo"
- Fix @fastify/rate-limit v10 compatibility: errorResponseBuilder return
  value is thrown directly by the plugin, so the previous plain-object
  return fell through to the catch-all 500 handler. Now returns a proper
  Error with statusCode=429 attached, and setErrorHandler handles the 429
  case explicitly, preserving the structured rate-limit error shape.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Comment thread src/repositories/prisma-standards-repository.ts Outdated
Comment thread src/repositories/prisma-standards-repository.ts Outdated
findLatestByRuleKey and findActiveByRuleKey already filter by ruleKey in
the where clause, so including ruleKey in orderBy has no effect and
misleadingly implies multi-key result ordering. Simplify both to just
orderBy: { version: 'desc' }.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lafronzt lafronzt merged commit 2f917d7 into main May 20, 2026
1 check failed
@lafronzt lafronzt deleted the feat/standards-api-hardening branch May 24, 2026 18:33
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.

2 participants