diff --git a/cmd/config.go b/cmd/config.go index 41440f7b..0226eb54 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -42,7 +42,11 @@ var settingsListCmd = &cobra.Command{ Short: "List every declared key's environment value, stored override, and next-boot winner", Args: cobra.MaximumNArgs(1), Run: func(cmd *cobra.Command, args []string) { - svc, closeDB, specs := connectSettings() + svc, closeDB, specs, err := connectSettings() + if err != nil { + fmt.Fprintln(os.Stderr, "config: "+err.Error()) + os.Exit(1) + } defer closeDB() rows, err := svc.List(context.Background(), specs) @@ -72,7 +76,11 @@ var settingsSetCmd = &cobra.Command{ Short: "Store a value as an override, parsed against its key's declared type", Args: cobra.ExactArgs(2), Run: func(cmd *cobra.Command, args []string) { - svc, closeDB, specs := connectSettings() + svc, closeDB, specs, err := connectSettings() + if err != nil { + fmt.Fprintln(os.Stderr, "config: "+err.Error()) + os.Exit(1) + } defer closeDB() key, raw := args[0], args[1] @@ -89,7 +97,11 @@ var settingsClearCmd = &cobra.Command{ Short: "Delete a stored override", Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { - svc, closeDB, _ := connectSettings() + svc, closeDB, _, err := connectSettings() + if err != nil { + fmt.Fprintln(os.Stderr, "config: "+err.Error()) + os.Exit(1) + } defer closeDB() key := args[0] @@ -109,8 +121,11 @@ var settingsClearCmd = &cobra.Command{ // connectSettings loads every installed integration's ConfigSpec and opens // a plain database connection, the same way config validate does (design // D10): no migrations, no server, just enough to read and write -// core.integration_config_overrides through the settings service. -func connectSettings() (*integration.SettingsService, func(), []config.ConfigKey) { +// core.integration_config_overrides through the settings service. It resolves +// the database's -file keys first (resolveDatabaseSecretFiles), and returns an +// error instead of exiting so a test can call it; the three Run functions +// print it and exit. +func connectSettings() (*integration.SettingsService, func(), []config.ConfigKey, error) { var specs []config.ConfigKey for _, integ := range integrations.All() { if cp, ok := integ.(config.ConfigProvider); ok { @@ -122,12 +137,14 @@ func connectSettings() (*integration.SettingsService, func(), []config.ConfigKey if logging.AppLogger != nil { logger = logging.AppLogger } + if err := resolveDatabaseSecretFiles(); err != nil { + return nil, nil, nil, err + } database, closeDB, err := openDB(context.Background(), logger, dbPlain) if err != nil { - fmt.Fprintln(os.Stderr, "config: connecting to the database: "+err.Error()) - os.Exit(1) + return nil, nil, nil, fmt.Errorf("connecting to the database: %w", err) } - return integration.NewSettingsService(database), closeDB, specs + return integration.NewSettingsService(database), closeDB, specs, nil } // integrationKeyNames returns the ConfigSpec names declared by the diff --git a/cmd/database.go b/cmd/database.go index c1cd5b4d..34938069 100644 --- a/cmd/database.go +++ b/cmd/database.go @@ -6,6 +6,7 @@ package cmd import ( "context" "database/sql" + "errors" "log/slog" "git.coopcloud.tech/wiki-cafe/member-console/internal/db" @@ -25,8 +26,18 @@ const ( // openDB opens the database the db-dsn setting names, in the given mode. The // returned closer is for the caller to defer; it logs a failed Close at Warn, // because by then the command has nothing left to do with the error. +// +// It does not resolve the -file keys: a second resolution after +// resolveSecretFiles would report the pair as ambiguous, so each command +// resolves once before it calls openDB. It refuses an empty db-dsn, so a +// command that forgot to resolve fails here and pgx never connects with +// libpq's defaults. func openDB(ctx context.Context, logger *slog.Logger, mode dbMode) (*sql.DB, func(), error) { - dbConfig := db.DefaultDBConfig(viper.GetString("db-dsn")) + dsn := viper.GetString("db-dsn") + if dsn == "" { + return nil, nil, errors.New("db-dsn is not set (MC_DB_DSN or MC_DB_DSN_FILE)") + } + dbConfig := db.DefaultDBConfig(dsn) var database *sql.DB var err error if mode == dbMigrated { diff --git a/cmd/database_files_test.go b/cmd/database_files_test.go new file mode 100644 index 00000000..05c2f6c3 --- /dev/null +++ b/cmd/database_files_test.go @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial +// SPDX-FileCopyrightText: 2025-2026 Christian Galo + +package cmd + +import ( + "net" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/viper" +) + +// databaseCommandPath is one way a command opens the database. run executes +// the path and returns the error it ends with. +type databaseCommandPath struct { + name string + run func() error +} + +func databaseCommandPaths() []databaseCommandPath { + return []databaseCommandPath{ + {"migrate up", func() error { return migrateUpCmd.RunE(migrateUpCmd, nil) }}, + {"migrate down", func() error { return migrateDownCmd.RunE(migrateDownCmd, nil) }}, + {"migrate status", func() error { return migrateStatusCmd.RunE(migrateStatusCmd, nil) }}, + {"connectSettings", func() error { + _, _, _, err := connectSettings() + return err + }}, + {"seed-demo", func() error { return seedDemoCmd.RunE(&cobra.Command{}, nil) }}, + } +} + +// closedPort returns a loopback port that nothing listens on. +func closedPort(t *testing.T) string { + t.Helper() + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + port := strconv.Itoa(l.Addr().(*net.TCPAddr).Port) + if err := l.Close(); err != nil { + t.Fatalf("close listener: %v", err) + } + return port +} + +const dbDSNNotSet = "db-dsn is not set (MC_DB_DSN or MC_DB_DSN_FILE)" + +// checkFileKeyRead sets only db-dsn-file and asserts that the path read the +// file: its error is the connection failure to the file's port. +func checkFileKeyRead(t *testing.T, p databaseCommandPath, port string) { + t.Helper() + viper.Reset() + t.Cleanup(viper.Reset) + + path := filepath.Join(t.TempDir(), "db-dsn") + dsn := "postgres://u:p@127.0.0.1:" + port + "/x?connect_timeout=1" + if err := os.WriteFile(path, []byte(dsn+"\n"), 0o600); err != nil { + t.Fatalf("write fixture file: %v", err) + } + viper.Set("db-dsn-file", path) + + err := p.run() + if err == nil { + t.Fatalf("%s: want a connection failure, got nil", p.name) + } + if strings.Contains(err.Error(), dbDSNNotSet) { + t.Errorf("%s: error = %q, want a connection failure, not the unset refusal", p.name, err.Error()) + } + if !strings.Contains(err.Error(), port) { + t.Errorf("%s: error = %q, want a connection failure that names port %s", p.name, err.Error(), port) + } +} + +// checkUnsetRefused sets neither key and asserts the unset refusal. +func checkUnsetRefused(t *testing.T, p databaseCommandPath) { + t.Helper() + viper.Reset() + t.Cleanup(viper.Reset) + + err := p.run() + if err == nil { + t.Fatalf("%s: want the unset refusal, got nil", p.name) + } + if !strings.Contains(err.Error(), dbDSNNotSet) { + t.Errorf("%s: error = %q, want it to contain %q", p.name, err.Error(), dbDSNNotSet) + } +} + +// TestDatabaseCommandsReadTheFileKeys covers task 2.4 (design D12): every +// command that opens the database resolves db-dsn-file first, and one that +// finds no connection string refuses instead of letting the driver fall back +// to its defaults. +func TestDatabaseCommandsReadTheFileKeys(t *testing.T) { + // viper.Reset drops the environment binding, so the rows never read + // MC_DB_DSN; blanking it as well keeps migrate down, which this test runs, + // off a real database even if that ever changes. + t.Setenv("MC_DB_DSN", "") + t.Setenv("MC_DB_DSN_FILE", "") + port := closedPort(t) + for _, p := range databaseCommandPaths() { + t.Run("file key read: "+p.name, func(t *testing.T) { + checkFileKeyRead(t, p, port) + }) + } + for _, p := range databaseCommandPaths() { + t.Run("neither key set: "+p.name, func(t *testing.T) { + checkUnsetRefused(t, p) + }) + } +} diff --git a/cmd/migrate.go b/cmd/migrate.go index b72d62f8..6b7ada37 100644 --- a/cmd/migrate.go +++ b/cmd/migrate.go @@ -29,6 +29,11 @@ var migrateUpCmd = &cobra.Command{ logger = logging.AppLogger } + if err := resolveDatabaseSecretFiles(); err != nil { + logger.Error("configuration error", slog.Any("error", err)) + return err + } + database, closeDB, err := openDB(context.Background(), logger, dbPlain) if err != nil { logger.Error("failed to connect to database", slog.Any("error", err)) @@ -56,6 +61,11 @@ var migrateDownCmd = &cobra.Command{ logger = logging.AppLogger } + if err := resolveDatabaseSecretFiles(); err != nil { + logger.Error("configuration error", slog.Any("error", err)) + return err + } + database, closeDB, err := openDB(context.Background(), logger, dbPlain) if err != nil { logger.Error("failed to connect to database", slog.Any("error", err)) @@ -83,6 +93,11 @@ var migrateStatusCmd = &cobra.Command{ logger = logging.AppLogger } + if err := resolveDatabaseSecretFiles(); err != nil { + logger.Error("configuration error", slog.Any("error", err)) + return err + } + database, closeDB, err := openDB(context.Background(), logger, dbPlain) if err != nil { logger.Error("failed to connect to database", slog.Any("error", err)) diff --git a/cmd/seed_demo.go b/cmd/seed_demo.go index 1942a5b4..886c4769 100644 --- a/cmd/seed_demo.go +++ b/cmd/seed_demo.go @@ -32,6 +32,11 @@ the org-type default) to exercise the OIDC lazy-creation path. See OpenSpec chan logger := logging.SetupLogger(env) ctx = logging.WithContext(ctx, logger) + if err := resolveDatabaseSecretFiles(); err != nil { + logger.Error("configuration error", slog.Any("error", err)) + return err + } + database, closeDB, err := openDB(ctx, logger, dbMigrated) if err != nil { logger.Error("failed to initialize database", slog.Any("error", err)) diff --git a/cmd/start.go b/cmd/start.go index 87844178..f06c7504 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -736,25 +736,50 @@ func loadFromFile(path string) (string, error) { return strings.TrimSpace(string(data)), nil } +// databaseSecretPairs are the database's connection-string pairs. Every +// command that opens the database resolves them (resolveDatabaseSecretFiles), +// and resolveSecretFiles includes them, so a container that mounts the string +// as a file runs migrate, config and seed-demo as it runs start. +var databaseSecretPairs = []config.SecretPair{ + {Name: "db-dsn", FileName: "db-dsn-file"}, +} + // resolveSecretFiles loads every "-file" secret variant (core's own -// small, hand-listed set plus every installed integration's declared -// Secret keys, config.SecretPairsFrom) into Viper, so a file-backed -// secret (e.g. stripe-api-key-file) is present before any validation reads it. -// Both a direct value and a file path for the same key is a fatal -// misconfiguration, reported as an error rather than exiting here: shared -// between start's own boot sequence and config validate (design D10, +// small, hand-listed set, the database pairs and every installed +// integration's declared Secret keys, config.SecretPairsFrom) into Viper, so a +// file-backed secret (e.g. stripe-api-key-file) is present before any +// validation reads it. Both a direct value and a file path for the same key is +// a fatal misconfiguration, reported as an error rather than exiting here: +// shared between start's own boot sequence and config validate (design D10, // typed-config-keys), so a deployment's secret-file wiring is read -// identically by both, and each caller decides how to report the failure -// and exit. +// identically by both, and each caller decides how to report the failure and +// exit. A command resolves once: a second call after this one would see both +// the value and the file set and report the pair as ambiguous. func resolveSecretFiles(integrationConfigSpecs []config.ConfigKey) error { secretPairs := []config.SecretPair{ {Name: "oidc-sp-client-secret", FileName: "oidc-sp-client-secret-file"}, {Name: "temporal-oauth-client-secret", FileName: "temporal-oauth-client-secret-file"}, {Name: "valkey-password", FileName: "valkey-password-file"}, - {Name: "db-dsn", FileName: "db-dsn-file"}, } + secretPairs = append(secretPairs, databaseSecretPairs...) secretPairs = append(secretPairs, config.SecretPairsFrom(integrationConfigSpecs)...) - for _, pair := range secretPairs { + return resolveSecretPairs(secretPairs) +} + +// resolveDatabaseSecretFiles resolves the database's pairs alone. migrate, +// config list|set|clear and seed-demo call it before openDB; they do not +// resolve the other secrets, because a one-shot migration container may mount +// only the database's secret while its environment names other secret files. +// start and config validate call resolveSecretFiles instead, which includes +// these pairs. +func resolveDatabaseSecretFiles() error { + return resolveSecretPairs(databaseSecretPairs) +} + +// resolveSecretPairs loads the file of each pair into the pair's key, and +// reports a pair with both a value and a file as ambiguous. +func resolveSecretPairs(pairs []config.SecretPair) error { + for _, pair := range pairs { value := viper.GetString(pair.Name) file := viper.GetString(pair.FileName) // Both a direct value and a file path is ambiguous. diff --git a/openspec/changes/runtime-database-user/design.md b/openspec/changes/runtime-database-user/design.md index fe8e015a..1ef4a88a 100644 --- a/openspec/changes/runtime-database-user/design.md +++ b/openspec/changes/runtime-database-user/design.md @@ -60,11 +60,13 @@ The process that migrates at startup holds the migration password for as long as The start-up check (D9) runs most of its checks after the migrations, on the runtime connection, because they read objects the migrations create. Five of its checks read nothing the migrations create: whether both connection strings log in as one user, whether they reach one database, whether the runtime user is a superuser or a member of a superuser role, whether it owns the database or is a member of the role that does, and whether it can create objects in `public` or in a schema of the five streams, or owns an object in `public` (check 12). `start` runs those five before it migrates. A deploy whose `db-dsn` is wrong then refuses without changing the schema, so the previous release, which may still be serving during a rolling deploy, is not left on a schema migrated by a process that never started. The superuser check also runs before the boot grant, so the console never grants `console_runtime` to a superuser. -**The qualification principle.** SQL that the console runs on the migration connection, and SQL whose output it prints as a statement for the operator to run, resolves no unqualified function and no unqualified relation. Every function is written `pg_catalog.`, every catalog relation `pg_catalog.`, and a goose ledger `public.`. The hole is one shape in several places: a role that can create an object in a schema on the search path (a database's owner through `public`, a user granted `CREATE`) plants a function whose argument types match a call exactly, Postgres prefers that function to the catalog's whatever the path, and the planted code then runs with the privilege of the session that made the call, or writes the statement an operator runs as the migration user. The principle applies to the boot grant (D8), to every query of the start-up check on both connections (D9), because a refusal's output is a statement the operator runs, and to the pending check (D10). The migrations' own SQL is the shipped files, whose name resolution the pool's `search_path=public` and check 12 bound. The scope is functions and relations: operators are not listed because each operator in these queries has an exact-type entry in `pg_catalog`, which the search path reaches first. A structural test scans the check queries, the boot-grant statement and the pending check's queries and fails, naming the query and the token, for a function call or a catalog relation without `pg_catalog.` (tasks 5.8); the planted-function rows of `TestOpenRuntimeDatabase` show the effect, the scan shows the cause. +**The qualification principle.** SQL that the console runs on the migration connection, and SQL whose output it prints as a statement for the operator to run, resolves no unqualified function and no unqualified relation. Every function is written `pg_catalog.`, every catalog relation `pg_catalog.`, and a goose ledger `public.`. The hole is one shape in several places: a role that can create an object in a schema on the search path (a database's owner through `public`, a user granted `CREATE`) plants a function whose argument types match a call exactly, Postgres prefers that function to the catalog's whatever the path, and the planted code then runs with the privilege of the session that made the call, or writes the statement an operator runs as the migration user. The principle applies to the boot grant (D8), to every query of the start-up check on both connections (D9), because a refusal's output is a statement the operator runs, and to the pending check (D10). The migrations' own SQL is the shipped files, whose name resolution the pool's `search_path=public` and check 12 bound. The qualification covers functions and relations. It cannot cover operators, because an operator is written between its operands and the queries compare an `oid` with an integer literal (`a.grantee = 0`) and a `name` with a `text` array element (`n.nspname = ANY($2)`), for neither of which `pg_catalog` has an exact-type operator; a planted `public.=(oid, integer)` is an exact match, wins whatever the path order, and its function runs as the session's user inside the check's own query, before the check can refuse. Qualifying each operator as `OPERATOR(pg_catalog.=)` would work but leaves a scan that must parse every operator, so operators are closed by the second line: + +**The search-path pin.** Every transaction in which the console runs a check query or the boot grant, on the migration connection and on the runtime connection, begins with `SET LOCAL search_path = pg_catalog, pg_temp`, before any other statement. With that path no planted schema is a candidate for operator or function resolution, so a planted operator never fires and the effect of every unqualified name that slips past the scan is bounded to `pg_catalog`; this is the pattern Postgres documents for privileged code. It also makes `regprocedure::text` print schema-qualified names on the runtime connection, whose startup path is `core,public`, so copies 7 and 8 name the object in full. The pin is a statement of its own, not a startup parameter, so the migrations and the pool's goose runs keep `search_path=public`; they are protected by check 12 refusing `CREATE` on `public` and by the database-ownership refusal, both before the migrations. The pin and the qualification are two lines of one defense: the pin covers operators and any name a later edit leaves bare, the qualification and its scan cover the statements whose text is printed for the operator to run (a pin does not change what a printed statement says) and keep the queries correct under any path. A structural test scans the check queries, the boot-grant statement and the pending check's queries and fails, naming the query and the token, for a function call or a catalog relation without `pg_catalog.` (tasks 5.8); it does not scan operators, which the pin covers, and `TestOpenRuntimeDatabase` and `TestCheckBeforeMigrating` each plant an operator to show the effect. The planted-function rows show the effect of the qualification, the scan shows the cause. The database-ownership check also protects the migration run itself, and four rules make that protection hold (D9, check 4): -1. **Until the runtime half of check 4 has passed, the migration connection runs one query, with no unqualified function and no unqualified relation:** `SELECT current_user, pg_catalog.current_database()`. A database's owner owns `public` through `pg_database_owner`, so it can create `public.quote_ident(name)`, which beats `pg_catalog.quote_ident(text)` for a `name` argument whatever the search path, and it can run `ALTER DATABASE ... SET search_path`. Either runs the owner's code in the migration user's superuser session. The superuser bit and the quoted name come from the runtime connection, through qualified names, so a function the runtime user planted never runs. +1. **Until the runtime half of check 4 has passed, the migration connection runs one query, with no unqualified function and no unqualified relation, in a transaction that begins with the search-path pin:** `SELECT current_user, pg_catalog.current_database()`. A database's owner owns `public` through `pg_database_owner`, so it can create `public.quote_ident(name)`, which beats `pg_catalog.quote_ident(text)` for a `name` argument whatever the search path, and it can run `ALTER DATABASE ... SET search_path`. Either runs the owner's code in the migration user's superuser session. The superuser bit and the quoted name come from the runtime connection, through qualified names, so a function the runtime user planted never runs. 2. **Both pools open with startup parameters that replace any database-level or role-level setting:** the migration pool with `search_path=public&role=none`, the runtime pool with `search_path=core,public&role=none`. A startup parameter outranks `ALTER DATABASE ... SET` and `ALTER ROLE ... SET`, and the pool's open replaces any value the connection string carries. `search_path=public` removes the case of failure row 41 and is where goose's ledgers are expected (D10). `role=none` keeps `current_user` the login: the database's owner can run `ALTER DATABASE ... SET role = ''`, which would start the migration user's session as that owner, so that the owner half of check 4 compares the owner with itself and goose runs as the owner; and the runtime login can run `ALTER ROLE CURRENT_USER SET role = 'console_runtime'`, after which `current_user` is the group role and a refusal names the wrong user. The `SET LOCAL ROLE` of the tests (D15) is unaffected, because it runs after the session starts. The older reason for a plain pool, a pgx and Postgres 18 interaction with the custom `core,public` path, concerned that path; section 4's fresh-database test migrates through the new pool and compares the resulting schema with a plain replay, so a difference fails `make test`. 3. **Every `migrate` command and `start` refuse a database whose owner is neither the migration user nor a superuser role, before goose runs** (copy 3d), `migrate status` included, because goose's `Status` creates any missing ledger in `public`. Without it, the separate-step `migrate up` would run the whole migration run against a database that the runtime login owns, and `start`'s check 4 would refuse only afterwards. 4. **The migration run waits for check 12.** A runtime user that holds `CREATE` on `public` or on a schema of the five streams, or owns a function in `public`, can plant a function that the boot grant or an unqualified call in a pending migration would run as the superuser. The qualification principle removes the boot grant's exposure; check 12 refuses the configuration before the migrations, whose own SQL the console does not control. @@ -162,6 +164,7 @@ Role memberships are cluster-wide, while a Down section runs in one database. A On the migration connection, after the migrations, the console grants `console_runtime` to the runtime user in one transaction: ```sql +SET LOCAL search_path = pg_catalog, pg_temp; SELECT pg_catalog.set_config('member_console.runtime_user', $1, true); DO $$ BEGIN @@ -173,6 +176,7 @@ $$; - **The name comes from the runtime connection** (`SELECT current_user` on it), not from parsing `db-dsn`, so a connection string whose user part is URL-encoded, defaulted by the driver or rewritten by a pooler still names the user the console serves as. - **The statement is built in the database.** The name travels as a bound parameter into a transaction-local setting, and `pg_catalog.format('%I')` quotes it, so no Go code concatenates SQL. +- **The transaction begins with the search-path pin** (D2): the migration connection's path is `public`, and the pin removes `public` from it for these statements, so nothing a role planted in `public`, an operator included, is a candidate for any name in them. - **Every function is qualified** (D2, the qualification principle): this statement runs as the migration user, a superuser, and a runtime user that can create objects in `public` could otherwise plant `public.format(text, text)`, which Postgres prefers to the catalog's `format(text, VARIADIC "any")` for these argument types. Check 12 refuses such a user before this statement runs, and the qualified names mean the statement would not call the function if one were there. - **`WITH INHERIT TRUE`** makes the grant pass privileges whatever the runtime login's own `INHERIT` attribute says. A login created `NOINHERIT` (as `console_reader` is) would otherwise get a grant through which it inherits nothing, and every query would fail on privileges. The membership check (D9) tests inheritance, `pg_has_role(..., 'USAGE')`, for the same reason, and the separate-step refusal names the same statement (copy 5). - **It runs at every start.** Granting a membership that exists from the same grantor changes nothing (Postgres reports a notice), so the grant is idempotent, and a deployment whose runtime user changes needs no hand step. @@ -180,7 +184,7 @@ $$; ### D9. The start-up check -The check refuses the start on the first failing condition, in this order, which is the numbers' order except that check 12 runs after check 4 and before check 5; it is numbered last so that the other numbers stay as they are. Every query of the check follows the qualification principle (D2): each function and catalog relation below is written with `pg_catalog.`. A superuser passes every membership test Postgres offers, so every check after the superuser check runs only for a user that is not one. The order also makes each refusal's fix reachable: the ledger read needs membership in `console_runtime`, so membership is checked first, and an owner-role membership or an owned object is reported, with the fix that names it, before the general rule that the user holds no other role. +The check refuses the start on the first failing condition, in this order, which is the numbers' order except that check 12 runs after check 4 and before check 5; it is numbered last so that the other numbers stay as they are. Every query of the check follows the qualification principle (D2): each function and catalog relation below is written with `pg_catalog.`, and each transaction that runs them begins with the search-path pin, on both connections. A superuser passes every membership test Postgres offers, so every check after the superuser check runs only for a user that is not one. The order also makes each refusal's fix reachable: the ledger read needs membership in `console_runtime`, so membership is checked first, and an owner-role membership or an owned object is reported, with the fix that names it, before the general rule that the user holds no other role. | # | Condition | Connection | When | Copy | |---|---|---|---|---| @@ -211,9 +215,12 @@ The check refuses the start on the first failing condition, in this order, which - **A failed check query refuses the start** (copy 11). A read that fails is never an answer. - On success the console logs one line naming both users (copy 19). -The queries of the runtime connection run in one `REPEATABLE READ` read-only transaction on the runtime pool so they read one snapshot. The migration connection's queries (the identity read, the owner half of check 4 and check 12) run on that pool, each in a read-only transaction of its own, before the migrations. Each query is a Go string constant whose name begins `query`; the structural test of the qualification principle (tasks 5.8) reads those constants. +The queries of the runtime connection run in one `REPEATABLE READ` read-only transaction on the runtime pool so they read one snapshot. The migration connection's queries (the identity read, the owner half of check 4 and check 12) run on that pool, each in a read-only transaction of its own, before the migrations. Each of these transactions begins with `SET LOCAL search_path = pg_catalog, pg_temp` (D2, the search-path pin), so that a planted operator or function is not a candidate for any name in a query, the superuser migration connection's queries in particular; on the runtime connection the pin also makes `regprocedure::text` print schema-qualified names, which copies 7 and 8 show. Each query is a Go string constant whose name begins `query`; the structural test of the qualification principle (tasks 5.8) reads those constants. ```sql +-- every transaction below, on both connections, begins with the pin (D2) before its first query: +SET LOCAL search_path = pg_catalog, pg_temp; + -- identity, on the migration connection, before the runtime half of check 4 passes: no unqualified function, no unqualified relation SELECT current_user, pg_catalog.current_database(); @@ -295,10 +302,11 @@ WHERE pg_catalog.pg_has_role($2::name, o.owner, 'MEMBER') ORDER BY kind, name LIMIT 1; -- 9 and 12, when the owner is not the user itself: the direct role that leads to it --- ($1 the oid the query above returned, $2 the login) +-- ($1 the oid the query above returned, $2 the login; the login is looked up by name in pg_roles, never cast +-- with regrole, which parses the value as an identifier and lowercases a mixed-case name) SELECT pg_catalog.quote_ident(d.rolname) FROM pg_catalog.pg_auth_members m JOIN pg_catalog.pg_roles d ON d.oid = m.roleid -WHERE m.member = $2::name::regrole::oid +WHERE m.member = (SELECT r.oid FROM pg_catalog.pg_roles r WHERE r.rolname = $2::name) AND pg_catalog.pg_has_role(m.roleid, $1::oid, 'MEMBER') ORDER BY d.rolname LIMIT 1; @@ -352,7 +360,7 @@ The Go side lives in `internal/db/runtime_user.go`: `RuntimeGroupRole` (the cons `db.PendingMigrations(ctx, q, sources)` returns, per stream, the embedded versions its ledger does not record as applied. For each source it lists the version numbers of the embedded files (the same parsing as `dbtest.highestVersion`), resolves the ledger with `pg_catalog.to_regclass('public.goose_db_version_')`, reads `SELECT version_id FROM public.goose_db_version_ WHERE is_applied` when `pg_catalog.has_table_privilege(current_user, 'public.goose_db_version_', 'SELECT')` holds, and compares. A missing ledger is a stream with every version pending (copy 4b); an unreadable ledger is reported as such (copy 4c), because the grant that makes it readable comes with this change's migrations and is missing on a database that has not had them. -The name is qualified on purpose. On the runtime connection's `search_path` (`core,public`), Postgres looks in the session's `pg_temp` schema before the others, so a runtime user with arbitrary SQL could create a temporary table named like a ledger and make an unqualified lookup read it. The functions are qualified for the reason of D2's qualification principle, and the pending check's queries are among those the structural test scans. The goose ledgers live in `public` because the migration connection opens with `search_path=public` (D2, rule 2). +The name is qualified on purpose. On the runtime connection's `search_path` (`core,public`), Postgres looks in the session's `pg_temp` schema before the others, so a runtime user with arbitrary SQL could create a temporary table named like a ledger and make an unqualified lookup read it. The functions are qualified for the reason of D2's qualification principle, and the pending check's queries are among those the structural test scans. The pending check's transaction begins with the search-path pin, as every check transaction does. The goose ledgers live in `public` because the migration connection opens with `search_path=public` (D2, rule 2). It does not use goose's provider: `Provider.HasPending` creates a ledger it does not find, and looks for it in `current_schema()`, which is `core` on the runtime connection. @@ -514,11 +522,13 @@ With `db-migrate-dsn` set: | 8 | runtime | checks 5 to 11 | exit 1 with copy 4 to 9, 8a, 9a or 11 | | 9 | runtime | log copy 19; the boot continues as today: overrides, boot data state (the partition function among it), Temporal, the server | as today | +Every transaction of steps 4, 5, 6 and 8 (the identity reads, the owner half of check 4, check 12, the boot grant, and checks 5 to 11 with the pending check) begins with `SET LOCAL search_path = pg_catalog, pg_temp` (D2, the search-path pin). The migrations of step 5 do not: they run with the pool's `search_path=public`. + With `db-migrate-dsn` left out: step 1; step 2; the identity read and checks 3 and 4, runtime half (copies 3, 3a, 3b, 3c); check 12 on the runtime connection (copies 3e, 7, 8); copy 18 logged; checks 5 to 11; copy 19; the boot continues. No migration runs and no grant is made. `seed-demo` runs the same sequence and then seeds. ## Failure table -Every way the switch could break something, the evidence, and how this design handles it. Rows 1 to 17 were known when the change was proposed; rows 18 to 47 were found while designing it; rows 48 to 54 were added by the review of the draft; rows 55 to 58 by the second review; rows 59 to 62 by the third. +Every way the switch could break something, the evidence, and how this design handles it. Rows 1 to 17 were known when the change was proposed; rows 18 to 47 were found while designing it; rows 48 to 54 were added by the review of the draft; rows 55 to 58 by the second review; rows 59 to 62 by the third; rows 63 and 64 by the fourth. | # | What breaks | Evidence | When it shows | Handling | |---|---|---|---|---| @@ -584,6 +594,8 @@ Every way the switch could break something, the evidence, and how this design ha | 60 | A database's owner runs `ALTER DATABASE ... SET role = ''`, so the migration user's session starts as that owner: the identity read returns the owner, the owner half compares the owner with itself and passes, and goose runs as the owner | a `role` setting applies at session start and a superuser can take any role | `start` or `migrate up` on a database owned by a role with a password outside the console | D2, rule 2: `role=none` on the migration pool; the owner-set-role row expects copy 3d and the ledgers untouched | | 61 | The runtime login runs `ALTER ROLE CURRENT_USER SET role = 'console_runtime'`, so its sessions report `current_user = console_runtime`; the boot grant fails with an unrelated error, and in the separate-step mode check 10 prints `REVOKE core_writer FROM console_runtime;`, which strips the writer from every runtime user in the cluster | the same | a runtime login with arbitrary SQL | D2, rule 2: `role=none` on the runtime pool; the self-set-role row expects a normal boot | | 62 | A runtime user holds `CREATE` on `public` (the ACL an upgraded database keeps from before Postgres 15, or a host's grant) or on a stream's schema, plants `public.format(text, text)`, and the next start's boot grant or an unqualified call in a pending migration runs it as the superuser; check 9 sees it only after the migrations | a probe on Postgres 18: the planted function ran as the superuser | a non-default configuration, plus arbitrary SQL at that time | D9 check 12, before the migrations: copy 3e for the privilege, copy 7 or 8 for an owned object in `public`; D8 calls only qualified functions | +| 63 | A runtime user that holds `CREATE` on `public` plants `public.=(oid, integer)` (or `=(name, text)`); check 12's own query, `a.grantee = 0` and `n.nspname = ANY($2)`, then runs the planted function as the superuser migration user before check 12 can refuse; qualification cannot prevent it, because `pg_catalog` has no operator for those operand types and the planted one is an exact match | a probe on Postgres 18.1: the planted operator ran as the superuser inside check 12's query, and with the pin it did not fire and the row was still returned | the configuration check 12 exists to refuse, at the start that checks it | D2, the search-path pin: every check and boot-grant transaction on both connections begins with `SET LOCAL search_path = pg_catalog, pg_temp`; `TestOpenRuntimeDatabase` and `TestCheckBeforeMigrating` plant the operator and expect no marker row and the normal refusal | +| 64 | The second query of checks 9 and 12 cast the login with `regrole`, which parses the value as an identifier: for a login such as `Mc-Rt` it fails with `role "mc-rt" does not exist`, and the refusal that should print copy 8 (or the object half of copy 3e) prints copy 11 and loses the `REVOKE` | a probe on Postgres 18.1 | any runtime login whose name needs quoting, on the path that already refuses | D9: the login is looked up with `pg_catalog.pg_roles` by `rolname = $2::name`; a mixed-case login row in `TestCheckRuntimeUser` and `TestCheckBeforeMigrating` | ## Copy @@ -675,10 +687,10 @@ Tests follow `docs/testing.md`. The tasks list each test with the breaks it must - **The partition function** (`internal/db`): `TestEnsureWebhookEventPartitions_ClosesTheGapAndIsIdempotent` runs, in one transaction it rolls back, under `SET LOCAL ROLE console_runtime`: the insert into the synthetic month fails in a savepoint, the ensure creates the four partitions, the insert lands, a second ensure creates nothing, and each created partition is owned by `core_owner`. `TestWebhookPartitionsBelongToCoreOwner` asserts the parent's and every partition's owner, the function's `SECURITY DEFINER`, owner, `search_path`, and that `PUBLIC` cannot execute it. `TestRuntimeRoleCannotCreatePartitionsDirectly` runs the partition DDL as `console_runtime` and expects `42501`. A row runs the function with 13 months and with a null time and expects `22023`. `TestWebhookOwnershipMoveRoundTrips` runs `00031` up, down and up again on a scratch database and compares `schemaSnapshot` (which records `relowner` and the ACLs, rendered order-insensitively: `schemaSnapshot` sorts the entries of `relacl`, `attacl`, `nspacl` and `proacl`, so a re-granted entry appended at the end compares equal) after the down with the snapshot before the up, and after the second up with the snapshot after the first. The Down's re-grant to `core_owner` (D4) is what makes the first comparison hold. `TestBaselinePartitionBoundsAreUTC` applies core version 1 through a connection whose `TimeZone` is `Pacific/Kiritimati` and asserts the three partitions' bounds are the UTC month starts. - **The obligation lock** (`internal/entitlements`, `internal/db`): `TestLockPlanActObligationRunsAsTheRuntimeRole` runs `LockPlanActObligation` and the reads around it in a transaction it rolls back under `SET LOCAL ROLE console_runtime`, and asserts the row of `TestLockPlanActObligationReadsTheMove`, the requeued row, and `sql.ErrNoRows` for a missing obligation; in the same transaction, a savepoint runs the bare `SELECT ... FOR UPDATE` on `core.plan_act_obligations` and expects `42501`. `TestPlanActWritersAreEnclosedInTheCatalog` (`internal/db/plan_act_obligations_test.go`) gains `lock_plan_act_obligation` in its list of functions and in its `PUBLIC` and `core_reader` refusal loop, which asserts that the function is `SECURITY DEFINER`, owned by `core_owner`, configured `search_path=core, pg_temp`, not executable by `PUBLIC` and `core_reader`, and executable by `core_writer` and `console_runtime`, with no new test. `TestLockPlanActObligationSurvivesARequeueItWaitedOn` and the plan-act drain tests run unchanged and prove that the lock is still taken and that the recheck still returns the row. - **The pending check** (`internal/db`): on scratch databases migrated in part with `dbtest.UpTo`: a stream behind lists its versions; a stream with no ledger lists all; a ledger `console_runtime` cannot read is reported; a database migrated in full has none; a temporary table named like a ledger, created on the checking connection, changes nothing. -- **The start-up check** (`internal/db`): `TestCheckBeforeMigrating` and `TestCheckRuntimeUser`, one row per refusal through a named function, each inside a transaction that creates the roles it needs and drops to one with `SET LOCAL ROLE` (the database-ownership rows run on a scratch database owned by a role the test creates): a clean member of `console_runtime` passes both; a member of a superuser role; the owner of the database, and a member of the role that owns it; a login that holds `CREATE` on `public` through a grant to `PUBLIC`, and the owner of a function in `public` (check 12, through `CheckSchemasBeforeMigrating`, as the migration connection would run it); not a member of `console_runtime` (a role name that needs quoting, so the copy's quoting shows); a direct and an indirect member of `core_owner`; the owner of a table in `core`, of a function in `core` and of a function in `public`, and a member of a role that owns a table; a direct member of an unrelated role, of `pg_execute_server_program`, of `pg_read_server_files` and of `pg_write_all_data`; a holder of `INSERT` on `core.pool_provisions` through a grant to the user and through a grant to `PUBLIC`; a holder of `TRIGGER` on a table of `core` through a grant to the user (copy 9 naming the user as grantee), of `TRUNCATE` on a table through a grant to `core_reader`, which `core_writer` holds (copy 9 naming `core_reader`), and of `MAINTAIN` through `pg_maintain` granted to `console_runtime` (copy 9a). The rows for copies 3c and 6 through another role use an intermediate role whose name needs quoting (`Mc-Via-`), so the quoted direct role shows. Each asserts the exact sentence. -- **The boot sequence** (`cmd`): `TestOpenRuntimeDatabase`, on scratch databases with login roles the test creates and drops: with both strings, the database is migrated, the runtime login inherits `console_runtime`, the returned pool serves as the runtime login, and no session of the migration user stays open on the scratch database; with one user for both, the refusal is copy 1 and the ledger is untouched; with a superuser runtime login, copy 3 and the ledger untouched; with a runtime login that owns the scratch database, copy 3b and the ledger untouched; with that login owning the scratch database and having planted `public.quote_ident(name)`, which writes a marker row (recording `current_user` and `is_superuser`) into a table the test created and the login may insert into, copy 3b, the ledger untouched and no marker row (neither connection runs the function: the migration connection's identity read is qualified and the runtime connection's is too); with a runtime login that holds `CREATE` on `public` and has planted `public.format(text, text)` with such a marker, copy 3e and no marker row; with a runtime login that owns a function in `public` and holds no `CREATE`, copy 7 before any migration; `migrate up`, `migrate down` and `migrate status` against a scratch database owned by a role that is neither the migration user nor a superuser, copy 3d and the ledger untouched; the same against a database owned by a third role that has run `ALTER DATABASE ... SET role = ''`, copy 3d from `migrate up` and the ledgers untouched; with a runtime login that is a member of `console_runtime` and has run `ALTER ROLE CURRENT_USER SET role = 'console_runtime'`, a normal boot with the login as `current_user` on the returned pool; with `ALTER DATABASE` setting a `search_path` that names a schema the test created, the migrations run through the new pool and the ledgers are in `public`; with a runtime login that holds `pg_read_server_files`, copy 8a; with two databases, copy 2; with `db-migrate-dsn` left out on a database behind, copy 4 and no migration ran; with it left out on a current database, the pool is returned and nothing ran. Every row runs with `env` set to `development`. +- **The start-up check** (`internal/db`): `TestCheckBeforeMigrating` and `TestCheckRuntimeUser`, one row per refusal through a named function, each inside a transaction that creates the roles it needs and drops to one with `SET LOCAL ROLE` (the database-ownership rows run on a scratch database owned by a role the test creates): a clean member of `console_runtime` passes both; a member of a superuser role; the owner of the database, and a member of the role that owns it; a login that holds `CREATE` on `public` through a grant to `PUBLIC`, and the owner of a function in `public` (check 12, through `CheckSchemasBeforeMigrating`, as the migration connection would run it); the same `CREATE` row with `public.=(oid, integer)` planted by a role the test creates, whose function writes a marker row (recording `current_user`), expecting the normal copy 3e and no marker row; a member of a role that owns a function in `public`, the login named `Mc-Rt-` so the name needs quoting, expecting copy 8 and not copy 11 (the second query of check 9 looks the login up by name); not a member of `console_runtime` (a role name that needs quoting, so the copy's quoting shows); a direct and an indirect member of `core_owner`; the owner of a table in `core`, of a function in `core` and of a function in `public`, and a member of a role that owns a table; a direct member of an unrelated role, of `pg_execute_server_program`, of `pg_read_server_files` and of `pg_write_all_data`; a holder of `INSERT` on `core.pool_provisions` through a grant to the user and through a grant to `PUBLIC`; a holder of `TRIGGER` on a table of `core` through a grant to the user (copy 9 naming the user as grantee), of `TRUNCATE` on a table through a grant to `core_reader`, which `core_writer` holds (copy 9 naming `core_reader`), and of `MAINTAIN` through `pg_maintain` granted to `console_runtime` (copy 9a). The rows for copies 3c and 6 through another role use an intermediate role whose name needs quoting (`Mc-Via-`), so the quoted direct role shows. Each asserts the exact sentence. +- **The boot sequence** (`cmd`): `TestOpenRuntimeDatabase`, on scratch databases with login roles the test creates and drops: with both strings, the database is migrated, the runtime login inherits `console_runtime`, the returned pool serves as the runtime login, and no session of the migration user stays open on the scratch database; with one user for both, the refusal is copy 1 and the ledger is untouched; with a superuser runtime login, copy 3 and the ledger untouched; with a runtime login that owns the scratch database, copy 3b and the ledger untouched; with that login owning the scratch database and having planted `public.quote_ident(name)`, which writes a marker row (recording `current_user` and `is_superuser`) into a table the test created and the login may insert into, copy 3b, the ledger untouched and no marker row (neither connection runs the function: the migration connection's identity read is qualified and the runtime connection's is too); with a runtime login that holds `CREATE` on `public` and has planted `public.format(text, text)` with such a marker, copy 3e and no marker row; with the same login having planted `public.=(oid, integer)` with such a marker, copy 3e and no marker row (check 12's own query would otherwise run it as the migration user; the search-path pin prevents it); with a runtime login that owns a function in `public` and holds no `CREATE`, copy 7 before any migration; `migrate up`, `migrate down` and `migrate status` against a scratch database owned by a role that is neither the migration user nor a superuser, copy 3d and the ledger untouched; the same against a database owned by a third role that has run `ALTER DATABASE ... SET role = ''`, copy 3d from `migrate up` and the ledgers untouched; with a runtime login that is a member of `console_runtime` and has run `ALTER ROLE CURRENT_USER SET role = 'console_runtime'`, a normal boot with the login as `current_user` on the returned pool; with `ALTER DATABASE` setting a `search_path` that names a schema the test created, the migrations run through the new pool and the ledgers are in `public`; with a runtime login that holds `pg_read_server_files`, copy 8a; with two databases, copy 2; with `db-migrate-dsn` left out on a database behind, copy 4 and no migration ran; with it left out on a current database, the pool is returned and nothing ran. Every row runs with `env` set to `development`. - **The migration pool** (`internal/db`, in a `db_test` file, because the test exercises `db.ConnectMigration` and `cmd` cannot import `schemaSnapshot`): `TestMigrationPoolIgnoresADatabaseSearchPath` on scratch databases the test creates: a database whose `ALTER DATABASE` sets `search_path` to a schema the test created, migrated through the pool, with `SHOW search_path` returning `public` and every ledger in `public`; a database that sets `role` to a role the test created, whose pool reports `current_user` as the migration login; and two fresh scratch databases with no extra schema, one migrated through the pool and one by a plain replay, with equal `schemaSnapshot`s. -- **The qualification principle** (`internal/db`): `TestCheckQueriesQualifyEveryName` reads the string constants whose names begin `query` in `runtime_user.go` and `migrations.go` (the check queries, the boot-grant statement and the pending check's queries) and fails, naming the query and the token, for a function call or a catalog relation that lacks `pg_catalog.`. A function call is an identifier followed by `(` that no `pg_catalog.` precedes; a catalog relation is any `pg_`-prefixed identifier without it. String literals and `--` comments are removed first. The exceptions are listed in the test, each with its reason: `ANY`, `ALL`, `IN`, `EXISTS`, `VALUES`, `CAST`, `COALESCE`, `CASE`, `AND`, `OR`, `ON`, `FROM`, `JOIN`, `SELECT`, `WHERE`, `WHEN`, `THEN` and `ELSE` are SQL syntax that precedes a parenthesis, and an identifier directly after `AS` is an alias with a column list (`AS p(priv)`). Any other identifier before a parenthesis fails the scan, so a new function must be qualified or listed with its reason. +- **The qualification principle** (`internal/db`): `TestCheckQueriesQualifyEveryName` reads the string constants whose names begin `query` in `runtime_user.go` and `migrations.go` (the check queries, the boot-grant statement and the pending check's queries) and fails, naming the query and the token, for a function call or a catalog relation that lacks `pg_catalog.`. A function call is an identifier followed by `(` that no `pg_catalog.` precedes; a catalog relation is any `pg_`-prefixed identifier without it. String literals and `--` comments are removed first. The exceptions are listed in the test, each with its reason: `ANY`, `ALL`, `IN`, `EXISTS`, `VALUES`, `CAST`, `COALESCE`, `CASE`, `AND`, `OR`, `ON`, `FROM`, `JOIN`, `SELECT`, `WHERE`, `WHEN`, `THEN` and `ELSE` are SQL syntax that precedes a parenthesis, and an identifier directly after `AS` is an alias with a column list (`AS p(priv)`). Any other identifier before a parenthesis fails the scan, so a new function must be qualified or listed with its reason. The scan does not cover operators: the search-path pin does (D2), and the planted-operator rows of `TestCheckBeforeMigrating` and `TestOpenRuntimeDatabase` show it. The pin statement's own constant is listed with its reason, because `pg_temp` is a schema name and not a catalog relation. - **The `-file` keys** (`cmd`): `TestDatabaseCommandsReadTheFileKeys`, one row per command (`migrate up`, `migrate status`, `config list`, `config set`, `config clear`, `seed-demo`): the key's `-file` variant names a file holding a string that reaches a closed port, and the command's error is a connection error to that port, not copy 12 or 13. - **The stack scripts** (`test`): `TestScriptsRefuseAnEnvWithoutTheMigrationDSN`, with the shims of `TestScriptsRefuseUnknownArguments`, runs the four scripts beside a `.env` that has every other key and asserts copy 28, exit status 1, and that no shim ran. - **Deliberate breaks.** Each new test is proved with `scripts/mutation-check` on the new side; the breaks are in `tasks.md` and in each commit message. @@ -690,6 +702,7 @@ Tests follow `docs/testing.md`. The tasks list each test with the breaks it must - **The check reads the catalog at every boot.** The queries read `pg_class` and `pg_proc` of the console's schemas and `public` once; that is a few thousand rows. - **A host can still weaken the runtime user after boot.** The check runs at start; a grant made while the console runs takes effect until the next start, when the check refuses it. - **A database-level setting or a function in `public` planted earlier is not found.** If the runtime user once owned the database and ran `ALTER DATABASE ... SET search_path` or `SET role`, or created a function in `public`, moving the ownership (check 4, copy 3b) does not clear the setting, and `REASSIGN OWNED` hands the function to the migration user, where check 9 no longer sees it. A later superuser session in that database (not the console's own sessions, which open with `search_path` and `role=none` startup parameters, D2) would honour the setting, and an exact-type shadow of a catalog function would keep capturing unqualified calls in later migrations. It takes a past misconfiguration plus arbitrary SQL at that time, and refusing database-level settings would also refuse legitimate ones, so it is a recorded risk and not a check. The procedure in `docs/production-deployment.md` reads `pg_db_role_setting` and the functions of `public` once. +- **The migrations themselves run with `search_path=public`, without the pin.** Their SQL is the shipped files, written for that path, and the pin would change what they create. What protects the run is the order: the database-ownership refusal and check 12 (no `CREATE` on `public` or a stream schema, no owned function in `public`) come before the first migration, so no role the console serves as can have planted an operator or function there. A role that is not the runtime user and can create objects in `public` is outside what the check can see, and is the host's own configuration. - **A membership granted by a non-superuser survives the printed `REVOKE`.** A membership granted by a role that holds `ADMIN` on the target and is not a superuser is recorded with that grantor, and `REVOKE` run by the migration user prints a warning and leaves the row; `REVOKE ... GRANTED BY ` removes it. With a superuser migration user this happens only for grants a non-superuser made, and the next start refuses again, naming the same role. - **The migration pool's `search_path=public` and `role=none` are different custom parameters from the one the plain pool avoided.** The reason for a plain pool, in `docs/database-management.md`, is a pgx and Postgres 18 interaction with the application's `core,public` path. `public` alone is what the default path resolves to when no schema is named for the user, which no documented deployment has; section 4's test migrates a scratch database through the new pool and compares its `schemaSnapshot` with a plain replay's, so a dropped object fails `make test`. - **A hosting provider that gives every new login administrative roles makes the first start refuse.** Check 10 refuses any direct membership but `console_runtime`; the refusal prints the `REVOKE`, and the operator runs it once. The alternative, a list of refused roles, would miss the next role a provider adds. diff --git a/openspec/changes/runtime-database-user/specs/startup-configuration/spec.md b/openspec/changes/runtime-database-user/specs/startup-configuration/spec.md index c122e9d7..b2f32891 100644 --- a/openspec/changes/runtime-database-user/specs/startup-configuration/spec.md +++ b/openspec/changes/runtime-database-user/specs/startup-configuration/spec.md @@ -2,7 +2,7 @@ ### Requirement: Start migrates on the migration connection and serves from the runtime connection -`db-dsn` SHALL name the runtime user, the user the console serves from, and `db-migrate-dsn` SHALL name the migration user, the user that owns the schemas and runs migrations. With `db-migrate-dsn` set, `start` SHALL open both connections, SHALL run the checks of "The console refuses a runtime user that can bypass the grants" that read no migrated object before it runs any migration, SHALL run every stream's migrations on the migration connection, SHALL then grant `console_runtime` to the runtime user on the migration connection, and SHALL close the migration connection before it serves. The grant SHALL name the user the runtime connection reports as `current_user`, SHALL be built in the database with identifier quoting (`format('%I')`) from a bound parameter, never by concatenating SQL in Go, SHALL carry `WITH INHERIT TRUE`, and SHALL be idempotent. With `db-migrate-dsn` left out, `start` SHALL run no migration, SHALL log that it ran none, and SHALL refuse to start while the ledger of any stream lacks a version the stream embeds, naming the command that applies them; it SHALL read each ledger by its schema-qualified name `public.goose_db_version_`, so that a temporary table of the same name cannot stand in for it. Every migration connection (`start`'s and each `migrate` command's) SHALL open with the startup parameters `search_path=public` and `role=none`, and the runtime connection with `search_path=core,public` and `role=none`; a startup parameter overrides a database-level or role-level setting, so `current_user` on each connection is the login that the string names. SQL that the console runs on the migration connection, and SQL whose output it prints as a statement for the operator to run, SHALL name every function and every catalog relation with its schema (`pg_catalog.`, and `public.` for a goose ledger), so that an object a role planted in a schema on the search path never runs in that session. Until the check that the runtime user does not own the database has passed, the migration connection SHALL run only a query with no unqualified function and no unqualified relation. `migrate up`, `migrate down`, `migrate status` and `start` SHALL refuse, before any migration runs, a database whose owner is neither the migration user nor a superuser role, naming the statement `ALTER DATABASE OWNER TO CURRENT_USER;` with the name quoted as the database quotes an identifier. `migrate up`, `migrate down` and `migrate status` SHALL use `db-migrate-dsn` and SHALL refuse to run without it. `seed-demo` SHALL open the database as `start` does. +`db-dsn` SHALL name the runtime user, the user the console serves from, and `db-migrate-dsn` SHALL name the migration user, the user that owns the schemas and runs migrations. With `db-migrate-dsn` set, `start` SHALL open both connections, SHALL run the checks of "The console refuses a runtime user that can bypass the grants" that read no migrated object before it runs any migration, SHALL run every stream's migrations on the migration connection, SHALL then grant `console_runtime` to the runtime user on the migration connection, and SHALL close the migration connection before it serves. The grant SHALL name the user the runtime connection reports as `current_user`, SHALL be built in the database with identifier quoting (`format('%I')`) from a bound parameter, never by concatenating SQL in Go, SHALL carry `WITH INHERIT TRUE`, and SHALL be idempotent. With `db-migrate-dsn` left out, `start` SHALL run no migration, SHALL log that it ran none, and SHALL refuse to start while the ledger of any stream lacks a version the stream embeds, naming the command that applies them; it SHALL read each ledger by its schema-qualified name `public.goose_db_version_`, so that a temporary table of the same name cannot stand in for it. Every migration connection (`start`'s and each `migrate` command's) SHALL open with the startup parameters `search_path=public` and `role=none`, and the runtime connection with `search_path=core,public` and `role=none`; a startup parameter overrides a database-level or role-level setting, so `current_user` on each connection is the login that the string names. SQL that the console runs on the migration connection, and SQL whose output it prints as a statement for the operator to run, SHALL name every function and every catalog relation with its schema (`pg_catalog.`, and `public.` for a goose ledger), so that an object a role planted in a schema on the search path never runs in that session. Every transaction in which the console runs a check query or the boot grant, on either connection, SHALL begin with `SET LOCAL search_path = pg_catalog, pg_temp`, so that no operator or function a role planted in another schema can be chosen for any name in it. Until the check that the runtime user does not own the database has passed, the migration connection SHALL run only a query with no unqualified function and no unqualified relation. `migrate up`, `migrate down`, `migrate status` and `start` SHALL refuse, before any migration runs, a database whose owner is neither the migration user nor a superuser role, naming the statement `ALTER DATABASE OWNER TO CURRENT_USER;` with the name quoted as the database quotes an identifier. `migrate up`, `migrate down` and `migrate status` SHALL use `db-migrate-dsn` and SHALL refuse to run without it. `seed-demo` SHALL open the database as `start` does. #### Scenario: Start migrates as the migration user and serves as the runtime user @@ -44,6 +44,11 @@ - **WHEN** `db-dsn` logs in as a user that owns the database and has created `public.quote_ident(name)` - **THEN** `start` SHALL refuse with the database-ownership refusal, the function SHALL NOT have run on either connection, and no migration SHALL have run +#### Scenario: A planted operator does not run in a check + +- **WHEN** `db-dsn` logs in as a user that holds `CREATE` on `public` and has created `public.=(oid, integer)` +- **THEN** `start` SHALL refuse with the refusal for `CREATE` on `public`, the operator SHALL NOT have run on either connection, and no migration SHALL have run + #### Scenario: A query that names a function without its schema fails the build - **WHEN** a check query, the boot-grant statement or a pending-check query calls a function, or reads a catalog relation, without `pg_catalog.` (SQL syntax that precedes a parenthesis excepted, by a listed reason) diff --git a/openspec/changes/runtime-database-user/tasks.md b/openspec/changes/runtime-database-user/tasks.md index a480947a..f72c9511 100644 --- a/openspec/changes/runtime-database-user/tasks.md +++ b/openspec/changes/runtime-database-user/tasks.md @@ -41,11 +41,11 @@ Sections 1 to 6 are one commit each, and their items are that commit's parts; se ## 2. Quick fix: every command that opens the database reads the `-file` keys -- [ ] 2.1 `cmd/start.go` (design D12): split the core secret pairs into `databaseSecretPairs` (`db-dsn`/`db-dsn-file`) and the rest; the loop of `resolveSecretFiles` becomes `resolveSecretPairs(pairs)`; `resolveSecretFiles` resolves the core pairs, the database pairs and the integrations' pairs as today; a new `resolveDatabaseSecretFiles` resolves `databaseSecretPairs` alone. Doc comments say which commands call which and why a command resolves once. -- [ ] 2.2 `migrate up`, `migrate down`, `migrate status` (`cmd/migrate.go`), `connectSettings` (`cmd/config.go`, the one path of `config list`, `set` and `clear`) and `seed-demo` (`cmd/seed_demo.go`) call `resolveDatabaseSecretFiles` before `openDB`. `connectSettings` returns its error, and the three `Run` functions print it and exit as they do today, so a test can call it. -- [ ] 2.3 `openDB` refuses an empty `db-dsn` with copy 13 before it opens anything. -- [ ] 2.4 `TestDatabaseCommandsReadTheFileKeys` (`cmd/database_files_test.go`), one row per path through a named function: `migrate up` and `migrate status` (their `RunE`), `connectSettings`, and `seed-demo` (its `RunE`). Each row sets only `db-dsn-file`, naming a file that holds `postgres://u:p@127.0.0.1:/x?connect_timeout=1`, and asserts that the error is a connection failure that names that port, not copy 13. A second table sets neither key and asserts copy 13 for each path. `TestResolveSecretFiles_DBDSN` passes unchanged. -- [ ] 2.5 Breaks: +- [x] 2.1 `cmd/start.go` (design D12): split the core secret pairs into `databaseSecretPairs` (`db-dsn`/`db-dsn-file`) and the rest; the loop of `resolveSecretFiles` becomes `resolveSecretPairs(pairs)`; `resolveSecretFiles` resolves the core pairs, the database pairs and the integrations' pairs as today; a new `resolveDatabaseSecretFiles` resolves `databaseSecretPairs` alone. Doc comments say which commands call which and why a command resolves once. +- [x] 2.2 `migrate up`, `migrate down`, `migrate status` (`cmd/migrate.go`), `connectSettings` (`cmd/config.go`, the one path of `config list`, `set` and `clear`) and `seed-demo` (`cmd/seed_demo.go`) call `resolveDatabaseSecretFiles` before `openDB`. `connectSettings` returns its error, and the three `Run` functions print it and exit as they do today, so a test can call it. +- [x] 2.3 `openDB` refuses an empty `db-dsn` with copy 13 before it opens anything. +- [x] 2.4 `TestDatabaseCommandsReadTheFileKeys` (`cmd/database_files_test.go`), one row per path through a named function: `migrate up` and `migrate status` (their `RunE`), `connectSettings`, and `seed-demo` (its `RunE`). Each row sets only `db-dsn-file`, naming a file that holds `postgres://u:p@127.0.0.1:/x?connect_timeout=1`, and asserts that the error is a connection failure that names that port, not copy 13. A second table sets neither key and asserts copy 13 for each path. `TestResolveSecretFiles_DBDSN` passes unchanged. +- [x] 2.5 Breaks: - `migrate up` without its `resolveDatabaseSecretFiles` call: caught by its row (copy 13 in place of the connection failure); - `connectSettings` without the call: caught by its row; - `seed-demo` without the call: caught by its row; @@ -106,7 +106,7 @@ The plan-act obligation lock (items 3.10 to 3.14) lives in this section, not in This section's `make test` passes only after the documented stack respin of item 4.9, which the implementer performs in the worktree before the section's suite runs: the guard refuses a `test/.env` without `MC_DB_MIGRATE_DSN`, and `postgres-seed` creates the runtime login only on `docker compose up`. "Every commit passes `make test`" therefore holds from a stack so respun, not from a fresh checkout beside an old `test/.env`. - [ ] 4.1 Keys (design D1): `startCmd` registers `db-migrate-dsn` and `db-migrate-dsn-file` (copies 23, 24), and the help of `db-dsn` and `db-dsn-file` changes (copies 21, 22); `databaseSecretPairs` gains `db-migrate-dsn`/`db-migrate-dsn-file`; `ValidateStart` refuses a malformed `db-migrate-dsn` (copy 14) and treats it as optional. -- [ ] 4.2 `internal/db`: `PendingMigrations(ctx, q, sources)` (design D10) in `migrations.go`, which reads each ledger as `public.goose_db_version_` by qualified name and calls `pg_catalog.to_regclass` and `pg_catalog.has_table_privilege` (design D2, the qualification principle), with a doc comment that says why it does not use goose's provider and why the names are qualified; `ConnectMigration(ctx, logger, config)`, which opens the migration pool with the startup parameters `search_path=public` and `role=none` as runtime parameters of the connection string, replacing any `search_path` or `role` the string carries (`ensureSearchPath`, `internal/db/database.go:43-60`, keeps an operator's value and is not reused, so a `db-migrate-dsn` copied from the runtime string cannot open with `core,public`); the runtime pool's open gains `role=none`, always replacing the string's value, beside its `search_path=core,public` (`ConnectPlain` stays for `dbConfig`); `CurrentUser` and `GrantRuntimeRole(ctx, tx, name)` (design D8, whose statement calls `pg_catalog.set_config`, `pg_catalog.format` and `pg_catalog.current_setting`; it takes a transaction because the setting lasts until the transaction ends, and `cmd` runs it through `db.InTx`) in `runtime_user.go`, and the refusal type `RuntimeUserRefusal`. Every SQL string of these functions is a constant whose name begins `query` (item 5.8 scans them). `ConnectAndMigrate` is deleted. +- [ ] 4.2 `internal/db`: `PendingMigrations(ctx, q, sources)` (design D10) in `migrations.go`, which begins its transaction with `SET LOCAL search_path = pg_catalog, pg_temp` and reads each ledger as `public.goose_db_version_` by qualified name and calls `pg_catalog.to_regclass` and `pg_catalog.has_table_privilege` (design D2, the qualification principle), with a doc comment that says why it does not use goose's provider and why the names are qualified; `ConnectMigration(ctx, logger, config)`, which opens the migration pool with the startup parameters `search_path=public` and `role=none` as runtime parameters of the connection string, replacing any `search_path` or `role` the string carries (`ensureSearchPath`, `internal/db/database.go:43-60`, keeps an operator's value and is not reused, so a `db-migrate-dsn` copied from the runtime string cannot open with `core,public`); the runtime pool's open gains `role=none`, always replacing the string's value, beside its `search_path=core,public` (`ConnectPlain` stays for `dbConfig`); `CurrentUser` and `GrantRuntimeRole(ctx, tx, name)` (design D8, which begins with `SET LOCAL search_path = pg_catalog, pg_temp`, and whose statement calls `pg_catalog.set_config`, `pg_catalog.format` and `pg_catalog.current_setting`; it takes a transaction because the setting lasts until the transaction ends, and `cmd` runs it through `db.InTx`) in `runtime_user.go`, and the refusal type `RuntimeUserRefusal`. Every SQL string of these functions is a constant whose name begins `query` (item 5.8 scans them). `ConnectAndMigrate` is deleted. - [ ] 4.3 `cmd/database.go` (design D11): modes `dbRuntime`, `dbMigration` and `dbConfig`. `dbMigration` opens `db-migrate-dsn` with `search_path=public` and `role=none` (design D2, rule 2) and refuses without it (copy 12). `dbConfig` is today's `dbPlain` on `db-dsn`. `dbRuntime` opens the runtime pool with `role=none` beside its `search_path`, and runs the boot sequence's steps 2, 3, 6 and 7, step 5's migrations, and the pending check in both modes (copies 4, 4b to 4d, 10, 18, 20); the other checks, and steps 4 and 8 in full, are section 5's, with copy 4a, which belongs to check 5. `start` and `seed-demo` open `dbRuntime`; `start` logs a refusal with copy 17; the `migrate` commands open `dbMigration`; the `config` commands open `dbConfig`. The comment on `openDB` says which mode each command uses and why. - [ ] 4.4 `internal/embeds/mc-config.yaml`: copies 27 and 27a. - [ ] 4.5 Test stack (design D14): `test/compose.yaml` gives `postgres` a health check and adds `postgres-seed`; `test/seed/postgres/seed-runtime-user.sh` (new, SPDX header, copies 30 and 31); `test/bootstrap-stack.sh` writes `MC_DB_DSN` and `MC_DB_MIGRATE_DSN`, prints copy 29, refuses a stale `.env` on a re-run with copy 28, and its header says what it writes; `test/reset-test-db.sh` refuses a stale `.env` (copy 28) after its existing guard and runs `MC_DB_MIGRATE_DSN="$dsn" go run . migrate up`; `test/reset-app-db.sh` refuses a stale `.env` and uses `MC_DB_MIGRATE_DSN` for `migrate up`, the pin, `pg_dump` and the snapshot's address, and its header says so; `test/seed-demo.sh` refuses a stale `.env`; `test/mc-config.yaml` carries both strings; `test/mockshot-nuke.sql`'s comment (`:111-114`) says that the stack's migration user is a superuser, which `SET ROLE core_owner` relies on; `test/seed/discourse/seed-discourse.sh`'s header says the runtime user needs `console_runtime`, which the first `start` or `reset-app-db.sh` grants. @@ -141,11 +141,11 @@ This section's `make test` passes only after the documented stack respin of item ## 5. The start-up check refuses a runtime user that can bypass the grants -- [ ] 5.1 `internal/db/runtime_user.go`: `CheckBeforeMigrating(ctx, q)` runs the membership half of check 3 and the runtime half of check 4 of design D9 in one read-only `REPEATABLE READ` transaction and returns a `*RuntimeUserRefusal` with copy 3a, 3b or 3c, or copy 11 on a failed query; it scans the direct role as nullable. `CheckMigrationDatabaseOwner(ctx, q)` runs the owner half of check 4 on the migration connection, with qualified names only, and returns a `*DatabaseOwnerRefusal` with copy 3d, or copy 11 on a failed query. The identity read on the migration connection is `SELECT current_user, pg_catalog.current_database()` and nothing else; the superuser bit and the quoted name come from the runtime connection. `CheckSchemasBeforeMigrating(ctx, q, login, schemas)` runs check 12 (design D9): the `CREATE` query over `public` and the five schemas, then check 9's query restricted to `public` with the login as a parameter, and returns a `*RuntimeUserRefusal` with copy 3e, 7 or 8, or copy 11; `cmd` calls it on the migration connection after `CheckMigrationDatabaseOwner`, and on the runtime connection in the separate-step mode. `CheckRuntimeUser(ctx, q, check)` runs checks 5 to 11 in that order in one such transaction and returns a `*RuntimeUserRefusal` with copies 4 to 9, 8a and 9a, or copy 11. `RuntimeCheck` carries the stream names and the owner role names; `cmd` builds both from `migrate.Sources()`. Every function and catalog relation of every query is written with `pg_catalog.` (design D2, the qualification principle), and every query is a constant whose name begins `query`. The queries of checks 3, 4, 8 and 9 select `pg_catalog.quote_ident` of the direct role; check 9's first query also selects `pg_catalog.pg_get_userbyid(owner)`, which copy 8 prints; and check 11's two reads return the grantee (`PUBLIC`, or a quoted role name) from `pg_catalog.aclexplode` of the relation's ACL, restricted to entries that reach the user (a grant to `PUBLIC` or to a role the user inherits), so copy 9 prints the one `REVOKE` that removes the privilege; when the grantee is null, a third query names the predefined role (`pg_write_all_data` for `INSERT`, `pg_maintain` for `MAINTAIN`) and the role it is granted to, and the refusal is copy 9a. Check 11 also reads the first relation of the five schemas on which the user holds `TRUNCATE`, `REFERENCES`, `TRIGGER` or `MAINTAIN`. The pending check of section 4 moves into `CheckRuntimeUser` as check 7. +- [ ] 5.1 `internal/db/runtime_user.go`: `CheckBeforeMigrating(ctx, q)` runs the membership half of check 3 and the runtime half of check 4 of design D9 in one read-only `REPEATABLE READ` transaction and returns a `*RuntimeUserRefusal` with copy 3a, 3b or 3c, or copy 11 on a failed query; it scans the direct role as nullable. `CheckMigrationDatabaseOwner(ctx, q)` runs the owner half of check 4 on the migration connection, with qualified names only, and returns a `*DatabaseOwnerRefusal` with copy 3d, or copy 11 on a failed query. The identity read on the migration connection is `SELECT current_user, pg_catalog.current_database()` and nothing else; the superuser bit and the quoted name come from the runtime connection. `CheckSchemasBeforeMigrating(ctx, q, login, schemas)` runs check 12 (design D9): the `CREATE` query over `public` and the five schemas, then check 9's query restricted to `public` with the login as a parameter, and returns a `*RuntimeUserRefusal` with copy 3e, 7 or 8, or copy 11; `cmd` calls it on the migration connection after `CheckMigrationDatabaseOwner`, and on the runtime connection in the separate-step mode. `CheckRuntimeUser(ctx, q, check)` runs checks 5 to 11 in that order in one such transaction and returns a `*RuntimeUserRefusal` with copies 4 to 9, 8a and 9a, or copy 11. `RuntimeCheck` carries the stream names and the owner role names; `cmd` builds both from `migrate.Sources()`. Every function and catalog relation of every query is written with `pg_catalog.` (design D2, the qualification principle), and every query is a constant whose name begins `query`. Every transaction these functions open, on either connection (the identity reads, the owner half of check 4, check 12, `CheckRuntimeUser`'s), begins with `SET LOCAL search_path = pg_catalog, pg_temp`, a constant of its own, before its first query (design D2, the search-path pin). The second query of checks 9 and 12 looks the login up as `(SELECT r.oid FROM pg_catalog.pg_roles r WHERE r.rolname = $2::name)`, never with `regrole`. The queries of checks 3, 4, 8 and 9 select `pg_catalog.quote_ident` of the direct role; check 9's first query also selects `pg_catalog.pg_get_userbyid(owner)`, which copy 8 prints; and check 11's two reads return the grantee (`PUBLIC`, or a quoted role name) from `pg_catalog.aclexplode` of the relation's ACL, restricted to entries that reach the user (a grant to `PUBLIC` or to a role the user inherits), so copy 9 prints the one `REVOKE` that removes the privilege; when the grantee is null, a third query names the predefined role (`pg_write_all_data` for `INSERT`, `pg_maintain` for `MAINTAIN`) and the role it is granted to, and the refusal is copy 9a. Check 11 also reads the first relation of the five schemas on which the user holds `TRUNCATE`, `REFERENCES`, `TRIGGER` or `MAINTAIN`. The pending check of section 4 moves into `CheckRuntimeUser` as check 7. - [ ] 5.2 `cmd/database.go`: `dbRuntime` reads both identities after it opens the pools (the migration connection's with the one qualified query) and refuses checks 1 to 3 (copies 1, 2, 3), calls `CheckBeforeMigrating` (copies 3a, 3b, 3c), then `CheckMigrationDatabaseOwner` (copy 3d), then `CheckSchemasBeforeMigrating` on the migration connection (copies 3e, 7, 8), before it migrates and before the boot grant (in the separate-step mode it calls `CheckSchemasBeforeMigrating` on the runtime connection before `CheckRuntimeUser`), and calls `CheckRuntimeUser` after it closes the migration pool; a passing check logs copy 19. `dbMigration` calls `CheckMigrationDatabaseOwner` before every `migrate` command (`up`, `down` and `status`) runs goose, because goose's `Status` creates any missing ledger in `public`. Nothing reads `env`. -- [ ] 5.3 `TestCheckBeforeMigrating` (`internal/db/runtime_user_test.go`), one row per case through a named function, each dropping to the row's role with `SET LOCAL ROLE` inside a transaction it rolls back that creates the roles the row needs (the database rows run on a scratch database that a role the test creates owns, dropped in cleanup after the database): a member of `console_runtime` that owns nothing passes; one that is also a member of a superuser role created in the transaction, copy 3a; the owner of the scratch database, copy 3b naming the database; a member of the role that owns the scratch database, through an intermediate role named `Mc-Via-`, copy 3c naming the owner and the direct role, quoted; a role that owns another database and not this one passes; and the check run on a closed database (`dbtest.Closed`), copy 11. (A superuser that does not own the database reaches copy 3a from this function, because the membership half of check 3 runs first and a superuser is a member of the bootstrap superuser; the superuser row of `TestOpenRuntimeDatabase` covers check 4's order.) The same test runs `CheckSchemasBeforeMigrating` (check 12) with a login role created in the transaction: a login that owns nothing and holds no `CREATE` passes; after `GRANT CREATE ON SCHEMA public TO PUBLIC` in the transaction, copy 3e naming `public` and the grantee `PUBLIC`; `GRANT CREATE ON SCHEMA public` to a role the test creates (`Mc-Creator`, mixed case so the quoting shows) and of which the login is a member with `INHERIT FALSE, SET TRUE`, copy 3e naming that role quoted, which proves both the grantee and the `MEMBER` test; after `GRANT CREATE ON SCHEMA core TO` the login, copy 3e naming `core`; a login that owns a function in `public`, copy 7 with the function's signature, and one that is a member of a role that owns such a function through `Mc-Via-`, copy 8 naming the owner by name and the direct role, quoted; and a closed database, copy 11. `TestCheckMigrationDatabaseOwner` adds rows: a scratch database owned by the migration user passes, one owned by a superuser role passes, one owned by a role the test creates, copy 3d naming the owner and the quoted database name, and a closed database, copy 11. Every row but the passing ones asserts the whole sentence. -- [ ] 5.4 `TestCheckRuntimeUser` (`internal/db/runtime_user_test.go`), one row per case through a named function, each in a transaction it rolls back that creates the roles the row needs and drops to the row's role with `SET LOCAL ROLE`: a member of `console_runtime` passes; a role named `Mc-Rt-` that is no member, copy 5 with the name quoted; a member of `console_runtime` granted `WITH INHERIT FALSE`, copy 5; a direct member of `core_owner`, copy 6 naming `core_owner` in both places, which also shows that the owner role is reported before check 10; a member of `core_owner` through another role named `Mc-Via-`, copy 6 naming that role, quoted, in the `REVOKE`; a member of `core_owner` granted `WITH INHERIT FALSE, SET TRUE`, copy 6; the owner of a table in `core` created in the transaction, copy 7; the owner of a function in `core`, copy 7 with its signature; the owner of a function in `public`, copy 7; a member of a role that owns a table, copy 8; a holder of `TRIGGER` on a table of `core` through a grant to the user, copy 9 naming `TRIGGER`, the table and the user as grantee; a holder of `TRUNCATE` on a table of `core` through a grant to `core_reader`, which `core_writer` holds, copy 9 naming `core_reader` as grantee; a holder of `MAINTAIN` through `pg_maintain` granted to `console_runtime`, copy 9a naming `pg_maintain` and `console_runtime`; a direct member of an unrelated role named `Mc-Extra-`, copy 8a with both names quoted; a direct member of `pg_execute_server_program`, of `pg_read_server_files` and of `pg_write_all_data`, copy 8a for each; a holder of `INSERT` on `core.pool_provisions` through a grant to the user, and through a grant to `PUBLIC`, copy 9 naming the user and `PUBLIC` as grantee; and the check run on a closed database (`dbtest.Closed`), copy 11. Every row but the passing one asserts the whole sentence. -- [ ] 5.5 `TestOpenRuntimeDatabase` (rows of section 4 stay) gains rows: one user for both strings, copy 1, and the ledgers untouched; a superuser runtime login the test creates (`LOGIN SUPERUSER`), copy 3, the ledgers untouched and no membership granted; a runtime login that owns the scratch database, copy 3b, the ledgers untouched and no membership granted; the same login having created `public.quote_ident(name)`, which writes a marker row (recording `current_user` and `is_superuser`) into a table the test created and the login may insert into, copy 3b, the ledgers untouched and no marker row, because neither connection runs the function; a runtime login that holds `CREATE` on `public` and has created `public.format(text, text)` with such a marker, copy 3e, the ledgers untouched and no marker row; a runtime login that owns a function in `public` and holds no `CREATE`, copy 7, the ledgers untouched; `migrate up`, `migrate down` and `migrate status` against a scratch database owned by a role that is neither the migration user nor a superuser, copy 3d and the ledgers untouched (for `status`, no ledger created); the same against a scratch database owned by a third role that has run `ALTER DATABASE ... SET role = ''`, copy 3d from `migrate up` and the ledgers untouched; a runtime login that holds `pg_read_server_files`, copy 8a after the migrations ran; two scratch databases, copy 2; and every row runs with `env` set to `development`. Each row's cleanup drops the scratch database before the login that owns it, because Go runs `t.Cleanup` functions in reverse order, `dbtest.Scratch` registers its drop first, and `DROP ROLE` fails while the role owns the database. Section 4's rows pass unchanged. +- [ ] 5.3 `TestCheckBeforeMigrating` (`internal/db/runtime_user_test.go`), one row per case through a named function, each dropping to the row's role with `SET LOCAL ROLE` inside a transaction it rolls back that creates the roles the row needs (the database rows run on a scratch database that a role the test creates owns, dropped in cleanup after the database): a member of `console_runtime` that owns nothing passes; one that is also a member of a superuser role created in the transaction, copy 3a; the owner of the scratch database, copy 3b naming the database; a member of the role that owns the scratch database, through an intermediate role named `Mc-Via-`, copy 3c naming the owner and the direct role, quoted; a role that owns another database and not this one passes; and the check run on a closed database (`dbtest.Closed`), copy 11. (A superuser that does not own the database reaches copy 3a from this function, because the membership half of check 3 runs first and a superuser is a member of the bootstrap superuser; the superuser row of `TestOpenRuntimeDatabase` covers check 4's order.) The same test runs `CheckSchemasBeforeMigrating` (check 12) with a login role created in the transaction: a login that owns nothing and holds no `CREATE` passes; after `GRANT CREATE ON SCHEMA public TO PUBLIC` in the transaction, copy 3e naming `public` and the grantee `PUBLIC`; `GRANT CREATE ON SCHEMA public` to a role the test creates (`Mc-Creator`, mixed case so the quoting shows) and of which the login is a member with `INHERIT FALSE, SET TRUE`, copy 3e naming that role quoted, which proves both the grantee and the `MEMBER` test; after `GRANT CREATE ON SCHEMA core TO` the login, copy 3e naming `core`; a login that owns a function in `public`, copy 7 with the function's signature, and one that is a member of a role that owns such a function through `Mc-Via-`, copy 8 naming the owner by name and the direct role, quoted, with the login itself named `Mc-Rt-` so that its name needs quoting (copy 8 and not copy 11, because the second query of check 9 looks the login up by name); a login holding `CREATE` on `public` through a grant to `PUBLIC` after a role the test creates has planted `public.=(oid, integer)`, whose function writes a marker row recording `current_user` into a table the test created, copy 3e and no marker row; and a closed database, copy 11. `TestCheckMigrationDatabaseOwner` adds rows: a scratch database owned by the migration user passes, one owned by a superuser role passes, one owned by a role the test creates, copy 3d naming the owner and the quoted database name, and a closed database, copy 11. Every row but the passing ones asserts the whole sentence. +- [ ] 5.4 `TestCheckRuntimeUser` (`internal/db/runtime_user_test.go`), one row per case through a named function, each in a transaction it rolls back that creates the roles the row needs and drops to the row's role with `SET LOCAL ROLE`: a member of `console_runtime` passes; a role named `Mc-Rt-` that is no member, copy 5 with the name quoted; a member of `console_runtime` granted `WITH INHERIT FALSE`, copy 5; a direct member of `core_owner`, copy 6 naming `core_owner` in both places, which also shows that the owner role is reported before check 10; a member of `core_owner` through another role named `Mc-Via-`, copy 6 naming that role, quoted, in the `REVOKE`; a member of `core_owner` granted `WITH INHERIT FALSE, SET TRUE`, copy 6; the owner of a table in `core` created in the transaction, copy 7; the owner of a function in `core`, copy 7 with its schema-qualified signature (the pin of design D2 makes `regprocedure::text` qualify it); the owner of a function in `public`, copy 7; a member of a role that owns a table, copy 8; a holder of `TRIGGER` on a table of `core` through a grant to the user, copy 9 naming `TRIGGER`, the table and the user as grantee; a holder of `TRUNCATE` on a table of `core` through a grant to `core_reader`, which `core_writer` holds, copy 9 naming `core_reader` as grantee; a holder of `MAINTAIN` through `pg_maintain` granted to `console_runtime`, copy 9a naming `pg_maintain` and `console_runtime`; a direct member of an unrelated role named `Mc-Extra-`, copy 8a with both names quoted; a direct member of `pg_execute_server_program`, of `pg_read_server_files` and of `pg_write_all_data`, copy 8a for each; a holder of `INSERT` on `core.pool_provisions` through a grant to the user, and through a grant to `PUBLIC`, copy 9 naming the user and `PUBLIC` as grantee; and the check run on a closed database (`dbtest.Closed`), copy 11. Every row but the passing one asserts the whole sentence. +- [ ] 5.5 `TestOpenRuntimeDatabase` (rows of section 4 stay) gains rows: one user for both strings, copy 1, and the ledgers untouched; a superuser runtime login the test creates (`LOGIN SUPERUSER`), copy 3, the ledgers untouched and no membership granted; a runtime login that owns the scratch database, copy 3b, the ledgers untouched and no membership granted; the same login having created `public.quote_ident(name)`, which writes a marker row (recording `current_user` and `is_superuser`) into a table the test created and the login may insert into, copy 3b, the ledgers untouched and no marker row, because neither connection runs the function; a runtime login that holds `CREATE` on `public` and has created `public.format(text, text)` with such a marker, copy 3e, the ledgers untouched and no marker row; the same login having created `public.=(oid, integer)` with such a marker, copy 3e, the ledgers untouched and no marker row; a runtime login that owns a function in `public` and holds no `CREATE`, copy 7, the ledgers untouched; `migrate up`, `migrate down` and `migrate status` against a scratch database owned by a role that is neither the migration user nor a superuser, copy 3d and the ledgers untouched (for `status`, no ledger created); the same against a scratch database owned by a third role that has run `ALTER DATABASE ... SET role = ''`, copy 3d from `migrate up` and the ledgers untouched; a runtime login that holds `pg_read_server_files`, copy 8a after the migrations ran; two scratch databases, copy 2; and every row runs with `env` set to `development`. Each row's cleanup drops the scratch database before the login that owns it, because Go runs `t.Cleanup` functions in reverse order, `dbtest.Scratch` registers its drop first, and `DROP ROLE` fails while the role owns the database. Section 4's rows pass unchanged. - [ ] 5.6 Start this worktree's application on the stack and check that the boot log shows copy 19 naming `member_console_runtime` and `member_console`. Set `MC_DB_DSN` to the migration user's string in one shell and check that the start refuses with copy 1. - [ ] 5.7 Breaks: - check 4 before check 3: caught by the superuser row of `TestOpenRuntimeDatabase` (a superuser is a member of every role and matches check 4's membership case, so copy 3c with an empty direct role would print in place of copy 3); @@ -163,6 +163,9 @@ This section's `make test` passes only after the documented stack respin of item - check 12's `public` half removed: caught by the owned-function rows of both tests; - check 12 run after the migrations: caught by the `public.format` and owned-function rows' ledger assertions; - the boot grant's `pg_catalog.format` written `format`: caught by the scan of item 5.8; + - a check transaction (check 12's, on the migration connection) without its `SET LOCAL search_path = pg_catalog, pg_temp`: caught by the planted-operator rows of `TestCheckBeforeMigrating` and `TestOpenRuntimeDatabase` (a marker row is written); + - `CheckRuntimeUser`'s transaction without the pin: caught by the function rows of `TestCheckRuntimeUser`, whose copy 7 asserts the schema-qualified signature (`core.(core.)`, not `()`); + - the second query of check 9 or 12 casting the login with `regrole`: caught by the mixed-case `Mc-Rt-` indirect-owner row of `TestCheckBeforeMigrating` (copy 11 in place of copy 8); - `CheckMigrationDatabaseOwner` removed from `dbMigration`: caught by the `migrate up` row of `TestOpenRuntimeDatabase`; - `dbMigration` checks before `up` only: caught by the `migrate down` and `migrate status` rows of `TestOpenRuntimeDatabase` (the `status` row finds a ledger created); - `CheckMigrationDatabaseOwner` removed from `dbRuntime`: caught by `start`'s copy 3d row; @@ -191,7 +194,7 @@ This section's `make test` passes only after the documented stack respin of item - an unqualified `pg_roles` in the owner check: caught by the scan; - `PendingMigrations` calls `to_regclass` without `pg_catalog.`: caught by the scan, naming the query and `to_regclass(`; - the queries' constants renamed so that none begins `query` (a test-side record): caught by the scan's own empty-collection failure. -- [ ] 5.8 `TestCheckQueriesQualifyEveryName` (`internal/db/qualified_names_test.go`, design D2, the qualification principle), in package `db_test`: parse `runtime_user.go` and `migrations.go` with `go/parser`, collect every string constant whose name begins `query` (the check queries, the boot-grant statement and the pending check's queries), remove string literals and `--` comments, and fail for each function call (an identifier directly followed by `(`, after optional spaces, that no `pg_catalog.` precedes) and each catalog relation (an identifier starting `pg_` that no `pg_catalog.` precedes and that is not `pg_catalog` itself), naming the query's constant and the token. The exceptions are a table in the test, each with its reason: `ANY`, `ALL`, `IN`, `EXISTS`, `VALUES`, `CAST`, `COALESCE`, `CASE`, `AND`, `OR`, `ON`, `FROM`, `JOIN`, `SELECT`, `WHERE`, `WHEN`, `THEN`, `ELSE` are SQL syntax that precedes a parenthesis, not calls; an identifier directly after `AS` is an alias with a column list (`AS p(priv)`). The test also fails when it finds no `query` constant in either file, so a rename cannot empty it. The breaks in item 5.7 name what the scan must catch. +- [ ] 5.8 `TestCheckQueriesQualifyEveryName` (`internal/db/qualified_names_test.go`, design D2, the qualification principle), in package `db_test`: parse `runtime_user.go` and `migrations.go` with `go/parser`, collect every string constant whose name begins `query` (the check queries, the boot-grant statement and the pending check's queries), remove string literals and `--` comments, and fail for each function call (an identifier directly followed by `(`, after optional spaces, that no `pg_catalog.` precedes) and each catalog relation (an identifier starting `pg_` that no `pg_catalog.` precedes and that is not `pg_catalog` itself), naming the query's constant and the token. The exceptions are a table in the test, each with its reason: `ANY`, `ALL`, `IN`, `EXISTS`, `VALUES`, `CAST`, `COALESCE`, `CASE`, `AND`, `OR`, `ON`, `FROM`, `JOIN`, `SELECT`, `WHERE`, `WHEN`, `THEN`, `ELSE` are SQL syntax that precedes a parenthesis, not calls; an identifier directly after `AS` is an alias with a column list (`AS p(priv)`). The scan does not look at operators; the search-path pin covers them (design D2), and the planted-operator rows of items 5.3 and 5.5 show it. The pin's own constant (`SET LOCAL search_path = pg_catalog, pg_temp`) is an exception listed with its reason: `pg_temp` is a schema name, not a catalog relation. The test also fails when it finds no `query` constant in either file, so a rename cannot empty it. The breaks in item 5.7 name what the scan must catch. ## 6. Deployment docs and the procedure for an existing deployment