fix: [SNOW-3417049] refuse to follow symlinks that escape the project root#2984
Draft
sfc-gh-olorek wants to merge 1 commit into
Draft
fix: [SNOW-3417049] refuse to follow symlinks that escape the project root#2984sfc-gh-olorek wants to merge 1 commit into
sfc-gh-olorek wants to merge 1 commit into
Conversation
… root `BundleMap._absolute_src` used `resolve_without_follow` (effectively `os.path.abspath`) to check that a source path stayed inside the project root. Because that helper never follows symlinks, a committed symlink whose lexical path was inside the root but whose target pointed outside (for example `project/data -> /etc`) passed the containment check. The three `os.walk(..., followlinks=True)` sites in the bundling pipeline would then traverse into the real target and copy those files into the deploy root, from where `snow app deploy` uploads them to a Snowflake stage — turning a repo checkout into an exfiltration vector for files like `~/.aws/credentials` or `~/.ssh/`. Harden the check on both sides: * `_absolute_src` now calls `os.path.realpath` on both the source and the project root and raises `ArtifactError` if the real path escapes. Legitimate symlinks whose real target stays inside the project root continue to resolve normally. * The two `os.walk(..., followlinks=True)` loops in `BundleMap` and `_ArtifactPathMap` prune any directory or file whose real path escapes the project root, so a nested escaping symlink inside a legitimate directory source is skipped instead of being copied. Adds unit tests covering the top-level, nested, and legitimate-symlink cases.
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
1 participant
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.
Pre-review checklist
Changes description
Jira: SNOW-3417049
BundleMap._absolute_srcusedresolve_without_follow(which is justos.path.abspath) to check that a source path stayed inside the project root. Because that helper never follows symlinks, a committed symlink whose lexical path was inside the project root but whose target pointed outside (for exampleproject/data -> /etc) passed the containment check. The threeos.walk(..., followlinks=True)call sites in the bundling pipeline would then traverse the real target and copy those files into the deploy root, from wheresnow app deployuploads them to a Snowflake stage. A committeddata -> /home/runner/.sshsymlink in any Native App repo is therefore enough to exfiltrate host secrets the next time someone runssnow app bundle/snow app deploy.This PR hardens the check on both sides:
BundleMap._absolute_srcnow callsos.path.realpathon both the resolved source and the project root, and raisesArtifactErrorwith a descriptive message if the real path escapes. Symlinks whose real target stays inside the project root continue to resolve normally — the fix only rejects escaping symlinks.os.walk(..., followlinks=True)loops inBundleMapand_ArtifactPathMapprune any subdirectory or file whose real path escapes the project root, so a nested escaping symlink inside an otherwise-legitimate directory source (e.g.src/escape -> /etcinside a legitimatesrc/mapping) is skipped rather than copied.Unit tests cover three cases: a top-level symlink that escapes (must error), a nested symlink that escapes inside a legitimate directory source (must be pruned; siblings still bundled), and a legitimate symlink whose target remains inside the project root (must continue to be followed).