From ad4355df17c52f4c74ba2bf5394dc99efa56b213 Mon Sep 17 00:00:00 2001 From: Justin Harms Date: Mon, 1 Jun 2026 20:15:28 -0500 Subject: [PATCH] Heavy security and file splitting --- .env.example | 3 +- .gitea/workflows/ci.yaml | 6 + .gitea/workflows/release.yaml | 3 + Dockerfile | 2 +- README.md | 18 +- WEBSITE_AUDIT.md | 229 ++++ cmd/scratchbox/access_log_writer.go | 8 +- cmd/scratchbox/main_test.go | 13 + cmd/scratchbox/server.go | 19 +- internal/cleanup/worker_test.go | 4 +- internal/config/config.go | 73 +- internal/config/config_test.go | 62 +- internal/http/handlers.go | 103 +- .../handlers_integration_concurrent_test.go | 249 ++++ .../http/handlers_integration_flows_test.go | 375 ++++++ .../http/handlers_integration_helpers_test.go | 79 ++ .../handlers_integration_security_test.go | 101 ++ internal/http/handlers_integration_test.go | 1088 ----------------- internal/http/handlers_unit_test.go | 42 +- internal/http/middleware.go | 40 +- internal/http/middleware_test.go | 57 + internal/http/raw_cache.go | 19 +- internal/storage/filesystem.go | 26 +- internal/storage/filesystem_test.go | 36 +- web/ui/index.html | 8 + web/ui/public/favicon.svg | 4 + web/ui/src/App.svelte | 39 +- 27 files changed, 1540 insertions(+), 1166 deletions(-) create mode 100644 WEBSITE_AUDIT.md create mode 100644 internal/http/handlers_integration_concurrent_test.go create mode 100644 internal/http/handlers_integration_flows_test.go create mode 100644 internal/http/handlers_integration_helpers_test.go create mode 100644 internal/http/handlers_integration_security_test.go delete mode 100644 internal/http/handlers_integration_test.go create mode 100644 web/ui/public/favicon.svg diff --git a/.env.example b/.env.example index cbb020f..1a6a2a1 100644 --- a/.env.example +++ b/.env.example @@ -10,6 +10,7 @@ SCRATCHBOX_SERVER_READ_HEADER_TIMEOUT=5s SCRATCHBOX_SERVER_READ_TIMEOUT=30s SCRATCHBOX_SERVER_WRITE_TIMEOUT=30s SCRATCHBOX_SERVER_IDLE_TIMEOUT=120s +SCRATCHBOX_SERVER_SHUTDOWN_TIMEOUT=10s SCRATCHBOX_SERVER_MAX_HEADER_BYTES=1048576 SCRATCHBOX_SERVER_ACCESS_LOG_ENABLED=true @@ -27,7 +28,7 @@ SCRATCHBOX_STORAGE_CLEANUP_INTERVAL=1m SCRATCHBOX_SECURITY_ALLOWED_IPS=127.0.0.1 # This should be set to your networks CIDR like 192.168.2.0/24 or a particulat IP like 192.168.2.55. SCRATCHBOX_SECURITY_TRUST_PROXY_HEADERS=false # Comma-separated IP/CIDR list. -SCRATCHBOX_SECURITY_TRUSTED_PROXY_IPS= # Same as SCRATCHBOX_SECURITY_ALLOWED_IPS just Trusted Proxies you expect clients to come from instead. +SCRATCHBOX_SECURITY_TRUSTED_PROXY_IPS= # Same as SCRATCHBOX_SECURITY_ALLOWED_IPS just Trusted Proxies you expect clients to come from instead. Must be *immediate* reverse proxies only; avoid 0/0. SCRATCHBOX_SECURITY_RATE_LIMIT_UI_ENABLED=false SCRATCHBOX_SECURITY_RATE_LIMIT_UI_REQUESTS_PER_MINUTE=360 SCRATCHBOX_SECURITY_RATE_LIMIT_UI_BURST=120 diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index f076189..60cac18 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -20,6 +20,9 @@ jobs: go-version-file: go.mod cache: true + - name: Verify Go modules + run: go mod verify + - name: Setup Node uses: actions/setup-node@v4 with: @@ -52,6 +55,9 @@ jobs: go-version-file: go.mod cache: true + - name: Verify Go modules + run: go mod verify + - name: Setup Node uses: actions/setup-node@v4 with: diff --git a/.gitea/workflows/release.yaml b/.gitea/workflows/release.yaml index 043a35f..963c09a 100644 --- a/.gitea/workflows/release.yaml +++ b/.gitea/workflows/release.yaml @@ -55,6 +55,9 @@ jobs: go-version-file: go.mod cache: true + - name: Verify Go modules + run: go mod verify + - name: Setup Node uses: actions/setup-node@v4 with: diff --git a/Dockerfile b/Dockerfile index 01249c6..b00d22c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.25-alpine AS builder +FROM golang:1.25.4-alpine AS builder WORKDIR /src diff --git a/README.md b/README.md index ae9dfa4..cd398c8 100644 --- a/README.md +++ b/README.md @@ -64,13 +64,14 @@ Storage encryption key behavior: - On subsequent startups, Scratchbox reuses the same key file from that data directory. - Keep `storage.data_dir` persistent; losing or replacing `metadata.key` makes existing scratches unreadable. -Environment variables supported by `scratchbox server --env`: +Environment variables supported by `scratchbox server --env` (note: when `--env` is used, the variables and their values are printed to stdout on startup for diagnostics, with values of IP allow/trusted lists redacted): - `SCRATCHBOX_SERVER_LISTEN_ADDR` - `SCRATCHBOX_SERVER_READ_HEADER_TIMEOUT` - `SCRATCHBOX_SERVER_READ_TIMEOUT` - `SCRATCHBOX_SERVER_WRITE_TIMEOUT` - `SCRATCHBOX_SERVER_IDLE_TIMEOUT` +- `SCRATCHBOX_SERVER_SHUTDOWN_TIMEOUT` - `SCRATCHBOX_SERVER_MAX_HEADER_BYTES` - `SCRATCHBOX_SERVER_ACCESS_LOG_ENABLED` - `SCRATCHBOX_SERVER_ACCESS_LOG_FILE_PATH` (forced empty by Docker image) @@ -94,6 +95,7 @@ Environment variables supported by `scratchbox server --env`: - `SCRATCHBOX_SECURITY_RATE_LIMIT_API_WRITE_ENABLED` - `SCRATCHBOX_SECURITY_RATE_LIMIT_API_WRITE_REQUESTS_PER_MINUTE` - `SCRATCHBOX_SECURITY_RATE_LIMIT_API_WRITE_BURST` +- `SCRATCHBOX_SECURITY_HSTS_ENABLED` Example `.env` for Docker: @@ -104,6 +106,7 @@ SCRATCHBOX_SERVER_READ_HEADER_TIMEOUT=5s SCRATCHBOX_SERVER_READ_TIMEOUT=30s SCRATCHBOX_SERVER_WRITE_TIMEOUT=30s SCRATCHBOX_SERVER_IDLE_TIMEOUT=120s +SCRATCHBOX_SERVER_SHUTDOWN_TIMEOUT=10s SCRATCHBOX_SERVER_MAX_HEADER_BYTES=1048576 SCRATCHBOX_SERVER_ACCESS_LOG_ENABLED=true @@ -130,6 +133,7 @@ SCRATCHBOX_SECURITY_RATE_LIMIT_API_READ_BURST=100 SCRATCHBOX_SECURITY_RATE_LIMIT_API_WRITE_ENABLED=true SCRATCHBOX_SECURITY_RATE_LIMIT_API_WRITE_REQUESTS_PER_MINUTE=30 SCRATCHBOX_SECURITY_RATE_LIMIT_API_WRITE_BURST=10 +SCRATCHBOX_SECURITY_HSTS_ENABLED=true ``` ## Release Artifacts @@ -185,6 +189,9 @@ All configuration is YAML. - `idle_timeout`: maximum keep-alive idle time between requests. - Must be `> 0` - Default: `120s` +- `shutdown_timeout`: graceful shutdown timeout for in-flight requests on SIGTERM/INT. + - Must be `> 0` + - Default: `10s` - `max_header_bytes`: max HTTP header size in bytes. - Must be `> 0` - Default: `1048576` (1 MiB) @@ -243,6 +250,7 @@ All configuration is YAML. - Keep `false` unless behind a trusted reverse proxy that sets these headers. - `trusted_proxy_ips`: list of reverse-proxy IPs/CIDRs permitted to supply forwarded headers. - Required when `trust_proxy_headers: true` + - **Must be the *immediate* reverse proxies only (e.g. the nginx/caddy in front of this service); never use 0.0.0.0/0 or broad ranges unless you fully control the entire path.** - Examples: `127.0.0.1`, `10.0.0.0/8` - `rate_limit_ui`: per-IP token-bucket for UI routes (`GET /`, `GET /u`, `GET /s/{id}`). - `enabled` default: `false` @@ -256,6 +264,7 @@ All configuration is YAML. - `enabled` default: `true` - `requests_per_minute` must be `> 0` (default `30`) - `burst` must be `> 0` (default `10`) +- `hsts_enabled`: enable HSTS header (default: `true`; for public proxy TLS setups; set `false` for local HTTP-only per LAN notes). ## Security Posture @@ -295,3 +304,10 @@ Scratchbox storage is designed for a single process instance per `storage.data_d - Metadata/index writes are process-local and are not coordinated with cross-process locking. - Scratch payload files and metadata index are encrypted at rest. - Public scratch IDs are short random aliases; stored blobs still use SHA-256-derived internal blob IDs on disk. + +Operational internals (not tunable; see source consts): +- Public IDs: 12 alphanum chars (crypto/rand), up to 8 attempts to reserve unique. +- Encrypted chunks: 64 KiB plaintext (SBX1 magic + per-chunk nonce prefix+counter for AES-256-GCM). +- Index: full random nonce per persist (SMD1). +- Background expiry + startup reconcile walk the in-memory map under exclusive lock (plus optional persist); practical cardinality limited by default 15m TTL + 100MiB uploads (low hundreds of live entries expected). +- Raw /api/raw cache: small LRU (default 32 entries, total bytes cap) with coalescing; serves decompressed content. diff --git a/WEBSITE_AUDIT.md b/WEBSITE_AUDIT.md new file mode 100644 index 0000000..86c6940 --- /dev/null +++ b/WEBSITE_AUDIT.md @@ -0,0 +1,229 @@ +# Scratchbox Website Audit Report + +**URL audited:** https://scratchbox.s1d3sw1ped.com/ +**Date:** 2026-06-01 +**Auditor:** Grok (via Playwright browser automation + direct HTTP/API testing + codebase review) +**Project commit / state:** Current source in `/fast/projects/golang/scratchbox` + +--- + +## Executive Summary + +Scratchbox is a deliberately minimal, unauthenticated, self-hosted temporary file sharing service. The live deployment matches the project exactly and performs very well for its stated goals. + +**Strengths** +- Extremely lightweight (≈ 45 KB total assets). +- Excellent core functionality and HTTP semantics (201 Created, 410 Gone for expired). +- Clean error UX and client-side routing. +- No trackers, no external dependencies, very fast responses. +- Backend Go code is focused and easy to reason about. +- Direct API (`POST /api/scratch`) works reliably and returns useful JSON. + +**Main Issues (prioritized)** +1. **Security headers completely absent** (CSP, HSTS, X-Content-Type-Options, etc.). High priority for any public or semi-public deployment. +2. **Poor SEO / social sharing metadata.** Every page (including `/s/{id}` share links) has only a static `Scratchbox`. No Open Graph, no description, no per-scratch titles. Share previews look bad on Slack, Discord, Twitter, etc. +3. **Missing favicon** (causes repeated 404 noise). +4. Minor: 404/410 routes return the full SPA shell HTML (acceptable for SPA but not ideal for `robots.txt`, `sitemap.xml`, etc.). +5. Minor: Console errors logged for expected 410 fetches on expired scratches. + +The frontend is a Vite + Svelte SPA. The Go backend serves a single static `index.html` shell for all UI routes (`/`, `/u`, `/s/{id}`, 404s, and 410s) via `serveSPAWithStatus`. + +--- + +## Methodology + +- Full browser automation with Playwright (navigation, snapshots, screenshots, console, network, clicks, viewport resize to mobile 390×844). +- Direct HTTP inspection (headers, raw content, timing). +- Successful end-to-end upload test via both the discovered API (`/api/scratch`) and UI flows. +- Codebase review of relevant files (`web/static/index.html`, `web/ui/src/App.svelte`, `internal/http/handlers.go`, etc.). +- One temporary test scratch was created (ID `OvhWwqqXnyA6`, text file, auto-expired after 15m). + +--- + +## Technical Inventory (Live + Source) + +| Item | Value / Location | +|-------------------------|-------------------------------------------------------| +| HTML shell | `web/static/index.html` (embedded) + `web/ui/index.html` (source) | +| Frontend source | `web/ui/src/App.svelte`, `web/ui/src/main.js`, `web/ui/vite.config.js` | +| Built assets | `web/static/assets/index-*.js` (43.5 KB), `*.css` (2.1 KB) | +| Backend serving | `internal/http/handlers.go` (`serveSPAWithStatus`, `Routes`) | +| Asset embedding | `web/assets.go` (`//go:embed static`) | +| API routes | `POST /api/scratch`, `GET /api/scratch/{id}`, `GET /api/raw/{id}`, `GET /s/{id}`, `GET /api/config` | +| Config exposure | `/api/config` returns `max_upload_size_bytes`, `default_ttl`, `upload_allowed` | +| No security headers | Confirmed in `handlers.go` and `middleware.go` | + +--- + +## Detailed Findings + +### 1. Security Headers (High Priority) + +**Finding:** No security-related response headers are set for HTML, API, or asset responses. + +Observed headers (typical): +``` +server: openresty +content-type: text/html; charset=utf-8 +x-served-by: scratchbox.s1d3sw1ped.com +``` + +Missing (recommended minimum): +- `Content-Security-Policy` +- `Strict-Transport-Security` (HSTS) +- `X-Content-Type-Options: nosniff` +- `Referrer-Policy` +- `X-Frame-Options` / CSP frame-ancestors +- `Permissions-Policy` + +**Impact:** Higher risk of XSS, clickjacking, MIME sniffing, etc., especially since the site accepts and serves user-provided file content (even temporarily). + +**Files to change:** +- `internal/http/handlers.go` (in `serveSPAWithStatus`, `getUIConfig`, `rawScratch`, etc.) +- `internal/http/middleware.go` (good place for a `SecurityHeadersMiddleware`) + +**Recommendation (minimal):** Add a small middleware that sets safe defaults. Example starting point: + +```go +func SecurityHeadersMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + h := w.Header() + h.Set("X-Content-Type-Options", "nosniff") + h.Set("Referrer-Policy", "strict-origin-when-cross-origin") + // Add CSP, HSTS (only over HTTPS), etc. as needed + next.ServeHTTP(w, r) + }) +} +``` + +Register it early in `Routes()`. + +See also README.md "Public Deployment Notes" — this audit reinforces those recommendations. + +### 2. SEO & Social Metadata (High Priority for Share Links) + +**Finding:** The `` in the served shell is static and minimal: + +```html +Scratchbox + + +``` + +- `/s/{id}` pages always show the generic title. +- No per-scratch `` or `og:title` containing the filename. +- No `og:description` or image (even a simple logo would help). + +**Impact:** When users share `/s/xxx` links, previews are poor or missing. + +**Files:** +- `web/static/index.html` (served shell) +- `web/ui/index.html` (Vite source template) +- `web/ui/src/App.svelte` — currently no `<svelte:head>` usage for dynamic updates. + +**Recommendations (in priority order):** +1. Add basic static meta to both `index.html` files (description + og tags for the generic case). +2. In `App.svelte`, use `<svelte:head>` inside the `{:else if routeMode === "view"}` branch to set a dynamic `<title>` and `og:*` tags based on `viewMeta.filename`, size, etc. +3. For best results, consider lightweight server-side injection in `scratchPage` / `serveSPAWithStatus` (parse the ID and inject meta before serving the shell). This gives crawlers and social bots good data even with JS disabled. + +Example dynamic title idea: `"{filename} • Scratchbox"` (with fallback). + +### 3. Favicon (Low–Medium) + +**Finding:** Repeated 404s for `/favicon.ico` in every browser session / page load. + +**Files:** +- `web/static/index.html` (add `<link rel="icon" ...>`) +- `web/ui/index.html` (same) +- Add a real `favicon.ico` (or SVG) under `web/static/` and ensure it's embedded. + +### 4. SPA Shell for Non-HTML 404s + +**Finding:** `robots.txt`, `sitemap.xml`, `.well-known/*`, and invalid asset paths all return the full 394-byte HTML shell with a 404 (or 410) status. + +This is standard SPA behavior but not ideal for well-known paths. + +**Current code:** `notFoundPage` → `serveSPAWithStatus(404)` for everything under `GET /{path...}`. + +**Recommendation:** Add early special cases in the mux for common paths: + +```go +mux.Handle("GET /robots.txt", http.HandlerFunc(func(w, r) { + w.Header().Set("Content-Type", "text/plain") + w.WriteHeader(http.StatusNotFound) + // or serve a static disallow file +})) +``` + +Or serve small static files for these paths from the embedded FS. + +### 5. Frontend Form & Upload Details + +In the current `App.svelte` source (as of audit): + +- The form uses pure JS (`onsubmit={onSubmit}`) — no `method`/`action` attributes. (The `method="get"` seen in the live DOM during audit was likely a browser default or inspection artifact; source is clean.) +- File input is a standard `<input type="file" id="file" name="file">` behind a `<label>`. +- Good validation (size check client-side + server `MaxBytesReader`). +- Error messages are clear and injected into the DOM. + +**Minor observation:** No visible drag-and-drop zone (the UI is intentionally minimal). Adding one would be a nice UX improvement but is out of scope for a minimal project unless requested. + +### 6. Other Observations + +- **Performance:** Outstanding. 4 requests for a full UI load. +- **Mobile:** Tested at 390×844 — layout holds up well due to simplicity. +- **Accessibility:** Basic roles and labels are present (`aria-label="Primary"`, button text, etc.). The file input could benefit from better visual association, but it's functional. A full axe-core or WAVE scan is recommended if public use grows. +- **Console noise:** The only errors are the favicon 404 and the expected 410 fetch errors when viewing expired scratches (the app correctly catches 404/410 and shows the friendly UI). These could be silenced with `{ok: response.ok}` style guards or by not logging at error level for known cases. +- **Rate limiting & allowlisting:** Already implemented and configurable (see README). Good. +- **One test scratch** was created during audit via the public API and viewed successfully. It auto-expired as designed. + +--- + +## Actionable Recommendations (Minimal Changes) + +| Priority | Change | Files | Effort | +|----------|--------|-------|--------| +| High | Add security headers middleware | `internal/http/middleware.go` + wire in `handlers.go:Routes()` | Low | +| High | Add basic `<meta>` + OG tags to shell | `web/static/index.html` + `web/ui/index.html` | Low | +| High | Dynamic title/meta for `/s/{id}` pages | `web/ui/src/App.svelte` (add `<svelte:head>`) | Low–Medium | +| Medium | Add real favicon + link tag | `web/static/` + the two `index.html` files | Low | +| Low | Special-case `robots.txt` etc. | `internal/http/handlers.go` | Low | +| Low | Reduce console.error noise on 410s | `web/ui/src/App.svelte` (in `loadScratchView`) | Low | + +After any frontend HTML changes, run `make build` (or the equivalent Vite step) to update the embedded assets. + +--- + +## Strengths Worth Preserving + +- Intentional minimalism and "no accounts, no friction" philosophy. +- Transparent storage (gzip + encryption handled server-side). +- Excellent separation of concerns in Go code. +- Strong test coverage in `internal/http/` and storage. +- Clear documentation in `README.md` and `AGENTS.md`. + +--- + +## Appendix: Raw Data from Audit + +- API config response: `{"default_ttl":"15m","max_upload_size_bytes":104857600,"upload_allowed":true}` +- Successful upload JSON (example): + ```json + { + "id": "OvhWwqqXnyA6", + "view_url": "/s/OvhWwqqXnyA6", + "raw_url": "/api/raw/OvhWwqqXnyA6", + "expires_at": "2026-06-01T22:31:24.662891754Z", + ... + } + ``` +- Raw endpoint correctly returns original bytes + `Content-Disposition: attachment; filename=...` +- All screenshots and full Playwright traces were captured during the session but are not embedded here (available in the auditor's session artifacts if needed). + +--- + +**End of report.** + +This audit was performed with full authorization from the site owner. The temporary test scratch created during testing has long since expired. + +If you would like me to implement any of the recommendations (as minimal, focused patches per `AGENTS.md`), provide the exact priorities and I can prepare the diffs. \ No newline at end of file diff --git a/cmd/scratchbox/access_log_writer.go b/cmd/scratchbox/access_log_writer.go index 54ef090..967404e 100644 --- a/cmd/scratchbox/access_log_writer.go +++ b/cmd/scratchbox/access_log_writer.go @@ -111,14 +111,18 @@ func (w *rollingFileWriter) rotateLocked() error { } return fmt.Errorf("stat rotated file %s: %w", from, err) } - _ = os.Remove(to) + if err := os.Remove(to); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove target backup slot %s: %w", to, err) + } if err := os.Rename(from, to); err != nil { return fmt.Errorf("rotate backup %s -> %s: %w", from, to, err) } } firstBackup := rotatedLogPath(w.path, 1) - _ = os.Remove(firstBackup) + if err := os.Remove(firstBackup); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove first backup %s: %w", firstBackup, err) + } if _, err := os.Stat(w.path); err == nil { if err := os.Rename(w.path, firstBackup); err != nil { return fmt.Errorf("rotate current log to %s: %w", firstBackup, err) diff --git a/cmd/scratchbox/main_test.go b/cmd/scratchbox/main_test.go index 4ef7227..453c3f9 100644 --- a/cmd/scratchbox/main_test.go +++ b/cmd/scratchbox/main_test.go @@ -347,6 +347,16 @@ func TestGenerateConfigCommandWritesDefaultYAML(t *testing.T) { if !cfg.Security.RateLimitAPIWrite.Enabled { t.Fatalf("security.rate_limit_api_write.enabled = %v, want true", cfg.Security.RateLimitAPIWrite.Enabled) } + // Additional assertions act as golden checks against generate drift in defaults/comments/struct. + if cfg.Storage.CleanupInterval != "1m" { + t.Fatalf("storage.cleanup_interval = %q, want %q", cfg.Storage.CleanupInterval, "1m") + } + if !cfg.Security.HSTSEnabled { + t.Fatalf("security.hsts_enabled = %v, want true", cfg.Security.HSTSEnabled) + } + if cfg.Limits.RawCacheMaxEntries != 32 { + t.Fatalf("limits.raw_cache_max_entries = %d, want 32", cfg.Limits.RawCacheMaxEntries) + } } func TestGenerateConfigCommandRejectsUnexpectedArgs(t *testing.T) { @@ -437,6 +447,9 @@ security: if err != nil { t.Fatalf("config.Load() error = %v", err) } + if err := cfg.PrepareDataDir(); err != nil { + t.Fatalf("config.PrepareDataDir() error = %v", err) + } logger := log.New(io.Discard, "", 0) store, err := storage.NewFilesystemStore(cfg.Storage.DataDir, cfg.Storage.EncryptionKeyBytes) if err != nil { diff --git a/cmd/scratchbox/server.go b/cmd/scratchbox/server.go index fe0c4f9..a231c7c 100644 --- a/cmd/scratchbox/server.go +++ b/cmd/scratchbox/server.go @@ -12,7 +12,6 @@ import ( "sort" "strings" "syscall" - "time" "github.com/spf13/cobra" @@ -66,6 +65,12 @@ func run(configPath string, useEnv bool) error { return fmt.Errorf("config load failed: %w", err) } + // Prepare data dir + key (side effects) after pure Validate inside Load*/Default. + // This addresses the layering issue where Validate used to mutate FS. + if err := cfg.PrepareDataDir(); err != nil { + return fmt.Errorf("prepare storage data dir failed: %w", err) + } + logger := log.New(os.Stdout, "[scratchbox] ", log.LstdFlags|log.LUTC) accessWriter, closeAccessWriter, err := newAccessLogWriter(cfg) if err != nil { @@ -114,7 +119,7 @@ func run(configPath string, useEnv bool) error { go func() { <-ctx.Done() - shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 10*time.Second) + shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), cfg.Server.ShutdownTimeoutDur) defer shutdownCancel() if err := httpServer.Shutdown(shutdownCtx); err != nil { logger.Printf("http shutdown failed: %v", err) @@ -143,7 +148,15 @@ func printEnvStartup() int { return 0 } for _, item := range entries { - fmt.Fprintf(os.Stdout, "[scratchbox] %s\n", item) + printItem := item + // Redact values for IP list config (security sensitive for allowlist/rate/proxy spoof surface). + // Rates and other are left as-is (numeric, not secret). + if strings.Contains(item, "ALLOWED_IPS=") || strings.Contains(item, "TRUSTED_PROXY_IPS=") { + if eq := strings.IndexByte(item, '='); eq >= 0 { + printItem = item[:eq+1] + "*** (redacted)" + } + } + fmt.Fprintf(os.Stdout, "[scratchbox] %s\n", printItem) } return len(entries) } diff --git a/internal/cleanup/worker_test.go b/internal/cleanup/worker_test.go index e4f8437..f74817a 100644 --- a/internal/cleanup/worker_test.go +++ b/internal/cleanup/worker_test.go @@ -35,7 +35,7 @@ func TestWorkerStartRemovesExpiredAndLogs(t *testing.T) { if err != nil { t.Fatalf("NewFilesystemStore() error = %v", err) } - if _, err := store.Create(context.Background(), strings.NewReader("expired"), "text/plain", -time.Second); err != nil { + if _, err := store.Create(strings.NewReader("expired"), "text/plain", -time.Second); err != nil { t.Fatalf("Create() error = %v", err) } @@ -59,7 +59,7 @@ func TestWorkerStartWithNoExpiredEntries(t *testing.T) { if err != nil { t.Fatalf("NewFilesystemStore() error = %v", err) } - if _, err := store.Create(context.Background(), strings.NewReader("active"), "text/plain", time.Hour); err != nil { + if _, err := store.Create(strings.NewReader("active"), "text/plain", time.Hour); err != nil { t.Fatalf("Create() error = %v", err) } diff --git a/internal/config/config.go b/internal/config/config.go index 0b5fe5d..4c633cb 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -22,6 +22,7 @@ const ( defaultReadTimeout = "30s" defaultWriteTimeout = "30s" defaultIdleTimeout = "120s" + defaultShutdownTimeout = "10s" defaultMaxHeaderBytes = 1 << 20 defaultAccessLogEnabled = true defaultAccessLogMaxSize = "100MiB" @@ -42,6 +43,7 @@ const ( defaultAPIReadRateLimitBurst = 100 defaultAPIWriteRequestsPerMinute = 30 defaultAPIWriteRateLimitBurst = 10 + defaultHSTSEnabled = true maxDefaultTTLLimit = 24 * time.Hour minCleanupInterval = 10 * time.Second envPrefix = "SCRATCHBOX_" @@ -51,7 +53,7 @@ type Config struct { Server ServerConfig `yaml:"server" comment:"HTTP listener settings."` Limits LimitsConfig `yaml:"limits" comment:"Upload, expiration, and raw cache defaults."` Storage StorageConfig `yaml:"storage" comment:"Filesystem storage location and cleanup cadence."` - Security SecurityConfig `yaml:"security" comment:"Write-route IP restrictions and rate limiting."` + Security SecurityConfig `yaml:"security" comment:"Write-route IP restrictions, rate limiting, and HSTS."` } type ServerConfig struct { @@ -60,12 +62,14 @@ type ServerConfig struct { ReadTimeout string `yaml:"read_timeout" comment:"Maximum duration for reading the full request, including body. Must be > 0."` WriteTimeout string `yaml:"write_timeout" comment:"Maximum duration before timing out writes of a response. Must be > 0."` IdleTimeout string `yaml:"idle_timeout" comment:"Maximum amount of time to wait for the next request when keep-alives are enabled. Must be > 0."` + ShutdownTimeout string `yaml:"shutdown_timeout" comment:"Graceful shutdown timeout for the HTTP server. Must be > 0."` MaxHeaderBytes int `yaml:"max_header_bytes" comment:"Maximum size of request headers in bytes. Must be > 0."` AccessLog AccessLogConfig `yaml:"access_log" comment:"Access log settings for all HTTP requests."` ReadHeaderTimeoutDur time.Duration `yaml:"-"` ReadTimeoutDur time.Duration `yaml:"-"` WriteTimeoutDur time.Duration `yaml:"-"` IdleTimeoutDur time.Duration `yaml:"-"` + ShutdownTimeoutDur time.Duration `yaml:"-"` } type AccessLogConfig struct { @@ -96,10 +100,11 @@ type StorageConfig struct { type SecurityConfig struct { AllowedIPs []string `yaml:"allowed_ips" comment:"Allowlist for POST /api/scratch as IPs or CIDRs (examples: 127.0.0.1, 192.168.1.0/24). Keep 127.0.0.1 for local access; add trusted ranges as needed. Empty means unrestricted."` TrustProxyHeaders bool `yaml:"trust_proxy_headers" comment:"Honor X-Forwarded-For / X-Real-IP only when request source IP matches security.trusted_proxy_ips."` - TrustedProxyIPs []string `yaml:"trusted_proxy_ips" comment:"IPs/CIDRs of reverse proxies allowed to supply forwarded client IP headers. Required when trust_proxy_headers is true."` + TrustedProxyIPs []string `yaml:"trusted_proxy_ips" comment:"IPs/CIDRs of reverse proxies allowed to supply forwarded client IP headers. Required when trust_proxy_headers is true. Must be the *immediate* proxies only (never 0/0 or untrusted ranges)."` RateLimitUI RateLimitConfig `yaml:"rate_limit_ui" comment:"Per-client token bucket on UI routes (/, /u, /s/{id})."` RateLimitAPIRead RateLimitConfig `yaml:"rate_limit_api_read" comment:"Per-client token bucket on API read routes (GET /api/config, GET /api/scratch/{id}, GET /api/raw/{id})."` RateLimitAPIWrite RateLimitConfig `yaml:"rate_limit_api_write" comment:"Per-client token bucket on API write route (POST /api/scratch)."` + HSTSEnabled bool `yaml:"hsts_enabled" comment:"Enable Strict-Transport-Security (HSTS) header. Recommended when behind a TLS-terminating reverse proxy for public deployments (default: true). Set to false for local-only HTTP use without proxies."` AllowedPrefixes []netip.Prefix `yaml:"-"` TrustedProxyPrefixes []netip.Prefix `yaml:"-"` } @@ -150,6 +155,7 @@ func defaults() Config { ReadTimeout: defaultReadTimeout, WriteTimeout: defaultWriteTimeout, IdleTimeout: defaultIdleTimeout, + ShutdownTimeout: defaultShutdownTimeout, MaxHeaderBytes: defaultMaxHeaderBytes, AccessLog: AccessLogConfig{ Enabled: defaultAccessLogEnabled, @@ -186,6 +192,7 @@ func defaults() Config { RequestsPerMinute: defaultAPIWriteRequestsPerMinute, Burst: defaultAPIWriteRateLimitBurst, }, + HSTSEnabled: defaultHSTSEnabled, }, } } @@ -230,6 +237,15 @@ func (c *Config) Validate() error { } c.Server.IdleTimeoutDur = idleTimeout + shutdownTimeout, err := time.ParseDuration(strings.TrimSpace(c.Server.ShutdownTimeout)) + if err != nil { + return fmt.Errorf("server.shutdown_timeout invalid: %w", err) + } + if shutdownTimeout <= 0 { + return errors.New("server.shutdown_timeout must be > 0") + } + c.Server.ShutdownTimeoutDur = shutdownTimeout + if c.Server.MaxHeaderBytes <= 0 { return errors.New("server.max_header_bytes must be > 0") } @@ -278,18 +294,10 @@ func (c *Config) Validate() error { if strings.TrimSpace(c.Storage.DataDir) == "" { return errors.New("storage.data_dir is required") } - if err := ensureWritableDir(c.Storage.DataDir); err != nil { - return fmt.Errorf("storage.data_dir not writable: %w", err) - } - encKey, err := ensureEncryptionKeyInDataDir(c.Storage.DataDir) - if err != nil { - return fmt.Errorf("storage encryption key setup failed: %w", err) - } - if len(encKey) != 32 { - return fmt.Errorf("storage encryption key must decode to 32 bytes, got %d", len(encKey)) - } - c.Storage.EncryptionKeyBytes = encKey - + // NOTE: directory creation + encryption key provisioning (with auto-gen on empty dir) + // moved to PrepareDataDir() to keep Validate() free of filesystem side effects. + // Callers (server startup, tests) must invoke PrepareDataDir after successful Validate/Load + // if they need EncryptionKeyBytes populated and data dir ensured. prefixes, err := parseAllowedPrefixes(c.Security.AllowedIPs) if err != nil { return fmt.Errorf("security.allowed_ips invalid: %w", err) @@ -326,6 +334,31 @@ func (c *Config) Validate() error { return nil } +// PrepareDataDir performs the filesystem side effects that were previously inside Validate: +// ensures the data dir is creatable/writable (via probe), and ensures the encryption key +// file exists at data_dir/metadata.key (auto-generating a fresh one only for a truly empty dir). +// It populates Storage.EncryptionKeyBytes. This keeps Validate pure (checks + duration/size +// parsing only). Must be called after Load/Validate (or on a manually built Config) before +// constructing a FilesystemStore that needs the key. Idempotent for subsequent calls if key +// already present. +func (c *Config) PrepareDataDir() error { + if strings.TrimSpace(c.Storage.DataDir) == "" { + return errors.New("storage.data_dir is required") + } + if err := ensureWritableDir(c.Storage.DataDir); err != nil { + return fmt.Errorf("storage.data_dir not writable: %w", err) + } + encKey, err := ensureEncryptionKeyInDataDir(c.Storage.DataDir) + if err != nil { + return fmt.Errorf("storage encryption key setup failed: %w", err) + } + if len(encKey) != 32 { + return fmt.Errorf("storage encryption key must decode to 32 bytes, got %d", len(encKey)) + } + c.Storage.EncryptionKeyBytes = encKey + return nil +} + func ensureWritableDir(dir string) error { if err := os.MkdirAll(dir, 0o755); err != nil { return err @@ -413,6 +446,11 @@ func parseHumanSize(raw string) (int64, error) { size := int64(number * multiplier) if size <= 0 { + if number > 0 { + // overflow or truncation from huge float (e.g. 1e20 * GiB); guard before + // passing surprising/negative limit downstream. + return 0, errors.New("size too large") + } return 0, errors.New("calculated size must be > 0") } return size, nil @@ -508,6 +546,9 @@ func (c *Config) applyEnvOverrides(prefix string) error { if err := applyStringEnv(prefix+"SERVER_IDLE_TIMEOUT", &c.Server.IdleTimeout); err != nil { return err } + if err := applyStringEnv(prefix+"SERVER_SHUTDOWN_TIMEOUT", &c.Server.ShutdownTimeout); err != nil { + return err + } if err := applyIntEnv(prefix+"SERVER_MAX_HEADER_BYTES", &c.Server.MaxHeaderBytes); err != nil { return err } @@ -580,6 +621,10 @@ func (c *Config) applyEnvOverrides(prefix string) error { return err } + if err := applyBoolEnv(prefix+"SECURITY_HSTS_ENABLED", &c.Security.HSTSEnabled); err != nil { + return err + } + return nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ab5b8e3..b6b1144 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -19,6 +19,9 @@ func TestValidateAppliesDefaultsAndParsedFields(t *testing.T) { if err := cfg.Validate(); err != nil { t.Fatalf("Validate() returned error: %v", err) } + if err := cfg.PrepareDataDir(); err != nil { + t.Fatalf("PrepareDataDir() returned error: %v", err) + } if cfg.Limits.MaxUploadSizeBytes != 100*1024*1024 { t.Fatalf("MaxUploadSizeBytes = %d, want %d", cfg.Limits.MaxUploadSizeBytes, int64(100*1024*1024)) @@ -35,6 +38,9 @@ func TestValidateAppliesDefaultsAndParsedFields(t *testing.T) { if cfg.Server.IdleTimeoutDur != 120*time.Second { t.Fatalf("IdleTimeoutDur = %s, want 120s", cfg.Server.IdleTimeoutDur) } + if cfg.Server.ShutdownTimeoutDur != 10*time.Second { + t.Fatalf("ShutdownTimeoutDur = %s, want 10s", cfg.Server.ShutdownTimeoutDur) + } if cfg.Server.MaxHeaderBytes != 1<<20 { t.Fatalf("MaxHeaderBytes = %d, want %d", cfg.Server.MaxHeaderBytes, 1<<20) } @@ -68,6 +74,9 @@ func TestValidateAppliesDefaultsAndParsedFields(t *testing.T) { if got := cfg.Security.AllowedPrefixes[0]; got != netip.MustParsePrefix("127.0.0.1/32") { t.Fatalf("AllowedPrefixes[0] = %v, want 127.0.0.1/32", got) } + if !cfg.Security.HSTSEnabled { + t.Fatalf("HSTSEnabled = false, want true (default)") + } } func TestLoadParsesConfiguredFields(t *testing.T) { @@ -116,6 +125,7 @@ security: enabled: true requests_per_minute: 12 burst: 4 + hsts_enabled: false ` if err := os.WriteFile(configPath, []byte(content), 0o644); err != nil { t.Fatalf("WriteFile() error = %v", err) @@ -125,6 +135,9 @@ security: if err != nil { t.Fatalf("Load() error = %v", err) } + if err := cfg.PrepareDataDir(); err != nil { + t.Fatalf("PrepareDataDir() error = %v", err) + } if cfg.Server.ListenAddr != "127.0.0.1:9090" { t.Fatalf("ListenAddr = %q, want %q", cfg.Server.ListenAddr, "127.0.0.1:9090") @@ -141,6 +154,9 @@ security: if cfg.Server.IdleTimeoutDur != 90*time.Second { t.Fatalf("IdleTimeoutDur = %s, want 90s", cfg.Server.IdleTimeoutDur) } + if cfg.Server.ShutdownTimeoutDur != 10*time.Second { + t.Fatalf("ShutdownTimeoutDur = %s, want 10s", cfg.Server.ShutdownTimeoutDur) + } if cfg.Server.MaxHeaderBytes != 2*1024*1024 { t.Fatalf("MaxHeaderBytes = %d, want %d", cfg.Server.MaxHeaderBytes, 2*1024*1024) } @@ -186,6 +202,9 @@ security: if cfg.Security.RateLimitAPIWrite.RequestsPerMinute != 12 || cfg.Security.RateLimitAPIWrite.Burst != 4 { t.Fatalf("unexpected rate_limit_api_write values: %+v", cfg.Security.RateLimitAPIWrite) } + if cfg.Security.HSTSEnabled { + t.Fatalf("HSTSEnabled = true, want false (from config)") + } if len(cfg.Security.AllowedPrefixes) != 2 { t.Fatalf("AllowedPrefixes len = %d, want 2", len(cfg.Security.AllowedPrefixes)) } @@ -244,6 +263,13 @@ func TestValidateRejectsInvalidValues(t *testing.T) { }, wantErr: "server.idle_timeout", }, + { + name: "invalid shutdown timeout", + mutate: func(cfg *Config) { + cfg.Server.ShutdownTimeout = "bogus" + }, + wantErr: "server.shutdown_timeout", + }, { name: "invalid max header bytes", mutate: func(cfg *Config) { @@ -432,13 +458,23 @@ storage: t.Fatalf("Load() error = %v, want nil", err) } + // First prepare creates the data dir (mkdir probe) so we can write the bad key file for test. + // (Load no longer has side effects.) + cfgForBad, err := Load(validConfigPath) + if err != nil { + t.Fatalf("Load(valid) error = %v", err) + } + if err := cfgForBad.PrepareDataDir(); err != nil { + t.Fatalf("PrepareDataDir (to create dir) error = %v", err) + } + keyPath := filepath.Join(tmp, "store", dataDirEncryptionKeyFilename) if err := os.WriteFile(keyPath, []byte("not-base64"), 0o600); err != nil { t.Fatalf("WriteFile(invalid key) error = %v", err) } - _, err = Load(validConfigPath) - if err == nil || !strings.Contains(err.Error(), "storage encryption key setup failed") { - t.Fatalf("Load() error = %v, want invalid key base64 error", err) + // Re-prepare (or load+prepare) now hits the bad key decode inside ensure. + if err := cfgForBad.PrepareDataDir(); err == nil || !strings.Contains(err.Error(), "storage encryption key setup failed") { + t.Fatalf("PrepareDataDir after bad key error = %v, want setup failed containing 'storage encryption key setup failed'", err) } } @@ -465,9 +501,13 @@ storage: t.Fatalf("WriteFile(config) error = %v", err) } - _, err := Load(configPath) + cfg, err := Load(configPath) + if err != nil { + t.Fatalf("Load() error = %v, want success (pure validate)", err) + } + err = cfg.PrepareDataDir() if err == nil || !strings.Contains(err.Error(), "refusing to generate missing") { - t.Fatalf("Load() error = %v, want missing-key safety error", err) + t.Fatalf("PrepareDataDir() error = %v, want missing-key safety error containing 'refusing to generate missing'", err) } } @@ -479,6 +519,7 @@ func TestLoadFromEnvParsesConfiguredFields(t *testing.T) { t.Setenv("SCRATCHBOX_SERVER_READ_TIMEOUT", "20s") t.Setenv("SCRATCHBOX_SERVER_WRITE_TIMEOUT", "40s") t.Setenv("SCRATCHBOX_SERVER_IDLE_TIMEOUT", "80s") + t.Setenv("SCRATCHBOX_SERVER_SHUTDOWN_TIMEOUT", "5s") t.Setenv("SCRATCHBOX_SERVER_MAX_HEADER_BYTES", "2097152") t.Setenv("SCRATCHBOX_SERVER_ACCESS_LOG_ENABLED", "false") t.Setenv("SCRATCHBOX_SERVER_ACCESS_LOG_FILE_PATH", "/tmp/access.log") @@ -505,11 +546,15 @@ func TestLoadFromEnvParsesConfiguredFields(t *testing.T) { t.Setenv("SCRATCHBOX_SECURITY_RATE_LIMIT_API_WRITE_ENABLED", "true") t.Setenv("SCRATCHBOX_SECURITY_RATE_LIMIT_API_WRITE_REQUESTS_PER_MINUTE", "40") t.Setenv("SCRATCHBOX_SECURITY_RATE_LIMIT_API_WRITE_BURST", "12") + t.Setenv("SCRATCHBOX_SECURITY_HSTS_ENABLED", "false") cfg, err := LoadFromEnv() if err != nil { t.Fatalf("LoadFromEnv() error = %v", err) } + if err := cfg.PrepareDataDir(); err != nil { + t.Fatalf("PrepareDataDir() error = %v", err) + } if cfg.Server.ListenAddr != "127.0.0.1:19090" { t.Fatalf("ListenAddr = %q, want %q", cfg.Server.ListenAddr, "127.0.0.1:19090") @@ -526,6 +571,9 @@ func TestLoadFromEnvParsesConfiguredFields(t *testing.T) { if cfg.Server.IdleTimeoutDur != 80*time.Second { t.Fatalf("IdleTimeoutDur = %s, want 80s", cfg.Server.IdleTimeoutDur) } + if cfg.Server.ShutdownTimeoutDur != 5*time.Second { + t.Fatalf("ShutdownTimeoutDur = %s, want 5s", cfg.Server.ShutdownTimeoutDur) + } if cfg.Server.MaxHeaderBytes != 2*1024*1024 { t.Fatalf("MaxHeaderBytes = %d, want %d", cfg.Server.MaxHeaderBytes, 2*1024*1024) } @@ -571,6 +619,9 @@ func TestLoadFromEnvParsesConfiguredFields(t *testing.T) { if cfg.Security.RateLimitAPIWrite.RequestsPerMinute != 40 || cfg.Security.RateLimitAPIWrite.Burst != 12 { t.Fatalf("unexpected rate_limit_api_write values: %+v", cfg.Security.RateLimitAPIWrite) } + if cfg.Security.HSTSEnabled { + t.Fatalf("HSTSEnabled from env = true, want false") + } } func TestLoadFromEnvRejectsInvalidBool(t *testing.T) { @@ -616,6 +667,7 @@ func TestParseHumanSizeVariants(t *testing.T) { {in: "abc", wantErr: "does not start with a number"}, {in: "5XB", wantErr: "unsupported unit"}, {in: "", wantErr: "size is empty"}, + {in: "100000000000000000GiB", wantErr: "size too large"}, } for _, tc := range tests { diff --git a/internal/http/handlers.go b/internal/http/handlers.go index 2e18840..10f072b 100644 --- a/internal/http/handlers.go +++ b/internal/http/handlers.go @@ -69,16 +69,36 @@ func (s *Server) Routes() http.Handler { mux.Handle("GET /assets/", http.FileServerFS(webassets.StaticFS())) mux.Handle("GET /static/", http.StripPrefix("/static/", http.FileServerFS(webassets.StaticFS()))) + + // Special-case well-known paths to return small plain-text responses instead of full SPA 404 shell. + // This addresses low-priority audit item for robots.txt etc. + mux.Handle("GET /robots.txt", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("User-agent: *\nDisallow: /\n")) + })) + mux.Handle("GET /sitemap.xml", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + w.WriteHeader(http.StatusNotFound) + })) + + // Serve the real favicon (added to eliminate 404 noise). Placed in static root via public/ in Vite build. + mux.Handle("GET /favicon.svg", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.ServeFileFS(w, r, webassets.StaticFS(), "favicon.svg") + })) + mux.Handle("GET /{path...}", http.HandlerFunc(s.notFoundPage)) - return AccessLogMiddleware( + + accessLogged := AccessLogMiddleware( s.accessLogger, s.cfg.Security.TrustProxyHeaders, s.cfg.Security.TrustedProxyPrefixes, )(mux) + return SecurityHeadersMiddleware(s.cfg.Security.HSTSEnabled, accessLogged) } func (s *Server) serveSPA(w http.ResponseWriter, r *http.Request) { - s.serveSPAWithStatus(w, http.StatusOK) + s.serveSPAWithStatus(w, http.StatusOK, nil) } func (s *Server) getUIConfig(w http.ResponseWriter, r *http.Request) { @@ -91,19 +111,22 @@ func (s *Server) getUIConfig(w http.ResponseWriter, r *http.Request) { } func (s *Server) notFoundPage(w http.ResponseWriter, _ *http.Request) { - s.serveSPAWithStatus(w, http.StatusNotFound) + s.serveSPAWithStatus(w, http.StatusNotFound, nil) } func (s *Server) expiredScratchPage(w http.ResponseWriter) { - s.serveSPAWithStatus(w, http.StatusGone) + s.serveSPAWithStatus(w, http.StatusGone, nil) } -func (s *Server) serveSPAWithStatus(w http.ResponseWriter, status int) { +func (s *Server) serveSPAWithStatus(w http.ResponseWriter, status int, modifier func([]byte) []byte) { b, err := fs.ReadFile(webassets.StaticFS(), "index.html") if err != nil { http.Error(w, "failed to render page", http.StatusInternalServerError) return } + if modifier != nil { + b = modifier(b) + } w.Header().Set("Content-Type", "text/html; charset=utf-8") w.WriteHeader(status) _, _ = w.Write(b) @@ -117,17 +140,36 @@ func (s *Server) scratchPage(w http.ResponseWriter, r *http.Request) { } meta, err := s.store.Get(id) - if err != nil { - s.expiredScratchPage(w) - return - } - if s.isExpired(meta) { - _ = s.store.Delete(id) - s.expiredScratchPage(w) + if err != nil || s.isExpired(meta) { + if err == nil { + _ = s.store.Delete(id) + } + // For /s/{id} 410/expired (including never-existed), inject id/filename based title+meta even on 410 shell. + // This provides good SEO/social for share links and bots (even on 410), per audit rec. + // We use the id (and name if we had meta before delete) for title. + goneTitle := id + " • Scratchbox" + if err == nil && meta.OriginalName != "" { + goneTitle = meta.OriginalName + " • Scratchbox" + } + s.serveSPAWithStatus(w, http.StatusGone, func(b []byte) []byte { + return injectMeta(b, goneTitle, goneTitle, "This scratch is no longer available.") + }) return } - s.serveSPA(w, r) + // Valid scratch: serve shell with per-scratch title and og meta injected server-side. + // (Client-side <svelte:head> also updates for SPA, but server inj benefits crawlers/JS-disabled.) + title := id + " • Scratchbox" + if meta.OriginalName != "" { + title = meta.OriginalName + " • Scratchbox" + } + desc := "Temporary scratch file shared on Scratchbox." + if meta.OriginalName != "" { + desc = meta.OriginalName + " — temporary file share on Scratchbox." + } + s.serveSPAWithStatus(w, http.StatusOK, func(b []byte) []byte { + return injectMeta(b, title, title, desc) + }) } func (s *Server) createScratch(w http.ResponseWriter, r *http.Request) { @@ -180,7 +222,7 @@ func (s *Server) createScratch(w http.ResponseWriter, r *http.Request) { } } - meta, err := s.store.CreateWithOriginalName(r.Context(), reader, contentType, originalName, s.cfg.Limits.DefaultTTLDuration) + meta, err := s.store.CreateWithOriginalName(reader, contentType, originalName, s.cfg.Limits.DefaultTTLDuration) if err != nil { if strings.Contains(err.Error(), "request body too large") { http.Error(w, s.maxUploadSizeError(), http.StatusRequestEntityTooLarge) @@ -332,3 +374,36 @@ func (s *Server) isUploadAllowedForRequest(r *http.Request) bool { return false } + +// injectMeta performs lightweight server-side injection of <title> and social meta tags +// into the static index.html shell bytes. Uses simple string replace (safe for this tiny known shell, +// no new deps). Generic pages keep the static metas from the HTML template; /s/{id} get dynamic. +func injectMeta(html []byte, pageTitle, ogTitle, ogDescription string) []byte { + if pageTitle == "" { + pageTitle = "Scratchbox" + } + if ogTitle == "" { + ogTitle = pageTitle + } + if ogDescription == "" { + ogDescription = "Minimal temporary file sharing. Upload once, share the link, auto-expires." + } + s := string(html) + // Override <title> + s = strings.Replace(s, "<title>Scratchbox", ""+htmlEscape(pageTitle)+"", 1) + // Override the static og/twitter metas added to the shells (first occurrence only; safe). + s = strings.Replace(s, ``, ``, 1) + s = strings.Replace(s, ``, ``, 1) + s = strings.Replace(s, ``, ``, 1) + s = strings.Replace(s, ``, ``, 1) + s = strings.Replace(s, ``, ``, 1) + return []byte(s) +} + +func htmlEscape(s string) string { + s = strings.ReplaceAll(s, "&", "&") + s = strings.ReplaceAll(s, "<", "<") + s = strings.ReplaceAll(s, ">", ">") + s = strings.ReplaceAll(s, `"`, """) + return s +} diff --git a/internal/http/handlers_integration_concurrent_test.go b/internal/http/handlers_integration_concurrent_test.go new file mode 100644 index 0000000..989c3e4 --- /dev/null +++ b/internal/http/handlers_integration_concurrent_test.go @@ -0,0 +1,249 @@ +package httpapi + +import ( + "bytes" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" +) + +func TestConcurrentUploadsRemainConsistentAndReadable(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024 * 1024, + defaultTTL: time.Hour, + rateLimitEnable: false, + }) + + const uploads = 80 + + type createdScratch struct { + id string + body string + } + + resultsCh := make(chan createdScratch, uploads) + errCh := make(chan error, uploads) + var wg sync.WaitGroup + + for i := 0; i < uploads; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + + body := fmt.Sprintf("parallel-upload-%03d-%s", i, strings.Repeat("x", i%17)) + createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString(body)) + createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") + createReq.RemoteAddr = fmt.Sprintf("127.0.0.1:%d", 8200+i) + createRec := httptest.NewRecorder() + handler.ServeHTTP(createRec, createReq) + if createRec.Code != http.StatusCreated { + errCh <- fmt.Errorf("create status = %d, want %d", createRec.Code, http.StatusCreated) + return + } + + var payload map[string]any + if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { + errCh <- fmt.Errorf("decode create payload error: %w", err) + return + } + id, _ := payload["id"].(string) + if id == "" { + errCh <- fmt.Errorf("create payload missing id") + return + } + + metaReq := httptest.NewRequest(http.MethodGet, "/api/scratch/"+id, nil) + metaRec := httptest.NewRecorder() + handler.ServeHTTP(metaRec, metaReq) + if metaRec.Code != http.StatusOK { + errCh <- fmt.Errorf("metadata status = %d, want %d", metaRec.Code, http.StatusOK) + return + } + + rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) + rawRec := httptest.NewRecorder() + handler.ServeHTTP(rawRec, rawReq) + if rawRec.Code != http.StatusOK { + errCh <- fmt.Errorf("raw status = %d, want %d", rawRec.Code, http.StatusOK) + return + } + if got := rawRec.Body.String(); got != body { + errCh <- fmt.Errorf("raw body mismatch = %q, want %q", got, body) + return + } + + resultsCh <- createdScratch{id: id, body: body} + }(i) + } + + wg.Wait() + close(resultsCh) + close(errCh) + for err := range errCh { + t.Error(err) + } + + seen := make(map[string]struct{}, uploads) + for result := range resultsCh { + if _, exists := seen[result.id]; exists { + t.Errorf("duplicate scratch id generated: %s", result.id) + continue + } + seen[result.id] = struct{}{} + } + if len(seen) != uploads { + t.Fatalf("unique IDs = %d, want %d", len(seen), uploads) + } +} + +func TestParallelClientsCanReadSameScratchAcrossAllReadRoutes(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: time.Hour, + rateLimitEnable: false, + }) + + const body = "parallel readers" + createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString(body)) + createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") + createReq.RemoteAddr = "127.0.0.1:6010" + createRec := httptest.NewRecorder() + handler.ServeHTTP(createRec, createReq) + if createRec.Code != http.StatusCreated { + t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) + } + + var payload map[string]any + if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { + t.Fatalf("decode create payload error = %v", err) + } + id, _ := payload["id"].(string) + if id == "" { + t.Fatalf("create payload missing id") + } + + paths := []string{ + "/api/raw/" + id, + "/s/" + id, + "/api/scratch/" + id, + } + const requests = 120 + + errCh := make(chan error, requests) + var wg sync.WaitGroup + for i := 0; i < requests; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + path := paths[i%len(paths)] + req := httptest.NewRequest(http.MethodGet, path, nil) + req.RemoteAddr = fmt.Sprintf("127.0.0.1:%d", 6100+i) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + errCh <- fmt.Errorf("path %s status = %d, want %d", path, rec.Code, http.StatusOK) + return + } + + switch { + case strings.HasPrefix(path, "/api/raw/"): + if got := rec.Body.String(); got != body { + errCh <- fmt.Errorf("raw body = %q, want %q", got, body) + } + case strings.HasPrefix(path, "/s/"): + if !strings.Contains(rec.Body.String(), `id="app"`) { + errCh <- fmt.Errorf("view body missing SPA marker") + } + case strings.HasPrefix(path, "/api/scratch/"): + var gotPayload map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &gotPayload); err != nil { + errCh <- fmt.Errorf("metadata decode error: %w", err) + return + } + if gotID, _ := gotPayload["id"].(string); gotID != id { + errCh <- fmt.Errorf("metadata id = %q, want %q", gotID, id) + } + } + }(i) + } + wg.Wait() + close(errCh) + for err := range errCh { + t.Error(err) + } +} + +func TestParallelClientsExpiredScratchReadsReturnGone(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: 80 * time.Millisecond, + rateLimitEnable: false, + }) + + createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString("expires soon")) + createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") + createReq.RemoteAddr = "127.0.0.1:7010" + createRec := httptest.NewRecorder() + handler.ServeHTTP(createRec, createReq) + if createRec.Code != http.StatusCreated { + t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) + } + + var payload map[string]any + if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { + t.Fatalf("decode create payload error = %v", err) + } + id, _ := payload["id"].(string) + if id == "" { + t.Fatalf("create payload missing id") + } + + time.Sleep(130 * time.Millisecond) + + paths := []string{ + "/api/raw/" + id, + "/s/" + id, + "/api/scratch/" + id, + } + const requests = 90 + + errCh := make(chan error, requests) + var wg sync.WaitGroup + for i := 0; i < requests; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + path := paths[i%len(paths)] + req := httptest.NewRequest(http.MethodGet, path, nil) + req.RemoteAddr = fmt.Sprintf("127.0.0.1:%d", 7100+i) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusGone { + errCh <- fmt.Errorf("expired path %s status = %d, want %d", path, rec.Code, http.StatusGone) + return + } + + if strings.HasPrefix(path, "/s/") && !strings.Contains(rec.Body.String(), `id="app"`) { + errCh <- fmt.Errorf("expired view body missing SPA marker") + } + }(i) + } + wg.Wait() + close(errCh) + for err := range errCh { + t.Error(err) + } +} diff --git a/internal/http/handlers_integration_flows_test.go b/internal/http/handlers_integration_flows_test.go new file mode 100644 index 0000000..722576c --- /dev/null +++ b/internal/http/handlers_integration_flows_test.go @@ -0,0 +1,375 @@ +package httpapi + +import ( + "bytes" + "encoding/json" + "mime/multipart" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +func TestCreateReadThenExpireScratch(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: 120 * time.Millisecond, + rateLimitEnable: false, + }) + + createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString("hello scratch")) + createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") + createReq.RemoteAddr = "127.0.0.1:9999" + createRec := httptest.NewRecorder() + handler.ServeHTTP(createRec, createReq) + if createRec.Code != http.StatusCreated { + t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) + } + + var payload map[string]any + if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { + t.Fatalf("decode create payload error = %v", err) + } + id, _ := payload["id"].(string) + if id == "" { + t.Fatalf("create payload missing id") + } + + metaReq := httptest.NewRequest(http.MethodGet, "/api/scratch/"+id, nil) + metaReq.RemoteAddr = "127.0.0.1:9999" + metaRec := httptest.NewRecorder() + handler.ServeHTTP(metaRec, metaReq) + if metaRec.Code != http.StatusOK { + t.Fatalf("read metadata status = %d, want %d", metaRec.Code, http.StatusOK) + } + + rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) + rawReq.RemoteAddr = "127.0.0.1:9999" + rawRec := httptest.NewRecorder() + handler.ServeHTTP(rawRec, rawReq) + if rawRec.Code != http.StatusOK { + t.Fatalf("read raw status = %d, want %d", rawRec.Code, http.StatusOK) + } + if got := rawRec.Body.String(); got != "hello scratch" { + t.Fatalf("raw body = %q, want %q", got, "hello scratch") + } + + time.Sleep(180 * time.Millisecond) + + expiredReq := httptest.NewRequest(http.MethodGet, "/api/scratch/"+id, nil) + expiredReq.RemoteAddr = "127.0.0.1:9999" + expiredRec := httptest.NewRecorder() + handler.ServeHTTP(expiredRec, expiredReq) + if expiredRec.Code != http.StatusGone { + t.Fatalf("expired status = %d, want %d", expiredRec.Code, http.StatusGone) + } +} + +func TestUploadedScratchViewAndRawBeforeAndAfterExpiry(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: 120 * time.Millisecond, + rateLimitEnable: false, + }) + + createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString("hello from upload flow")) + createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") + createReq.RemoteAddr = "127.0.0.1:5090" + createRec := httptest.NewRecorder() + handler.ServeHTTP(createRec, createReq) + if createRec.Code != http.StatusCreated { + t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) + } + + var payload map[string]any + if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { + t.Fatalf("decode create payload error = %v", err) + } + id, _ := payload["id"].(string) + if id == "" { + t.Fatalf("create payload missing id") + } + + viewReq := httptest.NewRequest(http.MethodGet, "/s/"+id, nil) + viewReq.RemoteAddr = "127.0.0.1:5091" + viewRec := httptest.NewRecorder() + handler.ServeHTTP(viewRec, viewReq) + if viewRec.Code != http.StatusOK { + t.Fatalf("view-before-expiry status = %d, want %d", viewRec.Code, http.StatusOK) + } + if !strings.Contains(viewRec.Body.String(), `id="app"`) { + t.Fatalf("expected SPA shell body for /s/{id}, got %q", viewRec.Body.String()) + } + + rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) + rawReq.RemoteAddr = "127.0.0.1:5092" + rawRec := httptest.NewRecorder() + handler.ServeHTTP(rawRec, rawReq) + if rawRec.Code != http.StatusOK { + t.Fatalf("raw-before-expiry status = %d, want %d", rawRec.Code, http.StatusOK) + } + if got := rawRec.Body.String(); got != "hello from upload flow" { + t.Fatalf("raw-before-expiry body = %q, want %q", got, "hello from upload flow") + } + + time.Sleep(180 * time.Millisecond) + + viewExpiredReq := httptest.NewRequest(http.MethodGet, "/s/"+id, nil) + viewExpiredReq.RemoteAddr = "127.0.0.1:5093" + viewExpiredRec := httptest.NewRecorder() + handler.ServeHTTP(viewExpiredRec, viewExpiredReq) + if viewExpiredRec.Code != http.StatusGone { + t.Fatalf("view-after-expiry status = %d, want %d", viewExpiredRec.Code, http.StatusGone) + } + if !strings.Contains(viewExpiredRec.Body.String(), `id="app"`) { + t.Fatalf("expected SPA shell body for expired /s/{id}, got %q", viewExpiredRec.Body.String()) + } + + rawExpiredReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) + rawExpiredReq.RemoteAddr = "127.0.0.1:5094" + rawExpiredRec := httptest.NewRecorder() + handler.ServeHTTP(rawExpiredRec, rawExpiredReq) + if rawExpiredRec.Code != http.StatusGone { + t.Fatalf("raw-after-expiry status = %d, want %d", rawExpiredRec.Code, http.StatusGone) + } + if rawExpiredRec.Body.Len() != 0 { + t.Fatalf("expected empty raw body after expiry, got %q", rawExpiredRec.Body.String()) + } +} + +func TestGetScratchNotFoundReturnsGone(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: time.Hour, + rateLimitEnable: false, + }) + + viewReq := httptest.NewRequest(http.MethodGet, "/s/does-not-exist", nil) + viewReq.RemoteAddr = "127.0.0.1:5095" + viewRec := httptest.NewRecorder() + handler.ServeHTTP(viewRec, viewReq) + if viewRec.Code != http.StatusGone { + t.Fatalf("view status = %d, want %d", viewRec.Code, http.StatusGone) + } + + rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/does-not-exist", nil) + rawRec := httptest.NewRecorder() + handler.ServeHTTP(rawRec, rawReq) + if rawRec.Code != http.StatusGone { + t.Fatalf("raw status = %d, want %d", rawRec.Code, http.StatusGone) + } + if rawRec.Body.Len() != 0 { + t.Fatalf("expected empty response body, got %q", rawRec.Body.String()) + } +} + +func TestViewAndRawUnknownIDsReturnGone(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: time.Hour, + rateLimitEnable: false, + }) + + viewReq := httptest.NewRequest(http.MethodGet, "/s/never-uploaded-id", nil) + viewReq.RemoteAddr = "127.0.0.1:5095" + viewRec := httptest.NewRecorder() + handler.ServeHTTP(viewRec, viewReq) + if viewRec.Code != http.StatusGone { + t.Fatalf("view-never-uploaded status = %d, want %d", viewRec.Code, http.StatusGone) + } + + rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/never-uploaded-id", nil) + rawReq.RemoteAddr = "127.0.0.1:5096" + rawRec := httptest.NewRecorder() + handler.ServeHTTP(rawRec, rawReq) + if rawRec.Code != http.StatusGone { + t.Fatalf("raw-never-uploaded status = %d, want %d", rawRec.Code, http.StatusGone) + } + if rawRec.Body.Len() != 0 { + t.Fatalf("expected empty raw body for never-uploaded id, got %q", rawRec.Body.String()) + } + + metaReq := httptest.NewRequest(http.MethodGet, "/api/scratch/never-uploaded-id", nil) + metaReq.RemoteAddr = "127.0.0.1:5097" + metaRec := httptest.NewRecorder() + handler.ServeHTTP(metaRec, metaReq) + if metaRec.Code != http.StatusGone { + t.Fatalf("metadata-never-uploaded status = %d, want %d", metaRec.Code, http.StatusGone) + } +} + +func TestCreateScratchRejectsMalformedMultipart(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: time.Hour, + rateLimitEnable: false, + }) + + req := httptest.NewRequest(http.MethodPost, "/api/scratch", strings.NewReader("not-multipart")) + req.Header.Set("Content-Type", "multipart/form-data; boundary=missing") + req.RemoteAddr = "127.0.0.1:5057" + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest) + } +} + +func TestUnknownPathReturnsNotFoundPage(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: time.Hour, + rateLimitEnable: false, + }) + + req := httptest.NewRequest(http.MethodGet, "/this-path-does-not-exist", nil) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusNotFound) + } + if !strings.Contains(rec.Body.String(), `id="app"`) { + t.Fatalf("unexpected not found body: %q", rec.Body.String()) + } +} + +// TestWellKnownPathsReturnPlainNotSPA verifies low-pri audit fix: robots/sitemap return small +// plain text (no full SPA html shell +404). +func TestWellKnownPathsReturnPlainNotSPA(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: time.Hour, + rateLimitEnable: false, + }) + + // robots: 200 plain with disallow + req := httptest.NewRequest(http.MethodGet, "/robots.txt", nil) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("/robots status=%d want 200", rec.Code) + } + ct := rec.Header().Get("Content-Type") + if !strings.Contains(ct, "text/plain") { + t.Fatalf("/robots ct=%q want text/plain", ct) + } + body := rec.Body.String() + if !strings.Contains(body, "User-agent: *") || strings.Contains(body, "id=\"app\"") || strings.Contains(body, " 100 { + t.Fatalf("/sitemap body unexpected (small plain 404, no SPA): %q", body2) + } + + // normal unknown path still gets SPA 404 shell + req3 := httptest.NewRequest(http.MethodGet, "/not-a-wellknown", nil) + rec3 := httptest.NewRecorder() + handler.ServeHTTP(rec3, req3) + if rec3.Code != http.StatusNotFound { + t.Fatalf("generic 404 status=%d want 404", rec3.Code) + } + if !strings.Contains(rec3.Body.String(), `id="app"`) { + t.Fatalf("generic 404 should still be SPA shell") + } +} + +func TestFaviconServed(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: time.Hour, + rateLimitEnable: false, + }) + + req := httptest.NewRequest(http.MethodGet, "/favicon.svg", nil) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("favicon status = %d, want 200", rec.Code) + } + ct := rec.Header().Get("Content-Type") + if !strings.Contains(ct, "image/svg+xml") { + t.Fatalf("favicon ct=%q want image/svg+xml", ct) + } +} + +func TestScratchPageServerMetaInjection(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: time.Hour, + rateLimitEnable: false, + }) + + // create one with filename + var body bytes.Buffer + writer := multipart.NewWriter(&body) + file, _ := writer.CreateFormFile("file", "report.pdf") + file.Write([]byte("pdf-bytes")) + writer.Close() + + createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", &body) + createReq.Header.Set("Content-Type", writer.FormDataContentType()) + createReq.RemoteAddr = "127.0.0.1:7777" + createRec := httptest.NewRecorder() + handler.ServeHTTP(createRec, createReq) + if createRec.Code != http.StatusCreated { + t.Fatalf("create status = %d", createRec.Code) + } + + var payload map[string]any + json.Unmarshal(createRec.Body.Bytes(), &payload) + id := payload["id"].(string) + + // view page should have injected title/meta + viewReq := httptest.NewRequest(http.MethodGet, "/s/"+id, nil) + viewRec := httptest.NewRecorder() + handler.ServeHTTP(viewRec, viewReq) + + bodyStr := viewRec.Body.String() + if !strings.Contains(bodyStr, "report.pdf • Scratchbox") { + t.Fatalf("expected injected title with filename, got head: %s", bodyStr[:min(800, len(bodyStr))]) + } + if !strings.Contains(bodyStr, `property="og:title"`) { + t.Fatalf("expected og:title meta") + } +} + +func min(a, b int) int { + if a < b { + return a + } + return b +} diff --git a/internal/http/handlers_integration_helpers_test.go b/internal/http/handlers_integration_helpers_test.go new file mode 100644 index 0000000..4c48684 --- /dev/null +++ b/internal/http/handlers_integration_helpers_test.go @@ -0,0 +1,79 @@ +package httpapi + +import ( + "io" + "log" + "net/http" + "path/filepath" + "testing" + "time" + + "scratchbox/internal/config" + "scratchbox/internal/storage" +) + +var testStorageKey = []byte("0123456789abcdef0123456789abcdef") + +type testServerOptions struct { + maxUploadBytes int64 + defaultTTL time.Duration + rateLimitEnable bool +} + +func newTestHandler(t *testing.T, opts testServerOptions) http.Handler { + t.Helper() + + handler, _ := newTestHandlerAndStore(t, opts) + return handler +} + +func newTestHandlerAndStore(t *testing.T, opts testServerOptions) (http.Handler, *storage.FilesystemStore) { + t.Helper() + + dataDir := filepath.Join(t.TempDir(), "data") + store, err := storage.NewFilesystemStore(dataDir, testStorageKey) + if err != nil { + t.Fatalf("NewFilesystemStore() error = %v", err) + } + + cfg := config.Config{ + Server: config.ServerConfig{ + ListenAddr: ":0", + }, + Limits: config.LimitsConfig{ + MaxUploadSizeBytes: opts.maxUploadBytes, + DefaultTTLDuration: opts.defaultTTL, + RawCacheMaxBytes: 64 * 1024 * 1024, + RawCacheMaxEntries: 32, + }, + Storage: config.StorageConfig{ + DataDir: dataDir, + }, + Security: config.SecurityConfig{ + RateLimitUI: config.RateLimitConfig{ + Enabled: opts.rateLimitEnable, + RequestsPerMinute: 120, + Burst: 1, + }, + RateLimitAPIRead: config.RateLimitConfig{ + Enabled: opts.rateLimitEnable, + RequestsPerMinute: 120, + Burst: 1, + }, + RateLimitAPIWrite: config.RateLimitConfig{ + Enabled: opts.rateLimitEnable, + RequestsPerMinute: 120, + Burst: 1, + }, + HSTSEnabled: true, + }, + } + + srv := &Server{ + cfg: cfg, + store: store, + logger: log.New(io.Discard, "", 0), + rawCache: newRawContentCache(cfg.Limits.RawCacheMaxEntries, cfg.Limits.RawCacheMaxBytes), + } + return srv.Routes(), store +} diff --git a/internal/http/handlers_integration_security_test.go b/internal/http/handlers_integration_security_test.go new file mode 100644 index 0000000..61b1e46 --- /dev/null +++ b/internal/http/handlers_integration_security_test.go @@ -0,0 +1,101 @@ +package httpapi + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +func TestReadRoutesAreRateLimitedWhenEnabled(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: time.Hour, + rateLimitEnable: true, + }) + + createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString("hello scratch")) + createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") + createReq.RemoteAddr = "127.0.0.2:9111" + createRec := httptest.NewRecorder() + handler.ServeHTTP(createRec, createReq) + if createRec.Code != http.StatusCreated { + t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) + } + + var payload map[string]any + if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { + t.Fatalf("decode create payload error = %v", err) + } + id, _ := payload["id"].(string) + if id == "" { + t.Fatalf("create payload missing id") + } + + firstRawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) + firstRawReq.RemoteAddr = "127.0.0.1:9111" + firstRawRec := httptest.NewRecorder() + handler.ServeHTTP(firstRawRec, firstRawReq) + if firstRawRec.Code != http.StatusOK { + t.Fatalf("first raw status = %d, want %d", firstRawRec.Code, http.StatusOK) + } + + secondRawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) + secondRawReq.RemoteAddr = "127.0.0.1:9111" + secondRawRec := httptest.NewRecorder() + handler.ServeHTTP(secondRawRec, secondRawReq) + if secondRawRec.Code != http.StatusTooManyRequests { + t.Fatalf("second raw status = %d, want %d", secondRawRec.Code, http.StatusTooManyRequests) + } +} + +// TestSecurityHeadersOnAllResponses verifies the new always-on security headers middleware +// (wired early, applies to UI/API/assets/plain/other responses per audit). +func TestSecurityHeadersOnAllResponses(t *testing.T) { + t.Parallel() + + handler := newTestHandler(t, testServerOptions{ + maxUploadBytes: 1024, + defaultTTL: time.Hour, + rateLimitEnable: false, + }) + + cases := []struct { + path string + method string + }{ + {"/", "GET"}, + {"/u", "GET"}, + {"/api/config", "GET"}, + {"/this-is-404", "GET"}, + {"/robots.txt", "GET"}, + {"/favicon.svg", "GET"}, + } + for _, c := range cases { + req := httptest.NewRequest(c.method, c.path, nil) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if got := rec.Header().Get("X-Content-Type-Options"); got != "nosniff" { + t.Errorf("path=%s X-Content-Type-Options=%q want nosniff", c.path, got) + } + if got := rec.Header().Get("Referrer-Policy"); got != "strict-origin-when-cross-origin" { + t.Errorf("path=%s Referrer-Policy=%q want strict-origin-when-cross-origin", c.path, got) + } + if got := rec.Header().Get("X-Frame-Options"); got != "DENY" { + t.Errorf("path=%s X-Frame-Options=%q want DENY", c.path, got) + } + if got := rec.Header().Get("Strict-Transport-Security"); !strings.Contains(got, "max-age=31536000") { + t.Errorf("path=%s HSTS=%q want max-age", c.path, got) + } + csp := rec.Header().Get("Content-Security-Policy") + if !strings.Contains(csp, "default-src 'self'") || !strings.Contains(csp, "frame-ancestors 'none'") { + t.Errorf("path=%s CSP=%q missing expected directives", c.path, csp) + } + } +} diff --git a/internal/http/handlers_integration_test.go b/internal/http/handlers_integration_test.go deleted file mode 100644 index b025bfb..0000000 --- a/internal/http/handlers_integration_test.go +++ /dev/null @@ -1,1088 +0,0 @@ -package httpapi - -import ( - "bytes" - "encoding/json" - "fmt" - "io" - "log" - "mime" - "mime/multipart" - "net/http" - "net/http/httptest" - "path/filepath" - "strings" - "sync" - "testing" - "time" - - "scratchbox/internal/config" - "scratchbox/internal/storage" -) - -func TestCreateReadThenExpireScratch(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: 120 * time.Millisecond, - rateLimitEnable: false, - }) - - createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString("hello scratch")) - createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") - createReq.RemoteAddr = "127.0.0.1:9999" - createRec := httptest.NewRecorder() - handler.ServeHTTP(createRec, createReq) - if createRec.Code != http.StatusCreated { - t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) - } - - var payload map[string]any - if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { - t.Fatalf("decode create payload error = %v", err) - } - id, _ := payload["id"].(string) - if id == "" { - t.Fatalf("create payload missing id") - } - - metaReq := httptest.NewRequest(http.MethodGet, "/api/scratch/"+id, nil) - metaReq.RemoteAddr = "127.0.0.1:9999" - metaRec := httptest.NewRecorder() - handler.ServeHTTP(metaRec, metaReq) - if metaRec.Code != http.StatusOK { - t.Fatalf("read metadata status = %d, want %d", metaRec.Code, http.StatusOK) - } - - rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) - rawReq.RemoteAddr = "127.0.0.1:9999" - rawRec := httptest.NewRecorder() - handler.ServeHTTP(rawRec, rawReq) - if rawRec.Code != http.StatusOK { - t.Fatalf("read raw status = %d, want %d", rawRec.Code, http.StatusOK) - } - if got := rawRec.Body.String(); got != "hello scratch" { - t.Fatalf("raw body = %q, want %q", got, "hello scratch") - } - - time.Sleep(180 * time.Millisecond) - - expiredReq := httptest.NewRequest(http.MethodGet, "/api/scratch/"+id, nil) - expiredReq.RemoteAddr = "127.0.0.1:9999" - expiredRec := httptest.NewRecorder() - handler.ServeHTTP(expiredRec, expiredReq) - if expiredRec.Code != http.StatusGone { - t.Fatalf("expired status = %d, want %d", expiredRec.Code, http.StatusGone) - } -} - -func TestUploadedScratchViewAndRawBeforeAndAfterExpiry(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: 120 * time.Millisecond, - rateLimitEnable: false, - }) - - createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString("hello from upload flow")) - createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") - createReq.RemoteAddr = "127.0.0.1:5090" - createRec := httptest.NewRecorder() - handler.ServeHTTP(createRec, createReq) - if createRec.Code != http.StatusCreated { - t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) - } - - var payload map[string]any - if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { - t.Fatalf("decode create payload error = %v", err) - } - id, _ := payload["id"].(string) - if id == "" { - t.Fatalf("create payload missing id") - } - - viewReq := httptest.NewRequest(http.MethodGet, "/s/"+id, nil) - viewReq.RemoteAddr = "127.0.0.1:5091" - viewRec := httptest.NewRecorder() - handler.ServeHTTP(viewRec, viewReq) - if viewRec.Code != http.StatusOK { - t.Fatalf("view-before-expiry status = %d, want %d", viewRec.Code, http.StatusOK) - } - if !strings.Contains(viewRec.Body.String(), `id="app"`) { - t.Fatalf("expected SPA shell body for /s/{id}, got %q", viewRec.Body.String()) - } - - rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) - rawReq.RemoteAddr = "127.0.0.1:5092" - rawRec := httptest.NewRecorder() - handler.ServeHTTP(rawRec, rawReq) - if rawRec.Code != http.StatusOK { - t.Fatalf("raw-before-expiry status = %d, want %d", rawRec.Code, http.StatusOK) - } - if got := rawRec.Body.String(); got != "hello from upload flow" { - t.Fatalf("raw-before-expiry body = %q, want %q", got, "hello from upload flow") - } - - time.Sleep(180 * time.Millisecond) - - viewExpiredReq := httptest.NewRequest(http.MethodGet, "/s/"+id, nil) - viewExpiredReq.RemoteAddr = "127.0.0.1:5093" - viewExpiredRec := httptest.NewRecorder() - handler.ServeHTTP(viewExpiredRec, viewExpiredReq) - if viewExpiredRec.Code != http.StatusGone { - t.Fatalf("view-after-expiry status = %d, want %d", viewExpiredRec.Code, http.StatusGone) - } - if !strings.Contains(viewExpiredRec.Body.String(), `id="app"`) { - t.Fatalf("expected SPA shell body for expired /s/{id}, got %q", viewExpiredRec.Body.String()) - } - - rawExpiredReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) - rawExpiredReq.RemoteAddr = "127.0.0.1:5094" - rawExpiredRec := httptest.NewRecorder() - handler.ServeHTTP(rawExpiredRec, rawExpiredReq) - if rawExpiredRec.Code != http.StatusGone { - t.Fatalf("raw-after-expiry status = %d, want %d", rawExpiredRec.Code, http.StatusGone) - } - if rawExpiredRec.Body.Len() != 0 { - t.Fatalf("expected empty raw body after expiry, got %q", rawExpiredRec.Body.String()) - } -} - -func TestReadRoutesAreRateLimitedWhenEnabled(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: true, - }) - - createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString("hello scratch")) - createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") - createReq.RemoteAddr = "127.0.0.2:9111" - createRec := httptest.NewRecorder() - handler.ServeHTTP(createRec, createReq) - if createRec.Code != http.StatusCreated { - t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) - } - - var payload map[string]any - if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { - t.Fatalf("decode create payload error = %v", err) - } - id, _ := payload["id"].(string) - if id == "" { - t.Fatalf("create payload missing id") - } - - firstRawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) - firstRawReq.RemoteAddr = "127.0.0.1:9111" - firstRawRec := httptest.NewRecorder() - handler.ServeHTTP(firstRawRec, firstRawReq) - if firstRawRec.Code != http.StatusOK { - t.Fatalf("first raw status = %d, want %d", firstRawRec.Code, http.StatusOK) - } - - secondRawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) - secondRawReq.RemoteAddr = "127.0.0.1:9111" - secondRawRec := httptest.NewRecorder() - handler.ServeHTTP(secondRawRec, secondRawReq) - if secondRawRec.Code != http.StatusTooManyRequests { - t.Fatalf("second raw status = %d, want %d", secondRawRec.Code, http.StatusTooManyRequests) - } -} - -func TestCreateScratchRejectsBodyOverLimit(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 8, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - req := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString("this is too large")) - req.Header.Set("Content-Type", "text/plain; charset=utf-8") - req.RemoteAddr = "127.0.0.1:5050" - rec := httptest.NewRecorder() - - handler.ServeHTTP(rec, req) - if rec.Code != http.StatusRequestEntityTooLarge { - t.Fatalf("status = %d, want %d", rec.Code, http.StatusRequestEntityTooLarge) - } - if !bytes.Contains(rec.Body.Bytes(), []byte("upload exceeds limits.max_upload_size")) { - t.Fatalf("unexpected body: %q", rec.Body.String()) - } - if !bytes.Contains(rec.Body.Bytes(), []byte("(8 bytes)")) { - t.Fatalf("expected response to include configured max size, got: %q", rec.Body.String()) - } -} - -func TestCreateScratchRejectsMultipartFileOverLimit(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 8, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - var body bytes.Buffer - writer := multipart.NewWriter(&body) - file, err := writer.CreateFormFile("file", "too-big.txt") - if err != nil { - t.Fatalf("CreateFormFile() error = %v", err) - } - if _, err := file.Write([]byte("this is too large")); err != nil { - t.Fatalf("write file error = %v", err) - } - if err := writer.Close(); err != nil { - t.Fatalf("multipart close error = %v", err) - } - - req := httptest.NewRequest(http.MethodPost, "/api/scratch", &body) - req.Header.Set("Content-Type", writer.FormDataContentType()) - req.RemoteAddr = "127.0.0.1:5060" - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - if rec.Code != http.StatusRequestEntityTooLarge { - t.Fatalf("status = %d, want %d", rec.Code, http.StatusRequestEntityTooLarge) - } - if !bytes.Contains(rec.Body.Bytes(), []byte("upload exceeds limits.max_upload_size")) { - t.Fatalf("unexpected body: %q", rec.Body.String()) - } - if !bytes.Contains(rec.Body.Bytes(), []byte("(8 bytes)")) { - t.Fatalf("expected response to include configured max size, got: %q", rec.Body.String()) - } -} - -func TestCreateScratchRejectsMultipleMultipartFiles(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - var body bytes.Buffer - writer := multipart.NewWriter(&body) - file1, err := writer.CreateFormFile("file", "first.txt") - if err != nil { - t.Fatalf("CreateFormFile first error = %v", err) - } - if _, err := file1.Write([]byte("first")); err != nil { - t.Fatalf("write first file error = %v", err) - } - file2, err := writer.CreateFormFile("file", "second.txt") - if err != nil { - t.Fatalf("CreateFormFile second error = %v", err) - } - if _, err := file2.Write([]byte("second")); err != nil { - t.Fatalf("write second file error = %v", err) - } - if err := writer.Close(); err != nil { - t.Fatalf("multipart close error = %v", err) - } - - req := httptest.NewRequest(http.MethodPost, "/api/scratch", &body) - req.Header.Set("Content-Type", writer.FormDataContentType()) - req.RemoteAddr = "127.0.0.1:5051" - rec := httptest.NewRecorder() - - handler.ServeHTTP(rec, req) - if rec.Code != http.StatusBadRequest { - t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest) - } - if !bytes.Contains(rec.Body.Bytes(), []byte("only one file is allowed per scratch upload")) { - t.Fatalf("unexpected body: %q", rec.Body.String()) - } -} - -func TestCreateScratchMultipartContentOnly(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - var body bytes.Buffer - writer := multipart.NewWriter(&body) - if err := writer.WriteField("content", "hello from content field"); err != nil { - t.Fatalf("WriteField() error = %v", err) - } - if err := writer.Close(); err != nil { - t.Fatalf("multipart close error = %v", err) - } - - req := httptest.NewRequest(http.MethodPost, "/api/scratch", &body) - req.Header.Set("Content-Type", writer.FormDataContentType()) - req.RemoteAddr = "127.0.0.1:5052" - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - if rec.Code != http.StatusCreated { - t.Fatalf("status = %d, want %d", rec.Code, http.StatusCreated) - } -} - -func TestCreateScratchMultipartPreservesFilenameForRawDownload(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - var body bytes.Buffer - writer := multipart.NewWriter(&body) - file, err := writer.CreateFormFile("file", "example upload.txt") - if err != nil { - t.Fatalf("CreateFormFile() error = %v", err) - } - if _, err := file.Write([]byte("uploaded file body")); err != nil { - t.Fatalf("write file error = %v", err) - } - if err := writer.Close(); err != nil { - t.Fatalf("multipart close error = %v", err) - } - - createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", &body) - createReq.Header.Set("Content-Type", writer.FormDataContentType()) - createReq.RemoteAddr = "127.0.0.1:5058" - createRec := httptest.NewRecorder() - handler.ServeHTTP(createRec, createReq) - if createRec.Code != http.StatusCreated { - t.Fatalf("status = %d, want %d", createRec.Code, http.StatusCreated) - } - - var payload map[string]any - if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { - t.Fatalf("decode create payload error = %v", err) - } - id, _ := payload["id"].(string) - if id == "" { - t.Fatalf("create payload missing id") - } - - rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) - rawReq.RemoteAddr = "127.0.0.1:5059" - rawRec := httptest.NewRecorder() - handler.ServeHTTP(rawRec, rawReq) - if rawRec.Code != http.StatusOK { - t.Fatalf("raw status = %d, want %d", rawRec.Code, http.StatusOK) - } - if got := rawRec.Body.String(); got != "uploaded file body" { - t.Fatalf("raw body = %q, want %q", got, "uploaded file body") - } - - disposition := rawRec.Header().Get("Content-Disposition") - if disposition == "" { - t.Fatalf("expected Content-Disposition header") - } - _, params, err := mime.ParseMediaType(disposition) - if err != nil { - t.Fatalf("ParseMediaType() error = %v", err) - } - if got := params["filename"]; got != "example upload.txt" { - t.Fatalf("filename = %q, want %q", got, "example upload.txt") - } -} - -func TestCreateScratchMultipartRejectsFileAndContent(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - var body bytes.Buffer - writer := multipart.NewWriter(&body) - file, err := writer.CreateFormFile("file", "one.txt") - if err != nil { - t.Fatalf("CreateFormFile() error = %v", err) - } - if _, err := file.Write([]byte("file content")); err != nil { - t.Fatalf("write file error = %v", err) - } - if err := writer.WriteField("content", "extra content"); err != nil { - t.Fatalf("WriteField() error = %v", err) - } - if err := writer.Close(); err != nil { - t.Fatalf("multipart close error = %v", err) - } - - req := httptest.NewRequest(http.MethodPost, "/api/scratch", &body) - req.Header.Set("Content-Type", writer.FormDataContentType()) - req.RemoteAddr = "127.0.0.1:5053" - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - if rec.Code != http.StatusBadRequest { - t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest) - } -} - -func TestCreateScratchMultipartRequiresFileOrContent(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - var body bytes.Buffer - writer := multipart.NewWriter(&body) - if err := writer.Close(); err != nil { - t.Fatalf("multipart close error = %v", err) - } - - req := httptest.NewRequest(http.MethodPost, "/api/scratch", &body) - req.Header.Set("Content-Type", writer.FormDataContentType()) - req.RemoteAddr = "127.0.0.1:5054" - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - if rec.Code != http.StatusBadRequest { - t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest) - } -} - -func TestGetScratchNotFoundReturnsGone(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - req := httptest.NewRequest(http.MethodGet, "/api/scratch/does-not-exist", nil) - req.RemoteAddr = "127.0.0.1:5055" - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - if rec.Code != http.StatusGone { - t.Fatalf("status = %d, want %d", rec.Code, http.StatusGone) - } -} - -func TestRawScratchDefaultsContentType(t *testing.T) { - t.Parallel() - - handler, store := newTestHandlerAndStore(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - meta, err := store.Create(t.Context(), strings.NewReader("no content type"), "", time.Hour) - if err != nil { - t.Fatalf("store.Create() error = %v", err) - } - - req := httptest.NewRequest(http.MethodGet, "/api/raw/"+meta.ID, nil) - req.RemoteAddr = "127.0.0.1:5056" - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) - } - if got := rec.Header().Get("Content-Type"); got != "application/octet-stream" { - t.Fatalf("Content-Type = %q, want %q", got, "application/octet-stream") - } -} - -func TestRawScratchSupportsRangeRequests(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString("hello scratch")) - createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") - createReq.RemoteAddr = "127.0.0.1:5061" - createRec := httptest.NewRecorder() - handler.ServeHTTP(createRec, createReq) - if createRec.Code != http.StatusCreated { - t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) - } - - var payload map[string]any - if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { - t.Fatalf("decode create payload error = %v", err) - } - id, _ := payload["id"].(string) - if id == "" { - t.Fatalf("create payload missing id") - } - - rangeReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) - rangeReq.Header.Set("Range", "bytes=0-4") - rangeReq.RemoteAddr = "127.0.0.1:5062" - rangeRec := httptest.NewRecorder() - handler.ServeHTTP(rangeRec, rangeReq) - if rangeRec.Code != http.StatusPartialContent { - t.Fatalf("range status = %d, want %d", rangeRec.Code, http.StatusPartialContent) - } - if got := rangeRec.Body.String(); got != "hello" { - t.Fatalf("range body = %q, want %q", got, "hello") - } - if got := rangeRec.Header().Get("Content-Range"); got != "bytes 0-4/13" { - t.Fatalf("Content-Range = %q, want %q", got, "bytes 0-4/13") - } -} - -func TestRawScratchRejectsUnsatisfiableRange(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString("hello scratch")) - createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") - createReq.RemoteAddr = "127.0.0.1:5063" - createRec := httptest.NewRecorder() - handler.ServeHTTP(createRec, createReq) - if createRec.Code != http.StatusCreated { - t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) - } - - var payload map[string]any - if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { - t.Fatalf("decode create payload error = %v", err) - } - id, _ := payload["id"].(string) - if id == "" { - t.Fatalf("create payload missing id") - } - - rangeReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) - rangeReq.Header.Set("Range", "bytes=999-1000") - rangeReq.RemoteAddr = "127.0.0.1:5064" - rangeRec := httptest.NewRecorder() - handler.ServeHTTP(rangeRec, rangeReq) - if rangeRec.Code != http.StatusRequestedRangeNotSatisfiable { - t.Fatalf("range status = %d, want %d", rangeRec.Code, http.StatusRequestedRangeNotSatisfiable) - } - if got := rangeRec.Header().Get("Content-Range"); got != "bytes */13" { - t.Fatalf("Content-Range = %q, want %q", got, "bytes */13") - } -} - -func TestViewUploadAndIndexRender(t *testing.T) { - t.Parallel() - - handler, store := newTestHandlerAndStore(t, testServerOptions{ - maxUploadBytes: 10 * 1024 * 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - meta, err := store.Create(t.Context(), strings.NewReader("hello view"), "text/plain; charset=utf-8", time.Hour) - if err != nil { - t.Fatalf("store.Create() error = %v", err) - } - - indexReq := httptest.NewRequest(http.MethodGet, "/", nil) - indexRec := httptest.NewRecorder() - handler.ServeHTTP(indexRec, indexReq) - if indexRec.Code != http.StatusOK { - t.Fatalf("index status = %d, want %d", indexRec.Code, http.StatusOK) - } - if !strings.Contains(indexRec.Body.String(), `id="app"`) { - t.Fatalf("expected index page to include SPA mount point") - } - if !strings.Contains(indexRec.Body.String(), `type="module"`) { - t.Fatalf("expected index page to load frontend bundle") - } - - uploadReq := httptest.NewRequest(http.MethodGet, "/u", nil) - uploadRec := httptest.NewRecorder() - handler.ServeHTTP(uploadRec, uploadReq) - if uploadRec.Code != http.StatusOK { - t.Fatalf("upload status = %d, want %d", uploadRec.Code, http.StatusOK) - } - if !strings.Contains(uploadRec.Body.String(), `id="app"`) { - t.Fatalf("expected upload page to include SPA mount point") - } - - viewReq := httptest.NewRequest(http.MethodGet, "/s/"+meta.ID, nil) - viewRec := httptest.NewRecorder() - handler.ServeHTTP(viewRec, viewReq) - if viewRec.Code != http.StatusOK { - t.Fatalf("view status = %d, want %d", viewRec.Code, http.StatusOK) - } - if !strings.Contains(viewRec.Body.String(), `id="app"`) { - t.Fatalf("expected scratch route to render SPA shell") - } -} - -func TestViewAndRawScratchExpired(t *testing.T) { - t.Parallel() - - handler, store := newTestHandlerAndStore(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - meta, err := store.Create(t.Context(), strings.NewReader("expired body"), "text/plain", -time.Second) - if err != nil { - t.Fatalf("store.Create() error = %v", err) - } - - viewReq := httptest.NewRequest(http.MethodGet, "/s/"+meta.ID, nil) - viewRec := httptest.NewRecorder() - handler.ServeHTTP(viewRec, viewReq) - if viewRec.Code != http.StatusGone { - t.Fatalf("view status = %d, want %d", viewRec.Code, http.StatusGone) - } - - rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+meta.ID, nil) - rawRec := httptest.NewRecorder() - handler.ServeHTTP(rawRec, rawReq) - if rawRec.Code != http.StatusGone { - t.Fatalf("raw status after lazy delete = %d, want %d", rawRec.Code, http.StatusGone) - } -} - -func TestRawScratchExpiredReturnsStatusOnly(t *testing.T) { - t.Parallel() - - handler, store := newTestHandlerAndStore(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - meta, err := store.Create(t.Context(), strings.NewReader("expired body"), "text/plain", -time.Second) - if err != nil { - t.Fatalf("store.Create() error = %v", err) - } - - rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+meta.ID, nil) - rawRec := httptest.NewRecorder() - handler.ServeHTTP(rawRec, rawReq) - if rawRec.Code != http.StatusGone { - t.Fatalf("raw status = %d, want %d", rawRec.Code, http.StatusGone) - } - if rawRec.Body.Len() != 0 { - t.Fatalf("expected empty response body, got %q", rawRec.Body.String()) - } -} - -func TestViewAndRawUnknownIDsReturnGone(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - viewReq := httptest.NewRequest(http.MethodGet, "/s/does-not-exist", nil) - viewRec := httptest.NewRecorder() - handler.ServeHTTP(viewRec, viewReq) - if viewRec.Code != http.StatusGone { - t.Fatalf("view status = %d, want %d", viewRec.Code, http.StatusGone) - } - - rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/does-not-exist", nil) - rawRec := httptest.NewRecorder() - handler.ServeHTTP(rawRec, rawReq) - if rawRec.Code != http.StatusGone { - t.Fatalf("raw status = %d, want %d", rawRec.Code, http.StatusGone) - } - if rawRec.Body.Len() != 0 { - t.Fatalf("expected empty response body, got %q", rawRec.Body.String()) - } -} - -func TestNeverUploadedScratchIDsAlsoReturnGone(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - viewReq := httptest.NewRequest(http.MethodGet, "/s/never-uploaded-id", nil) - viewReq.RemoteAddr = "127.0.0.1:5095" - viewRec := httptest.NewRecorder() - handler.ServeHTTP(viewRec, viewReq) - if viewRec.Code != http.StatusGone { - t.Fatalf("view-never-uploaded status = %d, want %d", viewRec.Code, http.StatusGone) - } - - rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/never-uploaded-id", nil) - rawReq.RemoteAddr = "127.0.0.1:5096" - rawRec := httptest.NewRecorder() - handler.ServeHTTP(rawRec, rawReq) - if rawRec.Code != http.StatusGone { - t.Fatalf("raw-never-uploaded status = %d, want %d", rawRec.Code, http.StatusGone) - } - if rawRec.Body.Len() != 0 { - t.Fatalf("expected empty raw body for never-uploaded id, got %q", rawRec.Body.String()) - } - - metaReq := httptest.NewRequest(http.MethodGet, "/api/scratch/never-uploaded-id", nil) - metaReq.RemoteAddr = "127.0.0.1:5097" - metaRec := httptest.NewRecorder() - handler.ServeHTTP(metaRec, metaReq) - if metaRec.Code != http.StatusGone { - t.Fatalf("metadata-never-uploaded status = %d, want %d", metaRec.Code, http.StatusGone) - } -} - -func TestCreateScratchRejectsMalformedMultipart(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - req := httptest.NewRequest(http.MethodPost, "/api/scratch", strings.NewReader("not-multipart")) - req.Header.Set("Content-Type", "multipart/form-data; boundary=missing") - req.RemoteAddr = "127.0.0.1:5057" - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - if rec.Code != http.StatusBadRequest { - t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest) - } -} - -func TestConcurrentUploadsRemainConsistentAndReadable(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024 * 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - const uploads = 80 - - type createdScratch struct { - id string - body string - } - - resultsCh := make(chan createdScratch, uploads) - errCh := make(chan error, uploads) - var wg sync.WaitGroup - - for i := 0; i < uploads; i++ { - wg.Add(1) - go func(i int) { - defer wg.Done() - - body := fmt.Sprintf("parallel-upload-%03d-%s", i, strings.Repeat("x", i%17)) - createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString(body)) - createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") - createReq.RemoteAddr = fmt.Sprintf("127.0.0.1:%d", 8200+i) - createRec := httptest.NewRecorder() - handler.ServeHTTP(createRec, createReq) - if createRec.Code != http.StatusCreated { - errCh <- fmt.Errorf("create status = %d, want %d", createRec.Code, http.StatusCreated) - return - } - - var payload map[string]any - if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { - errCh <- fmt.Errorf("decode create payload error: %w", err) - return - } - id, _ := payload["id"].(string) - if id == "" { - errCh <- fmt.Errorf("create payload missing id") - return - } - - metaReq := httptest.NewRequest(http.MethodGet, "/api/scratch/"+id, nil) - metaRec := httptest.NewRecorder() - handler.ServeHTTP(metaRec, metaReq) - if metaRec.Code != http.StatusOK { - errCh <- fmt.Errorf("metadata status = %d, want %d", metaRec.Code, http.StatusOK) - return - } - - rawReq := httptest.NewRequest(http.MethodGet, "/api/raw/"+id, nil) - rawRec := httptest.NewRecorder() - handler.ServeHTTP(rawRec, rawReq) - if rawRec.Code != http.StatusOK { - errCh <- fmt.Errorf("raw status = %d, want %d", rawRec.Code, http.StatusOK) - return - } - if got := rawRec.Body.String(); got != body { - errCh <- fmt.Errorf("raw body mismatch = %q, want %q", got, body) - return - } - - resultsCh <- createdScratch{id: id, body: body} - }(i) - } - - wg.Wait() - close(resultsCh) - close(errCh) - for err := range errCh { - t.Error(err) - } - - seen := make(map[string]struct{}, uploads) - for result := range resultsCh { - if _, exists := seen[result.id]; exists { - t.Errorf("duplicate scratch id generated: %s", result.id) - continue - } - seen[result.id] = struct{}{} - } - if len(seen) != uploads { - t.Fatalf("unique IDs = %d, want %d", len(seen), uploads) - } -} - -func TestParallelClientsCanReadSameScratchAcrossAllReadRoutes(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - const body = "parallel readers" - createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString(body)) - createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") - createReq.RemoteAddr = "127.0.0.1:6010" - createRec := httptest.NewRecorder() - handler.ServeHTTP(createRec, createReq) - if createRec.Code != http.StatusCreated { - t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) - } - - var payload map[string]any - if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { - t.Fatalf("decode create payload error = %v", err) - } - id, _ := payload["id"].(string) - if id == "" { - t.Fatalf("create payload missing id") - } - - paths := []string{ - "/api/raw/" + id, - "/s/" + id, - "/api/scratch/" + id, - } - const requests = 120 - - errCh := make(chan error, requests) - var wg sync.WaitGroup - for i := 0; i < requests; i++ { - wg.Add(1) - go func(i int) { - defer wg.Done() - path := paths[i%len(paths)] - req := httptest.NewRequest(http.MethodGet, path, nil) - req.RemoteAddr = fmt.Sprintf("127.0.0.1:%d", 6100+i) - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - if rec.Code != http.StatusOK { - errCh <- fmt.Errorf("path %s status = %d, want %d", path, rec.Code, http.StatusOK) - return - } - - switch { - case strings.HasPrefix(path, "/api/raw/"): - if got := rec.Body.String(); got != body { - errCh <- fmt.Errorf("raw body = %q, want %q", got, body) - } - case strings.HasPrefix(path, "/s/"): - if !strings.Contains(rec.Body.String(), `id="app"`) { - errCh <- fmt.Errorf("view body missing SPA marker") - } - case strings.HasPrefix(path, "/api/scratch/"): - var gotPayload map[string]any - if err := json.Unmarshal(rec.Body.Bytes(), &gotPayload); err != nil { - errCh <- fmt.Errorf("metadata decode error: %w", err) - return - } - if gotID, _ := gotPayload["id"].(string); gotID != id { - errCh <- fmt.Errorf("metadata id = %q, want %q", gotID, id) - } - } - }(i) - } - wg.Wait() - close(errCh) - for err := range errCh { - t.Error(err) - } -} - -func TestParallelClientsExpiredScratchReadsReturnGone(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: 80 * time.Millisecond, - rateLimitEnable: false, - }) - - createReq := httptest.NewRequest(http.MethodPost, "/api/scratch", bytes.NewBufferString("expires soon")) - createReq.Header.Set("Content-Type", "text/plain; charset=utf-8") - createReq.RemoteAddr = "127.0.0.1:7010" - createRec := httptest.NewRecorder() - handler.ServeHTTP(createRec, createReq) - if createRec.Code != http.StatusCreated { - t.Fatalf("create status = %d, want %d", createRec.Code, http.StatusCreated) - } - - var payload map[string]any - if err := json.Unmarshal(createRec.Body.Bytes(), &payload); err != nil { - t.Fatalf("decode create payload error = %v", err) - } - id, _ := payload["id"].(string) - if id == "" { - t.Fatalf("create payload missing id") - } - - time.Sleep(130 * time.Millisecond) - - paths := []string{ - "/api/raw/" + id, - "/s/" + id, - "/api/scratch/" + id, - } - const requests = 90 - - errCh := make(chan error, requests) - var wg sync.WaitGroup - for i := 0; i < requests; i++ { - wg.Add(1) - go func(i int) { - defer wg.Done() - path := paths[i%len(paths)] - req := httptest.NewRequest(http.MethodGet, path, nil) - req.RemoteAddr = fmt.Sprintf("127.0.0.1:%d", 7100+i) - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - if rec.Code != http.StatusGone { - errCh <- fmt.Errorf("expired path %s status = %d, want %d", path, rec.Code, http.StatusGone) - return - } - - if strings.HasPrefix(path, "/s/") && !strings.Contains(rec.Body.String(), `id="app"`) { - errCh <- fmt.Errorf("expired view body missing SPA marker") - } - }(i) - } - wg.Wait() - close(errCh) - for err := range errCh { - t.Error(err) - } -} - -func TestUnknownPathReturnsNotFoundPage(t *testing.T) { - t.Parallel() - - handler := newTestHandler(t, testServerOptions{ - maxUploadBytes: 1024, - defaultTTL: time.Hour, - rateLimitEnable: false, - }) - - req := httptest.NewRequest(http.MethodGet, "/this-path-does-not-exist", nil) - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - if rec.Code != http.StatusNotFound { - t.Fatalf("status = %d, want %d", rec.Code, http.StatusNotFound) - } - if !strings.Contains(rec.Body.String(), `id="app"`) { - t.Fatalf("unexpected not found body: %q", rec.Body.String()) - } -} - -type testServerOptions struct { - maxUploadBytes int64 - defaultTTL time.Duration - rateLimitEnable bool -} - -func newTestHandler(t *testing.T, opts testServerOptions) http.Handler { - t.Helper() - - handler, _ := newTestHandlerAndStore(t, opts) - return handler -} - -func newTestHandlerAndStore(t *testing.T, opts testServerOptions) (http.Handler, *storage.FilesystemStore) { - t.Helper() - - dataDir := filepath.Join(t.TempDir(), "data") - store, err := storage.NewFilesystemStore(dataDir, testStorageKey) - if err != nil { - t.Fatalf("NewFilesystemStore() error = %v", err) - } - - cfg := config.Config{ - Server: config.ServerConfig{ - ListenAddr: ":0", - }, - Limits: config.LimitsConfig{ - MaxUploadSizeBytes: opts.maxUploadBytes, - DefaultTTLDuration: opts.defaultTTL, - RawCacheMaxBytes: 64 * 1024 * 1024, - RawCacheMaxEntries: 32, - }, - Storage: config.StorageConfig{ - DataDir: dataDir, - }, - Security: config.SecurityConfig{ - RateLimitUI: config.RateLimitConfig{ - Enabled: opts.rateLimitEnable, - RequestsPerMinute: 120, - Burst: 1, - }, - RateLimitAPIRead: config.RateLimitConfig{ - Enabled: opts.rateLimitEnable, - RequestsPerMinute: 120, - Burst: 1, - }, - RateLimitAPIWrite: config.RateLimitConfig{ - Enabled: opts.rateLimitEnable, - RequestsPerMinute: 120, - Burst: 1, - }, - }, - } - - srv := &Server{ - cfg: cfg, - store: store, - logger: log.New(io.Discard, "", 0), - rawCache: newRawContentCache(cfg.Limits.RawCacheMaxEntries, cfg.Limits.RawCacheMaxBytes), - } - return srv.Routes(), store -} diff --git a/internal/http/handlers_unit_test.go b/internal/http/handlers_unit_test.go index 2ed8670..62dcfea 100644 --- a/internal/http/handlers_unit_test.go +++ b/internal/http/handlers_unit_test.go @@ -15,8 +15,6 @@ import ( "scratchbox/internal/storage" ) -var testStorageKey = []byte("0123456789abcdef0123456789abcdef") - func TestMultipartFileCount(t *testing.T) { t.Parallel() @@ -61,6 +59,7 @@ func TestNewServerInitializes(t *testing.T) { RateLimitUI: config.RateLimitConfig{Enabled: false, RequestsPerMinute: 30, Burst: 10}, RateLimitAPIRead: config.RateLimitConfig{Enabled: false, RequestsPerMinute: 30, Burst: 10}, RateLimitAPIWrite: config.RateLimitConfig{Enabled: false, RequestsPerMinute: 30, Burst: 10}, + HSTSEnabled: true, }, } @@ -132,6 +131,7 @@ func TestWriteCreateErrorBranchesWithConfig(t *testing.T) { RateLimitUI: config.RateLimitConfig{Enabled: false, RequestsPerMinute: 30, Burst: 10}, RateLimitAPIRead: config.RateLimitConfig{Enabled: false, RequestsPerMinute: 30, Burst: 10}, RateLimitAPIWrite: config.RateLimitConfig{Enabled: false, RequestsPerMinute: 30, Burst: 10}, + HSTSEnabled: true, }, } if _, err := NewServer(cfg, store, log.New(io.Discard, "", 0), log.New(io.Discard, "", 0)); err != nil { @@ -147,3 +147,41 @@ func containsAll(haystack string, needles ...string) bool { } return true } + +// TestInjectMetaEscapesSpecialChars exercises the server-side meta injection +// (used for /s/{id} and 410 pages) with filenames/titles containing HTML special +// chars. This provides basic coverage/golden-like check for the replace+escape +// logic against the exact literals in the committed static shell (addresses +// fragility note without changing to full template). +func TestInjectMetaEscapesSpecialChars(t *testing.T) { + t.Parallel() + + // Minimal shell containing exactly the replace targets used by injectMeta. + // (Real index.html from Vite is larger but uses these literal strings.) + shell := []byte(` + +Scratchbox + + + + + +`) + + tricky := `report & "notes" <2026> 'foo'` + gotBytes := injectMeta(shell, tricky, tricky, tricky) + got := string(gotBytes) + + // Must have escaped the specials. + if !strings.Contains(got, `&`) || !strings.Contains(got, `<`) || !strings.Contains(got, `>`) || !strings.Contains(got, `"`) { + t.Fatalf("injectMeta did not escape specials in output: %q", got) + } + // Title etc updated (escaped form present). + if !strings.Contains(got, `report & "notes" <2026>`) { + t.Fatalf("injectMeta title not updated or badly escaped: %q", got) + } + // Original static should be gone (count=1 replaces). + if strings.Contains(got, `Scratchbox`) { + t.Fatalf("static title not replaced") + } +} diff --git a/internal/http/middleware.go b/internal/http/middleware.go index f09c6f4..909e82a 100644 --- a/internal/http/middleware.go +++ b/internal/http/middleware.go @@ -18,6 +18,15 @@ type visitor struct { lastSeen time.Time } +// rateLimiterMaxVisitors caps the per-limiter visitor map (token buckets keyed by client IP). +// Prevents unbounded memory growth under many distinct clients (or DoS) while prune +// still removes stale (>ttl) entries on every Allow. 1024 is generous for the intended +// LAN/minimal-public use; scans are O(cap) worst case. +const ( + rateLimiterMaxVisitors = 1024 + rateLimiterPruneTTL = 10 * time.Minute +) + type RateLimiter struct { mu sync.Mutex visitors map[string]*visitor @@ -31,7 +40,7 @@ func NewRateLimiter(requestsPerMinute int, burst int) *RateLimiter { visitors: map[string]*visitor{}, rate: rate.Limit(float64(requestsPerMinute) / 60.0), burst: burst, - ttl: 10 * time.Minute, + ttl: rateLimiterPruneTTL, } } @@ -53,6 +62,13 @@ func (r *RateLimiter) Allow(ip string) bool { limiter: rate.NewLimiter(r.rate, r.burst), } r.visitors[ip] = v + if len(r.visitors) > rateLimiterMaxVisitors { + // evict arbitrary entry (prune already removed stales); keeps map bounded + for k := range r.visitors { + delete(r.visitors, k) + break + } + } } v.lastSeen = now @@ -235,3 +251,25 @@ func writeJSON(w http.ResponseWriter, status int, payload any) { w.WriteHeader(status) _ = json.NewEncoder(w).Encode(payload) } + +// SecurityHeadersMiddleware sets safe default security headers on all responses. +// HSTS is conditional on the hstsEnabled toggle (default true in config for public proxy+TLS use; +// false allows pure local HTTP as noted in README LAN Deployment Notes). +// Other headers always-on per minimal security defaults. +// CSP uses 'unsafe-inline' for style-src (and data: for media) due to current Vite/Svelte +// build output; this is an acknowledged defense-in-depth tradeoff for the minimal trusted UI +// (script-src remains strict 'self'; no user-controlled content in styles). +func SecurityHeadersMiddleware(hstsEnabled bool, next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + h := w.Header() + h.Set("X-Content-Type-Options", "nosniff") + h.Set("Referrer-Policy", "strict-origin-when-cross-origin") + h.Set("X-Frame-Options", "DENY") + if hstsEnabled { + h.Set("Strict-Transport-Security", "max-age=31536000; includeSubDomains") + } + h.Set("Content-Security-Policy", "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; media-src 'self' data:; font-src 'self'; connect-src 'self'; object-src 'none'; base-uri 'self'; frame-ancestors 'none'; form-action 'self';") + h.Set("Permissions-Policy", "camera=(), microphone=(), geolocation=()") + next.ServeHTTP(w, r) + }) +} diff --git a/internal/http/middleware_test.go b/internal/http/middleware_test.go index 01a51db..5365a6e 100644 --- a/internal/http/middleware_test.go +++ b/internal/http/middleware_test.go @@ -321,3 +321,60 @@ func TestAccessLogMiddlewareOnlyAllowsConfiguredRoutes(t *testing.T) { t.Fatalf("expected access log for allowed route, got %q", buf.String()) } } + +// TestSecurityHeadersMiddlewareSetsDefaults verifies the new middleware sets expected headers +// (X-Content-Type-Options, Referrer, CSP, HSTS conditional, etc.) on wrapped responses. +func TestSecurityHeadersMiddlewareSetsDefaults(t *testing.T) { + t.Parallel() + + next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/plain") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("ok")) + }) + + // HSTS enabled (default) + handler := SecurityHeadersMiddleware(true, next) + req := httptest.NewRequest(http.MethodGet, "/anything", nil) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status=%d want 200", rec.Code) + } + if got := rec.Header().Get("X-Content-Type-Options"); got != "nosniff" { + t.Fatalf("X-Content-Type-Options=%q want nosniff", got) + } + if got := rec.Header().Get("Referrer-Policy"); got != "strict-origin-when-cross-origin" { + t.Fatalf("Referrer-Policy=%q want strict-origin-when-cross-origin", got) + } + if got := rec.Header().Get("X-Frame-Options"); got != "DENY" { + t.Fatalf("X-Frame-Options=%q want DENY", got) + } + hsts := rec.Header().Get("Strict-Transport-Security") + if !strings.Contains(hsts, "max-age=31536000") { + t.Fatalf("HSTS (enabled)=%q missing max-age", hsts) + } + csp := rec.Header().Get("Content-Security-Policy") + if csp == "" || !strings.Contains(csp, "default-src 'self'") || !strings.Contains(csp, "frame-ancestors 'none'") { + t.Fatalf("CSP=%q missing required", csp) + } + pp := rec.Header().Get("Permissions-Policy") + if !strings.Contains(pp, "camera=()") { + t.Fatalf("Permissions-Policy=%q", pp) + } + if ct := rec.Header().Get("Content-Type"); ct != "text/plain" { + t.Fatalf("Content-Type was overwritten? got %q", ct) + } + + // HSTS disabled + handlerNoHSTS := SecurityHeadersMiddleware(false, next) + rec2 := httptest.NewRecorder() + handlerNoHSTS.ServeHTTP(rec2, req) + if hsts2 := rec2.Header().Get("Strict-Transport-Security"); hsts2 != "" { + t.Fatalf("HSTS (disabled) should be absent, got %q", hsts2) + } + // other headers still present + if got := rec2.Header().Get("X-Content-Type-Options"); got != "nosniff" { + t.Fatalf("X-Content-Type-Options (no hsts)=%q want nosniff", got) + } +} diff --git a/internal/http/raw_cache.go b/internal/http/raw_cache.go index e221bb5..b26a145 100644 --- a/internal/http/raw_cache.go +++ b/internal/http/raw_cache.go @@ -32,6 +32,19 @@ type rawContentLoad struct { err error } +// copyBytes returns a defensive copy so callers cannot mutate the cache's backing +// slice (which would affect concurrent/future hits for the same ID). Cost is +// acceptable: cache is small (default 32 entries) and holds decompressed raw +// content for range requests only. +func copyBytes(b []byte) []byte { + if b == nil { + return nil + } + c := make([]byte, len(b)) + copy(c, b) + return c +} + func newRawContentCache(maxEntries int, maxBytes int64) *rawContentCache { if maxEntries <= 0 { maxEntries = defaultRawCacheMaxEntries @@ -53,14 +66,14 @@ func (c *rawContentCache) GetOrLoad(id string, loader func() ([]byte, error)) ([ if elem, ok := c.items[id]; ok { c.lru.MoveToFront(elem) item := elem.Value.(*rawContentItem) - data := item.data + data := copyBytes(item.data) c.mu.Unlock() return data, nil, true } if load, ok := c.inFlight[id]; ok { c.mu.Unlock() <-load.wait - return load.data, load.err, false + return copyBytes(load.data), load.err, false } load := &rawContentLoad{wait: make(chan struct{})} @@ -79,7 +92,7 @@ func (c *rawContentCache) GetOrLoad(id string, loader func() ([]byte, error)) ([ c.mu.Unlock() close(load.wait) - return data, err, false + return copyBytes(data), err, false } func (c *rawContentCache) Delete(id string) { diff --git a/internal/storage/filesystem.go b/internal/storage/filesystem.go index 134c2db..eb4dc00 100644 --- a/internal/storage/filesystem.go +++ b/internal/storage/filesystem.go @@ -2,7 +2,6 @@ package storage import ( "compress/gzip" - "context" "crypto/aes" "crypto/cipher" "crypto/rand" @@ -107,11 +106,11 @@ func newFilesystemStore(dataDir string, defaultTTL time.Duration, encryptionKey return store, nil } -func (s *FilesystemStore) Create(ctx context.Context, body io.Reader, contentType string, ttl time.Duration) (Metadata, error) { - return s.CreateWithOriginalName(ctx, body, contentType, "", ttl) +func (s *FilesystemStore) Create(body io.Reader, contentType string, ttl time.Duration) (Metadata, error) { + return s.CreateWithOriginalName(body, contentType, "", ttl) } -func (s *FilesystemStore) CreateWithOriginalName(ctx context.Context, body io.Reader, contentType string, originalName string, ttl time.Duration) (Metadata, error) { +func (s *FilesystemStore) CreateWithOriginalName(body io.Reader, contentType string, originalName string, ttl time.Duration) (Metadata, error) { tempFile, err := os.CreateTemp(s.filesDir, "scratch.tmp-*") if err != nil { return Metadata{}, fmt.Errorf("create temp file: %w", err) @@ -264,9 +263,11 @@ func (s *FilesystemStore) Delete(id string) error { } s.mu.Unlock() - if err := os.Remove(meta.FilePath); err != nil && !errors.Is(err, os.ErrNotExist) { - return fmt.Errorf("remove scratch file: %w", err) - } + // Best-effort remove of the blob after metadata has been durably deleted from the index. + // On error we tolerate an orphan on-disk blob (no meta entry points to it); reads already + // fail cleanly with ErrNotFound. This prevents Delete from failing due to transient FS issues + // on the blob and avoids meta inconsistency. + _ = os.Remove(meta.FilePath) return nil } @@ -292,11 +293,20 @@ func (s *FilesystemStore) DeleteExpired(now time.Time) (int, error) { } s.mu.Unlock() + // Attempt *all* removes (do not abort on first); return count of expired + first remove err if any. + // Callers (cleanup worker) log errs; meta is already gone from index so orphans tolerated. + var firstRemoveErr error for _, meta := range expired { if err := os.Remove(meta.FilePath); err != nil && !errors.Is(err, os.ErrNotExist) { - return 0, fmt.Errorf("remove expired file %s: %w", meta.ID, err) + if firstRemoveErr == nil { + firstRemoveErr = fmt.Errorf("remove expired file %s: %w", meta.ID, err) + } + // continue to clean other entries } } + if firstRemoveErr != nil { + return len(expired), firstRemoveErr + } return len(expired), nil } diff --git a/internal/storage/filesystem_test.go b/internal/storage/filesystem_test.go index c152feb..926b8ea 100644 --- a/internal/storage/filesystem_test.go +++ b/internal/storage/filesystem_test.go @@ -2,7 +2,6 @@ package storage import ( "bytes" - "context" "crypto/aes" "crypto/cipher" "crypto/rand" @@ -44,7 +43,7 @@ func TestCreateStoresGzipAndOpenIsTransparent(t *testing.T) { } original := bytes.Repeat([]byte("hello scratch "), 1024) - meta, err := store.Create(context.Background(), bytes.NewReader(original), "text/plain; charset=utf-8", time.Hour) + meta, err := store.Create(bytes.NewReader(original), "text/plain; charset=utf-8", time.Hour) if err != nil { t.Fatalf("Create() error = %v", err) } @@ -89,7 +88,7 @@ func TestCreateEncryptedStoreDoesNotExposeGzipHeaderOnDisk(t *testing.T) { } original := bytes.Repeat([]byte("secret payload "), 1024) - meta, err := store.Create(context.Background(), bytes.NewReader(original), "text/plain", time.Hour) + meta, err := store.Create(bytes.NewReader(original), "text/plain", time.Hour) if err != nil { t.Fatalf("Create() error = %v", err) } @@ -149,7 +148,7 @@ func TestEncryptedStoreFailsToOpenWithWrongKey(t *testing.T) { if err != nil { t.Fatalf("NewEncryptedFilesystemStore(keyA) error = %v", err) } - _, err = storeA.Create(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", time.Hour) + _, err = storeA.Create(bytes.NewReader([]byte("hello")), "text/plain", time.Hour) if err != nil { t.Fatalf("Create() error = %v", err) } @@ -167,7 +166,7 @@ func TestCreateWithOriginalNameNormalizesFilename(t *testing.T) { t.Fatalf("NewFilesystemStore() error = %v", err) } - meta, err := store.CreateWithOriginalName(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", " ../../demo.txt ", time.Hour) + meta, err := store.CreateWithOriginalName(bytes.NewReader([]byte("hello")), "text/plain", " ../../demo.txt ", time.Hour) if err != nil { t.Fatalf("CreateWithOriginalName() error = %v", err) } @@ -196,11 +195,11 @@ func TestCreateWithOriginalNameSameContentGetsDifferentIDs(t *testing.T) { t.Fatalf("NewFilesystemStore() error = %v", err) } - first, err := store.CreateWithOriginalName(context.Background(), bytes.NewReader([]byte("same body")), "text/plain", "one.txt", time.Hour) + first, err := store.CreateWithOriginalName(bytes.NewReader([]byte("same body")), "text/plain", "one.txt", time.Hour) if err != nil { t.Fatalf("first CreateWithOriginalName() error = %v", err) } - second, err := store.CreateWithOriginalName(context.Background(), bytes.NewReader([]byte("same body")), "text/plain", "two.txt", time.Hour) + second, err := store.CreateWithOriginalName(bytes.NewReader([]byte("same body")), "text/plain", "two.txt", time.Hour) if err != nil { t.Fatalf("second CreateWithOriginalName() error = %v", err) } @@ -216,7 +215,7 @@ func TestGetDeleteAndNotFound(t *testing.T) { if err != nil { t.Fatalf("NewFilesystemStore() error = %v", err) } - meta, err := store.Create(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", time.Hour) + meta, err := store.Create(bytes.NewReader([]byte("hello")), "text/plain", time.Hour) if err != nil { t.Fatalf("Create() error = %v", err) } @@ -241,11 +240,11 @@ func TestDeleteExpired(t *testing.T) { if err != nil { t.Fatalf("NewFilesystemStore() error = %v", err) } - expired, err := store.Create(context.Background(), bytes.NewReader([]byte("expired")), "text/plain", -time.Second) + expired, err := store.Create(bytes.NewReader([]byte("expired")), "text/plain", -time.Second) if err != nil { t.Fatalf("Create expired() error = %v", err) } - active, err := store.Create(context.Background(), bytes.NewReader([]byte("active")), "text/plain", time.Hour) + active, err := store.Create(bytes.NewReader([]byte("active")), "text/plain", time.Hour) if err != nil { t.Fatalf("Create active() error = %v", err) } @@ -392,7 +391,7 @@ func TestNewFilesystemStoreWithDefaultTTLShiftsExpiriesOnTTLChange(t *testing.T) t.Fatalf("NewFilesystemStoreWithDefaultTTL() error = %v", err) } - meta, err := store.Create(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", time.Hour) + meta, err := store.Create(bytes.NewReader([]byte("hello")), "text/plain", time.Hour) if err != nil { t.Fatalf("Create() error = %v", err) } @@ -474,7 +473,7 @@ func TestCreateRollbackOnPersistFailure(t *testing.T) { } store.index = badIndexDir - if _, err := store.Create(context.Background(), bytes.NewReader([]byte("x")), "text/plain", time.Hour); err == nil { + if _, err := store.Create(bytes.NewReader([]byte("x")), "text/plain", time.Hour); err == nil { t.Fatalf("Create() error = nil, want persist failure") } if len(store.metadata) != 0 { @@ -489,7 +488,7 @@ func TestCreateReaderFailure(t *testing.T) { if err != nil { t.Fatalf("NewFilesystemStore() error = %v", err) } - if _, err := store.Create(context.Background(), failingReader{}, "text/plain", time.Hour); err == nil { + if _, err := store.Create(failingReader{}, "text/plain", time.Hour); err == nil { t.Fatalf("Create() error = nil, want reader failure") } } @@ -501,7 +500,7 @@ func TestDeletePersistFailureRestoresEntry(t *testing.T) { if err != nil { t.Fatalf("NewFilesystemStore() error = %v", err) } - meta, err := store.Create(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", time.Hour) + meta, err := store.Create(bytes.NewReader([]byte("hello")), "text/plain", time.Hour) if err != nil { t.Fatalf("Create() error = %v", err) } @@ -547,8 +546,8 @@ func TestDeleteRemoveFailure(t *testing.T) { if err := store.persistLocked(); err != nil { t.Fatalf("persistLocked() error = %v", err) } - if err := store.Delete(id); err == nil { - t.Fatalf("Delete() error = nil, want remove failure") + if err := store.Delete(id); err != nil { + t.Fatalf("Delete() error = %v, want nil (blob remove failure is tolerated after metadata removal; orphan on disk is acceptable since no meta references it)", err) } } @@ -559,7 +558,7 @@ func TestDeleteExpiredPersistFailureRestoresEntries(t *testing.T) { if err != nil { t.Fatalf("NewFilesystemStore() error = %v", err) } - meta, err := store.Create(context.Background(), bytes.NewReader([]byte("expired")), "text/plain", -time.Second) + meta, err := store.Create(bytes.NewReader([]byte("expired")), "text/plain", -time.Second) if err != nil { t.Fatalf("Create() error = %v", err) } @@ -642,7 +641,7 @@ func TestNewFilesystemStoreFailsToLoadEncryptedIndexWithWrongKey(t *testing.T) { if err != nil { t.Fatalf("NewFilesystemStore() error = %v", err) } - if _, err := store.Create(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", time.Hour); err != nil { + if _, err := store.Create(bytes.NewReader([]byte("hello")), "text/plain", time.Hour); err != nil { t.Fatalf("Create() error = %v", err) } @@ -672,7 +671,6 @@ func TestConcurrentCreateWithOriginalNameProducesUniqueReadableEntries(t *testin payload := fmt.Sprintf("storage-writer-%03d-%s", i, strings.Repeat("z", i%13)) meta, createErr := store.CreateWithOriginalName( - context.Background(), strings.NewReader(payload), "text/plain; charset=utf-8", fmt.Sprintf("w-%03d.txt", i), diff --git a/web/ui/index.html b/web/ui/index.html index bb11e60..3df2284 100644 --- a/web/ui/index.html +++ b/web/ui/index.html @@ -4,6 +4,14 @@ Scratchbox + + + + + + + +
diff --git a/web/ui/public/favicon.svg b/web/ui/public/favicon.svg new file mode 100644 index 0000000..829ff69 --- /dev/null +++ b/web/ui/public/favicon.svg @@ -0,0 +1,4 @@ + + + S + diff --git a/web/ui/src/App.svelte b/web/ui/src/App.svelte index cfe60cf..d9da6a3 100644 --- a/web/ui/src/App.svelte +++ b/web/ui/src/App.svelte @@ -30,6 +30,14 @@ const maxUploadSizeDisplay = $derived(formatBytes(maxUploadSizeBytes, true)); const defaultTTLDisplay = $derived(formatTTLDisplay(defaultTTL)); + const viewPageTitle = $derived( + viewMeta?.filename + ? `${viewMeta.filename} • Scratchbox` + : scratchID + ? `${scratchID} • Scratchbox` + : "Scratchbox" + ); + onMount(async () => { nowTicker = window.setInterval(() => { nowMs = Date.now(); @@ -127,14 +135,16 @@ return; } const msg = await metaResponse.text(); - throw new Error(msg || `Failed to load scratch metadata (${metaResponse.status}).`); + viewError = msg || `Failed to load scratch metadata (${metaResponse.status}).`; + return; } viewMeta = await metaResponse.json(); const rawResponse = await fetch(`/api/raw/${encodeURIComponent(scratchID)}`); if (!rawResponse.ok) { const msg = await rawResponse.text(); - throw new Error(msg || `Failed to load scratch content (${rawResponse.status}).`); + viewError = msg || `Failed to load scratch content (${rawResponse.status}).`; + return; } const contentType = rawResponse.headers.get("content-type") ?? ""; @@ -412,6 +422,31 @@ } + + {#if routeMode === "view"} + {viewPageTitle} + + + + + + {:else if routeMode === "expired"} + Scratch expired • Scratchbox + + + + + + {:else if routeMode === "notfound"} + Not found • Scratchbox + + + + + + {/if} + +