From bd20e7a47dd559cb59da0bb213579301cb3f167b Mon Sep 17 00:00:00 2001 From: Allan Thraen Date: Tue, 18 Aug 2026 14:32:54 +0200 Subject: [PATCH] fix(run): only ShellExecute http/https post-run URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PostRunUrl went straight to Process.Start with UseShellExecute=true and nothing validated it. ShellExecute resolves far more than web URLs — a local executable, a .ps1, a UNC path, or any registered protocol handler (ms-settings:, steam://) would all be launched. This fires automatically when a run exits 0, with no confirmation step. And ImportExportService deserializes an entire AppState — run commands included — from any JSON file the user points at, so an imported state file could carry a run command that launches a program on its first successful run. Import is a trust boundary either way (CommandLine is just as arbitrary), but the automatic trigger is worth closing. RunInstance.IsLaunchableUrl now gates the launch on an absolute http or https URI. Rejections are logged to crash.log alongside launch failures, since we can't pop UI from the PTY-exit callback thread. Scheme-less input like "localhost:5173" is rejected rather than silently upgraded to http:// — guessing would defeat the check. The dialog tooltip now says so explicitly. 19 new tests cover the accepted schemes, the rejected ones, and the empty/scheme-less cases. Suite is 241/241. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be --- CLAUDE.md | 2 +- src/CodeShellManager/Services/RunInstance.cs | 28 +++++++++-- .../Views/SessionRunCommandsDialog.xaml | 2 +- .../CodeShellManager.Tests/PostRunUrlTests.cs | 50 +++++++++++++++++++ 4 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 tests/CodeShellManager.Tests/PostRunUrlTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 7f16304..950f19a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -209,7 +209,7 @@ Each session can have a list of "run commands" — labelled command lines invoke **Data:** `ShellSession.RunCommands: List { Id, Label, CommandLine, IsDefault, Mode, PostRunUrl }`. Exactly one item has `IsDefault=true`; see `RunCommandItem.EnsureSingleDefault`. Persisted to `state.json`. - **`Mode`** (`RunMode.Process` default / `RunMode.PowerShell`) — `Process` runs through `cmd /c` as before; `PowerShell` wraps the command line in `pwsh.exe -NonInteractive -NoLogo -ExecutionPolicy Bypass -EncodedCommand ` (falls back to `powershell.exe` if `pwsh` isn't on PATH). SSH parents ignore `Mode` — remote runs always go through bash. Use PowerShell when the command relies on pipes (`|`), redirection (`>`), `$env:` variables, or cmdlets. -- **`PostRunUrl`** (`string?`, default `null`) — when set and the run exits with code 0, `Process.Start` opens the URL via `UseShellExecute=true` (default browser). Failures are swallowed; no health-check polling. +- **`PostRunUrl`** (`string?`, default `null`) — when set and the run exits with code 0, `Process.Start` opens the URL via `UseShellExecute=true` (default browser). No health-check polling. The value is gated by `RunInstance.IsLaunchableUrl` first: **only absolute `http`/`https` URLs are launched.** ShellExecute would otherwise run a local exe, a `.ps1`, a UNC path or any registered protocol handler, and this fires automatically with no confirmation — and `ImportExportService` deserializes a whole `AppState` (run commands included) from any JSON file the user points at, so the stored value is not trusted. Rejections and launch failures both append to `crash.log`; neither pops UI, since this runs on the PTY-exit callback thread. Scheme-less input (`localhost:5173`) is rejected rather than guessed at. **Templates:** `RunCommandTemplatesService.SeedFor(folder)` detects project type (top-level scan, first-match: dotnet → cargo → node → python → make) and returns a seed list with fresh Ids. Templates are *copied* onto new sessions at creation time; subsequent edits don't propagate back. SSH sessions skip detection (empty list). diff --git a/src/CodeShellManager/Services/RunInstance.cs b/src/CodeShellManager/Services/RunInstance.cs index f2dbe12..5ad9133 100644 --- a/src/CodeShellManager/Services/RunInstance.cs +++ b/src/CodeShellManager/Services/RunInstance.cs @@ -151,12 +151,34 @@ private void OnPtyExited() // so failures are logged to crash.log for diagnosability rather than silenced. if (State == RunState.ExitedOk && !string.IsNullOrWhiteSpace(PostRunUrl)) { + if (!IsLaunchableUrl(PostRunUrl)) + { + LogPostRunUrl(PostRunUrl, "rejected — only http and https URLs are opened"); + return; + } try { Process.Start(new ProcessStartInfo(PostRunUrl) { UseShellExecute = true }); } - catch (Exception ex) { LogPostRunUrlFailure(PostRunUrl, ex); } + catch (Exception ex) { LogPostRunUrl(PostRunUrl, ex.Message); } } } - private static void LogPostRunUrlFailure(string url, Exception ex) + /// + /// True when is safe to hand to ShellExecute — an absolute + /// http or https URL, and nothing else. + /// + /// This fires automatically when a run exits 0, with no confirmation step, and + /// ShellExecute will happily launch a local executable, a .ps1, a UNC path or any + /// registered protocol handler. A whole AppState — run commands included — can be + /// imported from a JSON file the user didn't write (see ImportExportService), so the + /// scheme is checked at launch time rather than trusting the stored value. + /// + /// Scheme-less input like "localhost:5173" is rejected too: Uri parses it as scheme + /// "localhost", and guessing http:// on the user's behalf would defeat the check. + /// + internal static bool IsLaunchableUrl(string? url) => + Uri.TryCreate(url, UriKind.Absolute, out Uri? uri) && + (uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps); + + private static void LogPostRunUrl(string url, string detail) { try { @@ -165,7 +187,7 @@ private static void LogPostRunUrlFailure(string url, Exception ex) "CodeShellManager", "crash.log"); Directory.CreateDirectory(Path.GetDirectoryName(path)!); File.AppendAllText(path, - $"[{DateTime.Now:HH:mm:ss.fff}] PostRunUrl failed '{url}': {ex.Message}\n"); + $"[{DateTime.Now:HH:mm:ss.fff}] PostRunUrl '{url}': {detail}\n"); } catch { /* logger failure is not actionable */ } } diff --git a/src/CodeShellManager/Views/SessionRunCommandsDialog.xaml b/src/CodeShellManager/Views/SessionRunCommandsDialog.xaml index 2f89971..496b253 100644 --- a/src/CodeShellManager/Views/SessionRunCommandsDialog.xaml +++ b/src/CodeShellManager/Views/SessionRunCommandsDialog.xaml @@ -106,7 +106,7 @@ Background="#313244" Foreground="#cdd6f4" BorderBrush="#45475a" CaretBrush="#cdd6f4" Padding="6,4" Margin="0,0,8,0" VerticalContentAlignment="Center" - ToolTip="Optional. e.g. http://localhost:5173 — opens in the default browser when the command exits 0."/> + ToolTip="Optional. Opens in the default browser when the command exits 0. Must be a full http:// or https:// URL — e.g. http://localhost:5173, not localhost:5173."/>