Merge branch 'security-hardening'

Security review fixes: SQL-injection sinks, SET ROLE quoting, empty-JWT-secret
hard-fail, TLS fail-closed, 0600 config file, plus regression tests and docs.

Author: sted

CHANGELOG.md

@@ -1,5 +1,14 @@
 # Change Log
 
+## Unreleased
+
+### Security
+* **SQL injection** — closed four sinks where request tokens were interpolated into SQL with bare quotes instead of being escaped/parameterized (a backslash escape in the parser let a client smuggle a literal `'`/`"`): the `select` alias, embedded-resource filter values, full-text-search config arguments, and JSON-path members. Top-level filter values and identifiers were already parameterized and unaffected.
+* **`SET ROLE`** — the JWT `role` claim is now quoted as an identifier so it cannot break out of the statement.
+* **Empty JWT secret** — the server now refuses to start when authentication is enabled and `JWTSecret` is empty (an empty HMAC key lets anyone forge tokens); previously only a warning. Debug mode auto-generates a random secret.
+* **TLS fail-closed** — a configured certificate that fails to load is now a fatal startup error instead of silently serving plaintext HTTP.
+* **Config file permissions** — written `0600` (was `0777`), since it holds the JWT secret and the database password.
+
 ## 0.8.0 - 2026-07-07
 
 ### Added

README.md

@@ -133,6 +133,9 @@ SmoothDB supports two authentication methods via the `LoginMode` configuration o
 
 - No explicit authorization step is needed beyond providing the JWT token with each request
 - Both authentication methods require email and password for token generation through the `/token` endpoint
+- `JWTSecret` must be set whenever authentication is enabled (`LoginMode` other than `none`): the server refuses to start with an empty secret, because an empty HMAC key would let anyone forge a token for any role. Set it in the configuration file or via the `SMOOTHDB_JWT_SECRET` environment variable. In debug mode (`SMOOTHDB_DEBUG=true`) a random secret is generated automatically for the run.
+- When TLS is configured (`CertFile`/`KeyFile`), a certificate that fails to load is a fatal startup error - SmoothDB will not silently fall back to plaintext HTTP.
+- The configuration file holds secrets (the JWT secret and the database URL with its password); it is written with `0600` permissions. Keep it that way and out of version control.
 
 We will omit the Authorization header in the following examples.
 
@@ -407,6 +410,14 @@ GET /api/testdb/employees?id=start.1&manager_id=recurse.3 HTTP/1.1
 
 Returns row `id=1` and its descendants up to 3 levels deep, following `manager_id → id`. Use `recurse.all` (or bare `recurse`) for unlimited depth (capped by `MaxRecursiveDepth`), and `after` instead of `start` to exclude the seed row.
 
