Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 49 additions & 8 deletions server/PlanShare/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,14 @@ created_at TEXT NOT NULL
cmd.ExecuteNonQuery();
}

// --- Rate limiters (in-memory) ---
// Created before Build() so they can be DI-registered and swept by CleanupService.
var rateLimiter = new RateLimiter(maxRequests: 10, windowSeconds: 60);
var analyticsRateLimiter = new RateLimiter(maxRequests: 30, windowSeconds: 60);

// Register the cleanup background service
builder.Services.AddSingleton(new PlanDbConfig(connectionString));
builder.Services.AddSingleton(new RateLimiters(rateLimiter, analyticsRateLimiter));
builder.Services.AddHostedService<CleanupService>();

// Request size limit (10 MB)
Expand All @@ -54,10 +60,6 @@ created_at TEXT NOT NULL
var app = builder.Build();
app.UseCors();

// --- Rate limiters (in-memory) ---
var rateLimiter = new RateLimiter(maxRequests: 10, windowSeconds: 60);
var analyticsRateLimiter = new RateLimiter(maxRequests: 30, windowSeconds: 60);

const int MaxTtlDays = 365;

// --- Endpoints ---
Expand Down Expand Up @@ -161,9 +163,15 @@ created_at TEXT NOT NULL
return Results.BadRequest("Invalid JSON");
}

// Strip referrer to domain only (no full URLs with query params)
if (!string.IsNullOrEmpty(referrer) && Uri.TryCreate(referrer, UriKind.Absolute, out var refUri))
referrer = refUri.Host;
// Strip referrer to domain only (no full URLs with query params).
// If it doesn't parse as an absolute URL, drop it — never persist raw
// client-supplied strings, since the dashboard renders referrers in HTML.
if (!string.IsNullOrEmpty(referrer))
{
referrer = Uri.TryCreate(referrer, UriKind.Absolute, out var refUri)
? refUri.Host
: null;
}

// Visitor hash: SHA256(IP + User-Agent + date) — unique per day, no PII stored
var ua = ctx.Request.Headers.UserAgent.FirstOrDefault() ?? "";
Expand Down Expand Up @@ -352,14 +360,18 @@ static string GenerateDeleteToken()

record PlanDbConfig(string ConnectionString);

record RateLimiters(RateLimiter Share, RateLimiter Analytics);

sealed class CleanupService : BackgroundService
{
private readonly PlanDbConfig _config;
private readonly RateLimiters _rateLimiters;
private readonly ILogger<CleanupService> _logger;

public CleanupService(PlanDbConfig config, ILogger<CleanupService> logger)
public CleanupService(PlanDbConfig config, RateLimiters rateLimiters, ILogger<CleanupService> logger)
{
_config = config;
_rateLimiters = rateLimiters;
_logger = logger;
}

Expand Down Expand Up @@ -401,6 +413,14 @@ private void Cleanup()
if (deleted > 0)
_logger.LogInformation("Cleaned up {Count} old page views", deleted);
}

// Evict stale rate-limiter keys so the dictionary doesn't grow forever.
var shareEvicted = _rateLimiters.Share.Sweep();
var analyticsEvicted = _rateLimiters.Analytics.Sweep();
if (shareEvicted + analyticsEvicted > 0)
_logger.LogInformation(
"Evicted {Share} share + {Analytics} analytics rate-limit keys",
shareEvicted, analyticsEvicted);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -441,4 +461,25 @@ public bool IsAllowed(string key)
return true;
}
}

/// <summary>
/// Evicts keys whose timestamp lists have gone empty. Call periodically
/// so the dictionary doesn't grow forever across unique IPs.
/// Returns the number of keys evicted.
/// </summary>
public int Sweep()
{
var cutoff = DateTime.UtcNow.AddSeconds(-_windowSeconds);
var evicted = 0;
foreach (var kvp in _requests)
{
lock (kvp.Value)
{
kvp.Value.RemoveAll(t => t < cutoff);
if (kvp.Value.Count == 0 && _requests.TryRemove(kvp))
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Race with IsAllowed: a caller that already resolved _requests.GetOrAdd(key, ...) → timestamps can be blocked on lock (timestamps) while Sweep holds the same lock, observes Count == 0, and TryRemoves the kvp. When the caller then acquires the lock it adds its timestamp to a list that's no longer in the dictionary — the next IsAllowed(key) call creates a fresh list, so that one request effectively bypasses the rate limit.

Not catastrophic at 10/min, but worth either (a) re-checking that _requests[key] still points at kvp.Value after the remove, or (b) adding a sentinel/"tombstoned" flag on the list so a late arrival can detect the eviction and retry via GetOrAdd.


Generated by Claude Code

evicted++;
}
}
return evicted;
}
}
23 changes: 19 additions & 4 deletions server/PlanShare/dashboard.html
Original file line number Diff line number Diff line change
Expand Up @@ -275,13 +275,28 @@ <h3>Top Referrers (30 days)</h3>
}
function updateReferrers() {
var tbody = document.querySelector("#referrersTable tbody");
tbody.replaceChildren();
if (!planStats || !planStats.traffic.top_referrers.length) {
tbody.innerHTML = "<tr><td colspan='2' style='color:#484f58'>No referrer data yet</td></tr>";
var tr = document.createElement("tr");
var td = document.createElement("td");
td.setAttribute("colspan", "2");
td.style.color = "#484f58";
td.textContent = "No referrer data yet";
tr.appendChild(td);
tbody.appendChild(tr);
return;
}
tbody.innerHTML = planStats.traffic.top_referrers.map(function(r) {
return "<tr><td>" + r.referrer + "</td><td class='num'>" + fmt(r.count) + "</td></tr>";
}).join("");
planStats.traffic.top_referrers.forEach(function(r) {
var tr = document.createElement("tr");
var tdRef = document.createElement("td");
tdRef.textContent = r.referrer;
var tdCount = document.createElement("td");
tdCount.className = "num";
tdCount.textContent = fmt(r.count);
tr.appendChild(tdRef);
tr.appendChild(tdCount);
tbody.appendChild(tr);
});
}
function updateCharts() {
updateChart("starsChart", "line", {
Expand Down
2 changes: 1 addition & 1 deletion src/PlanViewer.App/AboutWindow.axaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private void SaveMcpSettings()
}, new JsonSerializerOptions { WriteIndented = true });

Directory.CreateDirectory(settingsDir);
File.WriteAllText(settingsFile, json);
Services.AtomicFile.WriteAllText(settingsFile, json);
}

private void GitHubLink_Click(object? sender, PointerPressedEventArgs e) => OpenUrl(GitHubUrl);
Expand Down
7 changes: 5 additions & 2 deletions src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,9 @@ private static string FormatBytes(double bytes)
return $"{bytes / (1024L * 1024 * 1024):N1} GB";
}

private static string FormatBenefitPercent(double pct) =>
pct >= 100 ? $"{pct:N0}" : $"{pct:N1}";

#endregion

#region Node Selection & Properties Panel
Expand Down Expand Up @@ -1737,7 +1740,7 @@ private void ShowPropertiesPanel(PlanNode node)
: w.Severity == PlanWarningSeverity.Warning ? "#FFB347" : "#6BB5FF";
var warnPanel = new StackPanel { Margin = new Thickness(10, 2, 10, 2) };
var planWarnHeader = w.MaxBenefitPercent.HasValue
? $"\u26A0 {w.WarningType} \u2014 up to {w.MaxBenefitPercent:N0}% benefit"
? $"\u26A0 {w.WarningType} \u2014 up to {FormatBenefitPercent(w.MaxBenefitPercent.Value)}% benefit"
: $"\u26A0 {w.WarningType}";
warnPanel.Children.Add(new TextBlock
{
Expand Down Expand Up @@ -1819,7 +1822,7 @@ private void ShowPropertiesPanel(PlanNode node)
: w.Severity == PlanWarningSeverity.Warning ? "#FFB347" : "#6BB5FF";
var warnPanel = new StackPanel { Margin = new Thickness(10, 2, 10, 2) };
var nodeWarnHeader = w.MaxBenefitPercent.HasValue
? $"\u26A0 {w.WarningType} \u2014 up to {w.MaxBenefitPercent:N0}% benefit"
? $"\u26A0 {w.WarningType} \u2014 up to {FormatBenefitPercent(w.MaxBenefitPercent.Value)}% benefit"
: $"\u26A0 {w.WarningType}";
warnPanel.Children.Add(new TextBlock
{
Expand Down
6 changes: 5 additions & 1 deletion src/PlanViewer.App/Controls/QuerySessionControl.axaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,15 @@ public QuerySessionControl(ICredentialService credentialService, ConnectionStore
QueryEditor.TextArea.Focus();
};

// Dispose TextMate when detached (e.g. tab switch) to release renderers/transformers
// Dispose TextMate when detached (e.g. tab switch) to release renderers/transformers.
// Also cancel any in-flight status-clear dispatch so it doesn't fire on a dead control.
DetachedFromVisualTree += (_, _) =>
{
_textMateInstallation?.Dispose();
_textMateInstallation = null;
_statusClearCts?.Cancel();
_statusClearCts?.Dispose();
_statusClearCts = null;
};

// Focus the editor when the Editor tab is selected; toggle plan-dependent buttons
Expand Down
10 changes: 10 additions & 0 deletions src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@

// Select initial metric in the combo box
var metricTag = initialMetricTag;
foreach (ComboBoxItem item in MetricSelector.Items)

Check warning on line 114 in src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs

View workflow job for this annotation

GitHub Actions / release

Converting null literal or possible null value to non-nullable type.

Check warning on line 114 in src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs

View workflow job for this annotation

GitHub Actions / release

Converting null literal or possible null value to non-nullable type.

Check warning on line 114 in src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs

View workflow job for this annotation

GitHub Actions / release

Converting null literal or possible null value to non-nullable type.

Check warning on line 114 in src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs

View workflow job for this annotation

GitHub Actions / release

Converting null literal or possible null value to non-nullable type.

Check warning on line 114 in src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs

View workflow job for this annotation

GitHub Actions / release

Converting null literal or possible null value to non-nullable type.
{
if (item.Tag?.ToString() == metricTag)

Check warning on line 116 in src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs

View workflow job for this annotation

GitHub Actions / release

Dereference of a possibly null reference.

Check warning on line 116 in src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs

View workflow job for this annotation

GitHub Actions / release

Dereference of a possibly null reference.

Check warning on line 116 in src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs

View workflow job for this annotation

GitHub Actions / release

Dereference of a possibly null reference.
{
MetricSelector.SelectedItem = item;
break;
Expand Down Expand Up @@ -169,6 +169,16 @@
: "AvgCpuMs";
}

protected override void OnClosed(EventArgs e)
{
// Cancel any in-flight history fetch so the SqlConnection doesn't sit open on the
// server after the dialog is dismissed.
_fetchCts?.Cancel();
_fetchCts?.Dispose();
_fetchCts = null;
base.OnClosed(e);
}

private async System.Threading.Tasks.Task LoadHistoryAsync()
{
_fetchCts?.Cancel();
Expand Down Expand Up @@ -749,7 +759,7 @@

private void OnHighlightLoadingRow(object? sender, DataGridRowEventArgs e)
{
var idx = e.Row.GetIndex();

Check warning on line 762 in src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs

View workflow job for this annotation

GitHub Actions / release

'DataGridRow.GetIndex()' is obsolete: 'This API is going to be removed in a future version. Use the Index property instead.'

Check warning on line 762 in src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs

View workflow job for this annotation

GitHub Actions / release

'DataGridRow.GetIndex()' is obsolete: 'This API is going to be removed in a future version. Use the Index property instead.'

Check warning on line 762 in src/PlanViewer.App/Dialogs/QueryStoreHistoryWindow.axaml.cs

View workflow job for this annotation

GitHub Actions / release

'DataGridRow.GetIndex()' is obsolete: 'This API is going to be removed in a future version. Use the Index property instead.'
if (_selectedRowIndices.Contains(idx))
{
e.Row.Background = new SolidColorBrush(Avalonia.Media.Color.FromArgb(60, 79, 195, 247));
Expand Down
2 changes: 1 addition & 1 deletion src/PlanViewer.App/PlanViewer.App.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<ApplicationManifest>app.manifest</ApplicationManifest>
<ApplicationIcon>EDD.ico</ApplicationIcon>
<AvaloniaUseCompiledBindingsByDefault>true</AvaloniaUseCompiledBindingsByDefault>
<Version>1.7.1</Version>
<Version>1.7.2</Version>
<Authors>Erik Darling</Authors>
<Company>Darling Data LLC</Company>
<Product>Performance Studio</Product>
Expand Down
2 changes: 1 addition & 1 deletion src/PlanViewer.App/Services/AppSettingsService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public static void Save(AppSettings settings)
{
Directory.CreateDirectory(SettingsDir);
var json = JsonSerializer.Serialize(settings, JsonOptions);
File.WriteAllText(SettingsPath, json);
AtomicFile.WriteAllText(SettingsPath, json);
}
catch
{
Expand Down
27 changes: 27 additions & 0 deletions src/PlanViewer.App/Services/AtomicFile.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using System.IO;

namespace PlanViewer.App.Services;

/// <summary>
/// Helper for atomic text-file writes: write to a sibling .tmp and rename
/// into place so a crash mid-write can't truncate the target file. Callers
/// are responsible for creating the parent directory first.
/// </summary>
internal static class AtomicFile
{
/// <summary>
/// Writes <paramref name="contents"/> to <paramref name="path"/> atomically
/// with respect to process crashes. If the process dies before the rename,
/// <paramref name="path"/> keeps its previous contents and a stray
/// <c>.tmp</c> sibling is left behind (cleaned up on the next call).
/// </summary>
public static void WriteAllText(string path, string contents)
{
var tmp = path + ".tmp";
File.WriteAllText(tmp, contents);
// File.Move with overwrite:true maps to MoveFileEx(MOVEFILE_REPLACE_EXISTING)
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two concurrent writers (two Studio processes saving settings at once) both race on path + ".tmp"File.WriteAllText on the second one truncates the first's in-progress temp, then both File.Move calls succeed in some order, so the final content is well-formed but may be either process's payload rather than the later write. Settings don't change often enough for this to matter, but the docstring's "atomic with respect to process crashes" is accurate — not "atomic with respect to concurrent writers." A unique suffix (e.g. path + "." + Guid.NewGuid().ToString("N") + ".tmp") would close it if you ever need it.


Generated by Claude Code

// on Windows and rename(2) on Unix — both atomic when source and destination
// live on the same filesystem, which is always the case here.
File.Move(tmp, path, overwrite: true);
}
}
2 changes: 1 addition & 1 deletion src/PlanViewer.App/Services/ConnectionStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void Save(List<ServerConnection> connections)
{
Directory.CreateDirectory(ConfigDir);
var json = JsonSerializer.Serialize(connections, JsonOptions);
File.WriteAllText(ConfigFile, json);
AtomicFile.WriteAllText(ConfigFile, json);
}

public void AddOrUpdate(ServerConnection connection)
Expand Down
2 changes: 1 addition & 1 deletion src/PlanViewer.App/Services/SqlFormatSettingsService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public static bool Save(SqlFormatSettings settings, out string? error)
{
Directory.CreateDirectory(SettingsDir);
var json = JsonSerializer.Serialize(settings, JsonOptions);
File.WriteAllText(SettingsPath, json);
AtomicFile.WriteAllText(SettingsPath, json);
return true;
}
catch (Exception ex)
Expand Down
22 changes: 19 additions & 3 deletions src/PlanViewer.Cli/Commands/AnalyzeCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,11 @@ public static Command Create(ICredentialService? credentialService = null)

var passwordOption = new Option<string?>(
"--password",
"SQL Server password (bypasses credential store)");
"SQL Server password (bypasses credential store). Visible in process listings — prefer --password-stdin.");

var passwordStdinOption = new Option<bool>(
"--password-stdin",
"Read the SQL Server password from stdin (avoids process-listing exposure). Mutually exclusive with --password.");

var configOption = new Option<string?>(
"--config",
Expand All @@ -118,6 +122,7 @@ public static Command Create(ICredentialService? credentialService = null)
timeoutOption,
loginOption,
passwordOption,
passwordStdinOption,
configOption
};

Expand All @@ -137,7 +142,8 @@ public static Command Create(ICredentialService? credentialService = null)
var trustCert = ctx.ParseResult.GetValueForOption(trustCertOption);
var timeout = ctx.ParseResult.GetValueForOption(timeoutOption);
var login = ctx.ParseResult.GetValueForOption(loginOption);
var password = ctx.ParseResult.GetValueForOption(passwordOption);
var passwordInline = ctx.ParseResult.GetValueForOption(passwordOption);
var passwordStdin = ctx.ParseResult.GetValueForOption(passwordStdinOption);
var configPath = ctx.ParseResult.GetValueForOption(configOption);

// Load analyzer config
Expand All @@ -148,10 +154,20 @@ public static Command Create(ICredentialService? credentialService = null)
server ??= env.GetValueOrDefault("PLANVIEW_SERVER");
database ??= env.GetValueOrDefault("PLANVIEW_DATABASE");
login ??= env.GetValueOrDefault("PLANVIEW_LOGIN");
password ??= env.GetValueOrDefault("PLANVIEW_PASSWORD");
if (!trustCert && env.GetValueOrDefault("PLANVIEW_TRUST_CERT")?.Equals("true", StringComparison.OrdinalIgnoreCase) == true)
trustCert = true;

// Resolve password from --password-stdin, --password, or PLANVIEW_PASSWORD
// (in that order). --stdin for plan XML conflicts with --password-stdin.
if (!PasswordResolver.TryResolve(
passwordInline, passwordStdin, stdin,
env.GetValueOrDefault("PLANVIEW_PASSWORD"),
out var password))
{
Environment.ExitCode = 1;
return;
}

if (server != null)
{
await RunLiveAsync(file, server, database, query, outputDir, estimated,
Expand Down
3 changes: 3 additions & 0 deletions src/PlanViewer.Cli/Commands/CredentialCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ private static Command CreateAddCommand(ICredentialService credentialService)
string password;
if (!string.IsNullOrEmpty(passwordArg))
{
Console.Error.WriteLine(
"Warning: --password is visible in process listings and shell history. " +
"Prefer piping the password into stdin (e.g. `echo hunter2 | planview credential add ...`).");
password = passwordArg;
}
else if (Console.IsInputRedirected)
Expand Down
22 changes: 18 additions & 4 deletions src/PlanViewer.Cli/Commands/QueryStoreCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ public static Command Create(ICredentialService? credentialService = null)
"--login", "SQL Server login (bypasses credential store)");

var passwordOption = new Option<string?>(
"--password", "SQL Server password");
"--password", "SQL Server password. Visible in process listings — prefer --password-stdin.");

var passwordStdinOption = new Option<bool>(
"--password-stdin",
"Read the SQL Server password from stdin (avoids process-listing exposure). Mutually exclusive with --password.");

var queryIdOption = new Option<long?>(
"--query-id", "Filter by Query Store query ID");
Expand All @@ -93,7 +97,7 @@ public static Command Create(ICredentialService? credentialService = null)
{
serverOption, databaseOption, topOption, orderByOption, hoursBackOption,
outputDirOption, outputOption, compactOption, warningsOnlyOption, configOption,
authOption, trustCertOption, loginOption, passwordOption,
authOption, trustCertOption, loginOption, passwordOption, passwordStdinOption,
queryIdOption, planIdOption, queryHashOption, planHashOption, moduleOption
};

Expand All @@ -112,7 +116,8 @@ public static Command Create(ICredentialService? credentialService = null)
var auth = ctx.ParseResult.GetValueForOption(authOption);
var trustCert = ctx.ParseResult.GetValueForOption(trustCertOption);
var login = ctx.ParseResult.GetValueForOption(loginOption);
var password = ctx.ParseResult.GetValueForOption(passwordOption);
var passwordInline = ctx.ParseResult.GetValueForOption(passwordOption);
var passwordStdin = ctx.ParseResult.GetValueForOption(passwordStdinOption);
var filterQueryId = ctx.ParseResult.GetValueForOption(queryIdOption);
var filterPlanId = ctx.ParseResult.GetValueForOption(planIdOption);
var filterQueryHash = ctx.ParseResult.GetValueForOption(queryHashOption);
Expand All @@ -122,10 +127,19 @@ public static Command Create(ICredentialService? credentialService = null)
// Load .env file if present (CLI args take precedence)
var env = ConnectionHelper.LoadEnvFile();
login ??= env.GetValueOrDefault("PLANVIEW_LOGIN");
password ??= env.GetValueOrDefault("PLANVIEW_PASSWORD");
if (!trustCert && env.GetValueOrDefault("PLANVIEW_TRUST_CERT")?.Equals("true", StringComparison.OrdinalIgnoreCase) == true)
trustCert = true;

// Resolve password from --password-stdin, --password, or PLANVIEW_PASSWORD
if (!PasswordResolver.TryResolve(
passwordInline, passwordStdin, stdinAlreadyClaimed: false,
env.GetValueOrDefault("PLANVIEW_PASSWORD"),
out var password))
{
Environment.ExitCode = 1;
return;
}

if (top < 1)
{
Console.Error.WriteLine("--top must be >= 1");
Expand Down
Loading
Loading