Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 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: 2 additions & 0 deletions .github/skills/agentic-workflows/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ This skill is a dispatcher: identify the task type, load the matching workflow p
Read only the files you need:
Load these files from `github/gh-aw` (they are not available locally).
- `.github/aw/agentic-chat.md`
- `.github/aw/agentic-workflows-mcp.md`
- `.github/aw/asciicharts.md`
- `.github/aw/campaign.md`
- `.github/aw/charts-trending.md`
Expand All @@ -27,6 +28,7 @@ Load these files from `github/gh-aw` (they are not available locally).
- `.github/aw/github-agentic-workflows.md`
- `.github/aw/github-mcp-server.md`
- `.github/aw/llms.md`
- `.github/aw/mcp-clis.md`
- `.github/aw/memory.md`
- `.github/aw/messages.md`
- `.github/aw/network.md`
Expand Down
4 changes: 4 additions & 0 deletions actions/setup/js/create_pull_request.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,7 @@ async function main(config = {}) {
signedCommits,
resolvedTemporaryIds,
currentRepo: itemRepo,
validationConfig: config,
});
core.info("Changes pushed to branch (from bundle)");

Expand Down Expand Up @@ -1534,6 +1535,7 @@ async function main(config = {}) {
signedCommits,
resolvedTemporaryIds,
currentRepo: itemRepo,
validationConfig: config,
});
core.info("Changes pushed to branch after bundle rewrite retry");

Expand Down Expand Up @@ -1865,6 +1867,7 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo
signedCommits,
resolvedTemporaryIds,
currentRepo: itemRepo,
validationConfig: config,
});
core.info("Changes pushed to branch");

