Skip to content

Commit ecfeb5a

Browse files
committed
fix(ci): repair Slither SARIF upload for Code Scanning checks
- Stop continue-on-error on slither-clean (avoid upload without valid SARIF) - Upload via upload-sarif@v4, path from slither action outputs, fetch-depth 0, checkout v6 - Bump crytic/slither-action to v0.4.1; guard hacks artifact on file existence - CI.md: accurate jobs, Code Scanning troubleshooting; PROGRESS update Made-with: Cursor
1 parent 3e6cf19 commit ecfeb5a

3 files changed

Lines changed: 34 additions & 26 deletions

File tree

.github/workflows/slither.yml

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ jobs:
2525

2626
steps:
2727
- name: Checkout repository
28-
uses: actions/checkout@v4
28+
uses: actions/checkout@v6
29+
with:
30+
fetch-depth: 0
2931

3032
- name: Install Foundry toolchain
3133
uses: foundry-rs/foundry-toolchain@v1
@@ -35,21 +37,22 @@ jobs:
3537
- name: Install Forge dependencies
3638
run: forge install
3739

40+
# fail-on: none so findings never fail the step; do NOT use continue-on-error here — if Slither
41+
# crashes, we must skip SARIF upload or GitHub's "Code scanning results / Slither" check breaks.
3842
- name: Run Slither on clean contracts
39-
uses: crytic/slither-action@v0.4.0
43+
uses: crytic/slither-action@v0.4.1
4044
id: slither-clean
41-
continue-on-error: true # informational — teaching repo uses intentional patterns
4245
with:
4346
# slither --filter-paths expects a regex, not a comma-separated list
4447
slither-args: --filter-paths "(src/hacks|lib|test|script)(/|$)"
4548
sarif: slither-clean.sarif
4649
fail-on: none
4750

4851
- name: Upload SARIF to GitHub Code Scanning
49-
uses: github/codeql-action/upload-sarif@v3
50-
if: always()
52+
uses: github/codeql-action/upload-sarif@v4
53+
if: success() && hashFiles('slither-clean.sarif') != ''
5154
with:
52-
sarif_file: slither-clean.sarif
55+
sarif_file: ${{ steps.slither-clean.outputs.sarif }}
5356
category: slither-clean
5457

5558
# ── Job 2: Scan hacks/ (expected vulnerabilities) ───────────────────────────
@@ -62,7 +65,9 @@ jobs:
6265

6366
steps:
6467
- name: Checkout repository
65-
uses: actions/checkout@v4
68+
uses: actions/checkout@v6
69+
with:
70+
fetch-depth: 0
6671

6772
- name: Install Foundry toolchain
6873
uses: foundry-rs/foundry-toolchain@v1
@@ -73,7 +78,7 @@ jobs:
7378
run: forge install
7479

7580
- name: Run Slither on hacks/ (informational only)
76-
uses: crytic/slither-action@v0.4.0
81+
uses: crytic/slither-action@v0.4.1
7782
id: slither-hacks
7883
continue-on-error: true
7984
with:
@@ -84,7 +89,8 @@ jobs:
8489

8590
- name: Upload hacks SARIF as artifact
8691
uses: actions/upload-artifact@v4
87-
if: always()
92+
if: always() && hashFiles('slither-hacks.sarif') != ''
8893
with:
8994
name: slither-hacks-sarif
9095
path: slither-hacks.sarif
96+
if-no-files-found: ignore

CI.md

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ Three GitHub Actions workflows run on every push and pull request:
1111
| Workflow | File | Purpose | Blocks PRs? |
1212
|----------|------|---------|-------------|
1313
| **Forge Tests** | `forge-tests.yml` | Run all 922 Foundry tests | ✅ Yes |
14-
| **Slither Analysis** | `slither.yml` | Static analysis for vulnerabilities | ✅ Yes (clean code only) |
14+
| **Slither Analysis** | `slither.yml` | Static analysis → SARIF / Code Scanning | ✅ Yes — if Slither or `forge install` fails |
1515
| **Coverage** | `coverage.yml` | Test coverage report → Codecov | ❌ No (informational) |
1616

1717
---
@@ -47,32 +47,33 @@ This repository has a `hacks/` folder containing **intentionally vulnerable cont
4747

4848
Two jobs solve this cleanly:
4949

50-
### Job 1 — `slither-clean` (blocks PRs)
50+
### Job 1 — `slither-clean` (SARIF → Code Scanning)
5151

52-
Scans all production-quality contracts:
53-
- `src/basic/`
54-
- `src/applications/`
55-
- `src/defi/`
56-
- `src/evm/`
52+
Scans all production-quality contracts (excludes `src/hacks/`, `lib/`, `test/`, `script/` via `--filter-paths`):
5753

