Skip to content

Commit 2f6f210

Browse files
authored
safe-outputs: pre-flight workflow scope check + full-branch allowed_files validation (#42585)
1 parent 98073d0 commit 2f6f210

7 files changed

Lines changed: 447 additions & 17 deletions

actions/setup/js/copilot_harness.cjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,7 @@ async function main() {
715715
// correct SDK endpoint URI.
716716
const sdkEnv = buildCopilotSDKEnv();
717717
const copilotSDKMode = isCopilotSDKEnabled();
718-
let copilotConnectionToken;
718+
let copilotConnectionToken = "";
719719
if (copilotSDKMode) {
720720
// The harness always generates the connection token when SDK mode is active.
721721
// The token is injected into the driver subprocess env so the harness-managed

actions/setup/js/push_to_pull_request_branch.cjs

Lines changed: 97 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,17 +133,94 @@ function isWorkflowsScopeRejection(stderr) {
133133
return lower.includes("`workflows` scope") || lower.includes("workflow can be created or updated due to timeout");
134134
}
135135

136+
/**
137+
* Returns the list of unique workflow file paths (.github/workflows/**) present in the
138+
* local branch history beyond the PR's base branch. This is used as a pre-flight check
139+
* before pushing a new branch ref: GitHub rejects such pushes when the token lacks the
140+
* 'workflows' scope, even if the current changeset itself does not touch workflow files
141+
* (the rejection is based on ALL commits reachable from the pushed ref).
142+
*
143+
* Uses `origin/${baseBranch}` as the exclusion baseline so that commits already on the
144+
* PR's target branch (which GitHub has already accepted) are excluded. Falls back to
145+
* `origin/HEAD` when `baseBranch` is not available, and to an empty array (no workflow
146+
* changes detected) when the baseline ref is not resolvable or the git command fails —
147+
* in that case the push is still attempted and any real 'workflows' scope rejection will
148+
* be caught and surfaced as the typed error downstream.
149+
*
150+
* Note: `origin/${baseBranch}` and `origin/HEAD` are intentionally different baselines
151+
* for their respective layers. `origin/${baseBranch}` limits detection to commits the
152+
* agent actually introduced (correct for the PR delta). Using `origin/HEAD` here would
153+
* traverse commits on the target branch itself for PRs targeting non-default branches,
154+
* producing false-positive `workflows_scope_required` errors.
155+
*
156+
* @param {{ getExecOutput: Function }} exec - @actions/exec module (or compatible mock)
157+
* @param {Record<string, any>} gitOptions - Base git exec options (cwd, env, etc.)
158+
* @param {string | undefined} baseBranch - PR base branch name (e.g. "main"); falls back to origin/HEAD when not provided
159+
* @param {typeof core} coreLogger - Actions core logger used for debug output
160+
* @returns {Promise<string[]>} Unique workflow file paths found in the branch history
161+
*/
162+
async function detectWorkflowFileChanges(exec, gitOptions, baseBranch, coreLogger) {
163+
const baseline = baseBranch && baseBranch.trim() ? `origin/${baseBranch}` : "origin/HEAD";
164+
try {
165+
const result = await exec.getExecOutput("git", ["log", "--name-only", "--pretty=format:", "HEAD", "--not", baseline, "--", ".github/workflows/"], { ...gitOptions, ignoreReturnCode: true });
166+
if (result.exitCode !== 0) {
167+
// Non-zero exit means the baseline ref was not resolvable or git failed;
168+
// treat as no workflow changes so the push proceeds and any real scope
169+
// rejection surfaces downstream.
170+
coreLogger.debug(`detectWorkflowFileChanges: git log exited ${result.exitCode} (baseline '${baseline}' may be unavailable); skipping pre-flight`);
171+
return [];
172+
}
173+
return [
174+
...new Set(
175+
result.stdout
176+
.split("\n")
177+
.map(f => f.trim())
178+
.filter(Boolean)
179+
),
180+
];
181+
} catch (err) {
182+
coreLogger.debug(`detectWorkflowFileChanges: git log threw (baseline '${baseline}'); skipping pre-flight: ${err instanceof Error ? err.message : String(err)}`);
183+
return [];
184+
}
185+
}
186+
187+
/**
188+
* Performs a pre-flight workflow-scope check before pushing a new branch ref.
189+
* Returns the typed error object when the branch history contains workflow file changes
190+
* and `allowWorkflows` is false; returns null when the push may proceed.
191+
*
192+
* Extracts the duplicated guard that appears in both the review-branch and
193+
* fallback-branch push paths so future changes only need to be made in one place.
194+
*
195+
* @param {{ getExecOutput: Function }} exec - @actions/exec module (or compatible mock)
196+
* @param {Record<string, any>} gitOptions - Base git exec options (cwd, env, etc.)
197+
* @param {boolean} allowWorkflows - Whether the push token has the 'workflows' scope
198+
* @param {string | undefined} baseBranch - PR base branch name passed through to detectWorkflowFileChanges
199+
* @param {string} context - Short label for the push path (e.g. "Review branch", "Fallback branch")
200+
* @param {typeof core} coreLogger - Actions core logger
201+
* @returns {Promise<{ success: false, error_type: string, error: string } | null>}
202+
*/
203+
async function runWorkflowScopePreflightCheck(exec, gitOptions, allowWorkflows, baseBranch, context, coreLogger) {
204+
if (allowWorkflows) return null;
205+
const workflowFiles = await detectWorkflowFileChanges(exec, gitOptions, baseBranch, coreLogger);
206+
if (workflowFiles.length > 0) {
207+
coreLogger.info(`Pre-flight check: branch history contains workflow file changes (${workflowFiles.join(", ")}). Failing before push attempt.`);
208+
return buildWorkflowsScopeError(`${context} pre-flight`, coreLogger);
209+
}
210+
return null;
211+
}
212+
136213
/**
137214
* Builds the typed result and logs actionable guidance when a branch push fails
138215
* because the token lacks the 'workflows' scope.
139216
*
140217
* @param {string} context - Short label identifying the push path (e.g. "Review branch", "Fallback branch")
141-
* @param {typeof core} core - Actions core logger
218+
* @param {typeof core} coreLogger - Actions core logger
142219
* @returns {{ success: false, error_type: "workflows_scope_required", error: string }}
143220
*/
144-
function buildWorkflowsScopeError(context, core) {
145-
core.error(`${context} push rejected: the branch includes changes to workflow files (.github/workflows/**) that require the 'workflows' scope on the push token.`);
146-
core.error("To allow this workflow to push workflow file changes, configure 'push-to-pull-request-branch.allow-workflows: true' together with a GitHub App in 'safe-outputs.github-app'.");
221+
function buildWorkflowsScopeError(context, coreLogger) {
222+
coreLogger.error(`${context} push rejected: the branch includes changes to workflow files (.github/workflows/**) that require the 'workflows' scope on the push token.`);
223+
coreLogger.error("To allow this workflow to push workflow file changes, configure 'push-to-pull-request-branch.allow-workflows: true' together with a GitHub App in 'safe-outputs.github-app'.");
147224
return {
148225
success: false,
149226
error_type: "workflows_scope_required",
@@ -170,6 +247,7 @@ async function main(config = {}) {
170247
const commitTitleSuffix = config.commit_title_suffix || "";
171248
const maxSizeKb = parsePositiveInteger(config.max_patch_size) ?? 4096;
172249
const maxCount = config.max || 0; // 0 means no limit
250+
const allowWorkflows = config.allow_workflows === true;
173251

174252
// Cross-repo support: resolve target repository from config
175253
// This allows pushing to PRs in a different repository than the workflow
@@ -1041,6 +1119,15 @@ async function main(config = {}) {
10411119
// normalizeBranchName to enforce valid git ref characters + max length.
10421120
const reviewBranchName = normalizeBranchName(`${branchName}-review`, String(Date.now()));
10431121
try {
1122+
// Pre-flight: check full branch history for workflow file changes.
1123+
// GitHub rejects pushes of new branch refs whose commit history contains
1124+
// .github/workflows/** changes when the token lacks the 'workflows' scope —
1125+
// even if the current changeset itself does not touch workflow files.
1126+
// Failing here avoids leaving the local branch in a renamed state after
1127+
// a rejected push, and surfaces the error before any side effects.
1128+
const preflightError = await runWorkflowScopePreflightCheck(exec, baseGitOpts, allowWorkflows, pullRequest?.base?.ref, "Review branch", core);
1129+
if (preflightError) return preflightError;
1130+
10441131
// Rename current local branch to review branch
10451132
await exec.exec("git", ["checkout", "-b", reviewBranchName], baseGitOpts);
10461133
core.info(`Created review branch: ${reviewBranchName}`);
@@ -1210,6 +1297,12 @@ async function main(config = {}) {
12101297
const fallbackBranchName = normalizeBranchName(`${branchName}-fallback`, String(Date.now()));
12111298
core.warning(`Non-fast-forward push detected; creating fallback pull request from '${fallbackBranchName}' to '${branchName}'`);
12121299
try {
1300+
// Pre-flight: check full branch history for workflow file changes.
1301+
// Like the review branch path, creating a new fallback branch ref triggers
1302+
// GitHub's scope check on the full commit history, not just the new commits.
1303+
const preflightError = await runWorkflowScopePreflightCheck(exec, baseGitOpts, allowWorkflows, pullRequest?.base?.ref, "Fallback branch", core);
1304+
if (preflightError) return preflightError;
1305+
12131306
await exec.exec("git", ["checkout", "-b", fallbackBranchName], baseGitOpts);
12141307
// Use getExecOutput to capture stderr for 'workflows' scope diagnostics
12151308
const fallbackPushOutput = await exec.getExecOutput("git", ["push", "origin", fallbackBranchName], {

actions/setup/js/push_to_pull_request_branch.test.cjs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,11 @@ index 0000000..abc1234
11101110
return { exitCode: 0, stdout: "1111111111111111111111111111111111111111\trefs/heads/feature-branch\n", stderr: "" };
11111111
}
11121112
if (argList[0] === "log") {
1113+
// Pre-flight workflow check targets .github/workflows/; return empty to avoid
1114+
// short-circuiting the fallback path with a workflows_scope_required error.
1115+
if (argList.includes(".github/workflows/")) {
1116+
return { exitCode: 0, stdout: "", stderr: "" };
1117+
}
11131118
return { exitCode: 0, stdout: "Test commit\n", stderr: "" };
11141119
}
11151120
if (argList[0] === "diff-tree") {
@@ -1440,6 +1445,72 @@ index 0000000..abc1234
14401445
expect(result.error_type).toBeUndefined();
14411446
expect(result.error).toContain("Failed to create review PR");
14421447
});
1448+
1449+
it("should fail pre-flight with workflows_scope_required when branch history contains workflow files", async () => {
1450+
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
1451+
createPatchFile("review-branch-preflight-workflow-files");
1452+
1453+
const originalGetExecOutput = mockExec.getExecOutput;
1454+
mockExec.getExecOutput = vi.fn().mockImplementation(async (cmd, args, options) => {
1455+
const argList = Array.isArray(args) ? args : [];
1456+
// Pre-flight git log targets .github/workflows/ directory — returns a workflow
1457+
// file path to simulate branch history containing .github/workflows/** changes.
1458+
if (cmd === "git" && argList[0] === "log" && argList.includes(".github/workflows/")) {
1459+
return { exitCode: 0, stdout: ".github/workflows/ci.yml\n", stderr: "" };
1460+
}
1461+
// The git push should NOT be reached — pre-flight check fires first
1462+
if (cmd === "git" && argList[0] === "push" && argList[1] === "origin") {
1463+
throw new Error("git push should not be called when pre-flight check fires");
1464+
}
1465+
return originalGetExecOutput(cmd, args, options);
1466+
});
1467+
1468+
const module = await loadModule();
1469+
// allow_workflows not set (default false) — pre-flight check is active
1470+
const handler = await module.main({});
1471+
const result = await handler({ branch: "review-branch-preflight-workflow-files" }, {});
1472+
1473+
expect(result.success).toBe(false);
1474+
expect(result.error_type).toBe("workflows_scope_required");
1475+
expect(result.error).toContain("'workflows' scope");
1476+
expect(result.error).toContain("allow-workflows");
1477+
// Pre-flight fires before checkout — no "Failed to create review PR" message
1478+
const errorCalls = mockCore.error.mock.calls.map(c => c[0]);
1479+
expect(errorCalls.some(msg => msg.includes("Failed to create review PR"))).toBe(false);
1480+
expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Pre-flight check"));
1481+
});
1482+
1483+
it("should skip pre-flight check and attempt push when allow_workflows is true", async () => {
1484+
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
1485+
createPatchFile("review-branch-allow-workflows-skip-preflight");
1486+
1487+
let preflightCalled = false;
1488+
let pushCalled = false;
1489+
const originalGetExecOutput = mockExec.getExecOutput;
1490+
mockExec.getExecOutput = vi.fn().mockImplementation(async (cmd, args, options) => {
1491+
const argList = Array.isArray(args) ? args : [];
1492+
if (cmd === "git" && argList[0] === "log" && argList.includes(".github/workflows/")) {
1493+
preflightCalled = true;
1494+
return { exitCode: 0, stdout: ".github/workflows/ci.yml\n", stderr: "" };
1495+
}
1496+
if (cmd === "git" && argList[0] === "push" && argList[1] === "origin") {
1497+
pushCalled = true;
1498+
return { exitCode: 0, stdout: "", stderr: "" };
1499+
}
1500+
return originalGetExecOutput(cmd, args, options);
1501+
});
1502+
1503+
const module = await loadModule();
1504+
// allow_workflows: true — skip the pre-flight check
1505+
const handler = await module.main({ allow_workflows: true });
1506+
const result = await handler({ branch: "review-branch-allow-workflows-skip-preflight" }, {});
1507+
1508+
// Pre-flight check should NOT have run
1509+
expect(preflightCalled).toBe(false);
1510+
// Push should have been attempted
1511+
expect(pushCalled).toBe(true);
1512+
expect(result.success).toBe(true);
1513+
});
14431514
});
14441515

14451516
// ──────────────────────────────────────────────────────

actions/setup/js/safe_outputs_handlers.cjs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,6 +1250,67 @@ function createHandlers(server, appendSafeOutput, config = {}) {
12501250
pushPinnedSha = null;
12511251
}
12521252

1253+
// Full-branch allowed_files check: validate that ALL commits on the PR branch
1254+
// (relative to origin/baseBranch) only touch files permitted by allowed_files.
1255+
// The incremental patch check at apply-time only inspects the net diff between
1256+
// origin/<branch> and the local branch tip; this catches disallowed files that
1257+
// appear in earlier commits on the branch (e.g. a Copilot branch that also
1258+
// modified .github/workflows/**) and returns an actionable error to the agent
1259+
// before any transport artifacts are generated.
1260+
if (Array.isArray(pushConfig.allowed_files) && pushConfig.allowed_files.length > 0) {
1261+
try {
1262+
// Use the pinned SHA as the range head to avoid any TOCTOU window between
1263+
// the time the SHA was recorded and the time of the git log query. If no
1264+
// pinned SHA is available (e.g. non-bundle path), skip the check so we do
1265+
// not race against a mutable ref; the apply-time check still enforces policy.
1266+
if (!pushPinnedSha) {
1267+
server.debug("Full-branch allowed-files check skipped: branch SHA not pinned (non-bundle path)");
1268+
} else {
1269+
const branchHistoryFiles = execGitSync(["log", "--name-only", "--pretty=format:", `origin/${baseBranch}..${pushPinnedSha}`, "--"], { cwd: pushGitCwd })
1270+
.toString()
1271+
.split("\n")
1272+
.map(s => s.trim())
1273+
.filter(Boolean);
1274+
1275+
if (branchHistoryFiles.length > 0) {
1276+
const allowedPatterns = pushConfig.allowed_files.map(p => globPatternToRegex(p));
1277+
// Files matching excluded_files are intentionally exempt: they will be stripped
1278+
// from the patch at generation time via :(exclude) pathspecs, so they won't be
1279+
// present in the final changeset applied to the branch.
1280+
const excludedPatterns = Array.isArray(pushConfig.excluded_files) ? pushConfig.excluded_files.map(p => globPatternToRegex(p)) : [];
1281+
const uniqueFiles = [...new Set(branchHistoryFiles)];
1282+
const disallowedFiles = uniqueFiles.filter(f => !allowedPatterns.some(re => re.test(f)) && !excludedPatterns.some(re => re.test(f)));
1283+
1284+
if (disallowedFiles.length > 0) {
1285+
const sample = disallowedFiles.slice(0, 5);
1286+
const remaining = disallowedFiles.length - sample.length;
1287+
const filesStr = remaining > 0 ? `${sample.join(", ")} (+${remaining} more)` : sample.join(", ");
1288+
server.debug(`Full-branch allowed-files check failed: ${filesStr}`);
1289+
return {
1290+
content: [
1291+
{
1292+
type: "text",
1293+
text: JSON.stringify({
1294+
result: "error",
1295+
error: `Cannot push to pull request branch: the branch '${entry.branch}' history contains commits that modify files outside the allowed-files configuration: ${filesStr}. Remove the disallowed file changes from your commits and retry, or update the allowed-files configuration to include these files.`,
1296+
disallowed_files: disallowedFiles,
1297+
}),
1298+
},
1299+
],
1300+
isError: true,
1301+
};
1302+
}
1303+
}
1304+
}
1305+
} catch (fullBranchCheckError) {
1306+
// Non-fatal: if origin/baseBranch is not available locally or git fails,
1307+
// skip the full-branch check and continue. The apply-time policy check in
1308+
// push_to_pull_request_branch.cjs will still enforce allowed_files against
1309+
// the incremental patch content.
1310+
server.debug(`Full-branch allowed-files check skipped (non-fatal): ${getErrorMessage(fullBranchCheckError)}`);
1311+
}
1312+
}
1313+
12531314
// Always generate an incremental patch for policy enforcement (allowed-files/protected-files/excluded-files),
12541315
// even when bundle transport is selected for apply-time commit transport.
12551316
server.debug(`Generating incremental patch for push_to_pull_request_branch with branch: ${entry.branch}, baseBranch: ${baseBranch}`);

0 commit comments

Comments
 (0)