Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions coworker/secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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)
24 changes: 23 additions & 1 deletion tests/test_secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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})
Expand Down