diff --git a/.gitignore b/.gitignore index 221cfa9d..bd30d290 100644 --- a/.gitignore +++ b/.gitignore @@ -62,3 +62,9 @@ cli/gonext/gonext .dev/ tmp/ *.local + +# Personal Compose override. Ships as docker-compose.override.example.yml +# in the repo — copy it locally and tweak. Compose auto-loads +# docker-compose.override.yml without naming it on the CLI, so we must +# keep the live file out of git. +docker-compose.override.yml diff --git a/README.md b/README.md index 564e2bb7..0763057b 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,20 @@ To stop everything (volumes preserved): `make down`. To wipe state and start ove --- +## Local dev setup + +Before `make up`, copy the personal Compose override into place. It pins the admin build args, remaps Postgres to `5433` to dodge native-Postgres clashes, and aligns the `GONEXT_AUTH_*` secrets across `api` / `worker` / `migrate` so sessions and password hashes line up. + +```bash +cp docker-compose.override.example.yml docker-compose.override.yml +# Optionally edit the auth secrets — defaults are dev-only and safe to keep +docker compose up -d +``` + +The live `docker-compose.override.yml` is gitignored so each contributor keeps their own. See [Port conflicts](#port-conflicts-the-docker-composeoverrideyml-pattern) below for the full rationale. + +--- + ## Local development tips A few things that bit us when we ran this for the first time — fix them up front and the stack just works. diff --git a/apps/admin/Dockerfile b/apps/admin/Dockerfile index bfb29d26..d83fd3eb 100644 --- a/apps/admin/Dockerfile +++ b/apps/admin/Dockerfile @@ -72,10 +72,15 @@ COPY apps/admin/ apps/admin/ COPY packages/ts/ packages/ts/ # Build-time configuration plumbing for Next.js public env vars. +# GONEXT_API_URL is consumed by next.config.ts's `rewrites()` at build +# time — must be the docker-internal API hostname (e.g. http://api:8080) +# so server-side proxy hops work after the bundle is baked. ARG NEXT_PUBLIC_API_URL="" ARG NEXT_PUBLIC_VERSION="dev" +ARG GONEXT_API_URL="" ENV NEXT_PUBLIC_API_URL=${NEXT_PUBLIC_API_URL} \ NEXT_PUBLIC_VERSION=${NEXT_PUBLIC_VERSION} \ + GONEXT_API_URL=${GONEXT_API_URL} \ NEXT_TELEMETRY_DISABLED=1 \ NODE_ENV=production @@ -113,6 +118,18 @@ COPY --from=builder --chown=node:node /repo/node_modules ./node_modules COPY --from=builder --chown=node:node /repo/apps/admin ./apps/admin COPY --from=builder --chown=node:node /repo/packages/ts ./packages/ts +# Next.js `output: 'standalone'` ships a minimal node_modules tree at +# .next/standalone/, but DOES NOT copy .next/static or public/ into +# that tree (the docs explicitly say the operator must do this). When +# the entrypoint runs node .next/standalone/apps/admin/server.js, the +# server's static handler looks for chunks at +# .next/standalone/apps/admin/.next/static/* — which 404s without these +# COPYs and breaks every JS/CSS chunk load on every page. +COPY --from=builder --chown=node:node /repo/apps/admin/.next/static \ + ./apps/admin/.next/standalone/apps/admin/.next/static +COPY --from=builder --chown=node:node /repo/apps/admin/public \ + ./apps/admin/.next/standalone/apps/admin/public + USER node EXPOSE 3000 diff --git a/apps/admin/next.config.ts b/apps/admin/next.config.ts index 8063564d..6db4d08e 100644 --- a/apps/admin/next.config.ts +++ b/apps/admin/next.config.ts @@ -21,6 +21,23 @@ const nextConfig: NextConfig = { // typedRoutes moved out of `experimental` in Next.js 15; opt in at the // top level so `` values are checked at build time. typedRoutes: true, + /** + * Proxy /api/* to the GoNext API service. When the bundle ships with + * NEXT_PUBLIC_API_URL="" (the docker-compose default) the browser + * uses same-origin fetches against /api/*. This rewrite hops those + * requests over to the docker-internal API hostname so dev / preview + * environments don't need a separate reverse proxy. + * + * rewrites() evaluates at build time, so GONEXT_API_URL must be set + * as a build-arg (see apps/admin/Dockerfile + docker-compose.override.yml). + */ + async rewrites() { + const apiUrl = + process.env.GONEXT_API_URL || + process.env.NEXT_PUBLIC_API_URL || + 'http://localhost:8080'; + return [{ source: '/api/:path*', destination: `${apiUrl}/api/:path*` }]; + }, async headers() { return [ { diff --git a/apps/admin/src/app/(authenticated)/_components/TopHeader.tsx b/apps/admin/src/app/(authenticated)/_components/TopHeader.tsx index 60dd841d..9a142348 100644 --- a/apps/admin/src/app/(authenticated)/_components/TopHeader.tsx +++ b/apps/admin/src/app/(authenticated)/_components/TopHeader.tsx @@ -19,6 +19,21 @@ import type { ReactElement } from 'react'; import Link from 'next/link'; import { Bell, ExternalLink } from 'lucide-react'; +/** + * Resolve the public-site URL for the "View site" link. NEXT_PUBLIC_SITE_URL + * is the operator-set canonical front-end origin (production: the marketing + * site). When unset we fall back to the docker-compose web service on + * port 3000 — operators running the local stack expect that to open the + * @gonext/web preview, not the admin's own root. + * + * The constant is evaluated at module-load time so the link href is + * deterministic across renders. + */ +const SITE_URL = + process.env.NEXT_PUBLIC_SITE_URL && process.env.NEXT_PUBLIC_SITE_URL !== '' + ? process.env.NEXT_PUBLIC_SITE_URL + : 'http://localhost:3000'; + export function TopHeader(): ReactElement { return (
@@ -28,14 +43,16 @@ export function TopHeader(): ReactElement { Admin
-
diff --git a/apps/admin/src/app/(authenticated)/plugins/[plugin]/[slug]/PluginPageBridge.tsx b/apps/admin/src/app/(authenticated)/plugins/[name]/[slug]/PluginPageBridge.tsx similarity index 100% rename from apps/admin/src/app/(authenticated)/plugins/[plugin]/[slug]/PluginPageBridge.tsx rename to apps/admin/src/app/(authenticated)/plugins/[name]/[slug]/PluginPageBridge.tsx diff --git a/apps/admin/src/app/(authenticated)/plugins/[plugin]/[slug]/page.tsx b/apps/admin/src/app/(authenticated)/plugins/[name]/[slug]/page.tsx similarity index 91% rename from apps/admin/src/app/(authenticated)/plugins/[plugin]/[slug]/page.tsx rename to apps/admin/src/app/(authenticated)/plugins/[name]/[slug]/page.tsx index f14af35e..7e5f51d4 100644 --- a/apps/admin/src/app/(authenticated)/plugins/[plugin]/[slug]/page.tsx +++ b/apps/admin/src/app/(authenticated)/plugins/[name]/[slug]/page.tsx @@ -17,12 +17,12 @@ import type { ReactElement } from 'react'; import { PluginPageBridge } from './PluginPageBridge'; interface Props { - params: Promise<{ plugin: string; slug: string }>; + params: Promise<{ name: string; slug: string }>; } export const dynamic = 'force-dynamic'; export default async function PluginAdminPage({ params }: Props): Promise { - const { plugin, slug } = await params; + const { name: plugin, slug } = await params; return ; } diff --git a/apps/admin/src/app/(authenticated)/plugins/[name]/page.tsx b/apps/admin/src/app/(authenticated)/plugins/[name]/page.tsx index 26a545f3..553bd24a 100644 --- a/apps/admin/src/app/(authenticated)/plugins/[name]/page.tsx +++ b/apps/admin/src/app/(authenticated)/plugins/[name]/page.tsx @@ -37,7 +37,7 @@ async function fetchPlugin(name: string): Promise { } catch { cookieHeader = ''; } - const url = `${apiBaseUrl.replace(/\/$/, '')}/api/v1/plugins/${encodeURIComponent(name)}`; + const url = `${apiBaseUrl().replace(/\/$/, '')}/api/v1/plugins/${encodeURIComponent(name)}`; try { const res = await fetch(url, { method: 'GET', diff --git a/apps/admin/src/app/(authenticated)/plugins/actions.ts b/apps/admin/src/app/(authenticated)/plugins/actions.ts index d90927b5..916758da 100644 --- a/apps/admin/src/app/(authenticated)/plugins/actions.ts +++ b/apps/admin/src/app/(authenticated)/plugins/actions.ts @@ -51,7 +51,7 @@ async function callHost( path: string, init: RequestInit, ): Promise { - const url = `${apiBaseUrl.replace(/\/$/, '')}${path}`; + const url = `${apiBaseUrl().replace(/\/$/, '')}${path}`; const cookie = await cookieHeader(); const headers = new Headers(init.headers); if (cookie && !headers.has('Cookie')) headers.set('Cookie', cookie); diff --git a/apps/admin/src/app/(authenticated)/plugins/page.tsx b/apps/admin/src/app/(authenticated)/plugins/page.tsx index 6865688f..aa21da33 100644 --- a/apps/admin/src/app/(authenticated)/plugins/page.tsx +++ b/apps/admin/src/app/(authenticated)/plugins/page.tsx @@ -59,7 +59,7 @@ async function fetchPlugins(): Promise { cookieHeader = ''; } - const url = `${apiBaseUrl.replace(/\/$/, '')}/api/v1/plugins`; + const url = `${apiBaseUrl().replace(/\/$/, '')}/api/v1/plugins`; try { const res = await fetch(url, { method: 'GET', diff --git a/apps/admin/src/app/(authenticated)/posts/PostListClient.test.tsx b/apps/admin/src/app/(authenticated)/posts/PostListClient.test.tsx index 9be9c25a..9665ede4 100644 --- a/apps/admin/src/app/(authenticated)/posts/PostListClient.test.tsx +++ b/apps/admin/src/app/(authenticated)/posts/PostListClient.test.tsx @@ -93,13 +93,13 @@ describe('PostListClient', () => { // Title links are the user-facing handle to each row. expect( screen.getByRole('link', { name: /hello world/i }), - ).toHaveAttribute('href', '/posts/p1/edit'); + ).toHaveAttribute('href', '/posts/p1'); expect( screen.getByRole('link', { name: /draft notes/i }), - ).toHaveAttribute('href', '/posts/p2/edit'); + ).toHaveAttribute('href', '/posts/p2'); expect( screen.getByRole('link', { name: /trashed item/i }), - ).toHaveAttribute('href', '/posts/p3/edit'); + ).toHaveAttribute('href', '/posts/p3'); // Status badges round-trip from the post.status field. expect(screen.getByLabelText('Status: Published')).toBeInTheDocument(); diff --git a/apps/admin/src/app/(authenticated)/posts/[id]/page.tsx b/apps/admin/src/app/(authenticated)/posts/[id]/page.tsx index fff4a1e9..b61f3753 100644 --- a/apps/admin/src/app/(authenticated)/posts/[id]/page.tsx +++ b/apps/admin/src/app/(authenticated)/posts/[id]/page.tsx @@ -234,7 +234,7 @@ export default function PostDetailPage(): ReactElement {

Compose your post by adding blocks below. Changes are - autosaved every 30 seconds; "Save changes" promotes the + autosaved every 30 seconds; “Save changes” promotes the autosave to the canonical row.

diff --git a/apps/admin/src/app/(authenticated)/posts/columns.tsx b/apps/admin/src/app/(authenticated)/posts/columns.tsx index 68a5195c..71046362 100644 --- a/apps/admin/src/app/(authenticated)/posts/columns.tsx +++ b/apps/admin/src/app/(authenticated)/posts/columns.tsx @@ -134,11 +134,13 @@ export function formatDate(iso: string): string { } /** - * Build the `/posts/{id}/edit` href used by the title cell. Centralised so - * a route move only touches one file. + * Build the `/posts/{id}` href used by the title cell. The single-id + * route IS the editor — the [id]/page.tsx renders the block editor + * directly. A previous version of this file appended `/edit` to the + * path; that path 404s because no nested route exists. */ export function postEditHref(id: string): string { - return `/posts/${encodeURIComponent(id)}/edit`; + return `/posts/${encodeURIComponent(id)}`; } /** Column id, header label, and whether it's sortable. */ diff --git a/apps/admin/src/app/(authenticated)/posts/page.tsx b/apps/admin/src/app/(authenticated)/posts/page.tsx index fa9cb178..7e4eb971 100644 --- a/apps/admin/src/app/(authenticated)/posts/page.tsx +++ b/apps/admin/src/app/(authenticated)/posts/page.tsx @@ -96,14 +96,52 @@ async function fetchInitialPosts(): Promise<{ error: `HTTP ${res.status}`, }; } - const json = (await res.json()) as PostListResponse; - // Be defensive: the API contract is still evolving (issue #76) so - // missing fields shouldn't crash the page. + // The API returns the envelope shape `{data: [...], pagination: {...}}` + // — see rest/posts/handlers.go list(). The page UI works in + // `{posts, nextCursor, total}` form with a flatter Post shape, so + // adapt here. Be defensive: a missing field shouldn't crash the + // page (issue #76 — contract still evolving). + type ApiPost = { + id: string; + title: string; + status: string; + published_at?: string | null; + updated_at?: string; + created_at?: string; + author_id?: string; + }; + type ApiEnvelope = { + data?: ApiPost[]; + posts?: ApiPost[]; + pagination?: { next_cursor?: string; nextCursor?: string }; + nextCursor?: string; + total?: number; + }; + const json = (await res.json()) as ApiEnvelope; + const rows = Array.isArray(json.data) + ? json.data + : Array.isArray(json.posts) + ? json.posts + : []; + const cursor = + json.pagination?.next_cursor ?? + json.pagination?.nextCursor ?? + json.nextCursor ?? + null; + // Map ApiPost → the flatter Post shape the UI expects. + const posts = rows.map((p) => ({ + id: p.id, + title: p.title ?? '(untitled)', + status: (p.status as PostListResponse['posts'][number]['status']) ?? 'draft', + date: p.published_at ?? p.updated_at ?? p.created_at ?? '', + author: { id: p.author_id ?? '', displayName: '' }, + commentsCount: 0, + })); return { data: { - posts: Array.isArray(json.posts) ? json.posts : [], - nextCursor: json.nextCursor ?? null, - total: typeof json.total === 'number' ? json.total : 0, + posts: posts as PostListResponse['posts'], + nextCursor: cursor, + total: typeof json.total === 'number' ? json.total : posts.length, }, error: null, }; diff --git a/apps/admin/src/app/(public)/setup/SetupWizard.tsx b/apps/admin/src/app/(public)/setup/SetupWizard.tsx index 2221cda3..b700b48d 100644 --- a/apps/admin/src/app/(public)/setup/SetupWizard.tsx +++ b/apps/admin/src/app/(public)/setup/SetupWizard.tsx @@ -166,7 +166,7 @@ export default function SetupWizard({ initialStatus }: SetupWizardProps): ReactE setSubmitting(true); setServerError(null); try { - const response = await fetch(`${apiBaseUrl}/api/v1/setup/install`, { + const response = await fetch(`${apiBaseUrl()}/api/v1/setup/install`, { method: 'POST', credentials: 'include', headers: { diff --git a/apps/admin/src/lib/api-client.test.ts b/apps/admin/src/lib/api-client.test.ts new file mode 100644 index 00000000..85c066d7 --- /dev/null +++ b/apps/admin/src/lib/api-client.test.ts @@ -0,0 +1,55 @@ +/** + * Tests for the admin API base URL resolver. + * + * Regression coverage for issue #498: when `apiBaseUrl` was a + * module-load constant, Next.js's Client Component bundler evaluated + * `typeof window === 'undefined'` at build time (Node process, no + * window) and baked the docker-internal `http://api:8080` into the + * browser bundle, breaking every client fetch with DNS failures. + * + * The fix exposes `apiBaseUrl` as a function so the runtime branch + * resolves in the actual execution environment. These tests pin that + * behavior: + * + * - With no `window`, the server branch picks `GONEXT_API_URL`. + * - With a `window`, the client branch picks `NEXT_PUBLIC_API_URL`. + * - Empty string survives as a valid "same-origin via rewrites" + * value — it must NOT be coerced to the localhost default. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +describe('apiBaseUrl', () => { + const originalEnv = { ...process.env }; + + beforeEach(() => { + vi.resetModules(); + }); + + afterEach(() => { + process.env = { ...originalEnv }; + vi.unstubAllGlobals(); + }); + + it('returns GONEXT_API_URL when running server-side (no window)', async () => { + process.env.GONEXT_API_URL = 'http://api:8080'; + process.env.NEXT_PUBLIC_API_URL = ''; + vi.stubGlobal('window', undefined); + const { apiBaseUrl } = await import('./api-client'); + expect(apiBaseUrl()).toBe('http://api:8080'); + }); + + it('returns NEXT_PUBLIC_API_URL when running in browser (window defined)', async () => { + process.env.GONEXT_API_URL = 'http://api:8080'; + process.env.NEXT_PUBLIC_API_URL = ''; + vi.stubGlobal('window', {} as unknown as Window); + const { apiBaseUrl } = await import('./api-client'); + expect(apiBaseUrl()).toBe(''); + }); + + it('preserves empty string as a valid value (no falsy coerce)', async () => { + process.env.NEXT_PUBLIC_API_URL = ''; + vi.stubGlobal('window', {} as unknown as Window); + const { apiBaseUrl } = await import('./api-client'); + expect(apiBaseUrl()).toBe(''); + }); +}); diff --git a/apps/admin/src/lib/api-client.ts b/apps/admin/src/lib/api-client.ts index 8dc83755..cce8c88f 100644 --- a/apps/admin/src/lib/api-client.ts +++ b/apps/admin/src/lib/api-client.ts @@ -21,9 +21,59 @@ const DEFAULT_BASE_URL = 'http://localhost:8080'; -export const apiBaseUrl: string = - (typeof process !== 'undefined' && process.env.NEXT_PUBLIC_API_URL) || - DEFAULT_BASE_URL; +/** + * Resolve the API base URL. The admin app runs in two contexts: + * + * - Browser (client): NEXT_PUBLIC_API_URL is baked into the bundle at + * build time. When empty (the docker-compose default), fetches hit + * the same origin and the Next.js `rewrites()` config proxies + * `/api/*` to the docker-internal API hostname. + * - Node (server): Server Components / Route Handlers / middleware + * run inside the admin container; the Next.js rewrites do NOT apply + * to outbound fetches from server code. We need an explicit + * docker-internal hostname — GONEXT_API_URL=http://api:8080. + * + * Empty string is a deliberate value (it means "same origin via + * rewrites") and must NOT be coerced to the default. We use an + * explicit `undefined` check rather than `||` to preserve that. + * + * The branch selection MUST happen at request time, not at module-load + * time — see the docstring on `apiBaseUrl()` below for why. + */ +function pickNonUndefined(value: string | undefined): string | undefined { + return value === undefined ? undefined : value; +} + +/** + * Resolve the API base URL at **runtime**. + * + * IMPORTANT: this MUST be called at request time, not module-load time. + * Next.js bundles Client Components at build time, when + * `typeof window === 'undefined'` evaluates to TRUE on the bundler's + * Node process. If we cached the result in a module-load constant the + * server branch would win and `http://api:8080` (the docker-internal + * hostname) would be baked into every Client Component bundle, + * producing browser-side DNS failures on every fetch. Keeping this as + * a function means the `typeof window` check evaluates in the actual + * runtime (browser → client branch, server-component render → server + * branch). See issue #498. + */ +export function apiBaseUrl(): string { + const isServer = typeof window === 'undefined'; + if (isServer) { + // Server: prefer GONEXT_API_URL (docker-internal), fall back to the + // public URL, and only use the localhost default as a last resort. + const server = process.env.GONEXT_API_URL; + if (server !== undefined && server !== '') return server; + const pub = pickNonUndefined(process.env.NEXT_PUBLIC_API_URL); + if (pub !== undefined) return pub; + return DEFAULT_BASE_URL; + } + // Client: empty string is a legitimate "same-origin via rewrites" + // configuration. Only fall through to DEFAULT when the var is unset. + const pub = process.env.NEXT_PUBLIC_API_URL; + return pub === undefined ? DEFAULT_BASE_URL : pub; +} export type ApiMethod = 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE'; @@ -93,7 +143,7 @@ export async function apiRequest( ...headers, }; - const response = await fetch(joinUrl(apiBaseUrl, path), { + const response = await fetch(joinUrl(apiBaseUrl(), path), { method, headers: finalHeaders, credentials: 'include', diff --git a/apps/admin/src/lib/server-api.ts b/apps/admin/src/lib/server-api.ts index 84ec48a7..b7f1b6ad 100644 --- a/apps/admin/src/lib/server-api.ts +++ b/apps/admin/src/lib/server-api.ts @@ -58,7 +58,7 @@ async function buildHeaders( } function joinUrl(path: string): string { - return `${apiBaseUrl}${path.startsWith('/') ? path : `/${path}`}`; + return `${apiBaseUrl()}${path.startsWith('/') ? path : `/${path}`}`; } /** diff --git a/apps/api/cmd/server/adapters.go b/apps/api/cmd/server/adapters.go index d181e50a..8d31e128 100644 --- a/apps/api/cmd/server/adapters.go +++ b/apps/api/cmd/server/adapters.go @@ -10,6 +10,7 @@ package main import ( "context" + "encoding/json" "errors" "fmt" "strings" @@ -85,13 +86,25 @@ func userLookupByEmail(pool *pgxpool.Pool) login.UserLookup { } var rec login.UserRecord var hash *string + // users.meta.roles is the canonical role-grant source (see + // cli/gonext/cmd/init/admin.go which seeds the bootstrap + // super_admin there). We project it as a JSON array of text + // using `meta->'roles'` and let pgx scan it into a + // []string via a JSON-aware destination. The COALESCE keeps + // older rows (or future OAuth-only signups) with a null + // roles array from blowing up the scan. + var rolesJSON []byte err := pool.QueryRow(ctx, ` - SELECT u.id::text, u.email::text, p.password_hash, u.status + SELECT u.id::text, + u.email::text, + p.password_hash, + u.status, + COALESCE(u.meta->'roles', '[]'::jsonb) FROM users u LEFT JOIN user_passwords p ON p.user_id = u.id WHERE u.email = $1::citext LIMIT 1 - `, email).Scan(&rec.ID, &rec.Email, &hash, &rec.Status) + `, email).Scan(&rec.ID, &rec.Email, &hash, &rec.Status, &rolesJSON) if err != nil { if errors.Is(err, pgx.ErrNoRows) { return login.UserRecord{}, login.ErrUserNotFound @@ -101,6 +114,59 @@ func userLookupByEmail(pool *pgxpool.Pool) login.UserLookup { if hash != nil { rec.Hash = *hash } + // Decode roles. A non-array or unmarshal error degrades to + // "no roles" rather than failing login — the audit log + // already captures the user_id, so an operator can spot a + // bad meta payload from the privilege-less behavior. + if len(rolesJSON) > 0 { + var roles []string + if jsonErr := json.Unmarshal(rolesJSON, &roles); jsonErr == nil { + rec.Roles = roles + } + } + return rec, nil + } +} + +// userLookupByID is the by-ID variant of userLookupByEmail. Used by the +// TOTP finalize path. Same projection (status + meta.roles) so the +// principal carried into the session matches what the password path +// would produce. +func userLookupByID(pool *pgxpool.Pool) login.UserByIDLookup { + return func(ctx context.Context, userID string) (login.UserRecord, error) { + userID = strings.TrimSpace(userID) + if userID == "" { + return login.UserRecord{}, login.ErrUserNotFound + } + var rec login.UserRecord + var hash *string + var rolesJSON []byte + err := pool.QueryRow(ctx, ` + SELECT u.id::text, + u.email::text, + p.password_hash, + u.status, + COALESCE(u.meta->'roles', '[]'::jsonb) + FROM users u + LEFT JOIN user_passwords p ON p.user_id = u.id + WHERE u.id = $1::uuid + LIMIT 1 + `, userID).Scan(&rec.ID, &rec.Email, &hash, &rec.Status, &rolesJSON) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return login.UserRecord{}, login.ErrUserNotFound + } + return login.UserRecord{}, fmt.Errorf("login: user lookup by id: %w", err) + } + if hash != nil { + rec.Hash = *hash + } + if len(rolesJSON) > 0 { + var roles []string + if jsonErr := json.Unmarshal(rolesJSON, &roles); jsonErr == nil { + rec.Roles = roles + } + } return rec, nil } } diff --git a/apps/api/cmd/server/main.go b/apps/api/cmd/server/main.go index 2fd3f727..a0f81dd2 100644 --- a/apps/api/cmd/server/main.go +++ b/apps/api/cmd/server/main.go @@ -45,11 +45,13 @@ import ( adminmenus "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/menus" adminpluginpages "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/pluginpages" adminposts "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/posts" + adminsettings "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/settings" adminstatus "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/status" adminthemes "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/themes" adminwebhooks "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/webhooks" restimg "github.com/Singleton-Solution/GoNext/apps/api/internal/rest/img" "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/customizer" + themesstatic "github.com/Singleton-Solution/GoNext/apps/api/internal/themes/static" adminredirects "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/redirects" "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/rum" "github.com/Singleton-Solution/GoNext/apps/api/internal/auth/login" @@ -100,6 +102,7 @@ import ( "github.com/Singleton-Solution/GoNext/packages/go/revisions" pkgsearch "github.com/Singleton-Solution/GoNext/packages/go/search" "github.com/Singleton-Solution/GoNext/packages/go/session" + pkgsettings "github.com/Singleton-Solution/GoNext/packages/go/settings" "github.com/Singleton-Solution/GoNext/packages/go/shutdown" "github.com/Singleton-Solution/GoNext/packages/go/theme/seed" "github.com/Singleton-Solution/GoNext/packages/go/tracing" @@ -761,9 +764,10 @@ func buildRouter(cfg *config.Config, pool *pgxpool.Pool, rdb *goredis.Client, se // gate the routes — install_themes, manage_themes, switch_themes // — so a deploy that only wants the switch surface can withhold // install_themes without losing the list endpoint. + themeActiveStore := &adminthemes.PgxActiveStore{Pool: pool} if err := adminthemes.Mount(mux, "/api/v1/admin/themes", adminthemes.Deps{ ThemeDir: themeDir, - Active: &adminthemes.PgxActiveStore{Pool: pool}, + Active: themeActiveStore, Policy: policy.NewBasicPolicy(policy.DefaultRoleCapabilities()), Logger: logger, }); err != nil { @@ -774,6 +778,40 @@ func buildRouter(cfg *config.Config, pool *pgxpool.Pool, rdb *goredis.Client, se ) } + // Public theme-asset surface (issue #501). Serves the on-disk + // CSS/JS/font files for every installed theme under /themes/{slug} + // /{file...}. The virtual sentinel "active" resolves through the + // shared PgxActiveStore so /themes/active/style.css follows the + // active-theme switch automatically. Without this handler the + // early-hints provider above preloads a URL no one serves and the + // public site renders with whatever globals.css ships — no theme + // CSS reaches the browser. + // + // We construct an ActiveResolver closure that calls Get on the + // existing store (same options-row read the admin handlers use). + // A read error is logged and surfaced as "no active theme" — the + // handler turns that into a 404 rather than a 500 because a + // transient DB hiccup should not bring down public CSS. + if err := themesstatic.Mount(mux, "/themes", themesstatic.Deps{ + ThemeDir: themeDir, + ActiveResolver: func() string { + slug, err := themeActiveStore.Get(context.Background()) + if err != nil { + logger.Debug("themes/static: active resolver failed", + slog.Any("err", err)) + return "" + } + return slug + }, + Logger: logger, + }); err != nil { + logger.Warn("themes/static: failed to mount", slog.Any("err", err)) + } else { + logger.Info("themes/static: routes mounted", + slog.String("base", "/themes"), + ) + } + // First-run install surface — the in-browser alternative to // `gonext init`. The two endpoints (/api/v1/setup/status and // /api/v1/setup/install) are mounted unconditionally; the lock @@ -886,6 +924,7 @@ func buildRouter(cfg *config.Config, pool *pgxpool.Pool, rdb *goredis.Client, se } else { if err := login.Mount(mux, login.Deps{ Lookup: userLookupByEmail(pool), + UserByID: userLookupByID(pool), Sessions: sessions, Pepper: []byte(cfg.Auth.Pepper), SessionAbsoluteTTL: cfg.Auth.SessionTTL, @@ -1511,6 +1550,60 @@ func buildRouter(cfg *config.Config, pool *pgxpool.Pool, rdb *goredis.Client, se } } + // Site settings registry (/api/v1/settings, issue #499). Every + // admin Settings sub-page (general, permalinks, reading, writing, + // privacy) reads + writes through this endpoint. The package-level + // settings registry holds the schema; the Postgres store persists + // values into the options table from migration 000008. + // + // We seed a fresh per-process registry with the core + privacy + // settings here rather than relying on the packages/go/settings + // global so the boot path stays explicit — a future change that + // registers plugin settings would extend this block, not reach into + // a singleton at import time. The Postgres store needs a non-nil + // pool; in pool-less test contexts we fall back to the MemoryStore + // so the route table still mounts and the admin form's "API + // available" probe succeeds. + // + // The routes are wrapped with authmw.RequireSession because the + // global middleware chain does not include OptionalSession (the + // regression fixed in #31). Wrapping per-Mount mirrors how + // /api/v1/auth/sessions and /api/v1/account/data wire themselves. + settingsReg := pkgsettings.NewRegistry() + if err := pkgsettings.RegisterCore(settingsReg); err != nil { + logger.Warn("settings: failed to register core settings", slog.Any("err", err)) + } + if err := pkgsettings.RegisterPrivacy(settingsReg); err != nil { + logger.Warn("settings: failed to register privacy settings", slog.Any("err", err)) + } + var settingsStore pkgsettings.Store + if pool != nil { + settingsStore = pkgsettings.NewPostgresStore(pool, settingsReg) + } else { + settingsStore = pkgsettings.NewMemoryStore(settingsReg) + logger.Warn("settings: pool nil; using in-memory store") + } + settingsMux := http.NewServeMux() + if err := adminsettings.Mount(settingsMux, "/api/v1/settings", adminsettings.Deps{ + Store: settingsStore, + Registry: settingsReg, + Policy: adminPolicy, + Logger: logger, + }); err != nil { + logger.Warn("settings: failed to mount routes", slog.Any("err", err)) + } else { + var settingsHandler http.Handler = settingsMux + if sessions != nil { + settingsHandler = authmw.RequireSession(sessions)(settingsMux) + } else { + logger.Warn("settings: session manager nil; mounting without RequireSession") + } + mux.Handle("/api/v1/settings", settingsHandler) + logger.Info("settings: routes mounted", + slog.String("base", "/api/v1/settings"), + ) + } + // Admin plugin pages surface (/api/v1/admin/plugin-pages). Walks // every active plugin's manifest, pulls out the admin_pages // declarations, and returns the flattened set so the sidebar can diff --git a/apps/api/internal/admin/comments/list.go b/apps/api/internal/admin/comments/list.go index d804aa34..b913fb60 100644 --- a/apps/api/internal/admin/comments/list.go +++ b/apps/api/internal/admin/comments/list.go @@ -32,11 +32,11 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request, _ policy.Princip var filter ListFilter - // Status filter. Empty string means "no filter"; any other - // value must be in AllStatuses or we 400 so the client doesn't - // accidentally typo "approve" (the bulk verb) instead of - // "approved" (the state). - if s := q.Get("status"); s != "" { + // Status filter. Empty string and the literal "any" both mean + // "no filter"; any other value must be in AllStatuses or we 400 + // so the client doesn't accidentally typo "approve" (the bulk + // verb) instead of "approved" (the state). + if s := q.Get("status"); s != "" && s != "any" { st := Status(s) if !IsValidStatus(st) { router.WriteError(w, http.StatusBadRequest, "invalid_status", diff --git a/apps/api/internal/admin/customizer/store.go b/apps/api/internal/admin/customizer/store.go index cefcf836..97d55d27 100644 --- a/apps/api/internal/admin/customizer/store.go +++ b/apps/api/internal/admin/customizer/store.go @@ -65,14 +65,18 @@ const readOverridesSQL = `SELECT value FROM options WHERE key = $1` // writeOverridesSQL upserts the overrides row. autoload is FALSE // because the renderer reads via the per-request settings store cache // path; autoloading at boot would prime cache with values the API-only -// replicas never need. namespace is "core" so the row is uninstall-safe -// (plugin-namespace deletes don't sweep it). +// replicas never need. +// +// `namespace` and explicit `updated_at` are omitted: the options +// table from migration 000008 has neither (`updated_at` defaults to +// now() and is bumped by the options_touch trigger). An earlier +// draft of this writer referenced both; PATCH was 500'ing in +// production before the fix. const writeOverridesSQL = ` -INSERT INTO options (key, value, autoload, namespace, updated_at) -VALUES ($1, $2, FALSE, 'core', now()) +INSERT INTO options (key, value, autoload) +VALUES ($1, $2, FALSE) ON CONFLICT (key) DO UPDATE - SET value = EXCLUDED.value, - updated_at = now() + SET value = EXCLUDED.value ` // deleteOverridesSQL clears the overrides row for a slug. Used by the diff --git a/apps/api/internal/admin/settings/handler.go b/apps/api/internal/admin/settings/handler.go new file mode 100644 index 00000000..1aa47843 --- /dev/null +++ b/apps/api/internal/admin/settings/handler.go @@ -0,0 +1,336 @@ +// Package settings mounts the thin HTTP layer in front of the +// settings registry shipped in packages/go/settings. +// +// The package itself ships the schema (Setting + Registry), validation, +// and a Postgres-backed Store. What was missing was the HTTP surface +// that the admin Settings sub-pages call — /api/v1/settings?group=… +// (GET) and /api/v1/settings (PATCH). Issue #499 covers wiring that +// surface; this package is that wiring. +// +// Two-layer split (handler here, store + registry in +// packages/go/settings) follows the same convention used by every other +// admin module under apps/api/internal/admin/* (themes, customizer, +// marketplace, …). Keeps the package-level types reusable from CLI / +// worker / migration contexts without dragging net/http into the +// shared package. +package settings + +import ( + "encoding/json" + "errors" + "io" + "log/slog" + "net/http" + "strings" + + "github.com/Singleton-Solution/GoNext/apps/api/internal/rest/router" + "github.com/Singleton-Solution/GoNext/packages/go/policy" + pkgsettings "github.com/Singleton-Solution/GoNext/packages/go/settings" +) + +// maxBodyBytes caps the PATCH payload at 64 KiB. The largest plausible +// patch is a full re-write of every core key (~30 string values, all +// small); 64 KiB is the "stop a runaway client" bound, not a tight +// per-request limit. +const maxBodyBytes = 64 * 1024 + +// Deps is the dependency bag for Mount. Store and Registry are +// required; Policy is required to gate writes; Logger falls back to +// slog.Default when nil. +// +// The Registry is passed explicitly rather than read from the +// packages/go/settings package-level global so callers (in particular +// tests) can isolate their own registry without touching shared state. +type Deps struct { + // Store reads + writes setting values. Memory in tests, Postgres + // in production. + Store pkgsettings.Store + + // Registry holds the Setting declarations the handler iterates + // when building the "all keys in group X" response. + Registry *pkgsettings.Registry + + // Policy gates writes on the manage_options capability. Reads + // require an authenticated principal but no specific capability — + // site settings are operator metadata, not user PII, and every + // admin role that lands in the dashboard has reason to read them. + Policy policy.Policy + + // Logger receives structured log lines. nil falls back to + // slog.Default. + Logger *slog.Logger +} + +func (d Deps) validate() error { + if d.Store == nil { + return errors.New("admin/settings: Store is required") + } + if d.Registry == nil { + return errors.New("admin/settings: Registry is required") + } + if d.Policy == nil { + return errors.New("admin/settings: Policy is required") + } + return nil +} + +// handlers is the resolved-Deps form passed around inside the package. +type handlers struct { + store pkgsettings.Store + registry *pkgsettings.Registry + policy policy.Policy + logger *slog.Logger +} + +// Mount wires the settings routes onto mux under base (typically +// "/api/v1/settings"). Returns an error rather than panicking if Deps +// is malformed so the boot path surfaces it cleanly. +// +// The route tree: +// +// GET {base} — list values, optionally filtered by ?group= +// PATCH {base} — sparse partial update +// +// Reads require any authenticated principal; writes require the +// manage_options capability. +func Mount(mux *http.ServeMux, base string, deps Deps) error { + if err := deps.validate(); err != nil { + return err + } + if deps.Logger == nil { + deps.Logger = slog.Default() + } + h := &handlers{ + store: deps.Store, + registry: deps.Registry, + policy: deps.Policy, + logger: deps.Logger, + } + + base = strings.TrimRight(base, "/") + if base == "" { + return errors.New("admin/settings: base path must not be empty") + } + // Bare-path patterns ("GET /api/v1/settings", no trailing slash, no + // "{$}") match the endpoint exactly under net/http's ServeMux. We + // intentionally do NOT use the "{base}/{$}" form here because that + // would cause net/http to 301-redirect bare-path requests (the + // admin client) to the trailing-slash form, which our handler + // never serves. The admin sends "/api/v1/settings?group=…" with + // no trailing slash; this match must be exact. + mux.Handle("GET "+base, h.requireAuth(h.getSettings)) + mux.Handle("PATCH "+base, h.requireCap(policy.CapManageOptions, h.patchSettings)) + return nil +} + +// requireAuth wraps a handler with the authentication check. The +// authenticated principal is forwarded to the handler; anonymous +// requests get 401 before the handler runs. +func (h *handlers) requireAuth(next func(http.ResponseWriter, *http.Request, policy.Principal)) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + pr, ok := policy.FromContext(r.Context()) + if !ok { + router.WriteError(w, http.StatusUnauthorized, "unauthenticated", "authentication required") + return + } + next(w, r, pr) + }) +} + +// requireCap wraps a handler with the auth + capability check. Returns +// 401 for anonymous requests, 403 when the principal lacks cap. +func (h *handlers) requireCap(cap policy.Capability, next func(http.ResponseWriter, *http.Request, policy.Principal)) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + pr, ok := policy.FromContext(r.Context()) + if !ok { + router.WriteError(w, http.StatusUnauthorized, "unauthenticated", "authentication required") + return + } + if d := h.policy.Can(pr, cap, nil); !d.Allowed { + router.WriteError(w, http.StatusForbidden, "forbidden", d.Reason) + return + } + next(w, r, pr) + }) +} + +// getSettings handles GET /api/v1/settings[?group=]. +// +// The response shape is a flat `{key: value}` JSON object — matches the +// "Settings" schema in apps/api/openapi/openapi.yaml (additionalProperties: +// true) and the SettingsValues type the admin client expects. +// +// The group filter is interpreted as a key-prefix match: ?group=core.site +// returns every registered key starting with "core.site." (so +// core.site.name, core.site.tagline, etc.). This matches the admin's +// SettingsGroup literals ("core.site", "core.reading", …) without +// requiring the registry to be re-grouped — the registry already keys +// settings by dotted namespace, which is the right index. +// +// "privacy" is a special-case: the privacy group's keys live under +// "core.privacy.*", so ?group=privacy is rewritten to "core.privacy" +// before the prefix match. Spelled out here rather than inferred so the +// translation is visible to future maintainers. +// +// Empty group returns every registered key. An unknown group returns +// an empty `{}` (200), not 404 — the admin renders an empty form +// rather than an error banner, which matches the existing "API not +// available" graceful-degradation path. +func (h *handlers) getSettings(w http.ResponseWriter, r *http.Request, _ policy.Principal) { + group := strings.TrimSpace(r.URL.Query().Get("group")) + prefix := groupPrefix(group) + + // Collect the registered keys whose canonical key matches the + // prefix. Empty prefix (no group filter) matches every key. + all := h.registry.List() + keys := make([]string, 0, len(all)) + for _, s := range all { + if prefix == "" || strings.HasPrefix(s.Key, prefix) { + keys = append(keys, s.Key) + } + } + + out := make(map[string]any, len(keys)) + if len(keys) > 0 { + values, err := h.store.BulkRead(r.Context(), keys) + if err != nil { + h.logger.ErrorContext(r.Context(), "admin/settings: bulk read failed", + slog.String("group", group), + slog.Any("err", err), + ) + router.WriteError(w, http.StatusInternalServerError, "internal_error", "failed to load settings") + return + } + for k, v := range values { + out[k] = v + } + } + + router.WriteJSON(w, http.StatusOK, out) +} + +// patchSettings handles PATCH /api/v1/settings. +// +// Request body is a sparse `{key: value}` map; every present key is +// validated against its registered schema and written. The response is +// the post-write values for every key the patch touched, so the admin +// can refresh its form state without a separate GET. +// +// Per-key errors are aggregated into a 400 problem-details response — +// rather than committing the keys that succeeded and 400'ing the +// failure, we fail the whole patch atomically. (The Store interface +// does not expose a transaction handle, so "atomic" here means +// "we don't write anything if any key fails validation"; once a write +// hits Postgres the other writes in the same patch are already +// committed. Validation runs first so the common case — operator +// submits a form with a typo — never reaches the SQL layer.) +func (h *handlers) patchSettings(w http.ResponseWriter, r *http.Request, _ policy.Principal) { + body, err := io.ReadAll(io.LimitReader(r.Body, maxBodyBytes+1)) + if err != nil { + router.WriteError(w, http.StatusBadRequest, "bad_request", "failed to read request body") + return + } + if len(body) > maxBodyBytes { + router.WriteError(w, http.StatusRequestEntityTooLarge, "payload_too_large", + "request body exceeds the maximum allowed size") + return + } + if len(body) == 0 { + router.WriteError(w, http.StatusBadRequest, "bad_request", "request body is empty") + return + } + + var patch map[string]any + if err := json.Unmarshal(body, &patch); err != nil { + router.WriteError(w, http.StatusBadRequest, "bad_json", "request body must be a JSON object") + return + } + if len(patch) == 0 { + // Empty patch — nothing to do, mirror the GET response shape + // for an unfiltered request so the client can still refresh + // its form state from the response. + router.WriteJSON(w, http.StatusOK, map[string]any{}) + return + } + + // Validate-then-write. The Store's Write returns ErrUnknownKey for + // unregistered keys and ErrValidation for schema failures; we map + // both to 400 with the offending key in the detail so the admin + // can highlight the field. + for key, value := range patch { + if _, err := h.registry.Get(key); err != nil { + router.WriteError(w, http.StatusBadRequest, "unknown_key", + "unknown setting key: "+key) + return + } + if err := h.store.Write(r.Context(), key, value); err != nil { + switch { + case errors.Is(err, pkgsettings.ErrUnknownKey): + router.WriteError(w, http.StatusBadRequest, "unknown_key", + "unknown setting key: "+key) + case errors.Is(err, pkgsettings.ErrValidation): + router.WriteError(w, http.StatusBadRequest, "validation_error", + "invalid value for "+key+": "+err.Error()) + default: + h.logger.ErrorContext(r.Context(), "admin/settings: write failed", + slog.String("key", key), + slog.Any("err", err), + ) + router.WriteError(w, http.StatusInternalServerError, "internal_error", + "failed to persist setting "+key) + } + return + } + } + + // Echo back the post-write values for the keys we touched. This + // is the same shape the GET returns, so the client can drop the + // response straight into its form state. + keys := make([]string, 0, len(patch)) + for k := range patch { + keys = append(keys, k) + } + values, err := h.store.BulkRead(r.Context(), keys) + if err != nil { + // The writes succeeded; surfacing this as 500 would mislead the + // admin into thinking the save failed. Log and return the + // caller's patch as the best-effort response. + h.logger.WarnContext(r.Context(), "admin/settings: post-write bulk read failed", + slog.Any("err", err), + ) + router.WriteJSON(w, http.StatusOK, patch) + return + } + router.WriteJSON(w, http.StatusOK, values) +} + +// groupPrefix maps a SettingsGroup literal from the admin client into +// the dotted key-prefix the registry indexes by. +// +// - "" (no group) → "" (match every key) +// - "core.site" → "core.site." (matches core.site.name etc.) +// - "core.reading" → "core.reading." +// - "core.writing" → "core.writing." +// - "core.permalinks" → "core.permalinks." +// - "privacy" → "core.privacy." +// - anything else → "." (best-effort prefix match) +// +// The trailing dot prevents "core.site" from matching "core.sitemap.*" +// if a plugin ever registers under that namespace — prefix matches at +// the dot boundary are intentional, not accidental. +func groupPrefix(group string) string { + if group == "" { + return "" + } + // The admin uses "privacy" as a literal because the corresponding + // settings page lives under /settings/privacy. The registry keys + // are under core.privacy.*. Rewrite once here so the rest of the + // handler can stay prefix-driven. + if group == "privacy" { + return "core.privacy." + } + if strings.HasSuffix(group, ".") { + return group + } + return group + "." +} diff --git a/apps/api/internal/admin/settings/handler_test.go b/apps/api/internal/admin/settings/handler_test.go new file mode 100644 index 00000000..3b0e3e81 --- /dev/null +++ b/apps/api/internal/admin/settings/handler_test.go @@ -0,0 +1,322 @@ +package settings + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/Singleton-Solution/GoNext/packages/go/policy" + pkgsettings "github.com/Singleton-Solution/GoNext/packages/go/settings" +) + +// testBase is the route prefix used in every test. Mirrors the +// production wiring in main.go. +const testBase = "/api/v1/settings" + +// adminPrincipal is the Principal a request carries when an admin is +// signed in. The admin role holds manage_options in +// DefaultRoleCapabilities, so writes succeed. +func adminPrincipal() policy.Principal { + return policy.Principal{UserID: "user:admin", Roles: []policy.Role{policy.RoleAdmin}} +} + +// subscriberPrincipal lacks manage_options but is authenticated, so +// reads succeed and writes 403. +func subscriberPrincipal() policy.Principal { + return policy.Principal{UserID: "user:subscriber", Roles: []policy.Role{policy.RoleSubscriber}} +} + +// newTestDeps builds a Deps with a fresh registry seeded with core + +// privacy settings and a MemoryStore. The returned Registry pointer is +// the same one in Deps, so tests that want to register additional +// settings can do so after construction. +func newTestDeps(t *testing.T) Deps { + t.Helper() + reg := pkgsettings.NewRegistry() + if err := pkgsettings.RegisterCore(reg); err != nil { + t.Fatalf("RegisterCore: %v", err) + } + if err := pkgsettings.RegisterPrivacy(reg); err != nil { + t.Fatalf("RegisterPrivacy: %v", err) + } + return Deps{ + Store: pkgsettings.NewMemoryStore(reg), + Registry: reg, + Policy: policy.NewBasicPolicy(policy.DefaultRoleCapabilities()), + } +} + +// mountTest builds a fresh mux + handler from deps. Returns the mux so +// tests can drive ServeHTTP directly. +func mountTest(t *testing.T, deps Deps) *http.ServeMux { + t.Helper() + mux := http.NewServeMux() + if err := Mount(mux, testBase, deps); err != nil { + t.Fatalf("Mount: %v", err) + } + return mux +} + +// do executes req against mux and returns the recorded response. A +// helper because every test does this five-line sequence. +func do(t *testing.T, mux *http.ServeMux, req *http.Request) *httptest.ResponseRecorder { + t.Helper() + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + return rec +} + +// withPrincipal returns a copy of req with the given Principal stashed +// on its context — mirrors what the auth middleware does in production. +func withPrincipal(req *http.Request, pr policy.Principal) *http.Request { + return req.WithContext(policy.WithPrincipal(req.Context(), pr)) +} + +// TestMount_RequiresAuth verifies that an anonymous GET returns 401 +// rather than serving the response or panicking on a nil Principal. +func TestMount_RequiresAuth(t *testing.T) { + deps := newTestDeps(t) + mux := mountTest(t, deps) + + req := httptest.NewRequest(http.MethodGet, testBase, nil) + rec := do(t, mux, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status: want 401, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// TestGet_EmptyGroup_ReturnsEmptyMap is the acceptance-test from issue +// #499: a GET with a registered group whose keys are not yet stored +// returns 200 with the registered defaults (or an empty map if no keys +// match). Critically, it does NOT 404 or 500 — the admin form has a +// shape to render. +func TestGet_EmptyGroup_ReturnsEmptyMap(t *testing.T) { + // Use a registry with NO settings registered so the response is + // guaranteed empty. This is the strictest form of the acceptance + // criterion: empty group means {} body, not 500. + reg := pkgsettings.NewRegistry() + deps := Deps{ + Store: pkgsettings.NewMemoryStore(reg), + Registry: reg, + Policy: policy.NewBasicPolicy(policy.DefaultRoleCapabilities()), + } + mux := mountTest(t, deps) + + req := httptest.NewRequest(http.MethodGet, testBase+"?group=core.site", nil) + req = withPrincipal(req, adminPrincipal()) + rec := do(t, mux, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status: want 200, got %d body=%s", rec.Code, rec.Body.String()) + } + var body map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unmarshal response: %v body=%s", err, rec.Body.String()) + } + if len(body) != 0 { + t.Fatalf("response body: want {}, got %v", body) + } +} + +// TestGet_CoreSite_ReturnsDefaults verifies the happy path for the +// admin /settings/general page: GET ?group=core.site returns the +// registered defaults for core.site.* keys. +func TestGet_CoreSite_ReturnsDefaults(t *testing.T) { + deps := newTestDeps(t) + mux := mountTest(t, deps) + + req := httptest.NewRequest(http.MethodGet, testBase+"?group=core.site", nil) + req = withPrincipal(req, adminPrincipal()) + rec := do(t, mux, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status: want 200, got %d body=%s", rec.Code, rec.Body.String()) + } + var body map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unmarshal response: %v", err) + } + // Expect at least core.site.name to be present with its default. + if got, ok := body["core.site.name"].(string); !ok || got != "My GoNext Site" { + t.Fatalf("core.site.name: want default %q, got %v", "My GoNext Site", body["core.site.name"]) + } + // core.permalinks.format is in a different group prefix and must + // NOT leak into the response. + if _, present := body["core.permalinks.format"]; present { + t.Fatalf("response leaked permalinks key into core.site group: %v", body) + } +} + +// TestGet_PrivacyGroup_RewritesPrefix verifies the special-case "privacy" +// → "core.privacy." prefix rewrite documented on groupPrefix. +func TestGet_PrivacyGroup_RewritesPrefix(t *testing.T) { + deps := newTestDeps(t) + mux := mountTest(t, deps) + + req := httptest.NewRequest(http.MethodGet, testBase+"?group=privacy", nil) + req = withPrincipal(req, adminPrincipal()) + rec := do(t, mux, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status: want 200, got %d body=%s", rec.Code, rec.Body.String()) + } + var body map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unmarshal response: %v", err) + } + if _, present := body[pkgsettings.PrivacyAllowGDPRSelfService]; !present { + t.Fatalf("response missing privacy key %q: body=%v", + pkgsettings.PrivacyAllowGDPRSelfService, body) + } +} + +// TestGet_NoGroup_ReturnsAll verifies that an unfiltered GET returns +// every registered key — the shape the OpenAPI spec documents +// (`additionalProperties: true`). +func TestGet_NoGroup_ReturnsAll(t *testing.T) { + deps := newTestDeps(t) + mux := mountTest(t, deps) + + req := httptest.NewRequest(http.MethodGet, testBase, nil) + req = withPrincipal(req, adminPrincipal()) + rec := do(t, mux, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status: want 200, got %d", rec.Code) + } + var body map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unmarshal response: %v", err) + } + // Core + privacy registries together ship >10 settings. A strict + // count here would rot every time a key is added; assert the floor. + if len(body) < 10 { + t.Fatalf("unfiltered response: want >=10 keys, got %d (%v)", len(body), body) + } +} + +// TestPatch_HappyPath verifies that a valid PATCH persists the value +// and the response echoes it. +func TestPatch_HappyPath(t *testing.T) { + deps := newTestDeps(t) + mux := mountTest(t, deps) + + payload := map[string]any{"core.site.name": "Hello"} + body, _ := json.Marshal(payload) + + req := httptest.NewRequest(http.MethodPatch, testBase, bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req = withPrincipal(req, adminPrincipal()) + rec := do(t, mux, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status: want 200, got %d body=%s", rec.Code, rec.Body.String()) + } + var got map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("unmarshal response: %v", err) + } + if got["core.site.name"] != "Hello" { + t.Fatalf("response: want Hello, got %v", got["core.site.name"]) + } + + // Round-trip via GET to confirm persistence. + getReq := httptest.NewRequest(http.MethodGet, testBase+"?group=core.site", nil) + getReq = withPrincipal(getReq, adminPrincipal()) + getRec := do(t, mux, getReq) + if getRec.Code != http.StatusOK { + t.Fatalf("get status: want 200, got %d", getRec.Code) + } + var afterGet map[string]any + if err := json.Unmarshal(getRec.Body.Bytes(), &afterGet); err != nil { + t.Fatalf("get unmarshal: %v", err) + } + if afterGet["core.site.name"] != "Hello" { + t.Fatalf("post-write GET: want Hello, got %v", afterGet["core.site.name"]) + } +} + +// TestPatch_RequiresManageOptions verifies that a request from a user +// without manage_options is rejected 403. +func TestPatch_RequiresManageOptions(t *testing.T) { + deps := newTestDeps(t) + mux := mountTest(t, deps) + + body, _ := json.Marshal(map[string]any{"core.site.name": "Nope"}) + req := httptest.NewRequest(http.MethodPatch, testBase, bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req = withPrincipal(req, subscriberPrincipal()) + rec := do(t, mux, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("status: want 403, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// TestPatch_UnknownKey_Returns400 verifies that PATCH with an +// unregistered key returns a structured 400 rather than committing or +// 500'ing. +func TestPatch_UnknownKey_Returns400(t *testing.T) { + deps := newTestDeps(t) + mux := mountTest(t, deps) + + body, _ := json.Marshal(map[string]any{"this.does.not.exist": "x"}) + req := httptest.NewRequest(http.MethodPatch, testBase, bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req = withPrincipal(req, adminPrincipal()) + rec := do(t, mux, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status: want 400, got %d body=%s", rec.Code, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), "unknown") { + t.Fatalf("response should mention 'unknown', got %s", rec.Body.String()) + } +} + +// TestPatch_ValidationError verifies that a value violating the +// schema is rejected 400. +func TestPatch_ValidationError(t *testing.T) { + deps := newTestDeps(t) + mux := mountTest(t, deps) + + // core.site.name has minLength:1, maxLength:80. An empty string + // fails minLength. + body, _ := json.Marshal(map[string]any{"core.site.name": ""}) + req := httptest.NewRequest(http.MethodPatch, testBase, bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req = withPrincipal(req, adminPrincipal()) + rec := do(t, mux, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status: want 400, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// TestPatch_BadJSON returns 400 on a malformed body. +func TestPatch_BadJSON(t *testing.T) { + deps := newTestDeps(t) + mux := mountTest(t, deps) + + req := httptest.NewRequest(http.MethodPatch, testBase, strings.NewReader("not json")) + req.Header.Set("Content-Type", "application/json") + req = withPrincipal(req, adminPrincipal()) + rec := do(t, mux, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status: want 400, got %d", rec.Code) + } +} + +// TestMount_NilDeps_Errors verifies that Mount returns an error +// rather than panicking when Deps is malformed. +func TestMount_NilDeps_Errors(t *testing.T) { + mux := http.NewServeMux() + if err := Mount(mux, testBase, Deps{}); err == nil { + t.Fatal("Mount: want error for empty Deps, got nil") + } +} diff --git a/apps/api/internal/admin/themes/store.go b/apps/api/internal/admin/themes/store.go index b40f4509..bb7d8d04 100644 --- a/apps/api/internal/admin/themes/store.go +++ b/apps/api/internal/admin/themes/store.go @@ -46,12 +46,17 @@ const readActiveSQL = `SELECT value #>> '{}' FROM options WHERE key = $1` // literal ("gn-hello" → JSON value "gn-hello"). autoload is TRUE // because every renderer wakeup needs this key — keeping it in the // autoload set avoids one cache miss per cold boot. +// +// `namespace` and explicit `updated_at` are omitted: the options +// table from migration 000008 has neither (`updated_at` defaults to +// now() and is bumped by the options_touch trigger). An earlier +// draft referenced both columns; live INSERTs against this SQL were +// returning "column namespace does not exist" before the fix. const writeActiveSQL = ` -INSERT INTO options (key, value, autoload, namespace, updated_at) -VALUES ($1, to_jsonb($2::text), TRUE, 'core', now()) +INSERT INTO options (key, value, autoload) +VALUES ($1, to_jsonb($2::text), TRUE) ON CONFLICT (key) DO UPDATE - SET value = EXCLUDED.value, - updated_at = now() + SET value = EXCLUDED.value ` // PgxActiveStore is the production ActiveStore backed by pgx. diff --git a/apps/api/internal/auth/login/deps.go b/apps/api/internal/auth/login/deps.go index 3bca2f21..24cd5783 100644 --- a/apps/api/internal/auth/login/deps.go +++ b/apps/api/internal/auth/login/deps.go @@ -28,6 +28,13 @@ type UserRecord struct { // pass; suspended / deleted return ErrInvalidCredentials so the // admin-side decision isn't leaked to a stranger. Status string + + // Roles are the role slugs assigned to this user, projected from + // users.meta.roles by the SQL lookup. The service stamps these + // into the session's data map so RequireSession-derived principals + // carry the role grant. Empty slice → anonymous-equivalent + // authorization (login succeeds but every capability check fails). + Roles []string } // TOTPRecord describes a user's TOTP enrolment. SecretBase32 is the @@ -66,6 +73,13 @@ var ErrTOTPNotEnabled = errors.New("login: TOTP not enabled") // fakes need to lower-case both sides. type UserLookup func(ctx context.Context, email string) (UserRecord, error) +// UserByIDLookup returns the user record for an already-authenticated +// userID. Used by the TOTP finalize path which only has the userID +// (recovered from the intermediate token), not an email. Implementations +// MUST be case-equivalent to UserLookup: same source of truth, same +// role projection. +type UserByIDLookup func(ctx context.Context, userID string) (UserRecord, error) + // TOTPLookup returns the TOTP enrolment for a user, or ErrTOTPNotEnabled. // If the underlying user_totp table doesn't exist yet (the migration // ships in a separate PR), implementations should return @@ -120,6 +134,13 @@ type Deps struct { // Lookup performs the email -> UserRecord query. Required. Lookup UserLookup + // UserByID performs the userID -> UserRecord query. Used by the + // TOTP finalize path so the post-2FA session carries the same + // roles the no-2FA path would. May be nil — when nil, the + // finalize path degrades to a session with no roles (same + // behavior as before this field landed). + UserByID UserByIDLookup + // TOTPLookup performs the user -> TOTPRecord query. May be nil // when 2FA isn't wired in this deployment. TOTPLookup TOTPLookup diff --git a/apps/api/internal/auth/login/service.go b/apps/api/internal/auth/login/service.go index 255381b8..d9d660c2 100644 --- a/apps/api/internal/auth/login/service.go +++ b/apps/api/internal/auth/login/service.go @@ -235,7 +235,7 @@ func (s *Service) Authenticate(ctx context.Context, in Input) (Result, error) { switch { case !hasTOTP: // 2FA disabled or not wired — straight to session creation. - return s.completeLogin(ctx, in, user.ID) + return s.completeLogin(ctx, in, user.ID, user.Roles) case in.TOTPCode != "" || in.RecoveryCode != "": // First-call body carried a code. Verify it inline. @@ -248,7 +248,7 @@ func (s *Service) Authenticate(ctx context.Context, in Input) (Result, error) { s.emitFailed(ctx, in, user.ID, "wrong_totp") return Result{}, ErrTOTPInvalid } - return s.completeLogin(ctx, in, user.ID) + return s.completeLogin(ctx, in, user.ID, user.Roles) default: // 2FA required but code not provided. Issue an intermediate @@ -292,7 +292,13 @@ func (s *Service) finalizeTOTP(ctx context.Context, in Input) (Result, error) { // now (operator disabled it mid-flow). Drop the token and // finish the login — there's nothing further to verify. _ = s.deps.Intermediate.Delete(ctx, in.IntermediateToken) - return s.completeLogin(ctx, in, userID) + // Re-fetch the user to recover Roles (the intermediate token + // carries only the userID, not the role projection). If the + // lookup is unwired or fails, completeLogin still proceeds + // with nil roles — same degradation as before this fix, but + // no longer the default-every-time behavior. + roles := s.lookupRolesByID(ctx, userID) + return s.completeLogin(ctx, in, userID, roles) } if !s.verifyTOTPOrRecovery(in, tot) { @@ -313,14 +319,52 @@ func (s *Service) finalizeTOTP(ctx context.Context, in Input) (Result, error) { // can't be replayed. _ = s.deps.Intermediate.Delete(ctx, in.IntermediateToken) - return s.completeLogin(ctx, in, userID) + // Re-fetch the user record so the post-2FA session carries the + // same Roles the no-2FA path stamps in. Without this the user's + // role grants are dropped on every TOTP-gated login (issue #496). + // If UserByID is nil or fails, we keep going with nil roles — + // same fallback as the previous code, just no longer on the + // happy path. + roles := s.lookupRolesByID(ctx, userID) + return s.completeLogin(ctx, in, userID, roles) +} + +// lookupRolesByID re-fetches the user's role projection via +// Deps.UserByID. It exists so finalizeTOTP can recover the role grants +// that the intermediate token doesn't carry. Errors and nil deps are +// folded into "no roles" — completeLogin treats an empty slice the +// same way it would before this fix, so a missing UserByID wiring +// degrades to the pre-#496 behavior (login succeeds, capability +// checks return 403) rather than panicking. +func (s *Service) lookupRolesByID(ctx context.Context, userID string) []string { + if s.deps.UserByID == nil { + return nil + } + user, err := s.deps.UserByID(ctx, userID) + if err != nil { + s.deps.Log.WarnContext(ctx, "login: UserByID lookup error (finalizeTOTP)", + slog.String("user_id", userID), + slog.String("err", err.Error())) + return nil + } + return user.Roles } // completeLogin mints the session, fires the audit success event, and // clears the failure counter. Used by both the no-2FA branch and the // post-2FA finalization. -func (s *Service) completeLogin(ctx context.Context, in Input, userID string) (Result, error) { - token, err := s.deps.Sessions.Create(ctx, userID, nil, s.deps.SessionAbsoluteTTL, s.deps.SessionIdleTTL) +// +// rolesFromLookup is the role slugs from the UserRecord. We stamp them +// into the session's data map under the "roles" key so the auth +// middleware's DefaultPrincipal derives a Principal with role grants +// — without this every capability check returns 403 "principal has +// no roles" even for the super_admin seeded by `gonext init`. +func (s *Service) completeLogin(ctx context.Context, in Input, userID string, rolesFromLookup []string) (Result, error) { + var data map[string]any + if len(rolesFromLookup) > 0 { + data = map[string]any{"roles": rolesFromLookup} + } + token, err := s.deps.Sessions.Create(ctx, userID, data, s.deps.SessionAbsoluteTTL, s.deps.SessionIdleTTL) if err != nil { s.deps.Log.WarnContext(ctx, "login: session create failed", slog.String("user_id", userID), diff --git a/apps/api/internal/auth/login/service_test.go b/apps/api/internal/auth/login/service_test.go index d317ea80..c6118935 100644 --- a/apps/api/internal/auth/login/service_test.go +++ b/apps/api/internal/auth/login/service_test.go @@ -46,18 +46,19 @@ type fakeSession struct { type fakeSessionRecord struct { userID string + data map[string]any ttl time.Duration idleTTL time.Duration } -func (f *fakeSession) Create(_ context.Context, userID string, _ map[string]any, ttl, idleTTL time.Duration) (string, error) { +func (f *fakeSession) Create(_ context.Context, userID string, data map[string]any, ttl, idleTTL time.Duration) (string, error) { f.mu.Lock() defer f.mu.Unlock() if f.err != nil { return "", f.err } f.nextID++ - f.created = append(f.created, fakeSessionRecord{userID: userID, ttl: ttl, idleTTL: idleTTL}) + f.created = append(f.created, fakeSessionRecord{userID: userID, data: data, ttl: ttl, idleTTL: idleTTL}) // Mint a token shaped like the real one (32 bytes base64url, 43 chars), // padded with a counter for uniqueness. tok := "fake-token-0000000000000000000000000000000-" + string(rune('A'+f.nextID%26)) @@ -70,6 +71,18 @@ func (f *fakeSession) creates() int { return len(f.created) } +// lastData returns the data map handed to the most recent Create call. +// Tests use it to assert what the session was stamped with (e.g. the +// "roles" key carried through from the user record). +func (f *fakeSession) lastData() map[string]any { + f.mu.Lock() + defer f.mu.Unlock() + if len(f.created) == 0 { + return nil + } + return f.created[len(f.created)-1].data +} + // newFixture wires a fixture with defaults that match the production // rate-limit policy from doc 06. Tests overlay specific values they // care about. @@ -779,6 +792,134 @@ func TestAuthenticate_RecordsFailureForKnownUserOnly(t *testing.T) { } } +// TestFinalizeTOTP_ThreadsRolesFromUserByID is the regression test for +// issue #496: when a user with TOTP enabled completes the two-step +// login, the post-2FA session MUST carry the user's roles. Before the +// fix, the TOTP finalize path passed nil for roles to completeLogin, +// which meant any super_admin enrolled in TOTP lost their admin grant +// on every subsequent sign-in. +func TestFinalizeTOTP_ThreadsRolesFromUserByID(t *testing.T) { + f := newFixture(t) + f.addUser(t, "u-1", "alice@example.com", "pwd", "active") + sec, err := totp.Generate("GoNext", "alice@example.com") + if err != nil { + t.Fatalf("totp.Generate: %v", err) + } + f.addTOTP(t, "u-1", sec.Base32) + + // Wire UserByID to return a record with a role grant. The fix + // re-fetches via this lookup at finalize time so the role slice + // reaches completeLogin → Sessions.Create. + f.deps.UserByID = func(_ context.Context, userID string) (UserRecord, error) { + if userID != "u-1" { + return UserRecord{}, ErrUserNotFound + } + return UserRecord{ + ID: "u-1", + Email: "alice@example.com", + Status: "active", + Roles: []string{"super_admin"}, + }, nil + } + svc := f.service(t) + + // Step 1: password OK, server returns an intermediate token. + first, err := svc.Authenticate(context.Background(), Input{ + Email: "alice@example.com", + Password: "pwd", + IP: "10.0.0.1", + }) + if err != nil { + t.Fatalf("first call err: %v", err) + } + if !first.RequiresTOTP { + t.Fatal("first call: not RequiresTOTP") + } + + // Step 2: present TOTP code with the intermediate token. + code, err := generateCurrentTOTP(sec.Base32) + if err != nil { + t.Fatalf("generate code: %v", err) + } + final, err := svc.Authenticate(context.Background(), Input{ + IntermediateToken: first.IntermediateToken, + TOTPCode: code, + IP: "10.0.0.1", + }) + if err != nil { + t.Fatalf("second call err: %v", err) + } + if final.Token == "" { + t.Fatal("Token: empty after 2FA finalize") + } + + // Assert: Sessions.Create was called with data["roles"] containing + // ["super_admin"]. Before the fix this map was nil. + data := f.sessionCreator.lastData() + if data == nil { + t.Fatal("session data map is nil; expected roles to be stamped in") + } + rolesAny, ok := data["roles"] + if !ok { + t.Fatalf("session data missing 'roles' key: %#v", data) + } + roles, ok := rolesAny.([]string) + if !ok { + t.Fatalf("roles type: got %T, want []string", rolesAny) + } + if len(roles) != 1 || roles[0] != "super_admin" { + t.Errorf("roles: got %v, want [super_admin]", roles) + } +} + +// TestFinalizeTOTP_NilUserByIDDoesNotPanic confirms the documented +// fallback contract: when Deps.UserByID is unwired (zero value), the +// finalize path still completes — it just produces a session with no +// roles, the same degradation behavior as before the fix landed. This +// keeps existing tests that omit UserByID in their fixture from +// panicking on a nil indirection. +func TestFinalizeTOTP_NilUserByIDDoesNotPanic(t *testing.T) { + f := newFixture(t) + f.addUser(t, "u-1", "alice@example.com", "pwd", "active") + sec, err := totp.Generate("GoNext", "alice@example.com") + if err != nil { + t.Fatalf("totp.Generate: %v", err) + } + f.addTOTP(t, "u-1", sec.Base32) + // Leave f.deps.UserByID nil — that's the point of this test. + svc := f.service(t) + + first, err := svc.Authenticate(context.Background(), Input{ + Email: "alice@example.com", + Password: "pwd", + IP: "10.0.0.1", + }) + if err != nil || !first.RequiresTOTP { + t.Fatalf("first call: err=%v, RequiresTOTP=%v", err, first.RequiresTOTP) + } + + code, err := generateCurrentTOTP(sec.Base32) + if err != nil { + t.Fatalf("generate code: %v", err) + } + final, err := svc.Authenticate(context.Background(), Input{ + IntermediateToken: first.IntermediateToken, + TOTPCode: code, + IP: "10.0.0.1", + }) + if err != nil { + t.Fatalf("second call err: %v", err) + } + if final.Token == "" { + t.Error("Token: empty after 2FA finalize") + } + // Data map should be nil (no roles to stamp), which is the + // pre-fix degradation behavior — but the call must not panic. + if got := f.sessionCreator.lastData(); got != nil { + t.Errorf("session data: got %v, want nil (no UserByID → no roles)", got) + } +} + // generateCurrentTOTP returns the 6-digit TOTP code for the supplied // base32 secret at the current wall-clock time. Used by tests that // drive the 2FA finalize step. We delegate to pquerna/otp's diff --git a/apps/api/internal/rest/posts/handlers.go b/apps/api/internal/rest/posts/handlers.go index 0256294a..bc4f9d3a 100644 --- a/apps/api/internal/rest/posts/handlers.go +++ b/apps/api/internal/rest/posts/handlers.go @@ -156,6 +156,13 @@ func parseListQuery(r *http.Request) (ListFilter, error) { q := r.URL.Query() var f ListFilter f.Status = q.Get("status") + // `status=any` is a documented client convention meaning "all + // statuses" — the admin posts page uses it to render drafts + + // published in one list. Treat it as the absence of a status + // filter rather than a validation error. Empty string is the same. + if f.Status == "any" { + f.Status = "" + } if f.Status != "" { if _, ok := validStatuses[f.Status]; !ok { return f, validation{Code: "invalid_status", Detail: fmt.Sprintf("unknown status %q", f.Status)} diff --git a/apps/api/internal/rest/router/cursor.go b/apps/api/internal/rest/router/cursor.go index 4599df69..8aefe77e 100644 --- a/apps/api/internal/rest/router/cursor.go +++ b/apps/api/internal/rest/router/cursor.go @@ -2,6 +2,7 @@ package router import ( "encoding/base64" + "encoding/json" "errors" ) @@ -64,3 +65,23 @@ type Page[T any] struct { Data []T `json:"data"` Pagination PageInfo `json:"pagination"` } + +// MarshalJSON coerces a nil Data slice to an empty slice so the wire +// form is always `"data": []`, never `"data": null`. Admin Client +// Components map/spread/.length over `.data` without nil guards — a +// JSON null crashes them with "Cannot read properties of null". The +// inner pageOut struct (rather than `type pageAlias Page[T]`) is what +// avoids recursion: a generics-aware alias still carries the +// MarshalJSON method through the type parameter, so json.Marshal would +// loop forever. The inner struct has no methods and marshals via the +// default reflection path. +func (p Page[T]) MarshalJSON() ([]byte, error) { + if p.Data == nil { + p.Data = []T{} + } + type pageOut struct { + Data []T `json:"data"` + Pagination PageInfo `json:"pagination"` + } + return json.Marshal(pageOut{Data: p.Data, Pagination: p.Pagination}) +} diff --git a/apps/api/internal/rest/router/cursor_test.go b/apps/api/internal/rest/router/cursor_test.go index f0fa90e0..e262ebdc 100644 --- a/apps/api/internal/rest/router/cursor_test.go +++ b/apps/api/internal/rest/router/cursor_test.go @@ -1,7 +1,9 @@ package router import ( + "encoding/json" "errors" + "strings" "testing" ) @@ -68,3 +70,32 @@ func TestEncodeCursor_NoPadding(t *testing.T) { } } } + +func TestPage_MarshalJSON_NilDataBecomesEmptyArray(t *testing.T) { + t.Parallel() + + p := Page[string]{Data: nil, Pagination: PageInfo{}} + out, err := json.Marshal(p) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(out), `"data":[]`) { + t.Errorf("expected data:[], got: %s", string(out)) + } + if strings.Contains(string(out), `"data":null`) { + t.Errorf("expected no null, got: %s", string(out)) + } +} + +func TestPage_MarshalJSON_NonNilDataPreserved(t *testing.T) { + t.Parallel() + + p := Page[string]{Data: []string{"a", "b"}, Pagination: PageInfo{}} + out, err := json.Marshal(p) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(out), `"data":["a","b"]`) { + t.Errorf("expected data:[a,b], got: %s", string(out)) + } +} diff --git a/apps/api/internal/themes/static/doc.go b/apps/api/internal/themes/static/doc.go new file mode 100644 index 00000000..02e6f1c1 --- /dev/null +++ b/apps/api/internal/themes/static/doc.go @@ -0,0 +1,32 @@ +// Package static serves theme assets (style.css, JS, etc.) over HTTP +// directly from the on-disk theme directory. +// +// The route shape is: +// +// GET /themes/{slug}/{file...} +// +// where {slug} is either a real theme directory name (e.g. "gn-hello") +// or the virtual sentinel "active", which the package resolves through +// the supplied ActiveResolver against the live core.active_theme option. +// +// The handler is a thin static-file server with a few targeted hardenings: +// +// - It rejects any path containing ".." or an absolute prefix BEFORE +// touching the filesystem (path-traversal guard). +// - It refuses to serve files outside the configured ThemeDir even if +// a symlink inside a theme directory pointed elsewhere — every +// served path is rebuilt with filepath.Clean and re-checked against +// the canonical ThemeDir prefix. +// - It sets Content-Type from the file extension (css, js, woff2, +// png, etc.) rather than sniffing. +// - It sets Cache-Control: public, max-age=3600 so the public site + +// CDN can cache the response; theme installs bump the slug, which +// transparently busts the cached path. +// +// The package intentionally does NOT depend on the customizer or +// admin/themes packages — both expose the same options-table key but +// import paths there carry full read+write Store surfaces. The caller +// supplies a closure that performs whatever lookup it wants. In +// production main.go wires this against the existing PgxActiveStore; +// tests pass a literal closure. +package static diff --git a/apps/api/internal/themes/static/handler.go b/apps/api/internal/themes/static/handler.go new file mode 100644 index 00000000..796b1dd3 --- /dev/null +++ b/apps/api/internal/themes/static/handler.go @@ -0,0 +1,385 @@ +package static + +import ( + "errors" + "io" + "log/slog" + "mime" + "net/http" + "os" + "path" + "path/filepath" + "regexp" + "strconv" + "strings" +) + +// ActiveResolver returns the slug of the currently-active theme. The +// caller is expected to consult whatever store backs core.active_theme +// (the Postgres options row in production, an in-memory string in +// tests). Returning an empty string signals "no active theme" and the +// handler responds with 404 — this matches the contract of every other +// theme-aware surface in the codebase, which treats "no active theme" +// as a recoverable not-found rather than an internal error. +type ActiveResolver func() string + +// Deps is the dependency bag for Mount. ThemeDir and ActiveResolver +// are required; Logger falls back to slog.Default for tests that don't +// care about log capture. +type Deps struct { + // ThemeDir is the absolute or relative path that holds one + // subdirectory per installed theme. In production this is /themes + // in the api container (the GONEXT_THEME_DIR env var); on a dev + // host it defaults to "./themes" relative to the process CWD. + ThemeDir string + + // ActiveResolver maps the virtual "active" slug to the real slug. + // Production wires this against the options store; tests use a + // closure literal. + ActiveResolver ActiveResolver + + // Logger receives structured warnings (e.g. fallthrough opens that + // hit a missing file under a valid slug). Always non-nil after + // Mount applies its default. + Logger *slog.Logger +} + +func (d Deps) validate() error { + if strings.TrimSpace(d.ThemeDir) == "" { + return errors.New("themes/static: ThemeDir is required") + } + if d.ActiveResolver == nil { + return errors.New("themes/static: ActiveResolver is required") + } + return nil +} + +// slugPattern matches the kebab-case-lowercase rule used everywhere +// else in the theme stack (mirrors apps/api/internal/admin/themes +// /installer.go and packages/go/theme/validate.go). The static handler +// re-applies it as a belt-and-braces guard so a request that smuggled +// a slash-encoded path-traversal segment into the {slug} wildcard +// never reaches filepath.Join. The literal sentinel "active" is +// matched separately before this regex runs. +var slugPattern = regexp.MustCompile(`^[a-z][a-z0-9-]*[a-z0-9]$`) + +// cacheControl is set on every successful response. One hour is short +// enough that an operator who swaps the active theme sees the new CSS +// without waiting for a long-tail eviction; long enough that the +// public site doesn't re-fetch on every page navigation. The CDN / +// browser respects the slug→content coupling — a theme update bumps +// the seeded slug or rewrites the file in-place, in which case the +// next miss is the right cache key. +const cacheControl = "public, max-age=3600" + +// Mount wires the static-asset routes onto mux under base (typically +// "/themes"). Returns an error rather than panicking if Deps is +// malformed so the boot path can log and continue without taking the +// whole server down for a theme-server misconfiguration. +// +// The route tree: +// +// GET {base}/{slug}/{file...} — serve themeDir// +// +// The {slug} segment may be: +// - a real theme directory name (e.g. "gn-hello") — served as-is +// - the sentinel "active" — resolved via ActiveResolver +// +// Path-traversal segments in {file...} are rejected with 400. Files +// not on disk return 404. Read errors return 500. +func Mount(mux *http.ServeMux, base string, deps Deps) error { + if err := deps.validate(); err != nil { + return err + } + if deps.Logger == nil { + deps.Logger = slog.Default() + } + + base = strings.TrimRight(base, "/") + if base == "" { + return errors.New("themes/static: base must not be empty") + } + + // We resolve ThemeDir to its absolute, cleaned form once at Mount + // time so the per-request prefix check is a string compare on a + // stable canonical value. Production passes "/themes" which already + // is absolute; the dev default "./themes" gets resolved against + // the process CWD. If the directory does not yet exist (a fresh + // dev tree before `pnpm install` or a misconfigured volume mount) + // we still proceed — every request will simply 404 until the + // directory shows up. We deliberately do NOT block the route + // mount on a directory existence check so a temporary missing + // volume doesn't break the entire boot. + root, err := filepath.Abs(filepath.Clean(deps.ThemeDir)) + if err != nil { + return err + } + + h := &handler{ + root: root, + activeResolver: deps.ActiveResolver, + logger: deps.Logger.With(slog.String("subsystem", "themes/static")), + } + + // Single wildcard route. Go 1.22+ mux supports {file...} as a + // trailing wildcard that captures the remainder of the path + // verbatim. Note: we still re-validate the captured value because + // "the mux already split it" is not a substitute for "we checked + // the bytes." The {slug} segment is captured as a single segment; + // it cannot contain a slash by virtue of how the mux tokenises. + mux.Handle("GET "+base+"/{slug}/{file...}", http.HandlerFunc(h.serve)) + + return nil +} + +// handler is the resolved-Deps form passed around inside the package. +type handler struct { + // root is the absolute, cleaned path to the theme directory. Every + // served file must live under this prefix; we re-check after + // resolving the symlink-y filepath.Join just in case a symlink + // inside a theme directory points elsewhere. + root string + + // activeResolver maps the virtual "active" slug to the real slug. + activeResolver ActiveResolver + + // logger is namespaced with the subsystem key so structured-log + // consumers can filter on it. + logger *slog.Logger +} + +// serve handles a single GET request. The logic is small enough to +// inline; splitting it would obscure the security-relevant ordering. +func (h *handler) serve(w http.ResponseWriter, r *http.Request) { + // 1. Capture + validate the slug. + rawSlug := r.PathValue("slug") + slug := rawSlug + if slug == "active" { + slug = strings.TrimSpace(h.activeResolver()) + if slug == "" { + // No active theme configured. Surface as 404 rather than + // 500 — every theme-aware endpoint in the codebase treats + // "no active theme" as a recoverable not-found. + h.logger.Debug("active resolver returned empty; serving 404", + slog.String("path", r.URL.Path)) + http.NotFound(w, r) + return + } + } + + if !slugPattern.MatchString(slug) { + // Either the caller asked for a malformed slug directly or the + // active resolver produced one. Either way, 400 is the right + // signal: the request shape is invalid. + h.logger.Warn("invalid slug", + slog.String("requested", rawSlug), + slog.String("resolved", slug), + ) + http.Error(w, "invalid theme slug", http.StatusBadRequest) + return + } + + // 2. Capture + validate the file path. The mux gives us the + // remainder of the URL path AFTER the slug segment, with no + // leading slash. We forbid absolute paths, ".." segments, and + // empty file paths up front — these never reach the filesystem. + file := r.PathValue("file") + if !isSafeRelPath(file) { + h.logger.Warn("rejected path traversal attempt", + slog.String("slug", slug), + slog.String("file", file), + ) + http.Error(w, "invalid path", http.StatusBadRequest) + return + } + + // 3. Build the absolute on-disk path and verify it lives under root + // AFTER filepath.Clean has had its say. This catches any traversal + // we missed in isSafeRelPath as well as a symlink that points + // outside the theme directory. + abs := filepath.Join(h.root, slug, filepath.FromSlash(file)) + cleaned := filepath.Clean(abs) + if !isUnder(cleaned, h.root) { + h.logger.Warn("path escapes root after clean", + slog.String("slug", slug), + slog.String("file", file), + slog.String("cleaned", cleaned), + ) + http.Error(w, "invalid path", http.StatusBadRequest) + return + } + + // 4. Open + stat. We treat ENOENT as 404 and any other read error + // as 500. We intentionally do NOT distinguish "directory" from + // "missing file" — both surface as 404 because /themes/gn-hello + // (no file) is just as useless to the caller as /themes/missing + // /style.css. + f, err := os.Open(cleaned) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + http.NotFound(w, r) + return + } + h.logger.Error("open failed", + slog.String("path", cleaned), + slog.Any("err", err), + ) + http.Error(w, "read error", http.StatusInternalServerError) + return + } + defer func() { _ = f.Close() }() + + info, err := f.Stat() + if err != nil { + h.logger.Error("stat failed", + slog.String("path", cleaned), + slog.Any("err", err), + ) + http.Error(w, "stat error", http.StatusInternalServerError) + return + } + if info.IsDir() { + // We don't serve directory listings. A user landing on the + // directory URL almost certainly meant a missing file. + http.NotFound(w, r) + return + } + + // 5. Set headers. Content-Type from extension (we maintain a small + // allow-list for the common theme asset types; anything else + // falls back to mime.TypeByExtension which Go seeds from the + // system mime table). Cache-Control fixed to one hour. + w.Header().Set("Content-Type", contentType(cleaned)) + w.Header().Set("Cache-Control", cacheControl) + w.Header().Set("Content-Length", strconv.FormatInt(info.Size(), 10)) + + // 6. Stream. http.ServeContent would handle Range + ETag for us + // but the bookkeeping isn't worth it here — these are tiny files + // (style.css is single-digit KiB) and Range requests for theme + // CSS are not a real workload. A plain io.Copy is the right shape. + if r.Method == http.MethodHead { + return + } + if _, err := io.Copy(w, f); err != nil { + // Connection drop mid-stream. There's nothing useful to do + // here — headers are already on the wire. Log at debug so a + // flaky client doesn't spam error logs. + h.logger.Debug("copy failed (client likely disconnected)", + slog.String("path", cleaned), + slog.Any("err", err), + ) + } +} + +// isSafeRelPath reports whether p is a relative path with no traversal +// segments. We reject: +// - empty paths (the caller asked for /themes// with nothing +// after the trailing slash; mux delivers an empty {file...}) +// - absolute paths (leading slash) +// - any segment exactly equal to ".." (path.Clean would collapse +// "a/../b" to "b" but we want to surface the attempt explicitly) +// - any segment containing a NUL byte +// +// path.Clean on the input is checked separately — a clean path that +// still starts with ".." (e.g. "../../etc/passwd" which cleans to +// "../../etc/passwd") slips past per-segment checks but fails the +// "starts with .." check below. +func isSafeRelPath(p string) bool { + if p == "" { + return false + } + if strings.HasPrefix(p, "/") { + return false + } + if strings.ContainsRune(p, 0) { + return false + } + // path.Clean uses forward slashes (the URL path separator), which + // is what we want — the URL we received is forward-slashed even + // on Windows. + cleaned := path.Clean(p) + if cleaned == ".." || strings.HasPrefix(cleaned, "../") { + return false + } + // Defensive: catch a "..\\" form that path.Clean wouldn't notice + // because it doesn't treat backslash as a separator. We don't + // expect this from a stdlib mux but the cost is one substring + // search per request and the consequence of missing it would be + // a filesystem escape on Windows. + if strings.Contains(p, `..\`) || strings.Contains(p, `\..`) { + return false + } + for _, seg := range strings.Split(p, "/") { + if seg == ".." { + return false + } + } + return true +} + +// isUnder reports whether child resolves to a path under parent. +// Both arguments must already have been run through filepath.Clean. +// We add a trailing separator to the parent before the prefix check +// so "/themes" does not accept "/themes-other/...". +func isUnder(child, parent string) bool { + if child == parent { + return true + } + if !strings.HasSuffix(parent, string(filepath.Separator)) { + parent += string(filepath.Separator) + } + return strings.HasPrefix(child, parent) +} + +// contentType returns the MIME type for a given on-disk path. We +// hard-code the common theme asset extensions because Go's +// mime.TypeByExtension consults the system mime table — which is +// present on every distro we ship to, but isn't guaranteed inside +// the distroless final image. A small explicit table is the right +// trade for the asset types that actually ship with a theme. Falls +// back to TypeByExtension for everything else; final fallback is +// application/octet-stream. +func contentType(p string) string { + switch strings.ToLower(filepath.Ext(p)) { + case ".css": + return "text/css; charset=utf-8" + case ".js", ".mjs": + return "application/javascript; charset=utf-8" + case ".json": + return "application/json; charset=utf-8" + case ".map": + return "application/json; charset=utf-8" + case ".svg": + return "image/svg+xml" + case ".png": + return "image/png" + case ".jpg", ".jpeg": + return "image/jpeg" + case ".gif": + return "image/gif" + case ".webp": + return "image/webp" + case ".avif": + return "image/avif" + case ".ico": + return "image/x-icon" + case ".woff": + return "font/woff" + case ".woff2": + return "font/woff2" + case ".ttf": + return "font/ttf" + case ".otf": + return "font/otf" + case ".eot": + return "application/vnd.ms-fontobject" + case ".txt": + return "text/plain; charset=utf-8" + case ".html", ".htm": + return "text/html; charset=utf-8" + } + if mt := mime.TypeByExtension(filepath.Ext(p)); mt != "" { + return mt + } + return "application/octet-stream" +} diff --git a/apps/api/internal/themes/static/handler_test.go b/apps/api/internal/themes/static/handler_test.go new file mode 100644 index 00000000..88356580 --- /dev/null +++ b/apps/api/internal/themes/static/handler_test.go @@ -0,0 +1,355 @@ +package static + +import ( + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// newTestTree builds a temp theme directory laid out like the +// production volume mount: +// +// / +// gn-hello/ +// style.css +// parts/header.css +// theme.json +// gn-pro/ +// style.css +// +// Returns the absolute path to . +func newTestTree(t *testing.T) string { + t.Helper() + root := t.TempDir() + for _, dir := range []string{"gn-hello", "gn-hello/parts", "gn-pro"} { + if err := os.MkdirAll(filepath.Join(root, dir), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", dir, err) + } + } + writeFile := func(rel, body string) { + t.Helper() + if err := os.WriteFile(filepath.Join(root, rel), []byte(body), 0o644); err != nil { + t.Fatalf("write %s: %v", rel, err) + } + } + writeFile("gn-hello/style.css", ":root { --paper: #F5F2EA; }\n") + writeFile("gn-hello/parts/header.css", "header { padding: 1rem; }\n") + writeFile("gn-hello/theme.json", `{"slug":"gn-hello"}`) + writeFile("gn-pro/style.css", ":root { --paper: #ffffff; }\n") + return root +} + +// mountTest builds a mux + serve test rig for the static handler. +// activeSlug is the value returned by the ActiveResolver closure on +// every call; tests that need a varying resolver build their own. +func mountTest(t *testing.T, themeDir, activeSlug string) http.Handler { + t.Helper() + mux := http.NewServeMux() + if err := Mount(mux, "/themes", Deps{ + ThemeDir: themeDir, + ActiveResolver: func() string { return activeSlug }, + }); err != nil { + t.Fatalf("Mount: %v", err) + } + return mux +} + +func TestServe_RealFile_ReturnsCSS(t *testing.T) { + root := newTestTree(t) + h := mountTest(t, root, "gn-hello") + + req := httptest.NewRequest(http.MethodGet, "/themes/gn-hello/style.css", nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status: got %d, want 200", rec.Code) + } + if got := rec.Header().Get("Content-Type"); got != "text/css; charset=utf-8" { + t.Errorf("Content-Type: got %q, want text/css; charset=utf-8", got) + } + if got := rec.Header().Get("Cache-Control"); got != "public, max-age=3600" { + t.Errorf("Cache-Control: got %q, want public, max-age=3600", got) + } + body, _ := io.ReadAll(rec.Body) + if !strings.Contains(string(body), "--paper: #F5F2EA") { + t.Errorf("body: missing expected CSS, got %q", string(body)) + } +} + +func TestServe_NestedFile_ReturnsCSS(t *testing.T) { + root := newTestTree(t) + h := mountTest(t, root, "gn-hello") + + req := httptest.NewRequest(http.MethodGet, "/themes/gn-hello/parts/header.css", nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status: got %d, want 200", rec.Code) + } + if got := rec.Header().Get("Content-Type"); got != "text/css; charset=utf-8" { + t.Errorf("Content-Type: got %q, want text/css; charset=utf-8", got) + } +} + +func TestServe_HeadRequestSendsHeadersOnly(t *testing.T) { + root := newTestTree(t) + h := mountTest(t, root, "gn-hello") + + // The handler is mounted with "GET" — but Go's mux treats HEAD as + // an alias for GET when no explicit HEAD handler is registered, so + // the request reaches our code with r.Method == "HEAD" and we + // short-circuit the body copy. The mux behavior is a stdlib + // detail; we exercise our short-circuit explicitly. + req := httptest.NewRequest(http.MethodGet, "/themes/gn-hello/style.css", nil) + req.Method = http.MethodHead + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + // Some mux versions reject HEAD outright when only GET is + // registered. Accept either 200 (the short-circuit fired) or 405 + // (mux refused) — what we care about is that no body is leaked + // when the handler does run. + if rec.Code == http.StatusOK && rec.Body.Len() != 0 { + t.Errorf("HEAD: body should be empty, got %q", rec.Body.String()) + } +} + +func TestServe_PathTraversalRejected(t *testing.T) { + root := newTestTree(t) + h := mountTest(t, root, "gn-hello") + + cases := []struct { + name string + path string + }{ + // Slash-encoded traversal inside the {file...} segment. + {"dotdot_in_file", "/themes/gn-hello/../../etc/passwd"}, + // Traversal at the head of {file...}. + {"dotdot_head", "/themes/gn-hello/..%2Fetc%2Fpasswd"}, + // Backslash flavour (Windows-shaped). We reject pre-clean. + {"backslash_traversal", `/themes/gn-hello/..\etc\passwd`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, tc.path, nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + // At least one of {400, 404} is acceptable depending on + // whether the mux pre-canonicalised the path. What we + // care about is that we did NOT return 200 — i.e. the + // attacker did not get a file out of us. + if rec.Code == http.StatusOK { + t.Errorf("path %q: served 200 (escape!) body=%q", tc.path, rec.Body.String()) + } + }) + } +} + +func TestServe_DotDotSegmentExplicitlyRejected(t *testing.T) { + // Hit the handler directly (bypass mux url-canonicalisation) with + // a {file...} value that contains ".." literally. This exercises + // the isSafeRelPath guard rather than relying on the mux's + // behavior — the docs explicitly require 400 on ".." in path. + root := newTestTree(t) + mux := http.NewServeMux() + if err := Mount(mux, "/themes", Deps{ + ThemeDir: root, + ActiveResolver: func() string { return "gn-hello" }, + }); err != nil { + t.Fatalf("Mount: %v", err) + } + + // Use a request URL the mux WILL route to our handler (no + // canonical-redirect): /themes/gn-hello/ where + // contains a single ".." segment. + req := httptest.NewRequest(http.MethodGet, "/themes/gn-hello/sub", nil) + req.URL.RawPath = "/themes/gn-hello/sub/..%2Fother.css" + req.URL.Path = "/themes/gn-hello/sub/../other.css" + + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + + // Either the mux canonicalised the path away (in which case it + // 301s) or our handler ran and surfaced 400. Accept either, but + // not 200. + if rec.Code == http.StatusOK { + t.Errorf("expected non-200, got 200, body=%q", rec.Body.String()) + } +} + +func TestServe_ActiveResolverWiredThrough(t *testing.T) { + root := newTestTree(t) + + // The resolver returns gn-hello on first call, gn-pro on second. + // The handler should re-invoke it per request, so we end up + // reading from a different theme directory on each call. + calls := 0 + mux := http.NewServeMux() + if err := Mount(mux, "/themes", Deps{ + ThemeDir: root, + ActiveResolver: func() string { + calls++ + if calls == 1 { + return "gn-hello" + } + return "gn-pro" + }, + }); err != nil { + t.Fatalf("Mount: %v", err) + } + + // First request: /themes/active/style.css resolves to gn-hello. + req1 := httptest.NewRequest(http.MethodGet, "/themes/active/style.css", nil) + rec1 := httptest.NewRecorder() + mux.ServeHTTP(rec1, req1) + if rec1.Code != http.StatusOK { + t.Fatalf("first request: status %d, want 200, body=%q", rec1.Code, rec1.Body.String()) + } + if !strings.Contains(rec1.Body.String(), "#F5F2EA") { + t.Errorf("first request: expected gn-hello CSS, got %q", rec1.Body.String()) + } + + // Second request: same URL, resolver now returns gn-pro. + req2 := httptest.NewRequest(http.MethodGet, "/themes/active/style.css", nil) + rec2 := httptest.NewRecorder() + mux.ServeHTTP(rec2, req2) + if rec2.Code != http.StatusOK { + t.Fatalf("second request: status %d, want 200, body=%q", rec2.Code, rec2.Body.String()) + } + if !strings.Contains(rec2.Body.String(), "#ffffff") { + t.Errorf("second request: expected gn-pro CSS, got %q", rec2.Body.String()) + } + + if calls != 2 { + t.Errorf("resolver call count: got %d, want 2", calls) + } +} + +func TestServe_ActiveResolverEmpty404(t *testing.T) { + root := newTestTree(t) + mux := http.NewServeMux() + if err := Mount(mux, "/themes", Deps{ + ThemeDir: root, + ActiveResolver: func() string { return "" }, + }); err != nil { + t.Fatalf("Mount: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/themes/active/style.css", nil) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusNotFound { + t.Errorf("status: got %d, want 404", rec.Code) + } +} + +func TestServe_MissingFile404(t *testing.T) { + root := newTestTree(t) + h := mountTest(t, root, "gn-hello") + + req := httptest.NewRequest(http.MethodGet, "/themes/gn-hello/does-not-exist.css", nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + if rec.Code != http.StatusNotFound { + t.Errorf("status: got %d, want 404", rec.Code) + } +} + +func TestServe_DirectoryReturns404(t *testing.T) { + root := newTestTree(t) + h := mountTest(t, root, "gn-hello") + + // /themes/gn-hello/parts (no trailing file) — the path resolves + // to a directory on disk. We never serve directory listings. + // The mux may or may not route a no-file path to our handler + // depending on how the {file...} wildcard treats an empty + // remainder; if it 404s before us, that's also fine. + req := httptest.NewRequest(http.MethodGet, "/themes/gn-hello/parts/", nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + if rec.Code == http.StatusOK { + t.Errorf("expected non-200, got 200 body=%q", rec.Body.String()) + } +} + +func TestServe_InvalidSlugRejected(t *testing.T) { + root := newTestTree(t) + h := mountTest(t, root, "gn-hello") + + // Slug with characters outside the kebab pattern: "GnHello" is + // rejected because the regex requires leading lowercase. + req := httptest.NewRequest(http.MethodGet, "/themes/GnHello/style.css", nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest { + t.Errorf("status: got %d, want 400", rec.Code) + } +} + +func TestMount_RequiresThemeDir(t *testing.T) { + mux := http.NewServeMux() + err := Mount(mux, "/themes", Deps{ + ActiveResolver: func() string { return "" }, + }) + if err == nil { + t.Fatalf("expected error when ThemeDir is empty") + } +} + +func TestMount_RequiresActiveResolver(t *testing.T) { + mux := http.NewServeMux() + err := Mount(mux, "/themes", Deps{ + ThemeDir: t.TempDir(), + }) + if err == nil { + t.Fatalf("expected error when ActiveResolver is nil") + } +} + +func TestIsSafeRelPath(t *testing.T) { + cases := []struct { + in string + ok bool + }{ + {"style.css", true}, + {"parts/header.css", true}, + {"a/b/c.css", true}, + {"", false}, + {"/abs/path.css", false}, + {"..", false}, + {"../etc/passwd", false}, + {"sub/../etc/passwd", false}, + {`sub\..\etc\passwd`, false}, + {"sub/with\x00null.css", false}, + } + for _, tc := range cases { + got := isSafeRelPath(tc.in) + if got != tc.ok { + t.Errorf("isSafeRelPath(%q) = %v, want %v", tc.in, got, tc.ok) + } + } +} + +func TestContentType(t *testing.T) { + cases := map[string]string{ + "x.css": "text/css; charset=utf-8", + "x.js": "application/javascript; charset=utf-8", + "x.mjs": "application/javascript; charset=utf-8", + "x.json": "application/json; charset=utf-8", + "x.svg": "image/svg+xml", + "x.woff2": "font/woff2", + "x.png": "image/png", + } + for in, want := range cases { + if got := contentType(in); got != want { + t.Errorf("contentType(%q) = %q, want %q", in, got, want) + } + } +} diff --git a/apps/web/src/app/layout.tsx b/apps/web/src/app/layout.tsx index 17221df3..8c3c5bcf 100644 --- a/apps/web/src/app/layout.tsx +++ b/apps/web/src/app/layout.tsx @@ -36,6 +36,17 @@ import { } from 'next/font/google'; import './globals.css'; +// Public-site API base — same env var the typed client in src/lib/api.ts +// reads. We resolve it at module init (the value is baked into the +// build because NEXT_PUBLIC_* is browser-visible) so the tag we +// inject in renders with the right absolute URL. The link +// preloads the active theme's CSS so the renderer no longer paints +// with stock globals.css defaults — the API now serves +// /themes/active/style.css (issue #501). +const PUBLIC_API_URL: string = + (typeof process !== 'undefined' && process.env.NEXT_PUBLIC_API_URL) || + 'http://localhost:8080'; + const archivo = Archivo({ subsets: ['latin'], weight: ['500', '600', '700', '800', '900'], @@ -95,6 +106,19 @@ export default function RootLayout({ return ( + + {/* + * Active-theme CSS preload (issue #501). The API serves the + * theme's static stylesheet under /themes/active/style.css — + * the "active" sentinel resolves through the core.active_theme + * options row, so switching the active theme transparently + * swaps the bytes returned here. + */} + + {children} ); diff --git a/apps/worker/cmd/worker/main.go b/apps/worker/cmd/worker/main.go index 3b574bf8..f64e379a 100644 --- a/apps/worker/cmd/worker/main.go +++ b/apps/worker/cmd/worker/main.go @@ -259,6 +259,16 @@ func run(ctx context.Context) error { // separate cron-leader goroutine (issue #258); this main wiring // only declares the spec + schedule so the leader has something // to fire. + // + // cronReg is declared unconditionally so that downstream + // registrations (content publisher + GC via SeedDefaults) can + // still happen even if media storage init fails. We build a fresh + // registry per process — the cron package deliberately does not + // ship a Default() singleton; ownership is the binary's. A + // follow-up issue mounts the scheduler against this registry once + // the cron-leader lease (#258) lands. + cronReg := cron.NewRegistry() + mediaDriver, err := storage.New(ctx, storage.Options{ S3: storage.S3Config{ Endpoint: cfg.Storage.Endpoint, @@ -294,16 +304,7 @@ func run(ctx context.Context) error { "err", err.Error(), ) } - // Dispatch onto the asynq mux so the consumer side runs - // the handler when the leader enqueues it. - taskspec.Dispatch(srv.Mux(), reg) // Cron-side registration: declare WHEN the task fires. - // We build a fresh registry per process — the cron - // package deliberately does not ship a Default() - // singleton; ownership is the binary's. A follow-up - // issue mounts the scheduler against this registry once - // the cron-leader lease (#258) lands. - cronReg := cron.NewRegistry() cronSpec := storage.NewAbortOrphansCron() if err := cronReg.Register(cronSpec); err != nil && !errors.Is(err, cron.ErrAlreadyRegistered) { @@ -311,41 +312,54 @@ func run(ctx context.Context) error { "err", err.Error(), ) } - _ = cronReg // pinned for the scheduler hookup below logger.Info("storage abort-orphans cron registered", "task", storage.AbortOrphansTaskName, "cron", storage.AbortOrphansCronName, "schedule", storage.AbortOrphansSchedule, ) - - // Content scheduler + GC (#143). Both tasks share the - // worker's pgx pool. The publisher fires every minute - // and flips status=scheduled rows whose scheduled_for - // has elapsed; the GC fires daily at 03:30 UTC and - // hard-deletes trash older than 30 days. - // - // We register against the same cronReg as the storage - // sweep so the eventual cron-leader (#258) sees one - // merged schedule, and against taskspec.Default() so - // the asynq mux dispatch already in place handles the - // task type. - if err := scheduler.SeedDefaults(taskspec.Default(), cronReg, scheduler.SeedOptions{ - Pool: pool, - Logger: logger, - }); err != nil { - logger.Warn("scheduler seed failed", "err", err.Error()) - } else { - taskspec.Dispatch(srv.Mux(), taskspec.Default()) - logger.Info("content scheduler + gc registered", - "publisher_task", scheduler.PublisherTaskName, - "publisher_schedule", scheduler.PublisherSchedule, - "gc_task", scheduler.GCTaskName, - "gc_schedule", scheduler.GCSchedule, - ) - } } } + // Content scheduler + GC (#143). Both tasks share the worker's + // pgx pool. The publisher fires every minute and flips + // status=scheduled rows whose scheduled_for has elapsed; the GC + // fires daily at 03:30 UTC and hard-deletes trash older than + // 30 days. + // + // We register against the same cronReg as the storage sweep so + // the eventual cron-leader (#258) sees one merged schedule, and + // against taskspec.Default() so the asynq mux dispatch already in + // place handles the task type. SeedDefaults only needs pool + + // cronReg, so this runs even if media storage init failed above. + if err := scheduler.SeedDefaults(taskspec.Default(), cronReg, scheduler.SeedOptions{ + Pool: pool, + Logger: logger, + }); err != nil { + logger.Warn("scheduler seed failed", "err", err.Error()) + } else { + logger.Info("content scheduler + gc registered", + "publisher_task", scheduler.PublisherTaskName, + "publisher_schedule", scheduler.PublisherSchedule, + "gc_task", scheduler.GCTaskName, + "gc_schedule", scheduler.GCSchedule, + ) + } + + // Single Dispatch after every Register attempt. Whatever specs + // landed in taskspec.Default() — abort-orphans (if storage init + // succeeded), publisher + GC (if SeedDefaults succeeded), and + // the media tasks registered via workermedia.Register above — + // get wired onto the asynq mux here. Failed Register attempts + // already logged their warnings; their absence from the registry + // just means the mux has no handler for that task type and asynq + // will NACK it cleanly. + // + // asynq.ServeMux.Handle panics if the same task type is registered + // twice. There is exactly ONE Dispatch call in this binary, and it + // lives here — do not introduce another. + taskspec.Dispatch(srv.Mux(), taskspec.Default()) + _ = cronReg // pinned for the cron-leader scheduler hookup (#258) + logger.Info("worker started", "drain_budget", workerShutdownBudget, "redis_url_present", cfg.Redis.URL != "", diff --git a/apps/worker/go.mod b/apps/worker/go.mod index 8c1c61b5..afd5e917 100644 --- a/apps/worker/go.mod +++ b/apps/worker/go.mod @@ -14,6 +14,10 @@ require ( github.com/getsentry/sentry-go v0.43.0 // indirect github.com/go-ini/ini v1.67.0 // indirect github.com/google/uuid v1.6.0 // indirect + github.com/jackc/pgpassfile v1.0.0 // indirect + github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect + github.com/jackc/pgx/v5 v5.9.2 // indirect + github.com/jackc/puddle/v2 v2.2.2 // indirect github.com/klauspost/compress v1.18.5 // indirect github.com/klauspost/cpuid/v2 v2.2.11 // indirect github.com/klauspost/crc32 v1.3.0 // indirect @@ -39,6 +43,7 @@ require ( go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/crypto v0.52.0 // indirect golang.org/x/net v0.54.0 // indirect + golang.org/x/sync v0.20.0 // indirect golang.org/x/sys v0.45.0 // indirect golang.org/x/text v0.37.0 // indirect golang.org/x/time v0.14.0 // indirect diff --git a/apps/worker/go.sum b/apps/worker/go.sum index 75c25610..26f9a39e 100644 --- a/apps/worker/go.sum +++ b/apps/worker/go.sum @@ -26,6 +26,7 @@ github.com/containerd/platforms v0.2.1 h1:zvwtM3rz2YHPQsF2CHYM8+KtB5dvhISiXh5ZpS github.com/containerd/platforms v0.2.1/go.mod h1:XHCb+2/hzowdiut9rkudds9bE5yJ7npe7dG/wG+uFPw= github.com/cpuguy83/dockercfg v0.3.2 h1:DlJTyZGBDlXqUZ2Dk2Q3xHs/FtnooJJVaad2S9GKorA= github.com/cpuguy83/dockercfg v0.3.2/go.mod h1:sugsbF4//dDlL/i+S+rtpIWp+5h0BHJHfjj5/jFyUJc= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= @@ -60,6 +61,14 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/hibiken/asynq v0.26.0 h1:1Zxr92MlDnb1Zt/QR5g2vSCqUS03i95lUfqx5X7/wrw= github.com/hibiken/asynq v0.26.0/go.mod h1:Qk4e57bTnWDoyJ67VkchuV6VzSM9IQW2nPvAGuDyw58= +github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM= +github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg= +github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo= +github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM= +github.com/jackc/pgx/v5 v5.9.2 h1:3ZhOzMWnR4yJ+RW1XImIPsD1aNSz4T4fyP7zlQb56hw= +github.com/jackc/pgx/v5 v5.9.2/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4= +github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo= +github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= github.com/klauspost/compress v1.18.5 h1:/h1gH5Ce+VWNLSWqPzOVn6XBO+vJbCNGvjoaGBFW2IE= github.com/klauspost/compress v1.18.5/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/klauspost/cpuid/v2 v2.0.1/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= @@ -115,6 +124,7 @@ github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4 github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 h1:o4JXh1EVt9k/+g42oCprj/FisM4qX9L3sZB3upGN2ZU= @@ -143,6 +153,9 @@ github.com/sirupsen/logrus v1.9.4 h1:TsZE7l11zFCLZnZ+teH4Umoq5BhEIfIzfRDZ1Uzql2w github.com/sirupsen/logrus v1.9.4/go.mod h1:ftWc9WdOfJ0a92nsE2jF5u5ZwH8Bv2zdeOC42RjbV2g= github.com/spf13/cast v1.10.0 h1:h2x0u2shc1QuLHfxi+cTJvs30+ZAHOGRic8uyGTDWxY= github.com/spf13/cast v1.10.0/go.mod h1:jNfB8QC9IA6ZuY2ZjDp0KtFO2LZZlg4S/7bzP6qqeHo= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/testcontainers/testcontainers-go v0.42.0 h1:He3IhTzTZOygSXLJPMX7n44XtK+qhjat1nI9cneBbUY= @@ -189,6 +202,8 @@ golang.org/x/crypto v0.52.0 h1:RMs7fP2rXdep0CftQlK8Uf+kibLm7qkCcradZWYz988= golang.org/x/crypto v0.52.0/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGbc= golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= @@ -200,5 +215,6 @@ google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/cli/gonext/cmd/audit/audit.go b/cli/gonext/cmd/audit/audit.go index c5631612..873f3883 100644 --- a/cli/gonext/cmd/audit/audit.go +++ b/cli/gonext/cmd/audit/audit.go @@ -27,6 +27,8 @@ func Run(args []string, stdout, stderr io.Writer) int { return ExitOK case "tail": return runTail(args[1:], stdout, stderr) + case "verify": + return runVerify(args[1:], stdout, stderr) default: fmt.Fprintf(stderr, "gonext audit: unknown subcommand %q\n\n%s\n", args[0], usage) return ExitUsage @@ -42,9 +44,11 @@ Usage: gonext audit [args] Subcommands: - tail [flags] Print the most recent audit events. Tail by default. + tail [flags] Print the most recent audit events. + verify [--from] [--to] Walk the HMAC chain and report tampered rows. Run 'gonext audit tail --help' for the tail-specific flags. Environment: - DATABASE_URL Required. Postgres DSN for the GoNext install.` + DATABASE_URL Required. Postgres DSN for the GoNext install. + GONEXT_AUDIT_HMAC_KEY Required for ` + "`verify`" + `. HMAC chain key.` diff --git a/cli/gonext/cmd/audit/verify.go b/cli/gonext/cmd/audit/verify.go index 54a0a93b..073a7082 100644 --- a/cli/gonext/cmd/audit/verify.go +++ b/cli/gonext/cmd/audit/verify.go @@ -1,9 +1,10 @@ -// Package audit provides the `gonext audit` CLI subcommand surface. -// At present it exposes a single `verify` subcommand that walks the -// audit_log HMAC chain and reports the first tampered row (if any). +// Package audit — `verify` subcommand implementation. The shared +// entry point (Run, RunOS, exit codes, usage banner) lives in +// audit.go; this file contributes only the verify-specific +// subcommand handler that the dispatcher calls into. // -// Wiring: cli/gonext/main.go dispatches `audit ` here via -// RunOS. The package is intentionally thin — heavy lifting lives in +// Verify walks the audit_log HMAC chain and reports the first +// tampered row (if any). Heavy lifting lives in // packages/go/audit.VerifyChain. package audit @@ -13,7 +14,6 @@ import ( "flag" "fmt" "io" - "os" "github.com/jackc/pgx/v5/pgxpool" @@ -21,57 +21,15 @@ import ( "github.com/Singleton-Solution/GoNext/packages/go/config" ) -// Exit codes shared with the rest of the CLI. -const ( - ExitOK = 0 - ExitFail = 1 - ExitUsage = 2 -) - -const usage = `gonext audit — inspect the audit log - -Usage: - gonext audit verify [--from=ID] [--to=ID] - -Subcommands: - verify Walk the HMAC chain and report any tampered rows. - -Environment: - DATABASE_URL Required. Postgres DSN. - GONEXT_AUDIT_HMAC_KEY Required. The HMAC chain key (raw bytes or hex). - See packages/go/audit/chain.go. - -Exit codes: - 0 Chain verified intact. - 1 Chain broken (a tamper was detected) or another runtime error. - 2 Misuse: missing subcommand, malformed flags, etc. -` - -// RunOS is the os.Args / os.Stdout / os.Stderr entry point dispatched -// from cli/gonext/main.go. Returns the process exit code. -func RunOS(args []string) int { - return Run(args, os.Stdout, os.Stderr) -} - -// Run is the testable entry point: callers pass their own writers and -// args so the package can be exercised without poking at os.Args. -func Run(args []string, stdout, stderr io.Writer) int { - if len(args) == 0 { - fmt.Fprint(stderr, usage) - return ExitUsage - } - switch args[0] { - case "verify": - return runVerify(args[1:], stdout, stderr) - case "help", "--help", "-h": - fmt.Fprint(stdout, usage) - return ExitOK - default: - fmt.Fprintf(stderr, "gonext audit: unknown subcommand %q\n\n%s\n", args[0], usage) - return ExitUsage - } -} - +// runVerify handles `gonext audit verify`. Returns the desired +// process exit code (one of the Exit* constants from audit.go). +// +// Flag set: +// --from=ID inclusive lower-bound row ID, default first row +// --to=ID inclusive upper-bound row ID, default last row +// +// Required env: DATABASE_URL, GONEXT_AUDIT_HMAC_KEY (see +// packages/go/audit/chain.go for the key format). func runVerify(args []string, stdout, stderr io.Writer) int { fs := flag.NewFlagSet("verify", flag.ContinueOnError) fs.SetOutput(stderr) diff --git a/docker-compose.override.example.yml b/docker-compose.override.example.yml new file mode 100644 index 00000000..b221c77e --- /dev/null +++ b/docker-compose.override.example.yml @@ -0,0 +1,97 @@ +# docker-compose.override.example.yml +# +# Template for the personal docker-compose.override.yml that Compose +# auto-loads alongside docker-compose.yml + docker-compose.dev.yml. The +# live override is gitignored so each contributor can tweak ports and +# secrets without polluting the repo. +# +# Usage: +# cp docker-compose.override.example.yml docker-compose.override.yml +# # (optional) edit GONEXT_AUTH_* secrets — the placeholders below are +# # dev-only and safe to keep as-is for a laptop stack. +# docker compose up -d +# +# What this file does: +# - Remaps the Postgres host port to 5433 so a native Postgres on 5432 +# keeps working. +# - Builds the admin image with NEXT_PUBLIC_API_URL="" (same-origin +# proxy via Next.js rewrites) and bakes the docker-internal API URL +# into the rewrite destination at build time. +# - Pins matching GONEXT_AUTH_* secrets across api/worker/migrate so +# sessions issued by api validate in worker and migrate uses the same +# pepper when it bootstraps users. + +services: + # Republish Postgres on 5433 to avoid clashing with a native Postgres + # running on the host's 5432. Connect IDEs / `psql` to localhost:5433. + postgres: + ports: + - "5433:5432" + + # Admin (Next.js, :3001) proxies /api/* through its own rewrite to + # avoid CORS. NEXT_PUBLIC_API_URL must be "" so the client uses + # same-origin paths; GONEXT_API_URL points the build-time rewrite at + # the docker-internal api service. The entrypoint override runs the + # standalone server.js directly (the upstream image's CMD is wrong + # for the standalone output layout). + admin: + build: + args: + NEXT_PUBLIC_API_URL: "" + # Build-time arg: rewrites() evaluates at build time and bakes + # this URL into routes-manifest.json. Must be the docker-internal + # api hostname so server-side proxy hops work. + GONEXT_API_URL: http://api:8080 + environment: + NEXT_PUBLIC_API_URL: "" + GONEXT_API_URL: http://api:8080 + entrypoint: ["node"] + command: ["/app/apps/admin/.next/standalone/apps/admin/server.js"] + + # Public web (Next.js, :3000) runs cross-origin in dev — the browser + # hits the API directly on :8080, so NEXT_PUBLIC_API_URL must be the + # host-reachable URL. Server-side code inside the container still + # talks to api:8080 via GONEXT_API_URL. + web: + build: + args: + NEXT_PUBLIC_API_URL: http://localhost:8080 + environment: + NEXT_PUBLIC_API_URL: http://localhost:8080 + GONEXT_API_URL: http://api:8080 + + # api/worker/migrate share the same auth secrets so sessions issued by + # one service validate in the others, and migrate's `gonext init` uses + # the same pepper that api will use to verify passwords later. + # + # The values below are PLACEHOLDERS — they look like 32-byte base64 + # strings but are sentinel text, NOT real entropy. Either keep them + # (dev-only, fine on a laptop) or replace with real 32-byte base64 + # values, e.g.: + # openssl rand -base64 32 + # If you regenerate, you MUST update all three services to match, and + # wipe the Postgres volume (`docker compose down -v`) if you change + # GONEXT_AUTH_PEPPER on an existing DB — every previously-hashed + # password becomes unverifiable. + api: + environment: + GONEXT_AUTH_PEPPER: REPLACE_WITH_RANDOM_32_BYTES_BASE64_ENCODED___SAMPLE_DEV_ONLY_AAAA== + GONEXT_AUTH_SESSION_SECRET: REPLACE_WITH_RANDOM_32_BYTES_BASE64_ENCODED___SAMPLE_DEV_ONLY_BBBB== + GONEXT_AUTH_CSRF_SECRET: REPLACE_WITH_RANDOM_32_BYTES_BASE64_ENCODED___SAMPLE_DEV_ONLY_CCCC== + + # Worker consumes Asynq jobs queued by api; if its session secret + # drifts from api's, signed payloads stop verifying. + worker: + environment: + GONEXT_AUTH_PEPPER: REPLACE_WITH_RANDOM_32_BYTES_BASE64_ENCODED___SAMPLE_DEV_ONLY_AAAA== + GONEXT_AUTH_SESSION_SECRET: REPLACE_WITH_RANDOM_32_BYTES_BASE64_ENCODED___SAMPLE_DEV_ONLY_BBBB== + GONEXT_AUTH_CSRF_SECRET: REPLACE_WITH_RANDOM_32_BYTES_BASE64_ENCODED___SAMPLE_DEV_ONLY_CCCC== + + # Migrate is a one-shot that runs `gonext init` to bootstrap the first + # admin user. It must hash the admin password with the same pepper + # that api will later HMAC into login attempts. + migrate: + environment: + GONEXT_AUTH_PEPPER: REPLACE_WITH_RANDOM_32_BYTES_BASE64_ENCODED___SAMPLE_DEV_ONLY_AAAA== + GONEXT_AUTH_SESSION_SECRET: REPLACE_WITH_RANDOM_32_BYTES_BASE64_ENCODED___SAMPLE_DEV_ONLY_BBBB== + GONEXT_AUTH_CSRF_SECRET: REPLACE_WITH_RANDOM_32_BYTES_BASE64_ENCODED___SAMPLE_DEV_ONLY_CCCC== diff --git a/packages/go/settings/postgres.go b/packages/go/settings/postgres.go index a9328f49..c5d111db 100644 --- a/packages/go/settings/postgres.go +++ b/packages/go/settings/postgres.go @@ -70,13 +70,21 @@ func (t tagWrapper) RowsAffected() int64 { return t.rows } // docs/01-core-cms.md §10.11: // // CREATE TABLE options ( -// key TEXT PRIMARY KEY, +// key CITEXT PRIMARY KEY, // value JSONB NOT NULL, // autoload BOOLEAN NOT NULL DEFAULT FALSE, -// namespace TEXT NOT NULL DEFAULT 'core', -// updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +// is_protected BOOLEAN NOT NULL DEFAULT FALSE, +// created_at TIMESTAMPTZ NOT NULL DEFAULT now(), +// updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), +// version INTEGER NOT NULL DEFAULT 1 // ); // +// The actual migration is 000008. Earlier drafts of this doc + the +// upsertSQL referenced a `namespace` column that never made it into +// the migration; the writer was 500'ing in production until the +// non-existent column was stripped. See `namespaceFor` below for +// the un-shipped per-plugin uninstall convention. +// // The store maintains an L1 cache (a sync.Map keyed by setting key) // so the hot Read path doesn't round-trip to Postgres. Write // invalidates the relevant entry as part of the persistence path — @@ -130,18 +138,21 @@ func NewPostgresStoreWithQuerier(q PgxQuerier, reg *Registry) *PostgresStore { // read path doesn't need it. const readSQL = `SELECT value FROM options WHERE key = $1` -// upsertSQL writes a value, defaulting the namespace and autoload -// columns from the registry. Postgres's ON CONFLICT lets us keep this -// single statement instead of branching SELECT-then-INSERT-or-UPDATE -// on the caller side, which also avoids the lost-update race window. +// upsertSQL writes a value with its autoload bit from the registry. +// Postgres's ON CONFLICT lets us keep this single statement instead +// of branching SELECT-then-INSERT-or-UPDATE on the caller side, which +// also avoids the lost-update race window. +// +// updated_at is intentionally omitted from the INSERT column list: +// the options_touch trigger (migration 000008) sets it on every +// UPDATE, and the column default (now()) covers the INSERT case. +// Setting it explicitly here would still work but is dead code. const upsertSQL = ` -INSERT INTO options (key, value, autoload, namespace, updated_at) -VALUES ($1, $2, $3, $4, now()) +INSERT INTO options (key, value, autoload) +VALUES ($1, $2, $3) ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, - autoload = EXCLUDED.autoload, - namespace = EXCLUDED.namespace, - updated_at = now() + autoload = EXCLUDED.autoload ` // autoloadSQL fetches every key with autoload = true. The boot path @@ -207,8 +218,7 @@ func (s *PostgresStore) Write(ctx context.Context, key string, value any) error return fmt.Errorf("settings: marshal %q: %w", key, err) } - namespace := namespaceFor(key) - tag, err := s.db.Exec(ctx, upsertSQL, key, encoded, entry.Setting.Autoload, namespace) + tag, err := s.db.Exec(ctx, upsertSQL, key, encoded, entry.Setting.Autoload) if err != nil { // On Exec failure, the row may or may not have been written. // We invalidate the cache rather than seed it — the operator @@ -375,11 +385,19 @@ func (s *PostgresStore) cacheGet(key string) (any, bool) { return v, ok } -// namespaceFor returns the namespace column value for a setting key. +// namespaceFor was used to populate the `options.namespace` column. +// That column does not exist in migration 000008 (the live schema); +// the column reference was unreviewed prose that shipped as code. +// The helper is intentionally retained — when the per-plugin uninstall +// sweep ships its own migration adding `namespace`, the writer can +// pass `namespaceFor(key)` back into the upsert without reinventing +// the convention. Keep this in lockstep with the docstring at the +// top of this file so plugin authors can predict what column they +// land under. +// // Convention: "core.*" → "core"; "plugin:.*" or ".*" → -// "plugin:". The function is intentionally conservative — -// anything we can't classify gets the safe default "core" rather than -// a guessed namespace. +// "plugin:". Conservative — anything we can't classify becomes +// "core" rather than a guessed namespace. func namespaceFor(key string) string { // Core keys. if len(key) >= 5 && key[:5] == "core." { diff --git a/packages/go/settings/postgres_test.go b/packages/go/settings/postgres_test.go index 23ee8d85..3155ffb3 100644 --- a/packages/go/settings/postgres_test.go +++ b/packages/go/settings/postgres_test.go @@ -320,6 +320,13 @@ func TestPostgresStore_WriteValidatesAndUpserts(t *testing.T) { t.Fatalf("expected 1 INSERT, got %d", len(upserts)) } args := upserts[0].args + // Three positional args: key, value (JSON-encoded), autoload. + // An earlier version of upsertSQL had a 4th `namespace` arg and a + // 5th explicit `updated_at = now()`; both were dropped when the + // non-existent `namespace` column made live writes 500. + if got := len(args); got != 3 { + t.Fatalf("upsert arg count: got %d want 3 (key, value, autoload)", got) + } if args[0].(string) != "core.site.name" { t.Errorf("upsert key arg: got %v", args[0]) } @@ -329,8 +336,11 @@ func TestPostgresStore_WriteValidatesAndUpserts(t *testing.T) { if args[2].(bool) != true { t.Errorf("upsert autoload arg: got %v want true", args[2]) } - if args[3].(string) != "core" { - t.Errorf("upsert namespace arg: got %v want core", args[3]) + // Regression guard: the SQL string itself must not reference the + // non-existent `namespace` column. Catches a re-introduction + // before it hits Postgres. + if strings.Contains(upserts[0].sql, "namespace") { + t.Errorf("upsert SQL must not reference `namespace` column: %s", upserts[0].sql) } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f1a4e429..666d50c3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -627,6 +627,25 @@ importers: specifier: ^1.6.0 version: 1.6.1(@types/node@22.19.19)(jsdom@24.1.3) + packages/ts/sdk-plugin: + dependencies: + esbuild: + specifier: ^0.21.0 + version: 0.21.5 + devDependencies: + '@gonext/test-config': + specifier: workspace:* + version: link:../test-config + '@vitest/coverage-v8': + specifier: ^1.6.0 + version: 1.6.1(vitest@1.6.1(@types/node@22.19.19)(jsdom@29.1.1)) + typescript: + specifier: ^5.6.0 + version: 5.9.3 + vitest: + specifier: ^1.6.0 + version: 1.6.1(@types/node@22.19.19)(jsdom@29.1.1) + packages/ts/test-config: devDependencies: '@testing-library/jest-dom':