Compare commits

..
3 Commits
Author SHA1 Message Date
Aiqiao Yan c12eb249cb run prettier formatting 2026-06-15 16:18:29 +00:00
Aiqiao Yan 12a489776f address copilot and reviewer feedback 2026-06-15 16:12:03 +00:00
Aiqiao Yan b8447332b0 block checking out fork pr for some events 2026-06-12 19:12:01 +00:00
6 changed files with 31 additions and 173 deletions
+2 -5
View File
@@ -162,11 +162,8 @@ Please refer to the [release page](https://github.com/actions/checkout/releases/
github-server-url: '' github-server-url: ''
# Required to check out fork pull request code from a workflow triggered by # 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 # `pull_request_target` or `workflow_run`. See [Pwn Requests](todo:need-link) for
# repository's GITHUB_TOKEN, secrets, default-branch cache scope, and runner # the risks. Set to `true` only after reviewing the risks.
# 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 # Default: false
allow-unsafe-pr-checkout: '' allow-unsafe-pr-checkout: ''
``` ```
+3 -90
View File
@@ -12,24 +12,15 @@ const gitHubWorkspace = path.resolve('/checkout-tests/workspace')
// Inputs for mock @actions/core // Inputs for mock @actions/core
let inputs = {} as any 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 // Shallow clone original @actions/github context
let originalContext = {...github.context} let originalContext = {...github.context}
describe('input-helper tests', () => { describe('input-helper tests', () => {
beforeAll(() => { beforeAll(() => {
// Mock getInput // Mock getInput
jest.spyOn(core, 'getInput').mockImplementation(getInputImpl as any) jest.spyOn(core, 'getInput').mockImplementation((name: string) => {
return inputs[name]
})
// Mock error/warning/info/debug // Mock error/warning/info/debug
jest.spyOn(core, 'error').mockImplementation(jest.fn()) jest.spyOn(core, 'error').mockImplementation(jest.fn())
@@ -160,86 +151,8 @@ describe('input-helper tests', () => {
expect(settings.commit).toBeFalsy() expect(settings.commit).toBeFalsy()
}) })
it('does not reclassify a ref as sha when a BOM is prefixed', async () => {
// A fork branch named "<U+FEFF>" + 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 () => { it('sets workflow organization ID', async () => {
const settings: IGitSourceSettings = await inputHelper.getInputs() const settings: IGitSourceSettings = await inputHelper.getInputs()
expect(settings.workflowOrganizationId).toBe(123456) 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
}
})
})
}) })
+2 -5
View File
@@ -101,11 +101,8 @@ inputs:
allow-unsafe-pr-checkout: allow-unsafe-pr-checkout:
description: > description: >
Required to check out fork pull request code from a workflow triggered by Required to check out fork pull request code from a workflow triggered by
`pull_request_target` or `workflow_run`. These workflows run with the `pull_request_target` or `workflow_run`. See [Pwn Requests](todo:need-link)
base repository's GITHUB_TOKEN, secrets, default-branch cache scope, and for the risks. Set to `true` only after reviewing the risks.
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 default: false
outputs: outputs:
ref: ref:
+7 -31
View File
@@ -2059,23 +2059,6 @@ function getInputs() {
`${github.context.repo.owner}/${github.context.repo.repo}`.toUpperCase(); `${github.context.repo.owner}/${github.context.repo.repo}`.toUpperCase();
// Source branch, source version // Source branch, source version
result.ref = core.getInput('ref'); 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 "<BOM>" + 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 (!result.ref) {
if (isWorkflowRepository) { if (isWorkflowRepository) {
result.ref = github.context.ref; result.ref = github.context.ref;
@@ -2088,8 +2071,8 @@ function getInputs() {
} }
} }
// SHA? // SHA?
else if (asciiTrimmedRef.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) { else if (result.ref.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) {
result.commit = asciiTrimmedRef; result.commit = result.ref;
result.ref = ''; result.ref = '';
} }
core.debug(`ref = '${result.ref}'`); core.debug(`ref = '${result.ref}'`);
@@ -2167,19 +2150,12 @@ function getInputs() {
(core.getInput('allow-unsafe-pr-checkout') || 'false').toUpperCase() === (core.getInput('allow-unsafe-pr-checkout') || 'false').toUpperCase() ===
'TRUE'; 'TRUE';
core.debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`); core.debug(`allow unsafe PR checkout = ${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({ unsafePrCheckoutHelper.assertSafePrCheckout({
qualifiedRepository, qualifiedRepository,
ref: result.ref, ref: result.ref,
commit: result.commit, commit: result.commit,
allowUnsafePrCheckout: result.allowUnsafePrCheckout allowUnsafePrCheckout: result.allowUnsafePrCheckout
}); });
}
return result; return result;
}); });
} }
@@ -2846,7 +2822,8 @@ function assertSafePrCheckout(input) {
// (B) We cannot check for all fork PR refs so check to see // (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 // if the resolved input points to the fork PR sha we have in the payload
const repositoryMatchesPrHead = typeof prHeadRepoFullName === 'string' && const repositoryMatchesPrHead = typeof prHeadRepoFullName === 'string' &&
input.qualifiedRepository.toLowerCase() === prHeadRepoFullName.toLowerCase(); input.qualifiedRepository.toLowerCase() ===
prHeadRepoFullName.toLowerCase();
const refMatchesPullPattern = PR_REF_PATTERN.test(input.ref); const refMatchesPullPattern = PR_REF_PATTERN.test(input.ref);
const commitMatchesPrHeadSha = !!input.commit && prShas.includes(input.commit.toLowerCase()); const commitMatchesPrHeadSha = !!input.commit && prShas.includes(input.commit.toLowerCase());
if (!repositoryMatchesPrHead && if (!repositoryMatchesPrHead &&
@@ -2856,10 +2833,9 @@ function assertSafePrCheckout(input) {
} }
throw new Error(`Refusing to check out fork pull request code from a '${eventName}' workflow. ` + 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 ` + `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 ` + `cache scope, and runner access. Fetching fork's code in that trusted context is a ` +
`context commonly leads to "pwn request" vulnerabilities. To opt in, review the risks ` + `"pwn request" supply-chain attack pattern. To opt in after reviewing the risk, set ` +
`at https://gh.io/securely-using-pull_request_target and set 'allow-unsafe-pr-checkout: true' ` + `'allow-unsafe-pr-checkout: true' on the actions/checkout step.`);
`on the actions/checkout step.`);
} }
function pushIfSha(target, value) { function pushIfSha(target, value) {
if (typeof value === 'string' && value.length > 0) { if (typeof value === 'string' && value.length > 0) {
+2 -26
View File
@@ -59,23 +59,6 @@ export async function getInputs(): Promise<IGitSourceSettings> {
// Source branch, source version // Source branch, source version
result.ref = core.getInput('ref') 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 "<BOM>" + 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 (!result.ref) {
if (isWorkflowRepository) { if (isWorkflowRepository) {
result.ref = github.context.ref result.ref = github.context.ref
@@ -89,8 +72,8 @@ export async function getInputs(): Promise<IGitSourceSettings> {
} }
} }
// SHA? // SHA?
else if (asciiTrimmedRef.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) { else if (result.ref.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) {
result.commit = asciiTrimmedRef result.commit = result.ref
result.ref = '' result.ref = ''
} }
core.debug(`ref = '${result.ref}'`) core.debug(`ref = '${result.ref}'`)
@@ -185,19 +168,12 @@ export async function getInputs(): Promise<IGitSourceSettings> {
'TRUE' 'TRUE'
core.debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`) core.debug(`allow unsafe PR checkout = ${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({ unsafePrCheckoutHelper.assertSafePrCheckout({
qualifiedRepository, qualifiedRepository,
ref: result.ref, ref: result.ref,
commit: result.commit, commit: result.commit,
allowUnsafePrCheckout: result.allowUnsafePrCheckout allowUnsafePrCheckout: result.allowUnsafePrCheckout
}) })
}
return result return result
} }
+3 -4
View File
@@ -74,10 +74,9 @@ export function assertSafePrCheckout(input: IUnsafePrCheckoutInput): void {
throw new Error( throw new Error(
`Refusing to check out fork pull request code from a '${eventName}' workflow. ` + `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 ` + `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 ` + `cache scope, and runner access. Fetching fork's code in that trusted context is a ` +
`context commonly leads to "pwn request" vulnerabilities. To opt in, review the risks ` + `"pwn request" supply-chain attack pattern. To opt in after reviewing the risk, set ` +
`at https://gh.io/securely-using-pull_request_target and set 'allow-unsafe-pr-checkout: true' ` + `'allow-unsafe-pr-checkout: true' on the actions/checkout step.`
`on the actions/checkout step.`
) )
} }