Skip to content

fix: guard V4A Move File patch paths - #47380

Closed
rebel0789 wants to merge 1 commit into
NousResearch:mainfrom
rebel0789:audit/fresh-bugs-2026-06-16
Closed

rebel0789 wants to merge 1 commit into
NousResearch:mainfrom
rebel0789:audit/fresh-bugs-2026-06-16

Conversation

@rebel0789

Copy link
Copy Markdown

Summary

  • Extend V4A patch path extraction in \ to include \ headers.
  • Validate both move source and destination paths before dispatching to , so traversal, sensitive-path, cross-profile, and lock checks cover move operations too.
  • Align guard extraction with the V4A parser's accepted optional whitespace after , including .

Discovery / root cause

I found this by auditing file-tool path safety directly rather than working from an existing issue. The V4A parser accepts \ operations, but \ only extracted paths from \ headers before running the path guard pipeline. A \ header could therefore skip the preflight traversal/cross-profile/sensitive-path checks that other V4A operations receive.

Duplicate check: searched upstream issues for V4A/Move File/cross-profile patch variants and did not find an existing tracked issue for this specific bug.

Behavior before

A patch like this reached \ instead of being rejected by the path guard:

\\

The parser also accepts the no-space form , while the old guard regex required whitespace after .

Behavior after

  • \ source and destination paths are collected into the same guard list as \ paths.
  • Parser-accepted no-space headers are guarded before \ runs.
  • Traversal in either source or destination returns an error and does not invoke .

Test plan

  • ...... [100%]
    6 passed in 0.21s
    • Result: \
  • Static added-line scan for hardcoded secrets, shell injection, eval/exec, pickle, and simple SQL string formatting: no findings.
  • Independent reviewer pass: no blocking security or logic concerns after aligning the regex with parser spacing; added the reviewer-suggested source-path traversal regression test.

Copilot AI review requested due to automatic review settings June 16, 2026 17:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround tool/file File tools (read, write, patch, search) duplicate This issue or pull request already exists labels Jun 16, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

Duplicate of #9833 — both extend the V4A patch path-extraction in patch_tool (tools/file_tools.py) to cover *** Move File: source+destination headers and the no-space header variant, so move operations get the same traversal / sensitive-path / cross-profile guard checks before dispatch. Same code path, same mechanism; #9833 is the earlier open canonical. See also #29424 (already marked duplicate of #9833) and the multi-surface hardening in #41754.

@rebel0789

Copy link
Copy Markdown
Author

haha , i let hermes deal with the pr and didnt checked for known issues my bad

@rebel0789 rebel0789 closed this Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists P1 High — major feature broken, no workaround tool/file File tools (read, write, patch, search) type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants