From 7aa0116bc06aee5c51b9f9a178376422dfad9778 Mon Sep 17 00:00:00 2001 From: "J. Kirby Ross" Date: Wed, 28 Jan 2026 18:44:11 -0800 Subject: [PATCH 1/4] feat(sanitizer): allow 'show' command and expand history traversal tests - Add 'show' to the allowlist of safe Git commands. - Update version to 2.7.1. - Add unit tests for 'log' and 'show' sanitization. --- package.json | 2 +- src/domain/services/CommandSanitizer.js | 3 ++- test/domain/services/CommandSanitizer.test.js | 8 ++++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 3d53b8c..89ba83b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@git-stunts/plumbing", - "version": "2.7.0", + "version": "2.7.1", "description": "Robust async, stream-first Git plumbing for Node, Bun, and Deno.", "type": "module", "main": "index.js", diff --git a/src/domain/services/CommandSanitizer.js b/src/domain/services/CommandSanitizer.js index 8987e61..c0dcc8b 100644 --- a/src/domain/services/CommandSanitizer.js +++ b/src/domain/services/CommandSanitizer.js @@ -43,7 +43,8 @@ export default class CommandSanitizer { 'check-attr', 'init', 'config', - 'log' + 'log', + 'show' ]); /** diff --git a/test/domain/services/CommandSanitizer.test.js b/test/domain/services/CommandSanitizer.test.js index 3aa282b..4665e2c 100644 --- a/test/domain/services/CommandSanitizer.test.js +++ b/test/domain/services/CommandSanitizer.test.js @@ -13,6 +13,14 @@ describe('CommandSanitizer', () => { expect(() => sanitizer.sanitize(['rev-parse', 'HEAD'])).not.toThrow(); }); + it('allows log command for commit history traversal', () => { + expect(() => sanitizer.sanitize(['log', '--format=%H', '-z', 'HEAD'])).not.toThrow(); + }); + + it('allows show command for reading commit messages', () => { + expect(() => sanitizer.sanitize(['show', '--format=%B', '-s', 'HEAD'])).not.toThrow(); + }); + it('throws ValidationError for unlisted commands', () => { expect(() => sanitizer.sanitize(['push', 'origin', 'main'])).toThrow(ValidationError); }); From d532db32907b60515e34d608c4a177e293dfd0e3 Mon Sep 17 00:00:00 2001 From: "J. Kirby Ross" Date: Wed, 28 Jan 2026 18:55:55 -0800 Subject: [PATCH 2/4] feat(sanitizer): implement strict per-command flag allowlisting - Introduce `_COMMAND_FLAG_ALLOWLISTS` to restrict flags for 'show' and 'log'. - Add `_validateCommandFlags` to prevent unauthorized flag injection. - Add comprehensive test suite covering flag validation, shell metacharacter safety, and NUL-terminated output scenarios. --- src/domain/services/CommandSanitizer.js | 64 +++++++++ test/domain/services/CommandSanitizer.test.js | 134 +++++++++++++++++- 2 files changed, 196 insertions(+), 2 deletions(-) diff --git a/src/domain/services/CommandSanitizer.js b/src/domain/services/CommandSanitizer.js index c0dcc8b..124bf19 100644 --- a/src/domain/services/CommandSanitizer.js +++ b/src/domain/services/CommandSanitizer.js @@ -73,6 +73,67 @@ export default class CommandSanitizer { '--ext-cmd' ]; + /** + * Per-command flag allowlists for commands with restricted flag sets. + * Only flags listed here are permitted for these commands. + * Commands not in this map have no additional restrictions beyond global prohibitions. + * @private + */ + static _COMMAND_FLAG_ALLOWLISTS = { + 'show': new Set([ + '--format', '--pretty', '-s', '--no-patch', '--quiet', '-q', + '--name-only', '--name-status', '--stat', '--numstat', '--shortstat', + '--oneline', '--abbrev-commit', '--no-abbrev-commit', '--date', '--no-notes' + ]), + 'log': new Set([ + '--format', '--pretty', '-z', '--oneline', + '-n', '--max-count', '-1', '-2', '-3', '-4', '-5', '-10', '-20', '-50', '-100', + '--skip', '--since', '--until', '--after', '--before', + '--author', '--committer', '--grep', '--all-match', '--invert-grep', + '--regexp-ignore-case', '-i', '-E', '-F', '-P', + '--ancestry-path', '--first-parent', '--no-merges', '--merges', + '--reverse', '--date-order', '--author-date-order', '--topo-order', + '--abbrev-commit', '--no-abbrev-commit', '--abbrev', '--date', + '--relative-date', '--parents', '--children', '--left-right', + '--graph', '--decorate', '--no-decorate', '--source', + '--no-walk', '--stdin', '--cherry', '--cherry-pick', '--cherry-mark', + '--boundary', '--simplify-by-decoration' + ]) + }; + + /** + * Validates command-specific flags against the allowlist. + * @param {string} command - The git subcommand (e.g., 'show', 'log') + * @param {string[]} args - All arguments including flags + * @param {number} commandIndex - Index of the command in args array + * @throws {ProhibitedFlagError} If a flag is not in the allowlist + * @private + */ + static _validateCommandFlags(command, args, commandIndex) { + const allowlist = CommandSanitizer._COMMAND_FLAG_ALLOWLISTS[command]; + if (!allowlist) { + return; // No allowlist = no additional restrictions + } + + for (let i = commandIndex + 1; i < args.length; i++) { + const arg = args[i]; + + // Skip non-flag arguments (refs, paths, etc.) + if (!arg.startsWith('-')) { + continue; + } + + // Handle --flag=value format: extract just the flag portion + const flagPart = arg.includes('=') ? arg.split('=')[0] : arg; + + if (!allowlist.has(flagPart)) { + throw new ProhibitedFlagError(arg, 'CommandSanitizer.sanitize', { + message: `Flag '${flagPart}' is not allowed for '${command}' command` + }); + } + } + } + /** * Dynamically allows a command. * @param {string} commandName @@ -159,6 +220,9 @@ export default class CommandSanitizer { throw new ValidationError(`Prohibited git command detected: ${command}`, 'CommandSanitizer.sanitize', { command }); } + // Validate per-command flag allowlists + CommandSanitizer._validateCommandFlags(command, args, subcommandIndex !== -1 ? subcommandIndex : 0); + let totalLength = 0; for (const arg of args) { totalLength += arg.length; diff --git a/test/domain/services/CommandSanitizer.test.js b/test/domain/services/CommandSanitizer.test.js index 4665e2c..ddf0f94 100644 --- a/test/domain/services/CommandSanitizer.test.js +++ b/test/domain/services/CommandSanitizer.test.js @@ -54,13 +54,143 @@ describe('CommandSanitizer', () => { it('handles cache eviction', () => { const smallSanitizer = new CommandSanitizer({ maxCacheSize: 2 }); - + const args1 = ['rev-parse', 'HEAD']; const args2 = ['cat-file', '-p', '4b825dc642cb6eb9a060e54bf8d69288fbee4904']; const args3 = ['ls-tree', 'HEAD']; - + smallSanitizer.sanitize(args1); smallSanitizer.sanitize(args2); smallSanitizer.sanitize(args3); }); + + describe('Per-command flag allowlists', () => { + describe('show command', () => { + it('allows whitelisted flags for show', () => { + expect(() => sanitizer.sanitize(['show', '--format=%B', '-s', 'HEAD'])).not.toThrow(); + expect(() => sanitizer.sanitize(['show', '--pretty=oneline', 'HEAD'])).not.toThrow(); + expect(() => sanitizer.sanitize(['show', '--no-patch', 'HEAD'])).not.toThrow(); + expect(() => sanitizer.sanitize(['show', '--quiet', 'HEAD'])).not.toThrow(); + }); + + it('rejects non-whitelisted flags for show', () => { + expect(() => sanitizer.sanitize(['show', '--diff-filter=A', 'HEAD'])).toThrow(ProhibitedFlagError); + expect(() => sanitizer.sanitize(['show', '--follow', 'HEAD'])).toThrow(ProhibitedFlagError); + expect(() => sanitizer.sanitize(['show', '-p', 'HEAD'])).toThrow(ProhibitedFlagError); + }); + + it('allows show with no ref (defaults to HEAD)', () => { + expect(() => sanitizer.sanitize(['show'])).not.toThrow(); + expect(() => sanitizer.sanitize(['show', '--format=%B'])).not.toThrow(); + }); + }); + + describe('log command', () => { + it('allows whitelisted flags for log', () => { + expect(() => sanitizer.sanitize(['log', '--format=%H', '-z', 'HEAD'])).not.toThrow(); + expect(() => sanitizer.sanitize(['log', '-n', '10', 'HEAD'])).not.toThrow(); + expect(() => sanitizer.sanitize(['log', '--max-count=50', 'HEAD'])).not.toThrow(); + expect(() => sanitizer.sanitize(['log', '--oneline', 'HEAD'])).not.toThrow(); + expect(() => sanitizer.sanitize(['log', '--first-parent', 'HEAD'])).not.toThrow(); + }); + + it('rejects non-whitelisted flags for log', () => { + expect(() => sanitizer.sanitize(['log', '--diff-filter=M', 'HEAD'])).toThrow(ProhibitedFlagError); + expect(() => sanitizer.sanitize(['log', '--follow', 'HEAD'])).toThrow(ProhibitedFlagError); + expect(() => sanitizer.sanitize(['log', '-p', 'HEAD'])).toThrow(ProhibitedFlagError); + }); + + it('allows log with no arguments', () => { + expect(() => sanitizer.sanitize(['log'])).not.toThrow(); + }); + }); + + describe('other commands have no additional restrictions', () => { + it('allows any flags for rev-parse', () => { + expect(() => sanitizer.sanitize(['rev-parse', '--show-toplevel'])).not.toThrow(); + expect(() => sanitizer.sanitize(['rev-parse', '--abbrev-ref', 'HEAD'])).not.toThrow(); + }); + + it('allows any flags for cat-file', () => { + expect(() => sanitizer.sanitize(['cat-file', '-p', 'HEAD'])).not.toThrow(); + expect(() => sanitizer.sanitize(['cat-file', '-t', 'HEAD'])).not.toThrow(); + }); + }); + }); + + describe('Argument injection protection (spawn safety)', () => { + it('safely handles shell metacharacters in flag values', () => { + // spawn() passes args directly - no shell parsing + expect(() => sanitizer.sanitize(['log', '--format=%; rm -rf /', 'HEAD'])).not.toThrow(); + expect(() => sanitizer.sanitize(['log', '--author=foo; cat /etc/passwd', 'HEAD'])).not.toThrow(); + }); + + it('safely handles backticks in arguments', () => { + expect(() => sanitizer.sanitize(['log', '--format=`whoami`', 'HEAD'])).not.toThrow(); + }); + + it('safely handles $() command substitution in arguments', () => { + expect(() => sanitizer.sanitize(['log', '--format=$(id)', 'HEAD'])).not.toThrow(); + }); + + it('safely handles pipe characters in arguments', () => { + expect(() => sanitizer.sanitize(['log', '--format=%s | malicious', 'HEAD'])).not.toThrow(); + }); + + it('safely handles newlines in arguments', () => { + expect(() => sanitizer.sanitize(['log', '--format=%s\n%b', 'HEAD'])).not.toThrow(); + }); + + it('safely handles quotes in arguments', () => { + expect(() => sanitizer.sanitize(['log', '--author="John Doe"', 'HEAD'])).not.toThrow(); + expect(() => sanitizer.sanitize(['log', "--author='Jane Doe'", 'HEAD'])).not.toThrow(); + }); + }); + + describe('NUL-terminated output (log -z)', () => { + it('allows log with -z flag', () => { + expect(() => sanitizer.sanitize(['log', '-z', '--format=%H', 'HEAD'])).not.toThrow(); + }); + + it('allows log with -z and multiple format specifiers', () => { + expect(() => sanitizer.sanitize(['log', '-z', '--format=%H%x00%s%x00%b'])).not.toThrow(); + }); + + it('allows log with -z combined with -n', () => { + expect(() => sanitizer.sanitize(['log', '-z', '-n', '10', 'HEAD'])).not.toThrow(); + }); + + it('allows log with -z and --max-count', () => { + expect(() => sanitizer.sanitize(['log', '-z', '--max-count=50', 'main..HEAD'])).not.toThrow(); + }); + + it('allows log with -z and ancestry path traversal', () => { + expect(() => sanitizer.sanitize(['log', '-z', '--ancestry-path', '--format=%H', 'main..HEAD'])).not.toThrow(); + }); + + it('allows log with -z and first-parent for linear history', () => { + expect(() => sanitizer.sanitize(['log', '-z', '--first-parent', '--format=%H%x00%P', 'HEAD'])).not.toThrow(); + }); + + it('allows log with -z and reverse for chronological order', () => { + expect(() => sanitizer.sanitize(['log', '-z', '--reverse', '--format=%H', 'HEAD~10..HEAD'])).not.toThrow(); + }); + }); + + describe('Edge cases', () => { + it('throws for empty array', () => { + expect(() => sanitizer.sanitize([])).toThrow(ValidationError); + }); + + it('allows commands with only the command name (no flags or refs)', () => { + expect(() => sanitizer.sanitize(['show'])).not.toThrow(); + expect(() => sanitizer.sanitize(['log'])).not.toThrow(); + expect(() => sanitizer.sanitize(['rev-parse'])).not.toThrow(); + }); + + it('handles flags with = values correctly', () => { + expect(() => sanitizer.sanitize(['log', '--format=%H'])).not.toThrow(); + expect(() => sanitizer.sanitize(['log', '--max-count=10'])).not.toThrow(); + }); + }); }); \ No newline at end of file From 46e99ca50c9af5f790695f0bbe24e956f781d4eb Mon Sep 17 00:00:00 2001 From: "J. Kirby Ross" Date: Wed, 28 Jan 2026 19:27:15 -0800 Subject: [PATCH 3/4] fix(sanitizer): handle end-of-options marker (--) in flag validation Stop flag validation when encountering bare '--' marker, allowing pathspecs and refs that begin with '-' to pass through without being checked against the allowlist. --- src/domain/services/CommandSanitizer.js | 6 ++++++ test/domain/services/CommandSanitizer.test.js | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/src/domain/services/CommandSanitizer.js b/src/domain/services/CommandSanitizer.js index 124bf19..d2dd1a6 100644 --- a/src/domain/services/CommandSanitizer.js +++ b/src/domain/services/CommandSanitizer.js @@ -118,6 +118,12 @@ export default class CommandSanitizer { for (let i = commandIndex + 1; i < args.length; i++) { const arg = args[i]; + // Stop flag validation at end-of-options marker + // Everything after '--' is a pathspec or ref, not a flag + if (arg === '--') { + break; + } + // Skip non-flag arguments (refs, paths, etc.) if (!arg.startsWith('-')) { continue; diff --git a/test/domain/services/CommandSanitizer.test.js b/test/domain/services/CommandSanitizer.test.js index ddf0f94..374f50b 100644 --- a/test/domain/services/CommandSanitizer.test.js +++ b/test/domain/services/CommandSanitizer.test.js @@ -192,5 +192,13 @@ describe('CommandSanitizer', () => { expect(() => sanitizer.sanitize(['log', '--format=%H'])).not.toThrow(); expect(() => sanitizer.sanitize(['log', '--max-count=10'])).not.toThrow(); }); + + it('stops flag validation at end-of-options marker (--)', () => { + // Args after '--' are pathspecs/refs, not flags - should not be validated + expect(() => sanitizer.sanitize(['show', '--format=%B', '--', '-weird-ref-name'])).not.toThrow(); + expect(() => sanitizer.sanitize(['log', '--oneline', '--', '--not-a-flag'])).not.toThrow(); + // Disallowed flag BEFORE '--' should still throw + expect(() => sanitizer.sanitize(['show', '-p', '--', 'file.txt'])).toThrow(ProhibitedFlagError); + }); }); }); \ No newline at end of file From 61ab36d33e172fb0456db99eee90d3a0ac518bf4 Mon Sep 17 00:00:00 2001 From: "J. Kirby Ross" Date: Wed, 28 Jan 2026 19:38:06 -0800 Subject: [PATCH 4/4] fix(sanitizer): recognize combined numeric short forms (-n10, -15) Add regex check for /^-n?\d+$/ to recognize git's numeric shorthand forms like -n10 and -15 (equivalent to -n 10 and -n 15). These are now allowed when -n is in the command's flag allowlist. --- src/domain/services/CommandSanitizer.js | 8 ++++++++ test/domain/services/CommandSanitizer.test.js | 10 ++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/domain/services/CommandSanitizer.js b/src/domain/services/CommandSanitizer.js index d2dd1a6..e2facfa 100644 --- a/src/domain/services/CommandSanitizer.js +++ b/src/domain/services/CommandSanitizer.js @@ -129,6 +129,14 @@ export default class CommandSanitizer { continue; } + // Handle numeric short forms: -n10, -15 (equivalent to -n 10, -n 15) + // These are valid for commands like 'log' that have -n in their allowlist + if (/^-n?\d+$/.test(arg)) { + if (allowlist.has('-n')) { + continue; // Numeric form allowed if -n is in allowlist + } + } + // Handle --flag=value format: extract just the flag portion const flagPart = arg.includes('=') ? arg.split('=')[0] : arg; diff --git a/test/domain/services/CommandSanitizer.test.js b/test/domain/services/CommandSanitizer.test.js index 374f50b..001cc2e 100644 --- a/test/domain/services/CommandSanitizer.test.js +++ b/test/domain/services/CommandSanitizer.test.js @@ -103,6 +103,16 @@ describe('CommandSanitizer', () => { it('allows log with no arguments', () => { expect(() => sanitizer.sanitize(['log'])).not.toThrow(); }); + + it('allows combined numeric short forms (-n10, -15)', () => { + // -n10 is equivalent to -n 10 + expect(() => sanitizer.sanitize(['log', '-n10', 'HEAD'])).not.toThrow(); + // -15 is equivalent to -n 15 (git shorthand) + expect(() => sanitizer.sanitize(['log', '-15', 'HEAD'])).not.toThrow(); + // Combined with other flags + expect(() => sanitizer.sanitize(['log', '-n5', '--format=%H', 'HEAD'])).not.toThrow(); + expect(() => sanitizer.sanitize(['log', '-3', '--oneline'])).not.toThrow(); + }); }); describe('other commands have no additional restrictions', () => {