[mfkeeq199458]

Opening or dropping a shortcut (.lnk) now opens the log file it points at instead of the shortcut itself.

Resolution follows a chain of shortcuts to its final local target, bounded by MAX_SHORTCUT_HOPS = 3. A broken, cyclic, non-local or over-limit chain keeps the last resolvable path — the original shortcut when the hop cap is hit — so it still opens as before.

Safety: targets are normalized (environment variables expanded, relative paths resolved against the shortcut's directory) and only plain local drive paths are followed; UNC and device forms are rejected before any File.Exists probe. Mapped network drives are intentionally trusted, since the user created the mapping. A .lnk larger than 1 MiB is not resolved, but it is not skipped either: it opens as the file the user selected.

Review follow-up: shortcuts are now resolved once per entry point; a single dropped shortcut previously went through three resolutions (up to 9 COM loads and 3 cap warnings). ResolveLogFile still resolves once because the .lxj decision must run on the resolved name — removing it would open a shortcut-to-session as a raw log. LoadFilesWithOption no longer writes resolved names into the caller's array, but still sorts that array in place as documented. Because resolution moved to the entry points, multi-file order now follows shortcut names instead of target names. AddMultiFileTab's .lxj branch still delegates to AddFileTabs, which performs one extra extension-only Resolve on .lxj names (no COM load, no cap warning).

[Hirogen]

Thanks @mfkeeq199458 — nice feature, and good call to avoid IShellLink::Resolve entirely (no link tracking / network search) and to keep the target, not the .lnk, in file history. All entry paths (drop, command line, second-instance pipe, history, session restore, multi-file) go through it.

A few things before merging:

Security — UNC guard
target.StartsWith(@"\\") (ShortcutResolver.cs:95) only catches the canonical form. With SLGP_RAWPATH the target string comes straight from the .lnk bytes, so a hand-crafted link with //evil/share/x.log or \??\UNC\evil\share\x.log would get past the check and reach File.Exists, which is exactly the NetNTLM leak the comment describes. Could you normalise first (e.g. Path.GetFullPath) and then reject anything that isn't a rooted local drive path (Path.GetPathRoot is X:\, not UNC/device)? A test with forward slashes and with \\?\ / \\.\ prefixes would lock this in.

Use Vanara instead of hand-rolled COM
LogExpert.UI already references Vanara.PInvoke.Shell32, which provides ShellLink / IShellLinkW. Using it would drop the ~40 lines of [ComImport] declarations and the hand-ordered IPersistFile vtable, and match how the rest of the codebase does interop.

Path handling

• SLGP_RAWPATH leaves %VAR% targets unexpanded. Please call Environment.ExpandEnvironmentVariables (or drop RAWPATH).
• Relative targets should be resolved against the .lnk's directory, not the working directory.
• The 260-char buffer truncates long-path targets; consider a larger buffer.

Behaviour

• Shortcuts are resolved twice on the LoadFilesWithOption/AddFileTabs path (and again in AddFileTab), so a link-to-link is followed 2 hops there but 1 hop from history. Resolving in one place would make it consistent.
• Array.Sort runs before resolution (:281), so multi-file order follows shortcut names rather than target names. Resolve, then sort.
• When a shortcut can't be followed (UNC, broken, oversized) it currently opens the .lnk as binary garbage with only a log line. A short message to the user would be friendlier. Not blocking, but worth considering.

Small cleanups

• The "resolve then check .lxj" pattern is repeated three times in FileOperationService; a small helper would tidy it.
• COMException already derives from ExternalException in the catch filter.
• var at :79, a using System.Text; instead of the fully qualified StringBuilder, and the comments in FileOperationService that restate the code can go.
• Tests: CreateShortcut is duplicated across both test files, and the oversized test hardcodes 1024 * 1024 + 1. Consider exposing the limit as internal and reusing it. Please also release the WScript.Shell COM objects.

Thanks — the UNC check and Vanara are the two I'd like to see addressed; the rest is polish.

[Hirogen]

Thanks @mfkeeq199458 — big improvement. The guard is now string-only (expand → GetFullPath against the shortcut's directory → require an X:\ root), so nothing touches the network before the check. The tests for //host/share, \\?\UNC, \??\UNC, \\.\ and %LOGONSERVER% cover exactly what I was worried about. Vanara, env-var expansion, the relative-target handling, resolve-then-sort and the ResolveLogFile helper all look good, and the new test classes pass locally (85/85).

One functional thing left, plus some polish:

Hop count (please fix)
The commit message says "resolution is one hop", but a shortcut is still resolved several times along one call chain:

• drop or command line, single file: LoadFilesWithOption loop (:280) → ResolveLogFile (:349) → AddFileTab (:71) = 3×
• SingleFiles / AskUser: pre-resolve at :280 → ResolveLogFile → AddFileTab = 3×
• history and session restore: AddFileTab only = 1×

So a link-to-link chain is followed 1 to 3 hops depending on how the file was opened, and each step loads a COM object. The simplest fix is to make Resolve idempotent: if the target itself ends in .lnk, treat it as unfollowable and return the input. Then repeated calls are cheap no-ops (they short-circuit on the extension) and the hop count is always exactly one. Then please drop the redundant call sites. Resolving in ResolveLogFile and AddFileTab (plus AddMultiFileTab) should cover every path. A test with a shortcut pointing to another shortcut would lock this in.

Small things

• ShortcutResolver.cs:~86: please release the CShellLinkW in a finally (Marshal.FinalReleaseComObject), as TestShortcuts already does.
• FileOperationService.cs:71-75: the string.Equals check isn't needed. request = request with { FileName = ShortcutResolver.Resolve(request.FileName) }; is enough.
• LoadFilesWithOption now overwrites the caller's array with resolved paths, and LoadDroppedFiles relies on that implicitly. Fine if it stays, but it becomes moot once the extra resolves are gone.
• Comments: we keep them minimal in this repo. The class remarks, the per-constant doc comments, and the NetNTLM explanation, which appears twice, could shrink to a line or two. The 7-line test class summary can go.
• Test names: Sanitize_* → SanitizeLocalTarget_*, to match Resolve_*.
• The PR description says "oversized files are not followed", but the check is on the .lnk size, not the target. Please adjust the wording.

Mapped network drives: a target on a mapped drive (Z:\…) passes the guard, so File.Exists will contact that server. That's fine: the user mapped the drive themselves, so it's a trusted location. No change needed. A one-line comment at the root check saying this is intentional would help the next reader.

Once the hop count is consistent, I'm happy to merge. The rest is polish. Thanks!

[Hirogen]

Thanks @mfkeeq199458 — all the polish from last round landed: finally release, the string.Equals removal, trimmed comments, the SanitizeLocalTarget_* rename and the mapped-drive note. The guard is still solid too. Every intermediate hop goes through SanitizeLocalTarget before any File.Exists, so a chain can't reach the network either.

Chain following
This isn't what I suggested (one hop, idempotent), but I'm fine with it. Returning the original input when the cap is hit makes the result the same on every entry path, so history, session restore and drop now open the same file. Let's keep it.

Redundant call sites (please fix)
The other half of my last request is still open: "drop the redundant call sites". A single dropped file is still resolved three times: in the LoadFilesWithOption loop (:276), then in ResolveLogFile (:345), then in AddFileTab (:71). AddMultiFileTab (:209) is a fourth site. For a shortcut that can't be followed, or one that hits the cap, that's up to 9 COM loads and the cap warning logged three times. The comment at ShortcutResolver.cs:72-73 only exists to work around this. Please resolve once per entry point: AddFileTab and AddMultiFileTab cover history, session restore and multi-file, so LoadFilesWithOption and ResolveLogFile shouldn't need their own calls. Once those are gone, LoadFilesWithOption also no longer needs to overwrite the caller's array.

One ordering detail to check after that change: the Open File dialog's Ask → MultiFile path re-sorts openFileDialog.FileNames (the unresolved names), so multi-file order there follows shortcut names, not target names.

PR description
The description still says "oversized files are not followed" and still describes a single hop. The correction is only in the a4b40e2 commit message. Please move it into the PR description.

Optional polish

• ResolveOneHop, SanitizeLocalTarget, ResolveLogFile and the private test helper don't need XML doc comments. The comment at ShortcutResolver.cs:155 restates the GetFullPath call.
• The three new tests in FileOperationServiceTests.cs could use // Arrange / // Act / // Assert like the rest of the file.
• Temp-directory setup is duplicated across both test files; CreateTempDirectory could be shared via TestShortcuts.
• TestShortcuts.cs:17-21 keeps both object? shortcut and dynamic link for the same COM object.

With the redundant call sites gone and the description updated, this is good to merge from my side. Thanks!

[Hirogen]

Thanks @mfkeeq199458 — round 4 does what I asked. I traced every entry path (drop, command line, second-instance pipe, history, session restore, multi-file, and Open File SingleFiles/MultiFile/Ask): each one resolves a shortcut exactly once, at most 3 COM loads, and the cap warning is logged at most once. Every hop still goes through SanitizeLocalTarget before File.Exists. The .lxj reasoning for keeping the resolve in ResolveLogFile is right, and the new test covers it. Build adds no warnings and the 89 shortcut/service tests pass. The polish from last round landed too.

One thing before merge: multi-file sort order
Moving the resolve to the entry points undid the round-1 fix. LoadFilesWithOption (:276) sorts before anything is resolved, so multi-file order now follows shortcut names again. Thanks for saying so in the description. Still, I'd rather keep target-name order: sorting the resolved names inside AddResolvedMultiFileTab (after Select(ShortcutResolver.Resolve)) would give the same order on every path, Ask → MultiFile included.

Small things

• ShortcutResolverTests: please add // Arrange / // Act / // Assert like the rest of the test project.
• ShortcutResolver.cs: the XML docs on the internal class can go. A one-line summary on Resolve is fine if you want to keep one.
• FileOperationServiceTests.cs: please leave the existing CreateServiceForWindows doc and the three existing LoadDroppedFiles_* tests as they were. Using CreateTempDirectory in new tests only keeps the diff focused.
• The a4b40e2 commit message still says multi-file order "follows the target names". After the sort fix above, that'll be true again.

Optional, non-blocking

• A shortcut that's broken, cyclic or over the cap still opens the .lnk as binary with only a log line. A short message to the user ("Couldn't follow shortcut X, opening it as a file") would be friendlier. Happy to track it as a follow-up if you'd rather not do it here.
• STGM_READ and IPersistFile could come from Vanara too, since you already use it for ShellLink.

With the sort order fixed, this is good to merge from my side. Thanks!