You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
CLAUDE.md non-negotiable 5 has been prose since the repo started:
Browsing data never leaves the device. The only network calls are: page loads, StoreKit, RFC 3161 timestamping (Pro provenance), and a one-time license activation for the direct build. Add an endpoint and you've broken this rule.
Prose doesn't stop a commit. This makes it two queries and a gate.
Sources/ is untouched — the diff is .github/ only.
File
What it is
.github/codeql/OutboundNetworkCall.ql
The gate.error, baseline zero.
.github/codeql/RemoteCapableURLRead.ql
The audit.warning, baseline two.
.github/codeql/qlpack.yml
The pack.
.github/workflows/codeql.yml
Wiring, a fail-fast query compile, and a self-enforcing gate step.
⚠️ Read this before anything else: the CodeQL job is red, knowingly
CodeQL cannot currently build this project, and that is not caused by this PR.
CodeQL's tracer injects libtrace.dylib via DYLD_INSERT_LIBRARIES, and the architecture-specific helpers Xcode 27 spawns then die with EBADARCH:
Helper
Purpose
Status
sandbox-exec
SwiftPM manifest loading
worked around
swift-plugin-server
Swift macro expansion
no workaround exists
So @State and #Preview fail to expand inside the KeyboardShortcuts dependency and the traced build dies with exit 65.
It is not a CodeQL version problem — tested, not assumed. Both 2.27.0 and 2.27.1 were run against this failure and both died identically: same four errors, same 14 Bad CPU lines, exit 65. The run that passed on main on 2026-09-23 used this same action SHA and the same CodeQL 2.27.0, both pinned and neither moved — the xcode-27image moved.
Correction, for the record: I earlier asserted "there is no newer bundle, so no pin addresses it". That was false — codeql-bundle-v2.27.1 exists. tools: latest means the default for the pinned action SHA (v4.38.1 → 2.27.0), not the newest published, so that experiment compared 2.27.0 with itself and I misread the result as proof. 2.27.1 has now actually been tested. The conclusion is unchanged; the evidence for it is not.
Build AppStore, Build Direct and Test are green on every commit here. The app is fine; only the traced build is not. The scheduled scan on main fires Monday 06:17 UTC and will fail identically — a pre-existing breakage this PR surfaced rather than created.
Landing anyway, on Ryan's call: nothing needs changing when upstream fixes it — the job simply starts passing. A banner at the top of the workflow says all this so nobody re-diagnoses it from scratch.
The rule it enforces
Not a host allowlist — that was my first instinct and it's the wrong shape. None of the four permitted kinds of traffic is a socket this app opens by hand: page loads go through WKWebView, StoreKit does its own I/O inside the framework, and RFC 3161 and licence activation don't exist yet. So:
First-party code must not perform its own outbound network I/O.
When those two endpoints land, the gate should fire — and force a reviewed, recorded exemption rather than quietly ceasing to apply.
The gate flags the whole URLSession/NSURLSession family (minus teardown, introspection and URLSessionConfiguration), NSURLConnection, the four connection-opening NW* types, seven named CFNetwork constructors, and the BSD socket calls including connectx and sendfile.
The audit takes the genuinely ambiguous Data(contentsOf:) / String(contentsOf:) / URL.resourceBytes family, which is a network call or a local file read depending on a runtime value. Two legitimate baseline hits — defuddle.js from the app bundle, a .webarchive from Application Support — get dismissed once, and any new one is then a new alert.
Splitting them is the point: two permanent false positives on a security rule is how that rule dies.
The gate gates itself
analyze uploads findings but never fails the job, and there is no fail-on input. So the workflow enforces it directly: analyze writes SARIF, and a step fails the build if netrelish/outbound-network-call produced any result. A missing SARIF also fails — a gate that silently passes when it didn't run is the false assurance this whole pack exists to prevent.
Approved exceptions
An endpoint can be approved, but only in writing, next to the code:
Must be in a real // comment, and the cited ADR must exist under docs/adr/. Security-tab dismissal deliberately does not count — it's invisible from the repository. Consequence worth knowing: when RFC 3161 and licence activation land, each needs its ADR merged with the code, or CI blocks. That's CLAUDE.md's "propose it as a one-line ADR" enforced rather than remembered.
Demo — and what is not proven
Verified locally, against codeql/swift-all 6.8.4:
Check
Result
codeql query compile --warnings=error
clean
Same, on the runner's own CodeQL (fail-fast step)
clean — 2.27.0 and 2.27.1
Gate step vs synthetic SARIF
6 cases: gate hit fails, audit-only passes, missing SARIF fails
Exception path
string-literal marker fails, adr-9999 fails, real comment + real ADR passes
SARIF URI resolution
relative, file:///, percent-encoded, and missing-file cases
Not proven: that the queries fire on real Swift. The canary — a deliberate violation, carried through eleven commits of this branch — never got its run, because the traced build never reached analyze. The gate has been tested against synthetic SARIF only.
That is the honest limit of this PR, and it's the thing I most wanted to demonstrate. It stays unproven until CodeQL can build this project again. The canary commits remain in this branch's history; the merged tree has none.
A green CodeQL run later will not, by itself, close this. The canary is gone, so green would only mean "CodeQL can run here". Proving the gate needs the canary put back deliberately, watched, and removed again — see the handoff comment below.
Rule check
Non-negotiable 5 — the first thing in the repo that enforces it rather than describing it. No app behaviour, no endpoint, Sources/ untouched.
Residual limits, stated in the query header rather than left to be discovered: every prefix is a coverage claim that nothing verifies, and review found three holes of exactly that shape on this branch alone (URL.resourceBytes matched as a call when it's a property; BSD sockets missing; CFReadStream* not matched by CFStream%). A clean run means "nothing matched these patterns", never "this app makes no network calls". It also can't see into third-party package internals or the JavaScript injected at capture — Defuddle's isolated world is the mitigation there.
`chore(release): 0.2.0` moved both versions under apps/desktop/ but not the
root lockfile, which still recorded @netrelish/desktop at 0.1.0. The
workspace entry is generated from apps/desktop/package.json, so the two
disagreed from the moment of the bump.
Not a build blocker — run 34007890136 went green with the mismatch in
place, and `npm ci` accepts it — but a lockfile that names a version the
package no longer has is the kind of small lie that costs an afternoon
later. Regenerating by hand rather than by `npm install`: this npm is older
than the one that wrote the file and strips `libc` metadata from ~19
optional dependency entries on contact, which is churn, not a change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UuHYhiqbmF4dT7sq8a6AYm
v0.2.0 was tagged at the bump commit, 4979a46, and built green into a draft
(run 34007890136). Two commits later 02591bf corrected the updater endpoint
from Shepdesign/NetRelish to Shepdesign-LLC/NetRelish — so the artifacts in
that draft check the personal account for updates, and resolve today only
because GitHub 301-redirects the transferred path. The commit that fixed it
calls that unfixable after shipping, and it was written two hours after the
build that shipped it.
Nothing was published, so nothing reached a user. This is the re-cut.
Burning the number rather than moving the tag. v0.2.0 has already produced
signed, notarized artifacts; re-pointing a tag at a different tree and
rebuilding into the same draft means trusting that every asset gets replaced
rather than mixed. A version number costs nothing.
Cargo.toml joins the bump. It has said 0.1.0 since the file was created,
through two releases, because tauri.conf.json's version overrides it — real
but cosmetic, and it stops drifting here. Cargo.lock's netrelish entry and
the root lockfile's apps/desktop entry follow.
RELEASING.md step 1 now names all four files, since naming only two is why
the drift happened, and step 2 says to tag the tip you mean.
Verified: npm ci accepts the lockfile without rewriting it, typecheck clean
across both workspaces, 13/13 tests pass, and the updater endpoint reads
Shepdesign-LLC.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UuHYhiqbmF4dT7sq8a6AYm
The workflow accepts workflow_dispatch, and that is genuinely useful — it is
how notarization was first proven, on 2026-08-19, before any tag existed. But
it passes tagName: github.ref_name, and on a dispatch that is the branch name.
Dispatching on main drafted a release called "NetRelish main" whose tag_name
is `main`; publishing it would have created a git tag literally named main.
Run 34030599628, and the draft was thrown away.
Nothing in the runbook said not to. It does now, along with the distinction
worth keeping: a dispatch build proves the pipeline signs and notarizes, and
is never the build you ship.
Step 4 gains the artifact check as a step rather than as prose buried in the
notarization section. Two of the three green runs this repo has ever had
produced something that should not ship — the other carried the wrong updater
endpoint. Both were green. The only run that was ever verified on the artifact
is the one this file already documents with spctl and stapler output.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UuHYhiqbmF4dT7sq8a6AYm
`p0: retire tauri` (ad0bdff) deleted the Tauri codebase and the release
workflow with it. RELEASING.md and notarization.md were not touched, so for
six days main has carried a 184-line runbook telling you to set
TAURI_SIGNING_PRIVATE_KEY, bump tauri.conf.json and push a tag to fire
release.yml — a workflow that commit removed. Only ci.yml remains.
A doc describing a deleted system is worse than no doc, because it reads as
authoritative. This repo has now been bitten three times by exactly that: the
notarization-401 claim that was never true, the site copy promising "no
account, no cloud" while accounts shipped, and this.
RELEASING.md is rewritten rather than deleted. The Apple facts are about the
account, not the toolchain, and survive intact: Team ID D9QDJ44773, the Apple
ID that is ryanshepherd93@gmail.com and not ryan@shepdesign.com, the
app-specific password, the org transfer that did not carry its secrets.
What it does NOT do is describe the new process, because there isn't one yet
and inventing it is how this file went wrong the first time. It says so, and
names the real gap: the direct build has no update mechanism in the tree —
no Sparkle, no appcast — which wants an ADR when it is decided.
The four release runs are now a table rather than a claim. All four were
green; two shipped something broken. That is the argument for checking the
artifact, and it is worth more than the sentence it replaces — which said
three runs, because I counted before v0.2.1 existed.
notarization.md keeps almost everything: certificates, the app-specific
password, spctl and stapler are Apple's, not Tauri's, and the direct build
still needs all of it. Only the minisign keypair section, the two TAURI_ rows
in the secrets table, and one tauri.conf.json reference are replaced.
v0.2.1 stays published. It resolves, it is signed, and the Swift app's macOS
27 floor means nothing installed from it could upgrade anyway.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UuHYhiqbmF4dT7sq8a6AYm
The direct tier is Post-1.0, so nothing is blocked today. The decision is
written now because it stops being free the moment the first Developer ID
binary reaches anyone: an app can only update itself if the checking code was
compiled into it, so a v1.0 without an updater cannot be reached by v1.1.
That cost is not retroactive and lands on whoever downloads first.
The reason this is an ADR and not a task is §5. It lists every call the app
may make — page loads, StoreKit, timestamping, one-time licence activation —
and closes with "Add an endpoint and you've broken this rule." A Sparkle
appcast is a new endpoint polled on a schedule, so auto-update means amending
a non-negotiable. That is Ryan's call, not an implementation detail, and
CLAUDE.md says to stop and ask rather than pick.
Proposed: no updater. Announce by email to the address that bought the
licence, link the download from About. Every direct user buys a licence and
§5 already permits the activation call beside it, so an address on file is a
property of the tier rather than a hope, and Bento already holds them. What
is lost is silence — the user is told and clicks.
Status stays "proposed". If Ryan wants Sparkle instead, §5 gains a fifth
bullet naming the update check, written deliberately and in the same commit
rather than discovered later.
Status: proposed, not decided.
Ryan's call, 2026-09-23: the direct build ships with no updater. New versions
are announced by email to the address that bought the licence; About links to
the download page. Nothing polls, nothing self-installs, and §5 stands
unamended — which was the point. An appcast is an endpoint, and §5 ends "Add
an endpoint and you've broken this rule."
Status follows ADR 0003's form for a decision Ryan made rather than one
proposed to him.
RELEASING.md moves with it. Its table said "undecided — nothing in the tree"
and its prose called this a decision waiting to be made; both are now false,
and a runbook describing a state that no longer holds is the exact failure
PR #15 existed to fix. It now records the decision, points at the ADR for the
reasoning, and keeps the §5 constraint visible so a future session does not
reach for Sparkle without noticing what that costs.
What does not change: the direct tier is still Post-1.0, so nothing ships
differently before Dec 14. The decision needed making before the first direct
binary, not before launch.
The same gap netrelish-site#4 closed. No alert fired here because CodeQL does
not scan this repository — the exposure is identical, only unobserved. Without
a permissions block the workflow inherits the repository default for
GITHUB_TOKEN, which may be write-all: push, release and issue-write scope
handed to every job on every pull request, including one from a fork.
All three jobs check out, regenerate, build or test. None writes back.
The one that needed thinking about is `test`, which ends with
actions/upload-artifact. That was why this did not ride along with the site's
fix. Reading the pinned action (043fb46, v7.0.1): the upload path uses
ACTIONS_RUNTIME_TOKEN and ACTIONS_RESULTS_URL — the artifact service, a
separate credential from GITHUB_TOKEN — and the only GITHUB_TOKEN strings in
its bundle are JSDoc in a vendored @actions/github helper, not the upload
path. So contents: read should be sufficient.
"Should be" is why this is a pull request and not a push to main. CI running
green here, with the design-kit snapshots uploaded under the narrowed scope,
is the proof; reading a bundle is only a reason to expect it. If `test` fails,
the fix is the specific scope it names, never a widening back to write-all.
Nothing else changes: jobs, steps, matrix, pinned SHAs and triggers untouched.
Nothing has ever scanned the Swift. netrelish-site runs CodeQL and that is why
its missing-permissions alert surfaced; this repository had no scanner, so the
identical gap in its own ci.yml went unreported until it was looked for by
hand.
Advanced setup rather than the dashboard toggle, for a reason worth writing
down. CodeQL analyses Swift by building it, and default setup runs that build
on a GitHub-hosted macos-latest. ci.yml already records why that cannot work:
"xcode-27 is the only image with the macOS 27 SDK; macos-latest is still macOS
26." The OS floor is macOS 27. Default setup would produce a job that fails to
build on every push forever — a permanently red check that reports nothing,
which is worse than no scanner at all.
So this runs on xcode-27 and builds the way the rest of CI builds: the same
project, the App Store scheme, Release-AppStore, signing off because CI holds
no certificate. Not autobuild — this project builds through a named scheme,
and guessing at that is how the default path fails.
Permissions follow #20: contents: read at the top, and security-events: write
on the job alone, because uploading results to the Security tab is the one
thing here that writes. actions: read is deliberately absent — it is only
required on private repositories and this one is public.
Actions pinned to SHAs resolved from upstream, matching the repo's convention:
checkout at the same SHA ci.yml already uses, codeql-action at v4.38.1.
A weekly schedule is included. New queries ship regularly and find old bugs in
code nobody has touched; without it the analysis only ever sees what a push
happens to change.
Whether CodeQL can build this project is not something reading configuration
can answer. This PR's own run is the test.
CLAUDE.md rule 5 — browsing data never leaves the device, and adding an
endpoint breaks the rule — has been prose. This makes it a query.
netrelish/outbound-network-call flags any first-party call that opens a
connection: the whole URLSession family, NSURLConnection, the Network
framework, the CFNetwork free functions, and the quiet ones that read like
file reads — Data(contentsOf:), String(contentsOf:), URL.resourceBytes.
None of the four permitted kinds of traffic is a socket this app opens by
hand: page loads go through WKWebView and StoreKit does its own I/O inside
the framework. So any hit is a fifth endpoint or a bug.
Matched on type-name prefix rather than a method list, because URLSession
grows spellings faster than a list here would be updated, and a rule that
silently stops covering the API it names is worse than no rule.
Verified on Linux against codeql/swift-all 6.8.4 from the same bundle the
workflow pins: compiles with --warnings=error, resolves from the bundle
alone with no pack install step, and the metadata parses into valid
code-scanning fields. Whether it FIRES cannot be checked off-runner — Swift
extraction is macOS-only — so this commit carries two deliberate violations
in Workbench.swift, one per arm of the query. The next commit deletes them.
PR #21 was squash-merged, so origin's copy of this branch holds history that
is already on main by content. Repository rules forbid force-pushing, so it
is merged in rather than replaced. No file changes.
# Conflicts:
# .github/workflows/codeql.yml
The reason will be displayed to describe this comment to others. Learn more.
Correct, and that's the plan — it comes out before merge, exactly as you describe: the earlier commit keeps the alert as evidence, the tree that merges has no canary.
It's deliberately still here right now. Swift extraction is macOS-only, so there's no way to prove off-runner that these queries fire at all; this branch proves it on the runner instead. The removal is the next commit, once the run has shown the alerts.
Updated in 0c4e18e so each line names the query it exercises, since the rule was split in two:
Copilot caught a real defect: Data/String(contentsOf:) matches two
legitimate local file reads that already exist —
Bench/Seal/Extraction.swift:33 defuddle.js, out of the app bundle
Pantry/Store/SnapshotStore.swift:30 a .webarchive, out of App Support
so the baseline was never zero, and the claim in the PR body that removing
the canary leaves no alerts was wrong.
Neither can be proved local from the call site. SnapshotStore's directory
arrives as an init parameter, so its provenance is not visible there at all.
That ambiguity is real and no amount of QL removes it.
Parking two permanent false positives on the rule meant to mean something
when it fires is how a rule dies, so the ambiguous family moves out:
netrelish/outbound-network-call error — the gate. URLSession,
NSURLConnection, NW*, CFNetwork, and now the BSD socket calls Swift
imports from Darwin (also Copilot's, also real). Baseline zero.
netrelish/remote-capable-url-read warning — the audit. contentsOf and
URL.resourceBytes/.lines. Baseline two, both legitimate, dismissed
once with a note; any new hit is then a new alert on a pull request.
resourceBytes and lines are property accesses, never ApplyExpr, so they are
matched as member references now — as listed they could never have fired.
Not taken: the claim that `exists(vars | cond | body)` is a parse error. It
is valid three-part QL, and both queries compile with --warnings=error.
Adding the query pack only uploads findings; the CodeQL analyze action still succeeds when a custom query reports an error. Requiring this status check in branch protection therefore will not block a PR containing an outbound-network alert, contrary to the description's claim that this becomes a merge gate. Configure the analyze step to fail for the intended severity (or add an explicit SARIF-based gate) before relying on branch protection.
Use additive query pack selection
.github/workflows/codeql.yml:39
Because this value lacks the + prefix, CodeQL treats the custom pack as the query selection rather than adding it to the default suite; the workflow therefore does not run the standard CodeQL queries despite the adjacent comment claiming it does. Use the additive form so the built-in suite remains enabled.
Analyze (swift) failed identically on b21096d and 0c4e18e — the build died
before analysis, resolving package dependencies:
posix_spawn error: Bad CPU type in executable (86),
["/usr/bin/sandbox-exec", ...] (EBADARCH, exit 74)
for both the GRDB.swift and KeyboardShortcuts manifests.
It is the tracer, not the packages and not the queries. CodeQL sets
DYLD_INSERT_LIBRARIES to .../arm64/codeql/tools/osx64/libtrace.dylib and
injects it into every spawned process; SwiftPM compiles package manifests
inside sandbox-exec, and that spawn fails. On the same commit, Build
AppStore, Build Direct and Test run the same xcodebuild untraced and pass
in about two minutes.
Two identical failures on two heads, so this is deterministic rather than a
flake, and no job was re-run to "fix" it.
Resolving in a step ahead of codeql-action/init populates SourcePackages/
and SwiftPM's manifest cache outside the tracer, so the traced build has
nothing left to do but compile. The -derivedDataPath matches the build
step's or the checkouts are not found.
My error in 75a6b37. I dropped -scheme from the resolve step to keep it
short, and xcodebuild rejected it in under a second:
xcodebuild: error: The flag -scheme, -testProductsPath, or -xctestrun is
required when specifying -derivedDataPath.
The flag is not optional here: -derivedDataPath is the load-bearing part,
because the traced build must find the same SourcePackages/ checkouts. So
the scheme comes back, matching the build step below it.
Also takes Copilot's note that qlpack.yml still described a "one-query
pack" after the split. The header now says what each query is for and why
they are separate, so the second file does not read as a stray.
Restrict URLSession and NW matching to network I/O APIs
.github/codeql/OutboundNetworkCall.ql:47
This matches every member on any type whose name starts with URLSession, NSURLSession, or NW, not just methods that initiate network I/O. For example, lifecycle/monitoring operations such as URLSession.invalidateAndCancel() or NWPathMonitor.cancel() would be reported as error alerts even though they do not open an outbound connection, undermining the claimed zero-baseline gate as soon as legitimate framework usage is added. Restrict this to concrete request/connection APIs (or separate the broader API-use audit from this gate).
Analyze both App Store and Direct build configurations
.github/workflows/codeql.yml:57
The CodeQL database is built only with Release-AppStore here, while the repository has a separate Release-Direct configuration that defines DIRECT_BUILD (see project.yml:45-48 and ci.yml:60-62). Any network code compiled only for the direct build—including the planned direct licence activation path—will not be extracted or checked, so this does not enforce the rule across both shipped variants. Analyze both configurations (or otherwise include the direct-build sources) before treating this as the repository-wide gate.
Restrict network API matching to request and opening methods
.github/codeql/OutboundNetworkCall.ql:47
Matching only the declaring type makes every method on these types an error, not just methods that perform network I/O. For example, URLSession.getAllTasks()/invalidateAndCancel() and NWPathMonitor.cancel() are lifecycle or inspection calls and would create false positives in the high-severity, zero-baseline gate. Restrict this arm to request/opening methods (or explicitly exclude non-network methods) so ordinary API use does not block merges.
Copilot found both, an hour after I shipped the mechanism and called it
auditable. Both let code past the gate with no real approval.
1. The marker was matched against raw source, not comments. A string literal
containing the token — let s = "NETRELISH-ALLOW-ENDPOINT: adr-0006" — on
the flagged line or the one above counted as an exemption. Now the marker
must appear after a // on the line.
Still not a Swift parser: a // inside a string literal ahead of the marker
would fool it. That is far narrower than matching anywhere in the source,
and closing it properly needs parsed source rather than text. Said in the
code rather than left for someone to find.
2. Only the adr-NNNN SHAPE was checked, never whether the ADR exists, so
adr-9999 passed and the written-decision requirement was decoration. The
step now resolves docs/adr/NNNN-*.md and says so in the log when the
citation is invented.
Also narrows the CF stream matching, which I had got wrong in the other
direction two commits ago. "CFReadStream%" caught
CFReadStreamCreateWithFile, a local file read, and CFStreamCreateBoundPair,
which is in-memory — false positives on the zero-baseline gate, the very
mistake the audit-query split exists to avoid. Now a named list of the
network constructors, with CFSocket/CFHost/CFNetwork left as prefixes
because every member of those is network by definition.
An explicit list is safe here and not for URLSession: CFNetwork is a frozen
C API, so it cannot silently fall behind the way a Swift API list would.
Tested: string-literal marker fails; adr-9999 fails; real comment with a
real ADR passes; trailing comment with a real ADR passes; no marker fails.
Copilot. SARIF gives artifactLocation.uri as a URI, not a path. CodeQL
usually emits a repo-relative one, but file:///... is equally legal, and
pathlib.Path treated the whole URI as a literal relative filename. The
source would then never open, no marker would be found, and a VALID
exemption would have blocked the build.
It failed closed, so it was not a bypass — but closed-and-wrong is still
wrong, and it would have been baffling to debug: the exemption is right
there in the file and CI says there is no approved exception.
Now parses the URI, unquotes percent-encoding, and relativises absolute
paths against the checkout. Annotations use the resolved path too, so they
link in the GitHub UI rather than printing a bare file:// URI.
A source that cannot be read at all now says so explicitly rather than
silently counting as "no exemption":
::error::cannot read X (from SARIF uri Y); an exemption there could not
be checked
Tested: relative uri, file:/// absolute uri, percent-encoded path, and a
uri with no file behind it.
Using CodeQL CLI version 2.27.0 sourced from
https://github.com/github/codeql-action/releases/download/codeql-bundle-v2.27.0/codeql-bundle-osx64.tar.zst
My premise was wrong. I reasoned "CI uses 2.27.0, my local CLI is 2.27.1, so latest gets the newer tracer" — but the codeql-cli-binaries releases and the codeql-action bundle releases are versioned on different cadences. There is no bundle newer than v2.27.0 for the action to fetch. The experiment tested nothing.
Same failure, byte for byte:
error: external macro implementation type 'PreviewsMacros.SwiftUIView' could not be
found for macro 'Preview(_:body:)'; compiler plugin
'.../Xcode_27.app/.../swift-plugin-server' could not be loaded: Bad CPU type
error: external macro implementation type 'SwiftUIMacros.StateMacro' ... 'State()'
error: cannot use mutating member on immutable value: 'self' is immutable ← consequence
error: cannot assign to property: 'self' is immutable ← consequence
Why no CodeQL version can fix this
The decisive fact, which I should have led with:
Yesterday's successful run on main used the same CodeQL 2.27.0 and the same codeql-action SHA. Both are pinned and neither moved. The only thing that changed is the xcode-27 runner image.
So the failing variable is not CodeQL, and no tools: pin — older or newer — can address it. Pinning was the wrong lever and I spent a cycle proving it.
What this is
A genuine incompatibility between CodeQL's tracer and the current xcode-27 image. The tracer injects libtrace.dylib via DYLD_INSERT_LIBRARIES, and the architecture-specific helpers Xcode 27 spawns then fail with EBADARCH:
Helper
Purpose
Status
sandbox-exec
SwiftPM manifest loading
worked around (manifest sandbox disabled)
swift-plugin-server
Swift macro expansion
no workaround available
The second has no equivalent escape hatch. @State and #Preview are SwiftUI macros used throughout KeyboardShortcuts; macro expansion isn't optional, and whether Swift loads a plugin in-process or via the server isn't controllable from xcodebuild.
I said earlier that if flags ran out I'd say so rather than keep trying. They have.
Not caused by, and not limited to, this PR
Build AppStore, Build Direct and Test are green on every commit here. The app builds fine; only the traced build fails. The scheduled scan on main fires Monday 06:17 UTC and will fail identically — that's a pre-existing breakage this PR surfaced rather than created.
Where the work actually stands
Everything except "CodeQL can run here" is finished and verified:
Both queries compile on the runner's own CodeQL
✅ 45s, twice
Gate self-enforces in the workflow
✅ unit-tested, 6 cases
Exception path: comment-scoped, ADR-verified
✅ unit-tested, bypasses closed
SARIF URI resolution
✅ unit-tested, 4 forms
Coverage limits documented in the query header
✅
Canary proven to fire
❌ — blocked by the build, never observed
Putting the options to Ryan rather than choosing for him.
Ryan's call: land the queries and accept the CodeQL job red until the runner
image or CodeQL ships a fix.
Three things.
The canary is gone. Sources/ is now identical to main — the proof commits
remain in this branch's history, the merged tree has no URLSession call.
Note honestly that the canary was never observed firing: the traced build
never reached the analyze step, so the gate has been tested against
synthetic SARIF and never against a real CodeQL run. That is stated in the
PR body rather than glossed.
`tools: latest` is removed rather than replaced with a version. It was an
experiment and it resolved to 2.27.0 — the same version — so it tested
nothing. Removing it is also the right answer to the review finding about a
floating CLI: the action SHA already pins the bundle deterministically, and
`latest` was the only thing making it float. Adding a `tools:` pin would
duplicate that and drift from it.
A banner at the top of the workflow says the job is knowingly red and why,
so nobody re-diagnoses it from scratch on Monday. It records the decisive
fact: the run that passed on main used this same action SHA and the same
CodeQL 2.27.0, both pinned, neither moved — the xcode-27 image moved. The
job needs no change when it is fixed upstream; it simply starts passing.
Unpinned Swift pack dependency undermines reproducibility
.github/codeql/qlpack.yml:27
The * dependency contradicts the pack's reproducibility claim: codeql/swift-all can resolve to a different library release without a repository change, changing the query's semantics even if the action remains pinned. Pin the tested standard-library pack version and update it together with the CLI when revalidating the gate.
Correcting a claim I made as fact. I said "there is no newer bundle, so no
pin addresses this", and used it to conclude the failure was upstream and
unfixable. That was wrong.
codeql-bundle-v2.27.1 exists. It is the default in src/defaults.json on the
codeql-action main branch. What misled me: `tools: latest` does not mean the
newest published bundle, it means the default for the pinned action SHA —
v4.38.1 defaults to 2.27.0, so `latest` returned 2.27.0 and the experiment
compared 2.27.0 against itself. I read that as "no newer bundle exists"
rather than "I tested the same version twice".
So 2.27.1 has never actually been tried against the swift-plugin-server
EBADARCH failure, and tracer fixes for a new Xcode are precisely what a patch
release carries. This points tools: at that bundle explicitly.
It also answers the standing review finding about pinning, though not as
that finding framed it: the CLI was never floating. The action SHA pins the
bundle through defaults.json, so it cannot change without a repository
change. `tools: latest` was the only thing that made it vary, and that is
gone.
If 2.27.1 clears the build, bump the action SHA to a release whose default
is 2.27.1 and drop this line, so the action and its CLI move together.
Written while the context is fresh, so the next session picks this up from the repo rather than from chat.
Open follow-ups, roughly in priority order
1. Prove the canary. This is the one that matters.
The queries have never been observed firing on real Swift — the traced build failed every time, so analyze never ran. The gate is tested against synthetic SARIF only. Once CodeQL can build this project, re-add a deliberate violation, confirm the job goes red at Fail if the gate fired naming the file, then remove it. Until that happens, this pack is unproven, and a green CodeQL run does not prove otherwise.
2. Pin the CodeQL CLI properly.
If 2.27.1 fixed the build, tools: currently points at an explicit bundle URL — a stopgap. Bump the codeql-action SHA to a release whose src/defaults.json says 2.27.1 and drop the URL, so the action and its CLI move together. If 2.27.1 did not fix it, remove the URL entirely: the pinned action SHA already pins the bundle via defaults.json (v4.38.1 → 2.27.0), so nothing was ever floating.
3. qlpack.yml takes codeql/swift-all: "*".
Same criticism I made of the CLI, applied to the pack. It resolves to whatever the bundle ships. Pin it once the CLI is pinned, since the pack version follows the bundle.
4. Make the gate actually gate.
The workflow fails itself on a gate hit, so the Analyze (swift) check goes red. To block a merge on it, that check has to be required in branch protection for main — a repo setting, not a file. I can't write repo settings through this API proxy.
5. Monday 06:17 UTC.
The scheduled scan on main fires weekly. Green if 2.27.1 fixed the tracer, red otherwise — independent of this PR, since main already had the workflow from #21.
Context a fresh session needs
Everything load-bearing is in the repo, not in chat:
.github/workflows/codeql.yml — the banner at the top explains the tracer/swift-plugin-server failure and why it is not a CodeQL version problem.
.github/codeql/OutboundNetworkCall.ql — the header states the coverage limit: every prefix is a claim nothing verifies; three holes of that shape were found by review on this branch alone.
.github/codeql/qlpack.yml — why the gate and the audit are separate files, so nobody "tidies up" by merging them back.
Two things that cost me time today, recorded so they don't cost it twice
Use the per-step jobs API first.list_workflow_jobs gives per-step status in one cheap call and would have told me "the build step failed" immediately. I spent roughly fifteen calls inferring it instead, and inferred it wrong — I was convinced for an hour that analyze was failing.
get_job_logs silently caps tail_lines. CodeQL failures append ~250 lines of extractor diagnostics, so the real error is unreachable by tailing. Request the whole log; it overflows to a file you can grep. One call instead of six blind pages. The blob logs_url is proxy-blocked (403), so don't bother with curl.
Confirmed rather than assumed this time:
Using CodeQL CLI version 2.27.1 sourced from
.../codeql-bundle-v2.27.1/codeql-bundle-osx64.tar.zst
...
compiler plugin '.../swift-plugin-server' could not be loaded: Bad CPU type
** BUILD FAILED ** exit 65
Identical failure to 2.27.0: same four errors, same 14 Bad CPU lines, @State
and #Preview never expand inside the KeyboardShortcuts dependency.
So the earlier conclusion stands, but it now rests on evidence instead of on
my false claim that no newer bundle existed. Two CodeQL versions, same
failure, while the untraced builds stay green on the same commits — the
variable is the xcode-27 image, exactly as the rest of the evidence said.
The explicit tools: URL bought nothing, so it goes. The action SHA already
pins the bundle through its own src/defaults.json, which also means the CLI
was never floating — the review finding that prompted `tools: latest` was
mistaken about that, though right that a floating CLI would be bad.
The banner now records that both versions were tested, so nobody spends a
cycle re-running this experiment.
The SARIF resolver uses only artifactLocation.uri and ignores artifactLocation.uriBaseId plus run.originalUriBaseIds. A legal relative artifact URI that relies on a base will therefore be opened relative to the checkout instead of its resolved location, so a valid inline exemption can be reported as missing and block the job. Resolve the URI against the SARIF base before reading the source.
Copilot posted these as this was merging, so they're on main unfixed. Both are correct, both are real, and both belong at the top of the follow-up list. Adding them here rather than leaving them in a closed review thread.
1. Markers inside block comments are accepted (thread)
line.partition("//") scans whatever follows an embedded //, so this passes:
I documented the string-literal variant of this in the code and missed the block-comment one, which is the easier of the two to hit by accident — a commented-out block containing a marker would silently approve the line below it.
2. One marker approves every finding on the line (thread)
Approval is keyed to the SARIF startLine. Two gate findings on one line means a single valid marker approves both, so adding a second call beside an already-approved one is enough to get past the gate:
The fix is to key approval to the finding rather than the line, or fail closed when more than one gate finding shares a line. Failing closed is the cheaper and safer option and matches the rest of this step's behaviour.
Not exploitable today
The gate never executes: CodeQL cannot build this project on the current xcode-27 image, so analyze is skipped and the SARIF is never produced. These are latent, not live. They must be fixed before the gate is relied upon — which, per the main handoff, is also gated on proving the canary fires at all.
The pattern, since it is now four for four
Every defect in this exemption mechanism has been in the enforcement rather than the design: no approved path at all → markers matched in string literals and unverified ADRs → SARIF URIs treated as filenames → now block comments and shared lines. The idea was right each time; the implementation had a hole each time.
Whoever picks this up: the mechanism deserves a proper test suite before it is trusted, not another round of patches. The synthetic-SARIF harness used during this PR is the right shape — it caught nothing that review didn't, which is itself the lesson.
Exclude non-network CFNetwork APIs from the zero-baseline gate
.github/codeql/OutboundNetworkCall.ql:123
These prefixes still include non-network CFNetwork APIs, so they are not high-precision members of the zero-baseline gate. For example, CFNetworkCopySystemProxySettings/CFNetworkCopyProxiesForURL inspect proxy configuration, and CFSocketInvalidate is teardown; none opens a remote connection, but this predicate reports all of them. Use explicit connection-opening constructors/methods (and similarly narrow CFHost/CFSocket) rather than treating the whole prefixes as network by definition.
)
* ci: close two gate bypasses and test the exemption mechanism
The non-negotiable-5 gate had two bypasses, both found by review as #22 was
merging, both live on main:
1. `/* // NETRELISH-ALLOW-ENDPOINT: adr-0006 */` was accepted, because
`line.partition("//")` scanned whatever followed an embedded `//`. A
commented-out block containing a marker silently approved the line below it.
2. Approval was keyed to the SARIF startLine, so one marker approved every
finding on that line. Adding a call beside an approved one got past the gate.
Both are now closed, and a third hole the previous implementation documented as
unclosable — a `//` inside a string literal ahead of the marker — is closed with
them, because the fix is a real scanner rather than another string match.
A marker now only counts inside genuine line-comment text, decided by scanning
the file once while tracking the three contexts that can hide or reveal a `//`:
nested block comments, string literals, and raw strings of any hash count,
multiline forms included. Unterminated constructs fail closed for free.
Shared lines fail closed: a marker names a line, not a call, so a line carrying
more than one gate finding cannot be approved by one. Findings are deduplicated
on position first, so the same result appearing in two SARIF files is still one
finding rather than a false shared line.
Every defect this mechanism has had — five now — has been in its enforcement
rather than its design, and each was caught by a human reading it, or not caught
at all, because a heredoc inside a workflow cannot be run. So the logic moves to
scripts/codeql_gate.py with 44 tests beside it, and ci.yml gains a Ubuntu job
that runs them on every push — deliberately outside the CodeQL workflow, which
cannot build this project on the current xcode-27 image. A gate whose only proof
of correctness lives inside the job that is currently red is not proven at all.
Also renumbers one of two ADRs that both claimed 0006. The gate resolves a
citation by globbing docs/adr/NNNN-*.md, so `adr-0006` named two unrelated
decisions and could not be cited at all; it now fails closed on ambiguity, and
a test asserts the repo never reintroduces the clash. The later of the two
(direct-build updates, 2026-09-23) becomes 0007; the keychain ADR of 2026-09-21
keeps 0006.
Sources/ is untouched. No app behaviour, no endpoint.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
* ci: close three more gate holes found in review
Copilot found three, all correct, all failing open — which is the direction
that matters. Same pattern as the five before them: the design was right, the
enforcement had a gap.
1. Extended regex literals. `#/ ... /#` at any hash count was not a tracked
context, so `#/ // NETRELISH-ALLOW-ENDPOINT: adr-0007 /#` read as a comment
and approved the line below. Extended regex ignores whitespace, so that
spelling is natural rather than contrived. Now skipped to its matching
delimiter, unterminated included. The bare `/.../` form needs no handling:
two unescaped slashes would end the literal at the first, so `//` cannot
occur inside one.
2. A trailing marker also served as the comment "above" the next line — the
shared-line bypass wearing a newline:
_ = try await URLSession.shared.data(from: a) // ...ALLOW...: adr-0007
_ = try await URLSession.shared.data(from: b)
Both were approved by one exemption. The line above is now offered only when
it is not itself flagged; the standalone-comment-above form is untouched,
since a line holding only a comment is never a finding.
3. The message was part of positional identity, so the same call site reported
with different wording in two SARIF files split into two findings and blocked
a valid exemption. Columns now settle identity when present. When they are
absent the message stays in the key, because nothing else can tell two calls
on a line apart — splitting one finding blocks, merging two approves, and
only one of those errs safely.
Also fixes a reference this branch missed: the table in docs/RELEASING.md still
pointed the direct-build decision at "ADR 0006", which after the renumber is the
unrelated Keychain ADR. The remaining 0006 references in Sources/ and project.yml
are all about identity and are correct as they stand.
Eleven regression tests, one per hole and one per edge the fixes touch. 55 total.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
* ci: model string interpolation in the marker scanner
Sixth hole, same class as the other five, found by review. It fails open.
Swift interpolation returns to CODE inside a string literal, that code can open
another string, and that one can interpolate again. The scanner held string
state in three flat variables, so a nested opening quote was read as the outer
string's closer and the scan fell out into "code" mid-literal:
let s = "\("// NETRELISH-ALLOW-ENDPOINT: adr-0007")"
That recorded a line comment and approved the call below it.
Flags cannot express nesting, so the contexts are now a stack. Every construct
pushes and its terminator pops: strings and raw strings at any hash count,
their multiline forms, extended regex literals, and interpolations — which also
count their own parentheses, so `\(f(g(x)))` ends at the right one rather than
the first. `\(` is tested before the generic escape, since it is a prefix of it.
A `//` is recorded only when the stack is empty. One inside an interpolation
would comment out the closing paren and quote, so it cannot appear in code that
compiles, and code that does not compile is code CodeQL never flagged. Skipping
it there costs nothing and cannot approve anything.
Newline resync now pops only the directly-enclosing single-line string. A
newline inside an interpolation is legal within a multiline literal, and
guessing wrong would resync out of a construct that is genuinely open; not
resyncing merely swallows more of the file, which approves nothing.
Nine regression tests: nested, raw-string (`\#(`), doubly nested and multiline
interpolations; a comment genuinely following one; nested parens; `\\(` not
opening one; a string opened inside an interpolation still hiding a marker; and
the whole thing end to end through the gate. 64 total.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
* ci: three more fail-open paths in the gate, one of them mine
All three from review, all high, all approving a call nobody approved.
1. `docs/adr/NNNN-*.md` was globbed without checking what matched. glob returns
directories, so a directory named `0007-placeholder.md/` satisfied a citation
with no written decision anywhere inside it — an approval backed by nothing.
Matches are now filtered with is_file().
2. A missing SARIF `startLine` defaulted to 1. A finding whose location is
unknown was therefore handed line 1, and a marker that happened to sit there
approved it. An unlocatable finding cannot be exempted at all: it is now a
gate error, which blocks, and no location is guessed. Non-integer and
non-positive values are rejected the same way.
3. The columnless deduplication key included the message, and I defended that on
the review thread with an argument that does not hold. Two separate calls on
one line almost always carry the IDENTICAL message, so the key merged them
into one finding and a single marker approved both — the shared-line bypass
coming back through the door I had just closed, in the commit that closed it.
Findings with columns are still deduplicated by position, because a position
is a call site and the message is only description. Findings without columns
are now never deduplicated, since nothing about them can tell one call from
another. That can only over-count, which blocks.
Five regression tests: a directory matching an ADR citation, and a real ADR
beside one; a missing startLine and an invalid one; and two columnless findings
sharing a line with the same message. Verified the directory case is not vacuous
— glob does return it. 69 total.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
* ci: the gate trusted files that were never in the checkout
Two more from review, both high, both reproduced here before fixing. Both
approve a call using evidence that does not exist in this repository.
1. A symlink satisfied the ADR requirement. `is_file()` follows links, so
`docs/adr/0007-anything.md -> /etc/hosts` was accepted as the written
decision — reproduced, and the citation resolved cleanly. Matches must now
be regular files that actually live here: symlinks are skipped alongside the
directories the previous commit excluded. A symlink beside a real ADR of the
same number is ignored rather than read as an ambiguity.
2. SARIF named the file whose comments decide approval, and 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 carrying a valid marker was approved. Reproduced with a
SARIF pointing at /tmp: the gate printed "approved endpoint (adr-0007)" and
exited 0.
Source paths are now resolved and required to stay inside the checkout.
Resolution follows symlinks, so a tracked source file pointing outside is
refused for the same reason. A path that does not stay inside is an
exemption that could not be checked, which blocks.
Five regression tests: a symlinked ADR alone and beside a real one; a SARIF
location outside the checkout by absolute URI, by `..` traversal, and by a
tracked symlink. 74 total.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
* ci: a regex's own delimiter, a symlinked parent, and two spellings of one path
Round five. Three more fail-open holes, all reproduced before fixing, plus the
two stale counts in my own documentation.
1. The bare `/.../` regex form. I wrote that it needed no handling because "two
unescaped slashes would end the literal at the first, so the sequence `//`
cannot occur inside one" — true, and beside the point. The literal's own
ending supplies the second slash: in `let r = /\//` the escaped slash and
the closing delimiter are textually `//`, and everything after was read as a
comment. `let r = /\//; let s = "NETRELISH-ALLOW-ENDPOINT: adr-0007"` was an
exemption.
An escape in code is now opaque — skip it and whatever it escapes — which
leaves the closing delimiter with nothing to pair with. Regex and division
never have to be told apart, which is not something this scanner should
attempt.
2. The extended-regex terminator used source.find("/#"), which matched the `/`
of an escaped `\/` and ended the literal early, leaving the rest of the
pattern to be read as code. The search is now escape-aware.
3. ADR citations checked the matched file but not its PARENT, so
`docs/adr -> /tmp/decisions` returned perfectly ordinary regular files from
outside the checkout. Resolution now goes through inside_checkout, which
resolves the whole path instead of inspecting its last component.
4. SARIF picks the path spelling, and the raw string was the grouping key. So
`Sources/A.swift` and `Sources/../Sources/A.swift` 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. Locations are canonicalised to
checkout-relative paths as they are read, before anything groups on them.
Also corrects the enforcement history in this module's header and in
codeql.yml, both of which still said five. It is seventeen: five before the
module was extracted, twelve found by review of the branch that extracted it,
every one failing open, one of them introduced by the fix for another. The
header now lists them, because the ratio is the point — reading this code
carefully has never once been sufficient.
Six regression tests: a bare regex's closing delimiter, a real comment after
one, an escaped delimiter inside an extended regex, an ADR behind a symlinked
parent, two spellings of one path sharing a line, and a traversing URI that
lands back inside being canonicalised. 80 total.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
* ci: a citation prefix, the wrong checkout, and columns that were not columns
Round six. Three more fail-open holes, all reproduced. Together they produced
the worst single case yet: two unapproved network calls in one checkout,
approved by a marker in a DIFFERENT checkout, exit 0.
1. The citation pattern was not token-bounded. `\d{4}` matched the first four
digits of `adr-00070` and captured `adr-0007`, so a malformed citation was
accepted whenever ADR 0007 happened to exist. `adr-0007foo` likewise. The
number must now end where the citation ends; the documented forms
(`adr-0007`, and the same followed by an em dash, comma or full stop) are
pinned by a test so the guard cannot quietly reject real markers.
2. source_path rebased absolute paths against Path.cwd(), which is not
necessarily the checkout being judged. With --repo-root elsewhere, a
`file:///<cwd>/Sources/A.swift` URI was rewritten relative and then looked
up under the OTHER root — and where a file existed there too, the gate read
it and approved the finding with a marker from a checkout the finding had
nothing to do with. Absolute paths now stay absolute; canonicalising against
the root actually in use is inside_checkout's job, and it is the only thing
that knows which root that is.
3. `locatable` treated any non-None column as a position, so two distinct
findings both reported at startColumn 0 shared an identity and collapsed
into one, which a single marker approved. Columns are now validated as real
1-based coordinates; a malformed one is no position at all, so the finding
counts as columnless and is never deduplicated. The same validation now
covers startLine, replacing the open-coded check.
Also updates both defect counts: twenty, of which fifteen were found by review
of this branch.
Nine regression tests, including the cross-checkout case driven with the
working directory actually changed, so it exercises the bug rather than a
description of it. 85 total.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
* ci: an unknown URI scheme, and a range that ran backwards
Round seven. Two more fail-open holes, both reproduced.
1. Only `file:` was parsed as a URI; every other scheme fell through to Path as
a relative filename. `http://evil` became `http:/evil`, and had the checkout
contained that path, its markers would have been read and the finding
approved — from a URI that names no source file at all. A scheme this does
not understand is now a gate error.
The guard is narrow on purpose. A relative reference whose FIRST segment
contains a colon is invalid per RFC 3986 and must be written
`./Odd:Name.swift`, so urlparse reading it as a scheme is correct. Ordinary
paths are untouched, because a scheme cannot contain a slash —
`Sources/A:B.swift` still resolves, and there is a test pinning that so this
cannot start silently refusing real files.
2. positive_int validated each column alone, so `startColumn: 10,
endColumn: 5` passed both checks and counted as a position. Two findings
carrying that same impossible range collided in the identity map and one
marker approved both — the same collapse as the previous round, through a
gap the previous round's fix did not cover. A range that runs backwards
carries no position, so it is columnless, and columnless findings are never
merged.
Twenty-two defects now, seventeen of them found by review of this branch.
Three regression tests: an unsupported scheme whose path exists in the
checkout and carries a marker, an ordinary path with a colon after a slash,
and two findings sharing an impossible range. 88 total.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
* ci: a SARIF that ran no gate query was reading as a clean one
Round eight. Two more, both reproduced, and the second is the worst kind this
file can have: it is the exact failure the module's own docstring claims to
prevent.
1. The finding identity omitted endLine, so two distinct multi-line regions
sharing a path, startLine, startColumn and endColumn collapsed into one and
a single marker approved both. endLine is now in the key; adding a component
can only split a group, never merge one, so it errs toward blocking.
Validity had to follow: endColumn < startColumn is NORMAL when a region
spans lines, so the backwards-range check from the previous round now
applies only within a single line. A multi-line region staying locatable has
its own test, because getting that wrong would quietly push every
multi-line finding into the columnless path.
2. A present-but-empty SARIF passed. `{"runs": []}` — or runs from an analysis
that never loaded this pack — produced no gate results, and "no results" was
indistinguishable from "no findings". The gate printed "clean" and exited 0.
This file already refuses to read a MISSING SARIF as clean, and says why in
its header. A report that says nothing is the same false assurance with an
extra step, and a query that silently stops running is precisely how
enforcement disappears without anyone noticing.
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.
CAVEAT, stated because it cannot be tested here: this expectation about
CodeQL's SARIF metadata is unverified against real output, for the same
reason everything else in this PR is — CodeQL cannot build this project. If
CodeQL omits rules that produced no results, this blocks every clean run.
That is loud, immediate and fail-closed, which is the correct direction to
be wrong in; the alternative is the silent pass above.
The test helper now builds SARIF that attests the query ran, because that is
what real CodeQL emits and what a clean verdict now needs. Three of the
existing tests failed against this change, correctly.
Twenty-four defects now, nineteen found by review of this branch. Two of the
nineteen were produced by fixing another.
Seven regression tests. 95 total.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
* ci: a bare regex's punctuation steered the scanner out of its string
Round nine. One hole, reproduced.
A bare `/.../` literal was still ordinary code to the scanner, and its pattern
text can contain the exact characters the scanner steers by:
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, not a comment — was recorded as one
and approved the call below.
Two rounds ago I argued the bare form needed no tracking, because escape
opacity left its closing delimiter with nothing to pair with. That answered the
delimiter, and said nothing about the contents. The literal is now opaque as a
whole.
The interesting part is the other direction. Naively treating every `/` as a
regex opener would read `a/b // NETRELISH-ALLOW-ENDPOINT: adr-0007` as a
literal running up to the comment's own slashes, losing a REAL marker and
blocking a legitimate exemption. So this follows Swift's own rule: `/.../` is a
regex where an EXPRESSION is expected, division after a value — approximated by
the previous significant character, since after an identifier, a number, `)`,
`]` or a closing quote we are past a value. A `/` with no closing partner on
the line is not a regex either, and falls through rather than swallowing the
rest of the line.
Seven regression tests, and most of them guard that second direction rather
than the hole: division with a trailing marker, division twice on one line, a
regex literal alone, a comment after one, and a lone slash. Getting this wrong
the safe way would still be wrong. 102 total.
Twenty-five defects now, twenty found by review of this branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
* ci: a regex after `return`, and a malformed endLine passing as absent
Round ten. Two holes, both reproduced, and both are the previous round's fixes
being correct about what they addressed and silent about the rest.
1. `_regex_may_start` read the previous CHARACTER, so after `return` it saw a
letter, concluded "past a value", and refused to treat `/[)]"/` as a regex.
Its punctuation was live again and steered the scanner out of the string
containing it — hole 23 exactly, one keyword away from where I fixed it.
It now reads the previous TOKEN. An identifier is only a value if it is not
one of the keywords that end an expression context: return, throw, try,
await, case, where, in, is, as and the rest. None of them can be a variable
name, so reading them this way cannot misfire.
`return a/b` still divides, because the token before the slash is `a`, not
`return`. That has a test, along with division after `)`, since widening
what counts as a regex risks swallowing a real marker.
2. A present-but-malformed `endLine` was indistinguishable from an absent one:
positive_int returned None for both and the constructor defaulted both to
startLine. A finding with broken coordinates could therefore share an
identity with a sound single-line one, and a single marker approved both.
Findings now carry whether their coordinates are trusted, and an untrusted
one follows malformed columns into the columnless path, where nothing is
ever merged. An ABSENT endLine still means a single-line region and still
deduplicates — with its own test, because losing that would quietly stop
deduplication working at all.
Twenty-seven defects now, twenty-two found by review of this branch. Four of
those twenty-two were produced by fixing another.
Six regression tests. 108 total.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
* ci: a file:// URI's authority was discarded
Round eleven. One hole, reproduced, and it stood in the same function as the
guard that was supposed to have covered it.
`file://attacker/<checkout>/Sources/A.swift` names a file on another host.
source_path used only `parsed.path` and dropped `netloc`, so that URI resolved
against THIS checkout and was approved by the local marker there.
Two rounds ago I added a scheme check to that same function, for exactly this
class of problem — a URI that does not name a local file being treated as one.
I validated the scheme and never looked at the field beside it. Fifth time in
this PR that a fix has been right about what it addressed and silent about the
rest of its category, and the first where the gap was one line from the fix.
Per RFC 8089 only an empty authority or `localhost` means this machine;
anything else is refused, case-insensitively. `file:///path` and
`file://localhost/path` both still resolve, the latter with its own test,
because tightening this risks refusing legitimate locations and that failure
is silent — the gate simply blocks and the call reads as unapproved.
Twenty-eight defects now, twenty-three found by review of this branch. Five of
those twenty-three were produced by fixing another.
Two regression tests. 110 total.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTRH1R9NLhPzzAYH6Ginhf
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
CLAUDE.mdnon-negotiable 5 has been prose since the repo started:Prose doesn't stop a commit. This makes it two queries and a gate.
Sources/is untouched — the diff is.github/only..github/codeql/OutboundNetworkCall.qlerror, baseline zero..github/codeql/RemoteCapableURLRead.qlwarning, baseline two..github/codeql/qlpack.yml.github/workflows/codeql.ymlCodeQL cannot currently build this project, and that is not caused by this PR.
CodeQL's tracer injects
libtrace.dylibviaDYLD_INSERT_LIBRARIES, and the architecture-specific helpers Xcode 27 spawns then die withEBADARCH:sandbox-execswift-plugin-serverSo
@Stateand#Previewfail to expand inside theKeyboardShortcutsdependency and the traced build dies with exit 65.It is not a CodeQL version problem — tested, not assumed. Both 2.27.0 and 2.27.1 were run against this failure and both died identically: same four errors, same 14
Bad CPUlines, exit 65. The run that passed onmainon 2026-09-23 used this same action SHA and the same CodeQL 2.27.0, both pinned and neither moved — thexcode-27image moved.Correction, for the record: I earlier asserted "there is no newer bundle, so no pin addresses it". That was false —
codeql-bundle-v2.27.1exists.tools: latestmeans the default for the pinned action SHA (v4.38.1 → 2.27.0), not the newest published, so that experiment compared 2.27.0 with itself and I misread the result as proof. 2.27.1 has now actually been tested. The conclusion is unchanged; the evidence for it is not.Build AppStore,Build DirectandTestare green on every commit here. The app is fine; only the traced build is not. The scheduled scan onmainfires Monday 06:17 UTC and will fail identically — a pre-existing breakage this PR surfaced rather than created.Landing anyway, on Ryan's call: nothing needs changing when upstream fixes it — the job simply starts passing. A banner at the top of the workflow says all this so nobody re-diagnoses it from scratch.
The rule it enforces
Not a host allowlist — that was my first instinct and it's the wrong shape. None of the four permitted kinds of traffic is a socket this app opens by hand: page loads go through
WKWebView, StoreKit does its own I/O inside the framework, and RFC 3161 and licence activation don't exist yet. So:When those two endpoints land, the gate should fire — and force a reviewed, recorded exemption rather than quietly ceasing to apply.
The gate flags the whole
URLSession/NSURLSessionfamily (minus teardown, introspection andURLSessionConfiguration),NSURLConnection, the four connection-openingNW*types, seven named CFNetwork constructors, and the BSD socket calls includingconnectxandsendfile.The audit takes the genuinely ambiguous
Data(contentsOf:)/String(contentsOf:)/URL.resourceBytesfamily, which is a network call or a local file read depending on a runtime value. Two legitimate baseline hits —defuddle.jsfrom the app bundle, a.webarchivefrom Application Support — get dismissed once, and any new one is then a new alert.Splitting them is the point: two permanent false positives on a security rule is how that rule dies.
The gate gates itself
analyzeuploads findings but never fails the job, and there is nofail-oninput. So the workflow enforces it directly:analyzewrites SARIF, and a step fails the build ifnetrelish/outbound-network-callproduced any result. A missing SARIF also fails — a gate that silently passes when it didn't run is the false assurance this whole pack exists to prevent.Approved exceptions
An endpoint can be approved, but only in writing, next to the code:
Must be in a real
//comment, and the cited ADR must exist underdocs/adr/. Security-tab dismissal deliberately does not count — it's invisible from the repository. Consequence worth knowing: when RFC 3161 and licence activation land, each needs its ADR merged with the code, or CI blocks. That'sCLAUDE.md's "propose it as a one-line ADR" enforced rather than remembered.Demo — and what is not proven
Verified locally, against
codeql/swift-all6.8.4:codeql query compile --warnings=erroradr-9999fails, real comment + real ADR passesfile:///, percent-encoded, and missing-file casesNot proven: that the queries fire on real Swift. The canary — a deliberate violation, carried through eleven commits of this branch — never got its run, because the traced build never reached
analyze. The gate has been tested against synthetic SARIF only.That is the honest limit of this PR, and it's the thing I most wanted to demonstrate. It stays unproven until CodeQL can build this project again. The canary commits remain in this branch's history; the merged tree has none.
A green CodeQL run later will not, by itself, close this. The canary is gone, so green would only mean "CodeQL can run here". Proving the gate needs the canary put back deliberately, watched, and removed again — see the handoff comment below.
Rule check
Non-negotiable 5 — the first thing in the repo that enforces it rather than describing it. No app behaviour, no endpoint,
Sources/untouched.Residual limits, stated in the query header rather than left to be discovered: every prefix is a coverage claim that nothing verifies, and review found three holes of exactly that shape on this branch alone (
URL.resourceBytesmatched as a call when it's a property; BSD sockets missing;CFReadStream*not matched byCFStream%). A clean run means "nothing matched these patterns", never "this app makes no network calls". It also can't see into third-party package internals or the JavaScript injected at capture — Defuddle's isolated world is the mitigation there.Brand check
None. No UI, no assets, no tokens.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UuHYhiqbmF4dT7sq8a6AYm