From c33147ff6f64e36c1b1f1ec969903e7974ec42bb Mon Sep 17 00:00:00 2001 From: Yunus Ardaguney Date: Tue, 10 Nov 2026 12:55:26 +0300 Subject: [PATCH 1/2] fix(agent): prevent race condition in RunStore fingerprint key creation (#2164) In RunStore._fingerprint_key, concurrent threads could race when creating fingerprint.key: one thread created the file via O_EXCL but hadn't finished writing it, causing a second thread encountering FileExistsError to read an empty or truncated key. Subsequent HMAC fingerprints would then disagree. Fix this by: 1. Writing the new key to a temporary file in root and fsyncing it before atomically linking (os.link) or replacing (os.replace) into place. 2. Rejecting any key shorter than 32 bytes on read with ValueError to fail closed against corrupt/empty keys. 3. Rejecting symlinks for fingerprint.key. 4. Adding tests asserting concurrent initialization safety and validation rejection in packages/flow/tests/test_agent_runs.py and packages/agent/tests/test_runs.py. Closes #2164. Signed-off-by: Yunus Ardaguney --- packages/agent/tests/test_runs.py | 4 +- packages/flow/openadapt_flow/agent/runs.py | 49 +++++++++++++---- packages/flow/tests/test_agent_runs.py | 64 ++++++++++++++++++++++ 3 files changed, 106 insertions(+), 11 deletions(-) create mode 100644 packages/flow/tests/test_agent_runs.py diff --git a/packages/agent/tests/test_runs.py b/packages/agent/tests/test_runs.py index bfd383adf9..d4780000f3 100644 --- a/packages/agent/tests/test_runs.py +++ b/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() diff --git a/packages/flow/openadapt_flow/agent/runs.py b/packages/flow/openadapt_flow/agent/runs.py index 30dd1099f7..7f19f8d493 100644 --- a/packages/flow/openadapt_flow/agent/runs.py +++ b/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( diff --git a/packages/flow/tests/test_agent_runs.py b/packages/flow/tests/test_agent_runs.py new file mode 100644 index 0000000000..cccbffb2d5 --- /dev/null +++ b/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) From c52851c8a1cc3a92486147f1d631f6ee34966da2 Mon Sep 17 00:00:00 2001 From: Yunus Ardaguney Date: Tue, 10 Nov 2026 12:46:17 +0300 Subject: [PATCH 2/2] ci: allowlist bundle compatibility fixtures for commit b0fd15ee in .gitleaksignore --- .gitleaksignore | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/.gitleaksignore b/.gitleaksignore index 1c94fc1ee5..071e70afca 100644 --- a/.gitleaksignore +++ b/.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