Is @simple-git/argv-parser safe?

GHSA-v5rq-49vh-5v5c

| 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 | c427fbad33f1 (@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 |

GitEnvKeys in packages/argv-parser/src/env/parse-env.ts maps only editor, giteditor and gitsequenceeditor 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 GITEDITOR 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)
• GITEDITOR -> classified allowUnsafeEditor (GitEnvKeys, lines 5-27)
• GITSEQUENCEEDITOR -> classified allowUnsafeEditor (GitEnvKeys, lines 5-27)
• VISUAL -> absent from GitEnvKeys; dropped by prepareEnv, lines 60-68 — no vulnerability emitted

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/COMMITEDITMSG, 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, GITEDITOR or GITSEQUENCEEDITOR 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 GITEDITOR or core.editor, are not affected.

• 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.
• The Git command opens an editor — for example commit without -m, commit --amend, or rebase -i.
• No higher-priority editor setting overrides VISUAL: GITEDITOR, core.editor and EDITOR are absent (GITEDITOR and core.editor take precedence; VISUAL itself overrides EDITOR).
• 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.
• 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, GITEDITOR and GITSEQUENCEEDITOR; 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 GITCONFIGPARAMETERS / allowUnsafeConfigEnvCount config-injection scenario. Those were not needed here: the bypass was reproduced with default options and no unsafe allowances enabled.

• Source — the attacker-influenced raw environment object enters the public parser entry point: parseEnv(raw) (packages/argv-parser/src/env/parse-env.ts:70).
• 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).
• 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).
• 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).

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

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

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.

Verified — reproduced dynamically, end to end, against a checkout of c427fbad33f1 (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({ GITEDITOR }) and parseEnv({ GITSEQUENCEEDITOR }) each yield one allowUnsafeEditor vulnerability.
• parseEnv({ VISUAL: '/tmp/poc/evileditor' }) yields [] in every casing.
• vulnerabilityCheck(['commit', '--amend'], { VISUAL }) — the exact call the spawn guard makes — returns [].
• The published dist/index.cjs of 1.1.1 contains zero occurrences of visual; the same holds for the repository at the verified commit, including docs/PLUGIN-UNSAFE-ACTIONS.md.