security: fail-closed binary checksum verification + version-tag validation

cloakbrowser/config.py

@@ -5,10 +5,21 @@
 import os
 import platform
 import random
+import re
 from pathlib import Path
 
 from ._version import __version__
 
+# A Chromium version tag is a plain dotted-numeric string (e.g. "146.0.7680.177.5").
+# Validate any externally sourced version (GitHub release tags, cache markers)
+# before it flows into cache paths or download URLs.
+_SAFE_VERSION_RE = re.compile(r"^[0-9]+(\.[0-9]+){0,9}$")
+
+
+def is_safe_version_tag(version: str) -> bool:
+    """Return True if *version* is a safe dotted-numeric version string."""
+    return isinstance(version, str) and bool(_SAFE_VERSION_RE.match(version))
+
 # ---------------------------------------------------------------------------
 # Chromium version shipped with this release.
 # Different platforms may ship different versions during transition periods.
@@ -170,7 +181,7 @@ def get_effective_version() -> str:
         if marker.exists():
             try:
                 version = marker.read_text().strip()
-                if version and _version_newer(version, base):
+                if version and is_safe_version_tag(version) and _version_newer(version, base):
                     binary = get_binary_path(version)
                     if binary.exists():
                         return version

cloakbrowser/download.py

@@ -29,6 +29,7 @@
     GITHUB_DOWNLOAD_BASE_URL,
     _version_newer,
     check_platform_available,
+    is_safe_version_tag,
     get_archive_ext,
     get_archive_name,
     get_binary_dir,
@@ -174,18 +175,35 @@ def _download_and_extract(version: str | None = None) -> None:
 
 
 def _verify_download_checksum(file_path: Path, version: str | None = None) -> None:
-    """Fetch SHA256SUMS and verify the downloaded file. Warn if unavailable, fail on mismatch."""
+    """Fetch SHA256SUMS and verify the downloaded file.
+
+    Fails closed: if checksums cannot be fetched, or contain no entry for this
+    platform's archive, the binary is treated as unverified and the download is
+    rejected. Set ``CLOAKBROWSER_SKIP_CHECKSUM=true`` to bypass at your own risk
+    (e.g. a custom mirror that does not publish SHA256SUMS). The official
+    releases publish SHA256SUMS for every version, so this only triggers when a
+    mirror is misconfigured or the file was suppressed/tampered with in transit.
+    """
     checksums = _fetch_checksums(version)
     tarball_name = get_archive_name()
 
     if checksums is None:
-        logger.warning("SHA256SUMS not available for this release — skipping checksum verification")
-        return
+        raise RuntimeError(
+            f"Could not fetch SHA256SUMS for this release, so the downloaded binary "
+            f"({tarball_name}) cannot be integrity-verified. Refusing to use an "
+            f"unverified binary. If you are using a custom CLOAKBROWSER_DOWNLOAD_URL, "
+            f"publish a SHA256SUMS file alongside the archive, or set "
+            f"CLOAKBROWSER_SKIP_CHECKSUM=true to bypass this check at your own risk."
+        )
 
     expected = checksums.get(tarball_name)
     if expected is None:
-        logger.warning("SHA256SUMS found but no entry for %s — skipping verification", tarball_name)
-        return
+        raise RuntimeError(
+            f"SHA256SUMS was fetched but contains no entry for {tarball_name}, so the "
+            f"downloaded binary cannot be integrity-verified. Refusing to use an "
+            f"unverified binary. Set CLOAKBROWSER_SKIP_CHECKSUM=true to bypass at your "
+            f"own risk."
+        )
 
     _verify_checksum(file_path, expected)
 
@@ -204,14 +222,36 @@ def _fetch_checksums(version: str | None = None) -> dict[str, str] | None:
         try:
             resp = httpx.get(url, follow_redirects=True, timeout=10.0)
             resp.raise_for_status()
