diff --git a/.github/workflows/check-dist.yml b/.github/workflows/check-dist.yml index d5d68d672..43de31148 100644 --- a/.github/workflows/check-dist.yml +++ b/.github/workflows/check-dist.yml @@ -44,7 +44,7 @@ jobs: fi # If dist/ was different than expected, upload the expected version as an artifact - - uses: actions/upload-artifact@v2 + - uses: actions/upload-artifact@v4 if: ${{ failure() && steps.diff.conclusion == 'failure' }} with: name: dist diff --git a/README.md b/README.md index dd09e13f6..ff4849e5b 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,15 @@ Refer [here](https://github.com/actions/checkout/blob/v1/README.md) for previous # config --global --add safe.directory ` # Default: true set-safe-directory: '' + + # 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 a6731c2b4..4e7e9bf23 100644 --- a/__test__/git-auth-helper.test.ts +++ b/__test__/git-auth-helper.test.ts @@ -778,7 +778,8 @@ async function setup(testName: string): Promise { sshKnownHosts: '', sshStrict: true, workflowOrganizationId: 123456, - setSafeDirectory: true + setSafeDirectory: true, + allowUnsafePrCheckout: false } } diff --git a/__test__/input-helper.test.ts b/__test__/input-helper.test.ts index 1a8e5c96d..4e32e994a 100644 --- a/__test__/input-helper.test.ts +++ b/__test__/input-helper.test.ts @@ -86,6 +86,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 000000000..9efa246e8 --- /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 b78626366..9f346aa77 100644 --- a/action.yml +++ b/action.yml @@ -71,6 +71,15 @@ inputs: set-safe-directory: description: Add repository path as safe.directory for Git global config by running `git config --global --add safe.directory ` default: true + 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 runs: using: node12 main: dist/index.js diff --git a/dist/index.js b/dist/index.js index cfa77288a..dbd2dbbba 100644 --- a/dist/index.js +++ b/dist/index.js @@ -4740,7 +4740,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge }); }; Object.defineProperty(exports, "__esModule", { value: true }); -exports.checkCommitInfo = exports.testRef = exports.getRefSpec = exports.getRefSpecForAllHistory = exports.getCheckoutInfo = exports.tagsRefSpec = void 0; +exports.fromPayload = exports.checkCommitInfo = exports.testRef = exports.getRefSpec = exports.getRefSpecForAllHistory = exports.getCheckoutInfo = exports.tagsRefSpec = void 0; const url_1 = __webpack_require__(835); const core = __importStar(__webpack_require__(470)); const github = __importStar(__webpack_require__(469)); @@ -4958,6 +4958,7 @@ exports.checkCommitInfo = checkCommitInfo; function fromPayload(path) { return select(github.context.payload, path); } +exports.fromPayload = fromPayload; function select(obj, path) { if (!obj) { return undefined; @@ -13322,6 +13323,102 @@ function getNextPage (octokit, link, headers) { } +/***/ }), + +/***/ 554: +/***/ (function(__unusedmodule, exports, __webpack_require__) { + +"use strict"; + +var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) { + if (k2 === undefined) k2 = k; + Object.defineProperty(o, k2, { enumerable: true, get: function() { return m[k]; } }); +}) : (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 = void 0; +const github = __importStar(__webpack_require__(469)); +const ref_helper_1 = __webpack_require__(227); +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.`); +} +exports.assertSafePrCheckout = assertSafePrCheckout; +function pushIfSha(target, value) { + if (typeof value === 'string' && value.length > 0) { + target.push(value.toLowerCase()); + } +} + + /***/ }), /***/ 558: @@ -18372,6 +18469,7 @@ const core = __importStar(__webpack_require__(470)); const fsHelper = __importStar(__webpack_require__(618)); const github = __importStar(__webpack_require__(469)); const path = __importStar(__webpack_require__(622)); +const unsafePrCheckoutHelper = __importStar(__webpack_require__(554)); const workflowContextHelper = __importStar(__webpack_require__(642)); function getInputs() { return __awaiter(this, void 0, void 0, function* () { @@ -18465,6 +18563,17 @@ function getInputs() { // Set safe.directory in git global config. result.setSafeDirectory = (core.getInput('set-safe-directory') || 'true').toUpperCase() === 'TRUE'; + // 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/git-source-settings.ts b/src/git-source-settings.ts index 6fa3960fd..82eb983f4 100644 --- a/src/git-source-settings.ts +++ b/src/git-source-settings.ts @@ -83,4 +83,10 @@ export interface IGitSourceSettings { * Indicates whether to add repositoryPath as safe.directory in git global config */ setSafeDirectory: boolean + + /** + * 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 8c2f90157..1a090c798 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' @@ -125,5 +126,19 @@ export async function getInputs(): Promise { // Set safe.directory in git global config. result.setSafeDirectory = (core.getInput('set-safe-directory') || 'true').toUpperCase() === 'TRUE' + + // 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 209f49d70..c9492f523 100644 --- a/src/ref-helper.ts +++ b/src/ref-helper.ts @@ -259,7 +259,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 000000000..f6bf389cf --- /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()) + } +}