58-
Configured with `fail-on: high` — any high-severity finding **blocks the PR**.
54+
- `src/basic/`, `src/applications/`, `src/defi/`, `src/evm/`
5955

60-
Results are uploaded as SARIF and visible in **GitHub → Security → Code Scanning**:
61-
- Inline annotations on the affected lines
62-
- Persistent alert history
63-
- Severity filtering (high / medium / low / informational)
56+
Uses **`fail-on: none`** so Slither findings do not fail the step; reviewers use **GitHub Advanced Security** comments and the Security tab. The **job** still fails if compilation/analysis breaks (e.g. bad Solidity, `forge install` failure).
6457

65-
### Job 2 — `slither-hacks` (never blocks)
58+
SARIF upload follows [crytic/slither-action](https://github.com/crytic/slither-action): use the action’s **`outputs.sarif`** path with `github/codeql-action/upload-sarif`, and **do not** combine `continue-on-error` on the Slither step with an unconditional upload — that can yield a failed **Code scanning results / Slither** check (e.g. missing or invalid upload).
6659

67-
Scans only `src/hacks/` with `continue-on-error: true` and `fail-on: none`.
60+
Results appear under **GitHub → Security → Code Scanning** (category `slither-clean`).
6861

69-
Purpose: document that vulnerabilities **are** intentionally present. If someone accidentally fixes a vulnerability in `hacks/`, the finding disappears from the report — a useful signal that the educational example has been compromised.
62+
### Job 2 — `slither-hacks` (artifact only, non-blocking)
7063

71-
Results are uploaded to a separate SARIF category (`slither-hacks`) so they don't pollute the clean code findings.
64+
Scans roughly `src/hacks/` by filtering out the other `src/*` trees. Uses `continue-on-error: true` and `fail-on: none` so intentional vulnerable demos do not break CI.
65+
66+
The SARIF file is attached as a **workflow artifact** (`slither-hacks-sarif`) for inspection — it is **not** uploaded to Code Scanning (keeps hacks findings out of the main alert stream).
67+
68+
### Troubleshooting: “Code scanning results / Slither” / configuration issues
69+
70+
1. Open the **Slither — clean contracts** job log: confirm Slither finished and `slither-clean.sarif` exists.
71+
2. Under **Settings → Code security and analysis → Code scanning**, ensure you do not have a **stale** third-party configuration pointing at a deleted workflow; use the [tool status](https://docs.github.com/en/code-security/code-scanning/managing-code-scanning-for-your-repository/about-the-tool-status-page) page if available.
72+
3. For **fork PRs**, `security-events: write` may be restricted; uploads only run reliably for PRs from the same repository unless you adopt a [fork-safe pattern](https://docs.github.com/en/code-security/code-scanning/troubleshooting-code-scanning/resource-not-accessible).
7273

7374
### Viewing results
7475

75-
**GitHub Security → Code Scanning alerts**
76+
**GitHub Security → Code Scanning alerts**
7677
`https://github.com/KaelSensei/solidity-dojo/security/code-scanning`
7778

7879
---

PROGRESS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ A hands-on Solidity training ground based on solidity-by-example.org.
99
- [x] **Naming / NatSpec cleanup (2026-04)**: After Slither-oriented renames, fixed shadowing bugs (`value = value`, `num = num`), aligned bodies with new parameter names, removed drift-prone `/// @param` lines under `src/` (excluding `src/hacks/`), added `scripts/strip_natspec_params.py`, and updated tests (`ERC20Permit`, `GasGolf`, `Immutable`). Full suite green via `forge test` in Docker.
1010
- [x] **Slither / Code Scanning (zero address)**: Added `require(... != address(0), "Zero address")` on low-level call entry points flagged in PR review (`Call`, `SendingEther`, `TryCatch`, `Payable.withdrawTo`, `Immutable` constructor, `Delegatecall` proxy + demo), with matching revert tests.
1111
- [x] **Codecov**: Upgraded `codecov/codecov-action` to v5, added root `codecov.yml` (informational checks, PR comment layout), documented installing the [Codecov GitHub App](https://github.com/marketplace/codecov) in `CI.md` for reliable uploads and PR comments.
12+
- [x] **Code Scanning / Slither workflow**: Fixed SARIF upload (use `steps.slither-clean.outputs.sarif`, `upload-sarif@v4`, `fetch-depth: 0`, remove `continue-on-error` on clean scan so failed runs do not upload empty SARIF; `crytic/slither-action@v0.4.1`). Corrected `CI.md` (hacks job = artifact only; accurate fail-on / troubleshooting).
1213

1314
### ✅ Phase 1: Project Setup
1415
- [x] Copy training documentation to repo

0 commit comments

Comments
 (0)