Skip to content

fix(file-tools): ensure OS-agnostic sensitive path protection - #10682

Open
Junass1 wants to merge 1 commit into
NousResearch:mainfrom
Junass1:fix/os-agnostic-path-security
Open

fix(file-tools): ensure OS-agnostic sensitive path protection#10682
Junass1 wants to merge 1 commit into
NousResearch:mainfrom
Junass1:fix/os-agnostic-path-security

Conversation

@Junass1

@Junass1 Junass1 commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Normalize _check_sensitive_path() comparisons so sensitive POSIX targets stay blocked even when Hermes is running on a Windows host.

Problem

_check_sensitive_path() compared realpath() / normpath() output directly against POSIX-sensitive prefixes and exact paths such as:

  • /private/etc/
  • /private/var/
  • /var/run/docker.sock

On Windows hosts, those path helpers can produce backslash-normalized paths, which makes direct string comparisons host-OS dependent and risks missing sensitive POSIX targets.

Fix

  • Add a small path normalizer that:
    • expands ~
    • normalizes the path
    • converts backslashes to forward slashes
    • strips trailing slashes consistently
  • Use that normalized form for both resolved and literal-path comparisons in _check_sensitive_path()

Tests

Added regression coverage for Windows-host semantics by monkeypatching realpath() / normpath() to return backslash-normalized paths and verifying that:

  • /var/run/docker.sock is still blocked
  • /private/etc/hosts is still blocked
  • /tmp/safe_file.txt is still allowed

Verification

Passed locally via:

  • uv run pytest tests/tools/test_file_write_safety.py -q
  • uv run pytest tests/tools/test_write_deny.py -q

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) labels Apr 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #4993 — both address cross-platform path guard bypass where backslash-normalized paths on Windows skip POSIX-sensitive path checks in _check_sensitive_path().

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying the Windows-sensitive-path gap. The premise still holds on current main: tools/file_tools.py:639 uses host-specific os.path.normpath, then tools/file_tools.py:644-648 compares that value directly to forward-slash POSIX literals.

Problems

  • The submitted hunk predates the current resolver. _check_sensitive_path now accepts task_id and resolves via _resolve_path_for_task(filepath, task_id) at tools/file_tools.py:633-639; the old os.path.realpath-based context in this PR no longer matches current main.
  • Current regression coverage at tests/tools/test_file_write_safety.py:171-196 covers POSIX/macOS spellings but not Windows-style normalization.

Suggested changes

  • Reapply the normalizer to both current comparison operands without replacing the task-aware resolver.
  • Port the exact-path, prefix-path, and safe-path Windows-normalization tests to the current test file.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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