Changeset 0.24.1 (#157)

server/database/compat.go

@@ -1,6 +1,8 @@
 package database
 
 import (
+	"context"
+	"database/sql"
 	"fmt"
 	"strings"
 	"time"
@@ -52,6 +54,112 @@ func Q(query string) string {
 	return q
 }
 
+// QArgs adapts a Postgres query AND its args for the current dialect.
+//
+// Unlike Q() which only rewrites the query string, QArgs also expands the
+// args slice to match positional ? placeholders. In Postgres, $1 can appear
+// multiple times and references the same arg. In SQLite, each ? is
+// positional — if $1 appears twice, the arg must appear twice.
+//
+// Use QArgs for any handler query that references $N more than once
+// (typically subqueries like "WHERE user_id = $1 OR ... participant_id = $1").
+//
+// On Postgres, returns query and args unchanged.
+func QArgs(query string, args ...interface{}) (string, []interface{}) {
+	if !IsSQLite() {
+		return query, args
+	}
+
+	// First, apply the non-placeholder rewrites
+	q := query
+	q = strings.ReplaceAll(q, "::jsonb", "")
+	q = strings.ReplaceAll(q, "::text", "")
+	q = strings.ReplaceAll(q, "NOW()", "datetime('now')")
+	q = strings.ReplaceAll(q, "= true", "= 1")
+	q = strings.ReplaceAll(q, "= false", "= 0")
+	q = strings.ReplaceAll(q, "COALESCE(is_private, false)", "COALESCE(is_private, 0)")
+	q = strings.ReplaceAll(q, "NULLS LAST", "")
+	q = strings.ReplaceAll(q, "ILIKE", "LIKE")
+
+	// Walk the query left-to-right, find each $N, record which arg index
+	// it maps to, replace with ?, and build a new args list in order.
+	var newArgs []interface{}
+	var result strings.Builder
+	i := 0
+	for i < len(q) {
+		if q[i] == '$' && i+1 < len(q) && q[i+1] >= '1' && q[i+1] <= '9' {
+			// Parse the number after $
+			j := i + 1
+			for j < len(q) && q[j] >= '0' && q[j] <= '9' {
+				j++
+			}
+			numStr := q[i+1 : j]
+			n := 0
+			for _, c := range numStr {
+				n = n*10 + int(c-'0')
+			}
+			// $N is 1-based, args is 0-based
+			if n >= 1 && n <= len(args) {
+				newArgs = append(newArgs, args[n-1])
+			}
+			result.WriteByte('?')
+			i = j
+		} else {
+			result.WriteByte(q[i])
+			i++
+		}
+	}
+
+	return result.String(), newArgs
+}
+
+// ── Safe Query Wrappers ─────────────────────
+//
+// These wrappers replace the pattern:
+//   database.DB.QueryRow(database.Q(sql), args...)
+// with:
+//   database.QueryRow(sql, args...)
+//
+// They call QArgs internally, handling $N→? conversion AND arg expansion
+// for repeated placeholder references. Use these for ALL handler-level
+// SQL that uses $N placeholders.
+
+// QueryRow executes a query with dialect adaptation and arg expansion.
+func QueryRow(query string, args ...interface{}) *sql.Row {
+	q, expanded := QArgs(query, args...)
+	return DB.QueryRow(q, expanded...)
+}
+
+// QueryRowContext is the context-aware variant of QueryRow.
+func QueryRowContext(ctx context.Context, query string, args ...interface{}) *sql.Row {
+	q, expanded := QArgs(query, args...)
+	return DB.QueryRowContext(ctx, q, expanded...)
+}
+
+// Query executes a query returning rows, with dialect adaptation.
+func Query(query string, args ...interface{}) (*sql.Rows, error) {
+	q, expanded := QArgs(query, args...)
+	return DB.Query(q, expanded...)
+}
+
+// QueryContext is the context-aware variant of Query.
+func QueryContext(ctx context.Context, query string, args ...interface{}) (*sql.Rows, error) {
+	q, expanded := QArgs(query, args...)
+	return DB.QueryContext(ctx, q, expanded...)
+}
+
+// Exec executes a statement with dialect adaptation.
+func Exec(query string, args ...interface{}) (sql.Result, error) {
+	q, expanded := QArgs(query, args...)
+	return DB.Exec(q, expanded...)
+}
+
+// ExecContext is the context-aware variant of Exec.
+func ExecContext(ctx context.Context, query string, args ...interface{}) (sql.Result, error) {
+	q, expanded := QArgs(query, args...)
+	return DB.ExecContext(ctx, q, expanded...)
+}
+
 // InsertReturningID executes an INSERT … RETURNING id query.
 // On Postgres it uses RETURNING directly.
 // On SQLite it strips RETURNING, generates a UUID, prepends it to the args,

server/database/compat_test.go

@@ -0,0 +1,129 @@
+package database
+
+import (
+	"testing"
+)
+
+func TestQArgs_Postgres(t *testing.T) {
+	// Force Postgres dialect for test
+	CurrentDialect = DialectPostgres
+	defer func() { CurrentDialect = DialectSQLite }()
+
+	q, args := QArgs("SELECT * FROM t WHERE a = $1 AND b = $1", "val1")
+	if q != "SELECT * FROM t WHERE a = $1 AND b = $1" {
+		t.Errorf("Postgres: query should be unchanged, got %q", q)
+	}
+	if len(args) != 1 {
+		t.Errorf("Postgres: args should be unchanged, got %d", len(args))
+	}
+}
+
+func TestQArgs_SQLite_ReusedPlaceholder(t *testing.T) {
+	CurrentDialect = DialectSQLite
+	defer func() { CurrentDialect = DialectPostgres }()
+
+	// $1 appears twice — must be expanded to two ? with the same arg value
+	q, args := QArgs(
+		"SELECT COUNT(*) FROM channels c WHERE c.user_id = $1 OR c.id IN (SELECT channel_id FROM cp WHERE cp.user_id = $1) AND c.is_archived = $2",
+		"user-123", false,
+	)
+
+	// Should have 3 ? placeholders
+	qCount := 0
+	for _, c := range q {
+		if c == '?' {
+			qCount++
+		}
+	}
+	if qCount != 3 {
+		t.Errorf("expected 3 ? placeholders, got %d in %q", qCount, q)
+	}
+
+	// Should have 3 args: user-123, user-123, false
+	if len(args) != 3 {
+		t.Fatalf("expected 3 args, got %d: %v", len(args), args)
+	}
+	if args[0] != "user-123" {
+		t.Errorf("args[0] = %v, want user-123", args[0])
+	}
+	if args[1] != "user-123" {
+		t.Errorf("args[1] = %v, want user-123 (expanded from reused $1)", args[1])
+	}
+	if args[2] != false {
+		t.Errorf("args[2] = %v, want false", args[2])
+	}
+
+	// Should not contain $N
+	if contains(q, "$") {
+		t.Errorf("query still contains $: %q", q)
+	}
+}
+
+func TestQArgs_SQLite_NoReuse(t *testing.T) {
+	CurrentDialect = DialectSQLite
+	defer func() { CurrentDialect = DialectPostgres }()
+
+	q, args := QArgs("SELECT * FROM t WHERE a = $1 AND b = $2", "a", "b")
+
+	qCount := 0
+	for _, c := range q {
+		if c == '?' {
+			qCount++
+		}
+	}
+	if qCount != 2 {
+		t.Errorf("expected 2 ? placeholders, got %d", qCount)
+	}
+	if len(args) != 2 {
+		t.Errorf("expected 2 args, got %d", len(args))
+	}
+}
+
+func TestQArgs_SQLite_ILIKE(t *testing.T) {
+	CurrentDialect = DialectSQLite
+	defer func() { CurrentDialect = DialectPostgres }()
+
+	q, _ := QArgs("SELECT * FROM t WHERE name ILIKE $1", "%test%")
+	if contains(q, "ILIKE") {
+		t.Errorf("ILIKE should be rewritten to LIKE: %q", q)
+	}
+	if !contains(q, "LIKE") {
+		t.Errorf("should contain LIKE: %q", q)
+	}
+}
+
+func TestQArgs_SQLite_HighPlaceholders(t *testing.T) {
+	CurrentDialect = DialectSQLite
+	defer func() { CurrentDialect = DialectPostgres }()
+
+	q, args := QArgs(
+		"INSERT INTO t (a,b,c,d,e,f,g,h,i,j,k) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11)",
+		1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11,
+	)
+
+	qCount := 0
+	for _, c := range q {
+		if c == '?' {
+			qCount++
+		}
+	}
+	if qCount != 11 {
+		t.Errorf("expected 11 ?, got %d in %q", qCount, q)
+	}
+	if len(args) != 11 {
+		t.Errorf("expected 11 args, got %d", len(args))
+	}
+	// $10 and $11 should not be mangled
+	if contains(q, "$") {
+		t.Errorf("still contains $: %q", q)
+	}
+}
+
+func contains(s, substr string) bool {
+	for i := 0; i <= len(s)-len(substr); i++ {
+		if s[i:i+len(substr)] == substr {
+			return true
+		}
+	}
+	return false
+}

server/database/database.go

@@ -95,9 +95,9 @@ func connectSQLite(dsn string) error {
 	// _time_format tells the driver how to convert between Go time.Time and SQLite TEXT timestamps.
 	connStr := dsn
 	if dsn != ":memory:" && !strings.Contains(dsn, "?") {
-		connStr = fmt.Sprintf("file:%s?_pragma=journal_mode%%3Dwal&_pragma=busy_timeout%%3D5000&_pragma=foreign_keys%%3Don&_time_format=2006-01-02T15%%3A04%%3A05Z", dsn)
+		connStr = fmt.Sprintf("file:%s?_pragma=journal_mode%%3Dwal&_pragma=busy_timeout%%3D5000&_pragma=foreign_keys%%3Don", dsn)
 	} else if dsn == ":memory:" {
-		connStr = "file::memory:?_pragma=foreign_keys%%3Don&_time_format=2006-01-02T15%%3A04%%3A05Z"
+		connStr = "file::memory:?_pragma=foreign_keys%%3Don"
 	}
 
 	var err error

server/database/migrations/019_auth_providers.sql

@@ -0,0 +1,30 @@
+-- ==========================================
+-- Chat Switchboard — 019 Auth Providers
+-- ==========================================
+-- OIDC authorization state tracking.
+-- Group source column for OIDC-synced groups.
+-- ICD §1 (Users), §2 (Groups) — v0.24.1
+-- ==========================================
+
+-- ── OIDC authorization state (short-lived) ───────────────────────
+-- Stores state + nonce for the authorization code flow.
+-- Rows deleted after callback completes. Stale rows cleaned by age.
+CREATE TABLE IF NOT EXISTS oidc_auth_state (
+    state       TEXT PRIMARY KEY,
+    nonce       TEXT NOT NULL,
+    redirect_to TEXT,
+    created_at  TIMESTAMPTZ NOT NULL DEFAULT NOW()
+);
+
+CREATE INDEX IF NOT EXISTS idx_oidc_state_created
+    ON oidc_auth_state(created_at);
+
+-- ── Group source tracking ────────────────────────────────────────
+-- Distinguishes manually-created groups from OIDC-synced groups.
+-- OIDC groups are managed exclusively by the sync process.
+ALTER TABLE groups
+    ADD COLUMN IF NOT EXISTS source VARCHAR(20) NOT NULL DEFAULT 'manual'
+        CHECK (source IN ('manual', 'oidc'));
+
+COMMENT ON TABLE oidc_auth_state IS 'Ephemeral OIDC authorization code flow state. Rows deleted after callback.';
+COMMENT ON COLUMN groups.source IS 'manual=admin-created, oidc=synced from IdP groups claim';

server/database/migrations/sqlite/019_auth_providers.sql

@@ -0,0 +1,13 @@
+-- Chat Switchboard — 019 Auth Providers (SQLite) (v0.24.1)
+
+CREATE TABLE IF NOT EXISTS oidc_auth_state (
+    state       TEXT PRIMARY KEY,
+    nonce       TEXT NOT NULL,
+    redirect_to TEXT,
+    created_at  TEXT NOT NULL DEFAULT (datetime('now'))
+);
+
+CREATE INDEX IF NOT EXISTS idx_oidc_state_created
+    ON oidc_auth_state(created_at);
+
+ALTER TABLE groups ADD COLUMN source TEXT NOT NULL DEFAULT 'manual';