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]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
7a0f624fc3
commit
6b6b77c105
@@ -13,7 +13,7 @@ const sections: readonly DoctorSection[] = Object.freeze([
|
||||
section("Platform", ["platform.runtime"]),
|
||||
section("Modules", ["modules.discovery", "modules.dependencies", "modules.permissions"]),
|
||||
section("Data definitions", ["schemas.discovery", "migrations.discovery"]),
|
||||
section("PostgreSQL", ["postgresql.connection", "postgresql.plan"]),
|
||||
section("PostgreSQL", ["database.transport", "postgresql.connection", "postgresql.plan"]),
|
||||
section("Identity", ["identity.configuration", "identity.client-address"]),
|
||||
section("Object storage", ["object-storage.integration"]),
|
||||
]);
|
||||
|
||||
@@ -7,6 +7,14 @@ import {
|
||||
} from "@ekklesia/contracts";
|
||||
import { getTableName } from "drizzle-orm";
|
||||
import { type PermissionRegistry, PermissionRegistryError } from "../authorization/index.js";
|
||||
import {
|
||||
assertProductionDatabaseTransport,
|
||||
type DatabaseTransportEnvironment,
|
||||
type DatabaseTransportPolicyInput,
|
||||
DatabaseTransportError,
|
||||
createDatabaseTransportPolicy,
|
||||
describeDatabaseTransport,
|
||||
} from "../database/transport.js";
|
||||
import {
|
||||
type ClientAddressConfigInput,
|
||||
ClientAddressConfigError,
|
||||
@@ -41,6 +49,7 @@ import {
|
||||
} from "../modules/index.js";
|
||||
|
||||
export type DoctorCheckId =
|
||||
| "database.transport"
|
||||
| "platform.runtime"
|
||||
| "modules.discovery"
|
||||
| "modules.dependencies"
|
||||
@@ -78,6 +87,10 @@ export interface DoctorIdentityConfigurationOptions {
|
||||
|
||||
export interface DoctorOptions {
|
||||
readonly clientAddress?: ClientAddressConfigInput | undefined;
|
||||
/** The environment the executable resolved, used only for transport policy evaluation; identity
|
||||
* configuration keeps its own `identityConfiguration.environment` for the same purpose. */
|
||||
readonly databaseEnvironment?: DatabaseTransportEnvironment | undefined;
|
||||
readonly databaseTransport?: DatabaseTransportPolicyInput | undefined;
|
||||
readonly databaseUrl?: string | undefined;
|
||||
readonly identityConfiguration?: DoctorIdentityConfigurationOptions | undefined;
|
||||
readonly moduleDiscovery: ModuleDiscoveryOptions;
|
||||
@@ -417,6 +430,41 @@ function identityConfigurationCheck(
|
||||
* combination is a `failure`, distinguishing the three states this pass's audit specifically calls
|
||||
* for: safe direct mode, explicitly configured proxy mode, and an invalid configuration.
|
||||
*/
|
||||
/**
|
||||
* Pure configuration validation only: never opens a connection, and never prints the database URL,
|
||||
* host, port, database name, credentials, certificate, or CA material. An unsafe production
|
||||
* transport is a blocking `failure`, not a warning — a production deployment that would reach
|
||||
* PostgreSQL without certificate-verified TLS is not "ready with warnings".
|
||||
*/
|
||||
function databaseTransportCheck(
|
||||
input: DatabaseTransportPolicyInput | undefined,
|
||||
databaseUrl: string | undefined,
|
||||
environment: DatabaseTransportEnvironment | undefined,
|
||||
): DoctorCheck {
|
||||
const effectiveEnvironment = environment ?? "production";
|
||||
try {
|
||||
const policy = createDatabaseTransportPolicy(input, { environment: effectiveEnvironment });
|
||||
if (typeof databaseUrl === "string" && databaseUrl.trim() !== "") {
|
||||
// Also rejects a URL carrying TLS parameters that could override the policy.
|
||||
assertProductionDatabaseTransport(databaseUrl, policy, effectiveEnvironment);
|
||||
}
|
||||
return freezeCheck(
|
||||
"database.transport",
|
||||
"pass",
|
||||
policy.mode === "verify-full"
|
||||
? "Database TLS verified"
|
||||
: "Database TLS disabled (development)",
|
||||
[describeDatabaseTransport(policy)],
|
||||
);
|
||||
} catch (error) {
|
||||
const code =
|
||||
error instanceof DatabaseTransportError ? error.code : "DATABASE_TRANSPORT_INVALID";
|
||||
return freezeCheck("database.transport", "failure", "Database transport unsafe", [
|
||||
`[${code}] The configured database transport is not permitted.`,
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
function clientAddressCheck(input: ClientAddressConfigInput | undefined): DoctorCheck {
|
||||
try {
|
||||
const config = createClientAddressConfig(input, TRUSTED_CLIENT_ADDRESS_HEADER);
|
||||
@@ -580,9 +628,16 @@ export async function diagnoseEkklesia(
|
||||
);
|
||||
} else {
|
||||
try {
|
||||
// Doctor's own read-only connection uses the exact same validated transport policy every
|
||||
// other executable does, so a Doctor "connection established" can never mean something
|
||||
// weaker than what `pnpm api:start` or `pnpm migrations:apply` would use.
|
||||
const doctorTransport = createDatabaseTransportPolicy(options.databaseTransport, {
|
||||
environment: options.databaseEnvironment ?? "production",
|
||||
});
|
||||
const database = dependencies.createMigrationDatabase(databaseUrl, {
|
||||
connectionTimeoutMs: doctorPostgresTimeoutMs,
|
||||
operationTimeoutMs: doctorPostgresTimeoutMs,
|
||||
transport: doctorTransport,
|
||||
});
|
||||
const plan = await dependencies.planMigrations({ aggregate: migrations, database });
|
||||
postgresConnectionCheck = freezeCheck(
|
||||
@@ -598,6 +653,11 @@ export async function diagnoseEkklesia(
|
||||
|
||||
const identityCheck = identityConfigurationCheck(options.identityConfiguration);
|
||||
const clientAddressDoctorCheck = clientAddressCheck(options.clientAddress);
|
||||
const transportCheck = databaseTransportCheck(
|
||||
options.databaseTransport,
|
||||
options.databaseUrl,
|
||||
options.databaseEnvironment,
|
||||
);
|
||||
|
||||
const storageCheck = freezeCheck("object-storage.integration", "skipped", "Check unavailable", [
|
||||
"Core storage integration has not been implemented yet.",
|
||||
@@ -610,6 +670,7 @@ export async function diagnoseEkklesia(
|
||||
permissionsCheck,
|
||||
schemaCheck,
|
||||
migrationCheck,
|
||||
transportCheck,
|
||||
postgresConnectionCheck,
|
||||
postgresPlanCheck,
|
||||
identityCheck,
|
||||
|
||||
Reference in New Issue
Block a user