Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
83 changes: 80 additions & 3 deletions __test__/input-helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -139,8 +148,76 @@ 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 "<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('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
}
})
})
})
40 changes: 32 additions & 8 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,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 "<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 (isWorkflowRepository) {
result.ref = github.context.ref;
Expand All @@ -1712,8 +1729,8 @@ function getInputs() {
}
}
// SHA?
else if (result.ref.match(/^[0-9a-fA-F]{40}$/)) {
result.commit = result.ref;
else if (asciiTrimmedRef.match(/^[0-9a-fA-F]{40}$/)) {
result.commit = asciiTrimmedRef;
result.ref = '';
}
core.debug(`ref = '${result.ref}'`);
Expand Down Expand Up @@ -1779,12 +1796,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;
});
}
Expand Down
40 changes: 32 additions & 8 deletions src/input-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,23 @@ export async function getInputs(): Promise<IGitSourceSettings> {

// 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 "<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 (isWorkflowRepository) {
result.ref = github.context.ref
Expand All @@ -72,8 +89,8 @@ export async function getInputs(): Promise<IGitSourceSettings> {
}
}
// SHA?
else if (result.ref.match(/^[0-9a-fA-F]{40}$/)) {
result.commit = result.ref
else if (asciiTrimmedRef.match(/^[0-9a-fA-F]{40}$/)) {
result.commit = asciiTrimmedRef
result.ref = ''
}
core.debug(`ref = '${result.ref}'`)
Expand Down Expand Up @@ -153,12 +170,19 @@ export async function getInputs(): Promise<IGitSourceSettings> {
'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
}
Loading