From 7b7de6aea7a32398f48a2a02d0e4b105390e411f Mon Sep 17 00:00:00 2001 From: Claude Code Date: Thu, 8 Oct 2026 04:37:16 +0530 Subject: [PATCH 1/3] fix(engine): correct the version lookup, align tool pins and drop proven-dead members The engine looked up the distribution "strix-agent", which no build of this project installs. pyproject.toml names the package lyrashield-engine, so three call sites always raised PackageNotFoundError: the SARIF tool.driver.version field was silently omitted, the TUI header showed "dev" and the telemetry version was "unknown". Add lyrashield/version.py with one engine_version() helper over the real distribution name and an explicit fallback so a frozen or uninstalled build never raises. Route the three wrong call sites through it. The four lookups that already used the product name are untouched. Align the pre-commit hook revisions to the locked versions (ruff v0.15.20 and bandit 1.9.4), so a local hook cannot pass lint or formatting the locked CI gate rejects. Bandit reads no severity floor from pyproject.toml; only the -l flag filters. Remove the inert severity key and state the real floor with -l in every invocation. A medium floor was rejected: it would drop B311, B403 and B405-B409, and the ruff equivalents of B403/B405-B409 are preview-only and inactive, so medium would weaken the gate. Delete only members with whole-repo grep proof of zero references: ancillary_cost_total, set_sandbox_cleanup_status, wait_kind_of, the utils.process_pull_line shim, scripts/docker.sh, the nonexistent root Dockerfile path in .trivyignore.yaml and the tenacity hidden import. Bound the fake-daemon wait in the image-pull deadline test and cache the Chromium download in CI. Co-Authored-By: Claude Code --- .github/workflows/ci.yml | 9 ++ .pre-commit-config.yaml | 12 +-- .trivyignore.yaml | 1 - Makefile | 2 +- lyrashield/artifacts/state.py | 11 +-- lyrashield/artifacts/usage.py | 4 - lyrashield/interface/tui/app.py | 8 +- lyrashield/interface/utils.py | 8 -- lyrashield/lifecycle/agents.py | 4 - lyrashield/telemetry/_common.py | 11 +-- lyrashield/version.py | 28 ++++++ pyproject.toml | 9 +- scripts/customer-branding-allowlist.json | 4 - scripts/docker.sh | 16 ---- scripts/verify-controlled-derivative.sh | 2 +- strix.spec | 3 - tests/test_engine_version.py | 110 +++++++++++++++++++++++ tests/test_image_pull_deadline.py | 12 ++- tests/test_quality_environments.py | 56 ++++++++++++ tests/test_release_build.py | 40 +++++++++ 20 files changed, 281 insertions(+), 69 deletions(-) create mode 100644 lyrashield/version.py delete mode 100755 scripts/docker.sh create mode 100644 tests/test_engine_version.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b16ca2f5..bc639005 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -126,6 +126,15 @@ jobs: if: steps.changed_paths.outputs.docs_only != 'true' run: python3 scripts/report_twin_drift.py + - name: Cache Playwright browser download + if: steps.changed_paths.outputs.docs_only != 'true' + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: ~/.cache/ms-playwright + key: playwright-chromium-${{ runner.os }}-${{ hashFiles('lyrashield/interface/viewer/frontend/package-lock.json') }} + restore-keys: | + playwright-chromium-${{ runner.os }}- + - name: Verify owned viewer source and committed build if: steps.changed_paths.outputs.docs_only != 'true' working-directory: lyrashield/interface/viewer/frontend diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a5bd5cdb..56777bf2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,7 +1,9 @@ repos: - # Ruff for fast linting and formatting + # Ruff for fast linting and formatting. + # rev is the version uv.lock and CI resolve, so a local hook cannot pass + # formatting or lint that the locked gate rejects. - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.11.13 + rev: v0.15.20 hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] @@ -32,12 +34,12 @@ repos: - id: check-case-conflict - id: check-docstring-first - # Security checks with bandit + # Security checks with bandit, pinned to the locked version. - repo: https://github.com/PyCQA/bandit - rev: 1.8.3 + rev: 1.9.4 hooks: - id: bandit - args: [-c, pyproject.toml] + args: [-c, pyproject.toml, -l] # Additional Python code quality checks - repo: https://github.com/asottile/pyupgrade diff --git a/.trivyignore.yaml b/.trivyignore.yaml index 2006bdfc..e25dfc8c 100644 --- a/.trivyignore.yaml +++ b/.trivyignore.yaml @@ -12,5 +12,4 @@ misconfigurations: - id: AVD-DS-0002 paths: - containers/Dockerfile - - Dockerfile statement: Container starts as root then immediately drops to unprivileged pentester user via setpriv; no sudo or docker socket is available to the agent. diff --git a/Makefile b/Makefile index 18b61a52..00cc081b 100644 --- a/Makefile +++ b/Makefile @@ -58,7 +58,7 @@ type-check: security: @echo "🔒 Running security checks with bandit..." - uv run bandit -r strix lyrashield_adapter lyrashield -q -c pyproject.toml + uv run bandit -r strix lyrashield_adapter lyrashield -q -c pyproject.toml -l @echo "✅ Security checks complete!" check-all: diff --git a/lyrashield/artifacts/state.py b/lyrashield/artifacts/state.py index 66023969..0e8818e1 100644 --- a/lyrashield/artifacts/state.py +++ b/lyrashield/artifacts/state.py @@ -6,7 +6,6 @@ import threading from collections.abc import Callable from datetime import UTC, datetime -from importlib.metadata import PackageNotFoundError, version from pathlib import Path from typing import Any, Optional, cast @@ -149,6 +148,7 @@ from lyrashield.runtime.session_manager import CLEANUP_FAILED, CLEANUP_REMOVED from lyrashield.telemetry import posthog, scarf from lyrashield.utils.redaction import redact_text +from lyrashield.version import engine_version from strix.config import codex from strix.config.loader import load_settings from strix.core.paths import run_dir_for, runtime_state_dir @@ -192,10 +192,7 @@ def _strix_version() -> str | None: """Best-effort package version for the SARIF tool.driver.version field.""" - try: - return version("strix-agent") - except PackageNotFoundError: - return None + return engine_version() def get_global_report_state() -> Optional["ReportState"]: @@ -630,10 +627,6 @@ def set_terminal_reason(self, reason: str) -> None: if self.run_record.get("status") != "completed": self.run_record["terminal_reason"] = reason - def set_sandbox_cleanup_status(self, sandbox_removed: bool) -> None: - """Backward-compatible boolean wrapper around :meth:`set_cleanup_outcome`.""" - self.set_cleanup_outcome(CLEANUP_REMOVED if sandbox_removed else CLEANUP_FAILED) - def set_cleanup_outcome( self, outcome: str, diff --git a/lyrashield/artifacts/usage.py b/lyrashield/artifacts/usage.py index 36974a7a..d030379b 100644 --- a/lyrashield/artifacts/usage.py +++ b/lyrashield/artifacts/usage.py @@ -225,10 +225,6 @@ def record_ancillary_cost(self, category: str, cost: Any) -> None: self._ancillary_costs.get(category, 0.0) + numeric_cost ) - @property - def ancillary_cost_total(self) -> float: - return _round_cost(sum(self._ancillary_costs.values())) - @property def total_cost(self) -> float: return _round_cost(self._total_cost + sum(self._ancillary_costs.values())) diff --git a/lyrashield/interface/tui/app.py b/lyrashield/interface/tui/app.py index 664caf12..003075ca 100644 --- a/lyrashield/interface/tui/app.py +++ b/lyrashield/interface/tui/app.py @@ -9,8 +9,6 @@ import threading import webbrowser from collections.abc import Callable -from importlib.metadata import PackageNotFoundError -from importlib.metadata import version as pkg_version from pathlib import Path from typing import TYPE_CHECKING, Any, ClassVar @@ -51,6 +49,7 @@ from lyrashield.lifecycle.runner import run_strix_scan from lyrashield.policy.models import is_recommended_or_frontier_model from lyrashield.runtime import session_manager +from lyrashield.version import engine_version from strix.config import load_settings @@ -58,10 +57,7 @@ def get_package_version() -> str: - try: - return pkg_version("strix-agent") - except PackageNotFoundError: - return "dev" + return engine_version() or "dev" class ChatTextArea(TextArea): diff --git a/lyrashield/interface/utils.py b/lyrashield/interface/utils.py index 34054299..59c6947f 100644 --- a/lyrashield/interface/utils.py +++ b/lyrashield/interface/utils.py @@ -583,14 +583,6 @@ def update_layer_status(layers_info: dict[str, str], layer_id: str, layer_status _update_layer_status(layers_info, layer_id, layer_status) -def process_pull_line( - line: dict[str, Any], layers_info: dict[str, str], status: Any, last_update: str -) -> str: - from lyrashield.interface.image_pull import process_pull_line as _process_pull_line - - return _process_pull_line(line, layers_info, status, last_update) - - def validate_config_file(config_path: str) -> Path: console = Console() path = Path(config_path) diff --git a/lyrashield/lifecycle/agents.py b/lyrashield/lifecycle/agents.py index d9a467cd..08b3f50c 100644 --- a/lyrashield/lifecycle/agents.py +++ b/lyrashield/lifecycle/agents.py @@ -282,10 +282,6 @@ async def park_waiting(self, agent_id: str, *, wait_kind: WaitKind) -> None: self.wait_kinds[agent_id] = wait_kind await self.set_status(agent_id, "waiting") - async def wait_kind_of(self, agent_id: str) -> WaitKind | None: - async with self._lock: - return self.wait_kinds.get(agent_id) - async def record_recovery(self, agent_id: str) -> int: """Count a turn that ended without a lifecycle tool call; return the new total. diff --git a/lyrashield/telemetry/_common.py b/lyrashield/telemetry/_common.py index 2d1e98ec..5b32dca1 100644 --- a/lyrashield/telemetry/_common.py +++ b/lyrashield/telemetry/_common.py @@ -4,11 +4,12 @@ import logging import platform import sys -from importlib.metadata import PackageNotFoundError, version from pathlib import Path from typing import Any, Protocol from uuid import uuid4 +from lyrashield.version import engine_version + logger = logging.getLogger(__name__) @@ -33,11 +34,11 @@ def get_total_llm_usage(self) -> dict[str, Any]: ... def get_version() -> str: - try: - return version("strix-agent") - except PackageNotFoundError: - logger.debug("strix-agent version lookup failed", exc_info=True) + resolved = engine_version() + if resolved is None: + logger.debug("engine version lookup failed", exc_info=True) return "unknown" + return resolved def is_first_run() -> bool: diff --git a/lyrashield/version.py b/lyrashield/version.py new file mode 100644 index 00000000..2892a7b1 --- /dev/null +++ b/lyrashield/version.py @@ -0,0 +1,28 @@ +"""Single source of truth for the engine's own package version. + +The distribution is named ``lyrashield-engine`` in ``pyproject.toml``. Looking +up any other identifier raises ``PackageNotFoundError`` in every environment that +installed this project, which silently degrades the SARIF +``tool.driver.version`` field and the TUI header. + +``engine_version`` never raises: a frozen PyInstaller build or a source checkout +that was never installed has no distribution metadata at all, and a version +string is informational rather than load-bearing. +""" + +from __future__ import annotations + +from importlib.metadata import PackageNotFoundError, version + + +DISTRIBUTION_NAME = "lyrashield-engine" + + +def engine_version() -> str | None: + """Return the installed engine version, or ``None`` when metadata is absent.""" + try: + return version(DISTRIBUTION_NAME) + except PackageNotFoundError: + return None + except Exception: # noqa: BLE001 - metadata backends can fail in frozen builds + return None diff --git a/pyproject.toml b/pyproject.toml index d07f98f4..5bcad0a8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -433,4 +433,11 @@ line-ending = "auto" [tool.bandit] exclude_dirs = ["docs", "build", "dist"] skips = ["B101", "B105", "B601", "B404", "B603", "B607"] # Skip assert, hardcoded-password false positives, shell injection, subprocess import and partial path checks -severity = "medium" +# Bandit reads no severity floor from this file. Only the `-l/--level` CLI flag +# filters, so a `severity = "medium"` key here was inert and misdescribed the +# gate. It is deliberately absent: the enforced floor is LOW, and the +# invocations (Makefile `security`, .pre-commit-config.yaml, and +# scripts/verify-controlled-derivative.sh) carry the explicit `-l` flag that +# enforces it. A medium floor would drop B311, B403 and B405-B409, and the ruff +# equivalents of B403/B405-B409 (S403, S405-S409) are preview-only and inactive +# in this repo, so enforcing medium would silently weaken the security gate. diff --git a/scripts/customer-branding-allowlist.json b/scripts/customer-branding-allowlist.json index 4782d0f6..f66d1f78 100644 --- a/scripts/customer-branding-allowlist.json +++ b/scripts/customer-branding-allowlist.json @@ -179,7 +179,6 @@ "# Modifications \u00a9 2026 LyraShield; based on upstream Strix (Apache-2.0)", "from lyrashield.lifecycle.runner import run_strix_scan", "from strix.config import load_settings", - " return pkg_version(\"strix-agent\")", " self._app_reference: StrixTUIApp | None = None", " def set_app_reference(self, app: \"StrixTUIApp\") -> None:", " if event.button.id == \"stop_agent\" and isinstance(app, StrixTUIApp):", @@ -331,7 +330,6 @@ ], "lyrashield/artifacts/state.py": [ " tool_version=_strix_version(),", - " return version(\"strix-agent\")", " persistence. This store keeps only Strix-owned scan artifacts and", " scan streams, so the OpenRouter streaming handler (see strix.config.models)", "# Modifications \u00a9 2026 LyraShield; based on upstream Strix (Apache-2.0)", @@ -517,8 +515,6 @@ ], "lyrashield/telemetry/_common.py": [ " \"strix_version\": get_version(),", - " logger.debug(\"strix-agent version lookup failed\", exc_info=True)", - " return version(\"strix-agent\")", " marker = Path.home() / \".strix\" / \".seen\"", "# Modifications \u00a9 2026 LyraShield; based on upstream Strix (Apache-2.0)" ], diff --git a/scripts/docker.sh b/scripts/docker.sh deleted file mode 100755 index 088c4822..00000000 --- a/scripts/docker.sh +++ /dev/null @@ -1,16 +0,0 @@ -#!/bin/bash -set -e - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" - -IMAGE="strix-sandbox" -TAG="${1:-dev}" - -echo "Building $IMAGE:$TAG ..." -docker build \ - -f "$PROJECT_ROOT/containers/Dockerfile" \ - -t "$IMAGE:$TAG" \ - "$PROJECT_ROOT" - -echo "Done: $IMAGE:$TAG" diff --git a/scripts/verify-controlled-derivative.sh b/scripts/verify-controlled-derivative.sh index c2457a62..906b3f87 100755 --- a/scripts/verify-controlled-derivative.sh +++ b/scripts/verify-controlled-derivative.sh @@ -125,4 +125,4 @@ uv run ruff check . uv run ruff format --check . uv run pytest --durations=25 -W error::pydantic.PydanticDeprecatedSince211 uv run mypy strix lyrashield_adapter lyrashield -uv run bandit -c pyproject.toml -r strix lyrashield_adapter lyrashield -q +uv run bandit -c pyproject.toml -r strix lyrashield_adapter lyrashield -q -l diff --git a/strix.spec b/strix.spec index 913b405b..96fb3d9c 100644 --- a/strix.spec +++ b/strix.spec @@ -96,9 +96,6 @@ hiddenimports = [ 'tiktoken_ext', 'tiktoken_ext.openai_public', - # Tenacity retry - 'tenacity', - # CVSS scoring 'cvss', diff --git a/tests/test_engine_version.py b/tests/test_engine_version.py new file mode 100644 index 00000000..f6017c53 --- /dev/null +++ b/tests/test_engine_version.py @@ -0,0 +1,110 @@ +"""The engine must look up its own distribution name and never raise without it. + +Regression: the three legacy call sites used ``strix-agent``, a distribution +that does not exist in any build of this project, so the SARIF +``tool.driver.version`` field was silently omitted and the TUI header fell back +to ``dev``. The helper must also survive a frozen build where no distribution +metadata exists at all. +""" + +from __future__ import annotations + +import importlib.metadata +import json +from typing import TYPE_CHECKING, Any +from unittest.mock import Mock + +import pytest + +from lyrashield.artifacts.state import ReportState, set_global_report_state +from lyrashield.version import DISTRIBUTION_NAME, engine_version + + +if TYPE_CHECKING: + from pathlib import Path + + +def _finding() -> dict[str, Any]: + return { + "id": "vuln-0001", + "title": "SQL Injection in get_user", + "severity": "critical", + "cwe": "CWE-89", + "timestamp": "2026-07-02 10:00:00 UTC", + "code_locations": [{"file": "app.py", "start_line": 4}], + } + + +@pytest.fixture +def report_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> ReportState: + monkeypatch.chdir(tmp_path) + state = ReportState(run_name="test-run") + set_global_report_state(state) + return state + + +def test_distribution_name_is_the_project_name() -> None: + assert DISTRIBUTION_NAME == "lyrashield-engine" + + +def test_installed_build_reports_a_concrete_version() -> None: + # The repo .venv installs this project, so the helper must resolve a value + # here. A None result means the distribution name is wrong again. + assert engine_version() is not None + + +def test_sarif_write_path_emits_tool_driver_version(report_state: ReportState) -> None: + """The real projection path must put a version in SARIF tool.driver. + + ``tests/test_sarif.py`` passes ``tool_version`` explicitly, so it cannot + catch a broken lookup. This drives the production call chain instead: + ``ReportState._write_report_projections`` resolves the version itself. + """ + assert report_state._write_report_projections([_finding()], set()) is True + + document = json.loads( + (report_state.get_run_dir() / "findings.sarif").read_text(encoding="utf-8") + ) + driver = document["runs"][0]["tool"]["driver"] + assert driver["name"] == "LyraShield" + assert driver["version"] == engine_version() + assert driver["version"] is not None + + +def test_sarif_write_path_still_omits_version_when_metadata_is_absent( + report_state: ReportState, monkeypatch: pytest.MonkeyPatch +) -> None: + """A frozen build has no metadata; the field is omitted rather than faked.""" + monkeypatch.setattr("lyrashield.version.version", Mock(return_value=None)) + + assert report_state._write_report_projections([_finding()], set()) is True + document = json.loads( + (report_state.get_run_dir() / "findings.sarif").read_text(encoding="utf-8") + ) + assert "version" not in document["runs"][0]["tool"]["driver"] + + +def test_engine_version_falls_back_when_distribution_is_missing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Frozen and uninstalled builds must not raise.""" + + def _raise(name: str) -> str: + raise importlib.metadata.PackageNotFoundError(name) + + monkeypatch.setattr("lyrashield.version.version", _raise) + + assert engine_version() is None + + +def test_engine_version_survives_a_broken_metadata_backend( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Frozen loaders can fail with something other than PackageNotFoundError.""" + + def _raise(_name: str) -> str: + raise ValueError("no metadata in frozen build") + + monkeypatch.setattr("lyrashield.version.version", _raise) + + assert engine_version() is None diff --git a/tests/test_image_pull_deadline.py b/tests/test_image_pull_deadline.py index 4f35c187..a6ccb52b 100644 --- a/tests/test_image_pull_deadline.py +++ b/tests/test_image_pull_deadline.py @@ -16,6 +16,14 @@ from lyrashield.lifecycle.deadline import RunDeadline, RunDeadlineExceededError +# The fake daemon sets its event from a request the spawned worker makes, so the +# assertion that it was reached races the worker's import and connect time. The +# wait is bounded (it can never hang) but generous enough to absorb that startup +# jitter: the daemon only blocks for 12s, well beyond this window, so a healthy +# run still reaches it long before the timeout. +_DAEMON_REACH_TIMEOUT_SECONDS = 2.0 + + def _blocked_docker_worker(sender: Any, _image: str, _expected_digest: str) -> None: sender.send(("pulling", None)) time.sleep(2) @@ -165,7 +173,9 @@ def test_docker_connection_inspect_and_pull_are_deadline_bound( server.shutdown() server.server_close() - assert reached.wait(timeout=0.1), f"fake Docker daemon did not reach {stage}" + assert reached.wait(timeout=_DAEMON_REACH_TIMEOUT_SECONDS), ( + f"fake Docker daemon did not reach {stage} within {_DAEMON_REACH_TIMEOUT_SECONDS}s" + ) assert time.monotonic() - started < 6.5 assert {process.pid for process in multiprocessing.active_children()} <= active_before diff --git a/tests/test_quality_environments.py b/tests/test_quality_environments.py index 29a83fa1..9926f3bb 100644 --- a/tests/test_quality_environments.py +++ b/tests/test_quality_environments.py @@ -83,3 +83,59 @@ def test_npm_lock_meets_the_reviewed_parser_and_address_floors() -> None: if Version(actual[name]) < Version(minimum) } assert not below_floor, f"sandbox npm packages below reviewed security floors: {below_floor}" + + +def _locked_version(name: str) -> str: + lock = tomllib.loads((ROOT / "uv.lock").read_text(encoding="utf-8")) + return next(package["version"] for package in lock["package"] if package["name"] == name) + + +def test_precommit_hooks_use_the_locked_tool_versions() -> None: + """A local hook must not run a different ruff or bandit than CI does. + + Ruff 0.11.13 and 0.15.20 disagree on formatter and lint output, so an + unaligned rev lets a commit pass locally and fail the locked CI gate. + """ + config = yaml.safe_load((ROOT / ".pre-commit-config.yaml").read_text(encoding="utf-8")) + revs = { + repository["repo"]: str(repository["rev"]) + for repository in config["repos"] + if "rev" in repository + } + + assert revs["https://github.com/astral-sh/ruff-pre-commit"] == f"v{_locked_version('ruff')}" + assert revs["https://github.com/PyCQA/bandit"] == _locked_version("bandit") + + +def test_bandit_severity_floor_is_enforced_by_the_invocation() -> None: + """Bandit reads no severity floor from pyproject.toml; the flag must carry it. + + The config previously said ``severity = "medium"``, which bandit accepts and + never applies. The enforced floor is LOW (bandit's default) and every + invocation states it with ``-l``. A medium floor is rejected here because it + would drop B311/B403/B405-B409 while their ruff equivalents (S403, S405-S409) + are preview-only and inactive, so medium would weaken the gate. + """ + config = tomllib.loads((ROOT / "pyproject.toml").read_text(encoding="utf-8")) + bandit_config = config["tool"]["bandit"] + + assert "severity" not in bandit_config + assert "B101" in bandit_config["skips"] + + invocations = { + "Makefile": "uv run bandit -r strix lyrashield_adapter lyrashield -q -c pyproject.toml -l", + "scripts/verify-controlled-derivative.sh": ( + "uv run bandit -c pyproject.toml -r strix lyrashield_adapter lyrashield -q -l" + ), + } + for relative, expected in invocations.items(): + assert expected in (ROOT / relative).read_text(encoding="utf-8") + + hook_config = yaml.safe_load((ROOT / ".pre-commit-config.yaml").read_text(encoding="utf-8")) + bandit_hook = next( + hook + for repository in hook_config["repos"] + for hook in repository["hooks"] + if hook["id"] == "bandit" + ) + assert bandit_hook["args"] == ["-c", "pyproject.toml", "-l"] diff --git a/tests/test_release_build.py b/tests/test_release_build.py index c1fb705f..32cb9673 100644 --- a/tests/test_release_build.py +++ b/tests/test_release_build.py @@ -2,6 +2,7 @@ import subprocess import sys import tarfile +import tomllib from pathlib import Path from zipfile import ZipFile @@ -226,10 +227,49 @@ def test_binary_does_not_request_missing_hidden_imports() -> None: "strix.interface.tui.renderers.registry", "strix.tools.proxy._calls", "strix.tools.python.tool", + "tenacity", ): assert f"'{module}'" not in spec +def test_every_spec_hidden_import_is_a_declared_or_installed_distribution() -> None: + """A frozen build must not request a hidden import nothing provides. + + The spec listed ``tenacity`` while it is absent from pyproject, uv.lock and + the installed environment, so a frozen build would carry a dead entry. + """ + # Pre-existing pydantic optional extra, absent from the lock and the + # installed environment. It predates this change and is not part of the + # reviewed deletion set, so it is recorded rather than removed. Anything + # outside this set fails the test. + known_absent = {"email_validator"} + + spec = (ROOT / "strix.spec").read_text() + block = spec[spec.index("hiddenimports = [") :] + block = block[: block.index("]")] + requested = { + line.strip().strip("',").strip() + for line in block.splitlines() + if line.strip().startswith("'") and line.strip().endswith("',") + } + # Only bare top-level third-party distributions are checked; the spec also + # lists strix/lyrashield modules and stdlib-adjacent names. + first_party_prefixes = ("strix", "lyrashield", "agents", "litellm", "tiktoken", "pygments") + third_party = { + name + for name in requested + if name and "." not in name and not name.startswith(first_party_prefixes) + } + + lock = tomllib.loads((ROOT / "uv.lock").read_text(encoding="utf-8")) + locked = {package["name"].lower().replace("-", "_") for package in lock["package"]} + + missing = { + name for name in third_party if name.lower().replace("-", "_") not in locked + } - known_absent + assert not missing, f"strix.spec hidden imports missing from the lock: {sorted(missing)}" + + def test_build_script_fails_when_binary_smoke_test_fails() -> None: script = (ROOT / "scripts/build.sh").read_text() smoke = (ROOT / "scripts/smoke_release.py").read_text() From 69a0820db16828c916d309fc8225fe7a8cbed3cd Mon Sep 17 00:00:00 2001 From: Claude Code Date: Thu, 8 Oct 2026 04:37:49 +0530 Subject: [PATCH 2/3] docs: add PR body Co-Authored-By: Claude Code --- PR_BODY.md | 294 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 294 insertions(+) create mode 100644 PR_BODY.md diff --git a/PR_BODY.md b/PR_BODY.md new file mode 100644 index 00000000..0fa546f0 --- /dev/null +++ b/PR_BODY.md @@ -0,0 +1,294 @@ +# Engine core fixes: version lookup, tool pins, proven-dead members + +## Summary + +The engine looked up the distribution `strix-agent` for its own version. No +build of this project installs that name: `pyproject.toml:2` declares +`lyrashield-engine`. The three wrong call sites therefore always raised +`PackageNotFoundError`, which silently omitted the SARIF `tool.driver.version` +field, showed `dev` in the interactive TUI header and reported `unknown` to +telemetry. One shared helper now resolves the real distribution name with an +explicit fallback, and the three call sites use it. + +The pre-commit hook revisions are aligned to the versions `uv.lock` and CI +already resolve, so a local hook can no longer pass lint or formatting the +locked gate rejects. The bandit severity setting in `pyproject.toml` was inert, +and the fix states and enforces the floor the gate actually applies instead of +the medium floor the config only appeared to set. + +Seven unreferenced members and files are deleted, each with whole-repo `git +grep` proof. The image-pull deadline test gets a bounded daemon wait and CI +caches the Chromium download. + +## Per-change safety proof + +Every deletion below was checked with `git grep` across the whole repo +(`lyrashield`, `lyrashield_adapter`, `strix`, `tests`, `scripts`, `docs`, +`pyproject.toml`, `strix.spec`, `.github`, `Makefile`, `UPGRADES.md`, +`README.md`, `CONTRIBUTING.md`, all `*.mdx`). + +### 1. `lyrashield/artifacts/usage.py` — `ancillary_cost_total` property + +``` +$ git grep -n -w ancillary_cost_total +lyrashield/artifacts/usage.py:229: def ancillary_cost_total(self) -> float: +``` + +One hit, the definition itself. `total_cost` at `:234` computes the same sum +inline and is unaffected. `_round_cost` and `_ancillary_costs` remain used by +`total_cost`, `to_record` and `reset`, so no supporting symbol was orphaned. + +### 2. `lyrashield/artifacts/state.py` — `set_sandbox_cleanup_status` + +``` +$ git grep -n -w set_sandbox_cleanup_status +lyrashield/artifacts/state.py:630: def set_sandbox_cleanup_status(self, sandbox_removed: bool) -> None: +``` + +One hit, the definition itself. The method's own docstring calls it a +backward-compatible wrapper and no caller exists. The real method +`set_cleanup_outcome` keeps its callers (`interface/cli.py:311`, +`lifecycle/finalize.py:126`, plus tests), and `CLEANUP_REMOVED` and +`CLEANUP_FAILED` remain used inside `set_cleanup_outcome` itself. + +### 3. `lyrashield/lifecycle/agents.py` — `wait_kind_of` + +``` +$ git grep -n -w wait_kind_of +lyrashield/lifecycle/agents.py:285: async def wait_kind_of(self, agent_id: str) -> WaitKind | None: +strix/core/agents.py:214: async def wait_kind_of(self, agent_id: str) -> WaitKind | None: +``` + +The only other hit is the upstream substrate twin, which is a controlled +derivative boundary and was not touched. No `lyrashield/`, test or script +caller exists. The `wait_kinds` dict and `WaitKind` type stay in use by +`park_waiting`, snapshot and restore. + +### 4. `lyrashield/interface/utils.py` — `process_pull_line` shim + +``` +$ git grep -n 'process_pull_line' -- lyrashield tests scripts docs pyproject.toml strix.spec .github Makefile +lyrashield/interface/image_pull.py:169: ... process_pull_line(payload, layers_info, status, last_update) +lyrashield/interface/image_pull.py:320: ... process_pull_line(line, layers_info, status, last_update) +lyrashield/interface/image_pull.py:358: def process_pull_line( +lyrashield/interface/main.py:42: process_pull_line, # noqa: F401 +lyrashield/interface/utils.py:586: def process_pull_line( +lyrashield/interface/utils.py:589: from lyrashield.interface.image_pull import process_pull_line as _process_pull_line +lyrashield/interface/utils.py:591: return _process_pull_line(line, layers_info, status, last_update) +tests/test_image_digest.py:13: process_pull_line, +tests/test_image_digest.py:79: update = process_pull_line(...) +tests/test_main_helper_exports.py:21: ("process_pull_line", "image_pull", "process_pull_line"), +``` + +The only removed hits are the three `utils.py` lines. The test imports come +from `lyrashield.interface.main`, which re-exports the real +`image_pull.process_pull_line` at `main.py:42`. `tests/test_main_helper_exports.py:21` +asserts that `main` re-exports the `image_pull` implementation and still passes. +The other stub in the same file, `update_layer_status`, is a different function +and was left alone. + +### 5. `scripts/docker.sh` + +``` +$ git grep -n -F 'docker.sh' +(no output) +``` + +Zero references anywhere in the repo. The script also builds `strix-sandbox`, +a different image name from the product one. The remaining 11 files in +`scripts/` are referenced by CI, the Makefile, tests or docs. + +### 6. `.trivyignore.yaml` — root `Dockerfile` path + +``` +$ ls Dockerfile +ls: cannot access 'Dockerfile': No such file or directory +``` + +Only `containers/Dockerfile` exists. The `AVD-DS-0002` rule and its +`containers/Dockerfile` path are unchanged, so the reviewed trust-boundary +decision still applies to the real file. + +### 7. `strix.spec` — `tenacity` hidden import + +``` +$ git grep -n -i 'tenacity' -- strix.spec pyproject.toml lyrashield strix +strix.spec:99: # Tenacity retry +strix.spec:100: 'tenacity', +$ grep -n '^name = "tenacity"' uv.lock +(no hit) +$ ls .venv/lib/python3.14/site-packages/ | grep -i tenacity +(not installed) +$ git grep -n 'import tenacity\|from tenacity' +(no importer) +``` + +`tenacity` is absent from `pyproject.toml`, absent from `uv.lock`, not +installed, and imported nowhere. `containers/python-requirements.txt` pins it +for the sandbox image, which is a separate environment from the frozen engine +binary and is untouched. `strix.spec` is at the repo root, outside `strix/**`. + +## Tests added + +All new tests were made to fail locally before their fix and pass after. + +| Test | Failed before with | +| --- | --- | +| `tests/test_engine_version.py::test_sarif_write_path_emits_tool_driver_version` | `KeyError: 'version'` — `_write_report_projections` resolved the wrong distribution, so `sarif.py:262` omitted `driver.version` | +| `tests/test_engine_version.py::test_sarif_write_path_still_omits_version_when_metadata_is_absent` | passed only after the fallback existed; drives the real projection chain with metadata unavailable | +| `tests/test_engine_version.py::test_engine_version_falls_back_when_distribution_is_missing` | `PackageNotFoundError` escaped the old call sites | +| `tests/test_engine_version.py::test_engine_version_survives_a_broken_metadata_backend` | a frozen loader raises something other than `PackageNotFoundError` | +| `tests/test_quality_environments.py::test_precommit_hooks_use_the_locked_tool_versions` | `AssertionError: 'v0.11.13' == 'v0.15.20'` | +| `tests/test_quality_environments.py::test_bandit_severity_floor_is_enforced_by_the_invocation` | `AssertionError: 'severity' in {... 'severity': 'medium'}` | +| `tests/test_release_build.py::test_binary_does_not_request_missing_hidden_imports` | `AssertionError` — `'tenacity'` present in the spec | +| `tests/test_release_build.py::test_every_spec_hidden_import_is_a_declared_or_installed_distribution` | `AssertionError: ... ['tenacity']` | + +The SARIF regression is the one that matters. `tests/test_sarif.py:60` passes +`tool_version` explicitly, so it could never catch a broken lookup. The new test +drives `ReportState._write_report_projections`, the production call chain that +resolves the version itself, and reads the written `findings.sarif`. + +Existing behaviour is unchanged for the four lookups that were already correct +(`interface/arg_parser.py:40`, `lifecycle/runner.py:129`, +`tools/proxy/caido_api.py:678`, `lyrashield_adapter/cli.py:158`). They were not +edited. + +## Bandit: how the intent was verified and why medium was rejected + +The config key was inert. Verified with the installed bandit 1.9.4: + +``` +$ uv run python -c "from bandit.core.config import BanditConfig; c=BanditConfig('pyproject.toml'); print(repr(c.get_option('severity')))" +'medium' + +$ uv run bandit -c pyproject.toml -f json /tmp/bandittest/mix.py | python -c "import json,sys; print(sorted({(r['test_id'], r['issue_severity']) for r in json.load(sys.stdin)['results']}))" +[('B311', 'LOW'), ('B602', 'LOW')] + +$ uv run bandit -c pyproject.toml -ll -f json /tmp/bandittest/mix.py | python -c "import json,sys; print(sorted({(r['test_id'], r['issue_severity']) for r in json.load(sys.stdin)['results']}))" +[] +``` + +Bandit parses the key and never applies it as a filter. Only the `-l/--level` +CLI flag filters. The fixture is a temporary file outside the repo, and the +whole-repo run is unaffected: + +``` +$ uv run bandit -r strix lyrashield_adapter lyrashield -q -c pyproject.toml -f json # default +0 findings +$ uv run bandit -r strix lyrashield_adapter lyrashield -q -c pyproject.toml -l -f json +0 findings +``` + +Enforcing medium would weaken the gate, which the brief forbids. The LOW rules +enabled by the current skip list are `B311`, `B403`, `B405`, `B406`, `B407`, +`B408`, `B409`. Their ruff equivalents `S403` and `S405`-`S409` are +**preview-only** in the locked ruff 0.15.20 and this repo does not enable +preview: + +``` +$ uv run ruff rule S403 | head -4 +# suspicious-pickle-import (S403) +Derived from the **flake8-bandit** linter. +This rule is in preview and is not stable. The `--preview` flag is required for use. + +$ grep -n 'preview' pyproject.toml +(no setting) +``` + +So a medium floor would drop seven rules and leave six of them with no +replacement check at all. The applied fix states the real floor and enforces it: + +- `pyproject.toml`: the inert `severity = "medium"` key is removed, replaced by + a comment recording the verified behaviour and why medium is rejected. +- `Makefile` `security`, `.pre-commit-config.yaml` and + `scripts/verify-controlled-derivative.sh`: `-l` added. + +No severity threshold was lowered, no rule was skipped and no suppression was +added. `-l` is behaviourally identical to bandit's default: no rule in bandit +1.9.4 carries `UNDEFINED` severity, so the default `UNDEFINED` floor and the +`LOW` floor select the same rules. `-l` makes the floor explicit rather than +changing it. + +## Gates run + +| Command | Result | +| --- | --- | +| `uv sync --frozen` | ok | +| `uv run ruff check .` | All checks passed | +| `uv run ruff format --check .` | 448 files already formatted | +| `uv run mypy strix lyrashield_adapter lyrashield` | Success: no issues found in 251 source files | +| `uv run bandit -r strix lyrashield_adapter lyrashield -q -c pyproject.toml -l` | exit 0 | +| `uv run pytest tests/` (all 152 test modules plus `tests/tui` and `tests/upstream`, run in chunks) | 3088 passed, 5 skipped, 2 failed — both environmental, see below | +| `scripts/verify-controlled-derivative.sh` footprint and digest steps | `4 files changed, 22 insertions(+), 201 deletions(-)`; patch digest `3629e8f382fdd8eccf78553b102a25e99c73454c` matches | +| `python scripts/verify-customer-branding.py` | Customer branding gate passed | +| `uv run pre-commit validate-config .pre-commit-config.yaml` | ok | +| `python3 scripts/report_twin_drift.py` | runs clean | + +Two failures, both pre-existing and environmental, neither touched by this +change: + +1. `tests/test_quality_environments.py::test_precommit_and_make_run_the_same_type_check` + — `make` is not installed in this sandbox. +2. `tests/test_local_sources.py::test_clone_repository_checks_out_a_full_commit_sha_detached` + — this sandbox's git cannot resolve the detached test SHA. + +The brief lists both as environmental. The three historical pytest failures +remain unreproduced and were not "fixed". + +## PROTECTED + +- **Pins**: no pin bumped. `ENGINE_REVISION`, the engine + `.lyrashield-worker-pin` and every dependency cap (openai, litellm, + openai-agents, cryptography) are untouched. `uv.lock` is unchanged. +- **strix footprint**: `git diff --shortstat "$(cat .lyrashield-upstream-base)" -- strix/` + returns `4 files changed, 22 insertions(+), 201 deletions(-)`, unchanged. The + reviewed patch digest `3629e8f382fdd8eccf78553b102a25e99c73454c` matches. No + file under `strix/**` was edited. `strix.spec` is at the repo root, outside + `strix/**`. +- **Controlled-derivative boundary**: AST-identical upstream twins were left + alone. `strix/core/agents.py:214 wait_kind_of` is the upstream twin of the + member removed on the product side; the upstream file was not touched. Nothing + from the E2/E3 candidate lists beyond the seven proven-dead items was deleted. +- **Ruff**: 0.15.20 is what the lock already had. No upgrade, no new + suppression, no per-file ignore added. `ruff 0.16.6` (engine PR #139) stays + open and held. +- **Customer branding gate**: `scripts/customer-branding-allowlist.json` loses + four entries that named the removed `strix-agent` lookups. Leaving them would + have exempted the exact lines this change removes. The gate passes, and the + remaining inherited-identifier exemptions are unchanged. +- **Viewer, Windows, image and controlled-derivative coverage**: the CI change + adds a cache step only. Every step name, `if` condition and required check is + unchanged, and the cache key falls back to a per-OS restore key so a miss + still downloads. + +## Not done + +- **No broad flaky-test rewrite.** The image-pull test keeps its intent; only + the daemon-reach wait is bounded and documented. +- **No enforcement of a medium bandit floor.** Verified to weaken the gate, so + it was rejected rather than applied. Details above. +- **`email_validator` in `strix.spec`** is also absent from `uv.lock` and the + installed environment, like `tenacity`. It is a pre-existing pydantic optional + extra, outside the reviewed deletion set, so it was recorded in the new test's + `known_absent` set rather than removed. Flagging it for a separate decision. +- **A real PyInstaller frozen build could not run** in this sandbox: PyInstaller + requires `objdump` from `binutils`, which is not installable here. The + available substitute was run instead: parsing `strix.spec` and resolving all + 115 `hiddenimports` entries with `importlib.util.find_spec` under + `uv run --frozen --extra viewer` leaves only `email_validator` unresolved and + no `tenacity`. The frozen-build check therefore remains an operator step. +- **The full `verify-controlled-derivative.sh` did not run to completion** as one + command: its pytest stage exceeds the 120-second sandbox command cap. Its + footprint and digest invariants were run directly and pass, and its full pytest + stage was run in chunks with the same suite and `-W error::pydantic.PydanticDeprecatedSince211` + semantics; the bandit, ruff, mypy and format stages it wraps were each run + individually and pass. +- **Local TUI members** (`_encrypt`, `profile_for`, `update_run_status`, + `iter_runs`, `KEYCHAIN_CHATGPT_TOKEN`) were not deleted. They are in the E2/E3 + candidate list but not in this workstream's approved deletion set. + +## Rollback + +Code only, forward-only for any schema. Revert this PR. + +Generated with Claude Code From c6a791fbf238825779edf4eeb7de99feeb57ee90 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Thu, 8 Oct 2026 13:24:17 +0530 Subject: [PATCH 3/3] chore: keep the PR description in the pull request, not the tree PR_BODY.md was committed by mistake. The pull request body is the only place for it. No code changes. Co-Authored-By: Claude Code --- PR_BODY.md | 294 ----------------------------------------------------- 1 file changed, 294 deletions(-) delete mode 100644 PR_BODY.md diff --git a/PR_BODY.md b/PR_BODY.md deleted file mode 100644 index 0fa546f0..00000000 --- a/PR_BODY.md +++ /dev/null @@ -1,294 +0,0 @@ -# Engine core fixes: version lookup, tool pins, proven-dead members - -## Summary - -The engine looked up the distribution `strix-agent` for its own version. No -build of this project installs that name: `pyproject.toml:2` declares -`lyrashield-engine`. The three wrong call sites therefore always raised -`PackageNotFoundError`, which silently omitted the SARIF `tool.driver.version` -field, showed `dev` in the interactive TUI header and reported `unknown` to -telemetry. One shared helper now resolves the real distribution name with an -explicit fallback, and the three call sites use it. - -The pre-commit hook revisions are aligned to the versions `uv.lock` and CI -already resolve, so a local hook can no longer pass lint or formatting the -locked gate rejects. The bandit severity setting in `pyproject.toml` was inert, -and the fix states and enforces the floor the gate actually applies instead of -the medium floor the config only appeared to set. - -Seven unreferenced members and files are deleted, each with whole-repo `git -grep` proof. The image-pull deadline test gets a bounded daemon wait and CI -caches the Chromium download. - -## Per-change safety proof - -Every deletion below was checked with `git grep` across the whole repo -(`lyrashield`, `lyrashield_adapter`, `strix`, `tests`, `scripts`, `docs`, -`pyproject.toml`, `strix.spec`, `.github`, `Makefile`, `UPGRADES.md`, -`README.md`, `CONTRIBUTING.md`, all `*.mdx`). - -### 1. `lyrashield/artifacts/usage.py` — `ancillary_cost_total` property - -``` -$ git grep -n -w ancillary_cost_total -lyrashield/artifacts/usage.py:229: def ancillary_cost_total(self) -> float: -``` - -One hit, the definition itself. `total_cost` at `:234` computes the same sum -inline and is unaffected. `_round_cost` and `_ancillary_costs` remain used by -`total_cost`, `to_record` and `reset`, so no supporting symbol was orphaned. - -### 2. `lyrashield/artifacts/state.py` — `set_sandbox_cleanup_status` - -``` -$ git grep -n -w set_sandbox_cleanup_status -lyrashield/artifacts/state.py:630: def set_sandbox_cleanup_status(self, sandbox_removed: bool) -> None: -``` - -One hit, the definition itself. The method's own docstring calls it a -backward-compatible wrapper and no caller exists. The real method -`set_cleanup_outcome` keeps its callers (`interface/cli.py:311`, -`lifecycle/finalize.py:126`, plus tests), and `CLEANUP_REMOVED` and -`CLEANUP_FAILED` remain used inside `set_cleanup_outcome` itself. - -### 3. `lyrashield/lifecycle/agents.py` — `wait_kind_of` - -``` -$ git grep -n -w wait_kind_of -lyrashield/lifecycle/agents.py:285: async def wait_kind_of(self, agent_id: str) -> WaitKind | None: -strix/core/agents.py:214: async def wait_kind_of(self, agent_id: str) -> WaitKind | None: -``` - -The only other hit is the upstream substrate twin, which is a controlled -derivative boundary and was not touched. No `lyrashield/`, test or script -caller exists. The `wait_kinds` dict and `WaitKind` type stay in use by -`park_waiting`, snapshot and restore. - -### 4. `lyrashield/interface/utils.py` — `process_pull_line` shim - -``` -$ git grep -n 'process_pull_line' -- lyrashield tests scripts docs pyproject.toml strix.spec .github Makefile -lyrashield/interface/image_pull.py:169: ... process_pull_line(payload, layers_info, status, last_update) -lyrashield/interface/image_pull.py:320: ... process_pull_line(line, layers_info, status, last_update) -lyrashield/interface/image_pull.py:358: def process_pull_line( -lyrashield/interface/main.py:42: process_pull_line, # noqa: F401 -lyrashield/interface/utils.py:586: def process_pull_line( -lyrashield/interface/utils.py:589: from lyrashield.interface.image_pull import process_pull_line as _process_pull_line -lyrashield/interface/utils.py:591: return _process_pull_line(line, layers_info, status, last_update) -tests/test_image_digest.py:13: process_pull_line, -tests/test_image_digest.py:79: update = process_pull_line(...) -tests/test_main_helper_exports.py:21: ("process_pull_line", "image_pull", "process_pull_line"), -``` - -The only removed hits are the three `utils.py` lines. The test imports come -from `lyrashield.interface.main`, which re-exports the real -`image_pull.process_pull_line` at `main.py:42`. `tests/test_main_helper_exports.py:21` -asserts that `main` re-exports the `image_pull` implementation and still passes. -The other stub in the same file, `update_layer_status`, is a different function -and was left alone. - -### 5. `scripts/docker.sh` - -``` -$ git grep -n -F 'docker.sh' -(no output) -``` - -Zero references anywhere in the repo. The script also builds `strix-sandbox`, -a different image name from the product one. The remaining 11 files in -`scripts/` are referenced by CI, the Makefile, tests or docs. - -### 6. `.trivyignore.yaml` — root `Dockerfile` path - -``` -$ ls Dockerfile -ls: cannot access 'Dockerfile': No such file or directory -``` - -Only `containers/Dockerfile` exists. The `AVD-DS-0002` rule and its -`containers/Dockerfile` path are unchanged, so the reviewed trust-boundary -decision still applies to the real file. - -### 7. `strix.spec` — `tenacity` hidden import - -``` -$ git grep -n -i 'tenacity' -- strix.spec pyproject.toml lyrashield strix -strix.spec:99: # Tenacity retry -strix.spec:100: 'tenacity', -$ grep -n '^name = "tenacity"' uv.lock -(no hit) -$ ls .venv/lib/python3.14/site-packages/ | grep -i tenacity -(not installed) -$ git grep -n 'import tenacity\|from tenacity' -(no importer) -``` - -`tenacity` is absent from `pyproject.toml`, absent from `uv.lock`, not -installed, and imported nowhere. `containers/python-requirements.txt` pins it -for the sandbox image, which is a separate environment from the frozen engine -binary and is untouched. `strix.spec` is at the repo root, outside `strix/**`. - -## Tests added - -All new tests were made to fail locally before their fix and pass after. - -| Test | Failed before with | -| --- | --- | -| `tests/test_engine_version.py::test_sarif_write_path_emits_tool_driver_version` | `KeyError: 'version'` — `_write_report_projections` resolved the wrong distribution, so `sarif.py:262` omitted `driver.version` | -| `tests/test_engine_version.py::test_sarif_write_path_still_omits_version_when_metadata_is_absent` | passed only after the fallback existed; drives the real projection chain with metadata unavailable | -| `tests/test_engine_version.py::test_engine_version_falls_back_when_distribution_is_missing` | `PackageNotFoundError` escaped the old call sites | -| `tests/test_engine_version.py::test_engine_version_survives_a_broken_metadata_backend` | a frozen loader raises something other than `PackageNotFoundError` | -| `tests/test_quality_environments.py::test_precommit_hooks_use_the_locked_tool_versions` | `AssertionError: 'v0.11.13' == 'v0.15.20'` | -| `tests/test_quality_environments.py::test_bandit_severity_floor_is_enforced_by_the_invocation` | `AssertionError: 'severity' in {... 'severity': 'medium'}` | -| `tests/test_release_build.py::test_binary_does_not_request_missing_hidden_imports` | `AssertionError` — `'tenacity'` present in the spec | -| `tests/test_release_build.py::test_every_spec_hidden_import_is_a_declared_or_installed_distribution` | `AssertionError: ... ['tenacity']` | - -The SARIF regression is the one that matters. `tests/test_sarif.py:60` passes -`tool_version` explicitly, so it could never catch a broken lookup. The new test -drives `ReportState._write_report_projections`, the production call chain that -resolves the version itself, and reads the written `findings.sarif`. - -Existing behaviour is unchanged for the four lookups that were already correct -(`interface/arg_parser.py:40`, `lifecycle/runner.py:129`, -`tools/proxy/caido_api.py:678`, `lyrashield_adapter/cli.py:158`). They were not -edited. - -## Bandit: how the intent was verified and why medium was rejected - -The config key was inert. Verified with the installed bandit 1.9.4: - -``` -$ uv run python -c "from bandit.core.config import BanditConfig; c=BanditConfig('pyproject.toml'); print(repr(c.get_option('severity')))" -'medium' - -$ uv run bandit -c pyproject.toml -f json /tmp/bandittest/mix.py | python -c "import json,sys; print(sorted({(r['test_id'], r['issue_severity']) for r in json.load(sys.stdin)['results']}))" -[('B311', 'LOW'), ('B602', 'LOW')] - -$ uv run bandit -c pyproject.toml -ll -f json /tmp/bandittest/mix.py | python -c "import json,sys; print(sorted({(r['test_id'], r['issue_severity']) for r in json.load(sys.stdin)['results']}))" -[] -``` - -Bandit parses the key and never applies it as a filter. Only the `-l/--level` -CLI flag filters. The fixture is a temporary file outside the repo, and the -whole-repo run is unaffected: - -``` -$ uv run bandit -r strix lyrashield_adapter lyrashield -q -c pyproject.toml -f json # default -0 findings -$ uv run bandit -r strix lyrashield_adapter lyrashield -q -c pyproject.toml -l -f json -0 findings -``` - -Enforcing medium would weaken the gate, which the brief forbids. The LOW rules -enabled by the current skip list are `B311`, `B403`, `B405`, `B406`, `B407`, -`B408`, `B409`. Their ruff equivalents `S403` and `S405`-`S409` are -**preview-only** in the locked ruff 0.15.20 and this repo does not enable -preview: - -``` -$ uv run ruff rule S403 | head -4 -# suspicious-pickle-import (S403) -Derived from the **flake8-bandit** linter. -This rule is in preview and is not stable. The `--preview` flag is required for use. - -$ grep -n 'preview' pyproject.toml -(no setting) -``` - -So a medium floor would drop seven rules and leave six of them with no -replacement check at all. The applied fix states the real floor and enforces it: - -- `pyproject.toml`: the inert `severity = "medium"` key is removed, replaced by - a comment recording the verified behaviour and why medium is rejected. -- `Makefile` `security`, `.pre-commit-config.yaml` and - `scripts/verify-controlled-derivative.sh`: `-l` added. - -No severity threshold was lowered, no rule was skipped and no suppression was -added. `-l` is behaviourally identical to bandit's default: no rule in bandit -1.9.4 carries `UNDEFINED` severity, so the default `UNDEFINED` floor and the -`LOW` floor select the same rules. `-l` makes the floor explicit rather than -changing it. - -## Gates run - -| Command | Result | -| --- | --- | -| `uv sync --frozen` | ok | -| `uv run ruff check .` | All checks passed | -| `uv run ruff format --check .` | 448 files already formatted | -| `uv run mypy strix lyrashield_adapter lyrashield` | Success: no issues found in 251 source files | -| `uv run bandit -r strix lyrashield_adapter lyrashield -q -c pyproject.toml -l` | exit 0 | -| `uv run pytest tests/` (all 152 test modules plus `tests/tui` and `tests/upstream`, run in chunks) | 3088 passed, 5 skipped, 2 failed — both environmental, see below | -| `scripts/verify-controlled-derivative.sh` footprint and digest steps | `4 files changed, 22 insertions(+), 201 deletions(-)`; patch digest `3629e8f382fdd8eccf78553b102a25e99c73454c` matches | -| `python scripts/verify-customer-branding.py` | Customer branding gate passed | -| `uv run pre-commit validate-config .pre-commit-config.yaml` | ok | -| `python3 scripts/report_twin_drift.py` | runs clean | - -Two failures, both pre-existing and environmental, neither touched by this -change: - -1. `tests/test_quality_environments.py::test_precommit_and_make_run_the_same_type_check` - — `make` is not installed in this sandbox. -2. `tests/test_local_sources.py::test_clone_repository_checks_out_a_full_commit_sha_detached` - — this sandbox's git cannot resolve the detached test SHA. - -The brief lists both as environmental. The three historical pytest failures -remain unreproduced and were not "fixed". - -## PROTECTED - -- **Pins**: no pin bumped. `ENGINE_REVISION`, the engine - `.lyrashield-worker-pin` and every dependency cap (openai, litellm, - openai-agents, cryptography) are untouched. `uv.lock` is unchanged. -- **strix footprint**: `git diff --shortstat "$(cat .lyrashield-upstream-base)" -- strix/` - returns `4 files changed, 22 insertions(+), 201 deletions(-)`, unchanged. The - reviewed patch digest `3629e8f382fdd8eccf78553b102a25e99c73454c` matches. No - file under `strix/**` was edited. `strix.spec` is at the repo root, outside - `strix/**`. -- **Controlled-derivative boundary**: AST-identical upstream twins were left - alone. `strix/core/agents.py:214 wait_kind_of` is the upstream twin of the - member removed on the product side; the upstream file was not touched. Nothing - from the E2/E3 candidate lists beyond the seven proven-dead items was deleted. -- **Ruff**: 0.15.20 is what the lock already had. No upgrade, no new - suppression, no per-file ignore added. `ruff 0.16.6` (engine PR #139) stays - open and held. -- **Customer branding gate**: `scripts/customer-branding-allowlist.json` loses - four entries that named the removed `strix-agent` lookups. Leaving them would - have exempted the exact lines this change removes. The gate passes, and the - remaining inherited-identifier exemptions are unchanged. -- **Viewer, Windows, image and controlled-derivative coverage**: the CI change - adds a cache step only. Every step name, `if` condition and required check is - unchanged, and the cache key falls back to a per-OS restore key so a miss - still downloads. - -## Not done - -- **No broad flaky-test rewrite.** The image-pull test keeps its intent; only - the daemon-reach wait is bounded and documented. -- **No enforcement of a medium bandit floor.** Verified to weaken the gate, so - it was rejected rather than applied. Details above. -- **`email_validator` in `strix.spec`** is also absent from `uv.lock` and the - installed environment, like `tenacity`. It is a pre-existing pydantic optional - extra, outside the reviewed deletion set, so it was recorded in the new test's - `known_absent` set rather than removed. Flagging it for a separate decision. -- **A real PyInstaller frozen build could not run** in this sandbox: PyInstaller - requires `objdump` from `binutils`, which is not installable here. The - available substitute was run instead: parsing `strix.spec` and resolving all - 115 `hiddenimports` entries with `importlib.util.find_spec` under - `uv run --frozen --extra viewer` leaves only `email_validator` unresolved and - no `tenacity`. The frozen-build check therefore remains an operator step. -- **The full `verify-controlled-derivative.sh` did not run to completion** as one - command: its pytest stage exceeds the 120-second sandbox command cap. Its - footprint and digest invariants were run directly and pass, and its full pytest - stage was run in chunks with the same suite and `-W error::pydantic.PydanticDeprecatedSince211` - semantics; the bandit, ruff, mypy and format stages it wraps were each run - individually and pass. -- **Local TUI members** (`_encrypt`, `profile_for`, `update_run_status`, - `iter_runs`, `KEYCHAIN_CHATGPT_TOKEN`) were not deleted. They are in the E2/E3 - candidate list but not in this workstream's approved deletion set. - -## Rollback - -Code only, forward-only for any schema. Revert this PR. - -Generated with Claude Code