Compare commits

...

3 Commits

Author SHA1 Message Date
brian m. carlson 3b5c8bf0f1
Merge 02ade5d400 into 900f2210b1 2026-05-27 00:50:10 +08:00
Yashwanth Anantharaju 900f2210b1
fix: expand merge commit SHA regex and add SHA-256 test cases (#2414)
* fix: expand merge commit SHA regex and add SHA-256 test cases

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test: add checkCommitInfo SHA coverage

Add checkCommitInfo tests for SHA-1 and SHA-256 merge messages and reject invalid 50-character hex merge heads.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* style: fix Prettier formatting in test and source files

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-05-04 13:30:55 -04:00
brian m. carlson 02ade5d400
Don't overwrite annotated tags with commit object
When checking out a repository with full history, a full clone is done
and then the ref is finally updated to point to the commit that caused
the workflow to be run.  Normally, this is a good protection against
someone pushing to the repository twice in short succession, but it
causes problems with annotated tags.

Specifically, because the entry in refs/tags is set to the commit hash,
if an annotated tag was used, the tag is turned merely into a
lightweight one, which breaks `git describe`.  Every other tag in the
repository will continue to remain a valid annotated tag except the one
for which the workflow was invoked, which is not what the user expected.

Let's work around this by not performing a fetch if what we're fetching
is a tag.  Technically, annotated tags can be anywhere in the hierarchy
at any ref, but this should work as a suitable heuristic for now.

Note that the proper solution would be to expose the revision of the
actual object and check against that instead of the commit, but it
doesn't presently appear that that information is exposed.  Also, we
explicitly do not case-fold since Git refs are case sensitive.
2022-02-14 23:18:53 +00:00
6 changed files with 165 additions and 5 deletions

View File

@ -133,6 +133,16 @@ describe('input-helper tests', () => {
expect(settings.commit).toBe('1111111111222222222233333333334444444444') expect(settings.commit).toBe('1111111111222222222233333333334444444444')
}) })
it('sets ref to empty when explicit sha-256', async () => {
inputs.ref =
'1111111111222222222233333333334444444444555555555566666666667777'
const settings: IGitSourceSettings = await inputHelper.getInputs()
expect(settings.ref).toBeFalsy()
expect(settings.commit).toBe(
'1111111111222222222233333333334444444444555555555566666666667777'
)
})
it('sets sha to empty when explicit ref', async () => { it('sets sha to empty when explicit ref', async () => {
inputs.ref = 'refs/heads/some-other-ref' inputs.ref = 'refs/heads/some-other-ref'
const settings: IGitSourceSettings = await inputHelper.getInputs() const settings: IGitSourceSettings = await inputHelper.getInputs()

View File

@ -1,8 +1,12 @@
import * as assert from 'assert' import * as assert from 'assert'
import * as core from '@actions/core'
import * as github from '@actions/github'
import * as refHelper from '../lib/ref-helper' import * as refHelper from '../lib/ref-helper'
import {IGitCommandManager} from '../lib/git-command-manager' import {IGitCommandManager} from '../lib/git-command-manager'
const commit = '1234567890123456789012345678901234567890' const commit = '1234567890123456789012345678901234567890'
const sha256Commit =
'1234567890123456789012345678901234567890123456789012345678901234'
let git: IGitCommandManager let git: IGitCommandManager
describe('ref-helper tests', () => { describe('ref-helper tests', () => {
@ -37,6 +41,12 @@ describe('ref-helper tests', () => {
expect(checkoutInfo.startPoint).toBeFalsy() expect(checkoutInfo.startPoint).toBeFalsy()
}) })
it('getCheckoutInfo sha-256 only', async () => {
const checkoutInfo = await refHelper.getCheckoutInfo(git, '', sha256Commit)
expect(checkoutInfo.ref).toBe(sha256Commit)
expect(checkoutInfo.startPoint).toBeFalsy()
})
it('getCheckoutInfo refs/heads/', async () => { it('getCheckoutInfo refs/heads/', async () => {
const checkoutInfo = await refHelper.getCheckoutInfo( const checkoutInfo = await refHelper.getCheckoutInfo(
git, git,
@ -227,4 +237,142 @@ describe('ref-helper tests', () => {
'+refs/heads/my/branch:refs/remotes/origin/my/branch' '+refs/heads/my/branch:refs/remotes/origin/my/branch'
) )
}) })
describe('checkCommitInfo', () => {
const repositoryOwner = 'some-owner'
const repositoryName = 'some-repo'
const ref = 'refs/pull/123/merge'
const sha1Head = '1111111111222222222233333333334444444444'
const sha1Base = 'aaaaaaaaaabbbbbbbbbbccccccccccdddddddddd'
const sha256Head =
'1111111111222222222233333333334444444444555555555566666666667777'
const sha256Base =
'aaaaaaaaaabbbbbbbbbbccccccccccddddddddddeeeeeeeeeeffffffffff0000'
let debugSpy: jest.SpyInstance
let getOctokitSpy: jest.SpyInstance
let repoGetSpy: jest.Mock
let originalEventName: string
let originalPayload: unknown
let originalRef: string
let originalSha: string
function setPullRequestContext(
expectedHeadSha: string,
expectedBaseSha: string,
mergeCommit: string
): void {
;(github.context as any).eventName = 'pull_request'
github.context.ref = ref
github.context.sha = mergeCommit
;(github.context as any).payload = {
action: 'synchronize',
after: expectedHeadSha,
number: 123,
pull_request: {
base: {
sha: expectedBaseSha
}
},
repository: {
private: false
}
}
}
beforeEach(() => {
originalEventName = github.context.eventName
originalPayload = github.context.payload
originalRef = github.context.ref
originalSha = github.context.sha
jest.spyOn(github.context, 'repo', 'get').mockReturnValue({
owner: repositoryOwner,
repo: repositoryName
})
debugSpy = jest.spyOn(core, 'debug').mockImplementation(jest.fn())
repoGetSpy = jest.fn(async () => ({}))
getOctokitSpy = jest.spyOn(github, 'getOctokit').mockReturnValue({
rest: {
repos: {
get: repoGetSpy
}
}
} as any)
})
afterEach(() => {
;(github.context as any).eventName = originalEventName
;(github.context as any).payload = originalPayload
github.context.ref = originalRef
github.context.sha = originalSha
jest.restoreAllMocks()
})
it('returns early for SHA-1 merge commit', async () => {
setPullRequestContext(sha1Head, sha1Base, commit)
await refHelper.checkCommitInfo(
'token',
`Merge ${sha1Head} into ${sha1Base}`,
repositoryOwner,
repositoryName,
ref,
commit
)
expect(getOctokitSpy).not.toHaveBeenCalled()
expect(repoGetSpy).not.toHaveBeenCalled()
})
it('matches SHA-256 merge commit info', async () => {
const actualHeadSha =
'9999999999888888888877777777776666666666555555555544444444443333'
setPullRequestContext(sha256Head, sha256Base, sha256Commit)
await refHelper.checkCommitInfo(
'token',
`Merge ${actualHeadSha} into ${sha256Base}`,
repositoryOwner,
repositoryName,
ref,
sha256Commit
)
expect(getOctokitSpy).toHaveBeenCalledWith(
'token',
expect.objectContaining({
userAgent: expect.stringContaining(
`expected_head_sha=${sha256Head};actual_head_sha=${actualHeadSha}`
)
})
)
expect(repoGetSpy).toHaveBeenCalledWith({
owner: repositoryOwner,
repo: repositoryName
})
expect(debugSpy).toHaveBeenCalledWith(
`Expected head sha ${sha256Head}; actual head sha ${actualHeadSha}`
)
expect(debugSpy).not.toHaveBeenCalledWith('Unexpected message format')
})
it('does not match 50-char hex as a valid merge', async () => {
const invalidHeadSha =
'99999999998888888888777777777766666666665555555555'
setPullRequestContext(sha1Head, sha1Base, commit)
await refHelper.checkCommitInfo(
'token',
`Merge ${invalidHeadSha} into ${sha1Base}`,
repositoryOwner,
repositoryName,
ref,
commit
)
expect(getOctokitSpy).not.toHaveBeenCalled()
expect(repoGetSpy).not.toHaveBeenCalled()
expect(debugSpy).toHaveBeenCalledWith('Unexpected message format')
})
})
}) })

4
dist/index.js vendored
View File

@ -2021,7 +2021,7 @@ function getInputs() {
} }
} }
// SHA? // SHA?
else if (result.ref.match(/^[0-9a-fA-F]{40}$/)) { else if (result.ref.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) {
result.commit = result.ref; result.commit = result.ref;
result.ref = ''; result.ref = '';
} }
@ -2444,7 +2444,7 @@ function checkCommitInfo(token, commitInfo, repositoryOwner, repositoryName, ref
return; return;
} }
// Extract details from message // Extract details from message
const match = commitInfo.match(/Merge ([0-9a-f]{40}) into ([0-9a-f]{40})/); const match = commitInfo.match(/Merge ([0-9a-f]{40}|[0-9a-f]{64}) into ([0-9a-f]{40}|[0-9a-f]{64})/);
if (!match) { if (!match) {
core.debug('Unexpected message format'); core.debug('Unexpected message format');
return; return;

View File

@ -178,7 +178,7 @@ export async function getSource(settings: IGitSourceSettings): Promise<void> {
// When all history is fetched, the ref we're interested in may have moved to a different // When all history is fetched, the ref we're interested in may have moved to a different
// commit (push or force push). If so, fetch again with a targeted refspec. // commit (push or force push). If so, fetch again with a targeted refspec.
if (!(await refHelper.testRef(git, settings.ref, settings.commit))) { if (!settings.refs.startsWith("refs/tags") && !(await refHelper.testRef(git, settings.ref, settings.commit))) {
refSpec = refHelper.getRefSpec(settings.ref, settings.commit) refSpec = refHelper.getRefSpec(settings.ref, settings.commit)
await git.fetch(refSpec, fetchOptions) await git.fetch(refSpec, fetchOptions)

View File

@ -71,7 +71,7 @@ export async function getInputs(): Promise<IGitSourceSettings> {
} }
} }
// SHA? // SHA?
else if (result.ref.match(/^[0-9a-fA-F]{40}$/)) { else if (result.ref.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) {
result.commit = result.ref result.commit = result.ref
result.ref = '' result.ref = ''
} }

View File

@ -258,7 +258,9 @@ export async function checkCommitInfo(
} }
// Extract details from message // Extract details from message
const match = commitInfo.match(/Merge ([0-9a-f]{40}) into ([0-9a-f]{40})/) const match = commitInfo.match(
/Merge ([0-9a-f]{40}|[0-9a-f]{64}) into ([0-9a-f]{40}|[0-9a-f]{64})/
)
if (!match) { if (!match) {
core.debug('Unexpected message format') core.debug('Unexpected message format')
return return