Enhance environment variable documentation and configuration

Co-authored-by: Cursor <cursoragent@cursor.com>

internal/config/config.go

@@ -49,6 +49,62 @@ const (
 	envPrefix                        = "SCRATCHBOX_"
 )
 
+// envSuffixes is the single source of truth for every SCRATCHBOX_* environment
+// variable suffix supported by applyEnvOverrides (without the "SCRATCHBOX_" prefix).
+//
+// Usage sites that hardcode or document these vars MUST have a comment pointing here:
+// - applyEnvOverrides (this file)
+// - printEnvStartup + flag docs in cmd/scratchbox/server.go
+// - docker-entrypoint.sh
+// - .env.example
+// - README.md (supported list + example block)
+// - tests that setenv them (e.g. config_test.go, main_test.go)
+//
+// When you add, remove, or rename support in applyEnvOverrides (or the corresponding
+// struct fields), you MUST:
+//  1. Update this list.
+//  2. Add/update the pointer comment at every usage site.
+//  3. Update .env.example with the new var (plus a sensible default + comment).
+//  4. Update the "Environment variables supported by..." list in README.md.
+//  5. Update the example .env block in README.md.
+//
+// A test in config_test.go will fail if .env.example or README.md are missing any
+// entry from this list. This prevents the documentation drift that has happened
+// in the past when new config fields (e.g. shutdown_timeout, access_log details)
+// were added.
+var envSuffixes = []string{
+	"SERVER_LISTEN_ADDR",
+	"SERVER_READ_HEADER_TIMEOUT",
+	"SERVER_READ_TIMEOUT",
+	"SERVER_WRITE_TIMEOUT",
+	"SERVER_IDLE_TIMEOUT",
+	"SERVER_SHUTDOWN_TIMEOUT",
+	"SERVER_MAX_HEADER_BYTES",
+	"SERVER_ACCESS_LOG_ENABLED",
+	"SERVER_ACCESS_LOG_FILE_PATH",
+	"SERVER_ACCESS_LOG_MAX_SIZE",
+	"SERVER_ACCESS_LOG_MAX_BACKUPS",
+	"LIMITS_MAX_UPLOAD_SIZE",
+	"LIMITS_DEFAULT_TTL",
+	"LIMITS_RAW_CACHE_MAX_SIZE",
+	"LIMITS_RAW_CACHE_MAX_ENTRIES",
+	"STORAGE_DATA_DIR",
+	"STORAGE_CLEANUP_INTERVAL",
+	"SECURITY_ALLOWED_IPS",
+	"SECURITY_TRUST_PROXY_HEADERS",
+	"SECURITY_TRUSTED_PROXY_IPS",
+	"SECURITY_RATE_LIMIT_UI_ENABLED",
+	"SECURITY_RATE_LIMIT_UI_REQUESTS_PER_MINUTE",
+	"SECURITY_RATE_LIMIT_UI_BURST",
+	"SECURITY_RATE_LIMIT_API_READ_ENABLED",
+	"SECURITY_RATE_LIMIT_API_READ_REQUESTS_PER_MINUTE",
+	"SECURITY_RATE_LIMIT_API_READ_BURST",
+	"SECURITY_RATE_LIMIT_API_WRITE_ENABLED",
+	"SECURITY_RATE_LIMIT_API_WRITE_REQUESTS_PER_MINUTE",
+	"SECURITY_RATE_LIMIT_API_WRITE_BURST",
+	"SECURITY_HSTS_ENABLED",
+}
+
 type Config struct {
 	Server   ServerConfig   `yaml:"server" comment:"HTTP listener settings."`
 	Limits   LimitsConfig   `yaml:"limits" comment:"Upload, expiration, and raw cache defaults."`
@@ -59,8 +115,8 @@ type Config struct {
 type ServerConfig struct {
 	ListenAddr           string          `yaml:"listen_addr" comment:"Address to bind the HTTP server to."`
 	ReadHeaderTimeout    string          `yaml:"read_header_timeout" comment:"Maximum duration for reading request headers (for slowloris protection). Must be > 0."`
-	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."`
+	ReadTimeout          string          `yaml:"read_timeout" comment:"Maximum duration for reading the full request, including body. Use 0 to disable (recommended when limits.max_upload_size is multi-GiB or clients are slow). Must be >= 0."`
+	WriteTimeout         string          `yaml:"write_timeout" comment:"Maximum duration before timing out writes of a response (also bounds slow handlers after headers are read). Use 0 to disable (recommended for large uploads/downloads). 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."`
@@ -214,8 +270,9 @@ func (c *Config) Validate() error {
 	if err != nil {
 		return fmt.Errorf("server.read_timeout invalid: %w", err)
 	}
-	if readTimeout <= 0 {
-		return errors.New("server.read_timeout must be > 0")
+	// 0 means no timeout (matches net/http.Server.ReadTimeout). Negative is invalid.
+	if readTimeout < 0 {
+		return errors.New("server.read_timeout must be >= 0")
 	}
 	c.Server.ReadTimeoutDur = readTimeout
 
@@ -223,8 +280,9 @@ func (c *Config) Validate() error {
 	if err != nil {
 		return fmt.Errorf("server.write_timeout invalid: %w", err)
 	}
-	if writeTimeout <= 0 {
-		return errors.New("server.write_timeout must be > 0")
+	// 0 means no timeout (matches net/http.Server.WriteTimeout). Negative is invalid.
+	if writeTimeout < 0 {
+		return errors.New("server.write_timeout must be >= 0")
 	}
 	c.Server.WriteTimeoutDur = writeTimeout
 
@@ -530,6 +588,9 @@ func decodeEncryptionKey(raw string) ([]byte, error) {
 	return encKey, nil
 }
 
+// applyEnvOverrides applies all supported SCRATCHBOX_* vars.
+// The canonical list of supported suffixes is in envSuffixes (defined earlier in this file).
+// See the comment on envSuffixes for the full maintenance rules (update list + .env.example + README).
 func (c *Config) applyEnvOverrides(prefix string) error {
 	if err := applyStringEnv(prefix+"SERVER_LISTEN_ADDR", &c.Server.ListenAddr); err != nil {
 		return err

internal/config/config_test.go

@@ -79,6 +79,27 @@ func TestValidateAppliesDefaultsAndParsedFields(t *testing.T) {
 	}
 }
 
+func TestValidateAllowsZeroReadAndWriteTimeout(t *testing.T) {
+	t.Parallel()
+
+	cfg := defaults()
+	cfg.Storage.DataDir = t.TempDir()
+	// 0 disables the deadline (net/http semantics). Needed for multi-GiB uploads
+	// where a fixed 30s body/response timeout cannot work.
+	cfg.Server.ReadTimeout = "0s"
+	cfg.Server.WriteTimeout = "0"
+
+	if err := cfg.Validate(); err != nil {
+		t.Fatalf("Validate() returned error: %v", err)
+	}
+	if cfg.Server.ReadTimeoutDur != 0 {
+		t.Fatalf("ReadTimeoutDur = %s, want 0", cfg.Server.ReadTimeoutDur)
+	}
+	if cfg.Server.WriteTimeoutDur != 0 {
+		t.Fatalf("WriteTimeoutDur = %s, want 0", cfg.Server.WriteTimeoutDur)
+	}
+}
+
 func TestLoadParsesConfiguredFields(t *testing.T) {
 	t.Parallel()
 
@@ -249,6 +270,13 @@ func TestValidateRejectsInvalidValues(t *testing.T) {
 			},
 			wantErr: "server.read_timeout",
 		},
+		{
+			name: "negative read timeout",
+			mutate: func(cfg *Config) {
+				cfg.Server.ReadTimeout = "-1s"
+			},
+			wantErr: "server.read_timeout",
+		},
 		{
 			name: "invalid write timeout",
 			mutate: func(cfg *Config) {
@@ -256,6 +284,13 @@ func TestValidateRejectsInvalidValues(t *testing.T) {
 			},
 			wantErr: "server.write_timeout",
 		},
+		{
+			name: "negative write timeout",
+			mutate: func(cfg *Config) {
+				cfg.Server.WriteTimeout = "-1s"
+			},
+			wantErr: "server.write_timeout",
+		},
 		{
 			name: "invalid idle timeout",
 			mutate: func(cfg *Config) {
@@ -713,3 +748,46 @@ func TestGenerateEncryptionKey(t *testing.T) {
 		t.Fatalf("decoded key length = %d, want 32", len(b1))
 	}
 }
+
+// TestEnvDocumentationIsUpToDate ensures .env.example and the env var
+// documentation + example in README.md stay in sync with the env vars
+// actually supported by the code (envSuffixes in config.go).
+// See the comment on envSuffixes for the list of places that must point back to it.
+//
+// This is the guard against the recurring "example env and readme have
+// become out of sync" problem. When you add a new field that is loaded via
+// applyEnvOverrides, you must:
+//   - add its suffix to envSuffixes
+//   - add a line (with example value + comment) to .env.example
+//   - add it to the list and the ```dotenv example in README.md
+//
+// The test will fail loudly until the docs are updated.
+func TestEnvDocumentationIsUpToDate(t *testing.T) {
+	t.Parallel()
+
+	// Check .env.example (at repo root). Note: go test for this package
+	// runs with cwd set to the package directory (internal/config), so we
+	// need to go up two levels.
+	envExample, err := os.ReadFile("../../.env.example")
+	if err != nil {
+		t.Fatalf("failed to read .env.example: %v", err)
+	}
+	for _, suffix := range envSuffixes {
+		name := "SCRATCHBOX_" + suffix
+		if !strings.Contains(string(envExample), name) {
+			t.Errorf(".env.example is missing %s (add it with a default/example value and a comment matching the style of the others)", name)
+		}
+	}
+
+	// Check README.md env documentation (the supported list and the example block)
+	readme, err := os.ReadFile("../../README.md")
+	if err != nil {
+		t.Fatalf("failed to read README.md: %v", err)
+	}
+	for _, suffix := range envSuffixes {
+		name := "SCRATCHBOX_" + suffix
+		if !strings.Contains(string(readme), name) {
+			t.Errorf("README.md env docs (supported list or example .env block) are missing %s; update both the bullet list and the ```dotenv block", name)
+		}
+	}
+}