Strip exec-class keys from sanitized git config#128
Merged
Conversation
Picks up the security hardening series from https://github.com/stacklok/go-microvm/releases/tag/v0.0.32 — symlink-safe rootfs hooks, whiteout path re-validation during OCI layer apply, DNS answer filtering by CNAME bailiwick, TTL clamp on dynamic egress rules, relay frame-length cap, non-IPv4 drop under deny-default, hosted-service HTTP timeouts, stale-runner identity guard, symmetric provider wiring, and per-file integrity manifest on the extract cache. No brood-box code changes required — all hardening is wired through interfaces brood-box already uses. Verified end-to-end: VM boots, hooks run, workspace round-trip flushes, DNS egress allows api.anthropic.com and denies unknown hosts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SanitizeConfig preserved entire [core], [merge], [diff], [submodule],
and [lfs] sections once the section name was on the allowlist. This
let a malicious host .git/config smuggle command-execution keys into
the snapshot, where git (or git-lfs) would honor them on the next
operation inside the VM.
Fixes three distinct gaps:
1. Section-scoped denylist for exec-class keys: core.sshCommand,
core.pager, core.editor, core.fsmonitor, core.alternateRefsCommand,
core.hooksPath, merge.<name>.driver, diff.external,
diff.<name>.{driver,textconv,command}, and submodule.<name>.update
(CVE-2017-1000117). Applied in filterLine before section-specific
handling; matched case-insensitively.
2. Drop [lfs] from allowedSections. The [lfs "customtransfer.<name>"]
form carries a `path` key that git-lfs executes, and
parseSectionName collapses subsections so a per-key denylist cannot
distinguish lfs.customtransfer.*.path from a hypothetical benign
lfs.path. LFS configuration typically lives in ~/.gitconfig so
stripping the section from the repo-local snapshot config does not
affect real LFS workflows.
3. Drop continuation lines belonging to denied keys. A denied key with
a trailing backslash (`sshCommand = ssh \`) previously suppressed
the key line but still emitted attacker-controlled continuation
lines as orphan fragments under the allowed section. Track a
swallowContinuation flag alongside inContinuation to gate the
continuation branch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SanitizeConfigpreserved entire[core],[merge],[diff],[submodule], and[lfs]sections once the section name was on the allowlist. A malicious host.git/configcould therefore smuggle command-execution keys through the sanitizer into the VM snapshot, wheregit(orgit-lfs) would honor them on the next operation inside the guest — giving RCE on content an agent might fetch viagit clone/git fetchinside the VM.Three distinct gaps fixed, each with dedicated tests:
Section-scoped denylist for exec-class keys:
core.sshCommand,core.pager,core.editor,core.fsmonitor,core.alternateRefsCommand,core.hooksPath,merge.<name>.driver,diff.external,diff.<name>.{driver,textconv,command}, andsubmodule.<name>.update(CVE-2017-1000117). Applied infilterLinebefore section-specific handling; matched case-insensitively.[lfs]dropped fromallowedSections.[lfs \"customtransfer.<name>\"]carries apathkey that git-lfs executes, andparseSectionNamecollapses subsections so a per-key denylist cannot distinguishlfs.customtransfer.*.pathfrom a hypothetical benignlfs.path. LFS configuration typically lives in~/.gitconfig, so stripping the section from the repo-local snapshot does not affect real LFS workflows.Continuation lines belonging to denied keys are dropped. A denied key with a trailing backslash (
sshCommand = ssh \\) previously suppressed the key line but still emitted the attacker-controlled continuation lines as orphan fragments under the allowed section. Track aswallowContinuationflag alongsideinContinuation.Test plan
task fmt && task lint && task test— all greenTestSanitizeConfigcovers: each exec-class key per section, case-insensitive matching, subsection form ([merge \"driver1\"],[diff \"pdf\"],[submodule \"lib\"]), LFS customtransfer stripping, continuation-after-denied-key dropping, and continuation reset after a subsequent kept linecore.*keys (autocrlf,quotepath,whitespace, etc.) still pass throughbbox claude-code --no-mcp --exec /bin/sh -- -c 'git config --list'inside a VM shows only safe keys; nocore.sshCommand/core.pager/[lfs]/merge.driverin the output🤖 Generated with Claude Code