-            return _parse_checksums(resp.text)
+            parsed = _parse_checksums(resp.text)
+            # A redirect to an HTML error/login page (or any non-SHA256SUMS body)
+            # parses to an empty mapping. Treat that as "not found here" and try
+            # the next mirror, so a misbehaving primary can't shadow a good
+            # fallback (and can't satisfy the fail-closed check with junk).
+            if parsed:
+                return parsed
         except Exception:
             continue
     return None
 
 
+def _is_sha256_hex(value: str) -> bool:
+    """True if *value* is exactly a 64-character hex SHA-256 digest."""
+    if len(value) != 64:
+        return False
+    try:
+        int(value, 16)
+        return True
+    except ValueError:
+        return False
+
+
 def _parse_checksums(text: str) -> dict[str, str]:
-    """Parse SHA256SUMS format: 'hash  filename' per line."""
+    """Parse SHA256SUMS format: 'hash  filename' per line.
+
+    Only accepts lines whose first token is a valid 64-char hex SHA-256 digest,
+    so an HTML error page or other non-checksum body parses to an empty mapping
+    instead of producing bogus entries.
+    """
     result = {}
     for line in text.strip().splitlines():
         line = line.strip()
@@ -220,6 +260,8 @@ def _parse_checksums(text: str) -> dict[str, str]:
         parts = line.split(None, 1)
         if len(parts) == 2:
             hash_val, filename = parts
+            if not _is_sha256_hex(hash_val):
+                continue
             filename = filename.lstrip("*")
             result[filename] = hash_val.lower()
     return result
@@ -478,9 +520,17 @@ def _get_latest_chromium_version() -> str | None:
         for release in resp.json():
             tag = release.get("tag_name", "")
             if tag.startswith("chromium-v") and not release.get("draft"):
+                version = tag.removeprefix("chromium-v")
+                # Defense in depth: the version flows into cache paths and
+                # download URLs. Reject anything that isn't a plain dotted-numeric
+                # version so a malformed/hostile release tag can't inject path or
+                # URL components.
+                if not is_safe_version_tag(version):
+                    logger.debug("Skipping release with unsafe version tag: %r", tag)
+                    continue
                 asset_names = {a["name"] for a in release.get("assets", [])}
                 if platform_tarball in asset_names:
-                    return tag.removeprefix("chromium-v")
+                    return version
         return None
     except Exception:
         logger.debug("Auto-update check failed", exc_info=True)

js/src/config.ts

