Merge pull request #10719 from WhiteAssassins/feat/patch-advisor-list-verified

feat(tools): list verified patch catalog entries

core/relinker/relinker/tests/test_patch_advisor.py

@@ -25,28 +25,43 @@ def main():
         metadata = root / "param.json"
         metadata.write_text(json.dumps({"titleId": "TEST00001", "contentVersion": "01.000.000"}))
 
-        def advise(digest, patches=("patch.toml",)):
+        def advise(digest, patches=("patch.toml",), *options):
             (catalog / "index.json").write_text(json.dumps({"format": 1, "entries": [{
                 "titleId": "TEST00001", "contentVersion": "01.000.000", "patch": name,
                 "targets": {"eboot.bin": digest}
             } for name in patches]}))
             return subprocess.run([sys.executable, str(advisor), "--catalog", str(catalog),
                                    "--param-json", str(metadata), "--input", str(source),
-                                   "--relinker", f"./{relinker.name}", "--skip-sce-module"],
+                                   "--relinker", f"./{relinker.name}", "--skip-sce-module", *options],
                                   cwd=relinker.parent, capture_output=True, text=True, timeout=30)
 
-        result = advise(hashlib.sha256(image).hexdigest())
+        digest = hashlib.sha256(image).hexdigest()
+        result = advise(digest)
         assert result.returncode == 0 and f"Recommended: {patch}" in result.stdout, (result.stdout, result.stderr)
+        second = catalog / "second patch.toml"
+        second.write_text(patch_text(address, expected, "flip_bit"))
+        result = advise(digest, ("patch.toml", second.name))
+        assert result.returncode == 1 and "multiple verified catalog entries match; use --list-verified to see them" in result.stdout, (result.stdout, result.stderr)
+        result = advise(digest, ("patch.toml", second.name), "--list-verified")
+        assert result.returncode == 0 and "Verified catalog entries: 2" in result.stdout, (result.stdout, result.stderr)
+        assert "Choose one patch file; listed entries are not combined." in result.stdout, (result.stdout, result.stderr)
+        assert f"Catalog entry 1: {patch}" in result.stdout and f"Catalog entry 2: {second}" in result.stdout, (result.stdout, result.stderr)
+        assert "Recommended:" not in result.stdout, (result.stdout, result.stderr)
+        assert source.read_bytes() == image
         broken = catalog / "broken.toml"
         broken.write_text(patch_text(address, expected, "broken").replace('description = "flip one bit"',
                                                                      r'description = "flip \"one\" bit"'))
-        result = advise(hashlib.sha256(image).hexdigest(), ("broken.toml", "patch.toml"))
+        result = advise(digest, ("broken.toml", "patch.toml"))
         assert result.returncode == 0 and "relinker error (exit 2)" in result.stdout and f"Recommended: {patch}" in result.stdout, (result.stdout, result.stderr)
-        result = advise(hashlib.sha256(image).hexdigest(), ("broken.toml",))
+        result = advise(digest, ("broken.toml", "patch.toml"), "--list-verified")
+        assert result.returncode == 0 and "relinker error (exit 2)" in result.stdout and "Verified catalog entries: 1" in result.stdout, (result.stdout, result.stderr)
+        result = advise(digest, ("broken.toml",), "--list-verified")
         assert result.returncode == 2 and "relinker error (exit 2)" in result.stdout, (result.stdout, result.stderr)
         patch.write_text(patch_text(address, expected ^ 1, "wrong_byte"))
-        result = advise(hashlib.sha256(image).hexdigest())
+        result = advise(digest)
         assert result.returncode == 1 and "rejected by relinker" in result.stdout, (result.stdout, result.stderr)
+        result = advise(digest, ("patch.toml",), "--list-verified")
+        assert result.returncode == 1 and "rejected by relinker" in result.stdout and "Catalog entry 1:" not in result.stdout, (result.stdout, result.stderr)
         patch.write_text(patch_text(address, expected, "flip_bit"))
         result = advise(hashlib.sha256(b"other build").hexdigest())
         assert result.returncode == 1 and "SHA-256 mismatch" in result.stdout, (result.stdout, result.stderr)

docs/user/USAGE.md

@@ -105,7 +105,7 @@ A patch file changes bytes of the input executable or of a bundled module before
 The advisor requires Python 3.11 or newer. To check a local patch catalog, run:
 
 ```sh
-python3 tools/suggest_patches.py --catalog <directory> --param-json <sce_sys/param.json> --input <input.elf> --relinker <relinker> [--sce-module-path <directory>] [--module-dir <directory>]... [--executable <path>]... [--skip-sce-module] [--timeout-seconds <seconds>]
+python3 tools/suggest_patches.py --catalog <directory> --param-json <sce_sys/param.json> --input <input.elf> --relinker <relinker> [--sce-module-path <directory>] [--module-dir <directory>]... [--executable <path>]... [--skip-sce-module] [--timeout-seconds <seconds>] [--list-verified]
 ```
 
 The advisor reads `titleId` and `contentVersion` from the specified `param.json`, checks the SHA-256 of every file targeted by a candidate patch, then calls the relinker specified by `--relinker` to run `--verify-patches`. A relative path such as `--relinker ./relinker` resolves to that local file. The explicit metadata path also works when `param.json` and the unwrapped ELF files are stored separately. The catalog directory must contain an `index.json` with `format: 1` and an `entries` array. Each entry has `titleId`, `contentVersion`, a relative `patch` path, and `targets` mapping every patch target to its SHA-256. Hexadecimal digests are case-insensitive. For example:
@@ -122,7 +122,9 @@ The advisor reads `titleId` and `contentVersion` from the specified `param.json`
 }
 ```
 
-The advisor validates patch files only for entries matching the requested title and version. It reports and skips a malformed matching entry or a relinker verification error, so another valid entry can still be recommended. Verification times out after 120 seconds per entry by default; use `--timeout-seconds <positive integer>` to change the limit. For synthetic inputs without system modules, use `--skip-sce-module` if conversion also uses it; this option conflicts with `--sce-module-path`. No matching entry or no verified patch exits `1`; invalid input or no verified patch after a relinker error exits `2`. If multiple entries verify, the advisor recommends none because it cannot choose or combine them. The tool does not fetch or apply patches. A recommendation prints `--patches` and the module and executable options to use when converting. Keep catalog patch files outside the input executable's directory to avoid automatic `relinker-patches.toml` discovery.
+The advisor validates patch files only for entries matching the requested title and version. It reports and skips a malformed matching entry or a relinker verification error, so another valid entry can still be recommended. Verification times out after 120 seconds per entry by default; use `--timeout-seconds <positive integer>` to change the limit. For synthetic inputs without system modules, use `--skip-sce-module` if conversion also uses it; this option conflicts with `--sce-module-path`. No matching entry or no verified patch exits `1`; invalid input or no verified patch after a relinker error exits `2`. If multiple entries verify, the advisor recommends none because it cannot choose or combine them, and points to `--list-verified` to inspect the matches. The tool does not fetch or apply patches. A recommendation prints `--patches` and the module and executable options to use when converting. Keep catalog patch files outside the input executable's directory to avoid automatic `relinker-patches.toml` discovery.
+
+With `--list-verified`, the advisor prints every entry that passes metadata, hash and relinker checks, in catalog order. Each result includes its catalog number, patch file, target hashes and conversion options. It exits `0` when at least one entry verifies, even if another entry was rejected; with no verified entries, the existing `1` or `2` exit status still applies. Without this option, a single verified entry is recommended and multiple verified entries remain ambiguous. The listing tells users to choose one patch file. It does not combine patch files or apply them during conversion; review the chosen patch before applying it.
 
 The target hashes identify the exact unwrapped files changed by a patch, which might not include the main executable. Different dump tools could produce different bytes for the same title version; the advisor then rejects a patch even if it would work. It has not been confirmed that all dump tools produce identical files. The hashes do not authenticate a catalog or establish that a patch is safe or that the title is fully playable. Use catalogs from sources you trust and review the patch before applying it.
 

tools/suggest_patches.py

@@ -38,6 +38,19 @@ def command_line(arguments):
     return subprocess.list2cmdline(arguments) if os.name == "nt" else shlex.join(arguments)
 
 
+def conversion_options(args, patch):
+    options = ["--patches", str(patch)]
+    if args.skip_sce_module:
+        options.append("--skip-sce-module")
+    if args.sce_module_path:
+        options.extend(("--sce-module-path", str(args.sce_module_path)))
+    for module_dir in args.module_dir:
+        options.extend(("--module-dir", module_dir))
+    for executable in args.executable:
+        options.extend(("--executable", executable))
+    return command_line(options)
+
+
 def load_catalog(directory, title, version):
     data = json.loads(confined(directory, "index.json").read_text(encoding="utf-8"))
     if not isinstance(data, dict) or set(data) != {"format", "entries"} or type(data["format"]) is not int or data["format"] != 1 or not isinstance(data["entries"], list):
@@ -82,7 +95,9 @@ def main(argv=None):
     parser.add_argument("--executable", action="append", default=[], help="additional title executable relative to the module parent; repeatable")
     parser.add_argument("--skip-sce-module", action="store_true", help="skip bundled modules during verification")
     parser.add_argument("--timeout-seconds", type=int, default=120, help="maximum seconds per relinker verification (default: 120)")
+    parser.add_argument("--list-verified", action="store_true", help="list every verified catalog entry without choosing one")
     args = parser.parse_args(argv)
+    prefix = "No verified catalog entries" if args.list_verified else "No recommendation"
     try:
         metadata = json.loads(args.param_json.read_text(encoding="utf-8"))
         if not isinstance(metadata, dict):
@@ -105,7 +120,7 @@ def main(argv=None):
         matches = []
         entries = load_catalog(args.catalog.resolve(), title, version)
         if not entries:
-            print(f"No recommendation: no catalog entry for {title} {version}.")
+            print(f"{prefix}: no catalog entry for {title} {version}.")
             return 1
         relinker_errors = False
         for number, entry in entries:
@@ -161,29 +176,29 @@ def main(argv=None):
                 print(f"{label}: rejected: relinker did not confirm every patch")
                 continue
             matches.append((number, entry, patch, targets))
+        if args.list_verified and matches:
+            print(f"Verified catalog entries: {len(matches)}")
+            print("Choose one patch file; listed entries are not combined.")
+            for number, entry, patch, targets in matches:
+                print(f"Catalog entry {number}: {patch}")
+                for name, digest in targets.items():
+                    print(f"  {name} SHA-256 {digest}")
+                print(f"Use these options when converting:\n  {conversion_options(args, patch)}")
+            return 0
         if len(matches) == 1:
             number, entry, patch, targets = matches[0]
             print(f"Recommended: {patch} (catalog entry {number})")
             for name, digest in targets.items():
                 print(f"  {name} SHA-256 {digest}")
-            conversion = ["--patches", str(patch)]
-            if args.skip_sce_module:
-                conversion.append("--skip-sce-module")
-            if args.sce_module_path:
-                conversion.extend(("--sce-module-path", str(args.sce_module_path)))
-            for module_dir in args.module_dir:
-                conversion.extend(("--module-dir", module_dir))
-            for executable in args.executable:
-                conversion.extend(("--executable", executable))
-            print(f"Use these options when converting:\n  {command_line(conversion)}")
+            print(f"Use these options when converting:\n  {conversion_options(args, patch)}")
             return 0
         if len(matches) > 1:
-            print("No recommendation: multiple verified catalog entries match.")
+            print("No recommendation: multiple verified catalog entries match; use --list-verified to see them.")
             return 1
         if relinker_errors:
-            print("No recommendation: relinker errors occurred; see catalog entries above.", file=sys.stderr)
+            print(f"{prefix}: relinker errors occurred; see catalog entries above.", file=sys.stderr)
             return 2
-        print("No recommendation: no catalog entry passed all checks.")
+        print(f"{prefix}: no catalog entry passed all checks.")
         return 1
     except (OSError, ValueError, json.JSONDecodeError, tomllib.TOMLDecodeError) as error:
         print(f"Patch advisor: {error}", file=sys.stderr)

tools/tests/test_suggest_patches.py

@@ -2,6 +2,7 @@ import contextlib
 import hashlib
 import io
 import json
+import os
 import subprocess
 import sys
 import tempfile
@@ -241,9 +242,80 @@ class PatchAdvisorTests(unittest.TestCase):
         output = io.StringIO()
         with contextlib.redirect_stdout(output):
             self.assertEqual(self.advise(), 1)
-        self.assertIn("multiple verified catalog entries", output.getvalue())
+        self.assertIn("multiple verified catalog entries match; use --list-verified to see them.", output.getvalue())
         self.assertEqual(run.call_count, 2)
 
+    @patch.object(suggest_patches.subprocess, "run")
+    def test_list_verified_shows_each_match_and_quoted_patch_path(self, run):
+        second = self.catalog / "second patch.toml"
+        second.write_bytes((self.catalog / "patch.toml").read_bytes())
+        self.write_index([self.entry, dict(self.entry, patch=second.name)])
+        run.return_value.returncode = 0
+        run.return_value.stdout = VERIFIED
+        output = io.StringIO()
+        with contextlib.redirect_stdout(output):
+            self.assertEqual(self.advise("--list-verified"), 0)
+        self.assertIn("Verified catalog entries: 2", output.getvalue())
+        self.assertIn("Choose one patch file; listed entries are not combined.", output.getvalue())
+        self.assertIn(f"Catalog entry 1: {self.catalog / 'patch.toml'}", output.getvalue())
+        self.assertIn(f"Catalog entry 2: {second}", output.getvalue())
+        quoted = f'"{second}"' if os.name == "nt" else f"'{second}'"
+        self.assertIn(f"--patches {quoted} --module-dir Media/Modules", output.getvalue())
+        self.assertEqual(output.getvalue().count("SHA-256"), 2)
+        self.assertNotIn("Recommended:", output.getvalue())
+        self.assertEqual(run.call_count, 2)
+
+    @patch.object(suggest_patches.subprocess, "run")
+    def test_list_verified_single_match_preserves_conversion_options(self, run):
+        self.additional_executable()
+        run.return_value.returncode = 0
+        run.return_value.stdout = VERIFIED
+        output = io.StringIO()
+        with contextlib.redirect_stdout(output):
+            self.assertEqual(self.advise("--list-verified", "--executable", "picker/picker.bin",
+                                         "--sce-module-path", str(self.root)), 0)
+        expected = suggest_patches.command_line(["--patches", str(self.catalog / "patch.toml"),
+                                                  "--sce-module-path", str(self.root), "--module-dir",
+                                                  "Media/Modules", "--executable", "picker/picker.bin"])
+        self.assertIn(expected, output.getvalue())
+        self.assertIn("Verified catalog entries: 1", output.getvalue())
+        self.assertIn("Choose one patch file; listed entries are not combined.", output.getvalue())
+        self.assertNotIn("Recommended:", output.getvalue())
+
+    @patch.object(suggest_patches.subprocess, "run")
+    def test_list_verified_skips_individual_errors(self, run):
+        bad = dict(self.entry, patch="missing.toml")
+        wrong_hash = dict(self.entry, targets={"Media/Modules/module.prx": "0" * 64})
+        self.write_index([bad, wrong_hash, self.entry, self.entry, self.entry])
+        run.side_effect = [subprocess.CompletedProcess([], 2, "", "FAIL: invalid patch"),
+                           subprocess.CompletedProcess([], 1, "Verified 1 patches: 0 ok, 0 not-checked, 1 mismatch\n", ""),
+                           subprocess.CompletedProcess([], 0, VERIFIED, "")]
+        output = io.StringIO()
+        with contextlib.redirect_stdout(output):
+            self.assertEqual(self.advise("--list-verified"), 0)
+        self.assertIn("Catalog entry 1 ('missing.toml'): rejected", output.getvalue())
+        self.assertIn("Catalog entry 2 ('patch.toml'): rejected: Media/Modules/module.prx: SHA-256 mismatch", output.getvalue())
+        self.assertIn("Catalog entry 3 ('patch.toml'): relinker error (exit 2)", output.getvalue())
+        self.assertIn("Catalog entry 4 ('patch.toml'): rejected by relinker", output.getvalue())
+        self.assertIn("Verified catalog entries: 1", output.getvalue())
+        self.assertIn(f"Catalog entry 5: {self.catalog / 'patch.toml'}", output.getvalue())
+        self.assertEqual(run.call_count, 3)
+
+    @patch.object(suggest_patches.subprocess, "run")
+    def test_list_verified_no_match_keeps_exit_statuses(self, run):
+        self.write_index([])
+        output = io.StringIO()
+        with contextlib.redirect_stdout(output):
+            self.assertEqual(self.advise("--list-verified"), 1)
+        self.assertIn("No verified catalog entries: no catalog entry", output.getvalue())
+        run.assert_not_called()
+        self.write_index()
+        run.return_value = subprocess.CompletedProcess([], 2, "", "FAIL: invalid patch")
+        errors = io.StringIO()
+        with contextlib.redirect_stderr(errors):
+            self.assertEqual(self.advise("--list-verified"), 2)
+        self.assertIn("No verified catalog entries: relinker errors occurred", errors.getvalue())
+
     @patch.object(suggest_patches.subprocess, "run")
     def test_not_checked_patch_is_not_recommended(self, run):
         run.return_value.returncode = 0