Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions .github/workflows/diffguard.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
name: diffguard

on:
pull_request:
branches:
- '**'
types: [opened, synchronize, reopened]

concurrency:
group: diffguard-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true

jobs:
diffguard:
name: Quality metrics
runs-on: ubuntu-latest
Comment thread
Copilot marked this conversation as resolved.
steps:

Check warning on line 17 in .github/workflows/diffguard.yml

View check run for this annotation

Claude / Claude Code Review

diffguard.yml missing least-privilege permissions block

The new `diffguard.yml` workflow does not declare a `permissions:` block, while the two sibling workflows added in this PR (`nightly-govulncheck.yml` and `nightly-sonar.yml`, both at lines 16-17) explicitly declare `permissions: contents: read`. Adding `permissions: contents: read` at the job level (between `runs-on` and `steps`) would match the pattern established by the other new workflows and follow least-privilege defaults.
Comment on lines +14 to +17

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 The new diffguard.yml workflow does not declare a permissions: block, while the two sibling workflows added in this PR (nightly-govulncheck.yml and nightly-sonar.yml, both at lines 16-17) explicitly declare permissions: contents: read. Adding permissions: contents: read at the job level (between runs-on and steps) would match the pattern established by the other new workflows and follow least-privilege defaults.

Extended reasoning...

What the finding is

.github/workflows/diffguard.yml lines 13-17 declare the job as follows:

jobs:
  diffguard:
    name: Quality metrics
    runs-on: ubuntu-latest
    steps:

There is no permissions: key at either the workflow or job level. The two sibling workflows introduced in this same PR both declare it:

  • nightly-govulncheck.yml lines 16-17: permissions: contents: read
  • nightly-sonar.yml lines 16-17: permissions: contents: read

A repo-wide scan shows the pattern is established elsewhere too (codeql.yml, kurtosis-e2e.yml, release_ghcr.yml, stale.yml, mainnet_deb_profiles.yml, packager_deb.yml), so diffguard.yml is the outlier both within this PR and within the repo.

Why it matters

Without an explicit permissions: block, the workflow inherits whatever the repository/org default GITHUB_TOKEN permission set is. Depending on org policy, that default can be broader than contents: read (e.g. the legacy write-all default). Combined with the unpinned go install github.com/0xPolygon/diffguard/cmd/diffguard@latest on line 36 — which the author committed to pinning to SHA 527d7fd on 2026-06-24 but the current diff still shows @latest — a compromised diffguard release could exercise whatever the default token grants.

Realized impact

Small. This workflow triggers on pull_request (not pull_request_target), so for fork PRs GitHub already restricts GITHUB_TOKEN to read-only. The steps themselves only read source, write to $GITHUB_STEP_SUMMARY, and upload an artifact — none require elevated permissions. So today there is no active vulnerability. The concern is defense-in-depth and consistency with the pattern the PR itself is establishing.

Step-by-step example

  1. A malicious release of 0xPolygon/diffguard is published (or the @latest install pulls in a compromised transitive Go module).
  2. Diffguard runs during a synchronize event on an intra-repo PR (not a fork).
  3. GITHUB_TOKEN is available in the environment with whatever the repo default is. If that default is write-all (still the legacy default in some orgs), the malicious binary can call the GitHub API to comment on issues, push to branches, create releases, etc.
  4. Had permissions: contents: read been declared at the job level, the token would be scoped down to read-only regardless of repo-wide default, containing the blast radius.

Suggested fix

Add two lines between runs-on: ubuntu-latest and steps: to match the sibling workflows:

  diffguard:
    name: Quality metrics
    runs-on: ubuntu-latest
    permissions:
      contents: read
    steps:

- uses: actions/checkout@v6
with:
# diffguard needs full history for merge-base and file history checks.
fetch-depth: 0

- uses: actions/setup-go@v6
with:
go-version-file: go.mod

- uses: actions/cache@v4
with:
path: |
~/.cache/go-build
~/go/pkg/mod
key: ${{ runner.os }}-diffguard-${{ hashFiles('**/go.sum') }}
restore-keys: ${{ runner.os }}-diffguard-

