From 174ff883c9c1f6826407346380660b44f881cf8c Mon Sep 17 00:00:00 2001 From: Sascha Bratton Date: Thu, 2 Jul 2026 13:06:01 -0400 Subject: [PATCH] fix: force C locale for git error detection --- .../workflows/pull-request-verification.yml | 31 +++++++++++++++++++ __tests__/safe-directory.test.ts | 31 ++++++++++++++++--- dist/index.js | 26 ++++++++++------ src/safe-directory.ts | 29 ++++++++++------- 4 files changed, 91 insertions(+), 26 deletions(-) diff --git a/.github/workflows/pull-request-verification.yml b/.github/workflows/pull-request-verification.yml index 2084338..00899eb 100644 --- a/.github/workflows/pull-request-verification.yml +++ b/.github/workflows/pull-request-verification.yml @@ -82,17 +82,48 @@ jobs: # pre-2.32 -> ignores GIT_CONFIG_GLOBAL -> exercises the HOME-only path # bookworm: git 2.39, "dubious ownership" wording, honors GIT_CONFIG_GLOBAL container: ['node:24-bullseye', 'node:24-bookworm'] + locale: [''] + include: + # zh_CN: git translates the dubious-ownership message via gettext - proves + # detection works on non-English stderr regardless of the container's locale. + # A CJK locale is the most adversarial probe (multibyte, non-Latin) whose + # catalog actually translates this message (ja does not exist, ko lacks it) + - container: 'node:24-bookworm' + locale: 'zh_CN.UTF-8' container: ${{ matrix.container }} steps: - uses: actions/checkout@v6 + - name: Generate locale + if: matrix.locale != '' + run: | + apt-get update + apt-get install -y locales + echo '${{ matrix.locale }} UTF-8' >> /etc/locale.gen + locale-gen - name: Verify dubious ownership is reproduced run: | if git status; then echo "::error::git succeeded - environment no longer reproduces dubious ownership" exit 1 fi + - name: Verify git message is localized + if: matrix.locale != '' + env: + LC_ALL: ${{ matrix.locale }} + run: | + if stderr=$(git status 2>&1 >/dev/null); then + echo "::error::git succeeded - environment no longer reproduces dubious ownership" + exit 1 + fi + echo "$stderr" + if echo "$stderr" | grep -qE 'dubious ownership|unsafe repository'; then + echo "::error::git message is not translated - the locale variant would not test anything" + exit 1 + fi - uses: ./ id: filter + env: + LC_ALL: ${{ matrix.locale }} with: token: '' filters: '.github/filters.yml' diff --git a/__tests__/safe-directory.test.ts b/__tests__/safe-directory.test.ts index 936776b..caf5bf8 100644 --- a/__tests__/safe-directory.test.ts +++ b/__tests__/safe-directory.test.ts @@ -173,7 +173,7 @@ describe('ensureSafeDirectory', () => { }) test('activates temporary HOME and adds reported directories on first call', async () => { - expect(getGitEnv()).toBeUndefined() + expect(getGitEnv()['HOME']).not.toContain('paths-filter-git-home-') const added = await ensureSafeDirectory(DUBIOUS_STDERR) @@ -224,12 +224,33 @@ describe('ensureSafeDirectory', () => { test('cleanup removes the temporary HOME and resets state', async () => { await ensureSafeDirectory(DUBIOUS_STDERR) - const tempHome = getGitEnv()?.['HOME'] - expect(tempHome).toBeDefined() + const tempHome = getGitEnv()['HOME'] + expect(tempHome).toContain('paths-filter-git-home-') await cleanup() - expect(getGitEnv()).toBeUndefined() - expect(fs.existsSync(tempHome as string)).toBe(false) + 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') }) }) diff --git a/dist/index.js b/dist/index.js index a0bd7cf..a4eba75 100644 --- a/dist/index.js +++ b/dist/index.js @@ -903,10 +903,12 @@ function parseRepositoryPath(stderr) { return (_a = stderr.match(REPOSITORY_PATH_PATTERN)) === null || _a === void 0 ? void 0 : _a[1]; } exports.parseRepositoryPath = parseRepositoryPath; -// Returns undefined until the workaround is activated - git commands of currently -// working users keep inheriting process.env unchanged. +// Until the workaround is activated this mirrors process.env; afterwards it applies the +// temporary HOME redirect. In both cases LC_ALL=C is forced so git emits untranslated +// messages and isDubiousOwnershipError / parseRepositoryPath match regardless of the +// container's locale. function getGitEnv() { - return gitEnv; + return { ...(gitEnv !== null && gitEnv !== void 0 ? gitEnv : cloneDefinedEnv(process.env)), LC_ALL: 'C' }; } exports.getGitEnv = getGitEnv; // Marks directories reported by git as safe, using a temporary HOME so no configuration @@ -923,7 +925,7 @@ async function ensureSafeDirectory(stderr) { let added = false; for (const dir of [parseRepositoryPath(stderr), process.env.GITHUB_WORKSPACE, process.cwd()]) { if (dir && !safeDirectories.has(dir)) { - await (0, exec_1.exec)('git', ['config', '--global', '--add', 'safe.directory', dir], { env: gitEnv }); + await (0, exec_1.exec)('git', ['config', '--global', '--add', 'safe.directory', dir], { env: getGitEnv() }); safeDirectories.add(dir); added = true; } @@ -968,12 +970,7 @@ async function createTempGitHome(baseTempDir, env) { exports.createTempGitHome = createTempGitHome; // Exported for tests function buildGitEnv(tempHome, env) { - const newEnv = {}; - for (const [key, value] of Object.entries(env)) { - if (value !== undefined) { - newEnv[key] = value; - } - } + const newEnv = cloneDefinedEnv(env); // A changed HOME redirects git of any version to the temp config. GIT_CONFIG_GLOBAL is redirected // only when already set - on git >= 2.32 it replaces both global config files, so setting it // unconditionally would hide an existing $XDG_CONFIG_HOME/git/config from git @@ -989,6 +986,15 @@ function resolveTempBaseDir(env) { return env.RUNNER_TEMP || os.tmpdir(); } exports.resolveTempBaseDir = resolveTempBaseDir; +function cloneDefinedEnv(env) { + const newEnv = {}; + for (const [key, value] of Object.entries(env)) { + if (value !== undefined) { + newEnv[key] = value; + } + } + return newEnv; +} async function copyFileIfExists(source, destination) { try { await fs.promises.access(source, fs.constants.R_OK); diff --git a/src/safe-directory.ts b/src/safe-directory.ts index e81921d..bb218e6 100644 --- a/src/safe-directory.ts +++ b/src/safe-directory.ts @@ -22,10 +22,12 @@ export function parseRepositoryPath(stderr: string): string | undefined { return stderr.match(REPOSITORY_PATH_PATTERN)?.[1] } -// Returns undefined until the workaround is activated - git commands of currently -// working users keep inheriting process.env unchanged. -export function getGitEnv(): {[key: string]: string} | undefined { - return gitEnv +// Until the workaround is activated this mirrors process.env; afterwards it applies the +// temporary HOME redirect. In both cases LC_ALL=C is forced so git emits untranslated +// messages and isDubiousOwnershipError / parseRepositoryPath match regardless of the +// container's locale. +export function getGitEnv(): {[key: string]: string} { + return {...(gitEnv ?? cloneDefinedEnv(process.env)), LC_ALL: 'C'} } // Marks directories reported by git as safe, using a temporary HOME so no configuration @@ -45,7 +47,7 @@ export async function ensureSafeDirectory(stderr: string): Promise { let added = false for (const dir of [parseRepositoryPath(stderr), process.env.GITHUB_WORKSPACE, process.cwd()]) { if (dir && !safeDirectories.has(dir)) { - await exec('git', ['config', '--global', '--add', 'safe.directory', dir], {env: gitEnv}) + await exec('git', ['config', '--global', '--add', 'safe.directory', dir], {env: getGitEnv()}) safeDirectories.add(dir) added = true } @@ -96,12 +98,7 @@ export async function createTempGitHome( // Exported for tests export function buildGitEnv(tempHome: string, env: {[key: string]: string | undefined}): {[key: string]: string} { - const newEnv: {[key: string]: string} = {} - for (const [key, value] of Object.entries(env)) { - if (value !== undefined) { - newEnv[key] = value - } - } + const newEnv = cloneDefinedEnv(env) // A changed HOME redirects git of any version to the temp config. GIT_CONFIG_GLOBAL is redirected // only when already set - on git >= 2.32 it replaces both global config files, so setting it // unconditionally would hide an existing $XDG_CONFIG_HOME/git/config from git @@ -117,6 +114,16 @@ export function resolveTempBaseDir(env: {[key: string]: string | undefined}): st return env.RUNNER_TEMP || os.tmpdir() } +function cloneDefinedEnv(env: {[key: string]: string | undefined}): {[key: string]: string} { + const newEnv: {[key: string]: string} = {} + for (const [key, value] of Object.entries(env)) { + if (value !== undefined) { + newEnv[key] = value + } + } + return newEnv +} + async function copyFileIfExists(source: string, destination: string): Promise { try { await fs.promises.access(source, fs.constants.R_OK)