Skip to content

fix(credentials): never mount master credential stores into skill sandboxes - #67640

Merged
teknium1 merged 1 commit into
NousResearch:mainfrom
Frowtek:fix/skill-credential-mount-denylist
Jul 20, 2026
Merged

fix(credentials): never mount master credential stores into skill sandboxes#67640
teknium1 merged 1 commit into
NousResearch:mainfrom
Frowtek:fix/skill-credential-mount-denylist

Conversation

@Frowtek

@Frowtek Frowtek commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

register_credential_file() takes a skill-declared relative path from
required_credential_files frontmatter and bind-mounts it read-only into the
remote sandbox the skill's own code runs in
. It validates that the resolved
path stays inside HERMES_HOME, and the docstring names the threat directly:

so that a malicious skill cannot declare
required_credential_files: ['../../.ssh/id_rsa'] and exfiltrate sensitive
host files into a container sandbox

Containment is the wrong boundary on its own, because HERMES_HOME is exactly
where the master credential stores live. Traversal is blocked; asking for the
keys by name is not:

skill declares mounted? agent may read it?
.env ✅ YES ❌ DENIED
auth.json ✅ YES ❌ DENIED
.anthropic_oauth.json ✅ YES ❌ DENIED
cache/bws_cache.json ✅ YES ❌ DENIED
mcp-tokens/srv.json ✅ YES ❌ DENIED
google_token.json ✅ YES allowed
../../.ssh/id_rsa refused n/a
/etc/passwd refused n/a

Every DENIED row is refused by the canonical read guard
(agent.file_safety.get_read_block_error) — the agent cannot read_file them —
yet one line of hub-installed skill frontmatter gets them bind-mounted where that
skill can cat them. .env alone is every provider API key; auth.json is every
provider token and OAuth grant.

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/credential_files.py — reuse the canonical deny-list as the mount bar:
    what the agent is forbidden to read is not mountable either, so the mount
    surface can't hand a skill what the read surface denies it. Fails closed
    if the guard can't be consulted the mount is refused rather than risked.
  • tests/tools/test_credential_files.py — parametrised refusal coverage for all
    six master stores, plus the legitimate path, batch behaviour, and the
    pre-existing traversal guard.

The module keeps doing its job: a skill still mounts its own service token
(google_token.json, skills/*), and a refused entry is reported back through
register_credential_files' missing list instead of failing the batch.

Testing

Seven of the nine new tests fail on main (every master store plus the batch
case). The other two — legitimate token still mounts, traversal still refused —
pass both ways, which is what shows the fix doesn't trade a leak for a
regression.

tests/tools/test_credential_files.py -k MasterCredentialStores .... 9 passed

Credential/skill/sandbox/docker/modal selection baseline-compared against a clean
origin/main worktree: identical 28 pre-existing failures (all Windows-env —
symlink creation, docker discovery, path mapping), 1335 → 1344 passed.

Checklist

  • Bug is reproducible on main and covered by a failing-before/passing-after test
  • No regressions in the surrounding suite (baseline-compared against origin/main)
  • Change is scoped to the defect — no unrelated refactoring
  • Tested on Ubuntu 24.04

…dboxes

register_credential_file() takes a skill-declared relative path from
required_credential_files frontmatter and bind-mounts it read-only into the
remote sandbox the skill's own code runs in. It validates that the resolved
path stays inside HERMES_HOME — the docstring names the threat directly:

    so that a malicious skill cannot declare
    required_credential_files: ['../../.ssh/id_rsa'] and exfiltrate
    sensitive host files into a container sandbox

Containment is the wrong boundary on its own, because HERMES_HOME is exactly
where the master credential stores live. Traversal is blocked; asking for the
keys by name is not:

    skill declares               mounted?   agent may read it?
    .env                         YES        DENIED
    auth.json                    YES        DENIED
    .anthropic_oauth.json        YES        DENIED
    cache/bws_cache.json         YES        DENIED
    mcp-tokens/srv.json          YES        DENIED
    google_token.json            YES        allowed
    ../../.ssh/id_rsa            no         n/a

Every row marked DENIED is refused by the canonical read guard
(agent.file_safety.get_read_block_error) — the agent cannot read_file them —
yet one line of hub-installed skill frontmatter gets them bind-mounted where
that skill can cat them. .env alone is every provider API key.

Reuse the canonical deny-list as the mount bar: what the agent is forbidden
to read is not mountable either, so the mount surface cannot hand a skill
what the read surface denies it. Fails CLOSED — if the guard can't be
consulted the mount is refused rather than risked.

The module keeps doing its job: a skill still mounts its own service token
(google_token.json, skills/*), and a refused entry is reported back through
register_credential_files' missing list instead of failing the batch.

The three prior PRs here (NousResearch#3946, NousResearch#3951, NousResearch#4316) all hardened traversal; this
closes the half that traversal validation never covered.
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 19, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused security fix. The premise is confirmed on current main: tools/credential_files.py:81-103 registers every existing path contained by HERMES_HOME, while tools/skills_tool.py:1531-1541 forwards skill frontmatter into that registration path. Docker then consumes the shared mount registry at tools/environments/docker.py:709-742.

The proposed canonical-guard approach matches the existing fail-closed security pattern in agent/context_references.py:384-413; agent/file_safety.py:269-334 already defines the sensitive credential-store coverage.

Automated hermes-sweeper review.

@teknium1
teknium1 merged commit c8882c1 into NousResearch:main Jul 20, 2026
33 checks passed
@teknium1

Copy link
Copy Markdown
Contributor

Infographic — credential-hygiene cluster (merged)

Covers the three directly-merged PRs from this cluster: #67806 (explicit api_key precedence), #67640 (master credential stores never mountable), #67635 (live transcript redaction). The remaining two (#67802, #67797) landed via salvage PR #68074.

credential-hygiene-triple-drop

teknium1 added a commit that referenced this pull request Jul 20, 2026
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.
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 25, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…search#67665)

Follow-up to NousResearch#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants