Skip to content

security: close a TOCTOU window when writing openfusion.yaml in setup - #161

Open
shrdgn wants to merge 1 commit into
mainfrom
claude/cli-setup-key-file-perms
Open

security: close a TOCTOU window when writing openfusion.yaml in setup#161
shrdgn wants to merge 1 commit into
mainfrom
claude/cli-setup-key-file-perms

Conversation

@shrdgn

@shrdgn shrdgn commented Aug 23, 2026

Copy link
Copy Markdown
Owner

What & why

run_setup() (openfusion/cli.py) wrote the key-bearing openfusion.yaml
like this:

path.write_text(build_setup_yaml(preset, key), encoding="utf-8")
with contextlib.suppress(OSError):
    path.chmod(0o600)

Path.write_text creates the file via the process's default open() mode,
so under a permissive umask (e.g. 022) the file — which build_setup_yaml
embeds 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's save_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_fails
regression-tests it. run_setup() had the identical gap and no equivalent
test existed for it.

What changed

  • run_setup() now opens openfusion.yaml with os.open(..., 0o600) before
    writing, mirroring credentials.save_key(). The chmod(0o600) afterward
    is kept as belt-and-suspenders for a pre-existing file from before this fix.
  • Added two tests to tests/test_cli.py: one asserting the written config
    file is 0o600, and a regression test (mirroring test_credentials.py's
    chmod-failure case) confirming the file is created private even when
    Path.chmod raises.

How it was tested

  • ruff check . passes
  • pytest -q passes (no live network) — 480 passed
  • New behavior has a test — test_run_setup_writes_config_file_private,
    test_run_setup_config_file_created_private_even_if_chmod_fails
  • Docs updated if config / request surface / defaults changed — n/a, no
    surface change, only how an existing file gets written
  • No secrets, prompts, or response bodies added to logs or metrics
  • Quality/cost claims backed by a bench/run.py number — not applicable

Notes for reviewers

Only openfusion/cli.py's run_setup() changed (plus tests). Small,
self-contained, reuses an existing pattern already proven elsewhere in the
codebase.


Generated by Claude Code

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants