PRD: Security Review Remediation — Ledgerline API

Worked example for the Ledgerline invoicing API (Node + TypeScript + Express + PostgreSQL, bun). Findings from a whole-system security review, written in PRD format so they feed design-to-tasks directly. Produced by security-review; in a real repo this file would live at plans/security-review-2026-07-20/prd.md.

1. Executive Summary

• Scope reviewed: The Ledgerline API — a small multi-tenant invoicing service (auth, invoices, customers, email delivery via SendGrid). Whole-system posture sweep across the six threat dimensions; line-level diff issues were deferred to code-review.

• Posture summary: Authentication is sound (bcrypt + short-lived JWTs), but authorization stops at “is logged in” — object ownership is never checked, so any tenant can read any tenant’s data. One live secret is committed to the repo. Findings: 2 🔴 Critical / 3 🟡 Major / 1 ⚪ Minor.

• Asset / boundary map:

  Asset	Boundary at risk
  Customer invoice & contact data	authenticated user ↔ another tenant
  User credentials & sessions	anonymous ↔ authenticated
  SendGrid API key	repo/source control ↔ production email

• Success Criteria:

  1. No authenticated user can read or mutate another tenant’s invoices or customers (verified by cross-tenant tests).
  2. No secret value present anywhere in the working tree; the leaked key is rotated.
  3. Auth endpoints reject brute-force attempts (rate limit verified by test).
  4. Auth failures and authz denials appear in the security event log; no token or credential appears in any log line.

2. Goals and Non-Goals

Goals

1. Close both Critical findings before the next release (P0).
2. Close the three Major findings this cycle (P1).
3. Establish the systemic controls (shared authorization layer, CI dependency audit) so the same gaps do not reopen route by route.

Non-Goals

• We will not conduct a penetration test or run live exploits — findings were confirmed by tracing code paths only.
• We will not re-review dimensions found sound: password hashing, JWT signing/expiry, and TLS termination (handled at the platform load balancer).
• We will not add hardening beyond the modeled threats (e.g. WAF, mTLS between internal services) — recorded under Future Opportunities instead.

Constraints

• Must use the existing Node + TypeScript + Express + bun stack; new dependencies limited to well-maintained middleware (express-rate-limit, helmet).
• Secret storage must use the platform’s environment/secrets mechanism already used for the database URL — no new secrets manager.

Scope Check

One remediation initiative, six findings, two cross-cutting controls. A single PRD is appropriate.

3. Findings as Requirements

Severity maps to priority per the shared severity↔priority scale: 🔴 Critical → P0, 🟡 Major → P1, ⚪ Minor → P2.

Functional Requirements

• FR-1 🔴: The invoice and customer route handlers must scope every object read and mutation to the calling tenant, so an authenticated user cannot access another tenant’s data. (P0)
  • Finding: GET/PUT /api/invoices/:id and GET /api/customers/:id load records by :id with no ownership check (broken object-level authorization / IDOR). Reachable by any logged-in user incrementing ids — confirmed by tracing the handlers; one systemic finding across 3 endpoints, not 3 findings.
• FR-2 🔴: The SendGrid API key must be removed from source control, rotated, and loaded from the environment, so a repo read cannot compromise production email. (P0)
  • Finding: a live SendGrid key is present in config/default.ts (redacted here — the value is not reproduced). Anyone with repo access, including git history, holds a working production credential.
• FR-3 🟡: The login and password-reset endpoints must be rate-limited, so credentials cannot be brute-forced and reset tokens cannot be enumerated. (P1)
  • Finding: POST /auth/login and POST /auth/password-reset accept unlimited attempts; brute force is possible but not instant (bcrypt slows it), so Major rather than Critical.
• FR-4 🟡: The request logger must redact Authorization headers and token fields, so bearer tokens cannot be replayed from logs. (P1)
  • Finding: the morgan-style request logger writes full headers at debug level; staging runs at debug, so valid JWTs sit in the log store.
