Compare commits

...

3 Commits

Author SHA1 Message Date
Nathan Moin Vaziri 4cfef1b845
Merge 2d76d21b2e into 62661c4e71 2026-07-15 17:49:22 +00:00
Aiqiao Yan 62661c4e71
skip running unsafe pr check if input is default (#2518) 2026-07-15 10:02:03 -04:00
Nathan Moin Vaziri 2d76d21b2e
Pin GIT_CONFIG_GLOBAL to the temp config during global auth setup
configureTempGlobalConfig isolates global git config by overriding HOME to
a temporary directory. But GIT_CONFIG_GLOBAL takes precedence over HOME when
git locates the global config file, so when a workflow already has
GIT_CONFIG_GLOBAL set in the environment, 'git config --global' writes land
in that file instead of the temporary config. replaceTokenPlaceholder then
reads the temporary config, cannot find the placeholder, and fails with
'Unable to replace auth placeholder'.

Set GIT_CONFIG_GLOBAL to the temporary config alongside the HOME override so
global config operations always target the temp file regardless of any
inherited value, and unset it again in removeGlobalConfig.

Assisted-By: Claude Opus 4.8
2026-06-24 10:26:49 -07:00
5 changed files with 106 additions and 14 deletions

View File

@ -923,6 +923,25 @@ describe('git-auth-helper tests', () => {
} }
}) })
const configureGlobalAuth_overridesGitConfigGlobal =
'configureGlobalAuth overrides GIT_CONFIG_GLOBAL'
it(configureGlobalAuth_overridesGitConfigGlobal, async () => {
// Arrange
await setup(configureGlobalAuth_overridesGitConfigGlobal)
const authHelper = gitAuthHelper.createAuthHelper(git, settings)
// Act
await authHelper.configureAuth()
await authHelper.configureGlobalAuth()
// Assert GIT_CONFIG_GLOBAL is pinned to the temporary global config, so an
// inherited GIT_CONFIG_GLOBAL cannot redirect --global writes
expect(git.env['HOME']).toBeTruthy()
expect(git.env['GIT_CONFIG_GLOBAL']).toBe(
path.join(git.env['HOME'], '.gitconfig')
)
})
const removeGlobalConfig_removesOverride = const removeGlobalConfig_removesOverride =
'removeGlobalConfig removes override' 'removeGlobalConfig removes override'
it(removeGlobalConfig_removesOverride, async () => { it(removeGlobalConfig_removesOverride, async () => {
@ -933,6 +952,7 @@ describe('git-auth-helper tests', () => {
await authHelper.configureGlobalAuth() await authHelper.configureGlobalAuth()
const homeOverride = git.env['HOME'] // Sanity check const homeOverride = git.env['HOME'] // Sanity check
expect(homeOverride).toBeTruthy() expect(homeOverride).toBeTruthy()
expect(git.env['GIT_CONFIG_GLOBAL']).toBeTruthy()
await fs.promises.stat(path.join(git.env['HOME'], '.gitconfig')) await fs.promises.stat(path.join(git.env['HOME'], '.gitconfig'))
// Act // Act
@ -940,6 +960,7 @@ describe('git-auth-helper tests', () => {
// Assert // Assert
expect(git.env['HOME']).toBeUndefined() expect(git.env['HOME']).toBeUndefined()
expect(git.env['GIT_CONFIG_GLOBAL']).toBeUndefined()
try { try {
await fs.promises.stat(homeOverride) await fs.promises.stat(homeOverride)
throw new Error(`Should have been deleted '${homeOverride}'`) throw new Error(`Should have been deleted '${homeOverride}'`)

View File

@ -180,4 +180,50 @@ describe('input-helper tests', () => {
const settings: IGitSourceSettings = await inputHelper.getInputs() const settings: IGitSourceSettings = await inputHelper.getInputs()
expect(settings.workflowOrganizationId).toBe(123456) 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
}
})
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
}
})
})
}) })

26
dist/index.js vendored
View File

@ -35109,6 +35109,10 @@ class GitAuthHelper {
// Override HOME // Override HOME
info(`Temporarily overriding HOME='${this.temporaryHomePath}' before making global git config changes`); info(`Temporarily overriding HOME='${this.temporaryHomePath}' before making global git config changes`);
this.git.setEnvironmentVariable('HOME', this.temporaryHomePath); this.git.setEnvironmentVariable('HOME', this.temporaryHomePath);
// GIT_CONFIG_GLOBAL takes precedence over HOME when locating the global
// config file. Pin it to the temporary config so an inherited
// GIT_CONFIG_GLOBAL cannot redirect our global git config writes elsewhere.
this.git.setEnvironmentVariable('GIT_CONFIG_GLOBAL', newGitConfigPath);
return newGitConfigPath; return newGitConfigPath;
} }
async configureGlobalAuth() { async configureGlobalAuth() {
@ -35183,8 +35187,9 @@ class GitAuthHelper {
} }
async removeGlobalConfig() { async removeGlobalConfig() {
if (this.temporaryHomePath?.length > 0) { if (this.temporaryHomePath?.length > 0) {
core_debug(`Unsetting HOME override`); core_debug(`Unsetting HOME and GIT_CONFIG_GLOBAL overrides`);
this.git.removeEnvironmentVariable('HOME'); this.git.removeEnvironmentVariable('HOME');
this.git.removeEnvironmentVariable('GIT_CONFIG_GLOBAL');
await rmRF(this.temporaryHomePath); await rmRF(this.temporaryHomePath);
} }
} }
@ -42191,12 +42196,19 @@ async function getInputs() {
(getInput('allow-unsafe-pr-checkout') || 'false').toUpperCase() === (getInput('allow-unsafe-pr-checkout') || 'false').toUpperCase() ===
'TRUE'; 'TRUE';
core_debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`); core_debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`);
assertSafePrCheckout({ // The default self-checkout (this repository with no explicit ref) always
qualifiedRepository, // resolves to the trusted ref/commit GitHub set for the triggering event, so
ref: result.ref, // the fork-checkout guard only needs to run when the caller customized the
commit: result.commit, // repository or ref.
allowUnsafePrCheckout: result.allowUnsafePrCheckout const isDefaultCheckout = isWorkflowRepository && !getInput('ref');
}); if (!isDefaultCheckout) {
assertSafePrCheckout({
qualifiedRepository,
ref: result.ref,
commit: result.commit,
allowUnsafePrCheckout: result.allowUnsafePrCheckout
});
}
return result; return result;
} }

View File

@ -122,6 +122,11 @@ class GitAuthHelper {
) )
this.git.setEnvironmentVariable('HOME', this.temporaryHomePath) this.git.setEnvironmentVariable('HOME', this.temporaryHomePath)
// GIT_CONFIG_GLOBAL takes precedence over HOME when locating the global
// config file. Pin it to the temporary config so an inherited
// GIT_CONFIG_GLOBAL cannot redirect our global git config writes elsewhere.
this.git.setEnvironmentVariable('GIT_CONFIG_GLOBAL', newGitConfigPath)
return newGitConfigPath return newGitConfigPath
} }
@ -237,8 +242,9 @@ class GitAuthHelper {
async removeGlobalConfig(): Promise<void> { async removeGlobalConfig(): Promise<void> {
if (this.temporaryHomePath?.length > 0) { if (this.temporaryHomePath?.length > 0) {
core.debug(`Unsetting HOME override`) core.debug(`Unsetting HOME and GIT_CONFIG_GLOBAL overrides`)
this.git.removeEnvironmentVariable('HOME') this.git.removeEnvironmentVariable('HOME')
this.git.removeEnvironmentVariable('GIT_CONFIG_GLOBAL')
await io.rmRF(this.temporaryHomePath) await io.rmRF(this.temporaryHomePath)
} }
} }

View File

@ -168,12 +168,19 @@ export async function getInputs(): Promise<IGitSourceSettings> {
'TRUE' 'TRUE'
core.debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`) core.debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`)
unsafePrCheckoutHelper.assertSafePrCheckout({ // The default self-checkout (this repository with no explicit ref) always
qualifiedRepository, // resolves to the trusted ref/commit GitHub set for the triggering event, so
ref: result.ref, // the fork-checkout guard only needs to run when the caller customized the
commit: result.commit, // repository or ref.
allowUnsafePrCheckout: result.allowUnsafePrCheckout const isDefaultCheckout = isWorkflowRepository && !core.getInput('ref')
}) if (!isDefaultCheckout) {
unsafePrCheckoutHelper.assertSafePrCheckout({
qualifiedRepository,
ref: result.ref,
commit: result.commit,
allowUnsafePrCheckout: result.allowUnsafePrCheckout
})
}
return result return result
} }