+**Walking upward (`recurse!up`).** By default recursion follows the FK toward descendants. Add the `!up` hint to walk toward ancestors instead - the chain from a node up to the root:
+
+```http
+GET /api/testdb/employees?id=start.5&manager_id=recurse!up.all HTTP/1.1   # employee 5 and its manager chain
+```
+
+`recurse!up` is single-table only; it is rejected with `via()`, where the same reversal is expressed by swapping the edge's source/target columns.
+
 **Result vs. traversal filters.** A plain filter restricts the *result*: the whole subtree is walked, then non-matching rows are dropped. Prefix a filter with `walk.` to prune the *traversal* instead - a non-matching node, and everything beyond it, is skipped:
 
 ```http

config/config.go

@@ -96,10 +96,17 @@ func SaveConfig[T any](config T, configFile string) error {
 	if err != nil {
 		return fmt.Errorf("error writing the configuration file (%w)", err)
 	}
-	err = os.WriteFile(configFile, b, 0777)
+	// The config file holds secrets (JWT secret, DB URL with password), so it
+	// must not be group/world readable or writable.
+	err = os.WriteFile(configFile, b, 0600)
 	if err != nil {
 		return fmt.Errorf("error writing the configuration file (%w)", err)
 	}
+	// WriteFile does not change the mode of a pre-existing file, so tighten it
+	// explicitly to remediate files previously created world-accessible.
+	if err = os.Chmod(configFile, 0600); err != nil {
+		return fmt.Errorf("error securing the configuration file (%w)", err)
+	}
 	return nil
 }
 

config/config_test.go

@@ -0,0 +1,30 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// The config file holds secrets (JWT secret and the DB URL with its password),
+// so it must not be world-readable/writable. Regression test for the previous
+// 0777 mode.
+func TestSaveConfigPermissions(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "config.jsonc")
+
+	type cfg struct {
+		JWTSecret string `comment:"secret"`
+	}
+	if err := SaveConfig(&cfg{JWTSecret: "s3cr3t"}, path); err != nil {
+		t.Fatalf("SaveConfig: %v", err)
+	}
+
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatalf("stat: %v", err)
+	}
+	if perm := info.Mode().Perm(); perm != 0o600 {
+		t.Errorf("config file mode = %o, want 600 (secrets must not be group/world accessible)", perm)
+	}
+}

database/conn.go

@@ -43,7 +43,9 @@ func PrepareConnection(ctx context.Context, conn *DbPoolConn, role string, claim
 
 	// set role and other configurations only on the first acquire
 	if newAcquire && role != "" {
-		_, err := conn.Exec(ctx, "SET ROLE "+role)
+		// role comes from the JWT claim; quote it as an identifier so it cannot
+		// break out of the SET ROLE statement (it is otherwise unvalidated).
+		_, err := conn.Exec(ctx, "SET ROLE "+quote(role))
 		if err != nil {
 			return err
 		}

database/injection_test.go

@@ -0,0 +1,96 @@
+package database
+
+import (
+	"net/url"
+	"testing"
+)
+
+// These are regression tests for SQL-injection sinks where user-controlled
+// tokens were interpolated into SQL text with bare quotes instead of being
+// escaped/parameterized. The request parser lets a client smuggle a literal
+// ' or " (and even separators) into a token via a backslash escape, so every
+// interpolation site must quote/escape its input.
+//
+// Each case asserts the SAFE (properly escaped) SQL: the doubled quote is the
+// evidence that the breakout attempt was neutralized.
+
+func buildFromQuery(t *testing.T, query string) (string, []any) {
+	t.Helper()
+	u, err := url.Parse(query)
+	if err != nil {
+		t.Fatal(err)
+	}
+	parts, err := PostgRestParser{}.parse("table", u.Query())
+	if err != nil {
+		t.Fatalf("parse error for %q: %v", query, err)
+	}
+	sql, values, err := DirectQueryBuilder{}.BuildSelect("table", parts, &QueryOptions{}, nil)
+	if err != nil {
+		t.Fatalf("build error for %q: %v", query, err)
+	}
+	return sql, values
+}
+
+// Finding 4: the select alias (AS "...") was built with raw quotes, so a "
+// smuggled into the label broke out of the identifier into the SELECT list.
+func TestInjectionSelectAliasQuoting(t *testing.T) {
+	// label token becomes: x"    (the \" is an escaped literal double-quote)
+	sql, _ := buildFromQuery(t, `?select=x\":a`)
+	want := `SELECT "table"."a" AS "x""" FROM "table"`
+	if sql != want {
+		t.Errorf("alias not escaped\n want: %s\n  got: %s", want, sql)
+	}
+}
+
+// Finding 3: JSON-path members were wrapped in bare single quotes, so a '
+// smuggled into a path member broke out of the string literal.
+func TestInjectionJSONPathQuoting(t *testing.T) {
+	// path member token becomes: x'y
+	sql, _ := buildFromQuery(t, `?select=data->>x\'y`)
+	want := `SELECT ("table"."data"->>'x''y') AS "x'y" FROM "table"`
+	if sql != want {
+		t.Errorf("json path member not escaped\n want: %s\n  got: %s", want, sql)
+	}
+}
+
+// Finding 2: full-text-search config arguments were interpolated with bare
+// single quotes, so a ' in the fts(config) argument broke out of the literal.
+func TestInjectionFTSConfigQuoting(t *testing.T) {
+	// fts config token becomes: en'x
+	sql, values := buildFromQuery(t, `?body=fts(en\'x).cat`)
+	want := `SELECT * FROM "table" WHERE "table"."body" @@ to_tsquery('en''x', $1)`
+	if sql != want {
+		t.Errorf("fts config not escaped\n want: %s\n  got: %s", want, sql)
+	}
+	if len(values) != 1 || values[0] != "cat" {
+		t.Errorf("fts value should be parameterized, got %v", values)
+	}
+}
+
+// SET ROLE takes the role verbatim from the JWT claim. It is built by
+// concatenation (PrepareConnection needs a live connection, so this locks in the
+// quoting contract that line relies on): a malicious role must stay a single
+// quoted identifier and cannot break out into additional statements.
+func TestInjectionSetRoleQuoting(t *testing.T) {
+	cases := map[string]string{
+		`postgres; DROP TABLE users; --`: `SET ROLE "postgres; DROP TABLE users; --"`,
+		`admin" ; DROP`:                  `SET ROLE "admin"" ; DROP"`,
+	}
+	for role, want := range cases {
+		got := "SET ROLE " + quote(role)
+		if got != want {
+			t.Errorf("role %q not neutralized\n want: %s\n  got: %s", role, want, got)
+		}
+	}
+}
+
+// Finding 1: embedded-resource filter values take the nmarker == -1 branch of
+// appendValue, which interpolated the value with bare single quotes instead of
+// using a $N placeholder. This exercises that branch directly.
+func TestInjectionEmbeddedValueEscaping(t *testing.T) {
+	where, _, _ := appendValue("", `x' OR '1'='1`, nil, -1, false)
+	want := `'x'' OR ''1''=''1'`
+	if where != want {
+		t.Errorf("interpolated value not escaped\n want: %s\n  got: %s", want, where)
+	}
+}

