Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions devlog/_plan/260817_wave5_execution/090_wave6_closeout.md
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,22 @@ targeting the default branch. Every campaign PR targets `dev`, so none of them c
CodeQL feedback at all — verified: `refs/pull/1959/head` and `refs/pull/1963/head` have analyses
because they target `main`, while the `preview`-targeting pair have zero.

The actual root cause is duller and more useful: **`dev` is scanned on push, and nobody read
the result.** It carries 84 open alerts against `main`'s 71. The finding was observable on the
integration branch from the moment #1897 merged until promotion, and no step in this campaign
ever looked. CodeQL is structurally absent from the `dev` PR flow *and* the post-merge alert
list is not part of anyone's gate — those are two different holes, and I named neither the
first time.
**Second attempt, also wrong.** I then wrote that `dev` is scanned on push and nobody read the
result. Both halves are false: `dev`'s most recent analysis is `02abe0afa` from 2026-08-15,
default setup runs on a *weekly* schedule, and `0be660a2e` is not an ancestor of that commit —
so the code was never in a `dev` scan at all. Its 84 alerts are stale, not current, which is the
opposite of what I claimed they showed. Alert #87's only instances are `refs/heads/main` and
`refs/pull/1959/head`. "Nobody read it" described a page that never displayed it.

**The actual missed signal, third time.** `github-advanced-security[bot]` posted the finding as
an inline review comment on **#1959** at `src/providers/antigravity-models.ts:273` at
**02:38:08Z**. #1963 promoted at **02:55:04Z**. It was sitting on a promotion PR, in the review
thread, for **17 minutes** before the code reached `main` — and I was actively editing that PR's
description during the window. Not a coverage gap. I did not look at the review comments on a
PR I was in the middle of rewriting.

Three explanations for one mistake, the first two of which blamed infrastructure. The third is
the one that is true and the least comfortable.

Severity in context: the input is a configured `baseUrl`, so exploitation needs a hostile or
careless config rather than attacker-controlled traffic. Worth fixing, not urgent. Separately,
Expand All @@ -267,7 +277,7 @@ edge cases (empty string, all-slashes, no trailing slash, interior slashes).

**The root cause is a process one and belongs in the record.** #1897 merged on local focused
tests plus `tsc`. That substitutes for CI on the axis it covers — behavior — and silently skips
the axis it does not: static analysis. Waiting for full CI would have surfaced this before it
the axis it does not: static analysis. ~~Waiting for full CI would have surfaced this before it~~

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove the remaining false CI root-cause account

Striking out only this sentence leaves the next sentences claiming that skipping per-PR CI “traded away exactly this class of finding,” but the corrected investigation above establishes that CodeQL would not have run on #1897 even with full CI because it targeted dev; the actual available signal was the later inline comment on #1959. Rewrite the rest of this paragraph as well so the final campaign record does not retain the disproven process conclusion.

Useful? React with 👍 / 👎.

reached `main`. The instruction for this run was to stop waiting on per-PR CI and gate once at
the end, which is a reasonable trade for speed; the honest accounting is that it traded away
exactly this class of finding, and the end-gate I ran (`bun test`, `typecheck`, `privacy:scan`)
Expand Down
Loading