{"id":"GHSA-v5rq-49vh-5v5c","summary":"simple-git: `VISUAL` editor environment variable is omitted from unsafe editor detection","details":"# Security control bypass in `@simple-git/argv-parser`: Git's `VISUAL` editor fallback is not classified as `allowUnsafeEditor`\n\n## Report metadata\n\n| Field | Value |\n| --- | --- |\n| Package | `@simple-git/argv-parser` (npm, `pkg:npm/%40simple-git/argv-parser`) |\n| Repository | https://github.com/steveukx/git-js |\n| Component | `parseEnv` (`packages/argv-parser/src/env/parse-env.ts`), reached via `vulnerabilityCheck(tokens, env)` |\n| Vulnerability class | Security-control bypass — incomplete denylist in unsafe-editor detection |\n| Verified against | `c427fbad33f1f2b11341f1cf852eedecbb106400` (`@simple-git/argv-parser` 1.1.1), plus the published npm artifact 1.1.1; `main` at `98864c6` observed still unpatched |\n| API surface | Public documented API (`parseEnv(raw)` / `vulnerabilityCheck(tokens, env)`) |\n| Affected in default configuration | Yes — reproduced with `blockUnsafeOperationsPlugin` under default options, with no unsafe allowances enabled |\n\n## Summary\n\n`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.\n\nIn 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.\n\n- `EDITOR` -\u003e classified `allowUnsafeEditor` (`GitEnvKeys`, lines 5-27)\n- `GIT_EDITOR` -\u003e classified `allowUnsafeEditor` (`GitEnvKeys`, lines 5-27)\n- `GIT_SEQUENCE_EDITOR` -\u003e classified `allowUnsafeEditor` (`GitEnvKeys`, lines 5-27)\n- `VISUAL` -\u003e **absent from `GitEnvKeys`; dropped by `prepareEnv`, lines 60-68 — no vulnerability emitted**\n\n## Impact\n\nA 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`).\n\nThe 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.\n\nScoring 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.\n\n## Preconditions\n\n1. 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.\n2. The Git command opens an editor — for example `commit` without `-m`, `commit --amend`, or `rebase -i`.\n3. 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`).\n4. `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.\n5. The attacker-selected executable exists and is runnable on the host.\n\nThis 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.\n\nThe 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.\n\n## Data flow\n\n1. **Source** — the attacker-influenced `raw` environment object enters the public parser entry point: `parseEnv(raw)` ([`packages/argv-parser/src/env/parse-env.ts:70`](https://github.com/steveukx/git-js/blob/c427fbad33f1f2b11341f1cf852eedecbb106400/packages/argv-parser/src/env/parse-env.ts#L70)).\n2. **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`](https://github.com/steveukx/git-js/blob/c427fbad33f1f2b11341f1cf852eedecbb106400/packages/argv-parser/src/env/parse-env.ts#L60-L68)).\n3. **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`](https://github.com/steveukx/git-js/blob/c427fbad33f1f2b11341f1cf852eedecbb106400/packages/argv-parser/src/env/parse-env.ts#L45-L54)).\n4. **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`](https://github.com/steveukx/git-js/blob/c427fbad33f1f2b11341f1cf852eedecbb106400/simple-git/src/lib/plugins/block-unsafe-operations-plugin.ts#L12-L20)).\n\n## Vulnerable code\n\n`packages/argv-parser/src/env/parse-env.ts`, lines 5-27 at `c427fbad33f1f2b11341f1cf852eedecbb106400`:\n\n```ts\nconst GitEnvKeys = {\n   'editor': 'allowUnsafeEditor',\n   // ...\n   'git_editor': 'allowUnsafeEditor',\n   // ...\n   'git_sequence_editor': 'allowUnsafeEditor',\n   // VISUAL missing\n} as const satisfies Record\u003cstring, VulnerabilityCategory\u003e;\n```\n\nThe 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.\n\n## Reproduction\n\n**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.\n\nObserved at the parser level (vitest PoC run against the repo):\n\n- `parseEnv({ EDITOR })`, `parseEnv({ GIT_EDITOR })` and `parseEnv({ GIT_SEQUENCE_EDITOR })` each yield one `allowUnsafeEditor` vulnerability.\n- `parseEnv({ VISUAL: '/tmp/poc/evileditor' })` yields `[]` in every casing.\n- `vulnerabilityCheck(['commit', '--amend'], { VISUAL })` — the exact call the spawn guard makes — returns `[]`.\n- 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`.\n\nObserved at the Git level (only `VISUAL` set, `EDITOR` and `GIT_EDITOR` unset): `git var GIT_EDITOR` returned the attacker path; `git commit --amend` executed the attacker script (marker written, commit subject rewritten); `git rebase -i` executed it for the rebase-todo as well. `VISUAL` also took precedence over `EDITOR` (`EDITOR=/bin/true VISUAL=evil` -\u003e `evil`).\n\nObserved end to end through the real spawn path (simple-git built from this commit, default options, no unsafe allowances): the `EDITOR` and `GIT_EDITOR` variants both threw `GitPluginError` — \"Use of ... is not permitted without enabling allowUnsafeEditor\" — with no execution. The `VISUAL` variant was **not** blocked: the plugin saw an empty vulnerability list, Git spawned, and the attacker-supplied editor executed as the host user against `.git/COMMIT_EDITMSG`, rewriting the commit message.\n\nMinimal Git-level reproduction:\n\n```sh\n#!/bin/sh\nset -e\nrm -rf /tmp/visual-repo /tmp/GIT_VISUAL_POC /tmp/evileditor\nmkdir /tmp/visual-repo && cd /tmp/visual-repo\ngit init -q\ngit config user.email a@a && git config user.name a\ntouch a && git add a && git commit -qm init\nprintf '#!/bin/sh\\ntouch /tmp/GIT_VISUAL_POC\\nexit 1\\n' \u003e/tmp/evileditor\nchmod +x /tmp/evileditor\nenv -u EDITOR -u GIT_EDITOR VISUAL=/tmp/evileditor git commit --amend || true\ntest -e /tmp/GIT_VISUAL_POC && echo executed\n```\n\nGuard-level reproduction, using the public API:\n\n```ts\nimport { vulnerabilityCheck } from '@simple-git/argv-parser';\nimport { spawnSync } from 'node:child_process';\nconst args = ['commit', '--amend'];\nconst env = { VISUAL: '/tmp/evileditor' };\nif (vulnerabilityCheck(args, env).length === 0) spawnSync('git', args, { env });\n```\n\n- **Expected:** `parseEnv({ VISUAL: '/tmp/evileditor' }).vulnerabilities` contains `allowUnsafeEditor`, and the spawn guard refuses the operation unless the consumer has enabled `allowUnsafeEditor` — the behaviour already applied to `EDITOR`, `GIT_EDITOR` and `GIT_SEQUENCE_EDITOR`.\n- **Actual:** no vulnerability is reported, the guard permits the spawn, and Git executes the attacker-selected binary.\n\n## Suggested remediation\n\nTreat `VISUAL` as an editor source, so Git's own editor-resolution precedence is fully covered by the denylist.\n\n```ts\nconst GitEnvKeys = {\n   'editor': 'allowUnsafeEditor',\n   'visual': 'allowUnsafeEditor',\n   // ...\n   'git_editor': 'allowUnsafeEditor',\n   // ...\n   'git_sequence_editor': 'allowUnsafeEditor',\n} as const satisfies Record\u003cstring, VulnerabilityCategory\u003e;\n```\n\nBecause `prepareEnv` filters on `GitEnvKeys` membership, this single entry is enough to make `visual` survive filtering and be classified; no change to `prepareEnv` or `collectConfigVulnerabilities` is required.\n\nNotes:\n\n- `TERM` is likewise neither a `GitEnvKey` nor `git`-prefixed, and it is the variable that decides whether Git consults `VISUAL` at all (`TERM=dumb` or unset means `VISUAL` is ignored). An attacker who controls the environment object supplies it alongside `VISUAL`; whether `TERM` warrants its own classification is a maintainer judgement call, but it is worth considering while fixing this.\n- `git rebase -i` reaches the same sink: `git_sequence_editor` falls back to normal editor resolution, so the `VISUAL` path executes the attacker binary against the rebase-todo file as well.\n- `docs/PLUGIN-UNSAFE-ACTIONS.md` (\"Text editor\") should list `VISUAL` alongside `EDITOR` / `GIT_EDITOR` / `GIT_SEQUENCE_EDITOR`, since the documented scope of the control is what consumers rely on.\n- Already safe and needing no change: `EDITOR`, `GIT_EDITOR` and `GIT_SEQUENCE_EDITOR` are all correctly classified and enforced by the plugin, and the parser's handling of `git`-prefixed variables is unaffected.\n\nSuggested regression test alongside `test/parse-env.spec.ts` (which currently covers `EDITOR` / `GIT_EDITOR` / `GIT_SEQUENCE_EDITOR` / `PAGER` but has no `VISUAL` case): assert that `parseEnv({ VISUAL: '/tmp/evileditor' })` yields one `allowUnsafeEditor` vulnerability in every casing, and that `vulnerabilityCheck(['commit', '--amend'], { VISUAL: '/tmp/evileditor' })` returns that vulnerability rather than `[]`. A precedence case is worth adding too: `VISUAL` set together with `EDITOR` must still be flagged, since `VISUAL` wins in Git's resolution order.","aliases":["CVE-2026-102829"],"modified":"2026-10-06T00:00:09.535509971Z","published":"2026-10-05T23:48:21Z","database_specific":{"github_reviewed":true,"github_reviewed_at":"2026-10-05T23:48:21Z","nvd_published_at":"2026-09-29T19:17:25Z","cwe_ids":["CWE-184","CWE-78"],"severity":"CRITICAL"},"references":[{"type":"WEB","url":"https://github.com/steveukx/git-js/security/advisories/GHSA-v5rq-49vh-5v5c"},{"type":"ADVISORY","url":"https://nvd.nist.gov/vuln/detail/CVE-2026-102829"},{"type":"WEB","url":"https://github.com/steveukx/git-js/pull/1201"},{"type":"WEB","url":"https://github.com/steveukx/git-js/commit/68874c239f0c7a87f4a68c3d2c4a0d7c75bb27f4"},{"type":"PACKAGE","url":"https://github.com/steveukx/git-js"},{"type":"WEB","url":"https://github.com/steveukx/git-js/releases/tag/@simple-git/argv-parser@2.0.1"}],"affected":[{"package":{"name":"@simple-git/argv-parser","ecosystem":"npm","purl":"pkg:npm/%40simple-git/argv-parser"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"2.0.1"}]}],"database_specific":{"source":"https://github.com/github/advisory-database/blob/main/advisories/github-reviewed/2026/10/GHSA-v5rq-49vh-5v5c/GHSA-v5rq-49vh-5v5c.json"}}],"schema_version":"1.9.0","severity":[{"type":"CVSS_V4","score":"CVSS:4.0/AV:N/AC:H/AT:P/PR:N/UI:N/VC:H/VI:H/VA:H/SC:N/SI:N/SA:N"}]}