Skip to content

Commit bbce40d

Browse files
committed
fix(ci): match expression contexts and job names case-insensitively in verify-action-pins (modelcontextprotocol#4934)
1 parent 9c0cf8f commit bbce40d

2 files changed

Lines changed: 62 additions & 14 deletions

File tree

‎scripts/verify-action-pins.mjs‎

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ const repoRoot = path.resolve(
8585
// other `secrets` reference, so every spelling of every other name, a dynamic
8686
// index included, still reads as a secret.
8787
const DEFAULT_TOKEN =
88-
/\bsecrets\s*(?:\.\s*GITHUB_TOKEN\b|\[\s*(['"])GITHUB_TOKEN\1\s*\])/g;
88+
/\bsecrets\s*(?:\.\s*GITHUB_TOKEN\b|\[\s*(['"])GITHUB_TOKEN\1\s*\])/gi;
8989
// Where the first expression in a string opens. Everything from there on is
9090
// searched, rather than each `${{ … }}` body: finding where an expression ENDS
9191
// needs a parser, since a quoted string inside one may itself contain `}}`
@@ -124,7 +124,7 @@ function handedSecret(job, inherited) {
124124
const open = text.indexOf(EXPRESSION_OPEN);
125125
return (
126126
open !== -1 &&
127-
/\bsecrets\b/.test(text.slice(open).replace(DEFAULT_TOKEN, ""))
127+
/\bsecrets\b/i.test(text.slice(open).replace(DEFAULT_TOKEN, ""))
128128
);
129129
});
130130
}
@@ -188,6 +188,13 @@ export function credentialedJobs(yaml, file) {
188188
"permissions" in job ? job.permissions : workflow.permissions;
189189
if (mints(permissions) || handedSecret(job, workflow.env)) held.add(name);
190190
}
191+
// Job name lookup without regard to case: GitHub matches needs expressions
192+
// case-insensitively against jobs defined in the workflow.
193+
const jobsByName = new Map(
194+
jobs.map(([name, job]) => [name.toLowerCase(), { name, job }]),
195+
);
196+
const resolveJob = (ref) => jobsByName.get(String(ref).toLowerCase());
197+
191198
// Artifacts are scoped to the run, not to a `needs` edge: a job can download
192199
// what ANY job that finished before it uploaded, which is every job in its
193200
// transitive `needs`. So for `source (uploads) → bridge → publish
@@ -197,12 +204,13 @@ export function credentialedJobs(yaml, file) {
197204
// about the chain.
198205
const upstreamOf = (name) => {
199206
const seen = new Set();
200-
const queue = [...needsOf(workflow.jobs[name])];
207+
const queue = [...needsOf(resolveJob(name)?.job ?? {})];
201208
while (queue.length > 0) {
202-
const next = queue.pop();
203-
if (seen.has(next) || !workflow.jobs[next]) continue;
204-
seen.add(next);
205-
queue.push(...needsOf(workflow.jobs[next]));
209+
const nextRef = queue.pop();
210+
const resolved = resolveJob(nextRef);
211+
if (!resolved || seen.has(resolved.name)) continue;
212+
seen.add(resolved.name);
213+
queue.push(...needsOf(resolved.job));
206214
}
207215
return seen;
208216
};
@@ -213,19 +221,25 @@ export function credentialedJobs(yaml, file) {
213221
// reference that names no job (bare `needs`, `needs.*`, a dynamic index)
214222
// counts every job the reader needs.
215223
const NEEDS_USE =
216-
/\bneeds\b\s*(\.\s*([\w-]+|\*)|\[\s*(?:'([^']*)'|"([^"]*)"|[^\]]*)\s*\])?\s*(?:\.\s*(\w+)|\[\s*['"](\w+)['"]\s*\])?/g;
224+
/\bneeds\b\s*(\.\s*([\w-]+|\*)|\[\s*(?:'([^']*)'|"([^"]*)"|[^\]]*)\s*\])?\s*(?:\.\s*(\w+)|\[\s*['"](\w+)['"]\s*\])?/gi;
217225
const outputsReadBy = (job) => {
218226
const read = new Set();
219227
for (const text of stringsIn(job)) {
220228
const open = text.indexOf(EXPRESSION_OPEN);
221229
if (open === -1) continue;
222230
for (const m of text.slice(open).matchAll(NEEDS_USE)) {
223231
const named = m[2] ?? m[3] ?? m[4];
224-
const property = m[5] ?? m[6];
232+
const property = (m[5] ?? m[6])?.toLowerCase();
225233
if (m[1] !== undefined && property === "result") continue;
226-
if (named === undefined || named === "*")
227-
for (const n of needsOf(job)) read.add(n);
228-
else read.add(named);
234+
if (named === undefined || named === "*") {
235+
for (const n of needsOf(job)) {
236+
const resolved = resolveJob(n);
237+
if (resolved) read.add(resolved.name);
238+
}
239+
} else {
240+
const resolved = resolveJob(named);
241+
if (resolved) read.add(resolved.name);
242+
}
229243
}
230244
}
231245
return read;
@@ -235,14 +249,15 @@ export function credentialedJobs(yaml, file) {
235249
for (const [name, job] of jobs) {
236250
if (!held.has(name)) continue;
237251
for (const source of outputsReadBy(job)) {
238-
if (workflow.jobs[source] && !held.has(source)) {
252+
if (!held.has(source)) {
239253
held.add(source);
240254
grew = true;
241255
}
242256
}
243257
if (!mayDownloadArtifact(job)) continue;
244258
for (const producer of upstreamOf(name)) {
245-
if (!held.has(producer) && mayUploadArtifact(workflow.jobs[producer])) {
259+
const prodJob = resolveJob(producer)?.job;
260+
if (!held.has(producer) && mayUploadArtifact(prodJob)) {
246261
held.add(producer);
247262
grew = true;
248263
}

‎scripts/verify-action-pins.test.mjs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,14 @@ test("every spelling of a non-default secret counts; GITHUB_TOKEN in any spellin
107107
" uses: org/repo/.github/workflows/lint.yml@v1",
108108
" secrets:",
109109
" token: ${{ secrets.NPM_TOKEN }}",
110+
" uppercase-secrets:",
111+
" steps:",
112+
" - env:",
113+
" KEY: ${{ SECRETS.DEPLOY_TOKEN }}",
114+
" uppercase-default-exempt:",
115+
" steps:",
116+
" - env:",
117+
" GH_TOKEN: ${{ Secrets['github_token'] }}",
110118
" default-bracket:",
111119
" steps:",
112120
" - env:",
@@ -121,6 +129,7 @@ test("every spelling of a non-default secret counts; GITHUB_TOKEN in any spellin
121129
"dynamic",
122130
"inherits",
123131
"maps-real",
132+
"uppercase-secrets",
124133
]);
125134
});
126135

@@ -333,6 +342,30 @@ test("outputs read in a job's if, matrix or bracket form count too", () => {
333342
assert.deepEqual([...credentialedJobs(yaml)].sort(), ["a", "b", "publish"]);
334343
});
335344

345+
test("needs context and job names match case-insensitively", () => {
346+
const yaml = wf(
347+
"jobs:",
348+
" detect:",
349+
" runs-on: x",
350+
" build:",
351+
" runs-on: x",
352+
" publish:",
353+
" needs: [detect, build]",
354+
" permissions:",
355+
" id-token: write",
356+
" steps:",
357+
" - env:",
358+
" V1: ${{ NEEDS.detect.outputs.expected }}",
359+
" V2: ${{ needs.BUILD.outputs.artifact }}",
360+
" run: publish",
361+
);
362+
assert.deepEqual([...credentialedJobs(yaml)].sort(), [
363+
"build",
364+
"detect",
365+
"publish",
366+
]);
367+
});
368+
336369
test("needs.*.outputs counts every job the reader needs", () => {
337370
const yaml = wf(
338371
"jobs:",

0 commit comments

Comments
 (0)