From 57076820189b30c3c04aca5b1aaec9b63caf0fc2 Mon Sep 17 00:00:00 2001
From: dev_agent <dev_agent@cubecraftlabs.com>
Date: Fri, 2 Oct 2026 22:02:44 -0400
Subject: [PATCH 1/3] Add Go module, fail-fast config loader and /health
 endpoint

- go.mod: module code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest, go 1.25,
  sole dependency github.com/go-chi/chi/v5.
- internal/config: Load() reads MEALQUEST_* env vars and *_FILE secrets,
  collects every missing/invalid value into one error naming each variable,
  rejects inline secrets, and redacts secrets when formatted.
- internal/server: chi router serving GET /health -> {"status":"ok","version"}.
- cmd/server: wiring only; slog JSON logging, version via -ldflags
  "-X main.version=...", graceful shutdown on SIGINT/SIGTERM.
- .gitignore: cover .env*, deploy/secrets/, node_modules, dist, *.db.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 .gitignore                     |  11 ++
 cmd/server/main.go             |  74 +++++++++++
 go.mod                         |   5 +
 go.sum                         |   2 +
 internal/config/config.go      | 200 ++++++++++++++++++++++++++++++
 internal/config/config_test.go | 217 +++++++++++++++++++++++++++++++++
 internal/server/server.go      |  37 ++++++
 internal/server/server_test.go |  51 ++++++++
 8 files changed, 597 insertions(+)
 create mode 100644 cmd/server/main.go
 create mode 100644 go.mod
 create mode 100644 go.sum
 create mode 100644 internal/config/config.go
 create mode 100644 internal/config/config_test.go
 create mode 100644 internal/server/server.go
 create mode 100644 internal/server/server_test.go

diff --git a/.gitignore b/.gitignore
index ee38b4e..7523873 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,2 +1,13 @@
 .DS_Store
 design/.DS_Store
+
+# Secrets and local environment
+.env*
+deploy/secrets/
+
+# Build artifacts and dependencies
+node_modules
+dist
+
+# Local databases
+*.db
diff --git a/cmd/server/main.go b/cmd/server/main.go
new file mode 100644
index 0000000..b9e99bd
--- /dev/null
+++ b/cmd/server/main.go
@@ -0,0 +1,74 @@
+// Command server runs the MealQuest HTTP service.
+package main
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"log/slog"
+	"net/http"
+	"os"
+	"os/signal"
+	"syscall"
+	"time"
+
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/config"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/server"
+)
+
+// version is set at build time with -ldflags "-X main.version=<version>".
+var version = "dev"
+
+func main() {
+	cfg, err := config.Load()
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+
+	logger := slog.New(slog.NewJSONHandler(os.Stdout, nil))
+	if err := run(cfg, logger); err != nil {
+		logger.Error("server stopped", "error", err)
+		os.Exit(1)
+	}
+}
+
+func run(cfg config.Config, logger *slog.Logger) error {
+	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
+	defer stop()
+
+	srv := &http.Server{
+		Addr:              cfg.HTTPAddr,
+		Handler:           server.NewRouter(logger, version),
+		ReadHeaderTimeout: 10 * time.Second,
+		ErrorLog:          slog.NewLogLogger(logger.Handler(), slog.LevelError),
+	}
+
+	errc := make(chan error, 1)
+	go func() {
+		logger.Info("listening",
+			"addr", cfg.HTTPAddr,
+			"public_url", cfg.PublicURL.String(),
+			"data_dir", cfg.DataDir,
+			"version", version,
+		)
+		errc <- srv.ListenAndServe()
+	}()
+
+	select {
+	case err := <-errc:
+		return err
+	case <-ctx.Done():
+	}
+
+	logger.Info("shutting down", "timeout", cfg.ShutdownTimeout.String())
+	shutdownCtx, cancel := context.WithTimeout(context.Background(), cfg.ShutdownTimeout)
+	defer cancel()
+	if err := srv.Shutdown(shutdownCtx); err != nil {
+		return fmt.Errorf("shutdown: %w", err)
+	}
+	if err := <-errc; !errors.Is(err, http.ErrServerClosed) {
+		return err
+	}
+	return nil
+}
diff --git a/go.mod b/go.mod
new file mode 100644
index 0000000..37e681e
--- /dev/null
+++ b/go.mod
@@ -0,0 +1,5 @@
+module code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest
+
+go 1.25
+
+require github.com/go-chi/chi/v5 v5.3.1
diff --git a/go.sum b/go.sum
new file mode 100644
index 0000000..bdb0eaa
--- /dev/null
+++ b/go.sum
@@ -0,0 +1,2 @@
+github.com/go-chi/chi/v5 v5.3.1 h1:3j4HZLGZQ3JpMCrPJF/Jl3mYJfWLKBfNJ6quurUGCf8=
+github.com/go-chi/chi/v5 v5.3.1/go.mod h1:R+tYY2hNuVUUjxoPtqUdgBqevM9s9njzkTLutVsOCto=
diff --git a/internal/config/config.go b/internal/config/config.go
new file mode 100644
index 0000000..dd45f72
--- /dev/null
+++ b/internal/config/config.go
@@ -0,0 +1,200 @@
+// Package config loads MealQuest configuration from MEALQUEST_* environment
+// variables. Secrets are read only from files named by *_FILE variables.
+package config
+
+import (
+	"errors"
+	"fmt"
+	"net"
+	"net/url"
+	"os"
+	"strings"
+	"time"
+)
+
+// Environment variable names.
+const (
+	EnvHTTPAddr          = "MEALQUEST_HTTP_ADDR"
+	EnvPublicURL         = "MEALQUEST_PUBLIC_URL"
+	EnvDataDir           = "MEALQUEST_DATA_DIR"
+	EnvShutdownTimeout   = "MEALQUEST_SHUTDOWN_TIMEOUT"
+	EnvSessionSecretFile = "MEALQUEST_SESSION_SECRET_FILE"
+)
+
+// Config is the validated service configuration.
+type Config struct {
+	HTTPAddr        string
+	PublicURL       *url.URL
+	DataDir         string
+	ShutdownTimeout time.Duration
+	SessionSecret   Secret
+}
+
+// Load reads and validates configuration from the process environment.
+// It reports every missing or invalid variable in a single *Error.
+func Load() (Config, error) {
+	return load(os.LookupEnv)
+}
+
+func load(lookup func(string) (string, bool)) (Config, error) {
+	r := reader{lookup: lookup}
+	cfg := Config{
+		HTTPAddr:        r.listenAddr(EnvHTTPAddr),
+		PublicURL:       r.httpURL(EnvPublicURL),
+		DataDir:         r.dir(EnvDataDir),
+		ShutdownTimeout: r.duration(EnvShutdownTimeout),
+		SessionSecret:   r.secretFile(EnvSessionSecretFile),
+	}
+	if len(r.problems) > 0 {
+		return Config{}, &Error{Problems: r.problems}
+	}
+	return cfg, nil
+}
+
+// Problem describes one missing or invalid variable.
+type Problem struct {
+	Var    string
+	Reason string
+}
+
+// Error lists every configuration problem found by Load.
+type Error struct {
+	Problems []Problem
+}
+
+func (e *Error) Error() string {
+	var b strings.Builder
+	b.WriteString("invalid configuration:")
+	for _, p := range e.Problems {
+		fmt.Fprintf(&b, "\n  %s: %s", p.Var, p.Reason)
+	}
+	return b.String()
+}
+
+// Secret holds a sensitive value. It never prints its contents.
+type Secret struct {
+	value string
+}
+
+// Reveal returns the secret value. Call it only where the value is used.
+func (s Secret) Reveal() string { return s.value }
+
+// String redacts the secret so it cannot leak through fmt or logs.
+func (s Secret) String() string { return "[REDACTED]" }
+
+// GoString redacts the secret for %#v.
+func (s Secret) GoString() string { return "[REDACTED]" }
+
+// reader reads variables and collects every problem instead of stopping at
+// the first one.
+type reader struct {
+	lookup   func(string) (string, bool)
+	problems []Problem
+}
+
+func (r *reader) fail(name, format string, args ...any) {
+	r.problems = append(r.problems, Problem{Var: name, Reason: fmt.Sprintf(format, args...)})
+}
+
+// required returns the variable's value, recording a problem when it is
+// unset or empty.
+func (r *reader) required(name string) (string, bool) {
+	v, ok := r.lookup(name)
+	if !ok || strings.TrimSpace(v) == "" {
+		r.fail(name, "required but not set")
+		return "", false
+	}
+	return v, true
+}
+
+func (r *reader) listenAddr(name string) string {
+	v, ok := r.required(name)
+	if !ok {
+		return ""
+	}
+	if _, _, err := net.SplitHostPort(v); err != nil {
+		r.fail(name, "invalid listen address %q (want host:port or :port)", v)
+		return ""
+	}
+	return v
+}
+
+func (r *reader) httpURL(name string) *url.URL {
+	v, ok := r.required(name)
+	if !ok {
+		return nil
+	}
+	u, err := url.Parse(v)
+	if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" {
+		r.fail(name, "invalid URL %q (want absolute http:// or https:// URL)", v)
+		return nil
+	}
+	return u
+}
+
+func (r *reader) dir(name string) string {
+	v, ok := r.required(name)
+	if !ok {
+		return ""
+	}
+	info, err := os.Stat(v)
+	if err != nil {
+		r.fail(name, "cannot access directory %q: %v", v, err)
+		return ""
+	}
+	if !info.IsDir() {
+		r.fail(name, "%q is not a directory", v)
+		return ""
+	}
+	return v
+}
+
+func (r *reader) duration(name string) time.Duration {
+	v, ok := r.required(name)
+	if !ok {
+		return 0
+	}
+	d, err := time.ParseDuration(v)
+	if err != nil {
+		r.fail(name, "invalid duration %q (want e.g. 10s, 1m)", v)
+		return 0
+	}
+	if d <= 0 {
+		r.fail(name, "duration must be positive, got %q", v)
+		return 0
+	}
+	return d
+}
+
+// secretFile reads a secret from the file named by name, which must end in
+// _FILE. Setting the secret inline (name without _FILE) is rejected.
+func (r *reader) secretFile(name string) Secret {
+	inline := strings.TrimSuffix(name, "_FILE")
+	if _, set := r.lookup(inline); set {
+		r.fail(inline, "secrets must not be set inline; use %s", name)
+	}
+	path, ok := r.required(name)
+	if !ok {
+		return Secret{}
+	}
+	data, err := os.ReadFile(path)
+	if err != nil {
+		// Report the cause without echoing any file contents.
+		r.fail(name, "cannot read secret file %q: %v", path, unwrapPathError(err))
+		return Secret{}
+	}
+	value := strings.TrimRight(string(data), "\r\n")
+	if strings.TrimSpace(value) == "" {
+		r.fail(name, "secret file %q is empty", path)
+		return Secret{}
+	}
+	return Secret{value: value}
+}
+
+func unwrapPathError(err error) error {
+	var pe *os.PathError
+	if errors.As(err, &pe) {
+		return pe.Err
+	}
+	return err
+}
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
new file mode 100644
index 0000000..b617872
--- /dev/null
+++ b/internal/config/config_test.go
@@ -0,0 +1,217 @@
+package config
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"slices"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestLoad(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	secretPath := filepath.Join(dir, "session_secret")
+	if err := os.WriteFile(secretPath, []byte("s3cret-value\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	unreadablePath := filepath.Join(dir, "missing_secret")
+	emptyPath := filepath.Join(dir, "empty_secret")
+	if err := os.WriteFile(emptyPath, []byte("\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	valid := func() map[string]string {
+		return map[string]string{
+			EnvHTTPAddr:          ":8080",
+			EnvPublicURL:         "https://meals.example.com",
+			EnvDataDir:           dir,
+			EnvShutdownTimeout:   "15s",
+			EnvSessionSecretFile: secretPath,
+		}
+	}
+	with := func(changes map[string]string, unset ...string) map[string]string {
+		env := valid()
+		for k, v := range changes {
+			env[k] = v
+		}
+		for _, k := range unset {
+			delete(env, k)
+		}
+		return env
+	}
+
+	tests := []struct {
+		name    string
+		env     map[string]string
+		badVars []string // variables the error must name; nil means success
+	}{
+		{
+			name: "all required vars present",
+			env:  valid(),
+		},
+		{
+			name:    "one missing",
+			env:     with(nil, EnvHTTPAddr),
+			badVars: []string{EnvHTTPAddr},
+		},
+		{
+			name:    "empty value counts as missing",
+			env:     with(map[string]string{EnvDataDir: ""}),
+			badVars: []string{EnvDataDir},
+		},
+		{
+			name: "all missing are collected",
+			env:  map[string]string{},
+			badVars: []string{
+				EnvHTTPAddr, EnvPublicURL, EnvDataDir, EnvShutdownTimeout, EnvSessionSecretFile,
+			},
+		},
+		{
+			name:    "invalid duration",
+			env:     with(map[string]string{EnvShutdownTimeout: "ten seconds"}),
+			badVars: []string{EnvShutdownTimeout},
+		},
+		{
+			name:    "non-positive duration",
+			env:     with(map[string]string{EnvShutdownTimeout: "0s"}),
+			badVars: []string{EnvShutdownTimeout},
+		},
+		{
+			name:    "invalid listen address",
+			env:     with(map[string]string{EnvHTTPAddr: "8080"}),
+			badVars: []string{EnvHTTPAddr},
+		},
+		{
+			name:    "relative public URL",
+			env:     with(map[string]string{EnvPublicURL: "meals.example.com"}),
+			badVars: []string{EnvPublicURL},
+		},
+		{
+			name:    "data dir does not exist",
+			env:     with(map[string]string{EnvDataDir: filepath.Join(dir, "nope")}),
+			badVars: []string{EnvDataDir},
+		},
+		{
+			name:    "unreadable secret file",
+			env:     with(map[string]string{EnvSessionSecretFile: unreadablePath}),
+			badVars: []string{EnvSessionSecretFile},
+		},
+		{
+			name:    "secret file path is a directory",
+			env:     with(map[string]string{EnvSessionSecretFile: dir}),
+			badVars: []string{EnvSessionSecretFile},
+		},
+		{
+			name:    "empty secret file",
+			env:     with(map[string]string{EnvSessionSecretFile: emptyPath}),
+			badVars: []string{EnvSessionSecretFile},
+		},
+		{
+			name:    "inline secret rejected",
+			env:     with(map[string]string{"MEALQUEST_SESSION_SECRET": "inline"}),
+			badVars: []string{"MEALQUEST_SESSION_SECRET"},
+		},
+		{
+			name: "several invalid values are all named",
+			env: with(map[string]string{
+				EnvShutdownTimeout:   "soon",
+				EnvSessionSecretFile: unreadablePath,
+			}, EnvPublicURL),
+			badVars: []string{EnvPublicURL, EnvShutdownTimeout, EnvSessionSecretFile},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			lookup := func(k string) (string, bool) {
+				v, ok := tc.env[k]
+				return v, ok
+			}
+			cfg, err := load(lookup)
+
+			if tc.badVars == nil {
+				if err != nil {
+					t.Fatalf("load() error = %v, want nil", err)
+				}
+				return
+			}
+
+			var cerr *Error
+			if !errors.As(err, &cerr) {
+				t.Fatalf("load() error = %v, want *Error", err)
+			}
+			var got []string
+			for _, p := range cerr.Problems {
+				got = append(got, p.Var)
+			}
+			if !slices.Equal(got, tc.badVars) {
+				t.Errorf("problem vars = %v, want %v", got, tc.badVars)
+			}
+			for _, v := range tc.badVars {
+				if !strings.Contains(err.Error(), v) {
+					t.Errorf("error message %q does not name %s", err.Error(), v)
+				}
+			}
+			if cfg != (Config{}) {
+				t.Errorf("load() returned non-zero Config alongside error: %+v", cfg)
+			}
+		})
+	}
+}
+
+func TestLoadValues(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	secretPath := filepath.Join(dir, "session_secret")
+	if err := os.WriteFile(secretPath, []byte("s3cret-value\r\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	env := map[string]string{
+		EnvHTTPAddr:          "127.0.0.1:9000",
+		EnvPublicURL:         "http://localhost:9000",
+		EnvDataDir:           dir,
+		EnvShutdownTimeout:   "1m30s",
+		EnvSessionSecretFile: secretPath,
+	}
+	cfg, err := load(func(k string) (string, bool) { v, ok := env[k]; return v, ok })
+	if err != nil {
+		t.Fatalf("load() error = %v", err)
+	}
+
+	if cfg.HTTPAddr != "127.0.0.1:9000" {
+		t.Errorf("HTTPAddr = %q", cfg.HTTPAddr)
+	}
+	if cfg.PublicURL.String() != "http://localhost:9000" {
+		t.Errorf("PublicURL = %q", cfg.PublicURL)
+	}
+	if cfg.DataDir != dir {
+		t.Errorf("DataDir = %q, want %q", cfg.DataDir, dir)
+	}
+	if cfg.ShutdownTimeout != 90*time.Second {
+		t.Errorf("ShutdownTimeout = %v, want 1m30s", cfg.ShutdownTimeout)
+	}
+	if cfg.SessionSecret.Reveal() != "s3cret-value" {
+		t.Errorf("SessionSecret.Reveal() = %q, want trailing newline trimmed", cfg.SessionSecret.Reveal())
+	}
+}
+
+func TestSecretRedacted(t *testing.T) {
+	t.Parallel()
+
+	s := Secret{value: "s3cret-value"}
+	for _, format := range []string{"%v", "%s", "%+v", "%#v"} {
+		cfg := Config{SessionSecret: s}
+		for _, out := range []string{fmt.Sprintf(format, s), fmt.Sprintf(format, cfg)} {
+			if strings.Contains(out, "s3cret-value") {
+				t.Errorf("format %s leaked secret: %s", format, out)
+			}
+		}
+	}
+}
diff --git a/internal/server/server.go b/internal/server/server.go
new file mode 100644
index 0000000..fa4f203
--- /dev/null
+++ b/internal/server/server.go
@@ -0,0 +1,37 @@
+// Package server builds the MealQuest HTTP router.
+package server
+
+import (
+	"encoding/json"
+	"log/slog"
+	"net/http"
+
+	"github.com/go-chi/chi/v5"
+)
+
+// NewRouter returns the HTTP handler for the service. version is reported by
+// GET /health.
+func NewRouter(logger *slog.Logger, version string) http.Handler {
+	r := chi.NewRouter()
+	r.Get("/health", health(logger, version))
+	return r
+}
+
+type healthResponse struct {
+	Status  string `json:"status"`
+	Version string `json:"version"`
+}
+
+func health(logger *slog.Logger, version string) http.HandlerFunc {
+	body, err := json.Marshal(healthResponse{Status: "ok", Version: version})
+	if err != nil {
+		panic(err) // two string fields cannot fail to marshal
+	}
+	return func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		if _, err := w.Write(body); err != nil {
+			logger.WarnContext(r.Context(), "write health response", "error", err)
+		}
+	}
+}
diff --git a/internal/server/server_test.go b/internal/server/server_test.go
new file mode 100644
index 0000000..0e7e9f9
--- /dev/null
+++ b/internal/server/server_test.go
@@ -0,0 +1,51 @@
+package server
+
+import (
+	"encoding/json"
+	"io"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestHealth(t *testing.T) {
+	t.Parallel()
+
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+	srv := httptest.NewServer(NewRouter(logger, "1.2.3"))
+	t.Cleanup(srv.Close)
+
+	resp, err := http.Get(srv.URL + "/health")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		t.Errorf("status = %d, want 200", resp.StatusCode)
+	}
+	if ct := resp.Header.Get("Content-Type"); ct != "application/json" {
+		t.Errorf("Content-Type = %q, want application/json", ct)
+	}
+	var got map[string]string
+	if err := json.NewDecoder(resp.Body).Decode(&got); err != nil {
+		t.Fatal(err)
+	}
+	want := map[string]string{"status": "ok", "version": "1.2.3"}
+	if len(got) != len(want) || got["status"] != want["status"] || got["version"] != want["version"] {
+		t.Errorf("body = %v, want %v", got, want)
+	}
+}
+
+func TestHealthRejectsOtherMethods(t *testing.T) {
+	t.Parallel()
+
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+	rec := httptest.NewRecorder()
+	NewRouter(logger, "dev").ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/health", nil))
+
+	if rec.Code != http.StatusMethodNotAllowed {
+		t.Errorf("status = %d, want 405", rec.Code)
+	}
+}
-- 
2.55.0


From 60555bf8ec087171622b9358ce3664b5ce346bfa Mon Sep 17 00:00:00 2001
From: dev_agent <dev_agent@cubecraftlabs.com>
Date: Fri, 2 Oct 2026 22:10:25 -0400
Subject: [PATCH 2/3] Address review findings F1-F4 for config and /health

- F1: read the secret from MEALQUEST_SECRET_KEY_FILE, the documented name.
- F2: reject public URLs carrying userinfo, never echo the raw URL in
  config errors, and log only the public origin (scheme + host).
- F3: validate the MEALQUEST_HTTP_ADDR port (numeric, 0-65535) during Load.
- F4: return 405 with the API error JSON shape for unsupported methods.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 cmd/server/main.go             |  18 ++++--
 cmd/server/main_test.go        |  34 +++++++++++
 internal/config/config.go      |  35 ++++++++---
 internal/config/config_test.go | 103 ++++++++++++++++++++++++---------
 internal/server/server.go      |  28 +++++++++
 internal/server/server_test.go |  18 ++++++
 6 files changed, 194 insertions(+), 42 deletions(-)
 create mode 100644 cmd/server/main_test.go

diff --git a/cmd/server/main.go b/cmd/server/main.go
index b9e99bd..e0d649b 100644
--- a/cmd/server/main.go
+++ b/cmd/server/main.go
@@ -46,12 +46,7 @@ func run(cfg config.Config, logger *slog.Logger) error {
 
 	errc := make(chan error, 1)
 	go func() {
-		logger.Info("listening",
-			"addr", cfg.HTTPAddr,
-			"public_url", cfg.PublicURL.String(),
-			"data_dir", cfg.DataDir,
-			"version", version,
-		)
+		logListening(logger, cfg)
 		errc <- srv.ListenAndServe()
 	}()
 
@@ -72,3 +67,14 @@ func run(cfg config.Config, logger *slog.Logger) error {
 	}
 	return nil
 }
+
+// logListening logs the startup line. Only the public URL's origin is logged
+// so no credential-bearing URL component can reach the log.
+func logListening(logger *slog.Logger, cfg config.Config) {
+	logger.Info("listening",
+		"addr", cfg.HTTPAddr,
+		"public_origin", cfg.PublicOrigin(),
+		"data_dir", cfg.DataDir,
+		"version", version,
+	)
+}
diff --git a/cmd/server/main_test.go b/cmd/server/main_test.go
new file mode 100644
index 0000000..02748e8
--- /dev/null
+++ b/cmd/server/main_test.go
@@ -0,0 +1,34 @@
+package main
+
+import (
+	"bytes"
+	"log/slog"
+	"net/url"
+	"strings"
+	"testing"
+
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/config"
+)
+
+func TestLogListeningOmitsURLCredentials(t *testing.T) {
+	t.Parallel()
+
+	// Load rejects such URLs; this proves the log stays safe even if one
+	// reached the Config some other way.
+	u, err := url.Parse("https://alice:hunter2@meals.example.com/app?token=t0ken")
+	if err != nil {
+		t.Fatal(err)
+	}
+	var buf bytes.Buffer
+	logListening(slog.New(slog.NewJSONHandler(&buf, nil)), config.Config{HTTPAddr: ":8080", PublicURL: u})
+
+	out := buf.String()
+	for _, leak := range []string{"alice", "hunter2", "t0ken"} {
+		if strings.Contains(out, leak) {
+			t.Errorf("startup log leaks %q: %s", leak, out)
+		}
+	}
+	if !strings.Contains(out, `"public_origin":"https://meals.example.com"`) {
+		t.Errorf("startup log missing public origin: %s", out)
+	}
+}
diff --git a/internal/config/config.go b/internal/config/config.go
index dd45f72..1e84b99 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -8,17 +8,18 @@ import (
 	"net"
 	"net/url"
 	"os"
+	"strconv"
 	"strings"
 	"time"
 )
 
 // Environment variable names.
 const (
-	EnvHTTPAddr          = "MEALQUEST_HTTP_ADDR"
-	EnvPublicURL         = "MEALQUEST_PUBLIC_URL"
-	EnvDataDir           = "MEALQUEST_DATA_DIR"
-	EnvShutdownTimeout   = "MEALQUEST_SHUTDOWN_TIMEOUT"
-	EnvSessionSecretFile = "MEALQUEST_SESSION_SECRET_FILE"
+	EnvHTTPAddr        = "MEALQUEST_HTTP_ADDR"
+	EnvPublicURL       = "MEALQUEST_PUBLIC_URL"
+	EnvDataDir         = "MEALQUEST_DATA_DIR"
+	EnvShutdownTimeout = "MEALQUEST_SHUTDOWN_TIMEOUT"
+	EnvSecretKeyFile   = "MEALQUEST_SECRET_KEY_FILE"
 )
 
 // Config is the validated service configuration.
@@ -27,7 +28,13 @@ type Config struct {
 	PublicURL       *url.URL
 	DataDir         string
 	ShutdownTimeout time.Duration
-	SessionSecret   Secret
+	SecretKey       Secret
+}
+
+// PublicOrigin returns the scheme and host of PublicURL, the only parts of it
+// that are safe to log.
+func (c Config) PublicOrigin() string {
+	return c.PublicURL.Scheme + "://" + c.PublicURL.Host
 }
 
 // Load reads and validates configuration from the process environment.
@@ -43,7 +50,7 @@ func load(lookup func(string) (string, bool)) (Config, error) {
 		PublicURL:       r.httpURL(EnvPublicURL),
 		DataDir:         r.dir(EnvDataDir),
 		ShutdownTimeout: r.duration(EnvShutdownTimeout),
-		SessionSecret:   r.secretFile(EnvSessionSecretFile),
+		SecretKey:       r.secretFile(EnvSecretKeyFile),
 	}
 	if len(r.problems) > 0 {
 		return Config{}, &Error{Problems: r.problems}
@@ -112,10 +119,15 @@ func (r *reader) listenAddr(name string) string {
 	if !ok {
 		return ""
 	}
-	if _, _, err := net.SplitHostPort(v); err != nil {
+	_, port, err := net.SplitHostPort(v)
+	if err != nil {
 		r.fail(name, "invalid listen address %q (want host:port or :port)", v)
 		return ""
 	}
+	if _, err := strconv.ParseUint(port, 10, 16); err != nil {
+		r.fail(name, "invalid port %q in listen address (want 0-65535)", port)
+		return ""
+	}
 	return v
 }
 
@@ -124,9 +136,14 @@ func (r *reader) httpURL(name string) *url.URL {
 	if !ok {
 		return nil
 	}
+	// The value is never echoed: it may carry credentials.
 	u, err := url.Parse(v)
 	if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" {
-		r.fail(name, "invalid URL %q (want absolute http:// or https:// URL)", v)
+		r.fail(name, "invalid URL (want absolute http:// or https:// URL)")
+		return nil
+	}
+	if u.User != nil {
+		r.fail(name, "URL must not contain user credentials")
 		return nil
 	}
 	return u
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index b617872..ebaafa2 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -3,6 +3,7 @@ package config
 import (
 	"errors"
 	"fmt"
+	"net/url"
 	"os"
 	"path/filepath"
 	"slices"
@@ -15,7 +16,7 @@ func TestLoad(t *testing.T) {
 	t.Parallel()
 
 	dir := t.TempDir()
-	secretPath := filepath.Join(dir, "session_secret")
+	secretPath := filepath.Join(dir, "secret_key")
 	if err := os.WriteFile(secretPath, []byte("s3cret-value\n"), 0o600); err != nil {
 		t.Fatal(err)
 	}
@@ -27,11 +28,11 @@ func TestLoad(t *testing.T) {
 
 	valid := func() map[string]string {
 		return map[string]string{
-			EnvHTTPAddr:          ":8080",
-			EnvPublicURL:         "https://meals.example.com",
-			EnvDataDir:           dir,
-			EnvShutdownTimeout:   "15s",
-			EnvSessionSecretFile: secretPath,
+			EnvHTTPAddr:        ":8080",
+			EnvPublicURL:       "https://meals.example.com",
+			EnvDataDir:         dir,
+			EnvShutdownTimeout: "15s",
+			EnvSecretKeyFile:   secretPath,
 		}
 	}
 	with := func(changes map[string]string, unset ...string) map[string]string {
@@ -68,7 +69,7 @@ func TestLoad(t *testing.T) {
 			name: "all missing are collected",
 			env:  map[string]string{},
 			badVars: []string{
-				EnvHTTPAddr, EnvPublicURL, EnvDataDir, EnvShutdownTimeout, EnvSessionSecretFile,
+				EnvHTTPAddr, EnvPublicURL, EnvDataDir, EnvShutdownTimeout, EnvSecretKeyFile,
 			},
 		},
 		{
@@ -86,6 +87,21 @@ func TestLoad(t *testing.T) {
 			env:     with(map[string]string{EnvHTTPAddr: "8080"}),
 			badVars: []string{EnvHTTPAddr},
 		},
+		{
+			name:    "non-numeric port",
+			env:     with(map[string]string{EnvHTTPAddr: ":notaport"}),
+			badVars: []string{EnvHTTPAddr},
+		},
+		{
+			name:    "empty port",
+			env:     with(map[string]string{EnvHTTPAddr: "127.0.0.1:"}),
+			badVars: []string{EnvHTTPAddr},
+		},
+		{
+			name:    "port out of range",
+			env:     with(map[string]string{EnvHTTPAddr: ":65536"}),
+			badVars: []string{EnvHTTPAddr},
+		},
 		{
 			name:    "relative public URL",
 			env:     with(map[string]string{EnvPublicURL: "meals.example.com"}),
@@ -98,31 +114,31 @@ func TestLoad(t *testing.T) {
 		},
 		{
 			name:    "unreadable secret file",
-			env:     with(map[string]string{EnvSessionSecretFile: unreadablePath}),
-			badVars: []string{EnvSessionSecretFile},
+			env:     with(map[string]string{EnvSecretKeyFile: unreadablePath}),
+			badVars: []string{EnvSecretKeyFile},
 		},
 		{
 			name:    "secret file path is a directory",
-			env:     with(map[string]string{EnvSessionSecretFile: dir}),
-			badVars: []string{EnvSessionSecretFile},
+			env:     with(map[string]string{EnvSecretKeyFile: dir}),
+			badVars: []string{EnvSecretKeyFile},
 		},
 		{
 			name:    "empty secret file",
-			env:     with(map[string]string{EnvSessionSecretFile: emptyPath}),
-			badVars: []string{EnvSessionSecretFile},
+			env:     with(map[string]string{EnvSecretKeyFile: emptyPath}),
+			badVars: []string{EnvSecretKeyFile},
 		},
 		{
 			name:    "inline secret rejected",
-			env:     with(map[string]string{"MEALQUEST_SESSION_SECRET": "inline"}),
-			badVars: []string{"MEALQUEST_SESSION_SECRET"},
+			env:     with(map[string]string{"MEALQUEST_SECRET_KEY": "inline"}),
+			badVars: []string{"MEALQUEST_SECRET_KEY"},
 		},
 		{
 			name: "several invalid values are all named",
 			env: with(map[string]string{
-				EnvShutdownTimeout:   "soon",
-				EnvSessionSecretFile: unreadablePath,
+				EnvShutdownTimeout: "soon",
+				EnvSecretKeyFile:   unreadablePath,
 			}, EnvPublicURL),
-			badVars: []string{EnvPublicURL, EnvShutdownTimeout, EnvSessionSecretFile},
+			badVars: []string{EnvPublicURL, EnvShutdownTimeout, EnvSecretKeyFile},
 		},
 	}
 
@@ -169,16 +185,16 @@ func TestLoadValues(t *testing.T) {
 	t.Parallel()
 
 	dir := t.TempDir()
-	secretPath := filepath.Join(dir, "session_secret")
+	secretPath := filepath.Join(dir, "secret_key")
 	if err := os.WriteFile(secretPath, []byte("s3cret-value\r\n"), 0o600); err != nil {
 		t.Fatal(err)
 	}
 	env := map[string]string{
-		EnvHTTPAddr:          "127.0.0.1:9000",
-		EnvPublicURL:         "http://localhost:9000",
-		EnvDataDir:           dir,
-		EnvShutdownTimeout:   "1m30s",
-		EnvSessionSecretFile: secretPath,
+		EnvHTTPAddr:        "127.0.0.1:9000",
+		EnvPublicURL:       "http://localhost:9000",
+		EnvDataDir:         dir,
+		EnvShutdownTimeout: "1m30s",
+		EnvSecretKeyFile:   secretPath,
 	}
 	cfg, err := load(func(k string) (string, bool) { v, ok := env[k]; return v, ok })
 	if err != nil {
@@ -197,8 +213,8 @@ func TestLoadValues(t *testing.T) {
 	if cfg.ShutdownTimeout != 90*time.Second {
 		t.Errorf("ShutdownTimeout = %v, want 1m30s", cfg.ShutdownTimeout)
 	}
-	if cfg.SessionSecret.Reveal() != "s3cret-value" {
-		t.Errorf("SessionSecret.Reveal() = %q, want trailing newline trimmed", cfg.SessionSecret.Reveal())
+	if cfg.SecretKey.Reveal() != "s3cret-value" {
+		t.Errorf("SecretKey.Reveal() = %q, want trailing newline trimmed", cfg.SecretKey.Reveal())
 	}
 }
 
@@ -207,7 +223,7 @@ func TestSecretRedacted(t *testing.T) {
 
 	s := Secret{value: "s3cret-value"}
 	for _, format := range []string{"%v", "%s", "%+v", "%#v"} {
-		cfg := Config{SessionSecret: s}
+		cfg := Config{SecretKey: s}
 		for _, out := range []string{fmt.Sprintf(format, s), fmt.Sprintf(format, cfg)} {
 			if strings.Contains(out, "s3cret-value") {
 				t.Errorf("format %s leaked secret: %s", format, out)
@@ -215,3 +231,36 @@ func TestSecretRedacted(t *testing.T) {
 		}
 	}
 }
+
+func TestPublicURLWithCredentialsRejected(t *testing.T) {
+	t.Parallel()
+
+	for _, raw := range []string{
+		"https://alice:hunter2@example.com",
+		"https://alice@example.com",
+		"https://alice:hunter2@[bad",
+	} {
+		env := map[string]string{EnvPublicURL: raw}
+		_, err := load(func(k string) (string, bool) { v, ok := env[k]; return v, ok })
+
+		var cerr *Error
+		if !errors.As(err, &cerr) || !slices.ContainsFunc(cerr.Problems, func(p Problem) bool { return p.Var == EnvPublicURL }) {
+			t.Errorf("load(%q) error = %v, want problem for %s", raw, err, EnvPublicURL)
+		}
+		if err != nil && (strings.Contains(err.Error(), "hunter2") || strings.Contains(err.Error(), "alice")) {
+			t.Errorf("load(%q) error leaks credentials: %v", raw, err)
+		}
+	}
+}
+
+func TestPublicOrigin(t *testing.T) {
+	t.Parallel()
+
+	u, err := url.Parse("https://alice:hunter2@meals.example.com:8443/app?token=x#frag")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got, want := (Config{PublicURL: u}).PublicOrigin(), "https://meals.example.com:8443"; got != want {
+		t.Errorf("PublicOrigin() = %q, want %q", got, want)
+	}
+}
diff --git a/internal/server/server.go b/internal/server/server.go
index fa4f203..597e456 100644
--- a/internal/server/server.go
+++ b/internal/server/server.go
@@ -14,9 +14,37 @@ import (
 func NewRouter(logger *slog.Logger, version string) http.Handler {
 	r := chi.NewRouter()
 	r.Get("/health", health(logger, version))
+	r.MethodNotAllowed(methodNotAllowed(logger))
 	return r
 }
 
+type errorBody struct {
+	Error apiError `json:"error"`
+}
+
+type apiError struct {
+	Code      string `json:"code"`
+	Message   string `json:"message"`
+	Retryable bool   `json:"retryable"`
+}
+
+func methodNotAllowed(logger *slog.Logger) http.HandlerFunc {
+	body, err := json.Marshal(errorBody{Error: apiError{
+		Code:    "method_not_allowed",
+		Message: "method not allowed",
+	}})
+	if err != nil {
+		panic(err) // fixed string and bool fields cannot fail to marshal
+	}
+	return func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusMethodNotAllowed)
+		if _, err := w.Write(body); err != nil {
+			logger.WarnContext(r.Context(), "write method not allowed response", "error", err)
+		}
+	}
+}
+
 type healthResponse struct {
 	Status  string `json:"status"`
 	Version string `json:"version"`
diff --git a/internal/server/server_test.go b/internal/server/server_test.go
index 0e7e9f9..b79856e 100644
--- a/internal/server/server_test.go
+++ b/internal/server/server_test.go
@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"io"
 	"log/slog"
+	"maps"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -48,4 +49,21 @@ func TestHealthRejectsOtherMethods(t *testing.T) {
 	if rec.Code != http.StatusMethodNotAllowed {
 		t.Errorf("status = %d, want 405", rec.Code)
 	}
+	if ct := rec.Header().Get("Content-Type"); ct != "application/json" {
+		t.Errorf("Content-Type = %q, want application/json", ct)
+	}
+	var got struct {
+		Error map[string]any `json:"error"`
+	}
+	if err := json.NewDecoder(rec.Body).Decode(&got); err != nil {
+		t.Fatal(err)
+	}
+	want := map[string]any{
+		"code":      "method_not_allowed",
+		"message":   "method not allowed",
+		"retryable": false,
+	}
+	if !maps.Equal(got.Error, want) {
+		t.Errorf("error body = %v, want %v", got.Error, want)
+	}
 }
-- 
2.55.0


From 454faea429ac4b954972f6d328cdf0fcd087f777 Mon Sep 17 00:00:00 2001
From: dev_agent <dev_agent@cubecraftlabs.com>
Date: Fri, 2 Oct 2026 22:16:08 -0400
Subject: [PATCH 3/3] Address second review findings F1-F3 for 404, public URL
 and listen host

- F1: return the JSON API error shape for unknown paths (404 not_found)
- F2: require a non-empty hostname in MEALQUEST_PUBLIC_URL
- F3: validate the MEALQUEST_HTTP_ADDR host as an IP address or hostname

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/config/config.go      | 28 +++++++++++++++++++++++--
 internal/config/config_test.go | 37 ++++++++++++++++++++++++++++++++++
 internal/server/server.go      | 14 +++++++------
 internal/server/server_test.go | 19 +++++++++++++++++
 4 files changed, 90 insertions(+), 8 deletions(-)

diff --git a/internal/config/config.go b/internal/config/config.go
index 1e84b99..5673595 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -119,11 +119,16 @@ func (r *reader) listenAddr(name string) string {
 	if !ok {
 		return ""
 	}
-	_, port, err := net.SplitHostPort(v)
+	host, port, err := net.SplitHostPort(v)
 	if err != nil {
 		r.fail(name, "invalid listen address %q (want host:port or :port)", v)
 		return ""
 	}
+	// An empty host listens on all interfaces.
+	if host != "" && net.ParseIP(host) == nil && !validHostname(host) {
+		r.fail(name, "invalid host %q in listen address (want IP address or hostname)", host)
+		return ""
+	}
 	if _, err := strconv.ParseUint(port, 10, 16); err != nil {
 		r.fail(name, "invalid port %q in listen address (want 0-65535)", port)
 		return ""
@@ -131,6 +136,25 @@ func (r *reader) listenAddr(name string) string {
 	return v
 }
 
+// validHostname reports whether h is a DNS hostname: dot-separated labels of
+// 1-63 letters, digits and hyphens, not starting or ending with a hyphen.
+func validHostname(h string) bool {
+	if len(h) > 253 {
+		return false
+	}
+	for _, label := range strings.Split(strings.TrimSuffix(h, "."), ".") {
+		if len(label) < 1 || len(label) > 63 || label[0] == '-' || label[len(label)-1] == '-' {
+			return false
+		}
+		for _, c := range label {
+			if !(c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '-') {
+				return false
+			}
+		}
+	}
+	return true
+}
+
 func (r *reader) httpURL(name string) *url.URL {
 	v, ok := r.required(name)
 	if !ok {
@@ -138,7 +162,7 @@ func (r *reader) httpURL(name string) *url.URL {
 	}
 	// The value is never echoed: it may carry credentials.
 	u, err := url.Parse(v)
-	if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" {
+	if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Hostname() == "" {
 		r.fail(name, "invalid URL (want absolute http:// or https:// URL)")
 		return nil
 	}
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index ebaafa2..01a184f 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -102,6 +102,43 @@ func TestLoad(t *testing.T) {
 			env:     with(map[string]string{EnvHTTPAddr: ":65536"}),
 			badVars: []string{EnvHTTPAddr},
 		},
+		{
+			name: "listen address with IP host",
+			env:  with(map[string]string{EnvHTTPAddr: "127.0.0.1:8080"}),
+		},
+		{
+			name: "listen address with IPv6 host",
+			env:  with(map[string]string{EnvHTTPAddr: "[::1]:8080"}),
+		},
+		{
+			name: "listen address with hostname",
+			env:  with(map[string]string{EnvHTTPAddr: "localhost:8080"}),
+		},
+		{
+			name:    "listen host with space",
+			env:     with(map[string]string{EnvHTTPAddr: "bad host:8080"}),
+			badVars: []string{EnvHTTPAddr},
+		},
+		{
+			name:    "listen host with leading hyphen label",
+			env:     with(map[string]string{EnvHTTPAddr: "-bad.example.com:8080"}),
+			badVars: []string{EnvHTTPAddr},
+		},
+		{
+			name:    "listen host with empty label",
+			env:     with(map[string]string{EnvHTTPAddr: "bad..example.com:8080"}),
+			badVars: []string{EnvHTTPAddr},
+		},
+		{
+			name:    "listen host with label over 63 chars",
+			env:     with(map[string]string{EnvHTTPAddr: strings.Repeat("a", 64) + ".example.com:8080"}),
+			badVars: []string{EnvHTTPAddr},
+		},
+		{
+			name:    "public URL without hostname",
+			env:     with(map[string]string{EnvPublicURL: "http://:8080"}),
+			badVars: []string{EnvPublicURL},
+		},
 		{
 			name:    "relative public URL",
 			env:     with(map[string]string{EnvPublicURL: "meals.example.com"}),
diff --git a/internal/server/server.go b/internal/server/server.go
index 597e456..33b0e38 100644
--- a/internal/server/server.go
+++ b/internal/server/server.go
@@ -14,7 +14,8 @@ import (
 func NewRouter(logger *slog.Logger, version string) http.Handler {
 	r := chi.NewRouter()
 	r.Get("/health", health(logger, version))
-	r.MethodNotAllowed(methodNotAllowed(logger))
+	r.MethodNotAllowed(apiErrorHandler(logger, http.StatusMethodNotAllowed, "method_not_allowed", "method not allowed"))
+	r.NotFound(apiErrorHandler(logger, http.StatusNotFound, "not_found", "not found"))
 	return r
 }
 
@@ -28,19 +29,20 @@ type apiError struct {
 	Retryable bool   `json:"retryable"`
 }
 
-func methodNotAllowed(logger *slog.Logger) http.HandlerFunc {
+// apiErrorHandler responds with status and a fixed, non-retryable API error.
+func apiErrorHandler(logger *slog.Logger, status int, code, message string) http.HandlerFunc {
 	body, err := json.Marshal(errorBody{Error: apiError{
-		Code:    "method_not_allowed",
-		Message: "method not allowed",
+		Code:    code,
+		Message: message,
 	}})
 	if err != nil {
 		panic(err) // fixed string and bool fields cannot fail to marshal
 	}
 	return func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
-		w.WriteHeader(http.StatusMethodNotAllowed)
+		w.WriteHeader(status)
 		if _, err := w.Write(body); err != nil {
-			logger.WarnContext(r.Context(), "write method not allowed response", "error", err)
+			logger.WarnContext(r.Context(), "write error response", "code", code, "error", err)
 		}
 	}
 }
diff --git a/internal/server/server_test.go b/internal/server/server_test.go
index b79856e..f8e3cc5 100644
--- a/internal/server/server_test.go
+++ b/internal/server/server_test.go
@@ -67,3 +67,22 @@ func TestHealthRejectsOtherMethods(t *testing.T) {
 		t.Errorf("error body = %v, want %v", got.Error, want)
 	}
 }
+
+func TestUnknownPathNotFound(t *testing.T) {
+	t.Parallel()
+
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+	rec := httptest.NewRecorder()
+	NewRouter(logger, "dev").ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/unknown", nil))
+
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("status = %d, want 404", rec.Code)
+	}
+	if ct := rec.Header().Get("Content-Type"); ct != "application/json" {
+		t.Errorf("Content-Type = %q, want application/json", ct)
+	}
+	const want = `{"error":{"code":"not_found","message":"not found","retryable":false}}`
+	if got := rec.Body.String(); got != want {
+		t.Errorf("body = %s, want %s", got, want)
+	}
+}
-- 
2.55.0