fix(agent): prevent race condition in RunStore fingerprint key creation (#2164)

.gitleaksignore

@@ -103,6 +103,40 @@ e8ec96725c4451aa64cb97230c18d04b55d98503:packages/flow/tests/e2e/test_mcp_teach_
 8ccf2cf3360b7d7720af04c36eee4f8e756bdf6e:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:554
 8ccf2cf3360b7d7720af04c36eee4f8e756bdf6e:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:630
 8ccf2cf3360b7d7720af04c36eee4f8e756bdf6e:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:642
+# The same fixtures after PR #1928 was merged to main (commit b0fd15ee).
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.2.0/provenance.json:generic-api-key:66
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.2.0/provenance.json:generic-api-key:182
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.2.0/provenance.json:generic-api-key:193
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.2.0/provenance.json:generic-api-key:248
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.2.0/provenance.json:generic-api-key:290
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.2.0/provenance.json:generic-api-key:348
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.2.0/provenance.json:generic-api-key:397
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.2.0/provenance.json:generic-api-key:416
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.2.0/provenance.json:generic-api-key:428
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:85
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:155
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:190
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:275
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:351
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:414
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:488
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:508
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:528
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:570
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:643
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.5.0/provenance.json:generic-api-key:655
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:85
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:132
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:168
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:254
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:316
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:379
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:468
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:490
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:511
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:554
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:630
+b0fd15ee8584cc3b646ae9bab04b4f9acbc59292:packages/flow/tests/fixtures/bundle_compatibility/2.6.0/provenance.json:generic-api-key:642
 # OpenAdapt#1872 network-save redaction tests (PR #2014, first commit): synthetic
 # credentials the redactor must withhold. Later commits assemble them at runtime.
 ccc7c617e5cd8082909b455ec6176a5e4c0dd4f0:packages/flow/tests/test_extension_network.py:generic-api-key:44

packages/agent/tests/test_runs.py

@@ -82,7 +82,9 @@ def test_reused_request_id_with_different_inputs_is_a_conflict(store):
 
 def test_records_never_hold_input_values(store, tmp_path):
     secret = "Jane Roe MRN 8812345"
-    first = store.begin(request_id="req-0001", workflow="w", inputs={"a": secret}, mode="production")
+    first = store.begin(
+        request_id="req-0001", workflow="w", inputs={"a": secret}, mode="production"
+    )
     finish(store, first, "saved_and_checked")
     for path in (tmp_path / "runs" / "openadapt-agent").rglob("*.json"):
         assert secret not in path.read_text()

packages/flow/openadapt_flow/agent/runs.py

@@ -172,19 +172,48 @@ def __init__(
 
     def _fingerprint_key(self) -> bytes:
         path = self.root / "fingerprint.key"
-        existing = (
-            path.read_bytes() if path.is_file() and not path.is_symlink() else b""
-        )
-        if len(existing) >= 32:
+        if path.is_symlink():
+            raise ValueError(f"fingerprint key at {path} must not be a symlink")
+        if path.is_file():
+            existing = path.read_bytes()
+            if len(existing) < 32:
+                raise ValueError(
+                    f"fingerprint key at {path} is shorter than 32 bytes ({len(existing)} bytes)"
+                )
             return existing
+
         key = secrets.token_bytes(32)
+        fd, tmp = tempfile.mkstemp(prefix=".tmp-key-", dir=self.root)
         try:
-            fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600)
-        except FileExistsError:
-            return path.read_bytes()
-        with os.fdopen(fd, "wb") as handle:
-            handle.write(key)
-        return key
+            with os.fdopen(fd, "wb") as handle:
+                handle.write(key)
+                handle.flush()
+                os.fsync(handle.fileno())
+            os.chmod(tmp, 0o600)
+            try:
+                os.link(tmp, path)
+            except FileExistsError:
+                pass
+            except OSError:
+                if not path.exists():
+                    try:
+                        os.replace(tmp, path)
+                    except OSError:
+                        pass
+        finally:
+            try:
+                os.unlink(tmp)
+            except OSError:
+                pass
+
+        if path.is_symlink():
+            raise ValueError(f"fingerprint key at {path} must not be a symlink")
+        existing = path.read_bytes() if path.is_file() else b""
+        if len(existing) < 32:
+            raise ValueError(
+                f"fingerprint key at {path} is shorter than 32 bytes ({len(existing)} bytes)"
+            )
+        return existing
 
     def fingerprint(self, workflow: str, inputs: Any) -> str:
         material = json.dumps(

packages/flow/tests/test_agent_runs.py

@@ -0,0 +1,64 @@
+"""Tests for RunStore fingerprint key creation, concurrency and validation."""
+
+from __future__ import annotations
+
+import concurrent.futures
+from pathlib import Path
+
+import pytest
+
+from openadapt_flow.agent.runs import RunStore
+
+
+def test_fingerprint_key_creation_races_under_concurrency_produce_same_key(
+    tmp_path: Path,
+) -> None:
+    runs_dir = tmp_path / "concurrent_runs"
+
+    def make_store() -> RunStore:
+        return RunStore(runs_dir)
+
+    with concurrent.futures.ThreadPoolExecutor(max_workers=8) as executor:
+        futures = [executor.submit(make_store) for _ in range(20)]
+        stores = [f.result() for f in futures]
+
+    keys = {s._key for s in stores}
+    assert len(keys) == 1
+    key = keys.pop()
+    assert len(key) == 32
+    assert (runs_dir / "openadapt-agent" / "fingerprint.key").read_bytes() == key
+
+
+def test_fingerprint_key_shorter_than_32_bytes_is_rejected(tmp_path: Path) -> None:
+    runs_dir = tmp_path / "short_key_runs"
+    agent_dir = runs_dir / "openadapt-agent"
+    agent_dir.mkdir(parents=True, exist_ok=True)
+    key_path = agent_dir / "fingerprint.key"
+    key_path.write_bytes(b"short-key")
+
+    with pytest.raises(ValueError, match="shorter than 32 bytes"):
+        RunStore(runs_dir)
+
+
+def test_fingerprint_key_empty_is_rejected(tmp_path: Path) -> None:
+    runs_dir = tmp_path / "empty_key_runs"
+    agent_dir = runs_dir / "openadapt-agent"
+    agent_dir.mkdir(parents=True, exist_ok=True)
+    key_path = agent_dir / "fingerprint.key"
+    key_path.write_bytes(b"")
+
+    with pytest.raises(ValueError, match="shorter than 32 bytes"):
+        RunStore(runs_dir)
+
+
+def test_fingerprint_key_symlink_is_rejected(tmp_path: Path) -> None:
+    runs_dir = tmp_path / "symlink_runs"
+    agent_dir = runs_dir / "openadapt-agent"
+    agent_dir.mkdir(parents=True, exist_ok=True)
+    target = tmp_path / "target.key"
+    target.write_bytes(b"x" * 32)
+    key_path = agent_dir / "fingerprint.key"
+    key_path.symlink_to(target)
+
+    with pytest.raises(ValueError, match="must not be a symlink"):
+        RunStore(runs_dir)