From a272d75068fbe7d9d39a09e08b482b63e0d9f586 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Wed, 5 Aug 2026 20:16:58 -0700 Subject: [PATCH] fix(drift): pin the Ollama artifact the drift job unpacks as root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test-drift.yml's provisioning step fetched https://ollama.com/install.sh and ran it with no digest check of any kind. Executing that step's own run: body verbatim with curl serving substituted content, `sh` was handed "ATTACKER-CONTROLLED PAYLOAD" and the step exited 0. The step holds no provider key itself, and that protects nothing. It runs before `Run drift tests`, which is handed OPENAI_API_KEY, ANTHROPIC_API_KEY, GOOGLE_API_KEY, OPENROUTER_API_KEY, FAL_KEY, COHERE_API_KEY and ELEVENLABS_API_KEY. A payload that cannot read those keys can still plant a node/npx/git earlier on PATH, and the later key-holding steps execute it. Pinning install.sh would not have been enough — proven in fix-drift.yml's own writeup: the script streams an unversioned, undigested ollama-linux-.tar.zst through zstd -d into sudo tar -x, and it cannot be fixed in place because it never holds the file. So install.sh is not used. The release artifact is fetched from an immutable release tag and its sha256 checked before anything unpacks it, mirroring the shape fix-drift.yml is getting in #359. Digest dec2fa50…aadcfc for ollama-linux-amd64.tar.zst v0.32.6 (1420686963 bytes), agreed by three independent sources verified here: the release's own sha256sum.txt, the GitHub release API's asset digest field, and sha256 of the downloaded bytes. Archive layout confirmed by listing it — 54 entries, bin/ollama plus lib/ollama/*, so -C /usr/local puts the binary on the default PATH. This is NOT the last unpinned executable in the repo, and the new guard does not claim otherwise. Still unpinned and not fixed here: `npx pkg-pr-new publish` (publish-commit.yml) fetches an unlockfiled npm package and runs it; `pip install hatch` (publish-release.yml) runs an unpinned PyPI package in the job that holds PyPI OIDC publish rights; `pip install ./packages/aimock-pytest[test]` (test-pytest.yml) resolves unpinned transitive deps; and in this job the setup-node and pnpm/action-setup toolchain downloads carry no committed digest. fix-drift.yml:90 has the same Ollama defect in its weaker form and is #359's. New guard executes the step rather than reading it: tampered bytes must not reach `sh` or `tar`, and a positive control asserts tar was handed exactly the verified bytes. Three mutations confirmed red — neutering the digest comparison, pointing the fetch back at install.sh, and emptying the pin. --- .github/workflows/test-drift.yml | 76 ++++- src/__tests__/test-drift-workflow.test.ts | 373 ++++++++++++++++++++++ 2 files changed, 445 insertions(+), 4 deletions(-) create mode 100644 src/__tests__/test-drift-workflow.test.ts diff --git a/.github/workflows/test-drift.yml b/.github/workflows/test-drift.yml index 1b41346d..3123844c 100644 --- a/.github/workflows/test-drift.yml +++ b/.github/workflows/test-drift.yml @@ -169,13 +169,81 @@ jobs: # chat+generate-capable model (qwen2:0.5b, ~350MB) and point the leg at it # via OLLAMA_MODEL to keep the added cost minimal. COST: the install + model # pull adds ~2-4 min to every drift run. + # + # EVERY BYTE THIS STEP EXECUTES IS CHECKSUM-PINNED, and that is the whole + # point of the shape below. + # + # This step holds no provider key of its own, and that protects nothing. + # It runs BEFORE `Run drift tests`, which is handed OPENAI_API_KEY, + # ANTHROPIC_API_KEY, GOOGLE_API_KEY, OPENROUTER_API_KEY, FAL_KEY, + # COHERE_API_KEY and ELEVENLABS_API_KEY. Third-party bytes unpacked as root + # into /usr/local plant a `node`/`npx`/`git` earlier on PATH — or a shell rc + # — and the later, key-holding steps then execute them. Reordering cannot + # close that; only refusing to unpack unverified bytes can. + # + # ollama.com/install.sh IS NOT USED, and pinning its bytes would not have + # been enough. The script streams + # `https://ollama.com/download/ollama-linux-.tar.zst` — a mutable, + # unversioned URL carrying no digest — straight into `sudo tar -x` under + # /usr/local, so a SECOND unpinned payload plants root-owned binaries just + # as effectively. It cannot be fixed in place either: the script pipes that + # download through `zstd -d` into `tar`, so it never holds the file and has + # nothing to verify. (On a GPU host it also adds NVIDIA CUDA apt/yum repos + # and runs `$PACKAGE_MANAGER -y install` — more unpinned root execution this + # job has no use for.) Setting OLLAMA_VERSION on that script only appends a + # `?version=…` query param: a version pin, not a byte pin. + # + # So the release artifact is fetched DIRECTLY and verified before anything + # unpacks it: one URL, pinned to an immutable release tag, whose bytes must + # match a reviewed sha256 or the step hard-`exit 1`s before `tar` runs. The + # tarball is the whole product — `bin/ollama` plus `lib/ollama/*` — and this + # step already ran `ollama serve` itself rather than using the systemd unit + # install.sh sets up, so nothing else in that script was load-bearing here. + # + # WHAT IS STILL NOT BYTE-PINNED IN THIS JOB, honestly: the toolchain + # installers. `actions/setup-node` downloads a Node distribution and + # `pnpm/action-setup` fetches pnpm at the version named by package.json's + # `packageManager` field (`pnpm@10.28.2` — a version, with no integrity + # hash beside it). Both actions are themselves SHA-pinned and both resolve + # through registries that serve their own checksums, but neither digest is + # committed here, so neither is pinned to the same standard as this step. + # Everything else the job executes is: each `uses:` by commit SHA, + # `pnpm install --frozen-lockfile` by the lockfile's integrity hashes, and + # every `npx` invocation resolves from that installed tree (`tsx` and + # `vitest` are both devDependencies, so nothing is fetched at call time). + # + # WHEN THIS FAILS: Ollama cut a new release, or an artifact was rebuilt. + # Pick the version at https://github.com/ollama/ollama/releases, take + # `ollama-linux-amd64.tar.zst`'s digest from that release's own + # `sha256sum.txt`, and update BOTH values below together. The failure is + # loud and reds the run — unverified bytes must not be unpacked as root + # just because verifying them was inconvenient. - name: Provision Ollama daemon (live drift leg) + env: + # ollama-linux-amd64.tar.zst from the v0.32.6 release, 1420686963 bytes. + # Digest agreed on 2026-08-05 by three independent sources: the release's + # sha256sum.txt, the GitHub release API's own asset `digest` field, and + # sha256 of the downloaded bytes. + OLLAMA_VERSION: v0.32.6 + OLLAMA_TARBALL_SHA256: dec2fa50d24e6868ca3c4c977d69d059399372105f951a9acc320a5a79aadcfc run: | set -euo pipefail - # Download the installer to disk first, then execute it — avoids piping - # a remote, mutable script straight into a shell (no `curl | sh`). - curl -fsSL https://ollama.com/install.sh -o "${RUNNER_TEMP}/ollama-install.sh" - sh "${RUNNER_TEMP}/ollama-install.sh" + # Fail on a MISSING decompressor rather than discovering it mid-pipe, + # where `tar` would be handed a truncated stream as root. + if ! command -v zstd >/dev/null 2>&1; then + echo "::error::zstd is not installed on this runner, so the pinned Ollama tarball cannot be unpacked. Install zstd before this step." + exit 1 + fi + TARBALL="${RUNNER_TEMP}/ollama-linux-amd64.tar.zst" + curl -fsSL "https://github.com/ollama/ollama/releases/download/${OLLAMA_VERSION}/ollama-linux-amd64.tar.zst" -o "$TARBALL" + ACTUAL="$(sha256sum "$TARBALL" | cut -d' ' -f1)" + if [ "$ACTUAL" != "${OLLAMA_TARBALL_SHA256}" ]; then + echo "::error::the Ollama ${OLLAMA_VERSION} tarball does not match its pinned sha256 — REFUSING to unpack it as root. Expected ${OLLAMA_TARBALL_SHA256}, got ${ACTUAL}. Check the release's sha256sum.txt and re-pin OLLAMA_VERSION/OLLAMA_TARBALL_SHA256 in .github/workflows/test-drift.yml if the change is legitimate." + exit 1 + fi + # Only now, on bytes that matched. The archive is `bin/ollama` + + # `lib/ollama/*`, so /usr/local puts the binary on the default PATH. + zstd -d -c "$TARBALL" | sudo tar -xf - -C /usr/local ollama serve > /tmp/ollama-serve.log 2>&1 & for _ in $(seq 1 30); do if curl -sf http://127.0.0.1:11434/api/version >/dev/null 2>&1; then diff --git a/src/__tests__/test-drift-workflow.test.ts b/src/__tests__/test-drift-workflow.test.ts new file mode 100644 index 00000000..c9ab5bf9 --- /dev/null +++ b/src/__tests__/test-drift-workflow.test.ts @@ -0,0 +1,373 @@ +/** + * Assertions on the Ollama provisioning step of .github/workflows/test-drift.yml. + * + * WHAT THIS GUARDS. The `drift` job provisions a local Ollama daemon so the + * OLLAMA_HOST-gated live leg runs. That provisioning is the only place in the + * job that fetches third-party BYTES over the network and executes them. Every + * other executable in the job is already pinned: each `uses:` by commit SHA, and + * `pnpm install --frozen-lockfile` by the lockfile's own integrity hashes. + * + * WHY IT MATTERS HERE. The step holds no provider key of its own, and that is + * NOT the property that protects the keys. It runs BEFORE `Preflight — provider + * key freshness`'s successor steps and before `Run drift tests`, which is handed + * OPENAI_API_KEY, ANTHROPIC_API_KEY, GOOGLE_API_KEY, OPENROUTER_API_KEY, FAL_KEY, + * COHERE_API_KEY and ELEVENLABS_API_KEY. Unverified bytes unpacked as root into + * /usr/local plant a `node`/`npx`/`git` earlier on PATH (or a shell rc) that the + * later, key-holding steps then execute. Ordering cannot fix that; only refusing + * to unpack unverified bytes can. + * + * WHY THE QUESTION IS ABOUT `tar` AND `sh`, NOT ABOUT `curl`. A step that + * "fails" AFTER handing a payload to an executor has refused nothing. The + * harness below stubs both executors this step could reach — `sh` (the old + * install-script path) and `tar` (the extractor) — and records WHAT each was + * handed, so a refusal is the demonstrated ABSENCE of a payload at an executor + * rather than an inference from an exit code. + * + * PINNING `ollama.com/install.sh` WOULD NOT BE SUFFICIENT, and this file + * deliberately does not ask for it. That script streams an unversioned, + * undigested `ollama-linux-.tar.zst` through `zstd -d` into `sudo tar -x`; + * run verbatim out of the script's own reviewed bytes on 2026-08-05, attacker- + * supplied content reached `sudo tar -xf - -C ` and the script exited 0. + * It cannot be fixed in place either — the script never holds the file, so it + * has nothing to verify. So the release artifact is fetched directly from an + * immutable release tag and digest-checked before anything unpacks it. + * + * The repo ships no YAML dependency and this suite adds none; the parser below + * is scoped to one job's `steps:` sequence, and actionlint covers structural + * validity separately in CI. + */ +import { createHash } from "node:crypto"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { spawnSync } from "node:child_process"; + +import { describe, expect, it } from "vitest"; + +const WORKFLOW = resolve(__dirname, "../../.github/workflows/test-drift.yml"); +const wf = readFileSync(WORKFLOW, "utf8"); + +const OLLAMA_STEP = "Provision Ollama daemon (live drift leg)"; +const DRIFT_JOB = "drift"; +const KEY_STEP = "Run drift tests"; + +interface Step { + name?: string; + id?: string; + uses?: string; + run: string; + env: Record; +} + +const indentOf = (l: string): number => l.length - l.trimStart().length; + +/** + * The `steps:` sequence of ONE job. + * + * Job-scoped on purpose: test-drift.yml has four jobs, and a whole-file scan + * would silently answer with `agui-schema-drift`'s steps — the first `steps:` + * key in the file — for every question asked about `drift`. A locator that + * cannot find its job THROWS rather than yielding an empty or wrong slice. + */ +function stepsOfJob(job: string, src: string = wf): Step[] { + const lines = src.split("\n"); + const jobIdx = lines.findIndex((l) => l === ` ${job}:`); + if (jobIdx === -1) throw new Error(`test-drift.yml: no job \`${job}\``); + let jobEnd = lines.length; + for (let i = jobIdx + 1; i < lines.length; i++) { + if (lines[i].trim() && indentOf(lines[i]) <= 2) { + jobEnd = i; + break; + } + } + const body = lines.slice(jobIdx, jobEnd); + + const stepsIdx = body.findIndex((l) => /^\s+steps:\s*$/.test(l)); + if (stepsIdx === -1) throw new Error(`test-drift.yml: job \`${job}\` has no \`steps:\``); + const firstItem = body.findIndex((l, i) => i > stepsIdx && /^\s+- \S/.test(l)); + if (firstItem === -1) throw new Error(`test-drift.yml: job \`${job}\` \`steps:\` is empty`); + const itemIndent = indentOf(body[firstItem]); + const keyIndent = itemIndent + 2; + + const starts: number[] = []; + let end = body.length; + for (let i = firstItem; i < body.length; i++) { + const l = body[i]; + if (!l.trim()) continue; + if (indentOf(l) < itemIndent) { + end = i; + break; + } + if (indentOf(l) === itemIndent && /^\s+- \S/.test(l)) starts.push(i); + } + + return starts.map((from, n) => { + const to = n + 1 < starts.length ? starts[n + 1] : end; + // Normalise `- key: value` to ` key: value` so every key sits at keyIndent. + const item = body.slice(from, to).map((l, i) => (i === 0 ? l.replace(/- /, " ") : l)); + + const step: Step = { env: {}, run: "" }; + for (let i = 0; i < item.length; i++) { + const l = item[i]; + if (!l.trim() || indentOf(l) !== keyIndent) continue; + const m = /^\s*([A-Za-z_-]+):\s*(.*)$/.exec(l); + if (!m) continue; + const [, key, inline] = m; + + if (key === "run") { + // An INLINE `run:` is a run body too — dropping it would make a + // one-liner step invisible to every guard that reads step bodies. + if (!/^[|>]/.test(inline) && inline !== "") { + step.run = `${inline}\n`; + continue; + } + const child: string[] = []; + let blockIndent = -1; + for (let j = i + 1; j < item.length; j++) { + if (!item[j].trim()) { + child.push(""); + continue; + } + if (blockIndent === -1) blockIndent = indentOf(item[j]); + if (indentOf(item[j]) < blockIndent) break; + child.push(item[j].slice(blockIndent)); + } + while (child.length && child[child.length - 1] === "") child.pop(); + step.run = `${child.join("\n")}\n`; + continue; + } + + if (key === "env") { + for (let j = i + 1; j < item.length; j++) { + if (!item[j].trim()) continue; + if (indentOf(item[j]) <= keyIndent) break; + const em = /^\s*([A-Za-z_][A-Za-z0-9_]*):\s*(.*)$/.exec(item[j]); + if (em) step.env[em[1]] = em[2]; + } + continue; + } + + if (key === "name") step.name = inline.replace(/^["']|["']$/g, ""); + if (key === "id") step.id = inline.replace(/^["']|["']$/g, ""); + if (key === "uses") step.uses = inline.trim(); + } + return step; + }); +} + +function stepByName(name: string, job: string = DRIFT_JOB): Step { + const hits = stepsOfJob(job).filter((s) => s.name === name); + if (hits.length !== 1) + throw new Error(`test-drift.yml: ${hits.length} steps named \`${name}\` in job \`${job}\``); + return hits[0]; +} + +/** + * A step's run body with shell COMMENT lines removed. + * + * Every "does this step do X" question has to be asked of the code, not the + * prose: this step's rationale comment names both `install.sh` and `tar`, so a + * guard that greps the whole body answers about the comment. + */ +const codeOf = (s: Step): string => + s.run + .split("\n") + .filter((l) => !/^\s*#/.test(l)) + .join("\n"); + +interface Provisioned { + stepExit: number; + executorRan: boolean; + /** Exactly the bytes the executor was handed. */ + executorSaw: string; + /** Which executor was reached — `sh` or `tar`. */ + executor: string; + stdio: string; +} + +/** + * EXECUTE the provisioning step's own `run:` body with `curl` serving `served`. + * + * Only the network and the executors are stubbed. The verification itself — + * `sha256sum`, the comparison, the exit — is the workflow's code, run as + * written. BOTH executors this step could reach are stubbed and both write the + * same marker, so the question "did unverified bytes reach something that runs + * them" is answered the same way whether the step shells a script or unpacks an + * archive. + */ +const observeProvision = (served: string, expectedSha256?: string): Provisioned => { + const dir = mkdtempSync(join(tmpdir(), "test-drift-ollama-")); + try { + const bin = join(dir, "bin"); + mkdirSync(bin); + const servedFile = join(dir, "served"); + writeFileSync(servedFile, served); + const sawFile = join(dir, "executor-saw"); + const whichFile = join(dir, "executor-which"); + + // `-o ` is the only curl form this step uses for a download; the + // readiness poll (`curl -sf http://127.0.0.1:11434/...`) has no `-o` and + // must simply fail so the wait loop falls through. + writeFileSync( + join(bin, "curl"), + [ + "#!/bin/sh", + 'out=""', + 'while [ $# -gt 0 ]; do case "$1" in -o) out="$2"; shift;; esac; shift; done', + '[ -n "$out" ] || exit 7', + `cat ${JSON.stringify(servedFile)} > "$out"`, + ].join("\n"), + { mode: 0o755 }, + ); + // `sh