Skip to content

fix(security): scrub GIT_CONFIG_COUNT and secrets from checkpoint git env - #70332

Open
zapabob wants to merge 1 commit into
NousResearch:mainfrom
zapabob:fix/checkpoint-git-env-scrub
Open

fix(security): scrub GIT_CONFIG_COUNT and secrets from checkpoint git env#70332
zapabob wants to merge 1 commit into
NousResearch:mainfrom
zapabob:fix/checkpoint-git-env-scrub

Conversation

@zapabob

@zapabob zapabob commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Salvage: fix(checkpoints): isolate shadow git repo from user's global config (GPG pinentry popups) #11261 / existing checkpoint isolation nulls GIT_CONFIG_GLOBAL/SYSTEM, but Git still honours GIT_CONFIG_COUNT / GIT_CONFIG_KEY_* / GIT_CONFIG_VALUE_* from the parent environment, allowing config injection (e.g. core.sshCommand) during background snapshots.
  • Also strip GIT_SSH_COMMAND / askpass / proxy wrappers and build the env via hermes_subprocess_env(inherit_credentials=False) so provider API keys are not visible to git credential helpers.
  • Init path uses the same scrubbing.

Test plan

  • pytest tests/tools/test_checkpoint_manager.py::TestGpgAndGlobalConfigIsolation (5 passed)

@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/tools Tool registry, model_tools, toolsets area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have needs-repro Bug needs reproduction steps sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 23, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for hardening the checkpoint Git environment. The premise is still present on current main: tools/checkpoint_manager.py:264 and :449 use build_subprocess_env(scrub_secrets=False, ...), preserving inherited Git configuration and provider credentials.

Problems

  • The proposed cleanup omits GIT_CONFIG_PARAMETERS. Git honors it as configuration: GIT_CONFIG_PARAMETERS="'core.pager=cat'" git config --get core.pager returned cat during review. This leaves an environment configuration-injection path after the proposed GIT_CONFIG_COUNT / key/value cleanup.
  • tests/tools/test_checkpoint_manager.py adds dictionary assertions only. It does not execute a real Git child, despite this being a security-boundary change.

Suggested changes

  • Rebase the implementation concept onto the current build_subprocess_env call sites introduced by 3d48f893da, with secret scrubbing enabled and explicit removal of GIT_CONFIG_PARAMETERS alongside the count/key/value variables in both _git_env and _init_store.
  • Add a real Git-path test for all supported inherited configuration mechanisms.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 30, 2026
@andrexibiza

Copy link
Copy Markdown
Contributor

Bound to the child-process credential-inheritance class under #83565 (#83565) — same bug class, different surface. checkpoint git env (GIT_CONFIG_COUNT + secrets); Wave F — independent surface. The EPIC carries the live class table, dedup adjudication, and the dependency-driven merge order.

@zapabob
zapabob force-pushed the fix/checkpoint-git-env-scrub branch from 17a0d1e to 3db9c54 Compare August 22, 2026 13:31
@zapabob

zapabob commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Rebased the checkpoint Git environment fix onto frozen official-main anchor 8e475ed27b1199b8d0bbf094cf2e15fcd555f8cf as 3db9c54c13dede743b4ebedea7aa61d510dcf2f8. The current build_subprocess_env call sites in both _git_env and _init_store now use secret scrubbing and explicitly remove GIT_CONFIG_PARAMETERS, GIT_CONFIG_COUNT, indexed config keys/values, SSH/askpass/proxy wrappers, and diff helpers.

Added a real Git child regression: an inherited GIT_CONFIG_PARAMETERS / count-key-value injection cannot make git config --get core.pager succeed. Added direct assertions for credential wrappers and provider-secret removal.

Validation: the two new focused tests pass (2 passed, 47 deselected); explicit .venv/Scripts/python.exe -m py_compile, git diff --check, and fixed-anchor ancestry pass. The existing test_env_pins_store_worktree_and_ignores_ambient_git_state still fails on the fixed Windows baseline because it expects POSIX slash spelling for a native Windows path; that failure reproduces before this patch and was not changed. GitHub checks are pending for the new head.

@alt-glitch alt-glitch removed sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 22, 2026
@zapabob
zapabob force-pushed the fix/checkpoint-git-env-scrub branch from 3db9c54 to d5d08b5 Compare August 23, 2026 01:34
@zapabob

zapabob commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto the current official main fixed at 7a54ab2; new head d5d08b5.

The checkpoint Git child environment still removes Git config injection variables, credential wrappers, and provider secrets. Local evidence after the replay: focused regressions 2 passed (47 deselected), Python compile passed, and git diff --check passed. Required GitHub checks are running for the new SHA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/tools Tool registry, model_tools, toolsets needs-repro Bug needs reproduction steps P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants