Skip to content

feat: add OpenGrep static analysis before review - #9

Merged
kacigaya merged 1 commit into
mainfrom
feat/sast-triage
Nov 3, 2026
Merged

kacigaya merged 1 commit into
mainfrom
feat/sast-triage

Conversation

@kacigaya

@kacigaya kacigaya commented Nov 3, 2026

Copy link
Copy Markdown
Owner

Summary

This PR adds OpenGrep static analysis (SAST) to CroCode reviews. The worker scans the changed files before the agent runs. The agent receives the results as hints. It confirms real problems and drops false positives.

This PR is stacked on #8. Merge #8 first. GitHub then retargets this PR to main.

How it works

  1. After checkout, the worker starts a separate sandbox job with --network none and no credentials.
  2. The job loads only the rule directories for the languages of the changed files. It always adds the generic rules, such as secret detection.
  3. The worker keeps results that touch an added line. It keeps at most 50, most severe first.
  4. The prompt lists them inside the untrusted data boundary. The agent checks each result and names the rule ID in each finding it confirms.
  5. The review stores the results and the scan status. The review page shows them in a new Static analysis tab.

A failed or timed-out scan does not fail the review. The agent then reviews without hints. A scan that finishes with errors has status partial, and the agent still gets its results.

Security

  • A pull request cannot hide code from the scan. The scan ignores .semgrepignore files and nosemgrep comments. I tested this live: without these flags a nosemgrep comment hid the result, and with them the result was found.
  • Changed file paths follow --, so a file name cannot become a scan option.
  • The worker never stores or prompts with the matched code, so a detected secret does not leak.
  • Result writes check the worker lease, so a stale worker cannot overwrite another attempt's results.

Image

  • OpenGrep 1.33.1 (LGPL-2.1) and opengrep-rules at commit f1d2b56, pinned by SHA-256. The binary hashes match GitHub's published asset digests.
  • The build keeps only rule files from 26 language directories, 1,896 files in total. It drops test fixtures and tooling.
  • OpenGrep is a Nuitka onefile binary. The build unpacks it into /opt/opengrep-cache, so scans keep the read-only root and the noexec temp directories.

License: opengrep-rules uses LGPL-2.1 with the Commons Clause. The clause does not grant the right to sell a product or service whose value derives mainly from the rules. docker/README.md explains the impact and the opt-outs: CROCODE_SAST=off, or another rule set through CROCODE_SAST_RULES.

Configuration

Setting Default
CROCODE_SAST on
CROCODE_SAST_TIMEOUT_SECS 300
CROCODE_SAST_RULES /opt/opengrep-rules
.crocode.yml sast true

Migration 0005_sast.sql adds reviews.sast_status and the sast_results table.

Validation

  1. cargo fmt --all --check, cargo clippy --all-targets -- -D warnings
  2. cargo test --all -- --include-ignored on Postgres 17: 20 core and 69 server tests passed. New tests cover rule selection, argument safety, result parsing and filtering, long matches, report errors, the offline network flag, the prompt section, the repo setting, storage, the lease guard, and the retry reset.
  3. In web/: lint, typecheck, bun test, build, docs export, and the smoke test, which covers the Static analysis tab and path escaping.
  4. Live checks with the built image under runsc, read-only root, --cap-drop=ALL, and no network: the worker's exact scan command found 3 known issues with 0 errors.

Known limits

  • A scan of one or two languages takes about 30 s with runc and about 90 s with runsc. Most of the time goes to loading rules. The docs give the opt-outs.
  • In host mode (CROCODE_SANDBOX=none), the scan cannot be blocked from the network, and opengrep must be on PATH.
  • Community rules can be noisy. The agent triage reduces noise, but it depends on the model.

Copilot AI balanced review requested due to automatic review settings November 3, 2026 09:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@kacigaya
kacigaya changed the base branch from feat/review-controls-health-digest to main November 3, 2026 09:28
- Scan changed files with OpenGrep in an offline sandbox job with no
  credentials. Load only the rule dirs for the changed languages plus
  the generic rules.
- Pass results on added lines to the agent as untrusted hints. The
  agent confirms real issues and drops false positives.
- Ignore .semgrepignore files and nosemgrep comments, so a PR cannot
  hide code. Never store matched code.
- Bundle OpenGrep 1.33.1 and opengrep-rules f1d2b56, pinned by
  SHA-256. Unpack the onefile binary at build time, so scans keep the
  read-only, noexec sandbox.
- Store results and status per review, guarded by the worker lease.
  Show them in a Static analysis tab.
- Add CROCODE_SAST, CROCODE_SAST_TIMEOUT_SECS, CROCODE_SAST_RULES, and
  the sast repo setting.
@kacigaya
kacigaya merged commit 1115799 into main Nov 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants