fix: validate cache paths before building from extracted folders #1397

Merged
JooaoPedro15 merged 1 commit from fix/guard-extracted-build-cache into main 2026-11-10 04:36:18 +00:00
JooaoPedro15 commented 2026-11-10 00:32:52 +00:00 (Migrated from github.com)

The Builder validates the output ROMFS and retained ExeFS cache paths for NUP builds and updates. When the source is an extracted folder pair, it skips the ROMFS check and checks retained ExeFS only after it has already started copying. A redirected output ROMFS folder can therefore receive writes through a Windows junction.

Use the existing cache guard for both destinations before copying or unpacking the selected folders. The game-free regression simulates a Windows reparse point and verifies that both input modes stop before staging ExeFS or changing cached files.

python3 -I -B tests/run_all.py --strict make_game_cache passed on macOS. This change is independent of #1395 and the two patches merge cleanly.

AI assistance from OpenAI Codex is recorded in the commit trailers.

The Builder validates the output ROMFS and retained ExeFS cache paths for NUP builds and updates. When the source is an extracted folder pair, it skips the ROMFS check and checks retained ExeFS only after it has already started copying. A redirected output ROMFS folder can therefore receive writes through a Windows junction. Use the existing cache guard for both destinations before copying or unpacking the selected folders. The game-free regression simulates a Windows reparse point and verifies that both input modes stop before staging ExeFS or changing cached files. `python3 -I -B tests/run_all.py --strict make_game_cache` passed on macOS. This change is independent of #1395 and the two patches merge cleanly. AI assistance from OpenAI Codex is recorded in the commit trailers.
NotRexed commented 2026-11-10 03:50:02 +00:00 (Migrated from github.com)

This landed in the v2.0.12 branch (merged there for testing; it reaches main with that release), so closing. Thanks!

This landed in the v2.0.12 branch (merged there for testing; it reaches `main` with that release), so closing. Thanks!
NotRexed commented 2026-11-10 03:55:00 +00:00 (Migrated from github.com)

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.
hckrman101 (Migrated from github.com) approved these changes 2026-11-10 04:18:17 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. The folder-pair branch now runs the same check_cache_paths(out, (kept, romfs)) guard the NUP and update branches already use, and it runs before copytree/unpack/link_or_copy, so nothing is staged when the output is redirected. The guard is the existing one (realpath containment plus symlink/reparse-point check via st_file_attributes), so a normal existing romfs or .hookshot_exefs directory still passes and valid caches aren't rejected. No new path rules were added, so the Windows drive/UNC/case handling is unchanged from the NUP path.

The test change is sound: it runs the same simulated reparse point through both the NUP and --romfs/--exefs modes and checks that work/exefs isn't created, no extraction runs and the cache is untouched. The test file is picked up by run_all.py automatically (tests/test_*.py), nothing to register.

Overlap with the sibling PRs: #1395 edits this same else: branch and the same test file, and #1393 touches make_game.py and tests/test_make_game_cache.py too, so expect small rebase conflicts in run_to_lift/that block depending on merge order (you say #1395 merges cleanly, which is plausible). #1391 and #1412 only touch nx2/nup.py / nx2/test_nup_paths.py, no overlap.

CI: Python checks (ubuntu and windows), appimage and builder package all pass. A few jobs (Windows Vulkan llvm-mingw, SPIR-V lowering, actionlint) were still pending when I looked, but none of them exercise this change.

Looks good. The folder-pair branch now runs the same `check_cache_paths(out, (kept, romfs))` guard the NUP and update branches already use, and it runs before `copytree`/unpack/`link_or_copy`, so nothing is staged when the output is redirected. The guard is the existing one (realpath containment plus symlink/reparse-point check via `st_file_attributes`), so a normal existing `romfs` or `.hookshot_exefs` directory still passes and valid caches aren't rejected. No new path rules were added, so the Windows drive/UNC/case handling is unchanged from the NUP path. The test change is sound: it runs the same simulated reparse point through both the NUP and `--romfs/--exefs` modes and checks that `work/exefs` isn't created, no extraction runs and the cache is untouched. The test file is picked up by `run_all.py` automatically (`tests/test_*.py`), nothing to register. Overlap with the sibling PRs: #1395 edits this same `else:` branch and the same test file, and #1393 touches `make_game.py` and `tests/test_make_game_cache.py` too, so expect small rebase conflicts in `run_to_lift`/that block depending on merge order (you say #1395 merges cleanly, which is plausible). #1391 and #1412 only touch `nx2/nup.py` / `nx2/test_nup_paths.py`, no overlap. CI: Python checks (ubuntu and windows), appimage and builder package all pass. A few jobs (Windows Vulkan llvm-mingw, SPIR-V lowering, actionlint) were still pending when I looked, but none of them exercise this change.
Commenting is not possible because the repository is archived.
No description provided.