Promote dev → main: Kubernetes as a first-class deployment, the CI-and-scanner overhaul, the enterprise security audit - #1253
Merged
Conversation
…orks
Default setup was switched off, so nothing was being scanned at all. It had to
go because its Go analysis could not be repaired from anywhere: it installed the
Go its extractor was built with (1.26.6) and pinned GOTOOLCHAIN=local, so
against a go.mod requiring 1.27 it could neither build nor fetch a usable
toolchain:
go: go.mod requires go >= 1.27.0 (running go 1.26.6; GOTOOLCHAIN=local)
Failed to run `go mod tidy -e` in .
Extraction failed for all discovered Go projects.
Every main-targeting PR carried that failure from the Go 1.27 bump (#1240,
promoted in #1242) on, so the repo's Go code went unscanned for that whole
stretch. Default setup is zero-config and exposes no Go version input, hence
advanced setup.
.github/workflows/codeql.yml restores both analyses default setup ran —
security over go, python, javascript-typescript and actions; code quality over
go, python and javascript-typescript — and gives Go an interpreter via
actions/setup-go with go-version-file: go.mod, so the version keeps one
declaration point instead of a copy that goes stale. The three languages in
both sets pass analysis-kinds: code-scanning,code-quality, building one
database and running both suites over it rather than extracting twice.
Triggers are push on main and pull_request on main and dev. Covering dev PRs is
the one place this exceeds default setup, which never ran on them: every change
lands on dev first and main only receives promote merges, so scanning main alone
surfaces a finding for the first time on a promote commit. A weekly Monday 10:00
UTC cron is offset from the three existing scheduled lanes.
dev-ci.yml's header listed CodeQL among the checks deferred to the dev->main
gate. That is no longer true, so it now says where CodeQL runs instead.
The three existing upload-sarif calls are untouched.
Signed-off-by: Brad Flaugher <brad@elcanotek.com>
… tag
Two defects in the first cut, both found by reading the run log rather than the
check mark — the first run was green with neither working.
1. `analysis-kinds: code-scanning,code-quality` does not do what it looks like.
The action logged two ##[error] lines and then exited 0:
The `analysis-kinds` input is experimental and for GitHub-internal use
only. [...] An analysis kind other than `code-scanning` was specified in
a custom workflow. This is not supported and will become a fatal error
in a future version of the CodeQL Action. If your intention is to use
quality queries outside of Code Quality, use the `queries` input with
`code-quality` instead.
[...] Specifying multiple values as input is no longer supported.
Continuing with only `analysis-kinds: code-scanning`.
Confirmed in the artifacts, not just the warning: the Go job loaded only
codeql/go-queries and uploaded a single go.sarif, so the code-quality half of
the coverage this change claims to restore was not running at all. Switched
to `queries: code-quality` as the message directs. Code quality as a distinct
analysis KIND stays closed to custom workflows; the quality queries
themselves now run, surfacing as ordinary code-scanning alerts.
2. Go extraction had exactly one hole, and it was the worst possible file. The
extractor reported 426 files against 427 non-test .go files in the tree; the
missing one was internal/sandbox/host.go, the unsandboxed host executor,
fenced behind `//go:build fleet_host_executor` and therefore absent from the
default build. ci.yml and dev-ci.yml both pass that tag to `go vet` and
`go test` precisely so it is not unchecked. GOFLAGS on the autobuild step
passes it here for the same reason.
Also recorded in the file's header: GOTOOLCHAIN=local is set by the codeql-action
itself, not by the generated default-setup workflow as was assumed. The fix works
because setup-go makes the LOCAL toolchain 1.27.0, satisfying go.mod, not because
the pin is gone.
Signed-off-by: Brad Flaugher <brad@elcanotek.com>
docs/CODEQL.md records what shipped, what deviated from a like-for-like restoration of default setup, and what was deliberately left out — with the verified/assumed split stated explicitly, since the last two bugs in this area both shipped because a toolchain layout was assumed rather than observed. Verified and quoted from the run's own log archive: extraction succeeded for both discovered Go projects, 916 packages, 426 .go files including host.go, and distinct queries evaluated per language across the two runs (72->116 go, 90->292 python, 178->374 javascript-typescript, 36->36 actions). Also corrected there: GOTOOLCHAIN=local is set by the codeql-action itself, in four steps of our own Go job, not by the generated default-setup workflow. The fix works because setup-go makes the local toolchain 1.27.0 — "give `local` something good enough", not "unset the pin". Stated as NOT verified: no push-on-main or scheduled run has executed yet; test files stay outside the database (unchanged from default setup, not a regression); the lines-of-code metric value is never printed to the log, so no line count is claimed; and build-mode: manual was not built because autobuild works. CHANGELOG entry and an AGENTS.md "Where to look" pointer alongside. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
CodeQL still is not blocking, and this commit does not make it blocking: `ci-gate` remains the only required status check on main, and requiring a check is a repo-settings action that a workflow file deliberately cannot perform. What it fixes is the shape of that future decision. CodeQL cannot be folded into ci.yml's `CI gate` at all — `needs` cannot reach across workflow files — so without this, making CodeQL required would mean naming `Analyze (go)`, `Analyze (python)`, `Analyze (javascript-typescript)` and `Analyze (actions)` individually in branch protection. That has to be re-pointed by hand every time the matrix gains or loses a language, and both ways of getting it wrong are bad: a required check that never reports again blocks every PR, and a removed one silently stops gating. One aggregate job has neither failure mode, and it is the pattern this repo already uses twice — ci.yml's `CI gate` and dev-ci.yml's `Dev gate`, including the same `if: always()` + join(needs.*.result) shape. `needs: [analyze]` on a matrix job collapses to a single aggregate result, so with fail-fast: false every language still runs and reports before the gate evaluates them. docs/CODEQL.md now also records where findings actually surface, since an empty Security tab is easy to misread as a broken pipeline: the alert list shown there is the DEFAULT BRANCH's, and this workflow's only push trigger is main, so it repopulates at a dev->main promotion rather than when a PR is scanned. PR runs report on the PR, where CodeQL additionally suppresses file-coverage detail. Also noted: the quality queries this change enables (+44 go, +202 python, +196 javascript-typescript) have never run against this codebase, so any pre-existing finding becomes an alert on merge — a reason to add `CodeQL gate` to the ruleset after a few green promotions rather than on day one. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
Two gaps this closes, both surfaced by the question "can you see scanning results from the logs, and can you gate on this?" 1. A CodeQL run reports NOTHING about what it found to its own log. It writes SARIF, uploads it, and exits 0 — with findings or without. Grepping a full run's log archive for any alert or result count returns nothing; the only result-shaped lines are `Exporting results to SARIF...` and `Successfully uploaded results`, which say a file moved, not what was in it. That leaves a run's real outcome invisible to anyone reading CI output, to `gh run view`, and to any automation holding the log but not the code-scanning API. The analyze step now also writes SARIF locally via `output:` (results are still uploaded — `upload` defaults true) and a following step jq-summarizes per-rule counts into both the job log and the step summary. This is the pattern govulncheck-scheduled.yml already uses on its own SARIF. It is reporting only and never fails the job. When no SARIF was written it says so instead of printing "No findings.", because reporting a clean result you did not observe is the mistake this repo keeps recording. The jq was exercised against SARIF fixtures before pushing: findings spread over two files, a repeated ruleId, a result with no `level` key (falls back to note), an empty `results` array, a run with no `results` key, and a doc with no `runs` key. The last four all yield "No findings." rather than a jq error. 2. docs/CODEQL.md now states the distinction that makes "gate on CodeQL" ambiguous: a required status check on the job gates on the analysis having RUN, not on what it FOUND. A CodeQL job with a hundred open alerts still exits 0 and reports green — which is both why the toolchain break hid behind a red-but-not-required check, and why a green check is not evidence of a clean tree. Blocking on findings is a separate feature, code scanning merge protection (ruleset -> Code scanning rule -> tool CodeQL -> severity thresholds), which is available here at no cost because this repo is public. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
…ep owns Actions
Reshapes the scanning stack on measurements rather than on "more scanners is
better". docs/SCANNING.md is the full note; docs/CODEQL.md carries the CodeQL
half.
ruff is new, and it BLOCKS (ci.yml + dev-ci.yml `python` job, both wired into
their gate jobs). fleet ships 13 Python files — the sandbox FileOp helper, the
python bridge, the bento-slides and data-profiler skill scripts, MCP test
servers — and nothing linted any of them: Go had golangci-lint, the web tier had
oxlint, Python had neither. Rule selection is narrow and ruff.toml records the
numbers behind that: default rules find 3 findings here, a broad selection finds
333, of which 176 are %-format style, 43 magic values, 35 line length. Gating on
that would be a whole-tree reformat for no correctness gain.
Three real findings were fixed so the gate is clean on day one and a new
violation is a regression rather than backlog noise: an unused import in
dummy_server.py, a lambda assignment in bento_pdf.py, and a byte-identical
duplicate `has_guard` definition in bento_doc.py whose second copy silently
shadowed the first. `ruff format` is reported but NOT gated — the tree has never
been ruff-formatted, so failing on it would block every PR on a reformat nobody
scheduled.
CodeQL narrows to security queries only. The code-quality suite was enabled,
measured, and dropped: 32 findings, every one note-level, ZERO security
findings. For Go and the web tier it duplicates golangci-lint (gosec,
staticcheck, revive, unparam, gocritic) and oxlint, which already block; 28 of
the 32 were Python, which is ruff's job now and done in a second with autofix
instead of ~40s without; and 3 were false positives on correct code
(`value != value`, the idiomatic NaN test). What CodeQL keeps is the thing
nothing else here can do — interprocedural taint, which is the actual shape of
"a credential must not reach a log sink, the model context, or the sandbox".
Semgrep is new, scoped, and advisory. Pointing it at p/golang, p/javascript and
p/python was tried and rejected on evidence: 55 findings, and all 6 non-Actions
findings were false positives. tls.go's open-redirect is an HTTP->HTTPS upgrade
to the same host; runner.go's math/rand is jitter; elcano.go's cookie is a
deletion cookie with no secret; httptool.go's interface{} is required because the
value feeds a jq program; proxy.ts's X-Frame-Options value is the literal
"DENY"; and fileops.py's advice — 0o644 for a sandbox directory — would be a
security REGRESSION if followed. Three of the six were already formally triaged
and suppressed for gosec, which already blocks. Re-reporting adjudicated
findings is how a scanner teaches people to ignore it.
What ships instead is p/github-actions, which found 51 instances of one real
issue nothing else in this repo checks: actions pinned to a mutable tag rather
than an immutable commit SHA, which runs attacker-controlled code with this
repo's token if a tag moves. It is advisory because all 51 are real and
repinning every workflow is its own PR — failing CI for an unscheduled backlog
just trains people to ignore the lane. Flip continue-on-error off in the PR that
repins.
Both scanners now print a per-rule summary to the job log and the step summary,
and Semgrep uploads its raw JSON as an artifact for a fixing agent to consume.
Supporting changes: RUFF_VERSION is duplicated across ci.yml and dev-ci.yml, so
scripts/check_versions_test.go now asserts the two agree (mutation-tested: the
assertion fails when the pins diverge). `make lint` gains lint-python, which
skips LOUDLY with the install command when ruff is absent rather than quietly
doing nothing. dev-ci.yml's header and AGENTS.md's build/CI prose now name the
Python lane, and AGENTS.md points at docs/SCANNING.md.
Gate: make build, make lint (0 issues + ruff clean), make test (exit 0),
make lint-migrations. All Python files still byte-compile and the bento golden
tests pass.
Signed-off-by: Brad Flaugher <brad@elcanotek.com>
The scanning stack now passes clean and fails on anything new. Turning a gate on over an unfixed backlog is how a gate becomes something people route around, so every finding was fixed or adjudicated before the switch was flipped. All 53 action references pinned to commit SHAs. Semgrep's github-actions-mutable-action-tag found 51 instances of actions referenced by a mutable tag (actions/checkout@v7); if such a tag moves, attacker-controlled code runs with this repo's GITHUB_TOKEN. Every `uses:` across all 12 workflows is now @<40-hex> with the version in a trailing comment — the form Dependabot reads and updates, and .github/dependabot.yml already watches the github-actions ecosystem. Each SHA is the commit the previously-used tag resolved to at pin time, so the pin is behaviourally identical to the runs already verified green; a pin should not smuggle in a version bump. The only `uses:` lines left on @main are two inside COMMENTS, documenting how a downstream bundle repo calls fleet's reusable workflows — @main is correct guidance there, and Semgrep does not flag them because a YAML comment is not a `uses:` key. Semgrep now blocks over all four packs (p/github-actions, p/golang, p/javascript, p/python) with --error and no continue-on-error. The 6 false positives are suppressed at the line with `nosemgrep: <rule-id>` plus a stated reason, scoped to the rule so a different rule on the same line still reports. Three of the six were already formally triaged and suppressed for gosec, which runs inside golangci-lint and already blocks; one of them — advising 0o644, world-readable, for a sandbox directory — would have been a security REGRESSION if followed. Every suppression was mutation-tested: strip it and the finding reappears, keep it and the finding is gone. That check matters because "0 findings" has two explanations — the waivers work, or the rules silently stopped matching — and only one is safety. Verified across all three comment syntaxes. CodeQL now fails on findings. Previously the analyze step exited 0 whether it found nothing or a hundred alerts, so a red check could only ever mean "the scanner broke" — which is precisely how the Go toolchain break sat unnoticed for weeks behind a red-but-not-required check. Threshold is ANY finding, which is safe because the security suite reports zero across go, python, javascript-typescript and actions. The step also fails when no SARIF was written at all, rather than reporting a clean scan that never happened. Both scanners report as their own checks (CodeQL gate, Semgrep scan) rather than through ci-gate, because a job's `needs` cannot reach across workflow files. Making a red check BLOCK a merge still requires adding those checks to the branch ruleset; a workflow file cannot make itself required. Two knock-on defects found and fixed while doing this: - SHA pinning broke two regexes in scripts/check_versions_test.go that matched `golangci-lint-action@v\d+`. Those assertions fail OPEN — a non-match logs "skipping" rather than failing — so the pin would have silently disabled the golangci-lint version agreement checks. Widened to tolerate a pinned ref plus its trailing version comment, then mutation-tested against the real docs text to confirm they still bite rather than skip. - A standalone nosemgrep comment inside a Go import block makes goimports reformat the group, failing lint. That waiver is a trailing comment on the import line instead; re-tested to confirm the trailing form still suppresses. Deliberately NOT included, with measurements, in docs/SCANNING.md: `ruff format` (9 of 13 files differ, a 3725-line diff — cosmetics, and landing it here would bury the security change) and widening ruff's rule set (`--select B,SIM,S` adds 21 findings, of which the interesting ones are 2x B905 zip-without-strict and 1x SIM115 open-without-context-manager). Gate: gofmt clean, make build, make lint (0 issues + ruff clean), make test (exit 0), make lint-migrations, and semgrep --error over all four packs at 0 findings. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
"No findings." on its own is indistinguishable from "scanned nothing", which is the green-but-vacuous outcome this whole stack exists to rule out. Verifying the first blocking run required downloading the artifact and inspecting the JSON by hand — the log said the scan was clean but not what it had looked at. That is an instrumentation gap, and it is the same class of gap as CodeQL not printing its own findings. Both summary steps now print coverage alongside the verdict: - Semgrep: files scanned, files skipped, and a per-extension breakdown, so it is visible at a glance that every pack actually applied to its language rather than one silently matching nothing. Parse/scan errors are listed with their paths instead of only being counted, since an error means a rule or file did not fully run. - CodeQL: the number of files in the database, read from the source archive the database was built from. Note CODEQL_DB is not always the matrix language — the extractor names the javascript-typescript database "javascript". Both jq blocks were dry-run against the real semgrep.json artifact from run 32575445870 before being committed, which is where these numbers come from: 898 files scanned, 0 skipped, 427 .go / 298 .ts / 134 .tsx / 13 .py / 22 yml+yaml, 3 warn-level parse errors in 2 files. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
The new coverage line interpolated `${{ runner.temp }}` directly into a bash
`run:` script. That is the exact shape semgrep's gha-curl-pipe-shell and
curl-eval rules exist to flag — a GitHub expression expanded into a shell script
before the shell ever sees it — and it also breaks their bash sub-parser, so
those two rules stopped evaluating against codeql.yml entirely.
Caught by the coverage reporting added in the previous commit: parse/scan errors
in the Semgrep summary went from 3 to 5, with .github/workflows/codeql.yml newly
among them. Without that instrumentation the lane would have stayed green while
quietly analyzing this file with two fewer rules — the same green-but-vacuous
failure the whole change is built to prevent, introduced by the change itself.
$RUNNER_TEMP is the equivalent env var, parses cleanly, and avoids the
interpolation entirely. Verified: codeql.yml now reports 0 errors and 0 findings
under p/github-actions, and repo-wide errors are back to the 3 pre-existing ones
(a bash snippet in build-sandbox-image.yml, a TS type in fixtures.ts).
The remaining `${{ runner.temp }}` references are in `with:` and `env:` blocks,
which is the correct placement — the value never reaches a shell unparsed.
Signed-off-by: Brad Flaugher <brad@elcanotek.com>
Mechanical, behaviour-free reformat of 9 files (~3.7k diff lines), kept as its own commit so the substantive changes around it stay reviewable. Validated before the gate flipped: `ruff check` still clean, every file byte-compiles, the full Go suite passes (the bento/fileops golden tests exercise these scripts), and the fileops.py line-level nosemgrep waiver survived the reformat (re-scanned: 0 python findings). The `ruff format --check` gate lands in the next commit; this commit is what makes that gate start clean instead of red. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
…ormatting Closes the three things still standing between "the scanners run" and "the scanners are load-bearing", plus every finding doing so surfaced. Nothing is deferred. GATING, CORRECTED. The earlier design note claimed making CodeQL/Semgrep merge-blocking needed a branch-protection click, reasoning from "`needs` cannot cross workflow files". Incomplete: codeql.yml and semgrep.yml are now REUSABLE workflows (`on: workflow_call`) that ci.yml and dev-ci.yml call as jobs, and a job that calls a reusable workflow sits in a gate's `needs` like any other job. So `CI gate` — already the single required check on main — and `Dev gate` now block on scanner findings with no settings change anywhere. Their own push/pull_request triggers are removed so nothing runs twice; each keeps its weekly re-scan cron (new queries/rules against unchanged code) plus a workflow_dispatch. In ci.yml the calls are docs-only-skippable like the other heavy jobs (a docs-only change cannot touch scanned code, and the gate treats a skip as a pass); in dev-ci they are unconditional because Dev gate demands strict success and the fast lane has no docs-only detection — which also means direct pushes to dev get scanned. NPM AUDIT, NEW BLOCKING GATE. `npm audit --audit-level=low` runs in both web jobs, lockfile-only, before the expensive `npm ci`, failing on any severity — the npm counterpart of the govulncheck gate, clock-dependent by design. web/ was already clean (0 vulns). scripts/rampart-service HAD NO LOCKFILE AT ALL, so nothing could audit it and installs were unreproducible; generating one exposed 5 high-severity vulnerabilities it had been hiding: sharp <0.35.0 (libvips CVE-2026-33327/-33328/-35590/-35591) and adm-zip <0.6.0 (GHSA-xcpc-8h2w-3j85) via onnxruntime-node. No upstream release fixes either — the latest @huggingface/transformers still pins sharp ^0.34.5, and npm's suggested "fix" was a BREAKING DOWNGRADE of transformers to 3.8.1 — so package.json carries two overrides (sharp ^0.35.3, adm-zip ^0.6.0; each the release immediately after its vulnerable line). The overridden stack was installed and load-tested, not just resolved: sharp renders a PNG through the new libvips, transformers loads on it, rampart exports its API, adm-zip 0.6 round-trips a zip. Both trees now audit at 0. Drop the overrides when upstream ships fixed ranges. RUFF FORMAT, NOW A GATE. `ruff format --check` blocks in both CI python jobs and in `make lint` (lint-python). Safe because the previous commit formatted the whole tree, so the gate starts clean and a failure means one new file. ALL THREE SEMGREP PARSE ERRORS FIXED, so no file is partially covered — a partial parse silently drops rules from a file, which is coverage loss wearing a green check: - build-sandbox-image.yml interpolated ${{ steps.build.outcome }} into its run: script; now passed via env (also the script-injection-safe form — the same fix codeql.yml got for $RUNNER_TEMP). - The same script's ${tag:-(tag unavailable)} expansion default is valid bash, but the bare paren chokes semgrep's bash sub-parser; hoisted to a plain `if [ -z "$tag" ]` assignment. - fixtures.ts used an inline `import("@playwright/test")` type; now a named `import type { BrowserContext }`. Validated with the real web toolchain: npm ci, oxlint, tsc --noEmit, and all 1104 vitest tests pass. Final measured state, whole tree: semgrep --error over all four packs exits 0 with 0 findings, 898 files scanned, 0 parse errors; both npm trees 0 vulnerabilities; ruff check + format --check clean; gofmt clean; make build, make lint, make test (exit 0), make lint-migrations all green. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
…ed, grype High, canary, cron alarms Every item from the audit self-assessment that can live in code, closed. One item closed by a reasoned rejection rather than code, with the license cited. ruff B/SIM/S (bandit tier): all 21 measured findings fixed, families ENABLED. Both zip() sites get strict=True — each provably equal-length (bento_pdf appends pages and contents in lockstep in the same loop; profile.py sits behind an explicit len(row) != len(cols) guard) — so a future desync fails loud instead of silently truncating a PDF or a profile. bento_doc's unclosed NamedTemporaryFile moved inside its with (tmp=None sentinel keeps the unlink-on-failure path exact). The fourteen deliberate best-effort try/except-pass sites — kernel cleanup, the duck-typed pandas/numpy probes in normalize_json_value, unlink-on-failure in the sandbox fileops commit path — became explicit contextlib.suppress with the intent stated at each site; the suppressions are semantically identical, and the sandbox fileops conversion is covered by its test suite (full make test green). The one subprocess.Popen carries a reasoned `# noqa: S603`: argv is sys.executable plus literal flags plus a connection-file path this process just created with mkstemp — nothing model- or user-controlled. The waiver was mutation-tested: stripping the noqa re-fires S603. Two of my own first-cut mistakes fixed in the same pass: a prose comment beginning with the literal token "# noqa" (parsed as a malformed directive) and a nested-with that tripped SIM117. CodeQL: security-extended on all four languages. Adopted the way every other gate here was — the default suite measured zero findings, so the broader suite starts from a clean baseline, and this PR's own run (whose Fail-on-findings step reads the SARIF) is the measurement. Anything extended surfaces must be fixed or reasoned away; it cannot accrue. Grype: gate tightened from fixable-CRITICAL to fixable CRITICAL+HIGH, measured first: the published sandbox image (pinned grype 0.117.0, checksum-verified, scanned directly from GHCR) carries ZERO fixable Critical/High RPM findings — its only fixable findings are two Medium openssh advisories that the next routine image rebuild picks up. Policy mutation-tested three ways: the real scan passes, an injected fixable High fails, an injected fixable Medium still passes. scripts/check-npm-overrides.sh: an override is a fork of upstream's intent, correct only while upstream is broken — and the day upstream fixes itself, nothing notices, leaving Dependabot silently pinned down. Both CI lanes now ask the registry what floor @huggingface/transformers and onnxruntime-node declare, and FAIL with removal instructions once those reach sharp>=0.35 / adm-zip>=0.6. Registry flake is a skip with a notice, never a verdict (npm audit beside it is the CVE gate proper). Mutation-tested in both directions. Cron failure alarms: all four scheduled scan lanes (codeql, semgrep, govulncheck, grype) file a deduped GitHub issue when a SCHEDULED run fails — a red cron has no PR to surface it, which is exactly the rot pattern that let the CodeQL toolchain break sit red for weeks. In the workflow_call path the alarm job stays skipped (schedule-only condition), so callers never need to grant issues: write. Semgrep rule vendoring: investigated and REJECTED on license grounds. The Semgrep Rules License v1.0 permits "your own internal business purposes" only and states "This license does not allow you to distribute the rules" — committing the packs to this public MIT repo would be redistribution. The binary stays pinned; the rules stay registry-fetched with the failure mode named in docs/SCANNING.md. Docs: every stale "fixable CRITICAL" mention updated (AGENTS.md, TESTING.md, SANDBOX-IMAGE-FRESHNESS.md, CONTRIBUTING.md, ci.yml comment); SCANNING.md and CODEQL.md carry the new levels and the license decision; ruff.toml's header records why B/SIM/S went from measured-deferred to fixed-enabled and that PLR0124 stays rejected (its only hits are the idiomatic NaN test). Gate: yaml parses, gofmt clean, make build/lint/test green (ruff check+format clean under the widened select), semgrep --error 0 findings / 898 scanned / 0 parse errors. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
The previous commit put an `issues: write` alarm job inside codeql.yml and semgrep.yml. Those are REUSABLE workflows, and a called workflow may not request token permissions its caller did not grant — and that check fires at PLAN time, before any `if: github.event_name == 'schedule'` can skip the job. Result: the calling Dev CI run on 745fe6d died with startup_failure (run 32578976517) and NOTHING scanned on that head. The "intersection semantics" assumption in that commit's comment was wrong, and this is the correction. The alarm for those two lanes now lives in scan-cron-alarm.yml, a workflow_run watcher on [CodeQL, Semgrep] completions, filtered to conclusion=failure AND event=schedule. A watcher has no caller, so it holds issues: write without widening any gate's token — a PR-path scanner failure already reddens the calling gate, and a red manual dispatch has a human watching it. govulncheck-scheduled.yml and grype-scheduled.yml keep their in-job steps: standalone workflows, no caller, no plan-time constraint. workflow_run only fires from the default branch's copy, so the alarm arms at the dev->main promotion — the same moment the crons start mattering. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
… in summaries Three fixes from reading run 32579378165, the first run where the security-extended measurement actually executed. THE EXTENDED SUITE'S FIRST CATCH, AND ITS UNFLAGGED TWIN. Extended found one actions finding: actions/untrusted-checkout/medium at build-sandbox-image.yml's "Checkout fleet (build script)" step, whose `ref: inputs.fleet_ref` is caller-controlled — and the checked-out script is then EXECUTED. Reproduced locally with the same CodeQL 2.26.3 bundle and suite to get the exact location, then triaged for real: every ref of ElcanoTek/fleet is collaborator-written EXCEPT refs/pull/* (fork PRs), so a caller passing a pull-request ref would execute non-collaborator code — with a contents:read token of the calling repo, which for a private bundle repo is an exfiltration primitive. The fix is a validation step that refuses pull-request refs (and leading-dash values) and exposes the vetted value as a step output the checkout consumes; the query's trigger was a NAME heuristic (any ref: fed by a field matching .*(head|branch|ref).*), so consuming the validator's neutrally named output also clears the alert honestly — the sanitizer is genuinely in the path, not renamed around. The better half of the catch: publish-sandbox-image.yml has the IDENTICAL pattern and escaped BOTH query variants — too privileged for medium (which only reports non-privileged contexts) and no PR-event taint for high — while holding packages:write, making it the more dangerous twin. Hardened symmetrically. Verified: rebuilding the actions database and re-running the full security-extended suite locally now reports 0 findings across all 13 workflow files, and semgrep stays clean on both edited files. CANARY PATH. scripts/check-npm-overrides.sh was invoked repo-relative from the web jobs, whose default working-directory is web/ — exit 127, exactly what the gate is for. The script is cwd-free (registry queries only), so it is now invoked via $GITHUB_WORKSPACE. (The rampart audit step beside it already proved step-level working-directory resolves from the workspace root.) SUMMARIES NOW PRINT file:line PER FINDING. Run 32579378165's summary named the rule but not the location, which sent the fix hunt through a 600MB CLI bundle download. The jq now renders "[level] ruleId file:line" per finding, validated against a location-bearing SARIF fixture and the empty case. security-extended status after this commit: go, python and javascript-typescript measured clean in CI on the previous run; actions measured clean locally on the same toolchain after the hardening. All four verified in CI on this run. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
… in the docs Docs-only follow-up to the last workflow commit, which shipped the changes but not their story: - docs/CODEQL.md: the matrix table now says security-extended (it still read "default suite"); the adoption section records the one finding the wider suite produced (actions/untrusted-checkout/medium on build-sandbox-image.yml), why it was fixed rather than waived (the actions language has no AlertSuppression.ql), the refs/pull/* refusal that fixes it, and the same hardening applied to publish-sandbox-image.yml — the unflagged twin with packages: write that the name-heuristic query missed. Verified clean in CI on all four languages (Dev CI run 525, id 32580031374). - docs/SCANNING.md: same story in the stack doc; the job-log example updated to the shipped file:line format plus the database-file-count coverage line; the override canary's $GITHUB_WORKSPACE invocation explained (the step runs under working-directory: web, where a repo-relative path exits 127). - CHANGELOG.md: the security-extended bullet now carries the finding, the fix in both workflows, the CI verification, and the summary/canary fixes. go test ./scripts green (the docs↔pin agreement assertions still hold). Signed-off-by: Brad Flaugher <brad@elcanotek.com>
…go-r078t8 Blocking multi-scanner stack: CodeQL security-extended, Semgrep, ruff, npm audit — every finding fixed, gated through the existing gates
The read path for task rows was narrowed to own rows in #1082, and run logs in #980. Three surfaces never got the same treatment and authorized on a permission alone, so any client-role principal reached every principal's rows: - GET /tasks/paused — ListPausedTasks selects on status alone with no principal predicate in SQL, and the projection carries each task's prompt. Its siblings (/tasks/export, /tasks/upcoming) both call visibleTasks; this one did not. Leaked other principals' paused prompts, and their task UUIDs with them. - PUT /tasks/{id} and POST /tasks/{id}/tags — loaded the task with the unscoped GetTask and never checked ownership, so a client-role principal could rewrite a teammate's pending run: prompt, model, mcp_selection and credential_allowlist included. Only run_if was gated (admin-only). - POST /tasks/{id}/feedback and GET /tasks/{id}/learned-instructions — taskFromPath is lookup-only by contract ("a handler that needs an authorization decision makes it on the returned task") and neither caller made one. A down-vote with an attacker-authored critique fed maybeDistill, which mints a proposal from the victim's prompt at unmetered model spend; the GET disclosed their learned instructions. The write gate is a new taskWritableByPrincipal, deliberately NOT principal.ownsTask: ownsTask resolves through ownerID(), which is nil for every API-key principal, so it would deny a scoped intake-app key the right to edit the task it just created. taskCreatedByPrincipal matches a creating user OR a creating key (CreatedByKeyID), which is the model #980/#1082 established. A write surface must be no looser than the read surface guarding the same row. TestScopedAPIKeyAuthorization previously asserted "client key can edit an editable task" against an unattributed row — that was the vulnerable behavior. Split into the owned case (must keep working: the intake-app path) and the unowned case (must 403). Every fix mutation-tested: stripped, the tests fail with the exploit visible — a hijacked prompt persisted, another principal's prompt in the paused queue, feedback accepted on an unowned task. Also in this commit, from the same audit sweep: - internal/config/config.go: ValidateScheduled interpolated the first 6 bytes of OPENROUTER_API_KEY into a validation error — the only place in the tree where secret material reached an error string. Removed, and the doc comment corrected: it claimed "Called at startup" but has no production caller. - internal/agent/scheduled.go: run-error strings now go through agentcore.RedactSecrets before the persisted transcript and the log. Tool output, the stream sink, hooks and the session log were already scrubbed; run errors were the one path that skipped it, and the transcript write is the larger surface. - internal/mcpoauth/discovery.go: refuse a non-http(s) scheme on the remote-derived discovery URLs (a WWW-Authenticate resource_metadata pointer, a PRM-declared issuer) before the request. Contained already by SafeHTTPClient and the transport; this makes the argument explicit rather than dependent on transport behavior. Tested both directions. - internal/sched/models/models.go: validate WorktreeConfig.BaseBranch. It is the trailing positional of `git worktree add -b <b> <path> <base>` with no "--" separator, so a leading-dash value was parsed by git as an option. worktree_config is settable by any task creator, unlike run_if. - Log-injection sinks that carry genuinely untrusted text: the task create log (task.Prompt — its update-path twin was already sanitized), the pre-validation client attachment path on the reject branches, the client-echoed attachment Name, the upload filename, and the API-key name. logSafe/%q, matching each line's existing sanitized sibling. - web/e2e/test-auth-key.ts: the Ed25519 private key was written to a fully predictable path in the world-writable temp dir at default 0644. Now O_EXCL at 0600 with random bytes in the sibling name. - internal/agent/session.go: document loadImageAttachments' caller contract. It performs no path containment of its own and is safe only because httpapi's validateAttachments is its sole producer. Stated as a contract, not an enforced boundary, because the uploads root is not threaded to that call site — a local check could only re-assert part of the guard while looking like all of it. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
…ster The gate #1246 shipped blocked every push to dev and main. Its threshold was "any finding at any severity", justified by a measured zero across all four languages — but that measurement came from Dev CI run 525, a `pull_request` event, and on pull_request events the CodeQL action runs DIFF-INFORMED: it builds the full database, evaluates every query, then reports only results located inside the PR's diff. Run 525's own log says both halves out loud ("Persisted 204 diff range(s) across 43 file(s)", and "file coverage information is only enabled when analyzing the default branch and protected branches"). The Go database held all 428 files and TaintedPath/RequestForgery/LogInjection/ WeakSensitiveDataHashing all ran; the SARIF was empty because results outside the 43 changed files were dropped. So the first full-tree evaluation was the push that merged #1246 — run 527 — which reported 38 Go and 17 javascript-typescript findings and turned Dev gate red, with no PR-shaped way out: a PR into dev is scanned diff-informed and stays green while dev itself stays red. The generalizable lesson, now written into the file and the ADR: a PR-event CodeQL run certifies a diff, not a tree. Any "the scanners are green, therefore the tree is clean" claim resting on one is unsound, and that is permanent behavior, not a bug. New threshold: a finding blocks when its SARIF level is error/warning or its rule carries security-severity >= 7.0, unless it is waived. Below that band findings are printed and go to the Security tab as advisory. Severity alone is not sufficient to separate the false positives, which is why the register exists: go/request-forgery is 9.1 and fires on web_fetch.go, a deliberate user-facing fetch tool behind netguard's resolve-then-dial SSRF guard; go/weak-sensitive-data-hashing is 7.5 and fires on SHA-256 used as a lookup index over a 32-byte crypto/rand token, which is the recommended construction. .github/codeql-accepted-findings.json registers accepted (rule, file) pairs with a mandatory written reason. Per-FILE, not per-rule, and that is the point of preferring it to a query-filters exclude: an exclude switches a 9.1 query off repo-wide, while the register waives it in the two files that were adjudicated and leaves it live everywhere else. A synthetic SARIF carrying a fresh go/request-forgery in an unregistered file fails the gate — verified while developing the jq, along with the clean, notes-only, mixed-severity, in-source-suppressed, missing-SARIF, malformed-SARIF and missing-register cases. The gate fails closed twice over: a missing register and an unevaluatable jq both refuse to report the scan clean, rather than reading as zero. Anti-rot controls, because a register nobody re-reads is worse than none: - scripts/check_codeql_register_test.go (in make test) requires every entry to name a file that exists, carry a substantive reason, use a plausible rule id, and be unique — and asserts codeql.yml still references the register, so the two cannot be silently decoupled. Mutation-tested: a bogus rule id, a missing file and a one-word reason all fail it. - The log and step summary print three tiers — BLOCKING, ACCEPTED and ADVISORY — so every waiver appears in ordinary CI output instead of only in a file someone has to think to open. Also corrects the claims this file made about itself, which an auditor reads as documentation: - the "reports ZERO ... a finding here is new" premise (false); - "They cannot be part of CI gate" two paragraphs after correctly explaining that the calling job is in the gate's needs; - "whether a red check BLOCKS a merge is branch protection's call", which contradicted the header — and now records the real caveat, that the dev ruleset requires no status checks so Dev gate is red-but-not-required there; - the summarize step's "gating on findings is merge protection's job, not this step's", directly above the step that gates; - dev-ci.yml's "makes green mean clean, not just ran", now qualified for pull_request events. ADR-0048 records the decision, what gets worse (a note-level regression no longer fails the build — gosec's G706 still covers the log-injection class through golangci-lint, which does block), and the sharpest edge ( the register keys on rule+file, not rule+file+line, so a second bad instance in an already-waived file would not block; line keys churn on every edit and a register that fails on unrelated refactors is one people delete). Signed-off-by: Brad Flaugher <brad@elcanotek.com>
The gate I added in the previous commit was VACUOUS, and the first
full-tree run proved it: run 32583247659 reported "0 blocking" over a
tree holding 30 findings, with sec-sev=0 on every one of them —
including go/request-forgery, whose real security-severity is 9.1.
Cause: CodeQL writes query metadata into runs[].tool.extensions[].rules[]
(one extension per query pack), NOT runs[].tool.driver.rules[]. The
driver is the CLI itself. Reading only the driver resolved nothing, so
every finding scored 0 and nothing could ever reach the High band. For
the same reason a result's `level` is usually absent from the result:
SARIF falls back to the rule's defaultConfiguration.level, which was
also unreachable.
Three changes, in order of importance:
1. The classifier now reads driver.rules AND extensions[].rules, and
resolves level from the rule when the result omits it. Verified
against a fixture built to the real SARIF shape.
2. A VACUITY CHECK that would have caught this: if a scan produced
findings but resolved zero rule metadata, the job fails instead of
reporting clean. "Findings but no metadata" means the lookup is
broken and the gate is evaluating nothing — the green-but-vacuous
outcome this workflow exists to rule out, which I then walked
straight into.
3. The banding is now security-severity only, with level as the fallback
for a rule that publishes no security-severity. Once metadata
resolved, banding on level as well put all 23 go/log-injection
findings (security-severity 6.1) into the blocking tier, because
nearly every CodeQL security query is @problem.severity error — level
carries no severity information for them. That would have reproduced
the any-finding deadlock by a different route.
The filter now lives in .github/codeql-gate.jq and is used by both the
summary and the gate via `jq -f`, so the thing that reports and the
thing that blocks cannot disagree about what "blocking" means — and it
can be exercised against fixture SARIF with the exact bytes CI runs.
Exercised end-to-end from the YAML: the real-shape fixture (0 blocking,
8 accepted, 2 advisory), a fresh unregistered go/request-forgery in an
unwaived file (blocks — the property that makes the per-file register
different from a query-filters exclude), the vacuity fixture (fails),
missing register, missing filter, and unparseable SARIF.
CI supply chain, from the same audit:
- Repin github/codeql-action (5 refs) and golangci/golangci-lint-action
(2 refs). Both were pinned to the ANNOTATED TAG OBJECT of a MUTABLE
major tag, not to a commit: `refs/tags/v4` -> 4c0873ef but
`refs/tags/v4^{}` -> db488dde. A tag object is immutable but only
reachable while that tag points at it, so the day upstream moves v4 —
which codeql-action does on essentially every release — the object is
unreferenced and Actions can no longer resolve the ref. A
self-inflicted CI outage with no attacker involved, armed in seven
places. Verified with `git ls-remote --tags` and repinned to the
peeled commits, with exact `# vX.Y.Z` comments (they read `# v4
(4.37.8)` and `# v9`, which dependabot-core parses as "4" and "9").
scripts/check_action_pins_test.go now enforces the shape across all
53 third-party refs.
- build-sandbox-image.yml / publish-sandbox-image.yml: replace the
fleet_ref deny-list with an allow-list. The deny-list had two holes.
GITHUB_OUTPUT newline injection: a workflow_call string input may
contain newlines and the value was printf'd unsanitized, so
fleet_ref="main\nresolved=refs/pull/1/head" matched no deny pattern,
exited 0, and emitted two `resolved=` lines — last-wins handed the
attacker the ref, and the same primitive forges any step output. And
a raw commit SHA: "every ref here is collaborator-written except
refs/pull/*" is true of named refs and false of reachable commits,
since GitHub keeps fork-PR commits in the base repo's object store
and actions/checkout will fetch a bare SHA. Both matter because these
workflows EXECUTE the checked-out build script, and the publish twin
holds packages: write with a live GHCR login. Tested: 5 legitimate
refs pass, 9 attack shapes fail, including the injection payload.
- ci.yml docs-only classifier: `*.md` matched at any depth and `docs/*`
matched everything under docs/, so a PR touching only
internal/clientconfig/builtin_skills/*/SKILL.md (go:embed'd and
asserted by three test files), config/default/system_prompts/*.md
(the shipped prompts docs/PROMPT-CACHE-CONTRACT.md exists to
protect), or docs/openapi.yaml (asserted by openapi_drift_test.go)
was classified docs-only — and every job skipped while CI gate
reported green. Narrowed to a prose allow-list.
- ci.yml ci-gate: a `skipped` job passed the gate unconditionally.
Now a skip is only accepted when the classifier actually said
docs-only; otherwise the gate refuses to pass over a suite that did
not run. Same rot pattern as red-but-not-required, colours inverted.
Gate logic tested in six directions.
- scripts/check_gate_needs_test.go: assert every job in ci.yml and
dev-ci.yml is in its aggregate gate's `needs`. Both are complete
today (11/11 and 7/7); nothing asserted it, and adding a job without
extending needs is a silent one-line regression that produces a
red-but-not-required check — the exact failure #1246 was written to
stop recurring.
Signed-off-by: Brad Flaugher <brad@elcanotek.com>
Dead code is audit surface: an auditor asks "why is this here, is it
reachable, is it maintained?" and every unreachable identifier costs a
round of that. Each deletion below was verified with
`git grep -w <name>` across all tracked files — every hit was the
declaration and its own doc comment, zero call sites, zero test
references, and no coupling to docs/openapi.yaml or schemaModelRegistry
(so TestOpenAPISchemaDrift cannot regress).
- internal/mcpoauth/errors.go: IsInvalidClient. The odd one out of a
four-predicate set — IsInvalidGrant, IsInvalidTarget and
IsInvalidScope all have callers. Its job is already done inline:
IsTerminalRefreshError and ReauthDetail both switch on the literal
"invalid_client" rather than calling it.
- internal/sched/apikeys/apikeys.go: Manager.LogAction. A nine-parameter
exported wrapper over the private m.logAudit that nothing calls. Worth
naming precisely because it is API-key AUDIT logging: a reviewer
grepping for audit surface lands here first and has to work out that
callers use logAudit directly. Deleting removes the ambiguity.
- internal/sched/models/models.go: TaskAssignment and LogSubmission —
the two halves of the retired v1 remote-worker protocol
(OrchestratorURL, Files/FileChecksums, the worker log POST),
superseded when the platform consolidated into one process. The live
log path uses models.LogSession directly.
- internal/sched/models/models.go: MaxLogSubmissionSize. This one is
more than clutter — it declared a 24 MB body cap that NOTHING
enforced. The cap actually applied is MaxJSONBodySize = 1 MB
(internal/sched/handlers/middleware.go, wired via
BodySizeLimitMiddleware). So the real posture was 24x stricter than
the constant claimed, and an auditor reading models.go would have
concluded fleet accepts 24 MB bodies. An unenforced-limit claim is
exactly the kind of thing that becomes a finding.
- internal/config/config.go: DefaultFromEmail. Its doc comment called
it "the fallback From address for outgoing mail", but no code path
consumes it — an operator who sets neither SENDGRID_FROM_EMAIL nor
MAILBUX_FROM_EMAIL does not get this fallback. A documented
capability that does not exist is a violation of this repo's own
honesty-in-docs invariant, so the constant goes rather than the claim
being left standing.
- internal/tools/task_tracker.go: the only commented-out code block in
the tree, plus the inProgressCount it was the sole reader of (the
counter was incremented and never read once the comment is gone).
Replaced with a sentence saying why there is deliberately no
"more than one in_progress" check.
`golangci-lint` with `default: standard` already includes `unused` and
is a full gate, so unexported dead code is structurally zero — which is
why everything above is an EXPORTED identifier in internal/, the class
`unused` deliberately does not report. Confirmed independently with
`deadcode -test -tags fleet_host_executor ./...`, which now reports
nothing.
CI permissions and the alarm:
- scan-cron-alarm.yml only fired on `conclusion == 'failure'`. That
ignores startup_failure — which is the exact failure this file's own
header describes as the incident that motivated it (an in-job alarm
variant failed a whole Dev CI run that way, so NO scanning ran on
that head) — and timed_out, which matters given codeql.yml caps at 30
minutes and semgrep.yml at 15. Now alarms on any conclusion that is
not success or skipped. Also added the daily real-model canary to the
watched list: it had no alarm at all, and a silently red daily canary
is the rot pattern this file exists to prevent. Noted that the watcher
matches on workflow DISPLAY NAME, so renaming `name:` disarms it.
- ci.yml carried `pull-requests: read` at WORKFLOW level for
golangci-lint-action's only-new-issues, which is explicitly `false`.
So the scope had no consumer while still reaching every job that does
not override it — including web, playwright and e2e-live, which
npm-install and execute thousands of third-party packages. Removed.
- screenshots.yml held `contents: write` at workflow level for a single
job that runs `npm ci` and `playwright install`. It bought nothing:
the push it existed for cannot succeed, because the main ruleset
carries a pull_request rule with current_user_can_bypass: never and no
bypass actors. So it was a repo-writable token handed to third-party
code on every run, in exchange for a guaranteed-failing push whose
commit message also carried [skip ci]. Dropped to read, with the
shape a real implementation would take written down.
- auto-merge-dependabot.yml: the header asserted "CI is the approval
signal and it is never bypassed", because `gh pr merge --auto` holds
the merge until every REQUIRED check passes. That is only true where
something is required. The dev ruleset requires no status checks at
all, and dependabot.yml points every version update at dev — so there
was nothing holding the merge. Excluded `github_actions` from
auto-merge (that ecosystem's "dependency" IS the CI definition, and
cooldown is not even available for it — Dependabot supports cooldown
for gomod and npm only), added a `branches: [main, dev]` filter so
this can never silently apply to an unprotected branch, and moved the
write scopes from workflow level onto the one job that needs them.
Getting `Dev gate` into the dev ruleset is the real fix and is a
repo-settings action; it is flagged for the owner.
- codeql.yml / dev-ci.yml: moved the two remaining `${{ }}` expressions
out of `run:` blocks and into `env:`. The values come from
{success, failure, cancelled, skipped} so nothing attacker-controlled
reached the shell, but this is the shape the two sites fixed in #1246
were fixed away FROM, and it breaks semgrep's bash sub-parser, which
silently costs coverage on the very files it appears in.
Two stale claims settled rather than left for an auditor to find:
- docs/adr/0012: `cmd/fleet-admin` was to be "a deprecation shim for ONE
release ... removed next release". That clock never started — `git
tag` returns nothing, VERSION is 0.0.0, and CHANGELOG.md has only an
[Unreleased] heading, so "next release" is not a date. The shim also
turns out to be load-bearing rather than vestigial: the Makefile,
bootstrap.sh, update.sh and fleet-upgrade.sh all build or install it,
the last two hard-fail without it, and scripts_dryrun_test.go asserts
the "would install fleet + fleet-admin" string. Amended with a
concrete trigger — removed in the first release after 1.0.0 — and the
note that it forks no logic (it shares internal/admincli.Run).
docs/EVENT-TRIGGERS.md and docs/openapi.yaml were still teaching
`fleet-admin sched trigger …` as the primary command for HMAC-secret
rotation; those are security procedures, so they now say `fleet`.
- migration 022 carried the tree's only TODO(security) — and every
auditor greps for that string. Two problems beyond the deferred work:
it sat in an APPLIED migration, so it was parked where nobody can
close it in place, and it pointed at a source file in an unrelated
external codebase (a dangling cross-repo pointer in a security note).
Rewritten to state the fact plainly (the column holds account NAMES,
never credential values, which are brokered host-side per ADR-0003 /
ADR-0042), to say that whether account names are themselves in scope
is an open threat-model question for the owner, and to point at
SECURITY.md as where that gets answered. golang-migrate tracks by
version with no checksum, so editing the comment cannot re-run or
invalidate the applied DDL. The tree now has zero
TODO/FIXME/XXX/HACK markers.
- .golangci.yml's noctx exclusion said "the one production noctx
(cmd/fleet-admin bootstrap) is fixed in code via exec.CommandContext".
cmd/fleet-admin has held no exec call since the CLI was unified in
#461; the real call sites are in cmd/fleet and internal/admincli.
Repointed, because a lint suppression whose stated reason names dead
code is a suppression nobody can re-verify.
- scripts/generate-icons.py declared web/src/app/favicon.ico among its
outputs. That file has never been committed and is not gitignored
either, so it existed only on whoever last ran the script — while
every other declared output IS committed. The App Router serves
icon.svg and apple-icon.png, with favicon-16/32.png under public/, so
the .ico had no consumer. Dropped, and the docstring now records that
the outputs are committed and when to regenerate.
Signed-off-by: Brad Flaugher <brad@elcanotek.com>
Three tests broke on taskWritableByPrincipal, and all three for the same reason: their fixture tasks were inserted with neither CreatedBy nor CreatedByKeyID, so an unattributed row was being edited by a client-role key. That is precisely the shape the new gate refuses. None of them is about ownership: - TestUpdateTaskRunIfPersistsAndNormalizes and TestUpdateTaskKeepsGatedTaskOnSchedulerPath are about the run_if privilege boundary and the dispatch-state recompute. - TestTypedKeyRouteScope is about the #190 middleware type-scope gate. So the fix is the fixture, not the gate: each task is now attributed to the key that acts on it, which is also the realistic shape — a task a client key created really does carry that key's CreatedByKeyID. Left unattributed, these tests would have been asserting run_if and type-scope behavior through a path that the ownership check short-circuits first, which is a worse test than either intent. mustCreateTypedKeyWithID joins mustCreateRoleKeyWithID as the helper that returns the KeyID alongside the secret, with a note saying why a fixture wants it. Full Go suite green against a real Postgres 16 (`make test`, -p 1). Signed-off-by: Brad Flaugher <brad@elcanotek.com>
docs/implementation-plans-enhancements.md had zero inbound links from any markdown, Go file, workflow or script, and three of its four plans had shipped. The shipped copies are the problem, not the file: a plan sitting next to the authoritative record is the worse of the two, and this one was phrased in the present tense. The #167 entry is why this matters for an audit rather than being tidy-up. It carried a section headed "OAuth control-plane tokens parent-readable" whose resolution — accepted for v1, threat model written down (parent compromise implies stored remote-MCP tokens; agent runs stay child-side per ADR-0040) — was already recorded in SECURITY.md and docs/MCP-BROKER-SCOPES.md. A second copy, phrased as an open decision, in an unlinked file, reads like an unresolved security gap to anyone who greps their way into it. It is not one. What remains is #984 (the Fleet <-> Buzz bridge), which genuinely has not shipped, under a header that says so — and that says explicitly that its unchecked acceptance boxes (including "Bot token not logged; secrets in env only") are criteria the feature must meet BEFORE it ships, on code that does not exist yet, not a list of open findings. That distinction is invisible in a bare checklist and is exactly what an auditor would otherwise have to ask about. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
The tool-output scrubber's literal set was empty of connector
credentials in every production build, so its own doc comment
("OPENROUTER_API_KEY, connector credentials, …") described coverage it
did not have.
The mechanism is an ordering accident, not a missing feature.
agentcore's redactor is built lazily behind a sync.Once that fires on the
first tool output, and it seeds itself from os.Environ(). But the MCP
broker's boot path calls scrubParentConnectorState, which os.Unsetenv's
every connector environment key, and that runs during broker startup —
before the interactive engine is even constructed, let alone before any
turn produces output. So by the time the snapshot is taken those values
are gone, and RegisterEnvLiterals registered only what survives the
scrub.
That divestment is the entire point of the broker boundary and is not
touched here. What it left uncovered is the return path: a connector that
echoes its OWN credential back in a tool result or error string was
scrubbed only if the value happened to match one of internal/redact's
shape patterns (sk-*, ghp_*, AKIA*, Authorization:, marker=value). A
novel bare token would have reached the model context, the SSE stream and
the session log. internal/tools/browserbase_live_view.go is written as
though the literal set were richer than it was — it survives only because
it independently checks strings.Contains(liveView, apiKey) and refuses.
Two wirings close it:
- scrubParentConnectorState now hands each value to
agentcore.RegisterSecretLiteral immediately BEFORE unsetting it, so the
scrubber learns it at the last moment it is knowable.
- cmd/fleet/main.go wires the PARENT's remotemcp service to the same
hook, mirroring what the credential-owning child already does via
mcpbroker.RegisterSecretLiteral (#1124). The parent unseals per-user
api_key secrets and mints/refreshes OAuth bearers for its control
plane — browserbase_live_view among them — and those are acquired at
runtime, so no boot-time env snapshot could ever know them. That
SetSecretObserver call was simply absent.
RegisterSecretLiteral buffers values offered before the redactor exists
and drains them at construction, both under the same mutex that guards
the sharedRedactor nil-check — so a value offered concurrently with
construction is either buffered and drained or added directly, never
dropped between the two. Publication happens under that lock for the same
reason; the unlocked read in toolRedactor is safe on sync.Once's
happens-before. Verified with `go test -race` on internal/agentcore and
internal/redact.
Neither change widens what the parent can READ. A registered value is
held solely as a redaction target and is never emitted, logged or
persisted — and this is a backstop for what comes back across the broker
boundary, not a substitute for the boundary.
The test uses a token that matches none of the shape patterns, so it
passes only if literal registration genuinely reached the redactor, and
registers it before anything forces construction — the real boot order.
Mutation-tested: neutering RegisterSecretLiteral makes it fail with the
secret visible in the output. It also covers post-construction
registration (the runtime-acquired case) and asserts that an empty
registration does not turn the scrubber into a match-everything.
Signed-off-by: Brad Flaugher <brad@elcanotek.com>
An auditor reads the docs and then checks the code, so a doc that overstates is worse than one that says less. This repo has an explicit "Honesty in docs" invariant; these are the places it had drifted. The load-bearing corrections: - AGENTS.md said "Everything is at zero findings today; keeping it there is the point." False, and it was the one line in the agent-facing operating guide an auditor would hold up. The thresholds now differ by scanner, and the difference is stated: Semgrep, ruff and npm audit gate on any finding; CodeQL gates on the High band minus the reviewed register, with everything below it advisory. Also records the two facts an agent must not get wrong — a pull_request CodeQL run certifies a DIFF and never a tree, and `Dev gate` is not a required check on `dev`. - docs/CODEQL.md carried a whole Triggers section describing a workflow that no longer exists (it showed `push: [main]` and `pull_request: [main, dev]`; the file has only workflow_call, workflow_dispatch and schedule), plus the assertion that a push to dev "would re-analyze identical content". That reasoning is exactly the trap that broke dev — the push run is full-tree and the PR run is diff-informed, so they are not identical content — and it is now preserved as the error rather than the rule. Also removed "a CodeQL job with a hundred open alerts still exits 0 and reports green" (false since the Fail-on-findings step) and the sample log format that never matched what the workflow prints. - docs/SCANNING.md claimed the extended suite "reports zero findings on this tree (verified in CI across all four languages on Dev CI run 525)" and that a green check means "clean tree". Run 525 was a pull_request event. Corrected, with the run-527 numbers, and the known-gaps section rewritten — the dev-ruleset gap is now the FIRST gap, since it is the one that makes several other statements in the file conditional. - SECURITY.md had no SAST section at all, though CodeQL and Semgrep are the controls an enterprise auditor asks about by name. It now has one. Its Grype paragraph claimed the gate covers "the image's RPM or Python packages" at a fixable CRITICAL; the policy script selects `.artifact.type == "rpm"` and fires on CRITICAL *and* HIGH, so Python dist-info records are reported and deliberately do not gate — both halves were wrong in the direction that overstates coverage. Its supply-chain section omitted the npm CVE gate entirely. And "CI runs gitleaks on every push" is not true of a feature branch, which runs nothing. - docs/TESTING.md's lane table said the fast lane SKIPS CodeQL. It runs both scanners. The table also omitted four lanes that now block. - CONTRIBUTING.md contradicted itself inside one sentence: "fails the build on a fixable CRITICAL or HIGH CVE ... (HIGH and below are reported, not blocking)". - CHANGELOG.md's [Unreleased] section carried five overlapping entries from #1246 that contradicted each other — one said the scanners gate through ci-gate, another said "CodeQL and Semgrep stay advisory"; one said `ruff format` is reported but not gated, another said it gates; one said Semgrep ships only p/github-actions, another all four packs; one said the code-quality suite was restored when it was dropped. A reader could not tell which was current. Collapsed into one entry describing the shipped end state, and extended with this PR's work. - ruff.toml's "<- what we gate on" marker pointed at the default-only rule line while `select` includes B, SIM and S. - Makefile's .PHONY omitted lint-python. - README.md's documentation table did not list SCANNING.md or CODEQL.md — the two most audit-relevant docs, unreachable from the README — and its layout tree was abridged without saying so. ADR housekeeping, both mine: - ADR-0036 presents its host-side exception list as exhaustive, and an auditor reads it that way, so it has to be. It still named `fastio_upload` as a host-read exception; there is no such native tool any more (Fast.io is an MCP server behind the broker), so the ADR was claiming a hole that does not exist. Two real classes were missing: host `git worktree` management on the scheduled-run path, and the admin-gated host `podman` build for the rampart install. Neither is model-authored and neither weakens the invariant — but "is this exception in the ADR?" should have a reliable answer, and now does. - ADR-0048's two counts were off and are now measured, not recalled: 625 `_test.go` files (not 621) and 81 `//nolint:gosec // G706` sites (not 77 — four of the increase are this branch's own). Verification: make build, make lint (golangci-lint v2.13.1 + ruff + migration DDL lint) and make test all clean; web oxlint, tsc, vitest (1104 tests) and next build all clean. Every markdown link target in every file touched here was checked to resolve on disk. Two lint findings from my own earlier commits fixed here rather than left standing: four `//nolint:gosec // G706` directives I had added were UNUSED — once the value goes through logSafe, gosec stops flagging the line, so nolintlint was right and an unused suppression is worse than none — and a prealloc nit in the new gate-needs test. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
Next.js rewrites web/next-env.d.ts on every `next build`, and my verification run swept the added root-params reference into the docs commit. The file's own header says it should not be edited, and this branch changes nothing that would legitimately alter it — so it goes back to dev's version rather than carrying a build artifact through review. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
streamForceFinalSummary does not apply in.GuardStep while its sibling streamLeakedToolCallRetry does, and the asymmetry reads as an oversight — it was flagged as one during the audit. It is deliberate, so the reason now sits next to the code instead of in someone's head: the retry runs WITH tools and can buy an unbounded number of paid completions, so it must be held to the run's ceilings. The forced summary is a single tool-less completion bounded by tc.MaxTokens, it is metered via in.RecordUsage in OnStepFinish like any other call, and it only runs on the canFinish path — a run stopped by ErrCostCeilingExceeded never reaches Finalize, so it cannot be entered after a ceiling has already tripped mid-run. Worst case is one bounded, accounted completion of overshoot when a ceiling is reached on the final step, which is the price of returning a usable answer rather than a truncated one. The comment says explicitly that this stops being true if the function ever grows tools or a loop. No behavior change. Signed-off-by: Brad Flaugher <brad@elcanotek.com>
…dit-qomnnd Enterprise security audit: close three own-rows authorization holes, unblock the CodeQL gate, correct the docs
…elm chart (#989) (#1249) * Kubernetes as a first-class deployment: pluggable sandbox backend + Helm chart (#989) Ship the enterprise path in one pass per the issue plan: the fleet control plane split from execution runners, with a pluggable sandbox backend selected by FLEET_SANDBOX_BACKEND=podman|kubernetes (env over manifest sandbox.backend, mirroring sandbox.runtime; unrecognized values refuse to boot). - internal/sandbox: a third backend (k8sImpl) behind the same impl interface as podman/host — one ephemeral pod per sandbox, bash as one-shot execs over the apiserver's v4.channel.k8s.io WebSocket protocol, the python bridge as a held exec session, file ops running the same embedded fileops.py. Pods are read-only-rootfs, non-root, capabilities dropped, seccomp RuntimeDefault/Localhost, automountServiceAccountToken=false; the workspace is a shared RWX PVC mounted same-path. #796 poison-and-retire carries over (cancel/timeout deletes the pod with zero grace); orphaned pods are swept at boot like podman containers. No client-go: a minimal hand-rolled REST + WebSocket-exec client (gorilla/websocket was already in the tree; zero new modules), kubeconfig support deliberately narrow (token/client-cert; exec plugins and insecure-skip-tls-verify refused). - Fail-closed boot preflight (and fleet validate-config): apiserver + credentials, exact RBAC verbs, workspace claim, sealed-egress NetworkPolicy object, RuntimeClass when set. Podman-only knobs are refused, not ignored (FLEET_SANDBOX_RUNTIME, FLEET_SANDBOX_SECCOMP_PROFILE, allowlisted egress). - deploy/helm/fleet: single-replica control-plane Deployment (Recreate, no replica knob), runner RBAC, workspace/data PVCs, deny-all NetworkPolicy for egress=none pods + optional open-egress shaping, optional eval Postgres, optional web/Ingress. Linted + template-rendered by a new helm CI job in both gates. - Docs: docs/DEPLOYMENT-KUBERNETES.md (kind walkthrough + production checklist + honest deviations), DEPLOYMENT.md/EKS-DEPLOYMENT.md/ SANDBOX-RUNTIMES.md/TIMERS.md updates, README + AGENTS.md index rows, CHANGELOG. ADR-0049 amends ADR-0004: the single-box podman install stays the default and unchanged; only the no-k8s-artifacts enforcement clause is superseded. Closes #989 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119Ygu6B16k5wBR3vj471VW Signed-off-by: Brad Flaugher <brad@elcanotek.com> * Drop postgres.password from chart values; the Secret is the only password home CodeQL (High band) flagged the empty-string password default in deploy/helm/fleet/values.yaml. The real fix, not a waiver: no password belongs in a values file at all. The eval-Postgres template now always reuses the <release>-postgres Secret's password when one exists (an operator pre-creates it to choose their own) and generates a random one otherwise — same lookup logic as before, minus the values seam. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119Ygu6B16k5wBR3vj471VW Signed-off-by: Brad Flaugher <brad@elcanotek.com> * Clear the two blocking CodeQL findings on the k8s-backend PR - go/allocation-size-overflow (k8s_exec.go writeStdin): drop the manual `make([]byte, 1+n)` size arithmetic and let append size the backing array — the same shape podmanArgs already uses for the same rule. - go/command-injection (host.go runBash): accepted-findings register entry with the reason. The sink is the component's documented contract: the unsandboxed TEST/DEV-ONLY executor behind the fleet_host_executor build tag; a release build ships the fail-closed stub, so the sink cannot reach production (ADR-0002 enforcement). It surfaced on this PR only because the diff-informed run intersected the flow's path. The 12 medium log-injection findings are below the High band — advisory per ADR-0048, triaged in the Security tab, not gated. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119Ygu6B16k5wBR3vj471VW Signed-off-by: Brad Flaugher <brad@elcanotek.com> * Retire the EKS privileged-pod recipe; make the k8s guide the one reference docs/EKS-DEPLOYMENT.md documented running the whole single-box model — rootless Podman included — inside one privileged pod. With the first-class path landed, keeping it as a parallel track would imply support it never had (it was explicitly hand-verified, not CI-exercised). Removed; its durable content is folded into docs/DEPLOYMENT-KUBERNETES.md, which grows into the full reference: architecture diagram, both image builds (the control-plane Containerfile's FROM golang: stage is now the copy scripts/check_versions_test.go pins to go.mod, taking over the EKS doc's slot in that drift test), provider notes (EKS/GKE/AKS/bare metal), day-2 operations (CronJob equivalents of the systemd timers, upgrade story, metrics), troubleshooting, and a migration note for deployments built from the old recipe. All references repointed (DEPLOYMENT.md callout, ADR-0049, chart values, publish-sandbox-image.yml comment); CHANGELOG gains a Removed entry. Historical changelog entries are left as history. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119Ygu6B16k5wBR3vj471VW Signed-off-by: Brad Flaugher <brad@elcanotek.com> * k8s backend polish: dedicated runner pools, explicit pull policy, no migration prose - Sandbox pods can pin to a dedicated node pool: FLEET_SANDBOX_K8S_NODE_SELECTOR (k=v,k=v) + FLEET_SANDBOX_K8S_TOLERATIONS (JSON array), or the manifest's structured node_selector/tolerations (env wins; bundle values are canonicalized into the env string forms at boot so the pool build parses one source). Malformed values refuse to boot — a typo'd selector must not silently schedule sandboxes onto the wrong nodes. Chart values sandbox.kubernetes.nodeSelector/tolerations render to the env vars. - Sandbox pods set imagePullPolicy: IfNotPresent explicitly: the API default for a :latest tag is Always, which breaks side-loaded kind images and re-pulls a mutable tag mid-run. - Drop the old-EKS-recipe migration prose everywhere (guide, DEPLOYMENT.md callout, ADR-0049, CHANGELOG) — nobody deployed from it. - Document /metrics scraping honestly in the k8s guide: it is admin-API-key gated (X-API-Key), which stock Prometheus cannot send — use Alloy/vmagent or a header-injecting sidecar. (A ServiceMonitor was considered and deliberately NOT shipped: it would 401.) Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119Ygu6B16k5wBR3vj471VW Signed-off-by: Brad Flaugher <brad@elcanotek.com> * CodeQL green end-to-end: in-source suppression + cluster-text log sanitization Two changes so the Security tab, the SARIF, and CI tell one story with no manual alert-dismissal step: - host.go command-injection: the waiver moves INTO the source as a `// codeql[go/command-injection]` suppression on the sink line (running the caller's shell is this test-only executor's documented contract; the fail-closed stub ships in release builds). The Go CodeQL analysis now also runs the standard pack's AlertSuppression query so the annotation is honored end-to-end: the SARIF result carries suppressions[], the repo gate classifies it ACCEPTED, and code scanning closes the Security-tab alert — which is what turns GitHub's app-side CodeQL check green without a human dismissing anything. The register entry stays for this commit as a belt while CI proves the suppression parses; it is removed once confirmed. - go/log-injection (12 medium advisories): fixed at the source instead of waived. The kubernetes backend embeds cluster-API/pod-derived text (status messages, exec status, stderr snippets, upgrade-response bodies) into error strings that reach log.Printf sites across the codebase; a pod printing a crafted line to stderr could forge journal entries. sanitizeClusterText (newline stripping, the sanitizer shape CodeQL models) is applied at every point remote text enters an error: status errors at construction, pod phase/waiting messages, exec status and close-reason text, stderr snippets, list-returned pod names. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119Ygu6B16k5wBR3vj471VW Signed-off-by: Brad Flaugher <brad@elcanotek.com> * codeql: '+' prefix so the AlertSuppression pack actually runs Without the additive prefix the packs input is treated as a replacement set that the queries input then overrides — verified on the previous run: rule-metadata count stayed at 35 (no suppression query) and the gate's accepted tier showed the register waiver, not '(in-source suppression)'. The sanitization half of the previous commit did land: go/log-injection dropped from 12 findings to 9 (the error-string flows). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119Ygu6B16k5wBR3vj471VW Signed-off-by: Brad Flaugher <brad@elcanotek.com> * codeql: inline config so security-extended + AlertSuppression combine Third and definitive form: the separate queries:/packs: inputs override one another (verified on this repo with and without the '+' prefix — the suppression query never ran either way); an inline config block is where a suite and an extra pack have documented combine semantics. Every language keeps security-extended; go adds AlertSuppression so the in-source // codeql[rule-id] comment on host.go's test-only executor is honored end-to-end (SARIF suppressions -> gate ACCEPTED tier -> the Security-tab alert closes). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119Ygu6B16k5wBR3vj471VW Signed-off-by: Brad Flaugher <brad@elcanotek.com> * codeql: settle on the register as the one waiver mechanism (measured) In-source // codeql[rule-id] suppressions do not work with this pipeline. Three forms were tried on this PR (packs input, packs with the '+' additive prefix, inline config combining security-extended with codeql/go-queries:AlertSuppression.ql); in every case the uploaded SARIF carried no suppressions on the annotated result, the gate classified the host.go waiver from the register, and the Security-tab alert stayed open. The analyze action's interpret step is not configurable enough to change that, so: revert the workflow to plain security-extended, record the measurement in codeql.yml so nobody re-tries it blind, drop the inert suppression annotation from host.go (the explanatory comment and the register entry — the mechanism that demonstrably gates — stay), and close the deliberately-waived Security-tab alert with a one-time human dismissal, which persists across analyses. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119Ygu6B16k5wBR3vj471VW Signed-off-by: Brad Flaugher <brad@elcanotek.com> --------- Signed-off-by: Brad Flaugher <brad@elcanotek.com> Co-authored-by: Brad Flaugher <brad@elcanotek.com> Co-authored-by: Claude <noreply@anthropic.com>
… correct the CodeQL waiver docs
Four review passes over .github/workflows (3.1k lines) and the docs that
describe them. The gating architecture was already strong — SHA-pinned
actions with a test enforcing the shape, aggregate gates with a test
enforcing `needs` completeness, per-job least privilege — so this is
mostly about the seams around it.
Nothing here weakens a gate; two changes tighten one that could report
green over work that never ran.
New gate: workflow + shell lint (actionlint, shellcheck)
Nothing checked the 3.1k lines of workflow YAML that decide what every
other gate runs, and nothing checked the 6.2k lines of bash that ARE the
deploy path (update.sh / bootstrap.sh / doctor.sh). Both now gate in
ci.yml and dev-ci.yml, wired into `make lint` via `lint-actions`.
Both start CLEAN, measured rather than assumed — actionlint found 5 items
and shellcheck 3, all fixed here:
- `run: "$GITHUB_WORKSPACE/scripts/check-npm-overrides.sh"` in both
lanes passed the path to the shell UNQUOTED. The YAML parser consumes
the quotes, so the shell never saw them, despite the comment above the
line showing quoting was intended.
- SC2153 on `RESULTS` is env-supplied; waived in-line with a reason.
- SC2148/SC2034 in three scripts: a `shell=bash` directive for the
sourced-only lib, a reason on the documentary WEB_USER anchor, and a
throwaway loop variable in fleet-upgrade.sh.
Both are mutation-tested in both directions.
Green-but-vacuous holes closed
- ci.yml installed postgresql-client-18 best-effort, ending in `|| echo`.
An unreachable PGDG left client 16 in place and backup_test.go's
`t.Skipf("pg_dump major %d != server major %d")` turned the ONLY
coverage of `fleet backup`/`fleet restore` off — behind the single
required check on main. It now asserts the major.
- The docs-only classifier initialised `docs_only=true` and only ever
cleared it inside the loop, so an EMPTY diff classified as docs-only
and skipped the suite — and `ci-gate` trusts exactly that value when
deciding a skip is acceptable. An empty range is the absence of
evidence, not evidence of prose.
- dev-ci's `go test` lacked `-count=1` (ci.yml passes it on all three
invocations). setup-go restores the build cache, which holds test
RESULTS, and Go's cache key cannot see a Postgres service container.
Honesty in docs
Five documents advertised an in-source `// codeql[rule-id]` comment as a
waiver route. codeql.yml itself records that all three forms were tried on
#1249 and none produced a `suppressions` array — and carried BOTH claims,
the stale "honored end-to-end" paragraph directly above the note refuting
it, with no `packs:` input in the matrix. The register is the only route
that works; the gate's own runtime advice said otherwise to the one person
guaranteed to read it, a blocked contributor.
ADR-0048's normative Decision stated the threshold as an OR where the jq
implements a fallback; read literally it blocks on go/log-injection (error
@ 6.1), the exact deadlock the ADR exists to undo.
Also: CONTRIBUTING said Go 1.26.x (go.mod says 1.27.0); CODEOWNERS listed
7 of ci-gate's 13 needs; `make ci-web` ran 4 of the web job's 8 steps,
dropping both npm audits, the override canary and the explicit typecheck —
a clean local run and a red PR.
Least privilege
`issues: write` sat at WORKFLOW scope in both scheduled scan lanes, so it
was live for every step of a long job running `govulncheck@latest` and a
podman build pulling ~400 RPMs. Split into its own alarm job that checks
out nothing — the shape scan-cron-alarm.yml already uses. Added
`persist-credentials: false` to the write-scoped checkouts.
Runner economics
ci.yml — the expensive lane — had no `concurrency:`, so stacked pushes ran
full suites to completion. Cancellation is scoped to `pull_request`; a push
to main is the only tree-wide CodeQL verdict and must not be cancelled.
`timeout-minutes` now on every job (was 2 of 13 workflows); values sized
per job, after a first cut gave identical grype work 20 and 30 minutes.
New tests, because this repo asserts invariants rather than remembering
them: every workflow declares a top-level `permissions:` block, and the
actionlint version/checksum agree across both lanes.
Signed-off-by: Claude <brad@elcanotek.com>
…the semgrep pinning claim The workflow engineering here was already ahead of most projects its size. Where it was behind them was the community-facing surface: a contributor had no prompt for the obligations CONTRIBUTING.md and AGENTS.md impose, and no account of why CI might be red through no fault of theirs. - .github/PULL_REQUEST_TEMPLATE.md — prose first (what/why, how you verified, scope and deviations), then a short checklist for the things that are currently a reviewer's job to remember: DCO sign-off, a CHANGELOG entry, a docs/<FEATURE>.md note, and an ADR in the SAME PR when an invariant moves. - .github/ISSUE_TEMPLATE/ — bug and feature forms plus a config.yml that disables blank issues so the private security-disclosure link is unmissable. SECURITY.md and CODE_OF_CONDUCT.md both say "do not open a public issue for a vulnerability", and the New Issue button was offering a blank box with no such warning — the exact moment a reporter is most likely to get it wrong. The feature form asks up front whether the idea touches an invariant (ADR required) and whether it could ship as a client-config bundle instead of an engine change. - CONTRIBUTING.md gains "If CI is red and you don't recognise the failure". Three lanes depend on live external data and can redden an untouched tree — govulncheck's advisory DB, npm audit, and Semgrep's registry-fetched rules, which cannot be vendored for license reasons. All three are documented for maintainers in docs/SCANNING.md and were documented for contributors nowhere. Also notes that a first PR waits on maintainer approval before CI starts. - semgrep.yml claimed its `pip install semgrep==X` was pinned "like every other tool this repo installs in CI (gitleaks, grype, golangci-lint)". It is not: those are checksum-verified, while pip resolves semgrep's ~40-package dependency closure unverified, inside a job that sits in both merge gates. The comment now states the real guarantee and the mitigating fact (PyPI forbids re-uploading a version, so the exposure is a new malicious release rather than mutation of a pinned one). Signed-off-by: Claude <brad@elcanotek.com>
Two owner decisions, applied across every file that referenced them.
Auto-merge is gone
.github/workflows/auto-merge-dependabot.yml is deleted. Its own header had
already argued the case against it without noticing: it explained that
`gh pr merge --auto` holds a merge only on REQUIRED checks, named `dev` as
a branch whose ruleset requires none, said the `branches:` filter existed
so the workflow "can never silently start applying to a branch nobody
protected" — and then listed `dev`. Since every Dependabot version update
targets `dev`, the mitigation SECURITY.md and docs/SCANNING.md both
credited was in fact the delivery mechanism: same-day patch bumps landing
unattended while Dev gate was still running.
Every dependency bump now waits for a human, whatever the ecosystem or
bump level. References updated rather than merely deleted, since several
carried reasoning that only made sense under auto-merge:
- SECURITY.md: the cooldown rationale no longer rests on "patch bumps
are auto-merged", and the github-actions exception now says what
actually contains it.
- dependabot.yml: the cooldown header and both copies of the
semver-major note were justified in terms of the auto-merge path.
- CODEOWNERS: the "auto-merge interaction" paragraph described a hazard
that no longer exists.
- docs/SCANNING.md: the Known-gaps entry credited three auto-merge
mitigations. It now records that the workflow's removal closes the
compounding risk while the underlying gap — nothing requires Dev gate
on dev — remains open and still needs a repo-settings change.
Also corrected there: the action-pin census said 13 workflow files / 12
with a `uses:` / 53 references. It is now 12 / 11 / 56, with a note that
check_action_pins_test.go rather than the paragraph is what holds the
invariant — a hand-maintained count is exactly what goes stale.
DCO is gone
CONTRIBUTING.md required a Signed-off-by trailer and nothing enforced it.
Rather than add a gate, the requirement is dropped: the sign-off section,
the PR-template checkbox, and the pointers in AGENTS.md and the issue
config. "Commit messages" keeps the useful half — imperative subjects, and
why-not-what in the body.
CHANGELOG records this alongside the rest of the CI review. Historical
CHANGELOG entries that describe auto-merge as it existed are left alone;
they are a record of what shipped, not a claim about today.
…sures
The docs here are unusually thorough, which is what made the drift worth
chasing: a reader trusts them. Everything below was verified against the
YAML rather than taken from a previous revision of the prose.
Docs that overclaimed or understated
- ci.yml's Grype step said "Block only when a CRITICAL RPM finding has a
packaged fix". scripts/check-grype-policy.sh selects `critical or high`.
The comment understated the gate; every prose doc had it right.
- docs/TESTING.md said the web job was `npm ci -> npm run lint (ESLint) ->
vitest -> build`. `web/package.json` runs oxlint, and the SAME document
spends 40 lines above explaining why ESLint was replaced. It also omitted
four of the job's eight steps (both npm audits, the override canary, and
the explicit typecheck).
- docs/SCANNING.md described the sandbox-workflow hardening as "refuses
refs/pull/* refs". That deny-list was replaced by an allow-list BECAUSE
it was bypassable, and the two holes are worth stating rather than
quietly updating: a workflow_call input may carry newlines, so
`main\nresolved=refs/pull/1/head` matched no deny pattern and emitted a
second GITHUB_OUTPUT assignment (last wins); and "every named ref is
collaborator-written" is false of reachable COMMITS, since fork-PR
objects live in the base repo's store. A reader re-implementing the
documented form would have inherited both.
- scan-cron-alarm.yml watches three workflows, not the two documented.
Controls that existed but were cited nowhere
check_gate_needs_test.go is the strongest anti-rot control in this setup —
it fails `make test` when a job is missing from its gate's `needs`, which
is the regression that produced the red-but-not-required CodeQL break —
and it appeared in no document. docs/SCANNING.md now names it alongside
check_action_pins_test.go and the new check_permissions_test.go, spells
out ci-gate's full needs list, and covers the `helm` and `migrations`
lanes, which were in both gates and in no doc at all.
The stack table gains actionlint and shellcheck rows with an explicit note
on why they earn a place next to Semgrep's p/github-actions pack: that
pack and CodeQL's `actions` language are security rule sets, and neither
parses ${{ }} expressions or shellchecks a `run:` block. Also corrected
there: gitleaks was described as running on "every branch"; the job exists
in ci.yml and dev-ci.yml, so a feature branch with no open PR gets no
scan.
Residual exposures closed
- e2e-canary.yml uploaded `.e2e-run/logs/` on failure. That is the ONE job
booting fleet with the real OPENROUTER_API_KEY, the server's stdout is
redirected into that directory, and the repo is public. Nothing in the
Go tree logs the key today, so this was tail risk, not a known leak —
but the Playwright report is what a failure is diagnosed from anyway.
ci.yml's e2e-live keeps its logs: fake LLM, throwaway key.
- bundle_dir reached the same build-script invocation as fleet_ref with no
validation at all, while fleet_ref has a character allow-list, a
bare-SHA refusal and a newline fix. No injection (it goes through env:
and is quoted), but nothing stopped `../../.fleet-core` from repointing
the build at another manifest. Guard added to both reusable workflows
and exercised against absolute paths, `..`, spaces and `$(id)`.
AGENTS.md's `make lint` and CI-mirrors lines now include the new lanes and
the Helm lint.
Not changed, and deliberately: setup-go's inputs differ between lanes
(`check-latest: true` in some, `cache: true` in others). These are not
alternatives — they control different things, and go.mod pins an exact
1.27.0 — so "make them consistent" would be churn without a defect.
… holes, remove auto-merge (#1250) Four review passes over .github/workflows and the docs describing them. New blocking gate: actionlint + shellcheck, covering the workflow YAML that decides what every other gate runs and the ~6.2k lines of bash that are the deploy path. Both start at zero findings, measured; the 5 actionlint and 3 shellcheck items were fixed first, including a `run:` line that passed its path to the shell unquoted in both CI lanes because the YAML parser consumes the quotes. Two green-but-vacuous holes closed: a best-effort postgresql-client install that silently disabled the only backup/restore coverage behind the single required check on main, and a docs-only classifier that treated an empty diff as docs-only. Also -count=1 on dev-ci's go test, which could otherwise report cached results against a Postgres service container. Auto-merge removed entirely, per owner decision. Its own header argued the case against it: --auto holds only on required checks, dev has none, and dev was listed in its branches filter anyway. DCO dropped as unenforced. Honesty in docs: five places advertised a CodeQL waiver route that codeql.yml itself records as measured-not-working, and ADR-0048's normative Decision stated the gate threshold as an OR where the jq implements a fallback. Least privilege: issues: write moved out of workflow scope in both scheduled scan lanes into a dedicated alarm job; persist-credentials: false on the write-scoped checkouts; the canary stopped uploading server logs from the one job holding a real API key on a public repo; bundle_dir gained the validation fleet_ref already had. All 15 checks green on #1250, including the new lint job.
The k8s backend drops the supporting-doc bind mounts — a pod has no host
filesystem to bind from — and because the fileop path anchor only trusts roots
that are actually mounted, that made `view_file protocols/foo.yaml` a REFUSAL
("fileop root is not inside a sandbox bind mount"), not a miss. For a
protocol-driven bundle that is most of the product: the system prompt lists
protocols and skills by relative path and the agent reads them on demand.
A sandbox image can carry those dirs at the same absolute paths the control
plane reads them from — that is what makes the workspace symlinks resolve for
bash/run_python — but nothing could tell fleet so. Now something can:
sandbox.kubernetes.bundle_docs_in_image (FLEET_SANDBOX_K8S_BUNDLE_DOCS_IN_IMAGE)
chart: sandbox.kubernetes.bundleDocsInImage
With it, those roots keep their read anchors inside a pod and the file tools
work as they do under podman. The declaration cannot widen anything: it
re-admits READ-ONLY anchors for roots the operator already configured, the read
still executes inside the sandbox, and a write or a writable bind beneath one
is refused exactly as before (covered by a fake-apiserver test that runs the
real fileops.py). fleet cannot inspect an image, so it is trusted the way
sandbox.image and runtime_class are — a wrong declaration degrades to a
not-found read, the podman missing-dir behavior.
Scope of the declaration is deliberately narrow, and boot logs every decision:
- Only the BUNDLE's own doc dirs (personas, protocols, system_prompts, skills)
are covered. Other entries in the mount list — the uploads root — are
control-plane state no image can contain; they stay dropped, with a reason
logged per path.
- A materialized skills tree is NEVER covered. Inheriting fleet's built-in
pack resolves SkillsDir to $FLEET_DATA_DIR/skills-merged/<hash>, which
sandbox pods do not mount and no image can reproduce. The log names the fix
(skills_builtin: false) instead of leaving an operator to wonder why
protocols read and skills do not. No configuration yields both the built-in
pack and in-sandbox skill files on this backend; docs/SKILLS.md and the
guide's honest-scope list now say so.
- A non-boolean value refuses to boot, and `fleet validate-config` runs the
same parse plus reports which way it resolved.
The keep/drop rule is one pure, total function (k8sDocMounts) so the policy is
pinned by tests rather than read out of a boot log.
Docs: a new "Bundle docs inside a sandbox pod" section in
docs/DEPLOYMENT-KUBERNETES.md (mechanism, the derived-image recipe, the four
things to be honest about), the config-reference row, the reworked
honest-scope bullets, docs/SKILLS.md, config/default/manifest.yaml, CHANGELOG,
and ADR-0049's non-goals — which now records WHY fleet does not synthesize
these mounts (a ConfigMap projection or a push into the workspace claim would
put bundle content on a writable, agent-reachable surface).
Verified: go build/vet clean, gofmt clean, full tagged suite green,
`helm lint` + template render with the flag on and off. golangci-lint was not
run locally — the installed binary predates this repo's Go 1.27 target and
refuses to load the config; CI's lane covers it.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbgdPXBYGKKBrykZukQV9N
Signed-off-by: Brad Flaugher <brad@elcanotek.com>
nolintlint failed the lint lane: the log.Printf calls added with the bundle_docs_in_image knob use constant format strings, so G706 never fires on them and the suppressions were dead. Removed rather than kept "just in case" — an unused directive is exactly what nolintlint exists to catch, and the surrounding comments already say why the interpolated values (operator- configured bundle paths) are not request input. Verified with golangci-lint v2.13.1, the version and config CI runs: 0 issues. Previous local run used an older binary that refused this repo's Go 1.27 target, which is why this landed in CI instead. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WbgdPXBYGKKBrykZukQV9N Signed-off-by: Brad Flaugher <brad@elcanotek.com>
…-vd4jnc Kubernetes: let a sandbox image serve the bundle's doc reads
ADR-0049 made the sandbox backend pluggable (FLEET_SANDBOX_BACKEND= podman|kubernetes) and added an AGENTS.md index row — but nothing else in the file moved, so the headline paragraph and the "sandbox is mandatory" invariant still described the sandbox as *rootless-Podman*, full stop. An agent reading only AGENTS.md would read the kubernetes backend as an invariant violation rather than a supported deployment. Both now state the actual shape: the sandbox is mandatory, the backend is pluggable. While there, the invariant bullet gained the two mechanisms it had left implicit — the `fleet_host_executor` build-tag fence (#159), which is what makes "the host executor cannot ship enabled in a production build" a property of the artifact rather than a runtime flag, and the kubernetes backend's fail-closed boot preflight (no degrade to podman, none to host execution). The rest is drift between the file and the Makefile/workflows: - `make test` / `test-race` / `test-cover` were documented without `-tags fleet_host_executor`, which every one of them passes. A bare `go test ./...` builds a different tree than CI does, so the tag is documented as load-bearing rather than elided. - The web block was a stale hand-copy of the web CI job: it dropped the second npm tree (`scripts/rampart-service`) and the override canary. `make ci-web` runs all eight steps and is now the recommended path. - `make govulncheck`, `ci-go`, `ci-web`, `ci-local` were absent from the target list; `lint-python` and `lint-actions` skip loudly when the tool is missing, which a green local `make lint` does not distinguish. - CI was described as one lane. `ci.yml` fires on `main` only and `dev-ci.yml` is dev's only signal, deferring the -race lane, govulncheck, Grype and both Playwright suites — so the dev→main promotion PR is the first time the full gate ever sees the code. Said plainly, along with `CI gate` being the single required check and auto-merge being gone. - The merge-gate enumeration omitted actionlint/shellcheck, the Helm chart lint and the Playwright suites. - The repository map omitted `deploy/` (systemd units + the Helm chart), the harness binaries under `cmd/`, and the `/settings` + `/admin` web routes. Every claim in the diff was checked against the Makefile, the workflows, `.golangci.yml`, ADR-0049 and the tree; no docs link in the file is dead. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eFXcF6BM7bxFVEQH67iuk
Reconcile AGENTS.md with the tree it describes
…leet-promote-pw9idd Promotion of dev → main. Twenty files conflicted, all of them artifacts of promotion #1248 having been SQUASH-merged into main: the squash put dev's content on main as a new commit with no shared ancestry, so files that both branches had gained independently came back as add/add, and the workflow dev deleted came back as delete/modify. Resolved by taking dev in every case, and verified rather than assumed: for each of the twenty files, main's blob is byte-identical to a commit inside dev's own history (e90fc99, 9340efd, 9a573e8, 47a1e58, 5003949, def55c5), so main contributed nothing dev does not already have — main carries no independent work, only promotion merges. auto-merge-dependabot.yml stays deleted, per dev's removal of automatic merging. The resulting tree is byte-identical to origin/dev, which is the check that matters: no main-side content dropped, no dev-side content lost. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eFXcF6BM7bxFVEQH67iuk
The assertion added in this range failed on its first real outing — got=16 against server 18 — over an install that had plainly succeeded (`Setting up postgresql-client-18 (18.6-1.pgdg24.04+2)`). Installing the package is not what decides the answer: /usr/bin/pg_dump is a symlink to postgresql-common's pg_wrapper, and the wrapper's own header states the rule — it calls the client "with the version, cluster and default database specified in ~/.postgresqlrc or /etc/postgresql-common/user_clusters". That is cluster configuration, not "the newest client installed", so on a runner image carrying a PostgreSQL 16 cluster, adding a client package alongside it changes nothing about the dispatch. Verified rather than reasoned about: this container has the same shape as the runner (/usr/bin/pg_dump -> ../share/postgresql-common/pg_wrapper, the versioned tree at /usr/lib/postgresql/16/bin), and there the wrapper likewise reports 16 while prepending the versioned bin dir resolves pg_dump to it directly. So the versioned bin dir goes first on $GITHUB_PATH. That is also what the test needs: internal/admincli/backup.go execs `pg_dump` off PATH, with no versioned path of its own, so PATH resolution — not package presence — is the thing that decides whether backup_test.go's major-mismatch t.Skipf turns the only coverage of `fleet backup`/`fleet restore` off. Asserted in two places, because $GITHUB_PATH takes effect only from the next step: the install by absolute path in the step that performs it, and PATH resolution in a step of its own. The second one is the assertion that mirrors what the test actually invokes, so the two cannot drift apart again without going red. The CHANGELOG entry claiming the step "now asserts the major" is corrected in the same commit — the assertion was there and could not pass, which is the kind of half-true the honesty-in-docs invariant is about. Verified: actionlint clean over every workflow; go test ./scripts/ passes, including TestPostgresMajorAgreesAcrossCI, which requires every postgres major named anywhere in .github/workflows to agree. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eFXcF6BM7bxFVEQH67iuk
Merged
4 tasks
bradflaugher
added a commit
that referenced
this pull request
Aug 23, 2026
…both lanes keep it (#1255) Closes the two loose ends from promotion #1253. ci.yml's pg_dump PATH fix is cherry-picked onto dev, so dev's ci.yml is once again byte-identical to main's. dev-ci.yml gains the same Postgres-client lane. Its Go job runs the same `go test ./...` against the same server-18 service as the full gate and had no matching client, so TestBackupRestoreRoundTrip hit its major-mismatch t.Skipf and the only coverage of `fleet backup` / `fleet restore` ran as a SKIP on every dev push. It also left ci.yml as the sole home of that step, which is why a broken version of it could not surface until a promotion PR. TestGoSuiteLanesInstallMatchingPgClient asserts both halves — the install and the $GITHUB_PATH append — for both lanes. Each half has failed once in production, one per lane, neither visibly in its own lane. Verified by mutation (both failure modes produce their intended error) and by running TestBackupRestoreRoundTrip for real against a local PostgreSQL server with matching majors: PASS, not SKIP.
Merged
4 tasks
bradflaugher
added a commit
that referenced
this pull request
Aug 23, 2026
…keeps both lanes honest (#1256) Carries #1255. dev-ci.yml's Go job runs the same `go test ./...` against the same server-18 service as the full gate and had no matching client, so TestBackupRestoreRoundTrip hit its major-mismatch t.Skipf and the only coverage of `fleet backup` / `fleet restore` ran as a SKIP on every dev push. It also left ci.yml as the sole home of that step, which is why a broken version of it could not surface until promotion #1253. TestGoSuiteLanesInstallMatchingPgClient asserts both halves — the client install and the $GITHUB_PATH append — for both lanes. Each has failed once in production, one per lane, neither visibly in its own lane. ci.yml is absent from this diff by design: #1255 cherry-picked its pg_dump fix onto dev, producing content identical to main's, so the branches no longer diverge there. This promotion merged with zero conflicts, against twenty on #1253 — #1253 went in as a merge commit rather than a squash, so the ancestry link survived. Merged as a merge commit for the same reason.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 changed, and why
Promotion of
dev→main: 32 non-merge commits, 81 files, +7,149 / −1,640, plus one CI fix the full gate exposed (below).The substance, in rough order of weight:
FLEET_SANDBOX_BACKEND=podman|kubernetes, env over the bundle manifest'ssandbox.backend, unrecognized values refuse to boot.k8sImplruns one ephemeral pod per sandbox, bash as one-shot execs over the apiserver's WebSocket exec protocol, python as a held exec session, the same embeddedfileops.py. Pods are read-only-rootfs, non-root, caps dropped, no service-account token; the workspace is a shared RWX PVC mounted same-path. Fail-closed boot preflight (apiserver + credentials, exact RBAC verbs, workspace claim, sealed-egress NetworkPolicy object, RuntimeClass when set) with no degrade to podman or host execution; podman-only knobs are refused rather than ignored. Packaging is one Helm chart (deploy/helm/fleet) — no operator, no CRDs. No client-go: a hand-rolled REST + WebSocket-exec client, zero new modules.security-extendedand gated on the High band minus a reviewed accepted-findings register (ADR-0048); CodeQL and Semgrep wired intoCI gate/Dev gateas reusable-workflow calls, so a scanner finding blocks like a compile error; npm deps audited on both trees; ruffformat --checkenforced.actionlint,shellcheck) over the ~3.1k lines of workflow YAML and ~6.2k lines of bash that decide what every other gate runs — starting at zero findings, measured.auto-merge-dependabot.ymlis deleted; its own header had made the case against it (--autoholds only on required checks, anddevrequires none, so it was the delivery mechanism for unattended same-day patch bumps). Every dependency bump now waits for a human. The unenforced DCO request was dropped rather than gated.postgresql-client-18install that letbackup_test.goskip the only coverage offleet backup/restorebehind the single required check, and a docs-only classifier that treated an empty diff as docs-only and skipped the suite.AGENTS.mdreconciled with the tree (Reconcile AGENTS.md with the tree it describes #1252).The one thing the full gate caught
Go build / vet / lint / testfailed on the first run of this PR — and it was a real defect in the code being promoted, not a flake. The new major-version assertion failed withgot=16against server 18, over an install that had plainly succeeded (Setting up postgresql-client-18 (18.6-1.pgdg24.04+2)).Installing the package is not what decides the answer.
/usr/bin/pg_dumpis a symlink to postgresql-common'spg_wrapper, and the wrapper's own header states the rule: it calls the client "with the version, cluster and default database specified in~/.postgresqlrcor/etc/postgresql-common/user_clusters". That is cluster configuration, not "the newest client installed" — so on a runner image carrying a PostgreSQL 16 cluster, adding a client package alongside it changes nothing about the dispatch.Fix: the versioned bin dir goes first on
$GITHUB_PATH. That is also what the test needs —internal/admincli/backup.goexecspg_dumpoff PATH with no versioned path of its own, so PATH resolution, not package presence, is what decides whetherbackup_test.go's major-mismatcht.Skipfsilently turns off the only coverage offleet backup/restore. Asserted in two places, because$GITHUB_PATHtakes effect only from the next step on: the install by absolute path in the step that performs it, and PATH resolution in a step of its own — the latter being the assertion that mirrors what the test actually invokes, so the two cannot drift apart again without going red.This is a textbook instance of the gap
AGENTS.mdnow documents:dev-ci.ymlhas no Postgres-client lane at all, so this step only ever runs on a PR targetingmain.dev's copy ofci.ymlstill carries the unfixed version; merging this PR as a merge commit carries the fix tomainand leaves the next promotion conflict-free (base = this PR's dev tip, dev-side unchanged in that region, so main's fix wins). Squashing would break that, as #1248 already demonstrated.How you verified it
Merge resolution — verified, not assumed. Twenty files conflicted. All of it is fallout from promotion #1248 having been squash-merged into
main: the squash placed dev's content on main as a new commit with no shared ancestry, so files both branches had gained came back as add/add, and the workflow dev deleted came back as delete/modify.Resolved by taking
devin every case, after checking each one: for all twenty files,main's blob is byte-identical to a commit inside dev's own history (e90fc99,9340efd,9a573e8,47a1e58,5003949,def55c5) — so main contributed nothing dev does not already have.maincarries no independent work in this range; every main-only commit is a promotion merge or squash.auto-merge-dependabot.ymlstays deleted, per dev's removal of automatic merging. The check that settles it: the merge commit's tree is byte-identical toorigin/dev, so no main-side content was dropped and no dev-side content lost.The pg_dump fix — reproduced, not reasoned about. The container this ran in has the same shape as the runner (
/usr/bin/pg_dump -> ../share/postgresql-common/pg_wrapper, versioned tree at/usr/lib/postgresql/16/bin): there the wrapper likewise reports 16 through CI's exact parse, while prepending the versioned bin dir resolvespg_dumpto it directly. Thenactionlintclean over every workflow, andgo test ./scripts/green — includingTestPostgresMajorAgreesAcrossCI, which requires every postgres major named anywhere in.github/workflowsto agree.Local pre-flight of the gates
devnever runs, before the PR was opened:make compile(release config, host executor fenced out) — clean.make govulncheck— no vulnerabilities; 0 called, 0 in imported packages, 1 in a required-but-uncalled module.go test -race -p 1 -tags fleet_host_executorover the packages this range actually moves —internal/sandbox(the new k8s backend),internal/agent,internal/agentcore,internal/config,internal/clientconfig,scripts— all ok, no races.Not reproducible in this environment, left to CI: the Grype image scan (needs a podman build of the sandbox image) and the live Playwright lane (needs a real backend + rootless sandbox). Every check on the dev-side PR #1252 was green before it merged (15/15,
Dev gateincluded).Scope and deviations
A promotion, plus the one CI fix its own gate demanded. Per-feature notes, ADRs and CHANGELOG entries landed with their own PRs on
dev; the CHANGELOG entry that claimed the pg client step "now asserts the major" is corrected here, since the assertion existed and could not pass.Deliberately not done, and flagged instead:
dev-ci.ymlgains no Postgres-client lane here. It is the reason this defect reached a promotion PR, but adding a lane to the fast lane is a CI-policy change, not a promotion — it belongs in its own PR againstdev.CHANGELOG.md's stray model-lineup bullet sits above the "All notable changes" preamble, outside any section, and has since the file was created. Unrelated to this range.CHANGELOG.mdupdated, if this is a user-visible change — the pg-client claim corrected; feature entries landed with their own PRsdocs/<FEATURE>.md) added, if this ships a feature — n/a, landed with their own PRs (docs/DEPLOYMENT-KUBERNETES.md,docs/CODEQL.md, …)