build: prevent kodata symlinks from escaping the kodata root#1619
Open
evilgensec wants to merge 2 commits intoko-build:mainfrom
Open
build: prevent kodata symlinks from escaping the kodata root#1619evilgensec wants to merge 2 commits intoko-build:mainfrom
evilgensec wants to merge 2 commits intoko-build:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR mitigates a symlink traversal vulnerability in kodata/ packaging by ensuring dereferenced symlink targets remain within the original kodata root before adding content to image layers.
Changes:
- Extend
walkRecursivewith anabsKodataRootparameter and reject symlink targets that resolve outside the kodata root. - Canonicalize
kodataroot intarKoDatausingfilepath.EvalSymlinksthenfilepath.Absand pass it through recursion. - Add tests covering escaping vs. in-root symlink behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/build/gobuild.go | Adds canonical-root computation and enforces “resolved path must be within kodata root” during recursive tar packing. |
| pkg/build/gobuild_test.go | Adds regression tests for escaping symlinks and expected behavior for in-root symlinks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
walkRecursive dereferences symlinks in the kodata/ directory when packing files into container image layers. Previously there was no check that the resolved path remained within the kodata root, so a symlink pointing at an arbitrary host path (e.g. ~/.ssh/id_rsa or /etc/passwd) would cause ko to silently pack the target file into the published image. Fix this by computing the canonical absolute path of the kodata root (using both filepath.EvalSymlinks and filepath.Abs to handle platforms where the temp/home directory itself is behind a symlink, such as macOS) and verifying after each filepath.EvalSymlinks call that the resolved target path has the kodata root as a prefix. Symlinks whose targets lie outside the root now produce an explicit error instead of leaking host files. The boundary check uses: absEvalPath != absKodataRoot && !strings.HasPrefix(absEvalPath, absKodataRoot+sep) This allows a symlink that resolves exactly to the kodata root itself while still rejecting targets that escape it. Add two unit tests: - TestWalkRecursiveSymlinkTraversal: symlink escaping kodata root is rejected; skipped on Windows if os.Symlink is unavailable. - TestWalkRecursiveSymlinkWithinKodata: symlink within kodata root is accepted and the symlink target's content is verified in the resulting tar archive; skipped on Windows if os.Symlink is unavailable.
evilgensec
force-pushed
the
fix/kodata-symlink-traversal
branch
from
723719a to
54a0633
Compare
filepath.EvalSymlinks fails when the kodata directory does not exist. Guard the call with os.Stat so that the non-existent case falls through to walkRecursive, which already silently no-ops for missing roots via filepath.Walk semantics. Fixes test failures in TestGoBuildNoKoData, TestGoBuildConsistentMediaTypes, and TestDebugger.
Member
FYI I can't view this security report. |
Author
|
Dear @imjasonh, I received the following message while reporting a security issue. Is there any further way I can disclose the issue to the team responsibly? 
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
walkRecursivedereferences symlinks inkodata/when packing files into container image layers. There was no check that the resolved path remained within the kodata root, so a symlink pointing at an arbitrary host path (e.g.~/.ssh/id_rsa,/etc/passwd,/root/.ssh/authorized_keys) would silently pack the target file into the published container image.A module maintainer or CI pipeline operator who can place a symlink in
kodata/— including via a dependency'skodata/directory or a compromised build step — could cause credentials and secrets from the build host to be embedded in the image pushed to a registry.Changes
pkg/build/gobuild.gowalkRecursivegains anabsKodataRoot stringparameter: the canonical absolute path of the original kodata directory (resolved with bothfilepath.EvalSymlinksandfilepath.Abs).filepath.EvalSymlinks(hostPath), the resolved path is checked withstrings.HasPrefix(absEvalPath, absKodataRoot+string(filepath.Separator)). If the target is outside the kodata root the walk returns an error.absKodataRootso the check is enforced at all nesting levels.tarKoDatacomputesabsKodataRootusingfilepath.EvalSymlinksthenfilepath.Abs(order matters on macOS where/var→/private/var) and passes it towalkRecursive.pkg/build/gobuild_test.goTestWalkRecursiveSymlinkTraversal: symlink escaping kodata root returns an error.TestWalkRecursiveSymlinkWithinKodata: symlink within kodata root is still followed correctly.Test
Related security report: https://issuetracker.google.com/issues/495623327