From 1f9fb90d282a6fe4706f24cecbd0e11769fdeb0c Mon Sep 17 00:00:00 2001 From: Jhin Lee Date: Wed, 9 Sep 2026 08:41:15 -0400 Subject: [PATCH 1/2] test: exclude generated output from writer source scans --- .../tooling/safe_pr_head_update_test.dart | 184 +++++++++++++----- 1 file changed, 136 insertions(+), 48 deletions(-) diff --git a/test/unit/tooling/safe_pr_head_update_test.dart b/test/unit/tooling/safe_pr_head_update_test.dart index 7c4b248f..3ccf839d 100644 --- a/test/unit/tooling/safe_pr_head_update_test.dart +++ b/test/unit/tooling/safe_pr_head_update_test.dart @@ -10,18 +10,60 @@ import '../../../tool/git/safe_pr_head_update.dart'; const zeroOid = '0000000000000000000000000000000000000000'; +const generatedDirectoryNames = { + '.git', + '.dart_tool', + 'node_modules', + 'build', + '.docusaurus', +}; + bool isMaintainedMarkdown(String path) { final parts = path.replaceAll('\\', '/').split('/'); return parts.last.endsWith('.md') && - !parts.any( - const { - '.git', - '.dart_tool', - 'node_modules', - 'build', - '.docusaurus', - }.contains, - ); + !parts.any(generatedDirectoryNames.contains); +} + +bool isMaintainedWriterSource(String path) { + final normalized = path.replaceAll('\\', '/'); + final parts = normalized.split('/'); + if (parts.first == 'test' || parts.any(generatedDirectoryNames.contains)) { + return false; + } + return RegExp( + r'\.(dart|py|sh|bash|zsh|fish|ps1|cmd|bat|js|ts|ya?ml)$', + ).hasMatch(normalized) || + parts.last == 'package.json' || + parts.last == 'Makefile'; +} + +// Prune generated trees before listing their contents: site builds may replace +// them concurrently. Errors in maintained directories and files still propagate. +Iterable> maintainedFiles( + Directory root, [ + String prefix = '', +]) sync* { + for (final entity in root.listSync(followLinks: false)) { + final name = entity.uri.pathSegments.where((part) => part.isNotEmpty).last; + final path = '$prefix$name'; + if (entity is Directory) { + if (!generatedDirectoryNames.contains(name)) { + yield* maintainedFiles(entity, '$path/'); + } + } else if (entity is File) { + yield MapEntry(path, entity); + } + } +} + +Iterable> maintainedWriterSources( + Directory root, +) sync* { + for (final entry in maintainedFiles(root)) { + if (isMaintainedWriterSource(entry.key)) { + yield MapEntry(entry.key, entry.value.readAsStringSync()); + } + } } Future git(List args, Directory cwd) { @@ -1245,40 +1287,87 @@ void main() { } }, ); - test('all executable and task sources exclude bypass writers', () { - String repoPath(File file) { - final normalized = file.path.replaceAll('\\', '/'); - return normalized.startsWith('./') - ? normalized.substring(2) - : normalized; + test('writer ownership retains maintained executables and tasks', () { + for (final path in [ + '.github/workflows/sync_native_bindings.yml', + '.github/actions/check/action.yaml', + 'tool/git/safe_pr_head_update.dart', + 'scripts/build_chat_app_web.sh', + 'website/src/writer.ts', + 'website/package.json', + 'example/chat_app/Makefile', + 'website/build_tools/check.js', + ]) { + expect(isMaintainedWriterSource(path), isTrue, reason: path); + expect( + isMaintainedWriterSource(path.replaceAll('/', r'\')), + isTrue, + reason: path, + ); } + for (final path in [ + 'website/build/__server/assets/js/bundle.js', + 'website/.docusaurus/generated.js', + 'example/chat_app/build/web/main.dart.js', + 'website/node_modules/package/index.js', + '.dart_tool/generated.dart', + 'test/fixtures/writer.sh', + ]) { + expect(isMaintainedWriterSource(path), isFalse, reason: path); + expect( + isMaintainedWriterSource(path.replaceAll('/', r'\')), + isFalse, + reason: path, + ); + } + }); - final sourceFiles = Directory('.') - .listSync(recursive: true, followLinks: false) - .whereType() - .where((file) { - final path = repoPath(file); - if (path.startsWith('.git/') || - path.startsWith('build/') || - path.startsWith('.dart_tool/') || - path.startsWith('node_modules/') || - path.contains('/.dart_tool/') || - path.contains('/node_modules/') || - path.startsWith('test/')) { - return false; - } - return RegExp( - r'\.(dart|py|sh|bash|zsh|fish|ps1|cmd|bat|js|ts)$', - ).hasMatch(path) || - RegExp(r'\.ya?ml$').hasMatch(path) || - path.endsWith('/package.json') || - path == 'package.json' || - path.endsWith('/Makefile') || - path == 'Makefile'; - }); - for (final file in sourceFiles) { - final path = repoPath(file); - final content = file.readAsStringSync(); + test( + 'writer traversal prunes generated bytes and retains writer content', + () { + final root = Directory.systemTemp.createTempSync('writer-source-scan-'); + addTearDown(() => root.deleteSync(recursive: true)); + for (final path in [ + 'website/build/__server/assets/js/bundle.js', + 'website/.docusaurus/generated.js', + 'example/chat_app/build/web/main.dart.js', + ]) { + File('${root.path}/$path') + ..parent.createSync(recursive: true) + ..writeAsBytesSync([0xff]); + } + const paths = [ + '.github/workflows/writer.yml', + 'tool/git/writer.dart', + 'website/src/writer.ts', + 'website/package.json', + 'Makefile', + ]; + for (final path in paths) { + File('${root.path}/$path') + ..parent.createSync(recursive: true) + ..writeAsStringSync('git push origin HEAD'); + } + final sources = Map.fromEntries(maintainedWriterSources(root)); + expect(sources.keys, unorderedEquals(paths)); + expect(sources.values, everyElement(contains('git push origin HEAD'))); + }, + ); + + test('writer traversal propagates maintained file read errors', () { + final root = Directory.systemTemp.createTempSync('writer-source-error-'); + addTearDown(() => root.deleteSync(recursive: true)); + File('${root.path}/writer.dart').writeAsBytesSync([0xff]); + expect( + () => maintainedWriterSources(root).toList(), + throwsA(isA()), + ); + }); + + test('all executable and task sources exclude bypass writers', () { + for (final source in maintainedWriterSources(Directory('.'))) { + final path = source.key; + final content = source.value; expect( content, isNot(contains('peter-evans/create-pull-request')), @@ -1331,13 +1420,12 @@ void main() { } } - final markdownFiles = Directory('.') - .listSync(recursive: true, followLinks: false) - .whereType() - .where((file) => isMaintainedMarkdown(repoPath(file))); - for (final file in markdownFiles) { - final path = repoPath(file); - final content = file.readAsStringSync(); + final markdownFiles = maintainedFiles( + Directory('.'), + ).where((entry) => isMaintainedMarkdown(entry.key)); + for (final entry in markdownFiles) { + final path = entry.key; + final content = entry.value.readAsStringSync(); for (final line in content.split('\n')) { if (!RegExp(r'\bgit\s+push\b').hasMatch(line)) continue; if (path == 'doc/pr_branch_writer_inventory.md') { From e64cfb0777b54a48a7a1facfd07e52abbf985a77 Mon Sep 17 00:00:00 2001 From: Jhin Lee Date: Wed, 9 Sep 2026 08:50:31 -0400 Subject: [PATCH 2/2] test: protect generated directory pruning from bypass --- test/unit/tooling/safe_pr_head_update_test.dart | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/unit/tooling/safe_pr_head_update_test.dart b/test/unit/tooling/safe_pr_head_update_test.dart index 3ccf839d..8f7880e4 100644 --- a/test/unit/tooling/safe_pr_head_update_test.dart +++ b/test/unit/tooling/safe_pr_head_update_test.dart @@ -1348,6 +1348,12 @@ void main() { ..parent.createSync(recursive: true) ..writeAsStringSync('git push origin HEAD'); } + // Assert traversal itself, before the writer path filter: deleting the + // directory-pruning guard must expose the generated entries here. + expect( + maintainedFiles(root).map((entry) => entry.key), + unorderedEquals(paths), + ); final sources = Map.fromEntries(maintainedWriterSources(root)); expect(sources.keys, unorderedEquals(paths)); expect(sources.values, everyElement(contains('git push origin HEAD')));