fix(security): initialize the adapter before deriving CSP origins - #3484
Conversation
Derived origins have never worked for hosted production projects. This is the cause, and it is not the one #3474 fixed. `getAllSourceFiles` answers an empty list until the adapter has a content context, and the only thing that establishes one is `ensureSourceSnapshotFresh`, which awaits `ensureInitialized` first. The config load calls it before reading; the derivation path never did. Worse, the adapter's file-list warmup is itself gated on being initialized, so the empty read never filled in afterwards. Not a cold cache that warms a moment later -- one that never warms. That is why the retry introduced by #3474 changed nothing in production: it re-read an adapter that was never going to answer. Preview looked correct only because its adapter had already been initialized by the time derivation asked. The derivation now reads through the same door the config load uses, on both the wrapper and the adapter that owns the file list, since the wrapper may delegate without exposing the hook. `deriveProjectCspOrigins` is exported and tested for the first time. That seam had no coverage at all -- the extractor was tested and the header merge was tested, so both neighbours stayed green while the part that actually reads an adapter was broken in production. The hosted-release case fails without this change and passes with it; I verified that by removing the fix rather than trusting a green run.
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Comment |
`#veryfront/types` re-exports the interfaces that use it, not the type itself. Caught by CI's `lint:test-typecheck`, which is a separate task from `deno task lint` and so was not covered by the gate I ran locally.
This is the actual cause of derived origins never working for hosted production projects. #3474 was not it, and I said it was — the promotion notes on veryfront-server#304 are wrong about that.
The bug.
getAllSourceFilesanswers[]until the adapter has a content context, and the only thing that establishes one isensureSourceSnapshotFresh, which awaitsensureInitializedfirst (adapter.ts:917-919). The config load calls it before reading (adapter-factory.ts:298); the derivation path never did. Compounding it, the adapter's file-list warmup is itself gated onthis.initialized, so the empty read never filled in afterwards. Not a cold cache that warms a moment later — one that never warms.Why #3474 didn't help, and how that pointed here. #3474 stopped an empty read being cached forever, so after it shipped every request re-read the source. Production still derived nothing. That is what proved the emptiness was persistent rather than a cold-start race, and sent me looking for something structural instead. #3474 was still a real bug worth fixing; it just wasn't this one.
Why preview looked fine. Its adapter had already been initialized by the time derivation asked, so the same code read a working adapter. Same release, same source, different answer — which is what the probe project showed.
Hypotheses I falsified along the way, so the next person doesn't re-run them: release files carry no content (they do — verified against the real API through the client's own
listPublishedFilespath), and the file-list warmup is failing (no warmup failures in production, andRefreshed source snapshotshows the adapter reading source).The fix. Derivation reads through the same door the config load uses — on the wrapper and on the adapter that owns the file list, since the wrapper may delegate without exposing the hook.
Why this was never caught.
deriveProjectCspOriginshad no test at all. The extractor was covered and the header merge was covered, so both neighbours stayed green while the seam between them was broken in production. It is exported and tested here: the hosted-release case fails without this change and passes with it, which I verified by removing the fix rather than trusting a green run, plus the no-tenant and read-throws paths.Lint, typecheck, fmt, docs and the full unit suite green by exit code.
Still needs a release and promotion to confirm in production, and the check is one curl:
vf-csp-probe.production.veryfront.comshould gainhttps://images.unsplash.comandhttps://cdn.jsdelivr.netinimg-src. Pairs well with #3482, which makes every derivation outcome say which one it was, so the next failure of this kind is visible in logs instead of needing a live probe.