Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion actions/setup/js/copilot_harness.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ async function main() {
// correct SDK endpoint URI.
const sdkEnv = buildCopilotSDKEnv();
const copilotSDKMode = isCopilotSDKEnabled();
let copilotConnectionToken;
let copilotConnectionToken = "";
if (copilotSDKMode) {
// The harness always generates the connection token when SDK mode is active.
// The token is injected into the driver subprocess env so the harness-managed
Expand Down
101 changes: 97 additions & 4 deletions actions/setup/js/push_to_pull_request_branch.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -133,17 +133,94 @@ function isWorkflowsScopeRejection(stderr) {
return lower.includes("`workflows` scope") || lower.includes("workflow can be created or updated due to timeout");
}

/**
* Returns the list of unique workflow file paths (.github/workflows/**) present in the
* local branch history beyond the PR's base branch. This is used as a pre-flight check
* before pushing a new branch ref: GitHub rejects such pushes when the token lacks the
* 'workflows' scope, even if the current changeset itself does not touch workflow files
* (the rejection is based on ALL commits reachable from the pushed ref).
*
* Uses `origin/${baseBranch}` as the exclusion baseline so that commits already on the
* PR's target branch (which GitHub has already accepted) are excluded. Falls back to
* `origin/HEAD` when `baseBranch` is not available, and to an empty array (no workflow
* changes detected) when the baseline ref is not resolvable or the git command fails —
* in that case the push is still attempted and any real 'workflows' scope rejection will
* be caught and surfaced as the typed error downstream.
*
* Note: `origin/${baseBranch}` and `origin/HEAD` are intentionally different baselines
* for their respective layers. `origin/${baseBranch}` limits detection to commits the
* agent actually introduced (correct for the PR delta). Using `origin/HEAD` here would
* traverse commits on the target branch itself for PRs targeting non-default branches,
* producing false-positive `workflows_scope_required` errors.
*
* @param {{ getExecOutput: Function }} exec - @actions/exec module (or compatible mock)
* @param {Record<string, any>} gitOptions - Base git exec options (cwd, env, etc.)
* @param {string | undefined} baseBranch - PR base branch name (e.g. "main"); falls back to origin/HEAD when not provided
* @param {typeof core} coreLogger - Actions core logger used for debug output
* @returns {Promise<string[]>} Unique workflow file paths found in the branch history
*/
async function detectWorkflowFileChanges(exec, gitOptions, baseBranch, coreLogger) {
const baseline = baseBranch && baseBranch.trim() ? `origin/${baseBranch}` : "origin/HEAD";
try {
const result = await exec.getExecOutput("git", ["log", "--name-only", "--pretty=format:", "HEAD", "--not", baseline, "--", ".github/workflows/"], { ...gitOptions, ignoreReturnCode: true });
if (result.exitCode !== 0) {
// Non-zero exit means the baseline ref was not resolvable or git failed;
// treat as no workflow changes so the push proceeds and any real scope
// rejection surfaces downstream.
coreLogger.debug(`detectWorkflowFileChanges: git log exited ${result.exitCode} (baseline '${baseline}' may be unavailable); skipping pre-flight`);
return [];
}
return [
...new Set(
result.stdout
.split("\n")
.map(f => f.trim())
Comment thread
pelikhan marked this conversation as resolved.
.filter(Boolean)
),
];
} catch (err) {
coreLogger.debug(`detectWorkflowFileChanges: git log threw (baseline '${baseline}'); skipping pre-flight: ${err instanceof Error ? err.message : String(err)}`);
return [];
}
}

/**
* Performs a pre-flight workflow-scope check before pushing a new branch ref.
* Returns the typed error object when the branch history contains workflow file changes
* and `allowWorkflows` is false; returns null when the push may proceed.
*
* Extracts the duplicated guard that appears in both the review-branch and
* fallback-branch push paths so future changes only need to be made in one place.
*
* @param {{ getExecOutput: Function }} exec - @actions/exec module (or compatible mock)
* @param {Record<string, any>} gitOptions - Base git exec options (cwd, env, etc.)
* @param {boolean} allowWorkflows - Whether the push token has the 'workflows' scope
* @param {string | undefined} baseBranch - PR base branch name passed through to detectWorkflowFileChanges
* @param {string} context - Short label for the push path (e.g. "Review branch", "Fallback branch")
* @param {typeof core} coreLogger - Actions core logger
* @returns {Promise<{ success: false, error_type: string, error: string } | null>}
*/
async function runWorkflowScopePreflightCheck(exec, gitOptions, allowWorkflows, baseBranch, context, coreLogger) {
if (allowWorkflows) return null;
const workflowFiles = await detectWorkflowFileChanges(exec, gitOptions, baseBranch, coreLogger);
if (workflowFiles.length > 0) {
coreLogger.info(`Pre-flight check: branch history contains workflow file changes (${workflowFiles.join(", ")}). Failing before push attempt.`);
return buildWorkflowsScopeError(`${context} pre-flight`, coreLogger);
}
return null;
}

/**
* Builds the typed result and logs actionable guidance when a branch push fails
* because the token lacks the 'workflows' scope.
*
* @param {string} context - Short label identifying the push path (e.g. "Review branch", "Fallback branch")
* @param {typeof core} core - Actions core logger
* @param {typeof core} coreLogger - Actions core logger
* @returns {{ success: false, error_type: "workflows_scope_required", error: string }}
*/
function buildWorkflowsScopeError(context, core) {
core.error(`${context} push rejected: the branch includes changes to workflow files (.github/workflows/**) that require the 'workflows' scope on the push token.`);
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'.");
function buildWorkflowsScopeError(context, coreLogger) {
coreLogger.error(`${context} push rejected: the branch includes changes to workflow files (.github/workflows/**) that require the 'workflows' scope on the push token.`);
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'.");
return {
success: false,
error_type: "workflows_scope_required",
Expand All @@ -170,6 +247,7 @@ async function main(config = {}) {
const commitTitleSuffix = config.commit_title_suffix || "";
const maxSizeKb = parsePositiveInteger(config.max_patch_size) ?? 4096;
const maxCount = config.max || 0; // 0 means no limit
const allowWorkflows = config.allow_workflows === true;

// Cross-repo support: resolve target repository from config
// This allows pushing to PRs in a different repository than the workflow
Expand Down Expand Up @@ -1041,6 +1119,15 @@ async function main(config = {}) {
// normalizeBranchName to enforce valid git ref characters + max length.
const reviewBranchName = normalizeBranchName(`${branchName}-review`, String(Date.now()));
try {
// Pre-flight: check full branch history for workflow file changes.
// GitHub rejects pushes of new branch refs whose commit history contains
// .github/workflows/** changes when the token lacks the 'workflows' scope —
// even if the current changeset itself does not touch workflow files.
// Failing here avoids leaving the local branch in a renamed state after
// a rejected push, and surfaces the error before any side effects.
const preflightError = await runWorkflowScopePreflightCheck(exec, baseGitOpts, allowWorkflows, pullRequest?.base?.ref, "Review branch", core);
if (preflightError) return preflightError;

// Rename current local branch to review branch
await exec.exec("git", ["checkout", "-b", reviewBranchName], baseGitOpts);
core.info(`Created review branch: ${reviewBranchName}`);
Expand Down Expand Up @@ -1210,6 +1297,12 @@ async function main(config = {}) {
const fallbackBranchName = normalizeBranchName(`${branchName}-fallback`, String(Date.now()));
core.warning(`Non-fast-forward push detected; creating fallback pull request from '${fallbackBranchName}' to '${branchName}'`);
try {
// Pre-flight: check full branch history for workflow file changes.
// Like the review branch path, creating a new fallback branch ref triggers
// GitHub's scope check on the full commit history, not just the new commits.
const preflightError = await runWorkflowScopePreflightCheck(exec, baseGitOpts, allowWorkflows, pullRequest?.base?.ref, "Fallback branch", core);
if (preflightError) return preflightError;

await exec.exec("git", ["checkout", "-b", fallbackBranchName], baseGitOpts);
// Use getExecOutput to capture stderr for 'workflows' scope diagnostics
const fallbackPushOutput = await exec.getExecOutput("git", ["push", "origin", fallbackBranchName], {
Expand Down
71 changes: 71 additions & 0 deletions actions/setup/js/push_to_pull_request_branch.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,11 @@ index 0000000..abc1234
return { exitCode: 0, stdout: "1111111111111111111111111111111111111111\trefs/heads/feature-branch\n", stderr: "" };
}
if (argList[0] === "log") {
// Pre-flight workflow check targets .github/workflows/; return empty to avoid
// short-circuiting the fallback path with a workflows_scope_required error.
if (argList.includes(".github/workflows/")) {
return { exitCode: 0, stdout: "", stderr: "" };
}
Comment thread
pelikhan marked this conversation as resolved.
return { exitCode: 0, stdout: "Test commit\n", stderr: "" };
}
if (argList[0] === "diff-tree") {
Expand Down Expand Up @@ -1440,6 +1445,72 @@ index 0000000..abc1234
expect(result.error_type).toBeUndefined();
expect(result.error).toContain("Failed to create review PR");
});

it("should fail pre-flight with workflows_scope_required when branch history contains workflow files", async () => {
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
createPatchFile("review-branch-preflight-workflow-files");

const originalGetExecOutput = mockExec.getExecOutput;
mockExec.getExecOutput = vi.fn().mockImplementation(async (cmd, args, options) => {
const argList = Array.isArray(args) ? args : [];
// Pre-flight git log targets .github/workflows/ directory — returns a workflow
// file path to simulate branch history containing .github/workflows/** changes.
if (cmd === "git" && argList[0] === "log" && argList.includes(".github/workflows/")) {
return { exitCode: 0, stdout: ".github/workflows/ci.yml\n", stderr: "" };
}
// The git push should NOT be reached — pre-flight check fires first
if (cmd === "git" && argList[0] === "push" && argList[1] === "origin") {
throw new Error("git push should not be called when pre-flight check fires");
}
return originalGetExecOutput(cmd, args, options);
});

const module = await loadModule();
// allow_workflows not set (default false) — pre-flight check is active
const handler = await module.main({});
const result = await handler({ branch: "review-branch-preflight-workflow-files" }, {});

expect(result.success).toBe(false);
expect(result.error_type).toBe("workflows_scope_required");
expect(result.error).toContain("'workflows' scope");
expect(result.error).toContain("allow-workflows");
// Pre-flight fires before checkout — no "Failed to create review PR" message
const errorCalls = mockCore.error.mock.calls.map(c => c[0]);
expect(errorCalls.some(msg => msg.includes("Failed to create review PR"))).toBe(false);
expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Pre-flight check"));
});

it("should skip pre-flight check and attempt push when allow_workflows is true", async () => {
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
createPatchFile("review-branch-allow-workflows-skip-preflight");

let preflightCalled = false;
let pushCalled = false;
const originalGetExecOutput = mockExec.getExecOutput;
mockExec.getExecOutput = vi.fn().mockImplementation(async (cmd, args, options) => {
const argList = Array.isArray(args) ? args : [];
if (cmd === "git" && argList[0] === "log" && argList.includes(".github/workflows/")) {
preflightCalled = true;
return { exitCode: 0, stdout: ".github/workflows/ci.yml\n", stderr: "" };
}
if (cmd === "git" && argList[0] === "push" && argList[1] === "origin") {
pushCalled = true;
return { exitCode: 0, stdout: "", stderr: "" };
}
return originalGetExecOutput(cmd, args, options);
});

const module = await loadModule();
// allow_workflows: true — skip the pre-flight check
const handler = await module.main({ allow_workflows: true });
const result = await handler({ branch: "review-branch-allow-workflows-skip-preflight" }, {});

// Pre-flight check should NOT have run
expect(preflightCalled).toBe(false);
// Push should have been attempted
expect(pushCalled).toBe(true);
expect(result.success).toBe(true);
});
});

// ──────────────────────────────────────────────────────
Expand Down
61 changes: 61 additions & 0 deletions actions/setup/js/safe_outputs_handlers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,67 @@ function createHandlers(server, appendSafeOutput, config = {}) {
pushPinnedSha = null;
}

// Full-branch allowed_files check: validate that ALL commits on the PR branch
// (relative to origin/baseBranch) only touch files permitted by allowed_files.
// The incremental patch check at apply-time only inspects the net diff between
// origin/<branch> and the local branch tip; this catches disallowed files that
// appear in earlier commits on the branch (e.g. a Copilot branch that also
// modified .github/workflows/**) and returns an actionable error to the agent
// before any transport artifacts are generated.
if (Array.isArray(pushConfig.allowed_files) && pushConfig.allowed_files.length > 0) {
Comment thread
pelikhan marked this conversation as resolved.
Comment thread
pelikhan marked this conversation as resolved.
try {
// Use the pinned SHA as the range head to avoid any TOCTOU window between
// the time the SHA was recorded and the time of the git log query. If no
// pinned SHA is available (e.g. non-bundle path), skip the check so we do
// not race against a mutable ref; the apply-time check still enforces policy.
if (!pushPinnedSha) {
server.debug("Full-branch allowed-files check skipped: branch SHA not pinned (non-bundle path)");
} else {
const branchHistoryFiles = execGitSync(["log", "--name-only", "--pretty=format:", `origin/${baseBranch}..${pushPinnedSha}`, "--"], { cwd: pushGitCwd })
.toString()
.split("\n")
.map(s => s.trim())
.filter(Boolean);

if (branchHistoryFiles.length > 0) {
const allowedPatterns = pushConfig.allowed_files.map(p => globPatternToRegex(p));
// Files matching excluded_files are intentionally exempt: they will be stripped
// from the patch at generation time via :(exclude) pathspecs, so they won't be
// present in the final changeset applied to the branch.
const excludedPatterns = Array.isArray(pushConfig.excluded_files) ? pushConfig.excluded_files.map(p => globPatternToRegex(p)) : [];
const uniqueFiles = [...new Set(branchHistoryFiles)];
const disallowedFiles = uniqueFiles.filter(f => !allowedPatterns.some(re => re.test(f)) && !excludedPatterns.some(re => re.test(f)));

if (disallowedFiles.length > 0) {
const sample = disallowedFiles.slice(0, 5);
const remaining = disallowedFiles.length - sample.length;
const filesStr = remaining > 0 ? `${sample.join(", ")} (+${remaining} more)` : sample.join(", ");
server.debug(`Full-branch allowed-files check failed: ${filesStr}`);
return {
content: [
{
type: "text",
text: JSON.stringify({
result: "error",
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.`,
disallowed_files: disallowedFiles,
}),
},
],
isError: true,
};
}
}
}
} catch (fullBranchCheckError) {
Comment thread
pelikhan marked this conversation as resolved.
// Non-fatal: if origin/baseBranch is not available locally or git fails,
// skip the full-branch check and continue. The apply-time policy check in
// push_to_pull_request_branch.cjs will still enforce allowed_files against
// the incremental patch content.
Comment thread
pelikhan marked this conversation as resolved.
server.debug(`Full-branch allowed-files check skipped (non-fatal): ${getErrorMessage(fullBranchCheckError)}`);
}
}

// Always generate an incremental patch for policy enforcement (allowed-files/protected-files/excluded-files),
// even when bundle transport is selected for apply-time commit transport.
server.debug(`Generating incremental patch for push_to_pull_request_branch with branch: ${entry.branch}, baseBranch: ${baseBranch}`);
Expand Down
Loading
Loading