Skip to content

Commit d52b9e7

Browse files
authored
Add fork-backed safe-output pull request support (#45909)
1 parent f1e2e6b commit d52b9e7

21 files changed

Lines changed: 1295 additions & 237 deletions

actions/setup/js/create_pull_request.cjs

Lines changed: 106 additions & 28 deletions
Large diffs are not rendered by default.

actions/setup/js/create_pull_request.test.cjs

Lines changed: 53 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as path from "path";
77
import * as os from "os";
88

99
const require = createRequire(import.meta.url);
10+
const promptsSourceDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md");
1011

1112
const { getPatchPathForBranch, getPatchPathForBranchInRepo } = require("./git_patch_utils.cjs");
1213
const { getBundlePathForBranch, getBundlePathForBranchInRepo } = require("./generate_git_bundle.cjs");
@@ -41,8 +42,19 @@ function cleanupCanonicalTransports() {
4142
createdTransportPaths.clear();
4243
}
4344

45+
function copyPromptTemplate(promptsDir, templateName) {
46+
fs.copyFileSync(path.join(promptsSourceDir, templateName), path.join(promptsDir, templateName));
47+
}
48+
49+
function ensureDefaultDisclosureHeaderPrompt() {
50+
const promptsDir = path.join(process.env.RUNNER_TEMP || os.tmpdir(), "gh-aw", "prompts");
51+
fs.mkdirSync(promptsDir, { recursive: true });
52+
copyPromptTemplate(promptsDir, "safe_outputs_disclosure_header.md");
53+
}
54+
4455
beforeEach(() => {
4556
cleanupCanonicalTransports();
57+
ensureDefaultDisclosureHeaderPrompt();
4658
});
4759
afterEach(() => {
4860
cleanupCanonicalTransports();
@@ -1674,12 +1686,10 @@ describe("create_pull_request - allowed-files strict allowlist", () => {
16741686
pushSignedSpy = vi.spyOn(pushSignedCommitsModule, "pushSignedCommits").mockResolvedValue("bundle-tip");
16751687
const promptsDir = path.join(tempDir, "prompts");
16761688
fs.mkdirSync(promptsDir, { recursive: true });
1677-
const requestReviewTemplateSrc = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md/manifest_protection_request_review.md");
1678-
fs.copyFileSync(requestReviewTemplateSrc, path.join(promptsDir, "manifest_protection_request_review.md"));
1679-
const requestChangesTemplateSrc = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md/manifest_protection_request_changes_review.md");
1680-
fs.copyFileSync(requestChangesTemplateSrc, path.join(promptsDir, "manifest_protection_request_changes_review.md"));
1681-
const threatWarningTemplateSrc = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md/threat_warning_request_changes_review.md");
1682-
fs.copyFileSync(threatWarningTemplateSrc, path.join(promptsDir, "threat_warning_request_changes_review.md"));
1689+
copyPromptTemplate(promptsDir, "manifest_protection_request_review.md");
1690+
copyPromptTemplate(promptsDir, "manifest_protection_request_changes_review.md");
1691+
copyPromptTemplate(promptsDir, "threat_warning_request_changes_review.md");
1692+
copyPromptTemplate(promptsDir, "safe_outputs_disclosure_header.md");
16831693
process.env.GH_AW_PROMPTS_DIR = promptsDir;
16841694

16851695
// Clear module cache so globals are picked up fresh
@@ -1882,10 +1892,9 @@ ${diffs}
18821892
const patchPath = writePatch("feature/protected", createPatchWithFiles(".github/aw/instructions.md"));
18831893
const promptsDir = path.join(tempDir, "prompts");
18841894
fs.mkdirSync(promptsDir, { recursive: true });
1885-
const templateSrc = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md/manifest_protection_create_pr_fallback.md");
1886-
fs.copyFileSync(templateSrc, path.join(promptsDir, "manifest_protection_create_pr_fallback.md"));
1887-
const pushFailedTemplateSrc = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md/manifest_protection_push_failed_fallback.md");
1888-
fs.copyFileSync(pushFailedTemplateSrc, path.join(promptsDir, "manifest_protection_push_failed_fallback.md"));
1895+
copyPromptTemplate(promptsDir, "manifest_protection_create_pr_fallback.md");
1896+
copyPromptTemplate(promptsDir, "manifest_protection_push_failed_fallback.md");
1897+
copyPromptTemplate(promptsDir, "safe_outputs_disclosure_header.md");
18891898
process.env.GH_AW_PROMPTS_DIR = promptsDir;
18901899

18911900
global.github.rest.issues = {
@@ -1922,10 +1931,9 @@ ${diffs}
19221931
fs.writeFileSync(bundlePath, "bundle content");
19231932
const promptsDir = path.join(tempDir, "prompts");
19241933
fs.mkdirSync(promptsDir, { recursive: true });
1925-
const templateSrc = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md/manifest_protection_create_pr_fallback.md");
1926-
fs.copyFileSync(templateSrc, path.join(promptsDir, "manifest_protection_create_pr_fallback.md"));
1927-
const pushFailedTemplateSrc = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md/manifest_protection_push_failed_fallback.md");
1928-
fs.copyFileSync(pushFailedTemplateSrc, path.join(promptsDir, "manifest_protection_push_failed_fallback.md"));
1934+
copyPromptTemplate(promptsDir, "manifest_protection_create_pr_fallback.md");
1935+
copyPromptTemplate(promptsDir, "manifest_protection_push_failed_fallback.md");
1936+
copyPromptTemplate(promptsDir, "safe_outputs_disclosure_header.md");
19291937
process.env.GH_AW_PROMPTS_DIR = promptsDir;
19301938

19311939
global.github.rest.issues = {
@@ -2851,6 +2859,7 @@ describe("create_pull_request - patch apply fallback to original base commit", (
28512859
fs.mkdirSync(promptsDir, { recursive: true });
28522860
fs.writeFileSync(path.join(promptsDir, "manifest_protection_request_review.md"), "Protected files: {{files}}", "utf8");
28532861
fs.writeFileSync(path.join(promptsDir, "manifest_protection_request_changes_review.md"), "Protected files: {{files}}", "utf8");
2862+
copyPromptTemplate(promptsDir, "safe_outputs_disclosure_header.md");
28542863
process.env.GH_AW_PROMPTS_DIR = promptsDir;
28552864
global.exec = {
28562865
exec: vi.fn().mockImplementation((cmd, args) => {
@@ -3052,14 +3061,14 @@ describe("create_pull_request - patch apply fallback to original base commit", (
30523061
let renameCalled = false;
30533062
global.exec = {
30543063
exec: vi.fn().mockImplementation((cmd, args) => {
3055-
const cmdStr = typeof cmd === "string" ? cmd : `${cmd} ${(args || []).join(" ")}`;
3064+
const cmdStr = `${cmd} ${(args || []).join(" ")}`;
30563065
if (cmdStr.includes("git branch -m")) {
30573066
renameCalled = true;
30583067
}
30593068
return Promise.resolve(0);
30603069
}),
30613070
getExecOutput: vi.fn().mockImplementation((cmd, args) => {
3062-
const cmdStr = typeof cmd === "string" ? cmd : `${cmd} ${(args || []).join(" ")}`;
3071+
const cmdStr = `${cmd} ${(args || []).join(" ")}`;
30633072
if (cmdStr.includes("ls-remote --heads origin")) {
30643073
return Promise.resolve({ exitCode: 0, stdout: "abc123\trefs/heads/preserve-me\n", stderr: "" });
30653074
}
@@ -3092,7 +3101,7 @@ describe("create_pull_request - patch apply fallback to original base commit", (
30923101
global.exec = {
30933102
exec: vi.fn().mockResolvedValue(0),
30943103
getExecOutput: vi.fn().mockImplementation((cmd, args) => {
3095-
const cmdStr = typeof cmd === "string" ? cmd : `${cmd} ${(args || []).join(" ")}`;
3104+
const cmdStr = `${cmd} ${(args || []).join(" ")}`;
30963105
if (cmdStr.includes("ls-remote --heads origin")) {
30973106
return Promise.resolve({ exitCode: 0, stdout: "abc123\trefs/heads/preserve-me\n", stderr: "" });
30983107
}
@@ -3117,7 +3126,7 @@ describe("create_pull_request - patch apply fallback to original base commit", (
31173126
global.exec = {
31183127
exec: vi.fn().mockResolvedValue(0),
31193128
getExecOutput: vi.fn().mockImplementation((cmd, args) => {
3120-
const cmdStr = typeof cmd === "string" ? cmd : `${cmd} ${(args || []).join(" ")}`;
3129+
const cmdStr = `${cmd} ${(args || []).join(" ")}`;
31213130
if (cmdStr.includes("ls-remote --heads origin")) {
31223131
return Promise.resolve({ exitCode: 0, stdout: "abc123\trefs/heads/preserve-me\n", stderr: "" });
31233132
}
@@ -3142,7 +3151,7 @@ describe("create_pull_request - patch apply fallback to original base commit", (
31423151
let capturedRenamedBranch = null;
31433152
global.exec = {
31443153
exec: vi.fn().mockImplementation((cmd, args) => {
3145-
const cmdStr = typeof cmd === "string" ? cmd : `${cmd} ${(args || []).join(" ")}`;
3154+
const cmdStr = `${cmd} ${(args || []).join(" ")}`;
31463155
// Capture the new branch name from: git branch -m <old> <new>
31473156
const renameMatch = cmdStr.match(/git branch -m \S+ (\S+)/);
31483157
if (renameMatch) {
@@ -3151,7 +3160,7 @@ describe("create_pull_request - patch apply fallback to original base commit", (
31513160
return Promise.resolve(0);
31523161
}),
31533162
getExecOutput: vi.fn().mockImplementation((cmd, args) => {
3154-
const cmdStr = typeof cmd === "string" ? cmd : `${cmd} ${(args || []).join(" ")}`;
3163+
const cmdStr = `${cmd} ${(args || []).join(" ")}`;
31553164
if (cmdStr.includes("ls-remote --heads origin")) {
31563165
return Promise.resolve({ exitCode: 0, stdout: "abc123\trefs/heads/chaos/preserve-me\n", stderr: "" });
31573166
}
@@ -3196,14 +3205,14 @@ describe("create_pull_request - patch apply fallback to original base commit", (
31963205
let renameCalled = false;
31973206
global.exec = {
31983207
exec: vi.fn().mockImplementation((cmd, args) => {
3199-
const cmdStr = typeof cmd === "string" ? cmd : `${cmd} ${(args || []).join(" ")}`;
3208+
const cmdStr = `${cmd} ${(args || []).join(" ")}`;
32003209
if (cmdStr.includes("git branch -m")) {
32013210
renameCalled = true;
32023211
}
32033212
return Promise.resolve(0);
32043213
}),
32053214
getExecOutput: vi.fn().mockImplementation((cmd, args) => {
3206-
const cmdStr = typeof cmd === "string" ? cmd : `${cmd} ${(args || []).join(" ")}`;
3215+
const cmdStr = `${cmd} ${(args || []).join(" ")}`;
32073216
if (cmdStr.includes("ls-remote --heads origin")) {
32083217
return Promise.resolve({ exitCode: 0, stdout: "abc123\trefs/heads/some-branch\n", stderr: "" });
32093218
}
@@ -3393,12 +3402,10 @@ describe("create_pull_request - threat detection caution", () => {
33933402
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-threat-test-"));
33943403
const promptsDir = path.join(tempDir, "prompts");
33953404
fs.mkdirSync(promptsDir, { recursive: true });
3396-
const requestReviewTemplateSrc = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md/manifest_protection_request_review.md");
3397-
fs.copyFileSync(requestReviewTemplateSrc, path.join(promptsDir, "manifest_protection_request_review.md"));
3398-
const requestChangesTemplateSrc = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md/manifest_protection_request_changes_review.md");
3399-
fs.copyFileSync(requestChangesTemplateSrc, path.join(promptsDir, "manifest_protection_request_changes_review.md"));
3400-
const threatWarningTemplateSrc = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md/threat_warning_request_changes_review.md");
3401-
fs.copyFileSync(threatWarningTemplateSrc, path.join(promptsDir, "threat_warning_request_changes_review.md"));
3405+
copyPromptTemplate(promptsDir, "manifest_protection_request_review.md");
3406+
copyPromptTemplate(promptsDir, "manifest_protection_request_changes_review.md");
3407+
copyPromptTemplate(promptsDir, "threat_warning_request_changes_review.md");
3408+
copyPromptTemplate(promptsDir, "safe_outputs_disclosure_header.md");
34023409
process.env.GH_AW_PROMPTS_DIR = promptsDir;
34033410

34043411
global.core = {
@@ -4122,6 +4129,22 @@ describe("create_pull_request - branch-prefix config", () => {
41224129
expect(branchArg).not.toContain("signed/");
41234130
});
41244131

4132+
it("should use an owner-qualified PR head when head-repo differs from target-repo", async () => {
4133+
const { main } = require("./create_pull_request.cjs");
4134+
const handler = await main({
4135+
allow_empty: true,
4136+
"head-repo": "fork-owner/test-repo",
4137+
allowed_repos: ["test-owner/test-repo", "fork-owner/test-repo"],
4138+
});
4139+
4140+
await handler({ title: "Test PR", body: "body", branch: "my-feature" }, {});
4141+
4142+
const createCall = global.github.rest.pulls.create.mock.calls[0][0];
4143+
expect(createCall.owner).toBe("test-owner");
4144+
expect(createCall.repo).toBe("test-repo");
4145+
expect(createCall.head).toMatch(/^fork-owner:my-feature(?:-[0-9a-f]+)?$/);
4146+
});
4147+
41254148
it("should normalize an invalid branch-prefix and emit a warning", async () => {
41264149
const { main } = require("./create_pull_request.cjs");
41274150
const handler = await main({ branch_prefix: "bad prefix: ", allow_empty: true });
@@ -4149,8 +4172,8 @@ describe("create_pull_request - E003 file-limit fallback-to-issue", () => {
41494172
// Set up prompts directory with the E003 template so getPromptPath resolves
41504173
const promptsDir = path.join(tempDir, "prompts");
41514174
fs.mkdirSync(promptsDir, { recursive: true });
4152-
const templateSrc = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../md/e003_file_limit_fallback.md");
4153-
fs.copyFileSync(templateSrc, path.join(promptsDir, "e003_file_limit_fallback.md"));
4175+
copyPromptTemplate(promptsDir, "e003_file_limit_fallback.md");
4176+
copyPromptTemplate(promptsDir, "safe_outputs_disclosure_header.md");
41544177
process.env.GH_AW_PROMPTS_DIR = promptsDir;
41554178

41564179
global.core = {

actions/setup/js/create_pull_request_helpers.cjs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,11 +223,12 @@ function generatePatchPreview(patchContent) {
223223
* @param {string} branchName
224224
* @param {string} title
225225
* @param {number} [fallbackIssueNumber]
226+
* @param {string} [headRef]
226227
* @returns {string}
227228
*/
228-
function buildManifestProtectionCreatePrUrl(githubServer, repoParts, baseBranch, branchName, title, fallbackIssueNumber) {
229+
function buildManifestProtectionCreatePrUrl(githubServer, repoParts, baseBranch, branchName, title, fallbackIssueNumber, headRef) {
229230
const encodedBase = encodePathSegments(baseBranch);
230-
const encodedHead = encodePathSegments(branchName);
231+
const encodedHead = encodePathSegments(headRef || branchName);
231232
let createPrUrl = `${githubServer}/${repoParts.owner}/${repoParts.repo}/compare/${encodedBase}...${encodedHead}?expand=1&title=${encodeURIComponent(title)}`;
232233
if (typeof fallbackIssueNumber === "number") {
233234
createPrUrl += `&body=${encodeURIComponent(`Closes #${fallbackIssueNumber}`)}`;

actions/setup/js/git_auth_helpers.cjs

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,13 @@ function normalizeServerUrl(serverUrl) {
2222
* Returns an empty array when the key is absent (exit code ≠ 0).
2323
*
2424
* @param {string} serverUrl
25+
* @param {string} [cwd] - Optional working directory for the git config command
2526
* @returns {Promise<string[]>}
2627
*/
27-
async function getExtraheaderValues(serverUrl) {
28+
async function getExtraheaderValues(serverUrl, cwd) {
2829
const normalizedUrl = normalizeServerUrl(serverUrl);
29-
const result = await exec.getExecOutput("git", ["config", "--get-all", `http.${normalizedUrl}/.extraheader`], {
30-
silent: true,
31-
ignoreReturnCode: true,
32-
});
30+
const execOptions = cwd ? { silent: true, ignoreReturnCode: true, cwd } : { silent: true, ignoreReturnCode: true };
31+
const result = await exec.getExecOutput("git", ["config", "--get-all", `http.${normalizedUrl}/.extraheader`], execOptions);
3332
if (result.exitCode !== 0 || !result.stdout.trim()) {
3433
return [];
3534
}
@@ -61,21 +60,26 @@ async function checkoutHasPersistedExtraheader(serverUrl) {
6160
*
6261
* @param {string} serverUrl
6362
* @param {string} token
63+
* @param {string} [cwd] - Optional working directory for the git config command
6464
* @returns {Promise<string[]>}
6565
*/
66-
async function overridePersistedExtraheader(serverUrl, token) {
66+
async function overridePersistedExtraheader(serverUrl, token, cwd) {
6767
const normalizedUrl = normalizeServerUrl(serverUrl);
6868
let previousValues;
6969
try {
70-
previousValues = await getExtraheaderValues(serverUrl);
70+
previousValues = await getExtraheaderValues(serverUrl, cwd);
7171
core.info(`git_auth_helpers: read ${previousValues.length} existing extraheader value(s) for ${normalizedUrl}`);
7272
} catch (err) {
7373
core.warning(`git_auth_helpers: could not read existing extraheader values — restoration will proceed with empty defaults: ${getErrorMessage(err)}`);
7474
previousValues = [];
7575
}
7676
core.info(`git_auth_helpers: overriding http.${normalizedUrl}/.extraheader with CI trigger token`);
7777
const tokenBase64 = Buffer.from(`x-access-token:${token.trim()}`).toString("base64");
78-
await exec.exec("git", ["config", "--replace-all", `http.${normalizedUrl}/.extraheader`, `Authorization: basic ${tokenBase64}`]);
78+
if (cwd) {
79+
await exec.exec("git", ["config", "--replace-all", `http.${normalizedUrl}/.extraheader`, `Authorization: basic ${tokenBase64}`], { cwd });
80+
} else {
81+
await exec.exec("git", ["config", "--replace-all", `http.${normalizedUrl}/.extraheader`, `Authorization: basic ${tokenBase64}`]);
82+
}
7983
core.info(`git_auth_helpers: extraheader override applied`);
8084
return previousValues;
8185
}
@@ -85,14 +89,19 @@ async function overridePersistedExtraheader(serverUrl, token) {
8589
*
8690
* @param {string} serverUrl
8791
* @param {string[]} previousValues
92+
* @param {string} [cwd] - Optional working directory for the git config command
8893
* @returns {Promise<void>}
8994
*/
90-
async function restorePersistedExtraheader(serverUrl, previousValues) {
95+
async function restorePersistedExtraheader(serverUrl, previousValues, cwd) {
9196
const key = `http.${normalizeServerUrl(serverUrl)}/.extraheader`;
9297
if (!previousValues || previousValues.length === 0) {
9398
core.info(`git_auth_helpers: no previous extraheader values — unsetting ${key}`);
9499
try {
95-
await exec.exec("git", ["config", "--unset-all", key]);
100+
if (cwd) {
101+
await exec.exec("git", ["config", "--unset-all", key], { cwd });
102+
} else {
103+
await exec.exec("git", ["config", "--unset-all", key]);
104+
}
96105
} catch {
97106
// Nothing to restore/unset.
98107
}
@@ -108,14 +117,25 @@ async function restorePersistedExtraheader(serverUrl, previousValues) {
108117
// best-effort cleanup by unsetting the key entirely, then re-throw so the caller
109118
// is aware that restoration failed.
110119
try {
111-
await exec.exec("git", ["config", "--replace-all", key, previousValues[0]]);
112-
for (const value of previousValues.slice(1)) {
113-
await exec.exec("git", ["config", "--add", key, value]);
120+
if (cwd) {
121+
await exec.exec("git", ["config", "--replace-all", key, previousValues[0]], { cwd });
122+
for (const value of previousValues.slice(1)) {
123+
await exec.exec("git", ["config", "--add", key, value], { cwd });
124+
}
125+
} else {
126+
await exec.exec("git", ["config", "--replace-all", key, previousValues[0]]);
127+
for (const value of previousValues.slice(1)) {
128+
await exec.exec("git", ["config", "--add", key, value]);
129+
}
114130
}
115131
} catch (err) {
116132
core.warning(`git_auth_helpers: partial extraheader restore for ${key} — attempting cleanup: ${getErrorMessage(err)}`);
117133
try {
118-
await exec.exec("git", ["config", "--unset-all", key]);
134+
if (cwd) {
135+
await exec.exec("git", ["config", "--unset-all", key], { cwd });
136+
} else {
137+
await exec.exec("git", ["config", "--unset-all", key]);
138+
}
119139
} catch {
120140
// Best-effort only; ignore secondary failure.
121141
}

0 commit comments

Comments
 (0)