Skip to content

fix(permissions): stop decoy raw keys from shadowing real tool paths - #351

Open
Mr-Neutr0n wants to merge 1 commit into
HKUDS:mainfrom
Mr-Neutr0n:fix/permission-path-shadowing
Open

fix(permissions): stop decoy raw keys from shadowing real tool paths#351
Mr-Neutr0n wants to merge 1 commit into
HKUDS:mainfrom
Mr-Neutr0n:fix/permission-path-shadowing

Conversation

@Mr-Neutr0n

Copy link
Copy Markdown

Fixes #348

Root cause

_resolve_permission_file_path in src/openharness/engine/query.py checked raw tool_input keys (file_path first) before the validated model, but execution runs on parsed_input onlytool.execute(parsed_input, ...). Since every built-in file tool's schema field is path (read_file, write_file, edit), a model-supplied file_path key is dropped by Pydantic as an unknown field while still winning the permission check. Deny rules evaluated the decoy; execution touched the real path.

The same raw-first pattern existed in _extract_permission_command.

Change

Both resolvers now treat validated model fields as authoritative and fall back to raw keys only for dynamic-schema tools (MCP proxies) whose parsed models expose no path/command attributes:

# parsed first — execution uses exactly these values
for attr in ("file_path", "path", "root"):
    value = getattr(parsed_input, attr, None)
    ...
# fallback for dynamic-schema tools
for key in ("file_path", "path", "root"):
    value = raw_input.get(key)

Verification

Engine-level regressions reproduce the issue PoC without any live model endpoint:

  • read_file with {"file_path": "README.md", "path": "work/blocked/secret.txt"} under a deny rule → blocked (was: returned file contents)
  • write_file with the same decoy pair under FULL_AUTO + deny rule → blocked, nothing written (was: wrote into denied path)
  • Control cases preserved: no-decoy calls still deny; a real allowed path with a decoy pointing at a denied one still succeeds (no over-blocking)
pytest tests/test_engine/test_query_engine.py -k decoy        # 2 failed on vulnerable code -> 3 passed with fix
pytest tests/test_engine tests/test_permissions tests/test_tools  # 190 passed; identical 3 pre-existing env failures on clean main

This change was prepared with AI assistance under human direction and review.

The permission checker resolved file paths (and commands) from raw
tool_input before the validated model, while execution runs on
parsed_input only. A model-supplied file_path key that Pydantic drops
(read_file/write_file/edit all use path) therefore shadowed the real
path during evaluation, letting deny-ruled paths slip through in
full_auto mode via prompt injection (issue HKUDS#348).

Parsed fields are now authoritative; raw keys remain a fallback for
dynamic-schema tools such as MCP proxies whose models expose no path
attributes. Adds engine-level regressions reproducing the PoC pair for
read_file and write_file; both fail on vulnerable code.

Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>
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.

[Bug]: OpenHarness path deny bypass through parameter shadowing

1 participant