Security control bypass in @simple-git/argv-parser: Git's VISUAL editor fallback is not classified as allowUnsafeEditor

Report metadata

Field	Value
Package	@simple-git/argv-parser (npm, pkg:npm/%40simple-git/argv-parser)
Repository	https://github.com/steveukx/git-js
Component	parseEnv (packages/argv-parser/src/env/parse-env.ts), reached via vulnerabilityCheck(tokens, env)
Vulnerability class	Security-control bypass — incomplete denylist in unsafe-editor detection
Verified against	c427fbad33f1f2b11341f1cf852eedecbb106400 (@simple-git/argv-parser 1.1.1), plus the published npm artifact 1.1.1; main at 98864c6 observed still unpatched
API surface	Public documented API (parseEnv(raw) / vulnerabilityCheck(tokens, env))
Affected in default configuration	Yes — reproduced with blockUnsafeOperationsPlugin under default options, with no unsafe allowances enabled

Summary

GitEnvKeys in packages/argv-parser/src/env/parse-env.ts maps only editor, git_editor and git_sequence_editor to the allowUnsafeEditor category. prepareEnv keeps an environment entry only when its lowercased name is a known GitEnvKey or starts with git, so VISUAL is discarded before collectConfigVulnerabilities ever inspects it. Git, however, falls back to VISUAL when resolving an editor, so parseEnv({ VISUAL: '/tmp/evileditor' }) reports no vulnerability while an interactive Git operation will execute that binary.

In a consuming application the shape is: environment values derived from a request or job are forwarded into the child Git environment and classified by this parser before spawn. The parser exists to classify exactly such values, and the equivalent EDITOR or GIT_EDITOR value is rejected — so the attacker gains an editor substitution that the guard is specifically designed to block.

• EDITOR -> classified allowUnsafeEditor (GitEnvKeys, lines 5-27)
• GIT_EDITOR -> classified allowUnsafeEditor (GitEnvKeys, lines 5-27)
• GIT_SEQUENCE_EDITOR -> classified allowUnsafeEditor (GitEnvKeys, lines 5-27)
• VISUAL -> absent from GitEnvKeys; dropped by prepareEnv, lines 60-68 — no vulnerability emitted

Impact

A consuming application that allows attacker-influenced environment values can have an attacker-selected executable launched by Git during operations such as git commit --amend, bypassing the parser's default unsafe-editor protection. Execution happens as the host user running the Git child process, with the attacker's binary invoked against the repository's editor file (for example .git/COMMIT_EDITMSG, or .git/rebase-merge/git-rebase-todo for git rebase -i).

The new capability is the bypass itself: without this gap, the same attacker-supplied value under EDITOR, GIT_EDITOR or GIT_SEQUENCE_EDITOR is refused unless the consumer explicitly opts in to allowUnsafeEditor. With VISUAL, the equivalent code execution proceeds with no opt-in and no reported vulnerability. VISUAL also takes precedence over EDITOR, the variable the parser does flag.

Scoring note: no CVSS vector or score is available for this finding, and one is not asserted here. Exploitability depends on the consuming application's data flow — specifically whether attacker-influenced environment entries reach the Git child environment. Consumers that never forward untrusted environment values into Git, or that always set a higher-priority GIT_EDITOR or core.editor, are not affected.

Preconditions

1. The consumer forwards attacker-influenced environment entries into the environment passed to child Git (for example via simple-git's .env()), and classifies them with this parser before spawn.
2. The Git command opens an editor — for example commit without -m, commit --amend, or rebase -i.
3. No higher-priority editor setting overrides VISUAL: GIT_EDITOR, core.editor and EDITOR are absent (GIT_EDITOR and core.editor take precedence; VISUAL itself overrides EDITOR).
4. TERM is set to a value other than exactly dumb — Git consults VISUAL only then. TERM is neither a GitEnvKey nor git-prefixed, so an attacker who controls the environment object supplies it too and the parser reports nothing for it either.
5. The attacker-selected executable exists and is runnable on the host.

This requires no non-standard usage, no monkey-patching and no unusual configuration: the affected path is the documented, default-enabled guard. docs/PLUGIN-UNSAFE-ACTIONS.md ("Text editor") documents this control as covering editor environment variables that substitute an arbitrary binary, but lists only EDITOR, GIT_EDITOR and GIT_SEQUENCE_EDITOR; Git's VISUAL fallback is not mentioned, and the string visual does not appear anywhere in the repository at the verified commit. The docs do state that supplying environment values is the caller's responsibility, but they do not warn that VISUAL is outside the guard.

The feed payload's precondition list also carries entries relating to a separate GIT_CONFIG_PARAMETERS / allowUnsafeConfigEnvCount config-injection scenario. Those were not needed here: the bypass was reproduced with default options and no unsafe allowances enabled.

Data flow

1. Source — the attacker-influenced raw environment object enters the public parser entry point: parseEnv(raw) (packages/argv-parser/src/env/parse-env.ts:70).
2. Propagation — prepareEnv lowercases keys and retains only known GitEnvKeys or names starting with git; visual is neither, so the entry is dropped before any analysis sees it (packages/argv-parser/src/env/parse-env.ts:60-68).
3. Sink — collectConfigVulnerabilities therefore never emits allowUnsafeEditor for VISUAL, and vulnerabilityCheck(tokens, env) returns an empty list (packages/argv-parser/src/env/parse-env.ts:45-54).
4. Reachability — simple-git's blockUnsafeOperationsPlugin passes its environment into vulnerabilityCheck before spawn; an empty vulnerability list means the Git child process is allowed to start (simple-git/src/lib/plugins/block-unsafe-operations-plugin.ts:12-20).

Vulnerable code

packages/argv-parser/src/env/parse-env.ts, lines 5-27 at c427fbad33f1f2b11341f1cf852eedecbb106400:

const GitEnvKeys = {
   'editor': 'allowUnsafeEditor',
   // ...
   'git_editor': 'allowUnsafeEditor',
   // ...
   'git_sequence_editor': 'allowUnsafeEditor',
   // VISUAL missing
} as const satisfies Record<string, VulnerabilityCategory>;

The three mapped keys are the safe siblings; the missing visual entry is the gap. Because prepareEnv (lines 60-68) filters on this map plus a git prefix, the omission is not merely a missing classification — the value never reaches the classifier at all.

Reproduction

Verified — reproduced dynamically, end to end, against a checkout of c427fbad33f1f2b11341f1cf852eedecbb106400 (packages/argv-parser/package.json = 1.1.1) and against the published npm artifact 1.1.1.

Observed at the parser level (vitest PoC run against the repo):

• parseEnv({ EDITOR }), parseEnv({ GIT_EDITOR }) and parseEnv({ GIT_SEQUENCE_EDITOR }) each yield one allowUnsafeEditor vulnerability.
• parseEnv({ VISUAL: '/tmp/poc/evileditor' }) yields [] in every casing.
• vulnerabilityCheck(['commit', '--amend'], { VISUAL }) — the exact call the