Skip to content

fix(libc): follow a Windows symbolic link in access - #9474

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
Gruzzberry:fix/access-symbolic-link
Oct 25, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
Gruzzberry:fix/access-symbolic-link

Conversation

@Gruzzberry

@Gruzzberry Gruzzberry commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

What

On Windows, access read the attributes and the security descriptor of a symbolic link itself, not of its target: a dangling link passed F_OK and R_OK, a link to a read-only file passed W_OK, and a link to itself passed F_OK. FreeBSD 11.4's kern_accessat looks the path up with FOLLOW, so the target decides: ENOENT for a missing target, ELOOP for a loop, and the target's permissions.

When the resolved path is a reparse point, access now opens it without FILE_FLAG_OPEN_REPARSE_POINT (so Windows follows the link), takes the target's path with GetFinalPathNameByHandleW, and checks that path as before. A failed open goes through the existing error mapping, with ERROR_CANT_RESOLVE_FILENAME as ELOOP (62), as the guest stat maps it. Paths that are not reparse points take the same calls as on main. Linux already follows links in the host access and is unchanged.

Tested

Windows 11 (10.0.26200), WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release, -DBUILD_TESTING=ON, on main d6eafa5 (rebased), Developer Mode on so links can be created.

Overlaps

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); no silent stubs added
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source: none added
  • Generic behaviour, not specific to one title
  • Depends on: none. Merge order: after feat(libc): support archive-backed stdio reads #7447 if both are ready; this PR then rebases and keeps both the archive lookup and the link following.
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

AI-assisted (Claude Code).

🤖 Generated with Claude Code

@MarcZX10

Copy link
Copy Markdown
Contributor

The access change also edits the same access_nid_postfix region as #7447, which adds archive-aware access lookup. These changes address separate cases, but both need to compose at that function and may not apply cleanly if either lands first. Please coordinate the merge order with #7447’s author and update Depends on if one PR must land before the other; after rebasing, verify both symlink-target access and archive-entry access still pass.

@Gruzzberry

Copy link
Copy Markdown
Contributor Author

Full suite with this PR merged together with #9442, #9469, #9474 and #9487, against main on the same Windows 11 machine (MinGW-w64 GCC 15.2.0 r7, Release): 1875 tests, the same results except agc_import_settle_first_use, which also skips on main when its copies finish before the import returns (5 of 5 runs).

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Windows symlink handling and tests cover the relevant semantics: following targets for F_OK/R_OK/W_OK, missing targets, loops, target permissions and link-only attributes; the non-link path is unchanged. The PR itself says #7447 also edits access_nid_postfix for archive paths, but no author-agreed order is recorded. Please coordinate with #7447’s author, document the order/Depends on relationship, and preserve both archive lookup and symlink following in the combined function. This branch and test report are based on a6dcea9, while upstream main is ab347d1. Please rebase and rerun guest_access, the listed filesystem callers, conventions and current overlap checks. Hosted product jobs are skipped; I did not run local tests.

@Gruzzberry
Gruzzberry force-pushed the fix/access-symbolic-link branch from 1959fc9 to 17b707d Compare October 22, 2026 06:43
@Gruzzberry

Copy link
Copy Markdown
Contributor Author

Rebased onto main d6eafa5. guest_access, guest_filesystem, guest_path_case, guest_path_name_length and guest_appcontent pass on the new head, the new checks fail on that main without the change, and check_conventions.py passes. Full suite on main d6eafa5 and on main with #9442, #9469, #9474, #9487 and #9496 merged together: 1890 tests each, identical results.

On #7447: I asked its author on #7447. The two compose, as its archive branch returns before the host checks this PR changes, so either order works; whichever lands second keeps both and reruns guest_access and the archive access checks. The description records this under Overlaps.

@Gruzzberry

Copy link
Copy Markdown
Contributor Author

Holding this as a draft, following the direction on #5239: the open queue is above 700 and no named title needs this change on current main. I will only mark it ready again with a current-main title ID, its Game bring-up discussion and the log line where it stops on this, or once the queue is below 700. Thanks for the reviews so far; the requested fixes are being made on the branch in the meantime.

access read the attributes and the security descriptor of the link
itself on Windows, so a dangling link passed F_OK and R_OK and a link to
a read-only file passed W_OK. A reparse point is now opened through its
target and the target's path is checked, as FreeBSD 11.4's kern_accessat
looks the path up with FOLLOW: a missing target fails with ENOENT and a
link loop with ELOOP.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Gruzzberry
Gruzzberry force-pushed the fix/access-symbolic-link branch from 17b707d to 3e96c13 Compare October 23, 2026 14:17
@Gruzzberry
Gruzzberry marked this pull request as ready for review October 23, 2026 14:18
@Gruzzberry

Copy link
Copy Markdown
Contributor Author

Rebased onto main 938812d and marked ready for review, as @MarcZX10 noted the #5239 hold does not need to apply here. On Windows (MinGW GCC 15.2.0): the focused tests pass, check_conventions.py passes, and the full suite (2029 tests) on main merged with #9442, #9469, #9474, #9487, #9496, #9661 and #9682 matches main itself, apart from parallel-run flakes that pass when run alone.

@github-actions

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 7447 core/libs/prx/libc/src/Filesystem.cpp

@adriadam10 adriadam10 self-assigned this Oct 25, 2026
@adriadam10
adriadam10 dismissed MarcZX10’s stale review October 25, 2026 14:44

Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.

@adriadam10
adriadam10 merged commit baabfbc into boykopovar:main Oct 25, 2026
17 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the access change against FreeBSD 11.4 kern_accessat (FOLLOW lookup): dangling link gives ENOENT, a self-link gives ELOOP (62, same mapping on Linux through HostErrno), target permissions decide R_OK/W_OK, and non-reparse paths take the same calls as before. The new guest_access cases fail without the change (the dangling link returned 0). The ordering with #7447 is documented and the branches compose; merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants