[Rana Singh (ranadeepsingh)]

Related Issues/PRs

Related: #2628 also changes this script and must preserve these safeguards when integrated.

What changes are proposed in this pull request?

Prevent the version-bump script from changing files outside the selected repository when a file, parent directory, or repository-root component is replaced with a symlink.

The script refuses symlinks throughout root acquisition, using macOS's all-components no-follow flag or directory-relative component opens on Linux. Search-only ancestors remain supported. It then retains the root descriptor for reads, writes, discovery, validation, and docs working-directory selection. It rejects unsafe write targets and unsupported platforms, closes descriptors on failure, and leaves normal version replacements unchanged. Regression tests cover root-leaf and ancestor swaps, search-only ancestors, descriptor cleanup, and positive sweep warnings.

This is a standalone release-tooling fix: no product APIs, dependencies, pipeline definitions, or workflows change. POSIX descriptor support is required; Windows users can use WSL. External sbt/npm build definitions are not sandboxed.

Status: fix published in 82c7f9f723cbf2c810b34414b15a3afcd6dbca49; all four review threads are resolved. The maintainer-requested single light review is clean (runtime reviewer: gpt-6-astra), and the current-head automated review reports no open issues. Its Linux-validation concern is addressed by 57 passing non-root Linux integration tests on the unchanged commit. Azure build 243463209 succeeded on the exact target/head merge. GitHub has no failed, pending, or missing required checks, and the branch is up to date with master. Maintainer approval is the remaining merge gate. The independently reproduced local baseline snapshot failure remains disclosed below.

How is this patch tested?

• I have written tests (not required for typo or doc fix) and confirmed the proposed feature/bug-fix/change works.

• Before the root-acquisition fix, the new ancestor-swap CLI regression failed: the script reported success against the outside fixture.

• Before the permission-compatibility follow-up, PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python3 -I -B -m pytest -q -p no:cacheprovider scripts/test_bump_version.py -k 'cli_supports_search_only_ancestor or sweep_reports_unscanned_files': 1 failed, 1 passed; the search-only ancestor failed with a permission error.

• On the final local patch based on e58441ffd60e5a2f64b83e6521154ef840b85091, PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python3 -I -B -m pytest -q -p no:cacheprovider scripts/test_bump_version.py -k 'not test_no_unexpected_unanchored': 259 passed, 1 deselected.

• The deselected TestSnapshotRegression.test_no_unexpected_unanchored reports unrelated version strings in docs and existing review artifacts. The earlier full-suite run had 255 passed, 1 baseline failure, before four follow-up cases were added.

• On the unchanged target, PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python3 -I -B -m pytest -q -p no:cacheprovider scripts/test_bump_version.py -k TestSnapshotRegression: 3 passed, 1 failed, with the same unanchored references. The assertion was not weakened.

• Black 22.3.0, python -I -m black --check scripts/bump-version.py and python -I -m black --check scripts/test_bump_version.py: passed.

• git diff --check: passed.

• Scala compilation, codegen, and Spark tests were not run locally; no Scala/product code changes. The script regressions run locally, not through the current Azure pipeline. Full Azure CI is assessed separately.

• On the unchanged pushed commit, an isolated Linux aarch64/Python 3.11.18 container ran as UID 65534 with a read-only source mount and network disabled during tests. PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -I -B -m pytest -v -rs -p no:cacheprovider scripts/test_bump_version.py -k 'TestIntegration or TestDocsCommands': 57 passed, 203 deselected, zero skips. This exercises Linux O_PATH, search-only ancestors, root/ancestor swaps, descriptor cleanup, and docs child pre-exec success/failure paths. The temporary VM and containers were stopped after validation.

• Earlier review findings and their dispositions are retained under reviews/pr-2789/. The maintainer explicitly requested one light review in place of further six-round passes; task-unknown-attempt-9-review-0-harness.md records its clean result.

• Azure build 243463209 completed successfully on merge 1e321f25fd4879b99a88c5730c3b97c1846f7f92 (target e58441ffd60e5a2f64b83e6521154ef840b85091, head 82c7f9f723cbf2c810b34414b15a3afcd6dbca49). All 65 executed jobs succeeded. language and lightgbm5 initially failed during Azure TLS authentication setup, before tests ran; one targeted retry produced 98 and 35 passing tests respectively, with no skips, while preserving the other 63 successful jobs.

