sec: payload validation + reader/writer split + startup-delete (#28)

* sec: validate payload, split reader/writer, add startup-delete

Addresses three findings from the post-deploy security audit:

C3 (handlers): the previous `0x1f 0x8b` magic-byte check let an
attacker store any garbage prefixed with the gzip magic, which then
crashed every viewer. handleCreate now decompresses under a hard cap
(default 4 MiB, --max-inflated-bytes), unmarshals into share.Payload,
and rejects anything whose v != 1 or that fails to parse. Closes the
gzip-bomb-via-server vector too.

H2 (store): writer and reader connection pools opened separately
against the same file. Writer keeps MaxOpenConns(1); reader uses an
8-slot pool with ?mode=ro. A long write no longer stalls GETs.
Added an index on created_at so the retention sweep stops table-
scanning.

Operations (store + main):
- DeleteByCode method for removing a single payload.
- --delete-on-startup flag (comma-separated codes) wired in main.go,
  used by operators to take down a malicious entry on the next deploy
  without needing kubectl exec into the pod.
- --max-inflated-bytes flag exposes the new cap.

Tests:
- TestCreateRejectsGzipMagicWithGarbage
- TestCreateRejectsGzipBomb (cap honoured under attack)
- TestCreateRejectsWrongSchemaVersion
- TestCreateRejectsNotJSON
- TestCreateAcceptsValidPayload (positive control)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* sec: address review feedback on validation + lifecycle

- validatePayload: stream-decode JSON directly off the gzip+LimitReader
  pipeline instead of buffering the whole inflated payload in memory.
  Drops the transient ~2x cap allocation that io.ReadAll caused on
  every POST. Trailing bytes past the cap are drained and rejected.
- Store.Close: join reader and writer close errors via errors.Join so
  neither one disappears during a restart loop.
- startupDelete: validate each comma-separated entry with
  shareapi.ValidCode before issuing DELETE, so operator typos surface
  immediately and arbitrary log content cannot reach the logger.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: dmitriimaksimovdevelop <227611064+dmitriimaksimovdevelop@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: dmitriimaksimovdevelop

cmd/melisai-share-api/main.go

@@ -20,6 +20,7 @@ import (
 	"os"
 	"os/signal"
 	"path/filepath"
+	"strings"
 	"syscall"
 	"time"
 
@@ -28,13 +29,15 @@ import (
 
 func main() {
 	var (
-		addr            = flag.String("addr", ":8080", "HTTP listen address")
-		dbPath          = flag.String("db", "/var/lib/melisai-share-api/store.db", "SQLite database path")
-		publicBase      = flag.String("public-base", "https://melisai.dev/r", "Viewer base URL embedded in POST responses")
-		maxBodyBytes    = flag.Int64("max-body-bytes", shareapi.DefaultMaxBodyBytes, "Max upload size in bytes")
-		logLevel        = flag.String("log-level", "info", "Log level: debug, info, warn, error")
-		retention       = flag.Duration("retention", 0, "Delete reports older than this; 0 disables retention (e.g. 2160h for 90 days)")
-		cleanupInterval = flag.Duration("cleanup-interval", time.Hour, "How often the retention sweep runs (ignored when --retention=0)")
+		addr             = flag.String("addr", ":8080", "HTTP listen address")
+		dbPath           = flag.String("db", "/var/lib/melisai-share-api/store.db", "SQLite database path")
+		publicBase       = flag.String("public-base", "https://melisai.dev/r", "Viewer base URL embedded in POST responses")
+		maxBodyBytes     = flag.Int64("max-body-bytes", shareapi.DefaultMaxBodyBytes, "Max compressed upload size in bytes")
+		maxInflatedBytes = flag.Int64("max-inflated-bytes", shareapi.DefaultMaxInflatedBytes, "Max decompressed payload size in bytes (defends against gzip bombs)")
+		logLevel         = flag.String("log-level", "info", "Log level: debug, info, warn, error")
+		retention        = flag.Duration("retention", 0, "Delete reports older than this; 0 disables retention (e.g. 2160h for 90 days)")
+		cleanupInterval  = flag.Duration("cleanup-interval", time.Hour, "How often the retention sweep runs (ignored when --retention=0)")
+		deleteOnStartup  = flag.String("delete-on-startup", "", "Comma-separated codes to delete from the store at startup, then continue normally")
 	)
 	flag.Parse()
 
@@ -45,13 +48,13 @@ func main() {
 		os.Exit(2)
 	}
 
-	if err := run(*addr, *dbPath, *publicBase, *maxBodyBytes, *retention, *cleanupInterval, logger); err != nil {
+	if err := run(*addr, *dbPath, *publicBase, *maxBodyBytes, *maxInflatedBytes, *retention, *cleanupInterval, *deleteOnStartup, logger); err != nil {
 		logger.Error("server failed", "err", err)
 		os.Exit(1)
 	}
 }
 
-func run(addr, dbPath, publicBase string, maxBodyBytes int64, retention, cleanupInterval time.Duration, log *slog.Logger) error {
+func run(addr, dbPath, publicBase string, maxBodyBytes, maxInflatedBytes int64, retention, cleanupInterval time.Duration, deleteOnStartup string, log *slog.Logger) error {
 	if err := os.MkdirAll(filepath.Dir(dbPath), 0o755); err != nil {
 		return fmt.Errorf("create db dir: %w", err)
 	}
@@ -66,10 +69,17 @@ func run(addr, dbPath, publicBase string, maxBodyBytes int64, retention, cleanup
 		}
 	}()
 
+	if deleteOnStartup != "" {
+		if err := startupDelete(context.Background(), store, deleteOnStartup, log); err != nil {
+			return fmt.Errorf("delete-on-startup: %w", err)
+		}
+	}
+
 	srv, err := shareapi.NewServer(store, publicBase, maxBodyBytes, log)
 	if err != nil {
 		return fmt.Errorf("init server: %w", err)
 	}
+	srv = srv.WithMaxInflatedBytes(maxInflatedBytes)
 
 	httpSrv := &http.Server{
 		Addr:              addr,
@@ -123,6 +133,33 @@ func run(addr, dbPath, publicBase string, maxBodyBytes int64, retention, cleanup
 	return nil
 }
 
+// startupDelete removes the given comma-separated codes from the store
+// once before the HTTP server starts accepting traffic. Used to take
+// down malicious or accidentally-shared payloads without invoking
+// kubectl exec — set --delete-on-startup=Xxxx,Yyyy on the next deploy,
+// observe the log line, then remove the flag.
+//
+// Codes are validated against the same shape as GenerateCode produces,
+// so an operator typo cannot end up logging arbitrary control bytes
+// or silently no-op'ing a delete against a malformed code.
+func startupDelete(ctx context.Context, store *shareapi.Store, codes string, log *slog.Logger) error {
+	for _, code := range strings.Split(codes, ",") {
+		code = strings.TrimSpace(code)
+		if code == "" {
+			continue
+		}
+		if !shareapi.ValidCode(code) {
+			return fmt.Errorf("invalid code %q: must be %d base62 characters", code, shareapi.CodeLength)
+		}
+		n, err := store.DeleteByCode(ctx, code)
+		if err != nil {
+			return fmt.Errorf("delete %q: %w", code, err)
+		}
+		log.Info("delete-on-startup", "code", code, "rows", n)
+	}
+	return nil
+}
+
 // runRetention drops rows older than `age` on the given interval until
 // ctx is cancelled. The first sweep fires immediately on startup to
 // shrink any backlog from a previous run with longer retention.

internal/shareapi/handlers.go

@@ -1,6 +1,8 @@
 package shareapi
 
 import (
+	"bytes"
+	"compress/gzip"
 	"context"
 	"encoding/json"
 	"errors"
@@ -10,13 +12,22 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
+
+	"github.com/dmitriimaksimovdevelop/melisai/internal/share"
 )
 
-// DefaultMaxBodyBytes caps the size of an uploaded payload at 1 MB.
-// melisai's gzip-compressed summary is ~1–10 KB in practice; 1 MB
-// leaves a generous safety margin without making bombing easy.
+// DefaultMaxBodyBytes caps the size of an uploaded *compressed* payload
+// at 1 MB. melisai's gzip-compressed summary is ~1–10 KB in practice;
+// 1 MB leaves a generous safety margin without making bombing easy.
 const DefaultMaxBodyBytes = 1 << 20
 
+// DefaultMaxInflatedBytes caps the *decompressed* size we are willing
+// to accept and store. 4 MiB is ~160x a realistic standard-profile
+// summary; anything larger is almost certainly a gzip bomb. Without
+// this cap a 1 MB POST can store bytes that fan out to 1 GB on the
+// viewer (max DEFLATE ratio ≈ 1032×), OOMing every visitor's browser.
+const DefaultMaxInflatedBytes = 4 << 20
+
 // retryOnCollision is how many fresh codes we try before giving up on
 // an insert. With 62^8 ≈ 2×10^14 slots, three retries is already
 // extraordinary luck running out — anything beyond hints at storage
@@ -25,10 +36,11 @@ const retryOnCollision = 3
 
 // Server is the HTTP layer.
 type Server struct {
-	store        *Store
-	publicBase   string // e.g. "https://melisai.dev/r" — used to build the URL returned by POST
-	maxBodyBytes int64
-	log          *slog.Logger
+	store            *Store
+	publicBase       string // e.g. "https://melisai.dev/r" — used to build the URL returned by POST
+	maxBodyBytes     int64
+	maxInflatedBytes int64
+	log              *slog.Logger
 }
 
 // NewServer wires the dependencies. publicBase must be a valid http(s)
@@ -46,13 +58,23 @@ func NewServer(store *Store, publicBase string, maxBodyBytes int64, log *slog.Lo
 		log = slog.Default()
 	}
 	return &Server{
-		store:        store,
-		publicBase:   strings.TrimRight(publicBase, "/"),
-		maxBodyBytes: maxBodyBytes,
-		log:          log,
+		store:            store,
+		publicBase:       strings.TrimRight(publicBase, "/"),
+		maxBodyBytes:     maxBodyBytes,
+		maxInflatedBytes: DefaultMaxInflatedBytes,
+		log:              log,
 	}, nil
 }
 
+// WithMaxInflatedBytes overrides the decompressed-size cap. Pass <=0
+// to keep the default. Returned for fluent configuration in main.go.
+func (s *Server) WithMaxInflatedBytes(n int64) *Server {
+	if n > 0 {
+		s.maxInflatedBytes = n
+	}
+	return s
+}
+
 // Handler returns the HTTP handler covering /healthz and /api/r.
 // Mount this directly on a net/http server.
 func (s *Server) Handler() http.Handler {
@@ -95,8 +117,9 @@ func (s *Server) handleHealthz(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) handleCreate(w http.ResponseWriter, r *http.Request) {
-	// Cap the body before anything else — http.MaxBytesReader returns a
-	// typed error after the limit so the read fails fast on huge uploads.
+	// Cap the compressed body before anything else — http.MaxBytesReader
+	// returns a typed error after the limit so the read fails fast on
+	// huge uploads.
 	r.Body = http.MaxBytesReader(w, r.Body, s.maxBodyBytes)
 	defer r.Body.Close()
 
@@ -115,13 +138,24 @@ func (s *Server) handleCreate(w http.ResponseWriter, r *http.Request) {
 		writeError(w, http.StatusBadRequest, "empty body")
 		return
 	}
-	// Sanity-check the gzip magic so we reject obvious junk upfront.
-	// The viewer page assumes gzip; storing anything else is a footgun.
+	// Sanity-check the gzip magic so we reject obvious junk upfront —
+	// cheap fast-path before allocating a gzip.Reader.
 	if len(payload) < 2 || payload[0] != 0x1f || payload[1] != 0x8b {
 		writeError(w, http.StatusBadRequest, "body must be gzip-compressed")
 		return
 	}
 
+	// Decompress under a hard size cap, decode as a typed Payload, and
+	// reject anything we cannot serve back to a viewer. This is the
+	// defense against:
+	//   * gzip bombs (1 MB POST → 1 GB inflate → browser OOM on /r/{code})
+	//   * gzip-magic-prefixed garbage that stores fine but breaks viewers
+	//   * future schema bumps stored against old viewers (v != 1)
+	if err := s.validatePayload(payload); err != nil {
+		writeError(w, http.StatusBadRequest, err.Error())
+		return
+	}
+
 	code, err := s.putWithRetry(r.Context(), payload)
 	if err != nil {
 		s.log.Error("create: store put failed", "err", err)
@@ -135,6 +169,48 @@ func (s *Server) handleCreate(w http.ResponseWriter, r *http.Request) {
 	_ = json.NewEncoder(w).Encode(resp)
 }
 
+// validatePayload inflates payload under a size cap and confirms it
+// parses as a current-schema share.Payload. It returns a user-facing
+// error message on rejection.
+//
+// We stream-decode JSON directly off the gzip reader (wrapped in a
+// LimitReader) rather than holding the whole inflated buffer in
+// memory. This keeps peak transient allocation bounded by json's
+// internal buffer plus the unmarshaled struct, instead of 2× the cap
+// that io.ReadAll would temporarily reserve.
+func (s *Server) validatePayload(payload []byte) error {
+	gz, err := gzip.NewReader(bytes.NewReader(payload))
+	if err != nil {
+		return fmt.Errorf("payload is not valid gzip: %w", err)
+	}
+	defer gz.Close()
+
+	// We accept unknown JSON fields on purpose: future v=1-compatible
+	// clients may add optional fields (e.g. an extra meta.* tag); the
+	// viewer already ignores them, so the server should too.
+	limited := io.LimitReader(gz, s.maxInflatedBytes+1)
+	var parsed share.Payload
+	if err := json.NewDecoder(limited).Decode(&parsed); err != nil {
+		return fmt.Errorf("payload is not valid melisai share JSON: %w", err)
+	}
+	// After Decode succeeds, drain the rest under the same cap. If any
+	// bytes remain past maxInflatedBytes we know the payload was a
+	// bomb that happened to be valid JSON in its prefix — reject it.
+	overflow, err := io.Copy(io.Discard, limited)
+	if err != nil {
+		return fmt.Errorf("read trailing payload: %w", err)
+	}
+	if overflow > 0 {
+		// Decode advanced exactly maxInflatedBytes — anything left in
+		// the gzip stream past the cap is over-budget.
+		return fmt.Errorf("decompressed payload exceeds %d bytes (likely a gzip bomb)", s.maxInflatedBytes)
+	}
+	if parsed.V != share.PayloadVersion {
+		return fmt.Errorf("unsupported payload version %d (server expects %d)", parsed.V, share.PayloadVersion)
+	}
+	return nil
+}
+
 func (s *Server) handleFetch(w http.ResponseWriter, r *http.Request) {
 	code := r.PathValue("code")
 	if !ValidCode(code) {

internal/shareapi/handlers_test.go

@@ -24,6 +24,12 @@ func gzipBytes(t *testing.T, data string) []byte {
 	return buf.Bytes()
 }
 
+// validPayloadGzip returns the minimum payload accepted by handleCreate
+// after the C3 validation: schema v=1, parses into share.Payload.
+func validPayloadGzip(t *testing.T) []byte {
+	return gzipBytes(t, `{"v":1,"meta":{"hostname":"h","kernel":"k","cpus":1,"memory_gb":1,"profile":"quick","duration":"10s","timestamp":"t","tool_version":"v"},"summary":{"health_score":100,"anomalies":[],"resources":{},"recommendations":[]}}`)
+}
+
 func newTestServer(t *testing.T) (*Server, *httptest.Server) {
 	t.Helper()
 	store := openTestStore(t)
@@ -38,7 +44,7 @@ func newTestServer(t *testing.T) (*Server, *httptest.Server) {
 
 func TestCreateAndFetchRoundTrip(t *testing.T) {
 	_, ts := newTestServer(t)
-	payload := gzipBytes(t, `{"v":1,"hello":"world"}`)
+	payload := validPayloadGzip(t)
 
 	// POST
 	resp, err := http.Post(ts.URL+"/api/r", "application/octet-stream", bytes.NewReader(payload))
@@ -97,6 +103,82 @@ func TestCreateRejectsNonGzip(t *testing.T) {
 	}
 }
 
+// TestCreateRejectsGzipMagicWithGarbage covers the C3 finding from the
+// security audit: previously the handler only verified the two gzip
+// magic bytes, so a payload starting with 1f 8b followed by random
+// data was happily stored and later crashed every viewer.
+func TestCreateRejectsGzipMagicWithGarbage(t *testing.T) {
+	_, ts := newTestServer(t)
+	payload := append([]byte{0x1f, 0x8b}, []byte("not a real gzip stream")...)
+	resp, err := http.Post(ts.URL+"/api/r", "application/octet-stream", bytes.NewReader(payload))
+	if err != nil {
+		t.Fatalf("POST: %v", err)
+	}
+	if resp.StatusCode != http.StatusBadRequest {
+		t.Errorf("status = %d, want 400", resp.StatusCode)
+	}
+}
+
+func TestCreateRejectsGzipBomb(t *testing.T) {
+	store := openTestStore(t)
+	srv, err := NewServer(store, "https://example.test/r", 0, nil)
+	if err != nil {
+		t.Fatalf("NewServer: %v", err)
+	}
+	srv = srv.WithMaxInflatedBytes(1024) // 1 KiB cap for this test
+	ts := httptest.NewServer(srv.Handler())
+	defer ts.Close()
+
+	// Compress 16 KiB of zeros — gzip squeezes this to <100 bytes but
+	// inflates back to 16 KiB, well over our test cap.
+	bomb := gzipBytes(t, strings.Repeat("\x00", 16*1024))
+	resp, err := http.Post(ts.URL+"/api/r", "application/octet-stream", bytes.NewReader(bomb))
+	if err != nil {
+		t.Fatalf("POST: %v", err)
+	}
+	if resp.StatusCode != http.StatusBadRequest {
+		body, _ := io.ReadAll(resp.Body)
+		t.Errorf("bomb status = %d, want 400, body=%s", resp.StatusCode, body)
+	}
+}
+
+func TestCreateRejectsWrongSchemaVersion(t *testing.T) {
+	_, ts := newTestServer(t)
+	body := gzipBytes(t, `{"v":99,"meta":{},"summary":{}}`)
+	resp, err := http.Post(ts.URL+"/api/r", "application/octet-stream", bytes.NewReader(body))
+	if err != nil {
+		t.Fatalf("POST: %v", err)
+	}
+	if resp.StatusCode != http.StatusBadRequest {
+		t.Errorf("status = %d, want 400 (unsupported version)", resp.StatusCode)
+	}
+}
+
+func TestCreateRejectsNotJSON(t *testing.T) {
+	_, ts := newTestServer(t)
+	body := gzipBytes(t, "this is not json at all, just some text")
+	resp, err := http.Post(ts.URL+"/api/r", "application/octet-stream", bytes.NewReader(body))
+	if err != nil {
+		t.Fatalf("POST: %v", err)
+	}
+	if resp.StatusCode != http.StatusBadRequest {
+		t.Errorf("status = %d, want 400", resp.StatusCode)
+	}
+}
+
+func TestCreateAcceptsValidPayload(t *testing.T) {
+	_, ts := newTestServer(t)
+	body := gzipBytes(t, `{"v":1,"meta":{"hostname":"h","kernel":"k","cpus":1,"memory_gb":1,"profile":"quick","duration":"10s","timestamp":"now","tool_version":"0.0.0"},"summary":{"health_score":100,"anomalies":[],"resources":{},"recommendations":[]}}`)
+	resp, err := http.Post(ts.URL+"/api/r", "application/octet-stream", bytes.NewReader(body))
+	if err != nil {
+		t.Fatalf("POST: %v", err)
+	}
+	if resp.StatusCode != http.StatusCreated {
+		b, _ := io.ReadAll(resp.Body)
+		t.Errorf("status = %d, want 201, body=%s", resp.StatusCode, b)
+	}
+}
+
 func TestCreateRejectsEmpty(t *testing.T) {
 	_, ts := newTestServer(t)
 	resp, err := http.Post(ts.URL+"/api/r", "application/octet-stream",