What the bug sweep fixed, and what it deliberately left alone

A sweep of the codebase for defects, starting from a known OpenAPI mismatch and widening from there. Four findings were verified against source rather than taken on report. Three were fixed. One was investigated and deliberately not fixed, which is the entry most worth reading.

Fixed — the anonymous quote endpoint returned the company's cost basis. app/api/quote/route.ts ended with result: quoteRow, handing back the whole inserted row: wholesale_cost, target_margin, customer_name, user_id. Those are the exact four fields the RLS policy in supabase/migrations/quotes_table.sql exists to keep the anon role away from, and the reason the insert deliberately carries no .select(). The database was locked down correctly and the HTTP response gave it all back: curl -d '{"finishedWidth":48,"finishedDrop":72}' /api/quote printed the margin to anyone who asked.

The fix is a hand-built response object rather than a blacklist of fields to strip, because the two fail differently over time. A row shape follows the table, and a table grows columns for reasons that have nothing to do with what a public endpoint should say; the next migration that adds a cost field re-opens a blacklist silently. A response shape follows the caller and gains a field only when somebody writes one here on purpose. The cut list stays in the response — D-002 records why that line is the one that matters: cut sizes are shop-floor facts, prices are commercial ones.

Fixed — the login rate limiter could be bypassed with a space. The limiter in backend/src/routes/auth.routes.ts keyed on the account being attacked rather than the client IP, which is right. But it lowercased the email without trimming it, while loginSchema normalises with .trim().toLowerCase(). So "a@b.com", " a@b.com", "a@b.com " and "\ta@b.com" all authenticated the same account and all got their own ten-attempt counter, with no upper bound on the number of whitespace variants available. The control read as an account lockout and behaved like an open door.

Two changes, either of which fixes it alone: the key generator now trims as well as lowercases, and validateBody was moved ahead of the limiter so the limiter sees the schema's normalised output rather than raw client input. The pair is deliberate. A future reader who moves the middleware back for an unrelated reason has not silently re-opened the hole.

The cost of the new ordering is that a body failing schema validation never reaches the login counter. That is the right trade: a malformed request never tests a password, so counting it would measure nothing, and volumetric abuse is already bounded by the 300-per-minute global ceiling in app.ts.

Fixed — prospect PII in production logs. app/api/contact/route.ts logged company name and work email to stdout in every environment, contradicting the redaction policy stated in lib/email/client.ts. Production now logs the email domain only. A domain is not a personal identifier, and it keeps what the line was for — telling you which submissions made it through when someone reports a lost enquiry. Most of the recovery value, none of the individual.

Not fixed, on purpose — `forceRefresh` does not bypass `inFlightLogin`. In app/api/cpq/calculate/route.ts, a caller passing forceRefresh still joins an in-flight login rather than starting its own, which looks like a bug and was reported as one. It is not. A token can only exist once a login has completed, and ??= means logins never overlap, so an in-flight login is by construction newer than whatever token the caller is trying to refresh past. The single scenario where the current behaviour is wrong is a secret rotation landing inside the login window, which costs one 503 and self-corrects on the next request.

The proposed fix costs far more. Ten concurrent requests each forcing their own login is ten login attempts, which is the rate limit exactly. The failure mode stops being one dropped request and becomes a service credential locked out for fifteen minutes. The reasoning is now a comment above the ??= so the next reader does not helpfully repair it.

Logged for later. Three findings were real but out of scope for a sweep, and are recorded here rather than half-fixed.

public/openapi.json describes four paths that do not exist (/cpq/calculate-quote, /cpq/quotes, /fsm/jobs, /fsm/jobs/{job_id}) against servers that do not answer, while the backend serves nine real routes and the frontend calls a different host entirely. Patching the JSON by hand would produce a document that is accurate today and wrong again at the next schema change. Task 4 generates it from the Zod schemas and adds a drift check, which is the only version of this fix that stays fixed.

npm run lint is next lint with no ESLint configuration anywhere in the repository, so it prompts interactively and cannot run unattended; CI has no lint step at all. Introducing ESLint mid-sweep would bury three real security findings under several hundred stylistic ones.

The frontend has no test runner — no test script in the root package.json. That is why the quote-endpoint fix is structural rather than test-covered: with nowhere to put a regression test, the guarantee has to live in the shape of the code. The backend fix, which does have a suite, got a test instead.

How this was verified. Each fix was proven to fail before it was accepted. The rate-limit regression test was run against reverted code and confirmed to fail there (not ok - counts a whitespace-padded address against the same bucket) before being kept, and it is deliberately ordered to run after the test that exhausts the plain address's bucket — written the other way round it would pass against the broken code too. The narrowed quote response was checked against its only consumer, components/CPQSandbox.tsx, which reads exactly one field from it (data?.persisted).