Use validate_relative_path (not assert) for new-file path confinement in create_text_file#1589
Open
alex-schose wants to merge 1 commit into
Open
Conversation
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.
CreateTextFileTool.applyguards the new-file branch withassert abs_path.is_relative_to(project_root), while the overwrite branch uses thealways-on
validate_relative_path. Python stripsassertstatements under-O/PYTHONOPTIMIZE, so when the server runs optimized, the new-file branch performs nopath-containment check and a
create_text_filecall with a../relative path canwrite outside the project root (
Path.resolve()collapses the..).This change makes the new-file branch use the same always-on guard as the overwrite
branch — consistent behavior, not dependent on assertions being enabled. It also keeps
the new-file path consistent with Serena's documented symlink stance
(
validate_relative_path→is_path_in_projectis lexical), unlike the prior.resolve()-based check.Low severity (requires running under
-O); filed as a defense-in-depth hardening fix.