Skip to content

fix(security): block user credential reads - #35997

Open
zapabob wants to merge 1 commit into
NousResearch:mainfrom
zapabob:codex/read-file-credential-guards
Open

fix(security): block user credential reads#35997
zapabob wants to merge 1 commit into
NousResearch:mainfrom
zapabob:codex/read-file-credential-guards

Conversation

@zapabob

@zapabob zapabob commented May 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • block read_file from directly reading common user credential stores under the OS home directory (~/.ssh, ~/.aws, ~/.kube, ~/.docker, GitHub/gcloud credential dirs, and exact files like ~/.netrc, ~/.npmrc, ~/.pypirc, ~/.git-credentials)
  • keep the guard centralized in agent.file_safety.get_read_block_error(), so existing read_file and ACP read-text-file callers share the same defense-in-depth policy
  • preserve normal reads for non-secret home files and existing .env.example / non-Hermes auth fixtures

Refs #16809
Refs #16851
Refs #17656

Validation

  • python -m pytest -p no:cacheprovider tests\agent\test_file_safety_credentials.py -q --timeout-method=thread (35 passed, 1 skipped)
  • python -m pytest -p no:cacheprovider tests\agent\test_file_safety.py tests\agent\test_file_safety_credentials.py tests\tools\test_file_read_guards.py -q --timeout-method=thread (90 passed, 4 skipped)
  • ruff check agent\file_safety.py tests\agent\test_file_safety_credentials.py --no-cache
  • ruff check . --no-cache
  • git diff --check upstream/main..HEAD

Note: this is defense-in-depth, matching the existing file safety wording; the terminal tool runs as the same OS user and is not a sandbox boundary.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 31, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused hardening contribution. The premise remains valid on current main: agent/file_safety.py:239-304 has no external-home credential-store rule before returning None, while tools/file_tools.py:1300-1366 relies on that shared guard before reading. The proposed centralized implementation also reaches ACP through agent/copilot_acp_client.py:699-706 and context references through agent/context_references.py:394-400.

The current file has moved since the PR (get_safe_write_roots() is now at agent/file_safety.py:80), but the required salvage appears mechanical: retain the proposed external exact-path/prefix checks and its focused regression coverage.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint 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 tool/file File tools (read, write, patch, search) type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants