Harden PlanShare: storage limit, daily upload budget, one client key

.github/workflows/ci.yml

@@ -31,13 +31,15 @@ jobs:
           # stack of '!' patterns matches whenever a file fails ANY one of them, which for
           # a docs-only change is always true. Listing what IS code keeps the OR honest.
           # PlanViewer.Ssms and PlanViewer.Ssms.Installer stay out: they are not in the
-          # solution and ci.yml never built them.
+          # solution and ci.yml never built them. server/PlanShare is in: the test project
+          # references it, so this job builds and tests it.
           filters: |
             code:
               - 'src/PlanViewer.App/**'
               - 'src/PlanViewer.Cli/**'
               - 'src/PlanViewer.Core/**'
               - 'src/PlanViewer.Web/**'
+              - 'server/PlanShare/**'
               - 'src/Directory.Build.props'
               - 'tests/**'
               - 'PlanViewer.sln'

server/PlanShare/ClientKey.cs

@@ -0,0 +1,33 @@
+using System.Net;
+using System.Net.Sockets;
+
+namespace PlanShare;
+
+/// <summary>
+/// The one key every per-client limit (share, analytics, read, upload budget) is counted under.
+/// An IPv4 address is its own key. An IPv4-mapped IPv6 address (how a dual-stack socket reports
+/// an IPv4 caller) becomes that IPv4 address, so one caller has one key. Any other IPv6 address
+/// becomes its /64 prefix: a single subscriber is normally given a whole /64, so counting per /64
+/// makes one subscriber one client, however many addresses they use.
+/// </summary>
+internal static class ClientKey
+{
+    public const string Unknown = "unknown";
+
+    public static string From(IPAddress? address)
+    {
+        if (address is null)
+            return Unknown;
+
+        if (address.IsIPv4MappedToIPv6)
+            address = address.MapToIPv4();
+
+        if (address.AddressFamily != AddressFamily.InterNetworkV6)
+            return address.ToString();
+
+        Span<byte> bytes = stackalloc byte[16];
+        address.TryWriteBytes(bytes, out _);
+        bytes[8..].Clear();
+        return $"{new IPAddress(bytes)}/64";
+    }
+}

server/PlanShare/PlanShare.csproj

@@ -18,4 +18,9 @@
     <PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3" Version="3.0.5" />
   </ItemGroup>
 
+  <!-- tests/PlanViewer.Core.Tests references this project so CI builds and tests the server -->
+  <ItemGroup>
+    <InternalsVisibleTo Include="PlanViewer.Core.Tests" />
+  </ItemGroup>
+
 </Project>

server/PlanShare/Program.cs

@@ -4,6 +4,7 @@
 using System.Text.Json;
 using Microsoft.AspNetCore.HttpOverrides;
 using Microsoft.Data.Sqlite;
+using PlanShare;
 
 var builder = WebApplication.CreateBuilder(args);
 
@@ -14,8 +15,9 @@
         policy.AllowAnyOrigin().AllowAnyMethod().AllowAnyHeader());
 });
 
-// Database path — data/ subdirectory relative to the binary
-var dataDir = Path.Combine(AppContext.BaseDirectory, "data");
+// Database path — data/ subdirectory relative to the binary. PlanShare:DataDir moves it
+// (the endpoint tests and local runs point it at a temp folder).
+var dataDir = builder.Configuration["PlanShare:DataDir"] ?? Path.Combine(AppContext.BaseDirectory, "data");
 Directory.CreateDirectory(dataDir);
 var dbPath = Path.Combine(dataDir, "plans.db");
 var connectionString = $"Data Source={dbPath}";
@@ -71,9 +73,23 @@ created_at TEXT NOT NULL
 // store; 120/min per IP is far above any human's browsing rate.
 var readRateLimiter = new RateLimiter(maxRequests: 120, windowSeconds: 60);
 
+// --- Storage limit and daily upload budget ---
+// The production disk is 40 GB, so shares are refused (507) once the database uses 10 GB of pages.
+// The budget is the most plan data one client key can store per UTC day (429 after that).
+// PlanShare:MaxDatabaseBytes and PlanShare:DailyUploadBytes override the two limits, which lets
+// the endpoint tests use small values.
+const long MaxDatabaseBytes = 10L * 1024 * 1024 * 1024;
+const long DailyUploadBytes = 100L * 1024 * 1024;
+var storageCheck = new StorageCheck(
+    connectionString,
+    builder.Configuration.GetValue<long?>("PlanShare:MaxDatabaseBytes") ?? MaxDatabaseBytes);
+var uploadBudget = new UploadBudget(
+    builder.Configuration.GetValue<long?>("PlanShare:DailyUploadBytes") ?? DailyUploadBytes);
+
 // Register the cleanup background service
 builder.Services.AddSingleton(new PlanDbConfig(connectionString));
 builder.Services.AddSingleton(new RateLimiters(rateLimiter, analyticsRateLimiter, readRateLimiter));
+builder.Services.AddSingleton(uploadBudget);
 builder.Services.AddHostedService<CleanupService>();
 
 // Request size limit (10 MB)
@@ -105,6 +121,9 @@ created_at TEXT NOT NULL
 
 const int MaxTtlDays = 365;
 
+// Longest page path an analytics event may carry. Real paths are a few characters.
+const int MaxEventPathLength = 512;
+
 // Depth ceiling for parsing an uploaded share, mirroring PlanViewer.Core's
 // AnalysisJson.MaxDepth — that class is the source of truth for how deep a serialized
 // AnalysisResult can go (#431: an operator costs two JSON levels, so the JsonDocument
@@ -121,11 +140,11 @@ created_at TEXT NOT NULL
 
 app.MapPost("/api/share", async (HttpContext ctx) =>
 {
-    // Rate limit by IP
-    var ip = ctx.Connection.RemoteIpAddress?.ToString() ?? "unknown";
-    if (!rateLimiter.IsAllowed(ip))
+    // Rate limit by client key (IPv4 address, or the /64 of an IPv6 address)
+    var client = ClientKey.From(ctx.Connection.RemoteIpAddress);
+    if (!rateLimiter.IsAllowed(client))
     {
-        return Results.StatusCode(429);
+        return Error(429, "Too many shares from your network. Please wait a minute and try again.");
     }
 
     // Read raw body
@@ -134,7 +153,7 @@ created_at TEXT NOT NULL
 
     if (string.IsNullOrWhiteSpace(body))
     {
-        return Results.BadRequest("Empty body");
+        return Error(400, "Empty body");
     }
 
     // Parse and extract ttl_days from the JSON. shareDocumentOptions, not defaults: this body
@@ -144,12 +163,31 @@ created_at TEXT NOT NULL
     try
     {
         using var doc = JsonDocument.Parse(body, shareDocumentOptions);
-        if (doc.RootElement.TryGetProperty("ttl_days", out var ttlProp) && ttlProp.TryGetInt32(out var t))
-            ttlDays = Math.Clamp(t, 1, MaxTtlDays);
+        // TryGetProperty and TryGetInt32 throw (a 500) on the wrong kind of element, so check the kind first
+        if (doc.RootElement.ValueKind != JsonValueKind.Object)
+            return Error(400, "The request body must be a JSON object.");
+        if (doc.RootElement.TryGetProperty("ttl_days", out var ttlProp) && ttlProp.ValueKind != JsonValueKind.Null)
+        {
+            if (ttlProp.ValueKind != JsonValueKind.Number)
+                return Error(400, "ttl_days must be a number.");
+            if (ttlProp.TryGetInt32(out var t))
+                ttlDays = Math.Clamp(t, 1, MaxTtlDays);
+        }
     }
     catch (JsonException)
     {
-        return Results.BadRequest("Invalid JSON");
+        return Error(400, "Invalid JSON");
+    }
+
+    if (storageCheck.IsFull())
+    {
+        return Error(507, "Plan sharing is full right now. Please try again later.");
+    }
+
+    // The stored size is the UTF-8 length of the body; ContentLength can be absent
+    if (!uploadBudget.TryCharge(client, Encoding.UTF8.GetByteCount(body)))
+    {
+        return Error(429, "Daily sharing limit reached for your network. Please try again tomorrow.");
     }
 
     var id = GenerateId();
@@ -175,8 +213,8 @@ created_at TEXT NOT NULL
 
 app.MapGet("/api/plans/{id}", (string id, HttpContext ctx) =>
 {
-    var readIp = ctx.Connection.RemoteIpAddress?.ToString() ?? "unknown";
-    if (!readRateLimiter.IsAllowed(readIp))
+    var readClient = ClientKey.From(ctx.Connection.RemoteIpAddress);
+    if (!readRateLimiter.IsAllowed(readClient))
         return Results.StatusCode(429);
 
     using var conn = new SqliteConnection(connectionString);
@@ -199,9 +237,8 @@ created_at TEXT NOT NULL
 
 app.MapPost("/api/event", async (HttpContext ctx) =>
 {
-    // Rate limit: 30 events/min per IP (generous — covers page nav + shares)
-    var ip = ctx.Connection.RemoteIpAddress?.ToString() ?? "unknown";
-    if (!analyticsRateLimiter.IsAllowed(ip))
+    // Rate limit: 30 events/min per client key (generous — covers page nav + shares)
+    if (!analyticsRateLimiter.IsAllowed(ClientKey.From(ctx.Connection.RemoteIpAddress)))
         return Results.StatusCode(429);
 
     using var reader = new StreamReader(ctx.Request.Body);
@@ -212,16 +249,32 @@ created_at TEXT NOT NULL
     try
     {
         using var doc = JsonDocument.Parse(body);
-        if (doc.RootElement.TryGetProperty("path", out var p))
+        // GetString() throws (a 500) on a non-string element, so check the kind first.
+        // JSON null counts as "not sent", like a missing property.
+        if (doc.RootElement.ValueKind != JsonValueKind.Object)
+            return Error(400, "The request body must be a JSON object.");
+        if (doc.RootElement.TryGetProperty("path", out var p) && p.ValueKind != JsonValueKind.Null)
+        {
+            if (p.ValueKind != JsonValueKind.String)
+                return Error(400, "path must be a string.");
             path = p.GetString() ?? "/";
-        if (doc.RootElement.TryGetProperty("referrer", out var r))
+        }
+        if (doc.RootElement.TryGetProperty("referrer", out var r) && r.ValueKind != JsonValueKind.Null)
+        {
+            if (r.ValueKind != JsonValueKind.String)
+                return Error(400, "referrer must be a string.");
             referrer = r.GetString();
+        }
     }
-    catch (JsonException)
+    catch (Exception ex) when (ex is JsonException or InvalidOperationException)
     {
-        return Results.BadRequest("Invalid JSON");
+        // InvalidOperationException: GetString() on a string with a lone surrogate escape
+        return Error(400, "Invalid JSON");
     }
 
+    if (path.Length > MaxEventPathLength)
+        return Error(400, $"path must be {MaxEventPathLength} characters or fewer.");
+
     // 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.
@@ -237,11 +290,18 @@ created_at TEXT NOT NULL
     // 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.
+    // The hash takes the full address, not the client key: a /64 would count every host in
+    // it as one visitor.
+    var ip = ctx.Connection.RemoteIpAddress?.ToString() ?? "unknown";
     var ua = ctx.Request.Headers.UserAgent.FirstOrDefault() ?? "";
     var day = DateTime.UtcNow.ToString("yyyy-MM-dd");
     var visitorHash = Convert.ToHexString(
         HMACSHA256.HashData(visitorSalt, Encoding.UTF8.GetBytes($"{ip}|{ua}|{day}"))).ToLower()[..16];
 
+    // A full store drops the event and still answers 200: analytics must never show an error
+    if (storageCheck.IsFull())
+        return Results.Ok();
+
     using var conn = new SqliteConnection(connectionString);
     conn.Open();
     using var cmd = conn.CreateCommand();
@@ -409,10 +469,14 @@ GROUP BY referrer ORDER BY count DESC LIMIT 10
 
 app.MapDelete("/api/plans/{id}", (string id, HttpContext ctx) =>
 {
-    var token = ctx.Request.Query["token"].FirstOrDefault();
+    // Header first: a ?token= query string is written to the nginx access log. The query form
+    // stays for clients built before the header existed.
+    var token = ctx.Request.Headers["X-Delete-Token"].FirstOrDefault();
+    if (string.IsNullOrEmpty(token))
+        token = ctx.Request.Query["token"].FirstOrDefault();
     if (string.IsNullOrEmpty(token))
     {
-        return Results.BadRequest("Missing delete token");
+        return Error(400, "Missing delete token");
     }
 
     using var conn = new SqliteConnection(connectionString);
@@ -443,6 +507,12 @@ static string GenerateDeleteToken()
     return Convert.ToHexString(RandomNumberGenerator.GetBytes(16)).ToLower();
 }
 
+// A refusal the web client can show: it reads the "error" text out of the body.
+static IResult Error(int statusCode, string message)
+{
+    return Results.Json(new { error = message }, statusCode: statusCode);
+}
+
 // --- Supporting types ---
 
 record PlanDbConfig(string ConnectionString);
@@ -453,12 +523,14 @@ sealed class CleanupService : BackgroundService
 {
     private readonly PlanDbConfig _config;
     private readonly RateLimiters _rateLimiters;
+    private readonly UploadBudget _uploadBudget;
     private readonly ILogger<CleanupService> _logger;
 
-    public CleanupService(PlanDbConfig config, RateLimiters rateLimiters, ILogger<CleanupService> logger)
+    public CleanupService(PlanDbConfig config, RateLimiters rateLimiters, UploadBudget uploadBudget, ILogger<CleanupService> logger)
     {
         _config = config;
         _rateLimiters = rateLimiters;
+        _uploadBudget = uploadBudget;
         _logger = logger;
     }
 
@@ -503,14 +575,16 @@ private void Cleanup()
 
             // 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.
+            // entry per unique client IP for the lifetime of the process. The upload
+            // budget is swept the same way (its entries go stale at the UTC day change).
             var shareEvicted = _rateLimiters.Share.Sweep();
             var analyticsEvicted = _rateLimiters.Analytics.Sweep();
             var readEvicted = _rateLimiters.Read.Sweep();
-            if (shareEvicted + analyticsEvicted + readEvicted > 0)
+            var budgetEvicted = _uploadBudget.Sweep();
+            if (shareEvicted + analyticsEvicted + readEvicted + budgetEvicted > 0)
                 _logger.LogInformation(
-                    "Evicted {Share} share + {Analytics} analytics + {Read} read rate-limit keys",
-                    shareEvicted, analyticsEvicted, readEvicted);
+                    "Evicted {Share} share + {Analytics} analytics + {Read} read rate-limit keys and {Budget} upload-budget keys",
+                    shareEvicted, analyticsEvicted, readEvicted, budgetEvicted);
         }
         catch (Exception ex)
         {

server/PlanShare/StorageCheck.cs

@@ -0,0 +1,41 @@
+using Microsoft.Data.Sqlite;
+
+namespace PlanShare;
+
+/// <summary>
+/// Tells whether the plan database has reached its size limit. Size means the pages in use:
+/// (page_count - freelist_count) * page_size. The freelist is left out on purpose, because
+/// deleting expired plans returns their pages to it and the file itself does not shrink, so
+/// the file length would stay above the limit after cleanup had freed the room.
+/// </summary>
+internal sealed class StorageCheck
+{
+    private readonly string _connectionString;
+    private readonly long _maxBytes;
+
+    public StorageCheck(string connectionString, long maxBytes)
+    {
+        _connectionString = connectionString;
+        _maxBytes = maxBytes;
+    }
+
+    public long UsedBytes()
+    {
+        using var conn = new SqliteConnection(_connectionString);
+        conn.Open();
+        var pageCount = Pragma(conn, "page_count");
+        var freePages = Pragma(conn, "freelist_count");
+        var pageSize = Pragma(conn, "page_size");
+        return (pageCount - freePages) * pageSize;
+    }
+
+    /// <summary>True at or above the limit.</summary>
+    public bool IsFull() => UsedBytes() >= _maxBytes;
+
+    private static long Pragma(SqliteConnection conn, string name)
+    {
+        using var cmd = conn.CreateCommand();
+        cmd.CommandText = $"PRAGMA {name};";
+        return (long)cmd.ExecuteScalar()!;
+    }
+}