diff --git a/.lyrashield-worker-pin b/.lyrashield-worker-pin index 025f267c..5f64bd9c 100644 --- a/.lyrashield-worker-pin +++ b/.lyrashield-worker-pin @@ -1 +1 @@ -68a3190441ad81ddab2f6c5246d25c9f8234f3e8 +c99cd61dd6890390adf20d82d1a3c01d63305d2c diff --git a/lyrashield/interface/main.py b/lyrashield/interface/main.py index 9844d776..c8afd559 100644 --- a/lyrashield/interface/main.py +++ b/lyrashield/interface/main.py @@ -41,6 +41,7 @@ TARGET_TYPE_CHOICES, _is_full_git_commit_sha, _is_git_object_id, + _read_only_head_revision, assign_workspace_subdirs, build_final_stats_text, build_mount_targets_info, @@ -1162,6 +1163,16 @@ def _load_resume_state(args: argparse.Namespace, parser: argparse.ArgumentParser if not targets_info: parser.error(f"--resume {args.resume}: run.json has no targets_info") + # A recorded immutable revision binds every restored repository clone: + # acquisition pins each checkout to --repository-revision or the recorded + # diff head, so resume must find that exact HEAD. The comparison is + # read-only — never a checkout, fetch or repair — so a tampered cache is + # left untouched for inspection rather than silently reset. + expected_revision_raw = state.get("repository_revision") or state.get("diff_head") + expected_revision = ( + str(expected_revision_raw).strip().lower() if expected_revision_raw else None + ) + cloned_repo_paths: set[Path] = set() for target in targets_info: details_raw: Any = target.get("details") @@ -1186,6 +1197,16 @@ def _load_resume_state(args: argparse.Namespace, parser: argparse.ArgumentParser f"It was deleted between runs. Pick a fresh --run-name to " f"re-clone, or restore the directory before resuming." ) + if expected_revision: + actual_head = _read_only_head_revision(cloned_path) + if actual_head != expected_revision: + parser.error( + f"--resume {args.resume}: cloned repo at {cloned} has HEAD " + f"{actual_head or 'unresolved'} but the run recorded " + f"revision {expected_revision}. The cached clone changed " + "between runs; refusing to resume from altered source. " + "Pick a fresh --run-name to re-clone." + ) cloned_repo_paths.add(cloned_path) args.targets_info = targets_info diff --git a/lyrashield/interface/utils.py b/lyrashield/interface/utils.py index 79bee06d..9c409573 100644 --- a/lyrashield/interface/utils.py +++ b/lyrashield/interface/utils.py @@ -2022,6 +2022,23 @@ def _ensure_commit_available(repo_path: Path, sha: str, reason: str) -> None: ) +def _read_only_head_revision(repo_path: Path) -> str | None: + """Return HEAD's commit object ID or ``None`` when it cannot be resolved. + + Unlike :func:`_assert_checkout_revision` this is purely observational: + ``rev-parse HEAD`` never checks out, fetches or otherwise modifies the + clone, so a possibly-altered resume cache is left exactly as found. + """ + try: + result = _run_git_command(repo_path, ["rev-parse", "HEAD"], check=False) + except (OSError, subprocess.SubprocessError): + return None + if result.returncode != 0: + return None + sha = result.stdout.strip() + return sha or None + + def _assert_checkout_revision(repo_path: Path, revision: str) -> None: """Detach the checkout at *revision* and assert HEAD's object ID matches. diff --git a/lyrashield/tools/proxy/caido_api.py b/lyrashield/tools/proxy/caido_api.py index da11150f..e030ee18 100644 --- a/lyrashield/tools/proxy/caido_api.py +++ b/lyrashield/tools/proxy/caido_api.py @@ -4,6 +4,7 @@ import asyncio import dataclasses +import importlib.metadata import ipaddress import json import logging @@ -614,6 +615,20 @@ async def get_request_with_client( _INVALID_HEADER_RE = re.compile(r"[\r\n\x00]") +def _default_replay_user_agent() -> str: + """Default replay User-Agent identifying the LyraShield product. + + The ``lyrashield-engine`` dist may not be installed where this module runs + standalone inside the sandbox, so an absent package falls back to + ``unknown`` rather than failing the replay. + """ + try: + engine_version = importlib.metadata.version("lyrashield-engine") + except importlib.metadata.PackageNotFoundError: + engine_version = "unknown" + return f"LyraShield/{engine_version} (+https://lyrashieldai.com)" + + def build_raw_request( *, method: str, @@ -639,7 +654,10 @@ def build_raw_request( final_headers = {**headers} final_headers.setdefault("Host", parsed.netloc) - final_headers.setdefault("User-Agent", "strix") + # Header names are case-insensitive: a caller-supplied User-Agent in any + # case wins; only an absent one gets the LyraShield default. + if not any(name.lower() == "user-agent" for name in final_headers): + final_headers["User-Agent"] = _default_replay_user_agent() for k, v in final_headers.items(): if _INVALID_HEADER_RE.search(k) or _INVALID_HEADER_RE.search(v): raise ValueError(f"Header contains forbidden characters: {k!r}: {v!r}") diff --git a/scripts/customer-branding-allowlist.json b/scripts/customer-branding-allowlist.json index 1d65ea8d..6f7f1e69 100644 --- a/scripts/customer-branding-allowlist.json +++ b/scripts/customer-branding-allowlist.json @@ -575,7 +575,6 @@ " # STRIX_RUN_ID is set (inside the container). A wrong-run policy is", " ``STRIX_SANDBOX_ALLOW_PRIVATE_EGRESS`` opt-in is honored). When a policy", " expected_run_id = os.environ.get(\"STRIX_RUN_ID\", \"\").strip()", - " final_headers.setdefault(\"User-Agent\", \"strix\")", " return os.environ.get(\"STRIX_CAIDO_URL\", _DEFAULT_CAIDO_URL).rstrip(\"/\")", " return os.environ.get(\"STRIX_SANDBOX_ALLOW_HOST_GATEWAY\", \"\").strip().lower() in {", "_PRIVATE_EGRESS_OPT_IN_ENV = \"STRIX_SANDBOX_ALLOW_PRIVATE_EGRESS\"", diff --git a/scripts/worker-contract-tests.txt b/scripts/worker-contract-tests.txt index e7a0beff..6159cc0e 100644 --- a/scripts/worker-contract-tests.txt +++ b/scripts/worker-contract-tests.txt @@ -3,5 +3,6 @@ apps/worker/src/engine/command-builder.test.ts apps/worker/src/docker-runtime.test.ts apps/worker/src/engine/output-parser.test.ts apps/worker/src/engine/result-integrity.test.ts +apps/worker/src/engine/run-json-1-1.test.ts apps/worker/src/engine/runner.test.ts apps/worker/src/jobs/run-scan.job.test.ts diff --git a/tests/test_customer_branding.py b/tests/test_customer_branding.py index 17593184..19cd905f 100644 --- a/tests/test_customer_branding.py +++ b/tests/test_customer_branding.py @@ -59,3 +59,16 @@ def test_lifecycle_output_is_checked(tmp_path: Path) -> None: path.parent.mkdir(parents=True) path.write_text('logger.info("Strix scan done")') assert GATE.violations(tmp_path, {}) + + +def test_upstream_brand_default_header_fails_gate(tmp_path: Path) -> None: + """An upstream brand in an outbound default header is customer-visible. + + The shipped allowlist must not exempt the old default replay User-Agent, + so a regression to it fails the gate. + """ + allowed = json.loads((ROOT / "scripts/customer-branding-allowlist.json").read_text()) + path = tmp_path / "lyrashield/tools/proxy/caido_api.py" + path.parent.mkdir(parents=True) + path.write_text(' final_headers.setdefault("User-Agent", "strix")\n') + assert GATE.violations(tmp_path, allowed["source_lines"]) diff --git a/tests/test_replay_scope_admission.py b/tests/test_replay_scope_admission.py index 997b41d6..f6d9dad7 100644 --- a/tests/test_replay_scope_admission.py +++ b/tests/test_replay_scope_admission.py @@ -8,6 +8,7 @@ from __future__ import annotations import contextlib +from importlib.metadata import version from pathlib import Path from typing import TYPE_CHECKING, Any @@ -132,6 +133,44 @@ def test_fail_closed_policy_denies_everything(tmp_path: Path, monkeypatch: Any) assert decisions["violations"][0]["rule"] == "outside_authorized_scope" +def _header_values(raw: bytes, name: str) -> list[str]: + prefix = f"{name.lower()}:" + return [ + line.split(":", 1)[1].strip() + for line in raw.decode("iso-8859-1").split("\r\n") + if line.lower().startswith(prefix) + ] + + +def test_replay_default_user_agent_identifies_lyrashield() -> None: + """Replayed requests default to the LyraShield product User-Agent.""" + _conn, raw = _send("https://example.com/") + expected = f"LyraShield/{version('lyrashield-engine')} (+https://lyrashieldai.com)" + assert _header_values(raw, "user-agent") == [expected] + + +def test_replay_caller_user_agent_wins() -> None: + """A caller-supplied User-Agent is preserved; no default header is added.""" + _conn, raw = caido_api.build_raw_request( + method="GET", + url="https://example.com/", + headers={"User-Agent": "custom-agent/9"}, + body="", + ) + assert _header_values(raw, "user-agent") == ["custom-agent/9"] + + +def test_replay_caller_user_agent_wins_case_insensitively() -> None: + """Header names are case-insensitive: a lowercase caller UA still wins.""" + _conn, raw = caido_api.build_raw_request( + method="GET", + url="https://example.com/", + headers={"user-agent": "custom-agent/9"}, + body="", + ) + assert _header_values(raw, "user-agent") == ["custom-agent/9"] + + def test_violation_ledger_is_bounded(monkeypatch: Any) -> None: monkeypatch.setattr(caido_api, "_SCOPE_VIOLATION_LIMIT", 5) caido_api.clear_scope_decisions() diff --git a/tests/test_resume_state.py b/tests/test_resume_state.py index db219386..2e0a15b8 100644 --- a/tests/test_resume_state.py +++ b/tests/test_resume_state.py @@ -2,19 +2,98 @@ import argparse import importlib +import subprocess # nosec B404 from types import SimpleNamespace -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any if TYPE_CHECKING: from pathlib import Path - import pytest +import pytest main_module = importlib.import_module("lyrashield.interface.main") +def _git(repo: Path, *args: str) -> str: + result = subprocess.run( # noqa: S603 # nosec B603 + ["git", "-C", str(repo), *args], # noqa: S607 + capture_output=True, + text=True, + check=True, + timeout=30, + ) + return result.stdout.strip() + + +def _git_clone(path: Path) -> str: + """Create a fixture clone under the cache root and return its HEAD.""" + path.mkdir(parents=True) + subprocess.run( # noqa: S603 # nosec B603 + ["git", "init", "-b", "main", str(path)], # noqa: S607 + check=True, + capture_output=True, + ) + _git(path, "config", "user.email", "test@example.com") + _git(path, "config", "user.name", "Test") + _git(path, "config", "commit.gpgsign", "false") + (path / "app.py").write_text("print('hi')\n", encoding="utf-8") + _git(path, "add", "-A") + _git(path, "commit", "-m", "initial") + return _git(path, "rev-parse", "HEAD") + + +def _commit_file(repo: Path, name: str, content: str, message: str) -> str: + (repo / name).write_text(content, encoding="utf-8") + _git(repo, "add", "-A") + _git(repo, "commit", "-m", message) + return _git(repo, "rev-parse", "HEAD") + + +def _resume_args() -> argparse.Namespace: + return argparse.Namespace(resume="resume-run", instruction=None, scan_mode="deep") + + +def _stage_resumed_repository_run( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + run_record: dict[str, Any], + *, + clone: Path | None = None, +) -> Path: + """Stage a resumable run whose repository target is a fixture clone. + + When *clone* is omitted a fresh single-commit clone is created under the + fake cache root. Returns the clone path either way. + """ + run_dir = tmp_path / "runs" / "resume-run" + run_dir.mkdir(parents=True, exist_ok=True) + (run_dir / "run.json").write_text("{}", encoding="utf-8") + if clone is None: + clone = tmp_path / "strix_repos" / "resume-run" / "repo" + _git_clone(clone) + targets = [ + { + "type": "repository", + "details": { + "target_repo": "https://example.com/org/repo.git", + "cloned_repo_path": str(clone), + }, + } + ] + monkeypatch.setattr(main_module, "run_dir_for", lambda _name: run_dir) + monkeypatch.setattr(main_module, "runs_base_dir", lambda: tmp_path / "runs") + monkeypatch.setattr(main_module.tempfile, "gettempdir", lambda: str(tmp_path)) + monkeypatch.setattr(main_module, "read_run_record", lambda _run_dir: run_record) + monkeypatch.setattr( + main_module, + "read_resume_record", + lambda _run_dir: {"targets_info": targets}, + ) + return clone + + def test_resume_mounts_product_docker_repository_clone( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -53,3 +132,88 @@ def test_resume_mounts_product_docker_repository_clone( assert args.local_sources == [ {"source_path": str(clone), "workspace_subdir": "repo", "mount": True} ] + + +def test_resume_accepts_clone_at_recorded_revision( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A cached clone still at the recorded immutable revision resumes.""" + clone = tmp_path / "strix_repos" / "resume-run" / "repo" + recorded = _git_clone(clone) + _stage_resumed_repository_run( + tmp_path, + monkeypatch, + {"repository_revision": recorded, "scan_mode": "standard"}, + clone=clone, + ) + args = _resume_args() + + main_module._load_resume_state(args, argparse.ArgumentParser()) + + assert args.repository_revision == recorded + + +def test_resume_refuses_a_clone_whose_head_moved( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """A new commit on the cached clone between runs must refuse the resume.""" + clone = tmp_path / "strix_repos" / "resume-run" / "repo" + recorded = _git_clone(clone) + _stage_resumed_repository_run( + tmp_path, + monkeypatch, + {"repository_revision": recorded, "scan_mode": "standard"}, + clone=clone, + ) + # Simulate a cache alteration: a fresh commit moves HEAD off the revision. + _git(clone, "checkout", "--detach", recorded) + altered = _commit_file(clone, "app.py", "print('tampered')\n", "cache alteration") + assert altered != recorded + + with pytest.raises(SystemExit) as exc_info: + main_module._load_resume_state(_resume_args(), argparse.ArgumentParser()) + + assert exc_info.value.code == 2 + assert "--resume resume-run" in capsys.readouterr().err + # The refusal must leave the altered clone untouched: no checkout, no + # reset, no fetch — HEAD still points at the tampered commit. + assert _git(clone, "rev-parse", "HEAD") == altered + + +def test_resume_refuses_a_detached_clone_off_the_recorded_diff_head( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """The recorded diff head pins the clone even without repository_revision.""" + clone = tmp_path / "strix_repos" / "resume-run" / "repo" + recorded = _git_clone(clone) + _commit_file(clone, "later.py", "x = 1\n", "later") + _git(clone, "checkout", "--detach", recorded) + _stage_resumed_repository_run( + tmp_path, + monkeypatch, + {"diff_head": recorded, "scan_mode": "standard"}, + clone=clone, + ) + # Tamper: move HEAD to the newer commit the run never recorded. + _git(clone, "checkout", "--detach", "main") + moved = _git(clone, "rev-parse", "HEAD") + assert moved != recorded + + with pytest.raises(SystemExit) as exc_info: + main_module._load_resume_state(_resume_args(), argparse.ArgumentParser()) + + assert exc_info.value.code == 2 + assert "--resume resume-run" in capsys.readouterr().err + assert _git(clone, "rev-parse", "HEAD") == moved + + +def test_resume_without_recorded_revision_skips_head_check( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Runs recorded before immutable revisions resume without a HEAD check.""" + clone = _stage_resumed_repository_run(tmp_path, monkeypatch, {"scan_mode": "standard"}) + args = _resume_args() + + main_module._load_resume_state(args, argparse.ArgumentParser()) + + assert args.targets_info[0]["details"]["cloned_repo_path"] == str(clone)