Compare commits

...

15 Commits

Author SHA1 Message Date
eric sciple c85997ad0d Gate container image validation behind feature flag
Add containerImageValidation experimental feature flag that gates the
new container image validation behind an opt-in toggle. When the flag
is off (default), the legacy converter logic is used. When enabled,
the improved validation with expression handling runs.

The legacy code is duplicated to keep code paths fully isolated and
make the eventual cleanup diff minimal — just delete the legacy
functions and the flag guards.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-02-13 21:02:30 +00:00
eric sciple 671f92dbc6 Add validation for empty container image
Related PR:
- https://github.com/actions/runner/pull/4220

Relaxing schema non-empty-string for container/service image and moving to custom validation. This matches current production behavior which allows empty string at runtime, but not parse time.
2026-02-05 23:11:58 +00:00
eric sciple fb5c6e4f27 Add private repository access to step-uses description (#322)
Update the step-uses description to mention that actions can also be
used from private repositories when access is enabled via repository
settings.

Fixes #319
2026-01-30 09:23:48 -06:00
Allan Guigou f29f508cec Merge pull request #321 from actions/release/0.3.44
Release version 0.3.44
2026-01-29 15:36:01 -05:00
GitHub Actions d69c1fa0f3 Release extension version 0.3.44 2026-01-29 18:13:09 +00:00
Allan Guigou 191a7b6a00 Merge pull request #320 from actions/allanguigou/default-case
Remove experimental flag for `case` function
2026-01-29 13:10:33 -05:00
Allan Guigou 0410ab8302 Add featureFlags param with lint ignore 2026-01-29 17:24:35 +00:00
Allan Guigou 7ac83f43a6 Fix unused param 2026-01-29 16:51:18 +00:00
Allan Guigou ef457b29fa Remove unused feature flag param 2026-01-29 16:08:16 +00:00
Allan Guigou fea8440c1d Fix lint 2026-01-29 15:56:43 +00:00
Allan Guigou 3c0a5f79fc Remove experimental flag for case function 2026-01-29 14:34:51 +00:00
github-actions[bot] 448180bd7f Release extension version 0.3.43 (#318)
Co-authored-by: GitHub Actions <github-actions@github.com>
2026-01-27 08:57:45 -06:00
eric sciple d2f52a9043 Validate implicit if conditions in action.yml files (#317)
## Problem

In workflow YAML files, writing `if: foo == bar` shows an error because `foo` and `bar` are not valid contexts. However, the same invalid expression in an action.yml file showed no error.

## Solution

Add expression validation for implicit `if` conditions in action.yml files, matching the behavior of workflow YAML validation.

## What's new

1. **Pre-if/post-if validation** (node and docker actions)
   - `pre-if: foo == bar` now shows error for unknown context
   - `post-if: unknownFunc()` now shows error for unknown function

2. **Composite step `if` validation** (fix)
   - Errors from `convertToIfCondition` were being lost due to call ordering
   - Now captured correctly by calling conversion before retrieving errors

## Why the refactor?

The diff includes consolidating multiple validation loops into a single `validateAllTokens()` traversal. This matches the pattern used in workflow YAML validation (`additionalValidations`), making the code consistent between the two validation paths.
2026-01-27 08:37:42 -06:00
github-actions[bot] 46b216a6dc Release extension version 0.3.42 (#316)
Co-authored-by: GitHub Actions <github-actions@github.com>
2026-01-25 20:53:27 -06:00
eric sciple 0fe7798548 Support pre-if/post-if autocomplete and fix expression functions for action.yml (#314) 2026-01-25 20:47:30 -06:00
30 changed files with 1402 additions and 238 deletions
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "@actions/expressions",
"version": "0.3.41",
"version": "0.3.44",
"license": "MIT",
"type": "module",
"source": "./src/index.ts",
+3 -7
View File
@@ -35,6 +35,7 @@ export function complete(
context: Dictionary,
extensionFunctions: FunctionInfo[],
functions?: Map<string, FunctionDefinition>,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
featureFlags?: FeatureFlags
): CompletionItem[] {
// Lex
@@ -66,7 +67,7 @@ export function complete(
const result = contextKeys(context);
// Merge with functions
result.push(...functionItems(extensionFunctions, featureFlags));
result.push(...functionItems(extensionFunctions));
return result;
}
@@ -91,15 +92,10 @@ export function complete(
return contextKeys(result);
}
function functionItems(extensionFunctions: FunctionInfo[], featureFlags?: FeatureFlags): CompletionItem[] {
function functionItems(extensionFunctions: FunctionInfo[]): CompletionItem[] {
const result: CompletionItem[] = [];
const flags = featureFlags ?? new FeatureFlags();
for (const fdef of [...Object.values(wellKnownFunctions), ...extensionFunctions]) {
// Filter out case function if feature is disabled
if (fdef.name === "case" && !flags.isEnabled("allowCaseFunction")) {
continue;
}
result.push({
label: fdef.name,
description: fdef.description,
+1 -5
View File
@@ -51,11 +51,7 @@ describe("FeatureFlags", () => {
it("returns all features when all is enabled", () => {
const flags = new FeatureFlags({all: true});
expect(flags.getEnabledFeatures()).toEqual([
"missingInputsQuickfix",
"blockScalarChompingWarning",
"allowCaseFunction"
]);
expect(flags.getEnabledFeatures()).toEqual(["missingInputsQuickfix", "blockScalarChompingWarning"]);
});
});
});
+4 -3
View File
@@ -30,10 +30,11 @@ export interface ExperimentalFeatures {
blockScalarChompingWarning?: boolean;
/**
* Enable the case() function in expressions.
* Enable improved container image validation that handles
* expressions gracefully and validates empty/docker:// images.
* @default false
*/
allowCaseFunction?: boolean;
containerImageValidation?: boolean;
}
/**
@@ -48,7 +49,7 @@ export type ExperimentalFeatureKey = Exclude<keyof ExperimentalFeatures, "all">;
const allFeatureKeys: ExperimentalFeatureKey[] = [
"missingInputsQuickfix",
"blockScalarChompingWarning",
"allowCaseFunction"
"containerImageValidation"
];
export class FeatureFlags {
+3 -3
View File
@@ -1,6 +1,6 @@
{
"name": "@actions/languageserver",
"version": "0.3.41",
"version": "0.3.44",
"description": "Language server for GitHub Actions",
"license": "MIT",
"type": "module",
@@ -48,8 +48,8 @@
"actions-languageserver": "./bin/actions-languageserver"
},
"dependencies": {
"@actions/languageservice": "^0.3.41",
"@actions/workflow-parser": "^0.3.41",
"@actions/languageservice": "^0.3.44",
"@actions/workflow-parser": "^0.3.44",
"@octokit/rest": "^21.1.1",
"@octokit/types": "^9.0.0",
"vscode-languageserver": "^8.0.2",
+3 -3
View File
@@ -1,6 +1,6 @@
{
"name": "@actions/languageservice",
"version": "0.3.41",
"version": "0.3.44",
"description": "Language service for GitHub Actions",
"license": "MIT",
"type": "module",
@@ -47,8 +47,8 @@
"watch": "tsc --build tsconfig.build.json --watch"
},
"dependencies": {
"@actions/expressions": "^0.3.41",
"@actions/workflow-parser": "^0.3.41",
"@actions/expressions": "^0.3.44",
"@actions/workflow-parser": "^0.3.44",
"vscode-languageserver-textdocument": "^1.0.7",
"vscode-languageserver-types": "^3.17.2",
"vscode-uri": "^3.0.8",
+122
View File
@@ -134,6 +134,49 @@ runs:
expect(labels).toContain("arch");
expect(labels).toContain("temp");
});
it("completes if expression value for composite run step", async () => {
const [doc, position] = createActionDocument(`name: My Action
description: Test action
runs:
using: composite
steps:
- if: |
run: echo "hello"
shell: bash`);
const completions = await complete(doc, position);
const labels = completions.map(c => c.label);
// Should show expression-related completions (status functions and contexts)
expect(labels).toContain("always");
expect(labels).toContain("success");
expect(labels).toContain("failure");
expect(labels).toContain("cancelled");
expect(labels).toContain("runner");
expect(labels).toContain("github");
expect(labels).toContain("inputs");
expect(labels).toContain("steps");
});
it("completes if expression value for composite uses step", async () => {
const [doc, position] = createActionDocument(`name: My Action
description: Test action
runs:
using: composite
steps:
- if: |
uses: actions/checkout@v4`);
const completions = await complete(doc, position);
const labels = completions.map(c => c.label);
// Should show expression-related completions
expect(labels).toContain("always");
expect(labels).toContain("success");
expect(labels).toContain("failure");
expect(labels).toContain("cancelled");
expect(labels).toContain("runner");
expect(labels).toContain("github");
});
});
describe("top-level completions", () => {
@@ -207,6 +250,85 @@ runs:
expect(labels).not.toContain("entrypoint");
});
it("filters runs keys for node24 actions", async () => {
const [doc, position] = createActionDocument(`name: Test
description: Test
runs:
using: node24
|`);
const completions = await complete(doc, position);
const labels = completions.map(c => c.label);
// Should show Node.js action keys
expect(labels).toContain("main");
expect(labels).toContain("pre");
expect(labels).toContain("post");
expect(labels).toContain("pre-if");
expect(labels).toContain("post-if");
// Should NOT show composite or docker keys
expect(labels).not.toContain("steps");
expect(labels).not.toContain("image");
expect(labels).not.toContain("entrypoint");
});
it("completes pre-if expression value for node actions", async () => {
const [doc, position] = createActionDocument(`name: Test
description: Test
runs:
using: node24
main: index.js
pre: setup.js
pre-if: |`);
const completions = await complete(doc, position);
const labels = completions.map(c => c.label);
// Should show expression-related completions (context functions and namespaces)
expect(labels).toContain("always");
expect(labels).toContain("success");
expect(labels).toContain("failure");
expect(labels).toContain("cancelled");
expect(labels).toContain("runner");
expect(labels).toContain("github");
expect(labels).toContain("inputs");
expect(labels).toContain("hashFiles");
});
it("completes post-if expression value for node actions", async () => {
const [doc, position] = createActionDocument(`name: Test
description: Test
runs:
using: node24
main: index.js
post: cleanup.js
post-if: |`);
const completions = await complete(doc, position);
const labels = completions.map(c => c.label);
// Should show expression-related completions
expect(labels).toContain("always");
expect(labels).toContain("runner");
expect(labels).toContain("hashFiles");
});
it("completes pre-if expression value for docker actions", async () => {
const [doc, position] = createActionDocument(`name: Test
description: Test
runs:
using: docker
image: docker://alpine
pre-entrypoint: setup.sh
pre-if: |`);
const completions = await complete(doc, position);
const labels = completions.map(c => c.label);
// Should show expression-related completions
expect(labels).toContain("always");
expect(labels).toContain("runner");
expect(labels).toContain("github");
expect(labels).toContain("hashFiles");
});
it("filters runs keys for composite actions", async () => {
const [doc, position] = createActionDocument(`name: Test
description: Test
@@ -1,6 +1,6 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import {data, DescriptionDictionary, FeatureFlags} from "@actions/expressions";
import {CompletionItem, CompletionItemKind} from "vscode-languageserver-types";
import {data, DescriptionDictionary} from "@actions/expressions";
import {CompletionItem, CompletionItemKind, MarkupContent} from "vscode-languageserver-types";
import {complete, getExpressionInput} from "./complete.js";
import {ContextProviderConfig} from "./context-providers/config.js";
import {registerLogger} from "./log.js";
@@ -69,8 +69,7 @@ describe("expressions", () => {
it("single region", async () => {
const input = "run-name: ${{ | }}";
const result = await complete(...getPositionFromCursor(input), {
contextProviderConfig,
featureFlags: new FeatureFlags({allowCaseFunction: true})
contextProviderConfig
});
expect(result.map(x => x.label)).toEqual([
@@ -113,8 +112,7 @@ describe("expressions", () => {
it("single region with existing input", async () => {
const input = "run-name: ${{ g| }}";
const result = await complete(...getPositionFromCursor(input), {
contextProviderConfig,
featureFlags: new FeatureFlags({allowCaseFunction: true})
contextProviderConfig
});
expect(result.map(x => x.label)).toEqual([
@@ -135,8 +133,7 @@ describe("expressions", () => {
it("single region with existing condition", async () => {
const input = "run-name: ${{ g| == 'test' }}";
const result = await complete(...getPositionFromCursor(input), {
contextProviderConfig,
featureFlags: new FeatureFlags({allowCaseFunction: true})
contextProviderConfig
});
expect(result.map(x => x.label)).toEqual([
@@ -157,8 +154,7 @@ describe("expressions", () => {
it("multiple regions with partial function", async () => {
const input = "run-name: Run a ${{ inputs.test }} one-line script ${{ from|('test') == inputs.name }}";
const result = await complete(...getPositionFromCursor(input), {
contextProviderConfig,
featureFlags: new FeatureFlags({allowCaseFunction: true})
contextProviderConfig
});
expect(result.map(x => x.label)).toEqual([
@@ -179,8 +175,7 @@ describe("expressions", () => {
it("multiple regions - first region", async () => {
const input = "run-name: test-${{ git| == 1 }}-${{ github.event }}";
const result = await complete(...getPositionFromCursor(input), {
contextProviderConfig,
featureFlags: new FeatureFlags({allowCaseFunction: true})
contextProviderConfig
});
expect(result.map(x => x.label)).toEqual([
@@ -201,8 +196,7 @@ describe("expressions", () => {
it("multiple regions", async () => {
const input = "run-name: test-${{ github }}-${{ | }}";
const result = await complete(...getPositionFromCursor(input), {
contextProviderConfig,
featureFlags: new FeatureFlags({allowCaseFunction: true})
contextProviderConfig
});
expect(result.map(x => x.label)).toEqual([
@@ -419,6 +413,36 @@ jobs:
expect(result.map(x => x.label)).toEqual(["event"]);
});
it("includes both contexts and extension functions", async () => {
const input = `on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo
if: |`;
const result = await complete(...getPositionFromCursor(input), {contextProviderConfig});
const labels = result.map(x => x.label);
// Context namespaces should be present
expect(labels).toContain("github");
expect(labels).toContain("runner");
expect(labels).toContain("env");
expect(labels).toContain("steps");
// Extension functions should be present (from schema context array)
expect(labels).toContain("hashFiles");
expect(labels).toContain("always");
expect(labels).toContain("success");
expect(labels).toContain("failure");
expect(labels).toContain("cancelled");
// Built-in functions should be present
expect(labels).toContain("toJson");
expect(labels).toContain("fromJson");
expect(labels).toContain("contains");
});
});
});
@@ -1151,8 +1175,7 @@ jobs:
`;
const result = await complete(...getPositionFromCursor(input), {
contextProviderConfig,
featureFlags: new FeatureFlags({allowCaseFunction: true})
contextProviderConfig
});
expect(result.map(x => x.label)).toEqual([
"env",
@@ -1278,6 +1301,7 @@ jobs:
expect(hashFiles).toBeDefined();
expect(hashFiles!.kind).toBe(CompletionItemKind.Function);
expect(hashFiles!.insertText).toBe("hashFiles()");
expect((hashFiles!.documentation as MarkupContent)?.value).toContain("Returns a single hash for the set of files");
// Not a function
const github = result.find(x => x.label === "github");
+2 -18
View File
@@ -6,7 +6,6 @@ import {getPositionFromCursor} from "./test-utils/cursor-position.js";
import {TestLogger} from "./test-utils/logger.js";
import {clearCache} from "./utils/workflow-cache.js";
import {ValueProviderConfig, ValueProviderKind} from "./value-providers/config.js";
import {FeatureFlags} from "@actions/expressions/features";
registerLogger(new TestLogger());
@@ -898,11 +897,9 @@ jobs:
});
describe("expression completions", () => {
it("include case function when enabled", async () => {
it("includes case function", async () => {
const input = "on: push\njobs:\n build:\n runs-on: ${{ c|";
const result = await complete(...getPositionFromCursor(input), {
featureFlags: new FeatureFlags({allowCaseFunction: true})
});
const result = await complete(...getPositionFromCursor(input));
expect(result).not.toBeUndefined();
// Expression completions starting with 'c': case, contains
@@ -910,18 +907,5 @@ jobs:
expect(labels).toContain("case");
expect(labels).toContain("contains");
});
it("exclude case function when disabled", async () => {
const input = "on: push\njobs:\n build:\n runs-on: ${{ c|";
const result = await complete(...getPositionFromCursor(input), {
featureFlags: new FeatureFlags({allowCaseFunction: false})
});
expect(result).not.toBeUndefined();
// Expression completions starting with 'c': contains
const labels = result.map(x => x.label);
expect(labels).not.toContain("case");
expect(labels).toContain("contains");
});
});
});
+16 -6
View File
@@ -1,8 +1,10 @@
import {complete as completeExpression, DescriptionDictionary, FeatureFlags} from "@actions/expressions";
import {CompletionItem as ExpressionCompletionItem} from "@actions/expressions/completion";
import {FunctionInfo} from "@actions/expressions/funcs/info";
import {isBasicExpression, isSequence, isString} from "@actions/workflow-parser";
import {getActionSchema} from "@actions/workflow-parser/actions/action-schema";
import {ErrorPolicy} from "@actions/workflow-parser/model/convert";
import {splitAllowedContext} from "@actions/workflow-parser/templates/allowed-context";
import {DefinitionType} from "@actions/workflow-parser/templates/schema/definition-type";
import {OneOfDefinition} from "@actions/workflow-parser/templates/schema/one-of-definition";
import {TemplateSchema} from "@actions/workflow-parser/templates/schema/template-schema";
@@ -19,6 +21,7 @@ import {CompletionItem, CompletionItemKind, CompletionItemTag, Range, TextEdit}
import {filterActionRunsCompletions, getActionScaffoldingSnippets} from "./complete-action.js";
import {ContextProviderConfig} from "./context-providers/config.js";
import {getActionExpressionContext, getWorkflowExpressionContext, Mode} from "./context-providers/default.js";
import {getFunctionDescription} from "./context-providers/descriptions.js";
import {ActionContext, getActionContext} from "./context/action-context.js";
import {getWorkflowContext, WorkflowContext} from "./context/workflow-context.js";
import {validatorFunctions} from "./expression-validation/functions.js";
@@ -121,18 +124,24 @@ export async function complete(
}
// Expression completions
if (token && (isBasicExpression(token) || isPotentiallyExpression(token))) {
if (token && (isBasicExpression(token) || isPotentiallyExpression(token, isAction))) {
const allowedContext = token.definitionInfo?.allowedContext || [];
const {namedContexts, functions: extensionFunctions} = splitAllowedContext(allowedContext);
const context = isAction
? getActionExpressionContext(allowedContext, config?.contextProviderConfig, actionContext, Mode.Completion)
? getActionExpressionContext(namedContexts, config?.contextProviderConfig, actionContext, Mode.Completion)
: await getWorkflowExpressionContext(
allowedContext,
namedContexts,
config?.contextProviderConfig,
workflowContext,
Mode.Completion
);
return getExpressionCompletionItems(token, context, newPos, config?.featureFlags);
// Populate function descriptions for completion display
for (const func of extensionFunctions) {
func.description = getFunctionDescription(func.name);
}
return getExpressionCompletionItems(token, context, extensionFunctions, newPos, config?.featureFlags);
}
const indentation = guessIndentation(newDoc, 2, true); // Use 2 spaces as default and most common for YAML
@@ -521,6 +530,7 @@ export function getExistingValues(token: TemplateToken | null, parent: TemplateT
function getExpressionCompletionItems(
token: TemplateToken,
context: DescriptionDictionary,
extensionFunctions: FunctionInfo[],
pos: Position,
featureFlags?: FeatureFlags
): CompletionItem[] {
@@ -541,8 +551,8 @@ function getExpressionCompletionItems(
const expressionInput = (getExpressionInput(currentInput, cursorOffset) || "").trim();
try {
return completeExpression(expressionInput, context, [], validatorFunctions, featureFlags).map(item =>
mapExpressionCompletionItem(item, currentInput[cursorOffset])
return completeExpression(expressionInput, context, extensionFunctions, validatorFunctions, featureFlags).map(
item => mapExpressionCompletionItem(item, currentInput[cursorOffset])
);
} catch (e) {
error(`Error while completing expression: '${(e as Error)?.message || "<no details>"}'`);
+1 -1
View File
@@ -195,7 +195,7 @@ jobs:
const result = await hover(...getPositionFromCursor(input), testHoverConfig("uses", "step-uses", undefined));
expect(result).not.toBeUndefined();
expect(result?.contents).toEqual(
"Selects an action to run as part of a step in your job. An action is a reusable unit of code. You can use an action defined in the same repository as the workflow, a public repository, or in a published Docker container image."
"Selects an action to run as part of a step in your job. An action is a reusable unit of code. You can use an action defined in the same repository as the workflow, a public repository, a [private repository with access enabled](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#allowing-access-to-components-in-a-private-repository), or in a published Docker container image."
);
});
});
+1 -1
View File
@@ -71,7 +71,7 @@ export async function hover(document: TextDocument, position: Position, config?:
// Early exit if there's nothing to provide hover for
const hoverToken = token || keyToken;
const isExpressionHover =
token && tokenDefinitionInfo && (isBasicExpression(token) || isPotentiallyExpression(token));
token && tokenDefinitionInfo && (isBasicExpression(token) || isPotentiallyExpression(token, isAction));
if (!isExpressionHover && !hoverToken?.definition) {
return null;
}
@@ -0,0 +1,170 @@
import {isPotentiallyExpression} from "./expression-detection.js";
import {TemplateToken} from "@actions/workflow-parser/templates/tokens/template-token";
import {TokenType} from "@actions/workflow-parser/templates/tokens/types";
import {Definition} from "@actions/workflow-parser/templates/schema/definition";
// Helper to create a mock TemplateToken with the properties we need to test
function createMockToken(options: {value?: string; definitionKey?: string; isString?: boolean}): TemplateToken {
const {value = "", definitionKey, isString = true} = options;
const mockDefinition = definitionKey ? ({key: definitionKey} as Definition) : undefined;
return {
value: isString ? value : undefined,
definition: mockDefinition,
templateTokenType: isString ? TokenType.String : TokenType.Mapping,
// Required by isString type guard (isLiteral checks isLiteral property)
isLiteral: isString,
isScalar: isString
} as unknown as TemplateToken;
}
describe("isPotentiallyExpression", () => {
describe("expression markers", () => {
it("returns true when token value contains ${{", () => {
const token = createMockToken({value: "${{ github.actor }}"});
expect(isPotentiallyExpression(token, false)).toBe(true);
expect(isPotentiallyExpression(token, true)).toBe(true);
});
it("returns true when token value contains embedded ${{", () => {
const token = createMockToken({value: "Hello ${{ github.actor }}!"});
expect(isPotentiallyExpression(token, false)).toBe(true);
expect(isPotentiallyExpression(token, true)).toBe(true);
});
it("returns false when token value does not contain ${{", () => {
const token = createMockToken({value: "plain text"});
expect(isPotentiallyExpression(token, false)).toBe(false);
expect(isPotentiallyExpression(token, true)).toBe(false);
});
it("returns false for non-string tokens without expression marker", () => {
const token = createMockToken({isString: false});
expect(isPotentiallyExpression(token, false)).toBe(false);
expect(isPotentiallyExpression(token, true)).toBe(false);
});
});
describe("workflow schema if-conditions", () => {
it("returns true for job-if definition in workflow", () => {
const token = createMockToken({value: "success()", definitionKey: "job-if"});
expect(isPotentiallyExpression(token, false)).toBe(true);
});
it("returns false for job-if definition in action (not valid in action schema)", () => {
const token = createMockToken({value: "success()", definitionKey: "job-if"});
expect(isPotentiallyExpression(token, true)).toBe(false);
});
it("returns true for step-if definition in workflow", () => {
const token = createMockToken({value: "failure()", definitionKey: "step-if"});
expect(isPotentiallyExpression(token, false)).toBe(true);
});
it("returns true for snapshot-if definition in workflow", () => {
const token = createMockToken({value: "always()", definitionKey: "snapshot-if"});
expect(isPotentiallyExpression(token, false)).toBe(true);
});
it("returns false for snapshot-if definition in action (not valid in action schema)", () => {
const token = createMockToken({value: "always()", definitionKey: "snapshot-if"});
expect(isPotentiallyExpression(token, true)).toBe(false);
});
});
describe("action schema if-conditions", () => {
describe("composite action step if (run and uses)", () => {
it("returns true for step-if definition in action", () => {
const token = createMockToken({value: "success()", definitionKey: "step-if"});
expect(isPotentiallyExpression(token, true)).toBe(true);
});
it("returns true for step-if with run step condition", () => {
// Composite action run step: if condition
const token = createMockToken({value: "github.event_name == 'push'", definitionKey: "step-if"});
expect(isPotentiallyExpression(token, true)).toBe(true);
});
it("returns true for step-if with uses step condition", () => {
// Composite action uses step: if condition
const token = createMockToken({value: "runner.os == 'Linux'", definitionKey: "step-if"});
expect(isPotentiallyExpression(token, true)).toBe(true);
});
});
describe("pre-if and post-if (node/docker actions)", () => {
it("returns true for runs-if definition in action (pre-if)", () => {
const token = createMockToken({value: "runner.os == 'Linux'", definitionKey: "runs-if"});
expect(isPotentiallyExpression(token, true)).toBe(true);
});
it("returns true for runs-if definition in action (post-if)", () => {
const token = createMockToken({value: "always()", definitionKey: "runs-if"});
expect(isPotentiallyExpression(token, true)).toBe(true);
});
it("returns false for runs-if definition in workflow (not valid in workflow schema)", () => {
const token = createMockToken({value: "always()", definitionKey: "runs-if"});
expect(isPotentiallyExpression(token, false)).toBe(false);
});
});
});
describe("mixed scenarios", () => {
it("returns true when expression marker present even if definition is not if-related", () => {
const token = createMockToken({value: "${{ github.actor }}", definitionKey: "some-other-definition"});
expect(isPotentiallyExpression(token, false)).toBe(true);
expect(isPotentiallyExpression(token, true)).toBe(true);
});
it("returns true when both expression marker and if definition present", () => {
const token = createMockToken({value: "${{ success() }}", definitionKey: "step-if"});
expect(isPotentiallyExpression(token, false)).toBe(true);
expect(isPotentiallyExpression(token, true)).toBe(true);
});
it("returns false for plain text with non-if definition", () => {
const token = createMockToken({value: "plain text", definitionKey: "string"});
expect(isPotentiallyExpression(token, false)).toBe(false);
expect(isPotentiallyExpression(token, true)).toBe(false);
});
it("returns false when token has no definition and no expression marker", () => {
const token = createMockToken({value: "plain text"});
expect(isPotentiallyExpression(token, false)).toBe(false);
expect(isPotentiallyExpression(token, true)).toBe(false);
});
});
describe("edge cases", () => {
it("handles empty string value", () => {
const token = createMockToken({value: ""});
expect(isPotentiallyExpression(token, false)).toBe(false);
expect(isPotentiallyExpression(token, true)).toBe(false);
});
it("handles expression marker as if-condition value", () => {
const token = createMockToken({value: "${{ always() }}", definitionKey: "job-if"});
expect(isPotentiallyExpression(token, false)).toBe(true);
// For action, job-if is not valid, but ${{ is present
expect(isPotentiallyExpression(token, true)).toBe(true);
});
it("handles partial expression marker", () => {
const token = createMockToken({value: "${incomplete"});
expect(isPotentiallyExpression(token, false)).toBe(false);
expect(isPotentiallyExpression(token, true)).toBe(false);
});
it("handles ${{ at different positions", () => {
const startToken = createMockToken({value: "${{ foo }} bar"});
const middleToken = createMockToken({value: "bar ${{ foo }} baz"});
const endToken = createMockToken({value: "bar ${{ foo }}"});
expect(isPotentiallyExpression(startToken, false)).toBe(true);
expect(isPotentiallyExpression(middleToken, false)).toBe(true);
expect(isPotentiallyExpression(endToken, false)).toBe(true);
});
});
});
@@ -2,10 +2,36 @@ import {isString} from "@actions/workflow-parser";
import {OPEN_EXPRESSION} from "@actions/workflow-parser/templates/template-constants";
import {TemplateToken} from "@actions/workflow-parser/templates/tokens/index";
export function isPotentiallyExpression(token: TemplateToken): boolean {
const containsExpression = isString(token) && token.value != null && token.value.indexOf(OPEN_EXPRESSION) >= 0;
// If conditions are always expressions (job-if, step-if, snapshot-if)
const definitionKey = token.definition?.key;
const isIfCondition = definitionKey === "job-if" || definitionKey === "step-if" || definitionKey === "snapshot-if";
return containsExpression || isIfCondition;
/**
* Workflow schema if-condition definition keys.
* - job-if: job level if condition
* - step-if: step level if condition
* - snapshot-if: snapshot if condition
*/
const WORKFLOW_IF_DEFINITIONS = new Set(["job-if", "step-if", "snapshot-if"]);
/**
* Action schema if-condition definition keys.
* - step-if: composite action step if condition (run-step and uses-step)
* - runs-if: pre-if and post-if at the runs level (node/docker actions)
*/
const ACTION_IF_DEFINITIONS = new Set(["step-if", "runs-if"]);
export function isPotentiallyExpression(token: TemplateToken, isAction: boolean): boolean {
// Check if token contains expression syntax
if (isString(token) && token.value != null && token.value.indexOf(OPEN_EXPRESSION) >= 0) {
return true;
}
// Check if token is an if-condition (always treated as expressions)
if (!token.definition?.key) {
return false;
}
// Definition keys differ between workflow and action schemas
if (isAction) {
return ACTION_IF_DEFINITIONS.has(token.definition.key);
} else {
return WORKFLOW_IF_DEFINITIONS.has(token.definition.key);
}
}
+251
View File
@@ -1011,4 +1011,255 @@ runs:
expect(diagnostics.some(d => d.code === "format-arg-count-mismatch")).toBe(true);
});
});
describe("if condition context validation", () => {
it("warns on unknown context in composite step if", async () => {
const doc = createActionDocument(`
name: My Action
description: Unknown context in if
runs:
using: composite
steps:
- if: foo == bar
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized named-value"))).toBe(true);
});
it("warns on unknown context in pre-if for node action", async () => {
const doc = createActionDocument(`
name: My Action
description: Unknown context in pre-if
runs:
using: node20
main: index.js
pre: setup.js
pre-if: foo == bar
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized named-value"))).toBe(true);
});
it("warns on unknown context in post-if for node action", async () => {
const doc = createActionDocument(`
name: My Action
description: Unknown context in post-if
runs:
using: node20
main: index.js
post: cleanup.js
post-if: foo == bar
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized named-value"))).toBe(true);
});
it("warns on unknown context in pre-if for docker action", async () => {
const doc = createActionDocument(`
name: My Action
description: Unknown context in pre-if
runs:
using: docker
image: Dockerfile
pre-entrypoint: /setup.sh
pre-if: foo == bar
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized named-value"))).toBe(true);
});
it("warns on unknown context in post-if for docker action", async () => {
const doc = createActionDocument(`
name: My Action
description: Unknown context in post-if
runs:
using: docker
image: Dockerfile
post-entrypoint: /cleanup.sh
post-if: foo == bar
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized named-value"))).toBe(true);
});
it("allows valid contexts in composite step if", async () => {
const doc = createActionDocument(`
name: My Action
description: Valid context in if
runs:
using: composite
steps:
- if: github.event_name == 'push'
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized named-value"))).toBe(false);
});
it("allows valid contexts in pre-if", async () => {
const doc = createActionDocument(`
name: My Action
description: Valid context in pre-if
runs:
using: node20
main: index.js
pre: setup.js
pre-if: runner.os == 'Linux'
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized named-value"))).toBe(false);
});
it("allows valid contexts in post-if", async () => {
const doc = createActionDocument(`
name: My Action
description: Valid context in post-if
runs:
using: node20
main: index.js
post: cleanup.js
post-if: runner.os == 'Linux'
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized named-value"))).toBe(false);
});
it("allows hashFiles function in composite step if", async () => {
const doc = createActionDocument(`
name: My Action
description: hashFiles in if
runs:
using: composite
steps:
- if: hashFiles('**/package-lock.json') != ''
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized"))).toBe(false);
});
it("allows success, failure, always, cancelled functions in composite step if", async () => {
const doc = createActionDocument(`
name: My Action
description: Status functions in if
runs:
using: composite
steps:
- if: success() && !cancelled()
run: echo success
shell: bash
- if: failure()
run: echo failure
shell: bash
- if: always()
run: echo always
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized"))).toBe(false);
});
it("allows hashFiles function in pre-if", async () => {
const doc = createActionDocument(`
name: My Action
description: hashFiles in pre-if
runs:
using: node20
main: index.js
pre: setup.js
pre-if: hashFiles('**/package-lock.json') != ''
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized"))).toBe(false);
});
it("allows status functions in post-if", async () => {
const doc = createActionDocument(`
name: My Action
description: Status functions in post-if
runs:
using: node20
main: index.js
post: cleanup.js
post-if: always() || failure()
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized"))).toBe(false);
});
it("errors on unknown function in composite step if", async () => {
const doc = createActionDocument(`
name: My Action
description: Unknown function in if
runs:
using: composite
steps:
- if: unknownFunc()
run: echo hi
shell: bash
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized function"))).toBe(true);
});
it("errors on unknown function in pre-if for node action", async () => {
const doc = createActionDocument(`
name: My Action
description: Unknown function in pre-if
runs:
using: node20
main: index.js
pre: setup.js
pre-if: unknownFunc()
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized function"))).toBe(true);
});
it("errors on unknown function in post-if for node action", async () => {
const doc = createActionDocument(`
name: My Action
description: Unknown function in post-if
runs:
using: node20
main: index.js
post: cleanup.js
post-if: unknownFunc()
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized function"))).toBe(true);
});
it("errors on unknown function in pre-if for docker action", async () => {
const doc = createActionDocument(`
name: My Action
description: Unknown function in pre-if
runs:
using: docker
image: Dockerfile
pre-entrypoint: /setup.sh
pre-if: unknownFunc()
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized function"))).toBe(true);
});
it("errors on unknown function in post-if for docker action", async () => {
const doc = createActionDocument(`
name: My Action
description: Unknown function in post-if
runs:
using: docker
image: Dockerfile
post-entrypoint: /cleanup.sh
post-if: unknownFunc()
`);
const diagnostics = await validate(doc);
expect(diagnostics.some(d => d.message.includes("Unrecognized function"))).toBe(true);
});
});
});
+113 -137
View File
@@ -4,9 +4,10 @@
import {Lexer, Parser} from "@actions/expressions";
import {Expr} from "@actions/expressions/ast";
import {isBasicExpression, isMapping, isString} from "@actions/workflow-parser";
import {isMapping, isString} from "@actions/workflow-parser";
import {isActionStep} from "@actions/workflow-parser/model/type-guards";
import {ErrorPolicy} from "@actions/workflow-parser/model/convert";
import {ActionTemplate} from "@actions/workflow-parser/actions/action-template";
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";
@@ -75,7 +76,15 @@ export async function validateAction(textDocument: TextDocument, config?: Valida
return [];
}
// Get schema errors
// Convert the action template (this may add validation errors for pre-if/post-if)
let template: ActionTemplate | undefined;
if (result.value) {
template = getOrConvertActionTemplate(result.context, result.value, textDocument.uri, {
errorPolicy: ErrorPolicy.TryConversion
});
}
// Get schema and conversion errors (must be after conversion to include conversion errors)
const schemaErrors = result.context.errors.getErrors();
// Run custom runs key validation, which also filters redundant schema errors in place
@@ -103,13 +112,9 @@ export async function validateAction(textDocument: TextDocument, config?: Valida
}
// Validate composite action steps if we have a parsed result
if (result.value) {
const template = getOrConvertActionTemplate(result.context, result.value, textDocument.uri, {
errorPolicy: ErrorPolicy.TryConversion
});
if (result.value && template) {
// Only composite actions have steps to validate
if (template?.runs?.using === "composite") {
if (template.runs?.using === "composite") {
const steps = template.runs.steps ?? [];
// Find the steps sequence token from the raw parsed result
@@ -125,22 +130,16 @@ export async function validateAction(textDocument: TextDocument, config?: Valida
await validateActionReference(diagnostics, stepToken, step, config);
}
// Validate step tokens (uses format, if conditions)
// Validate step uses format
if (isMapping(stepToken)) {
validateCompositeStepTokens(diagnostics, stepToken);
validateStepUsesField(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);
// Single traversal for all expression validation (like workflow's additionalValidations)
validateAllTokens(diagnostics, result.value);
}
} catch (e) {
error(`Unhandled error while validating action file: ${(e as Error).message}`);
@@ -150,93 +149,124 @@ export async function validateAction(textDocument: TextDocument, config?: Valida
}
/**
* Validates tokens within a composite action step.
* Checks `uses` format and `if` literal text detection.
* Validates the `uses` field format in a composite action step.
*/
function validateCompositeStepTokens(diagnostics: Diagnostic[], stepToken: MappingToken): void {
function validateStepUsesField(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.
* Single traversal validation for all tokens in the action template.
* This follows the same pattern as workflow validation's additionalValidations:
* - For BasicExpressionToken: validate format() calls
* - For StringToken on if conditions: validate literal text detection and format() calls
* - For pre-if/post-if with explicit ${{ }}: report error (not supported by runner)
*
* Context validation (unknown named values) is handled by workflow-parser during conversion.
*/
function validateIfCondition(diagnostics: Diagnostic[], token: StringToken): void {
function validateAllTokens(diagnostics: Diagnostic[], root: TemplateToken): void {
for (const [parent, token] of TemplateToken.traverse(root)) {
const definitionKey = token.definition?.key;
// Validate all BasicExpressionToken instances for format() calls
if (token instanceof BasicExpressionToken && token.range) {
// Check for literal text in if conditions (format with literal text)
if (definitionKey === "step-if") {
validateIfLiteralText(diagnostics, token);
}
// Validate format() calls for all expressions
for (const expression of token.originalExpressions || [token]) {
validateExpressionFormatCalls(diagnostics, expression);
}
// Check for explicit ${{ }} in pre-if/post-if (not supported by runner)
if (definitionKey === "runs-if" && parent instanceof MappingToken) {
// Resolve the key name (pre-if or post-if) from parent mapping
let keyName: string | undefined;
for (let i = 0; i < parent.count; i++) {
const {key, value} = parent.get(i);
if (value === token) {
keyName = key.toString().toLowerCase();
break;
}
}
if (keyName) {
diagnostics.push({
message: `Explicit expression syntax \${{ }} is not supported for '${keyName}'. Remove the \${{ }} markers and use the expression directly.`,
range: mapRange(token.range),
severity: DiagnosticSeverity.Error,
code: "explicit-expression-not-allowed"
});
}
}
}
// Handle implicit if conditions (StringToken without ${{ }})
// These allow expression syntax without the markers
if (isString(token) && token.range) {
if (definitionKey === "step-if" || definitionKey === "runs-if") {
validateImplicitIfCondition(diagnostics, token);
}
}
}
}
const LITERAL_TEXT_IN_CONDITION_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 ${{ }}?";
const LITERAL_TEXT_IN_CONDITION_CODE = "expression-literal-text-in-condition";
/**
* Validates an implicit if condition (StringToken without ${{ }}).
* Checks for literal text detection and validates format() calls.
*/
function validateImplicitIfCondition(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 l = new Lexer(finalCondition);
const lr = l.lex();
const p = new Parser(lr.tokens, namedContexts, functions);
const expr = p.parse();
// Check for literal text in the expression (format with literal text)
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 ${{ }}?",
message: LITERAL_TEXT_IN_CONDITION_MESSAGE,
range: mapRange(token.range),
severity: DiagnosticSeverity.Error,
code: "expression-literal-text-in-condition"
code: LITERAL_TEXT_IN_CONDITION_CODE
});
}
// Validate format() function calls
validateFormatCallsAndAddDiagnostics(diagnostics, expr, token.range);
} catch {
// Ignore parse errors here - they'll be caught by schema validation
// Ignore parse errors - they'll be caught by schema validation or workflow-parser
}
}
/**
* 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.
* Validates a BasicExpressionToken for literal text in if conditions.
*/
function validateIfConditionExpression(diagnostics: Diagnostic[], token: BasicExpressionToken): void {
function validateIfLiteralText(diagnostics: Diagnostic[], token: BasicExpressionToken): void {
const allowedContext = token.definitionInfo?.allowedContext || [];
const {namedContexts, functions} = splitAllowedContext(allowedContext);
@@ -248,16 +278,33 @@ function validateIfConditionExpression(diagnostics: Diagnostic[], token: BasicEx
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 ${{ }}?",
message: LITERAL_TEXT_IN_CONDITION_MESSAGE,
range: mapRange(token.range),
severity: DiagnosticSeverity.Error,
code: "expression-literal-text-in-condition"
code: LITERAL_TEXT_IN_CONDITION_CODE
});
}
// Note: format() validation is done by validateAllExpressions() for all BasicExpressionTokens
} catch {
// Ignore parse errors here - they'll be caught by schema validation
// Ignore parse errors - they'll be caught by schema validation or workflow-parser
}
}
/**
* Validates format() function calls in an expression token.
*/
function validateExpressionFormatCalls(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();
validateFormatCallsAndAddDiagnostics(diagnostics, expr, token.range);
} catch {
// Ignore parse errors - they'll be caught by schema validation
}
}
@@ -304,77 +351,6 @@ 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.
@@ -160,6 +160,21 @@ jobs:
})
);
});
it("errors on unknown context in plain string if condition", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- if: foo == bar
run: echo hi
`;
const result = await validate(createDocument("wf.yaml", input));
expect(result.some(d => d.message.includes("Unrecognized named-value"))).toBe(true);
});
});
describe("snapshot-if", () => {
+1 -1
View File
@@ -6,5 +6,5 @@
"languageservice",
"languageserver"
],
"version": "0.3.41"
"version": "0.3.44"
}
+9 -9
View File
@@ -136,7 +136,7 @@
},
"expressions": {
"name": "@actions/expressions",
"version": "0.3.41",
"version": "0.3.44",
"license": "MIT",
"devDependencies": {
"@types/jest": "^29.0.3",
@@ -396,11 +396,11 @@
},
"languageserver": {
"name": "@actions/languageserver",
"version": "0.3.41",
"version": "0.3.44",
"license": "MIT",
"dependencies": {
"@actions/languageservice": "^0.3.41",
"@actions/workflow-parser": "^0.3.41",
"@actions/languageservice": "^0.3.44",
"@actions/workflow-parser": "^0.3.44",
"@octokit/rest": "^21.1.1",
"@octokit/types": "^9.0.0",
"vscode-languageserver": "^8.0.2",
@@ -940,11 +940,11 @@
},
"languageservice": {
"name": "@actions/languageservice",
"version": "0.3.41",
"version": "0.3.44",
"license": "MIT",
"dependencies": {
"@actions/expressions": "^0.3.41",
"@actions/workflow-parser": "^0.3.41",
"@actions/expressions": "^0.3.44",
"@actions/workflow-parser": "^0.3.44",
"vscode-languageserver-textdocument": "^1.0.7",
"vscode-languageserver-types": "^3.17.2",
"vscode-uri": "^3.0.8",
@@ -13345,10 +13345,10 @@
},
"workflow-parser": {
"name": "@actions/workflow-parser",
"version": "0.3.41",
"version": "0.3.44",
"license": "MIT",
"dependencies": {
"@actions/expressions": "^0.3.41",
"@actions/expressions": "^0.3.44",
"cronstrue": "^2.21.0",
"yaml": "^2.0.0-8"
},
+2 -2
View File
@@ -1,6 +1,6 @@
{
"name": "@actions/workflow-parser",
"version": "0.3.41",
"version": "0.3.44",
"license": "MIT",
"type": "module",
"source": "./src/index.ts",
@@ -48,7 +48,7 @@
"watch": "tsc --build tsconfig.build.json --watch"
},
"dependencies": {
"@actions/expressions": "^0.3.41",
"@actions/expressions": "^0.3.44",
"cronstrue": "^2.21.0",
"yaml": "^2.0.0-8"
},
+2 -1
View File
@@ -150,7 +150,8 @@
"always(0,0)",
"success(0,0)",
"failure(0,0)",
"cancelled(0,0)"
"cancelled(0,0)",
"hashFiles(1,255)"
],
"string": {}
},
@@ -317,4 +317,53 @@ runs:
}
}
});
it("reports error for invalid context in pre-if", () => {
const content = `
name: Node Action
description: A node action
runs:
using: node20
main: dist/index.js
pre: dist/setup.js
pre-if: foo == bar`;
const result = parseAction({name: "action.yml", content}, nullTrace);
expect(result.value).toBeDefined();
if (!result.value) return;
// Should have no errors before conversion
expect(result.context.errors.count).toBe(0);
// Convert the template - this should add the validation error
convertActionTemplate(result.context, result.value);
// Should have an error now about invalid context
expect(result.context.errors.count).toBeGreaterThan(0);
const errors = result.context.errors.getErrors();
expect(errors.some(e => e.rawMessage.includes("foo"))).toBe(true);
});
it("accepts valid context in pre-if", () => {
const content = `
name: Node Action
description: A node action
runs:
using: node20
main: dist/index.js
pre: dist/setup.js
pre-if: runner.os == 'Linux'`;
const result = parseAction({name: "action.yml", content}, nullTrace);
expect(result.value).toBeDefined();
if (!result.value) return;
const template = convertActionTemplate(result.context, result.value);
// Should have no errors
expect(result.context.errors.count).toBe(0);
if (template.runs.using === "node20") {
expect(template.runs.preIf).toBe("runner.os == 'Linux'");
}
});
});
@@ -9,7 +9,7 @@ import {TemplateContext} from "../templates/template-context.js";
import {isBoolean, isMapping, isScalar, isSequence, isString} from "../templates/tokens/type-guards.js";
import {ErrorPolicy} from "../model/convert.js";
import {Step} from "../model/workflow-template.js";
import {convertToIfCondition} from "../model/converter/if-condition.js";
import {convertToIfCondition, validateRunsIfCondition} from "../model/converter/if-condition.js";
/**
* Represents a parsed and converted action.yml file
@@ -310,7 +310,7 @@ function convertRuns(context: TemplateContext, token: TemplateToken): ActionRuns
case "pre-if":
if (isString(item.value)) {
preIf = item.value.value;
preIf = validateRunsIfCondition(context, item.value, item.value.value);
}
break;
@@ -322,7 +322,7 @@ function convertRuns(context: TemplateContext, token: TemplateToken): ActionRuns
case "post-if":
if (isString(item.value)) {
postIf = item.value.value;
postIf = validateRunsIfCondition(context, item.value, item.value.value);
}
break;
+16 -2
View File
@@ -1,3 +1,4 @@
import {FeatureFlags} from "@actions/expressions";
import {TemplateContext} from "../templates/template-context.js";
import {TemplateToken, TemplateTokenError} from "../templates/tokens/template-token.js";
import {FileProvider} from "../workflows/file-provider.js";
@@ -37,9 +38,15 @@ export type WorkflowTemplateConverterOptions = {
* By default, conversion will be skipped if there are errors in the {@link TemplateContext}.
*/
errorPolicy?: ErrorPolicy;
/**
* Feature flags for experimental features.
* When not provided, all experimental features are disabled.
*/
featureFlags?: FeatureFlags;
};
const defaultOptions: Required<WorkflowTemplateConverterOptions> = {
const defaultOptions: Omit<Required<WorkflowTemplateConverterOptions>, "featureFlags"> = {
maxReusableWorkflowDepth: 4,
fetchReusableWorkflowDepth: 0,
errorPolicy: ErrorPolicy.ReturnErrorsOnly
@@ -54,6 +61,11 @@ export async function convertWorkflowTemplate(
const result = {} as WorkflowTemplate;
const opts = getOptionsWithDefaults(options);
// Store feature flags in context for converter functions
if (options.featureFlags) {
context.state["featureFlags"] = options.featureFlags;
}
if (context.errors.getErrors().length > 0 && opts.errorPolicy === ErrorPolicy.ReturnErrorsOnly) {
result.errors = context.errors.getErrors().map(x => ({
Message: x.message
@@ -132,7 +144,9 @@ export async function convertWorkflowTemplate(
return result;
}
function getOptionsWithDefaults(options: WorkflowTemplateConverterOptions): Required<WorkflowTemplateConverterOptions> {
function getOptionsWithDefaults(
options: WorkflowTemplateConverterOptions
): Omit<Required<WorkflowTemplateConverterOptions>, "featureFlags"> {
return {
maxReusableWorkflowDepth:
options.maxReusableWorkflowDepth !== undefined
@@ -0,0 +1,318 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import {nullTrace} from "../../test-utils/null-trace.js";
import {parseWorkflow} from "../../workflows/workflow-parser.js";
import {convertWorkflowTemplate, ErrorPolicy} from "../convert.js";
// Minimal FeatureFlags-compatible object for tests
const featureFlags = {isEnabled: (f: string) => f === "containerImageValidation"};
async function getErrors(content: string): Promise<string[]> {
const result = parseWorkflow({name: "wf.yaml", content}, nullTrace);
result.context.state["featureFlags"] = featureFlags;
const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
errorPolicy: ErrorPolicy.TryConversion
});
return (template.errors ?? []).map((e: {Message: string}) => e.Message);
}
function expectNoContainerErrors(errors: string[]): void {
const containerErrors = errors.filter(e => e.includes("Container image"));
expect(containerErrors).toHaveLength(0);
}
function expectContainerError(errors: string[], count = 1): void {
const containerErrors = errors.filter(e => e.includes("Container image cannot be empty"));
expect(containerErrors).toHaveLength(count);
}
describe("container image validation", () => {
describe("shorthand form", () => {
it("container: '' is silent for job container", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container: ''
steps:
- run: echo hi`);
expectNoContainerErrors(errors);
});
it("container: docker:// errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container: docker://
steps:
- run: echo hi`);
expectContainerError(errors);
});
it("container: valid-image passes", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container: ubuntu:16.04
steps:
- run: echo hi`);
expectNoContainerErrors(errors);
});
});
describe("mapping form", () => {
it("container image: '' errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container:
image: ''
steps:
- run: echo hi`);
expectContainerError(errors);
});
it("container image: docker:// errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container:
image: docker://
steps:
- run: echo hi`);
expectContainerError(errors);
});
it("container: {} (empty object, missing image) errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container: {}
steps:
- run: echo hi`);
expectContainerError(errors);
});
it("container image: null errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container:
image:
steps:
- run: echo hi`);
expectContainerError(errors);
});
it("empty image with expression in other field still errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container:
image: ''
options: \${{ matrix.opts }}
steps:
- run: echo hi`);
expectContainerError(errors);
});
});
describe("services shorthand", () => {
it("services svc: '' errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
services:
svc: ''
steps:
- run: echo hi`);
expectContainerError(errors);
});
it("services svc: docker:// errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
services:
svc: docker://
steps:
- run: echo hi`);
expectContainerError(errors);
});
});
describe("services mapping", () => {
it("services svc image: '' errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
services:
svc:
image: ''
steps:
- run: echo hi`);
expectContainerError(errors);
});
it("services svc image: docker:// errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
services:
svc:
image: docker://
steps:
- run: echo hi`);
expectContainerError(errors);
});
it("services svc: {} (empty object) errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
services:
svc: {}
steps:
- run: echo hi`);
expectContainerError(errors);
});
it("empty image with expression sibling service still errors", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
services:
svc1:
image: ''
svc2: \${{ matrix.svc }}
steps:
- run: echo hi`);
expectContainerError(errors);
});
});
describe("expression safety", () => {
it("container: expression skips validation", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container: \${{ matrix.container }}
steps:
- run: echo hi`);
expectNoContainerErrors(errors);
});
it("container image: expression skips validation", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container:
image: \${{ matrix.image }}
options: --privileged
steps:
- run: echo hi`);
expectNoContainerErrors(errors);
});
it("container with expression key skips validation", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container:
\${{ vars.KEY }}: ubuntu
steps:
- run: echo hi`);
expectNoContainerErrors(errors);
});
it("services: expression skips validation", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
services: \${{ fromJSON(inputs.services) }}
steps:
- run: echo hi`);
expectNoContainerErrors(errors);
});
it("services with expression alias key skips validation", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
services:
\${{ matrix.alias }}: postgres
steps:
- run: echo hi`);
expectNoContainerErrors(errors);
});
it("services container with expression key skips validation", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
services:
db:
\${{ vars.KEY }}: postgres
steps:
- run: echo hi`);
expectNoContainerErrors(errors);
});
it("container with all expression fields skips validation", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
container:
image: \${{ matrix.image }}
options: \${{ matrix.options }}
steps:
- run: echo hi`);
expectNoContainerErrors(errors);
});
it("services svc: expression skips validation", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
services:
db: \${{ matrix.db }}
steps:
- run: echo hi`);
expectNoContainerErrors(errors);
});
it("services image: expression skips validation", async () => {
const errors = await getErrors(`on: push
jobs:
build:
runs-on: linux
services:
db:
image: \${{ matrix.db_image }}
options: --health-cmd pg_isready
steps:
- run: echo hi`);
expectNoContainerErrors(errors);
});
});
});
@@ -1,17 +1,199 @@
import {FeatureFlags} from "@actions/expressions";
import {TemplateContext} from "../../templates/template-context.js";
import {MappingToken, SequenceToken, StringToken, TemplateToken} from "../../templates/tokens/index.js";
import {isString} from "../../templates/tokens/type-guards.js";
import {Container, Credential} from "../workflow-template.js";
export function convertToJobContainer(context: TemplateContext, container: TemplateToken): Container | undefined {
function getFeatureFlags(context: TemplateContext): FeatureFlags | undefined {
return context.state["featureFlags"] as FeatureFlags | undefined;
}
const DOCKER_URI_PREFIX = "docker://";
function isEmptyImage(value: string): boolean {
const trimmed = value.startsWith(DOCKER_URI_PREFIX) ? value.substring(DOCKER_URI_PREFIX.length) : value;
return trimmed.length === 0;
}
export function convertToJobContainer(
context: TemplateContext,
container: TemplateToken,
isServiceContainer = false
): Container | undefined {
// Feature flag guard — use legacy implementation when flag is off
if (!getFeatureFlags(context)?.isEnabled("containerImageValidation")) {
return convertToJobContainerLegacy(context, container);
}
if (container.isExpression) {
return;
}
// Shorthand form
if (isString(container)) {
const image = container.assertString("container item");
if (!image || image.value.length === 0) {
if (isServiceContainer) {
context.error(container, "Container image cannot be empty");
}
return;
}
if (isEmptyImage(image.value)) {
context.error(container, "Container image cannot be empty");
return;
}
return {image};
}
// Mapping form
const mapping = container.assertMapping("container item");
if (!mapping) {
return;
}
let image: StringToken | undefined;
let env: MappingToken | undefined;
let ports: SequenceToken | undefined;
let volumes: SequenceToken | undefined;
let options: StringToken | undefined;
let credentials: Credential | undefined;
let hasExpressionKey = false;
let hasExpression = false;
for (const item of mapping) {
if (item.key.isExpression) {
hasExpressionKey = true;
continue;
}
const key = item.key.assertString("container item key");
switch (key.value) {
case "image":
if (item.value.isExpression) {
hasExpression = true;
break;
}
image = item.value.assertString("container image");
break;
case "credentials":
if (!item.value.isExpression) {
credentials = convertCredentials(context, item.value);
}
break;
case "env":
if (!item.value.isExpression) {
env = item.value.assertMapping("container env");
}
break;
case "ports":
if (!item.value.isExpression) {
ports = item.value.assertSequence("container ports");
}
break;
case "volumes":
if (!item.value.isExpression) {
volumes = item.value.assertSequence("container volumes");
}
break;
case "options":
if (!item.value.isExpression) {
options = item.value.assertString("container options");
}
break;
default:
context.error(key, `Unexpected container item key: ${key.value}`);
}
}
// Validate image
if (image) {
if (isEmptyImage(image.value)) {
context.error(image, "Container image cannot be empty");
return;
}
return {image, credentials, env, ports, volumes, options};
}
// No image key — skip error if expression keys could provide one
if (!hasExpressionKey && !hasExpression) {
context.error(container, "Container image cannot be empty");
}
}
export function convertToJobServices(context: TemplateContext, services: TemplateToken): Container[] | undefined {
// Feature flag guard — use legacy implementation when flag is off
if (!getFeatureFlags(context)?.isEnabled("containerImageValidation")) {
return convertToJobServicesLegacy(context, services);
}
if (services.isExpression) {
return;
}
const serviceList: Container[] = [];
const mapping = services.assertMapping("services");
for (const service of mapping) {
if (service.key.isExpression) {
continue;
}
service.key.assertString("service key");
const container = convertToJobContainer(context, service.value, true);
if (container) {
serviceList.push(container);
}
}
return serviceList;
}
function convertCredentials(context: TemplateContext, value: TemplateToken): Credential | undefined {
const mapping = value.assertMapping("credentials");
if (!mapping) {
return;
}
let username: StringToken | undefined;
let password: StringToken | undefined;
for (const item of mapping) {
if (item.key.isExpression) {
continue;
}
const key = item.key.assertString("credentials item");
if (item.value.isExpression) {
continue;
}
switch (key.value) {
case "username":
username = item.value.assertString("credentials username");
break;
case "password":
password = item.value.assertString("credentials password");
break;
default:
context.error(key, `credentials key ${key.value}`);
}
}
return {username, password};
}
// ===== Legacy implementations (remove when containerImageValidation graduates) =====
function convertToJobContainerLegacy(context: TemplateContext, container: TemplateToken): Container | undefined {
let image: StringToken | undefined;
let env: MappingToken | undefined;
let ports: SequenceToken | undefined;
let volumes: SequenceToken | undefined;
let options: StringToken | undefined;
// Skip validation for expressions for now to match
// behavior of the other parsers
for (const [, token] of TemplateToken.traverse(container)) {
if (token.isExpression) {
return;
@@ -19,7 +201,6 @@ export function convertToJobContainer(context: TemplateContext, container: Templ
}
if (isString(container)) {
// Workflow uses shorthand syntax `container: image-name`
image = container.assertString("container item");
return {image: image};
}
@@ -35,7 +216,7 @@ export function convertToJobContainer(context: TemplateContext, container: Templ
image = value.assertString("container image");
break;
case "credentials":
convertToJobCredentials(context, value);
convertToJobCredentialsLegacy(context, value);
break;
case "env":
env = value.assertMapping("container env");
@@ -70,13 +251,13 @@ export function convertToJobContainer(context: TemplateContext, container: Templ
}
}
export function convertToJobServices(context: TemplateContext, services: TemplateToken): Container[] | undefined {
function convertToJobServicesLegacy(context: TemplateContext, services: TemplateToken): Container[] | undefined {
const serviceList: Container[] = [];
const mapping = services.assertMapping("services");
for (const service of mapping) {
service.key.assertString("service key");
const container = convertToJobContainer(context, service.value);
const container = convertToJobContainerLegacy(context, service.value);
if (container) {
serviceList.push(container);
}
@@ -84,7 +265,7 @@ export function convertToJobServices(context: TemplateContext, services: Templat
return serviceList;
}
function convertToJobCredentials(context: TemplateContext, value: TemplateToken): Credential | undefined {
function convertToJobCredentialsLegacy(context: TemplateContext, value: TemplateToken): Credential | undefined {
const mapping = value.assertMapping("credentials");
let username: StringToken | undefined;
@@ -136,3 +136,32 @@ function walkTreeToFindStatusFunctionCalls(tree: Expr | undefined): boolean {
return false;
}
/**
* Validates a pre-if or post-if condition string.
* Unlike step if conditions, pre-if and post-if are evaluated as-is by the runner
* (they default to always() only when the field is missing entirely).
* This function validates the expression and reports errors through the context.
*
* @param context The template context for error reporting
* @param token The token containing the condition
* @param condition The condition string to validate
* @returns The validated condition string, or undefined on error
*/
export function validateRunsIfCondition(
context: TemplateContext,
token: TemplateToken,
condition: string
): string | undefined {
const allowedContext = token.definitionInfo?.allowedContext || [];
// Validate the expression directly - no wrapping needed for pre-if/post-if
try {
ExpressionToken.validateExpression(condition, allowedContext);
} catch (err) {
context.error(token, err as Error);
return undefined;
}
return condition;
}
+2 -2
View File
@@ -50,7 +50,7 @@ export function convertJob(context: TemplateContext, jobKey: StringToken, token:
break;
case "container":
convertToJobContainer(context, item.value);
handleTemplateTokenErrors(item.value, context, undefined, () => convertToJobContainer(context, item.value));
container = item.value;
break;
@@ -103,7 +103,7 @@ export function convertJob(context: TemplateContext, jobKey: StringToken, token:
break;
case "services":
convertToJobServices(context, item.value);
handleTemplateTokenErrors(item.value, context, undefined, () => convertToJobServices(context, item.value));
services = item.value;
break;
+3 -3
View File
@@ -2172,7 +2172,7 @@
}
},
"step-uses": {
"description": "Selects an action to run as part of a step in your job. An action is a reusable unit of code. You can use an action defined in the same repository as the workflow, a public repository, or in a published Docker container image.",
"description": "Selects an action to run as part of a step in your job. An action is a reusable unit of code. You can use an action defined in the same repository as the workflow, a public repository, a [private repository with access enabled](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#allowing-access-to-components-in-a-private-repository), or in a published Docker container image.",
"string": {
"require-non-empty": true
}
@@ -2345,7 +2345,7 @@
"mapping": {
"properties": {
"image": {
"type": "non-empty-string",
"type": "string",
"description": "Use `jobs.<job_id>.container.image` to define the Docker image to use as the container to run the action. The value can be the Docker Hub image or a registry name."
},
"options": {
@@ -2390,7 +2390,7 @@
"matrix"
],
"one-of": [
"non-empty-string",
"string",
"container-mapping"
]
},
+1
View File
@@ -91,3 +91,4 @@ yaml-schema-sequence.yml
yaml-schema-str-flow-styles.yml
yaml-schema-string.yml
yaml-schema-timestamp.yml
job-container-invalid.yml