Skip to content

Commit 3093789

Browse files
authored
safe-outputs: validate extracted base branch with git check-ref-format refs/heads/<name> (#40001)
1 parent 05425ab commit 3093789

3 files changed

Lines changed: 47 additions & 5 deletions

File tree

actions/setup/js/extract_base_branch_from_agent_output.cjs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
/// <reference types="@actions/github-script" />
33

44
const fs = require("fs");
5+
const { spawnSync } = require("child_process");
56

67
const AGENT_OUTPUT_PATH = "/tmp/gh-aw/agent_output.json";
7-
const SAFE_BRANCH_NAME_REGEX = /^[a-zA-Z0-9/_.-]+$/;
8+
const MAX_BRANCH_NAME_LENGTH = 255;
89

910
/**
1011
* @param {string} itemRepo
@@ -49,9 +50,26 @@ function extractBaseBranchFromAgentOutput(opts = {}) {
4950
async function main() {
5051
const baseBranch = extractBaseBranchFromAgentOutput();
5152
if (!baseBranch) return;
52-
if (!SAFE_BRANCH_NAME_REGEX.test(baseBranch) || baseBranch.length > 255) return;
53+
if (!isValidBaseBranchName(baseBranch)) return;
5354
core.setOutput("base-branch", baseBranch);
5455
core.info(`Extracted base branch from safe output: ${baseBranch}`);
5556
}
5657

57-
module.exports = { extractBaseBranchFromAgentOutput, isSameWorkflowRepo, main };
58+
/**
59+
* @param {string} branchName
60+
* @returns {boolean}
61+
*/
62+
function isValidBaseBranchName(branchName) {
63+
if (!branchName || branchName.length > MAX_BRANCH_NAME_LENGTH) {
64+
return false;
65+
}
66+
67+
// Use refs/heads/<name> to validate as a literal ref, not a branch expression.
68+
// --branch also accepts @{-N} git expressions; refs/heads/ form correctly rejects them.
69+
// Fail-closed: if git is unavailable (ENOENT) or times out (ETIMEDOUT), result.error is set
70+
// and we return false, safely dropping the base branch rather than passing an invalid value.
71+
const result = spawnSync("git", ["check-ref-format", `refs/heads/${branchName}`], { stdio: "ignore", timeout: 5000 });
72+
return !result.error && result.status === 0;
73+
}
74+
75+
module.exports = { extractBaseBranchFromAgentOutput, isSameWorkflowRepo, isValidBaseBranchName, main };

actions/setup/js/extract_base_branch_from_agent_output.test.cjs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import { describe, it, expect } from "vitest";
33
import fs from "fs";
44
import path from "path";
5-
import { extractBaseBranchFromAgentOutput, isSameWorkflowRepo } from "./extract_base_branch_from_agent_output.cjs";
5+
import { extractBaseBranchFromAgentOutput, isSameWorkflowRepo, isValidBaseBranchName } from "./extract_base_branch_from_agent_output.cjs";
66

77
describe("extract_base_branch_from_agent_output", () => {
88
it("matches fully-qualified repos", () => {
@@ -70,4 +70,28 @@ describe("extract_base_branch_from_agent_output", () => {
7070
fs.rmSync(tmpDir, { recursive: true, force: true });
7171
}
7272
});
73+
74+
it("accepts valid git branch names used in safe outputs", () => {
75+
expect(isValidBaseBranchName("feature/x")).toBe(true);
76+
expect(isValidBaseBranchName("release/v1.2+hotfix")).toBe(true);
77+
});
78+
79+
it("rejects invalid git branch names even if they look regex-safe", () => {
80+
expect(isValidBaseBranchName("foo..bar")).toBe(false);
81+
expect(isValidBaseBranchName("main.lock")).toBe(false);
82+
expect(isValidBaseBranchName(".foo")).toBe(false);
83+
expect(isValidBaseBranchName("foo/.bar")).toBe(false);
84+
});
85+
86+
it("rejects git branch expressions (@{-N} notation)", () => {
87+
expect(isValidBaseBranchName("@{-1}")).toBe(false);
88+
expect(isValidBaseBranchName("@{-2}")).toBe(false);
89+
});
90+
91+
it("enforces the 255-character length limit", () => {
92+
const atLimit = "a".repeat(255);
93+
const overLimit = "a".repeat(256);
94+
expect(isValidBaseBranchName(atLimit)).toBe(true);
95+
expect(isValidBaseBranchName(overLimit)).toBe(false);
96+
});
7397
});

docs/adr/30071-decouple-safe-outputs-base-branch-from-event-context.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ Insert a `gh api` call into the workflow itself (before checkout) to fetch the P
5656
### Workflow Extraction Step
5757

5858
1. Every compiled workflow job that performs a safe-outputs checkout **MUST** include an `extract-base-branch` step that runs after the agent artifact download and before the checkout step.
59-
2. The extraction step **MUST** validate the extracted branch name against the pattern `^[a-zA-Z0-9/_.-]+$` and enforce a maximum length of 255 characters before writing to `GITHUB_OUTPUT`.
59+
2. The extraction step **MUST** validate the extracted branch name using `git check-ref-format --branch` semantics and enforce a maximum length of 255 characters before writing to `GITHUB_OUTPUT`.
6060
3. The extraction step **MUST NOT** fail the workflow if `agent_output.json` is absent or if no matching safe-output entry contains `base_branch`; it **MUST** exit successfully (silently) in those cases.
6161
4. Checkout steps **MUST** lead the `ref` expression with `steps.extract-base-branch.outputs.base-branch` and **SHOULD** retain event-context fallbacks (`github.base_ref`, `github.event.pull_request.base.ref`, etc.) for backward compatibility.
6262

0 commit comments

Comments
 (0)