diff --git a/actions/setup/js/runtime_import.cjs b/actions/setup/js/runtime_import.cjs index ec31047b8c..0c3d736ef6 100644 --- a/actions/setup/js/runtime_import.cjs +++ b/actions/setup/js/runtime_import.cjs @@ -185,12 +185,36 @@ function isSafeExpression(expr) { // Check if right side is a literal string (single, double, or backtick quotes) const isStringLiteral = /^(['"`]).*\1$/.test(rightExpr); + if (isStringLiteral) { + // Validate string literal content for security + const contentMatch = rightExpr.match(/^(['"`])(.+)\1$/); + if (contentMatch) { + const content = contentMatch[2]; + + // Reject nested expressions + if (content.includes("${{") || content.includes("}}")) { + return false; + } + + // Reject escape sequences that could hide keywords + if (/\\[xu][\da-fA-F]/.test(content) || /\\[0-7]{1,3}/.test(content)) { + return false; + } + + // Reject zero-width characters + if (/[\u200B-\u200D\uFEFF]/.test(content)) { + return false; + } + } + return true; + } + // Check if right side is a number literal const isNumberLiteral = /^-?\d+(\.\d+)?$/.test(rightExpr); // Check if right side is a boolean literal const isBooleanLiteral = rightExpr === "true" || rightExpr === "false"; - if (isStringLiteral || isNumberLiteral || isBooleanLiteral) { + if (isNumberLiteral || isBooleanLiteral) { return true; } @@ -230,7 +254,9 @@ function evaluateExpression(expr) { // If right side is a literal, extract and return it const stringLiteralMatch = rightExpr.match(/^(['"`])(.+)\1$/); if (stringLiteralMatch) { - return stringLiteralMatch[2]; // Return the literal value without quotes + const content = stringLiteralMatch[2]; + // Neutralize any expression markers + return content.replace(/\$/g, "\\$").replace(/\{/g, "\\{"); } // If right side is a number or boolean literal, return it diff --git a/actions/setup/js/runtime_import.test.cjs b/actions/setup/js/runtime_import.test.cjs index fbc91e1af2..c12a57b502 100644 --- a/actions/setup/js/runtime_import.test.cjs +++ b/actions/setup/js/runtime_import.test.cjs @@ -207,6 +207,44 @@ describe("runtime_import", () => { // 5 levels (max) expect(isSafeExpression("needs.job.outputs.foo.bar")).toBe(!0); }); + it("should reject string literals with nested expressions", () => { + // Reject ${{ markers in string literals + expect(isSafeExpression("inputs.value || '${{ secrets.TOKEN }}'")).toBe(!1); + expect(isSafeExpression("inputs.value || 'text ${{ expr }}'")).toBe(!1); + // Reject }} markers in string literals + expect(isSafeExpression("inputs.value || 'text }} more'")).toBe(!1); + // Reject both markers + expect(isSafeExpression("inputs.value || '${{ }} combined'")).toBe(!1); + }); + it("should reject string literals with escape sequences", () => { + // Reject hex escape sequences (\x) + expect(isSafeExpression("inputs.value || '\\x41'")).toBe(!1); + expect(isSafeExpression("inputs.value || 'test\\x20value'")).toBe(!1); + // Reject unicode escape sequences (\u) + expect(isSafeExpression("inputs.value || '\\u0041'")).toBe(!1); + expect(isSafeExpression("inputs.value || 'test\\u0020value'")).toBe(!1); + // Reject octal escape sequences + expect(isSafeExpression("inputs.value || '\\101'")).toBe(!1); + expect(isSafeExpression("inputs.value || 'test\\040value'")).toBe(!1); + }); + it("should reject string literals with zero-width characters", () => { + // Reject zero-width space (U+200B) + expect(isSafeExpression("inputs.value || 'test\u200Bvalue'")).toBe(!1); + // Reject zero-width non-joiner (U+200C) + expect(isSafeExpression("inputs.value || 'test\u200Cvalue'")).toBe(!1); + // Reject zero-width joiner (U+200D) + expect(isSafeExpression("inputs.value || 'test\u200Dvalue'")).toBe(!1); + // Reject zero-width no-break space (U+FEFF) + expect(isSafeExpression("inputs.value || 'test\uFEFFvalue'")).toBe(!1); + }); + it("should allow safe string literals", () => { + // Normal strings should still work + expect(isSafeExpression("inputs.value || 'normal string'")).toBe(!0); + expect(isSafeExpression("inputs.value || 'with-dashes_and_underscores'")).toBe(!0); + // Safe escape sequences like \n, \t should work + expect(isSafeExpression("inputs.value || 'line\\nbreak'")).toBe(!0); + expect(isSafeExpression("inputs.value || 'tab\\there'")).toBe(!0); + }); }), describe("evaluateExpression", () => { beforeEach(() => { @@ -289,6 +327,24 @@ describe("runtime_import", () => { const outOfBounds = evaluateExpression("github.event.release.assets[999].id"); expect(outOfBounds).toContain("${{"); }); + it("should escape $ and { in extracted string literals", () => { + // Test escaping of $ character + expect(evaluateExpression("inputs.missing || 'test$value'")).toBe("test\\$value"); + expect(evaluateExpression("inputs.missing || 'multiple$$$signs'")).toBe("multiple\\$\\$\\$signs"); + // Test escaping of { character + expect(evaluateExpression("inputs.missing || 'test{value'")).toBe("test\\{value"); + expect(evaluateExpression("inputs.missing || 'multiple{{{braces'")).toBe("multiple\\{\\{\\{braces"); + // Test escaping of both $ and { + expect(evaluateExpression("inputs.missing || 'test${value}'")).toBe("test\\$\\{value}"); + expect(evaluateExpression("inputs.missing || '${combined}'")).toBe("\\$\\{combined}"); + }); + it("should not escape characters in non-literal expressions", () => { + // When left side is defined, should not escape + expect(evaluateExpression("inputs.repository || 'default'")).toBe("testorg/testrepo"); + // Number and boolean literals don't need escaping + expect(evaluateExpression("inputs.missing || 42")).toBe("42"); + expect(evaluateExpression("inputs.missing || true")).toBe("true"); + }); }), describe("processRuntimeImport", () => { (it("should read and return file content", async () => { diff --git a/docs/src/content/docs/agent-factory-status.mdx b/docs/src/content/docs/agent-factory-status.mdx index ffe071b46c..1e45720313 100644 --- a/docs/src/content/docs/agent-factory-status.mdx +++ b/docs/src/content/docs/agent-factory-status.mdx @@ -72,7 +72,6 @@ These are experimental agentic workflows used by the GitHub Next team to learn, | [Delight](https://github.com/github/gh-aw/blob/main/.github/workflows/delight.md) | copilot | [![Delight](https://github.com/github/gh-aw/actions/workflows/delight.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/delight.lock.yml) | - | - | | [Dependabot Burner](https://github.com/github/gh-aw/blob/main/.github/workflows/dependabot-burner.md) | copilot | [![Dependabot Burner](https://github.com/github/gh-aw/actions/workflows/dependabot-burner.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/dependabot-burner.lock.yml) | - | - | | [Dependabot Dependency Checker](https://github.com/github/gh-aw/blob/main/.github/workflows/dependabot-go-checker.md) | copilot | [![Dependabot Dependency Checker](https://github.com/github/gh-aw/actions/workflows/dependabot-go-checker.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/dependabot-go-checker.lock.yml) | `0 9 * * 1,3,5` | - | -| [Dependabot Project Manager](https://github.com/github/gh-aw/blob/main/.github/workflows/dependabot-project-manager.md) | copilot | [![Dependabot Project Manager](https://github.com/github/gh-aw/actions/workflows/dependabot-project-manager.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/dependabot-project-manager.lock.yml) | - | - | | [Dev](https://github.com/github/gh-aw/blob/main/.github/workflows/dev.md) | copilot | [![Dev](https://github.com/github/gh-aw/actions/workflows/dev.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/dev.lock.yml) | - | - | | [Dev Hawk](https://github.com/github/gh-aw/blob/main/.github/workflows/dev-hawk.md) | copilot | [![Dev Hawk](https://github.com/github/gh-aw/actions/workflows/dev-hawk.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/dev-hawk.lock.yml) | - | - | | [Developer Documentation Consolidator](https://github.com/github/gh-aw/blob/main/.github/workflows/developer-docs-consolidator.md) | claude | [![Developer Documentation Consolidator](https://github.com/github/gh-aw/actions/workflows/developer-docs-consolidator.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/developer-docs-consolidator.lock.yml) | - | - |