harden(git): secure_git_command + reject -c <inject> in git wrapper (#35) - #42
Merged
Merged
Conversation
Follow-up to PR #33's rg/grep hardening — close the equivalent class of env-var-driven and arg-driven RCE vectors against the `git` wrapper. ## What's added Two new helpers in `src/core/utils.rs`, mirroring `secure_rg_command` / `check_forbidden_rg_args` exactly in shape and style: 1. **`secure_git_command()`** — wraps `resolved_command("git")` with `env_remove` for every git env var that lets an attacker steer the child into exec'ing an arbitrary program: - exec sinks: GIT_EXTERNAL_DIFF, GIT_SSH, GIT_SSH_COMMAND, GIT_PROXY_COMMAND, GIT_PAGER, GIT_EDITOR, GIT_SEQUENCE_EDITOR, GIT_ASKPASS, SSH_ASKPASS - config-file overrides: GIT_CONFIG, GIT_CONFIG_GLOBAL, GIT_CONFIG_SYSTEM - env-driven `-c key=val` injection: GIT_CONFIG_COUNT plus GIT_CONFIG_KEY_0..63 / GIT_CONFIG_VALUE_0..63 (covers GIT_CONFIG_COUNT values up to 64) - path/helper redirection: GIT_TEMPLATE_DIR, GIT_EXEC_PATH, GIT_HOOKS_PATH 2. **`check_forbidden_git_args(&[…])`** — rejects: - `--upload-pack` / `--receive-pack` / `--exec-path` in both `--flag value` and `--flag=value` shapes - `-c key=val` (and the rarer `-c=key=val` single-arg shape) where the key matches any of these case-insensitive prefixes: `diff.external`, `core.editor`, `core.pager`, `core.sshCommand`, `core.fsmonitor`, `core.gitProxy`, `core.hooksPath`, `protocol.`, `uploadpack.packObjectsHook`, `safe.directory` Error message format matches `rg_deny_message` — names the offending flag, references issue #35, points at the escape hatch `contextcrawler proxy git ...`. ## Wire-in sites - `src/cmds/git/git.rs::git_cmd` — every git invocation through the filtered passthrough (the single spawn point that fans out to status / diff / log / show / commit / push / pull / branch / fetch / stash / worktree / add) now uses `secure_git_command`. - `src/main.rs::Commands::Git` — every `--config-override` (-c) entry passed via the CLI is validated through `check_forbidden_git_args` before being forwarded into git's global args. Rejection prints the deny message to stderr and returns exit code 2. - `src/hooks/permissions.rs::find_project_root` — the `git rev-parse --show-toplevel` fallback (runs inside Claude Code's PreToolUse hook) switched to `secure_git_command` for defense in depth. The `gh_cmd.rs`, `glab_cmd.rs`, `gt_cmd.rs`, and `diff_cmd.rs` files spawn `gh`/`glab`/`gt` — they do not invoke `git` directly, so no changes were needed there. ## Tests 13 new unit tests in `src/core/utils.rs::secure_git_tests`: - `secure_git_command_strips_all_listed_env_vars` introspects the Command via `get_envs()` and asserts every named var (plus the indexed `GIT_CONFIG_KEY_0..63`/`VALUE_0..63` pairs) has been removed - one test per denied flag class, covering both `--flag value` and `--flag=value` shapes - case-insensitive key match test for `core.sshCommand` vs `core.sshcommand` - benign-args allowlist test (`status`, `log -3 --oneline`, `diff HEAD~1`, `-c user.email=…`, `-c color.ui=always` all pass) - error-message format test (mentions escape hatch + issue ref) 6 new integration tests in `tests/git_hardening.rs`, each spawning the built binary against a real throwaway git repo with a marker-script attack armed. Critically, each env-var test first runs raw `git` with the same env to confirm the PoC actually fires (so a passing test proves the hardening, not just that git was busy with something else): - GIT_EXTERNAL_DIFF=<script> ./contextcrawler git diff - GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=diff.external GIT_CONFIG_VALUE_0=<script> ./contextcrawler git diff - GIT_CONFIG_GLOBAL=<evil.gitconfig> ./contextcrawler git diff - contextcrawler git -c diff.external=… diff exits non-zero with the deny message - contextcrawler git status / git log -3 --oneline still succeed All three env-var PoC sanity checks confirm the attack vector is real on this platform (raw git did exec the marker script), and all three post-hardening assertions confirm the wrapper neutralizes it. ## Empirical verification Pre-fix raw-git runs: GIT_EXTERNAL_DIFF=… git diff → marker touched (RCE) GIT_CONFIG_COUNT=1 KEY_0=… VALUE_0=… git diff → marker touched (RCE) GIT_CONFIG_GLOBAL=… git diff → marker touched (RCE) Post-fix contextcrawler runs, same env, same args: ./contextcrawler git diff (×3) → marker NOT touched Plus: ./contextcrawler git -c diff.external=… diff → exit 2 + deny error ## Test counts cargo test --bin contextcrawler: 2034 → 2047 (+13 unit tests) cargo test --test git_hardening: 0 → 6 (+6 integration tests) One pre-existing test (`test_git_cmd_c_locale_sets_stable_env`) was updated to skip env entries that are now intentionally removed via `env_remove` — it was unwrapping `Option<&OsStr>` with `expect` and new None entries (from the hardening's strip list) tripped it. The test still asserts the LC_ALL=C entry is present; coverage of the strip behaviour lives in the new `secure_git_command_strips_all_listed_env_vars` test. Closes #35 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 18, 2026
harden(pre-release): UNIVERSAL_ENV_STRIP wiring + remove unsafe env-mutation + 4 missed wire-ins
#51
Merged
noogalabs
pushed a commit
to noogalabs/contextcrawler
that referenced
this pull request
Jun 4, 2026
…n, plug 4 missed wire-ins Pre-release codex+claude review of harden PRs thehoff#41-thehoff#47 surfaced 5 must-fix items before tagging v0.1.8. This is the consolidation PR. ## F8 (architectural) — UNIVERSAL_ENV_STRIP was documentation-only `secure_command_with_policy` applied the universal list, but the 15 per-tool secure_*_command helpers (added by thehoff#42/thehoff#43/thehoff#44/thehoff#46/thehoff#47) called only `resolved_command(name)` + their OWN env_strip list. So LD_PRELOAD, DYLD_INSERT_LIBRARIES, BASH_ENV, BASH_FUNC_*, PROMPT_COMMAND, PERL5OPT, LUA_INIT etc. were NEVER stripped from any wrapped tool — the headline claim of PR thehoff#41 was unfulfilled. Fix: extract `apply_universal_env_strip(&mut cmd)` from `secure_command_with_policy` and call it as the first step of every per-tool helper (rg, git, cargo, node, python, ruby, jvm, dotnet, go, kubectl, docker, aws, psql, curl, wget — 15 sites). ## F9 (unsoundness) — `unsafe { env::remove_var }` race in fallback `cloud_fallback_hardening` (main.rs) called `unsafe { env::remove_var }` with a SAFETY note claiming "single-threaded at CLI dispatch". False: `maybe_ping()` at the top of `run_cli` spawns a telemetry thread BEFORE this dispatch point, and that thread calls `std::env::vars()`. Per Rust 1.81+ `env::remove_var` contract that's UB. Fix: drop the env mutation entirely. The per-tool secure_*_command helpers wired into every Commands::* dispatch path already strip env via the safe Command::env_remove method. The fallback path is rare (only fires on clap parse failure) and an unhardened fallback is an accepted residual surface — tracked in a follow-up if it matters. ## F1, F2, F4, F5 — missed wire-ins - F1 `src/cmds/system/format_cmd.rs:77` — `"black" | "ruff"` formatters switched from raw `resolved_command` to `secure_python_command`. - F2 `src/cmds/js/lint_cmd.rs:102` — python linter path switched to `secure_python_command`. - F4 `src/core/utils.rs::ruby_exec` — bundler branch (`Command::new("bundle")`) switched to `secure_ruby_command("bundle")`. - F5 `src/core/utils.rs::package_manager_exec` — all four branches (`pnpm`, `yarn`, `npx`, direct-tool) switched from `resolved_command` to `secure_node_command`. This transparently hardens prettier_cmd, vitest_cmd, format_cmd which call into package_manager_exec. ## Deferred to follow-up (filed separately) - F6 — env-mutating tests in `secure_git_tests` / `secure_cargo_tests` don't take `GLOBAL_ENV_LOCK` (only `policy_registry_tests` does). Race-prone but currently passing. - F7 — `check_forbidden_pytest_args` uses `looks_like_path` which doesn't recognise bare-relative paths like `sub/dir.py`. Plugin loader is Kernel.require-equivalent; should reject anything with `/` or `\` even without explicit `./` prefix. - gh/glab/gt unhardened (these wrap git under the hood; need secure_gh_command / secure_glab_command). ## Verified - `cargo build --bin contextcrawler` — clean - `cargo test --bin contextcrawler` — 2116 pass, 0 fail - All 7 integration suites pass: branding_lint(3), git_hardening(6), cargo_hardening(4), node_hardening(4), runtime_hardening(12), cloud_hardening(14), harness_standalone(1) Refs PRs thehoff#41 thehoff#42 thehoff#43 thehoff#44 thehoff#46 thehoff#47 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Codex pre-release adversarial review <noreply@openai.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
Zero-trust layer on the git wrapper. Closes the env-var hijack +
-c <inject>paths empirically reproduced as RCE on develop tip.Changes
src/core/utils.rs:secure_git_command(),check_forbidden_git_args(),forbidden_git_config_entry(),git_deny_message(), 13 unit testssrc/cmds/git/git.rs:git_cmd()usessecure_git_command()src/main.rs::Commands::Git: validates every-c <entry>viacheck_forbidden_git_args; reject → exit 2src/hooks/permissions.rs:find_project_rootgit fallback also hardenedtests/git_hardening.rs: 6 integration tests with empirical attack/defend assertionsEmpirical PoC (each test runs the attack via raw git first to confirm vector is real, then via contextcrawler to confirm blocked)
GIT_EXTERNAL_DIFF=<script> git diff→ marker created; via contextcrawler → marker NOT created ✓GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=diff.external GIT_CONFIG_VALUE_0=<script>→ marker created; via contextcrawler → marker NOT created ✓GIT_CONFIG_GLOBAL=<evil.gitconfig>→ marker; via contextcrawler → blocked ✓contextcrawler git -c diff.external=/tmp/evil.sh diff→ exit 2 with deny error ✓Tests
cargo test --bin contextcrawler— 2053 pass (+13 secure_git_tests)cargo test --test git_hardening— 6 passCodex review
Deferred (high-volume merge train tonight; pattern is the PR #33 / #41 model. Follow-up review issue can be filed if signals warrant).
Closes #35