Docs only. Nothing implemented, nothing dismissed, no scanner or workflow change.

This PR now carries three rounds of work on the same branch: the original CodeQL triage, its correction, and the two items below.

Item 1 — filesystem-gate symlink and TOCTOU residual risks: both closed, one was wrong

The earlier CodeQL entry named two residual risks "for the record, not as proposed work." Both were verified against the scanners' actual behaviour.

The symlink escape does not happen

Filesystem targets are Grype-only (support.py:18), so there is one code path. Four independent lines of evidence:

1. Version identity. grype 0.115.0's embedded Go build info records github.com/anchore/syft v1.46.0 — the pinned Syft version, and the same binary tested standalone. Grype's dir: source is Syft's directory source.
2. Mechanism, at the source. Syft's dir provider defaults base to the scan directory (directory_source_provider.go:60-65), which activates chroot-style re-rooting in addSymlinkToIndex (directory_indexer.go:362-406). Absolute targets become Join(base, Clean(target)); relative ones Join(base, Clean(Join("/", dir, target))). Both collapse under the scan root.
3. Empirically, five variants — none followed. Absolute→dir, relative→dir, absolute→file, relative→file, and →/etc, all planted inside an allowed root. An out-of-root package-lock.json declared lodash 4.17.15; an in-root one declared left-pad 1.3.0. Only left-pad was catalogued. Debug logs show each target re-rooted (escape → <root>/tmp/..., etc_escape → <root>/etc) then dropped as nonexistent.
4. Trivy agrees. trivy fs --scanners secret over the same fixture reported only the real in-root files.

A comment in that code says the opposite. indexAllRoots is introduced with "to cover cases when there is a symlink that references above the root path…" — read alone, that says the escape is real. The re-rooting runs first, so out-of-root targets never become new roots. The comment describes an intent the code no longer implements; behaviour was measured rather than inferred from it.

The one thing worth acting on

Containment rides entirely on Syft's basePath(), which upstream annotates:

// FIXME why is the base always being set instead of left as empty string?

If that FIXME is actioned, base becomes "", re-rooting is skipped, and indexAllRoots would add out-of-root targets as new roots — the escape becomes real silently, on a routine scanner bump, with no change on Scrye's side. A regression test (plant a symlink, assert the out-of-root package is absent) would fail loudly on that bump. Recommended, not implemented.

Hardlinks are followed — and that is fine

Demonstrated: lodash catalogued via /sub/package-lock.json, same inode as the outside file. Not a bypass — a hardlink is a second name for one inode, needs write access inside the root and read access to the source (fs.protected_hardlinks normally blocks otherwise), and cannot cross filesystems.

Methodology note: the first hardlink probe was negative only because the file was named hardlink-lock.json, which the JS cataloger doesn't glob. Renaming to package-lock.json flipped it. A negative from a cataloger probe means nothing unless the filename matches.

Impact ceiling — this class cannot reproduce H1/SEC-1

Grype dir: output carries no file contents: the SBOM's files entries expose digests, id, location, metadata only, and a marker planted in a hardlinked secret file never appeared in output. The worst case is package-inventory and digest disclosure. H1/SEC-1's severity came from trivy repo <local path> surfacing secret values as downloadable output — a strictly worse primitive. Future findings here should be rated against inventory disclosure, not against H1/SEC-1.

TOCTOU: real mechanism, not exploitable

NormalizeRootDirectory calls EvalSymlinks(root) at scan time (chroot_context.go:69-75), so a target swapped between the worker's re-check (inprocess.py:560) and the walk would be followed. Non-exploitable because all of these must hold at once: the feature is enabled (empty default), the attacker has host write access to the target's parent, an operator-role user triggers a scan in that instant, and the race is won — for an inventory disclosure. Closing it would need an O_PATH/fd handle, which grype … dir:<path> cannot express. Accepted; no work proposed.

Item 2 — CodeQL advanced setup

Recommendation: migrate, but after branch protection.

One premise corrected first. :latest is published by a semver tag push, not by the promotion merge (publish.yml, locked §6). So the order is: promotion merges → CodeQL runs on main (~1 min) → tag pushed → publish. Findings normally land before :latest exists. The premise holds for :dev, which dev-nightly.yml builds from a branch CodeQL never analyses. The real justification is simpler: CodeQL never sees a change while it is still reviewable — by the time it runs, a revert rather than a review comment is the remedy.

The suite is reproducible. Confirmed at the source: security-extended is in defaultSuites (codeql-action/src/analyze.ts:361-367) and resolveQuerySuiteAlias() maps it to <language>-security-extended.qls. So queries: security-extended reproduces exactly what runs today. The workflow needs SHA-pinned init/analyze, permissions: narrowed to security-events: write + contents: read + actions: read, a three-entry matrix (python, javascript-typescript, actions), and on: pull_request + push for both dev and main.

CI cost: ≈0 added wall-clock, measured. CodeQL's longest job was 62 s; the current pipeline's longest is 121 s (image build + dogfood), 130 s wall-clock across four jobs. CodeQL runs in parallel and finishes at about half the critical path, so the gate stays bounded by the image build. Public repo → Actions minutes free. Schedule-plus-PR is not better; per-PR feedback is the point and its marginal cost is nil.

Maintenance cost: ≈0 marginal. codeql-action ships ~4 releases/month (40 v4.x tags, 2025-10-07 → 2026-07-30) — but .github/dependabot.yml already groups all github-actions bumps into one weekly PR. It becomes another line in a PR that already exists, not a new stream.

What migrating loses — less than assumed. Managed query-pack updates are not lost: advanced setup pins the action, not the bundle; tools: defaults to the recommended CodeQL bundle, so packs keep updating. Automatic language detection is lost — and that is what silently added the actions language in the first place. Locked §2 fixes the stack, so the risk is small, but the matrix should carry a comment tying it to §2.

The case against (stated because it isn't weak): the empirical yield is 0 true positives in 6 alerts, three of them in test files — this is a bet on future regressions, not a response to demonstrated value. The :latest argument doesn't survive contact with publish.yml. And until branch protection lands, a CodeQL check on a dev PR cannot block a merge, so migrating first buys a check nobody must read. Hence the sequencing.

What changed

• docs/ARCHIVE.md — new §14 entry (2026-08-02) covering both items; index 123 → 124. The superseded residual-risk prose in the earlier CodeQL entry now carries a forward pointer marking it closed and noting that (b) was wrong.
• docs/ROADMAP.md — the CodeQL item's remaining-work note points at the assessment and carries the two premise corrections.

See docs/ARCHIVE.md § Deviations for changes made in this session.

Follow-ups (added after review)

Tracking issue opened — #135

The Syft basePath() regression test is now tracked as #135, in the repo's tracking-issue style: the mechanism, the upstream FIXME reference, the verification method and results table, the inventory-disclosure severity ceiling (recorded explicitly so it is not later mis-rated against H1/SEC-1), the hardlink case marked out of scope, and the positive-control requirement the false-negative during verification taught. The test itself is not implemented.

Item 2 sequencing premise was wrong — corrected

The recommendation said to migrate after the branch-protection item, on the premise that a CodeQL check on a dev PR could not block a merge until then. Reading protect-dev via the API shows that is false:

enforcement: active. So required status checks are already live on dev.

The conclusion survives for a sharper reason. required_status_checks is an explicit allowlist of contexts, not a switch. It names two jobs; neither image job is on it (both run on every PR and are not required), and CodeQL's contexts would not be either. CodeQL would run and be visible without blocking a merge — until its contexts are added to that list.

But that is adding two or three strings to a ruleset that already exists, done alongside the migration. The sequencing dependency is withdrawn, and the corresponding bullet in the "case against" is marked withdrawn rather than deleted.

On the admin bypass: it changes little for the owner — an actor who can bypass can merge past any required check. Required-ness still buys (a) real enforcement for external contributors on a public repo, and (b) an explicit bypass instead of a silent merge. (b) is the one that matters here, because §14 (2026-07-31, flaky cancellation test) already records this project normalizing red checks — "a test that reddens CI intermittently trains everyone to re-run without reading the failure." So a visibly-red-but-optional check is weaker than it looks, and required-ness is worth having even where bypassable.

Not independently confirmed: the rulesets endpoint returns bypass_actors: null and current_user_can_bypass: "never" for both rulesets, but that reflects this session's token identity, not the repository admin's — the bypass list is not readable at this permission level. That part cites the maintainer's statement plus §14's record of dev being deleted despite the deletion rule.

One hazard if CodeQL's contexts are made required: a required context that never reports blocks a PR indefinitely. A job skipped via if: reports skipped and satisfies the requirement (which is why Image — multi-arch build check showing skipped is harmless), but a workflow that never triggers reports nothing. So the CodeQL workflow must not carry paths: filters if its contexts are required.

Corrected recommendation

Migrate whenever convenient, and add the CodeQL contexts to protect-dev's required-checks list in the same change. No sequencing dependency. Treat the :dev coverage hole, not :latest, as the concrete thing being fixed. Still not implemented — the decision is yours.

Also re-scoped

docs/ROADMAP.md's governance checklist listed Branch protection as wholly open. Most of it is done. What actually remains: require_code_owner_review is false on both rulesets (so CODEOWNERS requests but does not require review); nothing restricts who may push tags on main, which matters because a semver tag push triggers publish.yml; and the image build + dogfood jobs are not on either required-checks list, which deserves a deliberate decision.