Compare commits

...

3 Commits

Author SHA1 Message Date
Michi Mutsuzaki
8202185c56
Merge e832aee124 into 900f2210b1 2026-05-05 08:41:55 +02:00
Yashwanth Anantharaju
900f2210b1
fix: expand merge commit SHA regex and add SHA-256 test cases (#2414)
Some checks failed
Check dist / check-dist (push) Has been cancelled
CodeQL / Analyze (javascript) (push) Has been cancelled
Licensed / Check licenses (push) Has been cancelled
Build and Test / build (push) Has been cancelled
Build and Test / test (macos-latest) (push) Has been cancelled
Build and Test / test (ubuntu-latest) (push) Has been cancelled
Build and Test / test (windows-latest) (push) Has been cancelled
Build and Test / test-proxy (push) Has been cancelled
Build and Test / test-bypass-proxy (push) Has been cancelled
Build and Test / test-git-container (push) Has been cancelled
Build and Test / test-output (push) Has been cancelled
* 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
Michi Mutsuzaki
e832aee124
Change the default value of persist-credentials to false
Change the default value of persist-credentials setting from true to
false to reduce the risk of unintentionally exposing the GITHUB_TOKEN
secret.

Fixes: #485

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
2024-04-20 23:37:24 +00:00
7 changed files with 167 additions and 7 deletions

View File

@ -22,7 +22,7 @@ This action checks-out your repository under `$GITHUB_WORKSPACE`, so your workfl
Only a single commit is fetched by default, for the ref/SHA that triggered the workflow. Set `fetch-depth: 0` to fetch all history for all branches and tags. Refer [here](https://docs.github.com/actions/using-workflows/events-that-trigger-workflows) to learn which commit `$GITHUB_SHA` points to for different events. Only a single commit is fetched by default, for the ref/SHA that triggered the workflow. Set `fetch-depth: 0` to fetch all history for all branches and tags. Refer [here](https://docs.github.com/actions/using-workflows/events-that-trigger-workflows) to learn which commit `$GITHUB_SHA` points to for different events.
The auth token is persisted in the local git config. This enables your scripts to run authenticated git commands. The token is removed during post-job cleanup. Set `persist-credentials: false` to opt-out. Set `persist-credentials: true` to opt-in to persist the auth token in the local git config. This enables your scripts to run authenticated git commands. The token is removed during post-job cleanup.
When Git 2.18 or higher is not in your PATH, falls back to the REST API to download the files. When Git 2.18 or higher is not in your PATH, falls back to the REST API to download the files.
@ -102,7 +102,7 @@ Please refer to the [release page](https://github.com/actions/checkout/releases/
ssh-user: '' ssh-user: ''
# Whether to configure the token or SSH key with the local git config # Whether to configure the token or SSH key with the local git config
# Default: true # Default: false
persist-credentials: '' persist-credentials: ''
# Relative path under $GITHUB_WORKSPACE to place the repository # Relative path under $GITHUB_WORKSPACE to place the repository

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')
})
})
}) })

View File

@ -51,7 +51,7 @@ inputs:
default: git default: git
persist-credentials: persist-credentials:
description: 'Whether to configure the token or SSH key with the local git config' description: 'Whether to configure the token or SSH key with the local git config'
default: true default: false
path: path:
description: 'Relative path under $GITHUB_WORKSPACE to place the repository' description: 'Relative path under $GITHUB_WORKSPACE to place the repository'
clean: clean:

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

@ -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