diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4cf958e..f3da1a7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,9 +1,10 @@ name: CI -# Runs on every push and PR. Three things must hold: +# Runs on every push and PR. Four things must hold: # 1. generated files match their sources (Tokens.swift, Symbols.swift, Sources/Pantry/Models, NetRelish.xcodeproj) # 2. both product flavors build — AppStore and Direct (DIRECT_BUILD) # 3. the tests pass (hosted NetRelishTests + unhosted GeneratorTests) +# 4. the non-negotiable-5 gate still enforces what it claims to # Read-only token. Every job here checks out, regenerates, builds or tests — # nothing writes back to the repository. Without this block the workflow @@ -87,3 +88,16 @@ jobs: with: name: design-kit-snapshots path: design-kit-snapshots/*.png + + gate-tests: + name: CodeQL gate self-test + # Ubuntu, and not xcode-27, on purpose. This tests scripts/codeql_gate.py, + # which is pure Python and touches no Swift — and it MUST keep running while + # the CodeQL workflow cannot build this project on the macOS image. A gate + # whose only proof of correctness lives inside the job that is currently red + # is not proven at all. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 (node24) + - name: The gate enforces what it claims to + run: python3 -m unittest discover -s scripts/tests -v diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index a006a60..5befdc9 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -151,132 +151,26 @@ jobs: category: "/language:swift" output: .codeql-results - # analyze uploads findings; it never fails the job on them. Without this - # step the gate is a report, not a gate — the CodeQL check can sit green - # with an alert on it, so requiring it in branch protection would still let - # a fifth endpoint merge. Gating repo-side works too (code scanning's - # check-failure severity, or a ruleset), but that is a console toggle a - # fork, a clone or a forgetful afternoon loses. This travels with the repo. + # analyze uploads findings; it never fails the job on them, and there is + # no `fail-on` input. Without this step the gate is a report, not a gate — + # the CodeQL check can sit green with an alert on it, so requiring it in + # branch protection would still let a fifth endpoint merge. Gating + # repo-side works too (code scanning's check-failure severity, or a + # ruleset), but that is a console toggle a fork, a clone or a forgetful + # afternoon loses. This travels with the repo. + # + # The logic is in scripts/codeql_gate.py, not inline here. It has had + # twenty-eight defects — five before it was extracted, and twenty-three more found + # by review of the branch that extracted it — every one in its + # enforcement rather than its design, and every one failing OPEN. A + # heredoc cannot be tested, so the first five were caught by a human + # reading it, or not caught at all. It now has a suite in scripts/tests/ + # that the `CodeQL gate self-test` job in ci.yml runs on every push, + # including while THIS job cannot build the project. The module header + # lists all twenty-eight; read it before adding anything here. # # Only the gate fails the build. netrelish/remote-capable-url-read has two # legitimate baseline hits; failing on it would teach everyone to ignore a # red CodeQL job, which is the opposite of what the split was for. - name: Fail if the gate fired - run: | - python3 - <<'PY' - import glob, json, pathlib, re, sys - from urllib.parse import unquote, urlparse - - GATE = "netrelish/outbound-network-call" - - # An approved exception is a comment on the flagged line or the one - # directly above it, citing the ADR that approved it: - # - # // NETRELISH-ALLOW-ENDPOINT: adr-0007 — RFC 3161 timestamping - # - # It lives beside the code rather than in a list, so it shows up in - # every diff that touches the call, cannot drift away from what it - # exempts, and dies with the line. The ADR reference is required - # because CLAUDE.md already says an uncovered decision gets one, and - # an exception with no written reason is how a rule rots. - # - # Dismissing the alert in the Security tab deliberately does NOT - # satisfy this: that is invisible from the repository. - MARKER = re.compile(r"NETRELISH-ALLOW-ENDPOINT:\s*(adr-\d{4})") - - def exemption(line): - """The marker, but only in a // comment and naming a real ADR. - - Two bypasses this closes. Searching raw source matched the token - inside a string literal, so a line like - let s = "NETRELISH-ALLOW-ENDPOINT: adr-0001" was an exemption. - And checking only the adr-NNNN SHAPE meant adr-9999 passed with - no written decision existing at all, which made the ADR - requirement decoration rather than a control. - - Still not a Swift parser: a // inside a string literal ahead of - the marker would fool it. That is far narrower than either hole - above, and closing it properly needs parsed source, not text. - """ - _, sep, comment = line.partition("//") - if not sep: - return None - found = MARKER.search(comment) - if not found: - return None - number = found.group(1)[len("adr-"):] - if not list(pathlib.Path("docs/adr").glob(number + "-*.md")): - print("::error::" + found.group(1) - + " is cited but no docs/adr/" + number + "-*.md exists") - return None - return found.group(1) - - def source_path(uri): - """The checkout-relative path for a SARIF artifactLocation. - - SARIF gives a URI, not a filename. CodeQL usually emits a - repo-relative one, but a file:///... form (often with a - uriBaseId) is equally legal and pathlib.Path would treat the - whole thing as a literal relative name. The source would then - never open, no marker would be found, and a VALID exemption - would block the build with a misleading message. - - Fails closed either way, but closed-and-wrong is still wrong. - """ - parsed = urlparse(uri) - raw = unquote(parsed.path) if parsed.scheme == "file" else unquote(uri) - path = pathlib.Path(raw) - if path.is_absolute(): - try: - path = path.relative_to(pathlib.Path.cwd()) - except ValueError: - pass # genuinely outside the checkout; leave it absolute - return path - - paths = sorted(glob.glob(".codeql-results/*.sarif")) - if not paths: - # A missing SARIF must never read as "clean" — that is precisely - # the false assurance this whole query pack exists to prevent. - print("::error::no SARIF in .codeql-results; the gate did not run") - sys.exit(1) - - approved, blocking = [], [] - for path in paths: - with open(path) as f: - sarif = json.load(f) - for run in sarif.get("runs", []): - for result in run.get("results", []): - if result.get("ruleId") != GATE: - continue - where = result["locations"][0]["physicalLocation"] - uri = where["artifactLocation"]["uri"] - line = where.get("region", {}).get("startLine", 1) - - source, found = source_path(uri), None - if source.is_file(): - body = source.read_text(errors="replace").splitlines() - for candidate in body[max(0, line - 2):line]: - found = found or exemption(candidate) - else: - print("::error::cannot read " + str(source) + " (from SARIF uri " - + uri + "); an exemption there could not be checked") - - entry = (str(source), line, result["message"]["text"], found) - (approved if found else blocking).append(entry) - - for uri, line, _, found in approved: - print(f"::notice file={uri},line={line}::approved endpoint ({found})") - for uri, line, message, _ in blocking: - print(f"::error file={uri},line={line}::{message}") - - if approved: - print(f"\n{len(approved)} approved exception(s), re-printed every run so they stay visible.") - - if blocking: - print(f"\n{GATE} fired {len(blocking)} time(s) with no approved exception. " - "CLAUDE.md non-negotiable 5 permits page loads, StoreKit, RFC 3161 " - "timestamping and direct-build licence activation, and nothing else.") - sys.exit(1) - - print(f"{GATE}: clean across {len(paths)} SARIF file(s).") - PY + run: python3 scripts/codeql_gate.py .codeql-results diff --git a/.gitignore b/.gitignore index e00168a..a9aa727 100644 --- a/.gitignore +++ b/.gitignore @@ -29,3 +29,7 @@ AuthKey_*.p8 # logs *.log + +# Python bytecode from scripts/ and its tests +__pycache__/ +*.pyc diff --git a/docs/RELEASING.md b/docs/RELEASING.md index afe15d0..a661286 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -17,13 +17,13 @@ Two builds ship from one codebase (`#if DIRECT_BUILD`): | Build | Signs with | Distributed by | Updates by | | :-- | :-- | :-- | :-- | | App Store | Apple Distribution | App Store Connect | the App Store | -| Direct | Developer ID | *undecided* | email — by design, see ADR 0006 | +| Direct | Developer ID | *undecided* | email — by design, see ADR 0007 | The direct build has **no updater, deliberately**: no Sparkle, no appcast, nothing that polls. A new version is announced by email to the address that bought the licence, and About links to the download page. Decided by Ryan on 2026-09-23; the reasoning, and what would overturn it, are in -`docs/adr/0006-direct-build-updates.md`. +`docs/adr/0007-direct-build-updates.md`. The constraint that drove it is CLAUDE.md §5, which lists every call the app may make and ends "Add an endpoint and you've broken this rule." An appcast diff --git a/docs/adr/0006-direct-build-updates.md b/docs/adr/0007-direct-build-updates.md similarity index 97% rename from docs/adr/0006-direct-build-updates.md rename to docs/adr/0007-direct-build-updates.md index a85c815..84a6245 100644 --- a/docs/adr/0006-direct-build-updates.md +++ b/docs/adr/0007-direct-build-updates.md @@ -1,4 +1,4 @@ -# ADR 0006 — The direct build announces updates; it does not fetch them +# ADR 0007 — The direct build announces updates; it does not fetch them **Status:** accepted (Ryan, 2026-09-23) · **Date:** 2026-09-23 · **PR:** docs: ADR for the direct-build updater diff --git a/scripts/codeql_gate.py b/scripts/codeql_gate.py new file mode 100644 index 0000000..d465255 --- /dev/null +++ b/scripts/codeql_gate.py @@ -0,0 +1,864 @@ +#!/usr/bin/env python3 +"""The gate for CLAUDE.md non-negotiable 5, and the exemption mechanism it honours. + +`github/codeql-action/analyze` uploads findings but never fails the job, and it +has no `fail-on` input. So the workflow enforces the rule itself: analyze writes +SARIF, and this script fails the build if `netrelish/outbound-network-call` +produced any result that is not approved in writing. A MISSING SARIF also fails +— a gate that silently passes when it did not run is exactly the false assurance +this query pack exists to prevent. + +Gating repo-side would work too (code scanning's check-failure severity, or a +ruleset), but that is a console toggle a fork, a clone or a forgetful afternoon +loses. This travels with the repo. + +WHY THIS IS A FILE AND NOT A HEREDOC IN THE WORKFLOW +---------------------------------------------------- +It used to be a heredoc. Every defect this mechanism has had — five now — was in +its ENFORCEMENT rather than its design: + + 1. no approved path at all + 2. markers matched inside string literals; cited ADRs never had to exist + 3. SARIF URIs treated as plain filenames + 4. markers inside /* block comments */ accepted + 5. one marker approving every finding that shared its line + +Each idea was right; each implementation had a hole. A heredoc cannot be tested, +so every one of those was caught by a human reading it, or not caught at all. + +That list is the HISTORICAL set — the five that existed before this file did. +Review of the branch that extracted it found TWENTY-THREE more, every one of them +failing open: markers hidden in extended regex literals, in interpolated nested +strings, and behind a bare regex's own closing delimiter; an escaped delimiter +ending an extended regex early; a trailing marker exempting the line below it; +ADR citations satisfied by a directory, by a symlink, by a symlinked parent +directory, and by a longer number that merely started with a real one; a +missing SARIF start line defaulting to 1; columnless findings merging on their +message; malformed columns, and then impossible column RANGES, merging two +findings into one; source paths escaping the checkout entirely; a URI scheme +this did not understand becoming a relative path inside it; absolute paths +rebased against the working directory rather than the checkout being judged; +two spellings of one path splitting a shared line into two; a finding identity +that ignored endLine, merging two distinct multi-line regions; and — worst of +them — a SARIF that ran no gate query at all reading as a clean one, which is +the missing-SARIF false assurance with an extra step; a bare regex inside an +interpolation whose own `)` and `"` steered the scanner out of the string it +was in, and then the same again for a regex following `return`, which the +previous-character test read as a value; a present-but-malformed endLine +being indistinguishable from an absent one; and a `file://host/path` URI whose +authority was discarded, resolving another machine's path against this +checkout. + +Five of the twenty-three were produced by fixing another: one introduced outright +and defended in review before it was checked, one a gap its own predecessor's +fix did not cover. A fix that is correct about what it checks and silent about +what it does not is the shape to watch for here. + +Twenty-eight defects, not one of them in the design. That is the case for the suite +in scripts/tests/, which runs on every push — including while CodeQL itself +cannot build this project. See scripts/tests/test_codeql_gate.py. + +The lesson is in the ratio rather than the number: reading this code carefully +has never once been sufficient. Anything added here needs a test that fails +without it. + +FAIL CLOSED, ALWAYS +------------------- +Every ambiguity here resolves toward blocking the build. An unreadable source +file, an ADR citation matching two files, two findings sharing one line, a +mangled SARIF URI: all block. Being wrong in the blocking direction costs +someone an explanation. Being wrong in the approving direction costs the rule +its meaning. +""" + +from __future__ import annotations + +import argparse +import glob +import json +import pathlib +import re +import sys +from urllib.parse import unquote, urlparse + +# The gate. `netrelish/remote-capable-url-read` is the audit and is deliberately +# NOT gated: it has two legitimate baseline hits, and failing on it would teach +# everyone to ignore a red CodeQL job — the opposite of what the split was for. +GATE = "netrelish/outbound-network-call" + +# An approved exception is a comment on the flagged line or the one directly +# above it, citing the ADR that approved it: +# +# // NETRELISH-ALLOW-ENDPOINT: adr-0007 — RFC 3161 timestamping +# +# It lives beside the code rather than in a list, so it shows up in every diff +# that touches the call, cannot drift away from what it exempts, and dies with +# the line. The ADR reference is required because CLAUDE.md already says an +# uncovered decision gets one, and an exception with no written reason is how a +# rule rots. +# +# Dismissing the alert in the Security tab deliberately does NOT satisfy this: +# that is invisible from the repository. +# +# The trailing guard matters: `\d{4}` alone matched the first four digits of +# `adr-00070` and captured `adr-0007`, so a malformed citation was silently +# accepted whenever ADR 0007 happened to exist. The number must end where the +# citation ends. +MARKER = re.compile(r"NETRELISH-ALLOW-ENDPOINT:\s*(adr-\d{4})(?![\w-])") + + +# -------------------------------------------------------------------------- +# Where a marker is allowed to live +# -------------------------------------------------------------------------- + +def line_comments(source: str) -> dict[int, str]: + """Map 1-based line number -> the text of the genuine `//` comment on it. + + A marker only counts inside real line-comment text. Deciding what that means + is the whole job, and text matching is not up to it: + + let s = "NETRELISH-ALLOW-ENDPOINT: adr-0001" // a string, not a comment + /* // NETRELISH-ALLOW-ENDPOINT: adr-0006 */ // commented-out, not a comment + let u = "https://x/" // NETRELISH-ALLOW-ENDPOINT: adr-0007 <- this one is real + + The earlier `line.partition("//")` accepted the first two. The second was + the dangerous one: a commented-out block that happens to contain a marker + silently approves whatever line follows it. + + So this scans the file once, front to back, tracking every lexical context + that can hide or reveal a `//`: block comments (which NEST in Swift), string + literals, raw strings (`#"..."#` at any hash count, where the escape is + `\\#`), extended regex literals (`#/.../#`), the multiline forms of each, + and string interpolation. + + Interpolation is why the contexts are a STACK rather than a few flags. + `\\(` returns to code INSIDE a string, that code can open another string, + and that one can interpolate again. Flat state mistook the nested opening + quote for the outer closer and fell out into "code" mid-literal, so + + let s = "\\("// NETRELISH-ALLOW-ENDPOINT: adr-0007")" + + recorded a line comment and approved the call below it. Each construct now + pushes; its terminator pops; a `//` is only recorded as a comment when the + stack is empty, which is the one place a genuine comment can be. + + Unterminated constructs fail closed for free: an unclosed `/*`, string or + `#/` swallows the rest of the file, so no further line records a comment, + so nothing below it can be approved. + + This is a comment/string scanner, not a Swift parser, and that is the + correct scope — it needs to answer one question, and it answers it for every + way Swift can spell these constructs. + """ + comments: dict[int, str] = {} + source_length = len(source) + index = 0 + line = 1 + + depth = 0 # block-comment nesting depth; 0 means we are not in one + + # Innermost context last. Either ["str", hashes, multiline] or + # ["interp", paren_depth] — the latter is code again, inside a literal. + stack: list[list] = [] + + def in_string() -> bool: + return bool(stack) and stack[-1][0] == "str" + + def advance(count: int) -> None: + """Step `count` characters, keeping the line number honest.""" + nonlocal index, line + line += source.count("\n", index, index + count) + index += count + + while index < source_length: + char = source[index] + + if char == "\n": + # A single-line string cannot span a newline. Reaching one means the + # source is malformed; resync at the line break rather than treating + # the remainder of the file as string content. + # + # Only the directly-enclosing string is resynced. A newline inside + # an interpolation is legal within a multiline literal, and guessing + # which one this is would risk resyncing out of a construct that is + # genuinely open. Not resyncing only swallows more of the file, + # which approves nothing. + if in_string() and not stack[-1][2]: + stack.pop() + advance(1) + continue + + if depth: + if source.startswith("/*", index): + depth += 1 + advance(2) + elif source.startswith("*/", index): + depth -= 1 + advance(2) + else: + advance(1) + continue + + if in_string(): + _, hashes, multiline = stack[-1] + escape = "\\" + "#" * hashes + closer = '"""' if multiline else '"' + + # Interpolation first: `\(` is also a prefix of the generic escape, + # and treating it as one would skip the paren and read the rest of + # the interpolation as string content. + if source.startswith(escape + "(", index): + stack.append(["interp", 0]) + advance(len(escape) + 1) + elif source.startswith(escape, index): + advance(len(escape) + 1) # the escape, then the character it escapes + elif source.startswith(closer + "#" * hashes, index): + stack.pop() + advance(len(closer) + hashes) + else: + advance(1) + continue + + # Code — either the top level, or inside an interpolation. + if stack: # ["interp", paren_depth] + if char == "(": + stack[-1][1] += 1 + advance(1) + continue + if char == ")": + if stack[-1][1]: + stack[-1][1] -= 1 + else: + stack.pop() # interpolation over; the string resumes + advance(1) + continue + + if source.startswith("//", index): + end = source.find("\n", index) + end = source_length if end == -1 else end + # First genuine `//` wins: everything after it on the line is inside + # that same comment. + # + # Recorded only at the top level. A `//` inside an interpolation + # would comment out the closing paren and quote, so it cannot occur + # in code that compiles — and code that does not compile is code + # CodeQL never flagged. Skipping it here costs nothing and cannot + # approve anything. + if not stack: + comments.setdefault(line, source[index + 2:end]) + advance(end - index) + continue + + if source.startswith("/*", index): + depth = 1 + advance(2) + continue + + # A run of `#` opens a string when a quote follows it and an extended + # regex literal when a slash does; `#if` and `#Preview` are neither. + run = 0 + while index + run < source_length and source[index + run] == "#": + run += 1 + opener_at = index + run + + if opener_at < source_length and source[opener_at] == '"': + is_multiline = source.startswith('"""', opener_at) + stack.append(["str", run, is_multiline]) + advance(run + (3 if is_multiline else 1)) + continue + + if run and opener_at < source_length and source[opener_at] == "/": + # Extended regex literal, `#/ ... /#` at any hash count, possibly + # spanning lines. Its contents are pattern text, so a `//` in there + # is not a comment — and in extended syntax whitespace is ignored, + # which makes `#/ // NETRELISH-ALLOW-ENDPOINT: adr-0007 /#` read + # exactly like an approval to the naive scanner. Skip to the + # matching delimiter; unterminated swallows the rest, which fails + # closed. + # + # Escape-aware, because `source.find("/#")` matched the `/` of an + # escaped `\/` and ended the literal early — leaving the rest of the + # pattern to be read as code, where a `//` in it became a comment. + closer = "/" + "#" * run + scan, end = opener_at + 1, source_length + while scan < source_length: + if source[scan] == "\\": + scan += 2 + continue + if source.startswith(closer, scan): + end = scan + len(closer) + break + scan += 1 + advance(end - index) + continue + + if char == "/" and _regex_may_start(source, index) \ + and index + 1 < source_length and source[index + 1] not in " \t\n/*": + # A bare `/.../` literal. Its contents are pattern text, and that + # text can contain the punctuation this scanner steers by: in + # + # let s = "\(/[)]"/) // NETRELISH-ALLOW-ENDPOINT: adr-0007" + # + # the regex's `)` closed the interpolation and its `"` closed the + # outer string, so the marker — which is string content — was + # recorded as a comment. Making the literal opaque keeps the + # scanner inside the construct it is actually in. + # + # A `/` with no closing partner on the line is not a regex, so it + # falls through rather than swallowing the rest of the line. + closes = _bare_regex_end(source, index) + if closes is not None: + advance(closes - index) + continue + + if char == "\\": + # An escape in code is opaque: skip it and whatever it escapes. + # + # This is what makes the BARE `/.../` regex form safe to leave + # untracked. I claimed two unescaped slashes would end such a + # literal at the first, so `//` could not occur inside one. True, + # and beside the point: the literal's own ending supplies the + # second slash. In `let r = /\//` the `\/` and the closing `/` are + # textually `//`, and the rest of the line was read as a comment. + # + # Skipping `\/` as one token leaves the closing `/` alone, with + # nothing after it to pair with. Disambiguating regex from division + # is not needed, and is not something this scanner should attempt. + advance(2) + continue + + advance(1) + + return comments + + +def cited_adr(comment: str, repo_root: pathlib.Path, report: "Report") -> str | None: + """The ADR this comment cites, if it cites exactly one that exists. + + Checking only the adr-NNNN SHAPE meant `adr-9999` passed with no written + decision existing at all, which made the ADR requirement decoration rather + than a control. So the file has to be there. + + It also has to be the ONLY file there. `docs/adr/` has held two different + ADRs both numbered 0006, and a citation that resolves to two unrelated + decisions approves against whichever one the reader assumes. That is an + ambiguous exemption, so it is refused and reported. + """ + found = MARKER.search(comment) + if not found: + return None + + citation = found.group(1) + number = citation[len("adr-"):] + # A match counts only if it is a regular file that actually lives here. + # + # `glob` also returns directories, so `0007-placeholder.md/` would satisfy + # the citation with no written decision inside it. `is_file()` follows + # symlinks, so `0007-anything.md -> /etc/hosts` would satisfy it with a + # decision that is not in the repository at all. And checking only the + # matched file left its PARENT unchecked, so `docs/adr -> /tmp/decisions` + # handed back perfectly ordinary regular files from outside the checkout. + # + # Hence `inside_checkout`, which resolves the whole path rather than + # inspecting its last component: every one of those is an approval backed + # by something this repository does not contain, which is the failure the + # ADR requirement exists to prevent. + matches = sorted( + p.name for p in (repo_root / "docs" / "adr").glob(number + "-*.md") + if not p.is_symlink() and p.is_file() + and inside_checkout(repo_root, p) is not None + ) + + if not matches: + report.errors.append( + citation + " is cited but no docs/adr/" + number + "-*.md exists" + ) + return None + + if len(matches) > 1: + report.errors.append( + citation + " is ambiguous: it matches " + ", ".join(matches) + + ". Renumber so the citation names one decision." + ) + return None + + return citation + + +# -------------------------------------------------------------------------- +# Reading what CodeQL produced +# -------------------------------------------------------------------------- + +def source_path(uri: str) -> pathlib.Path: + """The checkout-relative path for a SARIF artifactLocation. + + SARIF gives a URI, not a filename. CodeQL usually emits a repo-relative one, + but a file:///... form (often with a uriBaseId) is equally legal, and + pathlib.Path would treat the whole thing as a literal relative name. The + source would then never open, no marker would be found, and a VALID + exemption would block the build with a misleading message. + + Fails closed either way, but closed-and-wrong is still wrong. + + An absolute path is left absolute. It used to be rebased against + `Path.cwd()`, which is not necessarily the checkout being evaluated: with a + `--repo-root` elsewhere, `file:////Sources/A.swift` was rewritten to + `Sources/A.swift` and then looked up under the OTHER root. If a file lived + there too, the gate read it — and approved the finding using a marker from + a checkout the finding had nothing to do with. + + Canonicalising against the root that is actually in use is `inside_checkout`'s + job, and it is the only one that knows which root that is. + + Returns None for a URI that does not name a local file. Anything other than + `file:` and a plain relative reference used to fall through to `Path`, so + `http://evil` became the relative path `http:/evil` — and had the checkout + contained one, its markers would have been read. A scheme this does not + understand is not a source file, so the caller fails closed instead. + + The AUTHORITY has to be checked too, which the first version of that guard + missed while standing in the same function: `file://attacker/x/A.swift` + names a file on another host, and discarding `netloc` resolved its path + against this checkout and approved it from a local marker. Per RFC 8089 + only an empty authority or `localhost` is local. + + A relative reference whose first segment contains a colon is invalid per + RFC 3986 and must be written `./Odd:Name.swift`; urlparse reads the part + before that colon as a scheme, so such a URI is refused. `Sources/A:B.swift` + is unaffected — a scheme cannot contain a slash. + """ + parsed = urlparse(uri) + if parsed.scheme not in ("", "file"): + return None + if parsed.scheme == "file": + if parsed.netloc.lower() not in ("", "localhost"): + return None + return pathlib.Path(unquote(parsed.path)) + return pathlib.Path(unquote(uri)) + + +# Swift keywords after which an expression begins, so a `/` following one opens +# a regex rather than dividing. None of these can be an identifier, so reading +# them this way cannot misfire on a variable name. +EXPRESSION_KEYWORDS = frozenset({ + "return", "throw", "try", "await", "case", "where", "in", "is", "as", + "if", "guard", "while", "repeat", "else", "do", "defer", "yield", + "let", "var", "and", "or", "not", "some", "any", "each", +}) + + +def _regex_may_start(source: str, index: int) -> bool: + """Whether a `/` at `index` opens a bare regex rather than dividing. + + Swift resolves this by grammar: `/.../` is a regex where an EXPRESSION is + expected, and division after a value. Approximated by the previous + significant token — after an identifier, a number, `)`, `]` or a closing + quote we are past a value, so the slash divides. Anywhere else (`=`, `(`, + `,`, start of line) an expression is expected. + + An identifier is not always a value, which the first version of this missed: + the keywords below all END an expression context rather than producing one, + so `return /[)]"/` is a regex even though the character before the slash is + a letter. Left unrecognised, that regex's punctuation was live again and + steered the scanner out of the string containing it. + + Going the other way costs something real too: `a/b // + NETRELISH-ALLOW-ENDPOINT: adr-0007` must not be read as a regex running up + to the comment's own slashes, because that loses a REAL marker. Only + blocks — but blocking a legitimate exemption is still a bug. + """ + scan = index - 1 + while scan >= 0 and source[scan] in " \t": + scan -= 1 + if scan < 0: + return True + + previous = source[scan] + if previous in ')]"': + return False + if not (previous.isalnum() or previous == "_"): + return True + + end = scan + 1 + while scan >= 0 and (source[scan].isalnum() or source[scan] == "_"): + scan -= 1 + return source[scan + 1:end] in EXPRESSION_KEYWORDS + + +def _bare_regex_end(source: str, index: int) -> int | None: + """Where the bare regex opening at `index` ends, or None if it does not. + + Swift's closing delimiter is an unescaped `/` not preceded by whitespace, + on the same line. A `/` with no such partner is not a regex at all, so the + caller leaves it alone rather than swallowing the rest of the line. + """ + length = len(source) + scan = index + 1 + while scan < length and source[scan] != "\n": + if source[scan] == "\\": + scan += 2 + continue + if source[scan] == "/" and source[scan - 1] not in " \t": + return scan + 1 + scan += 1 + return None + + +def declares_gate(sarif) -> bool: + """Whether this SARIF is evidence that the gate query actually ran. + + The module already refuses to read a MISSING SARIF as clean. A present one + that says nothing is the same false assurance with an extra step: a report + containing `{"runs": []}`, or runs from a analysis that never loaded this + pack, produced no gate results — and "no results" was indistinguishable + from "no findings". A query that silently stops running is exactly how + enforcement disappears without anyone noticing. + + So a clean verdict now requires the rule to appear somewhere: in a run's + tool metadata (driver or extension, since a query pack lands in the latter) + or in a result. Either is proof the query was part of the analysis. + """ + for run in sarif.get("runs") or []: + tool = run.get("tool") or {} + components = [tool.get("driver") or {}] + list(tool.get("extensions") or []) + for component in components: + for rule in component.get("rules") or []: + if isinstance(rule, dict) and rule.get("id") == GATE: + return True + for result in run.get("results") or []: + if result.get("ruleId") == GATE: + return True + return False + + +def positive_int(value): + """The value if it is a real 1-based SARIF coordinate, else None. + + `isinstance(True, int)` is True in Python, so booleans are excluded + explicitly rather than by accident. + """ + if isinstance(value, bool) or not isinstance(value, int) or value < 1: + return None + return value + + +def inside_checkout(repo_root: pathlib.Path, path) -> pathlib.Path | None: + """The source file, but only if it really is one of ours. + + SARIF is input, and this one names the file whose comments decide whether a + network call is approved. Nothing constrained it: an absolute `file:` URI, + or a relative one containing `..`, escaped `repo_root` when joined and was + read anyway — so a location pointing at any file on the runner that happens + to contain a valid marker was approved. + + Resolution also follows symlinks, so a tracked source file pointing outside + the checkout is rejected for the same reason. + + Returns None when the path does not stay inside, which the caller treats as + an exemption it could not check, and therefore blocks. + """ + try: + resolved = (repo_root / path).resolve() + resolved.relative_to(repo_root.resolve()) + except (ValueError, OSError, RuntimeError): + return None + return resolved + + +class Finding: + """One gate result, reduced to the parts that decide its fate.""" + + __slots__ = ("path", "line", "end_line", "start_column", "end_column", + "message", "trusted") + + def __init__(self, path, line, start_column, end_column, message, + end_line=None, trusted=True): + self.path = path + self.line = line + # SARIF: endLine defaults to startLine, so a single-line region is + # exactly the case where it is absent. + # + # `trusted` is how an absent endLine is told apart from a PRESENT but + # malformed one. Both arrive here as None, and defaulting both to + # startLine let a finding with broken coordinates share an identity + # with a sound single-line one — so one marker approved both. A region + # that reports an impossible end is not a position, and follows + # malformed columns into the columnless path. + self.end_line = end_line if end_line is not None else line + self.trusted = trusted + self.start_column = start_column + self.end_column = end_column + self.message = message + + @property + def locatable(self) -> bool: + """Whether this finding can be told apart from another on its line. + + Only a real RANGE counts. Treating any non-None value as trustworthy + meant two distinct findings both reported at `startColumn: 0, + endColumn: 0` shared an identity and collapsed into one, which a single + marker then approved. Validating each column on its own was not enough + either: `startColumn: 10, endColumn: 5` passed both checks and two + findings carrying that same impossible range collided the same way. + + An invalid range carries no position, so it is columnless — and + columnless findings are never merged. + """ + return ( + self.trusted + and self.start_column is not None + and self.end_column is not None + and self.end_line >= self.line + and (self.end_line > self.line or self.end_column >= self.start_column) + ) + + def identity(self): + """What makes two findings the same call rather than two calls. + + Only ever asked of a locatable finding. A position is a call site, and + the message is only description, so two reports of one position are one + finding however they are worded. + + Findings WITHOUT columns are never deduplicated at all, because nothing + about them can distinguish one call from another — two separate calls on + one line usually carry the identical message, so any key built from the + message would merge them, and a single marker would then approve both. + That is the shared-line bypass returning through the back door. Keeping + every occurrence instead can only over-count, which blocks. + + `end_line` is in the key because leaving it out merged two distinct + multi-line regions that happened to share a start and an end column. + Adding a component to this key can only ever SPLIT a group, never merge + one, so it errs in the blocking direction. + """ + return (self.path, self.line, self.end_line, + self.start_column, self.end_column) + + +class Report: + def __init__(self): + self.approved = [] # (Finding, adr) + self.blocking = [] # (Finding, reason) + self.errors = [] # strings, printed as ::error and forcing failure + self.sarif_count = 0 + + @property + def ok(self) -> bool: + return not self.blocking and not self.errors + + +def evaluate(results_dir, repo_root=None) -> Report: + """Decide the build. Pure: it reads, it does not print or exit.""" + repo_root = pathlib.Path(repo_root or pathlib.Path.cwd()) + root = repo_root.resolve() + report = Report() + + paths = sorted(glob.glob(str(pathlib.Path(results_dir) / "*.sarif"))) + report.sarif_count = len(paths) + if not paths: + # A missing SARIF must never read as "clean" — that is precisely the + # false assurance this whole query pack exists to prevent. + report.errors.append( + "no SARIF in " + str(results_dir) + "; the gate did not run" + ) + return report + + findings = {} # locatable, deduplicated by position + loose = [] # columnless, never deduplicated (see Finding.identity) + + attested = False + for path in paths: + with open(path) as handle: + sarif = json.load(handle) + attested = declares_gate(sarif) or attested + for run in sarif.get("runs", []): + for result in run.get("results", []): + if result.get("ruleId") != GATE: + continue + where = result["locations"][0]["physicalLocation"] + region = where.get("region", {}) + uri = where["artifactLocation"]["uri"] + named = source_path(uri) + if named is None: + report.errors.append( + "a gate finding names " + uri + ", which is not a local " + "file, so it cannot be located or exempted" + ) + continue + + # Canonicalise here, before this path becomes a grouping key. + # + # SARIF chooses the spelling, and `Sources/A.swift` and + # `Sources/../Sources/A.swift` are the same file under two of + # them. Grouping on the raw string put two findings on one + # physical line into two groups of one, and a single marker + # approved both — the shared-line rule defeated by punctuation. + located = inside_checkout(repo_root, named) + if located is None: + report.errors.append( + str(named) + " is outside the checkout, so an exemption " + "there could not be trusted" + ) + continue + where_file = str(located.relative_to(root)) + + # Defaulting a missing startLine to 1 was not fail-closed: a + # finding whose location is unknown would be handed line 1, and + # a marker that happened to sit there would approve it. An + # unlocatable finding cannot be exempted at all. + start_line = positive_int(region.get("startLine")) + if start_line is None: + report.errors.append( + "a gate finding in " + where_file + " has no usable " + "startLine, so it cannot be located or exempted" + ) + continue + + finding = Finding( + path=where_file, + line=start_line, + # Validated, not merely present: a malformed column is no + # position at all, so the finding counts as columnless and + # is never deduplicated against another. + start_column=positive_int(region.get("startColumn")), + end_column=positive_int(region.get("endColumn")), + message=result["message"]["text"], + end_line=positive_int(region.get("endLine")), + # Present-but-malformed is not the same as absent. + trusted=("endLine" not in region + or positive_int(region.get("endLine")) is not None), + ) + if finding.locatable: + findings[finding.identity()] = finding + else: + loose.append(finding) + + if not attested: + report.errors.append( + "no SARIF in " + str(results_dir) + " mentions " + GATE + + " — the gate query did not run, and a report with no evidence of " + "it is not a clean one" + ) + + # Group by the unit the exemption mechanism can actually address. A marker + # names a line, not a call, so a line carrying two gate findings cannot be + # approved by one — that was bypass 5: adding a second call beside an + # already-approved one got past the gate. Fail closed instead of inventing + # an approval the author never wrote. + lines: dict[tuple, list[Finding]] = {} + for finding in list(findings.values()) + loose: + lines.setdefault((finding.path, finding.line), []).append(finding) + + source_cache: dict[str, dict[int, str] | None] = {} + + # A trailing marker approves the call it sits beside, and nothing else. The + # line above is offered as a second home for it only when that line is not + # itself flagged — otherwise + # + # _ = try await URLSession.shared.data(from: a) // ...ALLOW...: adr-0007 + # _ = try await URLSession.shared.data(from: b) + # + # would let one exemption cover both calls: the shared-line bypass wearing a + # newline. The standalone-comment-above form is unaffected, because a line + # holding only a comment is never a finding. + flagged = set(lines) + + for (path, line), group in sorted(lines.items()): + if path not in source_cache: + # Already canonical and already known to be inside the checkout — + # both were settled when the finding was read. + candidate = root / path + if candidate.is_file(): + source_cache[path] = line_comments( + candidate.read_text(errors="replace") + ) + else: + source_cache[path] = None + report.errors.append( + "cannot read " + path + + "; an exemption there could not be checked" + ) + comments = source_cache[path] + + adr = None + if comments is not None: + # The flagged line, or the one directly above it. + for candidate_line in (line - 1, line): + if adr: + break + if candidate_line != line and (path, candidate_line) in flagged: + continue + text = comments.get(candidate_line) + if text: + adr = cited_adr(text, repo_root, report) + + if len(group) > 1: + reason = ( + str(len(group)) + " gate findings share this line, so one marker " + "cannot approve them individually — split them across lines and " + "give each its own exemption" + ) + for finding in sorted(group, key=lambda f: (f.start_column or 0)): + report.blocking.append((finding, reason)) + elif adr: + report.approved.append((group[0], adr)) + else: + report.blocking.append((group[0], group[0].message)) + + return report + + +# -------------------------------------------------------------------------- +# Saying it out loud +# -------------------------------------------------------------------------- + +def main(argv=None) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "results", nargs="?", default=".codeql-results", + help="directory holding the SARIF that `analyze` wrote", + ) + parser.add_argument( + "--repo-root", default=".", + help="checkout root that SARIF paths and docs/adr resolve against", + ) + args = parser.parse_args(argv) + + report = evaluate(args.results, args.repo_root) + + for message in report.errors: + print("::error::" + message) + for finding, adr in report.approved: + print( + "::notice file=" + finding.path + ",line=" + str(finding.line) + + "::approved endpoint (" + adr + ")" + ) + for finding, reason in report.blocking: + print( + "::error file=" + finding.path + ",line=" + str(finding.line) + + "::" + reason + ) + + if report.approved: + print( + "\n" + str(len(report.approved)) + + " approved exception(s), re-printed every run so they stay visible." + ) + + if report.blocking: + print( + "\n" + GATE + " fired " + str(len(report.blocking)) + + " time(s) with no approved exception. CLAUDE.md non-negotiable 5 " + "permits page loads, StoreKit, RFC 3161 timestamping and direct-build " + "licence activation, and nothing else." + ) + + if not report.ok: + return 1 + + print(GATE + ": clean across " + str(report.sarif_count) + " SARIF file(s).") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/test_codeql_gate.py b/scripts/tests/test_codeql_gate.py new file mode 100644 index 0000000..0466563 --- /dev/null +++ b/scripts/tests/test_codeql_gate.py @@ -0,0 +1,927 @@ +#!/usr/bin/env python3 +"""Tests for the non-negotiable-5 gate. + +Every defect this exemption mechanism has had was in its enforcement rather +than its design, and every one was found by a human reading the code — because +the code was a heredoc inside a workflow and could not be run any other way. +Each named bypass below has a test here, and the tests run on every push, +independently of whether CodeQL itself can build the project. + +Run: python3 -m unittest discover -s scripts/tests +""" + +import json +import pathlib +import sys +import tempfile +import unittest + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parents[1])) + +import codeql_gate # noqa: E402 + +REPO = pathlib.Path(__file__).resolve().parents[2] + +MARKER = "NETRELISH-ALLOW-ENDPOINT: adr-0007" +CALL = "_ = try await URLSession.shared.data(from: url)" + + +def sarif(*results, declares=True): + """A SARIF run, by default one that attests the gate query ran. + + Real CodeQL reports the queries it ran in tool metadata, and a clean + verdict now requires that evidence — `declares=False` is the report that + contains none, which must never read as clean. + """ + run = {"results": list(results)} + if declares: + run["tool"] = { + "driver": {"name": "CodeQL"}, + "extensions": [{ + "name": "netrelish/swift-rules", + "rules": [{"id": codeql_gate.GATE}], + }], + } + return {"runs": [run]} + + +def result(uri, line, rule=codeql_gate.GATE, message="outbound network call", + start_column=None, end_column=None): + region = {"startLine": line} + if start_column is not None: + region["startColumn"] = start_column + if end_column is not None: + region["endColumn"] = end_column + return { + "ruleId": rule, + "message": {"text": message}, + "locations": [{ + "physicalLocation": { + "artifactLocation": {"uri": uri}, + "region": region, + } + }], + } + + +class Checkout: + """A throwaway checkout: some Swift, some ADRs, some SARIF.""" + + def __init__(self, stack): + self.root = pathlib.Path(stack.enter_context(tempfile.TemporaryDirectory())) + (self.root / "docs" / "adr").mkdir(parents=True) + (self.root / ".codeql-results").mkdir() + self.adr("0007-timestamping.md") + + def adr(self, name): + (self.root / "docs" / "adr" / name).write_text("# ADR\n") + + def swift(self, name, body): + path = self.root / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body) + return path + + def results(self, document, name="swift.sarif"): + (self.root / ".codeql-results" / name).write_text(json.dumps(document)) + + def evaluate(self): + return codeql_gate.evaluate(self.root / ".codeql-results", self.root) + + +class ScannerTests(unittest.TestCase): + """Where a marker is, and is not, a comment.""" + + def comment_on(self, source, line): + return codeql_gate.line_comments(source).get(line) + + def test_whole_line_comment(self): + self.assertIn(MARKER, self.comment_on("// " + MARKER + "\n" + CALL, 1)) + + def test_trailing_comment_after_code(self): + self.assertIn(MARKER, self.comment_on(CALL + " // " + MARKER, 1)) + + def test_doc_comment_counts(self): + self.assertIn(MARKER, self.comment_on("/// " + MARKER, 1)) + + def test_marker_in_string_literal_is_not_a_comment(self): + self.assertIsNone(self.comment_on('let s = "// ' + MARKER + '"', 1)) + + def test_marker_in_block_comment_is_not_a_comment(self): + # Bypass 4. `line.partition("//")` accepted this. + self.assertIsNone(self.comment_on("/* // " + MARKER + " */", 1)) + + def test_marker_in_multi_line_block_comment_is_not_a_comment(self): + source = "/* disabled for now\n // " + MARKER + "\n*/\n" + CALL + self.assertIsNone(self.comment_on(source, 2)) + + def test_nested_block_comment_stays_closed(self): + # Swift block comments nest, so the first `*/` does not end this one. + source = "/* outer /* inner */ // " + MARKER + " */\n" + CALL + self.assertIsNone(self.comment_on(source, 1)) + + def test_code_after_nested_block_comment_is_code_again(self): + source = "/* a /* b */ c */ // " + MARKER + self.assertIn(MARKER, self.comment_on(source, 1)) + + def test_unterminated_block_comment_swallows_the_rest(self): + source = "/* oops\n// " + MARKER + "\n" + CALL + self.assertEqual(codeql_gate.line_comments(source), {}) + + def test_comment_after_a_string_containing_slashes(self): + # The hole the previous implementation documented as unclosable: a `//` + # inside a string literal ahead of the real comment. + source = 'let u = URL(string: "https://example.com")! // ' + MARKER + self.assertIn(MARKER, self.comment_on(source, 1)) + + def test_escaped_quote_does_not_end_the_string(self): + source = 'let s = "a \\" // ' + MARKER + '"' + self.assertIsNone(self.comment_on(source, 1)) + + def test_raw_string_is_not_a_comment(self): + self.assertIsNone(self.comment_on('let s = #"// ' + MARKER + '"#', 1)) + + def test_raw_string_with_two_hashes(self): + self.assertIsNone(self.comment_on('let s = ##"// ' + MARKER + '"##', 1)) + + def test_quote_hash_inside_double_hash_raw_string_does_not_close_it(self): + source = 'let s = ##"a "# b // ' + MARKER + '"##' + self.assertIsNone(self.comment_on(source, 1)) + + def test_multiline_string_is_not_a_comment(self): + source = 'let s = """\n// ' + MARKER + '\n"""\n' + CALL + self.assertIsNone(self.comment_on(source, 2)) + + def test_line_after_multiline_string_is_code_again(self): + source = 'let s = """\nbody\n"""\n// ' + MARKER + self.assertIn(MARKER, self.comment_on(source, 4)) + + def test_marker_in_an_interpolated_string_is_not_a_comment(self): + # Flat state mistook the nested opening quote for the outer closer and + # fell out into "code" mid-literal, recording the marker as a comment. + source = 'let s = "\\("// ' + MARKER + '")"\n' + CALL + self.assertIsNone(self.comment_on(source, 1)) + + def test_marker_in_a_raw_string_interpolation_is_not_a_comment(self): + source = 'let s = #"\\#("// ' + MARKER + '")"#' + self.assertIsNone(self.comment_on(source, 1)) + + def test_marker_in_doubly_nested_interpolation_is_not_a_comment(self): + source = 'let s = "\\("\\("// ' + MARKER + '")")"' + self.assertIsNone(self.comment_on(source, 1)) + + def test_marker_in_a_multiline_string_interpolation_is_not_a_comment(self): + source = 'let s = """\n\\("// ' + MARKER + '")\n"""' + self.assertIsNone(self.comment_on(source, 2)) + + def test_comment_after_an_interpolation_is_a_comment(self): + source = 'let s = "\\(a)" // ' + MARKER + self.assertIn(MARKER, self.comment_on(source, 1)) + + def test_nested_parentheses_inside_an_interpolation(self): + # The interpolation ends at its own matching paren, not the first one. + source = 'let s = "\\(f(g(x)))" // ' + MARKER + self.assertIn(MARKER, self.comment_on(source, 1)) + + def test_escaped_backslash_does_not_open_an_interpolation(self): + source = 'let s = "\\\\(a)" // ' + MARKER + self.assertIn(MARKER, self.comment_on(source, 1)) + + def test_string_opened_inside_an_interpolation_still_hides_a_marker(self): + source = 'let s = "\\(x)" + "// ' + MARKER + '"' + self.assertIsNone(self.comment_on(source, 1)) + + def test_bare_regex_closing_delimiter_does_not_start_a_comment(self): + # In `/\//` the escaped slash and the closing delimiter are textually + # `//`, so the rest of the line was read as a comment. + source = r'let r = /\//; let s = "' + MARKER + '"' + self.assertIsNone(self.comment_on(source, 1)) + + def test_a_real_comment_after_a_bare_regex_still_counts(self): + source = r'let r = /\// // ' + MARKER + self.assertIn(MARKER, self.comment_on(source, 1)) + + def test_escaped_delimiter_does_not_end_an_extended_regex(self): + # `find("/#")` matched the `/` of an escaped `\/` and ended the literal + # early, leaving the rest of the pattern to be read as code. + source = r'let r = #/ a\/# b // ' + MARKER + ' /#' + self.assertIsNone(self.comment_on(source, 1)) + + def test_a_bare_regex_inside_interpolation_keeps_its_punctuation(self): + # `/[)]"/` carries the very characters the scanner steers by: the `)` + # closed the interpolation and the `"` closed the outer string, so the + # marker — which is string content — was recorded as a comment. + source = r'let s = "\(/[)]"/) // ' + MARKER + '"' + self.assertIsNone(self.comment_on(source, 1)) + + def test_a_regex_after_an_expression_keyword_is_opaque(self): + # The character before the slash is a letter, but `return` ends an + # expression context rather than producing a value — so this is a + # regex, and its `)` and `"` must not steer the scanner. + source = r'let s = "\({ return /[)]"/ }()) // ' + MARKER + '"' + self.assertIsNone(self.comment_on(source, 1)) + + def test_a_regex_after_try_is_opaque(self): + source = r'let s = "\({ try /[)]"/ }()) // ' + MARKER + '"' + self.assertIsNone(self.comment_on(source, 1)) + + def test_division_after_a_keyword_is_still_division(self): + # `return a/b` divides: the token before the slash is `a`, not `return`. + self.assertIn(MARKER, self.comment_on("return a/b // " + MARKER, 1)) + + def test_division_after_a_closing_paren(self): + self.assertIn(MARKER, self.comment_on("let x = f(y)/2 // " + MARKER, 1)) + + def test_division_is_not_mistaken_for_a_regex(self): + # The failure in the other direction: reading `a/b` as a regex would + # run it up to the comment's own slashes and lose a REAL marker. + self.assertIn(MARKER, self.comment_on("let x = a/b // " + MARKER, 1)) + + def test_division_between_two_values_on_one_line(self): + self.assertIsNone(self.comment_on("let x = a/b + c/d", 1)) + + def test_a_regex_literal_is_opaque(self): + self.assertIsNone(self.comment_on("let r = /[a-z]+/", 1)) + + def test_a_comment_after_a_regex_literal_still_counts(self): + self.assertIn(MARKER, self.comment_on("let r = /[a-z]+/ // " + MARKER, 1)) + + def test_a_lone_slash_is_not_a_regex(self): + # No closing partner on the line, so it must not swallow the rest. + self.assertIn(MARKER, self.comment_on("let x = f(/ ) // " + MARKER, 1)) + + def test_extended_regex_literal_is_not_a_comment(self): + # Extended regex ignores whitespace, so the marker inside one reads + # exactly like an approval to a scanner that does not know the form. + self.assertIsNone(self.comment_on("let r = #/ // " + MARKER + " /#", 1)) + + def test_extended_regex_literal_with_two_hashes(self): + self.assertIsNone(self.comment_on("let r = ##/ // " + MARKER + " /##", 1)) + + def test_slash_hash_inside_double_hash_regex_does_not_close_it(self): + source = "let r = ##/ a /# b // " + MARKER + " /##" + self.assertIsNone(self.comment_on(source, 1)) + + def test_code_after_a_regex_literal_is_code_again(self): + self.assertIn(MARKER, self.comment_on("let r = #/ a /# // " + MARKER, 1)) + + def test_multiline_regex_literal_is_not_a_comment(self): + source = "let r = #/\n// " + MARKER + "\n/#\n" + CALL + self.assertIsNone(self.comment_on(source, 2)) + + def test_unterminated_regex_literal_swallows_the_rest(self): + source = "let r = #/ oops\n// " + MARKER + "\n" + CALL + self.assertEqual(codeql_gate.line_comments(source), {}) + + def test_hash_directives_do_not_open_a_string(self): + source = "#if DIRECT_BUILD\n#Preview { EmptyView() }\n#endif\n// " + MARKER + self.assertIn(MARKER, self.comment_on(source, 4)) + + def test_line_numbers_survive_multiline_constructs(self): + source = 'let s = """\na\nb\n"""\n/* x\ny */\n// ' + MARKER + self.assertIn(MARKER, self.comment_on(source, 7)) + + def test_first_comment_on_the_line_wins(self): + self.assertEqual(self.comment_on("// a // b", 1), " a // b") + + +class CitationTests(unittest.TestCase): + """A marker is only an exemption if it names exactly one real decision.""" + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.root = pathlib.Path(self._tmp.name) + (self.root / "docs" / "adr").mkdir(parents=True) + self.addCleanup(self._tmp.cleanup) + + def cite(self, comment): + report = codeql_gate.Report() + return codeql_gate.cited_adr(comment, self.root, report), report + + def test_real_adr_is_accepted(self): + (self.root / "docs" / "adr" / "0007-timestamping.md").write_text("#\n") + adr, report = self.cite(" " + MARKER) + self.assertEqual(adr, "adr-0007") + self.assertEqual(report.errors, []) + + def test_absent_adr_is_refused(self): + adr, report = self.cite(" NETRELISH-ALLOW-ENDPOINT: adr-9999") + self.assertIsNone(adr) + self.assertIn("no docs/adr/9999", report.errors[0]) + + def test_ambiguous_adr_number_is_refused(self): + # docs/adr/ has held two different decisions both numbered 0006. + (self.root / "docs" / "adr" / "0006-one.md").write_text("#\n") + (self.root / "docs" / "adr" / "0006-two.md").write_text("#\n") + adr, report = self.cite(" NETRELISH-ALLOW-ENDPOINT: adr-0006") + self.assertIsNone(adr) + self.assertIn("ambiguous", report.errors[0]) + self.assertIn("0006-one.md", report.errors[0]) + self.assertIn("0006-two.md", report.errors[0]) + + def test_a_directory_named_like_an_adr_is_refused(self): + # glob() returns directories too, so `0007-placeholder.md/` would + # satisfy the citation with no written decision inside it. + (self.root / "docs" / "adr" / "0007-placeholder.md").mkdir() + adr, report = self.cite(" " + MARKER) + self.assertIsNone(adr) + self.assertIn("no docs/adr/0007", report.errors[0]) + + def test_a_real_adr_beside_a_directory_of_the_same_number(self): + (self.root / "docs" / "adr" / "0007-placeholder.md").mkdir() + (self.root / "docs" / "adr" / "0007-timestamping.md").write_text("#\n") + adr, report = self.cite(" " + MARKER) + self.assertEqual(adr, "adr-0007") # the directory is not a clash + self.assertEqual(report.errors, []) + + def test_a_symlinked_adr_is_refused(self): + # is_file() follows symlinks, so this would otherwise accept a decision + # that is not in the repository at all. + outside = self.root / "elsewhere.md" + outside.write_text("# not an ADR\n") + (self.root / "docs" / "adr" / "0007-fake.md").symlink_to(outside) + adr, report = self.cite(" " + MARKER) + self.assertIsNone(adr) + self.assertIn("no docs/adr/0007", report.errors[0]) + + def test_a_symlink_beside_a_real_adr_is_not_a_clash(self): + outside = self.root / "elsewhere.md" + outside.write_text("# not an ADR\n") + (self.root / "docs" / "adr" / "0007-fake.md").symlink_to(outside) + (self.root / "docs" / "adr" / "0007-timestamping.md").write_text("#\n") + adr, report = self.cite(" " + MARKER) + self.assertEqual(adr, "adr-0007") + self.assertEqual(report.errors, []) + + def test_an_adr_behind_a_symlinked_parent_is_refused(self): + # Checking only the matched file left its parent unchecked, so + # `docs/adr -> /tmp/decisions` returned ordinary regular files from + # outside the checkout. + import tempfile + with tempfile.TemporaryDirectory() as elsewhere: + (pathlib.Path(elsewhere) / "0007-planted.md").write_text("# not ours\n") + (self.root / "docs" / "adr").rmdir() + (self.root / "docs" / "adr").symlink_to(elsewhere) + adr, report = self.cite(" " + MARKER) + self.assertIsNone(adr) + self.assertIn("no docs/adr/0007", report.errors[0]) + + def test_a_longer_number_is_not_this_citation(self): + # `\d{4}` matched the first four digits of adr-00070 and captured + # adr-0007, so a malformed citation rode in on a real ADR. + (self.root / "docs" / "adr" / "0007-timestamping.md").write_text("#\n") + adr, report = self.cite(" NETRELISH-ALLOW-ENDPOINT: adr-00070") + self.assertIsNone(adr) + self.assertEqual(report.errors, []) # no citation at all, not a bad one + + def test_a_suffixed_citation_is_not_this_citation(self): + (self.root / "docs" / "adr" / "0007-timestamping.md").write_text("#\n") + adr, _ = self.cite(" NETRELISH-ALLOW-ENDPOINT: adr-0007foo") + self.assertIsNone(adr) + + def test_the_documented_citation_forms_still_match(self): + (self.root / "docs" / "adr" / "0007-timestamping.md").write_text("#\n") + for text in (" " + MARKER, + " " + MARKER + " — RFC 3161 timestamping", + " " + MARKER + ", and see below", + " " + MARKER + "."): + with self.subTest(text=text): + self.assertEqual(self.cite(text)[0], "adr-0007") + + def test_comment_without_a_marker_is_not_an_error(self): + adr, report = self.cite(" just an ordinary comment") + self.assertIsNone(adr) + self.assertEqual(report.errors, []) + + +class GateTests(unittest.TestCase): + """End to end: SARIF in, verdict out.""" + + def setUp(self): + import contextlib + self.stack = contextlib.ExitStack() + self.addCleanup(self.stack.close) + self.checkout = Checkout(self.stack) + + def test_missing_sarif_fails(self): + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertIn("the gate did not run", report.errors[0]) + + def test_a_sarif_that_never_ran_the_gate_is_not_clean(self): + # `{"runs": []}` produced no gate results, and "no results" was + # indistinguishable from "no findings". A query that silently stops + # running is how enforcement disappears unnoticed. + self.checkout.results({"runs": []}) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertTrue(any("did not run" in e for e in report.errors)) + + def test_a_run_without_the_gate_rule_is_not_clean(self): + # A real-looking run from an analysis that never loaded this pack. + self.checkout.results(sarif(declares=False)) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertTrue(any("did not run" in e for e in report.errors)) + + def test_a_gate_result_alone_attests_the_query_ran(self): + # Evidence can come from a result as well as from tool metadata. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + self.checkout.results(sarif(result("Sources/A.swift", 2), declares=False)) + self.assertTrue(self.checkout.evaluate().ok) + + def test_the_gate_rule_declared_in_the_driver_also_attests(self): + self.checkout.results({"runs": [{ + "tool": {"driver": {"name": "CodeQL", + "rules": [{"id": codeql_gate.GATE}]}}, + "results": [], + }]}) + self.assertTrue(self.checkout.evaluate().ok) + + def test_attestation_may_come_from_any_sarif_in_the_directory(self): + self.checkout.results({"runs": []}, "a.sarif") + self.checkout.results(sarif(), "b.sarif") + self.assertTrue(self.checkout.evaluate().ok) + + def test_clean_sarif_passes(self): + self.checkout.results(sarif()) + self.assertTrue(self.checkout.evaluate().ok) + + def test_audit_rule_alone_passes(self): + self.checkout.swift("Sources/A.swift", CALL) + self.checkout.results(sarif( + result("Sources/A.swift", 1, rule="netrelish/remote-capable-url-read") + )) + self.assertTrue(self.checkout.evaluate().ok) + + def test_unapproved_gate_hit_blocks(self): + self.checkout.swift("Sources/A.swift", CALL) + self.checkout.results(sarif(result("Sources/A.swift", 1))) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(len(report.blocking), 1) + + def test_marker_on_the_line_above_approves(self): + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + self.checkout.results(sarif(result("Sources/A.swift", 2))) + report = self.checkout.evaluate() + self.assertTrue(report.ok) + self.assertEqual(report.approved[0][1], "adr-0007") + + def test_marker_on_the_same_line_approves(self): + self.checkout.swift("Sources/A.swift", CALL + " // " + MARKER) + self.checkout.results(sarif(result("Sources/A.swift", 1))) + self.assertTrue(self.checkout.evaluate().ok) + + def test_marker_two_lines_above_does_not_approve(self): + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n\n" + CALL) + self.checkout.results(sarif(result("Sources/A.swift", 3))) + self.assertFalse(self.checkout.evaluate().ok) + + def test_block_comment_marker_does_not_approve(self): + # Bypass 4, end to end. + self.checkout.swift("Sources/A.swift", "/* // " + MARKER + " */\n" + CALL) + self.checkout.results(sarif(result("Sources/A.swift", 2))) + self.assertFalse(self.checkout.evaluate().ok) + + def test_string_literal_marker_does_not_approve(self): + self.checkout.swift("Sources/A.swift", 'let s = "' + MARKER + '"\n' + CALL) + self.checkout.results(sarif(result("Sources/A.swift", 2))) + self.assertFalse(self.checkout.evaluate().ok) + + def test_unknown_adr_does_not_approve(self): + self.checkout.swift( + "Sources/A.swift", + "// NETRELISH-ALLOW-ENDPOINT: adr-9999\n" + CALL, + ) + self.checkout.results(sarif(result("Sources/A.swift", 2))) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertTrue(any("9999" in e for e in report.errors)) + + def test_two_findings_on_one_line_fail_closed(self): + # Bypass 5: one valid marker used to approve every finding on the line, + # so a second call beside an approved one got in free. + self.checkout.swift( + "Sources/A.swift", + "// " + MARKER + "\n" + CALL + "; " + CALL, + ) + self.checkout.results(sarif( + result("Sources/A.swift", 2, start_column=5, end_column=40), + result("Sources/A.swift", 2, start_column=60, end_column=95), + )) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(len(report.blocking), 2) + self.assertEqual(report.approved, []) + self.assertIn("share this line", report.blocking[0][1]) + + def test_trailing_marker_does_not_approve_the_next_flagged_line(self): + # The shared-line bypass wearing a newline: a marker beside one call + # must not also serve as the comment "above" the call below it. + self.checkout.swift( + "Sources/A.swift", + CALL + " // " + MARKER + "\n" + CALL, + ) + self.checkout.results(sarif( + result("Sources/A.swift", 1, start_column=5, end_column=46), + result("Sources/A.swift", 2, start_column=5, end_column=46), + )) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(len(report.approved), 1) # line 1, beside its marker + self.assertEqual(len(report.blocking), 1) + self.assertEqual(report.blocking[0][0].line, 2) + + def test_standalone_comment_above_still_approves_a_flagged_line(self): + # The guard above must not break the ordinary form, where the line above + # holds only the comment and is never itself a finding. + self.checkout.swift( + "Sources/A.swift", + "// " + MARKER + "\n" + CALL + "\n// " + MARKER + "\n" + CALL, + ) + self.checkout.results(sarif( + result("Sources/A.swift", 2, start_column=5, end_column=46), + result("Sources/A.swift", 4, start_column=5, end_column=46), + )) + self.assertTrue(self.checkout.evaluate().ok) + + def test_interpolated_string_marker_does_not_approve(self): + self.checkout.swift( + "Sources/A.swift", + 'let s = "\\("// ' + MARKER + '")"\n' + CALL, + ) + self.checkout.results(sarif(result("Sources/A.swift", 2))) + self.assertFalse(self.checkout.evaluate().ok) + + def test_bare_regex_in_interpolation_marker_does_not_approve(self): + self.checkout.swift( + "Sources/A.swift", + r'let s = "\(/[)]"/) // ' + MARKER + '"\n' + CALL, + ) + self.checkout.results(sarif(result("Sources/A.swift", 2))) + self.assertFalse(self.checkout.evaluate().ok) + + def test_regex_literal_marker_does_not_approve(self): + self.checkout.swift( + "Sources/A.swift", + "let r = #/ // " + MARKER + " /#\n" + CALL, + ) + self.checkout.results(sarif(result("Sources/A.swift", 2))) + self.assertFalse(self.checkout.evaluate().ok) + + def test_one_position_reworded_is_still_one_finding(self): + # Same call site, different message text: description, not identity. + # Splitting it would invent a shared line and block a valid exemption. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + self.checkout.results(sarif( + result("Sources/A.swift", 2, start_column=5, end_column=46, + message="outbound network call"), + ), "a.sarif") + self.checkout.results(sarif( + result("Sources/A.swift", 2, start_column=5, end_column=46, + message="outbound network call via URLSession"), + ), "b.sarif") + report = self.checkout.evaluate() + self.assertTrue(report.ok) + self.assertEqual(len(report.approved), 1) + + def test_columnless_findings_on_one_line_still_fail_closed(self): + # With no columns there is nothing to tell two calls apart, so the + # message stays in the key and the shared line blocks. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL + "; " + CALL) + self.checkout.results(sarif( + result("Sources/A.swift", 2, message="call to data(from:)"), + result("Sources/A.swift", 2, message="call to bytes(from:)"), + )) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(len(report.blocking), 2) + + def test_columnless_findings_with_the_same_message_fail_closed(self): + # The common case, and the one a message-based key got wrong: two + # separate calls on one line usually carry identical text, so keying on + # the message merged them and one marker approved both. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL + "; " + CALL) + self.checkout.results(sarif( + result("Sources/A.swift", 2, message="outbound network call"), + result("Sources/A.swift", 2, message="outbound network call"), + )) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(len(report.blocking), 2) + self.assertEqual(report.approved, []) + + def test_a_finding_without_a_start_line_cannot_be_approved(self): + # Defaulting to line 1 let a marker on line 1 approve a finding whose + # location is unknown. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + hit = result("Sources/A.swift", 1) + del hit["locations"][0]["physicalLocation"]["region"]["startLine"] + self.checkout.results(sarif(hit)) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertTrue(any("no usable startLine" in e for e in report.errors)) + + def test_a_finding_with_an_invalid_start_line_cannot_be_approved(self): + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + hit = result("Sources/A.swift", 1) + hit["locations"][0]["physicalLocation"]["region"]["startLine"] = 0 + self.checkout.results(sarif(hit)) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertTrue(any("no usable startLine" in e for e in report.errors)) + + def test_the_same_finding_in_two_sarif_files_is_one_finding(self): + # Deduplication must not turn a duplicate report into a shared line. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + hit = result("Sources/A.swift", 2, start_column=5, end_column=40) + self.checkout.results(sarif(hit), "a.sarif") + self.checkout.results(sarif(hit), "b.sarif") + report = self.checkout.evaluate() + self.assertTrue(report.ok) + self.assertEqual(len(report.approved), 1) + + def test_two_approved_findings_on_separate_lines(self): + self.checkout.swift( + "Sources/A.swift", + "// " + MARKER + "\n" + CALL + "\n// " + MARKER + "\n" + CALL, + ) + self.checkout.results(sarif( + result("Sources/A.swift", 2), + result("Sources/A.swift", 4), + )) + report = self.checkout.evaluate() + self.assertTrue(report.ok) + self.assertEqual(len(report.approved), 2) + + def test_unreadable_source_blocks(self): + self.checkout.results(sarif(result("Sources/Gone.swift", 1))) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertTrue(any("cannot read" in e for e in report.errors)) + + def test_two_spellings_of_one_path_are_one_line(self): + # SARIF chooses the spelling. Grouping on the raw string put two + # findings on one physical line into two groups of one, and a single + # marker approved both. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL + "; " + CALL) + self.checkout.results(sarif( + result("Sources/A.swift", 2, start_column=5, end_column=46), + result("Sources/../Sources/A.swift", 2, start_column=60, end_column=101), + )) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertEqual(len(report.blocking), 2) + self.assertIn("share this line", report.blocking[0][1]) + + def test_a_traversing_uri_that_lands_back_inside_is_canonicalised(self): + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + self.checkout.results(sarif(result("Sources/../Sources/A.swift", 2))) + report = self.checkout.evaluate() + self.assertTrue(report.ok) + self.assertEqual(report.approved[0][0].path, "Sources/A.swift") + + def test_an_unsupported_uri_scheme_is_refused(self): + # `http://evil` fell through to Path and became the relative path + # `http:/evil`, which would have been read had the checkout held one. + planted = self.checkout.root / "http:" / "evil" + planted.parent.mkdir(parents=True, exist_ok=True) + planted.write_text("// " + MARKER + "\n" + CALL) + self.checkout.results(sarif(result("http://evil", 2))) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertTrue(any("not a local file" in e for e in report.errors)) + + def test_a_remote_file_authority_is_refused(self): + # `file://attacker//Sources/A.swift` names a file on another + # host; discarding the authority resolved its path against this + # checkout and approved it from a local marker. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + local = (self.checkout.root / "Sources" / "A.swift").resolve() + self.checkout.results(sarif(result("file://attacker" + str(local), 2))) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertTrue(any("not a local file" in e for e in report.errors)) + + def test_localhost_is_a_local_file_authority(self): + # RFC 8089: an empty authority and `localhost` both mean this machine. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + local = (self.checkout.root / "Sources" / "A.swift").resolve() + self.checkout.results(sarif(result("file://localhost" + str(local), 2))) + self.assertTrue(self.checkout.evaluate().ok) + + def test_an_ordinary_path_containing_a_colon_still_works(self): + # A colon after a slash is not a scheme, so this must not be refused. + self.checkout.swift("Sources/A:B.swift", "// " + MARKER + "\n" + CALL) + self.checkout.results(sarif(result("Sources/A:B.swift", 2))) + self.assertTrue(self.checkout.evaluate().ok) + + def test_distinct_multi_line_regions_do_not_merge(self): + # The identity omitted endLine, so two regions sharing a start and an + # end column collapsed however far apart they ended. + self.checkout.swift( + "Sources/A.swift", + "// " + MARKER + "\n_ = a(b(\n c))\n d))", + ) + hit_a = result("Sources/A.swift", 2, start_column=5, end_column=9) + hit_b = result("Sources/A.swift", 2, start_column=5, end_column=9) + hit_a["locations"][0]["physicalLocation"]["region"]["endLine"] = 3 + hit_b["locations"][0]["physicalLocation"]["region"]["endLine"] = 4 + self.checkout.results(sarif(hit_a, hit_b)) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertEqual(len(report.blocking), 2) + + def test_a_malformed_end_line_does_not_merge_with_a_sound_finding(self): + # Absent and present-but-invalid both became None and defaulted to + # startLine, so a finding with broken coordinates shared an identity + # with a sound single-line one. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL + "; " + CALL) + sound = result("Sources/A.swift", 2, start_column=5, end_column=46) + broken = result("Sources/A.swift", 2, start_column=5, end_column=46) + broken["locations"][0]["physicalLocation"]["region"]["endLine"] = 0 + self.checkout.results(sarif(sound, broken)) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertEqual(len(report.blocking), 2) + + def test_an_absent_end_line_is_still_a_single_line_region(self): + # The distinction must not cost the ordinary case its deduplication. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + hit = result("Sources/A.swift", 2, start_column=5, end_column=46) + self.checkout.results(sarif(hit), "a.sarif") + self.checkout.results(sarif(hit), "b.sarif") + report = self.checkout.evaluate() + self.assertTrue(report.ok) + self.assertEqual(len(report.approved), 1) + + def test_a_multi_line_region_is_still_locatable(self): + # endColumn < startColumn is normal when the region spans lines. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n_ = a(\n b)") + hit = result("Sources/A.swift", 2, start_column=40, end_column=5) + hit["locations"][0]["physicalLocation"]["region"]["endLine"] = 3 + self.checkout.results(sarif(hit)) + self.assertTrue(self.checkout.evaluate().ok) + + def test_an_impossible_column_range_does_not_merge_two_findings(self): + # Each column was validated alone, so startColumn 10 / endColumn 5 + # passed and two findings carrying it collided. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL + "; " + CALL) + self.checkout.results(sarif( + result("Sources/A.swift", 2, start_column=10, end_column=5), + result("Sources/A.swift", 2, start_column=10, end_column=5), + )) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertEqual(len(report.blocking), 2) + + def test_malformed_columns_do_not_merge_two_findings(self): + # locatable accepted any non-None column, so two distinct findings both + # at startColumn 0 shared an identity and collapsed into one. + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL + "; " + CALL) + self.checkout.results(sarif( + result("Sources/A.swift", 2, start_column=0, end_column=0), + result("Sources/A.swift", 2, start_column=0, end_column=0), + )) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertEqual(len(report.blocking), 2) + + def test_absolute_paths_are_judged_against_the_given_root_not_the_cwd(self): + # source_path rebased absolute paths against Path.cwd(), so a + # file:// URI under the current directory was rewritten relative and + # looked up under a DIFFERENT --repo-root. Where a file existed there + # too, its marker approved a finding from the other checkout. + import os, tempfile + with tempfile.TemporaryDirectory() as other: + elsewhere = pathlib.Path(other) + (elsewhere / "Sources").mkdir() + real = elsewhere / "Sources" / "A.swift" + real.write_text(CALL + "\n") # no marker here + + decoy = self.checkout.root / "Sources" / "A.swift" + decoy.parent.mkdir(parents=True, exist_ok=True) + decoy.write_text("// " + MARKER + "\n" + CALL) # marker here + + self.checkout.results(sarif(result(real.as_uri(), 1))) + + cwd = os.getcwd() + os.chdir(elsewhere) + try: + report = self.checkout.evaluate() + finally: + os.chdir(cwd) + + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertTrue(any("outside the checkout" in e for e in report.errors)) + + def test_a_source_outside_the_checkout_is_not_trusted(self): + # A SARIF location naming any file on the runner that happens to carry + # a valid marker used to be read, and approved. + import tempfile + with tempfile.TemporaryDirectory() as elsewhere: + planted = pathlib.Path(elsewhere) / "Evil.swift" + planted.write_text("// " + MARKER + "\n" + CALL) + self.checkout.results(sarif(result(planted.as_uri(), 2))) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertTrue(any("outside the checkout" in e for e in report.errors)) + + def test_a_traversing_relative_uri_is_not_trusted(self): + import tempfile + with tempfile.TemporaryDirectory() as elsewhere: + planted = pathlib.Path(elsewhere) / "Evil.swift" + planted.write_text("// " + MARKER + "\n" + CALL) + hops = pathlib.Path(*([".."] * 12)) / planted.relative_to("/") + self.checkout.results(sarif(result(str(hops), 2))) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertTrue(any("outside the checkout" in e for e in report.errors)) + + def test_a_symlinked_source_pointing_outside_is_not_trusted(self): + import tempfile + with tempfile.TemporaryDirectory() as elsewhere: + planted = pathlib.Path(elsewhere) / "Evil.swift" + planted.write_text("// " + MARKER + "\n" + CALL) + link = self.checkout.root / "Sources" / "A.swift" + link.parent.mkdir(parents=True, exist_ok=True) + link.symlink_to(planted) + self.checkout.results(sarif(result("Sources/A.swift", 2))) + report = self.checkout.evaluate() + self.assertFalse(report.ok) + self.assertEqual(report.approved, []) + self.assertTrue(any("outside the checkout" in e for e in report.errors)) + + def test_file_uri_resolves(self): + path = self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + self.checkout.results(sarif(result(path.as_uri(), 2))) + self.assertTrue(self.checkout.evaluate().ok) + + def test_percent_encoded_uri_resolves(self): + self.checkout.swift("Sources/My App.swift", "// " + MARKER + "\n" + CALL) + self.checkout.results(sarif(result("Sources/My%20App.swift", 2))) + self.assertTrue(self.checkout.evaluate().ok) + + +class ExitCodeTests(unittest.TestCase): + """main() is what CI actually runs.""" + + def setUp(self): + import contextlib + self.stack = contextlib.ExitStack() + self.addCleanup(self.stack.close) + self.checkout = Checkout(self.stack) + + def run_main(self): + return codeql_gate.main([ + str(self.checkout.root / ".codeql-results"), + "--repo-root", str(self.checkout.root), + ]) + + def test_clean_exits_zero(self): + self.checkout.results(sarif()) + self.assertEqual(self.run_main(), 0) + + def test_blocking_exits_one(self): + self.checkout.swift("Sources/A.swift", CALL) + self.checkout.results(sarif(result("Sources/A.swift", 1))) + self.assertEqual(self.run_main(), 1) + + def test_missing_sarif_exits_one(self): + self.assertEqual(self.run_main(), 1) + + def test_approved_exits_zero(self): + self.checkout.swift("Sources/A.swift", "// " + MARKER + "\n" + CALL) + self.checkout.results(sarif(result("Sources/A.swift", 2))) + self.assertEqual(self.run_main(), 0) + + +class RepositoryTests(unittest.TestCase): + """Invariants of this repo that the gate depends on.""" + + def test_adr_numbers_are_unique(self): + seen = {} + for path in sorted((REPO / "docs" / "adr").glob("[0-9][0-9][0-9][0-9]-*.md")): + seen.setdefault(path.name[:4], []).append(path.name) + clashes = {n: f for n, f in seen.items() if len(f) > 1} + self.assertEqual( + clashes, {}, + "Two ADRs share a number, so NETRELISH-ALLOW-ENDPOINT: adr-NNNN " + "cannot name one decision: " + repr(clashes), + ) + + +if __name__ == "__main__": + unittest.main()