fix(dashboard): block .env files from managed-files API - #57507
liuhao1024 wants to merge 2 commits into
Conversation
The dashboard Files tab could list, read, and download .env files containing API keys when running with a bind-mounted Hermes home directory (e.g. docker run -v ~/.hermes:/opt/data). Add _SENSITIVE_FILENAMES frozenset and filter these from list_managed_files(), read_managed_file(), and download_managed_file(). Return 403 for direct read/download attempts on sensitive files. Fixes NousResearch#57505
|
suggesting changes Security evidence:
Please make the sensitive-file guard cover Signed: GPT-5.5-xhigh in Codex |
Replace the exact-filename frozenset with _is_sensitive_filename() that matches .env plus any .env.<suffix> variant. This covers shorthand suffixes like .env.prod that the previous enumeration missed. Add test_sensitive_env_suffix_variants_blocked regression test covering .env.prod, .env.dev, .env.staging.local, and .env.ci. Addresses review feedback from egilewski on PR NousResearch#57507.
|
Thanks for the thorough security review! You're right — the exact-filename set missed shorthand variants like Pushed a fix that replaces def _is_sensitive_filename(name: str) -> bool:
"""Return True for ``.env`` and any ``.env.<suffix>`` variant."""
return name == ".env" or name.startswith(".env.")This covers all All 20 tests pass. |
|
suggesting changes Security evidence:
Please make the sensitive filename predicate case-insensitive for Signed: GPT-5.5-xhigh in Codex |
Replace the exact-filename frozenset with _is_sensitive_filename() that matches .env plus any .env.<suffix> variant. This covers shorthand suffixes like .env.prod that the previous enumeration missed. Add test_sensitive_env_suffix_variants_blocked regression test covering .env.prod, .env.dev, .env.staging.local, and .env.ci. Addresses review feedback from egilewski on PR #57507.
Follow-up to #57507: .ENV / .Env.local on case-insensitive filesystem mounts slipped past the guard. Lowercase the name before matching and add a regression test. Addresses egilewski's open review note.
|
Merged via PR #57660 — both of your commits (the initial guard and the pattern-match follow-up) were cherry-picked onto current main with your authorship preserved (rebase-merge). We added one small follow-up on top making the guard case-insensitive ( |
Replace the exact-filename frozenset with _is_sensitive_filename() that matches .env plus any .env.<suffix> variant. This covers shorthand suffixes like .env.prod that the previous enumeration missed. Add test_sensitive_env_suffix_variants_blocked regression test covering .env.prod, .env.dev, .env.staging.local, and .env.ci. Addresses review feedback from egilewski on PR NousResearch#57507.
Follow-up to NousResearch#57507: .ENV / .Env.local on case-insensitive filesystem mounts slipped past the guard. Lowercase the name before matching and add a regression test. Addresses egilewski's open review note.
Replace the exact-filename frozenset with _is_sensitive_filename() that matches .env plus any .env.<suffix> variant. This covers shorthand suffixes like .env.prod that the previous enumeration missed. Add test_sensitive_env_suffix_variants_blocked regression test covering .env.prod, .env.dev, .env.staging.local, and .env.ci. Addresses review feedback from egilewski on PR NousResearch#57507.
Follow-up to NousResearch#57507: .ENV / .Env.local on case-insensitive filesystem mounts slipped past the guard. Lowercase the name before matching and add a regression test. Addresses egilewski's open review note.
Replace the exact-filename frozenset with _is_sensitive_filename() that matches .env plus any .env.<suffix> variant. This covers shorthand suffixes like .env.prod that the previous enumeration missed. Add test_sensitive_env_suffix_variants_blocked regression test covering .env.prod, .env.dev, .env.staging.local, and .env.ci. Addresses review feedback from egilewski on PR NousResearch#57507.
Follow-up to NousResearch#57507: .ENV / .Env.local on case-insensitive filesystem mounts slipped past the guard. Lowercase the name before matching and add a regression test. Addresses egilewski's open review note.
Replace the exact-filename frozenset with _is_sensitive_filename() that matches .env plus any .env.<suffix> variant. This covers shorthand suffixes like .env.prod that the previous enumeration missed. Add test_sensitive_env_suffix_variants_blocked regression test covering .env.prod, .env.dev, .env.staging.local, and .env.ci. Addresses review feedback from egilewski on PR NousResearch#57507.
Follow-up to NousResearch#57507: .ENV / .Env.local on case-insensitive filesystem mounts slipped past the guard. Lowercase the name before matching and add a regression test. Addresses egilewski's open review note.
Replace the exact-filename frozenset with _is_sensitive_filename() that matches .env plus any .env.<suffix> variant. This covers shorthand suffixes like .env.prod that the previous enumeration missed. Add test_sensitive_env_suffix_variants_blocked regression test covering .env.prod, .env.dev, .env.staging.local, and .env.ci. Addresses review feedback from egilewski on PR NousResearch#57507.
Follow-up to NousResearch#57507: .ENV / .Env.local on case-insensitive filesystem mounts slipped past the guard. Lowercase the name before matching and add a regression test. Addresses egilewski's open review note.
What does this PR do?
Blocks the dashboard managed-files API from listing, reading, or downloading sensitive credential files (
.env,.env.local, etc.). Previously, when the dashboard was run with a bind-mounted Hermes home directory (e.g.docker run -v ~/.hermes:/opt/data), the Files tab could browse and download.envfiles containing API keys and tokens.Related Issue
Fixes #57505
Type of Change
Changes Made
hermes_cli/web_server.py: Added_SENSITIVE_FILENAMESfrozenset containing.envand common variants. Filtered these filenames fromlist_managed_files()directory listings and added 403 guards toread_managed_file()anddownload_managed_file().tests/hermes_cli/test_web_server_files.py: Added 3 regression tests verifying.envfiles are hidden from listings, blocked from read, and blocked from download.How to Test
python -m pytest tests/hermes_cli/test_web_server_files.py -q— all 19 tests should pass (including the 3 new regression tests).hermes dashboard, navigate to the Files tab, verify.envfiles do not appear in directory listings./api/files/read?path=.../.env— should return 403.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/hermes_cli/test_web_server_files.py -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
N/A — security fix with regression tests.