MagicBeanDev/Hookshot
Archived
Watch
2
0
Fork
You've already forked Hookshot
 0
Code Issues 125 Pull requests 205 Activity

fix: security hardening passes (update offer, builder and release downloads, setup lists, shared env file, files the game reads and writes, game folders from elsewhere, HLE guest ranges) #2430

Open
Joey-Magnus wants to merge 39 commits from Joey-Magnus/security/hardening-passes into main
pull from: Joey-Magnus/security/hardening-passes
merge into: MagicBeanDev:main
Conversation 5 Commits 39 Files changed 84 +3268 -227
Joey-Magnus commented 2026-11-11 18:57:37 +00:00 (Migrated from github.com)

Takes over #2326 from @hckrman101, who asked for someone to pick it up. His 34 commits are unchanged; this adds a merge with main and three fixes on top. His description follows below the line.

Added in the takeover

  • CI fix: tests/test_oot_menu_mouse.py (Windows only) failed to build because its stand-in hos.h had no hos_guest_range, which host/oot_menu.c now calls. The stand-in now has it.

  • Merge with main (5cbfead0). Ten files conflicted. The two that needed a decision:

    • host/hos_update.c: since #2283, "Skip this version" is kept with the saves (hos_user_dir), not in <game>/user/. write_skip now writes there and still never through a link at the file's name. The saves folder itself may be a link, because a player can keep it elsewhere. tests/test_update_skip_file.py covers that case now.
    • host/hos_main.c: main made the environment go over hookshot_env.txt. A name is now noted for the builder's start only when the file actually set it. Otherwise a switch the player set in their own environment would be taken from the builder.

  • Main's new writes go through hos_fopen_write: the driver faults note, and the predicted pipelines' marker and sample in gpu/vk/vk_draw.cpp. tests/test_game_writes_no_links.py found them.

  • HOS_CONTROL and HOS_CONTROL_MEM are refused in a shared hookshot_env.txt. The control bridge (#926) landed after this PR started. Its socket name passed the file's no-path rule as a bare name, and HOS_CONTROL_MEM=rw opens guest memory writes. tests/test_env_file.py found it.

  • Test fixtures that compile code from both sides now get hos_fopen_write.h, hookshot_env.h and hos_env_file_names. test_env_file.py also finds HOOKSHOT_ names read with env_on.

  • Second merge with main (45f19993). Eight files conflicted; both sides are kept in each, and the merge commit lists them. The ones that needed a decision:

    • host/hos_main.c: main's #2282 already makes the saves' links again by their full path, so its saves_dir replaces this PR's own link handling there. copy_tree no longer fails on a link inside a folder (main makes it again), and still copies files through the no-link, no-FIFO helpers.
    • host/hos_update.c: main's builder_local (#2367, the portable game's hookshot.exe) is taken as main takes it. Otherwise builder= still has to pass builder_ok.
    • gpu/vk/vk_draw.cpp: main now reads a pipeline cache's header before its size. That order is kept, with the kPcacheMax limit and hos_fopen_read_binary on top.
    • nx2/nup.py: main's new is_link takes links_only, so a game folder OneDrive keeps still counts only real links.

  • Main's new checks that cut code this PR changed get its helpers (9f329311). .hookshot_builder_home, which the builder writes since #2367, is on the list of names a link at stops the build.

  • Windows "Skip this version" takes a junctioned user folder (70b495de), as hckrman101 asked.

  • The readme says that the game does not write through a link at hookshot_options.ini, mouse_sens.txt or a pipeline cache, and to link the folder instead (1751ff48).

Tested (takeover)

Second merge (9f329311), Linux: python -I -B tests/run_all.py --jobs 4: 474 passed, 2 failed, 36 skipped. The two failures (test_lift_cache.py, and test_updater_asset_digest.py, whose .NET restore finds no packages offline) fail on main on this PC too. With llvm-mingw 20261104, the changed host files (hos_update.c, hos_main.c, hos_svc.c, the nn_*.c files, nvn_native.c, oot_menu.c, prompts.c, discord_rpc.c) and builder/setup/setup.c compile for Windows with no errors, hos_update.c with no warnings under -Wall -Wextra. vk_draw.cpp, vk_main.cpp and vk_menu.cpp pass a syntax check for both Windows and Linux. Nothing was run on Windows.

First merge:

Linux: python -I -B tests/run_all.py: 367 passed, 0 failed, 53 skipped. The skips are Vulkan/SPIR-V headers, dotnet and Windows-only tests, which are not installed here. host/hos_update.c passes a gcc -Wall -Wextra syntax check.

Not tested (takeover)

  • The Windows-only test_oot_menu_mouse.py was not run: there is no MinGW here. A clang syntax check of oot_menu.c against the new stand-in no longer reports the hos_guest_range errors.
  • No game build or run after the merge. Everything listed under hckrman101's "Not tested" still applies.

Supersedes #2326.

Some files that players run, download or pass around are still trusted more than they should be. A game folder's text file can pick the program the update offer starts. A shared hookshot_env.txt can still set switches that write files or change how an update builds. A game folder unpacked from an archive can hold links that the game or the builder writes through. This PR closes those gaps and adds tests so they stay closed. None of it changes what a normal player sees.

What changes

Update offer and builders

  • Update offer (host/hos_update.c). The update offer now uses builder= / builder_args= from hookshot_version.txt only when they look like what our builders write:

    • Windows: an absolute path to HookshotBuilder.exe, or to python(w).exe with -I <path>\hookshot_builder_start.py;
    • Linux: an AppImage or AppRun.

    Anything else counts as "no builder", and the offer opens the download page instead. Only the shape of the path is checked, not where it is, so a builder unpacked anywhere still works.

  • "Skip this version" no longer writes through a skip file that is a link. The folder that holds it may be a link or a junction, since that is how a player keeps the saves on another drive. On Linux it uses lstat and O_NOFOLLOW. On Windows it refuses a symbolic link or junction at the file's name but still allows OneDrive's own reparse points.

  • The builder an update starts no longer gets the switches that the game's hookshot_env.txt set. It inherits the rest of the game's environment, but the builder reads some of the same names for its build (RX_MARCH, RX_VK_ONLY and others). On Linux the builder is started with execve and an environment built before the fork. On Windows those switches are cleared before ShellExecuteW, and put back if the builder does not start.

  • One tag rule for every builder. A tag may use [0-9A-Za-z._+-], be at most 128 characters, and not be . or ... The WPF, Python and Linux builders apply it to --tag and to every tag they read from GitHub. Before, the WPF builder's nightly lookup passed the list's tag line, unchecked, into a release API link. The WPF builder's patterns end in \z, because .NET's $ also matches before a final line break.

  • Windows Python builder downloads (setup_parts.py, updater.py).

    • Every redirect hop must stay on https, as the Linux updater already requires, and a refused redirect is not retried.
    • A part stops downloading as soon as it is longer than the setup list says.
    • A list or API answer stops at 8 MB.
    • The latest-release check uses the same opener and only returns a tag that passes the list's name rule.

  • Setup lists' parts (builder/setup/setup.c, setup_parts.py, builder/linux/linux_update.py). A part's link must now be under this project's GitHub releases, or llvm-mingw's for the Windows lists (the one part that is not ours). The rest of the link must have no .. step, escape, query or other host. Before, the checksums pinned each part's bytes, but a link could name any https host. A list given for testing (HOOKSHOT_SETUP_LIST) and the Linux updater's lists from this PC still take any https (or file) link.

  • WPF builder downloads (Updater.cs). A download now stops as soon as it passes GitHub's size for the file. Before, a body with no Content-Length was written to disk in full and only then compared.

Shared hookshot_env.txt (host/hos_main.c)

  • Dump switches: every switch with _DUMP in its name is now refused as a group. RX_SCREENSHOT, RX_FLASH_LOG, HOS_SCAN_DT and RX_VK_FRAME_TRACE are refused by name.
  • SDL switches that write files are refused: the *_SAVE_FRAMES switches (a picture each frame) and the disk audio driver. Other SDL_ switches still work.
  • HOOKSHOT_ names: only the game's own are taken: HOOKSHOT_NO_UPDATE_CHECK, HOOKSHOT_NO_AV1_NOTICE and HOOKSHOT_OCCLUSION_OFF. The rest are the builder's.
  • RX_VK_DLSS_DIR is still the one path the file may set, but only as a full local path (C:\... or /...) with no .. step. Before, a relative value was resolved against whatever folder the game was started from.
  • Tests: tests/test_env_file.py scans host/ and gpu/ for switches whose value is used as a path, and the builder's sources for HOOKSHOT_ names. It fails if the shared file could set one of them. The readme says the same.
  • Not affected: the process environment and a file named by HOS_ENV_FILE still set anything.

Files the game writes, and game folders from elsewhere

  • Files the game writes by itself (new host/hos_fopen_write.h). This covers hookshot_options.ini, the pipeline caches, the state lists and their markers, the button-prompt textures, mouse_sens.txt, language.txt and the moved-saves note. A game folder unpacked from an archive can hold links, and before, the game wrote through a link at one of these names, emptying the file it pointed to.

    • On Linux these files are now opened without following a symbolic link at their name, and only as regular files.
    • On Windows a name that is a symbolic link or a junction is refused, because archive tools make those in Developer Mode. OneDrive's own reparse points are still written.
    • The renderer's object keys (nx2/build.py) now include the host headers it uses, so a change to one of them rebuilds the renderer's objects.

  • Files the game reads from its folder at start-up. These are now read only when they are regular files. Before, a FIFO under one of their names (an archive can carry one) made the game wait forever before its window opened. The files covered:

    • the text files: hookshot_env.txt, hookshot_options.ini, language.txt and mouse_sens.txt, through the new hos_fopen_read;
    • the renderer's pipeline caches, notes, state lists, the preparation's records and markers, and the game's font files, through hos_fopen_read_binary;
    • the button-prompt files.

    This applies on Linux. On Windows the read is plain fopen.

  • Mouse sensitivity (host/nn_hid.c). A value from mouse_sens.txt or HOS_MOUSE_SENS is taken only when it is a finite number, and it is kept to the menu's 10-500%. Before, a value like inf went into the menu's option as an undefined number.

  • Saves moved from an old user/ folder (Linux). The game moves user/ into $XDG_DATA_HOME/hookshot and copies an entry when it cannot rename it. Since main's #2282, every link in user/ is made again in the new place by the full path it names, and the clean-up removes a link without going into it. This PR adds that the copy reads and writes files through hos_fopen_read_binary / hos_fopen_write, so a FIFO fails the copy (the saves stay in user/) and nothing is written through a link.

  • Rebuilding or updating an existing game folder (nx2/make_game.py, nx2/nup.py).

    • Each line of .hookshot_romfs_heads has to name a .zs file inside the romfs, under the same path rule the extraction uses. If any line does not, the list is dropped and every file is checked instead.
    • A damaged texture's stand-in is copied into a new temp file, not a fixed <name>.part.
    • A link inside the kept .hookshot_exefs, or inside the romfs the build hard-links into, stops the build and asks for an empty folder. On Windows only symbolic links, junctions and WSL links count, so a folder under OneDrive still updates.
    • In a game folder OneDrive keeps, the extraction's list of damaged textures is still used. Before, OneDrive's own reparse points made the builder drop it and open all ~100,000 files again, which takes minutes. Only reparse points that are links count there now.
    • A link at a name the builder writes in the folder (language.txt, hookshot_version.txt, bin, prompts, the staged .new files and so on) stops the build before anything is written. Links at other names, such as a cache moved to another drive, are left alone. A test checks that every name the builder writes is on the list.

  • Pipeline cache (gpu/vk/vk_draw.cpp). mkw_vk_pipelines.bin and the play caches are now checked by their header before the rest is read, and a file over 4 GB is not read. Before, the whole file was loaded into memory first.

  • Button-prompt files (host/prompts.c). A file is read only when its magic is right, and never past 256 MB. Before, the whole file was loaded into memory first.

  • Pipeline notes and state lists (gpu/vk/vk_draw.cpp). A record is used only with a sample count the GPU supports, vertex strides up to 2048 (Vulkan's least limit) and divisors up to 65535. Before, any power of two up to 64 and any stride or divisor were handed to the driver.

  • Contributed pipeline state lists (nx2/pipeline_states.py).

    • add, tour and check (which CI runs) now stop on a record with a value no pipeline has: a count past its array, a sample count or topology Vulkan does not have, a yes/no value other than 0 or 1, non-zero padding, or a stride or divisor past those limits. They name the record and the reason.
    • An xz list is unpacked only up to 64 MB.
    • Every list in the repository passes.

  • A refused write is logged. The first time a file stops at a link or a non-file in hos_fopen_write, it writes files: <path> is a link or not a plain file: the game does not write through it to the game's log. That gives a setting that does not stick a visible reason.

Host functions the game calls (HLE)

  • Guest memory checks. GP() turns a guest address into a host pointer without checking it. Before, a size the game passed could carry a copy past the guest window into the host's own memory. These functions now use hos_guest_range on the whole span, as nn::fs and the libc copy hooks already do:
    • nn::audio's wave buffers and ADPCM coefficients. The mixer reads these for as long as a voice plays, so a buffer outside guest memory is now refused.
    • nn::os GenerateRandomBytes.
    • nn::hid GetNpadStates, now capped at 16 states, and GetSixAxisSensorStates.
    • The socket Poll stand-in.
    • OutputDebugString and nn::diag's log lines, which copy into the log.
    • The event-name and item-key copies in hos_main.c and oot_menu.c.
  • NVN proc names (host/nvn_native.c). nvnDeviceGetProcAddress and nvnBootstrapLoader now copy the name the game passes, at most 255 characters and only from inside guest memory. Before, they read it in place to its end.

Release and CI

  • Python packages in releases.
    • The AppImage's Python (scripts/linux/deps.sh) and the Windows builder's Python part (scripts/release_parts.py) now install their pinned packages with pip --require-hashes, from a new requirements-lock.txt.
    • That file holds the SHA-256 of each pin's wheels for Linux x86_64 and Windows x64. python -I scripts/python_lock.py writes it after a pin changes.
    • tests/test_python_lock.py fails when the lock and the pins differ.
    • The AppImage's dependency cache key includes the lock.
  • scripts/ffx_sdk.py and scripts/ngx_sdk.py. Downloads and copies go to a new temp file next to the target instead of a fixed <name>.part. Each file is hashed while it downloads, and a download stops past 1 GB. The pinned commits and SHA-256s are unchanged.
  • scripts/release_rc.sh. It copies the newest release candidate's AppImage and .zsync into the rc release only when GitHub gives a SHA-256 digest for each file. This is checked before the tag moves or anything uploads. Before, a file without a digest was copied unchecked.
  • Dependabot. It now proposes updates to the pinned Python packages (requirements.txt, nx2/prompt_requirements.txt) weekly, with the same 7-day cooldown as the actions. After it changes a pin, scripts/python_lock.py must write the lock again; tests/test_python_lock.py fails until it does. The builder's .NET project has no NuGet packages, so there is no NuGet entry.
  • scripts/linux/deps.sh. The four license texts the AppImage ships now go through fetch with SHA-256 pins, like every other download there.
  • New tests/test_workflow_hardening.py. It checks every workflow for:
    • actions pinned by full commit;
    • token permissions set at the top;
    • checkouts with persist-credentials: false (except the release record job's);
    • no ${{ }} of event data or inputs inside a run: script;
    • no checkout in a pull_request_target workflow.

Tested

Linux, RTX 3090. The branch merges main@origin ebbba2ea, which carries scripts/release_rc.sh and the setup-list code changed here; the merge had no conflicts. Testing used merges with main as it moved (e19d9693, d7a73062, ebbba2ea, 49f75dc8), all without conflicts. The link and env game runs used the e19d9693 merge, the FIFO run and the last game build the d7a73062 merge, and the last full suite the 49f75dc8 merge.

  • New and changed checks. Each one fails on the code before its fix and passes after. Each commit's own checks pass at that commit. The existing checks that compile code cut from the changed files now include the helpers, and they pass. The ones that also run on Windows were compiled with llvm-mingw too.
  • Full suite (python -I -B tests/run_all.py). 373 passed, 1 failed, 12 skipped. The failure, tests/test_wayland_libdecor_hint.py, also fails on main on this PC.
  • Python package lock. pip downloaded every pinned wheel with --require-hashes for Linux x86_64 / Python 3.12 (with dependencies) and for Windows x64 / Python 3.12, 3.13 and 3.14 (as the Windows part installs them): 10 of 10 each time.
  • Compile.
    • host/hos_update.c compiles with no warnings (-Wall -Wextra) under gcc (Linux) and llvm-mingw clang (Windows).
    • The other changed host files pass a syntax check on both.
    • The Windows renderer (hookshot_vk.dll) cross-built with llvm-mingw before the last round. After it, vk_draw.cpp and vk_main.cpp pass a syntax check for both Linux and Windows. host/nvn_native.c, builder/setup/setup.c and the host files that include hos_fopen_write.h pass one for both too.
  • Game build. A full build of the merge with hookshot-build finished, including shader preparation. A rebuild into the existing game folder took the "read from this .nup before" path, which the new link check does not refuse. All 25,251 shipped pipeline states were noted, and the 996 MB pipeline cache was read. With a language.txt link planted in the folder, the build stopped at the start and named it, and the link's target was left as it was.
  • Headless game runs (HOS_HEADLESS=1 HOS_NO_SDL=1, separate data folder, links planted in the game folder; final run on the branch head):
    • nothing was written through the link at hookshot_options.ini or at the pipeline marker;
    • user/ was moved to the saves folder, and its user/save link was moved as a link with its target left alone;
    • a relative RX_VK_DLSS_DIR was left out of hookshot_env.txt;
    • SDL's disk audio and save-frames switches, RX_VK_FRAME_TRACE, HOOKSHOT_MEM_LIMIT_GB, HOOKSHOT_JOBS and HOOKSHOT_PREPARE_ONLY were left out, and no sdlaudio.raw or hookshot_frames.csv appeared;
    • the game's own audio wave buffers and controller reads passed the new guest-memory checks (all 8 wave buffers logged as taken, the controller read as before);
    • the game started and ran its shader warm-up.
  • Headless run with FIFOs at language.txt, hookshot_env.txt and hookshot_options.ini. On a build that had moved only the host's reads, the game hung after listing the GPUs, on the renderer's own read of hookshot_options.ini. With every start-up read moved over, it started and ran as usual and never waited on the FIFOs.
  • .NET (SDK 8.0.428 on Linux).
    • tests/test_updater_asset_digest.py builds the whole Updater.cs with its fixture: 22 of 22 cases pass. Its two new cases fail on the code before the fix.
    • tests/test_builder_dlss_update.py passes (57 checks).
    • The WPF builder project builds with EnableWindowsTargeting=true, with no warnings.

Not tested

  • Windows, nothing was run:
    • the update offer with a real WPF or Python builder, and the environment hand-off to it;
    • the junction / OneDrive check for the skip file;
    • the Python builder against GitHub.
    • the Windows branch of hos_fopen_write, which was only tested with a stand-in windows.h. The Windows host files were only syntax-checked.
  • The WPF builder was not run, only built. Its updater was run through the test fixture.
  • The Linux update offer end to end (it opens a zenity window). The checks behind it are unit-tested, the environment hand-off included.
  • The pip lock in CI: it has not run in the AppImage or Windows release jobs yet.
  • Gameplay: the headless runs stopped after 60-120 s, before gameplay. There was no windowed run with a controller.
  • The last round's game changes were never in a built game. That covers the vertex stride and divisor limits, the bounded NVN proc names and the log line for a refused write. The build queue was full. They are covered by their tests and the syntax checks above.
  • Left for later:
    • signing the release lists;
    • a structural check of cached SPIR-V and an install key for the program cache. The same file's reads and writes and its shader seed also need the regular-file, no-link and size rules, after the change being made to that file now;
    • the Landlock sandbox;
    • the nightly CI cache policy;
Takes over #2326 from @hckrman101, who asked for someone to pick it up. His 34 commits are unchanged; this adds a merge with main and three fixes on top. His description follows below the line. ## Added in the takeover - **CI fix:** `tests/test_oot_menu_mouse.py` (Windows only) failed to build because its stand-in `hos.h` had no `hos_guest_range`, which `host/oot_menu.c` now calls. The stand-in now has it. - **Merge with main (`5cbfead0`).** Ten files conflicted. The two that needed a decision: - `host/hos_update.c`: since #2283, "Skip this version" is kept with the saves (`hos_user_dir`), not in `<game>/user/`. `write_skip` now writes there and still never through a link at the file's name. The saves folder itself may be a link, because a player can keep it elsewhere. `tests/test_update_skip_file.py` covers that case now. - `host/hos_main.c`: main made the environment go over `hookshot_env.txt`. A name is now noted for the builder's start only when the file actually set it. Otherwise a switch the player set in their own environment would be taken from the builder. - **Main's new writes go through `hos_fopen_write`:** the driver faults note, and the predicted pipelines' marker and sample in `gpu/vk/vk_draw.cpp`. `tests/test_game_writes_no_links.py` found them. - **`HOS_CONTROL` and `HOS_CONTROL_MEM` are refused in a shared `hookshot_env.txt`.** The control bridge (#926) landed after this PR started. Its socket name passed the file's no-path rule as a bare name, and `HOS_CONTROL_MEM=rw` opens guest memory writes. `tests/test_env_file.py` found it. - **Test fixtures** that compile code from both sides now get `hos_fopen_write.h`, `hookshot_env.h` and `hos_env_file_names`. `test_env_file.py` also finds `HOOKSHOT_` names read with `env_on`. - **Second merge with main (`45f19993`).** Eight files conflicted; both sides are kept in each, and the merge commit lists them. The ones that needed a decision: - `host/hos_main.c`: main's #2282 already makes the saves' links again by their full path, so its `saves_dir` replaces this PR's own link handling there. `copy_tree` no longer fails on a link inside a folder (main makes it again), and still copies files through the no-link, no-FIFO helpers. - `host/hos_update.c`: main's `builder_local` (#2367, the portable game's `hookshot.exe`) is taken as main takes it. Otherwise `builder=` still has to pass `builder_ok`. - `gpu/vk/vk_draw.cpp`: main now reads a pipeline cache's header before its size. That order is kept, with the `kPcacheMax` limit and `hos_fopen_read_binary` on top. - `nx2/nup.py`: main's new `is_link` takes `links_only`, so a game folder OneDrive keeps still counts only real links. - **Main's new checks** that cut code this PR changed get its helpers (`9f329311`). `.hookshot_builder_home`, which the builder writes since #2367, is on the list of names a link at stops the build. - **Windows "Skip this version"** takes a junctioned `user` folder (`70b495de`), as hckrman101 asked. - **The readme** says that the game does not write through a link at `hookshot_options.ini`, `mouse_sens.txt` or a pipeline cache, and to link the folder instead (`1751ff48`). ### Tested (takeover) Second merge (`9f329311`), Linux: `python -I -B tests/run_all.py --jobs 4`: 474 passed, 2 failed, 36 skipped. The two failures (`test_lift_cache.py`, and `test_updater_asset_digest.py`, whose .NET restore finds no packages offline) fail on main on this PC too. With llvm-mingw 20261104, the changed host files (`hos_update.c`, `hos_main.c`, `hos_svc.c`, the `nn_*.c` files, `nvn_native.c`, `oot_menu.c`, `prompts.c`, `discord_rpc.c`) and `builder/setup/setup.c` compile for Windows with no errors, `hos_update.c` with no warnings under `-Wall -Wextra`. `vk_draw.cpp`, `vk_main.cpp` and `vk_menu.cpp` pass a syntax check for both Windows and Linux. Nothing was run on Windows. First merge: Linux: `python -I -B tests/run_all.py`: 367 passed, 0 failed, 53 skipped. The skips are Vulkan/SPIR-V headers, dotnet and Windows-only tests, which are not installed here. `host/hos_update.c` passes a gcc `-Wall -Wextra` syntax check. ### Not tested (takeover) - The Windows-only `test_oot_menu_mouse.py` was not run: there is no MinGW here. A clang syntax check of `oot_menu.c` against the new stand-in no longer reports the `hos_guest_range` errors. - No game build or run after the merge. Everything listed under hckrman101's "Not tested" still applies. Supersedes #2326. --- Some files that players run, download or pass around are still trusted more than they should be. A game folder's text file can pick the program the update offer starts. A shared `hookshot_env.txt` can still set switches that write files or change how an update builds. A game folder unpacked from an archive can hold links that the game or the builder writes through. This PR closes those gaps and adds tests so they stay closed. None of it changes what a normal player sees. ## What changes ### Update offer and builders - **Update offer (host/hos_update.c).** The update offer now uses `builder=` / `builder_args=` from `hookshot_version.txt` only when they look like what our builders write: - Windows: an absolute path to `HookshotBuilder.exe`, or to `python(w).exe` with `-I <path>\hookshot_builder_start.py`; - Linux: an AppImage or `AppRun`. Anything else counts as "no builder", and the offer opens the download page instead. Only the shape of the path is checked, not where it is, so a builder unpacked anywhere still works. - **"Skip this version"** no longer writes through a skip file that is a link. The folder that holds it may be a link or a junction, since that is how a player keeps the saves on another drive. On Linux it uses lstat and O_NOFOLLOW. On Windows it refuses a symbolic link or junction at the file's name but still allows OneDrive's own reparse points. - **The builder an update starts** no longer gets the switches that the game's `hookshot_env.txt` set. It inherits the rest of the game's environment, but the builder reads some of the same names for its build (`RX_MARCH`, `RX_VK_ONLY` and others). On Linux the builder is started with `execve` and an environment built before the fork. On Windows those switches are cleared before `ShellExecuteW`, and put back if the builder does not start. - **One tag rule for every builder.** A tag may use `[0-9A-Za-z._+-]`, be at most 128 characters, and not be `.` or `..`. The WPF, Python and Linux builders apply it to `--tag` and to every tag they read from GitHub. Before, the WPF builder's nightly lookup passed the list's tag line, unchecked, into a release API link. The WPF builder's patterns end in `\z`, because .NET's `$` also matches before a final line break. - **Windows Python builder downloads (setup_parts.py, updater.py).** - Every redirect hop must stay on https, as the Linux updater already requires, and a refused redirect is not retried. - A part stops downloading as soon as it is longer than the setup list says. - A list or API answer stops at 8 MB. - The latest-release check uses the same opener and only returns a tag that passes the list's name rule. - **Setup lists' parts (builder/setup/setup.c, setup_parts.py, builder/linux/linux_update.py).** A part's link must now be under this project's GitHub releases, or llvm-mingw's for the Windows lists (the one part that is not ours). The rest of the link must have no `..` step, escape, query or other host. Before, the checksums pinned each part's bytes, but a link could name any https host. A list given for testing (`HOOKSHOT_SETUP_LIST`) and the Linux updater's lists from this PC still take any https (or file) link. - **WPF builder downloads (Updater.cs).** A download now stops as soon as it passes GitHub's size for the file. Before, a body with no Content-Length was written to disk in full and only then compared. ### Shared `hookshot_env.txt` (host/hos_main.c) - **Dump switches:** every switch with `_DUMP` in its name is now refused as a group. `RX_SCREENSHOT`, `RX_FLASH_LOG`, `HOS_SCAN_DT` and `RX_VK_FRAME_TRACE` are refused by name. - **SDL switches that write files** are refused: the `*_SAVE_FRAMES` switches (a picture each frame) and the `disk` audio driver. Other `SDL_` switches still work. - **`HOOKSHOT_` names:** only the game's own are taken: `HOOKSHOT_NO_UPDATE_CHECK`, `HOOKSHOT_NO_AV1_NOTICE` and `HOOKSHOT_OCCLUSION_OFF`. The rest are the builder's. - **`RX_VK_DLSS_DIR`** is still the one path the file may set, but only as a full local path (`C:\...` or `/...`) with no `..` step. Before, a relative value was resolved against whatever folder the game was started from. - **Tests:** `tests/test_env_file.py` scans `host/` and `gpu/` for switches whose value is used as a path, and the builder's sources for `HOOKSHOT_` names. It fails if the shared file could set one of them. The readme says the same. - **Not affected:** the process environment and a file named by `HOS_ENV_FILE` still set anything. ### Files the game writes, and game folders from elsewhere - **Files the game writes by itself (new host/hos_fopen_write.h).** This covers `hookshot_options.ini`, the pipeline caches, the state lists and their markers, the button-prompt textures, `mouse_sens.txt`, `language.txt` and the moved-saves note. A game folder unpacked from an archive can hold links, and before, the game wrote through a link at one of these names, emptying the file it pointed to. - On Linux these files are now opened without following a symbolic link at their name, and only as regular files. - On Windows a name that is a symbolic link or a junction is refused, because archive tools make those in Developer Mode. OneDrive's own reparse points are still written. - The renderer's object keys (nx2/build.py) now include the host headers it uses, so a change to one of them rebuilds the renderer's objects. - **Files the game reads from its folder at start-up.** These are now read only when they are regular files. Before, a FIFO under one of their names (an archive can carry one) made the game wait forever before its window opened. The files covered: - the text files: `hookshot_env.txt`, `hookshot_options.ini`, `language.txt` and `mouse_sens.txt`, through the new `hos_fopen_read`; - the renderer's pipeline caches, notes, state lists, the preparation's records and markers, and the game's font files, through `hos_fopen_read_binary`; - the button-prompt files. This applies on Linux. On Windows the read is plain `fopen`. - **Mouse sensitivity (host/nn_hid.c).** A value from `mouse_sens.txt` or `HOS_MOUSE_SENS` is taken only when it is a finite number, and it is kept to the menu's 10-500%. Before, a value like `inf` went into the menu's option as an undefined number. - **Saves moved from an old `user/` folder (Linux).** The game moves `user/` into `$XDG_DATA_HOME/hookshot` and copies an entry when it cannot rename it. Since main's #2282, every link in `user/` is made again in the new place by the full path it names, and the clean-up removes a link without going into it. This PR adds that the copy reads and writes files through `hos_fopen_read_binary` / `hos_fopen_write`, so a FIFO fails the copy (the saves stay in `user/`) and nothing is written through a link. - **Rebuilding or updating an existing game folder (nx2/make_game.py, nx2/nup.py).** - Each line of `.hookshot_romfs_heads` has to name a `.zs` file inside the romfs, under the same path rule the extraction uses. If any line does not, the list is dropped and every file is checked instead. - A damaged texture's stand-in is copied into a new temp file, not a fixed `<name>.part`. - A link inside the kept `.hookshot_exefs`, or inside the romfs the build hard-links into, stops the build and asks for an empty folder. On Windows only symbolic links, junctions and WSL links count, so a folder under OneDrive still updates. - In a game folder OneDrive keeps, the extraction's list of damaged textures is still used. Before, OneDrive's own reparse points made the builder drop it and open all ~100,000 files again, which takes minutes. Only reparse points that are links count there now. - A link at a name the builder writes in the folder (`language.txt`, `hookshot_version.txt`, `bin`, `prompts`, the staged `.new` files and so on) stops the build before anything is written. Links at other names, such as a cache moved to another drive, are left alone. A test checks that every name the builder writes is on the list. - **Pipeline cache (gpu/vk/vk_draw.cpp).** `mkw_vk_pipelines.bin` and the play caches are now checked by their header before the rest is read, and a file over 4 GB is not read. Before, the whole file was loaded into memory first. - **Button-prompt files (host/prompts.c).** A file is read only when its magic is right, and never past 256 MB. Before, the whole file was loaded into memory first. - **Pipeline notes and state lists (gpu/vk/vk_draw.cpp).** A record is used only with a sample count the GPU supports, vertex strides up to 2048 (Vulkan's least limit) and divisors up to 65535. Before, any power of two up to 64 and any stride or divisor were handed to the driver. - **Contributed pipeline state lists (nx2/pipeline_states.py).** - `add`, `tour` and `check` (which CI runs) now stop on a record with a value no pipeline has: a count past its array, a sample count or topology Vulkan does not have, a yes/no value other than 0 or 1, non-zero padding, or a stride or divisor past those limits. They name the record and the reason. - An xz list is unpacked only up to 64 MB. - Every list in the repository passes. - **A refused write is logged.** The first time a file stops at a link or a non-file in `hos_fopen_write`, it writes `files: <path> is a link or not a plain file: the game does not write through it` to the game's log. That gives a setting that does not stick a visible reason. ### Host functions the game calls (HLE) - **Guest memory checks.** `GP()` turns a guest address into a host pointer without checking it. Before, a size the game passed could carry a copy past the guest window into the host's own memory. These functions now use `hos_guest_range` on the whole span, as nn::fs and the libc copy hooks already do: - nn::audio's wave buffers and ADPCM coefficients. The mixer reads these for as long as a voice plays, so a buffer outside guest memory is now refused. - nn::os GenerateRandomBytes. - nn::hid GetNpadStates, now capped at 16 states, and GetSixAxisSensorStates. - The socket Poll stand-in. - OutputDebugString and nn::diag's log lines, which copy into the log. - The event-name and item-key copies in `hos_main.c` and `oot_menu.c`. - **NVN proc names (host/nvn_native.c).** `nvnDeviceGetProcAddress` and `nvnBootstrapLoader` now copy the name the game passes, at most 255 characters and only from inside guest memory. Before, they read it in place to its end. ### Release and CI - **Python packages in releases.** - The AppImage's Python (scripts/linux/deps.sh) and the Windows builder's Python part (scripts/release_parts.py) now install their pinned packages with pip `--require-hashes`, from a new `requirements-lock.txt`. - That file holds the SHA-256 of each pin's wheels for Linux x86_64 and Windows x64. `python -I scripts/python_lock.py` writes it after a pin changes. - `tests/test_python_lock.py` fails when the lock and the pins differ. - The AppImage's dependency cache key includes the lock. - **scripts/ffx_sdk.py and scripts/ngx_sdk.py.** Downloads and copies go to a new temp file next to the target instead of a fixed `<name>.part`. Each file is hashed while it downloads, and a download stops past 1 GB. The pinned commits and SHA-256s are unchanged. - **scripts/release_rc.sh.** It copies the newest release candidate's AppImage and `.zsync` into the `rc` release only when GitHub gives a SHA-256 digest for each file. This is checked before the tag moves or anything uploads. Before, a file without a digest was copied unchecked. - **Dependabot.** It now proposes updates to the pinned Python packages (`requirements.txt`, `nx2/prompt_requirements.txt`) weekly, with the same 7-day cooldown as the actions. After it changes a pin, `scripts/python_lock.py` must write the lock again; `tests/test_python_lock.py` fails until it does. The builder's .NET project has no NuGet packages, so there is no NuGet entry. - **scripts/linux/deps.sh.** The four license texts the AppImage ships now go through `fetch` with SHA-256 pins, like every other download there. - **New tests/test_workflow_hardening.py.** It checks every workflow for: - actions pinned by full commit; - token permissions set at the top; - checkouts with `persist-credentials: false` (except the release record job's); - no `${{ }}` of event data or inputs inside a `run:` script; - no checkout in a `pull_request_target` workflow. ## Tested Linux, RTX 3090. The branch merges main@origin `ebbba2ea`, which carries `scripts/release_rc.sh` and the setup-list code changed here; the merge had no conflicts. Testing used merges with main as it moved (`e19d9693`, `d7a73062`, `ebbba2ea`, `49f75dc8`), all without conflicts. The link and env game runs used the `e19d9693` merge, the FIFO run and the last game build the `d7a73062` merge, and the last full suite the `49f75dc8` merge. - **New and changed checks.** Each one fails on the code before its fix and passes after. Each commit's own checks pass at that commit. The existing checks that compile code cut from the changed files now include the helpers, and they pass. The ones that also run on Windows were compiled with llvm-mingw too. - **Full suite (`python -I -B tests/run_all.py`).** 373 passed, 1 failed, 12 skipped. The failure, `tests/test_wayland_libdecor_hint.py`, also fails on main on this PC. - **Python package lock.** pip downloaded every pinned wheel with `--require-hashes` for Linux x86_64 / Python 3.12 (with dependencies) and for Windows x64 / Python 3.12, 3.13 and 3.14 (as the Windows part installs them): 10 of 10 each time. - **Compile.** - `host/hos_update.c` compiles with no warnings (`-Wall -Wextra`) under gcc (Linux) and llvm-mingw clang (Windows). - The other changed host files pass a syntax check on both. - The Windows renderer (`hookshot_vk.dll`) cross-built with llvm-mingw before the last round. After it, `vk_draw.cpp` and `vk_main.cpp` pass a syntax check for both Linux and Windows. `host/nvn_native.c`, `builder/setup/setup.c` and the host files that include `hos_fopen_write.h` pass one for both too. - **Game build.** A full build of the merge with hookshot-build finished, including shader preparation. A rebuild into the existing game folder took the "read from this .nup before" path, which the new link check does not refuse. All 25,251 shipped pipeline states were noted, and the 996 MB pipeline cache was read. With a `language.txt` link planted in the folder, the build stopped at the start and named it, and the link's target was left as it was. - **Headless game runs** (`HOS_HEADLESS=1 HOS_NO_SDL=1`, separate data folder, links planted in the game folder; final run on the branch head): - nothing was written through the link at `hookshot_options.ini` or at the pipeline marker; - `user/` was moved to the saves folder, and its `user/save` link was moved as a link with its target left alone; - a relative `RX_VK_DLSS_DIR` was left out of `hookshot_env.txt`; - SDL's disk audio and save-frames switches, `RX_VK_FRAME_TRACE`, `HOOKSHOT_MEM_LIMIT_GB`, `HOOKSHOT_JOBS` and `HOOKSHOT_PREPARE_ONLY` were left out, and no `sdlaudio.raw` or `hookshot_frames.csv` appeared; - the game's own audio wave buffers and controller reads passed the new guest-memory checks (all 8 wave buffers logged as taken, the controller read as before); - the game started and ran its shader warm-up. - **Headless run with FIFOs** at `language.txt`, `hookshot_env.txt` and `hookshot_options.ini`. On a build that had moved only the host's reads, the game hung after listing the GPUs, on the renderer's own read of `hookshot_options.ini`. With every start-up read moved over, it started and ran as usual and never waited on the FIFOs. - **.NET (SDK 8.0.428 on Linux).** - `tests/test_updater_asset_digest.py` builds the whole `Updater.cs` with its fixture: 22 of 22 cases pass. Its two new cases fail on the code before the fix. - `tests/test_builder_dlss_update.py` passes (57 checks). - The WPF builder project builds with `EnableWindowsTargeting=true`, with no warnings. ## Not tested - **Windows, nothing was run:** - the update offer with a real WPF or Python builder, and the environment hand-off to it; - the junction / OneDrive check for the skip file; - the Python builder against GitHub. - the Windows branch of `hos_fopen_write`, which was only tested with a stand-in `windows.h`. The Windows host files were only syntax-checked. - **The WPF builder was not run**, only built. Its updater was run through the test fixture. - **The Linux update offer end to end** (it opens a zenity window). The checks behind it are unit-tested, the environment hand-off included. - **The pip lock in CI:** it has not run in the AppImage or Windows release jobs yet. - **Gameplay:** the headless runs stopped after 60-120 s, before gameplay. There was no windowed run with a controller. - **The last round's game changes were never in a built game.** That covers the vertex stride and divisor limits, the bounded NVN proc names and the log line for a refused write. The build queue was full. They are covered by their tests and the syntax checks above. - **Left for later:** - signing the release lists; - a structural check of cached SPIR-V and an install key for the program cache. The same file's reads and writes and its shader seed also need the regular-file, no-link and size rules, after the change being made to that file now; - the Landlock sandbox; - the nightly CI cache policy;
hckrman101 (Migrated from github.com) reviewed 2026-11-11 19:33:16 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for picking this up from #2326 and for the clear takeover notes. I went through the update offer, the builders' download and list checks, the env file rules, hos_fopen_write/hos_fopen_read, the saves move, the builder's folder-link checks and the HLE ranges. Mostly this does what it says. A few things below.

What holds up

  • Update offer: builder_ok matches what the three builders actually write (Runner.cs ProcessPath, paths.launcher_command() with list2cmdline(["-I", start]), AppRun's $APPIMAGE / $HERE/AppRun). The env hand-off is correct on both sides: Windows _putenv updates the process block ShellExecuteW passes on, and Linux builds envp before the fork.
  • Downloads: every redirect hop is checked now, not just the final URL. The size cap applies while reading, before anything is written, and the part URL prefix plus no ../%/?/@/: closes the "any https host" gap. release_rc.sh checks every digest before the tag moves.
  • hos_fopen_write on Linux: O_NOFOLLOW|O_NONBLOCK, then fstat for a regular file, then truncate. That order closes the link and FIFO cases. The pipeline caches still go .tmp then rename, so a link at the final name gets replaced, never written through.
  • HLE: the new hos_guest_range checks work like the existing fs/libc ones, and the wave-buffer mixer already bounds each read by w->size, so checking [buffer, size) up front is the right span. I didn't find a check that would reject a valid call.
  • Every new test is game-free, and the network ones only use localhost. Each one has a RUN_ALL_SKIP where it needs one.

Worth a look

  1. Windows "Skip this version" refuses a user folder that is a junction (inline). That's where Windows keeps the saves, and a junction is the usual way to move them to another drive. In that case the skip silently fails and the offer comes back every start. On Linux the takeover deliberately allows a linked saves folder. I'd keep only the file-name check on Windows too.
  2. A related trade-off on Linux: a player who deliberately symlinks hookshot_options.ini, mouse_sens.txt or a pipeline cache (dotfile managers, a cache on another disk) now gets no write, just one log line. It's defensible for archive-borne links, but a line in the readme's file section would save a confused report.
  3. Overlap with open PRs. The order needs deciding:
    • #2367 (portable Windows builder) records a relative builder_portable, which builder_ok would refuse. Arkex-ai already raised it on #2326 and it isn't settled yet.
    • #1414 makes the setup and updaters follow the repo they came from, while part_url_ok / PART_SOURCES hardcode NotRexed/Hookshot. Whichever lands second has to reconcile the two.
    • #2317 (per-user data folder) touches 22 of these files, including the paths the new write/read helpers wrap.

CI: the only red job is Windows part 1/2, on test_native_astc_texture_count.py (scanf deprecated under -Werror). That's main's: #2397 fixed it, and main's source checks are green since. A merge with main should clear it. GitHub reports no conflicts right now.

Splitting: the commits are already split by area, so I wouldn't force a split. If @NotRexed would rather take it in pieces, the natural cut is (a) release/CI/lock/dependabot, (b) builders and update offer, (c) game file I/O and env file, (d) HLE ranges and pipeline record limits. Only (b) collides with #2367/#1414.

Not run on Windows yet: the update offer with a real builder, the junction check and the hos_fopen_write Windows branch. A Windows run of the update offer before merge would be good.

Thanks for picking this up from #2326 and for the clear takeover notes. I went through the update offer, the builders' download and list checks, the env file rules, `hos_fopen_write`/`hos_fopen_read`, the saves move, the builder's folder-link checks and the HLE ranges. Mostly this does what it says. A few things below. **What holds up** - Update offer: `builder_ok` matches what the three builders actually write (`Runner.cs` ProcessPath, `paths.launcher_command()` with `list2cmdline(["-I", start])`, AppRun's `$APPIMAGE` / `$HERE/AppRun`). The env hand-off is correct on both sides: Windows `_putenv` updates the process block `ShellExecuteW` passes on, and Linux builds `envp` before the fork. - Downloads: every redirect hop is checked now, not just the final URL. The size cap applies while reading, before anything is written, and the part URL prefix plus no `..`/`%`/`?`/`@`/`:` closes the "any https host" gap. `release_rc.sh` checks every digest before the tag moves. - `hos_fopen_write` on Linux: `O_NOFOLLOW|O_NONBLOCK`, then `fstat` for a regular file, then truncate. That order closes the link and FIFO cases. The pipeline caches still go `.tmp` then rename, so a link at the final name gets replaced, never written through. - HLE: the new `hos_guest_range` checks work like the existing fs/libc ones, and the wave-buffer mixer already bounds each read by `w->size`, so checking `[buffer, size)` up front is the right span. I didn't find a check that would reject a valid call. - Every new test is game-free, and the network ones only use localhost. Each one has a `RUN_ALL_SKIP` where it needs one. **Worth a look** 1. Windows "Skip this version" refuses a `user` folder that is a junction (inline). That's where Windows keeps the saves, and a junction is the usual way to move them to another drive. In that case the skip silently fails and the offer comes back every start. On Linux the takeover deliberately allows a linked saves folder. I'd keep only the file-name check on Windows too. 2. A related trade-off on Linux: a player who deliberately symlinks `hookshot_options.ini`, `mouse_sens.txt` or a pipeline cache (dotfile managers, a cache on another disk) now gets no write, just one log line. It's defensible for archive-borne links, but a line in the readme's file section would save a confused report. 3. Overlap with open PRs. The order needs deciding: - #2367 (portable Windows builder) records a relative `builder_portable`, which `builder_ok` would refuse. Arkex-ai already raised it on #2326 and it isn't settled yet. - #1414 makes the setup and updaters follow the repo they came from, while `part_url_ok` / `PART_SOURCES` hardcode `NotRexed/Hookshot`. Whichever lands second has to reconcile the two. - #2317 (per-user data folder) touches 22 of these files, including the paths the new write/read helpers wrap. **CI**: the only red job is Windows part 1/2, on `test_native_astc_texture_count.py` (`scanf` deprecated under `-Werror`). That's main's: #2397 fixed it, and main's source checks are green since. A merge with main should clear it. GitHub reports no conflicts right now. **Splitting**: the commits are already split by area, so I wouldn't force a split. If @NotRexed would rather take it in pieces, the natural cut is (a) release/CI/lock/dependabot, (b) builders and update offer, (c) game file I/O and env file, (d) HLE ranges and pipeline record limits. Only (b) collides with #2367/#1414. Not run on Windows yet: the update offer with a real builder, the junction check and the `hos_fopen_write` Windows branch. A Windows run of the update offer before merge would be good.
host/hos_update.c
hckrman101 (Migrated from github.com) commented 2026-11-11 19:33:16 +00:00

!is_link(user, ua) refuses a junctioned <game>\user, which is where Windows keeps the saves and the usual way to move them to another drive. In that case the skip is silently not written and the offer comes back every start. The Linux write_skip deliberately allows a linked saves folder and only refuses a link at the file's own name. Dropping the folder check here and keeping !is_link(file, fa) would match that.

`!is_link(user, ua)` refuses a junctioned `<game>\user`, which is where Windows keeps the saves and the usual way to move them to another drive. In that case the skip is silently not written and the offer comes back every start. The Linux `write_skip` deliberately allows a linked saves folder and only refuses a link at the file's own name. Dropping the folder check here and keeping `!is_link(file, fa)` would match that.
hckrman101 (Migrated from github.com) reviewed 2026-11-11 20:32:16 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for the merge. I compared the PR's patch at 348d26cc with the one at d827111d file by file: all 84 files are line-for-line the same, so this push is the main merge only.

The make_game.py resolution is right. stand_in_damaged_files keeps all of #2096's flow (the resolved-v1 checked stamp and removing an old one, the note written before the stop, the fail on unresolved files, the stamp and heads removal only after that) and wraps the PR's checks around the heads list: .zs only, nup.extraction_path(..., links_only=True) for each entry, falling back to opening every file if one doesn't pass. Nothing from either side is dropped.

Earlier points, none changed by this push:

  1. Windows "Skip this version" still refuses a junctioned user folder (!is_link(user, ua) in host/hos_update.c, inline thread above). Still open; I'd keep only the file-name check, as on Linux.
  2. The readme line for players who symlink hookshot_options.ini, mouse_sens.txt or a pipeline cache on purpose isn't there yet. Small, but still open.
  3. Overlaps: #2367, #1414 and #2317 are all still open, and the builder_ok vs relative builder_portable question with #2367 is still unsettled. Nothing to fix here on its own, but the order needs deciding with @NotRexed.
  4. Still no Windows run of the update offer, the junction check or the hos_fopen_write Windows branch. The 414 local checks are a good sign, but they don't cover those.

CI: the new head's runs are only queued so far. Main now has #2397, so the Windows test_native_astc_texture_count.py failure from last time should be gone; worth a look once they finish.

Nothing new from the merge. Point 1 and a Windows run are what I'd still want before merge.

Thanks for the merge. I compared the PR's patch at 348d26cc with the one at d827111d file by file: all 84 files are line-for-line the same, so this push is the main merge only. **The make_game.py resolution** is right. `stand_in_damaged_files` keeps all of #2096's flow (the `resolved-v1` checked stamp and removing an old one, the note written before the stop, the `fail` on unresolved files, the stamp and heads removal only after that) and wraps the PR's checks around the heads list: `.zs` only, `nup.extraction_path(..., links_only=True)` for each entry, falling back to opening every file if one doesn't pass. Nothing from either side is dropped. **Earlier points**, none changed by this push: 1. Windows "Skip this version" still refuses a junctioned `user` folder (`!is_link(user, ua)` in `host/hos_update.c`, inline thread above). Still open; I'd keep only the file-name check, as on Linux. 2. The readme line for players who symlink `hookshot_options.ini`, `mouse_sens.txt` or a pipeline cache on purpose isn't there yet. Small, but still open. 3. Overlaps: #2367, #1414 and #2317 are all still open, and the `builder_ok` vs relative `builder_portable` question with #2367 is still unsettled. Nothing to fix here on its own, but the order needs deciding with @NotRexed. 4. Still no Windows run of the update offer, the junction check or the `hos_fopen_write` Windows branch. The 414 local checks are a good sign, but they don't cover those. **CI**: the new head's runs are only queued so far. Main now has #2397, so the Windows `test_native_astc_texture_count.py` failure from last time should be gone; worth a look once they finish. Nothing new from the merge. Point 1 and a Windows run are what I'd still want before merge.
hckrman101 commented 2026-11-11 22:17:26 +00:00 (Migrated from github.com)

This now conflicts with main in builder/linux/hookshot_builder.py, host/hos_main.c, host/hos_update.c, nx2/nup.py, tests/test_bound_keys.py. Could you merge main into the branch (or rebase) when you get a chance? I'll take another look once it's up to date.

This now conflicts with main in `builder/linux/hookshot_builder.py`, `host/hos_main.c`, `host/hos_update.c`, `nx2/nup.py`, `tests/test_bound_keys.py`. Could you merge main into the branch (or rebase) when you get a chance? I'll take another look once it's up to date.
github-sync commented 2026-11-12 17:58:00 +00:00
Owner

@Joey-Magnus commented on host/hos_update.c line 324 on GitHub · 2026-11-12 17:01 UTC · original

Fixed in 70b495de: the Windows skip no longer checks the user folder for a link, only the skip file's own name (!is_link(file, fa)), as Linux's write_skip does. The folder still has to be a folder, so a junctioned <game>\user takes the file. tests/test_update_skip_file.py now fails if the folder check comes back.

> [@Joey-Magnus](https://github.com/Joey-Magnus) commented on `host/hos_update.c` line 324 on GitHub · 2026-11-12 17:01 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2430#discussion_r4553623676) Fixed in 70b495de: the Windows skip no longer checks the `user` folder for a link, only the skip file's own name (`!is_link(file, fa)`), as Linux's `write_skip` does. The folder still has to be a folder, so a junctioned `<game>\user` takes the file. `tests/test_update_skip_file.py` now fails if the folder check comes back. <!-- github-sync:rc:4553623676 -->
github-sync commented 2026-11-12 17:58:00 +00:00
Owner

@Joey-Magnus commented on GitHub · 2026-11-12 17:01 UTC · original

Merged main again and took the review points that can be done in code.

Merge with main (45f19993). Eight files conflicted, and both sides are kept in each. The merge commit lists all of them. These needed a decision:

  • host/hos_main.c: main's #2282 already makes every link in user/ again in the new place, by the full path it names, and checks every entry before it moves any. Its saves_dir replaces this PR's link handling there. copy_tree no longer fails on a link inside a folder, because main now makes that link again instead of following it. File copies still go through hos_fopen_read_binary / hos_fopen_write, so a FIFO fails the copy and nothing is written through a link. tests/test_saves_move_links.py now expects links to be made again and still checks the rest.
  • host/hos_update.c: main's builder_local (#2367: a fixed hookshot.exe in the game folder) is accepted as main accepts it. Any other builder= still has to pass builder_ok with builder_args=.
  • gpu/vk/vk_draw.cpp: main now reads a pipeline cache's header before its size. That order is kept, and the kPcacheMax limit and hos_fopen_read_binary sit on top of it.
  • nx2/nup.py: main's new is_link takes links_only, so the read-back of a folder that OneDrive keeps still counts only symlinks, junctions and WSL links.
  • builder/linux/hookshot_builder.py, builder/setup/setup.c, scripts/ngx_sdk.py and tests/test_bound_keys.py just needed both sides kept. --tag is still checked next to main's --allow-downgrade-from, part_url_ok sits beside builder_ready, and main's DLSS and Frame Generation runtimes are copied through a new temp file each.

Some of main's new checks cut code that this PR changed. In 9f329311 they get this PR's helpers (hos_guest_range, hos_fopen_read.h, kPcacheMax, linux_update). The builder has written .hookshot_builder_home in the game folder since #2367, so that name is now on the list of names where a link stops the build. test_game_folder_links.py caught it.

Review points

  1. Windows "Skip this version" with a junctioned user folder: fixed in 70b495de. Only the skip file's own name is checked for a link or junction now, the same as Linux's write_skip. tests/test_update_skip_file.py fails if the folder check comes back.
  2. Readme: fixed in 1751ff48. "Where the files go" now says that the game does not write through a link at hookshot_options.ini, mouse_sens.txt or a pipeline cache, and that it says so once in the log. It tells players to link the folder instead.
  3. Order against the other PRs: still open, and it needs @NotRexed. #2367 has landed since, and the merge takes its builder_local as is. Its relative-path question no longer comes up through builder=. #1414 (whose repository the setup follows, against the hardcoded NotRexed/Hookshot in part_url_ok / PART_SOURCES) and #2317 (the per-user data folder) are still open. Whichever lands second has to reconcile them.
  4. Windows run: still open. Nobody has run the update offer with a real builder, the junction check for the skip file, or the Windows branch of hos_fopen_write on Windows. That needs someone with a Windows PC.

Tested. Linux: python -I -B tests/run_all.py --jobs 4 gives 474 passed, 2 failed, 36 skipped. The two failures (test_lift_cache.py, and test_updater_asset_digest.py, whose .NET restore finds no packages offline) also fail on main on this PC. With llvm-mingw 20261104, the changed host files (hos_update.c, hos_main.c, hos_svc.c, the changed nn_*.c files, nvn_native.c, oot_menu.c, prompts.c, discord_rpc.c) and builder/setup/setup.c compile for Windows with no errors. hos_update.c also has no warnings under -Wall -Wextra -Werror. vk_draw.cpp, vk_main.cpp and vk_menu.cpp pass a syntax check for both Windows and Linux. There was no game build or run after this merge.

(_trash.panda's agent)

> [@Joey-Magnus](https://github.com/Joey-Magnus) commented on GitHub · 2026-11-12 17:01 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2430#issuecomment-6830282454) Merged main again and took the review points that can be done in code. **Merge with main (`45f19993`).** Eight files conflicted, and both sides are kept in each. The merge commit lists all of them. These needed a decision: - `host/hos_main.c`: main's #2282 already makes every link in `user/` again in the new place, by the full path it names, and checks every entry before it moves any. Its `saves_dir` replaces this PR's link handling there. `copy_tree` no longer fails on a link inside a folder, because main now makes that link again instead of following it. File copies still go through `hos_fopen_read_binary` / `hos_fopen_write`, so a FIFO fails the copy and nothing is written through a link. `tests/test_saves_move_links.py` now expects links to be made again and still checks the rest. - `host/hos_update.c`: main's `builder_local` (#2367: a fixed `hookshot.exe` in the game folder) is accepted as main accepts it. Any other `builder=` still has to pass `builder_ok` with `builder_args=`. - `gpu/vk/vk_draw.cpp`: main now reads a pipeline cache's header before its size. That order is kept, and the `kPcacheMax` limit and `hos_fopen_read_binary` sit on top of it. - `nx2/nup.py`: main's new `is_link` takes `links_only`, so the read-back of a folder that OneDrive keeps still counts only symlinks, junctions and WSL links. - `builder/linux/hookshot_builder.py`, `builder/setup/setup.c`, `scripts/ngx_sdk.py` and `tests/test_bound_keys.py` just needed both sides kept. `--tag` is still checked next to main's `--allow-downgrade-from`, `part_url_ok` sits beside `builder_ready`, and main's DLSS and Frame Generation runtimes are copied through a new temp file each. Some of main's new checks cut code that this PR changed. In `9f329311` they get this PR's helpers (`hos_guest_range`, `hos_fopen_read.h`, `kPcacheMax`, `linux_update`). The builder has written `.hookshot_builder_home` in the game folder since #2367, so that name is now on the list of names where a link stops the build. `test_game_folder_links.py` caught it. **Review points** 1. Windows "Skip this version" with a junctioned `user` folder: fixed in `70b495de`. Only the skip file's own name is checked for a link or junction now, the same as Linux's `write_skip`. `tests/test_update_skip_file.py` fails if the folder check comes back. 2. Readme: fixed in `1751ff48`. "Where the files go" now says that the game does not write through a link at `hookshot_options.ini`, `mouse_sens.txt` or a pipeline cache, and that it says so once in the log. It tells players to link the folder instead. 3. Order against the other PRs: still open, and it needs @NotRexed. #2367 has landed since, and the merge takes its `builder_local` as is. Its relative-path question no longer comes up through `builder=`. #1414 (whose repository the setup follows, against the hardcoded `NotRexed/Hookshot` in `part_url_ok` / `PART_SOURCES`) and #2317 (the per-user data folder) are still open. Whichever lands second has to reconcile them. 4. Windows run: still open. Nobody has run the update offer with a real builder, the junction check for the skip file, or the Windows branch of `hos_fopen_write` on Windows. That needs someone with a Windows PC. **Tested.** Linux: `python -I -B tests/run_all.py --jobs 4` gives 474 passed, 2 failed, 36 skipped. The two failures (`test_lift_cache.py`, and `test_updater_asset_digest.py`, whose .NET restore finds no packages offline) also fail on main on this PC. With llvm-mingw 20261104, the changed host files (`hos_update.c`, `hos_main.c`, `hos_svc.c`, the changed `nn_*.c` files, `nvn_native.c`, `oot_menu.c`, `prompts.c`, `discord_rpc.c`) and `builder/setup/setup.c` compile for Windows with no errors. `hos_update.c` also has no warnings under `-Wall -Wextra -Werror`. `vk_draw.cpp`, `vk_main.cpp` and `vk_menu.cpp` pass a syntax check for both Windows and Linux. There was no game build or run after this merge. (_trash.panda's agent) <!-- github-sync:c:6830282454 -->
Commenting is not possible because the repository is archived.
Reviewers
No reviewers
Labels
Clear labels   
accessibility
Barrier affecting people with disabilities

  
autorelease: pending

   
autorelease: tagged

   
bug
Something isn't working

  
dependencies
Pull requests that update a dependency file

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
github_actions
Pull requests that update GitHub Actions code

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
testing wanted

   
wontfix
This will not be worked on

No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
2 participants
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Hookshot!2430
No description provided.