Skip to content

Comments

fix: resolve symlinks in safe_path to prevent false rejections#648

Merged
ChuxiJ merged 1 commit intomainfrom
fix/safe-path-symlink-resolution
Feb 20, 2026
Merged

fix: resolve symlinks in safe_path to prevent false rejections#648
ChuxiJ merged 1 commit intomainfrom
fix/safe-path-symlink-resolution

Conversation

@ChuxiJ
Copy link
Contributor

@ChuxiJ ChuxiJ commented Feb 20, 2026

The path safety module used os.path.normpath(os.path.abspath(...)) which does not resolve symlinks. On systems where the working directory is accessed through a symlink (e.g. /root/data -> /vepfs-d-data/q-ace), os.getcwd() returns the real path while user-entered paths retain the symlink prefix, causing startswith() to fail and legitimate dataset paths to be rejected as unsafe.

Switch to os.path.realpath() for all path normalisation so that both the safe root and user paths are resolved to their canonical form before comparison.

Summary by CodeRabbit

  • Bug Fixes
    • Improved path validation to consistently resolve symbolic links during security checks, ensuring more reliable and predictable path access control.

The path safety module used os.path.normpath(os.path.abspath(...)) which
does not resolve symlinks. On systems where the working directory is
accessed through a symlink (e.g. /root/data -> /vepfs-d-data/q-ace),
os.getcwd() returns the real path while user-entered paths retain the
symlink prefix, causing startswith() to fail and legitimate dataset
paths to be rejected as unsafe.

Switch to os.path.realpath() for all path normalisation so that both
the safe root and user paths are resolved to their canonical form
before comparison.
@ChuxiJ ChuxiJ merged commit 731bb9b into main Feb 20, 2026
1 of 2 checks passed
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 20, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Introduces a private _resolve helper function to normalize and resolve symlinks in file paths. Updates safe_path validation and safe root initialization to consistently use realpath-based resolution for both root and user paths before prefix validation. Public interfaces remain unchanged.

Changes

Cohort / File(s) Summary
Path Resolution Enhancement
acestep/training/path_safety.py
Added _resolve helper for consistent symlink-aware path normalization. Updated safe root initialization and set_safe_root to use resolver. Modified safe_path to resolve relative user paths against root using realpath before validation. Error messages now reference resolved paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Symlinks and paths, now resolved with care,
The rabbit hops through each hidden lair,
No more confusion, no twisted trail,
With _resolve helper, safety won't fail!

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/safe-path-symlink-resolution

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Comment @coderabbitai help to get the list of available commands and usage tips.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant