fix(security): clear the scheduled fuzz OOM and the 3 Semgrep SAST findings at base - #36
fix(security): clear the scheduled fuzz OOM and the 3 Semgrep SAST findings at base#36seonghobae wants to merge 3 commits into
Conversation
…harness OOM The scheduled Atheris fuzz job on the base branch fails: libFuzzer reports out-of-memory (used ~2049Mb / 2069Mb vs the 2048Mb rss_limit) and uploads two `oom-*` reproducers. The run itself completes without any AssertionError or uncaught exception — it is a memory-growth crash, not a per-input defect. Root cause: `orchestrator.execute_query` appends an `AuditEvent` to `catalog._AUDIT_LOG` (via `ingest_event`) and a `PolicyDecision` to `evidence._POLICY_DECISION_LOG` (via `policy.evaluate` -> `record_policy_decision`) on every call, and `orchestrator.draft_sql` appends a `PolicyDecision` on every call. These are unbounded module-level lists. A coverage-guided harness runs the target ~10^6 times in one process, so the lists grow without bound until the process OOMs. Measured: 50k execute+draft iterations -> _AUDIT_LOG=50000, _POLICY_DECISION_LOG=100000, RSS 257Mb and climbing. Fix (harness hygiene, mirroring the `isolate_in_memory_app_state` test fixture): add `invariants.reset_accumulating_state()` — clears `catalog._AUDIT_LOG`, `catalog._SCHEMA_HISTORY`, and `evidence._POLICY_DECISION_LOG` while leaving seed data (`catalog._DATA`) intact — and call it each iteration in the two state-mutating harnesses (`fuzz_draft_sql`, `fuzz_execute_query`). The read-only harnesses (`fuzz_resolve_terms`, `fuzz_search_catalog`) do not accumulate and are unchanged. Verification: - With the reset, 50k execute+draft iterations keep both logs at 0 and RSS flat at 36Mb (was 257Mb and growing). - `fuzz_execute_query` under `-rss_limit_mb=350` now completes 747,663 runs in 46s with RSS flat at 67Mb and exit 0 (previously OOM within seconds). - `tests/fuzz/test_fuzz_properties.py` (which shares invariants.py): 10 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Msz8Dni5zRaqPatNCmZJZ8
|
Warning Review limit reached
Next review available in: 44 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Comment |
…ings The central "Semgrep (multi-language SAST)" gate (p/default, fail on WARNING/ERROR) reports 3 pre-existing blocking findings on every SDP PR (it runs on pull_request, so `main` never surfaces them). Resolved all three: - src/sdp/authz.py:_load_jwks_from_url — REAL hardening. `urlopen(jwks_url)` had no scheme check, so a misconfigured SDP_OIDC_JWKS_URL could turn it into a local-file read (urllib honours file://). Now the scheme is validated to http/https and anything else (file/ftp/data/empty) raises ValueError before urlopen. (rule dynamic-urllib-use-detected) - src/sdp/observability.py:_export_to_sink — reviewed-safe suppression. The urlopen is only reached after the scheme is validated to http/https above (file:// takes the file-sink branch; every other scheme raises), so the file:// concern cannot apply. Scoped `# nosemgrep` with that rationale. - src/sdp/graph_store.py (AGE cypher exec) — false positive. The statement is a psycopg `sql.Composed` built from `sql.Literal(graph_name)` + `sql.Literal(query)` and an allowlisted result-columns fragment, with params bound as a `%s` argument — no untrusted string concatenation. Scoped `# nosemgrep` with that rationale. (rule sqlalchemy-execute-raw-query) Suppressions use the exact reported rule ids and each carries a one-line justification; only two of three findings are suppressed and both are reviewed-safe (the third is a genuine code fix), so the gate is not weakened. Verification: added tests/test_authz_jwks.py (4 rejected schemes); the new test plus tests/test_api.py pass. Local semgrep repro of p/default is blocked by the proxy (registry unreachable), so the suppression match is asserted against the exact rule ids logged by the failing CI run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Msz8Dni5zRaqPatNCmZJZ8
seonghobae
changed the title
atheris 3.0.0 ships cp311/cp312/cp313 manylinux x86_64 wheels but no cp314 wheel, so any wheel-only CPython 3.14 environment (e.g. the central coverage-evidence trusted image) cannot install atheris==3.0.0. atheris 3.1.0 ships cp312/cp313/cp314 wheels (it dropped cp311 and ships no sdist). This repo's fuzz image and clusterfuzzlite run on CPython 3.12, where 3.1.0 has a wheel too, so the bump is safe here and stays installable on newer interpreters. Listed hashes cover the cp312/cp313/cp314 Linux x86_64 wheels for --require-hashes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Msz8Dni5zRaqPatNCmZJZ8
|
Diagnosis for the blocked The cp314 image upgrade breaks coverage-evidence in two ways, both applicable to a repo that pins
Neither is fixable from this PR branch. My per-branch Generated by Claude Code |
|
Closing as superseded by #34, which contains the same scheduled-fuzz OOM remediation and the same 3 base Semgrep fixes (authz JWKS scheme allow-list, observability/graph_store documented suppressions) plus crash-log reproducer visibility and the coverage test suite. Consolidating on one head frees the OpenCode review-dispatch queue for the surviving PR. The branch is preserved; reopen if #34 stalls. Generated by Claude Code |
2 participants
Two base-branch check failures on
main, both surfaced by the current-HEAD audit and fixed here.1. Scheduled Atheris fuzz OOM
Red daily since the fuzz workflow landed. Not a per-input defect: no
AssertionError/uncaught exception — libFuzzer reports out-of-memory (~2049/2069Mb vs 2048Mbrss_limit) and uploads twooom-*reproducers.Root cause:
execute_queryappends tocatalog._AUDIT_LOG+evidence._POLICY_DECISION_LOGevery call,draft_sqlappends aPolicyDecisionevery call — unbounded module lists. A coverage-guided harness runs ~10⁶ times/process → the lists grow until OOM (measured: 50k iters →_AUDIT_LOG=50000,_POLICY_DECISION_LOG=100000, RSS 257Mb climbing).Fix:
invariants.reset_accumulating_state()(clears the two logs +_SCHEMA_HISTORY, keeps seed_DATA), called each iteration in the two state-mutating harnesses; the read-only harnesses are untouched. Mirrors theisolate_in_memory_app_statetest fixture.Verified: with the reset, 50k iters keep both logs at 0 / RSS flat 36Mb;
fuzz_execute_queryunder-rss_limit_mb=350runs 747,663 iters in 46s, RSS flat 67Mb, exit 0 (was OOM within seconds).tests/fuzz/test_fuzz_properties.py: 10 passed.2. Semgrep (multi-language SAST) — 3 Medium+ findings
p/default(fail on WARNING/ERROR) reports 3 pre-existing blocking findings on every PR (Semgrep runs onpull_request, somainnever surfaces them). It does not gate auto-merge, but these are real Medium+ findings.authz.py_load_jwks_from_urlurlopen(rejectsfile://LFI etc.)observability.py_export_to_sink# nosemgrep—urlopenreached only after scheme is validated to http/https abovegraph_store.py(AGE cypher exec)# nosemgrep— psycopgsql.Composedofsql.Literal(...)+ an allowlisted fragment, params bound as%s; no string concatSuppressions use the exact reported rule ids with one-line justifications; only 2 of 3 are suppressed (both reviewed-safe), the third is a genuine code fix — the gate is not weakened. Added
tests/test_authz_jwks.py(4 rejected schemes); it +test_api.pypass. (Localp/defaultrepro is blocked by the proxy — registry unreachable — so suppression rule ids are taken from the failing run's logged findings.)Note:
tests/test_graph_security.pyfailures reproduce identically on unmodifiedmain(they needsqlalchemy, absent locally, present in CI) — unrelated to this PR.