Skip to content

feat(ci): add Stage 1 static-analysis workflow for security review (EXP-480) [stack 3/7]#1885

Draft
gvladika wants to merge 2 commits into
pr/exp-479-audit-knowledge-corpusfrom
pr/exp-480-static-analysis-workflow
Draft

feat(ci): add Stage 1 static-analysis workflow for security review (EXP-480) [stack 3/7]#1885
gvladika wants to merge 2 commits into
pr/exp-479-audit-knowledge-corpusfrom
pr/exp-480-static-analysis-workflow

Conversation

@gvladika

@gvladika gvladika commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Which Linear task belongs to this PR?

EXP-480 — Stage 1 deterministic static-analysis workflow.

Why did I implement it this way?

PR 3 of 7 in the EXP-440 security-review stack. Builds on #1884 (EXP-479 corpus).

Adds .github/workflows/security-review.yml Stage 1 — three parallel jobs running on every non-draft PR touching src/** or audit/knowledge/**:

  • Slither (v0.11.3 via crytic/slither-action@v0.4.2) with --exclude-informational --exclude-low to drop pure-style detectors at the source. Outputs SARIF directly to Code Scanning + as an artifact for Stage 2.
  • Aderyn (aderyn-v0.6.8 from cyfrin's installer, run after forge build). Native SARIF output via --output X.sarif. Note: PR 6 (EXP-484) drops this job after dry-run evidence showed it adds only ~1 finding per PR beyond Slither, not worth the supply-chain attack surface.
  • Semgrep (v1.117.0) running only LI.FI's custom rules under audit/knowledge/semgrep/. Self-skips if the directory is empty.

Each job uses continue-on-error: true (advisory mode per EXP-480; EXP-485 will flip to gating). Action versions are SHA-pinned per [CONV:ACTIONS-IMMUTABLE]. Per-job permissions: contents: read + security-events: write (required by Code Scanning SARIF upload).

The workflow is concurrency-grouped per PR so new commits cancel stale in-flight runs.

Stage 2 (lifi-pr-review job) and Stage 3 (publish curated.sarif + sticky comment) land in PR 5 of this stack.

Stack context (this is PR 3 of 7)

PR 1: EXP-478 — /extract-audit-knowledge skill         #1883
PR 2: EXP-479 — corpus seed + backfill                 #1884
PR 3: EXP-480 — Stage 1 static-analysis workflow       ← this PR
PR 4: EXP-482 — noise baseline doc
PR 5: EXP-483 — lifi-pr-review skill + Stages 2/3
PR 6: EXP-484 — Slither + Semgrep tuning (drops Aderyn)
PR 7: EXP-481 — /add-audit chain + coverage check

/pr-ready notes

⚠️ CodeRabbit local review hit a rate limit after PRs 1+2 of this stack. Per pr-ready.md, surfacing rather than silently skipping. Used PR_READY_OK=1 bypass for this PR with the following compensating validation:

Will re-run /pr-ready once the CodeRabbit limit resets, before flipping this PR out of draft.

Checklist before requesting a review

  • I have performed a self-review of my code
  • This pull request is as small as possible and only tackles one problem
  • I have run /pr-ready (local CodeRabbit) on this branch — attempted; CodeRabbit rate-limited; will re-run before exiting draft. See note above. Author validated via actionlint + zizmor.
  • I have added tests that cover the functionality / test the bug — n/a, CI workflow; tested by 3 dry-runs (see PR 5 body)
  • For new facets: n/a, no facets
  • I have updated any required documentation

…XP-480)

Adds .github/workflows/security-review.yml that runs three SAST tools in
parallel on every non-draft PR touching src/** or audit/knowledge/**:

- Slither    (crytic/slither-action@v0.4.2) → slither.sarif
- Aderyn     (Cyfrin v0.6.8, installed at runtime) → aderyn.sarif
- Semgrep    (pip-installed v1.117.0) with audit/knowledge/semgrep/*.yml
             from the EXP-479 seed rules → semgrep.sarif

Each job uploads its SARIF to GitHub Code Scanning with a per-tool
category, so findings appear inline in the PR diff and aggregate in the
Security tab. Status check is published but NOT required (advisory only,
per EXP-480) — enforcement comes in EXP-485 after EXP-484 baseline tuning.

continue-on-error: true on all three so a single tool flake (pip rate
limit, slither dep failure, etc.) does not mask findings from the others.

Concurrency group dedupes in-flight runs per PR.

Per-job permissions: contents: read + security-events: write (scoped
narrowly per zizmor recommendation; workflow-level only has contents: read).

Validated locally:
- actionlint    → 0 findings
- zizmor 1.23.1 → 0 findings

Stacks on top of EXP-478 (skill) and EXP-479 (corpus + seed rules).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6f5971f2-6e97-4d44-9251-a6e9234b6f41

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pr/exp-480-static-analysis-workflow

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

PR-review feedback on #1885: Aderyn was included in the original
Stage 1 design but the dry-run measurements in EXP-483 (#1887)
showed it contributes ~1 finding per PR beyond Slither's coverage
on the same code. That marginal value did not justify the
supply-chain attack surface of Aderyn's installer step or the
extra CI job.

This commit removes the Aderyn job from .github/workflows/security-
review.yml on the same PR where it was originally introduced, so
PR #1885 ships the final Stage 1 design (Slither + Semgrep). The
EXP-484 follow-up PR (#1888) previously did this removal — it
becomes a no-op after this lands, which is the intended outcome
when stacked PRs merge in order.

If a second-opinion static-analysis pass is wanted later, it
belongs in EXP-486 (nightly deep scan), not per-PR Stage 1.

Validated:
- actionlint    → 0 findings
- zizmor 1.23.1 → 0 findings
- Workflow grep confirms no remaining Aderyn references
gvladika added a commit that referenced this pull request Jun 8, 2026
Follow-on to PR #1885's commit 041e524 which removed the Aderyn job
from Stage 1. PR 5's branch was created when PR 3 still had Aderyn, so
the branch state still contains:
  - The Aderyn job inherited from PR 3's original commit
  - Stage 2 `needs: [slither, aderyn, semgrep]` from cda4a89
  - aderyn.sarif references in the claude-code-action prompt
  - aderyn references in the lifi-pr-review skill input table + summary

This commit aligns PR 5's branch with the no-Aderyn end state shipped
in PR 3 (#1885). When the stack merges in order, PR 6's c263f42 commit
(which originally removed Aderyn) becomes a clean no-op — Aderyn is
already gone.

Files updated:
- .github/workflows/security-review.yml: header comment, full Aderyn
  job block, needs: list, claude-code-action prompt
- .agents/commands/lifi-pr-review.md: SARIF inputs table, summary
  template

Validated:
- actionlint    → 0 findings
- zizmor 1.23.1 → 0 findings
- markdownlint  → 0 errors on the skill file
- grep -i aderyn → no remaining references in workflow or skill
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.

1 participant