Expand Down Expand Up @@ -2013,6 +2016,7 @@ ${patchPreview}`;
signedCommits,
resolvedTemporaryIds,
currentRepo: itemRepo,
validationConfig: config,
});
core.info("Empty branch pushed successfully");

Expand Down
4 changes: 2 additions & 2 deletions actions/setup/js/create_pull_request.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ describe("create_pull_request - bundle transport shallow checkout", () => {
vi.clearAllMocks();
});

it("should fetch bundle without forcing an unshallow fetch", async () => {
it("should unshallow before fetching bundle in shallow repositories", async () => {
const patchPath = path.join(tempDir, "test.patch");
fs.writeFileSync(
patchPath,
Expand Down Expand Up @@ -265,7 +265,7 @@ index 0000000..abc1234
expect(global.exec.exec).toHaveBeenCalledWith("git", ["reset", "--hard"]);
const bundleFetchCallIndex = global.exec.getExecOutput.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath);
const unshallowCallIndex = global.exec.exec.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === "--unshallow");
expect(unshallowCallIndex).toBe(-1);
expect(unshallowCallIndex).toBeGreaterThanOrEqual(0);
expect(bundleFetchCallIndex).toBeGreaterThanOrEqual(0);
});

Expand Down
8 changes: 4 additions & 4 deletions actions/setup/js/git_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,8 @@ function hasMergeCommitsInRange(baseRef, headRef, options = {}) {
* depth-1 checkout may not contain those prerequisites, and `git fetch <bundle>`
* can reject the bundle before the caller can update refs.
*
* IMPORTANT: Do not unshallow here. Full-history fetches are prohibitively
* expensive for large monorepos. Callers recover from prerequisite failures by
* fetching only the missing commit objects from origin and retrying.
* When the repository is shallow, unshallow once so bundle prerequisite ancestry
* and merge-base calculations can run against the current base branch safely.
*
* @param {{ getExecOutput: Function, exec: Function }} execApi - Exec API to run git commands.
* @param {Object} [options] - Options passed through to exec calls.
Expand All @@ -176,7 +175,8 @@ async function ensureFullHistoryForBundle(execApi, options = {}) {
return;
}
if (stdout.trim() === "true") {
core.info("Repository is shallow; skipping full-history fetch and relying on prerequisite recovery");
core.info("Repository is shallow; fetching full history before bundle processing");
await execApi.exec("git", ["fetch", "--unshallow", "origin"], options);
}
}

Expand Down
4 changes: 2 additions & 2 deletions actions/setup/js/git_helpers.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ describe("git_helpers.cjs", () => {
});

describe("ensureFullHistoryForBundle", () => {
it("should not fetch full history when the repository is shallow", async () => {
it("should unshallow the repository when the repository is shallow", async () => {
const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs");
const execApi = {
getExecOutput: vi.fn().mockResolvedValue({ stdout: "true\n" }),
Expand All @@ -294,7 +294,7 @@ describe("git_helpers.cjs", () => {
await ensureFullHistoryForBundle(execApi, options);

expect(execApi.getExecOutput).toHaveBeenCalledWith("git", ["rev-parse", "--is-shallow-repository"], options);
expect(execApi.exec).not.toHaveBeenCalled();
expect(execApi.exec).toHaveBeenCalledWith("git", ["fetch", "--unshallow", "origin"], options);
});

it("should not fetch full history when the repository is not shallow", async () => {
Expand Down
128 changes: 125 additions & 3 deletions actions/setup/js/push_signed_commits.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

const { ERR_API } = require("./error_codes.cjs");
const { loadTemporaryIdMapFromResolved, replaceTemporaryIdReferencesInPatch, TEMPORARY_ID_CANDIDATE_REFERENCE_PATTERN } = require("./temporary_id.cjs");
const { checkFileProtectionPostApply } = require("./manifest_file_helpers.cjs");
const OID_PATTERN = /^[0-9a-f]{40}$/i;

/** Sentinel error class used to signal that the commit range contains a shape
Expand All @@ -27,6 +28,15 @@ class PushSignedCommitsUnsupportedShape extends Error {
}
}

/** Sentinel error class for synthesized-payload policy violations. */
class PushSignedCommitsPolicyViolation extends Error {
/** @param {string} message */
constructor(message) {
super(message);
this.name = "PushSignedCommitsPolicyViolation";
}
}

/**
* Unescape a C-quoted path returned by `git diff-tree --raw`.
*
Expand Down Expand Up @@ -183,6 +193,57 @@ async function pushBranchAndResolveHead({ branch, cwd, gitAuthEnv }) {
return resolveLocalHeadSha(cwd);
}

/**
* Enforce limits and file-protection policy on the synthesized GraphQL payload.
*
* @param {Array<{path: string, contents: string}>} additions
* @param {Array<{path: string}>} deletions
* @param {Record<string, any> | undefined} validationConfig
*/
function validateSynthesizedFileChanges(additions, deletions, validationConfig) {
if (!validationConfig) {
return;
}

const uniquePaths = Array.from(
new Set(
[...deletions.map(entry => entry.path), ...additions.map(entry => entry.path)]
.map(path => String(path || "").trim())
.filter(Boolean)
)
);

const maxFilesRaw = Number.parseInt(String(validationConfig.max_patch_files ?? ""), 10);
if (Number.isFinite(maxFilesRaw) && maxFilesRaw > 0 && uniquePaths.length > maxFilesRaw) {
throw new PushSignedCommitsPolicyViolation(
`E003: Signed-commit payload exceeds max-patch-files (${maxFilesRaw}). ` +
`Synthesized payload touches ${uniquePaths.length} file(s): ${uniquePaths.join(", ")}`
);
}

const maxSizeKbRaw = Number.parseInt(String(validationConfig.max_patch_size ?? ""), 10);
if (Number.isFinite(maxSizeKbRaw) && maxSizeKbRaw > 0) {
const additionsBytes = additions.reduce((total, entry) => total + Buffer.from(entry.contents, "base64").length, 0);
const additionsKb = Math.ceil(additionsBytes / 1024);
if (additionsKb > maxSizeKbRaw) {
throw new PushSignedCommitsPolicyViolation(
`E003: Signed-commit payload exceeds max-patch-size (${maxSizeKbRaw} KB). ` +
`Synthesized payload additions total ${additionsKb} KB`
);
}
}

const protection = checkFileProtectionPostApply(uniquePaths, {
...validationConfig,
protected_files_policy: validationConfig.protected_files_policy ?? "request_review",
});
if (protection.action !== "allow") {
throw new PushSignedCommitsPolicyViolation(
`Signed-commit payload violates file-protection policy (${protection.action}): ${protection.files.join(", ")}`
);
}
}

/**
* Resolve the local HEAD SHA.
*
Expand Down Expand Up @@ -211,9 +272,10 @@ async function resolveLocalHeadSha(cwd) {
* @param {boolean} [opts.allowGitPushFallback=true] - When false, refuse any fallback path that would use direct git push
* @param {Record<string, any>} [opts.resolvedTemporaryIds] - Resolved temporary IDs map
* @param {string} [opts.currentRepo] - Repository slug used for same-repo temporary ID resolution
* @param {Record<string, any>} [opts.validationConfig] - Optional safe-output policy config applied to synthesized GraphQL fileChanges
* @returns {Promise<string | undefined>} SHA of the commit that landed on the target branch
*/
async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, cwd, gitAuthEnv, signedCommits = true, allowGitPushFallback = true, resolvedTemporaryIds, currentRepo }) {
async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, cwd, gitAuthEnv, signedCommits = true, allowGitPushFallback = true, resolvedTemporaryIds, currentRepo, validationConfig }) {
const effectiveCurrentRepo = currentRepo || `${owner}/${repo}`;
const temporaryIdMap = loadTemporaryIdMapFromResolved(resolvedTemporaryIds, {
defaultRepo: effectiveCurrentRepo,
Expand Down Expand Up @@ -292,18 +354,74 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c
const fields = line.split(" ");
return { line, fields, sha: fields[0] };
});
const revListEntries = baseRefOid !== undefined ? revListEntriesRaw.filter(entry => entry.sha !== baseRefOid) : revListEntriesRaw;
let revListEntries = baseRefOid !== undefined ? revListEntriesRaw.filter(entry => entry.sha !== baseRefOid) : revListEntriesRaw;
const droppedBoundaryCount = revListEntriesRaw.length - revListEntries.length;
if (baseRefOid !== undefined && droppedBoundaryCount > 0) {
core.info(`pushSignedCommits: dropped ${droppedBoundaryCount} baseRef boundary commit(s) from replay set`);
}
const shas = revListEntries.map(entry => entry.sha);
let shas = revListEntries.map(entry => entry.sha);

if (shas.length === 0) {
core.info("pushSignedCommits: no new commits to push via GraphQL");
return undefined;
}

let remoteHeadOid;
try {
const { stdout: oidOut } = await exec.getExecOutput("git", ["ls-remote", "origin", `refs/heads/${branch}`], { cwd, env: { ...process.env, ...(gitAuthEnv || {}) } });
const resolved = oidOut.trim().split(/\s+/)[0];
if (OID_PATTERN.test(resolved)) {
remoteHeadOid = resolved;
}
} catch {
// Non-fatal. The existing branch-creation logic handles missing remote refs.
}
const firstReplayParentOid = revListEntries[0] && revListEntries[0].fields.length > 1 ? revListEntries[0].fields[1] : undefined;
const firstGraphqlParentOid = remoteHeadOid || baseRefOid;
let graphqlParentIsAncestorOfHead = true;
if (firstGraphqlParentOid) {
try {
const ancestryCheck = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", firstGraphqlParentOid, "HEAD"], { cwd, ignoreReturnCode: true });
graphqlParentIsAncestorOfHead = ancestryCheck.exitCode === 0;
} catch {
// If ancestry probing fails, keep the default (true) and avoid rewrite.
}
}
if (firstReplayParentOid && firstGraphqlParentOid && firstReplayParentOid !== firstGraphqlParentOid && !graphqlParentIsAncestorOfHead) {
core.warning(
`pushSignedCommits: replay parent ${firstReplayParentOid} does not match GraphQL parent ${firstGraphqlParentOid}; ` +
`rebasing commit range before signed replay to avoid stale-base file synthesis`
);
try {
await exec.exec("git", ["rebase", "--onto", firstGraphqlParentOid, firstReplayParentOid, "HEAD"], { cwd });
} catch (rebaseError) {
try {
await exec.exec("git", ["rebase", "--abort"], { cwd });
} catch {
// Ignore cleanup failures.
}
throw new Error(
`pushSignedCommits: failed to rebase commit range onto current GraphQL parent (${firstGraphqlParentOid}). ` +
`Resolve conflicts by rebasing/cherry-picking locally and retry. Root cause: ${rebaseError instanceof Error ? rebaseError.message : String(rebaseError)}`,
{ cause: rebaseError }
);
}
const { stdout: rebasedRevListOut } = await exec.getExecOutput("git", ["rev-list", "--parents", "--topo-order", "--reverse", `${firstGraphqlParentOid}..HEAD`], { cwd });
revListEntries = rebasedRevListOut
.trim()
.split("\n")
.filter(Boolean)
.map(line => {
const fields = line.split(" ");
return { line, fields, sha: fields[0] };
});
shas = revListEntries.map(entry => entry.sha);
if (shas.length === 0) {
core.info("pushSignedCommits: no new commits to replay after rebase");
return undefined;
}
}

core.info(`pushSignedCommits: replaying ${shas.length} commit(s) via GraphQL createCommitOnBranch (branch: ${branch}, repo: ${owner}/${repo})`);

try {
Expand Down Expand Up @@ -513,6 +631,7 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c

const additions = additionsMap.get(sha) || [];
const deletions = deletionsMap.get(sha) || [];
validateSynthesizedFileChanges(additions, deletions, validationConfig);
core.info(`pushSignedCommits: file changes: ${additions.length} addition(s), ${deletions.length} deletion(s)`);

/** @type {any} */
Expand Down Expand Up @@ -550,6 +669,9 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c
{ cause: err }
);
}
if (err instanceof PushSignedCommitsPolicyViolation) {
throw new Error(`pushSignedCommits: refusing unsigned push for branch '${branch}': ${err.message}`, { cause: err });
}
if (allowGitPushFallback === false) {
throw new Error(`pushSignedCommits: signed commit push failed for branch '${branch}' and git push fallback is disabled: ${err instanceof Error ? err.message : String(err)}`, { cause: err });
}
Expand Down
Loading
Loading