fix(file_tools): allowlist macOS user-writable temp subtrees under /private/var/ by briandevans · Pull Request #11983 · NousResearch/hermes-agent

briandevans · 2026-04-18T05:40:05Z

What does this PR do?

Fixes the file-tool sensitive-path guard so it permits writes to macOS per-user temp directories (`/private/var/folders/…`, `/private/tmp/…`) while continuing to block every genuinely sensitive subtree.

Root cause

Commit 311dac1 added `/private/var/` to `_SENSITIVE_PATH_PREFIXES` to close the `/etc` → `/private/etc` symlink bypass on macOS. The prefix is correct for blocking system-owned paths, but `/private/var/folders/` is the macOS per-user temporary directory (`tempfile.mkdtemp()` returns paths there, and `/tmp` is a symlink into `/private/tmp/`).

Fix

Adds `_is_macos_user_temp(resolved, normalized)` with a dual-form AND guard: both the `realpath`-resolved form and the `normpath`-normalized form of the path must fall under an allowlisted prefix. The AND requirement defeats a symlink-substitution bypass: if an attacker creates a symlink from an allowlisted path into a blocked system path, `realpath` resolves through the symlink to the blocked target and fails the check.

Allowlisted prefixes (macOS only, gated on `sys.platform == "darwin"`):

`/private/var/folders/` and `/var/folders/` (per-user temp, both symlink forms)
`/private/tmp/` and `/tmp/` (system temp, both symlink forms)

Tests

All 6 `TestWriteInvalidatesDedup` tests now pass (they use `tempfile.mkdtemp()` which resolves to `/private/var/folders/…` on macOS and were being blocked as sensitive paths).
All 28 tests in `tests/tools/test_file_read_guards.py` pass.
No change to Linux/Windows behavior (guard is `sys.platform == "darwin"` only).

Copilot

Pull request overview

Updates the sensitive-path write guard in tools/file_tools.py to stop incorrectly blocking macOS user-temp directories that resolve under /private/var/..., and adds targeted regression tests to lock in the new behavior.

Changes:

Add a sensitive-path allowlist for macOS per-user temp subtrees and integrate it into _check_sensitive_path.
Add a focused test suite validating the allowlist and ensuring key sensitive paths remain blocked.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`tools/file_tools.py`	Introduces an allowlist and helper to permit macOS temp paths while keeping sensitive prefixes blocked.
`tests/tools/test_file_write_safety.py`	Adds regression coverage for allowlisted macOS temp paths and canaries for still-blocked sensitive locations.

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

Copilot · 2026-04-18T05:43:04Z

+_SENSITIVE_PATH_ALLOWLIST = (
+    # macOS per-user temp directory (what ``tempfile.gettempdir()`` returns
+    # after symlink resolution).  User-writable by design.
+    "/private/var/folders/",
+    # ``/tmp`` on macOS is a symlink into ``/private/tmp/``.
+    "/private/tmp/",
+)


The allowlist is applied unconditionally across all platforms. That means _check_sensitive_path("/private/var/folders/..." ) becomes allowed on Linux/Windows too (previously anything under /private/var/ was blocked). This conflicts with the PR description claim that Linux behavior is byte-for-byte identical; if the intent is strictly macOS, consider gating the allowlist logic on sys.platform == "darwin" (and adjusting/marking tests accordingly).

Copilot · 2026-04-18T05:43:04Z

    for prefix in _SENSITIVE_PATH_PREFIXES:
        if resolved.startswith(prefix) or normalized.startswith(prefix):
+            if _is_allowlisted_sensitive_path(resolved, normalized):
+                # Looks sensitive by prefix, but it's a documented
+                # user-writable location (e.g. macOS temp) — permit.
+                return None


In _check_sensitive_path, if os.path.realpath(...) raises, resolved falls back to the raw, unexpanded filepath string. Because the allowlist check later uses resolved.startswith(...), a crafted path could be allowlisted based on the raw string even when the normalized/expanded form would not be. To keep the guard consistent/fail-closed, consider setting resolved = normalized in the exception branch (or otherwise ensure the allowlist only consults expanded+normalized values when realpath fails).

briandevans · 2026-04-18T05:51:57Z

Addressed both Copilot review points in ace9ff4a:

1. Cross-platform gate — the allowlist is now gated on sys.platform == 'darwin' (_ALLOWLIST_ACTIVE). Linux and Windows behaviour is now literally byte-for-byte identical to origin/main; no user-created /private/var/folders/... tree on Linux becomes writable. The PR-description claim is now self-consistent.

2. realpath-exception bypass — closed, via two defenses stacked:

_check_sensitive_path now falls back to normalized (already sanitised via normpath + expanduser) in the realpath exception branch instead of the raw caller-supplied string. No ..-carrying path can reach the allowlist check unsanitised.
_is_allowlisted_sensitive_path is now an AND guard: both resolved and normalized must independently match an allowlisted prefix. This also defeats a symlink-substitution attack I hadn't fully considered — an attacker planting /private/var/folders/evil -> /etc/sudoers would see resolved = /etc/sudoers fail the allowlist check even though normalized is still allowlisted. Strictly stronger than the OR form.

The AND guard required one more refinement: os.path.normpath doesn't follow symlinks, so on macOS the un-resolved /var/folders/... and the realpath-resolved /private/var/folders/... forms of the same tempfile path differ. The allowlist now contains both forms (plus /tmp/ + /private/tmp/) so the AND guard can validate each independently-sanitised form against the full allowlist set.

Regression coverage:

All allowlisted-path tests use a force_allowlist_active fixture that monkeypatches _ALLOWLIST_ACTIVE = True, so they exercise the logic identically on Linux CI.
New TestAllowlistPlatformGate class verifies the darwin gate (default + forced-off behaviour).
New test_allowlist_accepts_different_prefix_forms_for_resolved_and_normalized covers the real tempfile case.
New test_realpath_exception_cannot_bypass_guard monkeypatches os.path.realpath to raise and asserts a crafted /private/var/folders/../../etc/sudoers is blocked.
test_allowlist_requires_both_resolved_and_normalized updated to the symlink-substitution framing.

Validation: python -m pytest tests/tools/test_file_write_safety.py tests/tools/test_file_staleness.py -q -> 42 passed.

…te/var/ The /private/var/ sensitive-path prefix (added in 311dac1 to close the /etc→/private/etc symlink bypass) is too broad on macOS: it also blocks writes to /private/var/folders/, which is the realpath of the macOS per-user temporary directory (tempfile.mkdtemp(), /tmp). Add _is_macos_user_temp() with a dual-form AND guard: both the realpath- resolved path and the normpath-normalized path must fall under an allowlisted prefix (/private/var/folders/, /var/folders/, /private/tmp/, /tmp/). The AND requirement defeats a symlink-substitution bypass where an attacker creates a symlink from an allowlisted path into a blocked system path — realpath would resolve through to the blocked target and fail the check. Fixes TestWriteInvalidatesDedup tests that use tempfile.mkdtemp() (paths land in /private/var/folders/ on macOS) and were blocked as sensitive. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

briandevans · 2026-05-18T04:13:15Z

Closing to keep the queue clean — happy to reopen if this is still useful.

Copilot AI review requested due to automatic review settings April 18, 2026 05:40

Copilot started reviewing on behalf of briandevans April 18, 2026 05:40 View session

Copilot AI reviewed Apr 18, 2026

View reviewed changes

This was referenced Apr 18, 2026

fix(run_agent): preserve dotted Bedrock inference-profile model IDs (#11976) #11992

Closed

fix(delegate): read fresh config.yaml so delegation.model edits take effect (#11999) #12053

Closed

alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) labels Apr 24, 2026

briandevans force-pushed the fix/sensitive-path-macos-temp-allowlist branch from ace9ff4 to ae9224a Compare April 27, 2026 06:31

briandevans closed this May 18, 2026

This was referenced May 23, 2026

fix(file-tools): allow writes to platform temp directory on macOS #31021

Open

fix(file-tools): allow macOS var folders temp writes #32344

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(file_tools): allowlist macOS user-writable temp subtrees under /private/var/#11983

fix(file_tools): allowlist macOS user-writable temp subtrees under /private/var/#11983
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/sensitive-path-macos-temp-allowlist

briandevans commented Apr 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

briandevans commented Apr 18, 2026

Uh oh!

briandevans commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

briandevans commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Root cause

Fix

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

briandevans commented Apr 18, 2026

Uh oh!

briandevans commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

briandevans commented Apr 18, 2026 •

edited

Loading