• FR-5 🟡: The API must log authentication failures and authorization denials as structured security events, so an attack can be detected and reconstructed. (P1)
  • Finding: no security-event logging exists; a credential-stuffing run against FR-3’s endpoints would leave no trace.
• FR-6 ⚪: HTTP responses must carry baseline security headers (CSP, HSTS, X-Content-Type-Options), as defense-in-depth for the browser-facing docs pages. (P2)
  • Finding: no security headers are set; concrete risk is low (the API serves JSON), so Minor.

Non-Functional Requirements

• NFR-1 (Systemic authorization): Ownership checks must live in one shared authorization layer applied to every resource route — not per-handler — so new routes inherit the check instead of forgetting it. The per-handler gap is how FR-1 happened.
• NFR-2 (Dependency hygiene): CI must run the stack’s dependency audit (bun audit) and fail on known-critical CVEs on the attack surface.

4. Remediation Approach

Fix the two Criticals first and structurally: FR-1 is resolved by introducing the shared ownership middleware (NFR-1) and migrating the three affected routes onto it, rather than patching each handler. FR-2 is a rotate-then-remove: the key is rotated at SendGrid before the commit removing it lands, because git history preserves the old value. The Majors are middleware additions (express-rate-limit, a logger redaction list, a structured security-event logger) and land in one hardening phase with FR-6 and NFR-2.

5. Implementation Plan

Phased Rollout

• Phase 1 (Critical — release blockers): shared authorization layer + route migration, secret rotation and externalization. Delivers FR-1, FR-2, NFR-1.
• Phase 2 (Major & hardening): rate limiting, log redaction, security-event logging, security headers, CI dependency audit. Delivers FR-3–FR-6, NFR-2.

Tech Stack Alignment

• Express middleware following the existing src/middleware/ conventions; bun for all scripts. New dependencies limited to express-rate-limit and helmet.

6. Testing Strategy

Testing Levels

• Unit: ownership predicate (tenant match / mismatch), logger redaction of Authorization and token fields.
• Integration: cross-tenant requests against all three FR-1 endpoints return 404; the 6th login attempt within the window returns 429; auth failure and authz denial each emit one structured security event.
• Regression: a route-registration test asserting every /api/* resource route passes through the shared authorization layer (guards NFR-1 against new-route drift).

Quality Gates

• QG-1: bun run lint — no lint warnings or errors
• QG-2: bun run test — all existing and new tests pass
• QG-3: bun run build — build completes successfully
• QG-4: Code review completed

7. Risks and Mitigations

Risk	Likelihood	Impact	Mitigation
Ownership scoping changes query shapes and breaks a legitimate admin flow	Med	Med	Admin routes get an explicit role bypass in the shared layer; integration tests cover both tenant and admin paths (FR-1, NFR-1).
Old SendGrid key still live after the removal commit	Med	High	Rotate first, remove second; verify the old key is dead before closing FR-2.
Rate limit locks out legitimate users behind shared NATs	Low	Med	Key the limiter on IP + account identifier, not IP alone (FR-3).

8. Open Questions

1. Retention for security-event logs? How long must FR-5’s events be kept, and where (existing log store vs. dedicated stream)?
   • Owner: Platform.
   • Impact: Storage target for the security logger; does not block implementing the events themselves.
   • Proposed default: Existing log store, 90 days; revisit if compliance requires more.

9. Out of Scope / Future Opportunities

• WAF / bot protection in front of the API — beyond the modeled threats; revisit if abuse traffic appears.
• mTLS between internal services — single-service deployment today; becomes relevant if the worker splits out.
• Secrets manager migration — platform env vars are adequate at this scale; a dedicated manager is gold-plating for one key.

10. Appendix

• Glossary: IDOR — insecure direct object reference: reaching an object by id without an ownership check; tenant — one Ledgerline customer account and its data.
• Related: tasks.md · decisions.md · findings format