database/querybuilder.go

@@ -113,7 +113,7 @@ func prepareField(table, schema string, sfield SelectField, info *SchemaInfo) st
 	}
 
 	if sfield.label != "" {
-		fieldPart += " AS \"" + sfield.label + "\""
+		fieldPart += " AS " + quote(sfield.label)
 	}
 	return fieldPart
 }
@@ -578,7 +578,7 @@ func appendValue(where, value string, valueList []any, nmarker int, forceParam b
 		where += "$" + strconv.Itoa(nmarker)
 		valueList = append(valueList, value)
 	} else {
-		where += "'" + value + "'"
+		where += quoteLit(value)
 	}
 	return where, valueList, nmarker
 }
@@ -687,7 +687,7 @@ func whereClause(table, schema, label string, node *WhereConditionNode, nmarker
 					if ct != nil && ct.Type != "tsvector" {
 						lang := "'english'"
 						if len(node.opArgs) > 0 {
-							lang = "'" + node.opArgs[0] + "'"
+							lang = quoteLit(node.opArgs[0])
 						}
 						where = where[:len(where)-len(fieldname)] + "to_tsvector(" + lang + ", " + fieldname + ")"
 					}
@@ -715,7 +715,7 @@ func whereClause(table, schema, label string, node *WhereConditionNode, nmarker
 					where += "websearch_to_tsquery("
 				}
 				for _, arg := range node.opArgs {
-					where += "'" + arg + "'"
+					where += quoteLit(arg)
 					where += ", "
 				}
 				where, valueList, _ = appendValue(where, node.values[0], valueList, nmarker, false)

database/requestparser.go

@@ -424,7 +424,7 @@ func (p *PostgRestParser) field(mayHaveTable bool, mayBeEmpty bool) (f Field, er
 		if _, err = strconv.Atoi(token); err == nil {
 			f.jsonPath += token
 		} else {
-			f.jsonPath += "'" + token + "'"
+			f.jsonPath += quoteLit(token)
 			f.last = token
 		}
 		token = p.lookAhead()

server/config.go

@@ -2,6 +2,8 @@ package server
 
 import (
 	"bufio"
+	"crypto/rand"
+	"encoding/hex"
 	"flag"
 	"fmt"
 	"os"
@@ -105,6 +107,12 @@ func getEnvironment(c *Config) {
 		c.Logging.Level = "trace"
 		c.Logging.StdOut = true
 		c.EnableDebugRoute = true
+		// Debug mode enables auth (LoginMode: db) but shouldn't force the operator
+		// to set a secret by hand. Never fall back to the empty (forgeable) key:
+		// generate a strong random secret for this run.
+		if c.JWTSecret == "" {
+			c.JWTSecret = randomSecret()
+		}
 	}
 	allowAnon := os.Getenv("SMOOTHDB_ALLOW_ANON")
 	if allowAnon != "" {
@@ -228,13 +236,24 @@ func getConfig(baseConfig map[string]any, configOpts *ConfigOptions) (*Config, e
 	return cfg, nil
 }
 
+// randomSecret returns a 32-byte cryptographically-random secret, hex-encoded.
+func randomSecret() string {
+	b := make([]byte, 32)
+	if _, err := rand.Read(b); err != nil {
+		// crypto/rand should never fail; if it does, refuse to produce a weak key.
+		panic("cannot generate a random JWT secret: " + err.Error())
+	}
+	return hex.EncodeToString(b)
+}
+
 func checkConfig(cfg *Config) error {
 	if cfg.ShortAPIURL && len(cfg.Database.AllowedDatabases) != 1 {
 		fmt.Println("Warning: 'ShortAPIURL' requires a single db in 'Database.AllowedDatabases'")
 		cfg.ShortAPIURL = false
 	}
 	if cfg.LoginMode != "none" && cfg.JWTSecret == "" {
-		fmt.Println("Warning: 'JWTSecret' is empty while authentication is enabled (LoginMode:", cfg.LoginMode+")")
+		return fmt.Errorf("'JWTSecret' is empty while authentication is enabled (LoginMode: %s): "+
+			"an empty secret lets anyone forge tokens for any role; set 'JWTSecret' or SMOOTHDB_JWT_SECRET", cfg.LoginMode)
 	}
 	if cfg.CORSAllowCredentials {
 		for _, origin := range cfg.CORSAllowedOrigins {

server/config_test.go

@@ -0,0 +1,42 @@
+package server
+
+import (
+	"strings"
+	"testing"
+)
+
+// An empty JWT secret with authentication enabled means tokens are signed/verified
+// with an empty HMAC key — anyone can forge a token for any role. The server must
+// refuse to start rather than only warn.
+func TestCheckConfigRejectsEmptyJWTSecret(t *testing.T) {
+	cfg := defaultConfig()
+	cfg.LoginMode = "jwt"
+	cfg.JWTSecret = ""
+
+	err := checkConfig(cfg)
+	if err == nil {
+		t.Fatal("expected checkConfig to reject an empty JWTSecret with auth enabled")
+	}
+	if !strings.Contains(err.Error(), "JWTSecret") {
+		t.Errorf("expected error to mention JWTSecret, got: %v", err)
+	}
+}
+
+// Debug mode is a dev convenience and should not require the operator to set a
+// secret by hand — but it must not fall back to the empty (forgeable) key. It
+// should auto-generate a strong random secret.
+func TestDebugModeGeneratesJWTSecret(t *testing.T) {
+	t.Setenv("SMOOTHDB_DEBUG", "true")
+	t.Setenv("SMOOTHDB_JWT_SECRET", "")
+
+	cfg := defaultConfig()
+	cfg.JWTSecret = ""
+	getEnvironment(cfg)
+
+	if cfg.JWTSecret == "" {
+		t.Fatal("debug mode should auto-generate a JWTSecret instead of leaving it empty")
+	}
+	if len(cfg.JWTSecret) < 32 {
+		t.Errorf("auto-generated secret too short (%d chars); want a strong random value", len(cfg.JWTSecret))
+	}
+}