fix(security): enforce owner-only ACLs on Windows in _secure_file - #77527
fix(security): enforce owner-only ACLs on Windows in _secure_file#77527andrexibiza wants to merge 3 commits into
Conversation
alt-glitch
added
type/bug
Native Win11 verification (monerostar)Host: Windows 11 (build 26200), Python 3.11.15. Checked out this branch against current upstream chmod is a no-op for ACLs (bug confirmed)Fresh temp So main's This PR fixes itAfter Inherited SYSTEM/Administrators entries removed; owner-only full control. Same result after TestsIncluding Notes
LGTM — real at-rest ACL posture on Windows for secret files. (Approve blocked for non-maintainer; treating this as a verified +1 with live evidence.) |
SummaryOne pull request addresses Issue #77462. Its diff targets the Windows Related pull requests
Suggested consolidationkeep open with a salvage path: retain the Windows ACL helper, Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I77462(["issue #77462 (open)"])
P77527["PR #77527 (open)"]
P77527 -->|fixes| I77462
class I77462 open
class P77527 open
class P77527 target
click I77462 "https://github.com/NousResearch/hermes-agent/issues/77462"
click P77527 "https://github.com/NousResearch/hermes-agent/pull/77527"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 8 kB of PR diffs, 5 kB of issue/PR text, 2 kB of discussion (2 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
The helpers were tested; nothing proved main.ts called them. Reverting both call sites and both imports in readDesktopConnectionConfig / writeDesktopConnectionConfig left the whole suite green (947 passed / 2 skipped, tsc 0, eslint clean, e2e 1 passed 1 skipped) while connection.json went back to 0644 — the user-visible fix this PR promises was untested. The e2e spec could not catch it by construction: it asserts the ENCRYPTION contract with a raw-bytes scan, and safeStorage keeps the token opaque regardless of the file's mode, so a 0644 file passes that scan every time. There was no mode assertion anywhere in e2e/. Adds the missing third contract — unreadable by other local accounts — on all three paths that can produce the file: - write: assert the mode of the artifact test 1 already proves the app wrote. - read, valid file: seed the app's own encrypted connection.json back to 0644 and assert launch tightens it. Scoped to the MODE only, so it is independent of the still-deferred plaintext migration — the fixture's token is already ciphertext, so nothing re-encrypts, no NousResearch#62319 opt-in marker is involved, and no rotation guidance is owed. - read, corrupt file: a truncated file still holds the token bytes and throws into the swallowing catch, so it would be the one file never tightened. This is the only test that distinguishes the chmod's placement relative to the parse. Also moves the tighten above JSON.parse for exactly that reason, and pins the cache invariant the placement depends on: the tighten must be a chmod, not a rewrite, because it sits inside the function whose cache keys on mtimeMs. Asserted as `mode & 0o077 === 0` rather than `=== 0o600` to avoid a change-detector, and skipped on win32, where chmod maps to the read-only bit and the fix deliberately no-ops (ACLs are PR NousResearch#77527). Every assertion was mutation-tested: reverting the full wiring fails all three; reverting only the write path fails only the write test; deleting only the tighten-on-read fails only the two read tests; moving the tighten below the parse fails only the corrupt test; making the tighten a rewrite instead of a chmod fails the mtime assertions. Bundle greps confirmed each mutation reached dist/electron-main.mjs before the run.
|
Thank you for the native Win11 verification — the BEFORE/AFTER On the triage scope note (auth.json, state.db, secret-cache paths): agreed, this PR deliberately scopes to |
Copilot flagged that _open_launch_stderr_log() left an *existing* log's
mode alone: O_CREAT applies its mode argument only to a file it actually
creates, so a pre-existing 0644 log stayed 0644. Only fresh creates got
0600. Confirmed behaviourally against a temp HERMES_HOME under umask 022.
That is the case that matters. launch-stderr.log is the one artifact in
this PR with a fixed, guessable name (everything else is uuid4-named), so
under the documented HERMES_HOME_MODE=0701 traversal hatch it is the one
another local account can open by guess without a listable directory. An
older Hermes wrote it with a plain open() and left it 0644 on disk, so
every *upgrading* install kept exactly the exposure this PR claims to
close — contradicting the PR's own rationale for reconciling the profile
directory unconditionally ("that exposure is on disk today").
Reconciled through hermes_cli.config._secure_file rather than a
hand-rolled os.chmod, matching how the other three sites in this PR
delegate to _secure_dir: that helper is the single owner of the
owner-only file policy, it skips managed/NixOS installs and containers
where broader modes are deliberate, and it is where Windows ACL
enforcement lands (NousResearch#77527), so this inherits that instead of growing a
second implementation. The reconcile runs after the truncating open and
before any bytes are written, so the tighten lands while the file is
empty and no fresh Chromium stderr ever sits in a widely-readable file.
Safe against a running browser for the same reason the directory tighten
is: only group/other bits drop, the owner keeps rw, and POSIX checks the
mode at open() rather than on an already-open descriptor.
Also extends the managed/NixOS carve-out to the log's *creation*, which
the previous commit fixed for the directories and missed here. The log is
created lazily at runtime and is not in the module's systemd.tmpfiles
rules, so a hardcoded 0600 was the only thing setting its mode. Measured
under real managed conditions (2770 parent, UMask=0007): merge base 0o660,
this PR before 0o600, now 0o660. That regression is launch-breaking, not
cosmetic — the gateway and an interactive hostUsers CLI share one
$HERMES_HOME at two uids through the hermes group, every candidate binary
reuses this one log path, and an EACCES opening it makes launch_chrome_debug
report spawn-failed for every candidate (reproduced: launched=False).
Docstring fix: tools/computer_use/tool.py claimed "mode= ... is not masked
by umask", which is wrong — mkdir(mode=) IS subject to umask; it just
cannot widen the group/other bits from 0o700. Now matches the already
correct wording in tools/vision_tools.py.
Non-managed behaviour is unchanged. Re-measured all six scenarios from the
PR body (default, HERMES_SKIP_CHMOD=1, HERMES_CONTAINER=1,
HERMES_HOME_MODE=0701, managed with a pre-existing dir, managed with the
dir absent) across chrome-debug, cache/vision and cache/video: every
directory row is byte-identical to before this commit.
Tests: 6 new, all asserting the contract (no group/other bits; owner keeps
access) rather than a frozen octal. 4 of the 6 fail without this change.
alt-glitch
added
type/security
monerostar
left a comment
monerostar
left a comment
There was a problem hiding this comment.
Native Win11 live-verify (monerostar)
Host: Windows 11 build 26200 · CPython 3.11.15 · install tree %LOCALAPPDATA%\hermes\hermes-agent
PR tip: 9bcbbc562 · control: live main on same box
Bug still on main
_secure_file docstring still says no-op on Windows. Live call only leaves inherited ACLs in place. os.chmod(0o600) does not drop SYSTEM/Administrators.
Fixture: temp secret.env with normal inherited ACL, then call real _secure_file.
| Tree | has _secure_file_windows_acl |
Administrators after | SYSTEM after | owner after |
|---|---|---|---|---|
| main | no | yes (I)(F) | yes (I)(F) | yes |
| this PR | yes | gone | gone | DUSTIN7600X\Admin:(F) only |
Tests
pytest tests/hermes_cli/test_config.py -o addopts= -k "secure_file_restricts_acls or save_env_value_restricts_acls"
# 2 passed (Windows ACL E2E + save_env_value rotation path)
Notes
- icacls argv form looks right (no shell). Failures swallowed same as POSIX branch.
- Doc first line still says "No-op on Windows" while the body describes the Win path. Cosmetic only; behavior matches the body + tests.
- Multi-profile boxes write real
.envunder profile homes. Closing the inherited-Administrators hole matters here.
Looks good from this daily driver.
`connection.json` under the desktop app's Electron `userData` was written with no
file mode, so it landed at the `0644` umask default — while its two
credential-bearing neighbours in the same directory, `desktop-installation.json`
and `native-oauth-tokens.json`, were already `0600`. That file holds the
safeStorage-encrypted gateway token plus the fields that are NOT encrypted: the
gateway URL and the SSH host, user, and key path.
- Route the single write choke point through a helper that creates the file
owner-only and atomically.
- Tighten an already-existing `0644` file once per launch on the read path, so
installs that already have one do not stay world-readable until the next save.
- Refuse to act on a path that is a symlink or not owned by the current user,
matching the guards `desktop-installation.ts` already applies to its sibling.
The symlink guard alone turned out to be insufficient, and that is worth
recording: `writeSecretFileAtomic` tightens its *temp* path, so a symlink planted
at `connection.json.tmp` meant `writeFileSync` followed it, the guard correctly
bailed, and `renameSync` then moved the link onto `connection.json` permanently.
Measured, guard-only vs. as-landed:
guards only token leaked: true config is a symlink: true 755
guards + temp unlink token leaked: false config is a symlink: false 600
So the temp path is unlinked before the write.
Issue #77486's headline claim — that a dashboard session token is persisted in
plaintext — does not hold against main. The token has been safeStorage-encrypted
since the desktop app reached mainline in 51c68d4, and `encryptDesktopSecret`
aborts with an actionable message rather than degrading to plaintext when
safeStorage is unavailable. The `{ encoding: 'plain', value }` literal does exist
at main.ts:7084, but only on the `persistToken: false` branch, whose sole caller
is the connection-test handler, which never writes. So no mainline path *writes*
a plaintext token. The commits that did contain a plaintext-writing fallback
(d3d1772, d208f2c) are not ancestors of main — they live only on
upstream/bb/gui-* and the desktop-pr20059-installers pre-release tag.
At-rest migration of legacy non-safeStorage payloads is deliberately NOT included.
An earlier revision of this branch implemented it and it was removed after review
reproduced two token-loss paths: it force-converts the opt-in plaintext choice
PR #62319 adds (silently reverting the user's decision, then destroying the token
on the next launch without the `--password-store=basic` flag), and it converts a
portable credential into a keychain-bound one with no consent — destroying the
only recoverable copy while not remediating the real exposure, since every
existing backup still holds the plaintext and the true remedy is rotation. It also
persisted raw `parsed`, bypassing `sanitizeConnectionProfiles`. A comment at the
read path records the three preconditions any future attempt needs.
`decryptDesktopSecret`'s non-safeStorage read fallback is untouched — it is what
lets a pre-release or hand-edited config work at all.
Windows still inherits the userData directory ACL rather than an explicit
owner-only one; mode bits are advisory there, so that half is deferred to
PR #77527 rather than growing a second ACL implementation here.
e2e: `at-rest-connection-token.spec.ts` asserts the at-rest contract
implementation-independently — the token's plaintext value (and its base64 form)
must not appear in a raw-bytes scan of any file under userData or HERMES_HOME,
AND the app must still put the exact original token on the wire after a restart,
so a fix that simply drops the token cannot pass. Proven non-vacuous by mutation:
writing `{ encoding: 'plain', value }` still fails the scan while the
file-exists and gateway-URL guards pass. The migration case is a documented
`test.fixme` naming its three blockers.
Electron project 928 -> 924 tests (-9 migration, +5 new guard and
mechanism-isolation). Two of those five exist because reverting either owner-only
mechanism alone initially scored zero failures — they were masking each other, so
either could have been deleted green.
(cherry picked from commit 6e01add)
The helpers were tested; nothing proved main.ts called them. Reverting both call sites and both imports in readDesktopConnectionConfig / writeDesktopConnectionConfig left the whole suite green (947 passed / 2 skipped, tsc 0, eslint clean, e2e 1 passed 1 skipped) while connection.json went back to 0644 — the user-visible fix this PR promises was untested. The e2e spec could not catch it by construction: it asserts the ENCRYPTION contract with a raw-bytes scan, and safeStorage keeps the token opaque regardless of the file's mode, so a 0644 file passes that scan every time. There was no mode assertion anywhere in e2e/. Adds the missing third contract — unreadable by other local accounts — on all three paths that can produce the file: - write: assert the mode of the artifact test 1 already proves the app wrote. - read, valid file: seed the app's own encrypted connection.json back to 0644 and assert launch tightens it. Scoped to the MODE only, so it is independent of the still-deferred plaintext migration — the fixture's token is already ciphertext, so nothing re-encrypts, no #62319 opt-in marker is involved, and no rotation guidance is owed. - read, corrupt file: a truncated file still holds the token bytes and throws into the swallowing catch, so it would be the one file never tightened. This is the only test that distinguishes the chmod's placement relative to the parse. Also moves the tighten above JSON.parse for exactly that reason, and pins the cache invariant the placement depends on: the tighten must be a chmod, not a rewrite, because it sits inside the function whose cache keys on mtimeMs. Asserted as `mode & 0o077 === 0` rather than `=== 0o600` to avoid a change-detector, and skipped on win32, where chmod maps to the read-only bit and the fix deliberately no-ops (ACLs are PR #77527). Every assertion was mutation-tested: reverting the full wiring fails all three; reverting only the write path fails only the write test; deleting only the tighten-on-read fails only the two read tests; moving the tighten below the parse fails only the corrupt test; making the tighten a rewrite instead of a chmod fails the mtime assertions. Bundle greps confirmed each mutation reached dist/electron-main.mjs before the run. (cherry picked from commit 99cfc16)
`connection.json` under the desktop app's Electron `userData` was written with no
file mode, so it landed at the `0644` umask default — while its two
credential-bearing neighbours in the same directory, `desktop-installation.json`
and `native-oauth-tokens.json`, were already `0600`. That file holds the
safeStorage-encrypted gateway token plus the fields that are NOT encrypted: the
gateway URL and the SSH host, user, and key path.
- Route the single write choke point through a helper that creates the file
owner-only and atomically.
- Tighten an already-existing `0644` file once per launch on the read path, so
installs that already have one do not stay world-readable until the next save.
- Refuse to act on a path that is a symlink or not owned by the current user,
matching the guards `desktop-installation.ts` already applies to its sibling.
The symlink guard alone turned out to be insufficient, and that is worth
recording: `writeSecretFileAtomic` tightens its *temp* path, so a symlink planted
at `connection.json.tmp` meant `writeFileSync` followed it, the guard correctly
bailed, and `renameSync` then moved the link onto `connection.json` permanently.
Measured, guard-only vs. as-landed:
guards only token leaked: true config is a symlink: true 755
guards + temp unlink token leaked: false config is a symlink: false 600
So the temp path is unlinked before the write.
Issue #77486's headline claim — that a dashboard session token is persisted in
plaintext — does not hold against main. The token has been safeStorage-encrypted
since the desktop app reached mainline in 51c68d4, and `encryptDesktopSecret`
aborts with an actionable message rather than degrading to plaintext when
safeStorage is unavailable. The `{ encoding: 'plain', value }` literal does exist
at main.ts:7084, but only on the `persistToken: false` branch, whose sole caller
is the connection-test handler, which never writes. So no mainline path *writes*
a plaintext token. The commits that did contain a plaintext-writing fallback
(d3d1772, d208f2c) are not ancestors of main — they live only on
upstream/bb/gui-* and the desktop-pr20059-installers pre-release tag.
At-rest migration of legacy non-safeStorage payloads is deliberately NOT included.
An earlier revision of this branch implemented it and it was removed after review
reproduced two token-loss paths: it force-converts the opt-in plaintext choice
PR #62319 adds (silently reverting the user's decision, then destroying the token
on the next launch without the `--password-store=basic` flag), and it converts a
portable credential into a keychain-bound one with no consent — destroying the
only recoverable copy while not remediating the real exposure, since every
existing backup still holds the plaintext and the true remedy is rotation. It also
persisted raw `parsed`, bypassing `sanitizeConnectionProfiles`. A comment at the
read path records the three preconditions any future attempt needs.
`decryptDesktopSecret`'s non-safeStorage read fallback is untouched — it is what
lets a pre-release or hand-edited config work at all.
Windows still inherits the userData directory ACL rather than an explicit
owner-only one; mode bits are advisory there, so that half is deferred to
PR #77527 rather than growing a second ACL implementation here.
e2e: `at-rest-connection-token.spec.ts` asserts the at-rest contract
implementation-independently — the token's plaintext value (and its base64 form)
must not appear in a raw-bytes scan of any file under userData or HERMES_HOME,
AND the app must still put the exact original token on the wire after a restart,
so a fix that simply drops the token cannot pass. Proven non-vacuous by mutation:
writing `{ encoding: 'plain', value }` still fails the scan while the
file-exists and gateway-URL guards pass. The migration case is a documented
`test.fixme` naming its three blockers.
Electron project 928 -> 924 tests (-9 migration, +5 new guard and
mechanism-isolation). Two of those five exist because reverting either owner-only
mechanism alone initially scored zero failures — they were masking each other, so
either could have been deleted green.
(cherry picked from commit 6e01add)
The helpers were tested; nothing proved main.ts called them. Reverting both call sites and both imports in readDesktopConnectionConfig / writeDesktopConnectionConfig left the whole suite green (947 passed / 2 skipped, tsc 0, eslint clean, e2e 1 passed 1 skipped) while connection.json went back to 0644 — the user-visible fix this PR promises was untested. The e2e spec could not catch it by construction: it asserts the ENCRYPTION contract with a raw-bytes scan, and safeStorage keeps the token opaque regardless of the file's mode, so a 0644 file passes that scan every time. There was no mode assertion anywhere in e2e/. Adds the missing third contract — unreadable by other local accounts — on all three paths that can produce the file: - write: assert the mode of the artifact test 1 already proves the app wrote. - read, valid file: seed the app's own encrypted connection.json back to 0644 and assert launch tightens it. Scoped to the MODE only, so it is independent of the still-deferred plaintext migration — the fixture's token is already ciphertext, so nothing re-encrypts, no #62319 opt-in marker is involved, and no rotation guidance is owed. - read, corrupt file: a truncated file still holds the token bytes and throws into the swallowing catch, so it would be the one file never tightened. This is the only test that distinguishes the chmod's placement relative to the parse. Also moves the tighten above JSON.parse for exactly that reason, and pins the cache invariant the placement depends on: the tighten must be a chmod, not a rewrite, because it sits inside the function whose cache keys on mtimeMs. Asserted as `mode & 0o077 === 0` rather than `=== 0o600` to avoid a change-detector, and skipped on win32, where chmod maps to the read-only bit and the fix deliberately no-ops (ACLs are PR #77527). Every assertion was mutation-tested: reverting the full wiring fails all three; reverting only the write path fails only the write test; deleting only the tighten-on-read fails only the two read tests; moving the tighten below the parse fails only the corrupt test; making the tighten a rewrite instead of a chmod fails the mtime assertions. Bundle greps confirmed each mutation reached dist/electron-main.mjs before the run. (cherry picked from commit 99cfc16)
|
suggesting changes The Windows ACL branch addresses the missing DACL hardening, but it trusts mutable USERNAME/USER values when selecting the account to receive full control. The authenticated dashboard environment writer accepts arbitrary custom keys, so a caller can set USERNAME=Everyone and then trigger another secret write; the next ACL command grants Everyone:(F), defeating owner-only protection. Resolve the account from the Windows access token or another OS-authoritative identity before merging.
Security evidence:
Not checked:
Signed: GPT-5.6-luna-max in Codex |
Verification complete — P1 vulnerability fixedFinding confirmed: The Windows ACL principal was derived from mutable Fix applied: Regression test added: Tests passing:
All 8 Windows ACL/security tests pass. The Pushed to fork: |
_secure_file was a no-op on Windows: os.chmod(0o600) only toggles the read-only attribute and never restricts ACLs, so files written with an inherited ACL kept NT AUTHORITY\SYSTEM and BUILTIN\Administrators full control — the '0600 at rest' posture was fictional on Windows (NousResearch#77462). The Windows branch now invokes icacls (argv, never a shell) with /inheritance:r and /grant:r <user>:(F), removing inherited entries and granting owner-only access — the ACL equivalent of POSIX 0600. Failures are swallowed like the POSIX branch (best-effort hardening). Managed-mode and container skips are preserved. The mode-preservation branch in save_env_value (Docker volume mounts keep their 0640) now also restricts ACLs on Windows: chmod cannot express POSIX group/other bits there, and a rotation on a pre-existing .env must not leave the inherited SYSTEM/Administrators entries behind. Scope: _secure_file guards the .env write path (save_env_value / save_env_value_secure). auth.json and state.db have their own at-rest protections (atomic O_EXCL 0600 write for auth.json; separate state.db handling) and are not changed here. Tests (E2E on Windows): real file + real icacls — new-file ACL restriction (inherited Administrators removed, owner retained), and mode-preservation rotation on an existing .env (same assertion). POSIX branch regression-guarded (0600 chmod). Verified live on Windows: 75 passed / 1 skipped, ruff clean, diff --check clean, Windows-footgun lint clean on the changed lines (pre-existing file-wide read_text-encoding hits untouched — separate concern). Closes NousResearch#77462 (conquest PR 1/26).
…mutable env vars (P1 NousResearch#77527)
b974edd to
d11d9b5
Compare
|
P1 addressed on the current head ( Regression: The branch was also rebased onto current |
|
suggesting changes The patch improves Windows ACL handling, but the owner-only invariant remains bypassable through writable hardening opt-outs, post-publication ACL application, a mutable identity fallback, and preservation of explicit non-owner entries.
Security evidence:
Not checked:
Signed: GPT-5.6-luna-max in Codex |
alt-glitch
removed
the
sweeper:risk-security-boundary
monerostar
left a comment
monerostar
left a comment
There was a problem hiding this comment.
Win11 here, tip d32c93e2e.
Main still has no Windows ACL helper. os.chmod(0o600) is a no-op. I wrote a temp secret, set USERNAME=Everyone, called _secure_file. icacls stayed NT AUTHORITY\SYSTEM:(I)(F) + BUILTIN\Administrators:(I)(F).
PR resolves the principal with GetUserNameW. Same spoof on this box: only DUSTIN7600X\Admin:(F). No Everyone. No Administrators.
pytest on the tip (-k "secure_file_restricts_acls or save_env_value_restricts_acls or windows_acl_principal"): 3 passed, 79 deselected.
Looks good. New evidence only. Earlier note was on 9bcbbc5625 before this token-principal fix.
5 participants
Related #77462
What changed and why
Closes #77462 — the Windows at-rest ACL hole (conquest PR 1/26, from the red-team swarm).
_secure_file(hermes_cli/config.py:822) was a no-op on Windows:os.chmod(0o600)only toggles the read-only attribute and never restricts ACLs, so.envfiles written with an inherited ACL keptNT AUTHORITY\SYSTEMandBUILTIN\Administratorswith full control — the "0600 at rest" posture was fictional on Windows. Liveicaclson a Windows host confirmed(F)for SYSTEM/Administrators on secret files.The fix adds a Windows branch invoking
icacls(argv, never a shell) with/inheritance:rand/grant:r <user>:(F)— removing inherited entries and granting owner-only access, the ACL equivalent of POSIX 0600. Failures are swallowed like the POSIX branch (best-effort hardening). Managed-mode and container skips are preserved.The mode-preservation branch in
save_env_value(which keeps 0640 for Docker volume mounts) now also restricts ACLs on Windows:chmodcannot express POSIX group/other bits there, and a rotation on a pre-existing.envmust not leave inherited SYSTEM/Administrators entries behind.Scope:
_secure_fileguards the.envwrite path (save_env_value/save_env_value_secure).auth.jsonandstate.dbhave their own at-rest protections (atomicO_EXCL0600 write for auth.json; separate state.db handling) and are not changed here.Why this matters to users
Before: on Windows, your API keys and tokens in
~/.hermes/.envwere readable by every local admin, SYSTEM processes, and backup software — the file permissions looked private (0600) but were not. After: the file's ACL is restricted to your account, so a same-user or admin process can no longer silently read your credentials from disk.How to test
On Windows: create a
.env, run a rotation (save_env_valueorhermes secrets ... token), thenicacls ~/.hermes/.env— inheritedAdministrators/SYSTEMentries must be gone and only your account retains access. The new E2E tests assert exactly this with realicaclsoutput.Tests
test_secure_file_restricts_acls_on_windows— new-file ACL restriction (inherited Administrators removed, owner retained), real file + realicacls.test_save_env_value_restricts_acls_on_existing_windows_env— mode-preservation rotation on an existing.env(same assertion) — the QA-follow-up regression.test_secure_file_chmods_0600_on_posix— POSIX branch regression guard (runs on Linux CI).test_secure_file_windows_acl_principal_not_spoofable_via_env— P1 regression: USERNAME/USER set toEveryonevia the env writer cannot steer theicaclsgrant; the principal comes from the process token (GetUserNameW), asserted against realicaclsoutput.Platforms tested
Windows (native, this host) — both ACL tests pass. POSIX branch is regression-guarded for Linux CI.
Verification
tests/hermes_cli/test_config.py: 80 passed / 1 skipped (the sole failure,test_default_path, is a pre-existing HERMES_HOME env test proven identical on pristine main — this diff touchesget_hermes_homezero lines). Ruff clean,git diff --checkclean, Windows-footgun lint clean on changed lines. Rebased on current main (1b1975781f3); all required CI checks pass on the head. Independent QA critique: SAFE TO SHIP (both MAJOR follow-ups — mode-preservation bypass, message scope — fixed in48c3f50; the P1 review finding — ACL principal must come from the process token, not mutable env vars — fixed ind11d9b58).Part of #77462
Part of #77472