[mfkeeq199458]

LogExpert opened whatever path it was given as a log. A Windows shortcut is a binary file, so opening one showed its header bytes as log lines instead of the log the user meant.

Add ShellLinkResolver, which parses the MS-SHLLINK format directly rather than going through IShellLink: it lives in LogExpert.Core, which carries no COM or Windows-only dependency. Candidates are tried most specific first - LocalBasePath, LocalBasePath + CommonPathSuffix, the network path, and RelativePath resolved against the shortcut's own directory. Unicode strings are preferred over their ANSI rendering, and chained shortcuts are followed up to eight levels.

Wire it in where the reader is fed:

• PersisterHelpers.FindFilenameForSettings resolves before the .lxp lookup, so a shortcut to a settings file behaves like that file, and a shortcut to a log inside one resolves through both.
• FileOperationService.AddFileTab reads from and detects .lxp on the target, so the settings file is found behind the shortcut.
• FileOperationService.AddMultiFileTab resolves the array before it reaches the reader; without this, only the last file of a multi-file tab would be resolved.

Unresolvable input - a broken shortcut, a missing target, a shortcut to a folder - is returned unchanged, so it fails exactly as it did before. The request keeps the original path, which leaves file history, the recently used list and the rest of the UI untouched, and paths without the .lnk extension never touch the disk.

Verified with real shortcuts, absolute, relative-only, ANSI and Unicode targets, broken links, folder links, chains and loops: dotnet build src/LogExpert.UI reports 0 warnings and 0 errors, LogExpert.Persister. Tests passes 105/105, LogExpert.UI.Tests 369/369.

[Hirogen]

Review

Thanks, opening a shortcut and getting the log behind it instead of its header bytes is a real improvement. Parsing the format directly, without COM, is also a good fit for LogExpert.Core. I looked at this one with security in mind, since we will be reading untrusted binary files and following paths an attacker can choose.

Execution: fine. The resolver only reads LocalBasePath, CommonPathSuffix, NetworkPath and RelativePath. It never reads Arguments, WorkingDir, IconLocation or ExtraData, and it never starts a process or uses COM. A target that is an .exe, .bat, .ps1 and so on is just read as text. A shortcut pointing to an .lxp adds nothing that opening that .lxp directly can't already do.

I'd still like the following changed before merging.

Security

1. Network targets are accessed without asking the user
FindExistingTarget calls File.Exists on every candidate, including NetworkPath and UNC values in LocalBasePath. A shortcut whose target is \\some-host\share\x.log therefore makes Windows try to sign in to that host, which sends the user's NetNTLM hash. With \\host@SSL\… the same thing goes over WebDAV/HTTPS and gets past firewalls that block port 445.

• Today, dropping a .lnk on LogExpert only reads the local file, so this is a new way to trigger the leak.
• Environment.ExpandEnvironmentVariables also lets the shortcut put %USERNAME% / %COMPUTERNAME% into the host or share name, so those values leak too.
• It also runs synchronously on the UI thread, and twice (see below). An unreachable host freezes the window until the SMB timeout.

Suggestion:

• Don't follow UNC or WebDAV targets, or only after the user confirms.
• Drop ExpandEnvironmentVariables. Real environment-variable targets are stored in EnvironmentVariableDataBlock, which isn't parsed anyway.

2. No size limit before File.ReadAllBytes
A large file named *.lnk is read fully into memory. At around 1–2 GB this throws an OutOfMemoryException, which isn't caught. Real shortcuts are a few KB. Please reject anything above a small limit (for example 1 MB) before reading, ideally through a FileStream.

3. Device and pipe paths
Please reject targets in the \\.\ and \\?\ namespaces. Tail-follow on a named pipe or device could block.

4. The real target is hidden from the user
History and the recently used list keep the .lnk path, while the content comes from somewhere else. Showing the resolved target, for example in the tooltip or as "shortcut → target", makes it clear which file is actually open.

Correctness

• StringData terminator. MS-SHLLINK §2.4 says StringData strings are not NULL-terminated. But TryReadCountedString skips 1 or 2 extra bytes after every string (position = Math.Min(position + (isUnicode ? 2 : 1), …)), and the comment above it has the spec the wrong way round. When HasName is set, i.e. the shortcut has a comment, RelativePath is read from the wrong offset. This stays hidden as long as LocalBasePath resolves, which is why manual testing didn't catch it.
• CommonNetworkRelativeLink. NetNameOffsetUnicode should only be read when NetNameOffset > 0x14. Also, the slice isn't bounded by CommonNetworkRelativeLinkSize.
• "Unresolvable input is returned unchanged". That isn't quite true for chains. With a broken second link, or a loop, Resolve returns an intermediate .lnk, not the original input.
• "The rest of the UI untouched". CreateLogWindowFromRequest calls FindFilenameForSettings, so LogWindow.FileName becomes the target. The title, tooltip, file watcher, reload and .lxp location all follow the target, and only GivenFileName stays as the .lnk. That behaviour is reasonable, but the description should say so.
• Shortcut to a .lxj session. AddFileTabs and LoadFilesWithOption check for .lxj on the unresolved name, so a shortcut to a session file is opened as a raw log.
• Resolved twice. AddFileTab resolves, then FindFilenameForSettings resolves again. Please resolve in one place, at the entry point. That also keeps PersisterHelpers about persistence only.
• Extra candidates. NetworkPath on its own is a share root and can never be a file. Combined with point 1 above, I'd drop the network candidates entirely, or keep them only behind a confirmation.

Tests

No tests were added; the 105/105 and 369/369 runs only show that nothing broke. A binary parser that reads untrusted input needs fixture-based tests. Exposing the parsing core as internal (bytes in, targets out) would allow that without touching the disk. Cases to cover:

• relative-only, ANSI and Unicode targets
• a shortcut with HasName (this would catch the StringData bug)
• broken links, chains and loops
• truncated or corrupt headers, and an oversized file
• a .lnk → .lxp case next to the existing FindFilenameForSettings tests in SessionFileValidatorTests

Smaller points

• When the resolver gives up, it does so silently and LogExpert falls back to showing raw .lnk bytes. A _logger.Debug/Warn would help with support.
• The comments are rather long for this codebase. The 20-line <remarks> block and the 3-line comments in FileOperationService / PersisterHelpers could be much shorter.