security: close a TOCTOU window when writing openfusion.yaml in setup - #161
Open
shrdgn wants to merge 1 commit into
Open
security: close a TOCTOU window when writing openfusion.yaml in setup#161shrdgn wants to merge 1 commit into
setup#161shrdgn wants to merge 1 commit into
Conversation
run_setup() wrote the key-bearing openfusion.yaml via Path.write_text(), which uses the process's default open() mode, then narrowed it to 0o600 with a separate chmod afterward. Under a permissive umask (e.g. 022) the file -- which embeds the plaintext OpenRouter key -- was briefly world/group-readable before the chmod landed. credentials.py already hardened save_key() against this exact race (open with mode 0o600 up front via os.open); apply the same pattern here. Adds regression tests mirroring test_credentials.py's chmod-failure case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YMpvfCJEqQ4Lt6KaudAXC2
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.
What & why
run_setup()(openfusion/cli.py) wrote the key-bearingopenfusion.yamllike this:
Path.write_textcreates the file via the process's defaultopen()mode,so under a permissive umask (e.g.
022) the file — whichbuild_setup_yamlembeds the plaintext OpenRouter API key into — is briefly world/group-readable
before the subsequent
chmod(0o600)narrows it.This is exactly the TOCTOU race
openfusion/credentials.py'ssave_key()was already hardened against: it opens with
os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)up front (with a comment explaining why),and
tests/test_credentials.py::test_key_file_created_private_even_if_chmod_failsregression-tests it.
run_setup()had the identical gap and no equivalenttest existed for it.
What changed
run_setup()now opensopenfusion.yamlwithos.open(..., 0o600)beforewriting, mirroring
credentials.save_key(). Thechmod(0o600)afterwardis kept as belt-and-suspenders for a pre-existing file from before this fix.
tests/test_cli.py: one asserting the written configfile is
0o600, and a regression test (mirroringtest_credentials.py'schmod-failure case) confirming the file is created private even when
Path.chmodraises.How it was tested
ruff check .passespytest -qpasses (no live network) — 480 passedtest_run_setup_writes_config_file_private,test_run_setup_config_file_created_private_even_if_chmod_failssurface change, only how an existing file gets written
bench/run.pynumber — not applicableNotes for reviewers
Only
openfusion/cli.py'srun_setup()changed (plus tests). Small,self-contained, reuses an existing pattern already proven elsewhere in the
codebase.
Generated by Claude Code