Skip to content

Check symlinks when scoping paths to a directory - #1891

Open
himsngh wants to merge 1 commit into
carvel-dev:developfrom
himsngh:himsngh/fix-scopedpath-symlink
Open

himsngh wants to merge 1 commit into
carvel-dev:developfrom
himsngh:himsngh/fix-scopedpath-symlink

Conversation

@himsngh

@himsngh himsngh commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

memdir.ScopedPath is used to keep user-supplied paths (template paths, values paths, helm chart path, fetch subPath, inline file paths, etc.) within the App working directory. The check is lexical only, so a path that goes through a symlink inside the directory passes the check even when the symlink points outside of the directory, and the caller then reads from the symlink target.

In addition to the existing lexical check, resolve symlinks in both the base directory and the joined path and check containment again. Since some callers check paths that are about to be written, symlinks are resolved on the deepest existing ancestor and the remaining components are joined unchanged. The returned path is still the lexical path, so callers are not affected.

Symlinks that stay within the directory, and base directories that are themselves symlinks (e.g. /var -> /private/var on macOS), keep working.

vendir already rejects fetched content containing symlinks that point outside of the fetched directory, so this mainly makes ScopedPath not depend on that.

Changes:

  1. pkg/memdir/scoped.go

    • Re-check containment after resolving symlinks
  2. pkg/memdir/scoped_test.go (new)

    • Add tests for paths within the directory, not yet existing paths, symlinks within the directory, symlinks pointing outside of the directory (absolute and relative, existing and not yet existing targets), and a base directory that is a symlink
  3. pkg/template/ytt_test.go (new)

    • Add Test_Ytt_PathThroughSymlinkOutsideDir_IsRejected, which runs ytt with a path through a symlink in the working directory and verifies that content outside the working directory is not rendered. Without this change the outside content is rendered.

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?


Additional Notes for your reviewer:

Review Checklist:
  • Follows the developer guidelines
  • Relevant tests are added or updated
  • Relevant docs in this repo added or updated
  • Relevant carvel.dev docs added or updated in a separate PR and there's
    a link to that PR
  • Code is at least as readable and maintainable as it was before this
    change

Additional documentation e.g., Proposal, usage docs, etc.:


memdir.ScopedPath is used to keep user-supplied paths (template paths,
values paths, helm chart path, fetch subPath, inline file paths, etc.)
within the App working directory. The check is lexical only, so a path
that goes through a symlink inside the directory passes the check even
when the symlink points outside of the directory, and the caller then
reads from the symlink target.

In addition to the existing lexical check, resolve symlinks in both the
base directory and the joined path and check containment again. Since
some callers check paths that are about to be written, symlinks are
resolved on the deepest existing ancestor and the remaining components
are joined unchanged. The returned path is still the lexical path, so
callers are not affected.

Symlinks that stay within the directory, and base directories that are
themselves symlinks (e.g. /var -> /private/var on macOS), keep working.

vendir already rejects fetched content containing symlinks that point
outside of the fetched directory, so this mainly makes ScopedPath not
depend on that.

Changes:

1. pkg/memdir/scoped.go
   - Re-check containment after resolving symlinks

2. pkg/memdir/scoped_test.go (new)
   - Add tests for paths within the directory, not yet existing paths,
     symlinks within the directory, symlinks pointing outside of the
     directory (absolute and relative, existing and not yet existing
     targets), and a base directory that is a symlink

3. pkg/template/ytt_test.go (new)
   - Add Test_Ytt_PathThroughSymlinkOutsideDir_IsRejected, which runs
     ytt with a path through a symlink in the working directory and
     verifies that content outside the working directory is not
     rendered. Without this change the outside content is rendered.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Himanshu Singh <himansh.singh3@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant