draft(preservation): retain Coraza/CRS enforcement evidence for bounded successor - #95
draft(preservation): retain Coraza/CRS enforcement evidence for bounded successor#95seonghobae wants to merge 48 commits into
Conversation
When CORAZA_WAF_URL is set, each /gateway request is POSTed to that sidecar and the response is parsed with the existing Coraza audit adapter so OWASP CRS stays the detection authority. Sidecar outage is fail-closed when PROVEN_ENGINE_FAIL_CLOSED is true. Refs #86.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughIn-process libcoraza를 sidecar보다 우선하는 WAF 경로를 추가했습니다. Outbound 목적지 검증, DNSBL, transactional outbox, PostgreSQL 운영, 서명된 릴리스 및 실행 검증 계약도 갱신했습니다. Changes보안 경계와 운영 기반
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟡 Moderate · up to This PR adds synchronous Coraza sidecar decisions to live gateway traffic, so an impersonated sidecar could alter blocking decisions and a sidecar outage can silently degrade protection by default without making the gateway unready. Additional unresolved code and documentation issues affect error handling, secret initialization, and the accuracy of operational claims, so merge requires explicit owner acceptance or follow-up on these bounded risks. Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant InProcessCoraza
participant CorazaSidecar
participant DestinationPolicy
participant EgressDNS
Client->>Gateway: /gateway 요청
Gateway->>InProcessCoraza: URI, 헤더, 본문 평가
InProcessCoraza-->>Gateway: Clean 또는 interruption
Gateway->>CorazaSidecar: InProcessCoraza가 없을 때 평가
CorazaSidecar-->>Gateway: 상태 코드와 감사 JSON
Gateway->>DestinationPolicy: outbound 목적지 평가
DestinationPolicy->>EgressDNS: 승인된 주소 해석
EgressDNS-->>DestinationPolicy: 정책 검증 주소
Gateway-->>Client: 정상 응답, 403 또는 503
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Review-hardening for PR #95 findings: - Forward a bounded allowlist of client headers to the engine (host, user-agent, accept, content-type, referer, origin, x-requested-with, x-forwarded-for, x-real-ip, cookie; never Authorization) with 32-header and 8 KiB caps so CRS rules that inspect headers see real input without shipping credentials to sidecar logs. - Stream sidecar responses under a 1 MiB cap instead of unbounded text() buffering; oversized or non-UTF-8 answers are Unavailable. - Explicit status contract: 2xx parses audit JSON (empty stays clean), 403 without parseable JSON remains an interruption fallback, every other status is Unavailable so fail-closed deployments never allow on a confused sidecar answer. - Monitor-mode routes and sub-threshold hits now record engine_hit events with the cited CRS rule instead of dropping the evidence. - Fail-open outages record engine_unavailable events; scoring continues. Doctoring gains the Scarfone & Mell (2007) NIST SP 800-94 grounding with the redistributable public-domain PDF committed under docs/papers/. Tests: header allowlist bounding + credential exclusion, sidecar payload headers, non-success status Unavailable, oversized response Unavailable, fail-open outage event, monitor-route engine_hit evidence.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
In-process transactions now receive the same forwarded-header allowlist as the sidecar path (host, user-agent, accept, content-type, referer, origin, x-requested-with, x-forwarded-for, x-real-ip, cookie — never Authorization; 32 headers / 8 KiB caps enforced by proven_engine::engine_forwarded_headers). Each header crosses the C ABI via coraza_add_request_header before process_request_headers, so CRS rules that inspect headers evaluate real client input instead of a synthetic Host only. Brings in the PR #95 sidecar hardening via merge so both engines share one allowlist implementation and one status/bound contract. Behavioral header-battery evidence lands with the issue-11 battery fixture (PR #110); this slice ships the plumbing and keeps the stub contract unchanged.
CodeRabbit finding: a 403 with a non-JSON body (e.g. the CRS default block page) parsed to Unavailable, and the interruption fallback only fired on Clean — so fail-open deployments could allow a real CRS block. The fallback now covers every non-Hit outcome on 403: audit JSON keeps its parsed hit; empty or HTML bodies become the interrupted evidence Hit. Test: sidecar_403_with_non_json_body_is_an_interruption_hit.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head63b7930cdd6dbebd427f49e2c568bb198f490836. -
Head SHA:
63b7930cdd6dbebd427f49e2c568bb198f490836 -
Workflow run: 32702403014
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (6 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (6 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (5 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (5 files)"]
R2 --> V2["docs review"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (6 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (6 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (5 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (5 files)"]
R2 --> V2["docs review"]
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e79e44e9f
ℹ️ 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".
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Addressed on the current branch push for feat/issue-86-in-path-coraza: hash_partition_sql_for now keeps wardnet_runtime grants on the partitioned security_event parent only, so runtime access stays behind the parent RLS policy instead of direct child-table grants. Local verification on Friday, August 28, 2026: cargo fmt --check; cargo test. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
seonghobae
changed the title
3 participants
Preservation status — 2026-09-05 KST
This PR is intentionally Draft and is not an integration candidate. Fresh protected truth is
main@5829a0f08d78de464dd24393ce5d0f25fba9d126(merged #159), while GitHub still records this branch against obsoletemain@107117634764c901dff540044585d64088fafedb; current head is304f053013a76f11a03b5ed9b058ae1712f6ea12and GitHub reports it non-mergeable.The branch accumulated 52 changed paths across unrelated owners/bounded contexts: Coraza/CRS enforcement, destination/egress policy, PostgreSQL control-plane/outbox, release/SBOM/provenance, deployment manifest, credentials, load evidence and product-gap documentation. That violates the current single-writer/DDD repair discipline and now overlaps newer bounded candidates/owner paths. Do not mechanically restack or merge the aggregate branch.
Unique evidence to preserve
The still-relevant Wardnet-owned slice is the proven-engine WAF/IDS work for #86. Any bounded successor must inventory and either carry forward or prove obsolete every unique item below before this preservation PR can be retired:
/gatewayconsultation of a proven Coraza/OWASP CRS authority rather than treating hand-written scoring as detection authority;docs/doctoring/in-path-coraza-adapter.md/docs/doctoring/in-process-libcoraza.mdreasoning and any non-duplicated APA 7 traceability for the enforcement boundary;Historical local test claims and predecessor workflow results do not transfer to a successor head.
Superseded / separately owned material
Do not preserve aggregate ownership merely because these paths are present here:
docs/product-technical-gap-baseline.mdhas its own current single-writer lane; this stale copy is not documentation authority.Reconstruction acceptance
main, not this stale branch tip.No force-push/destructive rebase, self/model approval, routine administrator bypass, predecessor evidence reuse, or aggregate-branch merge is authorized.