fix(ci): stop the nightly Atheris OOM + clear 3 reviewed Semgrep base findings - #33
fix(ci): stop the nightly Atheris OOM + clear 3 reviewed Semgrep base findings#33seonghobae wants to merge 2 commits into
Conversation
…job stops OOMing The scheduled `fuzz` job has failed on `main` for 9 consecutive nights (Jul 21-29). Root cause is a harness state-isolation bug, not a defect in the code under test: - Every `orchestrator.execute_query` call records an audit event (`catalog._AUDIT_LOG.append`) and, via `policy.evaluate`, a policy decision (`evidence._POLICY_DECISION_LOG.append`); `draft_sql` records the policy decision. Both are unbounded module-level lists (the in-memory fallback used when no evidence store is configured, as in CI). - The Atheris harnesses drive these functions with coverage guidance and never reset that shared state, so the lists grow one entry per iteration. At the 60s PR budget this stays under libFuzzer's 2 GiB RSS limit; at the 300s nightly budget it does not — the run is OOM-killed (`libFuzzer: out-of-memory (used: 2060Mb; limit: 2048Mb)`, an `oom-*` reproducer) and the job fails. Reproduced locally: execute_query hit 2060 MB and OOMed at ~440k execs (~50s). Fix: add `invariants.reset_shared_state()` (clears the two accumulators via `list.clear()`, preserving object identity) and call it at the top of every Atheris harness's `TestOneInput`, plus an autouse fixture in the Hypothesis mirror. This isolates each iteration exactly as the API tests' `isolate_in_memory_app_state` fixture already does, keeping the crash-oracle focused on input handling rather than accumulated memory. No production code changes; the oracles are untouched. After the fix execute_query runs 915k iterations in 101s to a clean DONE with RSS flat at 64 MB (was OOM at ~50s). Added a fast regression test (`test_reset_shared_state_bounds_accumulators_across_a_call_loop`) so a future refactor that drops the reset fails in the unit suite, not only in nightly CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
|
Warning Review limit reached
Next review available in: 29 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 (9)
Comment |
…on base The central `Semgrep (multi-language SAST)` required gate (`--config=p/default`, fail on WARNING/ERROR) reports 3 Medium+ findings that fail on this repo's base and block the PR queue. All three are reviewed false positives or safe-by-construction; annotate each with the gate's sanctioned `# nosemgrep` marker (its SARIF-suppression step strips nosemgrep-annotated results). The gate is not weakened — any new/unsuppressed Medium+ finding still fails. - authz.py:116 `dynamic-urllib-use-detected` (WARNING): `urlopen(jwks_url)` fetches the OIDC JWKS; jwks_url is operator config (SDP_OIDC_JWKS_URL from the KV/env), never attacker-controlled. - observability.py:139 `dynamic-urllib-use-detected` (WARNING): the POST is reached only after the scheme is validated to http/https (file:// handled separately, any other scheme raises); sink_url is operator config (SDP_LOG_SINK_URL). - graph_store.py:478 `sqlalchemy-execute-raw-query` (ERROR): the statement is a psycopg.sql.SQL composition with graph_name/query as Literal()-escaped values and result_columns from a closed allowlist map; agtype params bind via %s. No untrusted string concatenation reaches raw SQL. Comment-only; no runtime behavior change. Clears the Semgrep gate on this PR and remediates the finding on base. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wtuYFp4E22QnEU1bFMhsr
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 head79febc12aed7f723162798643b8d6bf2e72694e7. -
Head SHA:
79febc12aed7f723162798643b8d6bf2e72694e7 -
Workflow run: 30501789385
-
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 (3 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (3 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test (6 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (6 files)"]
R2 --> V2["targeted test run"]
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 (3 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (3 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test (6 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (6 files)"]
R2 --> V2["targeted test run"]
|
|
Closing as superseded by #34, which carries the same nightly Atheris OOM fix ( Generated by Claude Code |
This PR remediates two standing base-branch CI failures on
main.1. Nightly
fuzz(Atheris) OOM — red 9 nights running (Jul 21–29)The push-triggered
fuzzrun (60s/target) is green; only the nightly (300s/target) fails, so it rots the static HEAD silently.Root cause (a harness state-isolation bug, not a defect in the code under test): every
orchestrator.execute_querycall records an audit event (catalog._AUDIT_LOG.append) and — viapolicy.evaluate— a policy decision (evidence._POLICY_DECISION_LOG.append);draft_sqlrecords the policy decision. Both are unbounded module-level lists (the in-memory fallback used when no evidence store is configured, as in CI). The Atheris harnesses drive these functions with coverage guidance and never reset that state, so the lists grow one entry per iteration — under the 2 GiB RSS limit at 60s, over it at 300s. Reproduced locally againstmainHEAD:Fix:
invariants.reset_shared_state()(clears the two accumulators vialist.clear(), preserving object identity), called at the top of every Atheris harness'sTestOneInputand via an autouse fixture in the Hypothesis mirror — the same isolation the API tests'isolate_in_memory_app_statefixture already provides. No production code change; the invariant oracles are untouched.Verified: after the fix
execute_queryruns 915,627 iterations in 101s to a cleanDONE, RSS flat at 64 MB (was OOM at ~50s). Addedtest_reset_shared_state_bounds_accumulators_across_a_call_loopso a future refactor dropping the reset fails in the unit suite, not only in nightly CI.2. Central
Semgrep (multi-language SAST)gate — 3 reviewed false positives on baseThe org-required
--config=p/defaultgate reports 3 Medium+ findings that fail on this repo's base; all three are reviewed false positives / safe-by-construction, annotated with the gate's sanctioned# nosemgrepmarker (its SARIF-suppression step strips annotated results — the gate is not weakened, any new/unsuppressed Medium+ finding still fails):authz.py:116dynamic-urllib-use-detected(WARN)urlopen(jwks_url)fetches OIDC JWKS;jwks_urlis operator config (SDP_OIDC_JWKS_URL), never attacker-controlledobservability.py:139dynamic-urllib-use-detected(WARN)sink_urlis operator config (SDP_LOG_SINK_URL)graph_store.py:478sqlalchemy-execute-raw-query(ERROR)psycopg.sql.SQLcomposition —graph_name/queryareLiteral()-escaped,result_columnsfrom a closed allowlist map, params bound via%s; no untrusted string concatenationComment-only; no runtime behavior change.
Verification
pytest tests/fuzz/test_fuzz_properties.py— 11 passed (10 existing + 1 new regression).oom-*/crash-*.py_compileclean on all edited files.Follow-up (out of scope)
The unbounded in-memory
_AUDIT_LOG/_POLICY_DECISION_LOGfallbacks would also grow without bound in a long-running server run without a configured evidence store; bounding/rotating them is a separate production-hardening change (it touches the audit-retrieval API and theisolate_in_memory_app_statefixture) and is intentionally not bundled here.