Add missing validation for action.yml (parity with workflow files) (#311)

* Add missing validation for action.yml (parity with workflow files)

- Add uses format validation for composite action steps
  - Validates owner/repo@ref format
  - Supports docker:// and ./ local references
  - Warns about shortened SHA refs (security concern)
  - Detects reusable workflow references in wrong context

- Add if literal text detection for composite action steps
  - Detects literal text outside ${{ }} that makes conditions always truthy
  - Works for both plain string and mixed expression formats
  - Uses shared hasFormatWithLiteralText() utility

- Add pre-if/post-if validation for node and docker actions
  - Errors on explicit ${{ }} syntax (runner only supports implicit expressions)
  - Literal text detection for implicit expressions
  - New runs-if schema type with proper context (runner, github, job, env, inputs, status functions)
  - Validates only in strict schema used by language services

- Add format() function validation for all expressions
  - Validates format string syntax in all expression contexts
  - Checks argument count matches placeholders

- Fix env and matrix context providers to return complete=false
  - Prevents false positive 'unknown context' errors
  - Matches behavior of other dynamic contexts (secrets, vars, etc.)

- Refactor validation utilities into utils/validate-uses.ts and utils/validate-if.ts
  - Shared between workflow and action validation
  - Consistent error messages and codes

* Add strategy and matrix contexts to runs-if definition

Based on runner source code analysis (actions/runner):
- ExecutionContext.InitializeJob() populates ExpressionValues from message.ContextData
- strategy and matrix are part of message.ContextData, available before any steps run
- StepsRunner evaluates all steps (pre, main, post) using the same code path

Did NOT add:
- steps: empty at pre-if time (no steps completed yet)
- hashFiles: workspace files don't exist at pre-step time
This commit is contained in:
eric sciple
2026-01-23 00:02:02 -06:00
committed by GitHub
parent 8511ae2e6d
commit 33291f0f8d
8 changed files with 945 additions and 182 deletions
@@ -198,9 +198,13 @@ function getDefaultActionContext(
case "runner":
return getRunnerContext();
case "env":
// Actions can access env but we don't have runtime values
return new DescriptionDictionary();
case "env": {
// Actions can access env but we don't know what env vars the calling workflow defines
// Mark as incomplete to avoid false positive "Context access might be invalid" warnings
const envContext = new DescriptionDictionary();
envContext.complete = false;
return envContext;
}
case "job": {
// https://docs.github.com/en/actions/learn-github-actions/contexts#job-context
@@ -218,9 +222,13 @@ function getDefaultActionContext(
case "strategy":
return getStrategyContext();
case "matrix":
// Actions can access matrix context at runtime
return new DescriptionDictionary();
case "matrix": {
// Actions can access matrix context at runtime but we don't know the calling workflow's matrix
// Mark as incomplete to avoid false positive "Context access might be invalid" warnings
const matrixContext = new DescriptionDictionary();
matrixContext.complete = false;
return matrixContext;
}
}
return undefined;
+65
View File
@@ -0,0 +1,65 @@
/**
* Shared validation utilities for `if` condition literal text detection.
* Used by both workflow and action validation.
*/
import {data} from "@actions/expressions";
import {Expr, FunctionCall, Literal, Logical} from "@actions/expressions/ast";
/**
* Checks if a format function contains literal text in its format string.
* This indicates user confusion about how expressions work.
*
* Example: format('push == {0}', github.event_name)
* The literal text "push == " will always evaluate to truthy.
*
* @param expr The expression to check
* @returns true if the expression is a format() call with literal text
*/
export function hasFormatWithLiteralText(expr: Expr): boolean {
// If this is a logical AND expression (from ensureStatusFunction wrapping)
// check the right side for the format call
if (expr instanceof Logical && expr.operator.lexeme === "&&" && expr.args.length === 2) {
return hasFormatWithLiteralText(expr.args[1]);
}
if (!(expr instanceof FunctionCall)) {
return false;
}
// Check if this is a format function
if (expr.functionName.lexeme.toLowerCase() !== "format") {
return false;
}
// Check if the first argument is a string literal
if (expr.args.length < 1) {
return false;
}
const firstArg = expr.args[0];
if (!(firstArg instanceof Literal) || firstArg.literal.kind !== data.Kind.String) {
return false;
}
// Get the format string and trim whitespace
const formatString = firstArg.literal.coerceString();
const trimmed = formatString.trim();
// Check if there's literal text (non-replacement tokens) after trimming
let inToken = false;
for (let i = 0; i < trimmed.length; i++) {
if (!inToken && trimmed[i] === "{") {
inToken = true;
} else if (inToken && trimmed[i] === "}") {
inToken = false;
} else if (inToken && trimmed[i] >= "0" && trimmed[i] <= "9") {
// OK - this is a replacement token like {0}, {1}, etc.
} else {
// Found literal text
return true;
}
}
return false;
}
+118
View File
@@ -0,0 +1,118 @@
/**
* Shared validation utilities for step `uses` field format.
* Used by both workflow and action validation.
*/
import {Diagnostic, DiagnosticSeverity} from "vscode-languageserver-types";
import {StringToken} from "@actions/workflow-parser/templates/tokens/string-token";
import {mapRange} from "./range.js";
// Matches a short SHA (7-8 hex characters) that looks like it should be a full SHA
const SHORT_SHA_PATTERN = /^[0-9a-f]{7,8}$/i;
const SHORT_SHA_DOCS_URL =
"https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions";
/**
* Checks if a ref looks like a short SHA and adds a warning if so.
* Returns true if a warning was added.
*/
export function warnIfShortSha(diagnostics: Diagnostic[], token: StringToken, ref: string): boolean {
if (SHORT_SHA_PATTERN.test(ref)) {
diagnostics.push({
message: `The provided ref '${ref}' may be a shortened commit SHA. If so, please use the full 40-character commit SHA instead, as short SHAs are not supported.`,
severity: DiagnosticSeverity.Warning,
range: mapRange(token.range),
code: "short-sha-ref",
codeDescription: {
href: SHORT_SHA_DOCS_URL
}
});
return true;
}
return false;
}
/**
* Validates the format of a step's `uses` field.
*
* Valid formats:
* - docker://image:tag
* - ./local/path
* - .\local\path (Windows)
* - {owner}/{repo}@{ref}
* - {owner}/{repo}/{path}@{ref}
*/
export function validateStepUsesFormat(diagnostics: Diagnostic[], token: StringToken): void {
const uses = token.value;
// Empty uses value
if (!uses) {
diagnostics.push({
message: "'uses' value in action cannot be blank",
severity: DiagnosticSeverity.Error,
range: mapRange(token.range),
code: "invalid-uses-format"
});
return;
}
// Docker image reference - always valid format
if (uses.startsWith("docker://")) {
return;
}
// Local action path - always valid format
if (uses.startsWith("./") || uses.startsWith(".\\")) {
return;
}
// Remote action: must be {owner}/{repo}[/path]@{ref}
const atSegments = uses.split("@");
// Must have exactly one @
if (atSegments.length !== 2) {
addStepUsesFormatError(diagnostics, token);
return;
}
const [repoPath, gitRef] = atSegments;
// Ref cannot be empty
if (!gitRef) {
addStepUsesFormatError(diagnostics, token);
return;
}
// Split by / or \ to get path segments
const pathSegments = repoPath.split(/[\\/]/);
// Must have at least owner and repo (both non-empty)
if (pathSegments.length < 2 || !pathSegments[0] || !pathSegments[1]) {
addStepUsesFormatError(diagnostics, token);
return;
}
// Check if this is a reusable workflow reference (should be at job level, not step)
// Path would be like: owner/repo/.github/workflows/file.yml
if (pathSegments.length >= 4 && pathSegments[2] === ".github" && pathSegments[3] === "workflows") {
diagnostics.push({
message: "Reusable workflows should be referenced at the top-level `jobs.<job_id>.uses` key, not within steps",
severity: DiagnosticSeverity.Error,
range: mapRange(token.range),
code: "invalid-uses-format"
});
return;
}
// Warn if ref looks like a short SHA
warnIfShortSha(diagnostics, token, gitRef);
}
function addStepUsesFormatError(diagnostics: Diagnostic[], token: StringToken): void {
diagnostics.push({
message: `Expected format {owner}/{repo}[/path]@{ref}. Actual '${token.value}'`,
severity: DiagnosticSeverity.Error,
range: mapRange(token.range),
code: "invalid-uses-format"
});
}
+484
View File
@@ -527,4 +527,488 @@ runs:
expect(diagnostics.some(d => d.message.includes("is not valid for"))).toBe(false);
});
});
describe("composite step uses format validation", () => {
it("validates valid uses format with version", async () => {
const doc = createActionDocument(`
name: My Action
description: Uses another action
runs:
using: composite
steps:
- uses: actions/checkout@v4
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "invalid-uses-format")).toBe(false);
});
it("validates docker:// uses format", async () => {
const doc = createActionDocument(`
name: My Action
description: Uses docker image
runs:
using: composite
steps:
- uses: docker://alpine:3.14
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "invalid-uses-format")).toBe(false);
});
it("validates local ./ uses format", async () => {
const doc = createActionDocument(`
name: My Action
description: Uses local action
runs:
using: composite
steps:
- uses: ./local-action
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "invalid-uses-format")).toBe(false);
});
it("errors on missing @ref", async () => {
const doc = createActionDocument(`
name: My Action
description: Missing version
runs:
using: composite
steps:
- uses: actions/checkout
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "invalid-uses-format")).toBe(true);
expect(diagnostics.some(d => d.message.includes("Expected format"))).toBe(true);
});
it("errors on invalid format", async () => {
const doc = createActionDocument(`
name: My Action
description: Invalid format
runs:
using: composite
steps:
- uses: invalid-format
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "invalid-uses-format")).toBe(true);
});
it("warns on short SHA", async () => {
const doc = createActionDocument(`
name: My Action
description: Short SHA
runs:
using: composite
steps:
- uses: actions/checkout@a1b2c3d
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "short-sha-ref")).toBe(true);
expect(diagnostics.some(d => d.message.includes("shortened commit SHA"))).toBe(true);
});
it("allows full SHA", async () => {
const doc = createActionDocument(`
name: My Action
description: Full SHA
runs:
using: composite
steps:
- uses: actions/checkout@a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "short-sha-ref")).toBe(false);
});
it("errors on reusable workflow in step uses", async () => {
const doc = createActionDocument(`
name: My Action
description: Wrong workflow reference
runs:
using: composite
steps:
- uses: owner/repo/.github/workflows/build.yml@main
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Reusable workflows should be referenced"))).toBe(true);
});
});
describe("composite step if literal text validation", () => {
it("errors when literal text mixed with embedded expression", async () => {
const doc = createActionDocument(`
name: My Action
description: Literal text in if
runs:
using: composite
steps:
- if: push == \${{ github.event_name }}
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "expression-literal-text-in-condition")).toBe(true);
expect(diagnostics.some(d => d.message.includes("literal text outside replacement tokens"))).toBe(true);
});
it("allows valid expression in if", async () => {
const doc = createActionDocument(`
name: My Action
description: Valid if expression
runs:
using: composite
steps:
- if: \${{ github.event_name == 'push' }}
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "expression-literal-text-in-condition")).toBe(false);
});
it("allows if without expression markers (auto-wrapped)", async () => {
const doc = createActionDocument(`
name: My Action
description: If without markers
runs:
using: composite
steps:
- if: github.event_name == 'push'
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "expression-literal-text-in-condition")).toBe(false);
});
it("allows success() function", async () => {
const doc = createActionDocument(`
name: My Action
description: Success function
runs:
using: composite
steps:
- if: success()
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "expression-literal-text-in-condition")).toBe(false);
});
it("errors on format with literal text in if", async () => {
const doc = createActionDocument(`
name: My Action
description: Format with literal text
runs:
using: composite
steps:
- if: \${{ format('event is {0}', github.event_name) }}
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "expression-literal-text-in-condition")).toBe(true);
});
it("allows format with only replacement tokens", async () => {
const doc = createActionDocument(`
name: My Action
description: Format with only tokens
runs:
using: composite
steps:
- if: \${{ format('{0}', github.event_name) }}
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "expression-literal-text-in-condition")).toBe(false);
});
it("validates if in uses-step", async () => {
const doc = createActionDocument(`
name: My Action
description: If in uses step
runs:
using: composite
steps:
- if: push == \${{ github.event_name }}
uses: actions/checkout@v4
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "expression-literal-text-in-condition")).toBe(true);
});
});
describe("pre-if and post-if validation", () => {
it("errors on explicit expression with literal text in pre-if for node action", async () => {
const doc = createActionDocument(`
name: My Action
description: Literal text in pre-if
runs:
using: node20
main: index.js
pre: setup.js
pre-if: push == \${{ github.event_name }}
`);
const diagnostics = await validate(doc);
// Explicit ${{ }} syntax is not allowed for pre-if, so we get that error
expect(diagnostics.some(d => d.code === "explicit-expression-not-allowed")).toBe(true);
});
it("errors on explicit expression with literal text in post-if for node action", async () => {
const doc = createActionDocument(`
name: My Action
description: Literal text in post-if
runs:
using: node20
main: index.js
post: cleanup.js
post-if: event == \${{ github.event_name }}
`);
const diagnostics = await validate(doc);
// Explicit ${{ }} syntax is not allowed for post-if, so we get that error
expect(diagnostics.some(d => d.code === "explicit-expression-not-allowed")).toBe(true);
});
it("errors on explicit expression with literal text in pre-if for docker action", async () => {
const doc = createActionDocument(`
name: My Action
description: Literal text in pre-if
runs:
using: docker
image: Dockerfile
pre-entrypoint: /setup.sh
pre-if: push == \${{ github.event_name }}
`);
const diagnostics = await validate(doc);
// Explicit ${{ }} syntax is not allowed for pre-if, so we get that error
expect(diagnostics.some(d => d.code === "explicit-expression-not-allowed")).toBe(true);
});
it("errors on explicit expression with literal text in post-if for docker action", async () => {
const doc = createActionDocument(`
name: My Action
description: Literal text in post-if
runs:
using: docker
image: Dockerfile
post-entrypoint: /cleanup.sh
post-if: event == \${{ github.event_name }}
`);
const diagnostics = await validate(doc);
// Explicit ${{ }} syntax is not allowed for post-if, so we get that error
expect(diagnostics.some(d => d.code === "explicit-expression-not-allowed")).toBe(true);
});
it("allows valid expression in pre-if for node action", async () => {
const doc = createActionDocument(`
name: My Action
description: Valid pre-if
runs:
using: node20
main: index.js
pre: setup.js
pre-if: success()
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "expression-literal-text-in-condition")).toBe(false);
});
it("allows valid expression in post-if for node action", async () => {
const doc = createActionDocument(`
name: My Action
description: Valid post-if
runs:
using: node20
main: index.js
post: cleanup.js
post-if: always()
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "expression-literal-text-in-condition")).toBe(false);
});
it("errors on explicit expression syntax in pre-if", async () => {
const doc = createActionDocument(`
name: My Action
description: Explicit expression in pre-if
runs:
using: node20
main: index.js
pre: setup.js
pre-if: \${{ runner.os == 'Windows' }}
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "explicit-expression-not-allowed")).toBe(true);
expect(diagnostics.some(d => d.message.includes("pre-if"))).toBe(true);
});
it("errors on explicit expression syntax in post-if", async () => {
const doc = createActionDocument(`
name: My Action
description: Explicit expression in post-if
runs:
using: node20
main: index.js
post: cleanup.js
post-if: \${{ always() }}
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "explicit-expression-not-allowed")).toBe(true);
expect(diagnostics.some(d => d.message.includes("post-if"))).toBe(true);
});
it("allows expression with failure() in post-if", async () => {
const doc = createActionDocument(`
name: My Action
description: Valid post-if
runs:
using: node20
main: index.js
post: cleanup.js
post-if: failure()
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "expression-literal-text-in-condition")).toBe(false);
});
it("allows expression with cancelled() in post-if", async () => {
const doc = createActionDocument(`
name: My Action
description: Valid post-if
runs:
using: node20
main: index.js
post: cleanup.js
post-if: cancelled()
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "expression-literal-text-in-condition")).toBe(false);
});
});
describe("format string validation", () => {
it("errors on format() with too few arguments in composite step if", async () => {
const doc = createActionDocument(`
name: My Action
description: Format mismatch
runs:
using: composite
steps:
- if: format('{0} {1}', 'only-one')
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "format-arg-count-mismatch")).toBe(true);
});
it("errors on invalid format string in composite step if", async () => {
const doc = createActionDocument(`
name: My Action
description: Invalid format
runs:
using: composite
steps:
- if: format('{', 'arg')
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "invalid-format-string")).toBe(true);
});
it("errors on format() with too few arguments in pre-if", async () => {
const doc = createActionDocument(`
name: My Action
description: Format mismatch in pre-if
runs:
using: node20
main: index.js
pre: setup.js
pre-if: format('{0} {1}', 'only-one')
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "format-arg-count-mismatch")).toBe(true);
});
it("errors on format() with too few arguments in post-if", async () => {
const doc = createActionDocument(`
name: My Action
description: Format mismatch in post-if
runs:
using: node20
main: index.js
post: cleanup.js
post-if: format('{0} {1} {2}', 'a', 'b')
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "format-arg-count-mismatch")).toBe(true);
});
it("allows valid format() call in composite step if", async () => {
const doc = createActionDocument(`
name: My Action
description: Valid format
runs:
using: composite
steps:
- if: format('{0} {1}', 'a', 'b') == 'a b'
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "format-arg-count-mismatch")).toBe(false);
expect(diagnostics.some(d => d.code === "invalid-format-string")).toBe(false);
});
it("allows valid format() call in pre-if", async () => {
const doc = createActionDocument(`
name: My Action
description: Valid format in pre-if
runs:
using: node20
main: index.js
pre: setup.js
pre-if: format('{0}', runner.os) == 'Linux'
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "format-arg-count-mismatch")).toBe(false);
expect(diagnostics.some(d => d.code === "invalid-format-string")).toBe(false);
});
it("errors on format() with too few arguments in run expression", async () => {
const doc = createActionDocument(`
name: My Action
description: Format mismatch in run
runs:
using: composite
steps:
- run: echo \${{ format('{0} {1}', 'only-one') }}
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "format-arg-count-mismatch")).toBe(true);
});
it("errors on format() with too few arguments in input default", async () => {
const doc = createActionDocument(`
name: My Action
description: Format mismatch in input default
inputs:
greeting:
description: Greeting message
default: \${{ format('{0} {1}', 'hello') }}
runs:
using: node20
main: index.js
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.code === "format-arg-count-mismatch")).toBe(true);
});
});
});
+238 -1
View File
@@ -2,20 +2,30 @@
* Validation for action.yml / action.yaml manifest files
*/
import {isMapping} from "@actions/workflow-parser";
import {Lexer, Parser} from "@actions/expressions";
import {Expr} from "@actions/expressions/ast";
import {isBasicExpression, isMapping, isString} from "@actions/workflow-parser";
import {isActionStep} from "@actions/workflow-parser/model/type-guards";
import {ErrorPolicy} from "@actions/workflow-parser/model/convert";
import {ensureStatusFunction} from "@actions/workflow-parser/model/converter/if-condition";
import {splitAllowedContext} from "@actions/workflow-parser/templates/allowed-context";
import {BasicExpressionToken} from "@actions/workflow-parser/templates/tokens/basic-expression-token";
import {MappingToken} from "@actions/workflow-parser/templates/tokens/mapping-token";
import {SequenceToken} from "@actions/workflow-parser/templates/tokens/sequence-token";
import {StringToken} from "@actions/workflow-parser/templates/tokens/string-token";
import {TemplateToken} from "@actions/workflow-parser/templates/tokens/template-token";
import {TokenRange} from "@actions/workflow-parser/templates/tokens/token-range";
import {TemplateValidationError} from "@actions/workflow-parser/templates/template-validation-error";
import {File} from "@actions/workflow-parser/workflows/file";
import {TextDocument} from "vscode-languageserver-textdocument";
import {Diagnostic, DiagnosticSeverity} from "vscode-languageserver-types";
import {error} from "./log.js";
import {mapRange} from "./utils/range.js";
import {hasFormatWithLiteralText} from "./utils/validate-if.js";
import {validateStepUsesFormat} from "./utils/validate-uses.js";
import {getOrConvertActionTemplate, getOrParseAction} from "./utils/workflow-cache.js";
import {validateActionReference} from "./validate-action-reference.js";
import {validateFormatCalls} from "./validate-format-string.js";
import {ValidationConfig} from "./validate.js";
/**
@@ -114,9 +124,23 @@ export async function validateAction(textDocument: TextDocument, config?: Valida
if (isActionStep(step) && isMapping(stepToken)) {
await validateActionReference(diagnostics, stepToken, step, config);
}
// Validate step tokens (uses format, if conditions)
if (isMapping(stepToken)) {
validateCompositeStepTokens(diagnostics, stepToken);
}
}
}
}
// Validate pre-if and post-if for node and docker actions
const runsMapping = findRunsMapping(result.value);
if (runsMapping) {
validateRunsIfConditions(diagnostics, runsMapping);
}
// Validate format() calls in all expressions throughout the action
validateAllExpressions(diagnostics, result.value);
}
} catch (e) {
error(`Unhandled error while validating action file: ${(e as Error).message}`);
@@ -125,6 +149,148 @@ export async function validateAction(textDocument: TextDocument, config?: Valida
return diagnostics;
}
/**
* Validates tokens within a composite action step.
* Checks `uses` format and `if` literal text detection.
*/
function validateCompositeStepTokens(diagnostics: Diagnostic[], stepToken: MappingToken): void {
for (let i = 0; i < stepToken.count; i++) {
const {key, value} = stepToken.get(i);
const keyStr = isString(key) ? key.value.toLowerCase() : "";
// Validate `uses` field format
if (keyStr === "uses" && isString(value)) {
validateStepUsesFormat(diagnostics, value);
}
// Validate `if` field for literal text outside expressions
if (keyStr === "if" && value.range) {
if (isString(value)) {
// Plain string if condition (no ${{ }} markers)
validateIfCondition(diagnostics, value);
} else if (isBasicExpression(value)) {
// Expression token - check for format() with literal text
// This happens when the parser converts "push == ${{ expr }}" to format('push == {0}', expr)
validateIfConditionExpression(diagnostics, value);
}
}
}
}
/**
* Validates an `if` condition (StringToken).
* Checks for literal text outside expressions and validates format() calls.
*/
function validateIfCondition(diagnostics: Diagnostic[], token: StringToken): void {
const condition = token.value.trim();
if (!condition) {
return;
}
// Get allowed context for step-if from the token's definition
const allowedContext = token.definitionInfo?.allowedContext || [];
const {namedContexts, functions} = splitAllowedContext(allowedContext);
// Ensure the condition has a status function, wrapping if needed
const finalCondition = ensureStatusFunction(condition, token.definitionInfo);
// Create a BasicExpressionToken for validation
const expressionToken = new BasicExpressionToken(
token.file,
token.range,
finalCondition,
token.definitionInfo,
undefined,
token.source,
undefined,
token.blockScalarHeader
);
// Check for literal text in the expression (format with literal text)
try {
const l = new Lexer(expressionToken.expression);
const lr = l.lex();
const p = new Parser(lr.tokens, namedContexts, functions);
const expr = p.parse();
if (hasFormatWithLiteralText(expr)) {
diagnostics.push({
message:
"Conditional expression contains literal text outside replacement tokens. This will cause the expression to always evaluate to truthy. Did you mean to put the entire expression inside ${{ }}?",
range: mapRange(token.range),
severity: DiagnosticSeverity.Error,
code: "expression-literal-text-in-condition"
});
}
// Validate format() function calls
validateFormatCallsAndAddDiagnostics(diagnostics, expr, token.range);
} catch {
// Ignore parse errors here - they'll be caught by schema validation
}
}
/**
* Validates an `if` condition (BasicExpressionToken).
* Checks for literal text outside expressions.
* Called when the parser has converted "push == ${{ expr }}" to format('push == {0}', expr).
* Note: format() validation is handled by validateAllExpressions for BasicExpressionTokens.
*/
function validateIfConditionExpression(diagnostics: Diagnostic[], token: BasicExpressionToken): void {
const allowedContext = token.definitionInfo?.allowedContext || [];
const {namedContexts, functions} = splitAllowedContext(allowedContext);
try {
const l = new Lexer(token.expression);
const lr = l.lex();
const p = new Parser(lr.tokens, namedContexts, functions);
const expr = p.parse();
if (hasFormatWithLiteralText(expr)) {
diagnostics.push({
message:
"Conditional expression contains literal text outside replacement tokens. This will cause the expression to always evaluate to truthy. Did you mean to put the entire expression inside ${{ }}?",
range: mapRange(token.range),
severity: DiagnosticSeverity.Error,
code: "expression-literal-text-in-condition"
});
}
// Note: format() validation is done by validateAllExpressions() for all BasicExpressionTokens
} catch {
// Ignore parse errors here - they'll be caught by schema validation
}
}
/**
* Helper to validate format() function calls and add diagnostics.
*/
function validateFormatCallsAndAddDiagnostics(
diagnostics: Diagnostic[],
expr: Expr,
range: TokenRange | undefined
): void {
const formatErrors = validateFormatCalls(expr);
for (const formatError of formatErrors) {
if (formatError.type === "invalid-syntax") {
diagnostics.push({
message: `Invalid format string: ${formatError.message}`,
range: mapRange(range),
severity: DiagnosticSeverity.Error,
code: "invalid-format-string"
});
} else if (formatError.type === "arg-count-mismatch") {
diagnostics.push({
message: `Format string references argument {${formatError.expected - 1}} but only ${
formatError.provided
} argument(s) provided`,
range: mapRange(range),
severity: DiagnosticSeverity.Error,
code: "format-arg-count-mismatch"
});
}
}
}
/**
* Find the steps sequence token from the raw action template.
* Traverses the token tree looking for the "composite-steps" definition.
@@ -138,6 +304,77 @@ function findStepsSequence(root: TemplateToken): SequenceToken | undefined {
return undefined;
}
/**
* Find the runs mapping token from the raw action template.
*/
function findRunsMapping(root: TemplateToken): MappingToken | undefined {
if (root instanceof MappingToken) {
for (let i = 0; i < root.count; i++) {
const {key, value} = root.get(i);
if (key.toString().toLowerCase() === "runs" && value instanceof MappingToken) {
return value;
}
}
}
return undefined;
}
/**
* Validates pre-if and post-if conditions at the runs level (for node and docker actions).
* Checks for literal text outside expressions that would always be truthy.
*/
function validateRunsIfConditions(diagnostics: Diagnostic[], runsMapping: MappingToken): void {
for (let i = 0; i < runsMapping.count; i++) {
const {key, value} = runsMapping.get(i);
const keyStr = key.toString().toLowerCase();
// Validate pre-if and post-if fields for literal text
if ((keyStr === "pre-if" || keyStr === "post-if") && value.range) {
if (isString(value)) {
// Plain string condition (no ${{ }} markers)
validateIfCondition(diagnostics, value);
} else if (isBasicExpression(value)) {
// The runner doesn't support explicit ${{ }} syntax for pre-if/post-if
// Only implicit expressions are allowed
diagnostics.push({
message: `Explicit expression syntax \${{ }} is not supported for '${keyStr}'. Remove the \${{ }} markers and use the expression directly.`,
range: mapRange(value.range),
severity: DiagnosticSeverity.Error,
code: "explicit-expression-not-allowed"
});
}
}
}
}
/**
* Validates format() function calls in all expressions throughout the action template.
* This catches format string errors in any expression, not just if conditions.
*/
function validateAllExpressions(diagnostics: Diagnostic[], root: TemplateToken): void {
for (const [, token] of TemplateToken.traverse(root)) {
if (token instanceof BasicExpressionToken) {
// Process original expressions if available (for combined expressions like "${{ a }} text ${{ b }}")
// This ensures error ranges point to the correct original expression location
for (const expression of token.originalExpressions || [token]) {
const allowedContext = expression.definitionInfo?.allowedContext || [];
const {namedContexts, functions} = splitAllowedContext(allowedContext);
try {
const l = new Lexer(expression.expression);
const lr = l.lex();
const p = new Parser(lr.tokens, namedContexts, functions);
const expr = p.parse();
validateFormatCallsAndAddDiagnostics(diagnostics, expr, expression.range);
} catch {
// Ignore parse errors - they'll be caught by schema validation
}
}
}
}
}
/**
* Validates that the keys under `runs:` are valid for the specified `using:` type.
* Also filters out schema errors (in place) that this validation replaces with more specific messages.
+4 -170
View File
@@ -1,5 +1,5 @@
import {FeatureFlags, Lexer, Parser, data} from "@actions/expressions";
import {Expr, FunctionCall, Literal, Logical} from "@actions/expressions/ast";
import {FeatureFlags, Lexer, Parser} from "@actions/expressions";
import {Expr} from "@actions/expressions/ast";
import {TemplateParseResult, WorkflowTemplate, isBasicExpression, isMapping, isString} from "@actions/workflow-parser";
import {ErrorPolicy} from "@actions/workflow-parser/model/convert";
import {getCronDescription, hasCronIntervalLessThan5Minutes} from "@actions/workflow-parser/model/converter/cron";
@@ -24,6 +24,8 @@ import {error} from "./log.js";
import {isActionDocument} from "./utils/document-type.js";
import {findToken} from "./utils/find-token.js";
import {mapRange} from "./utils/range.js";
import {hasFormatWithLiteralText} from "./utils/validate-if.js";
import {validateStepUsesFormat, warnIfShortSha} from "./utils/validate-uses.js";
import {getOrConvertWorkflowTemplate, getOrParseWorkflow} from "./utils/workflow-cache.js";
import {validateActionReference} from "./validate-action-reference.js";
import {validateAction} from "./validate-action.js";
@@ -285,116 +287,6 @@ function validateCronExpression(diagnostics: Diagnostic[], token: StringToken):
}
}
// Matches a short SHA (7-8 hex characters) that looks like it should be a full SHA
const SHORT_SHA_PATTERN = /^[0-9a-f]{7,8}$/i;
const SHORT_SHA_DOCS_URL =
"https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions";
/**
* Checks if a ref looks like a short SHA and adds a warning if so.
* Returns true if a warning was added.
*/
function warnIfShortSha(diagnostics: Diagnostic[], token: StringToken, ref: string): boolean {
if (SHORT_SHA_PATTERN.test(ref)) {
diagnostics.push({
message: `The provided ref '${ref}' may be a shortened commit SHA. If so, please use the full 40-character commit SHA instead, as short SHAs are not supported.`,
severity: DiagnosticSeverity.Warning,
range: mapRange(token.range),
code: "short-sha-ref",
codeDescription: {
href: SHORT_SHA_DOCS_URL
}
});
return true;
}
return false;
}
/**
* Validates the format of a step's `uses` field.
*
* Valid formats:
* - docker://image:tag
* - ./local/path
* - .\local\path (Windows)
* - {owner}/{repo}@{ref}
* - {owner}/{repo}/{path}@{ref}
*/
function validateStepUsesFormat(diagnostics: Diagnostic[], token: StringToken): void {
const uses = token.value;
// Empty uses value
if (!uses) {
diagnostics.push({
message: "`uses' value in action cannot be blank",
severity: DiagnosticSeverity.Error,
range: mapRange(token.range),
code: "invalid-uses-format"
});
return;
}
// Docker image reference - always valid format
if (uses.startsWith("docker://")) {
return;
}
// Local action path - always valid format
if (uses.startsWith("./") || uses.startsWith(".\\")) {
return;
}
// Remote action: must be {owner}/{repo}[/path]@{ref}
const atSegments = uses.split("@");
// Must have exactly one @
if (atSegments.length !== 2) {
addStepUsesFormatError(diagnostics, token);
return;
}
const [repoPath, gitRef] = atSegments;
// Ref cannot be empty
if (!gitRef) {
addStepUsesFormatError(diagnostics, token);
return;
}
// Split by / or \ to get path segments
const pathSegments = repoPath.split(/[\\/]/);
// Must have at least owner and repo (both non-empty)
if (pathSegments.length < 2 || !pathSegments[0] || !pathSegments[1]) {
addStepUsesFormatError(diagnostics, token);
return;
}
// Check if this is a reusable workflow reference (should be at job level, not step)
// Path would be like: owner/repo/.github/workflows/file.yml
if (pathSegments.length >= 4 && pathSegments[2] === ".github" && pathSegments[3] === "workflows") {
diagnostics.push({
message: "Reusable workflows should be referenced at the top-level `jobs.<job_id>.uses` key, not within steps",
severity: DiagnosticSeverity.Error,
range: mapRange(token.range),
code: "invalid-uses-format"
});
return;
}
// Warn if ref looks like a short SHA
warnIfShortSha(diagnostics, token, gitRef);
}
function addStepUsesFormatError(diagnostics: Diagnostic[], token: StringToken): void {
diagnostics.push({
message: `Expected format {owner}/{repo}[/path]@{ref}. Actual '${token.value}'`,
severity: DiagnosticSeverity.Error,
range: mapRange(token.range),
code: "invalid-uses-format"
});
}
/**
* Validates the format of a job's `uses` field (reusable workflow reference).
*
@@ -639,64 +531,6 @@ function getProviderContext(
return getWorkflowContext(documentUri, template, path);
}
/**
* Checks if a format function contains literal text in its format string.
* This indicates user confusion about how expressions work.
*
* Example: format('push == {0}', github.event_name)
* The literal text "push == " will always evaluate to truthy.
*
* @param expr The expression to check
* @returns true if the expression is a format() call with literal text
*/
function hasFormatWithLiteralText(expr: Expr): boolean {
// If this is a logical AND expression (from ensureStatusFunction wrapping)
// check the right side for the format call
if (expr instanceof Logical && expr.operator.lexeme === "&&" && expr.args.length === 2) {
return hasFormatWithLiteralText(expr.args[1]);
}
if (!(expr instanceof FunctionCall)) {
return false;
}
// Check if this is a format function
if (expr.functionName.lexeme.toLowerCase() !== "format") {
return false;
}
// Check if the first argument is a string literal
if (expr.args.length < 1) {
return false;
}
const firstArg = expr.args[0];
if (!(firstArg instanceof Literal) || firstArg.literal.kind !== data.Kind.String) {
return false;
}
// Get the format string and trim whitespace
const formatString = firstArg.literal.coerceString();
const trimmed = formatString.trim();
// Check if there's literal text (non-replacement tokens) after trimming
let inToken = false;
for (let i = 0; i < trimmed.length; i++) {
if (!inToken && trimmed[i] === "{") {
inToken = true;
} else if (inToken && trimmed[i] === "}") {
inToken = false;
} else if (inToken && trimmed[i] >= "0" && trimmed[i] <= "9") {
// OK - this is a replacement token like {0}, {1}, etc.
} else {
// Found literal text
return true;
}
}
return false;
}
async function validateExpression(
diagnostics: Diagnostic[],
token: BasicExpressionToken,
@@ -295,7 +295,7 @@ jobs:
`;
const result = await validate(createDocument("wf.yaml", input));
expect(result).toContainEqual({
message: "`uses' value in action cannot be blank",
message: "'uses' value in action cannot be blank",
severity: DiagnosticSeverity.Error,
range: {
start: {line: 5, character: 12},
+21 -4
View File
@@ -137,6 +137,23 @@
],
"string": {}
},
"runs-if": {
"description": "Condition to control when this action's pre or post script runs.",
"context": [
"runner",
"github",
"job",
"strategy",
"matrix",
"env",
"inputs",
"always(0,0)",
"success(0,0)",
"failure(0,0)",
"cancelled(0,0)"
],
"string": {}
},
"runs": {
"one-of": [
"container-runs",
@@ -242,7 +259,7 @@
"description": "Allows you to run a script before the entrypoint action begins.\n\n[Documentation](https://docs.github.com/actions/creating-actions/metadata-syntax-for-github-actions#runspre-entrypoint)"
},
"pre-if": {
"type": "non-empty-string",
"type": "runs-if",
"description": "Allows you to define conditions for the pre: action execution. The pre: action will only run if the conditions in pre-if are met. If not set, then pre-if defaults to always().\n\n[Documentation](https://docs.github.com/actions/creating-actions/metadata-syntax-for-github-actions#runspre-if)"
},
"post-entrypoint": {
@@ -250,7 +267,7 @@
"description": "Allows you to run a cleanup script once the runs.entrypoint action has completed.\n\n[Documentation](https://docs.github.com/actions/creating-actions/metadata-syntax-for-github-actions#runspost-entrypoint)"
},
"post-if": {
"type": "non-empty-string",
"type": "runs-if",
"description": "Allows you to define conditions for the post: action execution. The post: action will only run if the conditions in post-if are met. If not set, then post-if defaults to always().\n\n[Documentation](https://docs.github.com/actions/creating-actions/metadata-syntax-for-github-actions#runspost-if)"
}
}
@@ -275,7 +292,7 @@
"description": "Allows you to run a script at the start of a job, before the main: action begins. You can use pre: to run prerequisite setup scripts.\n\n[Documentation](https://docs.github.com/actions/creating-actions/metadata-syntax-for-github-actions#runspre)"
},
"pre-if": {
"type": "non-empty-string",
"type": "runs-if",
"description": "Allows you to define conditions for the pre: action execution. The pre: action will only run if the conditions in pre-if are met. If not set, then pre-if defaults to always().\n\n[Documentation](https://docs.github.com/actions/creating-actions/metadata-syntax-for-github-actions#runspre-if)"
},
"post": {
@@ -283,7 +300,7 @@
"description": "Allows you to run a script at the end of a job, once the main: action has completed. You can use post: to run cleanup scripts.\n\n[Documentation](https://docs.github.com/actions/creating-actions/metadata-syntax-for-github-actions#runspost)"
},
"post-if": {
"type": "non-empty-string",
"type": "runs-if",
"description": "Allows you to define conditions for the post: action execution. The post: action will only run if the conditions in post-if are met. If not set, then post-if defaults to always().\n\n[Documentation](https://docs.github.com/actions/creating-actions/metadata-syntax-for-github-actions#runspost-if)"
}
}