@@ -154,7 +154,7 @@ export function getEffectiveVersion(): string {
     try {
       if (fs.existsSync(marker)) {
         const version = fs.readFileSync(marker, "utf-8").trim();
-        if (version && versionNewer(version, base)) {
+        if (version && isSafeVersionTag(version) && versionNewer(version, base)) {
           const binary = getBinaryPath(version);
           if (fs.existsSync(binary)) {
             return version;
@@ -168,6 +168,15 @@ export function getEffectiveVersion(): string {
   return base;
 }
 
+// A Chromium version tag is a plain dotted-numeric string (e.g. "146.0.7680.177.5").
+// Validate any externally sourced version (GitHub release tags, cache markers)
+// before it flows into cache paths, download URLs, or shell/extraction commands.
+const SAFE_VERSION_RE = /^[0-9]+(\.[0-9]+){0,9}$/;
+
+export function isSafeVersionTag(version: string): boolean {
+  return typeof version === "string" && SAFE_VERSION_RE.test(version);
+}
+
 export function parseVersion(v: string): number[] {
   return v.split(".").map(Number);
 }

js/src/download.ts

@@ -30,6 +30,7 @@ import {
   getFallbackDownloadUrl,
   getLocalBinaryOverride,
   getPlatformTag,
+  isSafeVersionTag,
   versionNewer,
 } from "./config.js";
 
@@ -211,18 +212,29 @@ async function downloadAndExtract(version?: string): Promise<void> {
 }
 
 async function verifyDownloadChecksum(filePath: string, version?: string): Promise<void> {
+  // Fails closed: a missing SHA256SUMS, or one with no entry for this platform's
+  // archive, means the binary is unverified and the download is rejected. Set
+  // CLOAKBROWSER_SKIP_CHECKSUM=true to bypass (handled by the caller).
   const checksums = await fetchChecksums(version);
   const tarballName = getArchiveName();
 
   if (!checksums) {
-    console.warn("[cloakbrowser] SHA256SUMS not available for this release — skipping checksum verification");
-    return;
+    throw new Error(
+      `Could not fetch SHA256SUMS for this release, so the downloaded binary ` +
+      `(${tarballName}) cannot be integrity-verified. Refusing to use an unverified ` +
+      `binary. If you are using a custom CLOAKBROWSER_DOWNLOAD_URL, publish a ` +
+      `SHA256SUMS file alongside the archive, or set CLOAKBROWSER_SKIP_CHECKSUM=true ` +
+      `to bypass this check at your own risk.`
+    );
   }
 
   const expected = checksums.get(tarballName);
   if (!expected) {
-    console.warn(`[cloakbrowser] SHA256SUMS found but no entry for ${tarballName} — skipping verification`);
-    return;
+    throw new Error(
+      `SHA256SUMS was fetched but contains no entry for ${tarballName}, so the ` +
+      `downloaded binary cannot be integrity-verified. Refusing to use an unverified ` +
+      `binary. Set CLOAKBROWSER_SKIP_CHECKSUM=true to bypass at your own risk.`
+    );
   }
 
   await verifyChecksum(filePath, expected);
@@ -246,7 +258,12 @@ export async function fetchChecksums(version?: string): Promise<Map<string, stri
         signal: AbortSignal.timeout(10_000),
       });
       if (!resp.ok) continue;
-      return parseChecksums(await resp.text());
+      const parsed = parseChecksums(await resp.text());
+      // A redirect to an HTML error/login page (or any non-SHA256SUMS body)
+      // parses to an empty map. Treat that as "not found here" and try the next
+      // mirror, so a misbehaving primary can't shadow a good fallback (and can't
+      // satisfy the fail-closed check with junk).
+      if (parsed.size > 0) return parsed;
     } catch {
       continue;
     }
@@ -424,12 +441,19 @@ async function extractZip(archivePath: string, destDir: string): Promise<void> {
 
   if (process.platform === "win32") {
     // PowerShell 5.1's Expand-Archive uses .NET FileStream which can conflict
-    // with recently-closed Node.js file handles. Use ZipFile API directly.
+    // with recently-closed Node.js file handles. Use the ZipFile API directly.
+    // Pass the paths via environment variables rather than interpolating them
+    // into the -Command string, so a path containing a single quote (or other
+    // PowerShell metacharacter) cannot break out of the literal and execute as
+    // code. The paths are read inside PowerShell as $env: values (plain data).
     execFileSync("powershell", [
       "-NoProfile", "-Command",
       `Add-Type -AssemblyName System.IO.Compression.FileSystem; ` +
-      `[System.IO.Compression.ZipFile]::ExtractToDirectory('${archivePath}', '${destDir}')`,
-    ], { timeout: 120_000 });
+      `[System.IO.Compression.ZipFile]::ExtractToDirectory($env:CLOAKBROWSER_ZIP_SRC, $env:CLOAKBROWSER_ZIP_DEST)`,
+    ], {
+      timeout: 120_000,
+      env: { ...process.env, CLOAKBROWSER_ZIP_SRC: archivePath, CLOAKBROWSER_ZIP_DEST: destDir },
+    });
   } else {
     execFileSync("unzip", ["-o", archivePath, "-d", destDir], { timeout: 120_000 });
   }
@@ -511,11 +535,17 @@ export async function getLatestChromiumVersion(): Promise<string | null> {
     const platformTarball = getArchiveName();
     for (const release of releases) {
       if (release.tag_name.startsWith("chromium-v") && !release.draft) {
+        const version = release.tag_name.replace(/^chromium-v/, "");
+        // Defense in depth: the version flows into cache paths, download URLs,
+        // and the Windows zip-extraction command. Reject anything that isn't a
+        // plain dotted-numeric version so a malformed/hostile release tag can't
+        // inject path, URL, or command components.
+        if (!isSafeVersionTag(version)) continue;
         const assetNames = new Set(
           (release.assets ?? []).map((a) => a.name)
         );
         if (assetNames.has(platformTarball)) {
-          return release.tag_name.replace(/^chromium-v/, "");
+          return version;
         }
       }
     }

tests/test_supplychain_hardening.py

@@ -0,0 +1,85 @@
+"""Tests for binary supply-chain hardening.
+
+  * checksum verification fails CLOSED on missing/absent SHA256SUMS
+  * _fetch_checksums skips non-checksum (HTML) bodies and falls through mirrors
+  * _parse_checksums only accepts valid 64-hex digests
+  * version-tag validation (is_safe_version_tag) blocks injection-shaped tags
+"""
+
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+import cloakbrowser.download as download
+from cloakbrowser.config import get_archive_name, is_safe_version_tag
+
+
+class TestChecksumFailClosed:
+    def test_raises_when_checksums_unavailable(self, tmp_path):
+        f = tmp_path / "archive"
+        f.write_bytes(b"unverified")
+        with patch("cloakbrowser.download._fetch_checksums", return_value=None):
+            with pytest.raises(RuntimeError, match="cannot be integrity-verified"):
+                download._verify_download_checksum(f)
+
+    def test_raises_when_no_entry_for_platform(self, tmp_path):
+        f = tmp_path / "archive"
+        f.write_bytes(b"unverified")
+        with patch("cloakbrowser.download._fetch_checksums",
+                   return_value={"some-other-file.tar.gz": "00" * 32}):
+            with pytest.raises(RuntimeError, match="no entry for"):
+                download._verify_download_checksum(f)
+
+    def test_passes_when_entry_matches(self, tmp_path):
+        import hashlib
+        content = b"the real binary bytes"
+        f = tmp_path / "archive"
+        f.write_bytes(content)
+        digest = hashlib.sha256(content).hexdigest()
+        with patch("cloakbrowser.download._fetch_checksums",
+                   return_value={get_archive_name(): digest}):
+            download._verify_download_checksum(f)  # should not raise
+
+
+class TestFetchChecksumsSkipsJunk:
+    def test_html_body_is_skipped_in_favor_of_valid_mirror(self):
+        valid = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  file.tar.gz\n"
+
+        def mock_get(url, **kwargs):
+            resp = MagicMock()
+            resp.raise_for_status = MagicMock()
+            resp.text = ("<html><body>404 Not Found</body></html>"
+                         if "cloakbrowser.dev" in url else valid)
+            return resp
+
+        with patch.dict("os.environ", {"CLOAKBROWSER_DOWNLOAD_URL": ""}):
+            with patch("cloakbrowser.download.httpx.get", side_effect=mock_get):
+                result = download._fetch_checksums()
+        assert result == {"file.tar.gz": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}
+
+    def test_parse_rejects_non_hex_lines(self):
+        text = "notahash  evil.tar.gz\nGGGG  also-bad.tar.gz\n"
+        assert download._parse_checksums(text) == {}
+
+
+class TestVersionTagValidation:
+    @pytest.mark.parametrize("v", ["146.0.7680.177.5", "145.0.0.0", "1", "1.2.3.4.5"])
+    def test_valid_versions(self, v):
+        assert is_safe_version_tag(v)
+
+    @pytest.mark.parametrize("v", [
+        "", "../../etc", "146.0; calc", "146'+x", "v1.2.3", "1.2.3-beta", "1.2/3",
+    ])
+    def test_invalid_versions(self, v):
+        assert not is_safe_version_tag(v)
+
+    def test_get_latest_skips_unsafe_tag(self):
+        releases = [
+            {"tag_name": "chromium-v9'; evil", "draft": False,
+             "assets": [{"name": get_archive_name()}]},
+        ]
+        resp = MagicMock()
+        resp.json.return_value = releases
+        resp.raise_for_status = MagicMock()
+        with patch("cloakbrowser.download.httpx.get", return_value=resp):
+            assert download._get_latest_chromium_version() is None