Skip to content

fix(credentials): validate mounted credential file paths - #3946

Closed
Gutslabs wants to merge 1 commit into
NousResearch:mainfrom
Gutslabs:fix/credential-file-path-validation
Closed

fix(credentials): validate mounted credential file paths#3946
Gutslabs wants to merge 1 commit into
NousResearch:mainfrom
Gutslabs:fix/credential-file-path-validation

Conversation

@Gutslabs

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a credential-file mount safety issue for remote execution backends.

Previously, required_credential_files entries from skill frontmatter were passed through as relative paths under HERMES_HOME without validating traversal segments, absolute paths, or symlink escapes. That meant a skill could register entries like ../outside.txt or an absolute path and have arbitrary host files added to the sandbox mount list.

This change validates credential file paths as safe relative paths under HERMES_HOME, rejects paths that resolve outside that root, and applies the same guard to terminal.credential_files config entries.

Type of Change

  • Bug fix
  • Security fix
  • Tests

Changes Made

  • Validate credential file paths as relative paths under HERMES_HOME
  • Reject traversal segments, absolute paths, and drive-style paths
  • Reject symlinked credential files that resolve outside HERMES_HOME
  • Apply the same validation to config-based terminal.credential_files
  • Added regression tests for traversal, absolute-path, symlink-escape, and unsafe config entries

How to Test

  1. Run source .venv/bin/activate
  2. Run python -m pytest tests/tools/test_credential_files.py tests/tools/test_skills_tool.py -q
  3. Confirm register_credential_file('../outside.txt') returns False
  4. Confirm register_credential_file('/abs/path') returns False
  5. Confirm a symlink inside HERMES_HOME pointing outside is rejected and not mounted

Validation

  • python -m pytest tests/tools/test_credential_files.py -q16 passed
  • python -m pytest tests/tools/test_skills_tool.py -q77 passed
  • Manual repro before the fix: traversal and absolute paths appeared in get_credential_file_mounts()
  • Manual repro after the fix: both unsafe paths are rejected and the mount list stays empty

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the security-focused contribution! This fix has already been applied to main in two commits.

This is an automated hermes-sweeper review.

Evidence that the fix is already on main:

  • a97641b9ffix(security): reject path traversal in credential file registration — adds os.path.isabs() rejection and validate_within_dir() containment checks to register_credential_file(), with regression tests for traversal, absolute paths, and symlink escape.
  • 7f78deebefix: apply same path traversal checks to config-based credential files — extends the same guards to _load_config_files() / terminal.credential_files.
  • Both are present in tools/credential_files.py and tests/tools/test_credential_files.py on main, and shipped in release v2026.4.3.

Closing as implemented_on_main.

@teknium1 teknium1 closed this Apr 27, 2026
teknium1 pushed a commit that referenced this pull request Jul 20, 2026
…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 (#3946, #3951, #4316) all hardened traversal; this
closes the half that traversal validation never covered.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants