From f93ca50bde35f63a65c67fc94cc8821ead20e631 Mon Sep 17 00:00:00 2001 From: Aiqiao Yan <55104035+aiqiaoy@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:34:58 -0400 Subject: [PATCH 1/2] backport `allow-unsafe-pr-checkout` to v6 (#2500) * block checking out fork pr for pull_request_target and workflow_run (#2454) * block checking out fork pr for some events * address copilot and reviewer feedback * run prettier formatting * build * update urls * update readme * update description and url again * edit url one more time * update error wording (#2467) --- README.md | 9 + __test__/git-auth-helper.test.ts | 3 +- __test__/input-helper.test.ts | 1 + __test__/unsafe-pr-checkout-helper.test.ts | 267 +++++++++++++++++++++ action.yml | 9 + dist/index.js | 112 +++++++++ src/git-source-settings.ts | 6 + src/input-helper.ts | 14 ++ src/ref-helper.ts | 2 +- src/unsafe-pr-checkout-helper.ts | 88 +++++++ 10 files changed, 509 insertions(+), 2 deletions(-) create mode 100644 __test__/unsafe-pr-checkout-helper.test.ts create mode 100644 src/unsafe-pr-checkout-helper.ts diff --git a/README.md b/README.md index f0f65f9..240c4b6 100644 --- a/README.md +++ b/README.md @@ -160,6 +160,15 @@ Please refer to the [release page](https://github.com/actions/checkout/releases/ # running from unless specified. Example URLs are https://github.com or # https://my-ghes-server.example.com github-server-url: '' + + # Required to check out fork pull request code from a workflow triggered by + # `pull_request_target` or `workflow_run`. These workflows run with the base + # repository's GITHUB_TOKEN, secrets, default-branch cache scope, and runner + # access; fetching and executing a fork's code in that trusted context commonly + # leads to "pwn request" vulnerabilities. Set to `true` only after reviewing the + # risks at https://gh.io/securely-using-pull_request_target. + # Default: false + allow-unsafe-pr-checkout: '' ``` diff --git a/__test__/git-auth-helper.test.ts b/__test__/git-auth-helper.test.ts index ad3566a..3c4f049 100644 --- a/__test__/git-auth-helper.test.ts +++ b/__test__/git-auth-helper.test.ts @@ -1173,7 +1173,8 @@ async function setup(testName: string): Promise { sshUser: '', workflowOrganizationId: 123456, setSafeDirectory: true, - githubServerUrl: githubServerUrl + githubServerUrl: githubServerUrl, + allowUnsafePrCheckout: false } } diff --git a/__test__/input-helper.test.ts b/__test__/input-helper.test.ts index 09331eb..25b6d18 100644 --- a/__test__/input-helper.test.ts +++ b/__test__/input-helper.test.ts @@ -91,6 +91,7 @@ describe('input-helper tests', () => { expect(settings.repositoryOwner).toBe('some-owner') expect(settings.repositoryPath).toBe(gitHubWorkspace) expect(settings.setSafeDirectory).toBe(true) + expect(settings.allowUnsafePrCheckout).toBe(false) }) it('qualifies ref', async () => { diff --git a/__test__/unsafe-pr-checkout-helper.test.ts b/__test__/unsafe-pr-checkout-helper.test.ts new file mode 100644 index 0000000..9efa246 --- /dev/null +++ b/__test__/unsafe-pr-checkout-helper.test.ts @@ -0,0 +1,267 @@ +import * as github from '@actions/github' +import {assertSafePrCheckout} from '../lib/unsafe-pr-checkout-helper' + +// Shallow clone original @actions/github context +const originalContext = {...github.context} +const originalEventName = github.context.eventName +const originalPayload = github.context.payload + +const BASE_REPO_ID = 100 +const FORK_REPO_ID = 200 +const PR_HEAD_SHA = '1111111111111111111111111111111111111111' +const PR_MERGE_SHA = '2222222222222222222222222222222222222222' +const SAFE_BASE_SHA = '3333333333333333333333333333333333333333' +const WORKFLOW_RUN_HEAD_COMMIT_SHA = '4444444444444444444444444444444444444444' +const BASE_QUALIFIED_REPO = 'some-owner/some-repo' +const FORK_QUALIFIED_REPO = 'another-repo/fork' + +function setContext(eventName: string, payload: object): void { + ;(github.context as {eventName: string}).eventName = eventName + ;(github.context as {payload: object}).payload = payload +} + +function forkPullRequestTargetPayload(): object { + return { + repository: {id: BASE_REPO_ID}, + pull_request: { + head: { + sha: PR_HEAD_SHA, + repo: {id: FORK_REPO_ID, full_name: FORK_QUALIFIED_REPO} + }, + merge_commit_sha: PR_MERGE_SHA + } + } +} + +function sameRepoPullRequestTargetPayload(): object { + return { + repository: {id: BASE_REPO_ID}, + pull_request: { + head: { + sha: PR_HEAD_SHA, + repo: {id: BASE_REPO_ID, full_name: BASE_QUALIFIED_REPO} + }, + merge_commit_sha: PR_MERGE_SHA + } + } +} + +function forkWorkflowRunPayload(): object { + return { + repository: {id: BASE_REPO_ID}, + workflow_run: { + event: 'pull_request', + head_commit: {id: WORKFLOW_RUN_HEAD_COMMIT_SHA}, + head_repository: {id: FORK_REPO_ID, full_name: FORK_QUALIFIED_REPO} + } + } +} + +describe('unsafe-pr-checkout-helper', () => { + beforeAll(() => { + jest.spyOn(github.context, 'repo', 'get').mockReturnValue({ + owner: 'some-owner', + repo: 'some-repo' + }) + }) + + afterEach(() => { + ;(github.context as {eventName: string}).eventName = originalEventName + ;(github.context as {payload: object}).payload = originalPayload + }) + + afterAll(() => { + ;(github.context as {eventName: string}).eventName = + originalContext.eventName + ;(github.context as {payload: object}).payload = originalContext.payload + jest.restoreAllMocks() + }) + + it('allows pull_request events untouched', () => { + setContext('pull_request', forkPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: 'attacker/fork', + ref: 'refs/pull/1/merge', + commit: '', + allowUnsafePrCheckout: false + }) + ).not.toThrow() + }) + + it('allows pull_request_target default checkout (base branch)', () => { + setContext('pull_request_target', forkPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: BASE_QUALIFIED_REPO, + ref: 'refs/heads/main', + commit: SAFE_BASE_SHA, + allowUnsafePrCheckout: false + }) + ).not.toThrow() + }) + + it('allows same-repo pull_request_target checkout of PR head', () => { + setContext('pull_request_target', sameRepoPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: BASE_QUALIFIED_REPO, + ref: '', + commit: PR_HEAD_SHA, + allowUnsafePrCheckout: false + }) + ).not.toThrow() + }) + + it('refuses pull_request_target fork PR head SHA checkout', () => { + setContext('pull_request_target', forkPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: BASE_QUALIFIED_REPO, + ref: '', + commit: PR_HEAD_SHA, + allowUnsafePrCheckout: false + }) + ).toThrow(/Refusing to check out fork pull request code/) + }) + + it('refuses pull_request_target fork PR merge_commit_sha checkout', () => { + setContext('pull_request_target', forkPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: BASE_QUALIFIED_REPO, + ref: '', + commit: PR_MERGE_SHA, + allowUnsafePrCheckout: false + }) + ).toThrow(/allow-unsafe-pr-checkout/) + }) + + it('refuses pull_request_target fork PR ref pattern (head)', () => { + setContext('pull_request_target', forkPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: BASE_QUALIFIED_REPO, + ref: 'refs/pull/42/head', + commit: '', + allowUnsafePrCheckout: false + }) + ).toThrow() + }) + + it('refuses pull_request_target fork PR ref pattern (merge)', () => { + setContext('pull_request_target', forkPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: BASE_QUALIFIED_REPO, + ref: 'refs/pull/42/merge', + commit: '', + allowUnsafePrCheckout: false + }) + ).toThrow() + }) + + it('refuses pull_request_target when repository points at the fork', () => { + setContext('pull_request_target', forkPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: FORK_QUALIFIED_REPO, + ref: 'refs/heads/main', + commit: '', + allowUnsafePrCheckout: false + }) + ).toThrow() + }) + + it('allows pull_request_target checkout of an unrelated third-party repo', () => { + setContext('pull_request_target', forkPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: 'some-other/unrelated', + ref: 'refs/heads/main', + commit: '', + allowUnsafePrCheckout: false + }) + ).not.toThrow() + }) + + it('refuses pull_request_target ignoring repository case differences', () => { + setContext('pull_request_target', forkPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: FORK_QUALIFIED_REPO.toUpperCase(), + ref: '', + commit: '', + allowUnsafePrCheckout: false + }) + ).toThrow() + }) + + it('refuses pull_request_target ignoring commit SHA case differences', () => { + setContext('pull_request_target', forkPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: BASE_QUALIFIED_REPO, + ref: '', + commit: PR_HEAD_SHA.toUpperCase(), + allowUnsafePrCheckout: false + }) + ).toThrow() + }) + + it('allows pull_request_target fork PR checkout when opted in', () => { + setContext('pull_request_target', forkPullRequestTargetPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: BASE_QUALIFIED_REPO, + ref: 'refs/pull/42/merge', + commit: '', + allowUnsafePrCheckout: true + }) + ).not.toThrow() + }) + + it('refuses workflow_run fork PR head_commit.id checkout', () => { + setContext('workflow_run', forkWorkflowRunPayload()) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: BASE_QUALIFIED_REPO, + ref: '', + commit: WORKFLOW_RUN_HEAD_COMMIT_SHA, + allowUnsafePrCheckout: false + }) + ).toThrow() + }) + + it('refuses workflow_run with pull_request_target underlying event', () => { + const payload = forkWorkflowRunPayload() as { + workflow_run: {event: string} + } + payload.workflow_run.event = 'pull_request_target' + setContext('workflow_run', payload) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: BASE_QUALIFIED_REPO, + ref: '', + commit: WORKFLOW_RUN_HEAD_COMMIT_SHA, + allowUnsafePrCheckout: false + }) + ).toThrow() + }) + + it('allows workflow_run same-repo PR (head_repository.id matches base)', () => { + const payload = forkWorkflowRunPayload() as { + workflow_run: {head_repository: {id: number}} + } + payload.workflow_run.head_repository.id = BASE_REPO_ID + setContext('workflow_run', payload) + expect(() => + assertSafePrCheckout({ + qualifiedRepository: BASE_QUALIFIED_REPO, + ref: '', + commit: WORKFLOW_RUN_HEAD_COMMIT_SHA, + allowUnsafePrCheckout: false + }) + ).not.toThrow() + }) +}) diff --git a/action.yml b/action.yml index 767c416..5b0524f 100644 --- a/action.yml +++ b/action.yml @@ -98,6 +98,15 @@ inputs: github-server-url: description: The base URL for the GitHub instance that you are trying to clone from, will use environment defaults to fetch from the same instance that the workflow is running from unless specified. Example URLs are https://github.com or https://my-ghes-server.example.com required: false + allow-unsafe-pr-checkout: + description: > + Required to check out fork pull request code from a workflow triggered by + `pull_request_target` or `workflow_run`. These workflows run with the + base repository's GITHUB_TOKEN, secrets, default-branch cache scope, and + runner access; fetching and executing a fork's code in that trusted + context commonly leads to "pwn request" vulnerabilities. Set to `true` + only after reviewing the risks at https://gh.io/securely-using-pull_request_target. + default: false outputs: ref: description: 'The branch, tag or SHA that was checked out' diff --git a/dist/index.js b/dist/index.js index 906b59a..5fc2806 100644 --- a/dist/index.js +++ b/dist/index.js @@ -2023,6 +2023,7 @@ const core = __importStar(__nccwpck_require__(2186)); const fsHelper = __importStar(__nccwpck_require__(7219)); const github = __importStar(__nccwpck_require__(5438)); const path = __importStar(__nccwpck_require__(1017)); +const unsafePrCheckoutHelper = __importStar(__nccwpck_require__(843)); const workflowContextHelper = __importStar(__nccwpck_require__(9568)); function getInputs() { return __awaiter(this, void 0, void 0, function* () { @@ -2144,6 +2145,17 @@ function getInputs() { // Determine the GitHub URL that the repository is being hosted from result.githubServerUrl = core.getInput('github-server-url'); core.debug(`GitHub Host URL = ${result.githubServerUrl}`); + // Allow unsafe PR checkout (opt-in for pull_request_target / workflow_run fork PRs) + result.allowUnsafePrCheckout = + (core.getInput('allow-unsafe-pr-checkout') || 'false').toUpperCase() === + 'TRUE'; + core.debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`); + unsafePrCheckoutHelper.assertSafePrCheckout({ + qualifiedRepository, + ref: result.ref, + commit: result.commit, + allowUnsafePrCheckout: result.allowUnsafePrCheckout + }); return result; }); } @@ -2284,6 +2296,7 @@ exports.getRefSpecForAllHistory = getRefSpecForAllHistory; exports.getRefSpec = getRefSpec; exports.testRef = testRef; exports.checkCommitInfo = checkCommitInfo; +exports.fromPayload = fromPayload; const core = __importStar(__nccwpck_require__(2186)); const github = __importStar(__nccwpck_require__(5438)); const url_helper_1 = __nccwpck_require__(9437); @@ -2732,6 +2745,105 @@ if (!exports.IsPost) { } +/***/ }), + +/***/ 843: +/***/ (function(__unused_webpack_module, exports, __nccwpck_require__) { + +"use strict"; + +var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) { + if (k2 === undefined) k2 = k; + var desc = Object.getOwnPropertyDescriptor(m, k); + if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) { + desc = { enumerable: true, get: function() { return m[k]; } }; + } + Object.defineProperty(o, k2, desc); +}) : (function(o, m, k, k2) { + if (k2 === undefined) k2 = k; + o[k2] = m[k]; +})); +var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (function(o, v) { + Object.defineProperty(o, "default", { enumerable: true, value: v }); +}) : function(o, v) { + o["default"] = v; +}); +var __importStar = (this && this.__importStar) || function (mod) { + if (mod && mod.__esModule) return mod; + var result = {}; + if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k); + __setModuleDefault(result, mod); + return result; +}; +Object.defineProperty(exports, "__esModule", ({ value: true })); +exports.assertSafePrCheckout = assertSafePrCheckout; +const github = __importStar(__nccwpck_require__(5438)); +const ref_helper_1 = __nccwpck_require__(8601); +const PR_REF_PATTERN = /^refs\/pull\/[0-9]+\/(?:head|merge)$/; +function assertSafePrCheckout(input) { + if (input.allowUnsafePrCheckout) { + return; + } + const eventName = github.context.eventName; + if (eventName !== 'pull_request_target' && eventName !== 'workflow_run') { + return; + } + const baseRepoId = (0, ref_helper_1.fromPayload)('repository.id'); + if (typeof baseRepoId !== 'number') { + return; + } + let prHeadRepoId; + let prHeadRepoFullName; + const prShas = []; + if (eventName === 'pull_request_target') { + prHeadRepoId = (0, ref_helper_1.fromPayload)('pull_request.head.repo.id'); + prHeadRepoFullName = (0, ref_helper_1.fromPayload)('pull_request.head.repo.full_name'); + pushIfSha(prShas, (0, ref_helper_1.fromPayload)('pull_request.head.sha')); + pushIfSha(prShas, (0, ref_helper_1.fromPayload)('pull_request.merge_commit_sha')); + } + else { + const wrEvent = (0, ref_helper_1.fromPayload)('workflow_run.event'); + if (typeof wrEvent !== 'string' || !wrEvent.startsWith('pull_request')) { + return; + } + prHeadRepoId = (0, ref_helper_1.fromPayload)('workflow_run.head_repository.id'); + prHeadRepoFullName = (0, ref_helper_1.fromPayload)('workflow_run.head_repository.full_name'); + pushIfSha(prShas, (0, ref_helper_1.fromPayload)('workflow_run.head_commit.id')); + // For `pull_request_target`-triggered workflow_run, `head_sha` is the base + // default branch SHA (not the PR head) + if (wrEvent !== 'pull_request_target') { + pushIfSha(prShas, (0, ref_helper_1.fromPayload)('workflow_run.head_sha')); + } + } + // (A) Fork PR? + if (typeof prHeadRepoId !== 'number' || prHeadRepoId === baseRepoId) { + return; + } + // (B) We cannot check for all fork PR refs so check to see + // if the resolved input points to the fork PR sha we have in the payload + const repositoryMatchesPrHead = typeof prHeadRepoFullName === 'string' && + input.qualifiedRepository.toLowerCase() === prHeadRepoFullName.toLowerCase(); + const refMatchesPullPattern = PR_REF_PATTERN.test(input.ref); + const commitMatchesPrHeadSha = !!input.commit && prShas.includes(input.commit.toLowerCase()); + if (!repositoryMatchesPrHead && + !refMatchesPullPattern && + !commitMatchesPrHeadSha) { + return; + } + throw new Error(`Refusing to check out fork pull request code from a '${eventName}' workflow. ` + + `This workflow runs with the base repository's GITHUB_TOKEN, secrets, default-branch ` + + `cache scope, and runner access. Fetching and executing a fork's code in that trusted ` + + `context commonly leads to "pwn request" vulnerabilities. To opt in, review the risks ` + + `at https://gh.io/securely-using-pull_request_target and set 'allow-unsafe-pr-checkout: true' ` + + `on the actions/checkout step.`); +} +function pushIfSha(target, value) { + if (typeof value === 'string' && value.length > 0) { + target.push(value.toLowerCase()); + } +} + + /***/ }), /***/ 9437: diff --git a/src/git-source-settings.ts b/src/git-source-settings.ts index 4e41ac3..79041c4 100644 --- a/src/git-source-settings.ts +++ b/src/git-source-settings.ts @@ -118,4 +118,10 @@ export interface IGitSourceSettings { * User override on the GitHub Server/Host URL that hosts the repository to be cloned */ githubServerUrl: string | undefined + + /** + * Opt-in to allow checking out fork pull request code from a workflow + * triggered by pull_request_target or workflow_run. + */ + allowUnsafePrCheckout: boolean } diff --git a/src/input-helper.ts b/src/input-helper.ts index e0c61e2..2d20930 100644 --- a/src/input-helper.ts +++ b/src/input-helper.ts @@ -2,6 +2,7 @@ import * as core from '@actions/core' import * as fsHelper from './fs-helper' import * as github from '@actions/github' import * as path from 'path' +import * as unsafePrCheckoutHelper from './unsafe-pr-checkout-helper' import * as workflowContextHelper from './workflow-context-helper' import {IGitSourceSettings} from './git-source-settings' @@ -161,5 +162,18 @@ export async function getInputs(): Promise { result.githubServerUrl = core.getInput('github-server-url') core.debug(`GitHub Host URL = ${result.githubServerUrl}`) + // Allow unsafe PR checkout (opt-in for pull_request_target / workflow_run fork PRs) + result.allowUnsafePrCheckout = + (core.getInput('allow-unsafe-pr-checkout') || 'false').toUpperCase() === + 'TRUE' + core.debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`) + + unsafePrCheckoutHelper.assertSafePrCheckout({ + qualifiedRepository, + ref: result.ref, + commit: result.commit, + allowUnsafePrCheckout: result.allowUnsafePrCheckout + }) + return result } diff --git a/src/ref-helper.ts b/src/ref-helper.ts index 71e8b22..8751712 100644 --- a/src/ref-helper.ts +++ b/src/ref-helper.ts @@ -292,7 +292,7 @@ export async function checkCommitInfo( } } -function fromPayload(path: string): any { +export function fromPayload(path: string): any { return select(github.context.payload, path) } diff --git a/src/unsafe-pr-checkout-helper.ts b/src/unsafe-pr-checkout-helper.ts new file mode 100644 index 0000000..f6bf389 --- /dev/null +++ b/src/unsafe-pr-checkout-helper.ts @@ -0,0 +1,88 @@ +import * as github from '@actions/github' +import {fromPayload} from './ref-helper' + +const PR_REF_PATTERN = /^refs\/pull\/[0-9]+\/(?:head|merge)$/ + +export interface IUnsafePrCheckoutInput { + qualifiedRepository: string + ref: string + commit: string | undefined + allowUnsafePrCheckout: boolean +} + +export function assertSafePrCheckout(input: IUnsafePrCheckoutInput): void { + if (input.allowUnsafePrCheckout) { + return + } + + const eventName = github.context.eventName + if (eventName !== 'pull_request_target' && eventName !== 'workflow_run') { + return + } + + const baseRepoId = fromPayload('repository.id') + if (typeof baseRepoId !== 'number') { + return + } + + let prHeadRepoId: unknown + let prHeadRepoFullName: unknown + const prShas: string[] = [] + + if (eventName === 'pull_request_target') { + prHeadRepoId = fromPayload('pull_request.head.repo.id') + prHeadRepoFullName = fromPayload('pull_request.head.repo.full_name') + pushIfSha(prShas, fromPayload('pull_request.head.sha')) + pushIfSha(prShas, fromPayload('pull_request.merge_commit_sha')) + } else { + const wrEvent = fromPayload('workflow_run.event') + if (typeof wrEvent !== 'string' || !wrEvent.startsWith('pull_request')) { + return + } + prHeadRepoId = fromPayload('workflow_run.head_repository.id') + prHeadRepoFullName = fromPayload('workflow_run.head_repository.full_name') + pushIfSha(prShas, fromPayload('workflow_run.head_commit.id')) + // For `pull_request_target`-triggered workflow_run, `head_sha` is the base + // default branch SHA (not the PR head) + if (wrEvent !== 'pull_request_target') { + pushIfSha(prShas, fromPayload('workflow_run.head_sha')) + } + } + + // (A) Fork PR? + if (typeof prHeadRepoId !== 'number' || prHeadRepoId === baseRepoId) { + return + } + + // (B) We cannot check for all fork PR refs so check to see + // if the resolved input points to the fork PR sha we have in the payload + const repositoryMatchesPrHead = + typeof prHeadRepoFullName === 'string' && + input.qualifiedRepository.toLowerCase() === prHeadRepoFullName.toLowerCase() + const refMatchesPullPattern = PR_REF_PATTERN.test(input.ref) + const commitMatchesPrHeadSha = + !!input.commit && prShas.includes(input.commit.toLowerCase()) + + if ( + !repositoryMatchesPrHead && + !refMatchesPullPattern && + !commitMatchesPrHeadSha + ) { + return + } + + throw new Error( + `Refusing to check out fork pull request code from a '${eventName}' workflow. ` + + `This workflow runs with the base repository's GITHUB_TOKEN, secrets, default-branch ` + + `cache scope, and runner access. Fetching and executing a fork's code in that trusted ` + + `context commonly leads to "pwn request" vulnerabilities. To opt in, review the risks ` + + `at https://gh.io/securely-using-pull_request_target and set 'allow-unsafe-pr-checkout: true' ` + + `on the actions/checkout step.` + ) +} + +function pushIfSha(target: string[], value: unknown): void { + if (typeof value === 'string' && value.length > 0) { + target.push(value.toLowerCase()) + } +} From d23441a48e516b6c34aea4fa41551a30e30af803 Mon Sep 17 00:00:00 2001 From: Aiqiao Yan <55104035+aiqiaoy@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:43:33 -0400 Subject: [PATCH 2/2] backport fixes to releases-v6 (#2527) --- __test__/input-helper.test.ts | 93 +++++++++++++++++++++++++++++++++-- dist/index.js | 40 ++++++++++++--- src/input-helper.ts | 40 ++++++++++++--- 3 files changed, 154 insertions(+), 19 deletions(-) diff --git a/__test__/input-helper.test.ts b/__test__/input-helper.test.ts index 25b6d18..c35a735 100644 --- a/__test__/input-helper.test.ts +++ b/__test__/input-helper.test.ts @@ -12,15 +12,24 @@ const gitHubWorkspace = path.resolve('/checkout-tests/workspace') // Inputs for mock @actions/core let inputs = {} as any +// Replicate @actions/core getInput behavior: it trims whitespace by default +// (String.prototype.trim(), which strips characters such as a leading U+FEFF BOM) +// unless trimWhitespace is explicitly set to false. +const getInputImpl = (name: string, options?: {trimWhitespace?: boolean}) => { + const val = inputs[name] ?? '' + if (options && options.trimWhitespace === false) { + return val + } + return typeof val === 'string' ? val.trim() : val +} + // Shallow clone original @actions/github context let originalContext = {...github.context} describe('input-helper tests', () => { beforeAll(() => { // Mock getInput - jest.spyOn(core, 'getInput').mockImplementation((name: string) => { - return inputs[name] - }) + jest.spyOn(core, 'getInput').mockImplementation(getInputImpl as any) // Mock error/warning/info/debug jest.spyOn(core, 'error').mockImplementation(jest.fn()) @@ -151,8 +160,86 @@ describe('input-helper tests', () => { expect(settings.commit).toBeFalsy() }) + it('does not reclassify a ref as sha when a BOM is prefixed', async () => { + // A fork branch named "" + 40 hex chars. core.getInput trims the + // BOM by default, which previously collapsed this into a bare SHA and + // bypassed the unsafe fork PR checkout guard. + inputs.ref = '\uFEFF522d932fae5296da51fdf431934425ecf891c6a2' + const settings: IGitSourceSettings = await inputHelper.getInputs() + expect(settings.commit).toBeFalsy() + expect(settings.ref).toBe('522d932fae5296da51fdf431934425ecf891c6a2') + }) + + it('does not reclassify a sha-256 ref as sha when a BOM is prefixed', async () => { + inputs.ref = + '\uFEFF1111111111222222222233333333334444444444555555555566666666667777' + const settings: IGitSourceSettings = await inputHelper.getInputs() + expect(settings.commit).toBeFalsy() + expect(settings.ref).toBe( + '1111111111222222222233333333334444444444555555555566666666667777' + ) + }) + + it('treats a sha surrounded by ascii whitespace as a commit', async () => { + // ASCII whitespace can only come from the workflow author's YAML (git ref + // names cannot contain it), so trimming it and treating the value as a + // commit is safe. + inputs.ref = ' 1111111111222222222233333333334444444444 ' + const settings: IGitSourceSettings = await inputHelper.getInputs() + expect(settings.ref).toBeFalsy() + expect(settings.commit).toBe('1111111111222222222233333333334444444444') + }) + it('sets workflow organization ID', async () => { const settings: IGitSourceSettings = await inputHelper.getInputs() expect(settings.workflowOrganizationId).toBe(123456) }) + + describe('unsafe PR checkout guard', () => { + const forkPayload = { + repository: {id: 100}, + pull_request: { + head: { + sha: '1234567890123456789012345678901234567890', + repo: {id: 200, full_name: 'attacker/fork'} + }, + merge_commit_sha: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' + } + } + + it('allows the default self-checkout on a fork pull_request_target', async () => { + const originalEvent = github.context.eventName + const originalPayload = github.context.payload + const originalSha = github.context.sha + try { + github.context.eventName = 'pull_request_target' + github.context.payload = forkPayload as any + // Simulate a rebase/fast-forward merge where the base tip (event SHA) + // equals the PR head SHA. The default self-checkout must still succeed. + github.context.sha = '1234567890123456789012345678901234567890' + const settings: IGitSourceSettings = await inputHelper.getInputs() + expect(settings.commit).toBe('1234567890123456789012345678901234567890') + } finally { + github.context.eventName = originalEvent + github.context.payload = originalPayload + github.context.sha = originalSha + } + }) + + it('refuses an explicit fork repository on pull_request_target', async () => { + const originalEvent = github.context.eventName + const originalPayload = github.context.payload + try { + github.context.eventName = 'pull_request_target' + github.context.payload = forkPayload as any + inputs.repository = 'attacker/fork' + await expect(inputHelper.getInputs()).rejects.toThrow( + /Refusing to check out fork pull request code/ + ) + } finally { + github.context.eventName = originalEvent + github.context.payload = originalPayload + } + }) + }) }) diff --git a/dist/index.js b/dist/index.js index 5fc2806..bbd0bfa 100644 --- a/dist/index.js +++ b/dist/index.js @@ -2059,6 +2059,23 @@ function getInputs() { `${github.context.repo.owner}/${github.context.repo.repo}`.toUpperCase(); // Source branch, source version result.ref = core.getInput('ref'); + // core.getInput()'s default trim strips a range of Unicode characters such as a + // leading BOM (U+FEFF) or NBSP (U+00A0). Those are valid in a git ref name, so + // a fork branch named "" + 40 hex chars would trim down to a bare SHA and + // be silently reclassified as a commit, bypassing the unsafe fork PR checkout + // guard. + // + // The trim below strips only the ASCII whitespace characters which are all forbidden + // in a git branch name. + // \t U+0009 horizontal tab - ASCII control, forbidden in ref names + // \n U+000A line feed - ASCII control, forbidden in ref names + // \v U+000B vertical tab - ASCII control, forbidden in ref names + // \f U+000C form feed - ASCII control, forbidden in ref names + // \r U+000D carriage return - ASCII control, forbidden in ref names + // ' ' U+0020 space - forbidden in ref names + const asciiTrimmedRef = core + .getInput('ref', { trimWhitespace: false }) + .replace(/^[\t\n\v\f\r ]+|[\t\n\v\f\r ]+$/g, ''); if (!result.ref) { if (isWorkflowRepository) { result.ref = github.context.ref; @@ -2071,8 +2088,8 @@ function getInputs() { } } // SHA? - else if (result.ref.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) { - result.commit = result.ref; + else if (asciiTrimmedRef.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) { + result.commit = asciiTrimmedRef; result.ref = ''; } core.debug(`ref = '${result.ref}'`); @@ -2150,12 +2167,19 @@ function getInputs() { (core.getInput('allow-unsafe-pr-checkout') || 'false').toUpperCase() === 'TRUE'; core.debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`); - unsafePrCheckoutHelper.assertSafePrCheckout({ - qualifiedRepository, - ref: result.ref, - commit: result.commit, - allowUnsafePrCheckout: result.allowUnsafePrCheckout - }); + // The default self-checkout (this repository with no explicit ref) always + // resolves to the trusted ref/commit GitHub set for the triggering event, so + // the fork-checkout guard only needs to run when the caller customized the + // repository or ref. + const isDefaultCheckout = isWorkflowRepository && !core.getInput('ref'); + if (!isDefaultCheckout) { + unsafePrCheckoutHelper.assertSafePrCheckout({ + qualifiedRepository, + ref: result.ref, + commit: result.commit, + allowUnsafePrCheckout: result.allowUnsafePrCheckout + }); + } return result; }); } diff --git a/src/input-helper.ts b/src/input-helper.ts index 2d20930..f62ddc6 100644 --- a/src/input-helper.ts +++ b/src/input-helper.ts @@ -59,6 +59,23 @@ export async function getInputs(): Promise { // Source branch, source version result.ref = core.getInput('ref') + // core.getInput()'s default trim strips a range of Unicode characters such as a + // leading BOM (U+FEFF) or NBSP (U+00A0). Those are valid in a git ref name, so + // a fork branch named "" + 40 hex chars would trim down to a bare SHA and + // be silently reclassified as a commit, bypassing the unsafe fork PR checkout + // guard. + // + // The trim below strips only the ASCII whitespace characters which are all forbidden + // in a git branch name. + // \t U+0009 horizontal tab - ASCII control, forbidden in ref names + // \n U+000A line feed - ASCII control, forbidden in ref names + // \v U+000B vertical tab - ASCII control, forbidden in ref names + // \f U+000C form feed - ASCII control, forbidden in ref names + // \r U+000D carriage return - ASCII control, forbidden in ref names + // ' ' U+0020 space - forbidden in ref names + const asciiTrimmedRef = core + .getInput('ref', {trimWhitespace: false}) + .replace(/^[\t\n\v\f\r ]+|[\t\n\v\f\r ]+$/g, '') if (!result.ref) { if (isWorkflowRepository) { result.ref = github.context.ref @@ -72,8 +89,8 @@ export async function getInputs(): Promise { } } // SHA? - else if (result.ref.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) { - result.commit = result.ref + else if (asciiTrimmedRef.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) { + result.commit = asciiTrimmedRef result.ref = '' } core.debug(`ref = '${result.ref}'`) @@ -168,12 +185,19 @@ export async function getInputs(): Promise { 'TRUE' core.debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`) - unsafePrCheckoutHelper.assertSafePrCheckout({ - qualifiedRepository, - ref: result.ref, - commit: result.commit, - allowUnsafePrCheckout: result.allowUnsafePrCheckout - }) + // The default self-checkout (this repository with no explicit ref) always + // resolves to the trusted ref/commit GitHub set for the triggering event, so + // the fork-checkout guard only needs to run when the caller customized the + // repository or ref. + const isDefaultCheckout = isWorkflowRepository && !core.getInput('ref') + if (!isDefaultCheckout) { + unsafePrCheckoutHelper.assertSafePrCheckout({ + qualifiedRepository, + ref: result.ref, + commit: result.commit, + allowUnsafePrCheckout: result.allowUnsafePrCheckout + }) + } return result }