Fix delete and rename following symbolic links (Fixes #1099)

[p0dalirius]

Linked Issue

Closes #1099

Root Cause

hostPath does two things: it resolves a share-relative path to a host path through filepath.EvalSymlinks, and it checks the result lies under the share root. The resolution is what an operation on a file's contents needs — following a link inside the share to the file it names is correct for Open, Stat and ReadDir, and the containment check is what makes it safe.

The operations that act on a name rather than on contents used the same resolver. Remove, Rmdir and Rename therefore received the link's target and acted on that, so the entry the client named survived and a different one was destroyed or moved. The rename case is the worst of the three: it leaves the original link dangling and turns what was a link into a regular file at the new name.

Fix Description

Add hostPathNoFollow, which resolves and containment-checks the parent exactly as hostPath already does for a target that does not exist yet, and joins the final element unresolved. Remove, Rmdir and Rename use it for the names they act on; Rename uses it for both ends, so renaming onto an existing link replaces the link rather than writing through it.

Containment is unchanged. Every component but the last is still resolved and checked against the root, and a link can only be traversed through a component that exists — the final element is not traversed at all here, it is the object being operated on. The new test asserts this directly: Remove("escape/secret.txt"), where escape is a link out of the share, is still refused and the outside file is untouched.

Rmdir on a link to a directory now returns ErrNotDirectory, because os.Lstat reports a symbolic link rather than a directory. That matches POSIX, where rmdir on a symlink fails with ENOTDIR, and it is a refusal rather than a destructive action. Remove is what unlinks the link itself.

How Verified

• Tests: TestLocalFileSystemNameOperationsDoNotFollowSymlinks covers all three operations plus the cases that must not regress. Against the unpatched backend it fails on all three — "deleting the link deleted its target", "Rmdir() removed a directory through a symbolic link", "renaming the link moved its target away" — and passes against the patched one.
• Regression: go build ./..., go vet ./... and go test ./... are clean; 308 packages pass. TestFileServiceDeleteAndRename, TestFileServiceRefusesTraversal and TestLocalFileSystemRoundTrip continue to pass.

Test Coverage

Added: network/smb/smb_v10/server/file_service_test.go — TestLocalFileSystemNameOperationsDoNotFollowSymlinks, with six subtests: the three corrected operations, deletion of an ordinary file and directory, and a delete reaching through a link that leaves the share.

Modified: TestLocalFileSystemContainsSymlinkEscape — see below.

Scope of Change

• Files changed: network/smb/smb_v10/server/fs_local.go, network/smb/smb_v10/server/file_service_test.go
• Submodule pointer updated: no
• Behavioral changes outside the bug fix: one, described immediately below, arising directly from the fix.

Risk and Rollout

TestLocalFileSystemContainsSymlinkEscape asserted that Remove on a link pointing out of the share fails, and after this change it succeeds. That assertion is updated rather than preserved, and the reasoning should be reviewed rather than taken on trust.

The old behaviour was not a containment rule that had been chosen; it fell out of resolving the final element, which is the very thing this fix stops doing. It also conflated two different properties. Reaching through a link to something outside the share must be refused, and still is — Stat, Open and a delete of linkdir/secret.txt are all refused, and the test still asserts that. Deleting the link entry is a different matter: the entry belongs to this share, and unlinking it removes only that entry.

This was checked rather than assumed. With the fix, Remove("link.txt") and Remove("linkdir") on links pointing outside leave both the outside file and the outside directory intact — only the link entries go. The updated test asserts that the target is untouched after the link is removed.

The alternative was to keep refusing, which would leave an entry a client can list and can never delete. Blast radius is limited to shares that contain symbolic links; a share without them behaves identically.

Notes

SetAttr still resolves through hostPath, which is correct: it acts on the attributes of the file a path names, not on the name.