From c3876e380ee3eac6d6de7adf489a3222e6062620 Mon Sep 17 00:00:00 2001 From: openhands Date: Sun, 1 Nov 2026 10:02:40 +0000 Subject: [PATCH 1/7] task6a: real 7z compression and extraction Add SevenZip wrapper (core/lib/src/seven_zip.dart) that shells out to an installed 7z/7zz via Process.runSync with an argv list, always passing -- before user paths. Wire it into Compressor for both compress and extract, replacing the two UnsupportedError throws, and add the CLI error/exit-code path. - atomic replace of an existing .7z (temp .part- + rename) - pre-extraction safety listing, symlink/escape guard, staging dir - encrypted archives fail fast; no 7-Zip -> clear failure message - OPENROM_7Z / PATH / Windows Program Files lookup, no shell - tests: stub argv+safety group, parseSevenZipListing units, real 7-Zip round trips (files, dirs, corrupted, encrypted, replace-not-append) - CI: install p7zip-full and set OPENROM_REQUIRE_7Z=1 on Test core Co-authored-by: openhands --- .github/workflows/quality.yml | 5 + core/bin/openrom.dart | 22 +- core/lib/openrom_core.dart | 1 + core/lib/src/compressor.dart | 97 +++- core/lib/src/seven_zip.dart | 489 ++++++++++++++++++ core/test/compressor_7z_test.dart | 831 ++++++++++++++++++++++++++++++ core/test/compressor_test.dart | 9 +- 7 files changed, 1437 insertions(+), 17 deletions(-) create mode 100644 core/lib/src/seven_zip.dart create mode 100644 core/test/compressor_7z_test.dart diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 6b414f3..ee1a743 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -60,6 +60,11 @@ jobs: working-directory: core run: dart analyze + - name: Install 7-Zip (core 7z support) + run: sudo apt-get update && sudo apt-get install -y p7zip-full + - name: Test core working-directory: core + env: + OPENROM_REQUIRE_7Z: "1" run: dart test diff --git a/core/bin/openrom.dart b/core/bin/openrom.dart index 9b48de6..252fc98 100644 --- a/core/bin/openrom.dart +++ b/core/bin/openrom.dart @@ -38,12 +38,12 @@ ArgParser buildParser() { ..addOption('convert', help: 'single file to convert (alias for --input)') ..addMultiOption( 'compress', - help: 'file(s) to compress into ZIP', + help: 'file(s) to compress into ZIP or 7Z', splitCommas: false, ) ..addMultiOption( 'extract', - help: 'file(s) to extract from ZIP', + help: 'file(s) to extract from ZIP or 7Z', splitCommas: false, ) ..addMultiOption( @@ -190,7 +190,13 @@ ArgParser buildParser() { return parser; } -Future main(List args) async { +Future main(List args) async { + // The Dart VM ignores an `int` returned from `main` (both JIT and AOT), so + // propagate the exit code explicitly. + exitCode = await _runCli(args); +} + +Future _runCli(List args) async { if (args.isEmpty) { print('OpenROM CLI — Universal ROM Toolkit\nUsage: openrom [options]'); return 0; @@ -372,8 +378,11 @@ Future main(List args) async { if (isJson) _jsonPrint({'type': 'done', 'success': true}); } else { failed++; - if (isJson) + if (isJson) { _jsonPrint({'type': 'done', 'success': false, 'error': job.error}); + } else { + print('❌ Compression failed: ${job.error ?? "unknown error"}'); + } } } return failed == 0 ? 0 : 1; @@ -416,8 +425,11 @@ Future main(List args) async { if (isJson) _jsonPrint({'type': 'done', 'success': true}); } else { failed++; - if (isJson) + if (isJson) { _jsonPrint({'type': 'done', 'success': false, 'error': job.error}); + } else { + print('❌ Extraction failed: ${job.error ?? "unknown error"}'); + } } } return failed == 0 ? 0 : 1; diff --git a/core/lib/openrom_core.dart b/core/lib/openrom_core.dart index ec48376..dc85b5d 100644 --- a/core/lib/openrom_core.dart +++ b/core/lib/openrom_core.dart @@ -5,6 +5,7 @@ export 'src/config.dart'; export 'src/detector.dart'; export 'src/converter.dart'; export 'src/compressor.dart'; +export 'src/seven_zip.dart'; export 'src/cue_generator.dart'; export 'src/bin_merger.dart'; export 'src/header_remover.dart'; diff --git a/core/lib/src/compressor.dart b/core/lib/src/compressor.dart index 5543e86..aecf89e 100644 --- a/core/lib/src/compressor.dart +++ b/core/lib/src/compressor.dart @@ -6,6 +6,8 @@ import 'dart:io'; import 'package:archive/archive_io.dart'; import 'package:path/path.dart' as p; +import 'seven_zip.dart'; + const Set excludedExtensions = { '.chd', '.cso', @@ -44,9 +46,28 @@ class CompressionJob { class Compressor { final void Function(String msg)? onLog; final void Function(CompressionJob job, double pct)? onProgress; + final SevenZip? _injectedSevenZip; + SevenZip? _resolvedSevenZip; bool _stopped = false; - Compressor({this.onLog, this.onProgress}); + Compressor({this.onLog, this.onProgress, SevenZip? sevenZip}) + : _injectedSevenZip = sevenZip; + + /// The 7-Zip wrapper to use, located lazily on first need. + SevenZip? get _sevenZip { + if (_injectedSevenZip != null) return _injectedSevenZip; + return _resolvedSevenZip ??= _locateSevenZip(); + } + + SevenZip? _locateSevenZip() { + final exe = SevenZip.locate(onLog: _log); + if (exe == null) { + _log('[7Z] No 7-Zip executable found (looked for 7zz/7z/7za/7zr and ' + 'OPENROM_7Z).'); + return null; + } + return SevenZip(exe, onLog: _log); + } void stop() { _stopped = true; @@ -76,9 +97,6 @@ class Compressor { bool compress(CompressionJob job) { _stopped = false; - if (job.format == '7z' || job.format == '.7z') { - throw UnsupportedError('7z format is currently not supported; use zip'); - } if (job.format == 'extract') { return extract(job); @@ -103,12 +121,14 @@ class Compressor { return true; } + final is7z = job.format == '7z' || job.format == '.7z'; + job.status = 'Compressing'; _updateProgress(job, 0.0); Directory(job.outputDir).createSync(recursive: true); try { - final success = _compressZip(job); + final success = is7z ? _compress7z(job) : _compressZip(job); if (success) { job.status = 'Done'; job.progress = 100.0; @@ -131,9 +151,6 @@ class Compressor { bool extract(CompressionJob job) { _stopped = false; - if (p.extension(job.filepath).toLowerCase() == '.7z') { - throw UnsupportedError('7z format is currently not supported; use zip'); - } if (!File(job.filepath).existsSync()) { job.status = 'Failed'; @@ -142,12 +159,14 @@ class Compressor { return false; } + final is7z = p.extension(job.filepath).toLowerCase() == '.7z'; + job.status = 'Extracting'; _updateProgress(job, 0.0); Directory(job.outputDir).createSync(recursive: true); try { - final success = _extractZip(job); + final success = is7z ? _extract7z(job) : _extractZip(job); if (success) { job.status = 'Done'; job.progress = 100.0; @@ -168,6 +187,66 @@ class Compressor { } } + /// 7z compression via the external 7-Zip binary. + /// + /// `Process.runSync` cannot stream, so progress only moves 0 -> 100 around + /// the single blocking invocation rather than tracking the archive as it is + /// written. (Documented limitation; a streamed variant is a later task.) + bool _compress7z(CompressionJob job) { + final sevenZip = _sevenZip; + if (sevenZip == null) { + job.error = sevenZipNotFoundMessage; + _log('[ERROR] ${job.error}'); + return false; + } + + final baseName = p.basename(job.filepath); + _log('[7Z] Compressing $baseName -> ${baseName}.7z (level ${job.level})'); + + final error = sevenZip.compress( + source: job.filepath, + outputDir: job.outputDir, + level: job.level, + ); + + if (error != null) { + job.error = error; + _log('[ERROR] $error'); + return false; + } + + _updateProgress(job, 100.0); + return true; + } + + /// 7z extraction via the external 7-Zip binary. + bool _extract7z(CompressionJob job) { + final sevenZip = _sevenZip; + if (sevenZip == null) { + job.error = sevenZipNotFoundMessage; + _log('[ERROR] ${job.error}'); + return false; + } + + _log( + '[7Z] Extracting ${p.basename(job.filepath)} -> ${job.outputDir}', + ); + + final error = sevenZip.extract( + archive: job.filepath, + outputDir: job.outputDir, + ); + + if (error != null) { + job.error = error; + _log('[ERROR] $error'); + return false; + } + + _updateProgress(job, 100.0); + return true; + } + bool _compressZip(CompressionJob job) { final baseName = p.basenameWithoutExtension(job.filepath); final outPath = p.join(job.outputDir, '$baseName.zip'); diff --git a/core/lib/src/seven_zip.dart b/core/lib/src/seven_zip.dart new file mode 100644 index 0000000..5fa1da8 --- /dev/null +++ b/core/lib/src/seven_zip.dart @@ -0,0 +1,489 @@ +// OpenROM — Universal ROM Compression Suite +// M5 Dev | GPL v3 + +import 'dart:io'; +import 'dart:math'; + +import 'package:path/path.dart' as p; + +/// Shared "no 7-Zip installed" failure message. +const String sevenZipNotFoundMessage = + '7-Zip executable not found. Install 7-Zip (7zz/7z) or set OPENROM_7Z.'; + +/// One entry parsed from `7z l -slt` output. +class SevenZipEntry { + final String path; + final String attributes; + final String method; + final String encrypted; + final String size; + + const SevenZipEntry({ + required this.path, + this.attributes = '', + this.method = '', + this.encrypted = '', + this.size = '', + }); + + /// True when the entry is a directory (`Attributes` starts with `D`). + bool get isDirectory => attributes.trimLeft().startsWith('D'); + + /// True when 7-Zip flagged the entry as encrypted. + bool get isEncrypted => encrypted.trim() == '+'; + + @override + String toString() => 'SevenZipEntry($path)'; +} + +/// The result of parsing a `7z l -slt` listing. +class SevenZipListing { + /// Entries after the `----------` separator (real archive members). + final List entries; + + /// True when the header block before the separator reported encryption. + final bool headerEncrypted; + + const SevenZipListing(this.entries, {this.headerEncrypted = false}); + + bool get isEmpty => entries.isEmpty; +} + +/// Parses the entry blocks of a `7z l -slt` listing. +/// +/// Pure and side-effect free, so it can be unit-tested with captured output. +/// Only blocks after the `----------` separator line are considered; the header +/// block that precedes it also carries a `Path =` line (the archive itself) +/// which is not an archive member. +SevenZipListing parseSevenZipListing(String output) { + final lines = output.split(RegExp(r'\r?\n')); + + // 1) Header block up to and including the separator line. + var i = 0; + var headerEncrypted = false; + for (; i < lines.length; i++) { + final t = lines[i].trim(); + if (RegExp(r'^-{3,}$').hasMatch(t)) { + i++; + break; + } + if (t.startsWith('Encrypted =') && + t.substring('Encrypted ='.length).trim() == '+') { + headerEncrypted = true; + } + } + + // 2) Entry blocks after the separator. + final entries = []; + String? path; + String attributes = ''; + String method = ''; + String encrypted = ''; + String size = ''; + + void flush() { + if (path != null && path!.isNotEmpty) { + entries.add(SevenZipEntry( + path: path!, + attributes: attributes, + method: method, + encrypted: encrypted, + size: size, + )); + } + path = null; + attributes = ''; + method = ''; + encrypted = ''; + size = ''; + } + + for (; i < lines.length; i++) { + final t = lines[i].trim(); + if (t.isEmpty || t == '----------') { + flush(); + continue; + } + final eq = t.indexOf(' = '); + if (eq < 0) continue; + final key = t.substring(0, eq).trim(); + final value = t.substring(eq + 3); + switch (key) { + case 'Path': + // A new `Path` starts a new block; flush the previous one. + if (path != null) flush(); + path = value; + break; + case 'Attributes': + attributes = value; + break; + case 'Method': + method = value; + break; + case 'Encrypted': + encrypted = value; + break; + case 'Size': + size = value; + break; + } + } + flush(); + + return SevenZipListing(entries, headerEncrypted: headerEncrypted); +} + +/// Thin, shell-free wrapper around an installed 7-Zip executable. +/// +/// Security properties: +/// * **No shell.** Every invocation goes through +/// `Process.runSync(exe, argvList, ...)`; there is no string building, no +/// `runInShell`, and no `sh -c`. +/// * **No option injection.** `--` always precedes user-controlled paths, so +/// a name such as `-evil.bin` can never be parsed as a 7-Zip switch. +/// * **No passwords.** Passwords are never passed (nor `-p`-style switches); +/// encrypted archives are rejected up front instead. +/// * **Strict success.** Only exit code 0 counts as success; exit 1 (warning) +/// is a failure carrying the stderr tail. +class SevenZip { + /// Absolute path (or PATH-resolvable name) of the 7-Zip executable. + final String executable; + + /// Optional sink for human-readable diagnostics. + final void Function(String message)? onLog; + + const SevenZip(this.executable, {this.onLog}); + + /// Level name -> `-mx` switch value. + static const Map _levelMap = { + 'fast': '1', + 'normal': '5', + 'ultra': '9', + }; + + /// Maps a UI level string to its `-mx` value, defaulting to normal. + static String compressionLevel(String level) => + _levelMap[level.toLowerCase()] ?? _levelMap['normal']!; + + /// Locates a usable 7-Zip executable, or returns `null`. + /// + /// Search order: + /// 1. `$OPENROM_7Z` (ignored with a log line when it is not an existing + /// file), + /// 2. `7zz`, `7z`, `7za`, `7zr` (plus `.exe` on Windows) on `PATH`, + /// 3. Windows only: `%ProgramFiles%\7-Zip\7z.exe` and the `(x86)` variant. + /// + /// No `which`/`where` subprocess is spawned: `PATH` entries are scanned + /// directly with `File.existsSync`. + static String? locate({ + Map? environment, + void Function(String message)? onLog, + }) { + final env = environment ?? Platform.environment; + + final override = env['OPENROM_7Z']; + if (override != null && override.isNotEmpty) { + if (File(override).existsSync()) { + return override; + } + onLog?.call( + '[7Z] OPENROM_7Z is set to "$override" but that file does not exist; ' + 'falling back to PATH lookup.', + ); + } + + final names = ['7zz', '7z', '7za', '7zr']; + final candidates = Platform.isWindows + ? [for (final n in names) '$n.exe', ...names] + : names; + + final pathVar = env['PATH'] ?? env['Path'] ?? ''; + final sep = Platform.isWindows ? ';' : ':'; + for (final dir in pathVar.split(sep)) { + if (dir.trim().isEmpty) continue; + for (final name in candidates) { + final full = p.join(dir, name); + if (File(full).existsSync()) return full; + } + } + + if (Platform.isWindows) { + for (final key in ['ProgramFiles', 'ProgramFiles(x86)']) { + final base = env[key]; + if (base == null || base.isEmpty) continue; + final full = p.join(base, '7-Zip', '7z.exe'); + if (File(full).existsSync()) return full; + } + } + + return null; + } + + /// True when [path] is a directory component that must be rejected. + static bool _isUnsafeEntryPath(String raw) { + if (raw.isEmpty) return true; + if (raw.contains('\u0000')) return true; + if (raw.startsWith('/') || raw.startsWith('\\')) return true; + // Windows drive letter, e.g. `C:` or `C:\x`. + if (RegExp(r'^[a-zA-Z]:').hasMatch(raw)) return true; + // Any `..` component, on either separator. + final parts = raw.split(RegExp(r'[\\/]+')); + if (parts.contains('..')) return true; + return false; + } + + /// Lists the entries of [archive] (thin wrapper used by tests/tools). + static SevenZipListing parseListing(String output) => + parseSevenZipListing(output); + + /// Lists [archive] and rejects it when any entry path is unsafe. + /// + /// Returns `null` on success, or a human-readable error message when the + /// archive is unsafe, encrypted, or cannot be read. + String? _safetyError(SevenZipListing listing) { + if (listing.headerEncrypted || listing.entries.any((e) => e.isEncrypted)) { + return 'encrypted archives are not supported'; + } + for (final entry in listing.entries) { + if (_isUnsafeEntryPath(entry.path)) { + return 'unsafe path in archive: ${entry.path}'; + } + } + return null; + } + + static String _tail(String text, [int lines = 20]) { + final all = text.split(RegExp(r'\r?\n')); + final trimmed = all.length > lines ? all.sublist(all.length - lines) : all; + return trimmed.join('\n').trim(); + } + + /// Heuristic: 7-Zip prompts for a password on stdout when a header-encrypted + /// archive is opened, and `Process.runSync` closes stdin so it fails fast. + static bool _looksEncrypted(String text) => + text.toLowerCase().contains('password'); + + bool get _executableExists => File(executable).existsSync(); + + /// Compresses [source] into a fresh `.7z` inside [outputDir]. + /// + /// [source] may be a file or a directory. Returns `null` on success, or an + /// error message on failure. Any partial archive is removed on failure and a + /// pre-existing `.7z` is left untouched. + String? compress({ + required String source, + required String outputDir, + required String level, + }) { + final absSource = p.canonicalize(source); + final baseName = p.basename(absSource); + final parent = p.dirname(absSource); + final finalPath = p.join(outputDir, '$baseName.7z'); + + // 7z appends to an existing archive, so always build a fresh temp file. + final tmpPath = p.join(outputDir, '$baseName.7z.part-$pid'); + + if (File(tmpPath).existsSync()) { + File(tmpPath).deleteSync(); + } + + if (!_executableExists) { + return sevenZipNotFoundMessage; + } + + final mx = compressionLevel(level); + if (!_levelMap.containsKey(level.toLowerCase())) { + onLog?.call('[7Z] Unknown level "$level"; using normal (-mx=$mx).'); + } + + final argv = [ + 'a', + '-t7z', + '-mx=$mx', + '-y', + '-bd', + '-bso0', + '--', + tmpPath, + baseName, + ]; + + ProcessResult result; + try { + result = Process.runSync(executable, argv, workingDirectory: parent); + } on ProcessException { + _cleanup(tmpPath); + return sevenZipNotFoundMessage; + } catch (e) { + _cleanup(tmpPath); + return 'Failed to launch 7-Zip: $e'; + } + + if (result.exitCode != 0) { + final detail = _tail('${result.stderr}\n${result.stdout}'); + _cleanup(tmpPath); + return '7-Zip failed (exit ${result.exitCode})' + '${detail.isEmpty ? '' : ': $detail'}'; + } + + if (!File(tmpPath).existsSync()) { + return '7-Zip reported success but produced no archive.'; + } + + try { + final existing = File(finalPath); + if (existing.existsSync()) existing.deleteSync(); + File(tmpPath).renameSync(finalPath); + } catch (e) { + _cleanup(tmpPath); + return 'Failed to finalise archive: $e'; + } + + return null; + } + + /// Extracts [archive] into [outputDir]. + /// + /// Returns `null` on success, or an error message on failure. Nothing is + /// moved into [outputDir] unless the archive passed the listing safety check + /// and extraction produced no links or escaping paths. + String? extract({required String archive, required String outputDir}) { + final absArchive = p.canonicalize(archive); + + if (!_executableExists) { + return sevenZipNotFoundMessage; + } + + ProcessResult listResult; + try { + listResult = Process.runSync( + executable, + ['l', '-slt', '-y', '--', absArchive], + ); + } on ProcessException { + return sevenZipNotFoundMessage; + } catch (e) { + return 'Failed to launch 7-Zip: $e'; + } + if (listResult.exitCode != 0) { + if (_looksEncrypted('${listResult.stdout}\n${listResult.stderr}')) { + return 'encrypted archives are not supported'; + } + final detail = _tail('${listResult.stderr}\n${listResult.stdout}'); + return '7-Zip could not read the archive (exit ${listResult.exitCode})' + '${detail.isEmpty ? '' : ': $detail'}'; + } + + final listing = parseListing('${listResult.stdout}'); + final safety = _safetyError(listing); + if (safety != null) return safety; + + final staging = Directory(p.join( + outputDir, + '.openrom-extract-${Random().nextInt(1 << 32).toRadixString(16)}', + )); + + try { + staging.createSync(recursive: true); + final stagingRoot = p.canonicalize(staging.path); + + ProcessResult extractResult; + try { + extractResult = Process.runSync( + executable, + ['x', '-y', '-bd', '-o$stagingRoot', '--', absArchive], + ); + } on ProcessException { + return sevenZipNotFoundMessage; + } catch (e) { + return 'Failed to launch 7-Zip: $e'; + } + + if (extractResult.exitCode != 0) { + final detail = + _tail('${extractResult.stderr}\n${extractResult.stdout}'); + return '7-Zip extraction failed (exit ${extractResult.exitCode})' + '${detail.isEmpty ? '' : ': $detail'}'; + } + + final guard = _stagingGuardError(stagingRoot); + if (guard != null) return guard; + + return _moveStaged(stagingRoot, p.canonicalize(outputDir)); + } catch (e) { + return 'Extraction failed: $e'; + } finally { + _deleteDir(staging); + } + } + + /// Rejects any link or escaping path inside [stagingRoot]. + String? _stagingGuardError(String stagingRoot) { + final prefix = stagingRoot.endsWith(p.separator) + ? stagingRoot + : stagingRoot + p.separator; + + final entities = Directory(stagingRoot).listSync( + recursive: true, + followLinks: false, + ); + + for (final entity in entities) { + if (entity is Link) { + return 'archive contains a symbolic link: ${entity.path}'; + } + String resolved; + try { + resolved = entity.resolveSymbolicLinksSync(); + } catch (_) { + continue; // broken link already rejected above + } + if (resolved != stagingRoot && !resolved.startsWith(prefix)) { + return 'archive entry escapes the extraction directory: ' + '${entity.path}'; + } + } + return null; + } + + /// Moves every file from [stagingRoot] into [outputDir]. + String? _moveStaged(String stagingRoot, String outputDir) { + final entities = Directory(stagingRoot).listSync( + recursive: true, + followLinks: false, + ); + + for (final entity in entities) { + if (entity is! File) continue; + final rel = p.relative(entity.path, from: stagingRoot); + final dest = p.join(outputDir, rel); + final destFile = File(dest); + final parentDir = Directory(p.dirname(dest)); + if (!parentDir.existsSync()) { + parentDir.createSync(recursive: true); + } + if (destFile.existsSync()) destFile.deleteSync(); + try { + entity.renameSync(dest); + } on FileSystemException { + // Cross-device or otherwise non-atomic: fall back to copy + delete. + entity.copySync(dest); + entity.deleteSync(); + } + } + return null; + } + + void _cleanup(String path) { + final f = File(path); + try { + if (f.existsSync()) f.deleteSync(); + } catch (_) {} + } + + void _deleteDir(Directory dir) { + try { + if (dir.existsSync()) dir.deleteSync(recursive: true); + } catch (_) {} + } +} diff --git a/core/test/compressor_7z_test.dart b/core/test/compressor_7z_test.dart new file mode 100644 index 0000000..19dbd15 --- /dev/null +++ b/core/test/compressor_7z_test.dart @@ -0,0 +1,831 @@ +// OpenROM — Universal ROM Compression Suite +// M5 Dev | GPL v3 + +import 'dart:io'; + +import 'package:crypto/crypto.dart'; +import 'package:openrom_core/openrom_core.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +/// ───────────────────────────────────────────────────────────────────────── +/// Stub 7-Zip helper. +/// +/// Generates a POSIX `/bin/sh` script that (a) appends every argv element to a +/// log file in a parseable form, and (b) mimics the subset of 7-Zip behaviour +/// the tests need. All configuration is baked into the script so no environment +/// plumbing is required. +/// ───────────────────────────────────────────────────────────────────────── + +const String _defaultCompressBody = r''' +archive="" +prev="" +for a in "$@"; do + if [ "$prev" = "--" ]; then archive="$a"; break; fi + prev="$a" +done +if [ -n "$archive" ]; then printf 'STUB7Z' > "$archive"; fi +'''; + +const String _defaultExtractBody = r''' +outdir="" +prev="" +for a in "$@"; do + case "$a" in + -o*) outdir="${a#-o}" ;; + esac + if [ "$prev" = "--" ]; then archive="$a"; fi + prev="$a" +done +mkdir -p "$outdir" +printf 'extracted' > "$outdir/out.bin" +'''; + +String _writeSevenZipStub( + Directory dir, { + required String logPath, + String? listing, + int listExit = 0, + int compressExit = 0, + int extractExit = 0, + String? compressBody, + String? extractBody, + String name = '7z', +}) { + final listingBlock = listing == null + ? '' + : "cat <<'OPENROM_LISTING_MARKER'\n$listing\nOPENROM_LISTING_MARKER\n"; + + final script = ''' +#!/bin/sh +LOG="$logPath" +cmd="\$1" +{ + printf 'CMD %s\\n' "\$cmd" + for a in "\$@"; do printf 'ARG %s\\n' "\$a"; done +} >> "\$LOG" + +case "\$cmd" in + l) + $listingBlock + exit $listExit + ;; + a) + ${compressBody ?? _defaultCompressBody} + echo "WARNING: stub warning" >&2 + echo "ERROR: stub compress failed" >&2 + exit $compressExit + ;; + x) + ${extractBody ?? _defaultExtractBody} + exit $extractExit + ;; +esac +exit 0 +'''; + + final path = p.join(dir.path, name); + File(path).writeAsStringSync(script); + Process.runSync('chmod', ['+x', path]); + return path; +} + +/// Parses the stub log into one list of argv elements per invocation. +List> _readStubLog(String logPath) { + final invocations = >[]; + for (final line in File(logPath).readAsLinesSync()) { + if (line.startsWith('CMD ')) { + invocations.add([line.substring(4)]); + } else if (line.startsWith('ARG ')) { + invocations.last.add(line.substring(4)); + } + } + return invocations; +} + +bool _anyInvocation(List> invocations, String cmd) => + invocations.any((argv) => argv.isNotEmpty && argv.first == cmd); + +String _sha256OfFile(String path) => + sha256.convert(File(path).readAsBytesSync()).toString(); + +/// A real captured `7z l -slt` listing (7-Zip 25.01) used as a fixture. +const String _realListingSample = ''' +7-Zip 25.01 (x64) : Copyright (c) 1999-2025 Igor Pavlov : 2025-08-03 + 64-bit locale=C.UTF-8 Threads:4 OPEN_MAX:1048576, ASM + +Scanning the drive for archives: +1 file, 225 bytes (1 KiB) + +Listing archive: sample.7z + +-- +Path = sample.7z +Type = 7z +Physical Size = 225 +Headers Size = 213 +Method = LZMA2:12 +Solid = + +Blocks = 1 + +---------- +Path = sub +Size = 0 +Packed Size = 0 +Modified = 2026-11-01 09:51:45.5048655 +Attributes = D drwxr-xr-x +CRC = +Encrypted = - +Method = +Block = + +Path = Pokémon (Europe).gba +Size = 1 +Packed Size = 12 +Modified = 2026-11-01 09:51:45.5048655 +Attributes = A -rw-r--r-- +CRC = 8CDC1683 +Encrypted = - +Method = LZMA2:12 +Block = 0 + +Path = a.txt +Size = 6 +Packed Size = +Modified = 2026-11-01 09:51:45.5048655 +Attributes = A -rw-r--r-- +CRC = 363A3020 +Encrypted = - +Method = LZMA2:12 +Block = 0 + +Path = sub/b.bin +Size = 1 +Packed Size = +Modified = 2026-11-01 09:51:45.5048655 +Attributes = A -rw-r--r-- +CRC = FBDB2615 +Encrypted = - +Method = LZMA2:12 +Block = 0 +'''; + +void main() { + final isPosix = !Platform.isWindows; + + // ── Group 1: stub-based security/argv tests ──────────────────────────── + group('7z stub (argv + safety)', () { + late Directory tmp; + late String logPath; + + setUp(() { + tmp = Directory.systemTemp.createTempSync('openrom_7z_stub_'); + logPath = p.join(tmp.path, 'stub.log'); + File(logPath).writeAsStringSync(''); + }); + + tearDown(() { + try { + if (tmp.existsSync()) tmp.deleteSync(recursive: true); + } catch (_) {} + }); + + test('compress argv: -- precedes paths, -mx/-t7z/-y present', () { + final stub = _writeSevenZipStub(tmp, logPath: logPath); + final src = Directory(p.join(tmp.path, 'src'))..createSync(); + final file = File(p.join(src.path, 'game.iso'))..writeAsStringSync('x'); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + + for (final entry + in {'fast': '-mx=1', 'normal': '-mx=5', 'ultra': '-mx=9'}.entries) { + File(logPath).writeAsStringSync(''); + final compressor = Compressor(sevenZip: SevenZip(stub)); + final job = CompressionJob( + filepath: file.path, + outputDir: out.path, + format: '7z', + level: entry.key, + ); + expect(compressor.compress(job), isTrue, reason: job.error); + + final invocations = _readStubLog(logPath); + final argv = invocations.singleWhere((a) => a.first == 'a'); + expect(argv, contains('-t7z')); + expect(argv, contains('-y')); + expect(argv, contains(entry.value)); + final dashDash = argv.indexOf('--'); + expect(dashDash, greaterThan(0)); + // Everything after `--` is a path. + expect(argv.sublist(dashDash + 1).length, 2); + expect(argv.sublist(dashDash + 1).last, 'game.iso'); + } + }); + + test('extract argv: -o is one element after the subcommand', () { + final stub = _writeSevenZipStub( + tmp, + logPath: logPath, + listing: _realListingSample, + ); + final archive = File(p.join(tmp.path, 'a.7z'))..writeAsStringSync('z'); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + + final compressor = Compressor(sevenZip: SevenZip(stub)); + final job = CompressionJob( + filepath: archive.path, + outputDir: out.path, + format: 'extract', + ); + expect(compressor.extract(job), isTrue, reason: job.error); + + final invocations = _readStubLog(logPath); + expect(_anyInvocation(invocations, 'l'), isTrue); + final argv = invocations.singleWhere((a) => a.first == 'x'); + final dashDash = argv.indexOf('--'); + expect(dashDash, greaterThan(0)); + expect(argv[dashDash + 1], archive.path); + final outArg = argv.singleWhere((a) => a.startsWith('-o')); + expect(outArg.contains(' '), isFalse); + expect(outArg, startsWith('-o')); + expect(argv.where((a) => a.startsWith('-o')).length, 1); + }); + + test('hostile file names stay one argv element and are not executed', () { + final stub = _writeSevenZipStub(tmp, logPath: logPath); + final src = Directory(p.join(tmp.path, 'src'))..createSync(); + const hostile = "a; touch PWNED \$(id) 'q'.bin"; + const dashName = '-evil.bin'; + File(p.join(src.path, hostile)).writeAsStringSync('1'); + File(p.join(src.path, dashName)).writeAsStringSync('2'); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + + final compressor = Compressor(sevenZip: SevenZip(stub)); + + // Compress the hostile-named file on its own. + final job = CompressionJob( + filepath: p.join(src.path, hostile), + outputDir: out.path, + format: '7z', + ); + expect(compressor.compress(job), isTrue, reason: job.error); + + final argv = _readStubLog(logPath).singleWhere((a) => a.first == 'a'); + final dashDash = argv.indexOf('--'); + expect(argv[dashDash + 1], endsWith('.7z.part-$pid')); + expect(argv[dashDash + 2], hostile); + // No shell interpretation happened anywhere. + expect(File(p.join(out.path, 'PWNED')).existsSync(), isFalse); + expect( + File(p.join(Directory.current.path, 'PWNED')).existsSync(), isFalse); + + // A name starting with `-` must arrive after `--` intact. + File(logPath).writeAsStringSync(''); + final job2 = CompressionJob( + filepath: p.join(src.path, dashName), + outputDir: out.path, + format: '7z', + ); + expect(compressor.compress(job2), isTrue, reason: job2.error); + final argv2 = _readStubLog(logPath).singleWhere((a) => a.first == 'a'); + final dd2 = argv2.indexOf('--'); + expect(argv2[dd2 + 2], dashName); + }); + + test('existing archive is replaced, never appended', () { + final stub = _writeSevenZipStub(tmp, logPath: logPath); + final src = Directory(p.join(tmp.path, 'src'))..createSync(); + final file = File(p.join(src.path, 'game.iso'))..writeAsStringSync('x'); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + final finalArchive = File(p.join(out.path, 'game.iso.7z')) + ..writeAsStringSync('OLD-ARCHIVE-CONTENT'); + + final compressor = Compressor(sevenZip: SevenZip(stub)); + final job = CompressionJob( + filepath: file.path, + outputDir: out.path, + format: '7z', + ); + expect(compressor.compress(job), isTrue, reason: job.error); + + // Stub wrote a fresh file; the old content is gone. + expect(finalArchive.readAsStringSync(), 'STUB7Z'); + // No leftover part file. + expect( + out.listSync().where((e) => e.path.contains('.part-')), + isEmpty, + ); + }); + + test('on stub failure the previous archive is intact and no .part remains', + () { + final stub = _writeSevenZipStub(tmp, logPath: logPath, compressExit: 1); + final src = Directory(p.join(tmp.path, 'src'))..createSync(); + final file = File(p.join(src.path, 'game.iso'))..writeAsStringSync('x'); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + final finalArchive = File(p.join(out.path, 'game.iso.7z')) + ..writeAsStringSync('OLD-ARCHIVE-CONTENT'); + + final compressor = Compressor(sevenZip: SevenZip(stub)); + final job = CompressionJob( + filepath: file.path, + outputDir: out.path, + format: '7z', + ); + expect(compressor.compress(job), isFalse); + expect(job.status, 'Failed'); + expect(finalArchive.readAsStringSync(), 'OLD-ARCHIVE-CONTENT'); + expect(out.listSync().where((e) => e.path.contains('.part-')), isEmpty); + }); + + for (final unsafe in ['../x', '/etc/x', r'C:\x', r'a\..\b']) { + test('unsafe listing entry "$unsafe" is rejected before extraction', () { + final listing = ''' +-- +Path = a.7z +Type = 7z + +---------- +Path = $unsafe +Size = 1 +Attributes = A -rw-r--r-- + +'''; + final stub = _writeSevenZipStub( + tmp, + logPath: logPath, + listing: listing, + ); + final archive = File(p.join(tmp.path, 'a.7z'))..writeAsStringSync('z'); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + + final compressor = Compressor(sevenZip: SevenZip(stub)); + final job = CompressionJob( + filepath: archive.path, + outputDir: out.path, + format: 'extract', + ); + expect(compressor.extract(job), isFalse); + expect(job.status, 'Failed'); + expect(job.error, contains('unsafe path')); + + final invocations = _readStubLog(logPath); + expect(_anyInvocation(invocations, 'l'), isTrue); + // Extraction must never have been attempted. + expect(_anyInvocation(invocations, 'x'), isFalse); + }); + } + + test('symlink created by extract is rejected; staging removed', () { + final stub = _writeSevenZipStub( + tmp, + logPath: logPath, + listing: _realListingSample, + extractBody: r''' +outdir="" +prev="" +for a in "$@"; do + case "$a" in + -o*) outdir="${a#-o}" ;; + esac + prev="$a" +done +mkdir -p "$outdir" +ln -s /etc/passwd "$outdir/evil-link" +''', + ); + final archive = File(p.join(tmp.path, 'a.7z'))..writeAsStringSync('z'); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + + final compressor = Compressor(sevenZip: SevenZip(stub)); + final job = CompressionJob( + filepath: archive.path, + outputDir: out.path, + format: 'extract', + ); + expect(compressor.extract(job), isFalse); + expect(job.error, contains('symbolic link')); + // Nothing moved into outputDir and no staging dir left behind. + expect(out.listSync(), isEmpty); + }); + + test('stub exit codes 1 and 2 fail with a stderr tail', () { + for (final code in [1, 2]) { + final stub = _writeSevenZipStub( + tmp, + logPath: logPath, + listing: _realListingSample, + extractExit: code, + extractBody: r''' +outdir="" +for a in "$@"; do case "$a" in -o*) outdir="${a#-o}" ;; esac; done +echo "WARNING: something went wrong line 1" >&2 +echo "ERROR: fatal 7z problem line 2" >&2 +''', + ); + final archive = File(p.join(tmp.path, 'a$code.7z')) + ..writeAsStringSync('z'); + final out = Directory(p.join(tmp.path, 'out$code'))..createSync(); + + final compressor = Compressor(sevenZip: SevenZip(stub)); + final job = CompressionJob( + filepath: archive.path, + outputDir: out.path, + format: 'extract', + ); + expect(compressor.extract(job), isFalse); + expect(job.status, 'Failed'); + expect(job.error, contains('ERROR: fatal 7z problem line 2')); + } + }); + + test('missing executable fails cleanly with the not-found message', () { + final missing = p.join(tmp.path, 'does-not-exist-7z'); + final src = File(p.join(tmp.path, 'game.iso'))..writeAsStringSync('x'); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + + final compressor = Compressor(sevenZip: SevenZip(missing)); + final job = CompressionJob( + filepath: src.path, + outputDir: out.path, + format: '7z', + ); + expect(compressor.compress(job), isFalse); + expect(job.status, 'Failed'); + expect(job.error, contains('7-Zip executable not found')); + }, skip: !isPosix); + + test('locate() ignores a non-existent OPENROM_7Z and finds PATH 7z', () { + final stub = _writeSevenZipStub(tmp, logPath: logPath, name: '7z'); + final dir = Directory(p.join(tmp.path, 'bin'))..createSync(); + File(p.join(dir.path, '7z')).writeAsStringSync( + File(stub).readAsStringSync(), + ); + Process.runSync('chmod', ['+x', p.join(dir.path, '7z')]); + + final logs = []; + final found = SevenZip.locate( + environment: { + 'OPENROM_7Z': p.join(tmp.path, 'nope-7z'), + 'PATH': dir.path, + }, + onLog: logs.add, + ); + expect(found, p.join(dir.path, '7z')); + expect(logs.join('\n'), contains('does not exist')); + + // An existing OPENROM_7Z wins. + final found2 = SevenZip.locate( + environment: {'OPENROM_7Z': stub, 'PATH': ''}, + ); + expect(found2, stub); + }, skip: !isPosix); + }); + + // ── parseSevenZipListing unit tests ──────────────────────────────────── + group('parseSevenZipListing', () { + test('parses a real 7-Zip 25.01 listing', () { + final listing = parseSevenZipListing(_realListingSample); + expect(listing.entries.map((e) => e.path), [ + 'sub', + 'Pokémon (Europe).gba', + 'a.txt', + 'sub/b.bin', + ]); + expect(listing.entries.first.isDirectory, isTrue); + expect(listing.entries[1].isDirectory, isFalse); + expect(listing.entries[1].size, '1'); + expect(listing.headerEncrypted, isFalse); + }); + + test('ignores the header Path line before the separator', () { + final listing = parseSevenZipListing(_realListingSample); + expect(listing.entries.any((e) => e.path == 'sample.7z'), isFalse); + }); + + test('handles paths with spaces and unicode', () { + const sample = ''' +-- +Path = x.7z +Type = 7z + +---------- +Path = Tony Hawk's Pro Skater 2 (USA) (Track 1).bin +Size = 10 +Attributes = A -rw-r--r-- + +Path = Pokémon (Europe).gba +Size = 20 +Attributes = A -rw-r--r-- + +'''; + final listing = parseSevenZipListing(sample); + expect(listing.entries.map((e) => e.path), [ + "Tony Hawk's Pro Skater 2 (USA) (Track 1).bin", + 'Pokémon (Europe).gba', + ]); + }); + + test('empty archive (header only) yields no entries', () { + const sample = ''' +-- +Path = empty.7z +Type = 7z + +---------- +'''; + final listing = parseSevenZipListing(sample); + expect(listing.entries, isEmpty); + expect(listing.isEmpty, isTrue); + }); + + test('header-only output with no separator yields no entries', () { + const sample = ''' +7-Zip 25.01 (x64) : Copyright (c) 1999-2025 Igor Pavlov : 2025-08-03 + +Listing archive: empty.7z + +-- +Path = empty.7z +Type = 7z +'''; + expect(parseSevenZipListing(sample).entries, isEmpty); + }); + + test('detects header encryption', () { + const sample = ''' +-- +Path = enc.7z +Type = 7z +Encrypted = + + +---------- +Path = secret.txt +Size = 5 +Encrypted = + +Method = 7zAES + +'''; + final listing = parseSevenZipListing(sample); + expect(listing.headerEncrypted, isTrue); + expect(listing.entries.single.isEncrypted, isTrue); + }); + + test('handles CRLF line endings', () { + final sample = _realListingSample.replaceAll('\n', '\r\n'); + final listing = parseSevenZipListing(sample); + expect(listing.entries.length, 4); + expect(listing.entries[1].path, 'Pokémon (Europe).gba'); + }); + }); + + // ── Group 2: real 7-Zip round-trip tests ─────────────────────────────── + final realExe = SevenZip.locate(); + final require7z = Platform.environment['OPENROM_REQUIRE_7Z'] == '1'; + final skipReal = realExe == null && !require7z; + final realSkipReason = + 'no 7-Zip executable found (set OPENROM_REQUIRE_7Z=1 to force failure)'; + + group('7z real round trip', () { + late Directory tmp; + late Compressor compressor; + + setUp(() { + if (realExe == null) return; + tmp = Directory.systemTemp.createTempSync('openrom_7z_real_'); + compressor = Compressor(sevenZip: SevenZip(realExe)); + }); + + tearDown(() { + try { + if (realExe != null && tmp.existsSync()) { + tmp.deleteSync(recursive: true); + } + } catch (_) {} + }); + + test('locate() finds a real 7-Zip on this host', () { + expect(realExe, isNotNull, + reason: 'OPENROM_REQUIRE_7Z=1 but no 7-Zip was found'); + }, skip: skipReal ? realSkipReason : null); + + for (final level in ['fast', 'normal', 'ultra']) { + test('round trip ($level): bytes identical, ultra beats the source', () { + final rnd = + List.generate(1024 * 1024, (i) => (i * 2654435761) & 0xFF); + final zeros = List.filled(1024 * 1024, 0); + final src = File(p.join(tmp.path, 'blob.bin')) + ..writeAsBytesSync([...rnd, ...zeros]); + final srcSha = _sha256OfFile(src.path); + + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + final job = CompressionJob( + filepath: src.path, + outputDir: out.path, + format: '7z', + level: level, + ); + expect(compressor.compress(job), isTrue, reason: job.error); + + final archive = p.join(out.path, 'blob.bin.7z'); + expect(File(archive).existsSync(), isTrue); + + final back = Directory(p.join(tmp.path, 'back'))..createSync(); + final ex = CompressionJob( + filepath: archive, + outputDir: back.path, + format: 'extract', + ); + expect(compressor.extract(ex), isTrue, reason: ex.error); + + final extracted = p.join(back.path, 'blob.bin'); + expect(File(extracted).existsSync(), isTrue); + expect(_sha256OfFile(extracted), srcSha); + + if (level == 'ultra') { + expect(File(archive).lengthSync(), lessThan(src.lengthSync())); + } + }, skip: skipReal ? realSkipReason : null); + } + + test('directory round trip with tricky names', () { + final src = Directory(p.join(tmp.path, 'disc'))..createSync(); + Directory(p.join(src.path, 'nested', 'deeper')) + .createSync(recursive: true); + File(p.join(src.path, 'nested', 'deeper', 'empty.dat')) + .writeAsStringSync(''); + Directory(p.join(src.path, 'emptydir')).createSync(); + File(p.join(src.path, "Tony Hawk's Pro Skater 2 (USA) (Track 1).bin")) + .writeAsStringSync('track1'); + File(p.join(src.path, 'Pokémon (Europe).gba')).writeAsStringSync('poke'); + + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + final job = CompressionJob( + filepath: src.path, + outputDir: out.path, + format: '7z', + ); + expect(compressor.compress(job), isTrue, reason: job.error); + + final archive = p.join(out.path, 'disc.7z'); + final back = Directory(p.join(tmp.path, 'back'))..createSync(); + final ex = CompressionJob( + filepath: archive, + outputDir: back.path, + format: 'extract', + ); + expect(compressor.extract(ex), isTrue, reason: ex.error); + + // Entries are rooted at the folder name, matching the ZIP behaviour. + final root = p.join(back.path, 'disc'); + expect( + File(p.join(root, "Tony Hawk's Pro Skater 2 (USA) (Track 1).bin")) + .readAsStringSync(), + 'track1', + ); + expect( + File(p.join(root, 'Pokémon (Europe).gba')).readAsStringSync(), + 'poke', + ); + expect( + File(p.join(root, 'nested', 'deeper', 'empty.dat')).existsSync(), + isTrue, + ); + }, skip: skipReal ? realSkipReason : null); + + test('source untouched by default; deleteSource removes it on success', () { + final src = File(p.join(tmp.path, 'keep.bin'))..writeAsStringSync('data'); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + + final job = CompressionJob( + filepath: src.path, + outputDir: out.path, + format: '7z', + ); + expect(compressor.compress(job), isTrue, reason: job.error); + expect(src.existsSync(), isTrue); + + final src2 = File(p.join(tmp.path, 'gone.bin')) + ..writeAsStringSync('data'); + final job2 = CompressionJob( + filepath: src2.path, + outputDir: out.path, + format: '7z', + deleteSource: true, + ); + expect(compressor.compress(job2), isTrue, reason: job2.error); + expect(src2.existsSync(), isFalse); + }, skip: skipReal ? realSkipReason : null); + + test('compressing an existing .7z reports Skipped (already compressed)', + () { + final src = File(p.join(tmp.path, 'already.7z')) + ..writeAsStringSync('not really an archive'); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + final job = CompressionJob( + filepath: src.path, + outputDir: out.path, + format: '7z', + ); + expect(compressor.compress(job), isTrue); + expect(job.status, 'Done'); + expect(job.error, contains('already compressed')); + }, skip: skipReal ? realSkipReason : null); + + test('corrupted archive fails with no partial files left behind', () { + final src = File(p.join(tmp.path, 'blob.bin')) + ..writeAsBytesSync(List.generate(4096, (i) => i & 0xFF)); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + final job = CompressionJob( + filepath: src.path, + outputDir: out.path, + format: '7z', + ); + expect(compressor.compress(job), isTrue, reason: job.error); + + final archive = File(p.join(out.path, 'blob.bin.7z')); + final bytes = archive.readAsBytesSync(); + archive.writeAsBytesSync(bytes.sublist(0, bytes.length ~/ 2)); + + final back = Directory(p.join(tmp.path, 'back'))..createSync(); + final ex = CompressionJob( + filepath: archive.path, + outputDir: back.path, + format: 'extract', + ); + expect(compressor.extract(ex), isFalse); + expect(ex.status, 'Failed'); + expect(back.listSync(), isEmpty); + }, skip: skipReal ? realSkipReason : null); + + test('encrypted archive fails fast with the encrypted message', () { + final src = Directory(p.join(tmp.path, 'enc'))..createSync(); + final f = File(p.join(src.path, 'secret.txt'))..writeAsStringSync('hi'); + final archive = p.join(tmp.path, 'enc.7z'); + final r = Process.runSync( + realExe!, + [ + 'a', + '-t7z', + '-pSECRET', + '-mhe=on', + '-y', + '-bd', + '--', + archive, + 'secret.txt' + ], + workingDirectory: src.path, + ); + expect(r.exitCode, 0, reason: '${r.stdout}\n${r.stderr}'); + expect(File(archive).existsSync(), isTrue); + + final back = Directory(p.join(tmp.path, 'back'))..createSync(); + final ex = CompressionJob( + filepath: archive, + outputDir: back.path, + format: 'extract', + ); + final sw = Stopwatch()..start(); + expect(compressor.extract(ex), isFalse); + sw.stop(); + expect(ex.error, contains('encrypted archives are not supported')); + expect(back.listSync(), isEmpty); + expect(sw.elapsed.inSeconds, lessThan(30)); + // `f` exists so the fixture is real, not vacuous. + expect(f.existsSync(), isTrue); + }, + skip: skipReal ? realSkipReason : null, + timeout: const Timeout(Duration(seconds: 60))); + + test('existing output .7z is replaced, not appended', () { + final src = Directory(p.join(tmp.path, 'src'))..createSync(); + File(p.join(src.path, 'game.bin')).writeAsStringSync('payload'); + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + + // Seed the final archive with a decoy entry. + final decoyDir = Directory(p.join(tmp.path, 'decoy'))..createSync(); + File(p.join(decoyDir.path, 'decoy.txt')).writeAsStringSync('decoy'); + final finalArchive = p.join(out.path, 'game.bin.7z'); + final r = Process.runSync( + realExe!, + ['a', '-t7z', '-y', '-bd', '--', finalArchive, 'decoy.txt'], + workingDirectory: decoyDir.path, + ); + expect(r.exitCode, 0); + + final job = CompressionJob( + filepath: p.join(src.path, 'game.bin'), + outputDir: out.path, + format: '7z', + ); + expect(compressor.compress(job), isTrue, reason: job.error); + + final listing = Process.runSync( + realExe, + ['l', '-slt', '-y', '--', finalArchive], + ); + final entries = parseSevenZipListing('${listing.stdout}').entries; + expect(entries.map((e) => e.path), ['game.bin']); + }, skip: skipReal ? realSkipReason : null); + }); +} diff --git a/core/test/compressor_test.dart b/core/test/compressor_test.dart index e301e10..e5997b8 100644 --- a/core/test/compressor_test.dart +++ b/core/test/compressor_test.dart @@ -5,10 +5,10 @@ import 'package:path/path.dart' as p; import 'package:test/test.dart'; void main() { - test('Compressor throws UnsupportedError when 7z format is requested', () { + test('Compressor skips an already-7z file instead of throwing', () { final tmpDir = Directory.systemTemp.createTempSync('compressor_7z_test_'); try { - final sampleFile = p.join(tmpDir.path, 'sample.rom'); + final sampleFile = p.join(tmpDir.path, 'sample.7z'); File(sampleFile).writeAsBytesSync([1, 2, 3, 4, 5]); final compressor = Compressor(); @@ -18,7 +18,10 @@ void main() { format: '7z', ); - expect(() => compressor.compress(job), throwsA(isA())); + final ok = compressor.compress(job); + expect(ok, isTrue); + expect(job.status, 'Done'); + expect(job.error, contains('already compressed')); } finally { tmpDir.deleteSync(recursive: true); } From 2dedf29a49224aa94a504d5d579f1c19af486e36 Mon Sep 17 00:00:00 2001 From: openhands Date: Sun, 1 Nov 2026 10:05:16 +0000 Subject: [PATCH 2/7] task6b: fix plugin executor argv and harden executable containment The manifest arg vector normally starts with {kind:executable}, which names the program itself. The executor was passing that token to Process.start, so every tool received its own path as its first argument. Drop it from the argv (Dart's arguments already exclude argv[0]) and reject an executable token anywhere but first in the manifest validator. Containment now also resolves real paths (resolveSymbolicLinksSync on the plugin dir and the executable), so a symlink inside the plugin directory pointing outside is refused. The doc comment matches the code again. Tests: argv[0] no longer leaks for v2 and legacy v1 plugins, an out-of-position executable token is a validation error, and a symlinked executable is refused. Co-authored-by: openhands --- core/lib/src/plugin/plugin_executor.dart | 47 ++++++- core/lib/src/plugin/plugin_manifest.dart | 7 +- core/test/plugin_test.dart | 166 +++++++++++++++++++++++ 3 files changed, 212 insertions(+), 8 deletions(-) diff --git a/core/lib/src/plugin/plugin_executor.dart b/core/lib/src/plugin/plugin_executor.dart index ad91d06..f29f5f8 100644 --- a/core/lib/src/plugin/plugin_executor.dart +++ b/core/lib/src/plugin/plugin_executor.dart @@ -83,7 +83,7 @@ class PluginExecutor { // Resolve the executable strictly inside the plugin directory. final pluginDir = p.normalize(p.absolute(p.join(pluginsDir, manifest.id))); - final executable = _resolveContainedPath(pluginDir, env.executable); + final executable = _joinUnder(pluginDir, env.executable); if (executable == null) { return PluginRunResult.failure( 'Plugin "${manifest.id}" executable path "${env.executable}" escapes ' @@ -98,6 +98,26 @@ class PluginExecutor { ); } + // Containment by *real* path: resolves `..` and symlinks, so a link inside + // the plugin directory pointing outside is refused. + final String realPluginDir; + final String realExecutable; + try { + realPluginDir = Directory(pluginDir).resolveSymbolicLinksSync(); + realExecutable = exeFile.resolveSymbolicLinksSync(); + } on FileSystemException catch (e) { + return PluginRunResult.failure( + 'Plugin "${manifest.id}" executable could not be resolved: ' + '${e.message}', + ); + } + if (!_isInside(realPluginDir, realExecutable)) { + return PluginRunResult.failure( + 'Plugin "${manifest.id}" executable path "${env.executable}" escapes ' + 'the plugin directory.', + ); + } + // Re-verify integrity right before execution. if (env.sha256.isNotEmpty) { final actual = sha256.convert(exeFile.readAsBytesSync()).toString(); @@ -131,12 +151,14 @@ class PluginExecutor { 'resolved for this job.', ); } + // The `executable` token names the program itself. `Process.start` + // already receives it as its own argument, so it must not also appear in + // the argv handed to the tool (Dart's `arguments` exclude argv[0]). + if (arg.kind == PluginArgKind.executable) continue; argv.add(resolved); } - onLog( - ' cmd: ${[p.basename(executable), ...argv.skip(1)].join(' ')}', - ); + onLog(' cmd: ${[p.basename(executable), ...argv].join(' ')}'); final successCodes = manifest.execution.successExitCodes; final timeout = manifest.execution.timeout; @@ -229,9 +251,12 @@ class PluginExecutor { if (buffer.isNotEmpty) onLine(buffer); } - /// Returns the absolute path of [relative] resolved under [root], or `null` - /// when it would escape the root (zip-slip / `..` / absolute path). - static String? _resolveContainedPath(String root, String relative) { + /// Returns the absolute path of [relative] resolved lexically under [root], + /// or `null` when it would escape the root (zip-slip / `..` / absolute path). + /// + /// This is a *lexical* check only. Callers that guard execution must also + /// verify the real path via [_isInside] after resolving symbolic links. + static String? _joinUnder(String root, String relative) { if (relative.trim().isEmpty) return null; if (p.isAbsolute(relative)) return null; final resolved = p.normalize(p.absolute(p.join(root, relative))); @@ -243,6 +268,14 @@ class PluginExecutor { return resolved; } + /// Whether the real path [realChild] lies strictly inside [realRoot]. + static bool _isInside(String realRoot, String realChild) { + if (realChild == realRoot) return true; + return realChild.startsWith( + realRoot.endsWith(p.separator) ? realRoot : realRoot + p.separator, + ); + } + static void _ensureExecutable(String path) { if (!Platform.isWindows) { try { diff --git a/core/lib/src/plugin/plugin_manifest.dart b/core/lib/src/plugin/plugin_manifest.dart index 2df98dc..a6a8703 100644 --- a/core/lib/src/plugin/plugin_manifest.dart +++ b/core/lib/src/plugin/plugin_manifest.dart @@ -595,10 +595,15 @@ class PluginManifest { 'Plugin binary for "$key" has a malformed "sha256" (expected 64 hex chars).'); } }); - for (final arg in execution.args) { + for (var i = 0; i < execution.args.length; i++) { + final arg = execution.args[i]; if (arg.kind == PluginArgKind.literal && arg.value.isEmpty) { errors.add('Plugin "execution.args" contains an empty literal.'); } + if (arg.kind == PluginArgKind.executable && i != 0) { + errors.add( + 'Plugin "execution.args" may only use the "executable" token as its first argument.'); + } } } diff --git a/core/test/plugin_test.dart b/core/test/plugin_test.dart index 9785a72..19961be 100644 --- a/core/test/plugin_test.dart +++ b/core/test/plugin_test.dart @@ -151,6 +151,21 @@ void main() { expect(errors.any((e) => e.contains('relative path')), isTrue); }); + test('rejects an "executable" token anywhere but the first argument', () { + final manifest = _v2Manifest(); + (manifest['execution'] as Map)['args'] = [ + {'kind': 'input'}, + {'kind': 'executable'}, + ]; + final parsed = PluginManifest.fromJson(manifest); + final errors = parsed.validate(); + expect( + errors.any((e) => e.contains('"executable" token as its first')), + isTrue, + reason: errors.join('|'), + ); + }); + test('normalises legacy v1 manifests (binaries + command template)', () { final legacy = PluginManifest.fromJson({ 'id': 'legacy-plugin', @@ -417,5 +432,156 @@ void main() { // The literal path must arrive intact as a single argv element. expect(logs.join('\n'), contains('a; rm -rf x;')); }); + + test(r'drops the leading {executable} token so $1 is the first real arg', + () async { + if (Platform.isWindows) return; // uses a POSIX shell script + + const script = '#!/bin/sh\nprintf "%s" "\$1"\nexit 0\n'; + final manifest = _v2Manifest( + id: 'argv0', + from: 'EEEE', + to: 'FFFF', + executable: 'tool.sh', + binaryContent: script, + ); + (manifest['execution'] as Map)['args'] = [ + {'kind': 'executable'}, + {'kind': 'input'}, + ]; + + final archive = _writeOrplug(tempDir, 'argv0.orplug', { + 'plugin.json': utf8.encode(jsonEncode(manifest)), + 'tool.sh': utf8.encode(script), + }); + final install = + await PluginManager.instance.installPluginDetailed(archive); + expect(install.success, isTrue, reason: install.error); + + final input = File(p.join(tempDir.path, 'in.eeee')) + ..writeAsStringSync('data'); + final conv = PluginRegistry.instance.getConversion('EEEE', 'FFFF')!; + final logs = []; + final result = await PluginManager.instance.executor.run( + conversion: conv, + inputPath: input.path, + outputPath: p.join(tempDir.path, 'out.ffff'), + outputDir: tempDir.path, + onLog: logs.add, + ); + + expect(result.success, isTrue, reason: result.error); + // The script prints `$1`; it must be the input path, not the executable. + expect(logs.any((l) => l.trim() == input.path), isTrue, + reason: 'argv[0] leaked into the plugin: ${logs.join('|')}'); + }); + + test('legacy v1 command plugin still passes the right argv', () async { + if (Platform.isWindows) return; + + const script = '#!/bin/sh\nprintf "%s" "\$1"\nexit 0\n'; + final manifest = { + 'id': 'legacy-argv', + 'name': 'Legacy Argv', + 'version': '1.0.0', + 'author': 'Legacy Author', + 'minCoreVersion': '3.0.0', + 'binaries': { + currentPlatformKey(): { + 'executable': 'tool.sh', + 'sha256': _sha256(script), + } + }, + 'extensions': [ + { + 'ext': '.gggg', + 'format': 'GGGG', + 'platform': 'Test', + 'color': '#fff' + } + ], + 'conversions': [ + { + 'from': 'HHHH', + 'to': 'IIII', + 'command': '{bin} "{in}"', + } + ], + }; + + final archive = _writeOrplug(tempDir, 'legacy.orplug', { + 'plugin.json': utf8.encode(jsonEncode(manifest)), + 'tool.sh': utf8.encode(script), + }); + final install = + await PluginManager.instance.installPluginDetailed(archive); + expect(install.success, isTrue, reason: install.error); + + final input = File(p.join(tempDir.path, 'in.hhhh')) + ..writeAsStringSync('data'); + final conv = PluginRegistry.instance.getConversion('HHHH', 'IIII')!; + final logs = []; + final result = await PluginManager.instance.executor.run( + conversion: conv, + inputPath: input.path, + outputPath: p.join(tempDir.path, 'out.iiii'), + outputDir: tempDir.path, + onLog: logs.add, + ); + + expect(result.success, isTrue, reason: result.error); + expect(logs.any((l) => l.trim() == input.path), isTrue, + reason: 'legacy argv[0] leaked: ${logs.join('|')}'); + }); + + test('refuses a symlinked executable pointing outside the plugin dir', + () async { + if (Platform.isWindows) return; // needs symlink support + + // The link target has the exact content the manifest declares, so the + // SHA-256 integrity check passes and only the containment check can + // reject it. + const script = '#!/bin/sh\necho PWNED\n'; + final manifest = _v2Manifest( + id: 'linker', + from: 'JJJJ', + to: 'KKKK', + executable: 'tool.sh', + binaryContent: script, + ); + final archive = _writeOrplug(tempDir, 'linker.orplug', { + 'plugin.json': utf8.encode(jsonEncode(manifest)), + 'tool.sh': utf8.encode(script), + }); + final install = + await PluginManager.instance.installPluginDetailed(archive); + expect(install.success, isTrue, reason: install.error); + + // Replace the installed binary with a link to an identical script + // outside the plugin directory. + final outside = File(p.join(tempDir.path, 'outside.sh')) + ..writeAsStringSync(script); + Process.runSync('chmod', ['+x', outside.path]); + final installed = File( + p.join(tempDir.path, 'plugins', 'linker', 'tool.sh'), + )..deleteSync(); + Link(installed.path).createSync(outside.path); + + final input = File(p.join(tempDir.path, 'in.jjjj')) + ..writeAsStringSync('data'); + final conv = PluginRegistry.instance.getConversion('JJJJ', 'KKKK')!; + final logs = []; + final result = await PluginManager.instance.executor.run( + conversion: conv, + inputPath: input.path, + outputPath: p.join(tempDir.path, 'out.kkkk'), + outputDir: tempDir.path, + onLog: logs.add, + ); + + expect(result.success, isFalse); + expect(result.error, contains('escapes the plugin directory')); + expect(logs.join('\n'), isNot(contains('PWNED'))); + }); }); } From d796eec8128f1cbfb7d9453926ee42f073f441bb Mon Sep 17 00:00:00 2001 From: openhands Date: Sun, 1 Nov 2026 10:06:27 +0000 Subject: [PATCH 3/7] task6c: CUE parser handles apostrophes, quotes, BOM and CRLF The FILE regex used [^"']+ and anchored on BINARY, so any name containing an apostrophe (Tony Hawk's Pro Skater 2 (USA) (Track 1).bin) failed to parse. Replace it with a non-greedy, end-anchored match of the whole FILE line, then strip one pair of matching surrounding quotes. Non-BINARY file types now raise a clear FormatException. Reading is done with a BOM strip and an explicit CRLF/CR split instead of readAsLinesSync. Tests: apostrophes, parentheses, unicode, lowercase file, CRLF, BOM, single quoted and unquoted names, a name with both quote styles, and a full merge round trip (bytes are the exact concatenation, cue offsets correct). Co-authored-by: openhands --- core/lib/src/bin_merger.dart | 33 +++++- core/test/bin_merger_test.dart | 182 +++++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+), 4 deletions(-) create mode 100644 core/test/bin_merger_test.dart diff --git a/core/lib/src/bin_merger.dart b/core/lib/src/bin_merger.dart index ce362e1..345827f 100644 --- a/core/lib/src/bin_merger.dart +++ b/core/lib/src/bin_merger.dart @@ -25,6 +25,18 @@ int _getSectorSize(String mode) { return 2352; } +/// Removes one pair of matching surrounding quotes (`"` or `'`) if present. +String _stripSurroundingQuotes(String value) { + if (value.length >= 2) { + final first = value[0]; + final last = value[value.length - 1]; + if ((first == '"' && last == '"') || (first == "'" && last == "'")) { + return value.substring(1, value.length - 1); + } + } + return value; +} + class CueTrackIndex { final int number; final String msf; @@ -53,8 +65,10 @@ List parseCue(String cuePath) { final tracks = []; String? currentFile; + // Group 1 is the raw name; group 2 the file type. The name is non-greedy and + // anchored to the end so apostrophes, parentheses and spaces all survive. final fileRegex = RegExp( - 'FILE\\s+["\']?([^"\']+)["\']?\\s+BINARY', + r'^\s*FILE\s+(.+?)\s+(BINARY|WAVE|MP3|AIFF|MOTOROLA)\s*$', caseSensitive: false, ); final trackRegex = RegExp( @@ -66,13 +80,24 @@ List parseCue(String cuePath) { caseSensitive: false, ); - final lines = File(cuePath).readAsLinesSync(); + var content = File(cuePath).readAsStringSync(); + if (content.startsWith('\uFEFF')) { + content = content.substring(1); + } + final lines = content.split(RegExp(r'\r\n|\r|\n')); for (final line in lines) { final lineStr = line.trim(); final fm = fileRegex.firstMatch(lineStr); if (fm != null) { - final relFile = fm.group(1)!; - currentFile = p.join(cueDir, relFile); + final rawName = _stripSurroundingQuotes(fm.group(1)!.trim()); + final fileType = fm.group(2)!.toUpperCase(); + if (fileType != 'BINARY') { + throw FormatException( + 'Unsupported FILE type "$fileType" in CUE (only BINARY is ' + 'supported): $cuePath', + ); + } + currentFile = p.join(cueDir, rawName); continue; } diff --git a/core/test/bin_merger_test.dart b/core/test/bin_merger_test.dart new file mode 100644 index 0000000..2b99cfc --- /dev/null +++ b/core/test/bin_merger_test.dart @@ -0,0 +1,182 @@ +// OpenROM — Universal ROM Compression Suite +// M5 Dev | GPL v3 + +import 'dart:convert'; +import 'dart:io'; + +import 'package:openrom_core/openrom_core.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +/// Writes a CUE file, optionally with CRLF line endings and a UTF-8 BOM. +String _writeCue( + Directory dir, + String name, + String content, { + bool crlf = false, + bool bom = false, +}) { + final body = crlf ? content.replaceAll('\n', '\r\n') : content; + final bytes = [ + if (bom) ...const [0xEF, 0xBB, 0xBF], + ...utf8.encode(body), + ]; + final path = p.join(dir.path, name); + File(path).writeAsBytesSync(bytes); + return path; +} + +void main() { + late Directory tmp; + + setUp(() { + tmp = Directory.systemTemp.createTempSync('openrom_cue_test_'); + }); + + tearDown(() { + try { + if (tmp.existsSync()) tmp.deleteSync(recursive: true); + } catch (_) {} + }); + + group('parseCue FILE lines', () { + test('parses an apostrophe in a quoted name', () { + final cue = _writeCue(tmp, 'a.cue', ''' +FILE "Tony Hawk's Pro Skater 2 (USA) (Track 1).bin" BINARY + TRACK 01 MODE1/2352 + INDEX 01 00:00:00 +'''); + final tracks = parseCue(cue); + expect(tracks.single.relFile, + "Tony Hawk's Pro Skater 2 (USA) (Track 1).bin"); + }); + + test('parses parentheses and unicode', () { + final cue = _writeCue(tmp, 'b.cue', ''' +FILE "Pokémon (Europe) (Track 1).bin" BINARY + TRACK 01 MODE1/2352 + INDEX 01 00:00:00 +'''); + expect(parseCue(cue).single.relFile, 'Pokémon (Europe) (Track 1).bin'); + }); + + test('accepts lowercase "file"', () { + final cue = _writeCue(tmp, 'c.cue', ''' +file "lower.bin" BINARY + TRACK 01 MODE1/2352 + INDEX 01 00:00:00 +'''); + expect(parseCue(cue).single.relFile, 'lower.bin'); + }); + + test('parses single-quoted names', () { + final cue = _writeCue(tmp, 'd.cue', ''' +FILE 'single quoted name.bin' BINARY + TRACK 01 MODE1/2352 + INDEX 01 00:00:00 +'''); + expect(parseCue(cue).single.relFile, 'single quoted name.bin'); + }); + + test('parses unquoted names', () { + final cue = _writeCue(tmp, 'e.cue', ''' +FILE bare-name.bin BINARY + TRACK 01 MODE1/2352 + INDEX 01 00:00:00 +'''); + expect(parseCue(cue).single.relFile, 'bare-name.bin'); + }); + + test("parses a name containing both ' and spaces", () { + final cue = _writeCue(tmp, 'f.cue', ''' +FILE "Bob's 'great' game.bin" BINARY + TRACK 01 MODE1/2352 + INDEX 01 00:00:00 +'''); + expect(parseCue(cue).single.relFile, "Bob's 'great' game.bin"); + }); + + test('handles CRLF line endings', () { + final cue = _writeCue( + tmp, + 'g.cue', + ''' +FILE "crlf name.bin" BINARY + TRACK 01 MODE1/2352 + INDEX 01 00:00:00 +''', + crlf: true); + expect(parseCue(cue).single.relFile, 'crlf name.bin'); + }); + + test('handles a UTF-8 BOM at the start of the file', () { + final cue = _writeCue( + tmp, + 'h.cue', + ''' +FILE "bom name.bin" BINARY + TRACK 01 MODE1/2352 + INDEX 01 00:00:00 +''', + bom: true); + final tracks = parseCue(cue); + expect(tracks.single.relFile, 'bom name.bin'); + }); + + test('rejects a non-BINARY file type with a clear error', () { + final cue = _writeCue(tmp, 'i.cue', ''' +FILE "audio.wav" WAVE + TRACK 01 AUDIO + INDEX 01 00:00:00 +'''); + expect( + () => parseCue(cue), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('WAVE'), + ), + ), + ); + }); + }); + + group('mergeBins round trip with tricky names', () { + test('concatenates bytes and computes offsets', () { + // 2352-byte sectors so sector arithmetic is exact. + final a = List.generate(2352, (i) => i & 0xFF); + final b = List.generate(2352 * 2, (i) => (i * 7) & 0xFF); + File(p.join(tmp.path, "Tony Hawk's Pro Skater 2 (USA) (Track 1).bin")) + .writeAsBytesSync(a); + File(p.join(tmp.path, 'Pokémon (Europe) (Track 2).bin')) + .writeAsBytesSync(b); + + final cue = _writeCue(tmp, 'disc.cue', ''' +FILE "Tony Hawk's Pro Skater 2 (USA) (Track 1).bin" BINARY + TRACK 01 MODE1/2352 + INDEX 01 00:00:00 +FILE "Pokémon (Europe) (Track 2).bin" BINARY + TRACK 02 MODE1/2352 + INDEX 01 00:00:00 +'''); + + final out = Directory(p.join(tmp.path, 'out'))..createSync(); + final result = mergeBins(cue, outputDir: out.path); + + // Merged BIN is the exact concatenation of the two tracks. + final merged = File(result.mergedBinPath).readAsBytesSync(); + expect(merged.length, a.length + b.length); + expect(merged.sublist(0, a.length), a); + expect(merged.sublist(a.length), b); + + final mergedCue = File(result.mergedCuePath).readAsStringSync(); + expect(mergedCue, contains('FILE "disc_merged.bin" BINARY')); + expect(mergedCue, contains('TRACK 01 MODE1/2352')); + expect(mergedCue, contains('INDEX 01 00:00:00')); + expect(mergedCue, contains('TRACK 02 MODE1/2352')); + // Track 2 starts after track 1's 2352-byte sector = 00:00:01. + expect(mergedCue, contains('INDEX 01 00:00:01')); + }); + }); +} From 788514becaf0a7921fe9f1f9d575570858e3aa42 Mon Sep 17 00:00:00 2001 From: openhands Date: Sun, 1 Nov 2026 10:07:38 +0000 Subject: [PATCH 4/7] task6d: sanitise ROM renamer target filenames suggestedFilename returned the raw DAT rom_name and it was passed straight to p.join(folder, ...), so a malicious DAT could write outside the folder. Add a pure top-level safeRomFilename(name, {required fallback}): replace / \ : * ? " < > | and control chars with _, drop trailing dots and spaces, prefix Windows reserved device names, cap at 200 UTF-8 bytes preserving the extension, and fall back when nothing usable remains. Ordinary names are unchanged. Use it in suggestedFilename and add a final guard that the canonical dirname of the new path equals the canonical folder. Tests: ordinary names unchanged, traversal/reserved/control/length/empty cases, and a malicious DAT name that stays inside the folder. Co-authored-by: openhands --- core/lib/src/rom_renamer.dart | 84 +++++++++++++++++++++++- core/test/rom_renamer_test.dart | 109 ++++++++++++++++++++++++++++++++ 2 files changed, 192 insertions(+), 1 deletion(-) diff --git a/core/lib/src/rom_renamer.dart b/core/lib/src/rom_renamer.dart index 1187a72..5a86c6b 100644 --- a/core/lib/src/rom_renamer.dart +++ b/core/lib/src/rom_renamer.dart @@ -10,6 +10,72 @@ import 'package:xml/xml_events.dart'; import 'config.dart' as config; +/// Characters that are unsafe in a file name on at least one supported OS. +final RegExp _unsafeFilenameChars = RegExp(r'[/\\:*?"<>|]'); +final RegExp _controlChars = RegExp(r'[\x00-\x1F\x7F]'); +final RegExp _windowsReserved = RegExp( + r'^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])(\..*)?$', + caseSensitive: false, +); + +/// Returns [name] as a single, safe file name component. +/// +/// Separators, reserved characters and control characters become `_`; leading +/// and trailing spaces and trailing dots are trimmed; Windows reserved device +/// names are prefixed with `_`; the result is capped at 200 UTF-8 bytes (the +/// extension is preserved). An empty result returns [fallback]. +String safeRomFilename(String name, {required String fallback}) { + var safe = + name.replaceAll(_unsafeFilenameChars, '_').replaceAll(_controlChars, '_'); + + safe = safe.trim(); + safe = safe.replaceAll(RegExp(r'[. ]+$'), ''); + + // After the replacements a pure-dots name (e.g. "..") collapses to nothing. + if (safe.isEmpty) return fallback; + + if (_windowsReserved.hasMatch(safe)) { + safe = '_$safe'; + } + + if (utf8.encode(safe).length > 200) { + safe = _truncateUtf8(safe, 200); + } + + return safe.isEmpty ? fallback : safe; +} + +/// Truncates [value] to at most [maxBytes] UTF-8 bytes, preserving the +/// extension when one is present. +String _truncateUtf8(String value, int maxBytes) { + final dot = value.lastIndexOf('.'); + final hasExt = dot > 0 && dot < value.length - 1; + final ext = hasExt ? value.substring(dot) : ''; + final stem = hasExt ? value.substring(0, dot) : value; + + final extBytes = utf8.encode(ext).length; + final budget = maxBytes - extBytes; + if (budget <= 0) { + return _truncateToBytes(value, maxBytes); + } + return _truncateToBytes(stem, budget) + ext; +} + +/// Truncates [value] so its UTF-8 encoding is at most [maxBytes] long, without +/// splitting a multi-byte character. +String _truncateToBytes(String value, int maxBytes) { + final out = StringBuffer(); + var used = 0; + for (final rune in value.runes) { + final ch = String.fromCharCode(rune); + final len = utf8.encode(ch).length; + if (used + len > maxBytes) break; + out.write(ch); + used += len; + } + return out.toString(); +} + final Uint32List _crc32Table = () { final table = Uint32List(256); for (int i = 0; i < 256; i++) { @@ -274,7 +340,7 @@ class RomScanResult { String get suggestedFilename { if (!matched || romName.isEmpty) return filename; - return romName; + return safeRomFilename(romName, fallback: filename); } } @@ -457,6 +523,22 @@ List renameRoms( success: false, ); + // Final guard: the resolved directory must be the folder itself. + try { + if (p.dirname(p.canonicalize(newPath)) != p.canonicalize(folder)) { + r.error = 'Refusing to rename outside the target folder: ' + '${scan.suggestedFilename}'; + renameResults.add(r); + if (onRenamed != null) onRenamed(r); + continue; + } + } catch (e) { + r.error = e.toString(); + renameResults.add(r); + if (onRenamed != null) onRenamed(r); + continue; + } + try { if (!dryRun) { if (File(newPath).existsSync() && diff --git a/core/test/rom_renamer_test.dart b/core/test/rom_renamer_test.dart index 1130e7b..4bd0b93 100644 --- a/core/test/rom_renamer_test.dart +++ b/core/test/rom_renamer_test.dart @@ -1,3 +1,4 @@ +import 'dart:convert'; import 'dart:io'; import 'package:openrom_core/openrom_core.dart'; @@ -142,4 +143,112 @@ void main() { tmpDir.deleteSync(recursive: true); } }); + + group('safeRomFilename', () { + const fallback = 'fallback.bin'; + + test('leaves ordinary names untouched', () { + for (final name in [ + "Tony Hawk's Pro Skater 2 (USA).bin", + 'Pokémon (Europe).gba', + 'Game! & Co, Vol. 1 (Japan).sfc', + 'Game [!].nes', + ]) { + expect(safeRomFilename(name, fallback: fallback), name); + } + }); + + test('replaces path separators and reserved characters', () { + expect(safeRomFilename('a/b.bin', fallback: fallback), 'a_b.bin'); + expect(safeRomFilename(r'a\b.bin', fallback: fallback), 'a_b.bin'); + expect(safeRomFilename('a:b.bin', fallback: fallback), 'a_b.bin'); + expect(safeRomFilename('a*b.bin', fallback: fallback), 'a_b.bin'); + expect(safeRomFilename('a?b.bin', fallback: fallback), 'a_b.bin'); + expect(safeRomFilename('a"b.bin', fallback: fallback), 'a_b.bin'); + expect(safeRomFilename('a.bin', fallback: fallback), 'a_b_.bin'); + expect(safeRomFilename('a|b.bin', fallback: fallback), 'a_b.bin'); + }); + + test('strips control characters including NUL', () { + expect(safeRomFilename('a\u0000b.bin', fallback: fallback), 'a_b.bin'); + expect(safeRomFilename('a\u007Fb.bin', fallback: fallback), 'a_b.bin'); + expect(safeRomFilename('a\tb.bin', fallback: fallback), 'a_b.bin'); + }); + + test('defuses traversal names', () { + expect(safeRomFilename('../evil.bin', fallback: fallback), '.._evil.bin'); + expect( + safeRomFilename(r'..\evil.bin', fallback: fallback), + '.._evil.bin', + ); + expect(safeRomFilename(r'C:\x.bin', fallback: fallback), 'C__x.bin'); + }); + + test('a name that is only dots becomes the fallback', () { + expect(safeRomFilename('..', fallback: fallback), fallback); + expect(safeRomFilename('.', fallback: fallback), fallback); + expect(safeRomFilename('...', fallback: fallback), fallback); + }); + + test('trims leading/trailing spaces and trailing dots', () { + expect( + safeRomFilename(' spaced.bin ', fallback: fallback), 'spaced.bin'); + expect(safeRomFilename('trailing...', fallback: fallback), 'trailing'); + expect(safeRomFilename(' x.bin. ', fallback: fallback), 'x.bin'); + }); + + test('prefixes Windows reserved device names', () { + expect(safeRomFilename('CON.bin', fallback: fallback), '_CON.bin'); + expect(safeRomFilename('nul', fallback: fallback), '_nul'); + expect(safeRomFilename('com1.gba', fallback: fallback), '_com1.gba'); + expect(safeRomFilename('LPT9', fallback: fallback), '_LPT9'); + // Not a reserved name. + expect(safeRomFilename('CONTENT.bin', fallback: fallback), 'CONTENT.bin'); + }); + + test('caps length at 200 UTF-8 bytes, keeping the extension', () { + final long = '${'a' * 300}.bin'; + final safe = safeRomFilename(long, fallback: fallback); + expect(utf8.encode(safe).length, lessThanOrEqualTo(200)); + expect(safe.endsWith('.bin'), isTrue); + }); + + test('empty result falls back', () { + expect(safeRomFilename('', fallback: fallback), fallback); + expect(safeRomFilename(' ', fallback: fallback), fallback); + // Separators are replaced, so "///" is a valid (if odd) name. + expect(safeRomFilename('///', fallback: fallback), '___'); + }); + }); + + group('renameRoms containment', () { + test('a malicious DAT name cannot escape the folder', () { + final tmpDir = Directory.systemTemp.createTempSync('rename_safety_'); + try { + final romPath = p.join(tmpDir.path, 'evil.bin'); + File(romPath).writeAsStringSync('data'); + + final scan = RomScanResult( + filepath: romPath, + filename: 'evil.bin', + matched: true, + romName: '../escaped.bin', + ); + + final results = renameRoms([scan], dryRun: false); + expect(results.single.success, isTrue); + // The file must stay inside the folder. + expect(results.single.newPath, p.join(tmpDir.path, '.._escaped.bin')); + expect(p.dirname(p.canonicalize(results.single.newPath)), + p.canonicalize(tmpDir.path)); + expect(File(romPath).existsSync(), isFalse); + expect( + File(p.join(tmpDir.path, '.._escaped.bin')).existsSync(), isTrue); + expect(File(p.join(tmpDir.path, '..', 'escaped.bin')).existsSync(), + isFalse); + } finally { + tmpDir.deleteSync(recursive: true); + } + }); + }); } From fe823497844eac3ae12656d25e6ccb3e5cc7841e Mon Sep 17 00:00:00 2001 From: openhands Date: Sun, 1 Nov 2026 10:08:06 +0000 Subject: [PATCH 5/7] task6e: README accuracy for 7Z support 7Z compression and extraction are supported through an installed 7-Zip (7zz/7z, or OPENROM_7Z) and are not bundled; ZIP needs nothing. Split the single 'ZIP / 7Z' roadmap line accordingly and add a short 7-Zip requirement note to the compression section with per-OS install hints and the OPENROM_7Z lookup order. No other README claims were changed. Co-authored-by: openhands --- README.md | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0afa36c..2f75886 100644 --- a/README.md +++ b/README.md @@ -68,11 +68,25 @@ Each release includes the **Flutter GUI** (`OpenROM`) and the **headless CLI** ( - **GDI Track Inspector** — Load and inspect GDI disc image track structure — track number, LBA offset, type (Audio/Data), sector size, and filename. ### 🗜️ Compression & Extraction -- **ZIP support** — Compress ROMs or entire folders with fast / normal / ultra presets. +- **ZIP support** — Compress ROMs or entire folders with fast / normal / ultra presets. Needs nothing installed. +- **7Z support** — Compress and extract `.7z` archives (fast / normal / ultra) through an installed 7-Zip binary (`7zz`/`7z`), or one pointed to by `OPENROM_7Z`. Not bundled. - **Smart skip** — Already-compressed formats (CHD, CSO, RVZ, 7Z…) are automatically skipped. -- **Batch extraction** — Extract ZIP archives with progress tracking. +- **Batch extraction** — Extract ZIP and 7Z archives with progress tracking. - **Delete source option** — Auto-clean source files after successful compression. +#### 7-Zip requirement +ZIP compression and extraction work with no external tools. 7Z compression and +extraction shell out to a 7-Zip binary: + +- **Linux** — `sudo apt-get install p7zip-full` (or your distro's `p7zip` / `7zip` package). +- **macOS** — `brew install sevenzip` (provides `7zz`). +- **Windows** — install 7-Zip, or rely on the default `%ProgramFiles%\7-Zip\7z.exe` lookup. + +The lookup order is `OPENROM_7Z` (an explicit path), then `7zz`, `7z`, `7za`, +`7zr` on `PATH`, then the Windows Program Files locations. If no binary is found, +7Z jobs fail with a clear message instead of throwing. Encrypted archives are +not supported and fail fast. + ### 🧹 ROM Header Removal - **Copier header detection and removal** for NES (iNES), SNES (SMC), Game Boy / GBC — with confidence rating (certain / likely). - **Double-strip protection** — SNES internal header validation prevents stripping clean ROMs twice. @@ -290,7 +304,8 @@ build_windows.bat - [x] CHD header parsing - [x] Theme system - [x] ROM Patcher — IPS, BPS, UPS, xdelta3, PPF, SSP and more (v2.5.0) -- [x] ZIP / 7Z compression & extraction +- [x] ZIP compression & extraction (no external tools needed) +- [x] 7Z compression & extraction (requires an installed 7-Zip — `7zz`/`7z`, or `OPENROM_7Z`; not bundled) - [x] ROM header removal — NES, SNES, GB/GBC - [x] M3U playlist generator - [x] NKit / WUD / WUX support (v2.8.0) From 2b50395a74f6dfb341ab96fdc34b301e2e0dd5cb Mon Sep 17 00:00:00 2001 From: openhands Date: Sun, 1 Nov 2026 10:12:13 +0000 Subject: [PATCH 6/7] task6f: release workflow input hygiene and fetch-binaries ref/arch safety Release.yml: validate the workflow_dispatch version against ^[0-9]+\.[0-9]+\.[0-9]+([.-][A-Za-z0-9.]+)?$ in a first step and expose it as job-level env RELEASE_VERSION (plus OPENROM_BINS_REF and MATRIX_ARCH), then use the shell/PowerShell variables inside every run: block. No ${{ }} remains inside any run: block. Adds a bins_ref workflow_dispatch input (default main). fetch-binaries.sh: OPENROM_BINS_REF (default main, validated) builds the default checksums URL; OPENROM_BINS_CHECKSUMS_URL still wins. Without --arch, refuse to fetch more than one distinct arch into a flat dest instead of silently overwriting same-named files (android stays single-arch). Adds scripts/test-fetch-binaries.sh exercising a local HTTP fixture: success, checksum mismatch (exit 4), ambiguous arch (exit 1), android single arch, ref honoured, invalid ref. actionlint and shellcheck are clean for the touched files. Co-authored-by: openhands --- .github/workflows/Release.yml | 91 ++++++++++++++++++++------- scripts/fetch-binaries.sh | 32 +++++++++- scripts/test-fetch-binaries.sh | 112 +++++++++++++++++++++++++++++++++ 3 files changed, 211 insertions(+), 24 deletions(-) create mode 100755 scripts/test-fetch-binaries.sh diff --git a/.github/workflows/Release.yml b/.github/workflows/Release.yml index bfefdb4..1737bc9 100644 --- a/.github/workflows/Release.yml +++ b/.github/workflows/Release.yml @@ -7,6 +7,10 @@ on: description: 'Version (e.g., 3.5.0)' required: true default: '3.5.0' + bins_ref: + description: 'OpenROM-bins ref used for the default checksums URL' + required: false + default: 'main' permissions: contents: write @@ -16,7 +20,18 @@ jobs: # ── Windows — Flutter GUI + Dart CLI ──────────────────── build-windows: runs-on: windows-2022 + env: + RELEASE_VERSION: ${{ github.event.inputs.version }} + OPENROM_BINS_REF: ${{ github.event.inputs.bins_ref }} steps: + - name: Validate release version + shell: bash + run: | + if [[ ! "$RELEASE_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+([.-][A-Za-z0-9.]+)?$ ]]; then + echo "Error: invalid version '$RELEASE_VERSION' (expected e.g. 3.5.0)" >&2 + exit 1 + fi + - uses: actions/checkout@v5 - name: Fetch binaries from SourceForge @@ -34,11 +49,11 @@ jobs: -y --no-progress - name: Set version - run: echo "${{ github.event.inputs.version }}" | Out-File -FilePath VERSION -Encoding utf8 -NoNewline + run: echo "$env:RELEASE_VERSION" | Out-File -FilePath VERSION -Encoding utf8 -NoNewline - name: Patch pubspec version run: | - $version = "${{ github.event.inputs.version }}" + $version = "$env:RELEASE_VERSION" (Get-Content gui/pubspec.yaml) ` -replace '^version: .*', "version: $version+1" | Set-Content gui/pubspec.yaml @@ -77,7 +92,7 @@ jobs: Copy-Item -Recurse -Force "themes/" "$releaseDir/themes/" Copy-Item -Recurse -Force "assets/" "$releaseDir/assets/" - $version = "${{ github.event.inputs.version }}" + $version = "$env:RELEASE_VERSION" $readme = "OpenROM v$version - Windows`n======================================================`nRUN THIS: gui.exe`n`nopenrom-core.exe is the backend engine, do NOT run it directly.`n`nDocs: https://github.com/M5Devs/OpenROM" Set-Content -Path "$releaseDir/README.txt" -Value $readme @@ -91,7 +106,7 @@ jobs: run: | $releaseDir = "gui/build/windows/x64/runner/Release" Compress-Archive -Path "$releaseDir/*" ` - -DestinationPath OpenROM-v${{ github.event.inputs.version }}_Windows_Portable.zip + -DestinationPath OpenROM-v${env:RELEASE_VERSION}_Windows_Portable.zip - name: Install NSIS and dependencies run: | @@ -124,7 +139,7 @@ jobs: - name: Build Windows installer run: | - $version = "${{ github.event.inputs.version }}" + $version = "$env:RELEASE_VERSION" makensis /DAPP_VERSION=$version openrom.nsi - name: Upload ZIP artifact @@ -142,7 +157,18 @@ jobs: # ── Linux — Flutter AppImage + Dart CLI ───────────────── build-linux: runs-on: ubuntu-22.04 + env: + RELEASE_VERSION: ${{ github.event.inputs.version }} + OPENROM_BINS_REF: ${{ github.event.inputs.bins_ref }} steps: + - name: Validate release version + shell: bash + run: | + if [[ ! "$RELEASE_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+([.-][A-Za-z0-9.]+)?$ ]]; then + echo "Error: invalid version '$RELEASE_VERSION' (expected e.g. 3.5.0)" >&2 + exit 1 + fi + - uses: actions/checkout@v5 - name: Fetch binaries from SourceForge @@ -164,11 +190,11 @@ jobs: patchelf ccache - name: Set version - run: echo "${{ github.event.inputs.version }}" > VERSION + run: echo "$RELEASE_VERSION" > VERSION - name: Patch pubspec version run: | - VERSION="${{ github.event.inputs.version }}" + VERSION="$RELEASE_VERSION" sed -i "s/^version: .*/version: ${VERSION}+1/" gui/pubspec.yaml - name: Setup Dart SDK @@ -240,13 +266,13 @@ jobs: run: | export APPIMAGE_EXTRACT_AND_RUN=1 ARCH=x86_64 ./appimagetool AppDir \ - OpenROM-v${{ github.event.inputs.version }}_Linux_x86_64.AppImage - chmod +x OpenROM-v${{ github.event.inputs.version }}_Linux_x86_64.AppImage + "OpenROM-v${RELEASE_VERSION}_Linux_x86_64.AppImage" + chmod +x "OpenROM-v${RELEASE_VERSION}_Linux_x86_64.AppImage" - name: Package Core CLI ZIP run: | set -e - VERSION="${{ github.event.inputs.version }}" + VERSION="$RELEASE_VERSION" CORE="release-core" rm -rf "$CORE" mkdir -p "$CORE/assets/linux/x86_64" @@ -278,7 +304,18 @@ jobs: # ── Linux ARM64 ────────────────────────────────────────── build-linux-arm64: runs-on: ubuntu-24.04-arm + env: + RELEASE_VERSION: ${{ github.event.inputs.version }} + OPENROM_BINS_REF: ${{ github.event.inputs.bins_ref }} steps: + - name: Validate release version + shell: bash + run: | + if [[ ! "$RELEASE_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+([.-][A-Za-z0-9.]+)?$ ]]; then + echo "Error: invalid version '$RELEASE_VERSION' (expected e.g. 3.5.0)" >&2 + exit 1 + fi + - uses: actions/checkout@v5 - name: Fetch binaries from SourceForge @@ -297,11 +334,11 @@ jobs: patchelf ccache - name: Set version - run: echo "${{ github.event.inputs.version }}" > VERSION + run: echo "$RELEASE_VERSION" > VERSION - name: Patch pubspec version run: | - VERSION="${{ github.event.inputs.version }}" + VERSION="$RELEASE_VERSION" sed -i "s/^version: .*/version: ${VERSION}+1/" gui/pubspec.yaml - name: Setup Dart SDK @@ -378,13 +415,13 @@ jobs: run: | export APPIMAGE_EXTRACT_AND_RUN=1 ARCH=aarch64 ./appimagetool AppDir \ - OpenROM-v${{ github.event.inputs.version }}_Linux_arm64.AppImage - chmod +x OpenROM-v${{ github.event.inputs.version }}_Linux_arm64.AppImage + "OpenROM-v${RELEASE_VERSION}_Linux_arm64.AppImage" + chmod +x "OpenROM-v${RELEASE_VERSION}_Linux_arm64.AppImage" - name: Package Core CLI ZIP run: | set -e - VERSION="${{ github.event.inputs.version }}" + VERSION="$RELEASE_VERSION" CORE="release-core" rm -rf "$CORE" mkdir -p "$CORE/assets/linux/arm64" @@ -423,19 +460,31 @@ jobs: - os: macos-15-intel arch: x86_64 runs-on: ${{ matrix.os }} + env: + RELEASE_VERSION: ${{ github.event.inputs.version }} + OPENROM_BINS_REF: ${{ github.event.inputs.bins_ref }} + MATRIX_ARCH: ${{ matrix.arch }} steps: + - name: Validate release version + shell: bash + run: | + if [[ ! "$RELEASE_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+([.-][A-Za-z0-9.]+)?$ ]]; then + echo "Error: invalid version '$RELEASE_VERSION' (expected e.g. 3.5.0)" >&2 + exit 1 + fi + - uses: actions/checkout@v5 - name: Fetch binaries from SourceForge run: | - bash scripts/fetch-binaries.sh --os macos --arch ${{ matrix.arch }} --dest assets/macos/${{ matrix.arch }} + bash scripts/fetch-binaries.sh --os macos --arch "$MATRIX_ARCH" --dest "assets/macos/$MATRIX_ARCH" - name: Set version - run: echo "${{ github.event.inputs.version }}" > VERSION + run: echo "$RELEASE_VERSION" > VERSION - name: Patch pubspec version run: | - VERSION="${{ github.event.inputs.version }}" + VERSION="$RELEASE_VERSION" sed -i "" "s/^version: .*/version: ${VERSION}+1/" gui/pubspec.yaml - name: Setup Dart SDK @@ -498,16 +547,16 @@ jobs: -srcfolder dmg_contents \ -ov \ -format UDZO \ - OpenROM-v${{ github.event.inputs.version }}_macOS_${{ matrix.arch }}.dmg + "OpenROM-v${RELEASE_VERSION}_macOS_$MATRIX_ARCH.dmg" - name: Package ZIP run: | mkdir release - cp OpenROM-v${{ github.event.inputs.version }}_macOS_${{ matrix.arch }}.dmg release/ + cp "OpenROM-v${RELEASE_VERSION}_macOS_$MATRIX_ARCH.dmg" release/ printf 'OpenROM - macOS\n======================================================\nRUN THIS: Open the .dmg and drag OpenROM to Applications.\n\nDocs: https://github.com/M5Devs/OpenROM\n' \ > release/README.txt cd release - zip -r ../OpenROM-v${{ github.event.inputs.version }}_macOS_${{ matrix.arch }}.zip . + zip -r "../OpenROM-v${RELEASE_VERSION}_macOS_$MATRIX_ARCH.zip" . - name: Upload artifact uses: actions/upload-artifact@v7 diff --git a/scripts/fetch-binaries.sh b/scripts/fetch-binaries.sh index cfa786f..a8d5c8d 100755 --- a/scripts/fetch-binaries.sh +++ b/scripts/fetch-binaries.sh @@ -9,6 +9,12 @@ # --arch x86_64 | arm64 | arm64-v8a (omit to fetch every arch published for the OS) # --dest destination directory (files are placed flat, by basename) # +# Environment: +# OPENROM_BINS_BASE_URL override the binary download base URL +# OPENROM_BINS_REF OpenROM-bins ref for the default checksums URL +# (default: main) +# OPENROM_BINS_CHECKSUMS_URL override the checksums URL entirely (wins) +# # Examples: # scripts/fetch-binaries.sh --os windows --arch x86_64 --dest assets/windows # scripts/fetch-binaries.sh --os linux --arch arm64 --dest assets/linux/arm64 @@ -16,7 +22,7 @@ # # Exit codes: # 0 success -# 1 bad arguments / missing dependency +# 1 bad arguments / missing dependency / ambiguous arch # 2 no matching binaries in checksums.sha256 # 3 download failed # 4 checksum verification mismatch @@ -24,14 +30,20 @@ set -euo pipefail BASE_URL="${OPENROM_BINS_BASE_URL:-https://downloads.sourceforge.net/project/openrom-binaries}" -CHECKSUMS_URL="${OPENROM_BINS_CHECKSUMS_URL:-https://raw.githubusercontent.com/M5Devs/OpenROM-bins/main/checksums.sha256}" + +BINS_REF="${OPENROM_BINS_REF:-main}" +if [[ ! "$BINS_REF" =~ ^[A-Za-z0-9._/-]{1,100}$ ]]; then + echo "Error: invalid OPENROM_BINS_REF: $BINS_REF" >&2 + exit 1 +fi +CHECKSUMS_URL="${OPENROM_BINS_CHECKSUMS_URL:-https://raw.githubusercontent.com/M5Devs/OpenROM-bins/${BINS_REF}/checksums.sha256}" OS="" ARCH="" DEST="" usage() { - sed -n '2,23p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' + sed -n '2,28p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' exit 1 } @@ -98,6 +110,20 @@ if [ "$count" -eq 0 ]; then exit 2 fi +# Without --arch, a destination holds files flat by basename. Fetching two +# architectures at once would silently overwrite same-named files, so refuse. +if [ -z "$ARCH" ]; then + arch_count=$(awk '{ n=split($2, parts, "/"); print parts[2] }' "$tmp_entries" \ + | sort -u | wc -l | tr -d ' ') + if [ "$arch_count" -gt 1 ]; then + arch_list=$(awk '{ n=split($2, parts, "/"); print parts[2] }' "$tmp_entries" \ + | sort -u | paste -sd ', ' -) + echo "Error: $OS publishes $arch_count architectures ($arch_list);" >&2 + echo " pass --arch to choose one (they share a flat dest)." >&2 + exit 1 + fi +fi + echo "Fetching $count binaries for $OS/${ARCH:-*} into $DEST" while IFS= read -r entry; do diff --git a/scripts/test-fetch-binaries.sh b/scripts/test-fetch-binaries.sh new file mode 100755 index 0000000..646f4bb --- /dev/null +++ b/scripts/test-fetch-binaries.sh @@ -0,0 +1,112 @@ +#!/usr/bin/env bash +# test-fetch-binaries.sh — exercise scripts/fetch-binaries.sh against a local +# HTTP fixture. Covers: success, checksum mismatch (exit 4), multiple archs +# without --arch (exit 1), android single arch, and OPENROM_BINS_REF handling. +# +# Usage: scripts/test-fetch-binaries.sh +set -uo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SCRIPT="$REPO_ROOT/scripts/fetch-binaries.sh" + +WORK="$(mktemp -d)" +SERVER_PID="" +# shellcheck disable=SC2317 # invoked indirectly via the EXIT trap +cleanup() { + if [ -n "$SERVER_PID" ]; then + kill "$SERVER_PID" 2>/dev/null + fi + rm -rf "$WORK" +} +trap cleanup EXIT + +failures=0 +check() { + local desc="$1" expected="$2" actual="$3" + if [ "$expected" = "$actual" ]; then + echo "ok - $desc (exit $actual)" + else + echo "FAIL - $desc (expected exit $expected, got $actual)" + failures=$((failures + 1)) + fi +} + +# ── Fixture ────────────────────────────────────────────────────────────── +FIX="$WORK/fixture" +mkdir -p "$FIX/linux/x86_64" "$FIX/linux/arm64" \ + "$FIX/windows/x86_64" "$FIX/android/arm64-v8a" + +printf 'A' > "$FIX/linux/x86_64/tool-a" +printf 'B' > "$FIX/linux/arm64/tool-a" +printf 'C' > "$FIX/android/arm64-v8a/tool-c" +printf 'W' > "$FIX/windows/x86_64/wtool" + +sha() { sha256sum "$1" | awk '{print $1}'; } + +{ + echo "$(sha "$FIX/linux/x86_64/tool-a") linux/x86_64/tool-a" + echo "$(sha "$FIX/linux/arm64/tool-a") linux/arm64/tool-a" + echo "$(sha "$FIX/android/arm64-v8a/tool-c") android/arm64-v8a/tool-c" + # Deliberately wrong hash to trigger exit 4. + echo "0000000000000000000000000000000000000000000000000000000000000000 windows/x86_64/wtool" +} > "$FIX/checksums.sha256" + +PORT=$(( (RANDOM % 20000) + 20000 )) +python3 -m http.server "$PORT" --directory "$FIX" >/dev/null 2>&1 & +SERVER_PID=$! +sleep 1 + +export OPENROM_BINS_BASE_URL="http://127.0.0.1:$PORT" +export OPENROM_BINS_CHECKSUMS_URL="http://127.0.0.1:$PORT/checksums.sha256" + +echo "── success: linux/x86_64 ──────────────────────────────" +bash "$SCRIPT" --os linux --arch x86_64 --dest "$WORK/out1" >/dev/null 2>&1 +check "success fetch" 0 $? +[ -f "$WORK/out1/tool-a" ] && echo "ok - tool-a present" +if [ ! -f "$WORK/out1/tool-a" ]; then + echo "FAIL - tool-a missing" + failures=$((failures + 1)) +fi + +echo "── wrong hash -> exit 4 ───────────────────────────────" +bash "$SCRIPT" --os windows --arch x86_64 --dest "$WORK/out2" >/dev/null 2>&1 +check "checksum mismatch" 4 $? +[ -f "$WORK/out2/wtool" ] \ + && { echo "FAIL - mismatched file kept"; failures=$((failures + 1)); } \ + || echo "ok - mismatched file removed" + +echo "── multiple archs without --arch -> exit 1 ────────────" +bash "$SCRIPT" --os linux --dest "$WORK/out3" >/dev/null 2>&1 +check "ambiguous arch" 1 $? + +echo "── android single arch without --arch -> exit 0 ───────" +bash "$SCRIPT" --os android --dest "$WORK/out4" >/dev/null 2>&1 +check "android single arch" 0 $? +[ -f "$WORK/out4/tool-c" ] && echo "ok - tool-c present" +if [ ! -f "$WORK/out4/tool-c" ]; then + echo "FAIL - tool-c missing" + failures=$((failures + 1)) +fi + +echo "── OPENROM_BINS_REF is used in the default URL ────────" +unset OPENROM_BINS_CHECKSUMS_URL +out=$(OPENROM_BINS_REF=custom-ref bash "$SCRIPT" --os android --dest "$WORK/out5" 2>&1) +echo "$out" | head -1 +if echo "$out" | grep -q "OpenROM-bins/custom-ref/checksums.sha256"; then + echo "ok - custom ref appears in checksums URL" +else + echo "FAIL - custom ref not used" + failures=$((failures + 1)) +fi + +echo "── invalid OPENROM_BINS_REF -> exit 1 ─────────────────" +OPENROM_BINS_REF='bad ref!' bash "$SCRIPT" --os android --dest "$WORK/out6" >/dev/null 2>&1 +check "invalid ref" 1 $? + +echo +if [ "$failures" -eq 0 ]; then + echo "ALL CHECKS PASSED" + exit 0 +fi +echo "$failures CHECK(S) FAILED" +exit 1 From b937f3590acb7ffa0afed289b41d423c0b7395aa Mon Sep 17 00:00:00 2001 From: openhands Date: Sun, 1 Nov 2026 10:15:11 +0000 Subject: [PATCH 7/7] task6d: make the renamer containment test hermetic The malicious-name test used Directory.systemTemp directly, so a stray ../escaped.bin next to it (e.g. left by another run) could make the canonical-dirname assertion flake. Run inside a nested per-test folder so '..' resolves under the unique base dir. Co-authored-by: openhands --- core/test/rom_renamer_test.dart | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/test/rom_renamer_test.dart b/core/test/rom_renamer_test.dart index 4bd0b93..ebe11e7 100644 --- a/core/test/rom_renamer_test.dart +++ b/core/test/rom_renamer_test.dart @@ -223,7 +223,10 @@ void main() { group('renameRoms containment', () { test('a malicious DAT name cannot escape the folder', () { - final tmpDir = Directory.systemTemp.createTempSync('rename_safety_'); + // Use a nested dir so that a stray ".." resolves inside the unique + // per-test base dir rather than the shared system temp dir. + final baseDir = Directory.systemTemp.createTempSync('rename_safety_'); + final tmpDir = Directory(p.join(baseDir.path, 'folder'))..createSync(); try { final romPath = p.join(tmpDir.path, 'evil.bin'); File(romPath).writeAsStringSync('data'); @@ -247,7 +250,7 @@ void main() { expect(File(p.join(tmpDir.path, '..', 'escaped.bin')).existsSync(), isFalse); } finally { - tmpDir.deleteSync(recursive: true); + baseDir.deleteSync(recursive: true); } }); });