Commit Graph
6 Commits
Author SHA1 Message Date
ekklesiaandClaude Opus 5 6912680113 fix: prevent migration timeout overrides
pg-connection-string retains arbitrary query keys, so the driver's own
configuration key connectionTimeoutMillis reaches ConnectionParameters straight
from a URL — and it was not on the rejected list, while spellings that do
nothing were. Rejecting it closes that gap.

The precedence claims that guard was built on were partly wrong, so every source
was measured against the installed [email protected].0, a real blackhole listener, and a
real PostgreSQL rather than inferred from the merge:

  statement_timeout=0 / =900000   server reported 0 and 15min — wins outright
  query_timeout=0                 overrides the field, but the parsed value is
                                  the string "0", which is truthy, so every
                                  query gets a 0 ms read timer and fails
                                  nondeterministically — it breaks the client
                                  rather than loosening the bound
  connectionTimeoutMillis=0/900000 pollutes connectionParameters.connect_timeout
                                  (10 -> 0 or 900 s), but that field only feeds
                                  getLibpqConnectionString(), which the pure-JS
                                  client never calls; the effective timer was
                                  unchanged, with an explicit 500 ms still
                                  expiring at ~500 ms either way
  connect_timeout=0               no effect: recomputed from the explicit field
  options=-c statement_timeout=…  no effect, including a backslash-escaped
                                  spelling — the named startup parameter wins
  PGOPTIONS / PGCONNECT_TIMEOUT   no effect
  unrelated parameters            no effect, deliberately untouched

Only statement_timeout is a genuine downgrade. The rest are rejected as
unsupported configuration and defense in depth: no legitimate DATABASE_URL
carries a driver option, the polluted connect_timeout would take effect on the
native binding, and query_timeout produces intermittent failures an operator
could not diagnose. That distinction is now stated where it was previously
asserted wrongly — the earlier comment and ADR text claimed connect_timeout
overrode the explicit field, which measurement disproved.

Rejection still happens at the one shared construction boundary, before any
pg.Client exists, with the existing fixed POSTGRES_MIGRATION_CONFIGURATION_INVALID
and no URL, host, port, credential, parameter, or value in the message. The
10 s defaults, narrower test overrides, Pass 2 TLS policy and its URL-conflict
checks, and every unrelated parameter are unchanged.

Co-Authored-By: Claude Opus 5 <[email protected]>
2026-07-28 19:01:46 -05:00
ekklesiaandClaude Opus 5 2983789779 fix: harden migration command execution
Two problems in the migration commands: no timeouts, and SQL split on literal
bytes.

`migrations:plan` and `migrations:apply` constructed their connection with
neither a connection deadline nor an operation deadline — a real server reported
`statement_timeout = 0` on the migration connection. Both now default to 10
seconds for the connection deadline, the client query timeout, and PostgreSQL's
own statement_timeout, applied at the one shared construction boundary so a
future call site cannot silently omit them. History reads, advisory-lock
queries, transaction control, migration SQL, and history writes are all covered.
The existing lock-wait deadline, the Pass 2 TLS policy, and Doctor's and the
bootstrap CLI's own timeouts are untouched, and no environment variable or flag
was added.

Explicit client fields do not win under the pinned driver. [email protected].0 builds
parameters as `Object.assign({}, config, parse(config.connectionString))`, so
the URL overrides passed options — verified directly, where an explicit
statement_timeout of 10000 alongside `?statement_timeout=0` produced a
server-reported 0, while a clean URL produced 10s. A DATABASE_URL carrying
connect_timeout, query_timeout, statement_timeout, or an `options` value setting
statement_timeout is therefore refused with the existing fixed configuration
error, before any client is constructed and without echoing the URL, parameter,
or value. Unrelated parameters are left alone.

The runner split each migration on the literal `--> statement-breakpoint` bytes.
Those bytes inside a string literal, a dollar-quoted function body, a quoted
identifier, or a comment tore the migration into fragments that were no longer
valid SQL — each case verified to produce two fragments where one was required.
Each migration's exact SQL is now passed to the client once, with no values and
no prepared-statement name, inside the existing transaction, and its history row
inserted only after that batch succeeds. One transaction still spans all pending
migrations; advisory locking, checksum validation, approval preflight, ordering,
rollback, cleanup, and sanitized error identity are unchanged. The port method
is renamed executeStatement to executeMigrationSql so the contract says what it
receives.

