From 21ac118268aeae211cedfc77cfd12ed4c5a158be Mon Sep 17 00:00:00 2001 From: Steve King Date: Sun, 8 Mar 2026 14:58:40 +0000 Subject: [PATCH 1/3] Add detection for more potential attack vectors through `git -c` configuration switches --- .../plugins/block-unsafe-operations-plugin.ts | 48 +++++++++++++------ simple-git/src/lib/types/index.ts | 26 +++++++++- .../test/unit/plugins/plugin.unsafe.spec.ts | 39 +++++++++++---- 3 files changed, 88 insertions(+), 25 deletions(-) diff --git a/simple-git/src/lib/plugins/block-unsafe-operations-plugin.ts b/simple-git/src/lib/plugins/block-unsafe-operations-plugin.ts index 93f31d58..34763eef 100644 --- a/simple-git/src/lib/plugins/block-unsafe-operations-plugin.ts +++ b/simple-git/src/lib/plugins/block-unsafe-operations-plugin.ts @@ -16,22 +16,40 @@ function isCloneSwitch(char: string, arg: string | unknown) { return /^[\dlsqvnobucj]+\b/.test(token); } -function preventProtocolOverride(arg: string, next: string) { - if (!isConfigSwitch(arg)) { - return; - } - - if (!/^\s*protocol(.[a-z]+)?.allow/i.test(next)) { - return; - } +function preventConfigBuilder( + config: string | RegExp, + setting: keyof SimpleGitPluginConfig['unsafe'], + message = String(config) +) { + const regex = typeof config === 'string' ? new RegExp(`\\s*${config}`, 'i') : config; - throw new GitPluginError( - undefined, - 'unsafe', - 'Configuring protocol.allow is not permitted without enabling allowUnsafeExtProtocol' - ); + return function preventCommand( + options: Partial, + arg: string, + next: string + ) { + if (options[setting] !== true && isConfigSwitch(arg) && regex.test(next)) { + throw new GitPluginError( + undefined, + 'unsafe', + `Configuring ${message} is not permitted without enabling ${setting}` + ); + } + }; } +const preventUnsafeConfig = [ + preventConfigBuilder( + /^\s*protocol(.[a-z]+)?.allow/i, + 'allowUnsafeProtocolOverride', + 'protocol.allow' + ), + preventConfigBuilder('core.sshCommand', 'allowUnsafeSshCommand'), + preventConfigBuilder('core.gitProxy', 'allowUnsafeGitProxy'), + preventConfigBuilder('core.hooksPath', 'allowUnsafeHooksPath'), + preventConfigBuilder('diff.external', 'allowUnsafeDiffExternal'), +]; + function preventUploadPack(arg: string, method: string) { if (/^\s*--(upload|receive)-pack/.test(arg)) { throw new GitPluginError( @@ -59,8 +77,8 @@ function preventUploadPack(arg: string, method: string) { } export function blockUnsafeOperationsPlugin({ - allowUnsafeProtocolOverride = false, allowUnsafePack = false, + ...options }: SimpleGitPluginConfig['unsafe'] = {}): SimpleGitPlugin<'spawn.args'> { return { type: 'spawn.args', @@ -68,8 +86,8 @@ export function blockUnsafeOperationsPlugin({ args.forEach((current, index) => { const next = index < args.length ? args[index + 1] : ''; - allowUnsafeProtocolOverride || preventProtocolOverride(current, next); allowUnsafePack || preventUploadPack(current, context.method); + preventUnsafeConfig.forEach((helper) => helper(options, current, next)); }); return args; diff --git a/simple-git/src/lib/types/index.ts b/simple-git/src/lib/types/index.ts index 0842bb89..7b9f501a 100644 --- a/simple-git/src/lib/types/index.ts +++ b/simple-git/src/lib/types/index.ts @@ -137,7 +137,7 @@ export interface SimpleGitPluginConfig { allowUnsafeCustomBinary?: boolean; /** - * By default `simple-git` prevents the use of inline configuration + * By default, `simple-git` prevents the use of inline configuration * options to override the protocols available for the `git` child * process to prevent accidental security vulnerabilities when * unsanitised user data is passed directly into operations such as @@ -156,6 +156,30 @@ export interface SimpleGitPluginConfig { * Enable this override to permit the use of these arguments. */ allowUnsafePack?: boolean; + + /** + * Using a `-c` switch to enable custom SSH commands opens up a potential + * attack vector for running arbitrary commands. + */ + allowUnsafeSshCommand?: boolean; + + /** + * Using a `-c` switch to enable custom proxy command for the `git://` transport + * exposes and attack vector for running arbitrary commands. + */ + allowUnsafeGitProxy?: boolean; + + /** + * Using a `-c` switch to enable custom hooks path commands to be run automatically + * exposes and attack vector for running arbitrary commands. + */ + allowUnsafeHooksPath?: boolean; + + /** + * Using a `-c` switch to enable setting binary for processing diffs + * exposes and attack vector for running arbitrary commands. + */ + allowUnsafeDiffExternal?: boolean; }; } diff --git a/simple-git/test/unit/plugins/plugin.unsafe.spec.ts b/simple-git/test/unit/plugins/plugin.unsafe.spec.ts index 6c0313de..1d0e9ef1 100644 --- a/simple-git/test/unit/plugins/plugin.unsafe.spec.ts +++ b/simple-git/test/unit/plugins/plugin.unsafe.spec.ts @@ -1,10 +1,5 @@ import { promiseError } from '@kwsites/promise-result'; -import { - assertExecutedCommands, - assertGitError, - closeWithSuccess, - newSimpleGit, -} from '../__fixtures__'; +import { assertExecutedCommands, assertGitError, closeWithSuccess, newSimpleGit } from '../__fixtures__'; describe('blockUnsafeOperationsPlugin', () => { it.each([ @@ -18,7 +13,7 @@ describe('blockUnsafeOperationsPlugin', () => { assertGitError( await promiseError(newSimpleGit().raw(...task)), - 'allowUnsafeExtProtocol' + 'allowUnsafeExtProtocol', ); const err = promiseError( @@ -38,11 +33,11 @@ describe('blockUnsafeOperationsPlugin', () => { ])('allows %s %s only when using override', async (cmd, option) => { assertGitError( await promiseError(newSimpleGit({ unsafe: {} }).raw(cmd, option)), - 'allowUnsafePack' + 'allowUnsafePack', ); const err = promiseError( - newSimpleGit({ unsafe: { allowUnsafePack: true } }).raw(cmd, option) + newSimpleGit({ unsafe: { allowUnsafePack: true } }).raw(cmd, option), ); await closeWithSuccess(); @@ -50,6 +45,32 @@ describe('blockUnsafeOperationsPlugin', () => { assertExecutedCommands(cmd, option); }); + describe.each([ + ['allowUnsafeSshCommand', `core.sshCommand=sh -c 'id > pwned'`], + ['allowUnsafeGitProxy', `core.gitProxy=sh -c 'id > pwned'`], + ['allowUnsafeHooksPath', `core.hooksPath=sh -c 'id > pwned'`], + ['allowUnsafeDiffExternal', `diff.external=sh -c 'id > pwned'`], + ])('unsafe config option - %s', (setting, command) => { + + it('blocks by default', async () => { + const err = promiseError( + newSimpleGit().clone('remote', 'local', ['-c', command]), + ); + await promiseError(closeWithSuccess()); + + assertGitError(await err, setting); + }); + + it('allows with override', async () => { + const err = promiseError( + newSimpleGit({ unsafe: { [setting]: true } }).clone('remote', 'local', ['-c', command]), + ); + await closeWithSuccess(); + + expect(await err).toBeUndefined(); + }); + }); + it('allows -u for non-clone commands', async () => { const git = newSimpleGit({ unsafe: {} }); const err = promiseError(git.raw('push', '-u', 'origin/main')); From fde1a5adcc9dce3786213b96608c9552fad89efe Mon Sep 17 00:00:00 2001 From: Steve King Date: Sun, 8 Mar 2026 15:02:02 +0000 Subject: [PATCH 2/3] Changeset --- .changeset/shaky-readers-share.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/shaky-readers-share.md diff --git a/.changeset/shaky-readers-share.md b/.changeset/shaky-readers-share.md new file mode 100644 index 00000000..5bfbfdc2 --- /dev/null +++ b/.changeset/shaky-readers-share.md @@ -0,0 +1,7 @@ +--- +"simple-git": patch +--- + +Enhanced `git -c` checks in `unsafe` plugin. + +Thanks to @JohannesLks for identifying the issue From 6e3c95eadae03803e6ae2667a4943ec3d7624f72 Mon Sep 17 00:00:00 2001 From: Steve King Date: Sun, 8 Mar 2026 15:08:13 +0000 Subject: [PATCH 3/3] Fix typo --- simple-git/test/unit/plugins/plugin.unsafe.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/simple-git/test/unit/plugins/plugin.unsafe.spec.ts b/simple-git/test/unit/plugins/plugin.unsafe.spec.ts index 1d0e9ef1..75bef9a2 100644 --- a/simple-git/test/unit/plugins/plugin.unsafe.spec.ts +++ b/simple-git/test/unit/plugins/plugin.unsafe.spec.ts @@ -8,12 +8,12 @@ describe('blockUnsafeOperationsPlugin', () => { ['Protocol.Allow=always'], ['PROTOCOL.allow=always'], ['protocol.ALLOW=always'], - ])('blocks protocol overide in format %s', async (cmd) => { + ])('blocks protocol override in format %s', async (cmd) => { const task = ['config', '-c', cmd, 'config', '--list']; assertGitError( await promiseError(newSimpleGit().raw(...task)), - 'allowUnsafeExtProtocol', + 'allowUnsafeProtocolOverride', ); const err = promiseError(