fix(host): longshot_env.txt can't set HOS_CONTROL_PEER_SID #911
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/tooling
area/upscaling
ci: approved
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/macos
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
regression
risk
high
risk
low
risk
medium
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
ci: approved
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!911
Loading…
Reference in a new issue
No description provided.
Delete branch "dyst/Longshot:fix/env-file-peer-sid"
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?
Type: fix · Risk: low · Tested on: Windows 11, game-free checks; the host file built for Windows (llvm-mingw) and Linux (gcc) · Needs: none
Why
Another Windows user on the same PC could be let into the game's control pipe through a shared
longshot_env.txt.HOS_CONTROL_PEER_SID(#544) gives one more account read and write on the control bridge's pipe, enough to steer the game and, withHOS_CONTROL_MEM, read and write its memory.longshot_env.txtis the file players pass around, and it could set that switch, so a file from someone else could name their account. This keeps the switch to the environment the game is started with, where the Test Farm's agent sets it.How to reproduce
Seen on: Windows 11, any GPU (found reading the code; no report)
Settings and switches:
HOS_CONTROLset (the control bridge on);HOS_CONTROL_PEER_SIDnot set in the environmentSteps: put
HOS_CONTROL_PEER_SID=<another local user's SID>inlongshot_env.txtbeside the game and start itExpected: the line left out, as
longshot_env.txtleaves outHOS_ENV_FILEand the switches that write files; the pipe the game user's aloneActual: on main the line is taken, and the bridge logs "control: the pipe lets in HOS_CONTROL_PEER_SID's account too"
What changed
longshot_env.txtno longer setsHOS_CONTROL_PEER_SID: the line is left out and logged like the other refused ones. The switch still works from the environment the game starts with, and from a fileHOS_ENV_FILEnames (a test run's own file, which sets anything).env_line_allowedinhost/hos_main.crefusesHOS_CONTROL_PEER_SID, next toHOS_ENV_FILEandSDL_DYNAMIC_API; the comment aboveload_longshot_env's rules names it.tests/test_env_file.py: the player's file setsHOS_CONTROL_PEER_SID=...and the check wants it unset; then with the switch in the environment the game started with,load_longshot_envkeeps the environment's value.docs/control.md(Security,HOS_CONTROL_PEER_SID) anddocs/development.md(the bridge's paragraph): set it in the environment,longshot_env.txtleaves it out.Measurements
Verdict: passes: the player's file no longer sets
HOS_CONTROL_PEER_SIDand the environment's value is kept; the same check fails on main, where the file's line is taken. Nothing to time (a check of which lines the file sets).Hardware: Ryzen 9 3900X, 64 GB, Windows 11 (no game run: a game-free check)
Scene: none: game-free.
env_line_allowedandload_longshot_envcut out ofhost/hos_main.cand run with the check's filesMethod:
tests/test_env_file.py(Builder's llvm-mingw clang) on this branch, and the same check against main'shost/hos_main.c(main's file in a scratch tree with this branch's test)How to reproduce:
python -I -B tests/test_env_file.py --cc clang; for main, the same with main'shost/hos_main.cf637b42d0b156278HOS_CONTROL_PEER_SIDfrom the player'slongshot_env.txtHOS_CONTROL_PEER_SIDfrom the environmenttests/test_env_file.pyFlags and settings
One switch changes where it may be set:
HOS_CONTROL_PEER_SIDis no longer taken fromlongshot_env.txt.HOS_CONTROL_PEER_SIDHOS_ENV_FILE's file orlongshot_env.txtHOS_ENV_FILE's file onlylongshot_env.txtline for it is left out and loggedTesting
Tested: on this branch (
0b156278, on main @f637b42d), Windows with the Builder's llvm-mingw clang (6 at once): the 45 checks that read this diff (the tests that name a changed file,test_env_fileamong them), not the whole suite: 36 passed, 9 failed, each also failing on main:test_crash_dialog,test_oot_state_tap,test_vk_pgo,test_walk_keyin the full run on main @1310ecc6;test_driver_cache_folder,test_pipeline_tour,test_udmabuf_warning,test_vk_occlusion_querieson main @7bacf846;test_log_open_failureon main @f637b42d(run there alone: its fixture'sfcntlis undeclared with this compiler).tests/test_env_file.pyfails on main'shost/hos_main.cand passes here (Measurements).host/hos_main.ccompiled to an object for Windows (llvm-mingw clang,x86_64-w64-mingw32) and for Linux (gcc, Dockerpython:3.12, withhost/posix), with the runtime headers a lift makes (a64.h,a64_suyu.h,a64_fpx.h) copied from a local work folder: both clean.Not tested: in the game (the change is one refused name in the file's allow-list, which the check runs as it is); the whole game build on Linux.
Must be tested: None
Risks
What could break: someone who set
HOS_CONTROL_PEER_SIDinlongshot_env.txt(none known: the Test Farm's agent sets it in the environment) now has the pipe the game user's alone, with the line logged as left out; setting it in the environment restores it. With #62 (one list of the switches,host/switches.def): no textual conflict (the two merge cleanly: this change sits apart from the table #62 replaces), but #62 listsHOS_CONTROL_PEER_SIDasGAME; whichever merges second should mark itLOCALthere, one line, so the list says what the allow-list does.Turning it off: set the switch in the environment instead; reverting the commit brings the old behaviour back.
Compatibility: Nothing changes for a player: saves,
longshot_options.ini, the caches and the vk ABI (MKWVK_API_VERSION) are untouched; only alongshot_env.txtline for this switch is now left out.Issues
See #544 (the switch).
Checklist
Everyone
type(scope): description) of the status line's type.main.python -I -B tests/run_all.pypasses (any failure that also fails onmainis named).third_party/untouched (a new folder with aPROVENANCE.mdinstead).When it applies
Assisted-by:/Co-Authored-By:trailers.mainand passing here.tests/that reads changed source updated in this PR, not weakened.🤖 Generated with Claude Code
WIP: fix(host): longshot_env.txt can't set HOS_CONTROL_PEER_SIDto fix(host): longshot_env.txt can't set HOS_CONTROL_PEER_SIDView command line instructions
Checkout
From your project repository, check out a new branch and test the changes.