More hardening

enroll/ignore.py

@@ -193,10 +193,52 @@ HIGH_CONFIDENCE_SECRET_PATTERNS = [
     # start-of-input or any non-identifier character, which is comment-marker
     # agnostic and still avoids matching an identifier such as
     # ``my_authorization:``.
+    #
+    # Both the header form (``Authorization: Bearer ...``) and the config
+    # assignment form (``authorization = Bearer ...`` / ``http_authorization=...``)
+    # are matched: a populated Authorization header is an obvious credential
+    # whether it is written HTTP-style with ``:`` or config-style with ``=``. The
+    # ``(?:proxy|http)`` prefix accepts ``proxy-``/``proxy_``/``http_`` spellings
+    # (e.g. ``proxy_authorization``) that appear in real client/daemon configs.
+    # A scheme keyword (bearer/basic/token/digest) plus a value is still required,
+    # so ordinary keys such as ``AuthorizationEnabled = true`` do not match.
     re.compile(
-        rb"(?im)(?:^|[^A-Za-z0-9_.-])(?:proxy-)?authorization\s*:\s*"
+        rb"(?im)(?:^|[^A-Za-z0-9_.-])(?:(?:proxy|http)[-_])?authorization\s*[:=]\s*"
         rb"(?:bearer|basic|token|digest)\s+\S"
     ),
+    # Token-key credential assignments that the general assignment pattern above
+    # misses because of a leading underscore or a camelCase/prefix spelling.
+    #
+    #   _authToken=npm_xxxxxxxx
+    #   //registry.npmjs.org/:_authToken=npm_xxxxxxxx      (npm .npmrc form)
+    #   authToken=xxxxxxxx
+    #   bearerToken=xxxxxxxx / bearer_token=xxxxxxxx
+    #
+    # The general assignment pattern requires a non-key boundary
+    # (``[^A-Za-z0-9_.-]``) immediately before the credential keyword, which a
+    # leading ``_`` (a word char) defeats -- so ``_authToken`` slips past it --
+    # and its keyword list does not include a ``bearer[_-]?token`` spelling. This
+    # dedicated pattern covers exactly those ``auth``/``bearer`` token keys and,
+    # like the general one, requires a *populated, value-like* right-hand side
+    # (>= 8 non-space chars, or a value containing a digit / token-ish
+    # character), so a bare mention or a short boolean/number such as
+    # ``authTokenEnabled = true`` does not trip it. It is intentionally scoped to
+    # ``auth``/``bearer`` token keys only; broadening the general boundary to
+    # accept a leading underscore everywhere would risk false positives on
+    # value-less identifiers in ordinary config.
+    re.compile(
+        rb"""(?ix)
+        (?:^|[^A-Za-z0-9.-])
+        _?
+        (?:auth|bearer)[_-]?token
+        (?:[_.-][A-Za-z0-9]+)*
+        [\"']?
+        \s*[:=]\s*
+        [\"']?
+        (?=\S)
+        (?:\S{8,}|\S*[0-9/+=._-]\S*)
+        """
+    ),
 ]
 
 SENSITIVE_CONTENT_PATTERNS = [

enroll/remote.py

@@ -189,14 +189,35 @@ def remote_harvest(
             )
 
 
+# Resource caps for untrusted tar extraction. These mirror the directory-bundle
+# freeze limits (see manifest_safety._FREEZE_MAX_FILES / _FREEZE_MAX_FILE_BYTES)
+# so a harvest delivered as a tarball is bounded the same way as one delivered as
+# a directory. The total-size cap additionally guards against a decompression
+# bomb whose members are each individually under the per-file cap.
+_TAR_MAX_MEMBERS = 200_000
+_TAR_MAX_FILE_BYTES = 64 * 1024 * 1024
+_TAR_MAX_TOTAL_BYTES = 10 * 1024 * 1024 * 1024
+_TAR_MAX_PATH_DEPTH = 64
+
+
 def _safe_extract_tar(tar: tarfile.TarFile, dest: Path) -> None:
     """Safely extract a tar archive into dest.
 
-    Protects against path traversal (e.g. entries containing ../).
+    Protects against path traversal (e.g. entries containing ../) and, as
+    availability defence-in-depth, against resource-exhaustion by a structurally
+    valid but abusive archive (a decompression bomb, a huge member, or millions
+    of tiny members). The caps mirror the directory-bundle freeze limits so a
+    tar bundle and a directory bundle are bounded the same way. A remote or
+    user-supplied harvest tarball is untrusted input, so these limits keep a
+    malicious archive from exhausting disk, inodes, memory, or time during local
+    extraction/validation.
     """
     # Note: tar member names use POSIX separators regardless of platform.
     dest = dest.resolve()
 
+    member_count = 0
+    total_size = 0
+
     for m in tar.getmembers():
         name = m.name
 
@@ -205,16 +226,35 @@ def _safe_extract_tar(tar: tarfile.TarFile, dest: Path) -> None:
         if name in {".", "./"}:
             continue
 
+        member_count += 1
+        if member_count > _TAR_MAX_MEMBERS:
+            raise RuntimeError(
+                f"tar archive has too many members (> {_TAR_MAX_MEMBERS})"
+            )
+
         # Reject absolute paths and any '..' components up front.
         p = PurePosixPath(name)
         if p.is_absolute() or ".." in p.parts:
             raise RuntimeError(f"Unsafe tar member path: {name}")
 
+        if len(p.parts) > _TAR_MAX_PATH_DEPTH:
+            raise RuntimeError(f"tar member path is too deeply nested: {name}")
+
         # Refuse to extract links or device nodes from an untrusted archive.
         # (A symlink can be used to redirect subsequent writes outside dest.)
         if m.issym() or m.islnk() or m.isdev():
             raise RuntimeError(f"Refusing to extract special tar member: {name}")
 
+        if m.isfile():
+            if m.size > _TAR_MAX_FILE_BYTES:
+                raise RuntimeError(f"tar member is too large: {name}")
+            total_size += int(m.size)
+            if total_size > _TAR_MAX_TOTAL_BYTES:
+                raise RuntimeError(
+                    "tar archive uncompressed size exceeds limit "
+                    f"(> {_TAR_MAX_TOTAL_BYTES} bytes)"
+                )
+
         member_path = (dest / Path(*p.parts)).resolve()
         if member_path != dest and not str(member_path).startswith(str(dest) + os.sep):
             raise RuntimeError(f"Unsafe tar member path: {name}")

tests/test_remote.py

@@ -1183,3 +1183,99 @@ def test_build_enroll_pyz_excludes_tests_and_caches_and_returns_sha(tmp_path: Pa
         assert "/tests/" not in n
         assert not leaf.startswith("test_")
         assert leaf != "conftest.py"
+
+
+def test_safe_extract_tar_rejects_too_many_members(tmp_path: Path, monkeypatch):
+    import enroll.remote as r
+
+    monkeypatch.setattr(r, "_TAR_MAX_MEMBERS", 3)
+
+    bio = io.BytesIO()
+    with tarfile.open(fileobj=bio, mode="w:gz") as tf:
+        for i in range(10):
+            ti = tarfile.TarInfo(name=f"f{i}")
+            ti.size = 1
+            tf.addfile(ti, io.BytesIO(b"x"))
+
+    bio.seek(0)
+    with tarfile.open(fileobj=bio, mode="r:gz") as tf:
+        with pytest.raises(RuntimeError, match="too many members"):
+            r._safe_extract_tar(tf, tmp_path)
+
+
+def test_safe_extract_tar_rejects_oversized_member(tmp_path: Path, monkeypatch):
+    import enroll.remote as r
+
+    monkeypatch.setattr(r, "_TAR_MAX_FILE_BYTES", 100)
+
+    bio = io.BytesIO()
+    with tarfile.open(fileobj=bio, mode="w:gz") as tf:
+        ti = tarfile.TarInfo(name="big")
+        payload = b"x" * 500
+        ti.size = len(payload)
+        tf.addfile(ti, io.BytesIO(payload))
+
+    bio.seek(0)
+    with tarfile.open(fileobj=bio, mode="r:gz") as tf:
+        with pytest.raises(RuntimeError, match="too large"):
+            r._safe_extract_tar(tf, tmp_path)
+
+
+def test_safe_extract_tar_rejects_aggregate_size_bomb(tmp_path: Path, monkeypatch):
+    import enroll.remote as r
+
+    # Each member is under the per-file cap, but the sum exceeds the total cap.
+    monkeypatch.setattr(r, "_TAR_MAX_TOTAL_BYTES", 1000)
+
+    bio = io.BytesIO()
+    with tarfile.open(fileobj=bio, mode="w:gz") as tf:
+        for i in range(10):
+            ti = tarfile.TarInfo(name=f"f{i}")
+            payload = b"x" * 300
+            ti.size = len(payload)
+            tf.addfile(ti, io.BytesIO(payload))
+
+    bio.seek(0)
+    with tarfile.open(fileobj=bio, mode="r:gz") as tf:
+        with pytest.raises(RuntimeError, match="uncompressed size exceeds"):
+            r._safe_extract_tar(tf, tmp_path)
+
+
+def test_safe_extract_tar_rejects_excessive_path_depth(tmp_path: Path, monkeypatch):
+    import enroll.remote as r
+
+    monkeypatch.setattr(r, "_TAR_MAX_PATH_DEPTH", 4)
+
+    deep = "/".join(f"d{i}" for i in range(10)) + "/file"
+    bio = io.BytesIO()
+    with tarfile.open(fileobj=bio, mode="w:gz") as tf:
+        ti = tarfile.TarInfo(name=deep)
+        ti.size = 1
+        tf.addfile(ti, io.BytesIO(b"x"))
+
+    bio.seek(0)
+    with tarfile.open(fileobj=bio, mode="r:gz") as tf:
+        with pytest.raises(RuntimeError, match="too deeply nested"):
+            r._safe_extract_tar(tf, tmp_path)
+
+
+def test_safe_extract_tar_allows_normal_bundle(tmp_path: Path):
+    """A normal, modestly-sized bundle still extracts cleanly under the caps."""
+    import enroll.remote as r
+
+    bio = io.BytesIO()
+    with tarfile.open(fileobj=bio, mode="w:gz") as tf:
+        for name, data in (
+            ("state.json", b"{}"),
+            ("artifacts/etc_custom/etc/app.conf", b"k=1\n"),
+        ):
+            ti = tarfile.TarInfo(name=name)
+            ti.size = len(data)
+            tf.addfile(ti, io.BytesIO(data))
+
+    bio.seek(0)
+    with tarfile.open(fileobj=bio, mode="r:gz") as tf:
+        r._safe_extract_tar(tf, tmp_path)
+
+    assert (tmp_path / "state.json").is_file()
+    assert (tmp_path / "artifacts" / "etc_custom" / "etc" / "app.conf").is_file()

tests/test_secret_detection.py

@@ -371,3 +371,74 @@ def test_binary_keyring_allowlist_still_scans_for_private_key_material():
         dangerous._content_deny_reason("/etc/apt/keyrings/mykey.gpg", private_binary)
         is None
     )
+
+
+def test_authorization_assignment_form_is_denied():
+    """A populated Authorization/Proxy-Authorization credential is refused
+    whether written HTTP-style with ':' or config-style with '='.
+
+    The header scanner previously only anchored on ':', so a config assignment
+    such as ``Authorization = Bearer <token>`` (and ``proxy_authorization``/
+    ``http_authorization`` spellings) slipped through safe mode.
+    """
+    pol = IgnorePolicy(dangerous=False)
+    leaking = [
+        b"Authorization = Bearer supersecretvalue\n",
+        b"Authorization: Bearer supersecretvalue\n",
+        b"authorization=Bearer abcdef123456\n",
+        b"proxy_authorization = Basic abcdef123456\n",
+        b"Proxy-Authorization: Bearer abcdef123456\n",
+        b"http_authorization = Bearer abcdef123456\n",
+    ]
+    for data in leaking:
+        assert (
+            pol._content_deny_reason("/etc/example.conf", data) == "sensitive_content"
+        ), data
+
+
+def test_underscore_and_camelcase_token_keys_are_denied():
+    """Token keys the general assignment pattern misses -- a leading-underscore
+    npm token, a camelCase ``authToken``, or a ``bearerToken`` spelling -- are
+    refused when they carry a populated value."""
+    pol = IgnorePolicy(dangerous=False)
+    leaking = [
+        b"_authToken=npm_abcdefghijklmnopqrstuvwxyz\n",
+        b"//registry.npmjs.org/:_authToken=npm_abcdefghijklmnop\n",
+        b"authToken=abcdefghijklmnopqrstuvwxyz\n",
+        b"bearerToken=abcdefghijklmnopqrstuvwxyz\n",
+        b"bearer_token = abcdef123456\n",
+        b"auth_token: ya29.aRealLookingToken\n",
+    ]
+    for data in leaking:
+        assert (
+            pol._content_deny_reason("/etc/app/app.conf", data) == "sensitive_content"
+        ), data
+
+
+def test_authorization_and_token_keys_no_false_positives():
+    """The broadened Authorization/token-key rules must not fire on ordinary
+    config: a scheme keyword + value is required for the header form, and a
+    populated value is required for the token-key form, so boolean/numeric
+    settings and value-less mentions stay collectable."""
+    pol = IgnorePolicy(dangerous=False)
+    allowed = [
+        # Authorization-ish keys with no bearer/basic/... scheme + value.
+        b"AuthorizationEnabled = true\n",
+        b"authorization = true\n",
+        b"my_authorization: somevalue\n",
+        # Token-ish keys that are not populated credentials.
+        b"authTokenEnabled = true\n",
+        b"authTokenTimeout = 30\n",
+        b"AuthorizedKeysFile .ssh/authorized_keys\n",
+    ]
+    for data in allowed:
+        assert pol._content_deny_reason("/etc/app/app.conf", data) is None, data
+
+    # --dangerous still collects everything.
+    dangerous = IgnorePolicy(dangerous=True)
+    assert (
+        dangerous._content_deny_reason(
+            "/etc/example.conf", b"Authorization = Bearer secret\n"
+        )
+        is None
+    )