From 4e0c9adbfe8a237551295b8cfbc62ef9d2095f3a Mon Sep 17 00:00:00 2001 From: Rex Raphael Date: Thu, 13 Aug 2026 19:10:03 -0500 Subject: [PATCH 1/3] chore(security): fix reachable x/crypto ssh vulns, scan driver modules in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit govulncheck resolves imports per module, so the shared go-ci.yml security job — which runs at the repository root — never scanned the four driver sub-modules. Every third-party dependency the project ships lives there, so they were entirely unscanned while Dependabot reported 39 alerts against them. Scanning each module directly found five reachable advisories in golang.org/x/crypto/ssh from sftpdriver. Reachable, not merely present: Open calls ssh.Dial, ssh.ParsePrivateKey and sftp.NewClient directly, so the traces resolve to called symbols rather than being call-graph over-approximations. Bumped to v0.52.0, which clears all five. The go directive is unchanged — v0.52.0 needs only Go 1.25.0. The new CI step gates on reachable findings in third-party modules and reports standard-library findings without failing. A stdlib finding tracks the runner's Go patch release rather than anything here, so gating on it would red every branch whenever a new Go version lands. gcsdriver, s3driver and azuredriver still have reachable findings, all indirect. Rather than leave the new gate non-blocking, they are recorded in .github/govulncheck-allowlist.txt so the gate blocks anything new from today. That file is a backlog, not an exemption: removing a line is how a fix gets enforced, and adding one requires a reason. --- .github/govulncheck-allowlist.txt | 37 ++++++++++++++++++ .github/workflows/ci.yml | 62 +++++++++++++++++++++++++++++++ CHANGELOG.md | 9 +++++ drivers/sftpdriver/go.mod | 4 +- drivers/sftpdriver/go.sum | 12 +++--- 5 files changed, 116 insertions(+), 8 deletions(-) create mode 100644 .github/govulncheck-allowlist.txt diff --git a/.github/govulncheck-allowlist.txt b/.github/govulncheck-allowlist.txt new file mode 100644 index 0000000..5ba1639 --- /dev/null +++ b/.github/govulncheck-allowlist.txt @@ -0,0 +1,37 @@ +# Known-reachable dependency vulnerabilities in the driver sub-modules. +# +# The govulncheck step in ci.yml's `drivers` job fails on any reachable +# vulnerability in a third-party module that is NOT listed here. This file is +# therefore a backlog, not an exemption: an entry means "we know, it is +# tracked, it does not block the next unrelated PR" — never "this is fine". +# +# The gate exists to stop NEW findings landing. Removing a line is how you +# enforce a fix: bump the dependency, delete the entry, and the job goes green +# on its own. Adding a line requires a reason. +# +# Standard-library findings are reported but never gate, because they track +# the runner's Go patch release rather than anything in this repository — the +# toolchain moving would otherwise red every branch on a schedule nobody here +# controls. +# +# Format: +# +# Audited 2026-08-13 against govulncheck v1.6.0. sftpdriver is deliberately +# absent: its x/crypto/ssh findings were reachable through ssh.Dial and +# ssh.ParsePrivateKey in Open, and were fixed by bumping to v0.52.0 rather +# than being listed here. + +# gcsdriver — all indirect, via cloud.google.com/go/storage's dependency tree. +gcsdriver GO-2026-6061 # grpc: xDS RBAC + HTTP/2 server. Needs grpc v1.82.1. +gcsdriver GO-2026-4394 # otel/sdk: PATH hijacking → arbitrary code execution. +gcsdriver GO-2026-4918 # x/net: HTTP/2 infinite loop on bad SETTINGS_MAX_FRAME_SIZE. +gcsdriver GO-2026-5026 # x/net/idna: accepts ASCII-only Punycode labels. +gcsdriver GO-2026-5970 # x/text: infinite loop on invalid input. + +# s3driver — one finding, surfaced through two module paths. +s3driver GO-2026-5764 # aws eventstream decoder panic. Needs service/s3 v1.97.3. + +# azuredriver — all indirect, via azure-sdk-for-go's dependency tree. +azuredriver GO-2026-4918 # x/net: HTTP/2 infinite loop on bad SETTINGS_MAX_FRAME_SIZE. +azuredriver GO-2026-5026 # x/net/idna: accepts ASCII-only Punycode labels. +azuredriver GO-2026-5970 # x/text: infinite loop on invalid input. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 25567b9..8e9426f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,6 +69,68 @@ jobs: working-directory: drivers/${{ matrix.driver }} run: go test -race -count=1 ./... + # govulncheck resolves imports per module, so the shared go-ci.yml + # `security` job — which runs at the repository root — never sees these + # sub-modules at all. Every third-party dependency the project ships + # lives here, so without this step they were entirely unscanned. + # + # Module findings gate; standard-library findings only report. A stdlib + # finding tracks the runner's Go patch release rather than anything in + # this repository, so gating on it reds every branch whenever a new Go + # release lands, regardless of whether this code is affected. + - name: Vulnerability scan (govulncheck) + working-directory: drivers/${{ matrix.driver }} + env: + GOVULNCHECK_VERSION: v1.6.0 + DRIVER: ${{ matrix.driver }} + ALLOWLIST: ${{ github.workspace }}/.github/govulncheck-allowlist.txt + run: | + set -euo pipefail + go install "golang.org/x/vuln/cmd/govulncheck@$GOVULNCHECK_VERSION" + + REPORT="$RUNNER_TEMP/govulncheck-$DRIVER.json" + # JSON mode is for programmatic use and does not signal findings + # through its exit code; the parse below is what decides. + govulncheck -format json ./... > "$REPORT" || true + + # A finding gates only when its trace reaches a called symbol. + # Findings at module or package level mean the dependency is present + # but the vulnerable code is never invoked. + reachable() { + jq -sr --arg scope "$1" ' + [ .[] + | select(has("finding")) | .finding + | select(.trace[0].function != null) + | select(if $scope == "stdlib" + then .trace[0].module == "stdlib" + else .trace[0].module != "stdlib" end) + | .osv ] | unique | .[] + ' "$REPORT" + } + + STDLIB=$(reachable stdlib) + MODULE=$(reachable module) + ALLOWED=$(grep -E "^$DRIVER[[:space:]]" "$ALLOWLIST" 2>/dev/null | awk '{print $2}' | sort -u || true) + NEW=$(comm -23 <(printf '%s\n' "$MODULE" | sed '/^$/d') <(printf '%s\n' "$ALLOWED" | sed '/^$/d')) + + if [ -n "$STDLIB" ]; then + echo "- \`$DRIVER\` stdlib (toolchain, not gating): $(echo $STDLIB)" >> "$GITHUB_STEP_SUMMARY" + fi + if [ -n "$ALLOWED" ]; then + echo "- \`$DRIVER\` known backlog (allowlisted): $(echo $ALLOWED)" >> "$GITHUB_STEP_SUMMARY" + fi + + if [ -n "$NEW" ]; then + echo "- \`$DRIVER\` **new reachable dependency vulnerabilities**: $(echo $NEW)" >> "$GITHUB_STEP_SUMMARY" + echo "::error::new reachable dependency vulnerabilities in $DRIVER: $(echo $NEW)" + echo "Fix by upgrading the dependency. To accept it for now, add it to" + echo ".github/govulncheck-allowlist.txt with a reason." + govulncheck ./... || true + exit 1 + fi + + echo "- \`$DRIVER\`: no new reachable dependency vulnerabilities" >> "$GITHUB_STEP_SUMMARY" + # ─── Extension Module ───────────────────────────────────────────────── extension: name: Extension diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ccaaa8..1afca6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ All notable changes to Trove are documented in this file. ## [Unreleased] +### Dependencies and Supply Chain + +#### Fixed +- **`sftpdriver`: `golang.org/x/crypto` v0.37.0 → v0.52.0.** Five advisories in `x/crypto/ssh` were reachable, not merely present — `Open` calls `ssh.Dial`, `ssh.ParsePrivateKey`, and `sftp.NewClient` directly: infinite loop on large channel writes (GO-2026-5020), FIDO/U2F physical-interaction bypass (GO-2026-5019), DoS from pathological RSA/DSA parameters (GO-2026-5018), client-triggered server deadlock (GO-2026-5017), and a byte-arithmetic underflow panic (GO-2026-5013). The module's `go` directive is unchanged; x/crypto v0.52.0 requires only Go 1.25.0. + +#### Added +- **govulncheck now runs against each driver sub-module** in CI's `drivers` matrix job. It resolves imports per module, so the shared `go-ci.yml` `security` job — which runs at the repository root — never scanned `azuredriver`, `gcsdriver`, `s3driver`, or `sftpdriver` at all. Every third-party dependency the project ships lives in those modules, which is why the x/crypto findings above were invisible to CI while Dependabot reported them. +- **`.github/govulncheck-allowlist.txt`** records the reachable findings that remain in `gcsdriver`, `s3driver`, and `azuredriver`, all of them indirect. It is a backlog rather than an exemption: the gate fails on anything not listed, so removing a line is how a fix gets enforced. Standard-library findings are reported but never gate, since they track the runner's Go patch release rather than this repository, and gating on them would red every branch whenever a new Go version lands. + ### Phase 8: Cloud Drivers, Middleware, and Benchmarks #### Added — Storage Drivers diff --git a/drivers/sftpdriver/go.mod b/drivers/sftpdriver/go.mod index 6ba9f86..f3f05b1 100644 --- a/drivers/sftpdriver/go.mod +++ b/drivers/sftpdriver/go.mod @@ -6,14 +6,14 @@ require ( github.com/pkg/sftp v1.13.7 github.com/stretchr/testify v1.11.1 github.com/xraph/trove v0.0.0 - golang.org/x/crypto v0.37.0 + golang.org/x/crypto v0.52.0 ) require ( github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/kr/fs v0.1.0 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - golang.org/x/sys v0.32.0 // indirect + golang.org/x/sys v0.45.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/drivers/sftpdriver/go.sum b/drivers/sftpdriver/go.sum index 1c09402..4afc634 100644 --- a/drivers/sftpdriver/go.sum +++ b/drivers/sftpdriver/go.sum @@ -19,8 +19,8 @@ github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5t golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= -golang.org/x/crypto v0.37.0 h1:kJNSjF/Xp7kU0iB2Z+9viTPMW4EqqsrywMXLJOOsXSE= -golang.org/x/crypto v0.37.0/go.mod h1:vg+k43peMZ0pUMhYmVAWysMK35e6ioLh3wB8ZCAfbVc= +golang.org/x/crypto v0.52.0 h1:RMs7fP2rXdep0CftQlK8Uf+kibLm7qkCcradZWYz988= +golang.org/x/crypto v0.52.0/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGbc= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= @@ -39,15 +39,15 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.32.0 h1:s77OFDvIQeibCmezSnk/q6iAfkdiQaJi4VzroCFrN20= -golang.org/x/sys v0.32.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= -golang.org/x/term v0.31.0 h1:erwDkOK1Msy6offm1mOgvspSkslFnIGsFnxOKoufg3o= -golang.org/x/term v0.31.0/go.mod h1:R4BeIy7D95HzImkxGkTW1UQTtP54tio2RyHz7PwK0aw= +golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= +golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= From fc5386bd631a1201a06333e79c90aa1d1ba3f9ea Mon Sep 17 00:00:00 2001 From: Rex Raphael Date: Thu, 13 Aug 2026 19:14:40 -0500 Subject: [PATCH 2/3] ci: stop a root-module scan failure from skipping every other module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reusable workflow reports a single conclusion for all of its jobs, so when ci / Security failed, the whole `ci` job was marked failed and every job declaring `needs: ci` skipped: drivers, extension, bench and lint. The driver sub-modules and the extension module were therefore not being built or tested at all. A skipped job reports as neither pass nor fail, so this left no red check anywhere — the run looked healthy while four jobs did nothing. It also made the govulncheck step added in the previous commit unreachable, since it lives in the drivers job. The current trigger is two standard-library advisories describing the runner's Go patch release, which has no bearing on whether an independent module compiles or its tests pass. Keep `needs: ci` for ordering and run these jobs unless the workflow was actually cancelled. No gate is weakened: each job still reports its own result, and ci / Security still fails on its own findings. --- .github/workflows/ci.yml | 15 +++++++++++++++ CHANGELOG.md | 1 + 2 files changed, 16 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8e9426f..ee0d44d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,6 +30,14 @@ jobs: drivers: name: Driver (${{ matrix.driver }}) runs-on: ubuntu-latest + # `needs: ci` orders this after the root module's checks, but a reusable + # workflow reports one conclusion for all of its jobs — so any failure + # inside it, including a govulncheck finding about the runner's Go patch + # release, skipped this job entirely. These are independent modules whose + # build and tests have nothing to do with the root module's scan result, + # and a skipped job reports as neither pass nor fail, so the coverage + # disappeared silently. Run unless the workflow was actually cancelled. + if: ${{ !cancelled() }} needs: ci strategy: fail-fast: false @@ -135,6 +143,9 @@ jobs: extension: name: Extension runs-on: ubuntu-latest + # See the drivers job: a reusable-workflow failure must not silently + # skip an independent module's tests. + if: ${{ !cancelled() }} needs: ci steps: @@ -166,6 +177,8 @@ jobs: bench: name: Bench runs-on: ubuntu-latest + # See the drivers job. + if: ${{ !cancelled() }} needs: ci steps: @@ -197,6 +210,8 @@ jobs: lint: name: Lint runs-on: ubuntu-latest + # See the drivers job. + if: ${{ !cancelled() }} needs: ci steps: diff --git a/CHANGELOG.md b/CHANGELOG.md index 1afca6e..f123856 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to Trove are documented in this file. #### Added - **govulncheck now runs against each driver sub-module** in CI's `drivers` matrix job. It resolves imports per module, so the shared `go-ci.yml` `security` job — which runs at the repository root — never scanned `azuredriver`, `gcsdriver`, `s3driver`, or `sftpdriver` at all. Every third-party dependency the project ships lives in those modules, which is why the x/crypto findings above were invisible to CI while Dependabot reported them. +- **The `drivers`, `extension`, `bench` and `lint` jobs no longer skip when the shared root-module workflow fails.** A reusable workflow reports a single conclusion for all of its jobs, so a `ci / Security` failure — currently two standard-library advisories about the runner's Go patch release — marked the whole `ci` job failed and skipped every job that declared `needs: ci`. The driver sub-modules and the extension module were therefore not being built or tested at all, and a skipped job reports as neither pass nor fail, so the coverage vanished without a red check anywhere. They now run unless the workflow is cancelled; `needs: ci` is kept for ordering. - **`.github/govulncheck-allowlist.txt`** records the reachable findings that remain in `gcsdriver`, `s3driver`, and `azuredriver`, all of them indirect. It is a backlog rather than an exemption: the gate fails on anything not listed, so removing a line is how a fix gets enforced. Standard-library findings are reported but never gate, since they track the runner's Go patch release rather than this repository, and gating on them would red every branch whenever a new Go version lands. ### Phase 8: Cloud Drivers, Middleware, and Benchmarks From 9ac80edea580b2d7fbaeaa0a76de0b026adf38d5 Mon Sep 17 00:00:00 2001 From: Rex Raphael Date: Thu, 13 Aug 2026 19:19:27 -0500 Subject: [PATCH 3/3] ci: log govulncheck decisions to the job log, not only the run summary Every informative line went to GITHUB_STEP_SUMMARY, so a passing scan left no trace in stdout. Reading the step back with `gh run view --log` showed the script being echoed and nothing else, which is indistinguishable from a step that parsed nothing and passed vacuously. Report both places: the summary for the run page, stdout for anyone debugging from the CLI or reading a failed job's log. --- .github/workflows/ci.yml | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee0d44d..d5ce33f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -121,15 +121,24 @@ jobs: ALLOWED=$(grep -E "^$DRIVER[[:space:]]" "$ALLOWLIST" 2>/dev/null | awk '{print $2}' | sort -u || true) NEW=$(comm -23 <(printf '%s\n' "$MODULE" | sed '/^$/d') <(printf '%s\n' "$ALLOWED" | sed '/^$/d')) + # Report to both the job log and the run summary. The summary is for + # humans reading the run page; stdout is what `gh run view --log` + # and anyone debugging a failure actually gets. A step whose + # reasoning appears only in the summary looks like it did nothing. + emit() { + echo "govulncheck[$DRIVER] $1" + echo "- \`$DRIVER\` $1" >> "$GITHUB_STEP_SUMMARY" + } + if [ -n "$STDLIB" ]; then - echo "- \`$DRIVER\` stdlib (toolchain, not gating): $(echo $STDLIB)" >> "$GITHUB_STEP_SUMMARY" + emit "stdlib (toolchain, not gating): $(echo $STDLIB)" fi if [ -n "$ALLOWED" ]; then - echo "- \`$DRIVER\` known backlog (allowlisted): $(echo $ALLOWED)" >> "$GITHUB_STEP_SUMMARY" + emit "known backlog (allowlisted): $(echo $ALLOWED)" fi if [ -n "$NEW" ]; then - echo "- \`$DRIVER\` **new reachable dependency vulnerabilities**: $(echo $NEW)" >> "$GITHUB_STEP_SUMMARY" + emit "**new reachable dependency vulnerabilities**: $(echo $NEW)" echo "::error::new reachable dependency vulnerabilities in $DRIVER: $(echo $NEW)" echo "Fix by upgrading the dependency. To accept it for now, add it to" echo ".github/govulncheck-allowlist.txt with a reason." @@ -137,7 +146,7 @@ jobs: exit 1 fi - echo "- \`$DRIVER\`: no new reachable dependency vulnerabilities" >> "$GITHUB_STEP_SUMMARY" + emit "no new reachable dependency vulnerabilities" # ─── Extension Module ───────────────────────────────────────────────── extension: