fix(credentials): hoist read-guard import, fail closed loudly (#67665)

Follow-up to #67640: move the agent.file_safety import to module top
(stdlib-only, no circular-import concern), replace the over-broad
except Exception + logger.warning with an import sentinel plus
logger.exception so a guard failure is debuggable instead of silently
swallowed. Adds fail-closed tests asserting the diagnostic is emitted.
This commit is contained in:
Teknium 2026-07-20 08:59:23 -07:00
parent 28028cce55
commit faa4cec01b
2 changed files with 52 additions and 7 deletions

View file

@ -609,3 +609,36 @@ class TestMasterCredentialStoresAreNeverMountable:
with patch.dict(os.environ, {"HERMES_HOME": str(home)}):
assert register_credential_file("../../.ssh/id_rsa") is False
assert register_credential_file("/etc/passwd") is False
def test_missing_guard_fails_closed_with_error_log(self, tmp_path, caplog):
"""If agent.file_safety can't be imported the mount is refused loudly.
The fail-closed path must be observable (#67665): a silent deny with
no diagnostic reproduces the trust gap the deny-list was added to fix.
"""
import tools.credential_files as cf
home = self._home(tmp_path)
with patch.dict(os.environ, {"HERMES_HOME": str(home)}), \
patch.object(cf, "get_read_block_error", None):
with caplog.at_level("ERROR", logger="tools.credential_files"):
assert cf.register_credential_file("google_token.json") is False
assert cf.get_credential_file_mounts() == []
assert any("deny-list cannot be consulted" in r.message for r in caplog.records)
def test_guard_exception_fails_closed_with_traceback(self, tmp_path, caplog):
"""A raising guard refuses the mount and logs the stack trace."""
import tools.credential_files as cf
home = self._home(tmp_path)
def _boom(path):
raise RuntimeError("guard exploded")
with patch.dict(os.environ, {"HERMES_HOME": str(home)}), \
patch.object(cf, "get_read_block_error", _boom):
with caplog.at_level("ERROR", logger="tools.credential_files"):
assert cf.register_credential_file("google_token.json") is False
assert cf.get_credential_file_mounts() == []
rec = next(r for r in caplog.records if "read guard raised" in r.message)
assert rec.exc_info is not None, "traceback must be attached (logger.exception)"

View file

@ -28,6 +28,11 @@ from pathlib import Path
from typing import Dict, List, Optional
from hermes_cli.config import cfg_get
try: # pragma: no cover - exercised via the fail-closed test below
from agent.file_safety import get_read_block_error
except ImportError: # noqa: F401 - sentinel consumed in register_credential_file
get_read_block_error = None # type: ignore[assignment]
logger = logging.getLogger(__name__)
# Session-scoped list of credential files to mount.
@ -110,15 +115,22 @@ def register_credential_file(
# Master credential stores are never mountable, even though they sit
# inside HERMES_HOME and therefore pass the containment check above.
# Fails CLOSED: if the canonical guard can't be consulted we refuse the
# mount rather than risk bind-mounting auth.json into a sandbox.
# mount rather than risk bind-mounting auth.json into a sandbox. The
# import lives at module top (no circular-import concern — file_safety is
# stdlib-only); the sentinel + logger.exception keep guard failures
# debuggable instead of silently swallowed (#67665).
if get_read_block_error is None:
logger.error(
"credential_files: refusing %r — agent.file_safety could not be "
"imported, so the master-store deny-list cannot be consulted",
relative_path,
)
return False
try:
from agent.file_safety import get_read_block_error
denied = get_read_block_error(str(resolved))
except Exception as exc: # pragma: no cover - core module
logger.warning(
"credential_files: refusing %r — read guard unavailable (%s)",
relative_path, exc,
except Exception:
logger.exception(
"credential_files: refusing %r — read guard raised", relative_path
)
return False
if denied: