🐛 fix symlinks handling across various resources and connections by slntopp · Pull Request #8874 · mondoohq/mql

slntopp · 2026-07-01T16:41:18Z

Summary

Fix file.permissions.isSymlink always returning false for symlinks across all connection types (local, SSH/SFTP, SSH+sudo/SCP, Docker)
Fix files.find(type: "link") returning empty results on macOS (BSD find lacks -xtype)
Fix permissions.string rendering d instead of l for symlinks pointing to directories

Root Cause

isSymlink: Every stat path followed symlinks before checking the file mode. os.Stat, sftp.Stat, and stat -L all resolve symlinks to their target, so ModeSymlink was never present in the returned mode bits. The file resource correctly checked stat.Mode & ModeSymlink (file.go:111), but the bit was never set.

files.find: PR #8729 fixed files.find(type: "link") by using -xtype l under find -L. This works on GNU find (Linux) but macOS ships BSD find, which does not support -xtype and fails with exit code 1.

permissions.string: The id() method checked isDirectory before isSymlink, so a symlink to a directory rendered as drwxr-xr-x instead of lrwxr-xr-x.

Fix

`isSymlink` -- lstat-primary approach

All stat code paths now call lstat first, then stat only when a symlink is detected:

Connection type	Before	After
Local (`local.go`)	`afero.Stat` (follows symlinks)	`LstatIfPossible` first; if symlink, `Stat` for target metadata, then OR in `ModeSymlink`
SSH/SFTP (`ssh.go`)	`sftp.Stat` (SSH_FXP_STAT, follows)	`sftp.Lstat` (SSH_FXP_LSTAT) first; same two-step for symlinks
SSH+sudo / SCP / Docker (`statutil/stat.go`)	Separate `test -e` + `stat -L` (two round-trips, no symlink info)	Single compound command per platform that detects symlinks and stats in one round-trip

Compound command examples:

Linux: SL=0; test -L <path> && SL=1; test -e <path> -o $SL -eq 1 || exit 1; stat -L <path> -c "$SL.%s.%f.%u.%g.%X.%Y.%C" (falls back to stat without -L for dangling symlinks)
BSD/macOS: SL=0; test -L <path> && SL=1; stat -L -f "$SL:%z:%p:%u:%g:%a:%m" <path>
AIX: Perl's -l operator prepended to existing perl stat script

For non-symlinks, lstat and stat return identical results (single syscall, no extra cost). For symlinks, the returned mode carries ModeSymlink while size/permissions/ownership reflect the target -- except for dangling symlinks, which return lstat metadata.

`files.find(type: "link")` -- portable `find -H`

Replaced find -L ... -xtype l with find -H ... -type l:

-H follows only command-line symlinks (resolving paths like /tmp -> /private/tmp) but not symlinks found during traversal
-type l then correctly matches discovered symlinks
Works on both GNU find and BSD find
Other type searches (file, directory, etc.) continue using find -L unchanged

`permissions.string` -- symlink takes priority

Swapped the check order in file.go:id() so isSymlink is tested before isDirectory. A symlink to a directory now correctly renders as lrwxr-xr-x.

Test plan

mondoo-code-review

Symlink detection works but breaks under sudo and on BSD with dangling symlinks.

mondoo-code-review · 2026-07-01T16:43:40Z

+	sb.WriteString("SL=0; test -L ")
+	sb.WriteString(path)
+	sb.WriteString(" && SL=1; test -e ")
 	sb.WriteString(path)
-	sb.WriteString(" -c '%s.%f.%u.%g.%X.%Y.%C'")
+	sb.WriteString(` -o $SL -eq 1 || exit 1; stat -L `)
+	sb.WriteString(path)
+	sb.WriteString(` -c "$SL.%s.%f.%u.%g.%X.%Y.%C" 2>/dev/null || stat `)
+	sb.WriteString(path)
+	sb.WriteString(` -c "$SL.%s.%f.%u.%g.%X.%Y.%C"`)


🔴 critical — The compound command breaks under sudo. BuildSudoCommand prepends sudo to the whole string, producing sudo SL=0; test -L path && SL=1; stat .... The semicolons cause the shell to run sudo SL=0 (a no-op variable assignment under root), then test -L, stat, etc. without elevation. The original code ran two separate commands, each individually prefixed with sudo.

Fix: wrap the compound body so sudo applies to all of it:

sb.WriteString("sh -c 'SL=0; test -L ") // ... rest of compound command ... sb.WriteString("'")

This way BuildSudoCommand produces sudo sh -c '...', executing everything elevated. Alternatively, pass sudo into the builder and prefix each sub-command.

mondoo-code-review · 2026-07-01T16:43:40Z

+	sb.WriteString("SL=0; test -L ")
+	sb.WriteString(path)
+	sb.WriteString(` && SL=1; stat -L -f "$SL:%z:%p:%u:%g:%a:%m" `)
 	sb.WriteString(path)


🟡 warning — The unix (BSD/macOS) path has no fallback for dangling symlinks. stat -L will fail when the target doesn't exist, but unlike the linux path (line 95-97) there is no || stat path ... fallback without -L. A dangling symlink on BSD will return os.ErrNotExist instead of reporting the symlink's own metadata.

Add the same fallback pattern used in linux():

sb.WriteString(` && SL=1; stat -L -f "$SL:%z:%p:%u:%g:%a:%m" `) sb.WriteString(path) sb.WriteString(` 2>/dev/null || stat -f "$SL:%z:%p:%u:%g:%a:%m" `) sb.WriteString(path)

mondoo-code-review · 2026-07-01T16:43:40Z

 	//11 blksize  preferred block size for file system I/O
 	//12 blocks   actual number of blocks allocated
-	script := `perl -e '@a = stat(shift) or exit 2; $u = getpwuid($a[4]); $g = getgrgid($a[5]); printf("0%o:%s:%d:%s:%d:%d:%d", $a[2], $u, $a[4], $g, $a[5], $a[7], $a[9])'`
+	script := `perl -e '$p = shift; $l = -l $p ? 1 : 0; @a = stat($p) or exit 2; $u = getpwuid($a[4]); $g = getgrgid($a[5]); printf("%d:0%o:%s:%d:%s:%d:%d:%d", $l, $a[2], $u, $a[4], $g, $a[5], $a[7], $a[9])'`


🟡 warning — AIX: Perl's stat() follows symlinks (like stat -L), so for a dangling symlink @a = stat($p) will fail and the script exits with code 2, losing the symlink metadata. The linux/unix paths fall back to non-dereferencing stat for dangling links; AIX should use lstat($p) as a fallback:

$l = -l $p ? 1 : 0; @a = stat($p); @a = lstat($p) if !@a && $l; ...

mondoo-code-review · 2026-07-01T16:43:40Z

+	// -H follows only command-line symlinks (resolving the start path)
+	// while keeping -type l functional for discovered symlinks.
+	if isLinkSearch {
+		call.WriteString("find -H ")
+	} else {
+		call.WriteString("find -L ")
+	}


🟡 warning — Switching from -L to -H for link searches fixes -type l, but it also changes traversal semantics: -H does not follow symlinks to directories encountered during the walk, only the starting path. If a user has /home/user/link-to-dir -> /data/ and runs files.find(from: "/home/user", type: "link"), the tool will still descend into link-to-dir only if it's the starting path — but intermediate symlink-dirs won't be followed. This is a behavioral change vs. the old -L mode (which followed all symlinks). Consider documenting this trade-off or using -L with a post-filter on lstat to preserve traversal behavior while still detecting symlinks.

github-actions · 2026-07-01T16:57:36Z

Test Results

7 725 tests - 3 503 7 719 ✅ - 3 502 4m 23s ⏱️ +51s
305 suites - 241 5 💤 - 2
4 files - 36 1 ❌ + 1

For more details on these failures, see this check.

Results for commit 975bd8a. ± Comparison against base commit ae96214.

This pull request removes 3503 tests.

go.mondoo.com/mql/v13/providers/activedirectory/connection ‑ TestBuildSMB1NegotiateRequest
go.mondoo.com/mql/v13/providers/activedirectory/connection ‑ TestBuildSMB2NegotiateRequest
go.mondoo.com/mql/v13/providers/activedirectory/connection ‑ TestClassifyNamingContexts
go.mondoo.com/mql/v13/providers/activedirectory/connection ‑ TestDecodeSID
go.mondoo.com/mql/v13/providers/activedirectory/connection ‑ TestDecodeSID/BUILTIN_Administrators_S-1-5-32-544
go.mondoo.com/mql/v13/providers/activedirectory/connection ‑ TestDecodeSID/domain_SID_S-1-5-21-3623811015-3361044348-30300820
go.mondoo.com/mql/v13/providers/activedirectory/connection ‑ TestDecodeSID/too_short
go.mondoo.com/mql/v13/providers/activedirectory/connection ‑ TestDecodeSID/truncated_sub-authorities
go.mondoo.com/mql/v13/providers/activedirectory/connection ‑ TestDecodeSID/well-known_Everyone_SID_S-1-1-0
go.mondoo.com/mql/v13/providers/activedirectory/connection ‑ TestDialectString
…

slntopp added 3 commits July 1, 2026 18:39

🐛 detect symlinks via lstat-primary stat across all connection types

5bf2b0e

🐛 use find -H for portable symlink search on macOS

375649b

🐛 render symlink prefix 'l' over directory 'd' in permissions string

975bd8a

mondoo-code-review Bot requested changes Jul 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

🐛 fix symlinks handling across various resources and connections#8874

🐛 fix symlinks handling across various resources and connections#8874
slntopp wants to merge 3 commits into
mainfrom
mik/fix-find-symlinks

slntopp commented Jul 1, 2026

Uh oh!

mondoo-code-review Bot left a comment

Uh oh!

mondoo-code-review Bot Jul 1, 2026

Uh oh!

mondoo-code-review Bot Jul 1, 2026

Uh oh!

mondoo-code-review Bot Jul 1, 2026

Uh oh!

mondoo-code-review Bot Jul 1, 2026

Uh oh!

github-actions Bot commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

slntopp commented Jul 1, 2026

Summary

Root Cause

Fix

isSymlink -- lstat-primary approach

files.find(type: "link") -- portable find -H

permissions.string -- symlink takes priority

Test plan

Uh oh!

mondoo-code-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mondoo-code-review Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

mondoo-code-review Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

mondoo-code-review Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

mondoo-code-review Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 1, 2026

Test Results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

`isSymlink` -- lstat-primary approach

`files.find(type: "link")` -- portable `find -H`

`permissions.string` -- symlink takes priority