diff --git a/actions/setup/js/load_experiment_state_from_repo.cjs b/actions/setup/js/load_experiment_state_from_repo.cjs index 2b55ab778c8..1a301f459e2 100644 --- a/actions/setup/js/load_experiment_state_from_repo.cjs +++ b/actions/setup/js/load_experiment_state_from_repo.cjs @@ -23,6 +23,9 @@ const fs = require("fs"); const path = require("path"); const MAX_STATE_FILE_BYTES = 102400; +// Keep this allowlist aligned with actions/setup/js/normalize_branch_name.cjs valid characters. +const BRANCH_NAME_PATTERN = /^[A-Za-z0-9._/-]+$/; +const REPOSITORY_PATTERN = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/; /** * Returns true when decoded state content exceeds allowed byte length. @@ -35,6 +38,35 @@ function checkLimit(content, maxBytes) { return content.length > maxBytes; } +/** + * Validate required input values before any API calls. + * + * @param {string} branch + * @param {string} owner + * @param {string} repo + * @param {string} repository + * @returns {{valid: boolean, error?: string}} + */ +function validateInputs(branch, owner, repo, repository) { + if (!branch) { + return { valid: false, error: "GH_AW_EXPERIMENT_BRANCH is not set" }; + } + + if (!BRANCH_NAME_PATTERN.test(branch)) { + return { valid: false, error: "GH_AW_EXPERIMENT_BRANCH contains invalid characters" }; + } + + if (branch.includes("..")) { + return { valid: false, error: "GH_AW_EXPERIMENT_BRANCH contains invalid characters" }; + } + + if (!REPOSITORY_PATTERN.test(repository)) { + return { valid: false, error: "GITHUB_REPOSITORY is not set or invalid" }; + } + + return { valid: true }; +} + /** * Fetch experiment state from the git branch via the GitHub API. * Returns the raw file content as a string, or null when the branch/file is absent. @@ -76,16 +108,12 @@ async function main() { const stateFile = process.env.GH_AW_EXPERIMENT_STATE_FILE || "/tmp/gh-aw/experiments/state.json"; const stateDir = process.env.GH_AW_EXPERIMENT_STATE_DIR || "/tmp/gh-aw/experiments"; const branch = process.env.GH_AW_EXPERIMENT_BRANCH || ""; + const repository = process.env.GITHUB_REPOSITORY || ""; + const [owner, repo] = repository.split("/"); - if (!branch) { - core.warning("GH_AW_EXPERIMENT_BRANCH is not set – starting with empty experiment state"); - fs.mkdirSync(stateDir, { recursive: true }); - return; - } - - const [owner, repo] = (process.env.GITHUB_REPOSITORY || "/").split("/"); - if (!owner || !repo) { - core.warning("GITHUB_REPOSITORY is not set – starting with empty experiment state"); + const validationResult = validateInputs(branch, owner, repo, repository); + if (!validationResult.valid) { + core.warning(`${validationResult.error} – starting with empty experiment state`); fs.mkdirSync(stateDir, { recursive: true }); return; } @@ -133,4 +161,4 @@ async function main() { core.info(`Experiment state written to ${stateFile}`); } -module.exports = { main, fetchFileFromBranch }; +module.exports = { main, fetchFileFromBranch, validateInputs }; diff --git a/actions/setup/js/load_experiment_state_from_repo.test.cjs b/actions/setup/js/load_experiment_state_from_repo.test.cjs index 4013c562451..4c9ea475345 100644 --- a/actions/setup/js/load_experiment_state_from_repo.test.cjs +++ b/actions/setup/js/load_experiment_state_from_repo.test.cjs @@ -16,7 +16,7 @@ const mockGetOctokit = vi.fn(); global.core = mockCore; global.getOctokit = mockGetOctokit; -const { fetchFileFromBranch, main } = await import("./load_experiment_state_from_repo.cjs"); +const { fetchFileFromBranch, main, validateInputs } = await import("./load_experiment_state_from_repo.cjs"); const ENV_KEYS = ["GH_AW_EXPERIMENT_STATE_FILE", "GH_AW_EXPERIMENT_STATE_DIR", "GH_AW_EXPERIMENT_BRANCH", "GITHUB_REPOSITORY"]; const MAX_STATE_FILE_BYTES = 102400; @@ -131,7 +131,116 @@ describe("load_experiment_state_from_repo", () => { }); }); + describe("validateInputs", () => { + const cases = [ + { + name: "accepts valid inputs", + args: ["experiments/my-workflow", "owner", "repo", "owner/repo"], + expected: { valid: true }, + }, + { + name: "rejects empty branch", + args: ["", "owner", "repo", "owner/repo"], + expected: { valid: false, error: "GH_AW_EXPERIMENT_BRANCH is not set" }, + }, + { + name: "rejects branch names with invalid characters", + args: ["experiments/my workflow", "owner", "repo", "owner/repo"], + expected: { valid: false, error: "GH_AW_EXPERIMENT_BRANCH contains invalid characters" }, + }, + { + name: "rejects branch names with path traversal patterns", + args: ["experiments/../../etc/passwd", "owner", "repo", "owner/repo"], + expected: { valid: false, error: "GH_AW_EXPERIMENT_BRANCH contains invalid characters" }, + }, + { + name: "rejects missing owner", + args: ["experiments/my-workflow", "", "repo", "/repo"], + expected: { valid: false, error: "GITHUB_REPOSITORY is not set or invalid" }, + }, + { + name: "rejects missing repo", + args: ["experiments/my-workflow", "owner", "", "owner/"], + expected: { valid: false, error: "GITHUB_REPOSITORY is not set or invalid" }, + }, + { + name: "rejects repository with extra segments", + args: ["experiments/my-workflow", "owner", "repo", "owner/repo/extra"], + expected: { valid: false, error: "GITHUB_REPOSITORY is not set or invalid" }, + }, + { + name: "rejects repository with whitespace", + args: ["experiments/my-workflow", "ow ner", "repo", "ow ner/repo"], + expected: { valid: false, error: "GITHUB_REPOSITORY is not set or invalid" }, + }, + ]; + + for (const testCase of cases) { + it(testCase.name, () => { + expect(validateInputs(...testCase.args)).toEqual(testCase.expected); + }); + } + }); + describe("main", () => { + it("skips fetch when branch name is invalid", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-state-")); + try { + const stateFile = path.join(tmpDir, "state.json"); + const getContent = vi.fn(); + + process.env.GH_AW_EXPERIMENT_STATE_FILE = stateFile; + process.env.GH_AW_EXPERIMENT_STATE_DIR = tmpDir; + process.env.GH_AW_EXPERIMENT_BRANCH = "experiments/my workflow"; + process.env.GITHUB_REPOSITORY = "owner/repo"; + + global.github = { + rest: { + repos: { + getContent, + }, + }, + }; + + await main(); + + expect(getContent).not.toHaveBeenCalled(); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("contains invalid characters")); + expect(fs.existsSync(stateFile)).toBe(false); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("skips fetch when GITHUB_REPOSITORY format is invalid", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-state-")); + try { + const stateFile = path.join(tmpDir, "state.json"); + const getContent = vi.fn(); + + process.env.GH_AW_EXPERIMENT_STATE_FILE = stateFile; + process.env.GH_AW_EXPERIMENT_STATE_DIR = tmpDir; + process.env.GH_AW_EXPERIMENT_BRANCH = "experiments/myworkflow"; + process.env.GITHUB_REPOSITORY = "owner/repo/extra"; + + global.github = { + rest: { + repos: { + getContent, + }, + }, + }; + + await main(); + + expect(getContent).not.toHaveBeenCalled(); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("GITHUB_REPOSITORY is not set or invalid")); + expect(fs.existsSync(stateFile)).toBe(false); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + it("skips oversized state files", async () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-state-")); try {