- name: Install diffguard
run: go install github.com/0xPolygon/diffguard/cmd/diffguard@latest
Comment on lines +35 to +36
Comment thread
vbhattaccmu marked this conversation as resolved.
Comment thread
vbhattaccmu marked this conversation as resolved.
Comment on lines +35 to +36

- name: Run diffguard
id: diffguard
shell: bash
run: |
# Keep diffguard's exit code when tee writes the step summary.
set -o pipefail
diffguard \
--base origin/${{ github.base_ref }} \
--complexity-threshold 15 \
--complexity-delta-tolerance 5 \
--function-size-threshold 80 \
--function-size-delta-tolerance 15 \
--file-size-threshold 800 \
--file-size-delta-tolerance-pct 10 \
--file-size-delta-tolerance-floor 20 \
--tier1-threshold 80 \
--tier2-threshold 60 \
--mutation-sample-rate 20 \
--test-timeout 5m \
--output text \
--fail-on error \
. | tee "$GITHUB_STEP_SUMMARY"

- name: Upload JSON report
# The text report above contains mutation results; this second run is
# only for a cheap machine-readable shape report.
if: always()
run: |
diffguard \
--base origin/${{ github.base_ref }} \
--skip-mutation \
--output json \
--fail-on none \
. > diffguard-report.json

- uses: actions/upload-artifact@v6
if: always()
with:
name: diffguard-report
path: diffguard-report.json
retention-days: 14
24 changes: 0 additions & 24 deletions .github/workflows/govulncheck.yml

This file was deleted.

38 changes: 38 additions & 0 deletions .github/workflows/nightly-govulncheck.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
name: Nightly Govulncheck

on:
schedule:
- cron: '0 2 * * *'
workflow_dispatch:

Check warning on line 6 in .github/workflows/nightly-govulncheck.yml

View check run for this annotation

Claude / Claude Code Review

govulncheck moved to nightly-only removes pre-merge dependency-CVE gate

This PR deletes `.github/workflows/govulncheck.yml` (which ran `make vulncheck` on every push/PR gated on Go/`go.mod`/`go.sum`/`Makefile` changes) and replaces it with a nightly-only scan on `develop`. A PR that adds or bumps a dependency with a known Go CVE will now merge without any govulncheck gate — the finding only surfaces up to ~24h later against `develop`, after the vulnerable code has already landed. This weakens the pre-merge check the repo's own `.claude/rules/security.md` calls out u
Comment on lines +3 to +6

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 This PR deletes .github/workflows/govulncheck.yml (which ran make vulncheck on every push/PR gated on Go/go.mod/go.sum/Makefile changes) and replaces it with a nightly-only scan on develop. A PR that adds or bumps a dependency with a known Go CVE will now merge without any govulncheck gate — the finding only surfaces up to ~24h later against develop, after the vulnerable code has already landed. This weakens the pre-merge check the repo's own .claude/rules/security.md calls out under Dependency Security; consider keeping a lightweight PR-triggered make vulncheck (e.g. when go.mod/go.sum change) alongside the nightly develop scan.

Extended reasoning...

