Skip to content

test(libkernel): drop backup privileges in denied parent watch - #6599

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
MarcZX10:codex/kqueue-elevated-token-privileges-6597
Oct 17, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
MarcZX10:codex/kqueue-elevated-token-privileges-6597

Conversation

@MarcZX10

Copy link
Copy Markdown
Contributor

What

Remove SeBackupPrivilege and SeRestorePrivilege from the process token before DeniedParentWatch changes the parent directory DACL. Elevated Windows processes can otherwise bypass that DACL and abort the test instead of exercising the expected denied-open path. The privilege adjustment succeeds without changing non-elevated tokens that do not hold these privileges.

Fixes #6597. The related open PR #6173 changes the ACL trustee from a username to the process token SID; this change handles the separate backup-privilege bypass in elevated processes.

Tested

  • Linux: not run. Full configuration is blocked because the SDL2 submodule is uninitialized in this worktree and SDL reports missing XRandR support. The relinker-only configuration does not include guest_kqueue.
  • MinGW-w64 GCC 12: GuestKqueue.cpp compiled with -std=c++20 -D_WIN32 -Wall -Wextra -Werror.
  • Windows elevated run: not run locally. The issue reports failure before the change and 3/3 passes after it on Windows 11 26200 with MinGW-w64 GCC 15.2.0 r7.
  • python3 tools/check_conventions.py --base upstream/main: passed.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

An elevated Windows process can bypass the parent directory DACL while holding SeBackupPrivilege or SeRestorePrivilege. Remove both privileges before DeniedParentWatch so the test exercises the denied-open path reliably.

Co-Authored-By: Adam Franke <10361523+frankea@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 6044 core/libs/tests/GuestKqueue.cpp
PR 6343 core/libs/tests/GuestKqueue.cpp
PR 6510 core/libs/tests/GuestKqueue.cpp

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Manual overlap check for the bot warning: #6044 does not change GuestKqueue.cpp (its path list contains only test/build organization files). #6343 adds a StandardInput case near the file end and calls it at the start of main; #6510 adds kqueue flag tests and their calls before the existing file-watch cases. Neither changes DeniedParentWatch or the point where it runs. This PR’s privilege helpers are called only from DeniedParentWatch, so there is no dependency or overlapping behavior.

@harmsluis

Copy link
Copy Markdown

Tested the non-elevated Windows path (the ERROR_NOT_ALL_ASSIGNED case not run in #6597).

Windows 11, MinGW-w64 GCC 15.2.0 (WinLibs r7), Release, BUILD_TESTING=ON. The process is not elevated and its token holds neither SeBackupPrivilege nor SeRestorePrivilege (whoami /priv).

  • main 5d9dcc4: ctest -R '^guest_kqueue$' --repeat until-fail:3 passes 3 of 3.
  • This PR (083f1cc): GuestKqueue.cpp builds without warnings; same command passes 3 of 3.

Not run: elevated (covered in #6597), Linux (the change is inside #ifdef _WIN32).

AI-assisted: yes.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks @harmsluis for the independent Windows check. I’ve recorded the additional scope: on Windows 11 with WinLibs MinGW-w64 GCC 15.2.0 Release, the non-elevated guest_kqueue test passes 3/3 on both current main and this PR; the changed test file also builds without warnings. Elevated execution remains supported by the original 3/3 report in #6597, not a local run from this PR environment. Linux and the full suite remain untested here.

@PYTHONRETARDE

Copy link
Copy Markdown
Contributor

Non-elevated Windows data point (Windows 11 Pro 26200, WinLibs GCC 15.2.0 r7, Release), the case the issue lists as not run. On this machine the user name equals the computer name, so DeniedParentWatch also needs #6173.

Build guest_kqueue
main 03c4cb5e7 + #6452 + this PR (083f1cc9d) 3/3 fail: the vnode watch error lacks "parent" (#6173's cause, not this PR's)
the same + #6173 (c494f3352, merges cleanly) 3/3 pass

So the ERROR_NOT_ALL_ASSIGNED path works without elevation, and the two changes combine.

AI-assisted: yes (Claude Code).

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for testing the non-elevated ERROR_NOT_ALL_ASSIGNED path and for isolating the "parent" message failure to #6173. I have recorded the result: this PR passes when composed with #6173, while the standalone failure is attributable to that separate vnode-watch change.

@github-actions github-actions Bot added the fix error Closes an issue (Fixes #N) label Oct 17, 2026
@oneandonlydean oneandonlydean self-assigned this Oct 17, 2026
@oneandonlydean
oneandonlydean merged commit 9ab27e5 into boykopovar:main Oct 17, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix error Closes an issue (Fixes #N)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

guest_kqueue: DeniedParentWatch aborts in an elevated process on Windows since #5493; drop the backup privileges in the test, diff included

4 participants