Skip to content

Commit 69959a3

Browse files
ci: lint workflows and test the PR check's key-sharing decision
The PR check workflow decides whether a pull request gets this repository's Vapi keys. That decision was untested bash, and no workflow had ever been through actionlint. - tests/vapi-checks-workflow.test.ts runs the workflow's real steps from vapi-checks.yml with bash: the live-or-dry-run decision across own branches, manual runs, forks (including a look-alike repo name), Dependabot as actor or author, a missing head repo and other events; the run step's arguments (dry vs live, named check, merge-base selection, with and without a base ref) using a stub node; empty key variables being unset; and statically, that secrets are gated on a live run, the triggers are pull_request and workflow_dispatch only, the permissions are contents: read and statuses: write, and checkout doesn't persist credentials. Dropping the Dependabot-author guard fails the decision test. - The decision step now also requires a pull_request event before going live, so adding a trigger later can't widen who gets the keys. - ci.yml gets an actionlint job: a pinned release whose sha256 is checked before it runs, with shellcheck linting every run: block. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 5301080 commit 69959a3

3 files changed

Lines changed: 349 additions & 1 deletion

File tree

‎.github/workflows/ci.yml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,29 @@ jobs:
3838
- run: npm run build
3939

4040
- run: npm test
41+
42+
actionlint:
43+
name: Lint workflows (actionlint)
44+
runs-on: ubuntu-latest
45+
timeout-minutes: 5
46+
env:
47+
# Pinned version and the sha256 published with that release, so the
48+
# linter can't change underneath us. shellcheck (preinstalled on the
49+
# runner) lints the bash in every `run:` step too.
50+
ACTIONLINT_VERSION: 1.7.12
51+
ACTIONLINT_SHA256: 8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8
52+
53+
steps:
54+
- uses: actions/checkout@v4
55+
56+
- name: Install actionlint
57+
run: |
58+
set -euo pipefail
59+
tarball="actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz"
60+
curl -fsSL -o "$tarball" \
61+
"https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}/${tarball}"
62+
echo "${ACTIONLINT_SHA256} ${tarball}" | sha256sum --check --strict
63+
tar -xzf "$tarball" actionlint
64+
65+
- name: Lint workflows
66+
run: ./actionlint -color

‎.github/workflows/vapi-checks.yml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ jobs:
7373
live=false
7474
if [[ "$EVENT" == "workflow_dispatch" ]]; then
7575
live=true
76-
elif [[ "$HEAD_REPO" == "$GITHUB_REPOSITORY" && "$ACTOR" != "dependabot[bot]" && "$AUTHOR" != "dependabot[bot]" ]]; then
76+
elif [[ "$EVENT" == "pull_request" && -n "$HEAD_REPO" && "$HEAD_REPO" == "$GITHUB_REPOSITORY" &&
77+
"$ACTOR" != "dependabot[bot]" && "$AUTHOR" != "dependabot[bot]" ]]; then
7778
live=true
7879
fi
7980
echo "live=$live" >> "$GITHUB_OUTPUT"

‎tests/vapi-checks-workflow.test.ts‎

