security(state): create state.db and WAL sidecars at 0600, not process umask - #59716
Open
JoaoMarcos44 wants to merge 4 commits into
Open
security(state): create state.db and WAL sidecars at 0600, not process umask#59716JoaoMarcos44 wants to merge 4 commits into
JoaoMarcos44 wants to merge 4 commits into
Conversation
…s umask SessionDB.__init__ created state.db via a bare sqlite3.connect(), which opens the file at the process umask (0644 under the common 022 default) with no explicit chmod. On the default posture this is only shielded by ~/.hermes's 0700 mode; if that parent is ever widened (HERMES_HOME_MODE override, managed mode's 0750), state.db's own bits become load-bearing and expose full conversation history to other local accounts. Pre-create state.db via os.open(O_CREAT | O_EXCL, 0o600) before sqlite3.connect() ever touches it, so there's no window where the file exists at a wider mode. Also chmod the -wal/-shm sidecars to 0600 once WAL mode creates them -- SQLite creates those itself at the process umask and they don't inherit the main file's mode. An already-existing state.db (restored from backup, or created by an older Hermes version) is left untouched -- O_EXCL only sets the mode on first creation, matching the existing auth.json TOCTOU-safe pattern in hermes_cli/auth.py.
Closed
11 tasks
0600 would silently break the NixOS module's 0750 group-shared HERMES_HOME and volume-mounted container state, both of which intentionally rely on group/other read access between the gateway and interactive users. Mirrors hermes_cli.config._secure_file's own managed/container checks. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Thanks for tracing the SessionDB creation path and preserving the managed/container sharing exemptions. Problems
Suggested changes
This is an automated hermes-sweeper review. |
tools/async_delegation.py:_connect() opens the same state.db as SessionDB via a bare sqlite3.connect(), bypassing the owner-only (0600) hardening added for SessionDB. Apply the same _create_owner_only / _secure_wal_files policy here, reusing hermes_state's helpers (managed/container skip included). Addresses teknium1's review on NousResearch#59716. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #59706
Problem
SessionDB.__init__(hermes_state.py) createsstate.dbvia a baresqlite3.connect(), which opens the file at the process umask (0644under the common022default) rather than an explicit0600. On the default posture this is shielded by~/.hermes's0700mode, but that makes the directory the only protection -- it becomes load-bearing the instantHERMES_HOME_MODEis widened or managed mode's0750group-readable directory is in play, exposing full conversation history to other local accounts. WAL mode's-wal/-shmsidecars have the same problem and aren't covered by any existing chmod.Fix
hermes_state.py:Called from
SessionDB.__init__:_create_owner_only(self.db_path)right before thesqlite3.connect()that creates the file, and_secure_wal_files(self.db_path)right afterapply_wal_with_fallback()engages WAL mode.An already-existing
state.db(restored from a backup, or created by an older Hermes version) is left untouched --O_EXCLonly sets the mode on first creation, never chmods an existing file. This mirrors the TOCTOU-safe pattern already used forauth.jsoninhermes_cli/auth.py.Update: both hardening functions now also no-op in managed (NixOS) or container mode, mirroring
hermes_cli.config._secure_file's own checks (is_managed()/_is_container()). The NixOS module deliberately runsHERMES_HOMEat0750so interactive users in thehermesgroup can share session state with the gateway service, and containers with volume-mounted state often need the gateway/dashboard -- running as different UIDs -- to both reach it. Forcing0600would silently break both documented sharing setups, so a new_skip_state_db_hardening()gate (lazy-importshermes_cli.configto avoid a hard dependency from this lower-level module) short-circuits both functions when either check is true.Testing
New
tests/test_state_db_file_mode.py(POSIX-only, skipped on Windows same as the existingauth.jsonTOCTOU tests):state.dblands at0600underumask 0022-wal/-shmsidecars land at0600when WAL mode creates themstate.dbat a different mode (e.g.0640, restored from backup) is left untouched, not silently rewrittenis_managed() == True) and container mode (HERMES_CONTAINER=1) both skip hardening, leaving the file at the process umask's modeFull
tests/test_hermes_state.pysuite + the new tests run clean (327 passed in the latest full regression run). Windows run: mode-bit tests skip (mode bits are ignored there), matching the pre-existing suite exactly -- no regression.Scope
Out of scope for this PR, tracked separately: the
HERMES_HOME_MODEwarning (companion issue/PR), and a handful ofconfig.yamlwriters that bypasssave_config()'s permission enforcement. Both are real but independent fixes.