Files
Ekklesia/core/tests/host-configuration-boundaries.test.ts
T
ekklesiaandClaude Opus 5 3563549e86
CI / verify (pull_request) Successful in 42s
CI / infrastructure-smoke (pull_request) Successful in 20s
CI / migration-smoke (pull_request) Successful in 1m41s
fix: let a deployment name itself in the SMTP greeting
A real deployment could not send mail through an IP-authorized relay.
Every attempt surfaced as MAIL_PROVIDER_UNAVAILABLE while the relay was
healthy and swaks sent from the same machine.

The cause was the greeting, and it is generic rather than tied to any
provider. An SMTP client identifies itself in EHLO, and with nothing
configured Nodemailer derives that from the machine hostname —
substituting the address literal [127.0.0.1] whenever the hostname is
not fully qualified, which is the normal state of a container. The relay
closed the connection on it with a 421 before any message was offered,
which is indistinguishable from the relay being down.

EKKLESIA_SMTP_HELO_NAME now lets the deployment say what to greet as. It
is validated as a fully qualified domain name — no scheme, port,
brackets, whitespace, or control characters, and at least one dot, since
a single unqualified label is exactly the input Nodemailer already
discards. It is not a credential: it travels in the clear on every
connection, so Doctor prints it where an operator can compare it against
what the relay expects, and Doctor's probe performs the same greeting the
runtime does.

Absent preserves Nodemailer's behaviour exactly. It is deliberately not
defaulted to the sender's domain: a machine whose hostname is already a
valid FQDN may be greeting correctly, and some relays check that against
reverse DNS, so replacing it silently would trade one deployment's bug
for another's.

The regression guard is a raw SMTP server that refuses any greeting but
the expected one, because smtp-server exposes no EHLO hook and its
earliest hook is never reached by a send-nothing probe.

Co-Authored-By: Claude Opus 5 <[email protected]>
2026-08-25 20:28:30 -05:00

156 lines
6.1 KiB
TypeScript

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<ReadonlyArray<{ readonly path: string; readonly text: string }>> {
const results: Array<{ path: string; text: string }> = [];
const walk = async (current: string): Promise<void> => {
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_DASHBOARD_ORIGIN",
"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_HELO_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");
});
});