diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 37183af..48d7596 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -37,13 +37,11 @@ jobs: echo "EXISTS=false" >> $GITHUB_OUTPUT fi - - name: Create release - if: steps.check.outputs.EXISTS == 'false' - shell: bash - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - gh release create "v${{ steps.version.outputs.VERSION }}" --title "v${{ steps.version.outputs.VERSION }}" --generate-notes --target main + # NOTE: the release is deliberately NOT created here. It is created after + # signing succeeds, further down. Creating it up front meant a signing + # failure left a published, publicly-visible release with no app assets - + # which is exactly what happened to v1.17.0 when the SignPath approval + # window lapsed. Nothing before the signing step publishes anything now. - name: Setup .NET 10.0 uses: actions/setup-dotnet@v6 @@ -103,29 +101,12 @@ jobs: "BUILT=true" >> $env:GITHUB_OUTPUT Write-Host "SSMS extension built: $vsix" } else { - Write-Host "::warning::SSMS extension did not produce the expected artifacts - release published without the VSIX (issue 343)" + Write-Host "::warning::SSMS extension did not produce the expected artifacts - the release will be published without the VSIX (issue 343)" } - - name: Upload SSMS extension to release - if: steps.ssms.outputs.BUILT == 'true' - shell: pwsh - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - VERSION: ${{ steps.version.outputs.VERSION }} - run: gh release upload "v$env:VERSION" releases/PlanViewer.Ssms.vsix releases/InstallSsmsExtension.exe --clobber - - # Publish to the SSMS Gallery (https://ssmsgallery.azurewebsites.net, issue 343). - # The gallery extracts version/description from the vsixmanifest, so the - # upload filename is irrelevant. continue-on-error: a gallery outage must - # never fail the release. bash so `curl` is curl.exe, not the pwsh alias. - - name: Publish SSMS extension to SSMS Gallery - if: steps.ssms.outputs.BUILT == 'true' - continue-on-error: true - shell: bash - run: | - echo "Uploading PlanViewer.Ssms.vsix to the SSMS Gallery..." - curl -sS -f "https://ssmsgallery.azurewebsites.net/api/upload" \ - -F "file=@releases/PlanViewer.Ssms.vsix" + # The VSIX is built above but NOT uploaded here — uploading requires the + # release to exist, and the release is not created until signing succeeds. + # Both uploads happen after the signing step. # ── SignPath code signing (Windows). Signing is REQUIRED for a release: # if the SignPath token is missing the job fails loudly rather than @@ -172,6 +153,37 @@ jobs: Remove-Item -Recurse -Force publish/win-x64 Copy-Item -Recurse signed/win-x64 publish/win-x64 + # ── Everything above this line is reversible: nothing has been published. + # The release is created only now that signed binaries exist. ── + - name: Create release + if: steps.check.outputs.EXISTS == 'false' + shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + gh release create "v${{ steps.version.outputs.VERSION }}" --title "v${{ steps.version.outputs.VERSION }}" --generate-notes --target main + + - name: Upload SSMS extension to release + if: steps.ssms.outputs.BUILT == 'true' + shell: pwsh + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + VERSION: ${{ steps.version.outputs.VERSION }} + run: gh release upload "v$env:VERSION" releases/PlanViewer.Ssms.vsix releases/InstallSsmsExtension.exe --clobber + + # Publish to the SSMS Gallery (https://ssmsgallery.azurewebsites.net, issue 343). + # The gallery extracts version/description from the vsixmanifest, so the + # upload filename is irrelevant. continue-on-error: a gallery outage must + # never fail the release. bash so `curl` is curl.exe, not the pwsh alias. + - name: Publish SSMS extension to SSMS Gallery + if: steps.ssms.outputs.BUILT == 'true' + continue-on-error: true + shell: bash + run: | + echo "Uploading PlanViewer.Ssms.vsix to the SSMS Gallery..." + curl -sS -f "https://ssmsgallery.azurewebsites.net/api/upload" \ + -F "file=@releases/PlanViewer.Ssms.vsix" + # ── Velopack (uses signed Windows binaries) ─────────────────────── - name: Create Velopack release (Windows) shell: pwsh diff --git a/server/PlanShare/Program.cs b/server/PlanShare/Program.cs index 446c533..93148c7 100644 --- a/server/PlanShare/Program.cs +++ b/server/PlanShare/Program.cs @@ -2,6 +2,7 @@ using System.Security.Cryptography; using System.Text; using System.Text.Json; +using Microsoft.AspNetCore.HttpOverrides; using Microsoft.Data.Sqlite; var builder = WebApplication.CreateBuilder(args); @@ -44,20 +45,62 @@ created_at TEXT NOT NULL cmd.ExecuteNonQuery(); } +// --- Visitor hash salt --- +// SHA256(ip|ua|day) truncated to 16 hex is brute-forceable straight back to the +// source IP: IPv4 is only 2^32, the User-Agent is guessable, and the day is +// known. Without a secret in the mix, "no PII stored" is not true. The salt is +// generated once and kept in a file beside the DB, never in the DB itself, so +// a copy of plans.db does not carry the means to reverse its own hashes. +var saltPath = Path.Combine(dataDir, "visitor-salt.bin"); +byte[] visitorSalt; +if (File.Exists(saltPath)) +{ + visitorSalt = File.ReadAllBytes(saltPath); +} +else +{ + visitorSalt = RandomNumberGenerator.GetBytes(32); + File.WriteAllBytes(saltPath, visitorSalt); +} + // --- 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); +// Plan reads are cheap but unbounded reads let anyone enumerate or scrape the +// store; 120/min per IP is far above any human's browsing rate. +var readRateLimiter = new RateLimiter(maxRequests: 120, windowSeconds: 60); // Register the cleanup background service builder.Services.AddSingleton(new PlanDbConfig(connectionString)); -builder.Services.AddSingleton(new RateLimiters(rateLimiter, analyticsRateLimiter)); +builder.Services.AddSingleton(new RateLimiters(rateLimiter, analyticsRateLimiter, readRateLimiter)); builder.Services.AddHostedService(); // Request size limit (10 MB) builder.WebHost.ConfigureKestrel(o => o.Limits.MaxRequestBodySize = 10 * 1024 * 1024); var app = builder.Build(); + +// This runs behind nginx on loopback, which sets X-Real-IP and X-Forwarded-For. +// Without this middleware every request's RemoteIpAddress is 127.0.0.1, so all +// three rate limiters key on the same value and become a single GLOBAL cap +// shared by every visitor rather than per-IP limits — one busy client would +// throttle everyone, and one abusive client could not be isolated. +// +// KnownProxies is restricted to loopback deliberately: trusting X-Forwarded-For +// from arbitrary sources would let any caller spoof its address and bypass the +// limits entirely. Only the local nginx is believed. +var forwardedOptions = new ForwardedHeadersOptions +{ + ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto, + ForwardLimit = 1 +}; +forwardedOptions.KnownProxies.Clear(); +forwardedOptions.KnownIPNetworks.Clear(); +forwardedOptions.KnownProxies.Add(System.Net.IPAddress.Loopback); +forwardedOptions.KnownProxies.Add(System.Net.IPAddress.IPv6Loopback); +app.UseForwardedHeaders(forwardedOptions); + app.UseCors(); const int MaxTtlDays = 365; @@ -118,8 +161,12 @@ created_at TEXT NOT NULL "application/json"); }); -app.MapGet("/api/plans/{id}", (string id) => +app.MapGet("/api/plans/{id}", (string id, HttpContext ctx) => { + var readIp = ctx.Connection.RemoteIpAddress?.ToString() ?? "unknown"; + if (!readRateLimiter.IsAllowed(readIp)) + return Results.StatusCode(429); + using var conn = new SqliteConnection(connectionString); conn.Open(); using var cmd = conn.CreateCommand(); @@ -173,11 +220,15 @@ created_at TEXT NOT NULL : null; } - // Visitor hash: SHA256(IP + User-Agent + date) — unique per day, no PII stored + // Visitor hash: HMAC-SHA256(secret salt, IP + User-Agent + date) — unique per + // day. The salt is what makes this one-way in practice: without it the input + // space (IPv4 = 2^32, guessable UA, known date) is small enough to brute + // force straight back to the source IP, so an unsalted digest would still be + // personal data despite looking like a hash. var ua = ctx.Request.Headers.UserAgent.FirstOrDefault() ?? ""; var day = DateTime.UtcNow.ToString("yyyy-MM-dd"); var visitorHash = Convert.ToHexString( - SHA256.HashData(Encoding.UTF8.GetBytes($"{ip}|{ua}|{day}"))).ToLower()[..16]; + HMACSHA256.HashData(visitorSalt, Encoding.UTF8.GetBytes($"{ip}|{ua}|{day}"))).ToLower()[..16]; using var conn = new SqliteConnection(connectionString); conn.Open(); @@ -192,8 +243,26 @@ created_at TEXT NOT NULL return Results.Ok(); }); -app.MapGet("/api/stats", () => +app.MapGet("/api/stats", (HttpContext ctx) => { + // Opt-in auth. /api/stats exposes share counts, daily traffic, unique + // visitors and top referrers — operational data, not something that needs + // to be world-readable. Set STATS_TOKEN to require it; leaving it unset + // keeps the current public behaviour so the dashboard cannot break just by + // deploying this. Compared with a fixed-time comparison so the token cannot + // be recovered a byte at a time. + var expectedToken = Environment.GetEnvironmentVariable("STATS_TOKEN"); + if (!string.IsNullOrEmpty(expectedToken)) + { + var supplied = ctx.Request.Headers["X-Stats-Token"].FirstOrDefault() + ?? ctx.Request.Query["token"].FirstOrDefault() + ?? ""; + var a = Encoding.UTF8.GetBytes(supplied); + var b = Encoding.UTF8.GetBytes(expectedToken); + if (a.Length != b.Length || !CryptographicOperations.FixedTimeEquals(a, b)) + return Results.StatusCode(401); + } + using var conn = new SqliteConnection(connectionString); conn.Open(); var now = DateTime.UtcNow.ToString("o"); @@ -362,7 +431,7 @@ static string GenerateDeleteToken() record PlanDbConfig(string ConnectionString); -record RateLimiters(RateLimiter Share, RateLimiter Analytics); +record RateLimiters(RateLimiter Share, RateLimiter Analytics, RateLimiter Read); sealed class CleanupService : BackgroundService { @@ -416,13 +485,16 @@ private void Cleanup() _logger.LogInformation("Cleaned up {Count} old page views", deleted); } - // Evict stale rate-limiter keys so the dictionary doesn't grow forever. + // Evict stale rate-limiter keys so the dictionaries don't grow forever. + // Every limiter must be swept here: an unswept one keeps a permanent + // entry per unique client IP for the lifetime of the process. var shareEvicted = _rateLimiters.Share.Sweep(); var analyticsEvicted = _rateLimiters.Analytics.Sweep(); - if (shareEvicted + analyticsEvicted > 0) + var readEvicted = _rateLimiters.Read.Sweep(); + if (shareEvicted + analyticsEvicted + readEvicted > 0) _logger.LogInformation( - "Evicted {Share} share + {Analytics} analytics rate-limit keys", - shareEvicted, analyticsEvicted); + "Evicted {Share} share + {Analytics} analytics + {Read} read rate-limit keys", + shareEvicted, analyticsEvicted, readEvicted); } catch (Exception ex) { diff --git a/server/PlanShare/dashboard.html b/server/PlanShare/dashboard.html index f0a7fd5..3b24ac0 100644 --- a/server/PlanShare/dashboard.html +++ b/server/PlanShare/dashboard.html @@ -219,10 +219,45 @@

