fix(state): restrict sensitive store file permissions (salvage #30917) - #31469
Merged
Conversation
response_store.db (api server) holds conversation history including tool payloads, prompts, and results. webhook_subscriptions.json holds per-route HMAC secrets. Under a permissive umask (e.g. 0o022, default on most distros) both files were created mode 0o644 — readable by other local users on shared boxes. - gateway/platforms/api_server.py: ResponseStore tightens itself + WAL/SHM sidecars to 0o600 after __init__, then trusts the inode. (Original contributor patch chmod'd after every _commit() — wasteful on a hot api_server path; chmod-on-create is sufficient since SQLite preserves mode bits across writes.) - hermes_cli/webhook.py: _save_subscriptions writes via tempfile.mkstemp (which itself creates the file with 0o600), chmods the temp before the atomic rename, and re-asserts 0o600 on the destination so an existing permissive file from before this fix gets narrowed. Tests cover (a) creation under permissive umask leaves 0o600 and (b) an existing 0o644 webhook_subscriptions.json gets narrowed on next save. Tests guarded with skipif os.name=='nt' since POSIX mode bits don't apply on Windows. Salvaged from PR #30917 by @Hinotoi-agent. Reworked the api_server.py side from chmod-on-every-commit to chmod-on-create. Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>
9 tasks
Contributor
🔎 Lint report:
|
alt-glitch
added
type/security
Closed
11 tasks
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
3 participants
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.
Salvage of #30917 by @Hinotoi-agent onto current main, with the api_server.py rework.
Summary
response_store.db(api server conversation history with tool payloads, prompts, results) andwebhook_subscriptions.json(per-route HMAC secrets) were created mode 0o644 under typical umask 022 — readable by other local users on shared boxes. Both now end up 0o600 owner-only.Changes
gateway/platforms/api_server.py:ResponseStoretightens the DB and WAL/SHM sidecars to 0o600 once at__init__(after the initial commit creates the sidecars), then trusts the inode.hermes_cli/webhook.py:_save_subscriptionswrites viatempfile.mkstemp(creates with 0o600), chmods the temp before the atomic rename, then re-asserts 0o600 on the destination so an existing permissive file gets narrowed on next save.skipif os.name=='nt'since POSIX mode bits are platform-specific.Salvage scope
Original PR added a
_commit()wrapper that called_secure_file_mode()after every put/get/delete — chmod syscall per request on a hot api_server path. Reworked to chmod-once at__init__since SQLite preserves mode bits across writes (the inode is what matters, not the open fd). All the original PR's test intent is preserved.Test plan
Co-authored-by: Hinotoi-agent paperlantern.agent@gmail.com
Closes #30917