Skip to content

fix(mcp): stop tripping simple-git block-unsafe guard on every write#80

Merged
ABB65 merged 1 commit into
mainfrom
fix/mcp-block-unsafe-guard
Jul 13, 2026
Merged

fix(mcp): stop tripping simple-git block-unsafe guard on every write#80
ABB65 merged 1 commit into
mainfrom
fix/mcp-block-unsafe-guard

Conversation

@ABB65

@ABB65 ABB65 commented Jul 13, 2026

Copy link
Copy Markdown
Member

Problem

Every worktree write (content_save, model_save, merges) started failing on @contentrain/mcp with:

Use of "EDITOR" is not permitted without enabling allowUnsafeEditor

(or GIT_ASKPASS, PAGER, …) in any host that exports those variables — VS Code, Claude Code, CI. No Contentrain code changed: a transitive bump pulled simple-git to >= 3.34, which now bundles @simple-git/[email protected]. Its block-unsafe guard rejects a git invocation when a guard-listed variable (18 of them) is passed explicitly through .env(). The guard only scans the object handed to .env() — never the inherited process environment.

The transaction layer spread ...process.env into .env() to set the commit author, so the guard saw the host's EDITOR/GIT_ASKPASS and refused to run git commit.

Fix

Centralized in a new git/identity.ts so no future call site can reintroduce a process.env spread:

  • Commit identity → -c user.name/-c user.email config (authorConfig) instead of .env(). These keys are on no unsafe list and git honours them for author + committer alike, so the guard is never touched, regardless of what the host exports. CONTENTRAIN_AUTHOR_NAME/EMAIL overrides preserved.
  • networkGit (push/fetch) legitimately needs the inherited askpass/SSH/proxy environment to authenticate, so it keeps .env() but opts out of the affected guard categories via unsafe (NETWORK_UNSAFE), leaving arg-injection protections intact. This closes the same latent failure in contentrain_submit that the initial diagnosis missed (third .env() spread).
  • simple-git pinned to ^3.36.0 so the dev tree matches the runtime and reproduces the guard.

Three affected call sites

Site Before After
transaction.ts createTransaction .env(authorEnv()) { config: authorConfig() }
transaction.ts mergeBranch .env(authorEnv()) { config: authorConfig() }
branch-lifecycle.ts networkGit .env({ ...process.env }) + unsafe: NETWORK_UNSAFE

Verification

  • tests/git/identity.test.ts (5, new) — commits via config while EDITOR/GIT_ASKPASS are set (guard not tripped) + author/committer correct; a control asserts the old .env(process.env) path still trips the guard; networkGit-style instance runs via NETWORK_UNSAFE with a matching control.
  • Existing tests/git/ suites (49) green — transaction.test.ts (commit/merge/auto-merge/selectiveSync), branch-lifecycle + remote-branches (networkGit).
  • tsc --noEmit 0 errors · oxlint 0 warnings.

🤖 Generated with Claude Code

simple-git >= 3.34 bundles @simple-git/argv-parser, whose block-unsafe
guard rejects a git invocation when a guard-listed variable (EDITOR,
GIT_ASKPASS, PAGER, GIT_SSH_COMMAND, ...) is passed EXPLICITLY through
.env(). The transaction layer spread ...process.env into .env() to set
the commit author, so once a transitive bump pulled simple-git to 3.36.0
every worktree write (content_save, model_save, merge) started failing
with `Use of "EDITOR" is not permitted...` in any host that exports those
variables (VS Code, Claude Code, CI). The guard only scans the object
handed to .env(), never the inherited environment.

- Commit identity now flows through `-c user.name`/`-c user.email` config
  (authorConfig) instead of .env(). These keys are on no unsafe list and
  git honours them for author + committer alike, so the guard is never
  touched. CONTENTRAIN_AUTHOR_NAME/EMAIL overrides preserved.
- networkGit (push/fetch) legitimately needs the inherited askpass/SSH/
  proxy environment, so it keeps .env() but opts out of the affected guard
  categories via `unsafe` (NETWORK_UNSAFE). Closes the same latent failure
  in contentrain_submit that the report missed.
- Both concerns centralized in git/identity.ts so no future call site can
  reintroduce a process.env spread. simple-git pinned to ^3.36.0.

Tests: tests/git/identity.test.ts (5) incl. a control asserting the old
.env(process.env) path still trips the guard; existing tests/git suites
(49) green; tsc + oxlint clean.
@netlify

netlify Bot commented Jul 13, 2026

Copy link
Copy Markdown

Deploy Preview for contentrain-ai ready!

Name Link
🔨 Latest commit dca638d
🔍 Latest deploy log https://app.netlify.com/projects/contentrain-ai/deploys/6a55188ca338330008eb3889
😎 Deploy Preview https://deploy-preview-80--contentrain-ai.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@ABB65
ABB65 merged commit 570b075 into main Jul 13, 2026
6 checks passed
@ABB65
ABB65 deleted the fix/mcp-block-unsafe-guard branch July 13, 2026 17:00
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 13, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant