WIP: fix(security): shared files, links and lists can't make the game, Builder or tools write or read where they shouldn't #218

Draft
TrashPanda wants to merge 37 commits from TrashPanda/Longshot:security/hardening-passes into main
Contributor

Why

Files, settings and lists people share or download (a longshot_env.txt from a guide, a pipeline list someone sends, a game folder kept in OneDrive, a link left in the game folder) could make the game, the Builder or the tools write through a link, read past a file's end or take values nothing should have. This brings over the Hookshot branch security/hardening-passes, which was never merged there before Hookshot was taken down, so Longshot gets the same checks.

What changed

28 of the branch's 39 commits, each with its author, date and message (hckrman101 23, Joey Magnus 5), then 4 commits that fit them to Longshot, 4 that answer the first review and 1 that answers the second (b7a933da, by HumanBean). The other 11 are listed under Reviewer notes.

  • game, writes: host/hos_fopen_write.h. Every file the game writes on its own is written without following a symbolic link or, on Windows, a junction at its name: longshot_options.ini, mouse_sens.txt, language.txt (from the first start and from the menu), longshot_log.txt, the pipeline caches and lists, the program cache longshot_programs.bin and its .idx (with the "r+b" that cuts it back to its last whole record) and its lock, longshot_module_ids.bin, the markers (load-default, its strike file, "preparation skipped", the pipeline stamp's .tmp), the bug report's files, and the button-prompt textures. A refused write is said once in the log.
    • Linux: O_NOFOLLOW, then fstat on the descriptor, and a regular file only.
    • Windows: the name is opened as itself (FILE_FLAG_OPEN_REPARSE_POINT) and refused when it is a name surrogate (IsReparseTagNameSurrogate: symbolic links, junctions, WSL's, WCI_LINK, GLOBAL_REPARSE). A file with no reparse point (the usual case) is then opened for writing as itself in one step, and refused if that handle shows a reparse point or a folder: no file index is compared, so file systems that make it up for each open (some network redirectors, FUSE/WinFsp mounts, a virtual machine's shared folders) still save. A reparse point that is not a link (OneDrive's files, deduplicated or compressed ones) is opened again without the flag, so one kept only online is filled first, and kept only when it is the same file (volume and file index; an index of 0 is refused). Paths go from UTF-8 to CreateFileW (hos_fopen_write_handle, hos_fopen_kind).
    • Out of scope, on purpose: a link at a folder on the way to the file (the game folder, user/, bug-reports, prompts) is followed, as a linked game or user folder and OneDrive need; hard links are not told apart from the file. The README and the header say so.
    • The README says which files and folders, and to link the whole game folder (or user/) instead.
  • game, reads: the bug report copies longshot_log.txt and longshot_log.1.txt (which the player sends) only through hos_fopen_read_nofollow (host/hos_fopen_write.h, copy_log in gpu/vk/vk_bug_report.cpp): not through a link at the name and a regular file only, on Linux and Windows, so no other file can end up in the report and a FIFO there doesn't hold up the Insert menu. host/hos_fopen_read.h: start-up files from the game folder are read only when they are regular files (the program cache and its index, the markers and the stamp; and now longshot_module_ids.bin, longshot_shaders.bin, image.bin, the prompt files the game reads back, and RX_VK_DRAW_STATS' trigger file); the button-prompt file only with its magic and under 256 MB; a pipeline cache file only after its header and not past 4 GB (kPcacheMax, gpu/vk/vk_draw.cpp).
  • env file: a shared longshot_env.txt sets only the game's own LONGSHOT_ switches and no path-valued switch at all. That includes RX_VK_DLSS_DIR, which picks the folder NGX loads DLSS from; the game's ngx folder and %LOCALAPPDATA%\Longshot\ngx / $XDG_DATA_HOME/longshot/ngx need no switch. It also takes no dump or SDL file-writing switch, no frame trace, not HOS_PGO, and not the control bridge (env_line_allowed, host/hos_main.c). The environment and a file HOS_ENV_FILE names still set all of them.
  • guest memory: spans the game names are checked to lie in guest memory in more host functions (hos_guest_range: audio, HID, applets, NVN's proc names, oot_menu.c).
  • pipelines: a pipeline from the notes or a state list takes only a sample count the device draws with and the vertex strides and divisors Vulkan allows (targets_ok, streams_ok). nx2/pipeline_states.py's check_records refuses records with values no pipeline has before a list is added, merged or shipped.
  • input: only a finite mouse sensitivity within the menu's range.
  • builder: a link where the Builder writes in a game folder stops it (refuse_written_links, also at longshot_options.ini and .longshot_builder_home). For a folder it writes into (bin, romfs, prompts, ngx, ffx, .game, .longshot_exefs), the error names the folder and says to move the whole game folder to the other drive instead. What it reads back stays inside the folder. A folder OneDrive keeps still takes the known damaged-texture list (nup.extraction_path's links_only). The Builder and nup.py take a reparse point as a link by the name-surrogate bit (link_tag), the game's rule, not by three fixed tags.
  • scripts and release: SDK runtime downloads go to new temp files, hashed as they arrive and capped. The AppImage's license texts come through deps.sh's checked download. The tools' Python packages are installed with pip --require-hashes from requirements-lock.txt, both the Linux AppImage's (deps.sh) and the Windows builder's (scripts/tools.py assemble_python).
  • ci: tests/test_workflow_hardening.py keeps the workflows pinned, with no token left behind and no untrusted ${{ }} in a script. It now reads .forgejo/workflows and Forgejo's full-URL action names; third-party.yml's github.base_ref goes through env:.
  • checks: tests/test_game_writes_no_links.py looks through every source under host/ and gpu/vk/ (not host/posix or the vendored code) for opens that write. Each must go through hos_fopen_write, make its file new ("x", O_EXCL, O_NOFOLLOW, CREATE_NEW), or be in a reviewed list. That list holds the switch-gated dumps and traces, each with its switch and reason (a switch the shared file cannot set, checked), and four writes that name no file of the game folder: the save file system, Discord's pipe, the console and udmabuf. A new cache file written with a plain fopen fails it.

Fitting the branch to Longshot (the 4 commits after the port, and conflict notes in the ported ones):

  • pipeline_states.py: Longshot's read_sent and its xz limit kept. check_records runs beside check_kinds, and it skips compute-program records (a key and zeros), which check_kinds already holds to exactly that.
  • nup.py: Longshot's package_parts, is_link and Destination kept; is_link takes links_only.
  • test_workflow_hardening.py: release.yml and website.yml have no top-level permissions: (Forgejo 17 ignores the key), and pr-description.yml and website.yml check out under pull_request_target (main's script only, and a fork's commit only after a maintainer's preview label). The check lists these as reviewed, with the reasons.
  • Fixtures that cut hardened code get its helpers (test_linux_saves_move, test_mouse_sens_file, test_prepare_stall, test_mesh_mode_switch, test_predict_resume, test_env_file, test_pipeline_state_computes, and after the rebase test_udmabuf_warning and test_upscaler_round_trip); test_pipeline_share's records are ones the game can make.

Flags and settings

Flag What it does Default now Recommendation Helps on Doesn't help on Untested on
longshot_env.txt Takes fewer switches: no path-valued switch, RX_VK_DLSS_DIR included. Also not RX_SCREENSHOT, RX_FLASH_LOG, HOS_SCAN_DT (dumps), RX_VK_FRAME_TRACE, HOS_PGO, SDL's file-writing switches, HOS_CONTROL or HOS_CONTROL_MEM. Of the LONGSHOT_ names it takes only the game's own, LONGSHOT_NO_AV1_NOTICE and LONGSHOT_OCCLUSION_OFF. No switch is new; set in the environment, or in a file HOS_ENV_FILE names, each works as before. On On: a file shared by others should not set where the game writes, what it loads or what it runs All - -

Measurements

main: f87557ae (2026-11-13)
this PR: security/hardening-passes @ b7a933da

The rows below are from 46ec4bd7 on main d906c8f4. b7a933da (rebased onto f87557ae, plus the second review's fixes) has not been run or built yet: nothing was run on the PC that made it. The 36 earlier commits rebased with the same changed lines.

No speed numbers: checks on reads, writes and shared files, off the frame path. The game-free checks on both sides:

main @ d906c8f4 this PR @ 46ec4bd7
python -I -B tests/run_all.py --jobs 4 failures (Linux, this PC) test_lift_cache (no numpy here), test_menu_i18n (since #138) the same 2: 508 passed, 2 failed, 30 skipped
longshot_vk.dll (RX_TARGET=windows LONGSHOT_MINGW=<llvm-mingw> RX_VK_ONLY=1 nx2/build.py --standalone, on Linux) - builds and links, 0 failed, no warnings
longshot_vk.so objects (RX_VK_ONLY=1 nx2/build.py --standalone, Linux) - all compile, 0 failed; the link needs lld, which this PC lacks

Testing

Tested: on Linux (RTX 4070 Ti, driver 595.99.02, Ryzen; Python 3.14), without the game:

  • python -I -B tests/run_all.py --jobs 4: 508 passed, 2 failed, 30 skipped. Both failures also fail on main here: test_lift_cache (no numpy on this PC) and test_menu_i18n (pt_BR's text for a row #138 changed).
  • tests/test_game_writes_no_links.py fails on 83e9a080 (the head the review read) with every write the review named, plus the log, the bug report, the program cache's lock and the probe and draw-stats files, and passes here.
  • tests/test_env_file.py refuses RX_VK_DLSS_DIR from the shared file for every value (/tmp/x, /dev/shm/x, C:\Users\Public\..., Z:\..., /proc/self/cwd and the rest), and takes it from HOS_ENV_FILE's file and the environment.
  • hos_fopen_write.h built against llvm-mingw's real Windows headers, as C and as C++ (LONGSHOT_MINGW=... tests/test_game_writes_no_links.py). The Windows renderer longshot_vk.dll builds and links. The Windows branch's behaviour (surrogates refused before any write open, OneDrive tags written, a name swapped between the two opens refused, "r+b" not creating) runs against a stand-in windows.h (on 46ec4bd7; b7a933da adds plain files opened in one step, no file index for them, a link swapped in before the open refused, a 0 index refused for OneDrive's, and the bug report's read, not yet run).
  • pip install --require-hashes of nx2/prompt_requirements.txt for win_amd64 from the lock, against PyPI: installs.

Not run on b7a933da: tests/run_all.py, the llvm-mingw builds and every check above, for the head that answers the second review. They were read, not run; they still need to run.

Not tested: the game. The host C (host/*.c) built whole needs the lifted game's headers (a64.h), which only a build from a dump makes; the checks that cut and compile host functions pass. Windows: nothing ran on Windows itself (no Windows PC or Wine here), so CreateFileW/_open_osfhandle on a real NTFS junction, symlink or OneDrive placeholder is untested. The two Windows requests wait for the testing wanted label before anyone can claim them.

Needs other testers:

  • Windows: a full build and play, with the game folder under OneDrive, expecting the build to keep the damaged-texture list, and the game to save, keep its options and language, write its log and reuse its program cache (T-93FV)
  • Windows: a junction or link at longshot_options.ini and at longshot_programs.bin, expecting the files they point at to be left alone and one log line each (T-AJTA)
  • Linux: a full build and play from a dump, expecting the same as main: saves, options, the log, pipeline and program caches and button prompts load (request to open)

Screenshots

Nothing on screen

Risk and rollback

A check that is too strict would refuse a file the game needs: options or a save not written, a pipeline or program cache not read or kept, the log not opened, or a longshot_env.txt switch that someone relied on ignored (RX_VK_DLSS_DIR there now is: it goes in the environment, such as Steam's launch options). Each refusal says so in longshot_log.txt, but a refused log falls back to where stderr went before. PRs are squash-merged, so reverting this PR reverts all of it; the parts are in separate commits here for review, not for separate reverts on main.

Reviewer notes

Ported with work/port_to_longshot.py (renames hookshot to longshot, .github/ to .forgejo/), as #209 was. Commit messages are kept as written, so some still say hookshot_env.txt. Where to look first: host/hos_fopen_write.h, host/hos_fopen_read.h, env_line_allowed in host/hos_main.c, REVIEWED in tests/test_game_writes_no_links.py, and nx2/pipeline_states.py.

This is several topics in one PR (link-safe writes and the Builder, the env file, HLE guest ranges, pipeline-list validation, the supply chain). It can be split along those lines; see the answer to the review.

REVIEWED keeps the save file system's OpenFile (host/nn_fs.c, the game's saves in user/, opened again after the game made them) as a write that follows a link: it is not a name the game folder ships, and making it no-follow needs the Linux stand-in CreateFileA (host/posix) to take FILE_FLAG_OPEN_REPARSE_POINT. A follow-up if wanted.

Not covered, on purpose: links at folders along the path are followed (a linked user/, a linked game folder and OneDrive need it), and hard links aren't caught (an archive can only hard-link within itself; refusing them would break deduplicated or hard-link-copied folders).

Still on a plain fopen read, reviewed: the program cache's re-opens in vk_progcache.cpp (of the file the game has just opened through hos_fopen_write), own_handle in vk_programs.cpp (pipeline caches already read as regular files), the debugging reads behind switches a shared longshot_env.txt cannot set (RX_VK_SKIP_FILE, HOS_INPUT_LIVE, HOS_WATCH, HOS_SCAN_DT, HOS_CHAIN, the control bridge), and system files (/etc/os-release, /proc, /sys, user-dirs.dirs).

Not ported (11):

  • 875b8bdb, 7a56ee64, 70b495de: the game's update check and its skip file, removed in #13.
  • 1a7dd521: the Python builder, removed in #181.
  • c03691a7, 8f029bff, c5fdeb2b: the WPF builder's release-name and download rules (files Longshot removed).
  • b7fa0d7b: release_rc.sh (removed).
  • 6bb6763e: Dependabot (GitHub only).
  • 54dba137: Longshot already copies a link in user/ as a link and deletes only the link (copy_link, delete_tree).
  • 8d23c0a0 "setup lists' parts come only from this project's releases": needs a port of its own. Longshot's setup fetches the launcher and tools.lock's parts from Forgejo and its mirrors, so the GitHub host allowlist doesn't carry over; a follow-up.

Issues

See #209

Checklist

For everyone

  • One topic. (Not yet: several, see Reviewer notes.)
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main.
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above). (Not yet on b7a933da: last run on 46ec4bd7; see Measurements.)
  • Tested in the game, or what wasn't is named under Testing.
  • Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (Not applicable: no speed change; the checks on both sides are above.)
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Flags noted, and the switch reference updated (docs/development.md, the README).
  • A game-free test added for the fix, failing on main and passing here. (Added and updated for b7a933da, test_game_writes_no_links.py and test_game_folder_links.py, but not run yet.)
  • A check under tests/ that reads changed source updated in this PR, not weakened.
  • Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. (The renderer builds for Windows with llvm-mingw and compiles for Linux; the host C needs a dump's headers and is still to build: Testing.)
  • Docs updated.

🤖 Generated with Claude Code

## Why Files, settings and lists people share or download (a `longshot_env.txt` from a guide, a pipeline list someone sends, a game folder kept in OneDrive, a link left in the game folder) could make the game, the Builder or the tools write through a link, read past a file's end or take values nothing should have. This brings over the Hookshot branch `security/hardening-passes`, which was never merged there before Hookshot was taken down, so Longshot gets the same checks. ## What changed 28 of the branch's 39 commits, each with its author, date and message (hckrman101 23, Joey Magnus 5), then 4 commits that fit them to Longshot, 4 that answer the first review and 1 that answers the second (b7a933da, by HumanBean). The other 11 are listed under Reviewer notes. - game, writes: `host/hos_fopen_write.h`. Every file the game writes on its own is written without following a symbolic link or, on Windows, a junction at its name: `longshot_options.ini`, `mouse_sens.txt`, `language.txt` (from the first start and from the menu), `longshot_log.txt`, the pipeline caches and lists, the program cache `longshot_programs.bin` and its `.idx` (with the `"r+b"` that cuts it back to its last whole record) and its lock, `longshot_module_ids.bin`, the markers (load-default, its strike file, "preparation skipped", the pipeline stamp's `.tmp`), the bug report's files, and the button-prompt textures. A refused write is said once in the log. - Linux: `O_NOFOLLOW`, then `fstat` on the descriptor, and a regular file only. - Windows: the name is opened as itself (`FILE_FLAG_OPEN_REPARSE_POINT`) and refused when it is a name surrogate (`IsReparseTagNameSurrogate`: symbolic links, junctions, WSL's, `WCI_LINK`, `GLOBAL_REPARSE`). A file with no reparse point (the usual case) is then opened for writing as itself in one step, and refused if that handle shows a reparse point or a folder: no file index is compared, so file systems that make it up for each open (some network redirectors, FUSE/WinFsp mounts, a virtual machine's shared folders) still save. A reparse point that is not a link (OneDrive's files, deduplicated or compressed ones) is opened again without the flag, so one kept only online is filled first, and kept only when it is the same file (volume and file index; an index of 0 is refused). Paths go from UTF-8 to `CreateFileW` (`hos_fopen_write_handle`, `hos_fopen_kind`). - Out of scope, on purpose: a link at a folder on the way to the file (the game folder, `user/`, `bug-reports`, `prompts`) is followed, as a linked game or `user` folder and OneDrive need; hard links are not told apart from the file. The README and the header say so. - The README says which files and folders, and to link the whole game folder (or `user/`) instead. - game, reads: the bug report copies `longshot_log.txt` and `longshot_log.1.txt` (which the player sends) only through `hos_fopen_read_nofollow` (`host/hos_fopen_write.h`, `copy_log` in `gpu/vk/vk_bug_report.cpp`): not through a link at the name and a regular file only, on Linux and Windows, so no other file can end up in the report and a FIFO there doesn't hold up the Insert menu. `host/hos_fopen_read.h`: start-up files from the game folder are read only when they are regular files (the program cache and its index, the markers and the stamp; and now `longshot_module_ids.bin`, `longshot_shaders.bin`, `image.bin`, the prompt files the game reads back, and `RX_VK_DRAW_STATS`' trigger file); the button-prompt file only with its magic and under 256 MB; a pipeline cache file only after its header and not past 4 GB (`kPcacheMax`, `gpu/vk/vk_draw.cpp`). - env file: a shared `longshot_env.txt` sets only the game's own `LONGSHOT_` switches and no path-valued switch at all. That includes `RX_VK_DLSS_DIR`, which picks the folder NGX loads DLSS from; the game's `ngx` folder and `%LOCALAPPDATA%\Longshot\ngx` / `$XDG_DATA_HOME/longshot/ngx` need no switch. It also takes no dump or SDL file-writing switch, no frame trace, not `HOS_PGO`, and not the control bridge (`env_line_allowed`, `host/hos_main.c`). The environment and a file `HOS_ENV_FILE` names still set all of them. - guest memory: spans the game names are checked to lie in guest memory in more host functions (`hos_guest_range`: audio, HID, applets, NVN's proc names, `oot_menu.c`). - pipelines: a pipeline from the notes or a state list takes only a sample count the device draws with and the vertex strides and divisors Vulkan allows (`targets_ok`, `streams_ok`). `nx2/pipeline_states.py`'s `check_records` refuses records with values no pipeline has before a list is added, merged or shipped. - input: only a finite mouse sensitivity within the menu's range. - builder: a link where the Builder writes in a game folder stops it (`refuse_written_links`, also at `longshot_options.ini` and `.longshot_builder_home`). For a folder it writes into (`bin`, `romfs`, `prompts`, `ngx`, `ffx`, `.game`, `.longshot_exefs`), the error names the folder and says to move the whole game folder to the other drive instead. What it reads back stays inside the folder. A folder OneDrive keeps still takes the known damaged-texture list (`nup.extraction_path`'s `links_only`). The Builder and `nup.py` take a reparse point as a link by the name-surrogate bit (`link_tag`), the game's rule, not by three fixed tags. - scripts and release: SDK runtime downloads go to new temp files, hashed as they arrive and capped. The AppImage's license texts come through `deps.sh`'s checked download. The tools' Python packages are installed with `pip --require-hashes` from `requirements-lock.txt`, both the Linux AppImage's (`deps.sh`) and the Windows builder's (`scripts/tools.py assemble_python`). - ci: `tests/test_workflow_hardening.py` keeps the workflows pinned, with no token left behind and no untrusted `${{ }}` in a script. It now reads `.forgejo/workflows` and Forgejo's full-URL action names; `third-party.yml`'s `github.base_ref` goes through `env:`. - checks: `tests/test_game_writes_no_links.py` looks through every source under `host/` and `gpu/vk/` (not `host/posix` or the vendored code) for opens that write. Each must go through `hos_fopen_write`, make its file new (`"x"`, `O_EXCL`, `O_NOFOLLOW`, `CREATE_NEW`), or be in a reviewed list. That list holds the switch-gated dumps and traces, each with its switch and reason (a switch the shared file cannot set, checked), and four writes that name no file of the game folder: the save file system, Discord's pipe, the console and udmabuf. A new cache file written with a plain `fopen` fails it. Fitting the branch to Longshot (the 4 commits after the port, and conflict notes in the ported ones): - `pipeline_states.py`: Longshot's `read_sent` and its xz limit kept. `check_records` runs beside `check_kinds`, and it skips compute-program records (a key and zeros), which `check_kinds` already holds to exactly that. - `nup.py`: Longshot's `package_parts`, `is_link` and `Destination` kept; `is_link` takes `links_only`. - `test_workflow_hardening.py`: `release.yml` and `website.yml` have no top-level `permissions:` (Forgejo 17 ignores the key), and `pr-description.yml` and `website.yml` check out under `pull_request_target` (main's script only, and a fork's commit only after a maintainer's `preview` label). The check lists these as reviewed, with the reasons. - Fixtures that cut hardened code get its helpers (`test_linux_saves_move`, `test_mouse_sens_file`, `test_prepare_stall`, `test_mesh_mode_switch`, `test_predict_resume`, `test_env_file`, `test_pipeline_state_computes`, and after the rebase `test_udmabuf_warning` and `test_upscaler_round_trip`); `test_pipeline_share`'s records are ones the game can make. ## Flags and settings | Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on | |---|---|---|---|---|---|---| | `longshot_env.txt` | Takes fewer switches: no path-valued switch, `RX_VK_DLSS_DIR` included. Also not `RX_SCREENSHOT`, `RX_FLASH_LOG`, `HOS_SCAN_DT` (dumps), `RX_VK_FRAME_TRACE`, `HOS_PGO`, SDL's file-writing switches, `HOS_CONTROL` or `HOS_CONTROL_MEM`. Of the `LONGSHOT_` names it takes only the game's own, `LONGSHOT_NO_AV1_NOTICE` and `LONGSHOT_OCCLUSION_OFF`. No switch is new; set in the environment, or in a file `HOS_ENV_FILE` names, each works as before. | On | On: a file shared by others should not set where the game writes, what it loads or what it runs | All | - | - | ## Measurements main: f87557ae (2026-11-13) this PR: security/hardening-passes @ b7a933da The rows below are from 46ec4bd7 on main d906c8f4. b7a933da (rebased onto f87557ae, plus the second review's fixes) has not been run or built yet: nothing was run on the PC that made it. The 36 earlier commits rebased with the same changed lines. No speed numbers: checks on reads, writes and shared files, off the frame path. The game-free checks on both sides: | | main @ d906c8f4 | this PR @ 46ec4bd7 | |---|---|---| | `python -I -B tests/run_all.py --jobs 4` failures (Linux, this PC) | `test_lift_cache` (no numpy here), `test_menu_i18n` (since #138) | the same 2: 508 passed, 2 failed, 30 skipped | | `longshot_vk.dll` (`RX_TARGET=windows LONGSHOT_MINGW=<llvm-mingw> RX_VK_ONLY=1 nx2/build.py --standalone`, on Linux) | - | builds and links, 0 failed, no warnings | | `longshot_vk.so` objects (`RX_VK_ONLY=1 nx2/build.py --standalone`, Linux) | - | all compile, 0 failed; the link needs `lld`, which this PC lacks | ## Testing **Tested:** on Linux (RTX 4070 Ti, driver 595.99.02, Ryzen; Python 3.14), without the game: - `python -I -B tests/run_all.py --jobs 4`: 508 passed, 2 failed, 30 skipped. Both failures also fail on `main` here: `test_lift_cache` (no numpy on this PC) and `test_menu_i18n` (pt_BR's text for a row #138 changed). - `tests/test_game_writes_no_links.py` fails on 83e9a080 (the head the review read) with every write the review named, plus the log, the bug report, the program cache's lock and the probe and draw-stats files, and passes here. - `tests/test_env_file.py` refuses `RX_VK_DLSS_DIR` from the shared file for every value (`/tmp/x`, `/dev/shm/x`, `C:\Users\Public\...`, `Z:\...`, `/proc/self/cwd` and the rest), and takes it from `HOS_ENV_FILE`'s file and the environment. - `hos_fopen_write.h` built against llvm-mingw's real Windows headers, as C and as C++ (`LONGSHOT_MINGW=... tests/test_game_writes_no_links.py`). The Windows renderer `longshot_vk.dll` builds and links. The Windows branch's behaviour (surrogates refused before any write open, OneDrive tags written, a name swapped between the two opens refused, `"r+b"` not creating) runs against a stand-in `windows.h` (on 46ec4bd7; b7a933da adds plain files opened in one step, no file index for them, a link swapped in before the open refused, a 0 index refused for OneDrive's, and the bug report's read, not yet run). - `pip install --require-hashes` of `nx2/prompt_requirements.txt` for `win_amd64` from the lock, against PyPI: installs. **Not run on b7a933da:** `tests/run_all.py`, the llvm-mingw builds and every check above, for the head that answers the second review. They were read, not run; they still need to run. **Not tested:** the game. The host C (`host/*.c`) built whole needs the lifted game's headers (`a64.h`), which only a build from a dump makes; the checks that cut and compile host functions pass. Windows: nothing ran on Windows itself (no Windows PC or Wine here), so `CreateFileW`/`_open_osfhandle` on a real NTFS junction, symlink or OneDrive placeholder is untested. The two Windows requests wait for the `testing wanted` label before anyone can claim them. **Needs other testers:** - Windows: a full build and play, with the game folder under OneDrive, expecting the build to keep the damaged-texture list, and the game to save, keep its options and language, write its log and reuse its program cache (T-93FV) - Windows: a junction or link at `longshot_options.ini` and at `longshot_programs.bin`, expecting the files they point at to be left alone and one log line each (T-AJTA) - Linux: a full build and play from a dump, expecting the same as main: saves, options, the log, pipeline and program caches and button prompts load (request to open) ## Screenshots Nothing on screen ## Risk and rollback A check that is too strict would refuse a file the game needs: options or a save not written, a pipeline or program cache not read or kept, the log not opened, or a `longshot_env.txt` switch that someone relied on ignored (`RX_VK_DLSS_DIR` there now is: it goes in the environment, such as Steam's launch options). Each refusal says so in `longshot_log.txt`, but a refused log falls back to where stderr went before. PRs are squash-merged, so reverting this PR reverts all of it; the parts are in separate commits here for review, not for separate reverts on `main`. ## Reviewer notes Ported with `work/port_to_longshot.py` (renames `hookshot` to `longshot`, `.github/` to `.forgejo/`), as #209 was. Commit messages are kept as written, so some still say `hookshot_env.txt`. Where to look first: `host/hos_fopen_write.h`, `host/hos_fopen_read.h`, `env_line_allowed` in `host/hos_main.c`, `REVIEWED` in `tests/test_game_writes_no_links.py`, and `nx2/pipeline_states.py`. This is several topics in one PR (link-safe writes and the Builder, the env file, HLE guest ranges, pipeline-list validation, the supply chain). It can be split along those lines; see the answer to the review. `REVIEWED` keeps the save file system's `OpenFile` (`host/nn_fs.c`, the game's saves in `user/`, opened again after the game made them) as a write that follows a link: it is not a name the game folder ships, and making it no-follow needs the Linux stand-in `CreateFileA` (`host/posix`) to take `FILE_FLAG_OPEN_REPARSE_POINT`. A follow-up if wanted. Not covered, on purpose: links at folders along the path are followed (a linked `user/`, a linked game folder and OneDrive need it), and hard links aren't caught (an archive can only hard-link within itself; refusing them would break deduplicated or hard-link-copied folders). Still on a plain `fopen` read, reviewed: the program cache's re-opens in `vk_progcache.cpp` (of the file the game has just opened through `hos_fopen_write`), `own_handle` in `vk_programs.cpp` (pipeline caches already read as regular files), the debugging reads behind switches a shared `longshot_env.txt` cannot set (`RX_VK_SKIP_FILE`, `HOS_INPUT_LIVE`, `HOS_WATCH`, `HOS_SCAN_DT`, `HOS_CHAIN`, the control bridge), and system files (`/etc/os-release`, `/proc`, `/sys`, `user-dirs.dirs`). Not ported (11): - `875b8bdb`, `7a56ee64`, `70b495de`: the game's update check and its skip file, removed in #13. - `1a7dd521`: the Python builder, removed in #181. - `c03691a7`, `8f029bff`, `c5fdeb2b`: the WPF builder's release-name and download rules (files Longshot removed). - `b7fa0d7b`: `release_rc.sh` (removed). - `6bb6763e`: Dependabot (GitHub only). - `54dba137`: Longshot already copies a link in `user/` as a link and deletes only the link (`copy_link`, `delete_tree`). - `8d23c0a0` "setup lists' parts come only from this project's releases": **needs a port of its own**. Longshot's setup fetches the launcher and `tools.lock`'s parts from Forgejo and its mirrors, so the GitHub host allowlist doesn't carry over; a follow-up. ## Issues See #209 ## Checklist ### For everyone - [ ] One topic. (Not yet: several, see Reviewer notes.) - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [ ] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). (Not yet on b7a933da: last run on 46ec4bd7; see Measurements.) - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (Not applicable: no speed change; the checks on both sides are above.) - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Flags noted, and the switch reference updated (docs/development.md, the README). - [ ] A game-free test added for the fix, failing on `main` and passing here. (Added and updated for b7a933da, `test_game_writes_no_links.py` and `test_game_folder_links.py`, but not run yet.) - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. (The renderer builds for Windows with llvm-mingw and compiles for Linux; the host C needs a dump's headers and is still to build: Testing.) - [x] Docs updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
A hookshot_env.txt is passed around between players, so it may not set
switches that name a file or folder. The list had drifted: RX_DUMP_IR,
RX_SCREENSHOT, RX_FLASH_LOG and HOS_SCAN_DT take a folder or file
prefix and were still accepted with a bare name, and RX_VK_DUMP_NATIVE
wrote files into the current folder. Dump switches are now refused by
family (any name with _DUMP), the four are refused by name, and
tests/test_env_file.py scans host/ and gpu/ for switches whose value is
used as a path and fails when the shared file could set one.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
scripts/ffx_sdk.py and scripts/ngx_sdk.py downloaded and copied through
a fixed <name>.part, opened for writing as it was: a link left in that
place (in the game folder's ffx/ or ngx/ too) had the write land on what
it points to. Each download and copy now goes to a new temp file beside
the target (mkstemp), is hashed while it is read, and stops once it is
larger than 1 GB rather than being written whole before the SHA-256 is
checked. The pinned commits and SHA-256s are unchanged.

Co-Authored-By: Claude Opus 5.5 <[email protected]>


Ported to Longshot: Longshot places each of the SDK's runtimes in a loop; each one is copied through a new temp file.
The four license texts the AppImage ships were bare curl calls: not
limited to https, and not hash-checked like every other download in
the script. They now go through fetch with a pinned SHA-256 variable (for the
LLVM, FFmpeg and dav1d versions the script pins). A test keeps any curl
outside the CURL array out of the script and runs fetch over those
lines with a stand-in curl.

Co-Authored-By: Claude Opus 5.5 <[email protected]>


Ported to Longshot: the AppImage's license block this hardened went with Longshot's #181, so only the check that
deps.sh has no curl outside its https-only CURL array and checked fetch is kept.
A game-free check over .github/workflows: actions pinned by full commit,
top-level token permissions, checkouts without a persisted token (but
the release record job's), no event data or inputs expanded inside a
run: script, and no checkout in a pull_request_target workflow. Every
workflow passes it as it is; the check's own cases show it catches each
kind.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
RX_VK_DLSS_DIR is the one path a shared hookshot_env.txt may still set.
A relative value (ngx, ., ../x) was taken and NGX resolved it against
whatever folder the game was started from, and a .. step could lead
out of the folder the value names. The file now takes only a folder
named in full on this PC (C:\x on Windows, /x on Linux) with no ..
step; shares and device paths stay refused. The process environment and
HOS_ENV_FILE still take any value.

tests/test_env_file.py checks a table of values for both systems.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The AppImage's Python (scripts/linux/deps.sh) and the Windows builder's
Python part (scripts/release_parts.py package --python-packages) took
their pinned packages from PyPI by version only. Every other download a
release makes is checked against a SHA-256.

requirements-lock.txt, written by the new scripts/python_lock.py from
requirements.txt's pins, lists each pin's wheels for Linux x86_64 and
Windows x64 with their SHA-256, and both installers now use pip
--require-hashes with it, so a wheel added to PyPI later under a pinned
version is refused. The AppImage's dependency cache key and deps.sh's
install mark include the lock.

tests/test_python_lock.py checks the lock against the pins and that both
installers use it; tests/test_release_prompt_packages.py checks the
hashes reach pip.

Co-Authored-By: Claude Opus 5.5 <[email protected]>


Ported to Longshot: release_parts.py went with Longshot's #13 (the Windows builder's packages come through
scripts/tools.py and tools.lock), so the lock is used by deps.sh only, and the Windows packaging check is not carried over.
A game folder can come from elsewhere, and an update or a rebuild into it
trusted what it found there:
- .hookshot_romfs_heads (the damaged textures the extraction found) was
  joined to the romfs as it was, so a line with .., an absolute path or a
  linked folder chose a file outside the romfs to replace;
- the stand-in was copied through a fixed <name>.zs.part, which could be
  a link to a file elsewhere;
- the kept .hookshot_exefs was copied with its links followed, and the
  romfs links were made into an existing romfs folder that could hold a
  link.

Each heads line now goes through the extraction's own path rule (no ..,
no absolute path, no link on the way, a .zs name); one that does not
pass drops the list and every file is checked instead. A stand-in is
copied into a new file of its own (mkstemp) and a damaged file that is a
link is left alone. A link inside the kept exefs or the romfs being
linked into stops the build and asks for an empty folder.

New tests/test_game_folder_links.py.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The game rewrites hookshot_options.ini when an option changes and writes
its pipeline caches, state lists and their markers, the button-prompt
textures, mouse_sens.txt, language.txt and the moved-saves note by
itself. A game folder unpacked from an archive someone shared can hold
symbolic links (tar and unzip keep them on Linux), and a plain
fopen("w") through one emptied and rewrote the file it named.

host/hos_fopen_write.h opens these with O_NOFOLLOW, only as a regular
file (not a FIFO or a device), and empties it only once that is known.
Windows is unchanged: an archive's links are not made there without a
privilege. The program cache (gpu/vk/vk_shader.cpp) is left for later.

New tests/test_game_writes_no_links.py: the helper's behaviour, and every
write in those files goes through it but the switch-gated debugging ones.
The checks that cut those writers out of vk_draw.cpp now build them with
the helper.

Co-Authored-By: Claude Opus 5.5 <[email protected]>


Ported to Longshot: the pipeline caches are written by Longshot's PipelineCacheFile (a new mkstemp file, then a
rename), which never writes through a link at the name, so those two writers stay as they are. Longshot's own markers and
lists in vk_draw.cpp (the driver fault note, the precompile marker, the compute variants, the warm and predict markers, the
predicted lists) take hos_fopen_write too, and the two checks that cut them include it.
targets_ok let any power of two up to 64 through as a record's sample
count, and the precompile handed it to the driver as the pipeline's
rasterizationSamples. A shareable state list (or hookshot_pipelines.bin)
with 16 or 64 on a GPU that draws with up to 8 then asked the driver for
an invalid pipeline at every start. A record is now made only with a
count in the device's framebuffer sample counts.

tests/test_pipeline_state_counts.py gains the case; both state-list tests'
stand-in device reports 1 to 8 samples.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
pcache_file_read read mkw_vk_pipelines.bin (and the play caches beside
it) whole into memory, then looked at its header. A large or sparse file
under that name, such as another PC's, ran the game out of memory at
the start. The header is now read and checked first, the rest only for
this device's cache, and a file larger than 4 GB (NVIDIA's after a full
preparation is about 1.1 GB) is not read, with a line in the log.

New tests/test_pipeline_cache_file_read.py runs the reader with its
memory held to 1 GB.

Co-Authored-By: Claude Opus 5.5 <[email protected]>


Ported to Longshot: Longshot's PipelineCacheFile::read already checks the header before it maps the rest, so this adds
only the 4 GB cap (kPcacheMax) to the check pcache_file_read hands it, and tests/test_pcache_header_read.py gains a sparse
5 GB file in place of tests/test_pipeline_cache_file_read.py, which cut the reader before Longshot mapped it.
A shared hookshot_env.txt may set SDL_ switches. Two kinds of them make
the game write files where it was started, without end: the dummy and
offscreen video drivers' *_SAVE_FRAMES (a picture each frame) and the
disk audio driver (SDL_AUDIODRIVER / SDL_AUDIO_DRIVER=disk: the sound to
sdlaudio.raw). The file now leaves those out, as it does the game's own
dump switches; other SDL_ switches (SDL_VIDEODRIVER=x11 and the like)
are taken as before. RX_VK_FRAME_TRACE, which adds a line to
hookshot_frames.csv each frame, joins the switches it takes no value for.

tests/test_env_file.py gains the cases; the readme says so.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
hookshot_env.txt took every HOOKSHOT_ name without a path in its value.
Most of them are the builder's (HOOKSHOT_MEM_LIMIT_GB, HOOKSHOT_JOBS,
HOOKSHOT_LATEST_TAG, HOOKSHOT_KEEP_BUILD_FILES...), and the builder an
update starts from the game inherits the game's environment, so a shared
file could set how the update's build ran. HOOKSHOT_PREPARE_ONLY, set by
--prepare-shaders, kept the game from ever playing. The file now takes
only the game's own HOOKSHOT_NO_UPDATE_CHECK, HOOKSHOT_NO_AV1_NOTICE and
HOOKSHOT_OCCLUSION_OFF; the process environment and HOS_ENV_FILE still
set any.

tests/test_env_file.py looks through the builder's sources for HOOKSHOT_
names and checks the file takes none but the game's; the readme says so.

Co-Authored-By: Claude Opus 5.5 <[email protected]>


Ported to Longshot: LONGSHOT_NO_UPDATE_CHECK is not one of the game's (Longshot #13 removed the game's update check),
so the game's LONGSHOT_ names are LONGSHOT_NO_AV1_NOTICE and LONGSHOT_OCCLUSION_OFF.
An update builds into the game folder as it is, and the builder writes
language.txt, hookshot_version.txt, hookshot.png, the seeded pipeline
files, the shader list, prompts/ and bin/ there by name, and stages the
game's files as <name>.new. A folder from elsewhere can hold a link at
one of those names, and the builder wrote through it (a dangling .new
link too: copy2 created what it named). Before anything is written, a
link (a symbolic link, a junction or a WSL link) at one of the names it
writes now stops the build and asks to remove it or choose an empty
folder; links elsewhere in the folder (a cache moved to another drive)
are left alone. A staged .new that is a dangling link is removed first.

The check for links inside the kept exefs and the romfs (the commit
before) now counts only symbolic links and junctions on Windows, not the
reparse points OneDrive marks its files with: a game folder under a
OneDrive-synced Documents or Desktop updates as before.

tests/test_game_folder_links.py checks the refusal, and that every name
make_game.py joins to the game folder is on the list.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The button prompts' files in the game folder (prompts/<face>/textures.bin,
ring.bin, hint.bin, External.bfotf) are a few MB, made by the builder.
read_all allocated and read any file under those names whole before it
looked at its magic, so a large or sparse one (a folder from elsewhere)
ran the game out of memory at the start. The magic is now read first,
and a file over 256 MB is not read: the game shows its own icons, as it
does without the files.

New tests/test_prompt_file_read.py counts what read_all asks malloc for.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
hos_fopen_write wrote through any name on Windows, on the grounds that an
archive's links are not made there without a privilege. Archive tools do
make them in Developer Mode, which many players turn on. It now refuses
a name that is a symbolic link, a junction or WSL's link (by its reparse
tag), and writes as before through other reparse points, which OneDrive
puts on its files.

The back end's object keys (nx2/build.py) now hold the host headers it
includes, hos_fopen_write.h and hos_wide_size.h: a change to one of them
rebuilt no vk object, so vk_draw.cpp kept the header's old code (seen
here in the Windows build).

tests/test_game_writes_no_links.py builds the Windows branch with a
stand-in windows.h and checks each kind, and that every host header the
back end includes is in its object keys; tests/test_shader_stamp_cache.py's
stand-in tree has the two headers.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
GP() turns a guest address into a host pointer without a check, so a
size from the game walks a copy past the guest window into the host's
own memory. nn::fs and the libc copy hooks were moved onto
hos_guest_range before; these were not:
- nn::audio AppendWaveBuffer took a wave buffer's data and ADPCM context
  as given, and the mixer read them for as long as the voice played
  (the host's memory into the sound, or a fault in the mixer); a buffer
  outside guest memory is now refused. AcquireVoiceSlot's coefficients
  likewise;
- nn::os::GenerateRandomBytes wrote ARG(1) bytes from ARG(0);
- nn::hid GetNpadStates wrote any count of states (now at most 16, as
  the console keeps) and GetSixAxisSensorStates its states unchecked;
- the socket Poll stand-in cleared up to 4096 descriptors unchecked;
- OutputDebugString and nn::diag's log lines copied up to 4 KB into the
  log from an unchecked pointer;
- the event-name copies (hos_main.c) and the item key (oot_menu.c, whose
  own bound could wrap) wrote through pointers read from game objects.

New tests/test_hle_guest_ranges.py runs wb_in_guest and checks the
functions' text; the wave-buffer and input fixtures have hos_guest_range
over their own guest memory.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The game reads hookshot_env.txt, hookshot_options.ini, language.txt and
mouse_sens.txt from its folder at the start. A folder unpacked from an
archive can hold a FIFO under one of those names (tar keeps them), and
fopen("r") on it waited forever, before the window opened, at every
start. New host/hos_fopen_read.h has hos_fopen_read: on Linux it opens
without waiting and takes only a regular file (a link to one is fine);
on Windows it is fopen. The reads of those files go through it, and
hos_fopen_read_binary takes the renderer's reads of its caches, notes,
state lists, preparation records and markers (gpu/vk/vk_draw.cpp) and the
button-prompt files (host/prompts.c) the same way, as do the renderer's
start-up reads of hookshot_options.ini and its skip marker
(gpu/vk/vk_main.cpp) and of the game's font files (vk_game_font.h). On
Linux, the copy that moves an old user/ folder to the saves folder reads
through it too, so a FIFO there fails the copy (the entry stays) instead
of holding the start. The helpers live in
host/hos_fopen_read.h, which needs no windows.h, so the checks that cut a
reader out of the sources still build on Windows beside their own
stand-ins; hos_fopen_write.h includes it, and the renderer's object keys
take it in.

tests/test_game_writes_no_links.py checks the helper (a FIFO and a folder
refused at once, a file and a link to one read) and that each read goes
through it; the checks that cut those readers out include the header.

Co-Authored-By: Claude Opus 5.5 <[email protected]>


Ported to Longshot: Longshot's pipeline caches are read by PipelineCacheFile::read (a regular file only too), and
Longshot's own start-up reads in vk_draw.cpp (the driver faults, compute variants, state lists, warm and predict markers,
predicted lists, the precompile marker and the notes) go through hos_fopen_read too. Also gives
tests/test_pipeline_state_computes.py the device limits b59187f5's sample-count check reads.
mouse_setup (host/nn_hid.c) reads the sensitivity kept in
<user>/mouse_sens.txt, and HOS_MOUSE_SENS, and any positive number went
on: "inf" or "1e999" reached lround() when the value moved into the
menu's option, an undefined result kept in hookshot_options.ini. Saves
can come from elsewhere. A value is now taken only when finite, and kept
to the menu's 0.1-5 (10-500%).

New tests/test_mouse_sens_file.py cuts mouse_setup out of the source.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
A note in hookshot_pipelines.bin and a record of the shareable state
list carry the draw's vertex stream strides and divisors, and any number
reached vkCreateGraphicsPipelines. streams_ok now refuses a stride past
2048 (Vulkan's least maxVertexInputBindingStride) or a divisor past
65535; the game's own are under 100 and 0-1. note_sane and both
versions' record readers use it, as targets_ok and formats_ok.

tests/test_pipeline_state_counts.py: a record with a stride of 4096 or a
divisor of 65536 is left out, one at the limits kept.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The pipeline state lists contributors add (assets/pipeline_states) are
merged into the list every release ships, and the game builds each
record's pipeline at the first start. pipeline_states.py checked a list's
header and length only. Now add, tour and check (which CI runs) stop on a
record with a value no pipeline has: counts past their arrays (targets,
attributes, answers, questions), a sample count or topology Vulkan does
not have, a yes/no value other than 0 or 1, non-zero padding, or a
vertex stride or divisor past Vulkan's limits, saying which record and
why. An xz list is unpacked only up to 64 MB: a contributed file of a few
KB could unpack to gigabytes in CI. Every shipped list passes.

tests/test_pipeline_state_sources.py: such records are refused by add and
check, and an xz list of 128 MB is refused unread; the fixtures' records
have one sample.

Co-Authored-By: Claude Opus 5.5 <[email protected]>


(longshot) Longshot's compute-program records, a key and zeros, are checked by check_kinds and
left out of record_problem.

Assisted-by: Claude Opus 5.5
Assisted-by: Claude Opus 5.5
nvnDeviceGetProcAddress and nvnBootstrapLoader read the name the game
passes in place, to its end: a pointer near the guest window's end, or
a name with no end, read past the guest's memory into the host's. A new
guest_name copies at most 255 characters, only from inside the window,
and gives "" when there is no string there (which asks for no proc).

tests/test_hle_guest_ranges.py checks both callers and runs guest_name.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
hos_fopen_write refused a link or a non-file at the name silently, so a
setting that did not stick (hookshot_options.ini a link from an archive)
left no reason. The first refusal in each source file now writes
"files: <path> is a link or not a plain file: the game does not write
through it" on stderr, which is the game's log; on Linux only for a link
or a non-file (ELOOP, ENXIO, not S_ISREG), not for a missing folder.

tests/test_game_writes_no_links.py checks the line is said once, naming
the file.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
stand_in_damaged_files takes the extraction's list of damaged textures
(.hookshot_romfs_heads) only when each line passes nup.extraction_path,
which refused any reparse point on the way: in a folder OneDrive keeps,
whose files carry OneDrive's own reparse points, the list was dropped and
all ~100,000 files were opened again (minutes). extraction_path takes
links_only, which counts only the reparse points that are links (a
symbolic link, a junction, WSL's link); the read-back uses it, and the
unpack itself still refuses any.

tests/test_game_folder_links.py: with OneDrive's tag the list is taken;
with a symbolic link's it is dropped.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
refuse_written_links checks the names in FOLDER_WRITES; the Compile shaders
choice's longshot_options.ini, which the Builder writes in the game folder,
was not among them (tests/test_game_folder_links.py).

Assisted-by: Claude Opus 5.5
HOS_CONTROL's value is a socket path, and a bare name passed the file's
no-path rule (a socket in the current folder). HOS_CONTROL_MEM opens guest
memory reads and writes over it. Both are refused by name, like the other
switches that name a file; tests/test_env_file.py found HOS_CONTROL.
A player who links hookshot_options.ini, mouse_sens.txt or a pipeline
cache on purpose (a dotfile manager, a cache on another disk) now finds
the file no longer changes, with one line in the log. Where the files go
says so, and to link the folder that holds them instead.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Checks that came with main cut code this branch changed and compile or
run it with stand-ins of their own:

- test_audio_biquad (nn_audio.c) and test_teleport_minimap_flag
  (oot_menu.c) get a hos_guest_range for their guest memory.
- test_ocarina_notes and test_ocarina_note_bindings include
  hos_fopen_read.h, as test_bound_keys does.
- test_menu_keys finds menu_init's options read by hos_fopen_read and
  includes its header.
- test_pcache_header_read gets hos_fopen_read.h, kPcacheMax from
  vk_draw.cpp and a logf.
- test_linux_downgrade's main() runs with linux_update, which checks
  --tag now.

The builder writes .hookshot_builder_home in the game folder since
#2367 (a portable game's Builder). It is on FOLDER_WRITES now, so a
link at that name stops the build like the others;
test_game_folder_links found it.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The check looked in .github/workflows, which Longshot no longer has, so it found nothing. It now reads
.forgejo/workflows and takes an action named by its full URL (https://<host>/owner/repo@<40 hex>), as
Forgejo writes them. What it then found:

- third-party.yml pasted github.base_ref into a run: script: it goes through env: now.
- github.repository and github.server_url, which Forgejo sets, may be expanded in a script.
- release.yml and website.yml have no top-level permissions: (Forgejo 17 ignores the key; release.yml
  says so), and pr-description.yml and website.yml check out under pull_request_target: the first only
  main's script, the second a fork's commit only once a maintainer's preview label approves it. These
  are listed as reviewed, with their reasons.

Assisted-by: Claude Opus 5.5
test: fixtures that cut hardened code get its no-link reads and writes, env_on and pipelines the game can make
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 3s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
83e9a080af
After the hardening, code the checks cut calls hos_fopen_read_binary, hos_fopen_write and env_on:

- test_linux_saves_move includes hos_fopen_read.h and hos_fopen_write.h; test_mouse_sens_file
  longshot_env.h; test_prepare_stall's stand-ins add hos_fopen_read_binary.
- test_mesh_mode_switch and test_predict_resume find the ini and marker writes as hos_fopen_write.
- test_env_file finds the game's LONGSHOT_ names read with env_on too.
- test_pipeline_share's records are of pipelines the game can make (a key, one sample), which
  pipeline_states.py's check_records now asks of a list someone sends.
- host/hos_main.c: the env file's comment no longer names LONGSHOT_LATEST_TAG, which no builder looks
  for (test_no_release_lookup).

Assisted-by: Claude Opus 5.5
HumanBean requested changes 2026-11-13 16:21:07 +00:00
Dismissed
HumanBean left a comment

Read through the PR-only diff (dd02c6cc..83e9a080) and the code around it: the callers of the two new headers, env_line_allowed with the code that reads each switch, the Builder's link checks, nup.py, and the HLE guest-range changes. I built and ran nothing.

What's good

  • The Linux hos_fopen_write has no race at the file's name: O_NOFOLLOW | O_NONBLOCK, then fstat on the descriptor, then ftruncate only once it knows the file is regular. The read side refuses FIFOs the same way. A symlinked user/ (the saves move) and a symlinked game folder still work, because only the last part of the path is checked.
  • env_line_allowed is a real improvement. Every *_DUMP switch is blocked as a family, and so are HOS_CONTROL/HOS_CONTROL_MEM. LONGSHOT_ names are narrowed to the game's two, and RX_VK_FSR_DIR and the other _DIR/_FILE names are refused. The source scan in test_env_file.py will catch most new path switches.
  • Builder: links_only keeps a game folder that OneDrive manages working (its placeholders aren't links), while refuse_dump_links stays strict. stand_in_damaged now copies into an mkstemp file and no longer writes to a fixed .part name.
  • The HLE hos_guest_range checks are careful: nvnBootstrapLoader caps a name at the end of guest memory, the wave buffers are checked once and that checked copy is the one submitted, and npad_states limits its count.

Must change

  1. Several files the game writes on its own still use a plain fopen. The description says the game's own files, "the pipeline caches, the markers", are no longer written through a link, and the header comment names language.txt. At 83e9a080 these still write through a link at their name:

    • gpu/vk/vk_progcache.cpp:120,124,251,253,255,265: longshot_programs.bin and .idx, with "wb", "ab" and "r+b" plus _chsize_s. This is the largest file the game writes, and it is written on every first start.
    • gpu/vk/vk_menu_i18n.cpp:226: language.txt when the language row changes. Only the first write, in nn_applet.c, was converted.
    • gpu/vk/load_default.h:69,79,92: the load-fallback marker and its strike file.
    • gpu/vk/module_ids.h:83: longshot_module_ids.bin.
    • gpu/vk/vk_main.cpp:1090: the "preparation skipped" marker. The PR converted its read but not this write.
    • gpu/vk/pcache_stamp.h:38: mkw_vk_pipelines.stamp.tmp. A link there is written through before the rename.

    tests/test_game_writes_no_links.py misses all of them because ALLOWED lists only six files. Convert these writes, and have the check scan every file under host/ and gpu/vk/ with a list of reviewed switch-gated writes, not the other way round. Otherwise the next new cache file is back to the old behaviour.

  2. RX_VK_DLSS_DIR is still a library-load path that a shared file can set. It is the one longshot_env.txt switch left that names a path, and it decides where NGX loads nvngx_dlss.dll or libnvidia-ngx-dlss.so from. NGX takes the newest runtime it finds, so a planted copy wins over the game folder's.

    • The full-path rule still accepts /tmp/x, /dev/shm/x, C:\Users\Public\..., a mapped network drive (Z:\) and /proc/self/cwd. So a shared file, plus anything that can drop a file into a world-writable folder, loads a library.
    • Windows NGX checks NVIDIA's signature on the DLL, as far as I know. I couldn't confirm that the Linux side does.
    • The default search already covers <game>/ngx and $XDG_DATA_HOME/longshot/ngx (%LOCALAPPDATA%\Longshot\ngx). The simplest fix is to refuse this switch from the shared file too, like RX_VK_FSR_DIR; HOS_ENV_FILE still sets it. If it stays, at least refuse a folder the player doesn't own or that others can write to (on Linux, stat it: uid, and no S_IWGRP|S_IWOTH), and refuse non-fixed drives on Windows (GetDriveTypeW).
  3. The "Rebased on current main" box is ticked but isn't true.

    • The branch sits on dd02c6cc. main is 1ef93e79, four commits ahead (#54, #78, #147, #197). Three of them touch files this PR changes: host/hos_main.c, gpu/vk/vk_draw.cpp and gpu/vk/vk_main.cpp.
    • A trial merge has no conflicts, so it should be a clean rebase. Rebase, then rerun tests/run_all.py: #147's test_dev_packs.py and #197's test_vk_occlusion_queries.py cut code from files this PR changes.
    • Also untick or reword "One topic" (see below).

Should change

  • Windows hos_fopen_write (inline comment):
    • Match on IsReparseTagNameSurrogate(tag), not three fixed tags. That also covers WCI_LINK and GLOBAL_REPARSE, and still lets OneDrive's cloud tags through.
    • Better still, open with CreateFileW(..., FILE_FLAG_OPEN_REPARSE_POINT), check GetFileInformationByHandleEx(FileAttributeTagInfo) on the handle, and only then _open_osfhandle/_fdopen. That closes the window between GetFileAttributesA and fopen, the same way the Linux side has none. Low urgency: the threat here is files sitting in an archive, not a race.
  • Split the PR. It is several topics: link-safe writes and the Builder, the env file, HLE guest ranges, pipeline-list validation, and the supply chain (python_lock, the SDK downloads, deps.sh, the workflow check). PRs are squash-merged (CONTRIBUTING.md), so the description's "a single commit can be reverted on its own" won't hold on main. A regression in one part, such as a player's options not saving, would mean reverting all 2,086 lines.
  • The Builder now refuses any link at romfs, bin, prompts, ngx, ffx or .game (FOLDER_WRITES). That's right for safety. But the README change tells players to "link the folder that holds them, not the file", and a Steam Deck player who points romfs at the SD card now gets a hard stop. Say in the README which folders the Builder won't take as links. The error could also name the folder to move instead.
  • Only the Linux AppImage's Python is hash-pinned. --require-hashes covers deps.sh only. The Windows tools Python (scripts/tools.py assemble_python, nx2/prompt_requirements.txt) still installs unpinned, so the "the shipped Python's packages" line should say Linux. Or use the lock there too: its header says it lists the Windows x64 wheels too.

Not checked: I didn't build or run anything, Windows or Linux. I couldn't confirm whether NGX checks the runtime's signature on Linux. I didn't follow every guest-range change into the game's real call sites (for example, whether OoT ever asks GetNpadStates for more than 16 states).

Read through the PR-only diff (dd02c6cc..83e9a080) and the code around it: the callers of the two new headers, `env_line_allowed` with the code that reads each switch, the Builder's link checks, `nup.py`, and the HLE guest-range changes. I built and ran nothing. **What's good** - The Linux `hos_fopen_write` has no race at the file's name: `O_NOFOLLOW | O_NONBLOCK`, then `fstat` on the descriptor, then `ftruncate` only once it knows the file is regular. The read side refuses FIFOs the same way. A symlinked `user/` (the saves move) and a symlinked game folder still work, because only the last part of the path is checked. - `env_line_allowed` is a real improvement. Every `*_DUMP` switch is blocked as a family, and so are `HOS_CONTROL`/`HOS_CONTROL_MEM`. `LONGSHOT_` names are narrowed to the game's two, and `RX_VK_FSR_DIR` and the other `_DIR`/`_FILE` names are refused. The source scan in `test_env_file.py` will catch most new path switches. - Builder: `links_only` keeps a game folder that OneDrive manages working (its placeholders aren't links), while `refuse_dump_links` stays strict. `stand_in_damaged` now copies into an `mkstemp` file and no longer writes to a fixed `.part` name. - The HLE `hos_guest_range` checks are careful: `nvnBootstrapLoader` caps a name at the end of guest memory, the wave buffers are checked once and that checked copy is the one submitted, and `npad_states` limits its count. **Must change** 1. **Several files the game writes on its own still use a plain `fopen`.** The description says the game's own files, "the pipeline caches, the markers", are no longer written through a link, and the header comment names `language.txt`. At 83e9a080 these still write through a link at their name: - `gpu/vk/vk_progcache.cpp:120,124,251,253,255,265`: `longshot_programs.bin` and `.idx`, with `"wb"`, `"ab"` and `"r+b"` plus `_chsize_s`. This is the largest file the game writes, and it is written on every first start. - `gpu/vk/vk_menu_i18n.cpp:226`: `language.txt` when the language row changes. Only the first write, in `nn_applet.c`, was converted. - `gpu/vk/load_default.h:69,79,92`: the load-fallback marker and its strike file. - `gpu/vk/module_ids.h:83`: `longshot_module_ids.bin`. - `gpu/vk/vk_main.cpp:1090`: the "preparation skipped" marker. The PR converted its read but not this write. - `gpu/vk/pcache_stamp.h:38`: `mkw_vk_pipelines.stamp.tmp`. A link there is written through before the rename. `tests/test_game_writes_no_links.py` misses all of them because `ALLOWED` lists only six files. Convert these writes, and have the check scan every file under `host/` and `gpu/vk/` with a list of reviewed switch-gated writes, not the other way round. Otherwise the next new cache file is back to the old behaviour. 2. **`RX_VK_DLSS_DIR` is still a library-load path that a shared file can set.** It is the one `longshot_env.txt` switch left that names a path, and it decides where NGX loads `nvngx_dlss.dll` or `libnvidia-ngx-dlss.so` from. NGX takes the newest runtime it finds, so a planted copy wins over the game folder's. - The full-path rule still accepts `/tmp/x`, `/dev/shm/x`, `C:\Users\Public\...`, a mapped network drive (`Z:\`) and `/proc/self/cwd`. So a shared file, plus anything that can drop a file into a world-writable folder, loads a library. - Windows NGX checks NVIDIA's signature on the DLL, as far as I know. I couldn't confirm that the Linux side does. - The default search already covers `<game>/ngx` and `$XDG_DATA_HOME/longshot/ngx` (`%LOCALAPPDATA%\Longshot\ngx`). The simplest fix is to refuse this switch from the shared file too, like `RX_VK_FSR_DIR`; `HOS_ENV_FILE` still sets it. If it stays, at least refuse a folder the player doesn't own or that others can write to (on Linux, `stat` it: uid, and no `S_IWGRP|S_IWOTH`), and refuse non-fixed drives on Windows (`GetDriveTypeW`). 3. **The "Rebased on current `main`" box is ticked but isn't true.** - The branch sits on dd02c6cc. `main` is 1ef93e79, four commits ahead ([#54](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/54), [#78](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/78), [#147](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/147), [#197](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/197)). Three of them touch files this PR changes: `host/hos_main.c`, `gpu/vk/vk_draw.cpp` and `gpu/vk/vk_main.cpp`. - A trial merge has no conflicts, so it should be a clean rebase. Rebase, then rerun `tests/run_all.py`: [#147](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/147)'s `test_dev_packs.py` and [#197](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/197)'s `test_vk_occlusion_queries.py` cut code from files this PR changes. - Also untick or reword "One topic" (see below). **Should change** - **Windows `hos_fopen_write`** (inline comment): - Match on `IsReparseTagNameSurrogate(tag)`, not three fixed tags. That also covers `WCI_LINK` and `GLOBAL_REPARSE`, and still lets OneDrive's cloud tags through. - Better still, open with `CreateFileW(..., FILE_FLAG_OPEN_REPARSE_POINT)`, check `GetFileInformationByHandleEx(FileAttributeTagInfo)` on the handle, and only then `_open_osfhandle`/`_fdopen`. That closes the window between `GetFileAttributesA` and `fopen`, the same way the Linux side has none. Low urgency: the threat here is files sitting in an archive, not a race. - **Split the PR.** It is several topics: link-safe writes and the Builder, the env file, HLE guest ranges, pipeline-list validation, and the supply chain (`python_lock`, the SDK downloads, `deps.sh`, the workflow check). PRs are squash-merged (CONTRIBUTING.md), so the description's "a single commit can be reverted on its own" won't hold on `main`. A regression in one part, such as a player's options not saving, would mean reverting all 2,086 lines. - **The Builder now refuses any link at `romfs`, `bin`, `prompts`, `ngx`, `ffx` or `.game`** (`FOLDER_WRITES`). That's right for safety. But the README change tells players to "link the folder that holds them, not the file", and a Steam Deck player who points `romfs` at the SD card now gets a hard stop. Say in the README which folders the Builder won't take as links. The error could also name the folder to move instead. - **Only the Linux AppImage's Python is hash-pinned.** `--require-hashes` covers `deps.sh` only. The Windows tools Python (`scripts/tools.py assemble_python`, `nx2/prompt_requirements.txt`) still installs unpinned, so the "the shipped Python's packages" line should say Linux. Or use the lock there too: its header says it lists the Windows x64 wheels too. **Not checked:** I didn't build or run anything, Windows or Linux. I couldn't confirm whether NGX checks the runtime's signature on Linux. I didn't follow every guest-range change into the game's real call sites (for example, whether OoT ever asks `GetNpadStates` for more than 16 states).
@ -0,0 +1,65 @@
/* (longshot) fopen for the files the game writes by itself in its folder and the user folder (longshot_options.ini, the
Owner

The comment names language.txt, the pipeline caches and the markers, but some of the game's own writes still use a plain fopen. vk_menu_i18n.cpp:226 writes language.txt (the menu's write). vk_progcache.cpp writes longshot_programs.bin/.idx with wb, ab and r+b. Also still plain: load_default.h (the markers), module_ids.h:83, vk_main.cpp:1090 (the skipped marker) and pcache_stamp.h:38 (.tmp). Each of these writes through a link at its name.

The comment names `language.txt`, the pipeline caches and the markers, but some of the game's own writes still use a plain `fopen`. `vk_menu_i18n.cpp:226` writes `language.txt` (the menu's write). `vk_progcache.cpp` writes `longshot_programs.bin`/`.idx` with `wb`, `ab` and `r+b`. Also still plain: `load_default.h` (the markers), `module_ids.h:83`, `vk_main.cpp:1090` (the skipped marker) and `pcache_stamp.h:38` (`.tmp`). Each of these writes through a link at its name.
@ -0,0 +34,4 @@
const HANDLE h = FindFirstFileA(path, &fd);
if (h == INVALID_HANDLE_VALUE) return NULL;
FindClose(h);
if (fd.dwReserved0 == 0xA000000CUL || fd.dwReserved0 == 0xA0000003UL || fd.dwReserved0 == 0xA000001DUL) {
Owner

Two suggestions. First, IsReparseTagNameSurrogate(fd.dwReserved0) covers every link-like tag (WCI_LINK, GLOBAL_REPARSE, any added later) and still lets OneDrive's cloud tags through, which aren't surrogates. Second, the check and the fopen are two lookups of the name. CreateFileW(..., FILE_FLAG_OPEN_REPARSE_POINT) followed by GetFileInformationByHandleEx(FileAttributeTagInfo) and _open_osfhandle would check the very file it then writes, the way the Linux side already does.

Two suggestions. First, `IsReparseTagNameSurrogate(fd.dwReserved0)` covers every link-like tag (`WCI_LINK`, `GLOBAL_REPARSE`, any added later) and still lets OneDrive's cloud tags through, which aren't surrogates. Second, the check and the `fopen` are two lookups of the name. `CreateFileW(..., FILE_FLAG_OPEN_REPARSE_POINT)` followed by `GetFileInformationByHandleEx(FileAttributeTagInfo)` and `_open_osfhandle` would check the very file it then writes, the way the Linux side already does.
host/hos_main.c Outdated
@ -458,0 +472,4 @@
/* RX_VK_DLSS_DIR: a folder on this PC named in full (C:\x on Windows, /x on Linux), not a share (\\server\x or
//server/x) or a device path; (longshot) not a relative one either, which NGX resolved against whatever folder
the game was started from, and no .. step out of the folder the value names */
if (!strcmp(name, "RX_VK_DLSS_DIR")) {
Owner

This is the last switch a shared file can point at a path, and it picks which library NGX loads (it takes the newest runtime it finds). A full path still accepts /tmp/x, /dev/shm/x, C:\Users\Public\... and a mapped network drive (Z:\). The default search already covers <game>/ngx and $XDG_DATA_HOME/longshot/ngx. I'd refuse it here like RX_VK_FSR_DIR and leave it to HOS_ENV_FILE. If it stays, refuse a folder that is group- or world-writable or that the player doesn't own (Linux), and refuse anything but a fixed drive (Windows).

This is the last switch a shared file can point at a path, and it picks which library NGX loads (it takes the newest runtime it finds). A full path still accepts `/tmp/x`, `/dev/shm/x`, `C:\Users\Public\...` and a mapped network drive (`Z:\`). The default search already covers `<game>/ngx` and `$XDG_DATA_HOME/longshot/ngx`. I'd refuse it here like `RX_VK_FSR_DIR` and leave it to `HOS_ENV_FILE`. If it stays, refuse a folder that is group- or world-writable or that the player doesn't own (Linux), and refuse anything but a fixed drive (Windows).
nx2/make_game.py Outdated
@ -111,0 +111,4 @@
# (longshot) what this builder writes at the top of a game folder (and in its bin): a link at one of these names, which a folder
# from elsewhere can hold (an update builds into the folder as it is), was written through, and the file or folder it named
# rewritten. tests/test_game_folder_links.py checks the list against the names this file joins to the folder
FOLDER_WRITES = ["bin", "image.bin", "longshot_version.txt", "longshot_options.ini", "language.txt", "longshot.png", "longshot_pipelines.bin",
Owner

romfs, bin, prompts, ngx and ffx are folders, so the Builder now stops when any of them is a link. A Steam Deck player who points romfs at the SD card gets a hard stop, while the README's new note says to "link the folder that holds them". Please say in the README which folders mustn't be links, and have the error name the folder to move.

`romfs`, `bin`, `prompts`, `ngx` and `ffx` are folders, so the Builder now stops when any of them is a link. A Steam Deck player who points `romfs` at the SD card gets a hard stop, while the README's new note says to "link the folder that holds them". Please say in the README which folders mustn't be links, and have the error name the folder to move.
@ -0,0 +35,4 @@
# (the sources of the writes listed above, and how many plain fopen writes each keeps: switch-gated debugging only.
# vk_draw.cpp: RX_VK_PROBE's two files and RX_VK_DRAW_STATS' file)
ALLOWED = {"gpu/vk/vk_menu.cpp": 0, "gpu/vk/vk_draw.cpp": 3, "host/nn_applet.c": 0, "host/prompts.c": 0,
Owner

ALLOWED lists six files, so this check doesn't see the plain writes in vk_progcache.cpp, vk_menu_i18n.cpp, load_default.h, module_ids.h, pcache_stamp.h, or the write in vk_main.cpp. Scan every file under host/ and gpu/vk/ instead, and list the reviewed switch-gated writes, so a new cache file fails here.

`ALLOWED` lists six files, so this check doesn't see the plain writes in `vk_progcache.cpp`, `vk_menu_i18n.cpp`, `load_default.h`, `module_ids.h`, `pcache_stamp.h`, or the write in `vk_main.cpp`. Scan every file under `host/` and `gpu/vk/` instead, and list the reviewed switch-gated writes, so a new cache file fails here.
TrashPanda force-pushed security/hardening-passes from 83e9a080af
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 3s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to f7b79b8986
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
2026-11-13 18:31:20 +00:00
Compare
TrashPanda force-pushed security/hardening-passes from f7b79b8986
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
to 641bf324a8
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
2026-11-13 18:31:39 +00:00
Compare
Author
Contributor

Thanks for the careful read. The branch is rebased on main @ d906c8f4 (it had moved past 1ef93e79), with 4 new commits on top; the head is 46ec4bd7. (Edited: main moved during the work, so I rebased once more; the hashes here are the current ones.) Point by point:

Must change

  1. Several files the game writes on its own still use a plain fopen … Convert these writes, and have the check scan every file under host/ and gpu/vk/ with a list of reviewed switch-gated writes

Done in da41af87.

  • Converted every write you listed: vk_progcache.cpp (.bin/.idx with "wb", "ab", and "r+b" + _chsize_s, which hos_fopen_write now takes as an existing-file-only mode), vk_menu_i18n.cpp's language.txt, load_default.h's marker and strike file, module_ids.h, vk_main.cpp's skipped marker and pcache_stamp.h's .tmp.
  • The new scan found more, also converted: longshot_log.txt (the freopen onto stderr, now hos_freopen_write), the bug report's copied logs and report.txt, the program cache's .lock (O_NOFOLLOW), and RX_VK_PROBE's and RX_VK_DRAW_STATS' files, whose switches a shared longshot_env.txt can set.
  • The start-up reads of those files (the program cache and index, the marker, the stamp, language.txt) go through hos_fopen_read* too, so a FIFO there isn't waited on.
  • tests/test_game_writes_no_links.py is inverted. It walks every source under host/ and gpu/vk/ (not host/posix, eden, sirit, compat or gpu/vk/tests), strips comments, and finds every open that writes: fopen/freopen with a mode that writes, open with O_WRONLY/O_RDWR/O_CREAT/O_APPEND, CreateFile* with write access, ofstream, CopyFile/copy_file.
    • Each must go through the helper, make its file new ("x", O_EXCL, O_NOFOLLOW, CREATE_NEW), or be in REVIEWED. Each REVIEWED entry names its file, its call, its switch and a reason.
    • Every REVIEWED switch is compiled through env_line_allowed and must be refused from the shared file. That's how HOS_PGO turned up; it's refused now, in a1463821.
    • A stale entry fails, and the scanner has its own positive and negative cases, so it can't silently pass on nothing.
    • Four non-switch entries are listed with reasons: nn_fs.c's save OpenFile, Discord's pipe, the console handles, and /dev/udmabuf. The save file system is the one I'd flag. Making it no-follow means teaching the Linux CreateFileA stand-in FILE_FLAG_OPEN_REPARSE_POINT; I left it as a reviewed exception, open to a follow-up.
  • Run against 83e9a080, the new check fails on all of the above.
  1. RX_VK_DLSS_DIR is still a library-load path that a shared file can set … refuse it here like RX_VK_FSR_DIR and leave it to HOS_ENV_FILE

Done in a1463821, the simple way you suggested. Its special case in env_line_allowed is gone, so it falls under the _DIR rule. The environment (Steam launch options, a shell) and a file HOS_ENV_FILE names still set it, since env_line_allowed only applies to the shared file. test_env_file.py now checks all of that:

  • every value is refused from longshot_env.txt, including /tmp/x, /dev/shm/x, /proc/self/cwd, C:\Users\Public\ngx and Z:\ngx;
  • HOS_ENV_FILE's file sets it;
  • a value set in the environment survives the player's file.

The README's DLSS and switches sections say where to set it now. With the switch refused, the Linux NGX signature question no longer decides anything.

  1. The "Rebased on current main" box is ticked but isn't true … Also untick or reword "One topic"

Rebased on d906c8f4, which includes #54, #78, #147, #197 and the 17 merged since. The rebase was clean. Three checks that cut code this PR changes needed updates:

  • test_udmabuf_warning.py: the log's freopen became hos_freopen_write.
  • test_upscaler_round_trip.py: the options file is read with hos_fopen_read, and the fixture includes the helper.
  • test_prepare_stall.py: the watchdog's skipped marker is written with hos_fopen_write.

test_dev_packs.py and test_vk_occlusion_queries.py pass. tests/run_all.py --jobs 4: 508 passed, 2 failed, 30 skipped. Both failures also fail on main on this PC: test_lift_cache (no numpy) and test_menu_i18n (since #138). "One topic" is unticked, with a note.

Should change

Windows hos_fopen_write: match on IsReparseTagNameSurrogate(tag) … Better still, open with CreateFileW(..., FILE_FLAG_OPEN_REPARSE_POINT), check GetFileInformationByHandleEx(FileAttributeTagInfo) on the handle, and only then _open_osfhandle/_fdopen

Done in da41af87, both parts, with one change to the second:

  • The name is opened as itself with FILE_READ_ATTRIBUTES and FILE_FLAG_OPEN_REPARSE_POINT only, and its tag is checked on that handle with IsReparseTagNameSurrogate.
  • It's then opened for writing without the flag, and kept only if GetFileInformationByHandle gives the same volume and file index. After that come SetEndOfFile for "w", then _open_osfhandle (_O_APPEND/_O_TEXT as the mode says) and _fdopen.
  • The second open is there so that OneDrive fills a file kept only online before it's written: writing through a handle opened with FILE_FLAG_OPEN_REPARSE_POINT would skip hydration. The file-index match closes the window you pointed at all the same.
  • A name with nothing at it is made with CREATE_NEW, which fails if a link appears in between.
  • Paths go from UTF-8 to wide with MB_ERR_INVALID_CHARS, as pipeline_cache_file.h does.

The stand-in-windows.h fixture now checks that symlink, junction, LX, WCI_LINK_1 and GLOBAL_REPARSE are refused before any write-access open. It also checks that two cloud tags are written, a swap between the two opens is refused (and both handles closed), and "r+b" doesn't create. With LONGSHOT_MINGW set, the check also builds the header as C and C++ against llvm-mingw's real headers. The whole Windows longshot_vk.dll builds and links with llvm-mingw, without warnings. Nothing has run on Windows itself yet: T-AJTA asks for a real junction and symlink test.

Split the PR … A regression in one part … would mean reverting all 2,086 lines

Not done in this round. Answering the review on one branch kept the thread readable, and the must-changes cut across the parts: the env file's refusals now back the write check's reviewed list. The split I'd make:

  1. link-safe writes and reads, plus the Builder (this PR);
  2. the env file;
  3. the HLE guest ranges;
  4. pipeline-list validation;
  5. the supply chain (python_lock, SDK downloads, deps.sh, the workflow check).

Say if you want it before merge and I'll split along those lines; the commits already fall that way. Meanwhile the description no longer claims a single commit can be reverted on main.

The Builder now refuses any link at romfs, bin, prompts, ngx or ffx … Say in the README which folders the Builder won't take as links. The error could also name the folder to move instead.

Done in 0f1e28ed. FOLDERS_WRITTEN lists the folders the Builder writes into: bin, romfs, prompts, ngx, ffx, .game and .longshot_exefs. For a link at one of them the error now names the folder and says to move the whole game folder to the other drive (an SD card) and build into it, or else to remove the link. The README's "Files at a link" lists the same folders and says a link to the game folder itself, or to user/, is fine. test_game_folder_links.py checks the message and that every folder is named in the README.

Only the Linux AppImage's Python is hash-pinned … Or use the lock there too

Done in 46ec4bd7, using the lock. assemble_python writes the missing pins with requirements-lock.txt's SHA-256s (they already include the win_amd64 wheels) and runs pip with --require-hashes. A pin that isn't in the lock at its version stops it before pip runs. I ran the real command for nx2/prompt_requirements.txt against PyPI (--platform win_amd64) and it installs. python_lock.py's docstring and the description now say both Pythons are covered.

Line comments

  • host/hos_fopen_write.h (the header comment names writes still on plain fopen): converted, da41af87. The comment now lists the program cache, the markers and the log too.
  • host/hos_fopen_write.h (three fixed tags; two lookups of the name): IsReparseTagNameSurrogate and a handle-based check, da41af87, as above.
  • host/hos_main.c (RX_VK_DLSS_DIR): refused from the shared file, a1463821.
  • tests/test_game_writes_no_links.py (ALLOWED lists six files): replaced by the full scan with REVIEWED, da41af87.
  • nx2/make_game.py (FOLDER_WRITES stops a linked romfs): README list and an error that names the folder, 0f1e28ed.

Your "not checked" items

  • The NGX Linux signature check is moot now that the shared file can't set the folder.
  • I didn't follow GetNpadStates' count into the game's own calls either: no change there in this round.

Testing

Nothing ran in the game, so the PR stays WIP. Two Windows test requests are open:

  • T-93FV: a build and play from a OneDrive folder.
  • T-AJTA: a junction or symlink at longshot_options.ini and longshot_programs.bin.

Neither can be claimed until a maintainer adds testing wanted. A Linux build and play from a dump is still to request.

Thanks for the careful read. The branch is rebased on `main` @ d906c8f4 (it had moved past 1ef93e79), with 4 new commits on top; the head is 46ec4bd7. (Edited: `main` moved during the work, so I rebased once more; the hashes here are the current ones.) Point by point: **Must change** > 1. Several files the game writes on its own still use a plain `fopen` … Convert these writes, and have the check scan every file under `host/` and `gpu/vk/` with a list of reviewed switch-gated writes Done in da41af87. - Converted every write you listed: `vk_progcache.cpp` (`.bin`/`.idx` with `"wb"`, `"ab"`, and `"r+b"` + `_chsize_s`, which `hos_fopen_write` now takes as an existing-file-only mode), `vk_menu_i18n.cpp`'s `language.txt`, `load_default.h`'s marker and strike file, `module_ids.h`, `vk_main.cpp`'s skipped marker and `pcache_stamp.h`'s `.tmp`. - The new scan found more, also converted: `longshot_log.txt` (the `freopen` onto stderr, now `hos_freopen_write`), the bug report's copied logs and `report.txt`, the program cache's `.lock` (`O_NOFOLLOW`), and `RX_VK_PROBE`'s and `RX_VK_DRAW_STATS`' files, whose switches a shared `longshot_env.txt` can set. - The start-up reads of those files (the program cache and index, the marker, the stamp, `language.txt`) go through `hos_fopen_read*` too, so a FIFO there isn't waited on. - `tests/test_game_writes_no_links.py` is inverted. It walks every source under `host/` and `gpu/vk/` (not `host/posix`, `eden`, `sirit`, `compat` or `gpu/vk/tests`), strips comments, and finds every open that writes: `fopen`/`freopen` with a mode that writes, `open` with `O_WRONLY`/`O_RDWR`/`O_CREAT`/`O_APPEND`, `CreateFile*` with write access, `ofstream`, `CopyFile`/`copy_file`. - Each must go through the helper, make its file new (`"x"`, `O_EXCL`, `O_NOFOLLOW`, `CREATE_NEW`), or be in `REVIEWED`. Each `REVIEWED` entry names its file, its call, its switch and a reason. - Every `REVIEWED` switch is compiled through `env_line_allowed` and must be refused from the shared file. That's how `HOS_PGO` turned up; it's refused now, in a1463821. - A stale entry fails, and the scanner has its own positive and negative cases, so it can't silently pass on nothing. - Four non-switch entries are listed with reasons: `nn_fs.c`'s save `OpenFile`, Discord's pipe, the console handles, and `/dev/udmabuf`. The save file system is the one I'd flag. Making it no-follow means teaching the Linux `CreateFileA` stand-in `FILE_FLAG_OPEN_REPARSE_POINT`; I left it as a reviewed exception, open to a follow-up. - Run against 83e9a080, the new check fails on all of the above. > 2. `RX_VK_DLSS_DIR` is still a library-load path that a shared file can set … refuse it here like `RX_VK_FSR_DIR` and leave it to `HOS_ENV_FILE` Done in a1463821, the simple way you suggested. Its special case in `env_line_allowed` is gone, so it falls under the `_DIR` rule. The environment (Steam launch options, a shell) and a file `HOS_ENV_FILE` names still set it, since `env_line_allowed` only applies to the shared file. `test_env_file.py` now checks all of that: - every value is refused from `longshot_env.txt`, including `/tmp/x`, `/dev/shm/x`, `/proc/self/cwd`, `C:\Users\Public\ngx` and `Z:\ngx`; - `HOS_ENV_FILE`'s file sets it; - a value set in the environment survives the player's file. The README's DLSS and switches sections say where to set it now. With the switch refused, the Linux NGX signature question no longer decides anything. > 3. The "Rebased on current `main`" box is ticked but isn't true … Also untick or reword "One topic" Rebased on d906c8f4, which includes #54, #78, #147, #197 and the 17 merged since. The rebase was clean. Three checks that cut code this PR changes needed updates: - `test_udmabuf_warning.py`: the log's `freopen` became `hos_freopen_write`. - `test_upscaler_round_trip.py`: the options file is read with `hos_fopen_read`, and the fixture includes the helper. - `test_prepare_stall.py`: the watchdog's skipped marker is written with `hos_fopen_write`. `test_dev_packs.py` and `test_vk_occlusion_queries.py` pass. `tests/run_all.py --jobs 4`: 508 passed, 2 failed, 30 skipped. Both failures also fail on `main` on this PC: `test_lift_cache` (no numpy) and `test_menu_i18n` (since #138). "One topic" is unticked, with a note. **Should change** > Windows `hos_fopen_write`: match on `IsReparseTagNameSurrogate(tag)` … Better still, open with `CreateFileW(..., FILE_FLAG_OPEN_REPARSE_POINT)`, check `GetFileInformationByHandleEx(FileAttributeTagInfo)` on the handle, and only then `_open_osfhandle`/`_fdopen` Done in da41af87, both parts, with one change to the second: - The name is opened as itself with `FILE_READ_ATTRIBUTES` and `FILE_FLAG_OPEN_REPARSE_POINT` only, and its tag is checked on that handle with `IsReparseTagNameSurrogate`. - It's then opened for writing without the flag, and kept only if `GetFileInformationByHandle` gives the same volume and file index. After that come `SetEndOfFile` for `"w"`, then `_open_osfhandle` (`_O_APPEND`/`_O_TEXT` as the mode says) and `_fdopen`. - The second open is there so that OneDrive fills a file kept only online before it's written: writing through a handle opened with `FILE_FLAG_OPEN_REPARSE_POINT` would skip hydration. The file-index match closes the window you pointed at all the same. - A name with nothing at it is made with `CREATE_NEW`, which fails if a link appears in between. - Paths go from UTF-8 to wide with `MB_ERR_INVALID_CHARS`, as `pipeline_cache_file.h` does. The stand-in-`windows.h` fixture now checks that symlink, junction, LX, `WCI_LINK_1` and `GLOBAL_REPARSE` are refused before any write-access open. It also checks that two cloud tags are written, a swap between the two opens is refused (and both handles closed), and `"r+b"` doesn't create. With `LONGSHOT_MINGW` set, the check also builds the header as C and C++ against llvm-mingw's real headers. The whole Windows `longshot_vk.dll` builds and links with llvm-mingw, without warnings. Nothing has run on Windows itself yet: T-AJTA asks for a real junction and symlink test. > Split the PR … A regression in one part … would mean reverting all 2,086 lines Not done in this round. Answering the review on one branch kept the thread readable, and the must-changes cut across the parts: the env file's refusals now back the write check's reviewed list. The split I'd make: 1. link-safe writes and reads, plus the Builder (this PR); 2. the env file; 3. the HLE guest ranges; 4. pipeline-list validation; 5. the supply chain (`python_lock`, SDK downloads, `deps.sh`, the workflow check). Say if you want it before merge and I'll split along those lines; the commits already fall that way. Meanwhile the description no longer claims a single commit can be reverted on `main`. > The Builder now refuses any link at `romfs`, `bin`, `prompts`, `ngx` or `ffx` … Say in the README which folders the Builder won't take as links. The error could also name the folder to move instead. Done in 0f1e28ed. `FOLDERS_WRITTEN` lists the folders the Builder writes into: `bin`, `romfs`, `prompts`, `ngx`, `ffx`, `.game` and `.longshot_exefs`. For a link at one of them the error now names the folder and says to move the whole game folder to the other drive (an SD card) and build into it, or else to remove the link. The README's "Files at a link" lists the same folders and says a link to the game folder itself, or to `user/`, is fine. `test_game_folder_links.py` checks the message and that every folder is named in the README. > Only the Linux AppImage's Python is hash-pinned … Or use the lock there too Done in 46ec4bd7, using the lock. `assemble_python` writes the missing pins with `requirements-lock.txt`'s SHA-256s (they already include the `win_amd64` wheels) and runs pip with `--require-hashes`. A pin that isn't in the lock at its version stops it before pip runs. I ran the real command for `nx2/prompt_requirements.txt` against PyPI (`--platform win_amd64`) and it installs. `python_lock.py`'s docstring and the description now say both Pythons are covered. **Line comments** - `host/hos_fopen_write.h` (the header comment names writes still on plain `fopen`): converted, da41af87. The comment now lists the program cache, the markers and the log too. - `host/hos_fopen_write.h` (three fixed tags; two lookups of the name): `IsReparseTagNameSurrogate` and a handle-based check, da41af87, as above. - `host/hos_main.c` (`RX_VK_DLSS_DIR`): refused from the shared file, a1463821. - `tests/test_game_writes_no_links.py` (`ALLOWED` lists six files): replaced by the full scan with `REVIEWED`, da41af87. - `nx2/make_game.py` (`FOLDER_WRITES` stops a linked `romfs`): README list and an error that names the folder, 0f1e28ed. **Your "not checked" items** - The NGX Linux signature check is moot now that the shared file can't set the folder. - I didn't follow `GetNpadStates`' count into the game's own calls either: no change there in this round. **Testing** Nothing ran in the game, so the PR stays WIP. Two Windows test requests are open: - T-93FV: a build and play from a OneDrive folder. - T-AJTA: a junction or symlink at `longshot_options.ini` and `longshot_programs.bin`. Neither can be claimed until a maintainer adds `testing wanted`. A Linux build and play from a dump is still to request.
TrashPanda force-pushed security/hardening-passes from 641bf324a8
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to 46ec4bd792
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 7s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
2026-11-13 18:36:34 +00:00
Compare
Author
Contributor

Rebased once more onto main @ d906c8f4 (eight PRs merged while I worked); head 46ec4bd7, same changes. The answer above now has the current hashes. run_all: 508 passed, 2 failed (test_lift_cache and test_menu_i18n, both also failing on main here); longshot_vk.dll builds and links again with llvm-mingw.

Rebased once more onto main @ d906c8f4 (eight PRs merged while I worked); head 46ec4bd7, same changes. The answer above now has the current hashes. run_all: 508 passed, 2 failed (test_lift_cache and test_menu_i18n, both also failing on main here); longshot_vk.dll builds and links again with llvm-mingw.
HumanBean requested changes 2026-11-14 01:07:05 +00:00
Dismissed
HumanBean left a comment

Second review, of 46ec4bd7 (d906c8f4..46ec4bd7). Read only: nothing built or run here. No tester results on the PR yet (T-93FV/T-AJTA aren't claimable until a maintainer adds testing wanted), and the CI jobs for this head are pending, not run.

Fixed since the first review (83e9a080)

  • All the plain writes I listed are converted. The check is now a scan of host/ and gpu/vk/ with a reviewed list tied to switches the shared file refuses, and a self-test. That's the right shape. A spot check of the remaining plain fopen writes found only REVIEWED entries.
  • RX_VK_DLSS_DIR is refused from the shared file.
  • Windows hos_fopen_write: surrogate-bit test on a handle opened as the name itself, then a second open that must have the same volume and file index before anything is emptied. That closes the name race. The Linux side was already race-free at the name.
  • The Builder's folder error and the README list. The Windows tools Python now installs from the lock with --require-hashes.

Still open / new

  1. Must change: the bug report reads the logs through a link. copy_log (vk_bug_report.cpp:186) opens longshot_log.txt / longshot_log.1.txt from the game folder with a plain fopen. A folder from elsewhere can carry either name as a link (the game's own write now refuses it, so the report then reads the link's target as "the run before's log"). Whatever it points at gets copied into the report folder, which the player is asked to send. This is the one read whose content leaves the PC, so hold it to the write rule: no link at the name, and a regular file only (Linux O_NOFOLLOW + fstat; Windows: the same look-handle test as hos_fopen_write_handle). On Linux a FIFO there would also hang the Insert menu.
  2. Must change: rebase again. main is 14 commits past d906c8f4, and 7 of the PR's source files changed there (vk_bug_report.cpp, vk_main.cpp, vk_menu.cpp, nn_audio.c, nn_hid.c, oot_menu.c, plus tests). New writes on main will meet the inverted check, so rerun it after the rebase.
  3. Should change: the Builder and nup.py test for links with a fixed tag list (LINK_TAGS: symlink, mount point, LX), while the game tests the surrogate bit. WCI_LINK, GLOBAL_REPARSE and any future name surrogate pass the Builder but not the game. Use tag & 0x20000000 there too, so both sides agree; OneDrive's cloud tags still pass.
  4. Worth a look (Windows, legitimate setups): the swap check trusts the file index. On file systems that synthesise it per open (some network redirectors, WinFsp/FUSE mounts, guest shared folders), every write to an existing file would be refused, and options would never save. If both indexes are 0, the check means nothing. A narrower shape: a file with no reparse point at all (the usual case) can be opened for writing with FILE_FLAG_OPEN_REPARSE_POINT in one step, since the flag changes nothing there. Keep the second open and the index comparison only for non-surrogate reparse points (OneDrive's cloud tags, and also dedup/WOF-compressed files, which must not be written through the flag).
  5. Accepted by design, please say so in the README: a link at a folder on the way (the game folder, user/, the bug-report or prompts folders) is followed. That's right for linked user/ and OneDrive. Hard links aren't caught either (st_nlink/nNumberOfLinks > 1). That's low risk, since an archive can only hard-link within itself, and refusing them would break dedup'd or cp -al folders, so I wouldn't change it. Just name it as out of scope.
  6. "One topic" is still unticked. The split you proposed is fine. For the release it matters more than for review (point below).

v3.0.0: not as one PR. It is ~2,600 lines over five topics, untested in the game and on Windows, still WIP, and it changes what players' shared longshot_env.txt files do. If something should ride v3.0.0, split out the lowest-risk parts (the HLE guest-range checks, and the env-file refusals with a release note for RX_VK_DLSS_DIR). Land the link-safe writes and the Builder after the Windows OneDrive and junction runs (T-93FV, T-AJTA) and a Linux play run.

Second review, of 46ec4bd7 (d906c8f4..46ec4bd7). Read only: nothing built or run here. No tester results on the PR yet (T-93FV/T-AJTA aren't claimable until a maintainer adds `testing wanted`), and the CI jobs for this head are pending, not run. **Fixed since the first review (83e9a080)** - All the plain writes I listed are converted. The check is now a scan of `host/` and `gpu/vk/` with a reviewed list tied to switches the shared file refuses, and a self-test. That's the right shape. A spot check of the remaining plain `fopen` writes found only `REVIEWED` entries. - `RX_VK_DLSS_DIR` is refused from the shared file. - Windows `hos_fopen_write`: surrogate-bit test on a handle opened as the name itself, then a second open that must have the same volume and file index before anything is emptied. That closes the name race. The Linux side was already race-free at the name. - The Builder's folder error and the README list. The Windows tools Python now installs from the lock with `--require-hashes`. **Still open / new** 1. **Must change: the bug report reads the logs through a link.** `copy_log` (`vk_bug_report.cpp:186`) opens `longshot_log.txt` / `longshot_log.1.txt` from the game folder with a plain `fopen`. A folder from elsewhere can carry either name as a link (the game's own write now refuses it, so the report then reads the link's target as "the run before's log"). Whatever it points at gets copied into the report folder, which the player is asked to send. This is the one read whose content leaves the PC, so hold it to the write rule: no link at the name, and a regular file only (Linux `O_NOFOLLOW` + `fstat`; Windows: the same look-handle test as `hos_fopen_write_handle`). On Linux a FIFO there would also hang the Insert menu. 2. **Must change: rebase again.** `main` is 14 commits past d906c8f4, and 7 of the PR's source files changed there (`vk_bug_report.cpp`, `vk_main.cpp`, `vk_menu.cpp`, `nn_audio.c`, `nn_hid.c`, `oot_menu.c`, plus tests). New writes on main will meet the inverted check, so rerun it after the rebase. 3. **Should change: the Builder and `nup.py` test for links with a fixed tag list** (`LINK_TAGS`: symlink, mount point, LX), while the game tests the surrogate bit. `WCI_LINK`, `GLOBAL_REPARSE` and any future name surrogate pass the Builder but not the game. Use `tag & 0x20000000` there too, so both sides agree; OneDrive's cloud tags still pass. 4. **Worth a look (Windows, legitimate setups):** the swap check trusts the file index. On file systems that synthesise it per open (some network redirectors, WinFsp/FUSE mounts, guest shared folders), every write to an existing file would be refused, and options would never save. If both indexes are 0, the check means nothing. A narrower shape: a file with no reparse point at all (the usual case) can be opened for writing with `FILE_FLAG_OPEN_REPARSE_POINT` in one step, since the flag changes nothing there. Keep the second open and the index comparison only for non-surrogate reparse points (OneDrive's cloud tags, and also dedup/WOF-compressed files, which must not be written through the flag). 5. **Accepted by design, please say so in the README:** a link at a *folder* on the way (the game folder, `user/`, the bug-report or prompts folders) is followed. That's right for linked `user/` and OneDrive. Hard links aren't caught either (`st_nlink`/`nNumberOfLinks` > 1). That's low risk, since an archive can only hard-link within itself, and refusing them would break dedup'd or `cp -al` folders, so I wouldn't change it. Just name it as out of scope. 6. "One topic" is still unticked. The split you proposed is fine. For the release it matters more than for review (point below). **v3.0.0:** not as one PR. It is ~2,600 lines over five topics, untested in the game and on Windows, still WIP, and it changes what players' shared `longshot_env.txt` files do. If something should ride v3.0.0, split out the lowest-risk parts (the HLE guest-range checks, and the env-file refusals with a release note for `RX_VK_DLSS_DIR`). Land the link-safe writes and the Builder after the Windows OneDrive and junction runs (T-93FV, T-AJTA) and a Linux play run.
@ -187,3 +188,3 @@
exists(from, &f.written);
f.size = file_size(src);
FILE* dst = fopen(to.c_str(), "wb");
FILE* dst = hos_fopen_write(to.c_str(), "wb");
Owner

The destination is safe now, but the source two lines up (fopen(from.c_str(), "rb"), line 186) still follows a link at longshot_log.txt / longshot_log.1.txt in the game folder and reads a FIFO. This is the file the player is asked to send, so the source needs the no-link, regular-file rule too, e.g. a hos_fopen_read_nofollow beside hos_fopen_read (Linux O_NOFOLLOW|O_NONBLOCK + S_ISREG; Windows the look-handle test from hos_fopen_write_handle).

The destination is safe now, but the source two lines up (`fopen(from.c_str(), "rb")`, line 186) still follows a link at `longshot_log.txt` / `longshot_log.1.txt` in the game folder and reads a FIFO. This is the file the player is asked to send, so the source needs the no-link, regular-file rule too, e.g. a `hos_fopen_read_nofollow` beside `hos_fopen_read` (Linux `O_NOFOLLOW|O_NONBLOCK` + `S_ISREG`; Windows the look-handle test from `hos_fopen_write_handle`).
@ -0,0 +65,4 @@
written, and kept only if it is the file looked at) */
HANDLE h = plain ? CreateFileW(w, access, share, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL) : INVALID_HANDLE_VALUE;
const int opened = h != INVALID_HANDLE_VALUE;
const int same = opened && GetFileInformationByHandle(h, &is) && is.dwVolumeSerialNumber == was.dwVolumeSerialNumber &&
Owner

This holds on NTFS and ReFS. On file systems that make up the file index per open (some redirectors, WinFsp/FUSE mounts, guest shared folders), every existing file is refused here, so options and caches stop saving. And if both indexes are 0, the check passes vacuously. Consider keeping the second open and this comparison only when the look handle has FILE_ATTRIBUTE_REPARSE_POINT (non-surrogate: OneDrive, dedup, WOF). A plain file can be opened for writing with FILE_FLAG_OPEN_REPARSE_POINT in one step, since the flag is a no-op there, which leaves no second lookup to compare.

This holds on NTFS and ReFS. On file systems that make up the file index per open (some redirectors, WinFsp/FUSE mounts, guest shared folders), every existing file is refused here, so options and caches stop saving. And if both indexes are 0, the check passes vacuously. Consider keeping the second open and this comparison only when the look handle has `FILE_ATTRIBUTE_REPARSE_POINT` (non-surrogate: OneDrive, dedup, WOF). A plain file can be opened for writing with `FILE_FLAG_OPEN_REPARSE_POINT` in one step, since the flag is a no-op there, which leaves no second lookup to compare.
nx2/make_game.py Outdated
@ -1507,1 +1553,3 @@
names (anywhere on this PC) into the game folder. The folder itself may be one. (Walked as a long path: #1468)"""
# (Windows) the reparse points that are links: a symbolic link, a junction (mount point), WSL's link. OneDrive marks its files
# with others, and those are files
LINK_TAGS = (0xA000000C, 0xA0000003, 0xA000001D)
Owner

The game side (hos_fopen_write.h:62) refuses any name surrogate; this list has three tags, so IO_REPARSE_TAG_WCI_LINK_1 and IO_REPARSE_TAG_GLOBAL_REPARSE pass the Builder but not the game. tag & 0x20000000 (the surrogate bit) here and in nup.py:401 keeps both sides in step, and OneDrive's cloud tags still aren't surrogates.

The game side (`hos_fopen_write.h:62`) refuses any name surrogate; this list has three tags, so `IO_REPARSE_TAG_WCI_LINK_1` and `IO_REPARSE_TAG_GLOBAL_REPARSE` pass the Builder but not the game. `tag & 0x20000000` (the surrogate bit) here and in `nup.py:401` keeps both sides in step, and OneDrive's cloud tags still aren't surrogates.
HumanBean added this to the v3.0.0 milestone 2026-11-14 01:21:34 +00:00
HumanBean force-pushed security/hardening-passes from 46ec4bd792
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 7s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
to b7a933da0d
All checks were successful
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Third-party parts / The same as upstream (pull_request) Has been skipped
Website / Website on Cloudflare (pull_request) Has been skipped
CI / Conventional Commit title (pull_request) Successful in 3s
CI / Workflow lint (actionlint) (pull_request) Successful in 5s
Third-party parts / As their notes say (pull_request) Successful in 6s
PR description / Description follows the template (pull_request_target) Successful in 7s
Website / Website on Cloudflare (pull_request_target) Has been skipped
2026-11-14 01:54:32 +00:00
Compare
Owner

Answering the second review (comment), on behalf of the reviewer's side. The branch is rebased onto main @ f87557ae and the new head is b7a933da: the 36 earlier commits, each with its author, then one new commit (b7a933da). Nothing was run, built or compiled for this head: the code and the checks were read and reasoned through only.

1. Bug report reads the logs through a link (must)

  • copy_log (gpu/vk/vk_bug_report.cpp) now opens longshot_log.txt / longshot_log.1.txt with a new hos_fopen_read_nofollow (host/hos_fopen_write.h).
    • Linux: O_NOFOLLOW | O_NONBLOCK, then fstat and regular files only, then blocking again.
    • Windows: the same handle test the writes use (hos_fopen_write_handle with GENERIC_READ, existing files only).
    • So a link at those names can't put another file in the report, and a FIFO no longer hangs the Insert menu. A refused read is logged once. The copy was already size-capped (kLogCap).
  • Other plain reads of game-folder files, found by grepping host/ and gpu/vk/, now go through hos_fopen_read_binary (regular files only): longshot_module_ids.bin (module_ids.h), longshot_shaders.bin (seed_open), image.bin (hos_main.c), the three prompt files the game reads back (prompts.c) and RX_VK_DRAW_STATS' trigger file.
  • What's left on a plain read, and why, is under Reviewer notes in the description: the program cache re-opening a file the game just opened safely, debugging switches the shared file refuses, and system files.
  • tests/test_game_writes_no_links.py now covers the new read on both platforms: the Linux fixture, the stand-in Windows fixture, a copy_log source check and the read counts. The fixtures in test_prompts_live.py and test_seed_reader.py get the helper.

2. Rebase (must)

  • Main moved during the work, so I rebased twice and ended on f87557ae, which includes #96. This PR adds no tested-configurations rows, so the frozen table doesn't affect it.
  • One conflict: tests/test_menu_keys.py's fixture includes, in "test: main's new checks get this branch's helpers". I kept both main's #include "gpu/vk/vk_api.h" and the PR's #include "host/hos_fopen_read.h".
  • I checked that every rebased commit's added and removed lines match the original, and that the whole PR diff against the new main matches the old one against d906c8f4 line for line. So nothing of main is reverted.
  • Main added no new open calls in host/ or gpu/vk/ since d906c8f4, including the 7 files you named and the new oot_state_tap.c. So the inverted write check still holds by reading.

3. One link rule (should)

  • nx2/make_game.py and nx2/nup.py replace LINK_TAGS with link_tag(tag): the name-surrogate bit (0x20000000), the same rule as IsReparseTagNameSurrogate in the game. WCI_LINK and GLOBAL_REPARSE now stop the Builder too, and OneDrive's tags still pass.
  • test_game_folder_links.py adds those tags and a check that the Builder, nup.py and the header agree.

4. File index on Windows (should)

  • A file with no reparse point (the usual case) is now opened for writing as itself in one step (FILE_FLAG_OPEN_REPARSE_POINT, a no-op for such a file). It's refused if that handle shows a reparse point or a folder.
  • No file index is compared on that path, so file systems that make it up for each open can save again.
  • The second open and the volume and index check stay only for reparse points that aren't links (OneDrive, dedup, WOF), and an index of 0 is refused there.
  • Opens that query the handle also ask for FILE_READ_ATTRIBUTES.
  • The stand-in Windows fixture now tells the first open from the second and checks each case.

5. Out of scope

  • The README's "Files at a link" and the header comment say it now: a link at a folder along the path is followed by design (a linked user/, a linked game folder, OneDrive), and hard links aren't caught.

Split: not done; that's the author's call. For v3.0.0 the suggestion stands to ship the low-risk parts (the HLE guest ranges, and the env-file refusals with a release note for RX_VK_DLSS_DIR) on their own, and land the link-safe writes and the Builder after the Windows and Linux runs.

Description:

  • Measurements and Testing say this head hasn't been run.
  • The run_all box and the game-free test box are unticked until the checks run on b7a933da.
  • "One topic" and "Host code builds" stay unticked, and the PR stays WIP.

Still needed:

  • python -I -B tests/run_all.py on b7a933da (or CI).
  • The llvm-mingw build of the header and of longshot_vk.dll.
  • A Windows run: OneDrive, a junction or symlink at the options file and the program cache, and a network or FUSE folder saving (T-93FV, T-AJTA).
  • A Linux play run from a dump.
  • The split decision.

🤖 Generated with Claude Code

Answering the second review ([comment](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/218#issuecomment-15704)), on behalf of the reviewer's side. The branch is rebased onto `main` @ f87557ae and the new head is b7a933da: the 36 earlier commits, each with its author, then one new commit (b7a933da). **Nothing was run, built or compiled for this head:** the code and the checks were read and reasoned through only. **1. Bug report reads the logs through a link (must)** - `copy_log` (`gpu/vk/vk_bug_report.cpp`) now opens `longshot_log.txt` / `longshot_log.1.txt` with a new `hos_fopen_read_nofollow` (`host/hos_fopen_write.h`). - Linux: `O_NOFOLLOW | O_NONBLOCK`, then `fstat` and regular files only, then blocking again. - Windows: the same handle test the writes use (`hos_fopen_write_handle` with `GENERIC_READ`, existing files only). - So a link at those names can't put another file in the report, and a FIFO no longer hangs the Insert menu. A refused read is logged once. The copy was already size-capped (`kLogCap`). - Other plain reads of game-folder files, found by grepping `host/` and `gpu/vk/`, now go through `hos_fopen_read_binary` (regular files only): `longshot_module_ids.bin` (`module_ids.h`), `longshot_shaders.bin` (`seed_open`), `image.bin` (`hos_main.c`), the three prompt files the game reads back (`prompts.c`) and `RX_VK_DRAW_STATS`' trigger file. - What's left on a plain read, and why, is under Reviewer notes in the description: the program cache re-opening a file the game just opened safely, debugging switches the shared file refuses, and system files. - `tests/test_game_writes_no_links.py` now covers the new read on both platforms: the Linux fixture, the stand-in Windows fixture, a `copy_log` source check and the read counts. The fixtures in `test_prompts_live.py` and `test_seed_reader.py` get the helper. **2. Rebase (must)** - Main moved during the work, so I rebased twice and ended on f87557ae, which includes [#96](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/96). This PR adds no tested-configurations rows, so the frozen table doesn't affect it. - One conflict: `tests/test_menu_keys.py`'s fixture includes, in "test: main's new checks get this branch's helpers". I kept both main's `#include "gpu/vk/vk_api.h"` and the PR's `#include "host/hos_fopen_read.h"`. - I checked that every rebased commit's added and removed lines match the original, and that the whole PR diff against the new main matches the old one against d906c8f4 line for line. So nothing of main is reverted. - Main added no new open calls in `host/` or `gpu/vk/` since d906c8f4, including the 7 files you named and the new `oot_state_tap.c`. So the inverted write check still holds by reading. **3. One link rule (should)** - `nx2/make_game.py` and `nx2/nup.py` replace `LINK_TAGS` with `link_tag(tag)`: the name-surrogate bit (`0x20000000`), the same rule as `IsReparseTagNameSurrogate` in the game. `WCI_LINK` and `GLOBAL_REPARSE` now stop the Builder too, and OneDrive's tags still pass. - `test_game_folder_links.py` adds those tags and a check that the Builder, `nup.py` and the header agree. **4. File index on Windows (should)** - A file with no reparse point (the usual case) is now opened for writing as itself in one step (`FILE_FLAG_OPEN_REPARSE_POINT`, a no-op for such a file). It's refused if that handle shows a reparse point or a folder. - No file index is compared on that path, so file systems that make it up for each open can save again. - The second open and the volume and index check stay only for reparse points that aren't links (OneDrive, dedup, WOF), and an index of 0 is refused there. - Opens that query the handle also ask for `FILE_READ_ATTRIBUTES`. - The stand-in Windows fixture now tells the first open from the second and checks each case. **5. Out of scope** - The README's "Files at a link" and the header comment say it now: a link at a folder along the path is followed by design (a linked `user/`, a linked game folder, OneDrive), and hard links aren't caught. **Split:** not done; that's the author's call. For v3.0.0 the suggestion stands to ship the low-risk parts (the HLE guest ranges, and the env-file refusals with a release note for `RX_VK_DLSS_DIR`) on their own, and land the link-safe writes and the Builder after the Windows and Linux runs. **Description:** - Measurements and Testing say this head hasn't been run. - The `run_all` box and the game-free test box are unticked until the checks run on b7a933da. - "One topic" and "Host code builds" stay unticked, and the PR stays WIP. **Still needed:** - `python -I -B tests/run_all.py` on b7a933da (or CI). - The llvm-mingw build of the header and of `longshot_vk.dll`. - A Windows run: OneDrive, a junction or symlink at the options file and the program cache, and a network or FUSE folder saving (T-93FV, T-AJTA). - A Linux play run from a dump. - The split decision. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
HumanBean left a comment

Re-review of the fix on b7a933da (answering our review on 46ec4bd7 and the earlier one on 83e9a080; the fix is described in this comment). Read only: nothing was built or run for this review.

Verdict: what we asked for is fixed. No bug found in the new code. One thing needs a OneDrive test (inline, on host/hos_fopen_write.h), and the PR stays WIP until the items under What remains are done.

Checked

  • hos_fopen_read_nofollow, Linux: O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC, then fstat, and only S_ISREG gets past before anything is read. O_NONBLOCK is cleared before fdopen, and has no effect on a regular file anyway. The descriptor is closed on every error path, fdopen failing included. ELOOP (a link at the name) is logged once, and so is a FIFO, a folder or a device. A missing file is quiet.
  • hos_fopen_read_nofollow, Windows: it goes through hos_fopen_write_handle(GENERIC_READ, existing=1), so nothing is created where nothing is. The handle is closed if _open_osfhandle fails, and _close runs if _fdopen fails. The share modes are READ|WRITE. The game's own handle on the live log comes from the CRT's freopen, which shares read and write (_SH_DENYNO), so the bug report can open the log while the game is still writing it, and the read takes no access the game's handle refuses. A refused open takes no data access at all. The path is converted with CP_UTF8 and MB_ERR_INVALID_CHARS, so tests/test_utf8_paths.py holds.
  • The write open in one step (kind 0): the regular-file test runs on the same handle that then writes, opened with FILE_FLAG_OPEN_REPARSE_POINT. A link put at the name in between is opened as the link itself and refused. A folder fails without BACKUP_SEMANTICS. No file index is compared, so redirectors and FUSE mounts can save again. FILE_READ_ATTRIBUTES is added to the access, and GENERIC_WRITE needs it for FileAttributeTagInfo.
  • Kind 1 (OneDrive, dedup, WOF): this path is taken when the first look shows a reparse tag that isn't a name surrogate (CLOUD_* is 0x9…, bit 29 clear). The file is opened again without the flag, so OneDrive still downloads an online-only file, and it is kept only if the volume and file index match. An index of 0 is refused. Both handles are closed on every path.
  • link_tag(): 0x20000000 is the bit IsReparseTagNameSurrogate tests, and the header's fallback macro matches it. On Linux, os.lstat has no st_reparse_tag, so link_tag(0) is False as 0 in LINK_TAGS was, and the symlink and junction tests are unchanged. LINK_TAGS is used nowhere else.
  • Reads moved to hos_fopen_read_binary: module_ids.h, vk_shader.cpp (which includes hos_fopen_read.h), vk_draw.cpp, hos_main.c (its Linux branch only) and prompts.c. Each file includes the header, and each caller handles NULL as fopen did. The other plain reads left in host/ and gpu/vk/ are system files (/proc, os-release, /sys, user-dirs.dirs), debug or control switches, or vk_progcache's re-open of a cache that hos_fopen_write has just checked. None of them leaves the PC or makes the start wait.
  • Tests, read against the code: the Windows stand-in now answers per handle (LOOK and FILE_H) with separate attributes, tags and indices. The new cases match the new branches: one write open with the flag for a plain file, the index not compared for plain files, a link or folder put at the name before the open, a OneDrive index that differs or is 0, the read refused at every name surrogate, and the read opened with GENERIC_READ|FILE_READ_ATTRIBUTES and _O_RDONLY. Two checks were removed: "plain writes never use the flag" and "a plain file whose index changed is refused". Both are now false by design and have replacements, so nothing is weakened. The Linux fixture covers a link, a dangling link, a FIFO, a folder and a missing file. copy_log is checked to read through the helper only. The new counts in BINARY_READS and NOFOLLOW_READS match the sources. The test_prompts_live macro and test_seed_reader's -I host fit how those tests build. test_bug_report compiles vk_bug_report.cpp whole, and its include path is relative.
  • Rebase: a range-diff of d906c8f4..46ec4bd7 against f87557ae..1ee8f166 shows 32 commits unchanged and 4 changed in context lines only. Not one added or removed line differs: 234 deletions on each side and the same set of added lines. The only conflict was in tests/test_menu_keys.py, where vk_api.h (from main) and hos_fopen_read.h are both kept. Nothing of main is reverted.
  • CI on b7a933da: the combined status is success, but only the title, actionlint, third-party notes and description jobs ran. The Python source checks and SPIR-V fixtures were skipped, so no check has run on this head yet.

What remains (all before pr ready)

  • python -I -B tests/run_all.py (or CI's Python checks) passing on b7a933da, with test_game_writes_no_links.py run under LONGSHOT_MINGW so the header is built against Windows' real headers.
  • llvm-mingw builds of the host and the renderer, and a Linux host build.
  • The Windows runs T-93FV and T-AJTA: saves, options, caches and the bug report on a local disk, in OneDrive with online-only files (see inline), and on a network folder (SMB). The network run should also show that FileAttributeTagInfo answers there, since the game refuses a file when it doesn't, as it did before.
  • A Linux play run from a dump (its test request is still to open).
  • The decision on splitting the PR (the One topic box).
Re-review of the fix on b7a933da (answering [our review on 46ec4bd7](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/218#issuecomment-15704) and [the earlier one on 83e9a080](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/218#issuecomment-13998); the fix is described in [this comment](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/218#issuecomment-16295)). Read only: nothing was built or run for this review. **Verdict: what we asked for is fixed.** No bug found in the new code. One thing needs a OneDrive test (inline, on `host/hos_fopen_write.h`), and the PR stays WIP until the items under *What remains* are done. ### Checked - **`hos_fopen_read_nofollow`, Linux:** `O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC`, then `fstat`, and only `S_ISREG` gets past before anything is read. `O_NONBLOCK` is cleared before `fdopen`, and has no effect on a regular file anyway. The descriptor is closed on every error path, `fdopen` failing included. `ELOOP` (a link at the name) is logged once, and so is a FIFO, a folder or a device. A missing file is quiet. - **`hos_fopen_read_nofollow`, Windows:** it goes through `hos_fopen_write_handle(GENERIC_READ, existing=1)`, so nothing is created where nothing is. The handle is closed if `_open_osfhandle` fails, and `_close` runs if `_fdopen` fails. The share modes are READ|WRITE. The game's own handle on the live log comes from the CRT's `freopen`, which shares read and write (`_SH_DENYNO`), so the bug report can open the log while the game is still writing it, and the read takes no access the game's handle refuses. A refused open takes no data access at all. The path is converted with `CP_UTF8` and `MB_ERR_INVALID_CHARS`, so `tests/test_utf8_paths.py` holds. - **The write open in one step (kind 0):** the regular-file test runs on the same handle that then writes, opened with `FILE_FLAG_OPEN_REPARSE_POINT`. A link put at the name in between is opened as the link itself and refused. A folder fails without `BACKUP_SEMANTICS`. No file index is compared, so redirectors and FUSE mounts can save again. `FILE_READ_ATTRIBUTES` is added to the access, and `GENERIC_WRITE` needs it for `FileAttributeTagInfo`. - **Kind 1 (OneDrive, dedup, WOF):** this path is taken when the first look shows a reparse tag that isn't a name surrogate (`CLOUD_*` is 0x9…, bit 29 clear). The file is opened again without the flag, so OneDrive still downloads an online-only file, and it is kept only if the volume and file index match. An index of 0 is refused. Both handles are closed on every path. - **`link_tag()`:** 0x20000000 is the bit `IsReparseTagNameSurrogate` tests, and the header's fallback macro matches it. On Linux, `os.lstat` has no `st_reparse_tag`, so `link_tag(0)` is False as `0 in LINK_TAGS` was, and the symlink and junction tests are unchanged. `LINK_TAGS` is used nowhere else. - **Reads moved to `hos_fopen_read_binary`:** `module_ids.h`, `vk_shader.cpp` (which includes `hos_fopen_read.h`), `vk_draw.cpp`, `hos_main.c` (its Linux branch only) and `prompts.c`. Each file includes the header, and each caller handles NULL as `fopen` did. The other plain reads left in `host/` and `gpu/vk/` are system files (`/proc`, `os-release`, `/sys`, `user-dirs.dirs`), debug or control switches, or `vk_progcache`'s re-open of a cache that `hos_fopen_write` has just checked. None of them leaves the PC or makes the start wait. - **Tests, read against the code:** the Windows stand-in now answers per handle (LOOK and FILE_H) with separate attributes, tags and indices. The new cases match the new branches: one write open with the flag for a plain file, the index not compared for plain files, a link or folder put at the name before the open, a OneDrive index that differs or is 0, the read refused at every name surrogate, and the read opened with `GENERIC_READ|FILE_READ_ATTRIBUTES` and `_O_RDONLY`. Two checks were removed: "plain writes never use the flag" and "a plain file whose index changed is refused". Both are now false by design and have replacements, so nothing is weakened. The Linux fixture covers a link, a dangling link, a FIFO, a folder and a missing file. `copy_log` is checked to read through the helper only. The new counts in `BINARY_READS` and `NOFOLLOW_READS` match the sources. The `test_prompts_live` macro and `test_seed_reader`'s `-I host` fit how those tests build. `test_bug_report` compiles `vk_bug_report.cpp` whole, and its include path is relative. - **Rebase:** a `range-diff` of `d906c8f4..46ec4bd7` against `f87557ae..1ee8f166` shows 32 commits unchanged and 4 changed in context lines only. Not one added or removed line differs: 234 deletions on each side and the same set of added lines. The only conflict was in `tests/test_menu_keys.py`, where `vk_api.h` (from main) and `hos_fopen_read.h` are both kept. Nothing of main is reverted. - **CI on b7a933da:** the combined status is success, but only the title, actionlint, third-party notes and description jobs ran. The **Python source checks and SPIR-V fixtures were skipped**, so no check has run on this head yet. ### What remains (all before `pr ready`) - `python -I -B tests/run_all.py` (or CI's Python checks) passing on b7a933da, with `test_game_writes_no_links.py` run under `LONGSHOT_MINGW` so the header is built against Windows' real headers. - llvm-mingw builds of the host and the renderer, and a Linux host build. - The Windows runs T-93FV and T-AJTA: saves, options, caches and the bug report on a local disk, **in OneDrive with online-only files** (see inline), and on **a network folder** (SMB). The network run should also show that `FileAttributeTagInfo` answers there, since the game refuses a file when it doesn't, as it did before. - A Linux play run from a dump (its test request is still to open). - The decision on splitting the PR (the *One topic* box).
@ -0,0 +77,4 @@
BY_HANDLE_FILE_INFORMATION was, is;
HANDLE h = INVALID_HANDLE_VALUE;
int same = 0, refuse = kind < 0;
if (kind == 0) {
Owner

Correct as far as reading the code goes: the check runs on the handle that writes, and a link swapped in is opened as itself and refused. One thing for the OneDrive run, though: this branch is chosen because look showed no reparse tag. The old code always opened the file a second time without the flag, so OneDrive downloaded the file whatever look saw. Here, if the cloud filter hides placeholders from the game's process (the placeholder compatibility mode, PHCM_DISGUISE_PLACEHOLDER), an online-only file would land in kind 0. It would then be opened with FILE_FLAG_OPEN_REPARSE_POINT and never downloaded. With w that would hardly matter, but ab (the lists and markers) and r+b (the program cache) would add to, or read, data that isn't there. I'm not certain which mode a plain Win32 game gets by default, so I don't call it a bug. Please make the OneDrive part of T-93FV/T-AJTA cover it: set the game folder to Free up space, then start the game and check that longshot_options.ini, an appended pipeline list and the program cache load and save intact. If it fails, a fallback that skips the flag here when the volume is a cloud sync root would keep the one-step open everywhere else.

Correct as far as reading the code goes: the check runs on the handle that writes, and a link swapped in is opened as itself and refused. One thing for the OneDrive run, though: this branch is chosen because `look` showed **no** reparse tag. The old code always opened the file a second time without the flag, so OneDrive downloaded the file whatever `look` saw. Here, if the cloud filter hides placeholders from the game's process (the placeholder compatibility mode, `PHCM_DISGUISE_PLACEHOLDER`), an online-only file would land in kind 0. It would then be opened *with* `FILE_FLAG_OPEN_REPARSE_POINT` and never downloaded. With `w` that would hardly matter, but `ab` (the lists and markers) and `r+b` (the program cache) would add to, or read, data that isn't there. I'm not certain which mode a plain Win32 game gets by default, so I don't call it a bug. Please make the OneDrive part of T-93FV/T-AJTA cover it: set the game folder to *Free up space*, then start the game and check that `longshot_options.ini`, an appended pipeline list and the program cache load and save intact. If it fails, a fallback that skips the flag here when the volume is a cloud sync root would keep the one-step open everywhere else.
HumanBean dismissed HumanBean's review 2026-11-14 02:01:16 +00:00
Reason:

Superseded by #218 (comment): the fix on b7a933da resolves this review's points; what remains is testing (run_all/CI, builds, the Windows and Linux runs) and the split decision.

HumanBean dismissed HumanBean's review 2026-11-14 02:01:16 +00:00
Reason:

Superseded by #218 (comment): the fix on b7a933da resolves this review's points; what remains is testing (run_all/CI, builds, the Windows and Linux runs) and the split decision.

All checks were successful
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Third-party parts / The same as upstream (pull_request) Has been skipped
Website / Website on Cloudflare (pull_request) Has been skipped
CI / Conventional Commit title (pull_request) Successful in 3s
CI / Workflow lint (actionlint) (pull_request) Successful in 5s
Third-party parts / As their notes say (pull_request) Successful in 6s
PR description / Description follows the template (pull_request_target) Successful in 7s
Website / Website on Cloudflare (pull_request_target) Has been skipped
This pull request has changes conflicting with the target branch.
  • host/hos_main.c
  • host/prompts.c
  • tests/test_audio_output_menu.py
  • tests/test_menu_keys.py
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u 'security/hardening-passes:TrashPanda-security/hardening-passes'
git switch 'TrashPanda-security/hardening-passes'
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
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/Longshot!218
No description provided.