fix(gateway): deny sibling-profile credentials in media delivery - #70144
fix(gateway): deny sibling-profile credentials in media delivery#70144fangliquanflq wants to merge 7 commits into
Conversation
Related: #47220 addresses the same sibling-profile media-delivery gap with a narrower credential list. This PR applies the full current credential policy and adds broader regression coverage. |
|
Thanks for the focused security fix. The premise is confirmed on current main: The proposed reuse of the live profile-directory enumeration already used by Automated hermes-sweeper review. |
|
suggesting changes The PR closes the normal sibling-profile credential leak, but its new security decision depends on successfully listing the profiles directory. The helper explicitly converts every listing OSError into an empty profile set. On a POSIX profiles directory with execute permission but no read permission, a known sibling credential remains readable by path while enumeration fails, and PR-head validation accepts that credential for media delivery. Because this is a fail-open residual bypass at the exact boundary the PR intends to close, the change is not yet mergeable.
Security evidence:
Uncertainty: The frequency of execute-only or transiently unreadable profile directories in supported deployments is unknown; it affects exploit prevalence, not the demonstrated fail-open behavior.; The full focused pytest file and full suite were not runnable in the leased checkout because no pytest-capable interpreter was available.; The clean merge-tree result establishes coherent source integration onto current main, but the merged tree was not materialized or test-executed because checkout and Git metadata mutations were prohibited. Signed: GPT-5.6-sol-xhigh in Codex |
|
Addressed the P2 fail-open on sibling-profile credential denial.
Also fail-closed if Regression: Commit: 94753dcf7 |
|
suggesting changes The change denies credential-relative paths for ordinary sibling profiles even when profile enumeration is unavailable, while preserving delivery of sibling non-credential files. However, the new structural check receives the resolved target. Hermes profile discovery accepts directory symlinks, so a credential beneath a symlinked sibling resolves outside the structural profiles root and remains deliverable in default mode.
Security evidence:
Not checked:
Signed: GPT-5.6-sol-xhigh in Codex |
|
Addressed the directory-symlink sibling bypass.
Regression: Verified: |
|
suggesting changes The change closes the ordinary sibling-profile credential exposure and adds useful fail-closed and symlink coverage, but the two exceptional conditions can still be combined to bypass the denial. If the profile registry cannot be enumerated and the sibling profile is reached through a directory symlink, canonicalization removes the profile-tree ancestry before either denial layer can recognize it. One source-backed P2 finding remains.
Security evidence:
Not checked:
Signed: GPT-5.6-sol-xhigh in Codex |
|
Addressed the residual P2 (symlink sibling + failed Fix: keep a normalized lexical form of the submitted absolute path in Regression: |
|
suggesting changes Case-insensitive POSIX filesystems still permit a sibling-profile credential to cross the media-delivery boundary through mixed-case path spelling. Security evidence:
Not checked:
Signed: GPT-5.6-sol-xhigh in Codex |
…g fails Deny credentials under profiles/<name>/ by path structure so media delivery cannot accept a known sibling secret when profiles/ iterdir raises OSError.
Apply the credential-relative denylist to resolved homes from profile discovery so directory-symlink siblings cannot bypass the structural profiles-tree check after path canonicalization.
Retain lexical profile ancestry so resolve()+failed profiles/ enumeration cannot jointly bypass credential denial in media delivery.
Mixed-case aliases on case-insensitive filesystems could miss profile-tree and credential-relative deny checks while still opening the real secret.
560889f to
83e8f90
Compare
|
Addressed the case-insensitive filesystem bypass. Fix: deny-side path matching now uses casefolded comparisons for credential-relative checks ( Regression: Verified: Commit: 83e8f90 |
|
suggesting changes The sibling-profile credential denial closes the reported exfiltration path for ordinary, unreadable-parent, symlinked-profile, and mixed-case inputs while preserving delivery of non-credential files. However, the new unconditional casefold comparison also changes the running-home exception: on a case-sensitive filesystem it can treat two distinct directories as the same home and exempt an actual hard-denied system tree. This regression can make a model-controlled media path eligible for native attachment, so the case-insensitive comparison needs to preserve filesystem identity before merge.
Security evidence:
Not checked:
Signed: GPT-5.6-sol-xhigh in Codex |
Unconditional casefold equality incorrectly exempted a hard-denied system tree when a distinct configured home only casefolded to the same spelling. Require exact equality or Path.samefile, and fix mixed-case tests for case-sensitive CI volumes.
|
suggesting changes The PR closes the ordinary sibling-profile credential path, including mixed-case, unreadable-directory, and directory-symlink forms, while preserving delivery of ordinary sibling files. One deterministic, source-backed bypass remains: credential policy runs only after unconditional safe-root acceptance, so a sibling credential path that resolves into a profile cache or operator-allowed root is returned for delivery. The credential check must precede safe-root acceptance for lexically or structurally credential-shaped paths.
Security evidence:
Not checked:
Signed: GPT-5.6-sol-xhigh in Codex |
A sibling .env (or other credential-shaped path) that symlinks into cache/images was accepted because safe-root ran before lexical denial.
SummaryTwo open PRs address the same sibling-profile credential-exfiltration gap in default media delivery. #47220 adds a narrow four-path denylist without tests, while #70144 applies the full credential file/directory policy with lexical, structural, symlink, enumeration-failure, safe-root, and casefold handling plus regression coverage. Related pull requests
Duplicates#47220 and #70144 address the same sibling-profile media-delivery vulnerability; #70144 is the broader implementation and supersedes #47220. Suggested consolidationKeep open with a salvage path for #70144: obtain re-review from the blocking [contributor:6 commits] reviewer and record verification of the updated security boundary and tests. Close #47220 as duplicate of #70144; this differs from the MAINTAINER-BOT keep_open verdict on #47220 because its current diff is conflicting, narrower, and lacks the broader policy and regression coverage present in #70144. 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
subgraph Dup47220 ["PRs duplicating each other"]
P47220["PR #47220 (open)"]
P70144["PR #70144 (open)"]
end
class P47220 open
class P70144 open
class P70144 target
click P47220 "https://github.com/NousResearch/hermes-agent/pull/47220"
click P70144 "https://github.com/NousResearch/hermes-agent/pull/70144"
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 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 44 kB of PR diffs, 4 kB of issue/PR text, 29 kB of discussion (12 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
|
looks mergeable The current head addresses the remaining safe-root precedence bypass from the prior review. Sibling-profile credential checks now run before cache/operator allowlisting, so a credential-shaped Security evidence:
Not checked:
Signed: GPT-5.6-sol-xhigh in Codex |
What does this PR do?
Default (non-strict) media delivery only denied credential stores under the active HERMES_HOME and the shared Hermes root. Sibling profile paths under
<root>/profiles/<other>/were accepted, so a prompt-injectedMEDIA:tag could attach another profile's secrets as a chat document. This applies the full current credential file/dir policy to every live profile directory (same enumeration pattern as cache allowlisting).Bug Cause
_media_delivery_denied_paths()ingateway/platforms/base.pyonly appended_ROOT_CREDENTIAL_FILES/_ROOT_CREDENTIAL_DIRSfor(_HERMES_HOME, _HERMES_ROOT). In non-strict mode,validate_media_delivery_pathaccepts any existing regular file not on that denylist, soMEDIA:<root>/profiles/bob/.env(and auth/OAuth/mcp-tokens/pairing/etc.) delivered whilealice/.envand root.envstayed blocked.Reproduction Steps
profiles/aliceandprofiles/bob; write credential files under bob (.env,auth.json,.anthropic_oauth.json,mcp-tokens/,pairing/,google_token.json,webhook_subscriptions.json,credentials/,cache/bws_cache.json)._HERMES_HOMEto alice and_HERMES_ROOTto the temp root; leaveHERMES_MEDIA_DELIVERY_STRICTunset/off.validate_media_delivery_pathon each bob credential path and on alice/root.env.Expected: sibling credential paths return
None(same as active/root).Before fix: bob credential paths returned an allowed absolute path; alice/root
.envcorrectly returnedNone.Fix
_iter_hermes_profile_dirs()and reuse it from_profile_cache_roots()and_media_delivery_denied_paths().notes.mdstill delivers.Supersedes #47220 (that PR only denied four control files and lagged the expanded credential set on current main).
Related Issue
No issue
Type of Change
Changes Made
gateway/platforms/base.py- enumerate sibling profiles into the media-delivery credential denylist; share profile-dir iteration with cache allowlistingtests/gateway/test_platform_base.py- non-strict sibling credential / home-is-root / non-credential control testsHow to Test
HERMES_HOME=<root>/profiles/alice, assertvalidate_media_delivery_path(<root>/profiles/bob/.env)(and other credential rels) isNone, while<root>/profiles/bob/notes.mdstill resolves.Checklist
Code
scripts/run_tests.shon relevant tests and they passDocumentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys - N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows - N/A