Secret store writes plaintext secrets file before restricting permissions (TOCTOU window)

[thetelgote]

What happened?

SecretStore._write() and write_private_text() in coworker/secrets.py both write
the secrets/.tmp file to disk before calling _restrict_to_user() to chmod it to
0600:

https://github.com/andrewyng/openworker/blob/main/coworker/secrets.py#L99-L102
https://github.com/andrewyng/openworker/blob/main/coworker/secrets.py#L190-L194

tmp = self.path.with_name(self.path.name + ".tmp")
tmp.write_text(json.dumps(store, indent=2), encoding="utf-8")  # default umask perms
_restrict_to_user(tmp, is_dir=False)                            # chmod 0600 after
os.replace(tmp, self.path)

Path.write_text() creates the file using the process umask (commonly 0644 on
Linux/macOS), so for the moment between the write and the chmod, the plaintext
secrets/tokens file is world- or group-readable on disk. On a shared/multi-user
machine, another local user (or a background process) could read connector/model
credentials during that window.

Separately, in _write(), _restrict_to_user(tmp, is_dir=False) isn't wrapped in
try/except OSError the way the parent-directory chmod above it is. If it raises,
the .tmp file — containing full plaintext secrets — is left on disk at default
permissions indefinitely, since os.replace() never runs to move/overwrite it.

The module docstring states intent clearly ("v1 is a 0600 JSON file"), so this
looks like an ordering oversight rather than a design choice.

Suggested fix

Create the tmp file already restricted instead of restricting after the fact, e.g.:

fd = os.open(str(tmp), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
with os.fdopen(fd, "w", encoding="utf-8") as f:
    f.write(json.dumps(store, indent=2))

This removes the window entirely on POSIX (no separate chmod is needed), and the
Windows ACL step can stay as-is afterward for defense in depth.

Environment

• macOS / Linux (any POSIX system with default umask 022 or looser)
• Affects coworker/secrets.py on current main

[Mr-Neutr0n]

PR #291 is up for this.

Confirmed the window by sampling the temp file's mode at the moment it holds the plaintext:

before:  0o644  -> world/group readable
after:   0o600  -> user-only

Both writers (SecretStore._write and write_private_text) now go through one helper built on tempfile.mkstemp, so the file is created 0600 and empty and the content is written after. On Windows mkstemp gives no mode bits, so the icacls ACL is applied while the file is still empty.

Two related problems fell out of the same change, both with tests that fail on current main:

• the fixed <name>.tmp was predictable. Pre-creating it as a symlink redirected the write, so an unrelated file ended up containing secrets.json — an arbitrary overwrite that also discloses every key to an attacker-chosen path. mkstemp uses O_EXCL.
• a write that failed partway left the temp behind; it's now cleaned up with the previous file intact.

This doesn't touch #287 (plaintext at rest generally) — that needs a keychain and is a much bigger change.