From d7f72534fd598208e39a42bef654a7ed9b4ceec2 Mon Sep 17 00:00:00 2001 From: James Ross Date: Tue, 4 Aug 2026 12:12:39 -0700 Subject: [PATCH] fix: let git read the operator's own configuration EnvironmentPolicy filtered the environment down to an allowlist before spawning git, and that allowlist contained no way for git to find the operator's configuration. Without HOME, git cannot read ~/.gitconfig at all. It does not fail when that happens. It invents an identity from the system account and the hostname, so commits are written as addresses like user@laptop.local that exist nowhere, verify against nothing, and silently replace the identity the operator actually configured. A caller has no indication their configuration was never consulted. HOME, XDG_CONFIG_HOME, GIT_CONFIG_GLOBAL and USERPROFILE now pass through: the four paths git uses to locate user configuration, including the Windows home directory. The policy's real boundary is preserved. Letting git *find* configuration the operator owns is ordinary git behaviour; letting a caller *inject* configuration is not. GIT_CONFIG_PARAMETERS, GIT_EXEC_PATH and GIT_TEMPLATE_DIR stay blocked, and a test asserts they stay blocked even when HOME is present. Verified in Docker per the repository's guard: 203/203 across 26 files. The four new assertions failed before the change. Beyond the unit tests, test/UserGitConfig.test.js drives a real commit-tree with a configured ~/.gitconfig and asserts the resulting author is the configured identity rather than an invented one. Callers relying on git being unable to see user configuration will observe different behaviour; GIT_AUTHOR_* and GIT_COMMITTER_* still take precedence and remain the way to pin an identity explicitly. --- CHANGELOG.md | 21 +++++ src/domain/services/EnvironmentPolicy.js | 28 +++++-- test/UserGitConfig.test.js | 79 +++++++++++++++++++ .../domain/services/EnvironmentPolicy.test.js | 40 ++++++++++ 4 files changed, 163 insertions(+), 5 deletions(-) create mode 100644 test/UserGitConfig.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index fb7e72b..b0d5eae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,27 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- **User Git Configuration Reaches Git**: `EnvironmentPolicy` filtered out every + variable git uses to locate the operator's configuration, so the spawned + process could not read `~/.gitconfig`. Git fell back to inventing an identity + from the system account and hostname, and commits were attributed to addresses + such as `user@laptop.local` that exist nowhere and verify against nothing, + while the operator's configured identity sat unread on disk. `HOME`, + `XDG_CONFIG_HOME`, `GIT_CONFIG_GLOBAL` and `USERPROFILE` now pass through. + + Locating configuration is not the same as injecting it: `GIT_CONFIG_PARAMETERS`, + `GIT_EXEC_PATH` and `GIT_TEMPLATE_DIR` remain blocked, so a caller still cannot + push settings into the process directly. + + Callers that relied on git being unable to see user configuration — expecting a + fixed synthetic author, or an environment where `core.*` settings could not + apply — will observe different behavior and should pass `GIT_AUTHOR_*` / + `GIT_COMMITTER_*` explicitly, which continue to take precedence. + ## [3.2.0] - 2026-07-19 ### Added diff --git a/src/domain/services/EnvironmentPolicy.js b/src/domain/services/EnvironmentPolicy.js index fa41767..dc64941 100644 --- a/src/domain/services/EnvironmentPolicy.js +++ b/src/domain/services/EnvironmentPolicy.js @@ -3,16 +3,26 @@ */ /** - * EnvironmentPolicy defines which environment variables are safe to pass + * EnvironmentPolicy defines which environment variables are safe to pass * to the underlying Git process. - * - * It whitelists essential variables for identity and localization while - * explicitly blocking variables that could override security settings. + * + * It whitelists essential variables for identity, configuration discovery and + * localization while explicitly blocking variables that could override security + * settings. + * + * The distinction it draws is between letting git *find* the operator's + * configuration and letting a caller *inject* configuration. The first is + * ordinary git behaviour: without it git cannot read ~/.gitconfig, invents an + * identity from the system account and hostname, and writes commits attributed + * to an address that exists nowhere and verifies against nothing. The second + * stays blocked, because GIT_CONFIG_PARAMETERS and friends push settings into + * the process directly rather than pointing at a file the operator owns. */ export default class EnvironmentPolicy { /** * List of environment variables allowed to be passed to the git process. - * Whitelists identity (GIT_AUTHOR_*, GIT_COMMITTER_*) and localization (LANG, LC_ALL). + * Whitelists identity (GIT_AUTHOR_*, GIT_COMMITTER_*), the paths git uses to + * locate user configuration, and localization (LANG, LC_ALL). * @private */ static _ALLOWED_KEYS = [ @@ -28,6 +38,14 @@ export default class EnvironmentPolicy { 'GIT_COMMITTER_EMAIL', 'GIT_COMMITTER_DATE', 'GIT_COMMITTER_TZ', + // Configuration discovery: where git looks for the operator's own config. + // HOME finds ~/.gitconfig, XDG_CONFIG_HOME finds ~/.config/git/config, + // GIT_CONFIG_GLOBAL names the file outright, and USERPROFILE is how a home + // directory is resolved on Windows. + 'HOME', + 'XDG_CONFIG_HOME', + 'GIT_CONFIG_GLOBAL', + 'USERPROFILE', // Localization & Encoding 'LANG', 'LC_ALL', diff --git a/test/UserGitConfig.test.js b/test/UserGitConfig.test.js new file mode 100644 index 0000000..1bf35ff --- /dev/null +++ b/test/UserGitConfig.test.js @@ -0,0 +1,79 @@ +import GitPlumbing from '../index.js'; +import path from 'node:path'; +import fs from 'node:fs'; +import os from 'node:os'; + +/** + * A caller that shells out through this library should see the same identity + * git itself would use. + * + * The runner sanitizes the environment before spawning git. When that filter + * drops every variable git uses to locate the user's configuration, git cannot + * read ~/.gitconfig at all and manufactures an identity from the system account + * and hostname instead. The result is a commit attributed to an address like + * user@laptop.local, which exists nowhere, verifies against nothing, and quietly + * replaces the identity the operator actually configured. + * + * This drives a real commit with the configuration supplied the ordinary way and + * asserts the configured identity survives the filter. + */ +describe('User git configuration', () => { + let repoPath; + let homePath; + let originalHome; + let originalConfigGlobal; + + const CONFIGURED_NAME = 'Configured Operator'; + const CONFIGURED_EMAIL = 'operator@example.com'; + + beforeAll(() => { + const stamp = Math.random().toString(36).substring(7); + repoPath = path.join(os.tmpdir(), `git-plumbing-userconfig-repo-${stamp}`); + homePath = path.join(os.tmpdir(), `git-plumbing-userconfig-home-${stamp}`); + fs.mkdirSync(repoPath, { recursive: true }); + fs.mkdirSync(homePath, { recursive: true }); + + fs.writeFileSync( + path.join(homePath, '.gitconfig'), + `[user]\n\tname = ${CONFIGURED_NAME}\n\temail = ${CONFIGURED_EMAIL}\n` + ); + + originalHome = process.env.HOME; + originalConfigGlobal = process.env.GIT_CONFIG_GLOBAL; + process.env.HOME = homePath; + delete process.env.GIT_CONFIG_GLOBAL; + }); + + afterAll(() => { + if (originalHome === undefined) { + delete process.env.HOME; + } else { + process.env.HOME = originalHome; + } + + if (originalConfigGlobal === undefined) { + delete process.env.GIT_CONFIG_GLOBAL; + } else { + process.env.GIT_CONFIG_GLOBAL = originalConfigGlobal; + } + + fs.rmSync(repoPath, { recursive: true, force: true }); + fs.rmSync(homePath, { recursive: true, force: true }); + }); + + it('commits as the identity the operator configured', async () => { + const git = await GitPlumbing.createDefault({ cwd: repoPath }); + await git.execute({ args: ['init'] }); + + const tree = await git.execute({ args: ['write-tree'] }); + const commit = await git.execute({ + args: ['commit-tree', tree.trim(), '-m', 'configured identity'] + }); + + const author = await git.execute({ + args: ['log', '-1', '--format=%an <%ae>', commit.trim()] + }); + + expect(author.trim()).toBe(`${CONFIGURED_NAME} <${CONFIGURED_EMAIL}>`); + }); +}); diff --git a/test/domain/services/EnvironmentPolicy.test.js b/test/domain/services/EnvironmentPolicy.test.js index a3a99e4..7075471 100644 --- a/test/domain/services/EnvironmentPolicy.test.js +++ b/test/domain/services/EnvironmentPolicy.test.js @@ -54,4 +54,44 @@ describe('EnvironmentPolicy', () => { expect(EnvironmentPolicy.filter({})).toEqual({}); expect(EnvironmentPolicy.filter(undefined)).toEqual({}); }); + + // Without a way to locate the user's configuration, git cannot read + // ~/.gitconfig and falls back to inventing an identity from the system + // account and hostname. Callers then see commits attributed to addresses + // such as user@laptop.local that exist nowhere and verify against nothing, + // while the operator's real, configured identity sits unread on disk. + it('lets git locate the user configuration', () => { + const env = { + HOME: '/home/operator', + XDG_CONFIG_HOME: '/home/operator/.config', + GIT_CONFIG_GLOBAL: '/home/operator/.gitconfig' + }; + + const filtered = EnvironmentPolicy.filter(env); + + expect(filtered).toEqual(env); + }); + + it('passes USERPROFILE so Windows callers resolve a home directory too', () => { + const filtered = EnvironmentPolicy.filter({ USERPROFILE: 'C:\\Users\\operator' }); + + expect(filtered.USERPROFILE).toBe('C:\\Users\\operator'); + }); + + // Reading configuration is not the same as accepting arbitrary overrides. + // GIT_CONFIG_PARAMETERS injects settings directly into the process and stays + // blocked, so a caller cannot smuggle configuration past the policy. + it('still refuses direct configuration injection', () => { + const filtered = EnvironmentPolicy.filter({ + HOME: '/home/operator', + GIT_CONFIG_PARAMETERS: "'user.name=attacker'", + GIT_EXEC_PATH: '/tmp/evil', + GIT_TEMPLATE_DIR: '/tmp/evil' + }); + + expect(filtered.HOME).toBe('/home/operator'); + expect(filtered.GIT_CONFIG_PARAMETERS).toBeUndefined(); + expect(filtered.GIT_EXEC_PATH).toBeUndefined(); + expect(filtered.GIT_TEMPLATE_DIR).toBeUndefined(); + }); }); \ No newline at end of file