PostgreSQL parses the batch because it owns the only correct parser. A homegrown
splitter would have to track single quotes, dollar quoting with arbitrary tags,
quoted identifiers, line comments, nested block comments, and escape syntax to
answer a question the server already answers, and a parser dependency would add
supply-chain surface for the same reason. Drizzle's marker lines are ordinary
`--` comments when the batch is sent whole. No custom SQL parser exists here.

Co-Authored-By: Claude Opus 5 <[email protected]>
2026-07-26 23:13:52 -05:00
ekklesiaandClaude Opus 5 6b6b77c105 fix: enforce production transport policy
Every PostgreSQL connection this repository opened was plaintext, in every
environment: no connection site set `ssl` at all, so credentials, session
tokens, and row data crossed the network in the clear and nothing in
configuration, startup, or `pnpm doctor` said so. The public identity URL
boundary was also looser than it appeared.

`core/src/database/transport.ts` is now the one typed policy. It has exactly two
modes: `verify-full` (TLS required, certificate chain verified, and server
identity verified against the configured host — the production default) and
`disable` (the development/test default, refused outright in production). There
is deliberately no encryption-only `require` mode, no chain-only `verify-ca`,
and no `rejectUnauthorized: false` escape hatch anywhere in the configuration
surface; every loose or aliased spelling is rejected in both environments, so a
typo never quietly becomes a weaker mode. Production has no plaintext exception
at all, not even for a loopback database — a connection string cannot prove the
database is process-local.

Verification is certificate *and* hostname. The policy supplies an explicit
`checkServerIdentity` pinned to the host parsed from DATABASE_URL, because the
pinned driver sets TLS `servername` only for non-IP hosts and Node performs no
identity check when it is unset — so `rejectUnauthorized: true` alone accepts a
certificate from any host the trusted CA ever signed when connecting by IP.
Both halves are proven against a real TLS-enabled PostgreSQL, including that the
same connection without the pinned callback succeeds. Every CA, certificate, and
key is generated at test runtime and deleted afterwards.

A DATABASE_URL may no longer carry its own TLS parameters. The pinned driver
merges parsed connection-string values over explicitly passed options, so a
`?sslmode=disable` in the URL would silently replace the policy's verified `ssl`
object; that now fails with a stable code, reported by both `pnpm doctor` and
the migration CLIs rather than as "failed unexpectedly".

One shared environment reader keeps `process.env` out of Core's reusable
constructors, and every executable that opens a connection resolves the policy
identically: the API (validated before any pool, Better Auth instance, module
runtime, or listener exists), both migration commands, the bootstrap CLI, and
Doctor's own connection. Doctor reports it as a blocking failure, never a
warning, naming only the effective mode.

Separately, the identity base URL and every trusted origin must now be exactly
one canonical origin — the WHATWG origin serialization must equal the supplied
string — so a path, trailing slash, credentials, explicit default port, or
non-canonical case is rejected rather than normalized into something that no
longer matches what a browser sends. Production's loopback rejection now
classifies the address instead of comparing against three literals, covering
`.localhost`, all of 127.0.0.0/8, 0.0.0.0, `::`, expanded and IPv4-mapped
forms. This is a configuration-shape guarantee only: it does not resolve DNS or
prove reachability, and TLS protects the connection, not the exposure.

Co-Authored-By: Claude Opus 5 <[email protected]>
2026-07-26 17:03:52 -05:00
ekklesiaandClaude Sonnet 5 679a5d225c feat: establish core identity foundation
CI / verify (pull_request) Successful in 25s
CI / infrastructure-smoke (pull_request) Successful in 12s
CI / migration-smoke (pull_request) Successful in 22s
Adds Core-owned identity contracts, a Core-owned Better Auth database schema
(ekklesia_core_user/session/account/verification), a first-class Core-owned
migration source that coexists with module-owned migrations under the exact
existing migration system, and an explicit Better Auth construction boundary.
No authentication endpoint or sign-in UI is exposed yet (ADR 0010).

Co-Authored-By: Claude Sonnet 5 <[email protected]>
2026-07-21 16:39:15 -05:00
ekklesia 02ee3ccd80 feat: add Ekklesia doctor command
CI / verify (pull_request) Successful in 20s
CI / infrastructure-smoke (pull_request) Successful in 20s
CI / migration-smoke (pull_request) Successful in 25s
2026-07-21 10:42:21 -05:00
ekklesia 0b282865cb feat: add transactional module migration runner
CI / verify (pull_request) Successful in 19s
CI / infrastructure-smoke (pull_request) Successful in 20s
2026-07-21 08:52:41 -05:00