fix(proof): reconcile managed browser preflights - #457
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change adds a Playwright-managed preflight contract, sanitizes CDP network metadata, and extends live evidence recording to validate, reconcile, count, diagnose, and dispose browser-managed preflight lifecycles. ChangesBrowser-managed preflight evidence
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant CDPObserver
participant LiveEvidenceRecorder
participant Policy
participant Evidence
Browser->>CDPObserver: Emit preflight request and response events
CDPObserver->>LiveEvidenceRecorder: Provide normalized lifecycle metadata
LiveEvidenceRecorder->>Policy: Validate and correlate the preflight
LiveEvidenceRecorder->>Evidence: Record synthetic completion
LiveEvidenceRecorder->>Evidence: Reconcile counts during finalization
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@scripts/product-proof-cloud-playwright.mjs`:
- Around line 1716-1734: Update the header-validation loop processing
Object.entries(value) so name and value size limits are enforced only for
headers retained via cdpIdentityHeaderNames. Preserve the existing key-count
cap, duplicate-name detection, and CR/LF/NUL rejection for every header, while
continuing to set credentialHeaderPresent and forbiddenHeaderPresent as
currently implemented.
- Around line 43-44: Update the diagnostics configuration for browser-managed
preflight stages to require the cdp adapter for both
browser-managed-preflight-before-response and
browser-managed-preflight-response-only. Add the adapter check to the top-level
diagnostic and every corresponding open_lifecycles entry, preventing mediated
from being accepted even when policy_label is present.
In `@scripts/product-proof-cloud-playwright.test.mjs`:
- Around line 5438-5456: The bundle assertion around chromiumNetworkManager
should avoid coupling to generated identifiers and formatting. Keep the
preflight guard match and the responseCode: 204 and body: "" checks, but remove
the routeConstruction/indexOf dependency and the assertion requiring the exact
return/brace whitespace pattern.
In `@scripts/product-proof-live-evidence.mjs`:
- Around line 546-576: Deduplicate the header names assembled in
requestedHeaders within the browser-managed preflight detection loop before
sorting and joining them. Ensure base.request.headers keys are combined with
apikey, authorization, and x-client-info using unique-name semantics so
compareCanonical receives the same non-repeated Access-Control-Request-Headers
value Chromium emits.
In `@scripts/product-proof-live-evidence.test.mjs`:
- Around line 637-677: Update the requestFailures loop around
browserManagedPreflightRequestEvent so the missing forbidden posture case
removes the forbiddenHeaderPresent property when its override is undefined,
matching the existing redirected and credentialHeaderPresent handling. Preserve
the assertion and all other request override behavior unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c6293150-d617-40de-b57b-ff8b66ef207f
📒 Files selected for processing (4)
scripts/product-proof-cloud-playwright.mjsscripts/product-proof-cloud-playwright.test.mjsscripts/product-proof-live-evidence.mjsscripts/product-proof-live-evidence.test.mjs
Purpose
Repair product-proof HTTP teardown for Playwright 1.62.1 Chromium-managed CORS preflights. Chromium auto-fulfils intercepted preflight OPTIONS requests with a synthetic 204 before Playwright creates a Route, while the independent CDP observer still records the lifecycle. Those proven requests therefore remained uncorrelated and stopped governed Home proof with
live_http_teardown_not_idle.Included scope
*-optionspolicy, uncached 204/preflight response, then one loading-finished event.Explicit exclusions
No application behavior, timeout, generic request acceptance, schema, migration, dependency, workflow, credential, provider, billing, customer-data, staging, or production change.
Architecture placement
Current owner / directory: Product-proof browser/network harness in
scripts/product-proof-cloud-playwright.*andscripts/product-proof-live-evidence.*.Dependencies and duplicate-system check: Reuses the existing expected-request policy, HMAC identity, CDP mediation, zero-open teardown, and evidence attestation boundaries. No parallel network policy or observer was introduced.
Security and tenancy impact
Data impact
Verification
asset_vite_manifest_invalidlocal fixture failure.Release safety
Documentation and generated files
Completion check
origin/main.Policy exceptions (leave blank when none apply)
PR-size exception rationale: The +1,266/-29 four-file diff is atomic because this is a privacy/network evidence boundary; 883 added lines are adversarial tests covering the exact positive lifecycle and every required fail-closed identity, cache, ordering, limit, and non-disclosure branch. Splitting source from its security proof would create an unreviewable intermediate state.
Retained branch reason / owner / review date / maximum retention date:
Summary by CodeRabbit
Enhancements
Tests