fix(file): harden read_file device alias blocking - #50221
Conversation
Security-hardening fix for the read_file device guard, not a new sandbox
boundary. The guard already rejects direct device paths and upstream now
has a resolved-path pass for workspace symlinks to blocked devices, but
its concrete-path helper still compared the expanded path before
normalization. That leaves residual alias cases where the dangerous path
is visible before final terminal-specific resolution, for example:
1. /dev/../dev/zero and /dev/./urandom should match the blocked-device
list as concrete paths, not only after final realpath;
2. /dev/stdin-style aliases can disappear once realpath follows them
to /proc/self/fd/0 and then to a tty path;
3. a user symlink to /dev/../dev/stdin exposes the dangerous
intermediate target before final resolution, but not necessarily
after it.
Normalize expanded paths before matching and inspect each symlink hop
before falling back to realpath. This preserves the existing /proc fd and
/proc pseudo-file guards while enforcing the intended security invariant:
model-supplied read paths must not reach blocking or infinite device
streams through spelling, normalization, or symlink-hop tricks.
Classification: security hardening / residual bypass fix for the
read_file device blocklist. This is defensive code at the file-tool
boundary, but it fixes a concrete denial-of-service class tracked as
security in #10141 and #29158.
Tests:
- normalized /dev/../dev/zero and /dev/./urandom aliases
- symlink to /dev/../dev/stdin blocked before realpath
- existing symlink-to-device and regular-symlink guards still pass
Fixes #10141
Fixes #29158
🔎 Lint report:
|
|
Related: #34466 (original by @egilewski, salvaged here), tracking advisory #29158 (GHSA-3p9q-rxhm-5x3g), and #10141 (symlink-bypass facet). Salvage onto current main with authorship preserved. |
|
Follow-up: I rebased and force-pushed the original #34466 branch onto current #34466 now has the original hardening commit plus a separate signed follow-up for the
That follow-up covers the issue found while reviewing this salvage PR: a relative workspace symlink to Given that the original branch now carries the current-base fix directly, this salvage PR should not need another cherry-pick of the follow-up fix. Signed: GPT-5.5 in Codex |
Summary
Closes two residual
read_filedevice-blocklist bypasses on top of the existingrealpathpass:normpath-style path aliases (/dev/../dev/zero) and symlinks whose intermediate hop is a blocked stdio alias thatrealpathresolves through to a non-blocked tty.Changes
tools/file_tools.py:_is_blocked_device_pathnormalizes vianormpathbefore matching;_is_blocked_devicewalks each symlink hop (20-hop cap + cycle guard) and checks it against the blocklist before the finalrealpathcheck. Existing/proc/*/{fd,environ,cmdline,maps}protections preserved.tests/tools/test_file_read_guards.py: regression coverage for/dev/..//dev/.aliases and a symlink to a blocked alias.Validation
symlink -> /dev/stdinsymlink -> /dev/../dev/stdin/dev/../dev/zero,/dev/./urandom/dev/null,/tmp/test.py,/proc/cpuinfotests/tools/test_file_read_guards.py: 41/41 pass. E2E verified with real imports against a temp HERMES_HOME (all 11 cases above).Salvage of #34466 by @egilewski (cherry-picked onto current main, authorship preserved). Closes #10141 and tracking advisory #29158 (GHSA-3p9q-rxhm-5x3g).
Infographic