What changed. The old .github/workflows/govulncheck.yml was triggered on push and pull_request and used technote-space/get-diff-action to gate on changes to **/*.go, go.mod, go.sum, or Makefile; when any of those changed, it ran make vulncheck. This PR deletes that workflow and adds .github/workflows/nightly-govulncheck.yml, which only runs on schedule: '0 2 * * *' (plus manual workflow_dispatch) and always checks out develop.\n\nWhy the gate is gone. I grepped the rest of .github/workflows/ for any remaining PR-time govulncheck coverage: only nightly-govulncheck.yml references govulncheck/vulncheck, ci.yml builds/lints/tests but does not call make vulncheck, and the Makefile vulncheck target still exists but has no invoker on PRs. codeql.yml does run on PRs, but CodeQL is a semantic-analysis tool for CWE-style patterns; it does not consult the Go vulnerability database (golang.org/x/vuln) and will not flag an imported module version that matches a listed CVE, so it does not fill this gap.\n\nStep-by-step: how a vulnerable dependency now lands on develop.\n1. Author opens a PR that either (a) adds import "github.com/example/foo" where foo@vX.Y.Z has a listed vulnerability, or (b) bumps an existing dependency in go.mod/go.sum to a CVE-affected version.\n2. ci.yml runs build/lint/test/codecov. None of them run govulncheck; CodeQL's PR run will not match against golang.org/x/vuln.\n3. diffguard.yml runs quality metrics (complexity, size, mutation) — not a vulnerability scan.\n4. All required checks pass, the PR is merged into develop.\n5. Up to ~24 hours later, at 02:00 UTC, nightly-govulncheck.yml checks out develop and runs make vulncheck — the vulnerable module is now already in the tree.\n6. Feature branches (release/*, main, hotfix/*) get zero govulncheck coverage until the fix is cherry-picked or merged back into develop.\n\nAlignment with the repo's own policy. .claude/rules/security.md under "Dependency Security" explicitly lists "Run govulncheck ./... to check for known vulnerabilities" as a pre-commit / pre-PR requirement, and "Dependency updates (especially forked cosmos-sdk, cometbft, bor)" appears in the "When to Trigger Security Review" section. Moving govulncheck to nightly-only weakens the specific dependency-CVE gate the security guide names.\n\nAcknowledging the trade-off. The PR description explicitly frames this shift as intentional — the Go bump from 1.26.3 → 1.26.4 was driven by stdlib govulncheck findings, and running on PRs means unrelated PRs get blocked whenever a new stdlib CVE lands until a Go patch bump happens. That is a real papercut and nightly-on-develop does provide within-24h catch-up for stdlib drift. So this isn't necessarily wrong — hence nit.\n\nSuggested fix. Keep the nightly develop scan for stdlib CVE drift (it's genuinely useful for that), and re-add a lightweight PR-triggered govulncheck that only runs when go.mod/go.sum change (i.e. the paths-filter case where new dependencies are actually being introduced). That preserves the pre-merge gate for the case the security policy actually cares about ("newly-introduced vulnerable deps") while avoiding the noise of unrelated PRs being blocked by stdlib findings.


concurrency:
group: nightly-govulncheck-develop
cancel-in-progress: true
Comment thread
Copilot marked this conversation as resolved.

jobs:
govulncheck:
name: Run govulncheck on develop
runs-on: ubuntu-latest
permissions:
contents: read

steps:
- uses: actions/checkout@v6
with:
ref: develop

- uses: actions/setup-go@v6
with:
go-version-file: go.mod
check-latest: true
Comment on lines +24 to +27

- uses: actions/cache@v4
with:
path: |
~/.cache/go-build
~/go/pkg/mod
key: ${{ runner.os }}-govulncheck-${{ hashFiles('**/go.sum') }}
restore-keys: ${{ runner.os }}-govulncheck-

- name: Run govulncheck
run: make vulncheck

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Where would we be notified about outcome? bor nightly race tests for example pushes the results on an internal slack channel. We might want to do the same for nightly checks (vulncheck and sonar)

51 changes: 51 additions & 0 deletions .github/workflows/nightly-sonar.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
name: Nightly Sonar

on:
schedule:
- cron: '0 1 * * *'
workflow_dispatch:

concurrency:
group: nightly-sonar-develop
cancel-in-progress: true
Comment thread
Copilot marked this conversation as resolved.

jobs:
sonar:
name: Run SonarCloud on develop
runs-on: ubuntu-latest
permissions:
contents: read
env:
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}

steps:
- uses: actions/checkout@v6
with:
fetch-depth: 0
ref: develop

- name: Run SonarCloud analysis
id: sonar
if: env.SONAR_TOKEN != ''
continue-on-error: true
uses: SonarSource/sonarqube-scan-action@c7ee0f9df90b7aa20e8dcf9695dcfe2e7da5b4f2 # v7
env:
GITHUB_TOKEN: ${{ github.token }}

- name: Report informational status
if: always()
run: |
{
echo "## Nightly Sonar"
case "${{ steps.sonar.outcome }}" in
success)
echo "SonarCloud analysis completed successfully."
;;
skipped)
echo "SonarCloud analysis was skipped because SONAR_TOKEN is not available."
;;
*)
echo "SonarCloud analysis did not complete successfully. This check is informational and does not fail CI."
;;
esac
} >> "$GITHUB_STEP_SUMMARY"
22 changes: 0 additions & 22 deletions .github/workflows/security-build.yml

This file was deleted.

Loading