Uh oh!
There was an error while loading. Please reload this page.
ci: harden GitHub Actions against supply chain attacks - #1722
Conversation
- Pin all third-party actions to immutable commit SHAs - Add top-level permissions: contents: read to all workflows - Add StepSecurity Harden Runner (egress-policy: audit) to every job - Add SocketDev/action (firewall-free) + sfw install wrapper to tron-smart-contracts jobs - Pin github/codeql-action/upload-sarif to SHA (runs with security-events: write) ClosesRequestNetwork/private-issues#282
Greptile SummaryThis PR hardens seven GitHub Actions workflows against supply chain attacks by pinning all third-party actions to immutable commit SHAs, adding StepSecurity Harden Runner to every job, applying the SocketDev firewall to package install steps, and migrating
Confidence Score: 4/5Safe to merge for most jobs, but three workflows still forward PATs/tokens to externally-owned reusable workflows referenced by a mutable branch tag. The bulk of the hardening is solid — direct action pins, Harden Runner, SocketDev firewall, and the script-injection fixes are all correct. The remaining gap is that
|
| Filename | Overview |
|---|---|
| .github/workflows/pr-comments.yml | Adds top-level permissions block, but the reusable workflow call to RequestNetwork/auto-comments@main remains unpinned; highest-risk file due to pull_request_target + forwarded PAT. |
| .github/workflows/auto-project.yml | Adds permissions: contents: read; reusable workflow call to RequestNetwork/.github@main with PROJECT_TOKEN is still mutable. |
| .github/workflows/reopen-issue-if-prs-open.yml | Adds permissions: contents: read; reusable workflow call to RequestNetwork/.github@main with REOPEN_ISSUES_TOKEN remains pinned to a mutable branch. |
| .github/workflows/auto_assign_pr.yml | Harden Runner added and kentaro-m/auto-assign-action pinned to commit SHA; permissions block was already present. |
| .github/workflows/security-echidna.yml | All actions pinned to commit SHAs, Harden Runner added, issues: write permission added, github-script steps migrated from inline context interpolation to env vars. |
| .github/workflows/security-slither.yml | All actions pinned to commit SHAs including codeql-action/upload-sarif, Harden Runner added, github-script steps migrated to env vars; security-events: write was already present. |
| .github/workflows/tron-smart-contracts.yml | All actions pinned, Harden Runner added to all three jobs, SocketDev firewall applied to install steps, top-level permissions: contents: read added. |
Comments Outside Diff (1)
.github/workflows/pr-comments.yml, line 14-19 (link)Reusable workflow still referenced via mutable
@maintagThis workflow uses
pull_request_target(giving it write access to the base repo, even for fork PRs) and passesGH_PAT_AUTO_COMMENTSto an external reusable workflow pinned only at@main. IfRequestNetwork/auto-commentsmain branch is ever compromised, malicious workflow code runs with PR write access and the PAT. The same pattern exists inauto-project.yml(callsRequestNetwork/.github@mainwithPROJECT_TOKEN) andreopen-issue-if-prs-open.yml(same repo withREOPEN_ISSUES_TOKEN). All three need to be pinned to a commit SHA instead of@mainto close the gap this PR aims to address.
Reviews (4): Last reviewed commit: "fix(ci): address PR review comments on s..." | Re-trigger Greptile
✅ Slither Security AnalysisStatus: Passed Findings Summary
📄 Full report available in workflow artifacts. |
✅ Echidna Fuzzing ResultsMode: ci (50000 test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
The nightly failure-alert step calls github.rest.issues.create() to notify the team when Echidna properties fail. Without issues: write the call silently returns a 403 and the alert is never created.
✅ Slither Security AnalysisStatus: Passed Findings Summary
📄 Full report available in workflow artifacts. |
✅ Echidna Fuzzing ResultsMode: ci (50000 test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
The previous CircleCI failures on this PR were caused by the RequestNetwork CircleCI org dropping to the Free plan, which caps Docker resource classes at large. The repo's .circleci/config.yml declares xlarge for build/test jobs (deliberate; see #1703), so every build failed with resource-class-not-in-plan. Org was upgraded to Performance; this empty commit re-triggers the pipeline. No source changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Slither Security AnalysisStatus: Passed Findings Summary
📄 Full report available in workflow artifacts. |
✅ Echidna Fuzzing ResultsMode: ci (50000 test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
MantisClone
left a comment
There was a problem hiding this comment.
Approved 👍 pending comment resolution 🚧
Mostly clean; one bug and one scope gap to address before merge.
🔴 Must-fix before merge
github/codeql-action/upload-sarif@5e316336… does not exist in github/codeql-action. Verified three ways: 422 on direct commit lookup, no ref points at it, not in the tag list. The step is if: always() && hashFiles(...) != '' and Slither emits a SARIF every run, so it fails the SARIF upload (and the Slither job) on every contract PR. This is also the exact action #282 called out as highest priority.
Fix: github/codeql-action/upload-sarif@9e0d7b8d25671d64c341c19c0152d693099fb5ba # v4.35.5.
🟡 Scope gap
security-echidna.yml and security-slither.yml both run yarn install --frozen-lockfile unprotected by Socket Firewall. #282 task 4 said Socket on the install step across CI workflows; the PR only wraps the three Tron install steps. Either add sfw to the two security workflows or call out the rationale in the description.
🔵 Nice-to-have before merge
- Bump
# v2/# v4/# v7comments to exact-version (e.g.# v2.11.1,# v4.4.0) so the next reviewer can verify by eye. The floating major tags have already moved past several of the pinned SHAs. - Lift the dedicated
fix(ci): add issues: write…commit subject into the PR description so the permission expansion is documented in the body, not just in git log.
📋 UAT
PR is missing a UAT procedure. Suggested shape:
- After merge, push any contract change to master to trigger Slither, Echidna, and the three Tron jobs.
- Verify each job has Harden Runner as step 1 and (Tron only)
sfw yarn install --frozen-lockfile. - Open https://app.stepsecurity.io and confirm the audit-mode log shows an egress list on each run.
- Confirm the Slither SARIF upload to the Security tab succeeds (this is the failure mode of the must-fix above).
📌 Follow-ups (separate issues, not blockers)
Fresh-eyes pass surfaced things outside this PR's scope. Happy to file these as private-issues if you want:
- Add
Slither Static Analysis,Echidna Property-Based Fuzzing, and the three Tron checks tomaster's required status checks. Currently only CircleCI gates merge. - Enable
sha_pinning_required: trueat the repo or org level so the pinning policy is regression-proof. - Pin
actions/github-script@v6insideRequestNetwork/auto-comments/.github/workflows/pr-auto-comments.yml. That reusable workflow runs underpull_request_targetwithGH_PAT_AUTO_COMMENTS, so the supply-chain threat model leaks one hop in. - Pin the three first-party
@mainreusable workflows (auto-project.yml,pr-comments.yml,reopen-issue-if-prs-open.yml) to SHAs or document accepting the residual risk. - Add
.github/CODEOWNERSrequiring security-team review on.github/workflows/**. - Add
.github/dependabot.yml. The push to this branch surfaced 166 vulns on master (4 critical / 82 high) which is a related theme. - Set
dismiss_stale_reviews: trueon master branch protection.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Fix invalid codeql-action/upload-sarif SHA (5e316336 -> 9e0d7b8d, v4.35.5)
- Add Socket.dev Firewall to security-slither and security-echidna yarn install steps
- Gate tronbox global install through sfw by swapping step order in tron-smart-contracts
- Use env: + process.env pattern in github-script steps to eliminate ${{ }} script injection✅ Slither Security AnalysisStatus: Passed Findings Summary
📄 Full report available in workflow artifacts. |
✅ Echidna Fuzzing ResultsMode: ci (50000 test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
Merge activity
|
Implements all GitHub Actions hardening from RequestNetwork/private-issues#282.
Changes
github/codeql-action/upload-sarifwhich runs withsecurity-events: writepermissions: contents: readadded to workflows that lacked it (security workflows already had correct permission blocks)egress-policy: audit)sfw yarn install --frozen-lockfile)Next steps after merge
egress-policy: audit→blockwith the actual allowlist