feat(security): say why a CSP derivation produced nothing - #3482
Conversation
v0.1.1216 shipped the negative-caching fix and derived origins still do not appear in production. The probe project serves the bare floor while its preview serves both derived origins from the same release and the same source. That fix was real -- an empty read was being cached forever -- but it was not the cause, and the retry it introduced proves it: every request now re-reads and still derives nothing, so the empty result is persistent rather than a cold-start race. Which leaves the actual cause somewhere between the adapter and the extractor, and no way to tell from outside, because every path out of this code returns an empty derivation with debug-level logs. A derivation that never works is indistinguishable from a project that references no external origins. So each outcome now says which one it was: the adapter cannot run in a tenant context, it exposes no source listing, the read threw, the read returned nothing, or it read N files and derived M origins. The success line carries the file count because "read 40 files, derived 0 origins" is the shape of a broken extractor and reads identically to a correct one without it. Logged once per content version, which is what the cache key already is, so this cannot grow with traffic.
|
Warning Review limit reached
Next review available in: 16 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)
📝 WalkthroughWalkthroughCSP origin derivation now emits structured warning and info logs for source-read failures, empty sources, unavailable runtime context, and derivation errors. Successful derivations report file and origin counts. Non-fatal fallback behavior remains unchanged. ChangesCSP derivation observability
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c7ecbca5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/security/http/derived-csp-cache.ts`:
- Around line 107-113: Add a bounded per-key warning guard around the
logger.warn calls in getDerivedCspOrigins so repeated failures or empty reads
for the same lookup key do not emit warnings on every request. Preserve clearing
inFlight and retrying subsequent reads, and add a test that repeatedly calls the
same key and verifies warning emission is limited.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 212b9f19-55ec-4b1d-b880-920048c65088
📒 Files selected for processing (2)
src/security/http/derived-csp-cache.tssrc/server/runtime-handler/project-runtime-context.ts
…quest The two failure paths deliberately do not cache, so the read is retried on every request. The diagnostic followed it, which would emit a warning per request per pod for as long as a failure lasted -- the log flooding the report endpoint's own ceiling exists to prevent, and the opposite of what I claimed in this PR's description. The warning is now gated on a bounded per-content-version guard, so the retry is preserved and the signal stays one line per release. The guard is exported and tested directly rather than through log interception, which the component logger makes impossible.
1 participant
v0.1.1216 is live in production and derived origins still do not appear. I verified after the rollout:
vf-csp-probe.production.veryfront.comserves the bare floor, whilevf-csp-probe.preview.veryfront.comserves both derived origins from the same release and the same source.#3474 was a real bug but not this one. It stopped an empty read being cached forever, and the retry it introduced is what proves the point: every request now re-reads the source and still derives nothing. The empty result is persistent, not the cold-start race I diagnosed. I said that fix would close this out; it did not, and the promotion notes on veryfront-server#304 are wrong about that.
Two further hypotheses, both falsified:
File list warmup failedwarnings, andRefreshed source snapshotshows the adapter reading source for this project.Which leaves me unable to diagnose it from outside, because every path out of the derivation returns an empty result with
debug-level logs, and production runs above debug. A derivation that never works looks exactly like a project that references no external origins. That silence is the reason this shipped broken, survived a promotion, and then survived a fix aimed at the wrong cause.So this makes each outcome say which one it was:
The last one carries the file count deliberately: read 40 files, derived 0 origins is the signature of a broken extractor, and without the count it reads identically to a correct derivation of a project that uses no external origins.
Logged once per content version — which is precisely what the cache key already is — so it cannot grow with traffic.
This is not a fix. It is the instrument I should have added before claiming a diagnosis, and it will name the cause on the next release.
Lint, typecheck, fmt and the full unit suite green by exit code.
Summary by CodeRabbit
Bug Fixes
Logging