Compare commits

..

No commits in common. "c2edb9a7407067e94b7af06b290da20c8e50a2fd" and "b8447332b0339613f1faeb543152b81046bc41f4" have entirely different histories.

3 changed files with 16 additions and 43 deletions

View File

@ -13,7 +13,6 @@ const PR_MERGE_SHA = '2222222222222222222222222222222222222222'
const SAFE_BASE_SHA = '3333333333333333333333333333333333333333' const SAFE_BASE_SHA = '3333333333333333333333333333333333333333'
const WORKFLOW_RUN_HEAD_COMMIT_SHA = '4444444444444444444444444444444444444444' const WORKFLOW_RUN_HEAD_COMMIT_SHA = '4444444444444444444444444444444444444444'
const BASE_QUALIFIED_REPO = 'some-owner/some-repo' const BASE_QUALIFIED_REPO = 'some-owner/some-repo'
const FORK_QUALIFIED_REPO = 'another-repo/fork'
function setContext(eventName: string, payload: object): void { function setContext(eventName: string, payload: object): void {
;(github.context as {eventName: string}).eventName = eventName ;(github.context as {eventName: string}).eventName = eventName
@ -26,7 +25,7 @@ function forkPullRequestTargetPayload(): object {
pull_request: { pull_request: {
head: { head: {
sha: PR_HEAD_SHA, sha: PR_HEAD_SHA,
repo: {id: FORK_REPO_ID, full_name: FORK_QUALIFIED_REPO} repo: {id: FORK_REPO_ID}
}, },
merge_commit_sha: PR_MERGE_SHA merge_commit_sha: PR_MERGE_SHA
} }
@ -39,7 +38,7 @@ function sameRepoPullRequestTargetPayload(): object {
pull_request: { pull_request: {
head: { head: {
sha: PR_HEAD_SHA, sha: PR_HEAD_SHA,
repo: {id: BASE_REPO_ID, full_name: BASE_QUALIFIED_REPO} repo: {id: BASE_REPO_ID}
}, },
merge_commit_sha: PR_MERGE_SHA merge_commit_sha: PR_MERGE_SHA
} }
@ -52,7 +51,7 @@ function forkWorkflowRunPayload(): object {
workflow_run: { workflow_run: {
event: 'pull_request', event: 'pull_request',
head_commit: {id: WORKFLOW_RUN_HEAD_COMMIT_SHA}, head_commit: {id: WORKFLOW_RUN_HEAD_COMMIT_SHA},
head_repository: {id: FORK_REPO_ID, full_name: FORK_QUALIFIED_REPO} head_repository: {id: FORK_REPO_ID}
} }
} }
} }
@ -165,7 +164,7 @@ describe('unsafe-pr-checkout-helper', () => {
setContext('pull_request_target', forkPullRequestTargetPayload()) setContext('pull_request_target', forkPullRequestTargetPayload())
expect(() => expect(() =>
assertSafePrCheckout({ assertSafePrCheckout({
qualifiedRepository: FORK_QUALIFIED_REPO, qualifiedRepository: 'attacker/fork',
ref: 'refs/heads/main', ref: 'refs/heads/main',
commit: '', commit: '',
allowUnsafePrCheckout: false allowUnsafePrCheckout: false
@ -173,25 +172,13 @@ describe('unsafe-pr-checkout-helper', () => {
).toThrow() ).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', () => { it('refuses pull_request_target ignoring repository case differences', () => {
setContext('pull_request_target', forkPullRequestTargetPayload()) setContext('pull_request_target', forkPullRequestTargetPayload())
expect(() => expect(() =>
assertSafePrCheckout({ assertSafePrCheckout({
qualifiedRepository: FORK_QUALIFIED_REPO.toUpperCase(), qualifiedRepository: 'SOME-OWNER/SOME-REPO',
ref: '', ref: '',
commit: '', commit: PR_HEAD_SHA,
allowUnsafePrCheckout: false allowUnsafePrCheckout: false
}) })
).toThrow() ).toThrow()

15
dist/index.js vendored
View File

@ -2793,11 +2793,9 @@ function assertSafePrCheckout(input) {
return; return;
} }
let prHeadRepoId; let prHeadRepoId;
let prHeadRepoFullName;
const prShas = []; const prShas = [];
if (eventName === 'pull_request_target') { if (eventName === 'pull_request_target') {
prHeadRepoId = (0, ref_helper_1.fromPayload)('pull_request.head.repo.id'); 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.head.sha'));
pushIfSha(prShas, (0, ref_helper_1.fromPayload)('pull_request.merge_commit_sha')); pushIfSha(prShas, (0, ref_helper_1.fromPayload)('pull_request.merge_commit_sha'));
} }
@ -2807,13 +2805,7 @@ function assertSafePrCheckout(input) {
return; return;
} }
prHeadRepoId = (0, ref_helper_1.fromPayload)('workflow_run.head_repository.id'); 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')); 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? // (A) Fork PR?
if (typeof prHeadRepoId !== 'number' || prHeadRepoId === baseRepoId) { if (typeof prHeadRepoId !== 'number' || prHeadRepoId === baseRepoId) {
@ -2821,11 +2813,12 @@ 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 baseQualifiedRepository = `${github.context.repo.owner}/${github.context.repo.repo}`;
input.qualifiedRepository.toLowerCase() === prHeadRepoFullName.toLowerCase(); const repositoryDiffersFromBase = input.qualifiedRepository.toLowerCase() !==
baseQualifiedRepository.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 (!repositoryDiffersFromBase &&
!refMatchesPullPattern && !refMatchesPullPattern &&
!commitMatchesPrHeadSha) { !commitMatchesPrHeadSha) {
return; return;

View File

@ -6,7 +6,7 @@ const PR_REF_PATTERN = /^refs\/pull\/[0-9]+\/(?:head|merge)$/
export interface IUnsafePrCheckoutInput { export interface IUnsafePrCheckoutInput {
qualifiedRepository: string qualifiedRepository: string
ref: string ref: string
commit: string | undefined commit: string
allowUnsafePrCheckout: boolean allowUnsafePrCheckout: boolean
} }
@ -26,12 +26,10 @@ export function assertSafePrCheckout(input: IUnsafePrCheckoutInput): void {
} }
let prHeadRepoId: unknown let prHeadRepoId: unknown
let prHeadRepoFullName: unknown
const prShas: string[] = [] const prShas: string[] = []
if (eventName === 'pull_request_target') { if (eventName === 'pull_request_target') {
prHeadRepoId = fromPayload('pull_request.head.repo.id') 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.head.sha'))
pushIfSha(prShas, fromPayload('pull_request.merge_commit_sha')) pushIfSha(prShas, fromPayload('pull_request.merge_commit_sha'))
} else { } else {
@ -40,13 +38,7 @@ export function assertSafePrCheckout(input: IUnsafePrCheckoutInput): void {
return return
} }
prHeadRepoId = fromPayload('workflow_run.head_repository.id') prHeadRepoId = fromPayload('workflow_run.head_repository.id')
prHeadRepoFullName = fromPayload('workflow_run.head_repository.full_name')
pushIfSha(prShas, fromPayload('workflow_run.head_commit.id')) 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? // (A) Fork PR?
@ -56,15 +48,16 @@ export function assertSafePrCheckout(input: IUnsafePrCheckoutInput): void {
// (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 = const baseQualifiedRepository = `${github.context.repo.owner}/${github.context.repo.repo}`
typeof prHeadRepoFullName === 'string' && const repositoryDiffersFromBase =
input.qualifiedRepository.toLowerCase() === prHeadRepoFullName.toLowerCase() input.qualifiedRepository.toLowerCase() !==
baseQualifiedRepository.toLowerCase()
const refMatchesPullPattern = PR_REF_PATTERN.test(input.ref) const refMatchesPullPattern = PR_REF_PATTERN.test(input.ref)
const commitMatchesPrHeadSha = const commitMatchesPrHeadSha =
!!input.commit && prShas.includes(input.commit.toLowerCase()) !!input.commit && prShas.includes(input.commit.toLowerCase())
if ( if (
!repositoryMatchesPrHead && !repositoryDiffersFromBase &&
!refMatchesPullPattern && !refMatchesPullPattern &&
!commitMatchesPrHeadSha !commitMatchesPrHeadSha
) { ) {