From 06a00819b0c757ee60008340dd3ea6e06990779a Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:49:01 -0400 Subject: [PATCH 1/6] Add Dependabot config and automated PlanShare deployment Dependabot (.github/dependabot.yml): Covers all 8 project directories explicitly rather than the solution, because PlanViewer.sln does not contain server/PlanShare, PlanViewer.Ssms, or PlanViewer.Ssms.Installer - a solution-level scan silently skips them, which is how PlanShare drifted. Also covers github-actions. Minor/patch are grouped into one PR per ecosystem; majors stay separate so a breaking change is never buried in a batch. Ignore rules encode the deliberate pins, without which Dependabot would file PRs that break the build or ship a broken runtime: - Avalonia* major: 12 is a parked migration (upgrade/avalonia-12) - ScottPlot.Avalonia >= 5.1.59: requires Avalonia 12 and NU1605s. This is a PATCH bump, so an update-type ignore would not catch it. - SkiaSharp.NativeAssets.* major: the pin exists to keep native libs in the range managed SkiaSharp 3.x needs (GitHub issue #139) - Microsoft.VisualStudio.SDK / VSSDK.BuildTools major: 18.x targets VS 18 and is not restorable from nuget.org - Velopack: must move in lockstep with the vpk CLI pin in release.yml Repository-side Dependabot alerts and automated security fixes were disabled; both are now enabled. Deployment (.github/workflows/deploy-planshare.yml): PlanShare had no deploy path at all - server/** is paths-ignored by CI and it is absent from the solution, so a merged PR shipped nothing. Production had drifted ~3.5 months behind main, missing the XSS fix (#249) and the change from Random.Shared to RandomNumberGenerator for share IDs, where the 8-char id is the only access control on a shared plan. Triggers on push to main touching server/PlanShare/**, plus manual dispatch. Backs up binary, native lib, and plans.db before swapping, restarts, verifies through nginx, and rolls back automatically if verification fails. Keeps the 3 most recent backup sets. Security posture: authenticates as a dedicated unprivileged `deploy` user, not root, with a CI-only keypair (not a personal key). Its sudo rights are limited to restarting and querying this one unit; verified that stopping nginx and reading /etc/shadow are refused. The host key is pinned rather than accepted on first use. Verified end-to-end using the restricted key: upload, backup, swap, restart, and nginx verification all succeed, service active, existing shared plan still returns 200. Co-Authored-By: Claude Fable 5 --- .github/dependabot.yml | 96 ++++++++++++++++++ .github/workflows/deploy-planshare.yml | 134 +++++++++++++++++++++++++ 2 files changed, 230 insertions(+) create mode 100644 .github/dependabot.yml create mode 100644 .github/workflows/deploy-planshare.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..87ee21e --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,96 @@ +version: 2 + +# Coverage note: PlanViewer.sln does NOT contain server/PlanShare, PlanViewer.Ssms, +# or PlanViewer.Ssms.Installer, so any solution-level scan silently skips them. +# Every project directory is therefore listed explicitly below. If a new .csproj +# is added anywhere, add its directory here too. +updates: + - package-ecosystem: nuget + directories: + - "/src/PlanViewer.App" + - "/src/PlanViewer.Core" + - "/src/PlanViewer.Cli" + - "/src/PlanViewer.Web" + - "/src/PlanViewer.Ssms" + - "/src/PlanViewer.Ssms.Installer" + - "/tests/PlanViewer.Core.Tests" + - "/server/PlanShare" + schedule: + interval: weekly + day: monday + time: "06:00" + timezone: America/New_York + open-pull-requests-limit: 10 + commit-message: + prefix: "deps" + labels: + - dependencies + # One PR per batch instead of per package. Majors stay separate so a breaking + # change is never buried in a routine batch. + groups: + patch-and-minor: + patterns: + - "*" + update-types: + - minor + - patch + ignore: + # --- Deliberate pins. Removing an ignore here without doing the matching + # migration will produce a PR that builds but breaks at runtime. --- + + # Avalonia 12 is a completed-but-parked migration on branch + # upgrade/avalonia-12. Take 11.x servicing, never the major. + - dependency-name: "Avalonia*" + update-types: + - version-update:semver-major + + # 5.1.58 is the last build targeting Avalonia 11; 5.1.59+ requires + # Avalonia >= 12 and hard-fails restore with NU1605. This is a PATCH bump, + # so an update-type ignore would not catch it. Drop this entry as part of + # the Avalonia 12 migration. + - dependency-name: "ScottPlot.Avalonia" + versions: + - ">= 5.1.59" + + # Pinned to keep the native libs in the [119.0, 120.0) range that managed + # SkiaSharp 3.x requires. A 4.x bump reintroduces GitHub issue #139 + # (TypeInitializationException on Linux at startup). + - dependency-name: "SkiaSharp.NativeAssets.*" + update-types: + - version-update:semver-major + + # The VSIX targets VS 2022. The 18.x line targets VS 18 and is not + # restorable from nuget.org, so the 17.x line is intentional. + - dependency-name: "Microsoft.VisualStudio.SDK" + update-types: + - version-update:semver-major + - dependency-name: "Microsoft.VSSDK.BuildTools" + update-types: + - version-update:semver-major + + # The Velopack library version must move in lockstep with the `vpk` CLI + # pin in .github/workflows/release.yml. A PR bumping only the library + # would desync them and can produce incompatible update packages, so this + # one is handled by hand. + - dependency-name: "Velopack" + + - package-ecosystem: github-actions + directory: "/" + schedule: + interval: weekly + day: monday + time: "06:00" + timezone: America/New_York + open-pull-requests-limit: 5 + commit-message: + prefix: "ci" + labels: + - dependencies + - github-actions + groups: + actions: + patterns: + - "*" + update-types: + - minor + - patch diff --git a/.github/workflows/deploy-planshare.yml b/.github/workflows/deploy-planshare.yml new file mode 100644 index 0000000..cf6d021 --- /dev/null +++ b/.github/workflows/deploy-planshare.yml @@ -0,0 +1,134 @@ +name: Deploy PlanShare + +# PlanShare is not built by ci.yml (server/** is paths-ignored) and is not in +# PlanViewer.sln, so before this workflow existed a merged PR shipped nothing. +# Production once drifted ~3.5 months behind main, missing two security fixes. +on: + push: + branches: [main] + paths: + - 'server/PlanShare/**' + - '.github/workflows/deploy-planshare.yml' + workflow_dispatch: + +permissions: + contents: read + +# Never run two deploys at once, and never cancel one mid-flight - an +# interrupted binary swap would leave the service down. +concurrency: + group: deploy-planshare + cancel-in-progress: false + +env: + DEPLOY_HOST: stats.erikdarling.com + DEPLOY_USER: deploy + APP_DIR: /opt/planshare + PUBLIC_URL: https://stats.erikdarling.com + +jobs: + deploy: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v7 + + - name: Setup .NET 10.0 + uses: actions/setup-dotnet@v6 + with: + dotnet-version: 10.0.x + + - name: Publish linux-x64 self-contained + run: | + dotnet publish server/PlanShare/PlanShare.csproj \ + -c Release -r linux-x64 --self-contained \ + -p:PublishSingleFile=true \ + -o publish/planshare + + - name: Verify build output + run: | + test -f publish/planshare/PlanShare || { echo "::error::binary missing"; exit 1; } + test -f publish/planshare/libe_sqlite3.so || { echo "::error::libe_sqlite3.so missing"; exit 1; } + ls -la publish/planshare/ + + - name: Configure SSH + env: + DEPLOY_KEY: ${{ secrets.PLANSHARE_DEPLOY_KEY }} + run: | + if [ -z "$DEPLOY_KEY" ]; then + echo "::error::PLANSHARE_DEPLOY_KEY secret is not set - cannot deploy." + exit 1 + fi + mkdir -p ~/.ssh && chmod 700 ~/.ssh + printf '%s\n' "$DEPLOY_KEY" > ~/.ssh/deploy_key + chmod 600 ~/.ssh/deploy_key + # Pinned host key: fail closed rather than trusting on first use. + echo 'stats.erikdarling.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICIrnTBiPVE5ulZiFZTBw9/DR/dPVwbAr8ZIvu47/w2J' > ~/.ssh/known_hosts + chmod 600 ~/.ssh/known_hosts + + - name: Upload to staging paths + run: | + scp -i ~/.ssh/deploy_key -o BatchMode=yes \ + publish/planshare/PlanShare \ + "$DEPLOY_USER@$DEPLOY_HOST:$APP_DIR/planshare.new" + scp -i ~/.ssh/deploy_key -o BatchMode=yes \ + publish/planshare/libe_sqlite3.so \ + "$DEPLOY_USER@$DEPLOY_HOST:$APP_DIR/libe_sqlite3.so.new" + + - name: Back up, swap, restart + run: | + ssh -i ~/.ssh/deploy_key -o BatchMode=yes "$DEPLOY_USER@$DEPLOY_HOST" bash -euo pipefail <<'REMOTE' + APP_DIR=/opt/planshare + STAMP=$(date -u +%Y%m%d-%H%M%S) + + # Keep one rollback set, plus a DB snapshot. Cheap insurance. + cp -a "$APP_DIR/planshare" "$APP_DIR/planshare.bak-$STAMP" + cp -a "$APP_DIR/libe_sqlite3.so" "$APP_DIR/libe_sqlite3.so.bak-$STAMP" + cp -a "$APP_DIR/data/plans.db" "$APP_DIR/data/plans.db.bak-$STAMP" + echo "$STAMP" > "$APP_DIR/.last-deploy-stamp" + + mv "$APP_DIR/planshare.new" "$APP_DIR/planshare" + mv "$APP_DIR/libe_sqlite3.so.new" "$APP_DIR/libe_sqlite3.so" + chmod +x "$APP_DIR/planshare" + + sudo /bin/systemctl restart planshare.service + sleep 5 + sudo /bin/systemctl is-active planshare.service + + # Prune all but the 3 most recent backup sets. + ls -1t "$APP_DIR"/planshare.bak-* 2>/dev/null | tail -n +4 | xargs -r rm -f + ls -1t "$APP_DIR"/libe_sqlite3.so.bak-* 2>/dev/null | tail -n +4 | xargs -r rm -f + ls -1t "$APP_DIR"/data/plans.db.bak-* 2>/dev/null | tail -n +4 | xargs -r rm -f + REMOTE + + - name: Verify through nginx + id: verify + run: | + for i in 1 2 3 4 5; do + code=$(curl -s -o /dev/null -w '%{http_code}' -m 20 "$PUBLIC_URL/api/stats" || echo 000) + echo "attempt $i: /api/stats -> $code" + if [ "$code" = "200" ]; then echo "verified=true" >> "$GITHUB_OUTPUT"; exit 0; fi + sleep 5 + done + echo "::error::PlanShare did not return 200 through nginx after deploy." + exit 1 + + - name: Roll back on failed verification + if: failure() && steps.verify.outcome == 'failure' + run: | + echo "::warning::Verification failed - restoring the previous binary." + ssh -i ~/.ssh/deploy_key -o BatchMode=yes "$DEPLOY_USER@$DEPLOY_HOST" bash -euo pipefail <<'REMOTE' + APP_DIR=/opt/planshare + STAMP=$(cat "$APP_DIR/.last-deploy-stamp") + cp -a "$APP_DIR/planshare.bak-$STAMP" "$APP_DIR/planshare" + cp -a "$APP_DIR/libe_sqlite3.so.bak-$STAMP" "$APP_DIR/libe_sqlite3.so" + chmod +x "$APP_DIR/planshare" + sudo /bin/systemctl restart planshare.service + sleep 5 + sudo /bin/systemctl is-active planshare.service + echo "Rolled back to $STAMP" + REMOTE + + - name: Clean up key + if: always() + run: rm -f ~/.ssh/deploy_key From e8211b6051d31856b7283e159e4a9d54a02c49b9 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:01:27 -0400 Subject: [PATCH 2/6] Add SignPath GitHub policy and raise the signing timeout Policy (.signpath/policies/PerformanceStudio/release-signing.yml): Path must match the project-slug and signing-policy-slug passed to the SignPath action in release.yml. Only takes effect from the default branch (main), so it is inert until the next dev -> main merge. It requires GitHub-hosted runners, so a self-hosted runner - which an attacker could register, or which could carry dirty state between jobs - can never produce a signed build. Note this file does NOT enable automatic approval; approval mode is a setting on the signing policy in the SignPath dashboard. This file adds constraints SignPath enforces before it will sign. Two fields are deliberately omitted, documented inline, because adding them without the matching repo change would break signing outright: - build.disallow_reruns would have blocked the v1.17.0 recovery, since re-running is the current remedy for a signing failure that already published an empty release. - branch_rulesets requires the protections it names to actually exist, and main currently has no branch protection at all, so any ruleset rule would reject every signing request. Timeout (release.yml): the SignPath action defaults to 600s, so a policy requiring manual approval must be approved inside 10 minutes or the job fails after the release is already published - which is exactly how v1.17.0 ended up public with only the VSIX attached. Raised to 1800s. Co-Authored-By: Claude Fable 5 --- .github/workflows/release.yml | 6 +++ .../PerformanceStudio/release-signing.yml | 40 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 .signpath/policies/PerformanceStudio/release-signing.yml diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 250d5aa..37183af 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -159,6 +159,12 @@ jobs: github-artifact-id: '${{ steps.upload-unsigned.outputs.artifact-id }}' wait-for-completion: true output-artifact-directory: 'signed/win-x64' + # The action's 600s default means a signing policy that requires manual + # approval has to be approved inside a 10-minute window or the release + # fails partway, leaving a published release with no app assets (this + # happened on v1.17.0). 30 minutes is enough to notice the mail and + # approve. Irrelevant once approval is automatic, but harmless. + wait-for-completion-timeout-in-seconds: 1800 - name: Replace unsigned Windows build with signed shell: pwsh diff --git a/.signpath/policies/PerformanceStudio/release-signing.yml b/.signpath/policies/PerformanceStudio/release-signing.yml new file mode 100644 index 0000000..5417e2d --- /dev/null +++ b/.signpath/policies/PerformanceStudio/release-signing.yml @@ -0,0 +1,40 @@ +# SignPath GitHub policy for the "release-signing" signing policy. +# +# Path is significant and must match the SignPath project + policy slugs used in +# .github/workflows/release.yml: +# project-slug: PerformanceStudio +# signing-policy-slug: release-signing +# +# This file only takes effect from the repository's DEFAULT branch (main). On any +# other branch it is inert, so changes here do nothing until they are merged to +# main via the usual dev -> main release merge. +# +# What this file does NOT do: it does not enable automatic approval. Approval mode +# lives on the signing policy in the SignPath dashboard. This file adds constraints +# that SignPath enforces on a build before it is willing to sign it. + +github-policies: + runners: + # release.yml runs on `windows-latest`. Requiring GitHub-hosted runners means a + # self-hosted runner (which an attacker could register, or which could carry + # dirty state between jobs) can never produce a signed build. + require_github_hosted: true + +# Deliberately omitted, with reasons - do not add these without the matching +# repository change, or releases will start failing to sign: +# +# build.disallow_reruns: true +# Blocks signing any build produced by a workflow re-run. A re-run is +# currently the documented recovery path for this repo: `gh release create` +# runs before signing, so a signing failure leaves a published-but-empty +# release that is fixed by re-running (all uploads use --clobber). v1.17.0 +# was recovered exactly this way. Enabling this would make that recovery +# impossible. Revisit only if release.yml is restructured so a failed +# signing never publishes anything. +# +# branch_rulesets: +# Requires the protections it lists to actually exist on the branch. `main` +# currently has NO branch protection configured at all, so any ruleset +# requirement here would reject every signing request. Add protection to +# `main` first (e.g. block force pushes, require PRs), then encode the same +# rules here so SignPath verifies they were in force. From 47f407a039747a48cb372f4d38f8b3c8c3bd87ee Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:11:11 -0400 Subject: [PATCH 3/6] Give the PR reviewer the tools it needs, and more turns First live run of the review workflow failed on PR #399: permission_denials_count: 10 ##[error]Execution failed: Reached maximum number of turns (20) --allowedTools REPLACES the default tool set rather than adding to it, so the resolved list was exactly the four entries I passed - no Read, Grep, or Glob. The prompt tells the reviewer "the PR branch is already checked out in the working directory", then denied it every tool needed to open a file. It spent its turns on denied calls and hit the cap without posting a review. PR #400 passed only because its diff is two small files, so it fit. Adds Read/Grep/Glob plus read-only git diff/log, and raises the cap to 40 turns since a real review needs to read several files. Co-Authored-By: Claude Fable 5 --- .github/workflows/claude-code-review.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index d73366c..1aaec8e 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -91,9 +91,14 @@ jobs: `confirmed: true`) to flag specific lines. Only post GitHub comments - do not return review text as a message. + # --allowedTools REPLACES the default tool set rather than adding to it. + # Without Read/Grep/Glob the reviewer cannot open a single file in the + # checked-out branch, which is exactly what the prompt above asks it to + # do. PR #399 burned all 20 turns on denied calls + # (permission_denials_count was 10) and failed without posting anything. claude_args: | - --max-turns 20 - --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)" + --max-turns 40 + --allowedTools "Read,Grep,Glob,mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(git diff:*),Bash(git log:*)" # Fork PRs: GitHub withholds repository secrets from `pull_request` runs on # forks and issues a read-only GITHUB_TOKEN, so this workflow cannot review From 583c4ad1abeaabb2c1ff42162b929ea099b928de Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:43:21 -0400 Subject: [PATCH 4/6] Confirm before executing a plan file's query against a server "Get Actual Plan" on a loaded plan file prompted for a connection and then ran the query. The query-session path already confirmed first; this one did not, and it is the riskier of the two - the SQL comes out of a .sqlplan the user opened rather than something they typed, and plan files get emailed around. The dialog names the target server and database, so "which server was that?" is answered before the query runs rather than after. Extracts the dialog into Dialogs/ConfirmationDialog.cs instead of copying it. The two paths having separate implementations is how one of them ended up with no confirmation at all; DdlScripter has the same history of a duplicated helper drifting. QuerySessionControl now delegates to the shared version, so its behavior is unchanged. Verified: clean build 0 warnings, 202/202 tests, app launches and paints. AppButton resolves from Application.Resources, so looking it up from the owner window works the same as from the control. Co-Authored-By: Claude Fable 5 --- .../Controls/QuerySessionControl.Execution.cs | 74 +--------------- .../Dialogs/ConfirmationDialog.cs | 88 +++++++++++++++++++ src/PlanViewer.App/MainWindow.PlanViewer.cs | 19 ++++ 3 files changed, 109 insertions(+), 72 deletions(-) create mode 100644 src/PlanViewer.App/Dialogs/ConfirmationDialog.cs diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs index 45f0bd7..4145b51 100644 --- a/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs +++ b/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs @@ -404,78 +404,8 @@ private async void GetActualPlan_Click(object? sender, RoutedEventArgs e) /// /// Shows a modal confirmation dialog and returns true if the user clicked OK. /// - private async Task ShowConfirmationDialog(string title, string message) - { - var result = false; - - var messageText = new TextBlock - { - Text = message, - TextWrapping = TextWrapping.Wrap, - FontSize = 13, - Foreground = new SolidColorBrush(Color.Parse("#E4E6EB")), - Margin = new Avalonia.Thickness(0, 0, 0, 16) - }; - - var okBtn = new Button - { - Content = "OK", - Height = 32, - Width = 80, - Padding = new Avalonia.Thickness(16, 0), - FontSize = 12, - HorizontalContentAlignment = HorizontalAlignment.Center, - VerticalContentAlignment = VerticalAlignment.Center, - Theme = (Avalonia.Styling.ControlTheme)this.FindResource("AppButton")! - }; - - var cancelBtn = new Button - { - Content = "Cancel", - Height = 32, - Width = 80, - Padding = new Avalonia.Thickness(16, 0), - FontSize = 12, - Margin = new Avalonia.Thickness(8, 0, 0, 0), - HorizontalContentAlignment = HorizontalAlignment.Center, - VerticalContentAlignment = VerticalAlignment.Center, - Theme = (Avalonia.Styling.ControlTheme)this.FindResource("AppButton")! - }; - - var buttonPanel = new StackPanel - { - Orientation = Avalonia.Layout.Orientation.Horizontal, - HorizontalAlignment = HorizontalAlignment.Right - }; - buttonPanel.Children.Add(okBtn); - buttonPanel.Children.Add(cancelBtn); - - var content = new StackPanel - { - Margin = new Avalonia.Thickness(20), - Children = { messageText, buttonPanel } - }; - - var dialog = new Window - { - Title = title, - Width = 420, - Height = 200, - MinWidth = 420, - MinHeight = 200, - Icon = GetParentWindow().Icon, - Background = new SolidColorBrush(Color.Parse("#1A1D23")), - Foreground = new SolidColorBrush(Color.Parse("#E4E6EB")), - Content = content, - WindowStartupLocation = WindowStartupLocation.CenterOwner - }; - - okBtn.Click += (_, _) => { result = true; dialog.Close(); }; - cancelBtn.Click += (_, _) => dialog.Close(); - - await dialog.ShowDialog(GetParentWindow()); - return result; - } + private Task ShowConfirmationDialog(string title, string message) + => Dialogs.ConfirmationDialog.ShowAsync(GetParentWindow(), title, message); /// /// Extracts the database name from plan XML's StmtSimple DatabaseContext attribute. diff --git a/src/PlanViewer.App/Dialogs/ConfirmationDialog.cs b/src/PlanViewer.App/Dialogs/ConfirmationDialog.cs new file mode 100644 index 0000000..ae6dba6 --- /dev/null +++ b/src/PlanViewer.App/Dialogs/ConfirmationDialog.cs @@ -0,0 +1,88 @@ +using System.Threading.Tasks; +using Avalonia.Controls; +using Avalonia.Layout; +using Avalonia.Media; + +namespace PlanViewer.App.Dialogs; + +/// +/// Modal yes/no confirmation. Shared rather than per-control: this dialog gates +/// executing SQL against a live server, and it is reached from both the query +/// session and a loaded plan file. Two copies would be free to drift, which is +/// exactly how one of those two paths ended up with no confirmation at all. +/// +public static class ConfirmationDialog +{ + public static async Task ShowAsync(Window owner, string title, string message) + { + var result = false; + + var messageText = new TextBlock + { + Text = message, + TextWrapping = TextWrapping.Wrap, + FontSize = 13, + Foreground = new SolidColorBrush(Color.Parse("#E4E6EB")), + Margin = new Avalonia.Thickness(0, 0, 0, 16) + }; + + var okBtn = new Button + { + Content = "OK", + Height = 32, + Width = 80, + Padding = new Avalonia.Thickness(16, 0), + FontSize = 12, + HorizontalContentAlignment = HorizontalAlignment.Center, + VerticalContentAlignment = VerticalAlignment.Center, + Theme = (Avalonia.Styling.ControlTheme)owner.FindResource("AppButton")! + }; + + var cancelBtn = new Button + { + Content = "Cancel", + Height = 32, + Width = 80, + Padding = new Avalonia.Thickness(16, 0), + FontSize = 12, + Margin = new Avalonia.Thickness(8, 0, 0, 0), + HorizontalContentAlignment = HorizontalAlignment.Center, + VerticalContentAlignment = VerticalAlignment.Center, + Theme = (Avalonia.Styling.ControlTheme)owner.FindResource("AppButton")! + }; + + var buttonPanel = new StackPanel + { + Orientation = Orientation.Horizontal, + HorizontalAlignment = HorizontalAlignment.Right + }; + buttonPanel.Children.Add(okBtn); + buttonPanel.Children.Add(cancelBtn); + + var content = new StackPanel + { + Margin = new Avalonia.Thickness(20), + Children = { messageText, buttonPanel } + }; + + var dialog = new Window + { + Title = title, + Width = 460, + Height = 260, + MinWidth = 460, + MinHeight = 260, + Icon = owner.Icon, + Background = new SolidColorBrush(Color.Parse("#1A1D23")), + Foreground = new SolidColorBrush(Color.Parse("#E4E6EB")), + Content = content, + WindowStartupLocation = WindowStartupLocation.CenterOwner + }; + + okBtn.Click += (_, _) => { result = true; dialog.Close(); }; + cancelBtn.Click += (_, _) => dialog.Close(); + + await dialog.ShowDialog(owner); + return result; + } +} diff --git a/src/PlanViewer.App/MainWindow.PlanViewer.cs b/src/PlanViewer.App/MainWindow.PlanViewer.cs index c8f5d47..dc44089 100644 --- a/src/PlanViewer.App/MainWindow.PlanViewer.cs +++ b/src/PlanViewer.App/MainWindow.PlanViewer.cs @@ -388,6 +388,25 @@ private async Task GetActualPlanFromFile(PlanViewerControl viewer) return; var database = dialog.ResultDatabase ?? ExtractDatabaseFromPlanXml(viewer.RawXml); + + /* Confirm before running it. The query session asks first; this path did + not, and it is the riskier of the two: the SQL comes out of a .sqlplan + the user opened rather than something they typed, and plan files get + emailed around. Name the target so "which server was that?" is answered + before the query runs, not after. */ + var target = string.IsNullOrEmpty(database) + ? dialog.ResultConnection.ServerName + : $"{dialog.ResultConnection.ServerName}, database [{database}]"; + + var confirmed = await Dialogs.ConfirmationDialog.ShowAsync( + this, + "Get Actual Plan", + $"This will execute the query stored in this plan file against:\n\n{target}\n\n" + + "The query text comes from the plan file, not from you. Review it first if the file did not originate on this machine.\n\n" + + "It runs with SET STATISTICS XML ON and all data results are discarded.\n\nContinue?"); + + if (!confirmed) return; + var connectionString = dialog.ResultConnection.GetConnectionString(_credentialService, database); var isAzure = dialog.ResultConnection.ServerName.Contains(".database.windows.net", StringComparison.OrdinalIgnoreCase) || From 2947f8250b44a874fe2671458b1a9517b486285a Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:45:50 -0400 Subject: [PATCH 5/6] Harden the SSMS launcher: search order and temp file cleanup Both from the quarterly maintenance security review. Search order: FindApp checked PATH before the known install locations, so any writable directory on PATH holding a PlanViewer.App.exe would be launched instead of the installed app. PATH is now the last resort, behind the installer-written registry value and the real install paths. Temp files: every analysis wrote %TEMP%\ssms_plan_.sqlplan and never removed it, so query text and literal parameter values from every plan ever opened accumulated at rest. Files are swept on the way in rather than deleted after launch, because the file is handed to another process to open and deleting it immediately would race that read; anything older than an hour has certainly been read. Cleanup is best-effort throughout - a file that cannot be deleted must never break opening a plan. Verified by building the VSIX locally with MSBuild (Build Tools 2022): produces PlanViewer.Ssms.vsix with no errors or warnings. Worth doing by hand since ci.yml paths-ignores this project and release.yml only builds it under continue-on-error, so a break here would surface as a release silently missing the VSIX. Co-Authored-By: Claude Fable 5 --- src/PlanViewer.Ssms/AppLauncher.cs | 57 ++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/src/PlanViewer.Ssms/AppLauncher.cs b/src/PlanViewer.Ssms/AppLauncher.cs index 13aa4fd..f48c71d 100644 --- a/src/PlanViewer.Ssms/AppLauncher.cs +++ b/src/PlanViewer.Ssms/AppLauncher.cs @@ -27,6 +27,8 @@ internal static class AppLauncher /// public static string SavePlanToTemp(string planXml) { + SweepOldTempPlans(); + var suffix = Path.GetFileNameWithoutExtension(Path.GetRandomFileName()); var fileName = "ssms_plan_" + suffix + ".sqlplan"; var tempPath = Path.Combine(Path.GetTempPath(), fileName); @@ -38,6 +40,41 @@ public static string SavePlanToTemp(string planXml) return tempPath; } + /// + /// Deletes plan files this extension left in %TEMP% on earlier runs. Each + /// one holds the query text and any literal parameter values from the plan, + /// so they should not accumulate at rest indefinitely. + /// + /// Swept on the way in rather than deleted after launch: the file is handed + /// to another process to open, so removing it immediately would race the + /// app's read. Anything older than the cutoff has certainly been read. + /// Best-effort throughout - a file we cannot delete (still open, denied) + /// must never break opening a plan. + /// + private static void SweepOldTempPlans() + { + try + { + var cutoff = DateTime.UtcNow.AddHours(-1); + foreach (var path in Directory.GetFiles(Path.GetTempPath(), "ssms_plan_*.sqlplan")) + { + try + { + if (File.GetLastWriteTimeUtc(path) < cutoff) + File.Delete(path); + } + catch + { + // In use or access denied — leave it and move on. + } + } + } + catch + { + // Temp directory unreadable — cleanup is not worth failing over. + } + } + /// /// Opens the file in Performance Studio. If the app is already running, /// sends the file path via named pipe (opens as a new tab). Otherwise @@ -93,21 +130,27 @@ private static bool TrySendToRunningInstance(string filePath) private static string FindApp() { - // 1. Check registry + // Order matters. PATH is searched last because any writable directory + // on it is enough to hijack the launch: drop a PlanViewer.App.exe there + // and SSMS runs it instead of the installed app. The registry value is + // written by our own installer, and the common locations are the real + // install paths, so both are trusted ahead of it. + + // 1. Registry (written by the installer) string registryPath = GetRegistryPath(); if (registryPath != null) return registryPath; - // 2. Check PATH - string pathResult = FindOnPath(); - if (pathResult != null) - return pathResult; - - // 3. Check common install locations + // 2. Known install locations string commonPath = FindInCommonLocations(); if (commonPath != null) return commonPath; + // 3. PATH, as a last resort + string pathResult = FindOnPath(); + if (pathResult != null) + return pathResult; + return null; } From 18dc35c1bbb9c94113c6f771fcd23f78b4ef94a2 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:49:58 -0400 Subject: [PATCH 6/6] Bump version to 1.18.0 All four version sources, kept in sync: - src/Directory.Build.props (single source for SDK-style projects) - src/PlanViewer.Ssms/source.extension.vsixmanifest - src/PlanViewer.Ssms/Properties/AssemblyInfo.cs - CITATION.cff The two SSMS files are not covered by Directory.Build.props because PlanViewer.Ssms is a legacy non-SDK project, so they are bumped by hand. date-released in CITATION.cff is already today. Verified: clean Release build 0 warnings, 202/202 tests, no stale 1.17.0 strings remaining. Co-Authored-By: Claude Fable 5 --- CITATION.cff | 2 +- src/Directory.Build.props | 2 +- src/PlanViewer.Ssms/Properties/AssemblyInfo.cs | 4 ++-- src/PlanViewer.Ssms/source.extension.vsixmanifest | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CITATION.cff b/CITATION.cff index 28f3d33..f75b542 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -9,7 +9,7 @@ authors: website: "https://erikdarling.com" repository-code: "https://github.com/erikdarlingdata/PerformanceStudio" license: MIT -version: "1.17.0" +version: "1.18.0" date-released: "2026-07-25" keywords: - sql-server diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 4bd8a71..84071c9 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -15,7 +15,7 @@ Tests and server/ projects are outside src/ and are unaffected. --> - 1.17.0 + 1.18.0 Erik Darling Darling Data LLC Performance Studio diff --git a/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs b/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs index f972400..7f96cc0 100644 --- a/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs +++ b/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs @@ -7,5 +7,5 @@ [assembly: AssemblyProduct("Performance Studio for SSMS")] [assembly: AssemblyCopyright("Copyright Darling Data 2026")] [assembly: ComVisible(false)] -[assembly: AssemblyVersion("1.17.0.0")] -[assembly: AssemblyFileVersion("1.17.0.0")] +[assembly: AssemblyVersion("1.18.0.0")] +[assembly: AssemblyFileVersion("1.18.0.0")] diff --git a/src/PlanViewer.Ssms/source.extension.vsixmanifest b/src/PlanViewer.Ssms/source.extension.vsixmanifest index ee1fb42..b199247 100644 --- a/src/PlanViewer.Ssms/source.extension.vsixmanifest +++ b/src/PlanViewer.Ssms/source.extension.vsixmanifest @@ -3,7 +3,7 @@ xmlns:d="http://schemas.microsoft.com/developer/vsx-schema-design/2011"> Performance Studio for SSMS