Repository navigation
fix: clear remaining scanner findings (stacked on #535) - #536
coder[bot] wants to merge 9 commits into
Conversation
…/moby/api Drops github.com/docker/docker and the github.com/moby/moby monolith from the module graph. The monolith was only selected because docker/cli (+incompatible, no go.mod) imports moby/moby/api/types/network; requiring the split moby/moby/api module resolves that import instead.
runc is only reached through a coder/coder test dependency (ory/dockertest), but its checksum is recorded in go.sum, which module scanners report.
trivy is only reached through coder/coder test packages (coderdtest via coder/preview), but its checksum is recorded in go.sum, which module scanners report. v0.72.0 is the lowest release that fixes the reported advisories; MVS also raises aws-sdk-go-v2/service/ecr, go-redis, zap and gopsutil in the binary.
containerd v1 is only reached through kaniko's pkg/executor tests, but its checksum is recorded in go.sum. v1.7.36 clears the advisories that have a v1.7 fix; the remaining CRI advisories affect every v1 release.
Re-pins the kaniko fork to coder/kaniko@76b64cc0, which imports platforms from github.com/containerd/platforms instead of containerd v1. containerd v1 is no longer in the module graph, so the v1.7.36 requirement is dropped. The fork now declares go 1.26.9, which raises the go directive here and makes the separate toolchain line redundant.
…envbuilder-zero-vulns # Conflicts: # go.mod # go.sum
|
/coder-agents-review Please limit the review to P0, P1, and P2 issues only. |
|
Chat: Review posted | View chat Review history
deep-review v0.13.0 | Round 1 | Last posted: Round 1, 3 findings (3 P2), COMMENT. Review Finding inventoryFindings
Contested and acknowledgedLaw analysisNot run. Effective additions 86, below the 1000 threshold. Round logRound 1Full panel (round 1). Netero first pass: 1 P2, 2 Notes; panel proceeded. Panel: ging-go, ryosuke, pariston, mafuuu, bisky, zoro, gon, leorio, mafu-san, kurapika, wildcard razor (11; multi-domain because the change is a supply-chain security fix). Requester comment IC_kwDOJPSNMM8AAAABcH4Vzw asked to limit the review to P0-P2, so P3 and below were not posted. Orchestrator verified CRF-1 (git ls-remote), CRF-4 (kaniko diff 61a5b7da..76b64cc0 pkg/creds/acr/token.go and docs.azure.cn ACR scope), CRF-5 (go list -m all at head). Gon comment findings moved out of scope: comment lines unchanged by the diff. 3 P2 posted. Reviewed against 77452e6..37667dd. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR re-pins the kaniko fork, migrates the integration tests to moby/moby/client, and raises dependency floors to clear scanner findings. 3 P2 findings. As requested on this PR, findings below P2 are not posted.
Out of scope (needs a ticket or explicit acceptance by a human):
coder/kaniko pkg/creds/acr/token.go:39: the hard-codedhttps://containerregistry.azure.netaudience is the root cause of the Azure China finding below; sovereign-cloud ACR authentication fails until kaniko picks the audience per cloud.coder/kaniko go.mod:3: the fork'sgo 1.26.9directive forces every module that imports envbuilder to require Go 1.26.9, and thereplacedoes not apply to importers, so a library consumer keeps its own kaniko pin and containerd v1 until it re-pins..github/workflows/ci.yaml: no job runs govulncheck, trivy, or grype, so a later dependency bump can bring scanner findings back without any check failing..github/workflows/ci.yaml:40: CI installs Go~1.22and relies onGOTOOLCHAIN=autoto download 1.26.9; a runner withGOTOOLCHAIN=localfails the build.
🤖 This review was automatically generated with Coder Agents.
| // There are a few options we need added to Kaniko! | ||
| // See: https://github.com/GoogleContainerTools/kaniko/compare/main...coder:kaniko:main | ||
| replace github.com/GoogleContainerTools/kaniko => github.com/coder/kaniko v0.0.0-20261008191411-61a5b7da7ea3 | ||
| replace github.com/GoogleContainerTools/kaniko => github.com/coder/kaniko v0.0.0-20261011010809-76b64cc01bec |
There was a problem hiding this comment.
P2 [CRF-1] The kaniko replace pins coder/kaniko@76b64cc01bec, which exists only on the unmerged branch stirby/kaniko-containerd-platforms (refs/pull/40/head), not on kaniko main (a51d65b5). (Netero)
If coder/kaniko#40 is squash-merged or rebased and the branch is deleted, the commit stays fetchable only from proxy.golang.org's cache, so GOPROXY=direct builds fail and envbuilder releases ship kaniko code that is not on the fork's main. Keep this PR unmerged until the replace points at a commit on kaniko main.
🤖
| // There are a few options we need added to Kaniko! | ||
| // See: https://github.com/GoogleContainerTools/kaniko/compare/main...coder:kaniko:main | ||
| replace github.com/GoogleContainerTools/kaniko => github.com/coder/kaniko v0.0.0-20261008191411-61a5b7da7ea3 | ||
| replace github.com/GoogleContainerTools/kaniko => github.com/coder/kaniko v0.0.0-20261011010809-76b64cc01bec |
There was a problem hiding this comment.
P2 [CRF-4] The kaniko re-pin brings in a change that hard-codes the ACR token audience to https://containerregistry.azure.net, so ACR authentication in Azure China likely stops working. (Kurapika)
Between 61a5b7da and 76b64cc0, pkg/creds/acr/token.go:39 changed from settings.Environment.ResourceManagerEndpoint to that constant for the client secret, federated token, and managed identity paths, with a comment saying it is "One value for every Azure cloud." Azure China documents the ACR scope as https://containerregistry.chinacloudapi.cn/.default (docs.azure.cn, "Configure Registry Acceptance of Microsoft Entra Authentication Scopes"), while the token is still requested from that cloud's ActiveDirectoryEndpoint, so private base images, features, and cache repos on *.azurecr.cn that creds.GetKeychain() authenticates (envbuilder.go:500, devcontainer/devcontainer.go:487, devcontainer/features/features.go:29) would fail where the ARM audience worked. This is inferred from the docs and was not run against a China tenant. Fix in kaniko by choosing the audience from settings.Environment and keeping ResourceManagerEndpoint for clouds with no known ACR audience, then re-pin.
🤖
| // There are a few options we need added to Kaniko! | ||
| // See: https://github.com/GoogleContainerTools/kaniko/compare/main...coder:kaniko:main | ||
| replace github.com/GoogleContainerTools/kaniko => github.com/coder/kaniko v0.0.0-20261008191411-61a5b7da7ea3 | ||
| replace github.com/GoogleContainerTools/kaniko => github.com/coder/kaniko v0.0.0-20261011010809-76b64cc01bec |
There was a problem hiding this comment.
P2 [CRF-5] The description and commit f403dc1 say containerd v1, docker/docker, and the moby/moby monolith were removed from the module graph, but go list -m all at head still lists github.com/containerd/containerd v1.4.13 and github.com/docker/docker v28.5.2+incompatible. (Mafu-san, Razor, Pariston)
go mod graph shows gvisor requiring containerd v1.4.13, older than the v1.7.29 this PR started from, and trivy@v0.72.0 requiring docker/docker v28.5.2. Neither is in go.sum or the binary, so the six scanners in the table are unaffected, but a scanner or auditor that reads the build list reports both and contradicts the PR's stated result. Change "removed from module graph" to "absent from go.mod, go.sum, and the binary", and state that the build list still names both versions.
🤖
2 participants
Stacked on #535 (
stirby/kaniko-moby). Goal: get every package/container scanner to report zero findings by upgrading or removing flagged dependencies instead of leaving them as "not affected".Depends on coder/kaniko#40 (
stirby/kaniko-containerd-platforms,76b64cc0), which is stacked on coder/kaniko#38. The kanikoreplacepoints at that unmerged commit. Re-pin it once the kaniko change lands.Summary
This drops the scanner findings from 116 to 5.
grypeon the image now reports 0. Every remaining finding is the same advisory, GO-2026-5932 (golang.org/x/crypto/openpgp). It lists every x/crypto version as affected and has no fixed version. envbuilder doesn't import openpgp. See Unresolved.Before / after
Tools: trivy 0.75.0, grype 0.120.1, govulncheck v1.8.0 (vuln.go.dev DB 2026-10-08). The image is built with
scripts/build.sh --arch=amd64. It isFROM scratchwith no OS packages, so the only scanned target is the Go binary.govulncheck -show verbose ./...govulncheck -mode=binarytrivy fs --scanners vuln .grype dir:.trivy imagegrypeimageBaseline findings by advisory
Changes
fix: build with Go 1.26.9: the stdlib fixes. The last commit turns this intogo 1.26.9, because the kaniko fork now declares it.fix: bump golang.org/x/net to v0.60.0 and google.golang.org/grpc to v1.83.2:golang.org/x/cryptoalso moves to v0.58.0, and the other x/ modules move with them.fix: bump tailscale transitive deps:insomniacslk/dhcpandcilium/ebpf. Both come in through coder/tailscale, which coder/coder pulls in.test(integration): migrate Docker client to moby/moby/client and moby/moby/api: this removesdocker/dockerand themoby/mobymonolith from go.mod and go.sum. The monolith was only being selected because docker/cli (+incompatible, no go.mod) importsmoby/moby/api/types/network. Requiring the splitmoby/moby/apimodule resolves that import instead.fix: require opencontainers/runc v1.3.6,aquasecurity/trivy v0.72.0: neither module ends up in the binary. They're only reached through coder/coder test packages (dbtestutil/dockertest,coderdtest/preview). Butgo mod tidyrecords their checksums in go.sum, andgrype dir:.reports them from there.fix: require containerd/containerd v1.7.36was an intermediate step.fix: pin kaniko without containerd v1 and drop containerd/containerdreplaces it: it re-pins kaniko tocoder/kaniko@76b64cc0, which importsgithub.com/containerd/platformsinstead. containerd v1 is no longer in the module graph.Behavior changes
aws-sdk-go-v2/service/ecr(v1.27.4 to v1.57.2, used by the ECR credential helper),go-redis/v9(v9.7.3 to v9.20.0),zap(v1.27.1 to v1.28.0) andgopsutil/v4(v4.26.2 to v4.26.3).61a5b7da: ACR token handling updates, Go 1.26.9, and test/CI fixes.go.modnow declaresgo 1.26.9(previouslygo 1.26.5+toolchain go1.26.9).Validation
make lint,make fmt,make gen(includingscripts/docsgen): clean, no diff.make test(unit and integration, local registry viamake test-registry): 348 passed, 3 skipped, 0 failed in two consecutive full runs at the final commit.t.Skips):gitTestShallowCloneRepo/OK,TestCloneRepo/invalid_auth/AlreadyCloned,TestCloneRepo/auth_but_no_creds/AlreadyCloned.TestCloneFailsFallback/BadRepo, whereOpenFile(/.envbuilder/Dockerfile)returned ENOENT. It hasn't reproduced since, in 3 isolated and 4 full runs, and it passes on the fix: drop docker/docker and bump buildkit to v0.31.2 via the kaniko fork #535 base..devcontainer/Dockerfileanddevcontainer.json. It built the image, ranpostCreateCommand, and reached the init script (SMOKE_INIT_OK).Decision log
agentsdkandagent/protodRPC). Replacing it would mean a protocol reimplementation, which is out of scope.github.com/coder/trivy, which is what coder/coder does. That fork is based on v0.69.3 and doesn't include the fixes, so it would only hide the findings by changing the module path. I required upstream v0.72.0 (the lowest fixed version) instead. v0.75.0 needs Go 1.27 and pulls in buildkit and go-containerregistry upgrades.platforms, which already lives in thegithub.com/containerd/platformsmodule.Unresolved
golang.org/x/crypto/openpgp). envbuilder never imports openpgp; govulncheck reports it at module level only. Upstream x/crypto split openpgp out and deleted it on master (commits2f1f834..99e4382, 2026-10-09). Once x/crypto v0.59.0 is tagged and vuln.go.dev marks it fixed, bumping x/crypto will clear this from every scanner.replaceoncestirby/kaniko-containerd-platformslands.