diff --git a/.gitleaksignore b/.gitleaksignore index 1c94fc1ee..071e70afc 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 diff --git a/packages/agent/tests/test_runs.py b/packages/agent/tests/test_runs.py index bfd383adf..d4780000f 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 30dd1099f..7f19f8d49 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 000000000..cccbffb2d --- /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)