fix: throw FormatException on malformed commit/tree output and add fuzz workflow

.github/workflows/fuzz.yaml

@@ -0,0 +1,16 @@
+name: Fuzz
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+  schedule:
+    - cron: '0 6 * * 1'
+
+jobs:
+  fuzz:
+    uses: kevmoo/fuzz.dart/.github/workflows/fuzz.yaml@main
+    with:
+      target: test/fuzz/git_parser_fuzz.dart
+      instrument-packages: string_scanner,source_span
+      max-total-time: 30

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## 2.3.3-wip
 
+- `Commit.parse`, `Commit.parseRawRevList`, and `TreeEntry.fromLsTree` now
+  consistently throw `FormatException` on malformed input.
 - Require Dart 3.9
 
 ## 2.3.2

lib/src/commit.dart

@@ -2,6 +2,7 @@ import 'dart:collection';
 
 import 'package:string_scanner/string_scanner.dart';
 
+import 'top_level.dart';
 import 'util.dart';
 
 /// Represents a Git commit object.
@@ -73,9 +74,11 @@ class Commit {
     // at all, or might be empty.
     scanner.scan(RegExp(r'\r?\n'));
 
-    var message = '';
+    String? commitSha;
+    String message;
 
     if (isRevParse) {
+      commitSha = _singleHeader(headers, 'commit', scanner, requireSha: true);
       final msgLines = <String>[];
 
       while (scanner.scan(RegExp(r'    ([^\r\n]*)(?:\r?\n|$)'))) {
@@ -85,28 +88,27 @@ class Commit {
         }
       }
 
-      if (msgLines.isNotEmpty) {
-        message = msgLines.join('\n');
-      }
+      message = msgLines.join('\n');
     } else {
-      message = scanner.rest;
+      if (headers.containsKey('commit')) {
+        scanner.error('Unexpected "commit" header.');
+      }
+      final rest = scanner.rest;
       scanner.position = scanner.string.length;
-      assert(message.endsWith('\n'));
-      final originalMessageLength = message.length;
-      message = message.trim();
-      // message should be trimmed by git, so the only diff after trim
-      // should be 1 character - the removed new line
-      assert(message.length + 1 == originalMessageLength);
+      if (!rest.endsWith('\n')) {
+        scanner.error('Commit message must end with a newline.');
+      }
+      message = rest.replaceFirst(RegExp(r'\r?\n$'), '');
     }
 
-    final treeSha = headers['tree']!.single;
-    final author = headers['author']!.single;
-    final committer = headers['committer']!.single;
-    final commitSha = headers.containsKey('commit')
-        ? headers['commit']!.single
-        : null;
+    final treeSha = _singleHeader(headers, 'tree', scanner, requireSha: true);
+    final author = _singleHeader(headers, 'author', scanner);
+    final committer = _singleHeader(headers, 'committer', scanner);
 
     final parents = headers['parent'] ?? [];
+    if (!parents.every(isValidSha)) {
+      scanner.error('Invalid SHA1 value in "parent" header.');
+    }
 
     final endSpot = scanner.position;
 
@@ -117,4 +119,24 @@ class Commit {
       commit: Commit._(treeSha, author, committer, message, content, parents),
     );
   }
+
+  static String _singleHeader(
+    Map<String, List<String>> headers,
+    String name,
+    StringScanner scanner, {
+    bool requireSha = false,
+  }) {
+    final values = headers[name];
+    if (values == null || values.isEmpty) {
+      scanner.error('Missing required "$name" header.');
+    }
+    if (values.length > 1) {
+      scanner.error('Duplicate "$name" header.');
+    }
+    final value = values.single;
+    if (requireSha && !isValidSha(value)) {
+      scanner.error('Invalid SHA1 value in "$name" header: "$value".');
+    }
+    return value;
+  }
 }

lib/src/tree_entry.dart

@@ -33,8 +33,10 @@ class TreeEntry {
   }
 
   factory TreeEntry.fromLsTree(String value) {
-    // TODO: should catch and re-throw a descriptive error
-    final match = _lsTreeRegEx.allMatches(value).single;
+    final match = _lsTreeRegEx.firstMatch(value);
+    if (match == null) {
+      throw FormatException('Could not parse ls-tree line.', value);
+    }
 
     return TreeEntry(match[1]!, match[2]!, match[3]!, match[4]!);
   }

test/fuzz/git_parser_fuzz.dart

@@ -0,0 +1,32 @@
+import 'dart:convert';
+import 'dart:typed_data';
+
+import 'package:git/git.dart';
+
+void fuzzTarget(Uint8List bytes) {
+  final input = utf8.decode(bytes, allowMalformed: true);
+
+  try {
+    Commit.parse(input);
+  } on FormatException {
+    // Expected on malformed commit object.
+  }
+
+  try {
+    Commit.parseRawRevList(input);
+  } on FormatException {
+    // Expected on malformed rev-list output.
+  }
+
+  try {
+    TreeEntry.fromLsTree(input);
+  } on FormatException {
+    // Expected on malformed ls-tree line.
+  }
+
+  try {
+    TreeEntry.fromLsTreeOutput(input);
+  } on FormatException {
+    // Expected on malformed ls-tree output.
+  }
+}

test/parse_test.dart

