diff --git a/coworker/secrets.py b/coworker/secrets.py index 6c1c0326..719164df 100644 --- a/coworker/secrets.py +++ b/coworker/secrets.py @@ -88,6 +88,18 @@ def _restrict_to_user(path: Path, *, is_dir: bool) -> None: os.chmod(path, 0o700 if is_dir else 0o600) +def _write_user_only(path: Path, content: str) -> None: + """Write `content` to a file that is owner-only from its first byte. + + `Path.write_text` creates the file with umask-default permissions (typically 0644) and a + later chmod leaves a window where the plaintext is readable by other local users. Opening + with mode 0600 closes that window on POSIX. Windows ignores the mode bits, so callers + still run the icacls-based `_restrict_to_user` afterwards.""" + fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(content) + + def write_private_text(path: str | Path, content: str) -> Path: """Atomically write a user-only text file using the SecretStore's OS protections.""" target = Path(path).expanduser() @@ -97,7 +109,7 @@ def write_private_text(path: str | Path, content: str) -> Path: except OSError: pass tmp = target.with_name(target.name + ".tmp") - tmp.write_text(content, encoding="utf-8") + _write_user_only(tmp, content) _restrict_to_user(tmp, is_dir=False) os.replace(tmp, target) return target @@ -188,6 +200,6 @@ def _write(self, store: dict[str, Any]) -> None: except OSError: pass tmp = self.path.with_name(self.path.name + ".tmp") - tmp.write_text(json.dumps(store, indent=2), encoding="utf-8") + _write_user_only(tmp, json.dumps(store, indent=2)) _restrict_to_user(tmp, is_dir=False) os.replace(tmp, self.path) diff --git a/tests/test_secrets.py b/tests/test_secrets.py index c723fa95..d90ecdbb 100644 --- a/tests/test_secrets.py +++ b/tests/test_secrets.py @@ -8,7 +8,10 @@ import sys import time -from coworker.secrets import SecretStore +import pytest + +import coworker.secrets as secrets_mod +from coworker.secrets import SecretStore, write_private_text def test_put_get_round_trip(tmp_path): @@ -79,6 +82,25 @@ def test_secrets_file_is_restricted(tmp_path): assert stat.S_IMODE(os.stat(path).st_mode) == 0o600 +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX mode bits only") +def test_secrets_never_world_readable_during_write(tmp_path, monkeypatch): + """Files holding secrets must be owner-only from creation, not chmod'd after the + plaintext already landed on disk (that ordering leaves a window where other local + users can read credentials, and a failed chmod leaves them exposed indefinitely). + Neuter the after-the-fact chmod and assert the mode is still 0600.""" + monkeypatch.setattr(secrets_mod, "_restrict_to_user", lambda *a, **k: None) + old_umask = os.umask(0o022) # permissive umask so creation-time perms do the work + try: + store_path = tmp_path / "secrets.json" + SecretStore(store_path).put("x", {"a": 1}) + assert stat.S_IMODE(os.stat(store_path).st_mode) == 0o600 + + key_path = write_private_text(tmp_path / "id_key", "PRIVATE") + assert stat.S_IMODE(os.stat(key_path).st_mode) == 0o600 + finally: + os.umask(old_umask) + + def test_delete(tmp_path): store = SecretStore(tmp_path / "secrets.json") store.put("x", {"a": 1})