mirror of
https://github.com/dorny/paths-filter.git
synced 2026-08-24 07:34:43 +08:00
fix: work around git dubious ownership errors in container jobs (#317)
This commit is contained in:
Sascha Bratton
@@ -0,0 +1,113 @@
|
||||
import {getExecOutput, ExecOutput} from '@actions/exec'
|
||||
import {gitExec} from '../src/git'
|
||||
import {ensureSafeDirectory, getGitEnv} from '../src/safe-directory'
|
||||
|
||||
jest.mock('@actions/exec')
|
||||
jest.mock('../src/safe-directory', () => ({
|
||||
...jest.requireActual('../src/safe-directory'),
|
||||
ensureSafeDirectory: jest.fn(),
|
||||
getGitEnv: jest.fn()
|
||||
}))
|
||||
|
||||
const getExecOutputMock = getExecOutput as jest.MockedFunction<typeof getExecOutput>
|
||||
const ensureSafeDirectoryMock = ensureSafeDirectory as jest.MockedFunction<typeof ensureSafeDirectory>
|
||||
const getGitEnvMock = getGitEnv as jest.MockedFunction<typeof getGitEnv>
|
||||
|
||||
const SUCCESS_OUTPUT: ExecOutput = {exitCode: 0, stdout: 'ok', stderr: ''}
|
||||
const DUBIOUS_OUTPUT: ExecOutput = {
|
||||
exitCode: 128,
|
||||
stdout: '',
|
||||
stderr: "fatal: detected dubious ownership in repository at '/github/workspace'"
|
||||
}
|
||||
|
||||
// clearMocks in jest.config.js does not remove queued mockResolvedValueOnce values or implementations
|
||||
beforeEach(() => {
|
||||
getExecOutputMock.mockReset()
|
||||
ensureSafeDirectoryMock.mockReset()
|
||||
getGitEnvMock.mockReset()
|
||||
})
|
||||
|
||||
describe('gitExec', () => {
|
||||
test('returns result of successful command without invoking the workaround', async () => {
|
||||
getExecOutputMock.mockResolvedValueOnce(SUCCESS_OUTPUT)
|
||||
|
||||
const result = await gitExec(['status'])
|
||||
|
||||
expect(result).toBe(SUCCESS_OUTPUT)
|
||||
expect(getExecOutputMock).toHaveBeenCalledTimes(1)
|
||||
expect(getExecOutputMock).toHaveBeenCalledWith('git', ['status'], expect.objectContaining({ignoreReturnCode: true}))
|
||||
expect(ensureSafeDirectoryMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('passes environment from getGitEnv to git', async () => {
|
||||
const env = {HOME: '/temp/home'}
|
||||
getGitEnvMock.mockReturnValue(env)
|
||||
getExecOutputMock.mockResolvedValueOnce(SUCCESS_OUTPUT)
|
||||
|
||||
await gitExec(['status'])
|
||||
|
||||
expect(getExecOutputMock).toHaveBeenCalledWith('git', ['status'], expect.objectContaining({env}))
|
||||
})
|
||||
|
||||
test('retries once after dubious ownership error is worked around', async () => {
|
||||
getExecOutputMock.mockResolvedValueOnce(DUBIOUS_OUTPUT).mockResolvedValueOnce(SUCCESS_OUTPUT)
|
||||
ensureSafeDirectoryMock.mockResolvedValueOnce(true)
|
||||
|
||||
const result = await gitExec(['status'])
|
||||
|
||||
expect(result).toBe(SUCCESS_OUTPUT)
|
||||
expect(ensureSafeDirectoryMock).toHaveBeenCalledWith(DUBIOUS_OUTPUT.stderr)
|
||||
expect(getExecOutputMock).toHaveBeenCalledTimes(2)
|
||||
for (const call of getExecOutputMock.mock.calls) {
|
||||
expect(call[2]).toEqual(expect.objectContaining({ignoreReturnCode: true}))
|
||||
}
|
||||
})
|
||||
|
||||
test('throws actionable error when retry still fails with dubious ownership', async () => {
|
||||
getExecOutputMock.mockResolvedValueOnce(DUBIOUS_OUTPUT).mockResolvedValueOnce(DUBIOUS_OUTPUT)
|
||||
ensureSafeDirectoryMock.mockResolvedValueOnce(true)
|
||||
|
||||
const promise = gitExec(['status'])
|
||||
|
||||
await expect(promise).rejects.toThrow(/detected dubious ownership/)
|
||||
await expect(promise).rejects.toThrow(/safe\.directory/)
|
||||
await expect(promise).rejects.toThrow(/--user/)
|
||||
expect(getExecOutputMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
test('throws without retry when workaround adds nothing new', async () => {
|
||||
getExecOutputMock.mockResolvedValueOnce(DUBIOUS_OUTPUT)
|
||||
ensureSafeDirectoryMock.mockResolvedValueOnce(false)
|
||||
|
||||
await expect(gitExec(['status'])).rejects.toThrow(/safe\.directory/)
|
||||
expect(getExecOutputMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
test('returns non-dubious failure when ignoreReturnCode is set', async () => {
|
||||
const failure: ExecOutput = {exitCode: 1, stdout: '', stderr: 'some error'}
|
||||
getExecOutputMock.mockResolvedValueOnce(failure)
|
||||
|
||||
const result = await gitExec(['show-ref', 'master'], {ignoreReturnCode: true})
|
||||
|
||||
expect(result).toBe(failure)
|
||||
expect(ensureSafeDirectoryMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('retries dubious ownership error even when ignoreReturnCode is set', async () => {
|
||||
getExecOutputMock.mockResolvedValueOnce(DUBIOUS_OUTPUT).mockResolvedValueOnce(SUCCESS_OUTPUT)
|
||||
ensureSafeDirectoryMock.mockResolvedValueOnce(true)
|
||||
|
||||
const result = await gitExec(['show-ref', 'master'], {ignoreReturnCode: true})
|
||||
|
||||
expect(result).toBe(SUCCESS_OUTPUT)
|
||||
expect(getExecOutputMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
test('throws on non-dubious failure when ignoreReturnCode is not set', async () => {
|
||||
getExecOutputMock.mockResolvedValueOnce({exitCode: 1, stdout: '', stderr: 'some error'})
|
||||
|
||||
await expect(gitExec(['fetch'])).rejects.toThrow("The process 'git fetch' failed with exit code 1")
|
||||
expect(getExecOutputMock).toHaveBeenCalledTimes(1)
|
||||
expect(ensureSafeDirectoryMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,256 @@
|
||||
import * as fs from 'fs'
|
||||
import * as os from 'os'
|
||||
import * as path from 'path'
|
||||
import {exec} from '@actions/exec'
|
||||
import {
|
||||
buildGitEnv,
|
||||
cleanup,
|
||||
createTempGitHome,
|
||||
ensureSafeDirectory,
|
||||
getGitEnv,
|
||||
isDubiousOwnershipError,
|
||||
parseRepositoryPath,
|
||||
resolveTempBaseDir
|
||||
} from '../src/safe-directory'
|
||||
|
||||
jest.mock('@actions/exec')
|
||||
|
||||
const execMock = exec as jest.MockedFunction<typeof exec>
|
||||
|
||||
const DUBIOUS_STDERR = "fatal: detected dubious ownership in repository at '/github/workspace'"
|
||||
const UNSAFE_STDERR = "fatal: unsafe repository ('/github/workspace' is owned by someone else)"
|
||||
|
||||
describe('detection of dubious ownership errors', () => {
|
||||
test('detects "detected dubious ownership" wording at exit code 128', () => {
|
||||
expect(isDubiousOwnershipError(128, DUBIOUS_STDERR)).toBe(true)
|
||||
})
|
||||
|
||||
test('detects older "unsafe repository" wording at exit code 128', () => {
|
||||
expect(isDubiousOwnershipError(128, UNSAFE_STDERR)).toBe(true)
|
||||
})
|
||||
|
||||
test('does not match other git errors at exit code 128', () => {
|
||||
expect(isDubiousOwnershipError(128, 'fatal: not a git repository')).toBe(false)
|
||||
})
|
||||
|
||||
test('does not match dubious ownership text at other exit codes', () => {
|
||||
expect(isDubiousOwnershipError(1, DUBIOUS_STDERR)).toBe(false)
|
||||
expect(isDubiousOwnershipError(0, DUBIOUS_STDERR)).toBe(false)
|
||||
})
|
||||
|
||||
test('parseRepositoryPath extracts path from both wordings', () => {
|
||||
expect(parseRepositoryPath(DUBIOUS_STDERR)).toBe('/github/workspace')
|
||||
expect(parseRepositoryPath(UNSAFE_STDERR)).toBe('/github/workspace')
|
||||
expect(parseRepositoryPath('fatal: not a git repository')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('createTempGitHome', () => {
|
||||
const scratchDirs: string[] = []
|
||||
|
||||
async function makeScratchDir(): Promise<string> {
|
||||
const dir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'safe-directory-test-'))
|
||||
scratchDirs.push(dir)
|
||||
return dir
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
for (const dir of scratchDirs.splice(0)) {
|
||||
await fs.promises.rm(dir, {recursive: true, force: true})
|
||||
}
|
||||
})
|
||||
|
||||
test('copies file referenced by GIT_CONFIG_GLOBAL and skips XDG fallback', async () => {
|
||||
const base = await makeScratchDir()
|
||||
const home = await makeScratchDir()
|
||||
const configFile = path.join(home, 'custom-gitconfig')
|
||||
await fs.promises.writeFile(configFile, 'custom')
|
||||
await fs.promises.mkdir(path.join(home, '.config', 'git'), {recursive: true})
|
||||
await fs.promises.writeFile(path.join(home, '.config', 'git', 'config'), 'xdg')
|
||||
|
||||
const tempHome = await createTempGitHome(base, {GIT_CONFIG_GLOBAL: configFile, HOME: home})
|
||||
scratchDirs.push(tempHome)
|
||||
|
||||
expect(await fs.promises.readFile(path.join(tempHome, '.gitconfig'), 'utf8')).toBe('custom')
|
||||
expect(fs.existsSync(path.join(tempHome, '.config', 'git', 'config'))).toBe(false)
|
||||
})
|
||||
|
||||
test('does not throw when GIT_CONFIG_GLOBAL references missing file', async () => {
|
||||
const base = await makeScratchDir()
|
||||
|
||||
const tempHome = await createTempGitHome(base, {GIT_CONFIG_GLOBAL: path.join(base, 'missing-gitconfig')})
|
||||
scratchDirs.push(tempHome)
|
||||
|
||||
expect(await fs.promises.readFile(path.join(tempHome, '.gitconfig'), 'utf8')).toBe('')
|
||||
})
|
||||
|
||||
test('copies $HOME/.gitconfig', async () => {
|
||||
const base = await makeScratchDir()
|
||||
const home = await makeScratchDir()
|
||||
await fs.promises.writeFile(path.join(home, '.gitconfig'), 'home config')
|
||||
|
||||
const tempHome = await createTempGitHome(base, {HOME: home})
|
||||
scratchDirs.push(tempHome)
|
||||
|
||||
expect(await fs.promises.readFile(path.join(tempHome, '.gitconfig'), 'utf8')).toBe('home config')
|
||||
})
|
||||
|
||||
test('copies XDG fallback config only when XDG_CONFIG_HOME is unset', async () => {
|
||||
const base = await makeScratchDir()
|
||||
const home = await makeScratchDir()
|
||||
await fs.promises.mkdir(path.join(home, '.config', 'git'), {recursive: true})
|
||||
await fs.promises.writeFile(path.join(home, '.config', 'git', 'config'), 'xdg config')
|
||||
|
||||
const tempHome = await createTempGitHome(base, {HOME: home})
|
||||
scratchDirs.push(tempHome)
|
||||
expect(await fs.promises.readFile(path.join(tempHome, '.config', 'git', 'config'), 'utf8')).toBe('xdg config')
|
||||
|
||||
const tempHomeWithXdg = await createTempGitHome(base, {HOME: home, XDG_CONFIG_HOME: path.join(home, '.config')})
|
||||
scratchDirs.push(tempHomeWithXdg)
|
||||
expect(fs.existsSync(path.join(tempHomeWithXdg, '.config', 'git', 'config'))).toBe(false)
|
||||
})
|
||||
|
||||
test('creates an empty .gitconfig even when there is no config to copy', async () => {
|
||||
const base = await makeScratchDir()
|
||||
|
||||
const tempHome = await createTempGitHome(base, {})
|
||||
scratchDirs.push(tempHome)
|
||||
|
||||
expect(await fs.promises.readdir(tempHome)).toEqual(['.gitconfig'])
|
||||
expect(await fs.promises.readFile(path.join(tempHome, '.gitconfig'), 'utf8')).toBe('')
|
||||
})
|
||||
})
|
||||
|
||||
describe('buildGitEnv', () => {
|
||||
test('overrides HOME and GIT_CONFIG_GLOBAL, preserves other variables, drops undefined values', () => {
|
||||
const env = buildGitEnv('/temp/home', {
|
||||
HOME: '/root',
|
||||
GIT_CONFIG_GLOBAL: '/root/.gitconfig',
|
||||
PATH: '/usr/bin',
|
||||
UNDEFINED_VALUE: undefined
|
||||
})
|
||||
|
||||
expect(env['HOME']).toBe('/temp/home')
|
||||
expect(env['GIT_CONFIG_GLOBAL']).toBe(path.join('/temp/home', '.gitconfig'))
|
||||
expect(env['PATH']).toBe('/usr/bin')
|
||||
expect('UNDEFINED_VALUE' in env).toBe(false)
|
||||
})
|
||||
|
||||
test('leaves GIT_CONFIG_GLOBAL unset when not present in the original environment', () => {
|
||||
const env = buildGitEnv('/temp/home', {HOME: '/root', PATH: '/usr/bin'})
|
||||
|
||||
expect(env['HOME']).toBe('/temp/home')
|
||||
expect('GIT_CONFIG_GLOBAL' in env).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveTempBaseDir', () => {
|
||||
test('prefers RUNNER_TEMP and falls back to os.tmpdir()', () => {
|
||||
expect(resolveTempBaseDir({RUNNER_TEMP: '/runner/temp'})).toBe('/runner/temp')
|
||||
expect(resolveTempBaseDir({RUNNER_TEMP: ''})).toBe(os.tmpdir())
|
||||
expect(resolveTempBaseDir({})).toBe(os.tmpdir())
|
||||
})
|
||||
})
|
||||
|
||||
describe('ensureSafeDirectory', () => {
|
||||
const envBackup = process.env
|
||||
let runnerTemp: string
|
||||
|
||||
beforeEach(async () => {
|
||||
runnerTemp = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'safe-directory-test-runner-'))
|
||||
process.env = {...envBackup}
|
||||
process.env['RUNNER_TEMP'] = runnerTemp
|
||||
process.env['HOME'] = runnerTemp
|
||||
process.env['GITHUB_WORKSPACE'] = process.cwd()
|
||||
delete process.env['GIT_CONFIG_GLOBAL']
|
||||
delete process.env['XDG_CONFIG_HOME']
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await cleanup()
|
||||
await fs.promises.rm(runnerTemp, {recursive: true, force: true})
|
||||
process.env = envBackup
|
||||
})
|
||||
|
||||
test('activates temporary HOME and adds reported directories on first call', async () => {
|
||||
expect(getGitEnv()['HOME']).not.toContain('paths-filter-git-home-')
|
||||
|
||||
const added = await ensureSafeDirectory(DUBIOUS_STDERR)
|
||||
|
||||
expect(added).toBe(true)
|
||||
expect(getGitEnv()).toEqual(
|
||||
expect.objectContaining({
|
||||
HOME: expect.stringContaining('paths-filter-git-home-')
|
||||
})
|
||||
)
|
||||
// GIT_CONFIG_GLOBAL was not set in the original environment, so it must stay unset
|
||||
expect(getGitEnv()).not.toHaveProperty('GIT_CONFIG_GLOBAL')
|
||||
expect(execMock).toHaveBeenCalledWith(
|
||||
'git',
|
||||
['config', '--global', '--add', 'safe.directory', '/github/workspace'],
|
||||
expect.objectContaining({
|
||||
env: expect.objectContaining({HOME: expect.stringContaining('paths-filter-git-home-')})
|
||||
})
|
||||
)
|
||||
expect(execMock).toHaveBeenCalledWith(
|
||||
'git',
|
||||
['config', '--global', '--add', 'safe.directory', process.cwd()],
|
||||
expect.anything()
|
||||
)
|
||||
})
|
||||
|
||||
test('returns false when repeated stderr adds no new directory', async () => {
|
||||
await ensureSafeDirectory(DUBIOUS_STDERR)
|
||||
const callCount = execMock.mock.calls.length
|
||||
|
||||
const added = await ensureSafeDirectory(DUBIOUS_STDERR)
|
||||
|
||||
expect(added).toBe(false)
|
||||
expect(execMock.mock.calls.length).toBe(callCount)
|
||||
})
|
||||
|
||||
test('adds directory reported by a later error for a different path', async () => {
|
||||
await ensureSafeDirectory(DUBIOUS_STDERR)
|
||||
|
||||
const added = await ensureSafeDirectory("fatal: detected dubious ownership in repository at '/other/repo'")
|
||||
|
||||
expect(added).toBe(true)
|
||||
expect(execMock).toHaveBeenCalledWith(
|
||||
'git',
|
||||
['config', '--global', '--add', 'safe.directory', '/other/repo'],
|
||||
expect.anything()
|
||||
)
|
||||
})
|
||||
|
||||
test('cleanup removes the temporary HOME and resets state', async () => {
|
||||
await ensureSafeDirectory(DUBIOUS_STDERR)
|
||||
const tempHome = getGitEnv()['HOME']
|
||||
expect(tempHome).toContain('paths-filter-git-home-')
|
||||
|
||||
await cleanup()
|
||||
|
||||
expect(getGitEnv()['HOME']).not.toContain('paths-filter-git-home-')
|
||||
expect(fs.existsSync(tempHome)).toBe(false)
|
||||
})
|
||||
|
||||
test('getGitEnv mirrors process.env and forces LC_ALL=C before activation', () => {
|
||||
process.env['SOME_PRESERVED_VARIABLE'] = 'preserved'
|
||||
process.env['LC_ALL'] = 'de_DE.UTF-8'
|
||||
|
||||
const env = getGitEnv()
|
||||
|
||||
expect(env['SOME_PRESERVED_VARIABLE']).toBe('preserved')
|
||||
expect(env['HOME']).toBe(runnerTemp)
|
||||
expect(env['LC_ALL']).toBe('C')
|
||||
})
|
||||
|
||||
test('getGitEnv contains the temporary HOME and forces LC_ALL=C after activation', async () => {
|
||||
process.env['LC_ALL'] = 'de_DE.UTF-8'
|
||||
|
||||
await ensureSafeDirectory(DUBIOUS_STDERR)
|
||||
|
||||
const env = getGitEnv()
|
||||
expect(env['HOME']).toContain('paths-filter-git-home-')
|
||||
expect(env['LC_ALL']).toBe('C')
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user