@@ -50,6 +50,170 @@ bcd1284d805951a16e765cea5b2273a464ee2d86''',
     check(result.first).has((e) => e.name, 'name').equals('.gitignore');
     check(result).every((e) => e.isNotNull());
   });
+
+  group('TreeEntry.fromLsTree malformed input', () {
+    for (final invalid in [
+      '',
+      '\n',
+      'invalid',
+      '100644 invalid bcd1284d805951a16e765cea5b2273a464ee2d86\tfile.txt',
+      '100644 blob invalidsha\tfile.txt',
+      '100644 blob bcd1284d805951a16e765cea5b2273a464ee2d86\t',
+    ]) {
+      test('throws FormatException on "$invalid"', () {
+        check(() => TreeEntry.fromLsTree(invalid)).throws<FormatException>();
+      });
+    }
+
+    test('fromLsTreeOutput throws FormatException on blank line', () {
+      check(() => TreeEntry.fromLsTreeOutput('\n\n')).throws<FormatException>();
+    });
+  });
+
+  group('Commit.parse and Commit.parseRawRevList', () {
+    const validSha = 'bcd1284d805951a16e765cea5b2273a464ee2d86';
+
+    test('Commit.parse parses valid commit and preserves whitespace', () {
+      final commit = Commit.parse(
+        'tree $validSha\n'
+        'author Alice <alice@example.com> 1700000000 +0000\n'
+        'committer Bob <bob@example.com> 1700000000 +0000\n'
+        '\n'
+        '  leading and trailing whitespace  \n',
+      );
+      check(commit.treeSha).equals(validSha);
+      check(commit.message).equals('  leading and trailing whitespace  ');
+    });
+
+    test('Commit.parse throws FormatException on missing headers', () {
+      check(() => Commit.parse('')).throws<FormatException>();
+      check(
+        () => Commit.parse(
+          'author Alice <a@b.c>\ncommitter Bob <a@b.c>\n\nmsg\n',
+        ),
+      ).throws<FormatException>();
+      check(
+        () => Commit.parse('tree $validSha\ncommitter Bob <a@b.c>\n\nmsg\n'),
+      ).throws<FormatException>();
+      check(
+        () => Commit.parse('tree $validSha\nauthor Alice <a@b.c>\n\nmsg\n'),
+      ).throws<FormatException>();
+    });
+
+    test('Commit.parse throws FormatException on duplicate headers', () {
+      check(
+        () => Commit.parse(
+          'tree $validSha\n'
+          'tree $validSha\n'
+          'author Alice <a@b.c>\n'
+          'committer Bob <a@b.c>\n\n'
+          'msg\n',
+        ),
+      ).throws<FormatException>();
+      check(
+        () => Commit.parse(
+          'tree $validSha\n'
+          'author Alice <a@b.c>\n'
+          'author Alice <a@b.c>\n'
+          'committer Bob <a@b.c>\n\n'
+          'msg\n',
+        ),
+      ).throws<FormatException>();
+      check(
+        () => Commit.parse(
+          'tree $validSha\n'
+          'author Alice <a@b.c>\n'
+          'committer Bob <a@b.c>\n'
+          'committer Bob <a@b.c>\n\n'
+          'msg\n',
+        ),
+      ).throws<FormatException>();
+    });
+
+    test('Commit.parse throws FormatException on unexpected commit header', () {
+      check(
+        () => Commit.parse(
+          'commit $validSha\n'
+          'tree $validSha\n'
+          'author Alice <a@b.c>\n'
+          'committer Bob <a@b.c>\n\n'
+          'msg\n',
+        ),
+      ).throws<FormatException>();
+    });
+
+    test('Commit.parse throws FormatException on missing trailing newline', () {
+      check(
+        () => Commit.parse(
+          'tree $validSha\n'
+          'author Alice <a@b.c>\n'
+          'committer Bob <a@b.c>\n\n'
+          'msg without trailing newline',
+        ),
+      ).throws<FormatException>();
+    });
+
+    test(
+      'Commit.parseRawRevList throws FormatException on missing/dupe headers',
+      () {
+        check(
+          () => Commit.parseRawRevList(
+            'tree $validSha\n'
+            'author Alice <a@b.c>\n'
+            'committer Bob <a@b.c>\n\n'
+            '    msg\n',
+          ),
+        ).throws<FormatException>();
+        check(
+          () => Commit.parseRawRevList(
+            'commit $validSha\n'
+            'commit $validSha\n'
+            'tree $validSha\n'
+            'author Alice <a@b.c>\n'
+            'committer Bob <a@b.c>\n\n'
+            '    msg\n',
+          ),
+        ).throws<FormatException>();
+        check(
+          () => Commit.parseRawRevList(
+            'commit $validSha\n'
+            'author Alice <a@b.c>\n'
+            'committer Bob <a@b.c>\n\n'
+            '    msg\n',
+          ),
+        ).throws<FormatException>();
+      },
+    );
+
+    test('throws FormatException on invalid SHA-1 in headers', () {
+      check(
+        () => Commit.parse(
+          'tree not-a-sha\n'
+          'author Alice <a@b.c>\n'
+          'committer Bob <a@b.c>\n\n'
+          'msg\n',
+        ),
+      ).throws<FormatException>();
+      check(
+        () => Commit.parse(
+          'tree $validSha\n'
+          'parent not-a-sha\n'
+          'author Alice <a@b.c>\n'
+          'committer Bob <a@b.c>\n\n'
+          'msg\n',
+        ),
+      ).throws<FormatException>();
+      check(
+        () => Commit.parseRawRevList(
+          'commit not-a-sha\n'
+          'tree $validSha\n'
+          'author Alice <a@b.c>\n'
+          'committer Bob <a@b.c>\n\n'
+          '    msg\n',
+        ),
+      ).throws<FormatException>();
+    });
+  });
 }
 
 const _showRefOutput = '''ff1c31c454c4128a98dcd610d203820eeeb91923 HEAD

test/tag_test.dart

@@ -15,6 +15,7 @@ void main() {
 
     await runGit([
       'tag',
+      '--no-sign',
       givenTagName,
       branchRef.sha,
     ], processWorkingDir: testDir.path);