Merge branch 'main' into allanguigou/case

This commit is contained in:
Allan Guigou
2026-01-13 09:46:34 -05:00
committed by GitHub
16 changed files with 1974 additions and 25 deletions
@@ -0,0 +1,199 @@
/**
* Format string validation for format() function calls.
* Port of Go's format_validator.go from actions-workflow-parser.
*/
import {Expr, FunctionCall, Literal, Binary, Unary, Logical, Grouping, IndexAccess} from "@actions/expressions/ast";
import {Kind} from "@actions/expressions/data/expressiondata";
/**
* Error types for format string validation
*/
export type FormatStringError =
| {type: "invalid-syntax"; message: string}
| {type: "arg-count-mismatch"; expected: number; provided: number};
/**
* Validates a format string and returns the maximum placeholder index.
* Port of Go's validateFormatString from format_validator.go.
*
* @param formatString The format string to validate
* @returns { valid: boolean, maxArgIndex: number } where maxArgIndex is -1 if no placeholders
*/
export function validateFormatString(formatString: string): {valid: boolean; maxArgIndex: number} {
let maxIndex = -1;
let i = 0;
while (i < formatString.length) {
// Find next left brace
let lbrace = -1;
for (let j = i; j < formatString.length; j++) {
if (formatString[j] === "{") {
lbrace = j;
break;
}
}
// Find next right brace
let rbrace = -1;
for (let j = i; j < formatString.length; j++) {
if (formatString[j] === "}") {
rbrace = j;
break;
}
}
// No more braces
if (lbrace < 0 && rbrace < 0) {
break;
}
// Left brace comes first (or only left brace exists)
if (lbrace >= 0 && (rbrace < 0 || lbrace < rbrace)) {
// Check if it's escaped
if (lbrace + 1 < formatString.length && formatString[lbrace + 1] === "{") {
// Escaped left brace
i = lbrace + 2;
continue;
}
// This is a placeholder opening - find the closing brace
rbrace = -1;
for (let j = lbrace + 1; j < formatString.length; j++) {
if (formatString[j] === "}") {
rbrace = j;
break;
}
}
if (rbrace < 0) {
// Missing closing brace
return {valid: false, maxArgIndex: -1};
}
// Validate placeholder content (must be digits only)
if (rbrace === lbrace + 1) {
// Empty placeholder {}
return {valid: false, maxArgIndex: -1};
}
// Parse the index and validate it's all digits
let index = 0;
for (let j = lbrace + 1; j < rbrace; j++) {
const c = formatString[j];
if (c < "0" || c > "9") {
// Non-numeric character
return {valid: false, maxArgIndex: -1};
}
index = index * 10 + (c.charCodeAt(0) - "0".charCodeAt(0));
}
if (index > maxIndex) {
maxIndex = index;
}
i = rbrace + 1;
continue;
}
// Right brace comes first (or only right brace exists)
// Check if it's escaped
if (rbrace + 1 < formatString.length && formatString[rbrace + 1] === "}") {
// Escaped right brace
i = rbrace + 2;
continue;
}
// Unescaped right brace outside of placeholder
return {valid: false, maxArgIndex: -1};
}
return {valid: true, maxArgIndex: maxIndex};
}
/**
* Walks an expression AST to find and validate all format() function calls.
*
* @param expr The expression AST to validate
* @returns Array of validation errors found
*/
export function validateFormatCalls(expr: Expr): FormatStringError[] {
const errors: FormatStringError[] = [];
const stack: Expr[] = [expr];
while (stack.length > 0) {
const node = stack.pop();
if (!node) {
continue;
}
if (node instanceof FunctionCall) {
if (node.functionName.lexeme.toLowerCase() === "format") {
const error = validateSingleFormatCall(node);
if (error) {
errors.push(error);
}
}
// Push args for further processing (to find nested format calls)
for (const arg of node.args) {
stack.push(arg);
}
} else if (node instanceof Binary) {
stack.push(node.left, node.right);
} else if (node instanceof Unary) {
stack.push(node.expr);
} else if (node instanceof Logical) {
for (const arg of node.args) {
stack.push(arg);
}
} else if (node instanceof Grouping) {
stack.push(node.group);
} else if (node instanceof IndexAccess) {
stack.push(node.expr, node.index);
}
// Literal, ContextAccess - no children to process
}
return errors;
}
/**
* Validates a single format() function call.
*
* @param fc The FunctionCall AST node
* @returns Validation error if found, undefined if valid
*/
function validateSingleFormatCall(fc: FunctionCall): FormatStringError | undefined {
// Must have at least one argument (the format string)
if (fc.args.length < 1) {
return undefined;
}
// First argument must be a string literal
const firstArg = fc.args[0];
if (!(firstArg instanceof Literal) || firstArg.literal.kind !== Kind.String) {
return undefined; // Can't validate dynamic format strings
}
const formatString = firstArg.literal.coerceString();
const numArgs = fc.args.length - 1; // Subtract 1 for format string itself
const {valid, maxArgIndex} = validateFormatString(formatString);
if (!valid) {
return {
type: "invalid-syntax",
message: "Format string has invalid syntax (missing closing brace, unescaped braces, or invalid placeholder)"
};
}
if (maxArgIndex >= numArgs) {
return {
type: "arg-count-mismatch",
expected: maxArgIndex + 1, // Convert 0-based index to count
provided: numArgs
};
}
return undefined;
}
@@ -0,0 +1,835 @@
import {FeatureFlags} from "@actions/expressions";
import {DiagnosticSeverity} from "vscode-languageserver-types";
import {registerLogger} from "./log.js";
import {createDocument} from "./test-utils/document.js";
import {TestLogger} from "./test-utils/logger.js";
import {clearCache} from "./utils/workflow-cache.js";
import {validate, ValidationConfig} from "./validate.js";
registerLogger(new TestLogger());
const configWithFlag: ValidationConfig = {
featureFlags: new FeatureFlags({blockScalarChompingWarning: true})
};
beforeEach(() => {
clearCache();
});
describe("block scalar chomping - warning cases", () => {
describe("step-level env values", () => {
it("warns with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo $VAR
env:
VAR: |
\${{ matrix.value }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
it("does not warn with keep chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo $VAR
env:
VAR: |+
\${{ matrix.value }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("does not warn with strip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo $VAR
env:
VAR: |-
\${{ matrix.value }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("uses > indicator in warning message for folded scalars", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo $VAR
env:
VAR: >
\${{ matrix.value }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '>' implicitly adds a trailing newline that may be unintentional. Use '>-' to remove it, or '>+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
it("warns for plain string env value with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo $VAR
env:
VAR: |
hello world
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
});
describe("job-level env values", () => {
it("warns with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
env:
MY_VAR: |
some value
steps:
- run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
});
describe("workflow-level env values", () => {
it("warns with clip chomping", async () => {
const input = `
on: push
env:
GLOBAL_VAR: |
some value
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
});
describe("container env values", () => {
it("warns with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
container:
image: node:18
env:
CONTAINER_VAR: |
some value
steps:
- run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
});
describe("service container env values", () => {
it("warns with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
services:
redis:
image: redis
env:
REDIS_PASSWORD: |
secret123
steps:
- run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
});
describe("action input (with)", () => {
it("warns with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
script: |
\${{ matrix.value }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
it("does not warn with keep chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
script: |+
\${{ matrix.value }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("does not warn with strip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
script: |-
\${{ matrix.value }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
});
describe("reusable workflow inputs (with)", () => {
it("warns with clip chomping", async () => {
const input = `
on: push
jobs:
call-workflow:
uses: ./.github/workflows/reusable.yml
with:
my-input: |
some value
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
});
describe("reusable workflow secrets", () => {
it("warns with clip chomping", async () => {
const input = `
on: push
jobs:
call-workflow:
uses: ./.github/workflows/reusable.yml
secrets:
my-secret: |
\${{ secrets.TOKEN }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
});
describe("job outputs", () => {
it("warns with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
outputs:
my_output: |
\${{ steps.test.outputs.value }}
steps:
- id: test
run: echo "value=test" >> $GITHUB_OUTPUT
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
it("does not warn with strip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
outputs:
my_output: |-
\${{ steps.test.outputs.value }}
steps:
- id: test
run: echo "value=test" >> $GITHUB_OUTPUT
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
});
describe("matrix values", () => {
it("warns for matrix vector value with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
config:
- |
value1
- value2
steps:
- run: echo \${{ matrix.config }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
it("does not warn with strip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
config:
- |-
value1
- value2
steps:
- run: echo \${{ matrix.config }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("warns for matrix include value with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
os: [ubuntu-latest]
include:
- os: |
windows-latest
special: true
steps:
- run: echo \${{ matrix.os }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
it("warns for matrix exclude value with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
node: [16, 18]
exclude:
- os: |
windows-latest
node: 16
steps:
- run: echo \${{ matrix.os }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
it("warns for deeply nested matrix value with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
config:
- foo:
bar: |
baz
steps:
- run: echo \${{ matrix.config }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
it("warns for deeply nested matrix include value with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
os: [ubuntu-latest]
include:
- os: ubuntu-latest
config:
nested: |
value
steps:
- run: echo \${{ matrix.config }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
it("warns for deeply nested matrix exclude value with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
exclude:
- os: windows-latest
config:
nested: |
value
steps:
- run: echo \${{ matrix.os }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
});
describe("concurrency", () => {
it("warns for concurrency string with clip chomping", async () => {
const input = `
on: push
concurrency: |
my-group-\${{ github.ref }}
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
it("does not warn for concurrency with strip chomping", async () => {
const input = `
on: push
concurrency: |-
my-group-\${{ github.ref }}
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("warns for concurrency.group with clip chomping", async () => {
const input = `
on: push
concurrency:
group: |
my-group-\${{ github.ref }}
cancel-in-progress: true
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
it("warns for job-level concurrency with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
concurrency: |
job-group-\${{ github.ref }}
steps:
- run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result).toContainEqual(
expect.objectContaining({
message:
"Block scalar '|' implicitly adds a trailing newline that may be unintentional. Use '|-' to remove it, or '|+' to explicitly keep it.",
code: "block-scalar-chomping",
severity: DiagnosticSeverity.Warning
})
);
});
});
});
describe("block scalar chomping - no warning cases", () => {
describe("fields trimmed server-side", () => {
it("does not warn for job-if with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
if: |
github.ref == 'refs/heads/main'
steps:
- run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("does not warn for step-if with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo done
if: |
github.ref == 'refs/heads/main'
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("does not warn for runs-on with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: |
ubuntu-latest
steps:
- run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("does not warn for job name with clip chomping", async () => {
const input = `
on: push
jobs:
build:
name: |
My Job
runs-on: ubuntu-latest
steps:
- run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("does not warn for step name with clip chomping", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- name: |
My Step
run: echo done
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
});
describe("run field (intentionally allowed)", () => {
it("does not warn for step run field", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: |
echo hello
echo world
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("does not warn for run field with expression", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: |
echo \${{ github.ref }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
});
describe("non-block scalars", () => {
it("does not warn for quoted strings", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo $VAR
env:
VAR: "hello world"
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("does not warn for flow scalars", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo $VAR
env:
VAR: hello world
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
it("does not warn for inline expressions", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo $VAR
env:
VAR: \${{ matrix.value }}
`;
const result = await validate(createDocument("wf.yaml", input), configWithFlag);
expect(result.filter(d => d.code === "block-scalar-chomping")).toEqual([]);
});
});
});
@@ -0,0 +1,273 @@
import {DiagnosticSeverity} from "vscode-languageserver-types";
import {createDocument} from "./test-utils/document.js";
import {validate} from "./validate.js";
import {clearCache} from "./utils/workflow-cache.js";
import {validateFormatString} from "./validate-format-string.js";
beforeEach(() => {
clearCache();
});
describe("format string validation", () => {
describe("validateFormatString unit tests", () => {
it("returns valid for simple placeholder", () => {
const result = validateFormatString("{0}");
expect(result).toEqual({valid: true, maxArgIndex: 0});
});
it("returns valid for multiple placeholders", () => {
const result = validateFormatString("{0} {1} {2}");
expect(result).toEqual({valid: true, maxArgIndex: 2});
});
it("returns valid for text with placeholder", () => {
const result = validateFormatString("hello {0} world");
expect(result).toEqual({valid: true, maxArgIndex: 0});
});
it("returns valid for escaped left braces", () => {
const result = validateFormatString("{{0}} {0}");
expect(result).toEqual({valid: true, maxArgIndex: 0});
});
it("returns valid for escaped right braces", () => {
const result = validateFormatString("{0}}}");
expect(result).toEqual({valid: true, maxArgIndex: 0});
});
it("returns valid for no placeholders", () => {
const result = validateFormatString("hello world");
expect(result).toEqual({valid: true, maxArgIndex: -1});
});
it("returns invalid for missing closing brace", () => {
const result = validateFormatString("{0");
expect(result).toEqual({valid: false, maxArgIndex: -1});
});
it("returns invalid for empty placeholder", () => {
const result = validateFormatString("{}");
expect(result).toEqual({valid: false, maxArgIndex: -1});
});
it("returns invalid for non-numeric placeholder", () => {
const result = validateFormatString("{abc}");
expect(result).toEqual({valid: false, maxArgIndex: -1});
});
it("returns invalid for unescaped closing brace", () => {
const result = validateFormatString("text } more");
expect(result).toEqual({valid: false, maxArgIndex: -1});
});
it("handles out-of-order placeholders", () => {
const result = validateFormatString("{2} {0} {1}");
expect(result).toEqual({valid: true, maxArgIndex: 2});
});
it("handles repeated placeholders", () => {
const result = validateFormatString("{0} {0} {0}");
expect(result).toEqual({valid: true, maxArgIndex: 0});
});
});
describe("InvalidFormatString workflow validation", () => {
it("errors on missing closing brace", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo \${{ format('{0', github.event_name) }}
`;
const result = await validate(createDocument("wf.yaml", input));
expect(result).toContainEqual(
expect.objectContaining({
code: "invalid-format-string",
severity: DiagnosticSeverity.Error
})
);
});
it("errors on empty braces", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo \${{ format('{}', github.event_name) }}
`;
const result = await validate(createDocument("wf.yaml", input));
expect(result).toContainEqual(
expect.objectContaining({
code: "invalid-format-string"
})
);
});
it("errors on non-numeric placeholder", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo \${{ format('{abc}', github.event_name) }}
`;
const result = await validate(createDocument("wf.yaml", input));
expect(result).toContainEqual(
expect.objectContaining({
code: "invalid-format-string"
})
);
});
it("allows valid format strings", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo \${{ format('{0} {1}', github.event_name, github.ref) }}
`;
const result = await validate(createDocument("wf.yaml", input));
expect(result).not.toContainEqual(
expect.objectContaining({
code: "invalid-format-string"
})
);
});
it("allows escaped braces", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo \${{ format('{{0}} {0}', github.event_name) }}
`;
const result = await validate(createDocument("wf.yaml", input));
expect(result).not.toContainEqual(
expect.objectContaining({
code: "invalid-format-string"
})
);
});
});
describe("FormatArgCountMismatch workflow validation", () => {
it("errors when placeholder exceeds arg count", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo \${{ format('{2}', 'arg0', 'arg1') }}
`;
const result = await validate(createDocument("wf.yaml", input));
expect(result).toContainEqual(
expect.objectContaining({
code: "format-arg-count-mismatch",
severity: DiagnosticSeverity.Error
})
);
});
it("errors when referencing arg 0 with no args", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo \${{ format('{0}') }}
`;
const result = await validate(createDocument("wf.yaml", input));
expect(result).toContainEqual(
expect.objectContaining({
code: "format-arg-count-mismatch"
})
);
});
it("allows when arg count matches", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo \${{ format('{0} {1} {2}', 'a', 'b', 'c') }}
`;
const result = await validate(createDocument("wf.yaml", input));
expect(result).not.toContainEqual(
expect.objectContaining({
code: "format-arg-count-mismatch"
})
);
});
it("handles no placeholders correctly", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo \${{ format('hello world') }}
`;
const result = await validate(createDocument("wf.yaml", input));
expect(result).not.toContainEqual(
expect.objectContaining({
code: "format-arg-count-mismatch"
})
);
});
it("skips validation for dynamic format strings", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo \${{ format(env.FORMAT_STRING, 'arg') }}
`;
const result = await validate(createDocument("wf.yaml", input));
// Should not have format errors since we can't validate dynamic strings
expect(result).not.toContainEqual(
expect.objectContaining({
code: "invalid-format-string"
})
);
expect(result).not.toContainEqual(
expect.objectContaining({
code: "format-arg-count-mismatch"
})
);
});
it("validates nested format calls", async () => {
const input = `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- run: echo \${{ format('{0}', format('{2}', 'a')) }}
`;
const result = await validate(createDocument("wf.yaml", input));
// The inner format call has an error
expect(result).toContainEqual(
expect.objectContaining({
code: "format-arg-count-mismatch"
})
);
});
});
});
+125 -6
View File
@@ -1,4 +1,4 @@
import {Lexer, Parser, data} from "@actions/expressions";
import {FeatureFlags, Lexer, Parser, data} from "@actions/expressions";
import {Expr, FunctionCall, Literal, Logical} from "@actions/expressions/ast";
import {TemplateParseResult, WorkflowTemplate, isBasicExpression, isMapping, isString} from "@actions/workflow-parser";
import {ErrorPolicy} from "@actions/workflow-parser/model/convert";
@@ -27,6 +27,7 @@ import {mapRange} from "./utils/range.js";
import {getOrConvertWorkflowTemplate, getOrParseWorkflow} from "./utils/workflow-cache.js";
import {validateActionReference} from "./validate-action-reference.js";
import {validateAction} from "./validate-action.js";
import {validateFormatCalls} from "./validate-format-string.js";
import {ValueProviderConfig, ValueProviderKind} from "./value-providers/config.js";
import {defaultValueProviders} from "./value-providers/default.js";
@@ -38,6 +39,7 @@ export type ValidationConfig = {
contextProviderConfig?: ContextProviderConfig;
actionsMetadataProvider?: ActionsMetadataProvider;
fileProvider?: FileProvider;
featureFlags?: FeatureFlags;
};
export type ActionsMetadataProvider = {
@@ -84,7 +86,7 @@ async function validateWorkflow(textDocument: TextDocument, config?: ValidationC
});
// Validate expressions and value providers
await additionalValidations(diagnostics, textDocument.uri, template, result.value, config);
await additionalValidations(diagnostics, textDocument.uri, template, result.value, config, config?.featureFlags);
}
// For now map parser errors directly to diagnostics
@@ -108,9 +110,10 @@ async function additionalValidations(
documentUri: URI,
template: WorkflowTemplate,
root: TemplateToken,
config?: ValidationConfig
config?: ValidationConfig,
featureFlags?: FeatureFlags
) {
for (const [parent, token, key] of TemplateToken.traverse(root)) {
for (const [parent, token, key, ancestors] of TemplateToken.traverse(root)) {
// If the token is a value in a pair, use the key definition for validation
// If the token has a parent (map, sequence, etc), use this definition for validation
const validationToken = key || parent || token;
@@ -128,7 +131,12 @@ async function additionalValidations(
);
}
// If this is a job-if, step-if, or snapshot-if field (which are strings that should be treated as expressions), validate it
// Validate block scalar chomping for expressions and strings
if (featureFlags?.isEnabled("blockScalarChompingWarning")) {
validateBlockScalarChomping(diagnostics, token, parent, key, ancestors);
}
// `if` conditions allow omitting ${{ }}, so validate strings in these fields as expressions
const definitionKey = token.definition?.key;
if (
isString(token) &&
@@ -148,7 +156,9 @@ async function additionalValidations(
finalCondition,
token.definitionInfo,
undefined,
token.source
token.source,
undefined,
token.blockScalarHeader
);
await validateExpression(
@@ -735,6 +745,28 @@ async function validateExpression(
continue;
}
// Validate format() function calls
const formatErrors = validateFormatCalls(expr);
for (const formatError of formatErrors) {
if (formatError.type === "invalid-syntax") {
diagnostics.push({
message: `Invalid format string: ${formatError.message}`,
range: mapRange(expression.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(expression.range),
severity: DiagnosticSeverity.Error,
code: "format-arg-count-mismatch"
});
}
}
const context = await getWorkflowExpressionContext(
namedContexts,
contextProviderConfig,
@@ -822,3 +854,90 @@ function getStaticConcurrencyGroup(token: TemplateToken | undefined): StringToke
return undefined;
}
/**
* Validates YAML block scalar chomping.
*
* Block scalars (| and >) implicitly add a trailing newline by default ("clip" chomping).
* This is often unintended by the workflow author and can cause unexpected behavior.
* This function warns on certain fields when clip chomping is used (implicit trailing newline)
* and suggests they explicitly use strip (|-) or keep (|+) to clarify intent.
*
* Only specific fields are validated - those where trailing newlines may cause
* issues but aren't automatically trimmed server-side. For example env, inputs, outputs, etc.
*
* Skipped fields:
* - run: Multi-line scripts commonly have trailing newlines
* - Fields trimmed server-side: name, uses, shell, if, etc.
*/
function validateBlockScalarChomping(
diagnostics: Diagnostic[],
token: TemplateToken,
parent: TemplateToken | undefined,
key: TemplateToken | undefined,
ancestors: TemplateToken[]
): void {
// Not an expression or string?
if (!isBasicExpression(token) && !isString(token)) {
return;
}
// Not a block scalar?
const header = token.blockScalarHeader;
if (!header) {
return;
}
// Not "clip" chomp style?
if (header.includes("+") || header.includes("-")) {
return;
}
// Check if we should warn
let shouldWarn = false;
const parentDefinitionName = parent?.definition?.key;
const tokenDefinitionName = token.definition?.key;
const keyName = key && isString(key) ? key.value : undefined;
if (
parentDefinitionName &&
[
"workflow-env",
"job-env",
"step-env",
"container-env",
"step-with",
"job-outputs",
"workflow-job-with",
"workflow-job-secrets"
].includes(parentDefinitionName)
) {
// env, with, outputs, or secrets fields
shouldWarn = true;
} else if (
ancestors.some(ancestor => {
const ancestorKey = ancestor.definition?.key;
return ancestorKey === "matrix" || ancestorKey === "matrix-filter" || ancestorKey === "matrix-filter-item";
})
) {
// Matrix values (vectors, include, exclude)
shouldWarn = true;
} else if (tokenDefinitionName && ["workflow-concurrency", "job-concurrency"].includes(tokenDefinitionName)) {
// Concurrency shorthand
shouldWarn = true;
} else if (keyName === "group" && parentDefinitionName === "concurrency-mapping") {
// Concurrency group field
shouldWarn = true;
}
if (!shouldWarn) {
return;
}
const blockIndicator = header.startsWith("|") ? "|" : ">";
diagnostics.push({
message: `Block scalar '${blockIndicator}' implicitly adds a trailing newline that may be unintentional. Use '${blockIndicator}-' to remove it, or '${blockIndicator}+' to explicitly keep it.`,
range: mapRange(token.range),
severity: DiagnosticSeverity.Warning,
code: "block-scalar-chomping"
});
}