diff --git a/.env.example b/.env.example index 1b0533b..0d50d7c 100644 --- a/.env.example +++ b/.env.example @@ -7,7 +7,6 @@ GRAFANA_ADMIN_PASSWORD=change-this-admin-password OIDC_ISSUER_URL= OIDC_CLIENT_ID= OIDC_CLIENT_SECRET= -CMS_AUTO_PROMOTE_ALL_ADMINS=false CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP=false # Delta production variables live in deploy/cms.env.example. Do not put diff --git a/deploy/README.md b/deploy/README.md index e3f6676..823da02 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -114,12 +114,23 @@ Nginx will not start without `active-upstreams.conf`, since the site `include`s it unconditionally. A passing `nginx -t` *before* the site is enabled only validates the stock config and proves nothing. -The site sets `client_max_body_size 26m` on `/v1/` to sit just above the -backend's `MEDIA_MAX_UPLOAD_BYTES` (25 MiB default). Nginx's stock limit is 1m, +The site sets `client_max_body_size 91m` on `/v1/` to sit just above the +backend's `MEDIA_MAX_UPLOAD_BYTES` (90 MiB default). Nginx's stock limit is 1m, which rejects an ordinary phone photo with a 413 before the CMS ever sees it, so a deploy that skips re-copying this file leaves media uploads broken while the app looks correctly configured. Raise both together, never just one. +The 90 MiB figure is sized to the migrated corpus, which contains unresized +camera originals up to ~77 MiB (largest: `2025/07/BZ9A5771.jpg`). The hard +ceiling above it is Cloudflare's **100 MB** request-body limit on the tunnel +fronting Delta; a body that passes Nginx and the backend but exceeds that dies +at the edge with an error the CMS never sees, so do not raise the pair past +~95 MiB without moving media uploads off the tunnel. + +The backend streams uploads to a temp file rather than buffering them in RAM +(only the first 8 MiB stays in memory), so a large upload costs container disk, +not memory. + ### Media serving `location /wp-content/` reads the migrated WordPress corpus straight off CephFS. @@ -148,6 +159,48 @@ curl -I http://localhost/wp-content/uploads/YYYY/MM/name.jpg Expect `200` with `Cache-Control: public, max-age=2592000, immutable`. +### Making the media tree writable (uploads) + +The checks above only prove Nginx can *read*. `POST /v1/media` also has to +**write**, and the rsynced corpus arrives owned by whoever ran the rsync +(`tadmin`), mode 755, while the backend container runs as uid **10001**. Nothing +in the read path notices, so uploads fail long after media serving looks healthy: + +```bash +docker exec triangle-cms-backend-blue-1 \ + sh -c 'touch /mnt/cephfs/media/wp-content/uploads/.wtest && echo ok' +``` + +If that says `Permission denied`, grant the container uid write on upload +directories. CephFS is mounted with `acl`, so this is additive -- ownership and +the migrated files are untouched, and Nginx keeps reading as before. The mount +supporting ACLs does not mean the tools are installed; Ubuntu server images +generally lack them: + +```bash +sudo apt-get install -y acl # setfacl is not installed by default +sudo find /mnt/cephfs/media/wp-content/uploads -type d \ + -exec setfacl -m u:10001:rwx -m d:u:10001:rwx {} + +``` + +The `d:` (default) entry is what makes each new `YYYY/MM` directory inherit the +grant, so this does not need repeating every month. + +Without the `acl` package, setgid does the same job in plain POSIX, at the cost +of changing group ownership rather than adding a grant beside it: + +```bash +sudo find /mnt/cephfs/media/wp-content/uploads -type d -exec chgrp 101 {} + +sudo find /mnt/cephfs/media/wp-content/uploads -type d -exec chmod 2775 {} + +``` + +`101` is the container's gid; the setgid bit is what new directories inherit. + +The failure is easy to misread. `MkdirAll` returns nil for a directory that +already exists, and every migrated `YYYY/MM` directory does exist, so a +permission problem surfaces as `failed to store upload` rather than `failed to +create upload directory`. Check the backend log for the underlying `error=`. + ### Media library Serving the files is independent of *listing* them. The CMS media page reads a @@ -207,7 +260,6 @@ The file must contain the exact immutable image tag for the active deployment: - `FRONTEND_ORIGIN` - `OIDC_REDIRECT_URI` - `CMS_SESSION_TTL_SECONDS` -- `CMS_AUTO_PROMOTE_ALL_ADMINS` - `CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP` - `AKISMET_API_KEY` - optional; leave empty to disable comment spam filtering. - `AKISMET_BLOG_URL` - full public site URL Akismet should associate with @@ -218,10 +270,15 @@ The file must contain the exact immutable image tag for the active deployment: `/mnt/cephfs/media` unless the bind-mount target changes. - `MEDIA_BASE_URL` - public origin that serves `/wp-content/`, used to build media URLs returned by the upload endpoint. Empty yields relative URLs. +- `MEDIA_MAX_UPLOAD_BYTES` - per-file upload cap in bytes. Empty uses the + 90 MiB default. Must stay at or below Nginx's `client_max_body_size`. + +Keep `CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP=false` in production. Rebuild +taxonomy through the admin endpoint after deploys when needed. -Keep `CMS_AUTO_PROMOTE_ALL_ADMINS=false` and -`CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP=false` in production. Rebuild taxonomy -through the admin endpoint after deploys when needed. +New users are created as editors. The very first user to log in to an empty +`cms_users` table is bootstrapped as an admin; promote anyone else from the +users screen. ## Required GitHub Production Environment Variables diff --git a/deploy/cms.env.example b/deploy/cms.env.example index bb8e654..ab242d9 100644 --- a/deploy/cms.env.example +++ b/deploy/cms.env.example @@ -12,10 +12,10 @@ OIDC_CLIENT_SECRET= FRONTEND_ORIGIN= OIDC_REDIRECT_URI= CMS_SESSION_TTL_SECONDS= -CMS_AUTO_PROMOTE_ALL_ADMINS= CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP= AKISMET_API_KEY= AKISMET_BLOG_URL= MEDIA_HOST_PATH= MEDIA_ROOT= MEDIA_BASE_URL= +MEDIA_MAX_UPLOAD_BYTES= diff --git a/deploy/compose.cms.yml b/deploy/compose.cms.yml index a079034..c707afa 100644 --- a/deploy/compose.cms.yml +++ b/deploy/compose.cms.yml @@ -27,7 +27,6 @@ x-backend-base: &backend-base FRONTEND_ORIGIN: ${FRONTEND_ORIGIN:?FRONTEND_ORIGIN is required} OIDC_REDIRECT_URI: ${OIDC_REDIRECT_URI:?OIDC_REDIRECT_URI is required} CMS_SESSION_TTL_SECONDS: ${CMS_SESSION_TTL_SECONDS:-604800} - CMS_AUTO_PROMOTE_ALL_ADMINS: ${CMS_AUTO_PROMOTE_ALL_ADMINS:-false} CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP: ${CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP:-false} AKISMET_API_KEY: ${AKISMET_API_KEY:-} AKISMET_BLOG_URL: ${AKISMET_BLOG_URL:-} @@ -35,6 +34,10 @@ x-backend-base: &backend-base # assets under MEDIA_ROOT; MEDIA_BASE_URL is the public host that serves them. MEDIA_ROOT: ${MEDIA_ROOT:-/mnt/cephfs/media} MEDIA_BASE_URL: ${MEDIA_BASE_URL:-} + # Per-file upload cap. Must stay at or below Nginx's client_max_body_size + # (91m) and below Cloudflare's 100 MB tunnel limit; raise all of them + # together or the smallest one silently wins. + MEDIA_MAX_UPLOAD_BYTES: ${MEDIA_MAX_UPLOAD_BYTES:-94371840} volumes: # CephFS media tree (host). rw so the upload endpoint can store new files; # host Nginx serves the same tree read-only (see deploy/nginx/triangle-cms.conf). diff --git a/deploy/nginx/triangle-cms.conf b/deploy/nginx/triangle-cms.conf index 67f4a4a..4bd4bcd 100644 --- a/deploy/nginx/triangle-cms.conf +++ b/deploy/nginx/triangle-cms.conf @@ -39,12 +39,19 @@ server { } location /v1/ { - # POST /v1/media accepts up to MEDIA_MAX_UPLOAD_BYTES (25 MiB by - # default). Nginx's own default is 1m, so without this it rejects any + # POST /v1/media accepts up to MEDIA_MAX_UPLOAD_BYTES (90 MiB by + # default -- the legacy corpus has full-res camera originals near 77 + # MiB). Nginx's own default is 1m, so without this it rejects any # ordinary phone photo with a 413 before the request reaches the CMS, # and the app-side limit never gets a say. Keep this at or above the # backend's limit so the backend is the one that decides. - client_max_body_size 26m; + client_max_body_size 91m; + # A 90 MiB body over a slow uplink takes minutes; the stock 60s applies + # per read, but raise the ceilings so a stalled-but-alive upload is not + # killed mid-flight. + client_body_timeout 300s; + proxy_read_timeout 300s; + proxy_send_timeout 300s; proxy_pass $triangle_cms_backend; } diff --git a/docker-compose.yml b/docker-compose.yml index 39713b6..73b2dbf 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -43,7 +43,6 @@ services: TLS_KEY_FILE: /app/certs/localhost.key OIDC_ISSUER_URL: ${OIDC_ISSUER_URL:-} OIDC_CLIENT_ID: ${OIDC_CLIENT_ID:-} - CMS_AUTO_PROMOTE_ALL_ADMINS: ${CMS_AUTO_PROMOTE_ALL_ADMINS:-false} CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP: ${CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP:-false} depends_on: mariadb: diff --git a/frontend/src/components/Sidebar.tsx b/frontend/src/components/Sidebar.tsx index 121841f..5f4eb1a 100644 --- a/frontend/src/components/Sidebar.tsx +++ b/frontend/src/components/Sidebar.tsx @@ -4,7 +4,7 @@ import { LayoutDashboard, FileText, TrendingUp, - Mail, + // Mail, // used by the disabled Newsletter nav item Image, Users, Layers, @@ -52,7 +52,9 @@ const navGroups: NavGroup[] = [ { icon: TrendingUp, label: "Developing Stories", path: "/developing-stories" }, { icon: BarChart3, label: "Poll", path: "/poll" }, { icon: Image, label: "Media", path: "/media" }, - { icon: Mail, label: "Newsletter", path: "/newsletter" }, + // Newsletter is temporarily disabled; re-enable this entry (and the Mail + // import above) to bring it back. + // { icon: Mail, label: "Newsletter", path: "/newsletter" }, ], }, { diff --git a/server/internal/database/comments.go b/server/internal/database/comments.go index 5ebf826..2e9a9ea 100644 --- a/server/internal/database/comments.go +++ b/server/internal/database/comments.go @@ -110,6 +110,14 @@ func ScanComment(rows *sql.Rows) (Comment, error) { return comment, nil } +// GetApprovedCommentsByArticleSlug returns the reader comments shown under an +// article. Pingbacks and trackbacks are excluded: WordPress stored them in the +// same table, but they are automated link notifications rather than anything a +// reader wrote, and on the imported data they are almost entirely SEO spam. +// This matches the predicate adminCommentConditions already applies, which is +// what left the public endpoint as the one place they still surfaced. Rows stay +// in the table; nothing renders them. A NULL or empty type means a comment -- +// that is what WordPress wrote for ordinary rows. func GetApprovedCommentsByArticleSlug(ctx context.Context, conn *sql.DB, slug string) ([]Comment, error) { rows, err := conn.QueryContext(ctx, ` SELECT @@ -126,6 +134,7 @@ func GetApprovedCommentsByArticleSlug(ctx context.Context, conn *sql.DB, slug st FROM comments c JOIN articles a ON a.id = c.article_id WHERE a.slug = ? AND c.status = 'approved' + AND (c.`+"`type`"+` IS NULL OR c.`+"`type`"+` = '' OR c.`+"`type`"+` = 'comment') ORDER BY COALESCE(c.created_at_gmt, c.created_at), c.id `, slug) if err != nil { diff --git a/server/internal/database/users.go b/server/internal/database/users.go index 02c166e..5560925 100644 --- a/server/internal/database/users.go +++ b/server/internal/database/users.go @@ -3,7 +3,6 @@ package database import ( "context" "database/sql" - "os" "strings" "time" @@ -96,14 +95,8 @@ func EnsureUsersTable(ctx context.Context, conn *sql.DB) error { } func FindOrCreateUser(ctx context.Context, conn *sql.DB, sub, email, name string) (*models.User, error) { - autoPromoteAllAdmins := strings.EqualFold(strings.TrimSpace(os.Getenv("CMS_AUTO_PROMOTE_ALL_ADMINS")), "true") user, err := findUserBySub(ctx, conn, sub) if err == nil { - if autoPromoteAllAdmins && user.Role != models.RoleAdmin { - if err := UpdateUserRole(ctx, conn, user.ID, models.RoleAdmin); err == nil { - user.Role = models.RoleAdmin - } - } _ = updateLastLogin(ctx, conn, user.ID) return user, nil } @@ -113,19 +106,7 @@ func FindOrCreateUser(ctx context.Context, conn *sql.DB, sub, email, name string authorID := findAuthorIDByEmail(ctx, conn, email) - role := models.RoleEditor - if autoPromoteAllAdmins { - role = models.RoleAdmin - } - - res, err := conn.ExecContext(ctx, - "INSERT INTO cms_users (sub, email, name, role, author_id) VALUES (?, ?, ?, ?, ?)", - sub, email, name, role, authorID, - ) - if err != nil { - return nil, err - } - id, err := res.LastInsertId() + id, role, err := insertUser(ctx, conn, sub, email, name, authorID) if err != nil { return nil, err } @@ -143,6 +124,45 @@ func FindOrCreateUser(ctx context.Context, conn *sql.DB, sub, email, name string }, nil } +// insertUser creates a CMS user, bootstrapping the very first one as an admin so +// a fresh install has someone who can manage roles. Everyone after that starts as +// an editor and has to be promoted from the users screen. The count and the +// insert share a transaction (and a locking read) so two simultaneous first +// logins can't both come out as admin. +func insertUser(ctx context.Context, conn *sql.DB, sub, email, name string, authorID *int64) (int64, models.Role, error) { + tx, err := conn.BeginTx(ctx, nil) + if err != nil { + return 0, "", err + } + defer func() { _ = tx.Rollback() }() + + var existing int64 + if err := tx.QueryRowContext(ctx, "SELECT COUNT(*) FROM cms_users FOR UPDATE").Scan(&existing); err != nil { + return 0, "", err + } + + role := models.RoleEditor + if existing == 0 { + role = models.RoleAdmin + } + + res, err := tx.ExecContext(ctx, + "INSERT INTO cms_users (sub, email, name, role, author_id) VALUES (?, ?, ?, ?, ?)", + sub, email, name, role, authorID, + ) + if err != nil { + return 0, "", err + } + id, err := res.LastInsertId() + if err != nil { + return 0, "", err + } + if err := tx.Commit(); err != nil { + return 0, "", err + } + return id, role, nil +} + func findUserBySub(ctx context.Context, conn *sql.DB, sub string) (*models.User, error) { row := conn.QueryRowContext(ctx, "SELECT id, sub, email, name, role, author_id, created_at, last_login_at FROM cms_users WHERE sub = ?", diff --git a/server/internal/handlers/media.go b/server/internal/handlers/media.go index 5326cb4..a60739a 100644 --- a/server/internal/handlers/media.go +++ b/server/internal/handlers/media.go @@ -33,8 +33,18 @@ import ( const ( mediaFormField = "file" - // defaultMaxUploadBytes caps a single upload when MEDIA_MAX_UPLOAD_BYTES is unset. - defaultMaxUploadBytes int64 = 25 << 20 // 25 MiB + // defaultMaxUploadBytes caps a single upload when MEDIA_MAX_UPLOAD_BYTES is + // unset. The migrated WordPress corpus contains full-resolution camera + // originals up to ~77 MiB, so anything smaller than this rejects files the + // newsroom demonstrably produces. The ceiling is Cloudflare's 100 MB request + // limit on the tunnel in front of Delta -- stay below it, since a body that + // clears the backend but not the tunnel fails with an opaque edge error. + defaultMaxUploadBytes int64 = 90 << 20 // 90 MiB + // multipartMemoryBytes is how much of a multipart body ParseMultipartForm may + // hold in RAM before spilling the rest to temp files. It is deliberately + // decoupled from the size limit: passing the limit itself would let a single + // upload pin its full size in memory (and Go adds ~10 MiB of slack on top). + multipartMemoryBytes int64 = 8 << 20 // 8 MiB // uploadsSubdir is the WordPress-compatible prefix (under MEDIA_ROOT) that all // uploads live under, so new files line up with the rsynced legacy corpus and // the Nginx `location /wp-content/` root that serves them. @@ -111,7 +121,9 @@ func PostMedia(conn *sql.DB) http.Handler { // Cap the request body. A little slack over maxBytes covers multipart // framing so a file exactly at the limit still succeeds. r.Body = http.MaxBytesReader(w, r.Body, maxBytes+4096) - if err := r.ParseMultipartForm(maxBytes); err != nil { + // MaxBytesReader, not this argument, is what enforces the limit; this only + // decides where the bytes land on the way in (see multipartMemoryBytes). + if err := r.ParseMultipartForm(multipartMemoryBytes); err != nil { var tooLarge *http.MaxBytesError if errors.As(err, &tooLarge) { writeError(w, http.StatusRequestEntityTooLarge, @@ -157,6 +169,7 @@ func PostMedia(conn *sql.DB) http.Handler { relDir := path.Join(uploadsSubdir, now.Format("2006"), now.Format("01")) absDir := filepath.Join(root, filepath.FromSlash(relDir)) if err := os.MkdirAll(absDir, 0o775); err != nil { + slog.Error("media upload: create directory", "dir", absDir, "error", err) writeError(w, http.StatusInternalServerError, "failed to create upload directory") return } @@ -164,6 +177,13 @@ func PostMedia(conn *sql.DB) http.Handler { base := sanitizeBaseName(header.Filename) name, written, err := storeUpload(absDir, base, ext, file) if err != nil { + // Worth a log line rather than just a 500: the message the client + // gets cannot say whether the media volume is full, unmounted, or + // simply not writable by this container's uid, and those need very + // different fixes. A permission error here is easy to mistake for a + // mkdir failure, since MkdirAll returns nil for a directory that + // already exists -- which every migrated YYYY/MM directory does. + slog.Error("media upload: store file", "dir", absDir, "error", err) writeError(w, http.StatusInternalServerError, "failed to store upload") return } diff --git a/server/internal/handlers/media_test.go b/server/internal/handlers/media_test.go index 266885f..ac99ab5 100644 --- a/server/internal/handlers/media_test.go +++ b/server/internal/handlers/media_test.go @@ -12,6 +12,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strconv" "strings" "testing" @@ -69,6 +70,61 @@ func TestPostMedia_NotConfigured(t *testing.T) { } } +func TestPostMedia_RejectsOversizeUpload(t *testing.T) { + t.Setenv("MEDIA_ROOT", t.TempDir()) + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "1024") + rec := httptest.NewRecorder() + // Comfortably past the limit plus the multipart-framing slack, so this is + // the size check firing and not a boundary-arithmetic accident. + PostMedia(nil).ServeHTTP(rec, uploadRequest(t, "big.png", bytes.Repeat([]byte("a"), 64<<10))) + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want 413; body = %s", rec.Code, rec.Body.String()) + } +} + +// The size limit is enforced by MaxBytesReader, not by ParseMultipartForm's +// argument -- that one only decides how much stays in RAM before spilling to a +// temp file. A body well past multipartMemoryBytes but under the limit must +// therefore parse normally; reaching the 415 content-type check proves it did. +func TestPostMedia_AcceptsBodyLargerThanMultipartMemory(t *testing.T) { + t.Setenv("MEDIA_ROOT", t.TempDir()) + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", strconv.FormatInt(multipartMemoryBytes*4, 10)) + rec := httptest.NewRecorder() + PostMedia(nil).ServeHTTP(rec, uploadRequest(t, "notes.txt", bytes.Repeat([]byte("a"), int(multipartMemoryBytes)+1<<20))) + if rec.Code != http.StatusUnsupportedMediaType { + t.Fatalf("status = %d, want 415 (413 means the size cap fired early); body = %s", rec.Code, rec.Body.String()) + } +} + +func TestMaxUploadBytes(t *testing.T) { + // The migrated WordPress corpus holds camera originals near 77 MiB, so a + // default below that rejects files the newsroom already has. + if defaultMaxUploadBytes < 80<<20 { + t.Fatalf("defaultMaxUploadBytes = %d, too small for the legacy corpus", defaultMaxUploadBytes) + } + + t.Run("default when unset", func(t *testing.T) { + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "") + if got := maxUploadBytes(); got != defaultMaxUploadBytes { + t.Fatalf("maxUploadBytes = %d, want %d", got, defaultMaxUploadBytes) + } + }) + + t.Run("env override", func(t *testing.T) { + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "4096") + if got := maxUploadBytes(); got != 4096 { + t.Fatalf("maxUploadBytes = %d, want 4096", got) + } + }) + + t.Run("falls back on garbage", func(t *testing.T) { + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "90MiB") + if got := maxUploadBytes(); got != defaultMaxUploadBytes { + t.Fatalf("maxUploadBytes = %d, want the default %d", got, defaultMaxUploadBytes) + } + }) +} + func TestPostMediaIndex_NotConfigured(t *testing.T) { t.Setenv("MEDIA_ROOT", "") rec := httptest.NewRecorder()