fix(security): deny project-local .env/.envrc in media delivery - #59045
fix(security): deny project-local .env/.envrc in media delivery#59045cryptoyasenka wants to merge 1 commit into
Conversation
|
Bumping this one for a look. It's in the same credential-exposure area as the redirect-header fix that just went in with #62488, so figured it's worth surfacing here too. The gist: we already block the agent from reading The fix just mirrors the read-block basenames into If you'd rather scope it differently (strict-mode only, a config flag, whatever), glad to adjust. Mainly wanted to get eyes on the exfil path. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing this through the extensionless MEDIA: path. The premise is confirmed on current main: agent/file_safety.py:315 blocks these basenames, while gateway/platforms/base.py:1335-1338 returns a non-denied existing file in default mode; .env/.envrc reach that validator through extract_media() at gateway/platforms/base.py:3697-3704.
Problems
- The new import guard in
gateway/platforms/base.py:1296catches every exception and then continues without the basename rejection. For a credential-delivery guard, that is fail-open: an import-time regression restores approval of the sensitive path.
Suggested changes
- Import
is_project_env_basenamedirectly and apply its rejection unconditionally.agent/file_safety.py:1-16has no gateway dependency, so the inspected import graph does not justify bypassing the guard on errors. - Salvage will need a minor conflict resolution in
agent/file_safety.pybecause current main changed the preceding write-denial helper in55d826cce; the validator hunk otherwise remains at the correct centralized gate.
Automated hermes-sweeper review.
| # env file is never deliverable regardless of location or delivery mode. | ||
| try: | ||
| from agent.file_safety import is_project_env_basename | ||
| except Exception: |
There was a problem hiding this comment.
Do not fail open here. Catching every import exception and assigning None means the project-env denial is silently skipped if this import regresses. agent.file_safety has no gateway dependency; import the predicate directly (or reject on import failure) so credential delivery cannot be re-enabled by an unrelated import error.
4522475 to
0b16904
Compare
The read guard (agent/file_safety.get_read_block_error) blocks reading .env / .envrc anywhere on disk by basename, but the media-delivery denylist only enumerated <hermes-root>/.env. A user's own project .env (e.g. /home/user/app/.env) was therefore deliverable as a native attachment via a prompt-injected MEDIA: tag in default (non-strict) single-user mode, leaking API keys and database passwords. Add a shared is_project_env_basename() predicate in file_safety and reject those basenames in validate_media_delivery_path before the cache allowlist, so the delivery side mirrors the read guard in every mode. The predicate is bound at module import next to the existing first-party import and called unconditionally, so the gate fails closed: an import failure can no longer silently reopen the exfil path the gate exists to close. There is no cycle to defer around: agent/file_safety imports only stdlib and never imports gateway, and agent/__init__ only preloads jiter. Add regressions covering the guard in every delivery mode and one that poisons sys.modules["agent.file_safety"] and asserts a project .env stays undeliverable, so a call-time import can no longer decide whether the check runs.
0b16904 to
d8be06c
Compare
What does this PR do?
The read guard
agent/file_safety.get_read_block_errorblocks the agent from reading.env/.envrcanywhere on disk by basename (_BLOCKED_PROJECT_ENV_BASENAMES). The media-delivery denylist that gates auto-attachment does not mirror that rule:_media_delivery_denied_pathsonly denies<hermes-root>/.env, so a user's own project.env(e.g./home/user/app/.env) passesvalidate_media_delivery_pathand is delivered as a native attachment._media_delivery_denied_pathsstates the invariant in its own comment: a credential the agent is forbidden to read must also never be auto-attached to a chat reply. In default (non-strict) single-user mode, a prompt-injectedMEDIA:/path/.envtag exfiltrates API keys and database passwords into the chat transport..env/.envrcreach the delivery gate because they are extensionless (Path(".env").suffix == ""), soextract_mediatakes the extensionlessMEDIA:branch that callsvalidate_media_delivery_path.This adds a shared
is_project_env_basename()predicate inagent/file_safety(single source of truth for the basename set) and rejects those basenames invalidate_media_delivery_path, before the cache allowlist, so a secret env file is never deliverable regardless of location or mode. The delivery side now mirrors the read guard.Related Issue
Fixes #59044
Type of Change
Changes Made
agent/file_safety.py: addis_project_env_basename(name), a case-insensitive predicate over_BLOCKED_PROJECT_ENV_BASENAMES, shared by the read guard and the media-delivery denylist so the two cannot drift.gateway/platforms/base.py: invalidate_media_delivery_path, after theis_file()check and before the cache allowlist, reject any path whose basename is a project env file. Unconditional in both strict and non-strict mode, so the delivery side mirrors the read guard. The predicate is bound at module import, next to the existing first-party import, so the check cannot be skipped (see Review follow-up).tests/gateway/test_media_delivery_project_env_guard.py(new, 6 cases): the pathlib extensionless fact and its scope caveat (.env.productionis not extensionless); the delivery gate mirrors the read guard for.env/.envrc; e2e that a prompt-injectedMEDIA:tag does not surface the project env file as a deliverable attachment; and that the gate fails closed when the predicate is unavailable.Review follow-up
Review flagged the first cut of this gate as fail-open, and it was right.
validate_media_delivery_pathimported the predicate lazily behindtry/except Exception: is_project_env_basename = Noneand then called it onlyif ... is not None, so any import failure skipped the check outright and silently reopened the exfil path the gate exists to close. A guard whose failure path allows the action is not a guard.There was no import cycle to defer around:
agent/file_safetyimports only stdlib and never importsgateway, andagent/__init__only preloadsjiter. The predicate is now bound at module import and called unconditionally.test_project_env_gate_fails_closed_on_import_failurepins the property rather than the mechanism: it poisonssys.modules["agent.file_safety"], asserts that a call-time import really raisesImportError, then asserts the project.envis still not deliverable. It discriminates: against the previous fail-open commit it fails, withvalidate_media_delivery_pathreturning the.envpath instead ofNone, while the other five cases pass.How to Test
The 5 failures on the base are the gate-mirror and e2e-delivery assertions for
.env/.envrcplus the fail-closed regression; the fix turns them green. The 6 failures intest_platform_base.pyare pre-existing and identical with and without this change (environment-specific on Windows: recency / symlink / null-path / cache-roots), so this change adds zero regressions. The runs above are local (Windows 11, Python 3.13, cherry-picked onto current main). Fullpytest tests/in my environment has pre-existing optional-dep collection errors unrelated to this change; the touched path is covered above and by CI.Green ubuntu run for this head: https://github.com/cryptoyasenka/hermes-agent/actions/runs/31056695603
Related work (not duplicates)
This continues the "delivery side cannot trail the read/write guard" line (#51055, #56163) but covers a different file class:
~/.netrc,~/.pgpass,~/.npmrc,~/.pypirc,~/.git-credentials).~/.hermes/profiles/<other>/.~/.config,~/.gcloud, macOS Keychains).None of them block a user's project-local
.env/.envrc(an arbitrary working-directory file the read guard already blocks by basename), which is the only path this PR changes.Checklist
Code
fix(security): ...)pytest tests/has pre-existing optional-dep collection errors in my environment; the touched path is covered above and by CI.Documentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys (N/A: no config keys)CONTRIBUTING.mdorAGENTS.md(N/A)