From d40ea8407e461eae4f0c43eedba7a9e4b5b9ffd2 Mon Sep 17 00:00:00 2001 From: rajatnagda45 Date: Sun, 23 Aug 2026 12:56:49 +0530 Subject: [PATCH] fix(binaries): fail closed on HEADROOM_BINARIES_ALLOW_UNVERIFIED=0/false MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sha256-verification bypass was gated by a bare `if os.environ.get("HEADROOM_BINARIES_ALLOW_UNVERIFIED")` presence check in both `_verify_sha256` and `verify_download_bytes`. Any non-empty value — including `0`, `false`, `no`, `off` — is truthy, so an operator who set the variable to `0` to *keep* verification on instead silently disabled sha256 verification of fetched binaries (difft, scc, and the rtk/lean-ctx/ codebase-memory installers). This is a supply-chain integrity control, so it must fail closed. Parse the value as an affirmative flag (`1`/`true`/`yes`/`on`) via a shared helper; every other value — and an unset or empty variable — now enforces verification. Matches the truthy-set parsing already used elsewhere in the codebase (onnx_runtime, cold_prefix, ttl_observations, qdrant_env). --- headroom/binaries.py | 17 ++++++++++++++-- tests/test_binaries.py | 44 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/headroom/binaries.py b/headroom/binaries.py index 200257871..6ea41f01b 100644 --- a/headroom/binaries.py +++ b/headroom/binaries.py @@ -42,6 +42,19 @@ from headroom._subprocess import run logger = logging.getLogger(__name__) +# Affirmative values for the sha256-verification bypass. This is a security +# control (supply-chain integrity), so it must fail CLOSED: only an explicit +# affirmative disables verification. A bare presence check treated +# `HEADROOM_BINARIES_ALLOW_UNVERIFIED=0` / `=false` — the natural way to say +# "keep verifying" — as truthy and silently skipped verification. +_TRUTHY = frozenset({"1", "true", "yes", "on"}) + + +def _allow_unverified_binaries() -> bool: + """Whether sha256 verification of fetched binaries is bypassed.""" + return os.environ.get("HEADROOM_BINARIES_ALLOW_UNVERIFIED", "").strip().lower() in _TRUTHY + + __all__ = [ "BinaryError", "BinaryFetchError", @@ -311,7 +324,7 @@ def _sha256_file(path: Path) -> str: def _verify_sha256(path: Path, expected: str | None) -> None: - if os.environ.get("HEADROOM_BINARIES_ALLOW_UNVERIFIED"): + if _allow_unverified_binaries(): logger.warning( "skipping sha256 verification for %s (HEADROOM_BINARIES_ALLOW_UNVERIFIED=1)", path.name, @@ -347,7 +360,7 @@ def verify_download_bytes(data: bytes, *, url: str, name: str) -> None: the bytes against the tools.json pin for ``url`` and refuses an unpinned URL unless HEADROOM_BINARIES_ALLOW_UNVERIFIED=1. """ - if os.environ.get("HEADROOM_BINARIES_ALLOW_UNVERIFIED"): + if _allow_unverified_binaries(): logger.warning( "skipping sha256 verification for %s (HEADROOM_BINARIES_ALLOW_UNVERIFIED=1)", name ) diff --git a/tests/test_binaries.py b/tests/test_binaries.py index db7d48992..969530290 100644 --- a/tests/test_binaries.py +++ b/tests/test_binaries.py @@ -382,3 +382,47 @@ def test_status_reports_every_registered_tool(monkeypatch): assert {"difft", "scc", "ast-grep"} <= names for r in rows: assert r["state"] in ("on-path", "cached", "missing", "unsupported-platform") + + +# -------- HEADROOM_BINARIES_ALLOW_UNVERIFIED (security: fail closed) ------- # + + +def _write(tmp_path, data: bytes): + p = tmp_path / "asset.bin" + p.write_bytes(data) + return p + + +@pytest.mark.parametrize("value", ["0", "false", "no", "off", "", " "]) +def test_allow_unverified_non_affirmative_still_verifies(monkeypatch, tmp_path, value): + # A security bypass must fail CLOSED: only an explicit affirmative disables + # verification. Setting the var to "0"/"false" (the natural way to say "keep + # verifying") previously passed a bare presence check and silently skipped + # sha256 verification of the downloaded binary. + monkeypatch.setenv("HEADROOM_BINARIES_ALLOW_UNVERIFIED", value) + path = _write(tmp_path, b"payload") + with pytest.raises(binaries.Sha256Mismatch): + binaries._verify_sha256(path, "deadbeef" * 8) # wrong pin + + +def test_allow_unverified_unset_still_verifies(monkeypatch, tmp_path): + monkeypatch.delenv("HEADROOM_BINARIES_ALLOW_UNVERIFIED", raising=False) + path = _write(tmp_path, b"payload") + with pytest.raises(binaries.Sha256Mismatch): + binaries._verify_sha256(path, "deadbeef" * 8) + + +@pytest.mark.parametrize("value", ["1", "true", "TRUE", "yes", "on"]) +def test_allow_unverified_affirmative_skips_verification(monkeypatch, tmp_path, value): + monkeypatch.setenv("HEADROOM_BINARIES_ALLOW_UNVERIFIED", value) + path = _write(tmp_path, b"payload") + # Wrong pin, but an explicit affirmative bypass returns without raising. + binaries._verify_sha256(path, "deadbeef" * 8) + + +def test_verify_download_bytes_zero_still_verifies(monkeypatch): + # Same fail-closed contract on the in-memory installer path. + monkeypatch.setenv("HEADROOM_BINARIES_ALLOW_UNVERIFIED", "0") + monkeypatch.setattr(binaries, "sha256_for_url", lambda _url: "deadbeef" * 8) + with pytest.raises(binaries.Sha256Mismatch): + binaries.verify_download_bytes(b"payload", url="https://x/y", name="y")