fix(auth): refuse to send a sign-in code rather than log one

The rule was "throw when the environment says production, otherwise log
the code and carry on". The deployment sets no such marker, so the
branch that ran was the developer one: every recipient and every usable
sign-in code printed to a retained log, the screen reporting success,
and nobody receiving anything.

That is what a fail-open default costs. The deployment that forgets its
mail settings is exactly the deployment with no marker saying it is a
real one, so it takes the lenient branch precisely when it should not.

Turned around: printing a live code is asked for by name and anything
else is an error, so absence of configuration is a refusal instead of an
assumption. Two settings out of three is also an error now, because it
means somebody is halfway through wiring a provider up and a quiet
fallback would hide the missing half.

Stages anyone else can reach are checked at deploy time, so a missing
setting stops the deploy with the name of the variable it wanted rather
than surfacing later as a person waiting for mail that never comes.
This commit is contained in:
Wanjohi
2026-09-05 09:27:49 +03:00
parent affe1e3c73
commit 2c4e9d9b0b
3 changed files with 142 additions and 23 deletions

View File

@@ -8,6 +8,57 @@ const steamApiKey = Redacted.make(process.env.STEAM_API_KEY!);
const adminSharedSecret = const adminSharedSecret =
process.env.ADMIN_SHARED_SECRET || 'dev-admin-shared-secret-change-in-prod'; process.env.ADMIN_SHARED_SECRET || 'dev-admin-shared-secret-change-in-prod';
/**
* Stages where a missing setting is a deploy failure rather than a default.
*
* A stage somebody else can reach has to be configured; a throwaway one a
* developer made this morning does not. The list is the same one that decides
* observability and DNS below, named once so the two cannot drift apart.
*/
const PERMANENT_STAGES = ['production', 'sandbox', 'dev'];
/**
* Mail settings, refused rather than defaulted when a stage needs them.
*
* Verifying an address is the only way to sign in, so a worker that cannot
* send mail cannot sign anybody in — and the failure to catch is the one where
* that is discovered by a person staring at a screen that says "check your
* email". Checking here turns it into a deploy that stops with the name of the
* variable it wanted.
*/
function mailEnv(stage: string) {
const url = process.env.EMAIL_SEND_URL;
const key = process.env.EMAIL_API_KEY;
const from = process.env.EMAIL_FROM;
if (PERMANENT_STAGES.includes(stage)) {
const missing = [
['EMAIL_SEND_URL', url],
['EMAIL_API_KEY', key],
['EMAIL_FROM', from]
]
.filter(([, value]) => !value)
.map(([name]) => name);
if (missing.length > 0) {
throw new Error(
`Stage "${stage}" serves sign-in, so it needs mail delivery configured. ` +
`Missing: ${missing.join(', ')}.`
);
}
}
return {
...(url ? { EMAIL_SEND_URL: url } : {}),
...(key ? { EMAIL_API_KEY: Redacted.make(key) } : {}),
...(from ? { EMAIL_FROM: from } : {}),
// Printing a live sign-in code to the log is a thing you ask for by
// name. It is never set on a stage anyone else can reach, and the
// worker refuses to send without either this or real settings, so an
// unconfigured deploy fails loudly instead of quietly logging codes.
...(PERMANENT_STAGES.includes(stage) ? {} : { EMAIL_DEV_LOG: 'true' })
};
}
const AuthStorage = Cloudflare.KV.Namespace('auth-storage'); const AuthStorage = Cloudflare.KV.Namespace('auth-storage');
const Database = Effect.gen(function* () { const Database = Effect.gen(function* () {
@@ -36,7 +87,7 @@ const Database = Effect.gen(function* () {
export const Auth = Effect.gen(function* () { export const Auth = Effect.gen(function* () {
const { stage } = yield* Alchemy.Stack; const { stage } = yield* Alchemy.Stack;
const isPermanent = ['production', 'sandbox', 'dev'].includes(stage); const isPermanent = PERMANENT_STAGES.includes(stage);
return yield* Cloudflare.Worker('auth', { return yield* Cloudflare.Worker('auth', {
main: 'apps/auth/src/index.ts', main: 'apps/auth/src/index.ts',
compatibility: { flags: ['nodejs_compat'] }, compatibility: { flags: ['nodejs_compat'] },
@@ -45,7 +96,8 @@ export const Auth = Effect.gen(function* () {
// key is bound there. // key is bound there.
env: { env: {
AuthStorage, AuthStorage,
HYPERDRIVE: Database HYPERDRIVE: Database,
...mailEnv(stage)
}, },
...(isPermanent ? { observability: { enabled: true } } : {}) ...(isPermanent ? { observability: { enabled: true } } : {})
}); });
@@ -53,7 +105,7 @@ export const Auth = Effect.gen(function* () {
export const Api = Effect.gen(function* () { export const Api = Effect.gen(function* () {
const { stage } = yield* Alchemy.Stack; const { stage } = yield* Alchemy.Stack;
const isPermanent = ['production', 'sandbox', 'dev'].includes(stage); const isPermanent = PERMANENT_STAGES.includes(stage);
const prefix = stage === 'production' ? '' : `${stage}.`; const prefix = stage === 'production' ? '' : `${stage}.`;
const authDomain = ['production', 'sandbox'].includes(stage) const authDomain = ['production', 'sandbox'].includes(stage)
? `${prefix}auth.nestri.io` ? `${prefix}auth.nestri.io`

View File

@@ -3,39 +3,61 @@
* *
* Deliberately not tied to one mail vendor: it posts a small JSON body to * Deliberately not tied to one mail vendor: it posts a small JSON body to
* whatever endpoint is configured, so swapping providers is configuration and * whatever endpoint is configured, so swapping providers is configuration and
* not a code change. Three settings, all optional except in production — * not a code change. Three settings — `EMAIL_SEND_URL`, `EMAIL_API_KEY`,
* `EMAIL_SEND_URL`, `EMAIL_API_KEY`, `EMAIL_FROM`. * `EMAIL_FROM` — and a fourth, `EMAIL_DEV_LOG`, that asks for the code to be
* printed instead of sent.
*/ */
export interface MailerConfig { export interface MailerConfig {
EMAIL_SEND_URL?: string; EMAIL_SEND_URL?: string;
EMAIL_API_KEY?: string; EMAIL_API_KEY?: string;
EMAIL_FROM?: string; EMAIL_FROM?: string;
NODE_ENV?: string; /**
* Print the code to the log rather than sending it. `'true'` and nothing
* else, so a variable left holding `'false'` or `'0'` cannot switch it on.
*/
EMAIL_DEV_LOG?: string;
} }
/** /**
* Send the code, or fail loudly. * Send the code, or refuse.
* *
* With no mailer configured this logs the code and carries on, which is what * The rule is that printing a live sign-in code to a log is something you ask
* makes a local sign-in possible without a mail account. In production the * for by name, and that anything else is an error. It reads that way round
* same situation throws instead: a signup screen that says "check your email" * because the alternative — treat an unconfigured mailer as "must be a
* when nothing was sent is worse than one that says it is broken, because the * developer" — fails *open*: the deployment that forgets its mail settings is
* person waits instead of telling anybody. * exactly the deployment with no marker saying it is a real one, so it takes
* the developer branch, logs every recipient and every usable code to a
* retained log, and reports success while nobody receives anything.
*
* Configuration is also all-or-nothing. Two settings out of three is somebody
* halfway through wiring a provider up, and quietly falling back would hide
* the half that is missing.
*/ */
export async function sendVerificationCode( export async function sendVerificationCode(
config: MailerConfig, config: MailerConfig,
email: string, email: string,
code: string code: string
): Promise<void> { ): Promise<void> {
const configured = config.EMAIL_SEND_URL && config.EMAIL_API_KEY && config.EMAIL_FROM; const present = [config.EMAIL_SEND_URL, config.EMAIL_API_KEY, config.EMAIL_FROM].filter(Boolean);
if (!configured) { if (present.length === 0) {
if (config.NODE_ENV === 'production') { if (config.EMAIL_DEV_LOG === 'true') {
throw new Error('Email delivery is not configured, so no sign-in code can be sent');
}
console.log(`[auth] sign-in code for ${email}: ${code}`); console.log(`[auth] sign-in code for ${email}: ${code}`);
return; return;
} }
throw new Error(
'Email delivery is not configured, so no sign-in code can be sent. ' +
'Set EMAIL_SEND_URL, EMAIL_API_KEY and EMAIL_FROM, or set EMAIL_DEV_LOG=true ' +
'to print codes to the log instead.'
);
}
if (present.length < 3) {
throw new Error(
'Email delivery is half configured: EMAIL_SEND_URL, EMAIL_API_KEY and EMAIL_FROM ' +
'are needed together.'
);
}
const response = await fetch(config.EMAIL_SEND_URL!, { const response = await fetch(config.EMAIL_SEND_URL!, {
method: 'POST', method: 'POST',

View File

@@ -3,16 +3,36 @@ import { describe, expect, test } from 'bun:test';
import { sendVerificationCode } from '../src/email.js'; import { sendVerificationCode } from '../src/email.js';
describe('sending a sign-in code', () => { describe('sending a sign-in code', () => {
test('with nothing configured outside production, it does not block a sign-in', async () => { test('printing the code to the log has to be asked for by name', async () => {
await sendVerificationCode({ NODE_ENV: 'development' }, 'ada@example.com', '123456'); await sendVerificationCode({ EMAIL_DEV_LOG: 'true' }, 'ada@example.com', '123456');
}); });
test('with nothing configured in production, it says so instead of pretending', async () => { // The regression this holds: the previous rule was "throw only when the
// environment says production", which meant a deployment that set no
// marker at all — which is what the real one did — took the developer
// branch and logged live codes. Absence is now a refusal.
test('nothing configured and nothing asked for is a refusal, not a log', async () => {
await expect(sendVerificationCode({}, 'ada@example.com', '123456')).rejects.toThrow(
/not configured/
);
});
test('a variable left holding something other than true does not switch logging on', async () => {
await expect( await expect(
sendVerificationCode({ NODE_ENV: 'production' }, 'ada@example.com', '123456') sendVerificationCode({ EMAIL_DEV_LOG: 'false' }, 'ada@example.com', '123456')
).rejects.toThrow(/not configured/); ).rejects.toThrow(/not configured/);
}); });
test('half a mailer is an error rather than a fallback', async () => {
await expect(
sendVerificationCode(
{ EMAIL_SEND_URL: 'https://mail.example.com/send', EMAIL_DEV_LOG: 'true' },
'ada@example.com',
'123456'
)
).rejects.toThrow(/half configured/);
});
test('a configured mailer is called with the address and the code', async () => { test('a configured mailer is called with the address and the code', async () => {
let seen: { url: string; body: any; auth: string | null } | null = null; let seen: { url: string; body: any; auth: string | null } | null = null;
const original = globalThis.fetch; const original = globalThis.fetch;
@@ -28,7 +48,6 @@ describe('sending a sign-in code', () => {
try { try {
await sendVerificationCode( await sendVerificationCode(
{ {
NODE_ENV: 'production',
EMAIL_SEND_URL: 'https://mail.example.com/send', EMAIL_SEND_URL: 'https://mail.example.com/send',
EMAIL_API_KEY: 'key', EMAIL_API_KEY: 'key',
EMAIL_FROM: 'hello@nestri.io' EMAIL_FROM: 'hello@nestri.io'
@@ -47,6 +66,32 @@ describe('sending a sign-in code', () => {
expect(seen!.body.text).toContain('123456'); expect(seen!.body.text).toContain('123456');
}); });
test('a configured mailer sends even when dev logging is on', async () => {
let called = false;
const original = globalThis.fetch;
globalThis.fetch = (async () => {
called = true;
return new Response('{}', { status: 200 });
}) as unknown as typeof fetch;
try {
await sendVerificationCode(
{
EMAIL_SEND_URL: 'https://mail.example.com/send',
EMAIL_API_KEY: 'key',
EMAIL_FROM: 'hello@nestri.io',
EMAIL_DEV_LOG: 'true'
},
'ada@example.com',
'123456'
);
} finally {
globalThis.fetch = original;
}
expect(called).toBe(true);
});
test('a refusal from the mailer is not swallowed', async () => { test('a refusal from the mailer is not swallowed', async () => {
const original = globalThis.fetch; const original = globalThis.fetch;
globalThis.fetch = (async () => globalThis.fetch = (async () =>