Repository navigation
Let git read the operator's own configuration - #13
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used🪛 ast-grep (0.45.0)test/UserGitConfig.test.js[warning] 35-38: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use. (detect-non-literal-fs-filename) 🔇 Additional comments (4)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesGit configuration passthrough
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant UserGitConfigTest
participant EnvironmentPolicy
participant spawnedGit as spawned Git
participant userGitConfig as user .gitconfig
UserGitConfigTest->>EnvironmentPolicy: filter configuration discovery variables
EnvironmentPolicy->>spawnedGit: pass filtered environment
spawnedGit->>userGitConfig: discover user identity
spawnedGit-->>UserGitConfigTest: create commit with configured author
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The bug
EnvironmentPolicyfilters the environment down to an allowlist before spawning git. That allowlist contained no way for git to find the operator's configuration — noHOME, noGIT_CONFIG_GLOBAL, noXDG_CONFIG_HOME.Without
HOME, git cannot read~/.gitconfigat all. It does not fail when that happens — it invents an identity from the system account and hostname:That address exists nowhere and verifies against nothing. On a forge it reports as unverified with reason
no_user, so a branch rule requiring signed commits treats every such commit as unsigned. The caller gets no indication their configuration was never consulted.Found downstream in
think, where every memory commit was misattributed this way. The workaround there had been to writeuser.name/user.emailinto the target repository — which then rewrote the committer identity of whatever directory it was pointed at, including a developer's own source checkout.The fix
HOME,XDG_CONFIG_HOME,GIT_CONFIG_GLOBALandUSERPROFILEnow pass through — the four paths git uses to locate user configuration, including the Windows home directory.The boundary is preserved
The policy's stated purpose is blocking variables that override security settings, and that still holds. Letting git find configuration the operator owns is ordinary git behaviour; letting a caller inject configuration is not:
GIT_CONFIG_PARAMETERSGIT_EXEC_PATHGIT_TEMPLATE_DIRA test asserts all three stay blocked even when
HOMEis present, so the new keys cannot be used as a route around them.Verification
Run through the repository's own harness (
docker-guardrefuses to run tests on the host), all three runtimes:The four new assertions failed before the change (
4 failed | 199 passed).Beyond the unit tests,
test/UserGitConfig.test.jsdrives a realcommit-treeagainst a temporaryHOMEcontaining a known~/.gitconfigand asserts the resulting author is the configured identity rather than an invented one.End-to-end through a consumer: with this patch applied to
think's copy andthink's own identity workaround removed, captures commit asJames Ross <james@flyingrobots.dev>, matching the global config exactly. Unpatched, the same code producesjames@Jamess-MacBook-Pro-2.local.Behaviour change
Callers relying 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 behaviour.GIT_AUTHOR_*/GIT_COMMITTER_*still take precedence and remain the way to pin an identity explicitly.Worth a maintainer decision on version bump: it is a fix, but it changes what git sees. Minor seems right given the allowlist is internal; major is defensible.