Skip to content

feat(ci): govulncheck - add gVisor sandboxing via geomys/sandboxed-step#4598

Draft
kakkoyun wants to merge 6 commits intomainfrom
kakkoyun/govulncheck-sandboxed-step
Draft

feat(ci): govulncheck - add gVisor sandboxing via geomys/sandboxed-step#4598
kakkoyun wants to merge 6 commits intomainfrom
kakkoyun/govulncheck-sandboxed-step

Conversation

@kakkoyun
Copy link
Copy Markdown
Member

What does this PR do?

Wraps govulncheck execution steps in geomys/sandboxed-step@v1.2.1 to confine CI execution using gVisor syscall filtering.

This PR will fail CI because geomys/sandboxed-step is not in the DataDog enterprise action allowlist. This PR exists to start the discussion for adding it.

Motivation

Per Filippo Valsorda's article:

"Supply chain attacks have a short half-life! You can further mitigate the risk by using a CI sandboxing mechanism like geomys/sandboxed-step, which uses gVisor to remove the ambient authority that GitHub Actions grants every workflow, including supposedly read-only ones."

govulncheck runs go install and executes module code during analysis — if a dependency contains malicious code, it runs in CI with full ambient authority (network access, token access, filesystem access). The persist-credentials: false change in #4597 removes the checkout-persisted credential, but the runner still has network access and other ambient authority. gVisor sandboxing removes all of this.

To merge this PR: request adding geomys/sandboxed-step to the DataDog enterprise GitHub Actions allowlist.

What geomys/sandboxed-step provides

  • Syscall filtering: gVisor intercepts all syscalls, preventing unauthorized file access and process spawning
  • Network isolation: prevents exfiltration of secrets, tokens, or source code
  • Ambient authority removal: GitHub Actions grants every workflow ambient authority; the sandbox strips it

References

Reviewer's Checklist

  • N/A — CI/workflow-only change, no Go code modified

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.58%. Comparing base (31f8814) to head (f5cf6b9).

Additional details and impacted files

see 17 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@datadog-prod-us1-6
Copy link
Copy Markdown

