Repository navigation
Conversation
Pins coder/kaniko to 8e293b69, which removes github.com/docker/docker, bumps buildkit to v0.31.2 and go-archive to v0.3.3, and vendors a fixed ACR credential helper. Fixes GO-2026-4858, GO-2026-4859, GO-2026-6255, GO-2026-6253, GO-2026-6225, and removes docker/docker (GO-2026-4883, GO-2026-4887) from the envbuilder binary. Parse Dockerfiles with an initialized linter: buildkit v0.31 applies "# check=" comments to the linter and panicked with a nil one.
…v0.21.7 go-containerregistry v0.21 holds a pull limiter slot per open remote blob reader. The previous kaniko pin deadlocked extracting base images with more than four layers, and v0.21.6 tarball.Write hangs the same way. Also close the devcontainer feature layer reader.
| // 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-20251107135632-b20ff58093d8 | ||
| replace github.com/GoogleContainerTools/kaniko => github.com/coder/kaniko v0.0.0-20261008191411-61a5b7da7ea3 |
There was a problem hiding this comment.
The pin points at an unmerged commit(v0.0.0-20261008191411-61a5b7da7ea3) - needs re-pined after merged and make sure coder/kaniko#38 is merged first
There was a problem hiding this comment.
Agreed. coder/kaniko#38 is still open, so I've left the pin as is for now. Once #38 merges, I'll re-pin the replace to the merge commit on kaniko main and rerun the checks before this PR merges. This thread can stay open until then as a merge blocker.
Generated by Coder Agents on behalf of @stirby.
There was a problem hiding this comment.
P2 [CRF-2] coder/kaniko#38 merged as a51d65b5 on 2026-10-14, and the commits after the pinned 61a5b7da change Go code, so the re-pin is not a no-op. (Ryosuke, Mafu-san, Pariston, Mafuuu, Gon, Leorio)
cd77cdb9 and 8cd8fc68 change pkg/creds/acr/token.go to request the service principal token for https://containerregistry.azure.net instead of settings.Environment.ResourceManagerEndpoint, and b7fcaf3d raises kaniko's go directive from 1.25.9 to 1.26.9, which forces envbuilder's go 1.26.5 up. The description's statement that later commits "only change CI and integration fixtures, not Go code" is incorrect, and none of the listed validation covered the ACR change. Re-pin to a51d65b5 in this PR, run go mod tidy, and update the description and the coder/terraform-provider-envbuilder#138 pin to match.
🤖
| @@ -6,7 +6,7 @@ toolchain go1.26.6 | |||
|
|
|||
| // There are a few options we need added to Kaniko! | |||
There was a problem hiding this comment.
Not caused by this PR, but advisories published yesterday now flag toolchain go1.26.6 (12 reachable stdlib advisories, fixed in 1.26.9) and golang.org/x/net v0.58.0 (fixed in v0.60.0). Worth bumping both here or in a quick follow-up.
There was a problem hiding this comment.
Bumped in 77452e6: toolchain go1.26.9 and golang.org/x/net v0.60.0. x/net v0.60.0 also requires newer x/crypto (v0.57.0), x/sys, x/term, x/text, x/sync and x/mod, so go mod tidy moved those too.
Checks: govulncheck ./... reports 0 reachable advisories. make test (integration included), make lint/go, make fmt and make gen all pass with no diff.
Generated by Coder Agents on behalf of @stirby.
|
/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, 2 findings (2 P2), COMMENT. Review Finding inventoryFinding inventory: PR #535Findings
Contested and acknowledgedRound logRound 1Round type: round 1, full panel. Netero first pass: 1 P2 (CRF-1), no P0, so panel ran. Law not run (effective additions 56).
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 to drop docker/docker and bump buildkit and go-containerregistry, and adapts two envbuilder call sites to the new versions. 2 P2 findings. As requested in this thread, only P0-P2 findings are posted.
Out of scope (needs a ticket or explicit acceptance by a human):
devcontainer/features/features.go:68:filepath.Join(directory, header.Name)does not reject../tar entries, and the default filesystem isosfs.New("/"), so a feature image can write files anywhere on the build filesystem (inferred from code, not run).devcontainer/devcontainer.go:487: envbuilder's ownremote.Imagecalls also ignoreENVBUILDER_INSECUREfor TLS verification, so base images and features on a self-signed registry fail regardless of the flag.- buildkit v0.31.2
frontend/dockerfile/instructions/parse.go:67:ParseInstructionpasses a nil linter and panics on any# check=instruction comment, so any other caller of it crashes on such Dockerfiles.
🤖 This review was automatically generated with Coder Agents.
| github.com/go-git/go-git/v5 v5.19.2 | ||
| github.com/google/go-cmp v0.7.0 | ||
| github.com/google/go-containerregistry v0.20.7 | ||
| github.com/google/go-containerregistry v0.21.7 |
There was a problem hiding this comment.
P2 [CRF-1] go-containerregistry v0.21.7 no longer falls back to plain HTTP for *.local registries, and ENVBUILDER_INSECURE=true does not restore it for envbuilder's own image and feature fetches. (Netero, Mafuuu, Razor)
reLocal in pkg/name/registry.go:28 changed from .*\.local(?:host)? to .*\.localhost. UserFromImage (devcontainer/devcontainer.go:487), extractFromImage (devcontainer/features/features.go:29) and the SkipRebuild path (envbuilder.go:500) call remote.Image on references parsed without name.Insecure, so a devcontainer whose base image or features are on registry.ns.svc.cluster.local:5000 now fails in Compile or on restart, with or without the flag (read from both module versions, not run against a live registry). Parse these three references with name.Insecure when opts.Insecure is set, so the workaround in the description covers them.
🤖
3 participants
Pins the kaniko fork to coder/kaniko#38, which drops
github.com/docker/dockerand the chrismellard ACR helper and bumps buildkit to v0.31.2. This fixes the remaining reachable advisories in envbuilder.Depends on coder/kaniko#38. The
replacepoints at61a5b7daon that PR. Later commits there only change CI and integration fixtures, not Go code. Re-pin to the merge commit once #38 lands.Test template on dogfood, which builds envbuilder from this branch: https://dogfood.cdr.dev/templates/coder/kirby-envbuilder-rollup
Advisories
govulncheck ./...(reachable):main(a5b7329)moby/buildkit)docker/docker)moby/go-archive)chrismellard/docker-credential-acr-env)govulncheck -mode=binaryoncmd/envbuilderreports only GO-2026-6443 (grpc). grpc is intentionally left alone in this PR.docker/docker, themoby/mobymonolith andchrismellard.docker/dockerandmoby/mobystay ingo.modas the Docker client for./integrationtests only. Also x/crypto openpgp and dhcp findings, which are unreachable.Changes
devcontainer.UserFromDockerfile: passes a buildkit linter toParseInstructionWithLinter. Without one, buildkit v0.31 panics on Dockerfiles with# check=directives. Test:TestUserFromDockerfile_BuildArgs/LintCheckComment.features.Extract: now closes the feature layer reader. go-containerregistry v0.21 holds a pull slot until the reader is closed.Behavior changes
COPY/ADD --exclude,COPY --parents,ADD --unpack,RUN --security=insecureandRUN --devicewith "not supported by kaniko". 1.3.0 also failed on these, as unknown flags.*.localregistries: go-containerregistry v0.21 no longer falls back to plain HTTP for them. Onlylocalhost,*.localhostand loopback IPs fall back. Plain-HTTP in-cluster registries (for exampleregistry.ns.svc.cluster.local:5000) needENVBUILDER_INSECURE=true.Validation
make testpasses (all 36 integration tests, 0 skipped), as domake lint,make fmtandmake gen(no diff).maindevcontainer (64-layer base image) builds to INIT. An earlier pin of this branch deadlocked at layer 5 (go-containerregistry pull limiter, fixed in kaniko#38).FROM <stage>andCOPY --from=golang:1.25-bookwormbuilds. The earlier pin deadlocked in "Storing source image".# check=Dockerfile builds.COPY --excludefails with the new error, and a plain Dockerfile builds.envbuilder-rollup-mainon the test template builds coder/codermain. The agent is ready,/.envbuilder/builtexists, and code-server returns healthy. The binary reportsvcs.revision=9e64f32,coder/kaniko 61a5b7daand go-containerregistry v0.21.7.replacedirectives) and its acceptance tests pass.Decision log
github.com/moby/moby/v2betas, so kaniko ports the few helpers it used instead of upgrading.tarball.Writeleaks pull slots and hangs multi-stage builds. A subagent review of the v0.19 → v0.21 diff found it, and it was reproduced locally..localHTTP fallback documented, not shimmed: it is an upstream security hardening.ENVBUILDER_INSECUREcovers kaniko pulls, pushes and the cache repo. It does not cover envbuilder's own devcontainer image config and feature fetches, which never honored it../integrationoffdocker/docker.