feat(sanitizer): add per-command flag allowlists for show and log

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",

src/domain/services/CommandSanitizer.js

@@ -43,7 +43,8 @@ export default class CommandSanitizer {
     'check-attr',
     'init',
     'config',
-    'log'
+    'log',
+    'show'
   ]);
 
   /**
@@ -72,6 +73,81 @@ 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];
+
+      // 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;
+      }
+
+      // 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;
+
+      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
@@ -158,6 +234,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;

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);
   });
@@ -46,13 +54,161 @@ 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();
+      });
+
+      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', () => {
+      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();
+    });
+
+    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);
+    });
+  });
 });