• InternalCompat and FabricE2E were intentionally excluded by the existing fork-PR conditions. The website coverage warning is expected for its out-of-process Spark tests; its published test run passed 111/111. Existing non-gating task-runtime and dependency-scan warnings are not claimed fixed by this PR.

Does this PR change any dependencies?

• No. You can skip this section.
• Yes. Make sure the dependencies are resolved correctly, and list changes here.

Does this PR add a new feature? If so, have you added samples on website?

• No. You can skip this section.
• Yes. Make sure you have added samples following below steps.

1. Find the corresponding markdown file for your new feature in website/docs/documentation folder.
   Make sure you choose the correct class estimators/transformers and namespace.
2. Follow the pattern in markdown file and add another section for your new API, including pyspark, scala (and .NET potentially) samples.
3. Make sure the DocTable points to correct API link.
4. Navigate to website folder, and run npm start to make sure the website renders correctly.
5. Don't forget to add <!--pytest-codeblocks:cont--> before each python code blocks to enable auto-tests for python samples.
6. Make sure the WebsiteSamplesTests job pass in the pipeline.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[github-actions]

Hey Rana Singh (@ranadeepsingh) 👋!
Thank you so much for contributing to our repository 🙌.
Someone from SynapseML Team will be reviewing this pull request soon.

We use semantic commit messages to streamline the release process.
Before your pull request can be merged, you should make sure your first commit and PR title start with a semantic prefix.
This helps us to create release messages and credit you for your hard work!

Examples of commit messages with semantic prefixes:

• fix: Fix LightGBM crashes with empty partitions
• feat: Make HTTP on Spark back-offs configurable
• docs: Update Spark Serving usage
• build: Add codecov support
• perf: improve LightGBM memory usage
• refactor: make python code generation rely on classes
• style: Remove nulls from CNTKModel
• test: Add test coverage for CNTKModel

To test your commit locally, please follow our guild on building from source.
Check out the developer guide for additional guidance on testing your change.

[Copilot]

🟡 Changes recommended

Concurrent path replacement can bypass the checks and redirect a write outside the selected repository.

1 open issue

• Make writes race-safe against symlink replacement · New

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}

[Copilot]

🟡 Changes recommended

Root-directory swaps can still permit outside writes, and unsupported platforms can report success without updating files.

2 open issues

• Retain validated root descriptor to prevent symlink containment bypass · New
• Reject unsupported safe-dir-fd platforms at CLI startup · New

1 resolved since last review

• Make writes race-safe against symlink replacement

_{Review effort: Balanced}

[Rana Singh (ranadeepsingh)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🟡 Changes recommended

A verified ancestor-symlink race during initial root acquisition can redirect version writes into an outside repository.

1 open issue

• Prevent ancestor symlink escape during repository path opening · New

2 resolved since last review

• Retain validated root descriptor to prevent symlink containment bypass
• Reject unsupported safe-dir-fd platforms at CLI startup

_{Review effort: Balanced}

[Rana Singh (ranadeepsingh)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🔵 Needs a closer look

Platform-specific containment and child pre-exec behavior need maintainer confirmation because Linux execution remains unverified and current-head CI is still pending.

No open issues 🎉

1 resolved since last review

• Prevent ancestor symlink escape during repository path opening

_{Review effort: Balanced}

[Rana Singh (ranadeepsingh)]

Follow-up to the current-head automated review: Linux containment and child pre-exec behavior are now exercised on the unchanged pushed head 82c7f9f. In an isolated Linux aarch64 container (Python 3.11.18, UID 65534, read-only source mount, network disabled during tests), PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -I -B -m pytest -v -rs -p no:cacheprovider scripts/test_bump_version.py -k "TestIntegration or TestDocsCommands" passed 57 tests with no skips. This includes native Linux O_PATH ancestor/root swaps, search-only ancestors, descriptor cleanup, and docs child-cwd success/failure paths. The source fingerprint is unchanged; no additional code or review round was needed. Azure build 243463209 remains the separate repository-wide CI gate.

[Rana Singh (ranadeepsingh)]

Final validation: Azure build 243463209 succeeded on the reviewed head 82c7f9f merged with current master. The two infrastructure-failed jobs passed on attempt 2: language 98/98 and lightgbm5 35/35, with no skips. All 63 previously successful jobs were preserved. All four review threads are resolved, the single requested light review is clean, and non-root Linux regression evidence is linked in the description. Maintainer approval is the remaining merge gate; no merge has been performed.