diff --git a/.gitignore b/.gitignore index dad0e62..814bb12 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,9 @@ /server/certs/*.key /server/.env /server/**/*.sql +# ...except the canonical table definitions, which are source, not generated +# output: the CMS go:embeds them, so a checkout without them does not compile. +!/server/internal/database/schema/*.sql /report.json .vscode/settings.json .codex diff --git a/README.md b/README.md index e859c1f..998b39c 100644 --- a/README.md +++ b/README.md @@ -214,6 +214,8 @@ Generated files: - `04-seo.sql` - `05-article-embeddings.sql` (real file when available, placeholder otherwise) - `06-taxonomy.sql` +- `07-poll-counts.sql` +- `08-comments.sql` (real file when available, empty table placeholder otherwise) ## API Docs (Swagger) diff --git a/deploy/README.md b/deploy/README.md index 5d29a0a..a1e9ce8 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -203,6 +203,9 @@ The file must contain the exact immutable image tag for the active deployment: - `CMS_SESSION_TTL_SECONDS` - `CMS_AUTO_PROMOTE_ALL_ADMINS` - `CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP` +- `AKISMET_API_KEY` - optional; leave empty to disable comment spam filtering. +- `AKISMET_BLOG_URL` - full public site URL Akismet should associate with + comment checks. Required when `AKISMET_API_KEY` is set. - `MEDIA_HOST_PATH` - host path to the CephFS media tree, bind-mounted into the backend. Defaults to `/mnt/cephfs/media`. - `MEDIA_ROOT` - the same tree as seen *inside* the container. Leave at diff --git a/deploy/cms.env.example b/deploy/cms.env.example index 0b1952f..bb8e654 100644 --- a/deploy/cms.env.example +++ b/deploy/cms.env.example @@ -14,6 +14,8 @@ OIDC_REDIRECT_URI= CMS_SESSION_TTL_SECONDS= CMS_AUTO_PROMOTE_ALL_ADMINS= CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP= +AKISMET_API_KEY= +AKISMET_BLOG_URL= MEDIA_HOST_PATH= MEDIA_ROOT= MEDIA_BASE_URL= diff --git a/deploy/compose.cms.yml b/deploy/compose.cms.yml index 462b4e3..a079034 100644 --- a/deploy/compose.cms.yml +++ b/deploy/compose.cms.yml @@ -29,6 +29,8 @@ x-backend-base: &backend-base CMS_SESSION_TTL_SECONDS: ${CMS_SESSION_TTL_SECONDS:-604800} CMS_AUTO_PROMOTE_ALL_ADMINS: ${CMS_AUTO_PROMOTE_ALL_ADMINS:-false} CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP: ${CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP:-false} + AKISMET_API_KEY: ${AKISMET_API_KEY:-} + AKISMET_BLOG_URL: ${AKISMET_BLOG_URL:-} # Media: legacy WP uploads migrated to CephFS. The upload endpoint writes new # assets under MEDIA_ROOT; MEDIA_BASE_URL is the public host that serves them. MEDIA_ROOT: ${MEDIA_ROOT:-/mnt/cephfs/media} diff --git a/docker-compose.yml b/docker-compose.yml index a2db20e..39713b6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -16,6 +16,7 @@ services: - ./server/internal/database/wordpress_etl/05-article-embeddings.sql:/docker-entrypoint-initdb.d/05-article-embeddings.sql:ro,z - ./server/internal/database/wordpress_etl/06-taxonomy.sql:/docker-entrypoint-initdb.d/06-taxonomy.sql:ro,z - ./server/internal/database/wordpress_etl/07-poll-counts.sql:/docker-entrypoint-initdb.d/07-poll-counts.sql:ro,z + - ./server/internal/database/wordpress_etl/08-comments.sql:/docker-entrypoint-initdb.d/08-comments.sql:ro,z ports: - "127.0.0.1:${MARIADB_PORT_FORWARD:-3306}:3306" healthcheck: diff --git a/scripts/generate_wordpress_sql.py b/scripts/generate_wordpress_sql.py index a44fcf5..329b458 100644 --- a/scripts/generate_wordpress_sql.py +++ b/scripts/generate_wordpress_sql.py @@ -11,18 +11,9 @@ from pathlib import Path -TAXONOMY_SQL = """DROP TABLE IF EXISTS site_taxonomy; -CREATE TABLE site_taxonomy ( - id BIGINT PRIMARY KEY, - kind VARCHAR(32) NOT NULL, - slug VARCHAR(255) NOT NULL, - canonical_title VARCHAR(255) NOT NULL, - parent_slug VARCHAR(255) NULL, - article_count BIGINT UNSIGNED NOT NULL DEFAULT 0, - UNIQUE KEY uq_site_taxonomy_kind_slug (kind, slug) -); - -INSERT INTO site_taxonomy (id, kind, slug, canonical_title, parent_slug, article_count) VALUES +# Seed ROWS only. The table definition comes from the canonical schema file -- +# see canonical_schema() and server/internal/database/schema/README.md. +TAXONOMY_ROWS_SQL = """INSERT INTO site_taxonomy (id, kind, slug, canonical_title, parent_slug, article_count) VALUES (1, 'section', 'news', 'News', NULL, 0), (2, 'section', 'sports', 'Sports', NULL, 0), (3, 'section', 'opinion', 'Opinion', NULL, 0), @@ -64,17 +55,33 @@ -- No article_embeddings.sql found in ETL output. """ -POLL_COUNTS_SQL = """DROP TABLE IF EXISTS cms_poll_counts; -CREATE TABLE cms_poll_counts ( - option_name VARCHAR(128) NOT NULL, - vote_count BIGINT UNSIGNED NOT NULL DEFAULT 0, - updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, - PRIMARY KEY (option_name) -) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; -""" - NO_AUTO_VALUE_ON_ZERO_PREAMBLE = "SET sql_mode = CONCAT(@@sql_mode, ',NO_AUTO_VALUE_ON_ZERO');\n" +# Canonical table definitions, shared with the CMS. The same files are embedded +# into the binary and executed at startup (server/internal/database/schema.go), +# so a seeded dev database and production cannot drift apart. Never inline a +# CREATE TABLE for a CMS-owned table here -- that is the bug this replaced. +SCHEMA_DIR_PARTS = ("server", "internal", "database", "schema") + + +def canonical_schema(root_dir: Path, table: str, *, drop_first: bool = False) -> str: + """Return the canonical CREATE TABLE for `table`, as a runnable statement.""" + path = root_dir.joinpath(*SCHEMA_DIR_PARTS) / f"{table}.sql" + try: + body = path.read_text(encoding="utf-8").strip() + except OSError as exc: + print(f"Missing canonical schema for {table}: {path} ({exc})", file=sys.stderr) + print("The CMS embeds these files; the checkout is incomplete.", file=sys.stderr) + raise SystemExit(1) + + statement = f"{body};\n" + if drop_first: + # Seed files are re-runnable and own their table outright, so they start + # from a clean one rather than layering onto whatever is there. + statement = f"DROP TABLE IF EXISTS {table};\n{statement}" + return statement + + USAGE_ERROR = """Could not determine WordPress ETL SQL source directory. Expected one of: @@ -91,6 +98,7 @@ Optional ETL SQL file: - article_embeddings.sql + - comments.sql Examples: python ./scripts/generate_wordpress_sql.py ../wordpress-etl @@ -154,12 +162,48 @@ def ensure_pattern(path: Path, pattern: str, label: str) -> None: raise SystemExit(1) +# Matches an actual statement that turns the mode on, not a mere mention of the +# name. A bare substring test also matches a comment, which would silently skip +# the preamble and let the id=0 row be renumbered on load. +SQL_MODE_SET_PATTERN = re.compile( + r"^\s*SET\b[^;]*\bsql_mode\b[^;]*\bNO_AUTO_VALUE_ON_ZERO\b", + re.IGNORECASE | re.MULTILINE, +) + +# Any statement that writes rows; such a file must carry the sql_mode preamble. +INSERT_PATTERN = re.compile(r"^\s*(INSERT|REPLACE|LOAD\s+DATA)\b", re.IGNORECASE | re.MULTILINE) + + +def has_no_auto_value_on_zero(text: str) -> bool: + return SQL_MODE_SET_PATTERN.search(text) is not None + + def copy_sql_with_mariadb_mode(src: Path, dest: Path) -> None: + """Copy an ETL artifact, guaranteeing NO_AUTO_VALUE_ON_ZERO is in effect. + + `articles` has a legitimate row with id = 0. Without this mode MariaDB + treats 0 in an AUTO_INCREMENT column as "assign the next value", so that row + silently becomes id = 1 (or whatever is next) and every articles_authors / + seo row pointing at 0 is orphaned. sql_mode is session-scoped and each seed + file is loaded in its own session, so the statement has to lead every file + that inserts rows -- it cannot be set once globally for the batch. + """ text = src.read_text(encoding="utf-8", errors="replace") - if "NO_AUTO_VALUE_ON_ZERO" not in text: + if not has_no_auto_value_on_zero(text): text = NO_AUTO_VALUE_ON_ZERO_PREAMBLE + text dest.write_text(text, encoding="utf-8") + # Verify what actually landed on disk rather than trusting the branch above: + # this is the last point at which the mistake is cheap to catch. + written = dest.read_text(encoding="utf-8") + if INSERT_PATTERN.search(written) and not has_no_auto_value_on_zero(written): + print( + f"refusing to emit {dest.name}: it inserts rows without " + "NO_AUTO_VALUE_ON_ZERO, which would renumber the articles id=0 row", + file=sys.stderr, + ) + raise SystemExit(1) + def main() -> int: args = parse_args() @@ -260,6 +304,17 @@ def main() -> int: src_grandparent / "sql" / "article_embeddings.sql", ] ) + comments_sql = first_existing_file( + [ + src_dir / "comments.sql", + src_parent / "comments.sql", + src_parent / "logs" / "sql" / "comments.sql", + src_parent / "sql" / "comments.sql", + src_grandparent / "comments.sql", + src_grandparent / "logs" / "sql" / "comments.sql", + src_grandparent / "sql" / "comments.sql", + ] + ) authors_sql = require_file(authors_sql, "authors.sql") articles_sql = require_file(articles_sql, "articles.sql") @@ -279,6 +334,7 @@ def main() -> int: out_embeddings = out_dir / "05-article-embeddings.sql" out_taxonomy = out_dir / "06-taxonomy.sql" out_poll_counts = out_dir / "07-poll-counts.sql" + out_comments = out_dir / "08-comments.sql" copy_sql_with_mariadb_mode(authors_sql, out_authors) copy_sql_with_mariadb_mode(articles_sql, out_articles) @@ -290,8 +346,17 @@ def main() -> int: else: out_embeddings.write_text(PLACEHOLDER_EMBEDDINGS_SQL, encoding="utf-8") - out_taxonomy.write_text(TAXONOMY_SQL, encoding="utf-8") - out_poll_counts.write_text(POLL_COUNTS_SQL, encoding="utf-8") + out_taxonomy.write_text( + canonical_schema(root_dir, "site_taxonomy", drop_first=True) + "\n" + TAXONOMY_ROWS_SQL, + encoding="utf-8", + ) + out_poll_counts.write_text( + canonical_schema(root_dir, "cms_poll_counts", drop_first=True), encoding="utf-8" + ) + if comments_sql is not None: + copy_sql_with_mariadb_mode(comments_sql, out_comments) + else: + out_comments.write_text(canonical_schema(root_dir, "comments"), encoding="utf-8") print(f"Imported ETL SQL into: {out_dir}") print(f" 01-authors.sql <- {authors_sql}") @@ -304,6 +369,10 @@ def main() -> int: print(" 05-article-embeddings.sql <- placeholder (no ETL embeddings artifact found)") print(" 06-taxonomy.sql <- cms static taxonomy seed") print(" 07-poll-counts.sql <- cms poll counts schema seed") + if comments_sql is not None: + print(f" 08-comments.sql <- {comments_sql}") + else: + print(" 08-comments.sql <- placeholder (no ETL comments artifact found)") return 0 diff --git a/server/internal/akismet/akismet.go b/server/internal/akismet/akismet.go new file mode 100644 index 0000000..4165c19 --- /dev/null +++ b/server/internal/akismet/akismet.go @@ -0,0 +1,156 @@ +package akismet + +import ( + "context" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" +) + +const ( + defaultEndpoint = "https://rest.akismet.com/1.1/comment-check" + defaultUserAgent = "TriangleCMS/1.0 | Akismet/1.1" + defaultTimeout = 3 * time.Second +) + +type Checker interface { + CheckComment(ctx context.Context, comment Comment) (bool, error) +} + +type Config struct { + APIKey string + BlogURL string + Endpoint string + UserAgent string + HTTPClient *http.Client +} + +type Comment struct { + UserIP string + UserAgent string + Referrer string + Permalink string + Type string + Author string + AuthorEmail string + AuthorURL string + Content string + CreatedAt time.Time + IsTest bool +} + +type Client struct { + apiKey string + blogURL string + endpoint string + userAgent string + httpClient *http.Client +} + +func NewClient(cfg Config) (*Client, error) { + apiKey := strings.TrimSpace(cfg.APIKey) + if apiKey == "" { + return nil, fmt.Errorf("akismet api key is required") + } + + blogURL := strings.TrimSpace(cfg.BlogURL) + if blogURL == "" { + return nil, fmt.Errorf("akismet blog url is required") + } + parsedBlogURL, err := url.ParseRequestURI(blogURL) + if err != nil || parsedBlogURL == nil || parsedBlogURL.Scheme == "" || parsedBlogURL.Host == "" { + return nil, fmt.Errorf("akismet blog url must be a full URL") + } + + endpoint := strings.TrimSpace(cfg.Endpoint) + if endpoint == "" { + endpoint = defaultEndpoint + } + parsedEndpoint, err := url.ParseRequestURI(endpoint) + if err != nil || parsedEndpoint == nil || parsedEndpoint.Scheme == "" || parsedEndpoint.Host == "" { + return nil, fmt.Errorf("akismet endpoint must be a full URL") + } + + userAgent := strings.TrimSpace(cfg.UserAgent) + if userAgent == "" { + userAgent = defaultUserAgent + } + + httpClient := cfg.HTTPClient + if httpClient == nil { + httpClient = &http.Client{Timeout: defaultTimeout} + } + + return &Client{ + apiKey: apiKey, + blogURL: blogURL, + endpoint: endpoint, + userAgent: userAgent, + httpClient: httpClient, + }, nil +} + +func (c *Client) CheckComment(ctx context.Context, comment Comment) (bool, error) { + values := url.Values{} + values.Set("api_key", c.apiKey) + values.Set("blog", c.blogURL) + values.Set("user_ip", strings.TrimSpace(comment.UserIP)) + values.Set("user_agent", strings.TrimSpace(comment.UserAgent)) + values.Set("referrer", strings.TrimSpace(comment.Referrer)) + values.Set("permalink", strings.TrimSpace(comment.Permalink)) + values.Set("comment_type", valueOrDefault(comment.Type, "comment")) + values.Set("comment_author", strings.TrimSpace(comment.Author)) + values.Set("comment_author_email", strings.TrimSpace(comment.AuthorEmail)) + values.Set("comment_author_url", strings.TrimSpace(comment.AuthorURL)) + values.Set("comment_content", strings.TrimSpace(comment.Content)) + values.Set("blog_charset", "UTF-8") + if !comment.CreatedAt.IsZero() { + values.Set("comment_date_gmt", comment.CreatedAt.UTC().Format(time.RFC3339)) + } + if comment.IsTest { + values.Set("is_test", "1") + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.endpoint, strings.NewReader(values.Encode())) + if err != nil { + return false, fmt.Errorf("create akismet request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("User-Agent", c.userAgent) + + resp, err := c.httpClient.Do(req) + if err != nil { + return false, fmt.Errorf("akismet comment check failed: %w", err) + } + defer resp.Body.Close() + + bodyBytes, err := io.ReadAll(io.LimitReader(resp.Body, 1024)) + if err != nil { + return false, fmt.Errorf("read akismet response: %w", err) + } + body := strings.TrimSpace(string(bodyBytes)) + + if resp.StatusCode != http.StatusOK { + return false, fmt.Errorf("akismet returned %s: %s", resp.Status, body) + } + + switch body { + case "true": + return true, nil + case "false": + return false, nil + default: + return false, fmt.Errorf("akismet returned unexpected response %q", body) + } +} + +func valueOrDefault(value, fallback string) string { + value = strings.TrimSpace(value) + if value == "" { + return fallback + } + return value +} diff --git a/server/internal/akismet/akismet_test.go b/server/internal/akismet/akismet_test.go new file mode 100644 index 0000000..2a61607 --- /dev/null +++ b/server/internal/akismet/akismet_test.go @@ -0,0 +1,141 @@ +package akismet + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +func TestCheckCommentPostsExpectedFieldsAndMapsSpam(t *testing.T) { + var gotUserAgent string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Fatalf("expected POST, got %s", r.Method) + } + gotUserAgent = r.Header.Get("User-Agent") + if contentType := r.Header.Get("Content-Type"); contentType != "application/x-www-form-urlencoded" { + t.Fatalf("expected form content type, got %q", contentType) + } + if err := r.ParseForm(); err != nil { + t.Fatalf("failed to parse form: %v", err) + } + + want := map[string]string{ + "api_key": "test-key", + "blog": "https://example.org", + "user_ip": "203.0.113.10", + "user_agent": "Mozilla/5.0", + "referrer": "https://referrer.example/search", + "permalink": "https://example.org/article/story", + "comment_type": "comment", + "comment_author": "akismet-guaranteed-spam", + "comment_author_email": "spam@example.org", + "comment_author_url": "https://spammer.example", + "comment_content": "spam payload", + "comment_date_gmt": "2026-07-30T12:00:00Z", + "blog_charset": "UTF-8", + } + for key, value := range want { + if got := r.Form.Get(key); got != value { + t.Fatalf("form %s = %q, want %q", key, got, value) + } + } + + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("true")) + })) + defer server.Close() + + client, err := NewClient(Config{ + APIKey: "test-key", + BlogURL: "https://example.org", + Endpoint: server.URL, + UserAgent: "TriangleCMS/test", + }) + if err != nil { + t.Fatalf("NewClient returned error: %v", err) + } + + isSpam, err := client.CheckComment(t.Context(), Comment{ + UserIP: "203.0.113.10", + UserAgent: "Mozilla/5.0", + Referrer: "https://referrer.example/search", + Permalink: "https://example.org/article/story", + Author: "akismet-guaranteed-spam", + AuthorEmail: "spam@example.org", + AuthorURL: "https://spammer.example", + Content: "spam payload", + CreatedAt: time.Date(2026, 7, 30, 12, 0, 0, 0, time.UTC), + }) + if err != nil { + t.Fatalf("CheckComment returned error: %v", err) + } + if !isSpam { + t.Fatal("expected spam verdict") + } + if gotUserAgent != "TriangleCMS/test" { + t.Fatalf("request User-Agent = %q, want %q", gotUserAgent, "TriangleCMS/test") + } +} + +func TestCheckCommentMapsHam(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("false")) + })) + defer server.Close() + + client, err := NewClient(Config{ + APIKey: "test-key", + BlogURL: "https://example.org", + Endpoint: server.URL, + }) + if err != nil { + t.Fatalf("NewClient returned error: %v", err) + } + + isSpam, err := client.CheckComment(t.Context(), Comment{UserIP: "203.0.113.10", Content: "hello"}) + if err != nil { + t.Fatalf("CheckComment returned error: %v", err) + } + if isSpam { + t.Fatal("expected ham verdict") + } +} + +func TestCheckCommentRejectsUnexpectedResponse(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("invalid")) + })) + defer server.Close() + + client, err := NewClient(Config{ + APIKey: "test-key", + BlogURL: "https://example.org", + Endpoint: server.URL, + }) + if err != nil { + t.Fatalf("NewClient returned error: %v", err) + } + + _, err = client.CheckComment(t.Context(), Comment{UserIP: "203.0.113.10", Content: "hello"}) + if err == nil { + t.Fatal("expected unexpected response error") + } + if !strings.Contains(err.Error(), "unexpected response") { + t.Fatalf("expected unexpected response error, got %v", err) + } +} + +func TestNewClientRequiresFullBlogURL(t *testing.T) { + _, err := NewClient(Config{APIKey: "test-key", BlogURL: "example.org"}) + if err == nil { + t.Fatal("expected blog URL validation error") + } + if !strings.Contains(err.Error(), "full URL") { + t.Fatalf("expected full URL error, got %v", err) + } +} diff --git a/server/internal/database/comments.go b/server/internal/database/comments.go index a129c20..5ebf826 100644 --- a/server/internal/database/comments.go +++ b/server/internal/database/comments.go @@ -17,30 +17,13 @@ var ValidCommentStatuses = map[string]bool{ } func EnsureCommentsTable(ctx context.Context, conn *sql.DB) error { - if _, err := conn.ExecContext(ctx, ` - CREATE TABLE IF NOT EXISTS comments ( - id BIGINT NOT NULL AUTO_INCREMENT PRIMARY KEY, - article_id BIGINT NULL, - wp_post_id BIGINT NULL, - parent_id BIGINT NULL, - author_name LONGTEXT, - author_email LONGTEXT, - author_url LONGTEXT, - author_ip VARCHAR(255), - author_user_id BIGINT, - content LONGTEXT, - created_at DATETIME, - created_at_gmt DATETIME, - status VARCHAR(32), - `+"`type`"+` VARCHAR(32), - INDEX idx_comments_article_status_created (article_id, status, created_at_gmt), - INDEX idx_comments_wp_post_id (wp_post_id), - INDEX idx_comments_parent_id (parent_id) - ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 - `); err != nil { + if _, err := conn.ExecContext(ctx, TableSchema("comments")); err != nil { return err } + // Expand-only migration for databases that already have the table: CREATE + // TABLE above is a no-op there, so a column added to schema/comments.sql + // reaches them only through this block. Keep the two in step. if _, err := conn.ExecContext(ctx, ` ALTER TABLE comments ADD COLUMN IF NOT EXISTS article_id BIGINT NULL, diff --git a/server/internal/database/poll_schema.go b/server/internal/database/poll_schema.go index eafaa95..4e35cde 100644 --- a/server/internal/database/poll_schema.go +++ b/server/internal/database/poll_schema.go @@ -8,13 +8,6 @@ import ( const PollTableName = "cms_poll_counts" func EnsurePollsTable(ctx context.Context, conn *sql.DB) error { - _, err := conn.ExecContext(ctx, ` - CREATE TABLE IF NOT EXISTS `+PollTableName+` ( - option_name VARCHAR(128) NOT NULL, - vote_count BIGINT UNSIGNED NOT NULL DEFAULT 0, - updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, - PRIMARY KEY (option_name) - ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 - `) + _, err := conn.ExecContext(ctx, TableSchema(PollTableName)) return err } diff --git a/server/internal/database/schema.go b/server/internal/database/schema.go new file mode 100644 index 0000000..e70c0db --- /dev/null +++ b/server/internal/database/schema.go @@ -0,0 +1,28 @@ +package database + +import ( + "embed" + "fmt" + "strings" +) + +// schemaFS holds the canonical CREATE TABLE definition for every CMS-owned +// table. These same files are read by scripts/generate_wordpress_sql.py when it +// writes the local seed SQL, so the schema a dev database is seeded with and the +// schema the CMS converges to at startup cannot drift apart. See +// schema/README.md. +// +//go:embed schema/*.sql +var schemaFS embed.FS + +// TableSchema returns the canonical CREATE TABLE statement for table. It panics +// if the definition is missing, because that means the binary was built without +// a schema file it needs and it could only fail later, at first request, against +// a table that was never created. +func TableSchema(table string) string { + body, err := schemaFS.ReadFile("schema/" + table + ".sql") + if err != nil { + panic(fmt.Sprintf("database: no canonical schema for table %q: %v", table, err)) + } + return strings.TrimSpace(string(body)) +} diff --git a/server/internal/database/schema/README.md b/server/internal/database/schema/README.md new file mode 100644 index 0000000..1c62187 --- /dev/null +++ b/server/internal/database/schema/README.md @@ -0,0 +1,30 @@ +# Canonical table definitions + +Every `.sql` file here is the **single source of truth** for one CMS-owned +table. Two consumers read these exact files, so there is no second copy to +drift: + +1. **The CMS at startup** — embedded via `go:embed` in + [../schema.go](../schema.go) and executed by the matching `Ensure*Table` + function in this package. +2. **The local seed generator** — `scripts/generate_wordpress_sql.py` reads them + when it writes `wordpress_etl/*.sql`, so a freshly seeded dev database gets + byte-identical DDL to what production converges to. + +Before this existed, the definitions were duplicated between Go string literals +and Python constants, and they had already drifted: the seeded `site_taxonomy` +had a *signed* `id`, no `ENGINE`/`CHARSET`, and was missing +`idx_site_taxonomy_kind` and `idx_site_taxonomy_parent_slug`. + +## Rules + +- One `CREATE TABLE IF NOT EXISTS` per file, named after the table. `IF NOT + EXISTS` matters: startup runs these against live databases that already have + the table. +- **Expand-only.** `CREATE TABLE` alone never alters an existing table, so a new + column here reaches an existing database only through the `ADD COLUMN IF NOT + EXISTS` block in the corresponding `Ensure*` function. Add it in both places, + and never drop or retype a column a rolled-back binary still reads. +- Tables owned by the WordPress ETL (`articles`, `authors`, `seo`, …) are **not** + here — the ETL emits their DDL. This directory is only for tables the CMS + creates itself. diff --git a/server/internal/database/schema/cms_poll_counts.sql b/server/internal/database/schema/cms_poll_counts.sql new file mode 100644 index 0000000..6d236ce --- /dev/null +++ b/server/internal/database/schema/cms_poll_counts.sql @@ -0,0 +1,6 @@ +CREATE TABLE IF NOT EXISTS cms_poll_counts ( + option_name VARCHAR(128) NOT NULL, + vote_count BIGINT UNSIGNED NOT NULL DEFAULT 0, + updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + PRIMARY KEY (option_name) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 diff --git a/server/internal/database/schema/comments.sql b/server/internal/database/schema/comments.sql new file mode 100644 index 0000000..ba0e686 --- /dev/null +++ b/server/internal/database/schema/comments.sql @@ -0,0 +1,19 @@ +CREATE TABLE IF NOT EXISTS comments ( + id BIGINT NOT NULL AUTO_INCREMENT PRIMARY KEY, + article_id BIGINT NULL, + wp_post_id BIGINT NULL, + parent_id BIGINT NULL, + author_name LONGTEXT, + author_email LONGTEXT, + author_url LONGTEXT, + author_ip VARCHAR(255), + author_user_id BIGINT, + content LONGTEXT, + created_at DATETIME, + created_at_gmt DATETIME, + status VARCHAR(32), + `type` VARCHAR(32), + INDEX idx_comments_article_status_created (article_id, status, created_at_gmt), + INDEX idx_comments_wp_post_id (wp_post_id), + INDEX idx_comments_parent_id (parent_id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 diff --git a/server/internal/database/schema/site_taxonomy.sql b/server/internal/database/schema/site_taxonomy.sql new file mode 100644 index 0000000..801ff43 --- /dev/null +++ b/server/internal/database/schema/site_taxonomy.sql @@ -0,0 +1,11 @@ +CREATE TABLE IF NOT EXISTS site_taxonomy ( + id BIGINT UNSIGNED NOT NULL PRIMARY KEY, + kind VARCHAR(32) NOT NULL, + slug VARCHAR(255) NOT NULL, + canonical_title VARCHAR(255) NOT NULL, + parent_slug VARCHAR(255) NULL, + article_count BIGINT UNSIGNED NOT NULL DEFAULT 0, + UNIQUE KEY uq_site_taxonomy_kind_slug (kind, slug), + KEY idx_site_taxonomy_kind (kind), + KEY idx_site_taxonomy_parent_slug (parent_slug) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 diff --git a/server/internal/database/schema_test.go b/server/internal/database/schema_test.go new file mode 100644 index 0000000..411271c --- /dev/null +++ b/server/internal/database/schema_test.go @@ -0,0 +1,101 @@ +package database + +import ( + "context" + "database/sql" + "os" + "strings" + "testing" + + _ "github.com/go-sql-driver/mysql" +) + +// Every table the CMS creates itself must have a canonical definition on disk, +// because scripts/generate_wordpress_sql.py reads these same files to build the +// local seed. A rename here silently breaks the seed generator, which has no +// tests of its own, so this is the guard for both consumers. +func TestTableSchema_CoversEveryCMSOwnedTable(t *testing.T) { + for _, table := range []string{"comments", "cms_poll_counts", "site_taxonomy"} { + t.Run(table, func(t *testing.T) { + got := TableSchema(table) + + if !strings.Contains(got, "CREATE TABLE IF NOT EXISTS "+table) { + t.Fatalf("schema/%s.sql must CREATE TABLE IF NOT EXISTS %s, got:\n%s", table, table, got) + } + // Startup runs these against databases that already have the table, + // so a plain CREATE TABLE would fail the boot. + if strings.HasSuffix(strings.TrimSpace(got), ";") { + t.Fatalf("schema/%s.sql must not end in a semicolon; it is executed as a single statement", table) + } + }) + } +} + +func TestTableSchema_PollTableNameMatchesItsFile(t *testing.T) { + // EnsurePollsTable looks the file up by PollTableName, so the constant and + // the filename have to agree or startup panics. + if got := TableSchema(PollTableName); !strings.Contains(got, PollTableName) { + t.Fatalf("schema for %q does not define that table:\n%s", PollTableName, got) + } +} + +func TestTableSchema_PanicsOnUnknownTable(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("expected a panic for a table with no canonical schema") + } + }() + + TableSchema("no_such_table") +} + +// TestArticleIDZeroSurvivesSeed proves the thing the NO_AUTO_VALUE_ON_ZERO +// preamble exists for: `articles` has a real row with id = 0, and without that +// mode MariaDB reads 0 in an AUTO_INCREMENT column as "assign the next value", +// renumbering it and orphaning every articles_authors and seo row that +// references it. Needs a real server — the behaviour is entirely server-side. +// +// CMS_TEST_DSN='root:pw@tcp(127.0.0.1:3306)/seed_test?multiStatements=true' go test ./internal/database/ -run IDZero -v +func TestArticleIDZeroSurvivesSeed(t *testing.T) { + dsn := os.Getenv("CMS_TEST_DSN") + if dsn == "" { + t.Skip("CMS_TEST_DSN not set; skipping id=0 seed integration test") + } + + conn, err := sql.Open("mysql", dsn) + if err != nil { + t.Fatalf("open test database: %v", err) + } + if err := conn.Ping(); err != nil { + t.Fatalf("ping test database: %v", err) + } + t.Cleanup(func() { conn.Close() }) + + ctx := context.Background() + + if _, err := conn.ExecContext(ctx, "DROP TABLE IF EXISTS id_zero_probe"); err != nil { + t.Fatalf("drop probe table: %v", err) + } + if _, err := conn.ExecContext(ctx, "CREATE TABLE id_zero_probe (id BIGINT NOT NULL AUTO_INCREMENT PRIMARY KEY, title VARCHAR(64))"); err != nil { + t.Fatalf("create probe table: %v", err) + } + t.Cleanup(func() { + _, _ = conn.ExecContext(context.Background(), "DROP TABLE IF EXISTS id_zero_probe") + }) + + // Exactly what scripts/generate_wordpress_sql.py prepends to each seed file. + if _, err := conn.ExecContext(ctx, "SET sql_mode = CONCAT(@@sql_mode, ',NO_AUTO_VALUE_ON_ZERO')"); err != nil { + t.Fatalf("set sql_mode: %v", err) + } + if _, err := conn.ExecContext(ctx, "INSERT INTO id_zero_probe (id, title) VALUES (0, 'legacy')"); err != nil { + t.Fatalf("insert id=0: %v", err) + } + + var minID int64 + if err := conn.QueryRowContext(ctx, "SELECT MIN(id) FROM id_zero_probe").Scan(&minID); err != nil { + t.Fatalf("read back min id: %v", err) + } + if minID != 0 { + t.Fatalf("id=0 row was renumbered to %d; the seed preamble is not taking effect", minID) + } +} diff --git a/server/internal/database/taxonomy.go b/server/internal/database/taxonomy.go index 245a9b6..a52d640 100644 --- a/server/internal/database/taxonomy.go +++ b/server/internal/database/taxonomy.go @@ -8,19 +8,7 @@ import ( ) func EnsureTaxonomyTable(ctx context.Context, conn *sql.DB) error { - _, err := conn.ExecContext(ctx, ` - CREATE TABLE IF NOT EXISTS site_taxonomy ( - id BIGINT UNSIGNED NOT NULL PRIMARY KEY, - kind VARCHAR(32) NOT NULL, - slug VARCHAR(255) NOT NULL, - canonical_title VARCHAR(255) NOT NULL, - parent_slug VARCHAR(255) NULL, - article_count BIGINT UNSIGNED NOT NULL DEFAULT 0, - UNIQUE KEY uq_site_taxonomy_kind_slug (kind, slug), - KEY idx_site_taxonomy_kind (kind), - KEY idx_site_taxonomy_parent_slug (parent_slug) - ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 - `) + _, err := conn.ExecContext(ctx, TableSchema("site_taxonomy")) if err != nil { return err } diff --git a/server/internal/handlers/handlers.go b/server/internal/handlers/handlers.go index 305cc53..0b95d4a 100644 --- a/server/internal/handlers/handlers.go +++ b/server/internal/handlers/handlers.go @@ -5,6 +5,7 @@ import ( "database/sql" "encoding/json" "fmt" + "log/slog" "net" "net/http" "net/url" @@ -13,6 +14,7 @@ import ( "time" "server/internal/activity" + "server/internal/akismet" db "server/internal/database" "server/internal/middleware" "server/internal/models" @@ -1548,7 +1550,7 @@ func GetArticleComments(conn *sql.DB) http.HandlerFunc { // @Failure 413 {object} models.ErrorResponse // @Failure 500 {object} models.ErrorResponse // @Router /v1/articles/{slug}/comments [post] -func PostArticleComment(conn *sql.DB) http.HandlerFunc { +func PostArticleComment(conn *sql.DB, spamChecker akismet.Checker) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { slug := strings.TrimSpace(r.PathValue("slug")) if !isValidCanonicalSlug(slug) { @@ -1620,6 +1622,29 @@ func PostArticleComment(conn *sql.DB) http.HandlerFunc { return } + now := time.Now().UTC() + status := "approved" + if spamChecker != nil { + isSpam, err := spamChecker.CheckComment(r.Context(), akismet.Comment{ + UserIP: clientIP(r), + UserAgent: r.UserAgent(), + Referrer: r.Referer(), + Permalink: articlePermalink(r, slug), + Type: "comment", + Author: body.AuthorName, + AuthorEmail: body.AuthorEmail, + AuthorURL: body.AuthorURL, + Content: body.Content, + CreatedAt: now, + }) + if err != nil { + status = "pending" + slog.Warn("akismet comment check failed; comment requires moderation", "article_slug", slug, "error", err) + } else if isSpam { + status = "spam" + } + } + comment, err := db.CreateComment(r.Context(), conn, db.CreateCommentParams{ ArticleID: articleID, ParentID: body.ParentID, @@ -1628,9 +1653,9 @@ func PostArticleComment(conn *sql.DB) http.HandlerFunc { AuthorURL: body.AuthorURL, AuthorIP: clientIP(r), Content: body.Content, - Status: "approved", + Status: status, Type: "comment", - CreatedAt: time.Now().UTC(), + CreatedAt: now, }) if err != nil { writeError(w, http.StatusInternalServerError, err.Error()) @@ -1652,6 +1677,26 @@ func PostArticleComment(conn *sql.DB) http.HandlerFunc { } } +func articlePermalink(r *http.Request, slug string) string { + if forwardedHost := strings.TrimSpace(r.Header.Get("X-Forwarded-Host")); forwardedHost != "" { + scheme := strings.TrimSpace(r.Header.Get("X-Forwarded-Proto")) + if scheme == "" { + scheme = "https" + } + return scheme + "://" + forwardedHost + "/article/" + slug + } + + if r.Host != "" { + scheme := "http" + if r.TLS != nil { + scheme = "https" + } + return scheme + "://" + r.Host + "/article/" + slug + } + + return "/article/" + slug +} + func adminCommentResponse(comment db.AdminComment) models.AdminCommentResponse { return models.AdminCommentResponse{ ID: comment.ID, diff --git a/server/internal/routes/routes.go b/server/internal/routes/routes.go index ca195fd..13370da 100644 --- a/server/internal/routes/routes.go +++ b/server/internal/routes/routes.go @@ -3,6 +3,7 @@ package routes import ( "database/sql" "net/http" + "server/internal/akismet" "server/internal/auth" "server/internal/handlers" "server/internal/middleware" @@ -12,7 +13,7 @@ import ( httpSwagger "github.com/swaggo/http-swagger" ) -func Register(mux *http.ServeMux, conn *sql.DB, verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig) { +func Register(mux *http.ServeMux, conn *sql.DB, verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig, spamChecker akismet.Checker) { mux.Handle("/swagger/", httpSwagger.WrapHandler) mux.HandleFunc("GET /v1/health", handlers.HealthCheck) @@ -46,7 +47,7 @@ func Register(mux *http.ServeMux, conn *sql.DB, verifier *oidc.IDTokenVerifier, mux.Handle("GET /v1/articles", optionalAuth(handlers.GetArticles(conn))) mux.Handle("GET /v1/articles/{slug}", handlers.GetArticle(conn)) mux.Handle("GET /v1/articles/{slug}/comments", handlers.GetArticleComments(conn)) - mux.Handle("POST /v1/articles/{slug}/comments", middleware.RateLimitByIP(5, time.Minute)(handlers.PostArticleComment(conn))) + mux.Handle("POST /v1/articles/{slug}/comments", middleware.RateLimitByIP(5, time.Minute)(handlers.PostArticleComment(conn, spamChecker))) mux.Handle("GET /v1/search", handlers.GetSearch(conn)) mux.Handle("GET /v1/sections/{section_slug}/articles", optionalAuth(handlers.GetSectionArticles(conn))) mux.Handle("GET /v1/subsections/{subsection_slug}/articles", optionalAuth(handlers.GetSubsectionArticles(conn))) diff --git a/server/internal/routes/routes_test.go b/server/internal/routes/routes_test.go index decf9b8..1664a9a 100644 --- a/server/internal/routes/routes_test.go +++ b/server/internal/routes/routes_test.go @@ -30,7 +30,7 @@ func TestRegister_ReadEndpointsPublicWithVerifier(t *testing.T) { defer conn.Close() mux := http.NewServeMux() - Register(mux, conn, verifier, auth.OIDCConfig{}) + Register(mux, conn, verifier, auth.OIDCConfig{}, nil) public := []string{ "/v1/articles", @@ -72,7 +72,7 @@ func TestRegister_ReadEndpointsPublicWithVerifier(t *testing.T) { func TestRegister_PublicRoute(t *testing.T) { mux := http.NewServeMux() - Register(mux, nil, nil, auth.OIDCConfig{}) + Register(mux, nil, nil, auth.OIDCConfig{}, nil) tests := []struct { name string @@ -138,7 +138,7 @@ func TestRegister_MediaEndpointsGated(t *testing.T) { }) mux := http.NewServeMux() - Register(mux, nil, verifier, auth.OIDCConfig{}) + Register(mux, nil, verifier, auth.OIDCConfig{}, nil) tests := []struct { method string diff --git a/server/main.go b/server/main.go index 1ccc53a..c582519 100644 --- a/server/main.go +++ b/server/main.go @@ -11,6 +11,7 @@ import ( "os" "os/signal" "server/internal/activity" + "server/internal/akismet" "server/internal/auth" "server/internal/database" "server/internal/middleware" @@ -35,6 +36,8 @@ const ( serverModeEnv = "CMS_SERVER_MODE" serverModeHTTPS = "https" serverModeInternalHTTP = "internal-http" + akismetAPIKeyEnv = "AKISMET_API_KEY" + akismetBlogURLEnv = "AKISMET_BLOG_URL" defaultShutdownTimeout = 10 * time.Second ) @@ -58,6 +61,7 @@ type runDeps struct { shutdownTimeout time.Duration oidcVerifier *oidc.IDTokenVerifier oidcCfg auth.OIDCConfig + spamChecker akismet.Checker } // @title Triangle CMS API @@ -213,7 +217,18 @@ func main() { slog.Warn("AUTH DISABLED: protected routes are NOT registered; only public read routes are available") } - if err := run(defaultRunDeps(verifier, oidcCfg), db); err != nil { + spamChecker, err := akismetCheckerFromEnv() + if err != nil { + slog.Error("invalid Akismet configuration", "error", err) + os.Exit(1) + } + if spamChecker == nil { + slog.Warn("Akismet comment spam filtering disabled: AKISMET_API_KEY not set") + } else { + slog.Info("Akismet comment spam filtering enabled") + } + + if err := run(defaultRunDeps(verifier, oidcCfg, spamChecker), db); err != nil { slog.Error("server terminated", "error", err) os.Exit(1) } @@ -246,7 +261,7 @@ func dbConfigFromEnv() (dbName, user, password, host string, port int, err error return dbName, user, password, host, port, nil } -func defaultRunDeps(verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig) runDeps { +func defaultRunDeps(verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig, spamChecker akismet.Checker) runDeps { return runDeps{ loadX509KeyPair: tls.LoadX509KeyPair, newServer: newDefaultServer, @@ -255,6 +270,7 @@ func defaultRunDeps(verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig) run shutdownTimeout: defaultShutdownTimeout, oidcVerifier: verifier, oidcCfg: oidcCfg, + spamChecker: spamChecker, } } @@ -311,7 +327,7 @@ func run(deps runDeps, conn *sql.DB) error { } mux := http.NewServeMux() - routes.Register(mux, conn, deps.oidcVerifier, deps.oidcCfg) + routes.Register(mux, conn, deps.oidcVerifier, deps.oidcCfg, deps.spamChecker) server := deps.newServer(cert, mux, slog.Default()) serverErr := make(chan error, 1) @@ -370,3 +386,20 @@ func getenvOrDefault(key, fallback string) string { } return value } + +func akismetCheckerFromEnv() (akismet.Checker, error) { + apiKey := strings.TrimSpace(os.Getenv(akismetAPIKeyEnv)) + if apiKey == "" { + return nil, nil + } + + blogURL := strings.TrimSpace(os.Getenv(akismetBlogURLEnv)) + if blogURL == "" { + return nil, fmt.Errorf("%s is required when %s is set", akismetBlogURLEnv, akismetAPIKeyEnv) + } + + return akismet.NewClient(akismet.Config{ + APIKey: apiKey, + BlogURL: blogURL, + }) +} diff --git a/server/main_test.go b/server/main_test.go index 4801f03..3a35b48 100644 --- a/server/main_test.go +++ b/server/main_test.go @@ -147,6 +147,32 @@ func TestRun_TLSLoadPathsFromEnv(t *testing.T) { } } +func TestAkismetCheckerFromEnvDisabledWithoutAPIKey(t *testing.T) { + t.Setenv(akismetAPIKeyEnv, "") + t.Setenv(akismetBlogURLEnv, "") + + checker, err := akismetCheckerFromEnv() + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if checker != nil { + t.Fatal("expected nil checker when API key is unset") + } +} + +func TestAkismetCheckerFromEnvRequiresBlogURLWhenAPIKeyIsSet(t *testing.T) { + t.Setenv(akismetAPIKeyEnv, "test-key") + t.Setenv(akismetBlogURLEnv, "") + + _, err := akismetCheckerFromEnv() + if err == nil { + t.Fatal("expected an error") + } + if !strings.Contains(err.Error(), akismetBlogURLEnv) { + t.Fatalf("expected blog URL error, got %v", err) + } +} + func TestRun_ServerExitError(t *testing.T) { srv := &fakeServer{ listenFn: func(certFile, keyFile string) error {