[samgbafa]

Linear: TC-1581 (with TC-1585 and TC-1586, each its own commit; the last commit only restores existing signatures).

What changes

tinycloud.sql/write covers data statements. Statements that change the schema (CREATE, ALTER and DROP of tables, views and indexes, including CREATE TABLE ... AS SELECT and temp objects) need a schema-conferring ability: sql/schema, sql/admin or sql/*. PRAGMA is unchanged (still admin). SQLite DDL only; DuckDB is a separate change.

The node used to choose one "preferred" ability per invocation (write before schema). It now judges each statement against every SQL ability the invocation holds: schema statements need a schema-conferring ability, data statements keep their write/read tiers, and a request that mixes both needs both (or admin/*). This covers query, execute, the schema list of execute, batch and named statements. The SQLite authorizer enforces the same rule on its own callbacks (and now also refuses CREATE TRIGGER), so a statement the parser misclassifies is still refused. The storage-quota gate classifies a statement as a write whoever may run it.

[global.storage.sql] ddl_requires_schema = "enforce" | "log" | "off", default enforce (no compose change, no migration, no version bump). log runs the statement and records it: one constant warning per request and a counter tinycloud_ddl_without_schema_total{service,kind}, with no SQL text, space ids, DIDs or table names. off keeps the earlier behaviour. See docs/sql-schema-authority.md and CHANGELOG.md.

Commits

1. TC-1581: the change above.
2. TC-1585: a WITH ... INSERT/UPDATE parses as a Query and was classed read-only, so it skipped the quota gate, emitted no write hook, reported no touched table (and so was not persisted by the durable save). Any statement tree containing a write is now a write. WITH ... DELETE is still not accepted by the parser.
3. TC-1586: after a request that held a PRAGMA, the connection is returned to its opening state (writable_schema, query_only, foreign_keys, ...), and the authorizer no longer lets a data statement write sqlite_master/sqlite_schema (a schema change, a pure query and admin still may).
4. Signatures: keeps the existing public signatures stable for the PRs that rebase on this one (feat(sql): a multi-statement request is one transaction (TC-1491, 1a part A) #384, feat(sql): durable local commit and reservation before commit (TC-1491, 1a part B) #386); see below.

For PRs rebasing on this (SQL-B)

Files touched in tinycloud-core/src/sql/: authority.rs (new), authorizer.rs, database.rs, parser.rs, service.rs, mod.rs (two lines). storage.rs is untouched. Elsewhere: tinycloud-node-server/src/routes/mod.rs (the /invoke SQL path and tests), config.rs, lib.rs (one builder call), tinycloud.toml, docs, CHANGELOG, and test fixtures (below).

Existing signatures are unchanged: SqlService::execute and execute_without_growth (still take a single String ability), parser::validate_sql(sql, caveats, &str), authorizer::create_authorizer(caveats, String, bool), DatabaseHandle::execute, sql_request_is_write(request, caveats, ability) (the ability is no longer consulted), and the in-module test helpers handle_message / handle_message_without_growth (now #[cfg(test)] wrappers).

New or changed pub items:

Item	Where
SqlAuthority, DdlPolicy (re-exported from sql), DdlKind, record_ddl_without_schema, ddl_without_schema_count	sql/authority.rs
SqlService::with_ddl_policy, execute_authorized, execute_without_growth_authorized	sql/service.rs
DatabaseHandle::execute_authorized	sql/database.rs
validate_sql_authorized, classify_sql, SqlClass; ParsedQuery gains ddl_without_schema: Option<DdlKind>	sql/parser.rs
create_authorizer_scoped, StatementScope	sql/authorizer.rs
SqlStorageConfig::ddl_requires_schema	config.rs

Behaviour to adapt to:

• classify_sql(sql) answers "is this a write / DDL" without any ability, and ParsedQuery.write_targets / is_read_only now cover writes nested in a query tree (TC-1585). New code that decides "is this a write" should use these.
• A test or caller that runs CREATE/ALTER/DROP with only sql/write is now refused. Use sql/schema or sql/admin for the fixture (or execute_authorized with [write, schema]).
• Private: DbMessage::Execute carries authority instead of ability; handle_message_authorized / handle_message_without_growth_authorized are the real implementations.

Migration

• First-party callers were audited. Clients that use the SDK are unaffected: it has requested sql/schema for DDL since js-sdk 2.5.0 and sends [write, schema] for execute({ schema }) and migrations. Every SDK pin found is >= 2.5.
• Two non-SDK callers need their own fixes (separate PRs): tinyboilerplate examples/notes (manifest needs schema) and listen otter-importer.
• js-sdk actionContains ignoring admin implying schema is a separate js-sdk fix.
• The log warning is not mapped in log-shipper/diagnostics.json, so it ships as an unclassified node_warning until it is. Mapping it changes the compose hash and is not part of this PR.

Tests

• Statement kind x ability matrix (write, schema, [write, schema], admin, *, read) in the parser, the authorizer and every request path; mixed execute{schema, sql} and batch; named statements; the off/log/enforce switch; an authorizer test with a deliberately misclassifying scope; PRAGMA unchanged; data statements on write alone (including CTE-wrapped). New tests are in new ddl_authority_tests modules; the existing test text is kept except for four authorizer tests that asserted write could run DDL.
• /invoke route tests: a [write] invocation with CREATE/DROP/ALTER is 403, a [write, schema] one passes, schema alone runs DDL only; CTE-wrapped writes on a full space are 402.
• Existing fixtures that created tables under write now use sql/admin (service/route seeds) or hold schema (HTTP sessions); the test/ e2e crates were updated the same way.

Not run here: the PostgreSQL jobs (CI only).

Review round 2 (commits 4081f17..1eecc62)

• Unregistered tinycloud.sql/* abilities confer nothing: they no longer turn schema-only into "has a data ability" or a read grant into a data grant (is_schema_only, is_read_tier_only).
• Admin PRAGMAs are allow-listed in the authorizer (header values set/read, check pragmas, read-only reports; everything else refused for everyone). Any request where a non-report PRAGMA was allowed resets the connection afterwards, for in-memory and file-backed databases; a failed reset retires the actor (deregistered before the error reply) and the next request rebuilds it. This also closes process-wide settings reachable by an admin through PRAGMA. The first-token detection is removed.
• Nested block comments are refused for every statement, not only PRAGMA.
• A read-only statement gets only the schema-table UPDATE SQLite makes for pragma_* functions, not INSERT or DELETE.
• Docs/CHANGELOG: exact meaning of off, log counts parser-detected DDL only, mixed schema+sql requests are not transactional, the PRAGMA allow-list.
• Log-mode counter tests are serialised (they raced under parallel runs).

CI fixture fix (e146b55)

postgres_round_trip_budget runs the account-index DDL, which the SDK sends with [write, schema]. The fixture delegated and invoked sql/write only, so it was correctly refused. Its session now delegates read, write and schema, and the account SQL invocation holds [write, schema] (new invocation_with helper). Test-only: tinycloud-node-server/tests/postgres_round_trip_budget.rs.

Verified against a local PostgreSQL 14 (the CI job uses 16): postgres_round_trip_budget passes with the budgets unchanged (sign-in 77, kv put 38, kv get 22, kv batch get 22); it failed with the CI message before the change.

Audit of every test that runs DDL or is Postgres-gated (run against that local Postgres):

• Already holding schema/admin from the earlier commits: postgres_pool_timeouts, request_telemetry_authenticated, space_size_restart, the test/ e2e crates, core data_owners.
• Run and passing with no change needed: postgres_connection_recovery (its DDL is Postgres-side, not the SQL service), postgres_kv_write_throughput (ignored), admin_stats, kv_batch_read, request_telemetry, sql_identity_cli, at_rest_harness, core sqlx_request_telemetry and embedded_request_telemetry, the core postgres_* lib tests, and the node lib tests (613).
• Gated tests in src/ (webhook_dispatcher, policy_v3/test_harness, core test_support) use no SQL ability.
• Not run: the iptables blackhole_* tests; core db::test::postgres_space_filter_survives_a_collation_that_ignores_the_sentinel needs PostgreSQL 16 collation rules and fails on 14 (it does not touch SQL abilities or any file in this PR).
• SDK-driven probes test/probe/write-probe.ts and personal-node-check.ts run CREATE TABLE through the SDK; they depend on the SDK session holding schema (js-sdk >= 2.5), so no Rust fixture applies. Not run here.

[samgbafa]

CHANGES REQUESTED at 90fd2a3

Independent Opus review of #389 (TC-1581, TC-1585, TC-1586) at this exact head. The reviewer session ended on a tool error while writing its notes, so this comment was posted by the Node lead from the reviewer's findings. The gate is a fresh Opus review at the next head. Details are deliberately high-level; no reproduction text is posted.

MAJOR-1: held-set handling of unregistered tinycloud.sql/* abilities (tinycloud-core/src/sql/authority.rs, has_non_schema_ability / is_read_tier_only). An unrecognised ability in the held set is treated as "not schema-only", so the schema tier's no-data rule can be skipped where main treated the same invocation as plain schema. Fix: consider only abilities that actually confer read, write or admin through ability_matches; ignore unknown ones; keep is_read_tier_only consistent. Add a regression test with an unregistered ability next to schema.

MAJOR-2: the TC-1586 reset is incomplete. (a) It resets only file-backed databases; connection-level state on in-memory databases persists across requests, including state that defeats the growth guard and the in-memory-to-file promotion. (b) Other connection-level settings are not reset. (c) Detection is by the first token of a statement, which can miss a PRAGMA that is not in first position. (d) A reset failure is only logged, so the path fails open. Fix: reset unconditionally after any request that was allowed a non-whitelisted PRAGMA (or restrict admin PRAGMAs to an allow-list in the authorizer); fail closed by retiring the actor if the reset fails; cover in-memory databases. Tests: a state change by an admin request is not visible to a following write-only request on the same database, for both file-backed and in-memory databases.

MAJOR (pre-existing, not introduced here): process-wide SQLite settings reachable by an sql/admin holder can affect other spaces on the node. The allow-list in MAJOR-2 likely closes it. Recommend a separate issue.

MINOR/NIT. Block-comment nesting differs between the SQL parser and SQLite in request schema entries (class: parser divergence; fix by rejecting /* nesting or classifying on SQLite's own tokenisation). The authorizer's read-only exemption for sqlite_master DML looks unnecessary and keeps the MAJOR-2 chain open. Log mode counts only parser-detected DDL (acceptable as a migration aid; say so in the docs). CHANGELOG "off keeps the earlier behaviour" is not exact because some denials apply in every mode. The ability parameter of sql_request_is_write is unused. Mixed schema + sql requests are not transactional (the schema part applies before a later refusal); document it.

Verified OK. The DDL gate (parser and authorizer) over Query, Execute.sql, Execute.schema[], Batch and ExecuteStatement, including case, comments, BOM, WITH, EXPLAIN and multi-statement input; DDL needs schema, admin or *; [write,schema], [schema], [admin], [*], [read] regression sweep; caveat allow-lists apply to DDL in both layers; TC-1585 nested writes are classified as writes and a read-tier WITH ... SELECT still works; default enforce in code and an unknown config value fails startup; log and metric output carries no SQL text, names or ids; the changed tests are legitimate; the core sql tests pass (one needs a Postgres env var). The w5 failure legacy_inventory_attributes_from_invoked_abilities_without_a_grant reproduces on clean main e8926a6 and is not caused by this PR.

[samgbafa]

APPROVE at 1eecc62

Independent Opus security review, round 2, of #389 (TC-1581, TC-1585, TC-1586) at this exact head (base main e8926a6). The round-1 findings at 90fd2a3 are fixed. I found no blocker or major. Below are two minors and two nits; none of them blocks merge. No reproduction text is posted.

Round-1 findings

• MAJOR-1 (unregistered tinycloud.sql/* abilities): fixed. has_non_schema_ability, the new is_schema_only and is_read_tier_only (tinycloud-core/src/sql/authority.rs:142-157) now count only abilities that confer read, write or admin through ability_matches, so an unrecognised URN confers nothing. The parser (parser.rs:156) and the authorizer Tier (authorizer.rs:63-65) both use is_schema_only. Regression tests cover the authority, the parser, the authorizer and the actor (unregistered_abilities_confer_nothing and siblings). The [read, X], [write, X], [schema, X] and [X] cases all behave like the registered abilities alone.
• MAJOR-2 and TC-1589 (admin PRAGMA state, process-wide settings): fixed. The authorizer allow-list (authorizer.rs:199-291) gives everyone the seven plain schema reports. An admin also gets three things: user_version and application_id (set or read), the three check pragmas, and a short list of read-only reports, which are refused when a value is given. Every other PRAGMA is refused. The check runs on the name SQLite reports, so it holds for case variants, schema-qualified names, the function-call value form and pragma_* table-valued functions (SQLite prepares the inner PRAGMA through the same installed authorizer). I probed all of these at this head: no connection-level, process-wide or maintenance setting form got through on any request path. After a request in which a non-report PRAGMA was allowed (a tracker on the authorizer, not a first-token check), the connection is reset for in-memory (journal_mode = memory) and file-backed (WAL) databases. A failed reset retires the actor: it is deregistered by id, the request gets an error and nothing more is served on that connection. No registry entry points at a dead actor, and no request is served on a half-reset connection. Acknowledged writes are persisted under the operation lock before the reply (service.rs:339-363), so the retire loses no acknowledged data, apart from the narrow ordering issue in MINOR-1.
• Round-1 minors: nested block comments are now refused for every statement (0422927). sqlite_master writes by a read-only statement are limited to UPDATE (4096f73). The off/log wording, the log-mode scope and the non-transactional mixed request are documented. The unused ability parameter is now _ability and documented.

Findings

MINOR-1: the retire path closes its connection after deregistering (tinycloud-core/src/sql/database.rs:286-292, compared with the shutdown tail at :330-341). When a reset fails, the actor removes its registry entry and sends the error reply before break, so drop(conn) runs after both. That breaks an invariant the module documents as load-bearing (database.rs:330-338, service.rs:561-567, service.rs:795-797): a connection must be closed before its registry entry goes away, because the next request then hydrates and rewrites the cache files by path while the old connection is still closing. SQLite's close-time WAL checkpoint and cleanup also work by path. The Forbid + Absent error path (service.rs:330-334) skips its shutdown wait for the same reason. This is reachable only after a reset fails, which needs an I/O-class SQLite error, and only inside a short scheduling window.

• Fix: break out of the loop with the response sender, and in the shared tail run drop(conn), then remove_if by id, then send the error. That keeps "deregistered before the error reply" and restores "closed before deregistered".
• Regression test: with the injected reset failure on a file-backed database, assert that the retiring connection is closed (for example, a test-only closed flag set after drop(conn)) before the caller sees the error.

MINOR-2 (pre-existing, now documented as supported): admin header-value sets are not made durable (parser.rs:81-119 returns a PRAGMA with no write targets, and service.rs:339 persists only when there are write targets). PRAGMA user_version or application_id set by an admin applies to the live connection, but it is not saved to the artifact. A fresh service that rehydrates from the repository reads the old value. I confirmed this at this head for in-memory and file-backed databases. Base e8926a6 has the same code path, so this PR did not cause it, but docs/sql-schema-authority.md now says these values can be set.

• Fix (separate issue, outside TC-1581): treat an allowed header-value set as a write, so it is quota-gated and persisted.
• Regression test: set user_version as admin, rehydrate into a new service from the same repository, and assert the value is kept.

NIT-1: docs/sql-schema-authority.md:54 still says a pure query may write the schema tables. Since 4096f73 it may only issue the single UPDATE that SQLite makes for a pragma_* function.

NIT-2 (pre-existing on main): authorizer.rs:79-84. A schema-restricted tier may write SQLite's schema tables in any statement scope, although the comment says "while it runs DDL". The parser refuses non-DDL to schema-only holders, and SQLite refuses direct schema-table writes because writable_schema can no longer be set, so nothing is reachable through this. Tightening it to scope.is_ddl would make the authorizer match the parser on its own.

What I ran (CARGO_TARGET_DIR shared scratch, no Docker)

• gh pr view 389 --json headRefOid: 1eecc628a9bb50b99ec4421618c36385c0c271d0
• cargo fmt --all --check: clean
• RUSTFLAGS=-Dwarnings cargo clippy --workspace --all-targets: clean
• cargo test -p tinycloud-core: lib 581 passed, all integration targets passed. Run with CI unset: with CI set, the 61 postgres_* tests panic for lack of TINYCLOUD_TEST_POSTGRES_URL, which is environmental.
• cargo test -p tinycloud-node --lib: 613 passed
• node-server integration targets space_size_restart (6), request_telemetry_authenticated (4), sql_identity_cli (1), request_log_redaction (3), postgres_pool_timeouts (10): all passed
• w5 challenge_to_native_read and tc119_registry_wire_paths: passed. w5 sql_isolation: 8 passed, 1 failed, and the failure is legacy_inventory_attributes_from_invoked_abilities_without_a_grant (accepted, fails on clean main).
• m1 cross_layer_contract: 3 passed. This needed the target's requirements.txt in a temp venv; without it the grant generator fails on a missing module.
• cargo build -p tinycloud-node --features duckdb: OK
• policy_v3::account_alias_tests: 10 of 10 passed alone, and 15 of 15 passed alongside three concurrent full node-lib runs (those 15 full runs all passed, 613 each). It did not reproduce. The module tests the policy store and its SQLite writer lock, not the SQL service, and this PR does not touch policy_v3. Its fixture setup is wrapped in 10 s timeouts, which fits a load flake. It is not related to this PR.
• Throwaway probes in my worktree (not copied out):
  • read, write and admin tiers running pragma_* table-valued reports on a fresh connection: OK for in-memory and file-backed databases, so 4096f73 does not break them
  • the admin PRAGMA forms listed above: allow-list held, and connection state was unchanged afterwards
  • commented SQL: leading, trailing and inline comments, comments inside CTEs, comment markers inside literals, the SDK migration batch shape, and the SDK's commented table_info probe all pass. Nested or unterminated comments fail with a clean InvalidStatement or ParseError, with no panic.
  • header-value durability: MINOR-2

Verified OK (full pass)

• Every path into validate_sql_authorized and the authorizer: Query, Execute.sql, each Execute.schema entry, Batch, and ExecuteStatement (chain caveat or facts fallback). Each is validated against the pooled authority, the authorizer is installed for prepare and step, and the tracker covers all of them. Production code has no remaining caller of the single-ability validate_sql, create_authorizer or handle_message, and no prepared-statement cache.
• Authorizer parity with a bypassed parser: DDL actions need can_change_schema. CREATE TRIGGER, transaction control, savepoints and ATTACH are always refused. The CTAS read is refused to schema-only holders. A read-only scope gets UPDATE only on schema tables, and SQLite refuses even that without writable_schema, which nobody can set now.
• Ability set: it comes from the same-space capabilities of the admitted invocation, which verify_auth_admitted verifies. A single exact database path is enforced by select_database_scope. Pooling is no wider than the per-capability verification. The PRAGMA admin gate (require_sql_admin_for_request) is unchanged.
• The switch fails safe: DdlPolicy::default() is Enforce in code and config, and an unknown value fails to deserialize.
• Log hygiene: the new log lines are constant strings (the retire error and the log-mode warning). The counter label is a closed kind set. No SQL text, ids, DIDs or names appear.
• TC-1585: CTE-wrapped writes count as writes for the quota gate, hooks, durability, the read tier and readOnly. Read-tier WITH … SELECT and table-valued reports still work.
• Docs and CHANGELOG are neutral: no payloads and no step-by-step reproductions.

[samgbafa]

APPROVE at e146b55

Independent re-review of the delta from the approved head 1eecc62.

• The delta is test-only. It is one commit (e146b55), and git diff --stat 1eecc628..e146b553 lists a single file, tinycloud-node-server/tests/postgres_round_trip_budget.rs (+22/-2). No product code changed, and every other file is byte-identical to the approved head.
• The fixture now matches what the SDK sends. In js-sdk origin/master, SQLService dedupes the account-index migration batch to tinycloud.sql/schema + tinycloud.sql/write and sends both in one multi-ability invocation through invokeAny (node-sdk CHANGELOG 3841be4). The fixture now delegates sql read/write/schema on the account database and invokes the DDL with [write, schema] through the new invocation_with. invocation delegates to it, so the other call sites are unchanged.
• The measurement is not weakened. The DDL is still one measured POST /invoke. BUDGETS and ADMIN_STATS_BUDGET are untouched by the diff (sign-in 77, kv put 38, kv get 22, batch get 22).
• CI is green: 16 pass, 9 skipping. All 9 skipped jobs are release/deploy jobs (promote-watch, build-dstack, deploy-phala, rollback-audit, host, announce, build-local/global-artifacts, publish-homebrew-formula), which are expected to skip on a PR. test (workspace, PostgreSQL-backed), the at-rest harness, clippy, fmt and build all pass.

The round-1/round-2 approval carries to this head.