Repository navigation
fix: prevent version bump script from following symlinks - #2789
Open
Rana Singh (ranadeepsingh) wants to merge 1 commit into
Open
Rana Singh (ranadeepsingh) wants to merge 1 commit into
Rana Singh (ranadeepsingh) wants to merge 1 commit into
Conversation
Rana Singh (ranadeepsingh)
requested a review
from Brendan Walsh (BrendanWalsh)
as a code owner
October 23, 2026 10:13
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hey Rana Singh (@ranadeepsingh) 👋! We use semantic commit messages to streamline the release process. Examples of commit messages with semantic prefixes:
To test your commit locally, please follow our guild on building from source. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Concurrent path replacement can bypass the checks and redirect a write outside the selected repository.
1 open issue
What changed in this PR
This PR hardens the version-bump maintenance script against symlinks that point outside the selected repository.
Changes:
- Skips symlinked files during discovery and warns when they are encountered.
- Adds containment checks before writing and excludes unsafe paths from broad verification sweeps.
- Adds a regression test confirming that an existing external symlink target remains unchanged.
| File | Description |
|---|---|
scripts/test_bump_version.py |
Adds coverage for an external symlink target. |
scripts/bump-version.py |
Adds symlink and repository-containment checks. |
Review effort: Balanced
## Summary Keep the version bump script inside the repository selected with `--repo-root`. Skip symlinks, reject unsafe paths, and use rooted directory file descriptors for race-safe reads and writes. ## Prompting Intent The engineer asked to implement the confirmed hardening case from the private security review, add focused regression coverage, validate it, and prepare the public pull request. The change should be small and should not describe private report details in public Git or PR text. ## Linked Sources - Private security triage: reviewed in the SynapseML advisory thread; detailed report information deliberately omitted from public history. - Repository guidance: AGENTS.md and the master branch context. - Existing test suite: scripts/test_bump_version.py. ## Rationale Resolving and checking each candidate path closes the concrete symlink escape while preserving normal in-repository version replacement. The script now skips symlinks during discovery and post-verification, and opens files by their relative path through the repository root. It also rejects symlinked or non-regular final paths and fails closed on platforms without the directory file descriptor and no-follow support needed for a race-safe open. The change is narrower than changing CLI behavior or adding a global scanner suppression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: af31ef86-1171-422a-abeb-c63e597a01a2
Rana Singh (ranadeepsingh)
force-pushed
the
fix/prevent-version-bump-symlink-writes
branch
from
October 23, 2026 11:20
2ed1380 to
a9bb6ec
Compare
|
|
||
|
|
||
| def _safe_open_relative(root, rel, flags, mode=0): | ||
| root = root.resolve(strict=True) |
| FileNotFoundError, | ||
| UnicodeDecodeError, | ||
| PermissionError, | ||
| UnsafeRootedPath, |
This branch has not been deployed
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.
Related Issues/PRs
None.
What changes are proposed in this pull request?
This hardens
scripts/bump-version.pyso it cannot follow symlinks outside the repository selected with--repo-root.The script now:
This is a focused maintenance-tool hardening change. It does not change normal in-repository version replacement behavior.
How is this patch tested?
python3 -m pytest -q scripts/test_bump_version.py -k 'not TestSnapshotRegression': 215 passed, 4 deselected.git diff --check: passed.Does this PR change any dependencies?
Does this PR add a new feature? If so, have you added samples on website?