Skip to content

fix(sync): reject --subdir that escapes --dest - #13

Open
MarkAtwood wants to merge 1 commit into
masterfrom
fix/wolfglass-sync-path-guard
Open

fix(sync): reject --subdir that escapes --dest#13
MarkAtwood wants to merge 1 commit into
masterfrom
fix/wolfglass-sync-path-guard

Conversation

@MarkAtwood

Copy link
Copy Markdown
Contributor

dest_root = os.path.join(--dest, --subdir) let an absolute --subdir discard --dest entirely (os.path.join("/product", "/etc/x")/etc/x), and a --subdir containing .. walk out of the dest tree; copy mode then shutil.copy2-overwrote files there. Now requires --subdir to be relative and to resolve to a path inside --dest.

New tests/test_wolfglass_sync.py (absolute and .. rejected with nothing written outside; a relative subdir still copies in), wired into selftest.yml. Independent of the other open review PRs (only selftest.yml is also touched by #7, in a different region).

Addresses SBOM-gpex.12.

CopilotAI review requested due to automatic review settings July 24, 2026 00:08

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens tools/wolfglass-sync against path traversal/escape via --subdir so syncing cannot overwrite files outside the intended --dest tree, and adds CI-backed regression tests for the new guard.

Changes:

  • Add --subdir validation to require a relative path that resolves within --dest.
  • Add unit tests covering absolute and ..-based escape attempts, plus a valid relative subdir case.
  • Wire the new test into selftest.yml (compile + unittest execution).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

FileDescription
tools/wolfglass-syncAdds --subdir containment checks to prevent writing outside --dest.
tests/test_wolfglass_sync.pyNew tests validating the containment guard (absolute and .. rejected; relative allowed).
.github/workflows/selftest.ymlRuns the new unit test in CI and includes it in the syntax-compile step.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadtools/wolfglass-sync
Comment on lines +61 to +66
dest_abs = os.path.realpath(args.dest)
dest_root_abs = os.path.realpath(dest_root)
if dest_root_abs != dest_abs and \
not dest_root_abs.startswith(dest_abs + os.sep):
sys.exit(f"ERROR: --subdir {args.subdir!r} escapes --dest; it must "
f"resolve to a path inside {args.dest!r}.")
@sameehj
sameehjforce-pushed the master branch 4 times, most recently from 3ab77f9 to 9bdf5b7CompareJuly 24, 2026 14:09
dest_root = os.path.join(--dest, --subdir) let an absolute --subdir
discard --dest entirely, and a --subdir containing .. walk out of the
dest tree; copy mode then shutil.copy2-overwrote files there. Require
--subdir to be relative and to resolve to a path inside --dest.
Adds tests/test_wolfglass_sync.py (absolute and .. rejected, relative
copies inside) and wires it into selftest.yml.
@MarkAtwood
MarkAtwoodforce-pushed the fix/wolfglass-sync-path-guard branch from a6770c6 to 481f19fCompareAugust 13, 2026 01:15
Sign up for freeto 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.

2 participants

@MarkAtwood