From 2b4f0afb726fca204dad3e7533a95d7100fdd7a1 Mon Sep 17 00:00:00 2001 From: ekklesia Date: Mon, 24 Aug 2026 19:15:49 -0500 Subject: [PATCH] fix: give Doctor the host configuration the runtime receives `pnpm doctor` and `startApi` each read their own subset of the same environment variables, and the subsets had drifted. Doctor never read `S3_*` or module secrets at all, so a correctly configured installation was told its object storage was "not configured" and every module requiring a bucket or a secret was reported blocked -- while the runtime, from the identical environment, bound those buckets and ran the modules. Both executables now map the environment through one `loadHostConfiguration(env)` in Core, which takes the environment as an argument and never reads `process.env` itself. Validation stays with the existing per-domain validators, so each caller keeps its own failure behaviour: startup refuses a rejected value, Doctor reports it as a sanitized code. Doctor's storage option becomes raw input like its mail option, so a half-configured provider is reported as a blocking configuration failure instead of throwing before a report exists. The outbound-email check -- computed, counted toward the result, but absent from every rendered section -- is now printed, and the rendered set is asserted against a real report so a future check cannot go missing the same way. Co-Authored-By: Claude Opus 5 --- .env.example | 4 + README.md | 23 +- apps/api/src/config.ts | 2 +- apps/api/src/server.ts | 107 +---- .../tests/host-configuration-parity.test.ts | 169 +++++++ core/package.json | 4 + .../src/cli/database-transport-environment.ts | 35 +- core/src/cli/doctor.ts | 116 +++-- core/src/diagnostics/doctor-report.ts | 14 + core/src/diagnostics/doctor.ts | 75 +++- core/src/host/configuration.ts | 154 +++++++ core/src/host/index.ts | 7 + core/tests/doctor-host-configuration.test.ts | 425 ++++++++++++++++++ core/tests/doctor-report.test.ts | 29 ++ core/tests/doctor.test.ts | 8 +- .../host-configuration-boundaries.test.ts | 153 +++++++ core/tests/host-configuration.test.ts | 192 ++++++++ core/tests/mail-boundaries.test.ts | 22 +- docs/architecture/platform-architecture.md | 11 + docs/development/api.md | 8 +- docs/development/email.md | 4 + docs/development/repository-workflow.md | 50 ++- docs/module-development/README.md | 8 +- 23 files changed, 1411 insertions(+), 209 deletions(-) create mode 100644 apps/api/tests/host-configuration-parity.test.ts create mode 100644 core/src/host/configuration.ts create mode 100644 core/src/host/index.ts create mode 100644 core/tests/doctor-host-configuration.test.ts create mode 100644 core/tests/host-configuration-boundaries.test.ts create mode 100644 core/tests/host-configuration.test.ts diff --git a/.env.example b/.env.example index a803d81..dedf229 100644 --- a/.env.example +++ b/.env.example @@ -1,4 +1,8 @@ # Local development only. Replace every credential outside a developer workstation. +# +# `pnpm doctor` loads this file's uncommitted `.env` counterpart and evaluates exactly the host +# infrastructure configuration `pnpm api:start` would use, so the readiness report reflects the real +# deployment without the API process running. # Required for a local workstation, and only ever correct there. Anything else — including an # unset value — is validated as production, which requires certificate-verified PostgreSQL TLS diff --git a/README.md b/README.md index 2a58011..e795d0b 100644 --- a/README.md +++ b/README.md @@ -791,8 +791,18 @@ pnpm verify `pnpm doctor` builds the workspace first, then reports the supported Node.js runtime, installed modules and dependency order, schema and migration inventories, the effective PostgreSQL transport -(TLS) policy, optional read-only PostgreSQL planning, and the current object-storage integration -state. `.env.example` sets `NODE_ENV=development` deliberately: anything else, including an unset +(TLS) policy, optional read-only PostgreSQL planning, and the current object-storage and +outbound-email integration state. + +Doctor reads its host infrastructure configuration through the same +`loadHostConfiguration(process.env)` the API executable uses, so both derive identical settings and +identical defaults from one deployment environment: configured object storage is reported as +configured, a module secret the deployment has set is reported as satisfied, and a value neither +process could accept is reported as a configuration failure rather than silently interpreted one way +by the diagnostic and another way at startup. An optional service that is genuinely absent — no +`S3_ENDPOINT`, no `EKKLESIA_SMTP_HOST` — is still a complete deployment and reported as skipped. + +`.env.example` sets `NODE_ENV=development` deliberately: anything else, including an unset value, is validated as production, and production requires certificate-verified PostgreSQL TLS and rejects loopback/`http` identity origins — so the plaintext loopback database this repository's own development infrastructure provides is only usable with it set. @@ -817,7 +827,7 @@ Modules ✓ Dependency graph valid Dependency order is empty. ✓ Permission registry composed - 5 permissions registered + 6 permissions registered ○ Module lifecycle skipped DATABASE_URL is not configured. ○ Module configuration skipped @@ -827,13 +837,14 @@ Data definitions ✓ Schemas validated 0 modules · 0 tables ✓ Migrations validated - 6 migrations · checksums valid + 7 migrations · checksums valid core:0000_identity_core_tables · f8f2086ef06f core:0001_authorization_core_tables · 830e1fbc26d0 core:0002_authorization_integrity · 634294731180 core:0003_dashboard_navigation_tables · 2cde27807aa4 core:0004_module_lifecycle_state · c8b5d8b3d48b core:0005_module_configuration · c8d57f24f84b + core:0006_device_identity · 1db97b7e3ac5 PostgreSQL ✓ Database TLS verified @@ -856,6 +867,10 @@ Object storage ✓ No module requires object storage 0 modules declare a logical storage bucket. +Outbound email + ○ Outbound email not configured + No EKKLESIA_SMTP_HOST is set. This is a complete deployment: nothing in the platform sends email yet. + Result: READY WITH WARNINGS ``` diff --git a/apps/api/src/config.ts b/apps/api/src/config.ts index 5b99862..e409377 100644 --- a/apps/api/src/config.ts +++ b/apps/api/src/config.ts @@ -90,7 +90,7 @@ export interface ApiRuntimeConfig { export interface ApiRuntimeConfigInput { /** Optional. Defaults to direct mode (trusts only the actual immediate socket peer) — no proxy * trust unless explicitly configured. See `./client-address.js`. */ - readonly clientAddress?: ClientAddressConfigInput; + readonly clientAddress?: ClientAddressConfigInput | undefined; readonly databaseUrl?: unknown; /** Optional. Defaults to certificate-verified TLS in production and plaintext in development; * production can never select plaintext. See `@ekklesia/core/database`'s transport policy. */ diff --git a/apps/api/src/server.ts b/apps/api/src/server.ts index 27126f1..b8df129 100644 --- a/apps/api/src/server.ts +++ b/apps/api/src/server.ts @@ -12,10 +12,10 @@ import { } from "@ekklesia/core/authorization"; import { createDashboardStore } from "@ekklesia/core/dashboard"; import { createDeviceStore } from "@ekklesia/core/devices"; +import { type HostEnvironment, loadHostConfiguration } from "@ekklesia/core/host"; import { createIdentityRuntime, type IdentityRateLimitOverride } from "@ekklesia/core/identity"; import { createSmtpMailProvider, type MailProvider } from "@ekklesia/core/mail"; import { - createEnvironmentModuleSecretSource, createModuleConfigurationStore, createModuleLifecycleStore, createModuleUiPublicationRegistry, @@ -35,7 +35,6 @@ import { type ServerType, serve } from "@hono/node-server"; import type { Hono } from "hono"; import { createApiApp } from "./app.js"; import { - type ClientAddressConfigInput, type ClientAddressResolver, createClientAddressResolver, wrapFetchWithClientAddress, @@ -719,98 +718,40 @@ export function installShutdownSignalHandlers( process.on("SIGTERM", handleSignal); } -function readClientAddressInputFromEnvironment(): ClientAddressConfigInput | undefined { - const mode = process.env.EKKLESIA_CLIENT_ADDRESS_MODE; - if (mode === undefined) { - // Omitted entirely: `createClientAddressConfig` defaults to direct mode. Reading `header`/ - // `trustedProxies` only when `mode` is actually set keeps an operator from accidentally - // half-configuring proxy mode by setting only one of the three variables. - return undefined; - } - const trustedProxiesValue = process.env.EKKLESIA_CLIENT_ADDRESS_TRUSTED_PROXIES; - return { - header: process.env.EKKLESIA_CLIENT_ADDRESS_HEADER, - mode, - trustedProxies: trustedProxiesValue - ?.split(",") - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0), - }; -} - -function readConfigFromEnvironment(): { +/** + * Turns one deployment environment into this process's startup inputs. + * + * Every host infrastructure value comes from `loadHostConfiguration` — the same function + * `pnpm doctor` calls with the same environment — so the runtime and the diagnostic that reports on + * it can never disagree about what this deployment is configured to use. Only the two values that + * describe *this listener* rather than the host's infrastructure are read here. + */ +export function readConfigFromEnvironment(processEnvironment: HostEnvironment): { readonly input: Parameters[0]; readonly environment: "development" | "production"; + readonly moduleSecrets: ModuleSecretSource; } { - const trustedOriginsValue = process.env.EKKLESIA_IDENTITY_TRUSTED_ORIGINS; - const trustedOrigins = trustedOriginsValue - ?.split(",") - .map((origin) => origin.trim()) - .filter((origin) => origin.length > 0); - const clientAddress = readClientAddressInputFromEnvironment(); - + const host = loadHostConfiguration(processEnvironment); return { - environment: process.env.NODE_ENV === "development" ? "development" : "production", + environment: host.environment, input: { - ...(clientAddress === undefined ? {} : { clientAddress }), - databaseTransport: { - ...(process.env.EKKLESIA_DATABASE_CA === undefined - ? {} - : { certificateAuthority: process.env.EKKLESIA_DATABASE_CA }), - ...(process.env.EKKLESIA_DATABASE_TLS_MODE === undefined - ? {} - : { mode: process.env.EKKLESIA_DATABASE_TLS_MODE }), - }, - databaseUrl: process.env.DATABASE_URL, - host: process.env.EKKLESIA_API_HOST, - identity: { - baseUrl: process.env.EKKLESIA_IDENTITY_BASE_URL, - publicSignupEnabled: process.env.EKKLESIA_PUBLIC_SIGNUP_ENABLED, - secret: process.env.EKKLESIA_IDENTITY_SECRET, - trustedOrigins, - }, - port: process.env.EKKLESIA_API_PORT, - // The one place this process turns its environment into storage configuration. The names are - // the `S3_*` ones ADR 0004 already established for this repository's Compose stack, and Core's - // validator never reads `process.env` itself. - // The one place this process turns its environment into mail configuration. Names follow the - // platform's own `EKKLESIA__` convention rather than a provider's, and Core's - // validator never reads `process.env` itself. Absent host means this installation sends no - // outbound email, which is a complete deployment (ADR 0037). - mail: { - fromAddress: process.env.EKKLESIA_SMTP_FROM_ADDRESS, - fromName: process.env.EKKLESIA_SMTP_FROM_NAME, - host: process.env.EKKLESIA_SMTP_HOST, - // Optional as a pair: a relay authorized by source IP has neither. - password: process.env.EKKLESIA_SMTP_PASSWORD, - port: process.env.EKKLESIA_SMTP_PORT, - security: process.env.EKKLESIA_SMTP_SECURITY, - username: process.env.EKKLESIA_SMTP_USERNAME, - }, - storage: { - accessKey: process.env.S3_ACCESS_KEY, - bucketPrefix: process.env.S3_BUCKET_PREFIX, - // Optional, and unset is the ordinary case: it matters only where the name Core reaches - // storage by is not a name a browser could resolve. - deliveryEndpoint: process.env.S3_DELIVERY_ENDPOINT, - endpoint: process.env.S3_ENDPOINT, - forcePathStyle: process.env.S3_FORCE_PATH_STYLE, - region: process.env.S3_REGION, - secretKey: process.env.S3_SECRET_KEY, - }, + clientAddress: host.clientAddress, + databaseTransport: host.databaseTransport, + databaseUrl: host.databaseUrl, + host: processEnvironment.EKKLESIA_API_HOST, + identity: host.identity, + mail: host.mail, + port: processEnvironment.EKKLESIA_API_PORT, + storage: host.storage, }, + moduleSecrets: host.moduleSecrets, }; } export async function main(): Promise { - const { environment, input } = readConfigFromEnvironment(); + const { environment, input, moduleSecrets } = readConfigFromEnvironment(process.env); const config = createApiRuntimeConfig(input, { environment }); - // The one place this process turns its environment into module secrets. Core's factory takes the - // environment as an argument and never touches `process.env` itself, preserving the rule that - // only executables read the environment. - const running = await startApi(config, { - moduleSecrets: createEnvironmentModuleSecretSource(process.env), - }); + const running = await startApi(config, { moduleSecrets }); const address = running.server.address(); const boundPort = address && typeof address === "object" ? address.port : config.port; diff --git a/apps/api/tests/host-configuration-parity.test.ts b/apps/api/tests/host-configuration-parity.test.ts new file mode 100644 index 0000000..1f662e4 --- /dev/null +++ b/apps/api/tests/host-configuration-parity.test.ts @@ -0,0 +1,169 @@ +import { loadHostConfiguration } from "@ekklesia/core/host"; +import { createMailProviderConfig } from "@ekklesia/core/mail"; +import { createStorageProviderConfig } from "@ekklesia/core/storage"; +import { describe, expect, it } from "vitest"; +import { createApiRuntimeConfig } from "../src/config.js"; +import { readConfigFromEnvironment } from "../src/server.js"; + +/** + * `pnpm doctor` and `startApi` must reach the same conclusions from the same environment. They + * previously could not: each executable read its own subset of the variables, and Doctor's subset + * omitted `S3_*` and the module secrets entirely. + * + * These tests assert the property directly — the API's startup inputs and the inputs Doctor + * validates come from one `loadHostConfiguration` call and produce identical validated + * configuration, including every default. + */ + +const secretKey = "sk-example-secret-material"; +const smtpPassword = "sup3r-secret-relay-password"; + +function environment( + overrides: Readonly> = {}, +): Readonly> { + return { + DATABASE_URL: "postgres://ekklesia@db.example.org:5432/ekklesia", + EKKLESIA_IDENTITY_BASE_URL: "https://ekklesia.example.org", + EKKLESIA_IDENTITY_SECRET: "b7QwZ2rTn4Xs9LmVpKdF6yHc3JgEuNaW", + EKKLESIA_IDENTITY_TRUSTED_ORIGINS: "https://ekklesia.example.org", + NODE_ENV: "production", + ...overrides, + }; +} + +const configuredInfrastructure = Object.freeze({ + EKKLESIA_SMTP_FROM_ADDRESS: "noreply@example.org", + EKKLESIA_SMTP_HOST: "smtp.example.org", + EKKLESIA_SMTP_PASSWORD: smtpPassword, + EKKLESIA_SMTP_USERNAME: "relay@example.org", + S3_ACCESS_KEY: "AKIAEXAMPLEACCESSID", + S3_ENDPOINT: "https://storage.example.org", + S3_SECRET_KEY: secretKey, +}); + +/** Exactly what Doctor validates, from exactly the same load the API performs. */ +function doctorView(env: Readonly>) { + const host = loadHostConfiguration(env); + return { + mail: createMailProviderConfig(host.mail, { environment: host.environment }), + storage: createStorageProviderConfig(host.storage, { environment: host.environment }), + }; +} + +function apiView(env: Readonly>) { + const { environment: resolved, input } = readConfigFromEnvironment(env); + return createApiRuntimeConfig(input, { environment: resolved }); +} + +describe("host configuration parity between startApi and Doctor", () => { + it("derives identical object-storage configuration, defaults included", () => { + const env = environment(configuredInfrastructure); + + expect(apiView(env).storage).toEqual(doctorView(env).storage); + // Named explicitly rather than only compared, so a change to either default fails here loudly. + expect(apiView(env).storage).toEqual({ + accessKey: "AKIAEXAMPLEACCESSID", + bucketPrefix: "ekklesia", + deliveryEndpoint: "https://storage.example.org", + endpoint: "https://storage.example.org", + forcePathStyle: true, + region: "us-east-1", + secretKey, + }); + }); + + it("derives identical outbound-email configuration, defaults included", () => { + const env = environment(configuredInfrastructure); + + expect(apiView(env).mail).toEqual(doctorView(env).mail); + expect(apiView(env).mail).toMatchObject({ + host: "smtp.example.org", + port: 587, + security: "starttls", + }); + }); + + it("agrees that an installation with neither service is complete", () => { + const env = environment(); + + expect(apiView(env).storage).toBeUndefined(); + expect(doctorView(env).storage).toBeUndefined(); + expect(apiView(env).mail).toBeUndefined(); + expect(doctorView(env).mail).toBeUndefined(); + }); + + it("carries every storage field the direct-delivery pass established", () => { + const env = environment({ + ...configuredInfrastructure, + S3_BUCKET_PREFIX: "rogc", + S3_DELIVERY_ENDPOINT: "https://files.example.org", + S3_FORCE_PATH_STYLE: "false", + S3_REGION: "eu-west-1", + }); + + expect(apiView(env).storage).toEqual(doctorView(env).storage); + expect(apiView(env).storage).toMatchObject({ + bucketPrefix: "rogc", + deliveryEndpoint: "https://files.example.org", + forcePathStyle: false, + region: "eu-west-1", + }); + }); + + it("rejects the same malformed environment with the same sanitized code", () => { + for (const [overrides, code] of [ + [{ S3_ENDPOINT: "https://storage.example.org" }, "STORAGE_ACCESS_KEY_MISSING"], + [{ ...configuredInfrastructure, S3_ENDPOINT: "not-a-url" }, "STORAGE_ENDPOINT_INVALID"], + [ + { ...configuredInfrastructure, S3_ENDPOINT: "http://storage.example.org" }, + "STORAGE_ENDPOINT_INSECURE_IN_PRODUCTION", + ], + [ + { ...configuredInfrastructure, EKKLESIA_SMTP_PASSWORD: undefined }, + "MAIL_USERNAME_WITHOUT_PASSWORD", + ], + [{ ...configuredInfrastructure, EKKLESIA_SMTP_PORT: "70000" }, "MAIL_PORT_INVALID"], + ] as const) { + const env = environment(overrides); + expect(() => apiView(env)).toThrowError(expect.objectContaining({ code })); + expect(() => doctorView(env)).toThrowError(expect.objectContaining({ code })); + } + }); + + it("resolves module secrets from the same source the runtime is given", () => { + const env = environment({ + EKKLESIA_MODULE_SECRET__ANNOUNCEMENTS__STORAGE_CREDENTIAL: "module-secret-material", + }); + + expect( + readConfigFromEnvironment(env).moduleSecrets.get("announcements", "storageCredential"), + ).toBe("module-secret-material"); + expect(loadHostConfiguration(env).moduleSecrets.get("announcements", "storageCredential")).toBe( + "module-secret-material", + ); + }); + + it("reads only this listener's own address and port outside the shared loader", () => { + const config = apiView( + environment({ EKKLESIA_API_HOST: "0.0.0.0", EKKLESIA_API_PORT: "8080" }), + ); + + expect(config.host).toBe("0.0.0.0"); + expect(config.port).toBe(8080); + // Defaults, when unset, stay where they always were. + const defaults = apiView(environment()); + expect(defaults.host).toBe("127.0.0.1"); + expect(defaults.port).toBe(3000); + }); + + it("interprets a set-but-empty flag identically instead of one side ignoring it", () => { + // Doctor used to drop any falsy identity value before validating, so `startApi` refused to + // start on `EKKLESIA_PUBLIC_SIGNUP_ENABLED=` while Doctor reported the deployment ready. + const env = environment({ EKKLESIA_PUBLIC_SIGNUP_ENABLED: "" }); + + expect(loadHostConfiguration(env).identity.publicSignupEnabled).toBe(""); + expect(() => apiView(env)).toThrowError( + expect.objectContaining({ code: "PUBLIC_SIGNUP_ENABLED_INVALID" }), + ); + }); +}); diff --git a/core/package.json b/core/package.json index 0bc84be..5c48be7 100644 --- a/core/package.json +++ b/core/package.json @@ -50,6 +50,10 @@ "./mail": { "types": "./dist/mail/index.d.ts", "import": "./dist/mail/index.js" + }, + "./host": { + "types": "./dist/host/index.d.ts", + "import": "./dist/host/index.js" } }, "scripts": { diff --git a/core/src/cli/database-transport-environment.ts b/core/src/cli/database-transport-environment.ts index 0940f9e..f96940a 100644 --- a/core/src/cli/database-transport-environment.ts +++ b/core/src/cli/database-transport-environment.ts @@ -4,35 +4,28 @@ import { type DatabaseTransportPolicy, createDatabaseTransportPolicy, } from "../database/transport.js"; +import { loadHostConfiguration } from "../host/index.js"; /** - * The one place every Core-owned executable reads its database transport policy from the process - * environment, so `pnpm doctor`, `pnpm migrations:plan`, `pnpm migrations:apply`, and - * `pnpm authorization:bootstrap-owner` cannot drift from each other or from `apps/api`. + * The database transport policy for the small Core-owned executables — `pnpm migrations:plan`, + * `pnpm migrations:apply`, `pnpm modules:enable`, and the owner-bootstrap commands — resolved from + * the process environment. * - * Environment reading lives here, in the executable layer, deliberately: the reusable Core - * database/migration constructors keep taking explicit typed inputs and never touch `process.env`. - * `NODE_ENV` is interpreted exactly as `apps/api` interprets it — only the literal string - * `"development"` relaxes anything, so an unset or misspelled value validates as production. + * The mapping itself belongs to `loadHostConfiguration`, which `startApi` and `pnpm doctor` also + * call, so `NODE_ENV` and the two TLS variables have exactly one interpretation across every entry + * point. Environment *reading* still happens here, in the executable layer: the reusable Core + * database and migration constructors keep taking explicit typed inputs and never touch + * `process.env`. */ export function readDatabaseTransportFromEnvironment(): { readonly environment: DatabaseTransportEnvironment; readonly transport: DatabaseTransportPolicy; } { - const environment: DatabaseTransportEnvironment = - process.env.NODE_ENV === "development" ? "development" : "production"; - const transport = createDatabaseTransportPolicy( - { - ...(process.env.EKKLESIA_DATABASE_CA === undefined - ? {} - : { certificateAuthority: process.env.EKKLESIA_DATABASE_CA }), - ...(process.env.EKKLESIA_DATABASE_TLS_MODE === undefined - ? {} - : { mode: process.env.EKKLESIA_DATABASE_TLS_MODE }), - }, - { environment }, - ); - return { environment, transport }; + const host = loadHostConfiguration(process.env); + const transport = createDatabaseTransportPolicy(host.databaseTransport, { + environment: host.environment, + }); + return { environment: host.environment, transport }; } /** diff --git a/core/src/cli/doctor.ts b/core/src/cli/doctor.ts index c1deef4..3eed5b9 100644 --- a/core/src/cli/doctor.ts +++ b/core/src/cli/doctor.ts @@ -7,6 +7,7 @@ import { doctorExitCode, formatDoctorReport, } from "../diagnostics/index.js"; +import { type HostEnvironment, loadHostConfiguration } from "../host/index.js"; interface ModuleSdkPackageDocument { readonly version?: unknown; @@ -18,6 +19,18 @@ interface RootPackageDocument extends ModuleSdkPackageDocument { }; } +/** + * Everything Doctor needs that comes from the repository itself rather than the deployment + * environment, resolved by the caller so the environment mapping below stays pure and testable. + */ +export interface DoctorCommandMetadata { + readonly moduleSdkVersion: string; + readonly modulesDirectory: string; + readonly platformVersion: string; + readonly runtimeRange: string; + readonly runtimeVersion: string; +} + async function readJson(pathname: string): Promise { return JSON.parse(await readFile(pathname, "utf8")) as unknown; } @@ -36,40 +49,40 @@ function requiredString(value: unknown): string { return value; } -function readIdentityConfigurationFromEnvironment(): DoctorOptions["identityConfiguration"] { - const secret = process.env.EKKLESIA_IDENTITY_SECRET; - const baseUrl = process.env.EKKLESIA_IDENTITY_BASE_URL; - const publicSignupEnabled = process.env.EKKLESIA_PUBLIC_SIGNUP_ENABLED; - const trustedOriginsValue = process.env.EKKLESIA_IDENTITY_TRUSTED_ORIGINS; - const trustedOrigins = trustedOriginsValue - ? trustedOriginsValue - .split(",") - .map((origin) => origin.trim()) - .filter((origin) => origin.length > 0) - : undefined; - +/** + * Turns one deployment environment into Doctor's inputs. + * + * Every infrastructure value comes from `loadHostConfiguration` — the same function `apps/api` calls + * with the same environment — so `pnpm doctor` and `startApi` cannot disagree about what this + * deployment is configured to use. Doctor previously read its own subset here and silently omitted + * object storage and module secrets entirely, which made it report a correctly configured + * installation as having neither. + * + * Everything is handed over **raw**. Doctor validates it and reports a sanitized code, so a + * malformed value appears in the report — which is the whole reason an operator runs this command — + * instead of throwing before a report exists. + */ +export function createDoctorOptions( + environment: HostEnvironment, + metadata: DoctorCommandMetadata, +): DoctorOptions { + const host = loadHostConfiguration(environment); return { - ...(baseUrl ? { baseUrl } : {}), - environment: process.env.NODE_ENV === "development" ? "development" : "production", - ...(publicSignupEnabled ? { publicSignupEnabled } : {}), - ...(secret ? { secret } : {}), - ...(trustedOrigins ? { trustedOrigins } : {}), - }; -} - -function readClientAddressFromEnvironment(): DoctorOptions["clientAddress"] { - const mode = process.env.EKKLESIA_CLIENT_ADDRESS_MODE; - if (mode === undefined) { - return undefined; - } - const trustedProxiesValue = process.env.EKKLESIA_CLIENT_ADDRESS_TRUSTED_PROXIES; - return { - header: process.env.EKKLESIA_CLIENT_ADDRESS_HEADER, - mode, - trustedProxies: trustedProxiesValue - ?.split(",") - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0), + clientAddress: host.clientAddress, + databaseEnvironment: host.environment, + databaseTransport: host.databaseTransport, + databaseUrl: host.databaseUrl, + identityConfiguration: { ...host.identity, environment: host.environment }, + mailConfiguration: host.mail, + moduleDiscovery: { + moduleSdkVersion: metadata.moduleSdkVersion, + modulesDirectory: metadata.modulesDirectory, + platformVersion: metadata.platformVersion, + }, + moduleSecrets: host.moduleSecrets, + runtimeRange: metadata.runtimeRange, + runtimeVersion: metadata.runtimeVersion, + storageConfiguration: host.storage, }; } @@ -81,40 +94,15 @@ export async function runDoctorCommand(repositoryRoot = process.cwd()): Promise< const rootPackage = rootPackageDocument(rootValue); const moduleSdkPackage = rootPackageDocument(moduleSdkValue); - const databaseEnvironment = - process.env.NODE_ENV === "development" ? ("development" as const) : ("production" as const); - const report = await diagnoseEkklesia({ - clientAddress: readClientAddressFromEnvironment(), - databaseEnvironment, - databaseTransport: { - ...(process.env.EKKLESIA_DATABASE_CA === undefined - ? {} - : { certificateAuthority: process.env.EKKLESIA_DATABASE_CA }), - ...(process.env.EKKLESIA_DATABASE_TLS_MODE === undefined - ? {} - : { mode: process.env.EKKLESIA_DATABASE_TLS_MODE }), - }, - databaseUrl: process.env.DATABASE_URL, - identityConfiguration: readIdentityConfigurationFromEnvironment(), - // Raw, exactly like the identity settings above: Doctor validates them and reports a sanitized - // code, so a malformed SMTP value appears in the report instead of crashing the command. - mailConfiguration: { - fromAddress: process.env.EKKLESIA_SMTP_FROM_ADDRESS, - fromName: process.env.EKKLESIA_SMTP_FROM_NAME, - host: process.env.EKKLESIA_SMTP_HOST, - password: process.env.EKKLESIA_SMTP_PASSWORD, - port: process.env.EKKLESIA_SMTP_PORT, - security: process.env.EKKLESIA_SMTP_SECURITY, - username: process.env.EKKLESIA_SMTP_USERNAME, - }, - moduleDiscovery: { + const report = await diagnoseEkklesia( + createDoctorOptions(process.env, { moduleSdkVersion: requiredString(moduleSdkPackage.version), modulesDirectory: path.join(repositoryRoot, "modules"), platformVersion: requiredString(rootPackage.version), - }, - runtimeRange: requiredString(rootPackage.engines?.node), - runtimeVersion: process.versions.node, - }); + runtimeRange: requiredString(rootPackage.engines?.node), + runtimeVersion: process.versions.node, + }), + ); console.log(formatDoctorReport(report).join("\n")); return doctorExitCode(report); diff --git a/core/src/diagnostics/doctor-report.ts b/core/src/diagnostics/doctor-report.ts index 1e7d3fe..51fd4cd 100644 --- a/core/src/diagnostics/doctor-report.ts +++ b/core/src/diagnostics/doctor-report.ts @@ -22,8 +22,22 @@ const sections: readonly DoctorSection[] = Object.freeze([ section("PostgreSQL", ["database.transport", "postgresql.connection", "postgresql.plan"]), section("Identity", ["identity.configuration", "identity.client-address"]), section("Object storage", ["object-storage.integration", "object-storage.bindings"]), + section("Outbound email", ["outbound-email.configuration"]), ]); +/** + * Every check id, exactly once. + * + * A check absent from `sections` is still computed and still counted toward the overall result, but + * is never shown — so a configuration failure would flip an operator's report to NOT READY with + * nothing on screen explaining why. `outbound-email.configuration` was in precisely that state, and + * `doctor-report.test.ts` now compares this list against the ids of a real report, so a new check + * cannot be added without being rendered. + */ +export const RENDERED_DOCTOR_CHECK_IDS: readonly DoctorCheckId[] = Object.freeze( + sections.flatMap((entry) => [...entry.checkIds]), +); + const statusSymbols: Readonly> = Object.freeze({ failure: "✗", pass: "✓", diff --git a/core/src/diagnostics/doctor.ts b/core/src/diagnostics/doctor.ts index aa89d58..e68b84c 100644 --- a/core/src/diagnostics/doctor.ts +++ b/core/src/diagnostics/doctor.ts @@ -73,9 +73,12 @@ import { } from "../modules/index.js"; import { createS3StorageProvider, + createStorageProviderConfig, describeStorageProvider, physicalBucketName, + StorageConfigError, type StorageProviderConfig, + type StorageProviderConfigInput, } from "../storage/index.js"; export type DoctorCheckId = @@ -129,19 +132,27 @@ export interface DoctorOptions { readonly identityConfiguration?: DoctorIdentityConfigurationOptions | undefined; readonly moduleDiscovery: ModuleDiscoveryOptions; /** Where deployment-supplied module secrets come from, so Doctor's answer about a required secret - * matches what `startApi` would resolve. Omitted supplies nothing — Doctor then reports a required - * secret as missing, which is the honest answer for a deployment that has not set it. */ + * matches what `startApi` would resolve. Omitted supplies nothing — Doctor then reports every + * required secret as missing, which is only the honest answer for a deployment that has actually + * set none, so an executable must pass the same source it would give the runtime. */ readonly moduleSecrets?: ModuleSecretSource; - /** The validated host storage provider, or `undefined` when this installation has none. Doctor - * never receives raw environment values and never prints a credential. */ - readonly storage?: StorageProviderConfig | undefined; + /** + * The host's object-storage settings as the executable read them, validated here. + * + * **Raw input**, for the same reason `mailConfiguration` below is raw: a half-configured provider + * is precisely the thing an operator runs Doctor to find, so Doctor has to be the one that + * validates it and reports the sanitized code. A pre-validated option meant a bad value threw + * before a report existed, and — because the executable then had to validate it itself — meant + * `pnpm doctor` simply never passed one at all, reporting configured storage as absent. + */ + readonly storageConfiguration?: StorageProviderConfigInput | undefined; /** * The host's outbound mail settings as the executable read them, validated here. * - * Deliberately **raw input** rather than a validated config, exactly like `identityConfiguration`: - * a malformed SMTP setting is precisely the thing an operator runs Doctor to find, so Doctor has - * to be the one that validates it and reports the sanitized code. A pre-validated option would - * mean a bad value crashed the command instead of appearing in its report. + * Deliberately **raw input**, exactly like `identityConfiguration` and `storageConfiguration`: a + * malformed SMTP setting is precisely the thing an operator runs Doctor to find, so Doctor has to + * be the one that validates it and reports the sanitized code. A pre-validated option would mean + * a bad value crashed the command instead of appearing in its report. */ readonly mailConfiguration?: MailProviderConfigInput | undefined; readonly runtimeRange: string; @@ -851,6 +862,23 @@ export async function diagnoseEkklesia( options: DoctorOptions, dependencies: DoctorDependencies = productionDependencies, ): Promise { + // Resolved first, before module eligibility is decided: whether a provider is configured is an + // input to whether a module that declares a bucket can run, so validating it later would mean + // deciding eligibility against a question that had not been answered yet. + const hostEnvironment = + options.databaseEnvironment === "development" ? "development" : "production"; + let storageConfig: StorageProviderConfig | undefined; + let storageConfigurationError: string | undefined; + try { + storageConfig = createStorageProviderConfig(options.storageConfiguration, { + environment: hostEnvironment, + }); + } catch (error) { + // The sanitized code only — never the endpoint, the access key, or the secret key. + storageConfigurationError = + error instanceof StorageConfigError ? error.code : "STORAGE_CONFIGURATION_INVALID"; + } + let runtimeCheck: DoctorCheck; try { runtimeCheck = dependencies.runtimeSatisfies(options.runtimeVersion, options.runtimeRange) @@ -960,7 +988,7 @@ export async function diagnoseEkklesia( discoveredModules, configuration, { secrets: options.moduleSecrets ?? EMPTY_MODULE_SECRET_SOURCE, settings }, - { providerConfigured: options.storage !== undefined }, + { providerConfigured: storageConfig !== undefined }, ); eligibilityEntries = resolution.entries; lifecycleCheck = moduleLifecycleCheck(resolution.entries); @@ -1075,7 +1103,20 @@ export async function diagnoseEkklesia( // whether modules run. Conflating them would let a momentary network blip read as a module // problem (ADR 0030). let storageCheck: DoctorCheck; - if (!options.storage) { + if (storageConfigurationError !== undefined) { + // A failure, unlike an unreachable provider: this is not a network condition an operator can + // wait out. `startApi` refuses to start on exactly this input, so a deployment carrying it is + // genuinely not ready, and saying so is what stops the operator from discovering it at boot. + storageCheck = freezeCheck( + "object-storage.integration", + "failure", + "Object storage configuration invalid", + [ + `The object-storage settings were rejected: ${storageConfigurationError}.`, + "Nothing was connected to and no credential is shown. Correct the S3_* values and run Doctor again.", + ], + ); + } else if (!storageConfig) { storageCheck = freezeCheck( "object-storage.integration", "skipped", @@ -1084,14 +1125,14 @@ export async function diagnoseEkklesia( ); } else { try { - await dependencies.probeStorage(options.storage); + await dependencies.probeStorage(storageConfig); storageCheck = freezeCheck("object-storage.integration", "pass", "Object storage reachable", [ - describeStorageProvider(options.storage), + describeStorageProvider(storageConfig), // Said plainly rather than implied: this probe used the endpoint Core talks to, and a // separate browser-facing origin is by definition something a browser reaches and this // process may not. Reporting "reachable" without that caveat would let an operator conclude // direct delivery works when only half of it was tested. - ...(options.storage.deliveryEndpoint === options.storage.endpoint + ...(storageConfig.deliveryEndpoint === storageConfig.endpoint ? [] : [ "The browser delivery origin is configured separately and was not probed; verify it from a browser. Any reverse proxy in front of it must pass the original Host header through unchanged, or signed read URLs will be rejected.", @@ -1106,7 +1147,7 @@ export async function diagnoseEkklesia( "warning", "Object storage unreachable", [ - describeStorageProvider(options.storage), + describeStorageProvider(storageConfig), "The configured storage service did not respond. Modules already running are unaffected.", ], ); @@ -1114,7 +1155,7 @@ export async function diagnoseEkklesia( } const storageBindings = discoveredModules - ? storageBindingsCheck(discoveredModules, eligibilityEntries, options.storage) + ? storageBindingsCheck(discoveredModules, eligibilityEntries, storageConfig) : freezeCheck("object-storage.bindings", "skipped", "Storage bindings skipped", [ "Complete module discovery is required.", ]); @@ -1128,7 +1169,7 @@ export async function diagnoseEkklesia( let mailConfigurationError: string | undefined; try { mailConfig = createMailProviderConfig(options.mailConfiguration, { - environment: options.databaseEnvironment === "development" ? "development" : "production", + environment: hostEnvironment, }); } catch (error) { // The sanitized code only — never the host, the username, or the password. diff --git a/core/src/host/configuration.ts b/core/src/host/configuration.ts new file mode 100644 index 0000000..6b07c0e --- /dev/null +++ b/core/src/host/configuration.ts @@ -0,0 +1,154 @@ +import type { ModuleSecretSource } from "@ekklesia/contracts"; +import type { DatabaseTransportPolicyInput } from "../database/index.js"; +import type { ClientAddressConfigInput } from "../identity/index.js"; +import type { MailProviderConfigInput } from "../mail/index.js"; +import { createEnvironmentModuleSecretSource } from "../modules/index.js"; +import type { StorageProviderConfigInput } from "../storage/index.js"; + +/** + * A deployment's environment, passed **as an argument**. + * + * `process.env` satisfies this, but nothing here reads it: the repository rule that only + * executables touch the process environment is preserved exactly as + * `createEnvironmentModuleSecretSource` preserves it, and it is what makes every variable this + * module understands testable without mutating global state. + */ +export type HostEnvironment = Readonly>; + +export type HostEnvironmentName = "development" | "production"; + +/** + * The identity settings as they appear in the environment — strings, unvalidated. + * + * Typed as `string | undefined` rather than `unknown` so this one object can be handed to both + * `createIdentityConfig` (which validates it) and Doctor's `identityConfiguration` (which validates + * it and reports the sanitized code) without either side re-deriving it. + */ +export interface HostIdentityEnvironmentInput { + readonly baseUrl: string | undefined; + readonly publicSignupEnabled: string | undefined; + readonly secret: string | undefined; + readonly trustedOrigins: readonly string[] | undefined; +} + +/** + * Every host infrastructure input an Ekklesia executable resolves from its environment. + * + * Deliberately **unvalidated**: each consumer runs the same Core validator over these values, but + * they do so at different moments and want different failure behaviour. `startApi` wants a + * malformed value to abort startup; Doctor wants it reported as a sanitized code in a check. If this + * type were pre-validated, Doctor could never report a configuration error — the load would have + * thrown before a report existed. + * + * The API listener's own `host`/`port` are deliberately absent: they describe the API process, not + * the host infrastructure a diagnostic evaluates, and Doctor has no use for them. + */ +export interface HostConfiguration { + /** `"development"` only when `NODE_ENV` is exactly that string, so an unset or misspelled value + * validates as production and never silently relaxes a security rule. */ + readonly environment: HostEnvironmentName; + /** `undefined` when no mode is set at all, which selects direct mode. */ + readonly clientAddress: ClientAddressConfigInput | undefined; + readonly databaseTransport: DatabaseTransportPolicyInput; + readonly databaseUrl: string | undefined; + readonly identity: HostIdentityEnvironmentInput; + readonly mail: MailProviderConfigInput; + /** The deployment's module secrets, resolvable only under names Core derives from a module's own + * declared keys — there is no arbitrary variable lookup. */ + readonly moduleSecrets: ModuleSecretSource; + readonly storage: StorageProviderConfigInput; +} + +function splitList(value: string | undefined): readonly string[] | undefined { + if (value === undefined) { + return undefined; + } + return value + .split(",") + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0); +} + +/** Omitted entirely when no mode is set: reading `header`/`trustedProxies` only when `mode` is + * actually present keeps an operator from half-configuring proxy mode by setting one of three + * variables. */ +function readClientAddress(environment: HostEnvironment): ClientAddressConfigInput | undefined { + const mode = environment.EKKLESIA_CLIENT_ADDRESS_MODE; + if (mode === undefined) { + return undefined; + } + return { + header: environment.EKKLESIA_CLIENT_ADDRESS_HEADER, + mode, + trustedProxies: splitList(environment.EKKLESIA_CLIENT_ADDRESS_TRUSTED_PROXIES), + }; +} + +/** + * The one place any Ekklesia executable turns a deployment environment into host infrastructure + * configuration. + * + * It exists because the alternative was demonstrably wrong: `apps/api` and `pnpm doctor` each read + * their own subset of the same variables, and the subsets drifted. Doctor never read `S3_*` at all, + * so a correctly configured installation was told its object storage was "not configured" and every + * module requiring a bucket was reported blocked — while `startApi`, from the identical environment, + * bound those buckets and ran the modules. A diagnostic that disagrees with the runtime it + * diagnoses is worse than no diagnostic. + * + * So there is now exactly one mapping from variable names to typed inputs, and both executables call + * it. Validation stays where it already lives — `createIdentityConfig`, + * `createDatabaseTransportPolicy`, `createStorageProviderConfig`, `createMailProviderConfig` — and + * is deliberately *not* performed here, so each caller keeps its own failure behaviour. + * + * Nothing is read from `process.env`, and nothing here logs, prints, or serializes: the returned + * object carries the identity secret, both storage credentials, and the SMTP password. + */ +export function loadHostConfiguration(environment: HostEnvironment): HostConfiguration { + return Object.freeze({ + clientAddress: readClientAddress(environment), + databaseTransport: { + ...(environment.EKKLESIA_DATABASE_CA === undefined + ? {} + : { certificateAuthority: environment.EKKLESIA_DATABASE_CA }), + ...(environment.EKKLESIA_DATABASE_TLS_MODE === undefined + ? {} + : { mode: environment.EKKLESIA_DATABASE_TLS_MODE }), + }, + databaseUrl: environment.DATABASE_URL, + environment: environment.NODE_ENV === "development" ? "development" : "production", + identity: { + baseUrl: environment.EKKLESIA_IDENTITY_BASE_URL, + publicSignupEnabled: environment.EKKLESIA_PUBLIC_SIGNUP_ENABLED, + secret: environment.EKKLESIA_IDENTITY_SECRET, + trustedOrigins: splitList(environment.EKKLESIA_IDENTITY_TRUSTED_ORIGINS), + }, + // Names follow the platform's own `EKKLESIA__` convention rather than a + // provider's. Absent host means this installation sends no outbound email, which is a complete + // deployment (ADR 0037); username and password are optional as a pair, because a relay + // authorized by source IP has neither. + mail: { + fromAddress: environment.EKKLESIA_SMTP_FROM_ADDRESS, + fromName: environment.EKKLESIA_SMTP_FROM_NAME, + host: environment.EKKLESIA_SMTP_HOST, + password: environment.EKKLESIA_SMTP_PASSWORD, + port: environment.EKKLESIA_SMTP_PORT, + security: environment.EKKLESIA_SMTP_SECURITY, + username: environment.EKKLESIA_SMTP_USERNAME, + }, + moduleSecrets: createEnvironmentModuleSecretSource(environment), + // The `S3_*` names ADR 0004 already established for this repository's Compose stack. Absent + // endpoint means this installation has no object storage, which is a complete deployment + // (ADR 0030). + storage: { + accessKey: environment.S3_ACCESS_KEY, + bucketPrefix: environment.S3_BUCKET_PREFIX, + // Optional, and unset is the ordinary case: it matters only where the name Core reaches + // storage by is not a name a browser could resolve. + deliveryEndpoint: environment.S3_DELIVERY_ENDPOINT, + endpoint: environment.S3_ENDPOINT, + forcePathStyle: environment.S3_FORCE_PATH_STYLE, + region: environment.S3_REGION, + secretKey: environment.S3_SECRET_KEY, + }, + }); +} diff --git a/core/src/host/index.ts b/core/src/host/index.ts new file mode 100644 index 0000000..00bc76f --- /dev/null +++ b/core/src/host/index.ts @@ -0,0 +1,7 @@ +export { + type HostConfiguration, + type HostEnvironment, + type HostEnvironmentName, + type HostIdentityEnvironmentInput, + loadHostConfiguration, +} from "./configuration.js"; diff --git a/core/tests/doctor-host-configuration.test.ts b/core/tests/doctor-host-configuration.test.ts new file mode 100644 index 0000000..032a7ee --- /dev/null +++ b/core/tests/doctor-host-configuration.test.ts @@ -0,0 +1,425 @@ +import type { ModuleManifest } from "@ekklesia/contracts"; +import { describe, expect, it, vi } from "vitest"; +import { createPermissionRegistry } from "../src/authorization/index.js"; +import { createDoctorOptions } from "../src/cli/doctor.js"; +import { + type DoctorCheck, + type DoctorDependencies, + diagnoseEkklesia, + formatDoctorReport, +} from "../src/diagnostics/index.js"; +import type { + DiscoveredModule, + ModuleMigrationAggregate, + ModuleSchemaAggregate, +} from "../src/modules/index.js"; + +/** + * The bug this suite exists for was operational, not theoretical: `pnpm doctor` built its own + * `DoctorOptions` and simply never included object storage or module secrets, so a correctly + * configured installation was told its storage was "not configured" and every module requiring a + * bucket was reported blocked — while `startApi`, from the identical environment, ran them. + * + * These tests therefore go through `createDoctorOptions`, the exact function the CLI calls, with an + * environment fixture rather than a hand-built options object. A helper-only test would have passed + * happily on the broken code. + */ + +const databaseUrl = "postgres://ekklesia@db.example.org:5432/ekklesia"; +const accessKey = "AKIAEXAMPLEACCESSID"; +const secretKey = "sk-example-secret-material"; +const smtpPassword = "sup3r-secret-relay-password"; +const identitySecret = "b7QwZ2rTn4Xs9LmVpKdF6yHc3JgEuNaW"; +const moduleSecret = "module-secret-material"; + +const metadata = Object.freeze({ + moduleSdkVersion: "0.1.0", + modulesDirectory: "/not-rendered/modules", + platformVersion: "0.1.0", + runtimeRange: ">=24.11.0 <25", + runtimeVersion: "24.15.0", +}); + +function environment( + overrides: Readonly> = {}, +): Readonly> { + return { + DATABASE_URL: databaseUrl, + EKKLESIA_IDENTITY_BASE_URL: "https://ekklesia.example.org", + EKKLESIA_IDENTITY_SECRET: identitySecret, + EKKLESIA_IDENTITY_TRUSTED_ORIGINS: "https://ekklesia.example.org", + NODE_ENV: "production", + ...overrides, + }; +} + +const storageEnvironment = Object.freeze({ + S3_ACCESS_KEY: accessKey, + S3_ENDPOINT: "https://storage.example.org", + S3_SECRET_KEY: secretKey, +}); + +const mailEnvironment = Object.freeze({ + EKKLESIA_SMTP_FROM_ADDRESS: "noreply@example.org", + EKKLESIA_SMTP_HOST: "smtp.example.org", + EKKLESIA_SMTP_PASSWORD: smtpPassword, + EKKLESIA_SMTP_SECURITY: "starttls", + EKKLESIA_SMTP_USERNAME: "relay@example.org", +}); + +function discoveredModule(id: string, overrides: Record = {}): DiscoveredModule { + return { + directory: `/unpublished/${id}`, + entrypoints: { server: `/unpublished/${id}/server.js` }, + manifest: { + capabilities: [], + compatibility: { moduleSdk: "^0.1.0", platform: "^0.1.0" }, + configuration: [], + dependencies: { + capabilities: { optional: {}, required: {} }, + modules: { optional: {}, required: {} }, + }, + entrypoints: { server: "./dist/server.js" }, + id, + manifestVersion: 6, + name: `${id} module`, + permissions: [], + storage: { buckets: [] }, + version: "0.1.0", + ...overrides, + } as unknown as ModuleManifest, + manifestPath: `/unpublished/${id}/ekklesia.module.json`, + packageName: `@test/${id}`, + }; +} + +const storingModule = discoveredModule("announcements", { + storage: { buckets: [{ id: "announcement-assets" }] }, +}); + +const secretRequiringModule = discoveredModule("announcements", { + configuration: [{ key: "storageCredential", required: true, type: "secret" }], +}); + +function schemaAggregate(): ModuleSchemaAggregate { + return Object.freeze({ + discoveredModules: Object.freeze([]), + drizzleSchema: Object.freeze({}), + modules: Object.freeze([]), + tables: Object.freeze([]), + }); +} + +function migrationAggregate(): ModuleMigrationAggregate { + return Object.freeze({ + discoveredModules: Object.freeze([]), + migrations: Object.freeze([]), + modules: Object.freeze([]), + }); +} + +function dependencies(overrides: Partial = {}): DoctorDependencies { + return { + composePermissions: vi.fn((modules: readonly DiscoveredModule[]) => + createPermissionRegistry( + modules.flatMap((discovered) => [...discovered.manifest.permissions]), + ), + ), + createMigrationDatabase: vi.fn(() => { + throw new Error("not used"); + }), + discoverMigrations: vi.fn(async () => migrationAggregate()), + discoverModules: vi.fn(async () => Object.freeze([])), + discoverSchemas: vi.fn(async () => schemaAggregate()), + planMigrations: vi.fn(async () => { + throw new Error("not used"); + }), + probeMail: vi.fn(async () => undefined), + probeStorage: vi.fn(async () => undefined), + readModuleConfiguration: vi.fn(async () => Object.freeze({ records: Object.freeze([]) })), + readModuleLifecycle: vi.fn(async () => Object.freeze({ modules: Object.freeze([]) })), + runtimeSatisfies: vi.fn(() => true), + ...overrides, + }; +} + +async function diagnose( + env: Readonly>, + services: DoctorDependencies = dependencies(), +): Promise<{ readonly report: Awaited> }> { + return { report: await diagnoseEkklesia(createDoctorOptions(env, metadata), services) }; +} + +function check( + report: Awaited>, + id: DoctorCheck["id"], +): DoctorCheck { + const found = report.checks.find((candidate) => candidate.id === id); + if (!found) { + throw new Error(`Missing test check ${id}.`); + } + return found; +} + +describe("Doctor host-configuration parity — object storage", () => { + it("recognizes configured object storage from the deployment environment", async () => { + const services = dependencies(); + + const { report } = await diagnose(environment(storageEnvironment), services); + const integration = check(report, "object-storage.integration"); + + // The regression itself: before this pass the CLI passed no storage at all and this read + // "skipped · Object storage not configured" from exactly this environment. + expect(integration.status).toBe("pass"); + expect(integration.summary).toBe("Object storage reachable"); + expect(services.probeStorage).toHaveBeenCalledWith({ + accessKey, + bucketPrefix: "ekklesia", + deliveryEndpoint: "https://storage.example.org", + endpoint: "https://storage.example.org", + forcePathStyle: true, + region: "us-east-1", + secretKey, + }); + }); + + it("still reports genuinely absent storage as absent", async () => { + const services = dependencies(); + + const { report } = await diagnose(environment(), services); + const integration = check(report, "object-storage.integration"); + + expect(integration.status).toBe("skipped"); + expect(integration.summary).toBe("Object storage not configured"); + expect(services.probeStorage).not.toHaveBeenCalled(); + }); + + it("reports a half-configured provider as a configuration failure, naming no value", async () => { + const services = dependencies(); + + const { report } = await diagnose( + environment({ S3_ENDPOINT: "https://storage.example.org" }), + services, + ); + const integration = check(report, "object-storage.integration"); + + // `startApi` refuses to start on this input, so the deployment is genuinely not ready — this is + // a failure rather than the warning an unreachable-but-valid provider gets. + expect(integration.status).toBe("failure"); + expect(integration.details[0]).toBe( + "The object-storage settings were rejected: STORAGE_ACCESS_KEY_MISSING.", + ); + expect(report.result).toBe("not-ready"); + // Nothing was connected to: a malformed configuration must not produce network traffic. + expect(services.probeStorage).not.toHaveBeenCalled(); + }); + + it("reports a malformed endpoint safely rather than crashing the command", async () => { + const { report } = await diagnose( + environment({ ...storageEnvironment, S3_ENDPOINT: "not-a-url" }), + ); + const integration = check(report, "object-storage.integration"); + + expect(integration.status).toBe("failure"); + expect(integration.details[0]).toBe( + "The object-storage settings were rejected: STORAGE_ENDPOINT_INVALID.", + ); + expect(JSON.stringify(integration)).not.toContain("not-a-url"); + }); + + it("refuses a plaintext production endpoint exactly as startup would", async () => { + const { report } = await diagnose( + environment({ ...storageEnvironment, S3_ENDPOINT: "http://storage.example.org" }), + ); + + expect(check(report, "object-storage.integration").details[0]).toBe( + "The object-storage settings were rejected: STORAGE_ENDPOINT_INSECURE_IN_PRODUCTION.", + ); + }); + + it("applies the same region and bucket-prefix defaults the runtime applies", async () => { + const services = dependencies(); + + await diagnose(environment(storageEnvironment), services); + + expect(services.probeStorage).toHaveBeenCalledWith( + expect.objectContaining({ + bucketPrefix: "ekklesia", + forcePathStyle: true, + region: "us-east-1", + }), + ); + }); + + it("defaults the browser delivery origin to the endpoint, and flags a separate one", async () => { + const { report: shared } = await diagnose(environment(storageEnvironment)); + expect(check(shared, "object-storage.integration").details).toHaveLength(1); + + const { report: separate } = await diagnose( + environment({ ...storageEnvironment, S3_DELIVERY_ENDPOINT: "https://files.example.org" }), + ); + const integration = check(separate, "object-storage.integration"); + expect(integration.details[0]).toContain("browser delivery https://files.example.org"); + expect(integration.details[1]).toContain("was not probed"); + }); + + it("binds a module's declared bucket instead of reporting it blocked", async () => { + const services = dependencies({ + discoverModules: vi.fn(async () => Object.freeze([storingModule])), + }); + + const { report } = await diagnose(environment(storageEnvironment), services); + + // The operator-visible consequence of the bug: this module was reported blocked by a Doctor run + // against the very environment that ran it. + expect(check(report, "object-storage.bindings").status).toBe("pass"); + expect(check(report, "modules.lifecycle").details).toContain("announcements 0.1.0 · enabled"); + }); + + it("still reports a bucket as unbound when no provider is configured", async () => { + const services = dependencies({ + discoverModules: vi.fn(async () => Object.freeze([storingModule])), + }); + + const { report } = await diagnose(environment(), services); + + expect(check(report, "object-storage.bindings").status).toBe("failure"); + expect(check(report, "modules.lifecycle").details).toContain( + "announcements 0.1.0 · blocked · storage_unavailable · announcement-assets", + ); + }); + + it("treats a malformed provider as unusable when deciding module eligibility", async () => { + const services = dependencies({ + discoverModules: vi.fn(async () => Object.freeze([storingModule])), + }); + + const { report } = await diagnose( + environment({ S3_ENDPOINT: "https://storage.example.org" }), + services, + ); + + // A provider that cannot be constructed cannot bind a bucket, and saying otherwise would report + // a module as runnable that startup would refuse to start at all. + expect(check(report, "object-storage.bindings").status).toBe("failure"); + }); +}); + +describe("Doctor host-configuration parity — module secrets", () => { + it("sees a secret the deployment actually set", async () => { + const services = dependencies({ + discoverModules: vi.fn(async () => Object.freeze([secretRequiringModule])), + }); + + const { report } = await diagnose( + environment({ + EKKLESIA_MODULE_SECRET__ANNOUNCEMENTS__STORAGE_CREDENTIAL: moduleSecret, + }), + services, + ); + const configuration = check(report, "modules.configuration"); + + // Same class of bug as storage: the CLI never passed a secret source, so a configured secret + // was always reported missing. + expect(configuration.status).toBe("pass"); + expect(check(report, "modules.lifecycle").details).toContain("announcements 0.1.0 · enabled"); + expect(JSON.stringify(report)).not.toContain(moduleSecret); + }); + + it("still reports a genuinely unset secret as missing", async () => { + const services = dependencies({ + discoverModules: vi.fn(async () => Object.freeze([secretRequiringModule])), + }); + + const { report } = await diagnose(environment(), services); + + expect(check(report, "modules.configuration").status).toBe("warning"); + expect(check(report, "modules.lifecycle").details).toContain( + "announcements 0.1.0 · blocked · secret_missing · storageCredential", + ); + }); +}); + +describe("Doctor host-configuration parity — outbound email regression", () => { + it("leaves an installation with no SMTP host reported as complete", async () => { + const services = dependencies(); + + const { report } = await diagnose(environment(), services); + const mailCheck = check(report, "outbound-email.configuration"); + + expect(mailCheck.status).toBe("skipped"); + expect(mailCheck.summary).toBe("Outbound email not configured"); + expect(services.probeMail).not.toHaveBeenCalled(); + }); + + it("leaves a configured relay reported as reachable, without its password", async () => { + const services = dependencies(); + + const { report } = await diagnose(environment(mailEnvironment), services); + const mailCheck = check(report, "outbound-email.configuration"); + + expect(mailCheck.status).toBe("pass"); + expect(services.probeMail).toHaveBeenCalledTimes(1); + expect(JSON.stringify(mailCheck)).not.toContain(smtpPassword); + }); + + it("leaves a malformed SMTP setting reported as a typed configuration failure", async () => { + const { report } = await diagnose( + environment({ ...mailEnvironment, EKKLESIA_SMTP_PORT: "70000" }), + ); + const mailCheck = check(report, "outbound-email.configuration"); + + expect(mailCheck.status).toBe("failure"); + expect(mailCheck.details[0]).toBe("The SMTP settings were rejected: MAIL_PORT_INVALID."); + expect(JSON.stringify(mailCheck)).not.toContain("70000"); + }); + + it("still refuses a plaintext production relay", async () => { + const { report } = await diagnose( + environment({ ...mailEnvironment, EKKLESIA_SMTP_SECURITY: "plaintext" }), + ); + + expect(check(report, "outbound-email.configuration").details[0]).toBe( + "The SMTP settings were rejected: MAIL_SECURITY_INSECURE_IN_PRODUCTION.", + ); + }); +}); + +describe("Doctor host-configuration parity — sanitization", () => { + it("prints no secret material anywhere in a fully configured report", async () => { + const services = dependencies({ + discoverModules: vi.fn(async () => Object.freeze([storingModule])), + }); + + const { report } = await diagnose( + environment({ + ...mailEnvironment, + ...storageEnvironment, + EKKLESIA_MODULE_SECRET__ANNOUNCEMENTS__STORAGE_CREDENTIAL: moduleSecret, + }), + services, + ); + const rendered = formatDoctorReport(report).join("\n"); + + for (const secret of [accessKey, secretKey, smtpPassword, identitySecret, moduleSecret]) { + expect(rendered).not.toContain(secret); + expect(JSON.stringify(report)).not.toContain(secret); + } + // The infrastructure host names are deliberately still shown: they carry no credential and are + // what an operator needs in order to act on the report. + expect(rendered).toContain("https://storage.example.org"); + expect(rendered).toContain("smtp.example.org"); + }); + + it("never serializes the environment object wholesale", async () => { + const services = dependencies(); + + const { report } = await diagnose( + environment({ ...storageEnvironment, TOTALLY_UNRELATED_SECRET: "unrelated-value" }), + services, + ); + + expect(JSON.stringify(report)).not.toContain("unrelated-value"); + expect(JSON.stringify(report)).not.toContain("S3_ACCESS_KEY"); + }); +}); diff --git a/core/tests/doctor-report.test.ts b/core/tests/doctor-report.test.ts index d6271b5..1e5f938 100644 --- a/core/tests/doctor-report.test.ts +++ b/core/tests/doctor-report.test.ts @@ -3,6 +3,7 @@ import { type DoctorDependencies, diagnoseEkklesia, formatDoctorReport, + RENDERED_DOCTOR_CHECK_IDS, } from "../src/diagnostics/index.js"; import type { MigrationDatabase } from "../src/migrations/index.js"; import type { ModuleMigrationAggregate, ModuleSchemaAggregate } from "../src/modules/index.js"; @@ -123,9 +124,37 @@ Object storage ✓ No module requires object storage 0 modules declare a logical storage bucket. +Outbound email + ○ Outbound email not configured + No EKKLESIA_SMTP_HOST is set. This is a complete deployment: nothing in the platform sends email yet. + Result: READY WITH WARNINGS`); }); + it("renders every check the report contains, so none can silently affect the result", async () => { + const report = await diagnoseEkklesia( + { + moduleDiscovery: { + moduleSdkVersion: "0.1.0", + modulesDirectory: "/not-rendered/modules", + platformVersion: "0.1.0", + }, + runtimeRange: ">=24.11.0 <25", + runtimeVersion: "24.15.0", + }, + noConfigurationDependencies(), + ); + + // `outbound-email.configuration` was computed, counted toward the overall result, and never + // printed — so a rejected SMTP setting turned an operator's report NOT READY with nothing on + // screen saying why. Asserting the two sets against each other is what keeps a future check + // from repeating it. + expect([...RENDERED_DOCTOR_CHECK_IDS].sort()).toEqual( + report.checks.map((entry) => entry.id).sort(), + ); + expect(new Set(RENDERED_DOCTOR_CHECK_IDS).size).toBe(RENDERED_DOCTOR_CHECK_IDS.length); + }); + it("does not depend on NO_COLOR or terminal interactivity", async () => { const report = await diagnoseEkklesia( { diff --git a/core/tests/doctor.test.ts b/core/tests/doctor.test.ts index fd6b04f..7154b2f 100644 --- a/core/tests/doctor.test.ts +++ b/core/tests/doctor.test.ts @@ -1378,7 +1378,7 @@ describe("Ekklesia Doctor — object storage", () => { }); const report = await diagnoseEkklesia( - { ...options(databaseUrl), storage: storageConfig }, + { ...options(databaseUrl), storageConfiguration: storageConfig }, services, ); const bindings = check(report, "object-storage.bindings"); @@ -1398,7 +1398,7 @@ describe("Ekklesia Doctor — object storage", () => { }); const report = await diagnoseEkklesia( - { ...options(databaseUrl), storage: storageConfig }, + { ...options(databaseUrl), storageConfiguration: storageConfig }, services, ); const integration = check(report, "object-storage.integration"); @@ -1420,7 +1420,7 @@ describe("Ekklesia Doctor — object storage", () => { }); const report = await diagnoseEkklesia( - { ...options(databaseUrl), storage: storageConfig }, + { ...options(databaseUrl), storageConfiguration: storageConfig }, services, ); const integration = check(report, "object-storage.integration"); @@ -1448,7 +1448,7 @@ describe("Ekklesia Doctor — object storage", () => { }); const report = await diagnoseEkklesia( - { ...options(databaseUrl), storage: storageConfig }, + { ...options(databaseUrl), storageConfiguration: storageConfig }, services, ); diff --git a/core/tests/host-configuration-boundaries.test.ts b/core/tests/host-configuration-boundaries.test.ts new file mode 100644 index 0000000..99d9762 --- /dev/null +++ b/core/tests/host-configuration-boundaries.test.ts @@ -0,0 +1,153 @@ +import { readdir, readFile } from "node:fs/promises"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +/** + * Structural guarantees about where deployment configuration is read, which no runtime assertion can + * make. + * + * The defect this pass fixed was structural: two executables each read their own subset of the same + * environment variables, and the subsets drifted until `pnpm doctor` reported a correctly configured + * installation as having no object storage. Restoring parity once is not enough — the arrangement + * that allowed the drift has to stay gone. + */ +const repositoryRoot = path.join(path.dirname(fileURLToPath(import.meta.url)), "..", ".."); + +async function readTypeScriptFiles( + directory: string, +): Promise> { + const results: Array<{ path: string; text: string }> = []; + const walk = async (current: string): Promise => { + for (const entry of await readdir(current, { withFileTypes: true })) { + const full = path.join(current, entry.name); + if (entry.isDirectory()) { + if (entry.name === "node_modules" || entry.name === "dist") { + continue; + } + await walk(full); + continue; + } + if (entry.name.endsWith(".ts") || entry.name.endsWith(".tsx")) { + results.push({ path: full, text: await readFile(full, "utf8") }); + } + } + }; + await walk(path.join(repositoryRoot, directory)); + return results; +} + +/** + * Strips block comments and whole-line `//` comments. + * + * These assertions are about what the *code* does, and prose that explains a rule inevitably quotes + * the very token the rule forbids. Trailing comments are deliberately left alone so a URL inside a + * string literal is never truncated. + */ +function withoutComments(text: string): string { + return text + .replaceAll(/\/\*[\s\S]*?\*\//gu, "") + .split("\n") + .filter((line) => !line.trimStart().startsWith("//")) + .join("\n"); +} + +/** + * Every variable the shared loader *interprets* — parses, defaults, splits, or classifies. + * + * `DATABASE_URL` is deliberately absent: it is passed through verbatim, and the small migration and + * owner-bootstrap commands legitimately read it directly. Interpretation is what drifts. + */ +const INTERPRETED_VARIABLES = Object.freeze([ + "EKKLESIA_CLIENT_ADDRESS_HEADER", + "EKKLESIA_CLIENT_ADDRESS_MODE", + "EKKLESIA_CLIENT_ADDRESS_TRUSTED_PROXIES", + "EKKLESIA_DATABASE_CA", + "EKKLESIA_DATABASE_TLS_MODE", + "EKKLESIA_IDENTITY_BASE_URL", + "EKKLESIA_IDENTITY_SECRET", + "EKKLESIA_IDENTITY_TRUSTED_ORIGINS", + "EKKLESIA_PUBLIC_SIGNUP_ENABLED", + "EKKLESIA_SMTP_FROM_ADDRESS", + "EKKLESIA_SMTP_FROM_NAME", + "EKKLESIA_SMTP_HOST", + "EKKLESIA_SMTP_PASSWORD", + "EKKLESIA_SMTP_PORT", + "EKKLESIA_SMTP_SECURITY", + "EKKLESIA_SMTP_USERNAME", + "NODE_ENV", + "S3_ACCESS_KEY", + "S3_BUCKET_PREFIX", + "S3_DELIVERY_ENDPOINT", + "S3_ENDPOINT", + "S3_FORCE_PATH_STYLE", + "S3_REGION", + "S3_SECRET_KEY", +]); + +describe("host configuration is read in exactly one place", () => { + it("never reads the process environment from the shared loader itself", async () => { + for (const file of await readTypeScriptFiles(path.join("core", "src", "host"))) { + // The loader is reusable Core code: it takes the environment as an argument, exactly as + // `createEnvironmentModuleSecretSource` does, so both executables can hand it theirs. + expect(withoutComments(file.text), file.path).not.toContain("process.env"); + } + }); + + it("interprets each host infrastructure variable in exactly one file", async () => { + const sources = [ + ...(await readTypeScriptFiles(path.join("core", "src"))), + ...(await readTypeScriptFiles(path.join("apps", "api", "src"))), + ]; + + for (const file of sources) { + const code = withoutComments(file.text); + for (const variable of INTERPRETED_VARIABLES) { + // Reading one of these anywhere else means a second, independently drifting interpretation + // of the same deployment — which is exactly the defect this pass fixed. Operator-facing + // message text that *names* a variable is untouched: only an actual read is forbidden. + expect(code, `${file.path} reads ${variable}`).not.toContain(`process.env.${variable}`); + expect(code, `${file.path} reads ${variable}`).not.toContain(`process.env["${variable}"]`); + } + } + }); + + it("leaves the two executables reading nothing but their own listener address", async () => { + const doctorCli = withoutComments( + await readFile(path.join(repositoryRoot, "core", "src", "cli", "doctor.ts"), "utf8"), + ); + const server = withoutComments( + await readFile(path.join(repositoryRoot, "apps", "api", "src", "server.ts"), "utf8"), + ); + + for (const source of [doctorCli, server]) { + expect(source).toContain("loadHostConfiguration"); + expect(source).not.toContain("DATABASE_URL"); + } + // `EKKLESIA_API_HOST`/`EKKLESIA_API_PORT` describe this listener rather than the host's + // infrastructure, so Doctor has no use for them and they stay with the process that binds. + expect(server).toContain("EKKLESIA_API_HOST"); + expect(doctorCli).not.toContain("EKKLESIA_API_"); + }); + + it("reads the environment only from executables, never from reusable Core", async () => { + for (const file of await readTypeScriptFiles(path.join("core", "src"))) { + if (file.path.includes(`${path.sep}cli${path.sep}`)) { + continue; + } + expect(withoutComments(file.text), file.path).not.toContain("process.env"); + } + }); + + it("gives Doctor the same object-storage and secret inputs the runtime receives", async () => { + const doctorCli = await readFile( + path.join(repositoryRoot, "core", "src", "cli", "doctor.ts"), + "utf8", + ); + + // The two options whose absence was the bug. A future edit that drops either fails here rather + // than in a support conversation about storage that Doctor says is missing. + expect(doctorCli).toContain("storageConfiguration: host.storage"); + expect(doctorCli).toContain("moduleSecrets: host.moduleSecrets"); + }); +}); diff --git a/core/tests/host-configuration.test.ts b/core/tests/host-configuration.test.ts new file mode 100644 index 0000000..0e9eabf --- /dev/null +++ b/core/tests/host-configuration.test.ts @@ -0,0 +1,192 @@ +import { describe, expect, it } from "vitest"; +import { createDatabaseTransportPolicy } from "../src/database/index.js"; +import { loadHostConfiguration } from "../src/host/index.js"; +import { createIdentityConfig } from "../src/identity/index.js"; +import { createMailProviderConfig } from "../src/mail/index.js"; +import { createStorageProviderConfig } from "../src/storage/index.js"; + +/** + * A complete, valid production deployment. Every optional service is present, because the bug this + * suite exists for was a *configured* service being reported absent — an empty environment would + * never have shown it. + */ +const productionEnvironment = Object.freeze({ + DATABASE_URL: "postgres://ekklesia@db.example.org:5432/ekklesia", + EKKLESIA_CLIENT_ADDRESS_MODE: "trusted-proxy", + EKKLESIA_CLIENT_ADDRESS_TRUSTED_PROXIES: "10.40.0.1, 10.40.0.2 ,", + EKKLESIA_IDENTITY_BASE_URL: "https://ekklesia.example.org", + EKKLESIA_IDENTITY_SECRET: "b7QwZ2rTn4Xs9LmVpKdF6yHc3JgEuNaW", + EKKLESIA_IDENTITY_TRUSTED_ORIGINS: "https://ekklesia.example.org, https://display.example.org", + EKKLESIA_MODULE_SECRET__ANNOUNCEMENTS__STORAGE_CREDENTIAL: "module-secret-material", + EKKLESIA_SMTP_FROM_ADDRESS: "noreply@example.org", + EKKLESIA_SMTP_FROM_NAME: "Ekklesia", + EKKLESIA_SMTP_HOST: "smtp.example.org", + EKKLESIA_SMTP_PASSWORD: "sup3r-secret-relay-password", + EKKLESIA_SMTP_PORT: "587", + EKKLESIA_SMTP_SECURITY: "starttls", + EKKLESIA_SMTP_USERNAME: "relay@example.org", + NODE_ENV: "production", + S3_ACCESS_KEY: "AKIAEXAMPLEACCESSID", + S3_BUCKET_PREFIX: "ekklesia", + S3_DELIVERY_ENDPOINT: "https://files.example.org", + S3_ENDPOINT: "https://storage.example.org", + S3_FORCE_PATH_STYLE: "true", + S3_REGION: "us-east-1", + S3_SECRET_KEY: "sk-example-secret-material", +}); + +describe("host configuration loading", () => { + it("never reads the process environment, only the object it is given", () => { + // Proved by construction: an empty environment produces an unconfigured deployment even though + // this test process's own `process.env` may carry DATABASE_URL, S3_*, or EKKLESIA_SMTP_*. + const host = loadHostConfiguration({}); + + expect(host.databaseUrl).toBeUndefined(); + expect(host.storage.endpoint).toBeUndefined(); + expect(host.mail.host).toBeUndefined(); + expect(host.identity.secret).toBeUndefined(); + }); + + it("carries every object-storage field the storage passes established", () => { + const host = loadHostConfiguration(productionEnvironment); + + expect(host.storage).toEqual({ + accessKey: "AKIAEXAMPLEACCESSID", + bucketPrefix: "ekklesia", + deliveryEndpoint: "https://files.example.org", + endpoint: "https://storage.example.org", + forcePathStyle: "true", + region: "us-east-1", + secretKey: "sk-example-secret-material", + }); + }); + + it("carries every SMTP field, leaving credentials optional as a pair", () => { + const host = loadHostConfiguration(productionEnvironment); + + expect(host.mail).toEqual({ + fromAddress: "noreply@example.org", + fromName: "Ekklesia", + host: "smtp.example.org", + password: "sup3r-secret-relay-password", + port: "587", + security: "starttls", + username: "relay@example.org", + }); + + const relayByAddress = loadHostConfiguration({ + EKKLESIA_SMTP_FROM_ADDRESS: "noreply@example.org", + EKKLESIA_SMTP_HOST: "smtp-relay.example.org", + }); + expect(relayByAddress.mail.username).toBeUndefined(); + expect(relayByAddress.mail.password).toBeUndefined(); + // A source-IP-authorized relay has no credentials at all, and that must still validate. + expect(createMailProviderConfig(relayByAddress.mail)).toBeDefined(); + }); + + it("resolves module secrets only under names derived from a module's own declared key", () => { + const host = loadHostConfiguration(productionEnvironment); + + expect(host.moduleSecrets.get("announcements", "storageCredential")).toBe( + "module-secret-material", + ); + expect(host.moduleSecrets.get("announcements", "unknownKey")).toBeUndefined(); + // There is no accessor that takes a raw variable name, so a module can never reach these. + expect(host.moduleSecrets.get("announcements", "DATABASE_URL")).toBeUndefined(); + }); + + it("treats only the exact string development as development", () => { + expect(loadHostConfiguration({ NODE_ENV: "development" }).environment).toBe("development"); + for (const value of ["Development", "dev", "test", "", "production", undefined]) { + expect(loadHostConfiguration({ NODE_ENV: value }).environment).toBe("production"); + } + }); + + it("omits client-address settings entirely unless a mode is set", () => { + expect(loadHostConfiguration({}).clientAddress).toBeUndefined(); + // Half-configuring proxy mode by setting only the header must not silently enable it. + expect( + loadHostConfiguration({ EKKLESIA_CLIENT_ADDRESS_HEADER: "x-forwarded-for" }).clientAddress, + ).toBeUndefined(); + expect(loadHostConfiguration(productionEnvironment).clientAddress).toEqual({ + header: undefined, + mode: "trusted-proxy", + trustedProxies: ["10.40.0.1", "10.40.0.2"], + }); + }); + + it("omits database transport keys that are unset rather than passing undefined values", () => { + expect(loadHostConfiguration({}).databaseTransport).toEqual({}); + expect( + loadHostConfiguration({ + EKKLESIA_DATABASE_CA: "-----BEGIN CERTIFICATE-----", + EKKLESIA_DATABASE_TLS_MODE: "verify-full", + }).databaseTransport, + ).toEqual({ + certificateAuthority: "-----BEGIN CERTIFICATE-----", + mode: "verify-full", + }); + }); + + it("splits comma-separated lists identically wherever they appear", () => { + const host = loadHostConfiguration({ + EKKLESIA_CLIENT_ADDRESS_MODE: "trusted-proxy", + EKKLESIA_CLIENT_ADDRESS_TRUSTED_PROXIES: " 10.0.0.1 ,, 10.0.0.2 ", + EKKLESIA_IDENTITY_TRUSTED_ORIGINS: " https://a.example.org ,, https://b.example.org ", + }); + + expect(host.identity.trustedOrigins).toEqual([ + "https://a.example.org", + "https://b.example.org", + ]); + expect(host.clientAddress?.trustedProxies).toEqual(["10.0.0.1", "10.0.0.2"]); + // A variable that is set but empty yields an empty list, not `undefined` — the two mean + // different things to the identity validator (`TRUSTED_ORIGINS_EMPTY` vs + // `TRUSTED_ORIGINS_MISSING`), and both executables must see the same one. + expect( + loadHostConfiguration({ EKKLESIA_IDENTITY_TRUSTED_ORIGINS: "" }).identity.trustedOrigins, + ).toEqual([]); + expect(loadHostConfiguration({}).identity.trustedOrigins).toBeUndefined(); + }); + + it("passes a malformed value through unchanged rather than repairing or discarding it", () => { + // The loader deliberately does not validate: `startApi` wants a malformed value to abort + // startup and Doctor wants it reported, and neither can happen if the load already threw or + // silently dropped it. + const host = loadHostConfiguration({ + EKKLESIA_PUBLIC_SIGNUP_ENABLED: "", + S3_ENDPOINT: "not-a-url", + }); + + expect(host.identity.publicSignupEnabled).toBe(""); + expect(host.storage.endpoint).toBe("not-a-url"); + expect(() => createStorageProviderConfig(host.storage)).toThrowError(/absolute http/u); + expect(() => createIdentityConfig(host.identity)).toThrowError(); + }); + + it("is accepted unchanged by every Core validator that consumes it", () => { + const host = loadHostConfiguration(productionEnvironment); + + expect(createIdentityConfig(host.identity, { environment: host.environment })).toMatchObject({ + baseUrl: "https://ekklesia.example.org", + publicSignupEnabled: false, + }); + expect( + createDatabaseTransportPolicy(host.databaseTransport, { environment: host.environment }), + ).toBeDefined(); + expect(createStorageProviderConfig(host.storage, { environment: host.environment })).toEqual({ + accessKey: "AKIAEXAMPLEACCESSID", + bucketPrefix: "ekklesia", + deliveryEndpoint: "https://files.example.org", + endpoint: "https://storage.example.org", + forcePathStyle: true, + region: "us-east-1", + secretKey: "sk-example-secret-material", + }); + expect(createMailProviderConfig(host.mail, { environment: host.environment })).toMatchObject({ + host: "smtp.example.org", + port: 587, + security: "starttls", + }); + }); +}); diff --git a/core/tests/mail-boundaries.test.ts b/core/tests/mail-boundaries.test.ts index 42fb61a..0751b53 100644 --- a/core/tests/mail-boundaries.test.ts +++ b/core/tests/mail-boundaries.test.ts @@ -118,15 +118,25 @@ describe("outbound email stays server-only", () => { } }); - it("reads SMTP environment only in the executable that owns configuration", async () => { - // Reusable Core code never touches `process.env`; the composition root turns environment into - // validated configuration and injects it. + it("maps SMTP environment in exactly one place, and never inside the mail provider", async () => { + // The mail provider takes validated configuration and nothing else — it never reads the + // environment, under any name. for (const file of await readTextFiles("core/src/mail")) { expect(file.text, file.path).not.toContain("process.env"); + expect(file.text, file.path).not.toContain("EKKLESIA_SMTP_"); } - const server = await readFile(path.join(repositoryRoot, "apps/api/src/server.ts"), "utf8"); - expect(server).toContain("EKKLESIA_SMTP_HOST"); - expect(server).toContain("EKKLESIA_SMTP_PASSWORD"); + // The variable names live in the shared host-configuration loader, which both `startApi` and + // `pnpm doctor` call with their own environment. Two executables mapping these names + // independently is what let Doctor and the runtime disagree about object storage. + const loader = await readFile( + path.join(repositoryRoot, "core/src/host/configuration.ts"), + "utf8", + ); + expect(loader).toContain("EKKLESIA_SMTP_HOST"); + expect(loader).toContain("EKKLESIA_SMTP_PASSWORD"); + // That the loader itself reads no environment is asserted in + // `host-configuration-boundaries.test.ts`, which strips comments first — this file's plain + // substring match would trip over the loader's own prose about the rule. }); it("keeps no SMTP password out of the committed environment example", async () => { diff --git a/docs/architecture/platform-architecture.md b/docs/architecture/platform-architecture.md index ec033a0..3dcade6 100644 --- a/docs/architecture/platform-architecture.md +++ b/docs/architecture/platform-architecture.md @@ -508,6 +508,17 @@ something real needs one, and nothing does. Email is optional: an installation w complete deployment, and Better Auth's verification and reset hooks remain unimplemented until Pass 2. +Phase 7C pass 1B makes host infrastructure configuration a single shared mapping. Every executable — +`apps/api`, `pnpm doctor`, and the migration and owner-bootstrap commands — turns its environment +into typed inputs through one `loadHostConfiguration(env)` in Core, which takes the environment **as +an argument** and never reads `process.env` itself, exactly as the module secret source does. +Validation stays with the existing per-domain validators, so each caller keeps its own failure +behaviour: `startApi` refuses to start on a rejected value, while Doctor reports it as a sanitized +code. This replaced two executables that each read their own subset of the same variables and had +drifted — Doctor never read `S3_*` or module secrets at all, so it reported a correctly configured +installation as having neither, and reported modules blocked that the runtime was running. A +diagnostic that disagrees with the runtime it diagnoses is worse than no diagnostic. + The two browser roles share the existing `defineModuleUi({ id, version, mount })` export and exactly the same `ModuleUiHostContext`; role selection comes from separate artifacts, never a `mode`, `presentation.kind`, or host-identity branch. The canonical builder reads only diff --git a/docs/development/api.md b/docs/development/api.md index 74ce8b6..6324539 100644 --- a/docs/development/api.md +++ b/docs/development/api.md @@ -1361,9 +1361,11 @@ rotation and client-certificate authentication. The same policy is resolved identically by every executable that opens a connection — `apps/api`, `pnpm migrations:plan`, `pnpm migrations:apply`, the platform-owner bootstrap CLI, and -`pnpm doctor`'s own connection — through one shared environment reader -(`core/src/cli/database-transport-environment.ts`), so `process.env` stays out of Core's reusable -constructors. `apps/api` validates it in `createApiRuntimeConfig`, *before* any pool, Better Auth +`pnpm doctor`'s own connection — because they all map the environment through the one shared host +configuration loader (`core/src/host/configuration.ts`, reached by the small migration commands via +`core/src/cli/database-transport-environment.ts`). The loader takes the environment **as an +argument**, so `process.env` stays out of Core's reusable constructors and only executables read +it. `apps/api` validates it in `createApiRuntimeConfig`, *before* any pool, Better Auth instance, module runtime, or listener exists. `pnpm doctor` reports it as `database.transport` in its PostgreSQL section: a verified production policy passes, and an unsafe or invalid one is a blocking `NOT READY` failure — never a warning — naming only the effective mode, never the URL, diff --git a/docs/development/email.md b/docs/development/email.md index a1481ef..111fdd0 100644 --- a/docs/development/email.md +++ b/docs/development/email.md @@ -102,6 +102,10 @@ carries no value. A successful probe proves the relay accepts this deployment. It does **not** prove a message would reach an inbox — that depends on the relay's authorization policy and the domain's DNS records. +Doctor reads the `EKKLESIA_SMTP_*` values through the same `loadHostConfiguration` the API +executable uses, so the relay it reports on is the relay the runtime would use. The check is +rendered in Doctor's own **Outbound email** section. + ## Logging Transactional email carries verification links, reset links, and personal data. The transport logs diff --git a/docs/development/repository-workflow.md b/docs/development/repository-workflow.md index fa3b6d8..7e52ec8 100644 --- a/docs/development/repository-workflow.md +++ b/docs/development/repository-workflow.md @@ -67,6 +67,18 @@ the diagnostics that follow are operationally read-only. Doctor never edits conf migrations, creates the Core history table, acquires the migration apply lock, starts module server entrypoints, provisions storage, or invokes Docker. +Doctor evaluates the **same host infrastructure configuration the runtime uses**. Both `pnpm doctor` +and `apps/api` map one deployment environment through the same `loadHostConfiguration(env)` in Core, +so a configured object-storage provider, a set module secret, and a configured SMTP relay are all +recognized by Doctor exactly as `startApi` would recognize them — and neither process can silently +interpret a value differently from the other. Doctor needs no running API process to do this; export +or set the environment the platform would start with and run the command. + +Each optional service is reported in one of three distinct states: **not configured** (absent, and a +complete deployment), **configured incorrectly** (a blocking failure, reported as a sanitized code +with no value echoed), or **configured but unreachable** (a warning, because a transient outage is +not a broken deployment). + Doctor reports a fixed, deterministic set of checks: - The active Node.js version against the repository's declared engine range. @@ -80,7 +92,17 @@ Doctor reports a fixed, deterministic set of checks: - Identity runtime configuration: skipped (not a failure) when no secret, base URL, or trusted origins are set; otherwise validated with `createIdentityConfig` and reported pass or failure. This check never constructs Better Auth or opens a database connection. -- Object storage as skipped while Core has no production storage client or configuration contract. +- Module lifecycle and module configuration, resolved exactly as startup resolves them, so Doctor + can never report a module as runnable that startup would block, or the reverse. +- Object storage: skipped when no `S3_ENDPOINT` is set, a blocking failure when the `S3_*` settings + are structurally invalid, and otherwise probed for reachability. Bindings for every module-declared + logical bucket are reported separately, because binding is not health. +- Outbound email: skipped when no `EKKLESIA_SMTP_HOST` is set, a blocking failure when the + `EKKLESIA_SMTP_*` settings are structurally invalid, and otherwise a connection that authenticates + where credentials are set and **sends nothing**. + +No check prints a credential: not the storage access or secret key, not the SMTP password, not the +identity secret, not a module secret value. Every check has a stable ID and one of four states: pass, warning, failure, or skipped. Independent checks continue after a failure; a failed prerequisite causes only its dependent checks to be @@ -102,18 +124,28 @@ Modules ✓ Dependency graph valid Dependency order is empty. ✓ Permission registry composed - 1 permission registered + 6 permissions registered + ○ Module lifecycle skipped + DATABASE_URL is not configured. + ○ Module configuration skipped + DATABASE_URL is not configured. Data definitions ✓ Schemas validated 0 modules · 0 tables ✓ Migrations validated - 3 migrations · checksums valid + 7 migrations · checksums valid core:0000_identity_core_tables · f8f2086ef06f core:0001_authorization_core_tables · 830e1fbc26d0 core:0002_authorization_integrity · 634294731180 + core:0003_dashboard_navigation_tables · 2cde27807aa4 + core:0004_module_lifecycle_state · c8b5d8b3d48b + core:0005_module_configuration · c8d57f24f84b + core:0006_device_identity · 1db97b7e3ac5 PostgreSQL + ✓ Database TLS verified + TLS required with certificate and hostname verification. ○ Connection check skipped DATABASE_URL is not configured. ○ Migration plan skipped @@ -123,10 +155,18 @@ Identity ○ Identity configuration skipped Core identity schema defines 4 tables (ekklesia_core_account, ekklesia_core_session, ekklesia_core_user, ekklesia_core_verification). Identity runtime configuration (signing key, base URL, trusted origins) is not set. + ✓ Direct client-address mode + Trusts only the actual immediate socket peer for rate limiting; no forwarded header is trusted. Object storage - ○ Check unavailable - Core storage integration has not been implemented yet. + ○ Object storage not configured + No S3_ENDPOINT is set. This is a complete deployment unless a module requires storage. + ✓ No module requires object storage + 0 modules declare a logical storage bucket. + +Outbound email + ○ Outbound email not configured + No EKKLESIA_SMTP_HOST is set. This is a complete deployment: nothing in the platform sends email yet. Result: READY WITH WARNINGS ``` diff --git a/docs/module-development/README.md b/docs/module-development/README.md index a9f8118..3952f61 100644 --- a/docs/module-development/README.md +++ b/docs/module-development/README.md @@ -1133,12 +1133,18 @@ by the process such a module takes down. and reachable, and `object-storage.bindings` lists which modules require storage and where each logical bucket is bound. Neither prints a credential. A module requiring storage on an installation with none configured makes the bindings check fail with a remediation line; a provider that is merely -unreachable is a warning, because modules already running are unaffected. +unreachable is a warning, because modules already running are unaffected. A provider whose settings +are structurally invalid is a blocking failure, reported as a sanitized code — the runtime refuses to +start on the same input. `pnpm doctor`'s `modules.configuration` check reports whether each installed module's configuration would let it run, naming the offending key when it would not. It prints **no configuration values** — secret or otherwise — only counts, keys, and reasons. +Both checks evaluate the same host configuration the runtime receives: Doctor and the API executable +map one deployment environment through the same loader, so a bucket the platform would bind is never +reported unbound, and a module secret the deployment has set is never reported missing. + `pnpm doctor` builds first and presents the same production discovery results as a concise, read-only dashboard. It lists discovered module IDs and versions in dependency order, reports schema and migration counts, and shows safe migration identities with truncated checksums. Schema