datadog-prod-us1-6 bot commented Mar 25, 2026

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 60.05% (+3.91%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: aa79d34 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!

@kakkoyun kakkoyun changed the title ci(govulncheck): add gVisor sandboxing via geomys/sandboxed-step feat(ci): govulncheck - add gVisor sandboxing via geomys/sandboxed-step Mar 25, 2026
@kakkoyun kakkoyun added the AI Assisted AI/LLM assistance used in this PR (partially or fully) label Mar 25, 2026
@pr-commenter
Copy link
Copy Markdown

pr-commenter bot commented Mar 25, 2026

Benchmarks

Benchmark execution time: 2026-03-27 11:41:33

Comparing candidate commit aa79d34 in PR branch kakkoyun/govulncheck-sandboxed-step with baseline commit 54c1038 in branch kakkoyun/govulncheck.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 216 metrics, 8 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

@kakkoyun kakkoyun force-pushed the kakkoyun/govulncheck-persist-credentials branch from 34af022 to 31f8814 Compare March 26, 2026 10:07
@kakkoyun kakkoyun force-pushed the kakkoyun/govulncheck-sandboxed-step branch from 42b3d61 to f5cf6b9 Compare March 26, 2026 10:10
kakkoyun and others added 4 commits March 27, 2026 11:51
…mediation

Replace Dependabot Security Alerts with govulncheck-based scanning.
Dependabot flags every CVE in transitive deps regardless of reachability,
generating alert fatigue. govulncheck uses call graph analysis to only
report vulnerabilities in code paths that are actually called.

Changes:
- govulncheck.yml: add pull_request trigger (blocks PRs with reachable
  vulns), SARIF upload to GitHub Code Scanning via the official
  golang/govulncheck-action, always installs govulncheck@latest, adds
  concurrency group, pins all actions to latest SHA
- govulncheck-fix.yml: new weekly remediation workflow that scans all
  modules (core + 65+ contrib), identifies fixes, updates go.mod files,
  and opens a PR automatically
- govulncheck-fix.sh: remediation script that parses govulncheck JSON
  output, extracts module@fixedVersion pairs, runs go get + go mod tidy
  across all affected modules

Next step: disable Dependabot Security Alerts for Go in GitHub repo
settings once this merges (Settings > Code security > Dependabot alerts).
…tion

golang/govulncheck-action is blocked by the DataDog enterprise action
allowlist (it internally uses actions/checkout@v4.1.1 and
actions/setup-go@v5.0.0 which are not in the allowlist).

Replace with: setup-go + go install govulncheck@latest + govulncheck
-format sarif, which uses only GitHub-created actions (allowed).
…ntrib SARIF gap

Add go.work and go.work.sum to pull_request.paths so workspace-level
dependency changes (replacements, workspace pins) trigger govulncheck
instead of silently bypassing it.

Add a TODO comment documenting that contrib module vulnerabilities are
caught by the blocking govulncheck-tests check but do not appear in
GitHub Code Scanning alerts — contrib SARIF upload is a follow-up
improvement requiring multi-SARIF tooling.

Addresses Codex review feedback on PR #4595.
…eps (#4597)

### What does this PR do?

Adds `persist-credentials: false` to all `actions/checkout` steps in the
govulncheck workflows (`govulncheck.yml` and `govulncheck-fix.yml`).

### Motivation

Per [Filippo Valsorda's
recommendation](https://words.filippo.io/dependabot/),
`actions/checkout` leaves a temporary GitHub token on the runner
filesystem by default. Any code executed in the job — including
`govulncheck` downloading the vuln DB, `go get` fetching modules, and
transitive dependency code — can read this token.

Adding `persist-credentials: false` revokes the token from the
filesystem immediately after checkout. The `govulncheck-fix.yml`
workflow still works correctly because its push step uses `gh auth
setup-git` with `GH_TOKEN` env var, which configures its own credential
helper independently of the checkout-persisted credential.

No existing workflow in this repo currently uses `persist-credentials:
false`. This change starts with the govulncheck workflows as a natural
follow-up to [#4595](#4595) —
the team can decide whether to adopt it repo-wide.

Technically we should adopt `persist-credentials: false` across all
workflows in this repo. See [actions/checkout
docs](https://github.com/actions/checkout#persist-credentials).

### Reviewer's Checklist

- [x] N/A — CI/workflow-only change, no Go code modified

### References

- [Filippo Valsorda: Turn Dependabot
Off](https://words.filippo.io/dependabot/) — section on supply chain
attack mitigation
- [actions/checkout:
persist-credentials](https://github.com/actions/checkout#persist-credentials)
— official docs
- [PR #4595](#4595) — parent
govulncheck migration PR
Base automatically changed from kakkoyun/govulncheck-persist-credentials to kakkoyun/govulncheck March 27, 2026 10:53
kakkoyun added a commit that referenced this pull request Mar 27, 2026
…#4605)

## Why

The `govulncheck-analysis` job (non-blocking SARIF upload) only scanned
core packages. Contrib module vulnerabilities were caught by
`govulncheck-tests` (blocking, sandboxed) but never appeared in the
GitHub Security tab / Code Scanning. This created a gap: a contrib CVE
would fail the PR check but leave no trace in the Security dashboard.

This PR closes that gap by adding a parallel non-blocking
`govulncheck-contribs-analysis` job that scans every contrib module and
uploads results to GitHub Code Scanning.

## What

- **New script**: `govulncheck-contribs-sarif.sh` — mirrors
`govulncheck-contribs-v2.sh` but uses `-format sarif`. It scans each
contrib module, writes a per-module SARIF file to a temp directory, then
merges all runs into a single output file via `jq`.
- **New job**: `govulncheck-contribs-analysis` — sets up Go, installs
govulncheck, runs the script, and uploads the merged SARIF under the
`govulncheck-contribs` category.
- **Updated comment** in `govulncheck-analysis`: replaces the `TODO`
with a reference to the new job.
- **Updated `paths` filter**: adds `govulncheck-contribs-sarif.sh` so
changes to the script trigger the workflow.

## Design decisions

**Why merge SARIF with `jq` instead of uploading per-module files?**
The `upload-sarif` action accepts a directory, but uploading N files
(one per contrib module) would create N separate tool runs in Code
Scanning, making the Security tab noisy. Merging runs into one SARIF
file keeps results grouped under a single `govulncheck-contribs`
category.

**Why not sandbox this job?**
The `govulncheck-contribs-analysis` job is non-blocking and
informational, consistent with `govulncheck-analysis`. The sandboxed
`govulncheck-tests` job provides the security boundary for blocking
checks.

## Stack

This PR is part of a stack:

```
main
 └── #4595 kakkoyun/govulncheck (base)
      └── #4597 kakkoyun/govulncheck-persist-credentials
           └── #4598 kakkoyun/govulncheck-sandboxed-step
                └── #4599 kakkoyun/govulncheck-action-allowlist
                     └── this PR kakkoyun/govulncheck-contrib-sarif
```

Merge order: #4595#4597#4598#4599 → this PR.

## Test plan

- [ ] CI passes on this branch (all three jobs green)
- [ ] `govulncheck-contribs-analysis` job appears in the Actions run
- [ ] SARIF upload succeeds (check `upload-sarif` step output)
- [ ] GitHub Security tab shows findings under `govulncheck-contribs`
category after merge to main
The contrib SARIF gap is addressed by govulncheck-contribs-analysis
(PR #4605), which scans each contrib module and uploads results to
GitHub Code Scanning under a distinct category.
…andboxing

Confines govulncheck execution using gVisor to prevent supply chain
attacks from exfiltrating tokens or making network calls in CI. Any
malicious code in a scanned dependency runs inside the sandbox with
no ambient authority.

This PR will fail CI: geomys/sandboxed-step is not in the DataDog
enterprise action allowlist. It exists to start the discussion for
adding it. See PR description for context.

Ref: https://words.filippo.io/dependabot/
Ref: https://github.com/geomys/sandboxed-step
@kakkoyun kakkoyun force-pushed the kakkoyun/govulncheck-sandboxed-step branch from f5cf6b9 to aa79d34 Compare March 27, 2026 11:13
Base automatically changed from kakkoyun/govulncheck to main March 27, 2026 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Assisted AI/LLM assistance used in this PR (partially or fully)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant