diff --git a/.changeset/checkout-success-conditions.md b/.changeset/checkout-success-conditions.md new file mode 100644 index 00000000..1ecaff29 --- /dev/null +++ b/.changeset/checkout-success-conditions.md @@ -0,0 +1,5 @@ +--- +"@stll/oxlint-config": patch +--- + +Recognize reviewed checkout consumers whose status-overriding condition proves the selected checkout completed successfully. diff --git a/packages/oxlint-config/README.md b/packages/oxlint-config/README.md index cb36f31c..031fa6ff 100644 --- a/packages/oxlint-config/README.md +++ b/packages/oxlint-config/README.md @@ -280,7 +280,9 @@ checkout condition must match exactly; a conditional checkout requires the consumer to have the identical condition with immutable operands. Step-output references must name a unique producer before the checkout; mutable contexts, status functions and dynamic property access are rejected. Consumers of an -unconditional checkout also cannot override its implicit success requirement. The selected checkout must precede +unconditional checkout may override implicit success only when every condition +branch requires `steps..outcome == 'success'`. This allows +post-failure checks to retain their source identity. The selected checkout must precede the consumer, use the approved checkout pin and the same repository, retain cleaning, and omit `continue-on-error`. A later overlapping or unknown checkout destination before the consumer invalidates the proof. An earlier writer is diff --git a/packages/oxlint-config/src/toolchain-checkout-sources.test.ts b/packages/oxlint-config/src/toolchain-checkout-sources.test.ts index 313745c7..7ee6faab 100644 --- a/packages/oxlint-config/src/toolchain-checkout-sources.test.ts +++ b/packages/oxlint-config/src/toolchain-checkout-sources.test.ts @@ -398,3 +398,23 @@ test("directory-only sparse equality covers a local action directory", () => { ]), ).toBe(true); }); + +test("status-overriding consumers must prove the selected checkout succeeded on every branch", () => { + const succeeded = "steps.source.outcome == 'success'"; + for (const condition of [ + `${succeeded} && !cancelled()`, + `always() && (${succeeded})`, + `(${succeeded} && inputs.one) || (${succeeded} && inputs.two)`, + `\${{ !cancelled() && (${succeeded}) && steps.install.outcome == 'success' }}`, + ]) + expect(prove([checkout, { ...consumer, if: condition }])).toBe(true); + for (const condition of [ + `${succeeded} || always()`, + `!(${succeeded}) && always()`, + `always() && steps.other.outcome == 'success'`, + `always() && steps.source.conclusion == 'success'`, + `always() && ('${succeeded.replaceAll("'", "''")}')`, + `always() && ((${succeeded} && inputs.one) || inputs.two)`, + ]) + expect(prove([checkout, { ...consumer, if: condition }])).toBe(false); +}); diff --git a/packages/oxlint-config/src/toolchain-checkout-sources.ts b/packages/oxlint-config/src/toolchain-checkout-sources.ts index fb1119e1..56654edf 100644 --- a/packages/oxlint-config/src/toolchain-checkout-sources.ts +++ b/packages/oxlint-config/src/toolchain-checkout-sources.ts @@ -8,6 +8,7 @@ import { selfCheckoutRepository, sparseCheckoutPaths, } from "./toolchain-checkout-syntax"; +import { conditionRequires } from "./toolchain-conditions"; export type CheckoutSource = { path: string; @@ -215,7 +216,14 @@ export const proveCheckoutSource = ({ declaration.condition === null && consumerCondition !== undefined && (typeof consumerCondition !== "string" || - !stableCheckoutCondition(consumerCondition, steps, consumerIndex)) + (!stableCheckoutCondition(consumerCondition, steps, consumerIndex) && + !conditionRequires(consumerCondition, (source) => { + const match = + /^steps\.([A-Za-z_][\w-]*)\.outcome\s*==\s*'success'$/i.exec( + source, + ); + return match?.at(1) === declaration.checkout; + }))) ) return false; const selected = normalizeCheckoutPath(selector); diff --git a/packages/oxlint-config/src/toolchain-conditions.ts b/packages/oxlint-config/src/toolchain-conditions.ts new file mode 100644 index 00000000..e1a214ad --- /dev/null +++ b/packages/oxlint-config/src/toolchain-conditions.ts @@ -0,0 +1,64 @@ +// Split only boolean operators outside parentheses and quoted string literals. +const booleanParts = (source: string, operator: "&&" | "||") => { + const parts: string[] = []; + let depth = 0; + let quoted = false; + let start = 0; + for (let index = 0; index < source.length; index++) { + const character = source[index]; + if (character === "'") { + if (quoted && source[index + 1] === "'") index++; + else quoted = !quoted; + } + if (quoted) continue; + if (character === "(") depth++; + if (character === ")") depth--; + if (depth === 0 && source.slice(index, index + 2) === operator) { + parts.push(source.slice(start, index)); + start = index + 2; + index++; + } + } + parts.push(source.slice(start)); + return parts; +}; + +const withoutOuterParentheses = (source: string): string => { + if (!source.startsWith("(") || !source.endsWith(")")) return source; + let depth = 0; + let quoted = false; + for (let index = 0; index < source.length; index++) { + const character = source[index]; + if (character === "'") { + if (quoted && source[index + 1] === "'") index++; + else quoted = !quoted; + } + if (quoted) continue; + if (character === "(") depth++; + if (character === ")") depth--; + if (depth === 0 && index !== source.length - 1) return source; + } + return withoutOuterParentheses(source.slice(1, -1).trim()); +}; + +/** Proves a necessary leaf predicate across every disjunction; unknown atoms never authorize it. */ +export const conditionRequires = ( + condition: unknown, + matches: (source: string) => boolean, +): boolean => { + if (condition === false) return true; + if (typeof condition !== "string") return false; + const source = withoutOuterParentheses( + condition + .trim() + .replace(/^\$\{\{([\s\S]*)\}\}$/u, "$1") + .trim(), + ); + const alternatives = booleanParts(source, "||"); + if (alternatives.length > 1) + return alternatives.every((part) => conditionRequires(part, matches)); + const conjuncts = booleanParts(source, "&&"); + if (conjuncts.length > 1) + return conjuncts.some((part) => conditionRequires(part, matches)); + return source === "false" || matches(source); +}; diff --git a/packages/oxlint-config/src/toolchain-workflow-trust.ts b/packages/oxlint-config/src/toolchain-workflow-trust.ts index a7045649..036f3fb2 100644 --- a/packages/oxlint-config/src/toolchain-workflow-trust.ts +++ b/packages/oxlint-config/src/toolchain-workflow-trust.ts @@ -1,3 +1,4 @@ +import { conditionRequires } from "./toolchain-conditions"; import { decodeShellWord, shellCommandSegments, @@ -7,49 +8,6 @@ import { const record = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value); -// Split only boolean operators outside parentheses and quoted string literals. -const booleanParts = (source: string, operator: "&&" | "||") => { - const parts: string[] = []; - let depth = 0; - let quoted = false; - let start = 0; - for (let index = 0; index < source.length; index++) { - const character = source[index]; - if (character === "'") { - if (quoted && source[index + 1] === "'") index++; - else quoted = !quoted; - } - if (quoted) continue; - if (character === "(") depth++; - if (character === ")") depth--; - if (depth === 0 && source.slice(index, index + 2) === operator) { - parts.push(source.slice(start, index)); - start = index + 2; - index++; - } - } - parts.push(source.slice(start)); - return parts; -}; - -const withoutOuterParentheses = (source: string): string => { - if (!source.startsWith("(") || !source.endsWith(")")) return source; - let depth = 0; - let quoted = false; - for (let index = 0; index < source.length; index++) { - const character = source[index]; - if (character === "'") { - if (quoted && source[index + 1] === "'") index++; - else quoted = !quoted; - } - if (quoted) continue; - if (character === "(") depth++; - if (character === ")") depth--; - if (depth === 0 && index !== source.length - 1) return source; - } - return withoutOuterParentheses(source.slice(1, -1).trim()); -}; - // A sufficient static proof, not an evaluator for arbitrary Actions expressions. // Unknown predicates can never authorize repository execution before trust. type TrustConditionOptions = { @@ -62,34 +20,19 @@ const excludesUntrusted = ({ event, allowTrust = true, }: TrustConditionOptions): boolean => { - if (condition === false) return true; - if (typeof condition !== "string") return false; - const source = withoutOuterParentheses( - condition - .trim() - .replace(/^\$\{\{([\s\S]*)\}\}$/u, "$1") - .trim(), - ); - const alternatives = booleanParts(source, "||"); - if (alternatives.length > 1) - return alternatives.every((part) => - excludesUntrusted({ condition: part, event, allowTrust }), - ); - const conjuncts = booleanParts(source, "&&"); - if (conjuncts.length > 1) - return conjuncts.some((part) => - excludesUntrusted({ condition: part, event, allowTrust }), + return conditionRequires(condition, (source) => { + const trustComparison = + /^needs\.trust-check\.outputs\.trusted\s*==\s*'([^']*)'$/u.exec(source); + if (allowTrust && trustComparison?.at(1)?.toLowerCase() === "true") + return true; + const comparison = /^github\.event_name\s*(==|!=)\s*'([^']+)'$/u.exec( + source, ); - const trustComparison = - /^needs\.trust-check\.outputs\.trusted\s*==\s*'([^']*)'$/u.exec(source); - if (allowTrust && trustComparison?.at(1)?.toLowerCase() === "true") - return true; - if (source === "false") return true; - const comparison = /^github\.event_name\s*(==|!=)\s*'([^']+)'$/u.exec(source); - if (comparison === null) return false; - // Actions compares string values without regard to case. - const equal = comparison.at(2)?.toLowerCase() === event.toLowerCase(); - return comparison.at(1) === "==" ? !equal : equal; + if (comparison === null) return false; + // Actions compares string values without regard to case. + const equal = comparison.at(2)?.toLowerCase() === event.toLowerCase(); + return comparison.at(1) === "==" ? !equal : equal; + }); }; // This is a deliberately small proof language for pre-trust metadata commands.