Lines changed: 321 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,321 @@
1+
import assert from "node:assert/strict";
2+
import { execFileSync, spawnSync } from "node:child_process";
3+
import {
4+
chmodSync,
5+
mkdirSync,
6+
mkdtempSync,
7+
readFileSync,
8+
rmSync,
9+
writeFileSync,
10+
} from "node:fs";
11+
import { tmpdir } from "node:os";
12+
import { join } from "node:path";
13+
import test from "node:test";
14+
import { fileURLToPath } from "node:url";
15+
import { parse as parseYaml } from "yaml";
16+
17+
// The PR check workflow decides whether a pull request gets this
18+
// repository's Vapi keys. Forks and Dependabot must never get them. These
19+
// tests run the workflow's real steps (read from vapi-checks.yml, run with
20+
// bash) so an edit that weakens the decision fails CI.
21+
22+
const REPO = fileURLToPath(new URL("..", import.meta.url));
23+
const OWN_REPO = "acme/gitops";
24+
25+
interface Step {
26+
name?: string;
27+
run?: string;
28+
env?: Record<string, string>;
29+
uses?: string;
30+
with?: Record<string, unknown>;
31+
}
32+
33+
interface Workflow {
34+
on: Record<string, unknown>;
35+
permissions: Record<string, string>;
36+
jobs: { "vapi-checks": { steps: Step[] } };
37+
}
38+
39+
const WORKFLOW = parseYaml(
40+
readFileSync(join(REPO, ".github/workflows/vapi-checks.yml"), "utf8"),
41+
) as Workflow;
42+
const STEPS = WORKFLOW.jobs["vapi-checks"].steps;
43+
const step = (name: string) => STEPS.find((s) => s.name === name)!;
44+
45+
interface PullRequest {
46+
event: string;
47+
headRepo?: string;
48+
actor?: string;
49+
author?: string;
50+
}
51+
52+
// Run the "Choose live or dry run" step and return its `live` output.
53+
function liveDecision(pr: PullRequest): string {
54+
const dir = mkdtempSync(join(tmpdir(), "vapi-checks-mode-"));
55+
try {
56+
const output = join(dir, "output");
57+
writeFileSync(output, "");
58+
const result = spawnSync(
59+
"bash",
60+
["-e", "-c", step("Choose live or dry run").run!],
61+
{
62+
encoding: "utf8",
63+
env: {
64+
PATH: process.env.PATH,
65+
GITHUB_OUTPUT: output,
66+
GITHUB_REPOSITORY: OWN_REPO,
67+
EVENT: pr.event,
68+
HEAD_REPO: pr.headRepo ?? "",
69+
ACTOR: pr.actor ?? "",
70+
AUTHOR: pr.author ?? "",
71+
},
72+
},
73+
);
74+
assert.equal(result.status, 0, result.stderr);
75+
return readFileSync(output, "utf8").trim();
76+
} finally {
77+
rmSync(dir, { recursive: true, force: true });
78+
}
79+
}
80+
81+
test("only same-repository, non-Dependabot pull requests and manual runs go live", () => {
82+
const cases: Array<[string, PullRequest, string]> = [
83+
[
84+
"own branch",
85+
{
86+
event: "pull_request",
87+
headRepo: OWN_REPO,
88+
actor: "dev",
89+
author: "dev",
90+
},
91+
"live=true",
92+
],
93+
[
94+
"manual dispatch",
95+
{ event: "workflow_dispatch", actor: "maintainer" },
96+
"live=true",
97+
],
98+
[
99+
"fork",
100+
{
101+
event: "pull_request",
102+
headRepo: "someone/gitops",
103+
actor: "someone",
104+
author: "someone",
105+
},
106+
"live=false",
107+
],
108+
[
109+
"fork with a look-alike name",
110+
{
111+
event: "pull_request",
112+
headRepo: "acme/gitops-fork",
113+
actor: "x",
114+
author: "x",
115+
},
116+
"live=false",
117+
],
118+
[
119+
"Dependabot",
120+
{
121+
event: "pull_request",
122+
headRepo: OWN_REPO,
123+
actor: "dependabot[bot]",
124+
author: "dependabot[bot]",
125+
},
126+
"live=false",
127+
],
128+
[
129+
"a maintainer re-running Dependabot's PR",
130+
{
131+
event: "pull_request",
132+
headRepo: OWN_REPO,
133+
actor: "maintainer",
134+
author: "dependabot[bot]",
135+
},
136+
"live=false",
137+
],
138+
[
139+
"Dependabot pushing to a maintainer's PR",
140+
{
141+
event: "pull_request",
142+
headRepo: OWN_REPO,
143+
actor: "dependabot[bot]",
144+
author: "dev",
145+
},
146+
"live=false",
147+
],
148+
[
149+
"missing head repository",
150+
{ event: "pull_request", actor: "dev", author: "dev" },
151+
"live=false",
152+
],
153+
[
154+
"any other event",
155+
{
156+
event: "pull_request_target",
157+
headRepo: OWN_REPO,
158+
actor: "dev",
159+
author: "dev",
160+
},
161+
"live=false",
162+
],
163+
];
164+
assert.deepEqual(
165+
cases.map(([label, pr]) => [label, liveDecision(pr)]),
166+
cases.map(([label, , expected]) => [label, expected]),
167+
);
168+
});
169+
170+
interface RunArgs {
171+
live: boolean;
172+
check?: string;
173+
event?: string;
174+
baseRefExists?: boolean;
175+
checkTokens?: string;
176+
privateKey?: string;
177+
}
178+
179+
// Run the "Run Vapi checks" step with a stub `node` that records its
180+
// arguments and which key variables it received.
181+
function runStep(args: RunArgs): {
182+
argv: string[];
183+
tokens: string;
184+
key: string;
185+
} {
186+
const dir = mkdtempSync(join(tmpdir(), "vapi-checks-run-"));
187+
try {
188+
const bin = join(dir, "bin");
189+
mkdirSync(bin);
190+
const record = join(dir, "record");
191+
writeFileSync(
192+
join(bin, "node"),
193+
`#!/usr/bin/env bash\nprintf '%s\\n' "$@" > "${record}"\necho "TOKENS=\${VAPI_CHECK_TOKENS-unset}" >> "${record}.env"\necho "KEY=\${VAPI_PRIVATE_API_KEY-unset}" >> "${record}.env"\n`,
194+
);
195+
chmodSync(join(bin, "node"), 0o755);
196+
const repo = join(dir, "repo");
197+
mkdirSync(repo);
198+
const git = (...a: string[]) =>
199+
execFileSync(
200+
"git",
201+
["-c", "user.name=t", "-c", "user.email=t@example.com", ...a],
202+
{ cwd: repo, stdio: "pipe" },
203+
);
204+
git("init", "-q", "-b", "feature");
205+
git("commit", "-q", "--allow-empty", "-m", "base");
206+
if (args.baseRefExists ?? true)
207+
git("update-ref", "refs/remotes/origin/main", "HEAD");
208+
const result = spawnSync(
209+
"bash",
210+
["-e", "-c", step("Run Vapi checks").run!],
211+
{
212+
cwd: repo,
213+
encoding: "utf8",
214+
env: {
215+
PATH: `${bin}:${process.env.PATH}`,
216+
LIVE: String(args.live),
217+
CHECK: args.check ?? "",
218+
GITHUB_EVENT_NAME: args.event ?? "pull_request",
219+
BASE_REF: "main",
220+
VAPI_CHECK_TOKENS: args.checkTokens ?? "",
221+
VAPI_PRIVATE_API_KEY: args.privateKey ?? "",
222+
},
223+
},
224+
);
225+
assert.equal(result.status, 0, result.stderr);
226+
const env = readFileSync(`${record}.env`, "utf8");
227+
return {
228+
argv: readFileSync(record, "utf8").trim().split("\n"),
229+
tokens: env.match(/TOKENS=(.*)/)![1]!,
230+
key: env.match(/KEY=(.*)/)![1]!,
231+
};
232+
} finally {
233+
rmSync(dir, { recursive: true, force: true });
234+
}
235+
}
236+
237+
test("a dry run builds payloads only; a live run refreshes bindings; PRs select changed checks", () => {
238+
const dry = runStep({ live: false });
239+
const live = runStep({ live: true, checkTokens: '{"acme":"k"}' });
240+
const named = runStep({
241+
live: true,
242+
check: "core",
243+
event: "workflow_dispatch",
244+
privateKey: "k",
245+
});
246+
const noBase = runStep({ live: false, baseRefExists: false });
247+
const tail = ["--budget-minutes", "22"];
248+
assert.deepEqual(
249+
{ dry: dry.argv, live: live.argv, named: named.argv, noBase: noBase.argv },
250+
{
251+
dry: [
252+
"--import",
253+
"tsx",
254+
"src/check-cmd.ts",
255+
"--all",
256+
"--dry-run",
257+
"--changed-since",
258+
"origin/main",
259+
...tail,
260+
],
261+
live: [
262+
"--import",
263+
"tsx",
264+
"src/check-cmd.ts",
265+
"--all",
266+
"--refresh-bindings",
267+
"--changed-since",
268+
"origin/main",
269+
...tail,
270+
],
271+
named: [
272+
"--import",
273+
"tsx",
274+
"src/check-cmd.ts",
275+
"core",
276+
"--refresh-bindings",
277+
...tail,
278+
],
279+
noBase: [
280+
"--import",
281+
"tsx",
282+
"src/check-cmd.ts",
283+
"--all",
284+
"--dry-run",
285+
...tail,
286+
],
287+
},
288+
);
289+
});
290+
291+
test("empty key variables are unset rather than passed as empty strings", () => {
292+
const dry = runStep({ live: false });
293+
const live = runStep({ live: true, checkTokens: '{"acme":"k"}' });
294+
assert.deepEqual(
295+
[dry.tokens, dry.key, live.tokens, live.key],
296+
["unset", "unset", '{"acme":"k"}', "unset"],
297+
);
298+
});
299+
300+
test("secrets reach the run step only for live runs, and the workflow never runs fork code with secrets", () => {
301+
const env = step("Run Vapi checks").env!;
302+
const checkout = STEPS.find((s) => s.uses?.startsWith("actions/checkout"))!;
303+
assert.deepEqual(
304+
{
305+
checkTokens: env.VAPI_CHECK_TOKENS,
306+
privateKey: env.VAPI_PRIVATE_API_KEY,
307+
triggers: Object.keys(WORKFLOW.on).sort(),
308+
permissions: WORKFLOW.permissions,
309+
persistCredentials: checkout.with?.["persist-credentials"],
310+
},
311+
{
312+
checkTokens:
313+
"${{ steps.mode.outputs.live == 'true' && secrets.VAPI_CHECK_TOKENS || '' }}",
314+
privateKey:
315+
"${{ steps.mode.outputs.live == 'true' && secrets.VAPI_PRIVATE_API_KEY || '' }}",
316+
triggers: ["pull_request", "workflow_dispatch"],
317+
permissions: { contents: "read", statuses: "write" },
318+
persistCredentials: false,
319+
},
320+
);
321+
});

0 commit comments

Comments
 (0)