fix(security): annotate reviewed Semgrep p/default false-positives - #16
Conversation
Suppress the 15 Medium+ findings from the central "Semgrep
(multi-language SAST)" gate that are reviewed false positives, using the
sanctioned `# nosemgrep` mechanism (the gate strips nosemgrep-annotated
findings from the SARIF). No gate is weakened; only documented
false-positives are annotated.
- pg_llm_batch/config.py: 7 sqlalchemy-execute-raw-query /
formatted-sql-query findings on cur.execute() calls where the only
interpolated token is the fixed class constant TABLE_NAME
("com_config"/"com_secrets") and every value is bound via %s
placeholders; 1 python-logger-credential-disclosure where the only
logged argument is the literal mask "***".
- pg_llm_batch/token_counter.py: 2 python-logger-credential-disclosure
findings that log a model name / a generic pg_tiktoken exception at
debug level, never a credential.
- docker/postgres/Dockerfile: 2 missing-user findings on the
postgres-base intermediate build stage, which is never run directly;
both deployable targets (with-tiktoken, runtime) end with an explicit
USER postgres. Placed on the preceding line because exec-form
ENTRYPOINT/CMD cannot carry an inline comment.
Verified with semgrep 1.172.0 that each annotated line is marked
suppressed (inSource) in the SARIF. Unblocks pg-llm-batch #10-13.
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 (3)
Comment |
![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 headebca495b28510cf695dcc92cf7b200de643d7efb. -
Head SHA:
ebca495b28510cf695dcc92cf7b200de643d7efb -
Workflow run: 30494851566
-
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"]
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"]
|
Superseded by the current head de0cea0. The requested change was generated for old head ebca495 because the then-current central coverage-evidence workflow failed; the central workflow was subsequently repaired. The current head is a no-tree-change merge refresh (compare shows zero changed files versus ebca495), and exact-tree Semgrep and Security Scan evidence on ebca495 passed. Fresh current-head checks remain queued for revalidation.
Refresh both builder and runtime python:3.14-slim image digests to d3400aa. The branch was integrated with current main and the effective diff was verified as Dockerfile-only. The stale old-head review referenced Semgrep findings resolved by #16; fresh current-head security workflows were triggered and remained in the repository Actions queue.
Refresh the immutable Rust toolchain image digest to 6d220bf. The branch was integrated with current main and the effective diff verified as a single Dockerfile digest replacement. The stale old-head review referenced repository Semgrep findings resolved by #16; fresh current-head security workflows were triggered and remained queued.
Update the immutable uv image from 0.11.28 to 0.12.0. The existing-project Docker workflow uses supported sync flags, and the lockfile contains no source archives rejected by uv 0.12. The current-main effective diff is a single Dockerfile tag/digest replacement. The stale old-head review referenced Semgrep findings resolved by #16; fresh current-head security workflows were triggered and remained queued.
2 participants
Why
The central
Semgrep (multi-language SAST)required gate (semgrep scan --config=p/default --severity=WARNING --severity=ERROR --error) reports 15 Medium+ findings on the base code and fails, blocking the open dependabot PRs #10, #11, #12, #13 (and the newer #14/#15) in the merge queue.Every one of the 15 findings is a reviewed false positive or an intentional, default-secure design. This PR annotates each with the sanctioned
# nosemgrepsuppression the gate already supports — its suppression step stripsnosemgrep-annotated findings from the SARIF before evaluating the result. The gate itself is not weakened, disabled, or edited, and no genuine finding is suppressed.Pattern mirrors the proven approach used in
contextual-orchestrator#75: a# nosemgrep -- <rule>: <justification>comment on the exact reported line, placed after any pre-existing# nosecso Bandit's marker stays first.Findings suppressed (all reviewed safe)
pg_llm_batch/config.py— SQL rules (sqlalchemy-execute-raw-queryERROR +formatted-sql-queryWARNING)Reported on the
cur.execute(calls at lines 158, 172, 189, 201, 219, 287, 320. In every one, the only value interpolated into the f-string is the fixed class constantTABLE_NAME("com_config"/"com_secrets") — a hard-coded identifier that never derives from user input — and all data values are bound via%sDB-API placeholders. No string concatenation of untrusted input occurs. Textbook false positive for parameterized queries with a constant table name.pg_llm_batch/config.py:301—python-logger-credential-disclosure(WARNING)The message template contains the word "secret", which trips the heuristic, but the only logged argument is the literal mask
"***"— the code deliberately logs a mask precisely to avoid leaking. No secret value is logged.pg_llm_batch/token_counter.py:119, 149—python-logger-credential-disclosure(WARNING)modelname (e.g. an encoding identifier) at debug level — not a credential.exc) at debug level — not a credential.docker/postgres/Dockerfile—missing-user-entrypoint/missing-user(ERROR)Reported on the
ENTRYPOINT/CMDof thepostgres-baseintermediate build stage, which is never run directly. Both deployable targets that consume it —with-tiktokenandruntime— end with an explicitUSER postgres. The suppressions are placed on the preceding line because exec-form (JSON-array)ENTRYPOINT/CMDcannot carry an inline#comment without Docker folding it into the instruction and breaking the build; Semgrep honoursnosemgrepon the line directly above a finding.No genuine issues found
All 15 findings were individually reviewed; none is a real vulnerability (no untrusted input reaches a sink, no insecure default), so nothing is flagged for follow-up.
Verification
python -m py_compile pg_llm_batch/config.py pg_llm_batch/token_counter.py— passes.semgrep1.172.0 locally against the edited files; every annotated line is markedsuppressions: [{ "kind": "inSource" }]in the SARIF, including the two Dockerfile lines via the preceding-line placement. (The orgp/defaultruleset itself couldn't be fetched from the sandbox — registry egress is blocked — so an equivalent local rule set matching the same lines was used to confirm the suppression mechanism.)Unblocks pg-llm-batch #10–#13.
Generated by Claude Code