security: apply the Windows ACL protection to the workspace trust store

coworker/workspace_trust.py

@@ -9,11 +9,10 @@
 from __future__ import annotations
 
 import json
-import os
 from pathlib import Path
 from typing import Optional
 
-from .secrets import state_dir
+from .secrets import state_dir, write_private_text
 
 
 class WorkspaceTrustStore:
@@ -51,12 +50,14 @@ def set_trusted(self, workspace: str | Path, trusted: bool) -> str:
             values.add(canonical)
         else:
             values.discard(canonical)
-        self.path.parent.mkdir(parents=True, exist_ok=True)
-        tmp = self.path.with_name(f".{self.path.name}.{os.getpid()}.tmp")
-        tmp.write_text(
+        # `write_private_text` owns the atomic temp-write plus the platform-correct
+        # restriction. A bare `os.chmod(0o600)` here was a silent no-op on Windows,
+        # where os.chmod only toggles the read-only bit, leaving this file with the
+        # inherited SYSTEM/Administrators ACEs its parent carries. Since this file is
+        # the allowlist governing auto-approved command execution, it needs the same
+        # protection the SecretStore already applies to the sidecar token.
+        write_private_text(
+            self.path,
             json.dumps({"trusted_workspaces": sorted(values)}, indent=2) + "\n",
-            encoding="utf-8",
         )
-        os.chmod(tmp, 0o600)
-        tmp.replace(self.path)
         return canonical

tests/conftest.py

@@ -8,12 +8,38 @@
 
 from __future__ import annotations
 
+import subprocess
+import sys
+from pathlib import Path
+
 import pytest
 import pytest_asyncio
 
 from coworker.testing.fake_slack import FakeSlack
 
 
+def assert_user_only(path: Path) -> None:
+    """Assert `path` is readable by the current user alone, per the platform's own model.
+
+    POSIX says this with mode bits. Windows has no such bits: `os.stat` derives `st_mode`
+    from the read-only attribute only, so every writable file reports 0o666 and a
+    `st_mode & 0o777 == 0o600` check can never pass there, not even for a file that
+    `secrets._restrict_to_user` has correctly locked down with `icacls`. Assert the ACL
+    instead, mirroring how the protection is applied in the first place.
+    """
+    if sys.platform == "win32":
+        acl = subprocess.run(
+            ["icacls", str(path)], capture_output=True, text=True
+        ).stdout
+        # `/inheritance:r` strips inherited ACEs; their absence is what proves the file
+        # was restricted rather than left to inherit SYSTEM/Administrators from its parent.
+        assert "(I)" not in acl, f"{path} still carries inherited ACEs:\n{acl}"
+        for principal in ("NT AUTHORITY\\SYSTEM", "BUILTIN\\Administrators"):
+            assert principal not in acl, f"{path} grants {principal}:\n{acl}"
+    else:
+        assert (path.stat().st_mode & 0o777) == 0o600
+
+
 @pytest.fixture(autouse=True)
 def _isolated_state_dir(tmp_path, monkeypatch):
     """EVERY test gets an isolated SecretStore/state dir. Without this, any test that builds

tests/test_config.py

@@ -4,6 +4,8 @@
 
 from pathlib import Path
 
+from conftest import assert_user_only
+
 from coworker.config import load_config
 
 
@@ -77,7 +79,7 @@ def test_workspace_trust_is_canonical_and_user_owned(tmp_path):
     assert canonical == str(real.resolve())
     assert store.is_trusted(real)
     assert store.list() == [str(real.resolve())]
-    assert (store.path.stat().st_mode & 0o777) == 0o600
+    assert_user_only(store.path)
 
     store.set_trusted(real, False)
     assert not store.is_trusted(alias)

tests/test_server.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import pytest
+from conftest import assert_user_only
 from fastapi.testclient import TestClient
 
 from coworker.providers import (
@@ -477,7 +478,7 @@ def test_standalone_server_token_file_is_user_only(tmp_path, monkeypatch):
         assert path == tmp_path / "coworker-state" / "sidecar-9876.token"
         assert path.read_text().strip() == os.environ["COWORKER_API_TOKEN"]
         assert len(path.read_text().strip()) == 64
-        assert (path.stat().st_mode & 0o777) == 0o600
+        assert_user_only(path)
     finally:
         path.unlink(missing_ok=True)
         os.environ.pop("COWORKER_API_TOKEN", None)