Skip to content

feat(csp): wire report-only CSP to a real reporting endpoint#263

Open
kilbot wants to merge 2 commits into
mainfrom
worktree-csp-reporting-endpoint
Open

feat(csp): wire report-only CSP to a real reporting endpoint#263
kilbot wants to merge 2 commits into
mainfrom
worktree-csp-reporting-endpoint

Conversation

@kilbot

@kilbot kilbot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Problem

The site was emitting this console error (twice — once per document, e.g. page + Stripe iframe) on every page load:

The Content Security Policy '…' was delivered in report-only mode, but does not specify a 'report-to'; the policy will have no effect.

The Content-Security-Policy-Report-Only header in next.config.ts had no report-to/report-uri directive and no collection endpoint anywhere in the app. A report-only policy never blocks (that's the point), so with nowhere to send reports it does nothing at all — the browser is correct that it "will have no effect." It's been inert this whole time, which also means the code comment's "verify clean against real traffic, then promote to enforcing" plan could never actually gather any data.

Fix (best-practice reporting wiring)

Rather than delete the header or blindly enforce an unvalidated policy, make the report-only phase actually work:

  • New POST /api/csp-report sink modeled on the existing report-failure route:
    • Accepts both wire formats: legacy application/csp-report (report-uri, used by Safari/Firefox) and the modern application/reports+json array (report-to, Reporting API, Chromium).
    • Rate-limited per IP (60/min, fail-open) + 8 KB body cap — it's an unauthenticated, browser-driven endpoint.
    • Sanitizes report fields (strips CR/LF/control chars, bounds length) before logging — log-injection guard.
    • Logs via infraLogger.warn → Loki/Sentry. Deliberately not the sale/Discord path: a CSP violation is diagnostic, not something to page anyone about.
    • Always returns 204 — a broken/abusive report must never surface to the visitor.
  • CSP directives: added report-uri /api/csp-report (legacy fallback) and report-to csp-endpoint (modern).
  • Reporting-Endpoints header: names the csp-endpoint group with the same-origin path, so it follows whichever host served the page (wcpos.com / www).

Once real violations have been observed clean, the header can be flipped from -Report-Only to enforcing.

Verification

  • 7 new unit tests (src/app/api/csp-report/route.test.ts): both formats, multi-violation arrays, non-CSP report filtering, log-injection sanitization, rate-limit drop, malformed JSON, oversized body. All green.
  • Full API-route suite: 240/240 passing.
  • ESLint + tsc --noEmit: clean on changed files.
  • Live dev-server check (next dev):
    • Response headers now include Reporting-Endpoints: csp-endpoint="/api/csp-report" and the CSP ends with …; report-uri /api/csp-report; report-to csp-endpoint.
    • POST /api/csp-report returns 204 for legacy, modern, and malformed bodies.
    • Server log confirmed both a legacy and a modern violation logged under wcpos·infra; malformed body logged nothing.

Notes

  • No behavior change for visitors — still report-only, still non-blocking.
  • Endpoint hardening mirrors the deferred follow-up noted for report-failure (abuse cap + no raw logging of attacker-controlled input).

Summary by CodeRabbit

  • New Features
    • Added a same-origin endpoint to receive Content Security Policy violation reports.
    • Support added for both legacy and modern CSP reporting formats.
  • Bug Fixes
    • Improved resilience against malformed payloads, oversized bodies, and unexpected report shapes (reports are safely ignored).
    • Enhanced sanitization to prevent unsafe or noisy content from reaching logs.
    • Requests beyond the rate limit are dropped without affecting the user experience.
  • Tests
    • Expanded automated coverage for size caps, batching limits, and sanitization behaviors.

The Content-Security-Policy-Report-Only header had no report-to/report-uri
directive, so browsers warned it 'will have no effect' — a report-only policy
with nowhere to send reports neither blocks nor reports, making it inert (and
noisy in the console).

Add the reporting plumbing so the header does its job:
- New POST /api/csp-report sink that accepts both wire formats (legacy
  application/csp-report and the Reporting API's application/reports+json),
  rate-limited + size-capped + always 204, logging via infraLogger (Loki/
  Sentry, not the Discord sale path).
- report-uri + report-to directives on the CSP, plus a Reporting-Endpoints
  header naming the csp-endpoint group.

This finally makes the 'verify clean, then enforce' plan actionable: real
violations now land in the log sink instead of being silently discarded.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d93410fe-66e5-4d88-85f3-d8854b4b306b

📥 Commits

Reviewing files that changed from the base of the PR and between 2d6173c and db6356b.

📒 Files selected for processing (2)
  • src/app/api/csp-report/route.test.ts
  • src/app/api/csp-report/route.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/app/api/csp-report/route.ts

📝 Walkthrough

Walkthrough

This PR adds a same-origin CSP reporting endpoint at /api/csp-report. next.config.ts now routes report-only CSP violations to that path, and the new POST handler enforces size limits, rate limiting, payload normalization, sanitization, logging, and a 204 response. Tests cover the endpoint’s payload shapes and edge cases.

Changes

CSP Reporting

Layer / File(s) Summary
CSP header configuration
next.config.ts
Adds CSP_REPORT_PATH, report-uri/report-to directives on the report-only CSP, and a Reporting-Endpoints header mapping csp-endpoint to that path.
CSP report route implementation
src/app/api/csp-report/route.ts
Adds POST handling for request-size caps, IP rate limiting, legacy and Reporting API payload normalization, field sanitization, violation logging, and always returning 204.
Route test suite
src/app/api/csp-report/route.test.ts
Adds mocks for logger/rate-limiter and tests for legacy and modern payloads, URL and log-injection sanitization, batched-report limits, rate-limit fail-open, malformed JSON, unrecognized payloads, and size-cap enforcement.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: wiring report-only CSP to a real reporting endpoint.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-csp-reporting-endpoint

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/app/api/csp-report/route.test.ts (1)

120-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test only exercises the spoofable declared-length shortcut, not the real byte cap.

This test sends a small body with a large declared content-length header, which only proves the early declared-length check works. It doesn't cover the case of an actual oversized body without a (or with an honest small) content-length header, which is exactly the path that currently buffers the whole body via request.text() before checking size (see comment on route.ts). Consider adding a test that sends a real >8KB body to confirm the post-read cap actually works as intended.

🤖 Prompt for 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.

In `@src/app/api/csp-report/route.test.ts` around lines 120 - 129, The current
csp-report test only covers the spoofable declared content-length shortcut, so
add coverage in the POST test suite for the real byte-size limit path in POST by
sending an actually oversized request body and verifying it is dropped after
read. Update the test around POST in csp-report/route.test.ts to use a payload
larger than the 8KB cap with an honest or absent content-length header, so it
exercises the request.text() buffering path and the post-read size check instead
of only the declared-length branch.
🤖 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/app/api/csp-report/route.ts`:
- Around line 84-99: The POST handler in POST currently buffers the full request
body with request.text() before enforcing MAX_BODY_BYTES, so oversized payloads
can still consume memory. Update the body handling in
src/app/api/csp-report/route.ts to read the request as a stream, count bytes
incrementally, and abort immediately once the limit is exceeded; keep the
existing noContent behavior and preserve the limiter/clientIp/extractViolations
flow around the new streaming read.

---

Nitpick comments:
In `@src/app/api/csp-report/route.test.ts`:
- Around line 120-129: The current csp-report test only covers the spoofable
declared content-length shortcut, so add coverage in the POST test suite for the
real byte-size limit path in POST by sending an actually oversized request body
and verifying it is dropped after read. Update the test around POST in
csp-report/route.test.ts to use a payload larger than the 8KB cap with an honest
or absent content-length header, so it exercises the request.text() buffering
path and the post-read size check instead of only the declared-length branch.
🪄 Autofix (Beta)

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5f45c2b1-35ca-4b80-b75c-d19b70176380

📥 Commits

Reviewing files that changed from the base of the PR and between 2ef1925 and 2d6173c.

📒 Files selected for processing (3)
  • next.config.ts
  • src/app/api/csp-report/route.test.ts
  • src/app/api/csp-report/route.ts

Comment thread src/app/api/csp-report/route.ts
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

🚀 Preview: https://wcpos-5i1j801yu-wcpos.vercel.app

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d6173cb23

ℹ️ 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".

Comment thread src/app/api/csp-report/route.ts
Comment thread src/app/api/csp-report/route.ts Outdated
Comment thread src/app/api/csp-report/route.ts

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@wcpos-bot

wcpos-bot Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Post-fix triage for review feedback:

Thread File Issue Decision Commit
CodeRabbit inline route.ts Body cap was enforced after request.text() buffered the request Fixed with bounded stream read and oversized-stream regression test db6356b
Codex inline route.ts Reported URLs could log query/fragment secrets Fixed by stripping query strings and fragments from logged URL fields db6356b
Codex inline route.ts Duplicate body-cap-before-buffering concern Fixed by the same bounded stream reader db6356b
Codex inline route.ts Batched Reporting API payloads could emit too many warning logs Fixed by capping processed CSP violations per body db6356b
CodeRabbit review body route.test.ts Oversized-body coverage only used spoofed content-length Fixed with an actually oversized streamed-body test db6356b

Skipped threads:

Thread Reason for skipping
None All fresh unresolved actionable review feedback was fixed and resolved.

Validation:

  • pnpm install --frozen-lockfile passed.
  • pnpm exec vitest run src/app/api/csp-report/route.test.ts passed: 10 tests.
  • pnpm lint passed with existing warnings outside this change.
  • pnpm exec tsc --noEmit passed.
  • pnpm test:unit passed: 160 files, 1160 tests.
  • pnpm test:e2e --project=chromium could not complete in this container because Chromium cannot load libglib-2.0.so.0; pnpm exec playwright install-deps chromium also failed because the container cannot elevate via su.

Remaining unresolved inline review threads after resolving: 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant