Implement B01-B06 with regression coverage and retained validation evidence. Preserve the production ingress preflight; F04 runtime verification remains open.
29 KiB
Findings and implementation packages
Baseline and evidence labels: README. Reproductions never require production data. P1 fixes are launch prerequisites under their stated exposure; P2s remain tracked. Estimates are relative engineering scope, not promised delivery dates.
F01 — P1: stale saves silently overwrite newer CV/job/profile state
Observed: two clients read CV revision 1. Saving “Newer edit” returned 200; saving the stale copy also returned 200 and replaced it. Two simultaneous PUTs both returned 200. CvVariantController.cs:34,124 does not accept an expected revision; CvVariantService.cs:108 increments its own freshly read counter. JobTrackerContext.cs:498 has a non-unique version index, not a concurrency token. CareerProfileService.cs:51 and job PUT have the same missing client precondition. Identity's ConcurrencyStamp is not a CV conflict protocol. History may preserve settings, but it does not protect the user from overwriting newer work or guarantee unique revision labels.
Root cause/scope: shared persistence contract; module-wide first, then other editable aggregates. The editor's promise queue only serializes its own requests.
Reproduction/failing test: use stale CV save in the harness; require the second stale save to return 409/412. Add two independent EF contexts and a real relational database test; cover job status, Career Profile, restore-versus-autosave, and delete-versus-save. Assert that the newer state and a unique history revision remain.
Fix: add expected revision to request/response contracts and perform compare-and-update atomically; use a provider-compatible integer concurrency token. Add uniqueness after reconciling existing duplicated history versions. Return current revision and a safe conflict code. Keep edits in the editor; offer compare/reload/save-as-copy. An unconditional retry is wrong. Make profile child writes, snapshot and legacy projection one transaction or give the projection a repairable state.
Passing/staging/detection: replay five tabs against SQLite and MariaDB, including delayed responses and restore. One writer wins; others retain their drafts. Count conflicts and version-integrity failures without logging CV text. Expand schema first, ship compatible readers/writers, then require preconditions; test an older open frontend. Medium scope; do not refactor all controllers at once.
F02 — P1: billing has no complete account lifecycle or concurrent reconciliation
Source: BillingController.cs:61,147 creates new checkout sessions without a durable account idempotency key (StripeBillingGateway.cs:14). Webhooks verify signatures and re-fetch the subscription, which protects ordinary sequential out-of-order delivery. They then update user subscription fields and roles separately. There is no event inbox, per-customer serialization, periodic reconciliation or rule reconciling multiple subscriptions for the same user. AccountDeletionService.cs:110,231 deletes the user and billing identifiers without cancelling an active Stripe subscription. Enabling deletion while billing is active can remove product access while charging continues.
Scope: billing module plus deletion orchestration. Not a claim that duplicate webhook delivery itself charges five times; this handler does not issue charges.
Reproduction/failing tests: fake gateway returns two checkout sessions for simultaneous calls. Model two subscriptions for one customer, one cancelled and one active, and delay one handler after its provider read. Require one correct account entitlement result. Request deletion with an active subscription and assert cancellation/verified resolution happens before identity deletion. Crash between user and role writes.
Fix: durable checkout attempt/key per intended purchase; store received event identity and processing outcome; serialize reconciliation per customer and derive entitlements from the authoritative relevant subscriptions. Re-read on processing, commit local state and entitlements together, retry safely, reconcile periodically. Add a billing-cancellation deletion stage; on unavailable Stripe retain pending state and explain it. Store only the minimal retained billing audit data approved by policy.
Passing/staging/detection: Stripe sandbox sends the same event five times, reverse order, conflicting concurrent events, deletion and checkout double-clicks. No duplicate purchase, no stale privilege restoration, no orphan active subscription. Alert on unresolved events, local/provider divergence and deletion stuck at cancellation. Preserve event records during rollback. Medium scope; actual Stripe behavior Needs runtime verification.
F03 — P1: PDF generation can exhaust the backend host
Source: PlaywrightCvPdfExporter.cs:37 starts a Chromium process per request, holds output bytes in memory, scans export directories before each run and keeps each generated file. It limits each process to 60 seconds but has no shared semaphore/queue. CvVariantController.cs:206 has no PDF rate policy. Public PDF is limited by IP+slug only. JobTrackerApi/Dockerfile runs the API as its default root user and exporter flags include --no-sandbox and local-file access. Backend Compose has no corresponding CPU/memory/PID cap.
Scope: shared exporter and backend container. A malicious registered user need not buy AI credits to issue PDF requests. No overload or renderer exploit was executed.
Reproduction/failing test: replace browser launch with a controlled slow fake; invoke 20 exports through distinct entry points and assert active launches never exceed a configured small bound. Verify excess admission returns 429/503 promptly, cancellation frees the permit and partial artifacts are removed. A host test is not allowed to launch 1,000 real browsers.
Fix: bounded shared PDF admission, per-owner burst and sustained limits, explicit queue limits/deadlines and maximum rendered input. Run the backend non-root with constrained resources; verify whether Chromium sandbox can be enabled in that deployment. Isolate the renderer if it cannot be meaningfully constrained in the API container. Remove unnecessary local-file access after renderer regression tests. A durable PDF queue is justified only if exports must survive disconnect/restart.
Passing/staging/detection: small synthetic load verifies peak processes/RSS, retained files and timeouts, including many public slugs. Alert on queue delay, admission rejection and renderer memory. Roll out low capacity first. Small admission change, medium container validation.
F04 — P1 conditional: direct nginx ingress can bypass trusted-header assumptions
Source: docker-compose.yml:140 publishes 3000:80 on all host interfaces. nginx.conf.template:64 forwards caller-supplied X-Forwarded-For/Proto. The backend trusts the nginx subnet and processes one forwarded hop (ForwardedProxyConfiguration.cs:12). The comment assumes an upstream proxy sanitizes headers. A direct connection with the canonical Host can skip that proxy. Host allowlisting does not authenticate the sender. Exploitability depends on the actual host firewall/proxy placement, which was not inspected.
Reproduction/failing test: in isolated Compose, request host port 3000 with canonical Host and forged forwarding headers; assert they cannot choose the API client address or bypass login throttling. Test the normal proxy route at the same time.
Fix: make ingress reachable only by the expected proxy, or discard forwarding headers on direct ingress and accept them only from a verified trusted hop. Do not blindly bind to loopback if the real proxy is on another machine. Document one topology and source of client identity. Keep backend/AI/DB ports private.
Passing/staging/detection: packet/firewall and application-IP evidence from both ingress routes, including IPv6; alert on bypass-port reachability. Small configuration scope; coordinate with the operator because an incorrect ingress change can make the site unavailable. Needs runtime verification.
F05 — P1: CSV quoting does not neutralize spreadsheet formulas
Observed: a job title =1+1 was exported as an unescaped formula cell. ExportController.cs:78 only handles commas, quotes and line breaks. Imported job/company text can carry the same payload. Exploitation requires opening the export in a spreadsheet; no spreadsheet or command was executed here.
Reproduction/failing test: create harmless formula-like fields and export; assert that spreadsheet-safe export cannot evaluate leading =, +, -, @, tabs/CR and leading whitespace variants. Include quoted delimiters and Unicode. Preserve ordinary negative numeric salary cells as numbers through an explicit typed policy.
Fix/scope: shared CSV field writer for user-controlled text, with a documented spreadsheet-safe representation; preserve raw values in JSON for portability. Do not use quotation marks as the entire mitigation.
Passing/staging/detection: parser-level CSV tests plus manual Excel/LibreOffice open using harmless arithmetic fixtures. Test downstream import expectations. Small change; log export failures/counts, never field values. No bulk data migration.
F06 — P2: account rate limits actually partition by IP
Observed: two successful account exports by A caused B's first export from the same address to return 429. Program.cs:602 runs RateLimiter before Authentication at 642; email-send/account-data policies inspect an empty User and fall back to the IP. This also means one account can spread these operations across addresses. Authentication-sensitive IP policies are intentionally IP-based and should remain so.
Move authentication before owner-partitioned limiting, preserve independent anonymous/IP controls, and add a real middleware test with two accounts/one address and one account/two addresses. Return Retry-After when available. Retain a host-wide limit. A per-policy helper unit test alone does not prove middleware order. Small scope; counts of rejects by policy are sufficient detection.
F07 — P2: email delivery is not a reliable, durable outcome
Source: FollowUpReminderHostedService.cs:99 sends inside a loop but persists all sent timestamps at the end. A later send/commit failure can resend earlier messages on the next pass. SmtpEmailSender.cs:32 returns successfully when sending is disabled; the worker can mark a suppressed reminder as sent. Auth verification/reset sends are direct attempts without a durable retry record. A failed verification remains recoverable through resend, but delivery is not guaranteed. Synchronous SMTP in Task.Run does not become cancellable once sending begins.
Reuse the durable attempt approach already used by employer email. Persist intent with an owner/job/date dedup key, record suppressed/accepted/failed/unknown separately, and do not blindly retry uncertain SMTP delivery. Retry unsent transactional verification mail within token validity. Test failure on message two, failure at commit, disabled sender and process termination. Stage with a local mailbox sink. Alert on backlog age and permanent failures. Module scope; do not build a second email client or claim exactly-once SMTP delivery.
F08 — P2: logout in one tab leaves private editor state visible in another
Observed: after logging out and removing the shared auth marker in one tab, another tab's CV editor remained mounted. auth.ts emits same-window events; App.tsx:263 listens to those, not the storage event. Theme/language already synchronize using storage listeners. Backend session revocation still denies subsequent authenticated requests: this is stale private UI, not proof of an authorization bypass.
Subscribe once to cross-tab auth changes; cancel/ignore outstanding responses, clear owner caches, and route to a reauthentication state. Preserve pending edits in a safe owner-scoped recovery flow where possible. Test logout, account switch, password/role revocation, late old-account responses and browser Back. Medium UI scope. Do not store bearer credentials in localStorage to solve synchronization.
F09 — P2: interrupted edits have incomplete recovery
Observed: CV autosave displayed failure while offline, but after accepting the browser's unload warning and refreshing, the unsaved draft was gone. That warning is useful and should remain. The editor has no durable draft recovery. Session expiry clears authentication globally and can unmount long forms. Career Profile and notes need equivalent coverage; this run did not exercise every editor.
Prioritize recoverable long-form drafts, with explicit account ownership, schema version, expiry, clear-on-account-delete, and a user-visible restore choice. Decide whether device-local sensitive content is acceptable; a server draft only helps after network access resumes. Add conflict handling F01 before wider autosave. Test quota-denied browser storage, offline/reconnect, expired session, restore-version and failed saves. Medium scope; do not promise zero loss on device failure.
F10 — P1: request limits do not bound total storage, CPU or database growth
Source/Observed: AttachmentsController.cs:269 sums only attachment rows before writing, so simultaneous uploads race quota checks and generated PDFs, CV artifacts, package copies/history are outside that accounting. There is no total job/history quota; duplicate POSTs created two distinct jobs (201/201). Account export writes a ZIP but loads large table collections and JSON byte buffers; jobs CSV/JSON export loads all jobs. Board and several analytics paths materialize whole owner datasets. Monthly AI limits are estimates, not a global CPU limit.
Failing tests: concurrent near-quota uploads must reserve bytes atomically; package copies/PDF retention must be counted; repeated POST with one idempotency key returns one resource, and a different payload with the same key returns conflict. Test very large synthetic history with capped memory. Verify negative/extreme pagination and malformed large content without allocating unbounded bodies.
Fix: owner-level byte reservations plus reconciliation across owned file categories, hard request/document bounds, bounded PDF admission F03, and intent idempotency for creation/import actions. Keep legitimate repeat applications possible. Stream or batch exports, or queue large exports with bounded working memory. Add history retention only after a product decision about restore promises.
Staging/detection: synthetic 1k/10k/100k records with query plans/RSS/response bytes; disk/row growth and rejected admissions tracked. Small quotas first, larger export work only after measurement. Module-wide; old data remains readable on downgrade.
F11 — P1 conditional: import DNS validation is not bound to the actual connection
Source: JobImportService.cs:175 resolves and rejects non-public addresses, then a named HttpClient connects separately. No ConnectCallback or connected-peer validation was found. Redirects are rejected, which correctly blocks a redirect to localhost, but a changed DNS answer between validation and connection is a remaining SSRF boundary. IMAP has a similar preconnection guard to verify. No attack against live DNS was attempted.
Failing test: deterministic resolver returns a public address at validation and private/metadata address at connection; assert no private connect. Include IPv6/mapped IPv4/mixed answers, timeout, TLS name validation and legitimate DNS failover.
Fix/staging/detection: connect to an approved resolved address while retaining TLS/SNI host validation, or use a restrictive outbound proxy/firewall. Test against a local controlled resolver and sink. Alert on denied private targets using categories, not full private URLs. Medium network-boundary scope. Do not disable URL import globally if bounded egress solves it.
F12 — P1: deploy recovery-set consistency and rollback compatibility are not proven
Source: daily SQLite backup uses VACUUM INTO (DatabaseBackupRunner.cs:68), a useful consistent DB snapshot. In contrast, SQLite pre-deploy backup tars the live data volume (deploy.sh:314) and validation checks archive shape/DB presence, not a restored database. DB/WAL/files may change during tar. MariaDB SQL backup and file-volume archives also need a coordinated recovery point. Automatic previous-image restoration at deploy.sh:638 does not reverse migrations. No actual corrupt backup is claimed.
Failing test: continuous local writes while taking a recovery set, restore it into an isolated environment, verify DB integrity, FK ownership, file hashes, encrypted provider-token decryptability and deletion-tombstone replay. Assert old binaries can read the new schema before allowing automatic image rollback. Kill the backup and ensure a partial artifact cannot become “latest verified.”
Fix: reuse the consistent DB snapshot path, coordinate files/manifest with that checkpoint or a brief write pause, checksum all artifacts, and mark verified only after restore. Preserve prior verified sets and encryption/key recovery separately. Stop automatic image rollback for incompatible schema changes; favor forward repair after a successful destructive migration.
Staging/detection: restore latest and previous sets within agreed RPO/RTO, rehearse a bad migration, disk-full backup and missing off-site copy. Alert on last successful restored/off-site backup age, not file existence. Medium operational scope; production commands require operator approval.
F13 — P1 launch gate: privileged support, security auditing and retention need deliberate controls
Source: AdminAuditController.cs:42 bypasses owner filters and returns cross-user job event notes/old/new values, company/title and owner identifiers. This is a user activity timeline, not a tamper-resistant security ledger. UsersController can create users/roles, change roles and trigger resets; no dedicated security-event store was found for these mutations. There is no implemented restricted support/impersonation role. Infrastructure root can read all stored personal content and keys. Deletion is default-disabled pending policy approval.
Failing tests: normal users cannot call every privileged verb (extend current posture tests); admin actions produce safe durable security events; read-only diagnostics cannot return document/note contents. Verify deletion/export events and privilege revocation. A metadata audit must not accept arbitrary private notes as an event description.
Fix: retain current user/admin roles; do not introduce speculative role hierarchies. Remove unnecessary content from global diagnostics, scope each privilege, require recent authentication for high-impact actions, and record actor/action/target/outcome/time/reference in append-oriented storage with restricted writes/off-host retention. Decide support needs before adding impersonation. Approve a retention/deletion/backup policy and implement it; claims of legal compliance require separate review.
Staging/detection: attempt all admin actions as user/admin, inspect event integrity and alert on privilege changes or unusual bulk access. Audit operator access separately. Medium scope; retention policy and custody Need runtime/owner verification.
F14 — P2, launch quality gate: builder accessibility is outside the green axe suite
Observed: the isolated builder probe reported serious color-contrast and scrollable-region-focusable violations. No outer viewport overflow was observed at the tested 375/1440 widths. Existing e2e/accessibility.spec.ts checks dashboard/jobs/kanban/career/settings/admin-system, but not /career/builder/:id. See retained JSON for exact nodes and screenshots for both locales/themes.
Fix contrast through shared theme tokens, make the editor scrolling surface keyboard reachable, and distinguish inaccessible template previews from decorative thumbnails. Add a builder case with real synthetic content and verify focused controls, inactive/locked states and both themes. Manual screen-reader, text-spacing, forced-colors, 200% and 400% zoom remain required; axe alone cannot certify them. Small/medium component scope.
F15 — P2: private caching and browser security headers are incomplete
Observed: /auth/me, private CV JSON and job CSV export returned no Cache-Control. This does not prove that a browser exposed data to another user; it means explicit private-response policy is absent. nginx sets nosniff/frame/referrer/permissions headers, but no CSP or explicit static index/hashed-asset cache policy. Public CV HTML sets noindex; the PDF route does not set the same directive. The authenticated preview uses allow-same-origin allow-scripts and generates repeated browser sandbox warnings; encoding remains the primary defense.
Apply private, no-store to sensitive API/file/export responses and noindex to both public token-link representations. Give index revalidation and hashed static assets immutable caching. Deploy a tested CSP (report-only first; minimize report URLs), then constrain preview scripting to required functionality. Test stored HTML/URL payloads and missing-script/fonts behavior. Do not call current encoding a proven XSS exploit. Small cross-cutting policy change plus renderer compatibility verification.
F16 — P2: operational logs can contain private content and unsanitized URLs
Source: SmtpEmailSender.cs:34 logs recipient and subject when mail is suppressed; reminders embed role/company in that subject. Auth errors log email addresses. ClientErrorsController.cs:32 truncates but preserves reported URL, message and stack prefixes. Thus tokens/query content can survive if a client submits them. The request logger excludes query strings and has trace IDs, which should be preserved.
Allowlist safe error fields, remove query/fragment and free-text/stack contents before logging, hash/pseudonymize only where useful, and rate-limit error ingestion. Connect the existing traceId to visible UI failures. Distinguish operational logs, security records and job timeline retention. Test synthetic secret/PII canaries and verify their values never reach a logger sink. Small shared-boundary scope.
F17 — P2: build/test success hides a strict test-type error and documentation drift
Observed: backend 753, frontend 279 and normal browser 13 tests pass; production static build passes. Standalone tsc --noEmit fails at src/interview-prep.test.tsx:120 because mock.calls[0][1] is unknown. Next's build result does not replace this test-source gate. Existing browser warning reports stale Browserslist data; dependency freshness was not checked online.
Type the test payload at its declaration/assertion boundary and run standalone type checking explicitly in CI. Refresh only the authoritative architecture/setup document, linking dated reports instead of copying them. Preserve old renderers/migrations until call-site and stored-ID compatibility is proven. This is a small correction, not justification for replacing Next or TypeScript.
F18 — P2: supply-chain and production-environment proof remain incomplete
Source: locks and the tracked-secret scanner are present and scanner tests passed. CI actions and base images use mutable tags; pip bootstrap is fetched at runtime; the API Docker build does not copy root global.json/build props and does not explicitly restore with locked mode. The .NET vulnerability-list command is reporting, not an explicit parsed severity gate. No retained container scan, current CVE/license review, branch-protection inspection, host patch/firewall audit or signing evidence was collected.
Pin release dependencies by verified digest/SHA, run locked restores in the actual image build, retain SBOM+scan artifacts and define expiring advisory exceptions. Check supported runtime versions through current official sources when external access is authorized. Protect deploy credentials from untrusted PR execution, especially on a persistent self-hosted runner. Avoid claiming the no-value tracked scan proves Git history or runtime environments contain no secrets.
F19 — P2: application startup silently falls back to SQLite
Source: Program.cs:95–115 replaces a configured provider with SQLite when the connection string is missing and treats unknown providers as SQLite. Deployment validation is stronger, but starting the production API by another path can create an apparently empty database. Fix provider/connection validation in application startup, preserving deliberate development SQLite defaults. Test production missing/invalid provider/connection plus explicit SQLite and MariaDB choices. Small scope; no migration required.
F20 — P1 when enabled: an unhandled AI-worker dependency failure can stop the API host
Source: AiOperationQueue.cs:254 runs worker loops without a catch around RunOnceAsync. Claiming a lease, loading the owner, reading roles and persisting the result can throw database errors outside the classified provider-failure paths. No overriding background-service exception policy was found. Under the framework's default BackgroundService exception handling, an unhandled worker failure stops the host. This was not fault-injected into a running host during the audit.
Failing test: enable the worker against an isolated DB, make lease-claim or result persistence fail once, and assert that core API liveness remains available, the operation is recoverable and processing resumes after a bounded delay. Include shutdown cancellation and an unexpected handler exception.
Fix/scope: catch and classify failures at the worker-loop boundary; use bounded backoff with jitter, safe logging and a degraded-worker signal. Preserve the persisted lease/result protocol. Do not simply ignore BackgroundService exceptions globally, which could leave a permanently dead worker behind a green health endpoint. Module-wide, small/medium scope.
Passing/staging/detection: 30-second and longer controlled DB outages, followed by recovery, must not lose accepted work or trigger an endless restart storm. Alert on failed worker passes, stalled heartbeats and expired leases. Keep the worker kill switch available while proving this behavior. Needs runtime verification.
F21 — P1: calendar export changes scheduled instants around DST
Observed: a follow-up submitted as 2026-03-29T09:00:00+02:00 exported as DTSTART:20260329T090000Z, rather than 07:00Z. Both occurrences of the repeated autumn time 2026-10-25T02:30 (+02:00 and +01:00) exported as the same 02:30Z. All four offset-aware calendar probes failed the expected-instant check while returning HTTP 200.
Root cause/scope: job fields use DateTime without a durable timezone/offset contract; SQLite materialization loses Kind, and ApplicationCalendarController.cs:129 treats unspecified values as UTC. The application also uses server-local time for reminder scheduling. This affects externally scheduled follow-ups and needs a date/time boundary fix, not a display-format patch.
Failing tests: replay the four DST calendar instant cases in the harness on SQLite and MariaDB with server timezone UTC and Europe/Oslo. Preserve the same instant through POST, DB round-trip, JSON and ICS. Test date-only deadlines separately and reject or explicitly resolve nonexistent/ambiguous local times.
Fix: specify UTC instants/DateTimeOffset at timed-event boundaries, preserve date-only values as date-only, and keep a named timezone for future wall-clock recurrence if that feature is supported. Plan migration of existing values; their intended timezone cannot be safely inferred globally. Do not shift historical data blindly.
Passing/staging/detection: imported calendar events display 09:00 locally before/after DST and distinguish the autumn repeated hour. Rehearse existing-data migration with synthetic representative dates. Detect serialization/round-trip mismatches in tests rather than collecting users' event contents. Medium scope with migration risk.
Implementation order and rollback
| Batch | Dependencies | Acceptance and rollback |
|---|---|---|
| A: ingress/DNS, CSV, shared PDF admission and account rate identity | Confirm actual proxy topology; validate provider config in app startup | Boundary tests; previous config/image retained; no schema change |
| B: revision contracts, duplicate intents and timed-event conversion | Additive schema first; determine legacy date meaning | Conflict/history/offset round-trip tests on both providers; compatible old-client period; preserve drafts and historical dates |
| C: billing and delivery lifecycle | Durable state + sandbox | Event replay/deletion tests; disable new purchase/send admission if unhealthy; retain queued records |
| D: quota/storage/export accounting | A/B semantics established | Byte reconciliation and bounded export stress; preserve existing data on quota changes |
| E: recovery, worker failure containment, security audit and retention | Policy/custody owner; DB-outage fixture | Restore/worker/operator drills; keep independent verified backups/tombstones; no casual DB downgrade |
| F: editor/session/a11y and source hygiene | B conflict UX | EN/NB, keyboard, themes, failure states, strict types; staged route-by-route rollout |
Every P1 closure must attach the failing-before/passing-after evidence, changed scope, provider coverage, staging outcome and recurrence signal. A review saying “looks fixed” is insufficient. No wholesale redesign, broker, Redis, public PWA, generic permission platform or custom encryption scheme is required to begin these batches.