Top Referrers (30 days)

}).join(""); updateCharts(); updateReleases(repo); } + // Token for /api/stats when the server has STATS_TOKEN set. + // + // Bootstrapped from the URL *fragment* (#token=...), not a query string. + // A fragment is never transmitted to the server, so the token cannot land + // in nginx access logs. A ?token= would be logged the moment the page is + // requested — before any client-side code could strip it — which would + // leave the secret sitting in plaintext logs forever. + // + // Stored once, then cleared from the address bar so it does not persist in + // browser history either. Requests always send it as a header. + function statsToken() { + var hash = window.location.hash || ""; + var m = hash.match(/(?:^#|&)token=([^&]+)/); + if (m) { + var t = decodeURIComponent(m[1]); + try { localStorage.setItem("planshare_stats_token", t); } catch (e) { /* private mode */ } + history.replaceState(null, "", window.location.pathname + window.location.search); + return t; + } + try { return localStorage.getItem("planshare_stats_token") || ""; } catch (e) { return ""; } + } + var planStatsError = null; async function loadPlanStats() { + planStatsError = null; try { - const resp = await fetch("/api/stats"); - planStats = await resp.json(); + var token = statsToken(); + const resp = await fetch("/api/stats", token ? { headers: { "X-Stats-Token": token } } : undefined); + if (resp.status === 401) { + // Distinct from "no data": the server is fine, we are just not + // authorised. Swallowing this as an empty dashboard is how you + // spend an afternoon debugging the wrong thing. + planStatsError = "Stats require a token. Open this page once with #token=YOUR_TOKEN"; + planStats = null; + } else if (!resp.ok) { + planStatsError = "Stats unavailable (HTTP " + resp.status + ")"; + planStats = null; + } else { + planStats = await resp.json(); + } } catch (e) { planStats = null; } @@ -231,7 +266,9 @@

Top Referrers (30 days)

function updatePlanView() { var row = document.getElementById("statsRow"); if (!planStats) { - row.innerHTML = "
-
No data yet
"; + var label = planStatsError || "No data yet"; + row.innerHTML = "
-
"; + row.querySelector(".stat-label").textContent = label; return; } var t = planStats.traffic;