python-security/sast-semgrep: false-positive dynamic urllib-use findings (Bandit B310 + Semgrep dynamic-urllib-use-detected) now block every PR

[seonghobae]

Symptom

Semgrep (multi-language SAST) fails with "Semgrep found WARNING/ERROR (Medium+) findings" on .github PR #2065 (run 35240824316, job 105268505006), against merged head f2b5ecae4865ef2ba630d15abcde007ba57bd126:

SEMGREP_FINDING rule=python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected level=warning path=scripts/ci/codeql_ghas_configuration_identity.py line=158 message=Detected a dynamic value being used with urllib. ...
SEMGREP_FINDING rule=python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected level=warning path=scripts/ci/strix_evidence_binding.py line=264 message=Detected a dynamic value being used with urllib. ...

Update: Bandit (Python SAST) (job 105335869601, same PR, same head) independently failed on the same two call sites with its own rule for the identical pattern:

B310: Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected. (severity=MEDIUM, confidence=HIGH)

(2 findings — Bandit's console output doesn't echo path/line, but scripts/ci/ has exactly two unsuppressed urlopen() call sites on main, both in the files above; see below.)

Both flagged lines are on main, unmodified by PR #2065 (whose own diff is .gitleaks.toml only) — confirmed via git grep -n "urlopen(" origin/main -- scripts/.

Why this is a false positive

• scripts/ci/codeql_ghas_configuration_identity.py:158 — _request_json(url, ...) calls urllib.request.urlopen(request, ...) where request wraps url. Its only caller (list_codeql_analyses, line ~191) builds url as f"https://api.github.com/repos/{repository}/code-scanning/analyses?{query}" — the scheme and host are a fixed literal; only the owner/repo path segment and a urlencoded query string are dynamic.
• scripts/ci/strix_evidence_binding.py:264 — same shape: Request(url, ...) then urlopen(request, timeout=30) # noqa: S310 - GitHub HTTPS only, with url built against the fixed https://api.github.com base.

Semgrep's dynamic-urllib-use-detected and Bandit's B310 both flag any urlopen() call whose argument isn't a string literal, regardless of whether the dynamic part can actually change the scheme/host. Neither url here is influenced by PR input, issue/comment bodies, or any other untrusted source — both are internal GitHub REST API calls with a hardcoded trusted host. Same character as the CodeQL false positive already tracked and fixed in #2208 (py/incomplete-url-substring-sanitization), just two more scanners/rules catching the same syntactic pattern in the same two files.

Notable finding: strix_evidence_binding.py:264 already carries a # noqa: S310 comment — but # noqa is the Ruff/flake8 suppression syntax, not Bandit's. Bandit only recognizes # nosec or # nosec B310. So this line already shows someone reasoned about the Bandit finding and tried to suppress it, but used the wrong directive — it has never actually silenced Bandit. scripts/ci/materialize_base_python_requirements.py:363 shows the correct dual-suppression pattern already in use elsewhere in this same repo: # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected # nosec B310.

Why it will affect every PR

Like #2208, the findings are on main and unscoped by any Semgrep/Bandit config (sast-semgrep.yml passes only --config=p/default, --exclude=.github/workflows, --exclude='docs/research/**/standards'; no path-ignore for scripts/ci). Every PR that triggers a fresh whole-tree scan will hit both until resolved on main.

Remediation options (mirroring #2208's structure)

1. Narrow, no new mechanism: assert the resolved URL's scheme/host immediately before each urlopen/Request call, e.g. assert url.startswith("https://api.github.com/"), so the dynamic value is validated in a way both scanners' heuristics recognize. Two call sites, two files.
2. Suppress correctly: replace strix_evidence_binding.py:264's # noqa: S310 with the same dual-suppression comment already used at materialize_base_python_requirements.py:363 (# nosemgrep: ... # nosec B310), and add the equivalent to codeql_ghas_configuration_identity.py:158 (currently unsuppressed for either tool). Fixes both scanners at once, matches existing repo convention, but doesn't remove the underlying heuristic mismatch the way option 1 does.
3. Scope the scan to exclude scripts/ci from these rules — broadest effect, needs its own justification since it would stop scanning that directory for this whole rule class, for both scanners.

Either option 1 or 2 looks right (2 is the path of least resistance since materialize_base_python_requirements.py already establishes the convention), but I am not the writer of codeql_ghas_configuration_identity.py or strix_evidence_binding.py and my open PR (#2065) doesn't touch either file, so I'm not taking it there — filing this per the same not-bypass-eligible governance rule #2208 documented.

Notes for whoever picks this up

• Zero open PRs currently touch either flagged file (checked via search_issues/PR search at filing time), so the lane should be free.
• Whichever option is chosen, fix both call sites together — partial suppression (e.g. only Semgrep or only Bandit) will leave the other scanner red on the next PR.

Filed from PR #2065, whose own diff is one unrelated .gitleaks.toml allowlist entry.

[seonghobae]

Triage (2026-09-18): Ranked #1 in org CI/security backlog (#2251 / docs/doctoring/issue-backlog-org-impact-20260918.md) — Semgrep FP blocking every PR. Recommended next action: implement (suppress/prove FP on the contract-test urllib finding; no open covering PR found).

[seonghobae]

Owner repair is now in Draft PR #2279 at exact 77c1dc201d19dd68d49f5db1f5a13c3425697a98.

The repair does not merely suppress B310 / dynamic-urllib-use-detected: RED 4732f3e29ab8cd0b88506beecd4e70bdfaafb8da requires both central clients to reject non-HTTPS, lookalike-host, userinfo-host and file:// authorities before the opener boundary. Production guards require exact https://api.github.com authority, then retain narrowly scoped scanner annotations at the proved call sites. No repo-wide rule/path exclusion or threshold change.

Fresh #2271/#2275 Semgrep jobs both acquired hosted runners, completed exact checkout + scan + SARIF upload, and failed only at the retained Medium+ gate, so the earlier queue-only diagnosis is superseded for this SAST blocker. #2279 remains Draft until exact-head hosted Semgrep/Python security + focused contract suites + independent review are terminal.

[seonghobae]

#2279 current exact advanced to cb50c8a28708e4d2b244d968b314fa3fb99194af. The authority contract now also covers explicit :443 and fragment spellings as non-canonical and has a positive control proving exact https://api.github.com/... still reaches each opener. All touched fixture/helper functions have docstrings. Final diff remains four files (+209/-4); current exact-head security/quality workflows are queued, so the issue remains open and the repair remains Draft.

[seonghobae]

Owner repair #2279 current exact is now 9ba43f284da51bfa6aaa389d3fb67f8b232fbba5. The final source adjustment puts the CodeQL nosemgrep directive on the exact guarded urlopen line; Semgrep documents nosemgrep as line-scoped, and the repository's existing audited URL precedent uses the same inline form. Final diff remains four files (+208/-4). Exact-head Security Scan, SAST Semgrep, Python Security, CodeQL PR and Agent Review Runtime Quality are queued, so #2248 stays open and #2279 stays Draft.