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
46 changes: 46 additions & 0 deletions __test__/input-helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,4 +180,50 @@ describe('input-helper tests', () => {
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 = mockGithubContext.eventName
const originalPayload = mockGithubContext.payload
try {
mockGithubContext.eventName = 'pull_request_target'
mockGithubContext.payload = forkPayload
// Simulate a rebase/fast-forward merge where the base tip (event SHA)
// equals the PR head SHA. The default self-checkout must still succeed.
mockGithubContext.sha = '1234567890123456789012345678901234567890'
const settings: IGitSourceSettings = await inputHelper.getInputs()
expect(settings.commit).toBe('1234567890123456789012345678901234567890')
} finally {
mockGithubContext.eventName = originalEvent
mockGithubContext.payload = originalPayload
}
Comment thread
aiqiaoy marked this conversation as resolved.
})

it('refuses an explicit fork repository on pull_request_target', async () => {
const originalEvent = mockGithubContext.eventName
const originalPayload = mockGithubContext.payload
try {
mockGithubContext.eventName = 'pull_request_target'
mockGithubContext.payload = forkPayload
inputs.repository = 'attacker/fork'
await expect(inputHelper.getInputs()).rejects.toThrow(
/Refusing to check out fork pull request code/
)
} finally {
mockGithubContext.eventName = originalEvent
mockGithubContext.payload = originalPayload
}
})
})
})
19 changes: 13 additions & 6 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42191,12 +42191,19 @@ async function getInputs() {
(getInput('allow-unsafe-pr-checkout') || 'false').toUpperCase() ===
'TRUE';
core_debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`);
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 && !getInput('ref');
if (!isDefaultCheckout) {
assertSafePrCheckout({
qualifiedRepository,
ref: result.ref,
commit: result.commit,
allowUnsafePrCheckout: result.allowUnsafePrCheckout
});
}
return result;
}

Expand Down
19 changes: 13 additions & 6 deletions src/input-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,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