Create auth secret and token files with owner-only permissions - #741
Open
lukiod wants to merge 2 commits into
Open
Create auth secret and token files with owner-only permissions#741lukiod wants to merge 2 commits into
lukiod wants to merge 2 commits into
Conversation
Three sites wrote secrets with Path.write_text and narrowed them with chmod afterwards. write_text creates using 0o666 & ~umask, so the contents were readable by other local users until the chmod landed, and permanently whenever it raised, since the error was discarded. Affected: the auth secret in multi_user/identity.py, minted with secrets.token_hex(32) and written world readable at creation, the same file on the legacy migration path, and the skill hub tokens in services/skill/credentials.py. write_secret_text passes the mode to os.open so it applies at creation. The parent directory is created 0700, and a leftover from an interrupted run is replaced rather than truncated. Measured on Linux under the default umask: 0644 holding the secret before being narrowed to 0600.
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
1 participant
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.
Secret files were written and then chmod'd:
The file exists with the default umask (usually 0644) between those two calls, so the secret is briefly readable by any local user. And the chmod is swallowed, so if it fails the file stays 0644 and nothing says so.
Adds
deeptutor/utils/secret_files.pywithwrite_secret_text, which opens withO_CREAT | O_EXCLand mode 0600 so the file is never wider than intended, and uses it inservices/skill/credentials.pyandmulti_user/identity.py.It also unlinks a leftover first, since
O_CREATdoes not narrow the mode of an existing file — an interrupted earlier run would otherwise keep its old permissions.Tests in
tests/utils/test_secret_files.py; the permission assertions are POSIX-only and skip on Windows, where I ran them (3 passed, 3 skipped). The rest of the suite doesn't collect in my environment — 124 import errors from missing optional deps (pydantic_settings,openai,aiohttp), all pre-existing and unrelated.