WIP: fix(security): shared files, links and lists can't make the game, Builder or tools write or read where they shouldn't #218
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!218
Loading…
Reference in a new issue
No description provided.
Delete branch "TrashPanda/Longshot:security/hardening-passes"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Why
Files, settings and lists people share or download (a
longshot_env.txtfrom 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 branchsecurity/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.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 cachelongshot_programs.binand 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.O_NOFOLLOW, thenfstaton the descriptor, and a regular file only.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 toCreateFileW(hos_fopen_write_handle,hos_fopen_kind).user/,bug-reports,prompts) is followed, as a linked game oruserfolder and OneDrive need; hard links are not told apart from the file. The README and the header say so.user/) instead.longshot_log.txtandlongshot_log.1.txt(which the player sends) only throughhos_fopen_read_nofollow(host/hos_fopen_write.h,copy_logingpu/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 nowlongshot_module_ids.bin,longshot_shaders.bin,image.bin, the prompt files the game reads back, andRX_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).longshot_env.txtsets only the game's ownLONGSHOT_switches and no path-valued switch at all. That includesRX_VK_DLSS_DIR, which picks the folder NGX loads DLSS from; the game'sngxfolder and%LOCALAPPDATA%\Longshot\ngx/$XDG_DATA_HOME/longshot/ngxneed no switch. It also takes no dump or SDL file-writing switch, no frame trace, notHOS_PGO, and not the control bridge (env_line_allowed,host/hos_main.c). The environment and a fileHOS_ENV_FILEnames still set all of them.hos_guest_range: audio, HID, applets, NVN's proc names,oot_menu.c).targets_ok,streams_ok).nx2/pipeline_states.py'scheck_recordsrefuses records with values no pipeline has before a list is added, merged or shipped.refuse_written_links, also atlongshot_options.iniand.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'slinks_only). The Builder andnup.pytake a reparse point as a link by the name-surrogate bit (link_tag), the game's rule, not by three fixed tags.deps.sh's checked download. The tools' Python packages are installed withpip --require-hashesfromrequirements-lock.txt, both the Linux AppImage's (deps.sh) and the Windows builder's (scripts/tools.py assemble_python).tests/test_workflow_hardening.pykeeps the workflows pinned, with no token left behind and no untrusted${{ }}in a script. It now reads.forgejo/workflowsand Forgejo's full-URL action names;third-party.yml'sgithub.base_refgoes throughenv:.tests/test_game_writes_no_links.pylooks through every source underhost/andgpu/vk/(nothost/posixor the vendored code) for opens that write. Each must go throughhos_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 plainfopenfails it.Fitting the branch to Longshot (the 4 commits after the port, and conflict notes in the ported ones):
pipeline_states.py: Longshot'sread_sentand its xz limit kept.check_recordsruns besidecheck_kinds, and it skips compute-program records (a key and zeros), whichcheck_kindsalready holds to exactly that.nup.py: Longshot'spackage_parts,is_linkandDestinationkept;is_linktakeslinks_only.test_workflow_hardening.py:release.ymlandwebsite.ymlhave no top-levelpermissions:(Forgejo 17 ignores the key), andpr-description.ymlandwebsite.ymlcheck out underpull_request_target(main's script only, and a fork's commit only after a maintainer'spreviewlabel). The check lists these as reviewed, with the reasons.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 rebasetest_udmabuf_warningandtest_upscaler_round_trip);test_pipeline_share's records are ones the game can make.Flags and settings
longshot_env.txtRX_VK_DLSS_DIRincluded. Also notRX_SCREENSHOT,RX_FLASH_LOG,HOS_SCAN_DT(dumps),RX_VK_FRAME_TRACE,HOS_PGO, SDL's file-writing switches,HOS_CONTROLorHOS_CONTROL_MEM. Of theLONGSHOT_names it takes only the game's own,LONGSHOT_NO_AV1_NOTICEandLONGSHOT_OCCLUSION_OFF. No switch is new; set in the environment, or in a fileHOS_ENV_FILEnames, each works as before.Measurements
main:
f87557ae(2026-11-13)this PR: security/hardening-passes @
b7a933daThe rows below are from
46ec4bd7on maind906c8f4.b7a933da(rebased ontof87557ae, 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:
d906c8f446ec4bd7python -I -B tests/run_all.py --jobs 4failures (Linux, this PC)test_lift_cache(no numpy here),test_menu_i18n(since #138)longshot_vk.dll(RX_TARGET=windows LONGSHOT_MINGW=<llvm-mingw> RX_VK_ONLY=1 nx2/build.py --standalone, on Linux)longshot_vk.soobjects (RX_VK_ONLY=1 nx2/build.py --standalone, Linux)lld, which this PC lacksTesting
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 onmainhere:test_lift_cache(no numpy on this PC) andtest_menu_i18n(pt_BR's text for a row #138 changed).tests/test_game_writes_no_links.pyfails on83e9a080(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.pyrefusesRX_VK_DLSS_DIRfrom the shared file for every value (/tmp/x,/dev/shm/x,C:\Users\Public\...,Z:\...,/proc/self/cwdand the rest), and takes it fromHOS_ENV_FILE's file and the environment.hos_fopen_write.hbuilt against llvm-mingw's real Windows headers, as C and as C++ (LONGSHOT_MINGW=... tests/test_game_writes_no_links.py). The Windows rendererlongshot_vk.dllbuilds 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-inwindows.h(on46ec4bd7;b7a933daadds 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-hashesofnx2/prompt_requirements.txtforwin_amd64from 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), soCreateFileW/_open_osfhandleon a real NTFS junction, symlink or OneDrive placeholder is untested. The two Windows requests wait for thetesting wantedlabel before anyone can claim them.Needs other testers:
longshot_options.iniand atlongshot_programs.bin, expecting the files they point at to be left alone and one log line each (T-AJTA)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.txtswitch that someone relied on ignored (RX_VK_DLSS_DIRthere now is: it goes in the environment, such as Steam's launch options). Each refusal says so inlongshot_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 onmain.Reviewer notes
Ported with
work/port_to_longshot.py(renameshookshottolongshot,.github/to.forgejo/), as #209 was. Commit messages are kept as written, so some still sayhookshot_env.txt. Where to look first:host/hos_fopen_write.h,host/hos_fopen_read.h,env_line_allowedinhost/hos_main.c,REVIEWEDintests/test_game_writes_no_links.py, andnx2/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.
REVIEWEDkeeps the save file system'sOpenFile(host/nn_fs.c, the game's saves inuser/, 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-inCreateFileA(host/posix) to takeFILE_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
fopenread, reviewed: the program cache's re-opens invk_progcache.cpp(of the file the game has just opened throughhos_fopen_write),own_handleinvk_programs.cpp(pipeline caches already read as regular files), the debugging reads behind switches a sharedlongshot_env.txtcannot 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 inuser/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 andtools.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
type(scope): description).main.python -I -B tests/run_all.pyrun and passing (any failure that also fails onmainis named above). (Not yet onb7a933da: last run on46ec4bd7; see Measurements.)third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.mainand passing here. (Added and updated forb7a933da,test_game_writes_no_links.pyandtest_game_folder_links.py, but not run yet.)tests/that reads changed source updated in this PR, not weakened.🤖 Generated with Claude Code
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.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.Read through the PR-only diff (dd02c6cc..83e9a080) and the code around it: the callers of the two new headers,
env_line_allowedwith 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
hos_fopen_writehas no race at the file's name:O_NOFOLLOW | O_NONBLOCK, thenfstaton the descriptor, thenftruncateonly once it knows the file is regular. The read side refuses FIFOs the same way. A symlinkeduser/(the saves move) and a symlinked game folder still work, because only the last part of the path is checked.env_line_allowedis a real improvement. Every*_DUMPswitch is blocked as a family, and so areHOS_CONTROL/HOS_CONTROL_MEM.LONGSHOT_names are narrowed to the game's two, andRX_VK_FSR_DIRand the other_DIR/_FILEnames are refused. The source scan intest_env_file.pywill catch most new path switches.links_onlykeeps a game folder that OneDrive manages working (its placeholders aren't links), whilerefuse_dump_linksstays strict.stand_in_damagednow copies into anmkstempfile and no longer writes to a fixed.partname.hos_guest_rangechecks are careful:nvnBootstrapLoadercaps a name at the end of guest memory, the wave buffers are checked once and that checked copy is the one submitted, andnpad_stateslimits its count.Must change
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 nameslanguage.txt. At83e9a080these still write through a link at their name:gpu/vk/vk_progcache.cpp:120,124,251,253,255,265:longshot_programs.binand.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.txtwhen the language row changes. Only the first write, innn_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.pymisses all of them becauseALLOWEDlists only six files. Convert these writes, and have the check scan every file underhost/andgpu/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.RX_VK_DLSS_DIRis still a library-load path that a shared file can set. It is the onelongshot_env.txtswitch left that names a path, and it decides where NGX loadsnvngx_dlss.dllorlibnvidia-ngx-dlss.sofrom. NGX takes the newest runtime it finds, so a planted copy wins over the game folder's./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.<game>/ngxand$XDG_DATA_HOME/longshot/ngx(%LOCALAPPDATA%\Longshot\ngx). The simplest fix is to refuse this switch from the shared file too, likeRX_VK_FSR_DIR;HOS_ENV_FILEstill sets it. If it stays, at least refuse a folder the player doesn't own or that others can write to (on Linux,statit: uid, and noS_IWGRP|S_IWOTH), and refuse non-fixed drives on Windows (GetDriveTypeW).The "Rebased on current
main" box is ticked but isn't true.dd02c6cc.mainis1ef93e79, four commits ahead (#54, #78, #147, #197). Three of them touch files this PR changes:host/hos_main.c,gpu/vk/vk_draw.cppandgpu/vk/vk_main.cpp.tests/run_all.py: #147'stest_dev_packs.pyand #197'stest_vk_occlusion_queries.pycut code from files this PR changes.Should change
hos_fopen_write(inline comment):IsReparseTagNameSurrogate(tag), not three fixed tags. That also coversWCI_LINKandGLOBAL_REPARSE, and still lets OneDrive's cloud tags through.CreateFileW(..., FILE_FLAG_OPEN_REPARSE_POINT), checkGetFileInformationByHandleEx(FileAttributeTagInfo)on the handle, and only then_open_osfhandle/_fdopen. That closes the window betweenGetFileAttributesAandfopen, the same way the Linux side has none. Low urgency: the threat here is files sitting in an archive, not a race.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 onmain. A regression in one part, such as a player's options not saving, would mean reverting all 2,086 lines.romfs,bin,prompts,ngx,ffxor.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 pointsromfsat 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.--require-hashescoversdeps.shonly. 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
GetNpadStatesfor 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, theThe comment names
language.txt, the pipeline caches and the markers, but some of the game's own writes still use a plainfopen.vk_menu_i18n.cpp:226writeslanguage.txt(the menu's write).vk_progcache.cppwriteslongshot_programs.bin/.idxwithwb,abandr+b. Also still plain:load_default.h(the markers),module_ids.h:83,vk_main.cpp:1090(the skipped marker) andpcache_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) {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 thefopenare two lookups of the name.CreateFileW(..., FILE_FLAG_OPEN_REPARSE_POINT)followed byGetFileInformationByHandleEx(FileAttributeTagInfo)and_open_osfhandlewould check the very file it then writes, the way the Linux side already does.@ -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 folderthe game was started from, and no .. step out of the folder the value names */if (!strcmp(name, "RX_VK_DLSS_DIR")) {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>/ngxand$XDG_DATA_HOME/longshot/ngx. I'd refuse it here likeRX_VK_FSR_DIRand leave it toHOS_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).@ -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 folderFOLDER_WRITES = ["bin", "image.bin", "longshot_version.txt", "longshot_options.ini", "language.txt", "longshot.png", "longshot_pipelines.bin",romfs,bin,prompts,ngxandffxare folders, so the Builder now stops when any of them is a link. A Steam Deck player who pointsromfsat 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,ALLOWEDlists six files, so this check doesn't see the plain writes invk_progcache.cpp,vk_menu_i18n.cpp,load_default.h,module_ids.h,pcache_stamp.h, or the write invk_main.cpp. Scan every file underhost/andgpu/vk/instead, and list the reviewed switch-gated writes, so a new cache file fails here.83e9a080aff7b79b8986f7b79b8986641bf324a8Thanks for the careful read. The branch is rebased on
main@d906c8f4(it had moved past1ef93e79), with 4 new commits on top; the head is46ec4bd7. (Edited:mainmoved during the work, so I rebased once more; the hashes here are the current ones.) Point by point:Must change
Done in
da41af87.vk_progcache.cpp(.bin/.idxwith"wb","ab", and"r+b"+_chsize_s, whichhos_fopen_writenow takes as an existing-file-only mode),vk_menu_i18n.cpp'slanguage.txt,load_default.h's marker and strike file,module_ids.h,vk_main.cpp's skipped marker andpcache_stamp.h's.tmp.longshot_log.txt(thefreopenonto stderr, nowhos_freopen_write), the bug report's copied logs andreport.txt, the program cache's.lock(O_NOFOLLOW), andRX_VK_PROBE's andRX_VK_DRAW_STATS' files, whose switches a sharedlongshot_env.txtcan set.language.txt) go throughhos_fopen_read*too, so a FIFO there isn't waited on.tests/test_game_writes_no_links.pyis inverted. It walks every source underhost/andgpu/vk/(nothost/posix,eden,sirit,compatorgpu/vk/tests), strips comments, and finds every open that writes:fopen/freopenwith a mode that writes,openwithO_WRONLY/O_RDWR/O_CREAT/O_APPEND,CreateFile*with write access,ofstream,CopyFile/copy_file."x",O_EXCL,O_NOFOLLOW,CREATE_NEW), or be inREVIEWED. EachREVIEWEDentry names its file, its call, its switch and a reason.REVIEWEDswitch is compiled throughenv_line_allowedand must be refused from the shared file. That's howHOS_PGOturned up; it's refused now, ina1463821.nn_fs.c's saveOpenFile, 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 LinuxCreateFileAstand-inFILE_FLAG_OPEN_REPARSE_POINT; I left it as a reviewed exception, open to a follow-up.83e9a080, the new check fails on all of the above.Done in
a1463821, the simple way you suggested. Its special case inenv_line_allowedis gone, so it falls under the_DIRrule. The environment (Steam launch options, a shell) and a fileHOS_ENV_FILEnames still set it, sinceenv_line_allowedonly applies to the shared file.test_env_file.pynow checks all of that:longshot_env.txt, including/tmp/x,/dev/shm/x,/proc/self/cwd,C:\Users\Public\ngxandZ:\ngx;HOS_ENV_FILE's file sets it;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.
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'sfreopenbecamehos_freopen_write.test_upscaler_round_trip.py: the options file is read withhos_fopen_read, and the fixture includes the helper.test_prepare_stall.py: the watchdog's skipped marker is written withhos_fopen_write.test_dev_packs.pyandtest_vk_occlusion_queries.pypass.tests/run_all.py --jobs 4: 508 passed, 2 failed, 30 skipped. Both failures also fail onmainon this PC:test_lift_cache(no numpy) andtest_menu_i18n(since #138). "One topic" is unticked, with a note.Should change
Done in
da41af87, both parts, with one change to the second:FILE_READ_ATTRIBUTESandFILE_FLAG_OPEN_REPARSE_POINTonly, and its tag is checked on that handle withIsReparseTagNameSurrogate.GetFileInformationByHandlegives the same volume and file index. After that comeSetEndOfFilefor"w", then_open_osfhandle(_O_APPEND/_O_TEXTas the mode says) and_fdopen.FILE_FLAG_OPEN_REPARSE_POINTwould skip hydration. The file-index match closes the window you pointed at all the same.CREATE_NEW, which fails if a link appears in between.MB_ERR_INVALID_CHARS, aspipeline_cache_file.hdoes.The stand-in-
windows.hfixture now checks that symlink, junction, LX,WCI_LINK_1andGLOBAL_REPARSEare 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. WithLONGSHOT_MINGWset, the check also builds the header as C and C++ against llvm-mingw's real headers. The whole Windowslongshot_vk.dllbuilds and links with llvm-mingw, without warnings. Nothing has run on Windows itself yet: T-AJTA asks for a real junction and symlink test.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:
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.Done in
0f1e28ed.FOLDERS_WRITTENlists the folders the Builder writes into:bin,romfs,prompts,ngx,ffx,.gameand.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 touser/, is fine.test_game_folder_links.pychecks the message and that every folder is named in the README.Done in
46ec4bd7, using the lock.assemble_pythonwrites the missing pins withrequirements-lock.txt's SHA-256s (they already include thewin_amd64wheels) 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 fornx2/prompt_requirements.txtagainst 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 plainfopen): 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):IsReparseTagNameSurrogateand 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(ALLOWEDlists six files): replaced by the full scan withREVIEWED,da41af87.nx2/make_game.py(FOLDER_WRITESstops a linkedromfs): README list and an error that names the folder,0f1e28ed.Your "not checked" items
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:
longshot_options.iniandlongshot_programs.bin.Neither can be claimed until a maintainer adds
testing wanted. A Linux build and play from a dump is still to request.641bf324a846ec4bd792Rebased once more onto main @
d906c8f4(eight PRs merged while I worked); head46ec4bd7, 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.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 addstesting wanted), and the CI jobs for this head are pending, not run.Fixed since the first review (
83e9a080)host/andgpu/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 plainfopenwrites found onlyREVIEWEDentries.RX_VK_DLSS_DIRis refused from the shared file.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.--require-hashes.Still open / new
copy_log(vk_bug_report.cpp:186) openslongshot_log.txt/longshot_log.1.txtfrom the game folder with a plainfopen. 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 (LinuxO_NOFOLLOW+fstat; Windows: the same look-handle test ashos_fopen_write_handle). On Linux a FIFO there would also hang the Insert menu.mainis 14 commits pastd906c8f4, 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.nup.pytest for links with a fixed tag list (LINK_TAGS: symlink, mount point, LX), while the game tests the surrogate bit.WCI_LINK,GLOBAL_REPARSEand any future name surrogate pass the Builder but not the game. Usetag & 0x20000000there too, so both sides agree; OneDrive's cloud tags still pass.FILE_FLAG_OPEN_REPARSE_POINTin 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).user/, the bug-report or prompts folders) is followed. That's right for linkeduser/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 orcp -alfolders, so I wouldn't change it. Just name it as out of scope.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.txtfiles 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 forRX_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");The destination is safe now, but the source two lines up (
fopen(from.c_str(), "rb"), line 186) still follows a link atlongshot_log.txt/longshot_log.1.txtin 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. ahos_fopen_read_nofollowbesidehos_fopen_read(LinuxO_NOFOLLOW|O_NONBLOCK+S_ISREG; Windows the look-handle test fromhos_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 &&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 withFILE_FLAG_OPEN_REPARSE_POINTin one step, since the flag is a no-op there, which leaves no second lookup to compare.@ -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 filesLINK_TAGS = (0xA000000C, 0xA0000003, 0xA000001D)The game side (
hos_fopen_write.h:62) refuses any name surrogate; this list has three tags, soIO_REPARSE_TAG_WCI_LINK_1andIO_REPARSE_TAG_GLOBAL_REPARSEpass the Builder but not the game.tag & 0x20000000(the surrogate bit) here and innup.py:401keeps both sides in step, and OneDrive's cloud tags still aren't surrogates.46ec4bd792b7a933da0dAnswering the second review (comment), on behalf of the reviewer's side. The branch is rebased onto
main@f87557aeand the new head isb7a933da: 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 openslongshot_log.txt/longshot_log.1.txtwith a newhos_fopen_read_nofollow(host/hos_fopen_write.h).O_NOFOLLOW | O_NONBLOCK, thenfstatand regular files only, then blocking again.hos_fopen_write_handlewithGENERIC_READ, existing files only).kLogCap).host/andgpu/vk/, now go throughhos_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) andRX_VK_DRAW_STATS' trigger file.tests/test_game_writes_no_links.pynow covers the new read on both platforms: the Linux fixture, the stand-in Windows fixture, acopy_logsource check and the read counts. The fixtures intest_prompts_live.pyandtest_seed_reader.pyget the helper.2. Rebase (must)
f87557ae, which includes #96. This PR adds no tested-configurations rows, so the frozen table doesn't affect it.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".d906c8f4line for line. So nothing of main is reverted.host/orgpu/vk/sinced906c8f4, including the 7 files you named and the newoot_state_tap.c. So the inverted write check still holds by reading.3. One link rule (should)
nx2/make_game.pyandnx2/nup.pyreplaceLINK_TAGSwithlink_tag(tag): the name-surrogate bit (0x20000000), the same rule asIsReparseTagNameSurrogatein the game.WCI_LINKandGLOBAL_REPARSEnow stop the Builder too, and OneDrive's tags still pass.test_game_folder_links.pyadds those tags and a check that the Builder,nup.pyand the header agree.4. File index on Windows (should)
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.FILE_READ_ATTRIBUTES.5. Out of scope
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:
run_allbox and the game-free test box are unticked until the checks run onb7a933da.Still needed:
python -I -B tests/run_all.pyonb7a933da(or CI).longshot_vk.dll.🤖 Generated with Claude Code
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, thenfstat, and onlyS_ISREGgets past before anything is read.O_NONBLOCKis cleared beforefdopen, and has no effect on a regular file anyway. The descriptor is closed on every error path,fdopenfailing 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 throughhos_fopen_write_handle(GENERIC_READ, existing=1), so nothing is created where nothing is. The handle is closed if_open_osfhandlefails, and_closeruns if_fdopenfails. The share modes are READ|WRITE. The game's own handle on the live log comes from the CRT'sfreopen, 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 withCP_UTF8andMB_ERR_INVALID_CHARS, sotests/test_utf8_paths.pyholds.FILE_FLAG_OPEN_REPARSE_POINT. A link put at the name in between is opened as the link itself and refused. A folder fails withoutBACKUP_SEMANTICS. No file index is compared, so redirectors and FUSE mounts can save again.FILE_READ_ATTRIBUTESis added to the access, andGENERIC_WRITEneeds it forFileAttributeTagInfo.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 bitIsReparseTagNameSurrogatetests, and the header's fallback macro matches it. On Linux,os.lstathas nost_reparse_tag, solink_tag(0)is False as0 in LINK_TAGSwas, and the symlink and junction tests are unchanged.LINK_TAGSis used nowhere else.hos_fopen_read_binary:module_ids.h,vk_shader.cpp(which includeshos_fopen_read.h),vk_draw.cpp,hos_main.c(its Linux branch only) andprompts.c. Each file includes the header, and each caller handles NULL asfopendid. The other plain reads left inhost/andgpu/vk/are system files (/proc,os-release,/sys,user-dirs.dirs), debug or control switches, orvk_progcache's re-open of a cache thathos_fopen_writehas just checked. None of them leaves the PC or makes the start wait.GENERIC_READ|FILE_READ_ATTRIBUTESand_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_logis checked to read through the helper only. The new counts inBINARY_READSandNOFOLLOW_READSmatch the sources. Thetest_prompts_livemacro andtest_seed_reader's-I hostfit how those tests build.test_bug_reportcompilesvk_bug_report.cppwhole, and its include path is relative.range-diffofd906c8f4..46ec4bd7againstf87557ae..1ee8f166shows 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 intests/test_menu_keys.py, wherevk_api.h(from main) andhos_fopen_read.hare both kept. Nothing of main is reverted.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 onb7a933da, withtest_game_writes_no_links.pyrun underLONGSHOT_MINGWso the header is built against Windows' real headers.FileAttributeTagInfoanswers there, since the game refuses a file when it doesn't, as it did before.@ -0,0 +77,4 @@BY_HANDLE_FILE_INFORMATION was, is;HANDLE h = INVALID_HANDLE_VALUE;int same = 0, refuse = kind < 0;if (kind == 0) {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
lookshowed no reparse tag. The old code always opened the file a second time without the flag, so OneDrive downloaded the file whateverlooksaw. 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 withFILE_FLAG_OPEN_REPARSE_POINTand never downloaded. Withwthat would hardly matter, butab(the lists and markers) andr+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 thatlongshot_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.Superseded by #218 (comment): the fix on
b7a933daresolves this review's points; what remains is testing (run_all/CI, builds, the Windows and Linux runs) and the split decision.Superseded by #218 (comment): the fix on
b7a933daresolves this review's points; what remains is testing (run_all/CI, builds, the Windows and Linux runs) and the split decision.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.