Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions app/api/cron/admin-digest/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { getTranslations } from '@/lib/i18n/server';
import { createServiceClient } from '@/lib/supabase/server';
import { hasCronAuthorization } from '@/lib/server/cron-auth';
import { readAll } from '@/lib/supabase/read-all';
import { allSuperAdminEmails } from '@/lib/feedback/notify';
import { readSuperAdminRecipients } from '@/lib/feedback/notify';
import { sendEmail } from '@/lib/server/email';
import {
buildAdminDigest, digestSubject, renderAdminDigestHtml, summarizeDigestDelivery, type DigestRow,
Expand Down Expand Up @@ -72,7 +72,16 @@ export async function GET(req: NextRequest) {
return NextResponse.json({ ok: true, sent: 0, total: 0, reason: 'no activity in the last 24h' });
}

const emails = await allSuperAdminEmails(admin);
// Fail CLOSED on the recipient list. Sending to the env allowlist alone after a
// failed super_admins read silently drops every admin who exists only in the
// table, and used to answer 200 ok while doing it. An empty table is fine: the
// allowlist is then the whole list.
const recipients = await readSuperAdminRecipients(admin);
if (recipients.failure !== null) {
console.error('[admin-digest] super-admin recipient read failed', recipients.failure, recipients.cause);
return NextResponse.json({ ok: false, error: t('adminDigest.recipientsUnavailable') }, { status: 502 });
}
const emails = recipients.emails;
if (emails.length === 0) {
return NextResponse.json({ ok: true, sent: 0, total: digest.total, reason: 'no super-admin recipients' });
}
Expand Down
11 changes: 11 additions & 0 deletions docs/final-audit/admin-digest-replay-contract-2026-09-30/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,14 @@ Each change was made to the route in a scratch checkout only, and then reverted.
- **The fake database sorts stably,** so the tie test proves every row is counted across pages. It does not prove that `.order('id')` is needed: a nondeterministic server order is not modelled.
- **Resend is modelled from its documentation, not called.** Rate limits, bounces and key retention were not measured.
- **Nothing here touched production.** `finalaudit.md` records `CRON_SECRET` as unset in production, and the latest observed GitHub dispatch exits before dispatching. With the secret unset the route answers 401 to every call, as the unauthorised case shows, so today the digest does not run at all. Once the secret is set, both schedulers fire at 12:30 UTC and the sequential-repeat row above applies every day a GitHub tick lands in the 12:30–12:34 window.

## Addendum (2026-09-30): combined with #685, the recipient fail-closed fix

Everything above is the **dated record at `231e8140`**, before #685; `characterization-output.txt` and `desired-contract-output.txt` are kept unchanged as that record. On the combined source (#685 plus this folder, at #677's reviewed head `e90fa5a2`):

- **Characterization** (`tests/admin-digest-replay-contract.test.ts`, 20/20, saved as `characterization-output-combined-with-685.txt`):
- The two `super_admins` read cases (error, throw) are re-pinned from "200 ok, built-in admin only" to **502 `adminDigest.recipientsUnavailable`, zero provider attempts**.
- The empty-recipient override now also answers the strict reader the route uses (`readSuperAdminRecipients` → `{ emails: [], failure: null }`), so that case still exercises the empty-recipient branch.
- Recipient reads are paged since #685 (a one-row page plus an empty page per call). The concurrency barrier now holds only the **first page of each invocation**, so it no longer pins how many pages a read takes. The replay and concurrency send counts are unchanged: 2 per admin.
- **Desired contract** (`tests/desired-contract.test.ts`): with #685, C6 passes and C1–C5 still fail (`desired-contract-output-combined-with-685.txt`: 5 failed, 2 passed). **Only C6 is promoted**, into the characterization file. C1–C5 stay here as unmet contract cases: one digest per occurrence; retry only the unaccepted recipient; same key and frozen payload on an ambiguous send; no row in two digests. Run now, this file shows 5 failed and 1 passed (the control).
- Duplicate delivery is **not** fixed. No live email was sent, and no provider delivery is claimed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@

RUN v4.1.10 <repo>

✓ tests/admin-digest-replay-contract.test.ts > fixture > two recipients (the built-in admin and one super_admins row), and both schedulers name 12:30 UTC
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > refuses every unauthorised call with 401 before touching the database or the provider
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a quiet day (nothing in the last 24 h) sends nothing, does not look up recipients, and answers 200
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > empty recipients answers 200 with the count and sends nothing — reachable only by replacing the lookup, because the built-in admin is always present
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a super_admins row that repeats the env allowlist in another case is one recipient, sent once
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > with no RESEND_API_KEY it answers 200 ok with sent 0 and skipped 2, having delivered nothing (the JOB-EF2453D9F633 body)
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > pages past a 1,000-row server cap: 2,345 rows in the window (all tied on created_at and title) are counted exactly, in 4 reads
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > exactly 20,000 rows is a complete read; 20,001 is a truncated one and answers 502 without looking up recipients or sending
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a failed first page of the notification feed answers 502 and sends nothing
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a failed THIRD page discards the two good pages: 502, nothing sent
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a super_admins read that answers an error fails closed (#685): 502, and not one email
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > a super_admins read that throws fails closed (#685): 502, and not one email
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > SEQUENTIAL REPEAT: a second call for the same 12:30 occurrence sends every admin the identical digest again — 4 attempted, 4 accepted, nothing written
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > SEQUENTIAL REPEAT inside the dispatcher window: Vercel at 12:30 and GitHub at 12:33 both send; the second digest re-reports the first one’s rows
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > CONCURRENT: two workers that both finish reading before either sends each email every admin — 4 attempted, 4 accepted
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > ONE RECIPIENT FAILS (refuse) AFTER ANOTHER SUCCEEDS: 502 {sent 1, failed 1}; the retry re-sends to the admin who already had it
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > ONE RECIPIENT FAILS (network-error) AFTER ANOTHER SUCCEEDS: 502 {sent 1, failed 1}; the retry re-sends to the admin who already had it
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > ACCEPTED BEFORE A TIMEOUT: the provider accepts both, the handler reports one failure and 502; the retry gives BOTH admins a second copy
✓ tests/admin-digest-replay-contract.test.ts > admin-digest replay: current main, observed (not endorsed) > PARTIAL DELIVERY STATUS: 502 whenever any recipient fails, 200 only when none does; the body always carries the per-run counts
✓ tests/admin-digest-replay-contract.test.ts > admin-digest contract (promoted) > C6 a run that could not read its recipient list does not report a clean success

Test Files 1 passed (1)
Tests 20 passed (20)
Start at <elided>
Duration <elided>

Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@

RUN v4.1.10 <repo>

✓ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > CONTROL (passes): one on-time run delivers exactly one digest to each admin
× docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C1 one occurrence, two sequential calls: each admin receives exactly one digest
→ expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 1 }
× docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C2 one occurrence, two concurrent calls: each admin receives exactly one digest
→ expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 1 }
× docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C3 a retry after one recipient failed attempts ONLY that recipient
→ expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 2 }
× docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C4 an ambiguous send (accepted, then timed out) is retried under the SAME idempotency key, and nobody else is re-sent
→ expected 'object' to be 'string' // Object.is equality
× docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C5 Vercel at 12:30 and GitHub at 12:33: no activity row is reported in two digests
→ expected [ { …(5) }, { …(5) } ] to have a length of 1 but got 2
✓ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C6 a run that could not read its recipient list does not report a clean success

⎯⎯⎯⎯⎯⎯⎯ Failed Tests 5 ⎯⎯⎯⎯⎯⎯⎯

FAIL docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C1 one occurrence, two sequential calls: each admin receives exactly one digest
AssertionError: expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 1 }

- Expected
+ Received

{
- "admin-1": 1,
- "admin-2": 1,
+ "admin-1": 2,
+ "admin-2": 2,
}

❯ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts:116:24
114| await runAt('2026-09-30T12:30:00Z');
115| await runAt('2026-09-30T12:30:00Z');
116| expect(accepted()).toEqual(once());
| ^
117| });
118|

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/5]⎯

FAIL docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C2 one occurrence, two concurrent calls: each admin receives exactly one digest
AssertionError: expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 1 }

- Expected
+ Received

{
- "admin-1": 1,
- "admin-2": 1,
+ "admin-1": 2,
+ "admin-2": 2,
}

❯ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts:134:24
132| await Promise.all([call(), call()].map((p) => p.finally(() => re…
133| } finally { clearTimeout(bound); }
134| expect(accepted()).toEqual(once());
| ^
135| });
136|

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/5]⎯

FAIL docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C3 a retry after one recipient failed attempts ONLY that recipient
AssertionError: expected { 'admin-1': 2, 'admin-2': 2 } to deeply equal { 'admin-1': 1, 'admin-2': 2 }

- Expected
+ Received

{
- "admin-1": 1,
+ "admin-1": 2,
"admin-2": 2,
}

❯ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts:143:25
141| state.mode = {};
142| await runAt('2026-09-30T12:30:00Z');
143| expect(attempted()).toEqual({ 'admin-1': 1, 'admin-2': 2 });
| ^
144| expect(accepted()).toEqual(once());
145| });

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[3/5]⎯

FAIL docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C4 an ambiguous send (accepted, then timed out) is retried under the SAME idempotency key, and nobody else is re-sent
AssertionError: expected 'object' to be 'string' // Object.is equality

Expected: "string"
Received: "object"

❯ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts:158:41
156| const [first, retry] = ambiguous;
157| // A real key, the same key, and the same payload, so the provider…
158| expect(typeof first.idempotencyKey).toBe('string');
| ^
159| expect(first.idempotencyKey).not.toBe('');
160| expect(retry.idempotencyKey).toBe(first.idempotencyKey);

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[4/5]⎯

FAIL docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts > admin-digest idempotency contract (UNRESOLVED on current main — expected to fail) > C5 Vercel at 12:30 and GitHub at 12:33: no activity row is reported in two digests
AssertionError: expected [ { …(5) }, { …(5) } ] to have a length of 1 but got 2

- Expected
+ Received

- 1
+ 2

❯ docs/final-audit/admin-digest-replay-contract-2026-09-30/tests/desired-contract.test.ts:172:69
170| await runAt('2026-09-30T12:33:00Z');
171| const toSecond = state.attempts.filter((a) => a.to === SECOND && a…
172| expect(toSecond.filter((a) => a.html.includes('Signup Alpha'))).to…
| ^
173| });
174|

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[5/5]⎯


Test Files 1 failed (1)
Tests 5 failed | 2 passed (7)
Start at <elided>
Duration <elided>

Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ afterEach(() => {
}
});

describe('admin-digest idempotency contract (UNRESOLVED on current main — expected to fail)', () => {
describe('admin-digest idempotency contract (UNRESOLVED — C1–C5 expected to fail)', () => {
it('CONTROL (passes): one on-time run delivers exactly one digest to each admin', async () => {
happen('2026-09-30T08:00:00Z', 'Signup Alpha');
expect((await runAt('2026-09-30T12:30:00Z')).status).toBe(200);
Expand Down Expand Up @@ -172,10 +172,7 @@ describe('admin-digest idempotency contract (UNRESOLVED on current main — expe
expect(toSecond.filter((a) => a.html.includes('Signup Alpha'))).toHaveLength(1);
});

it('C6 a run that could not read its recipient list does not report a clean success', async () => {
happen('2026-09-30T08:00:00Z', 'Signup Alpha');
state.failSuperAdmins = true;
const res = await runAt('2026-09-30T12:30:00Z');
expect(res.body.ok).toBe(false);
});
// C6 (a run that could not read its recipient list does not report a clean success) failed on
// main at 231e8140, passes with #685 (desired-contract-output-combined-with-685.txt), and was
// promoted to tests/admin-digest-replay-contract.test.ts. C1–C5 remain unmet.
});
49 changes: 43 additions & 6 deletions lib/feedback/notify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from '@/lib/integrations/github';
import { issueTitle, issueBody, githubLabels } from '@/lib/feedback/github-map';
import { wroteNoRows } from '@/lib/supabase/errors';
import { readAll } from '@/lib/supabase/read-all';

type Admin = ReturnType<typeof createServiceClient>;

Expand All @@ -17,14 +18,50 @@ function appUrl(): string {
return (process.env.NEXT_PUBLIC_APP_URL || 'https://www.bubaly.com').replace(/\/$/, '');
}

/** Every super-admin email: the code/env allowlist ∪ the super_admins table. */
export async function allSuperAdminEmails(admin: Admin): Promise<string[]> {
/** The most `super_admins` rows a recipient read accepts; one more is a truncated read. */
const SUPER_ADMIN_READ_MAX = 1_000;

/**
* `failure` is a classification, not a message: the raw cause is kept apart for
* the server log only, so nothing a database said can reach a response body.
*/
export type SuperAdminRecipients =
| { emails: string[]; failure: null }
| { emails: null; failure: 'read_failed' | 'truncated'; cause: unknown };

/**
* Every super-admin email — the code/env allowlist ∪ the super_admins table — or
* an error when the table could not be read COMPLETELY.
*
* An empty table is not an error: the allowlist is then the whole, legitimate
* list. A failed read, a thrown query and a read past the ceiling are errors,
* because the allowlist alone would silently drop every admin who exists only in
* the table. `{ data }` alone could not tell those apart: a PostgREST call
* RESOLVES with `{ data: null, error }`, which read as "no rows".
*/
export async function readSuperAdminRecipients(admin: Admin): Promise<SuperAdminRecipients> {
const set = new Set(envSuperAdminEmails());
try {
const { data } = await admin.from('super_admins').select('email');
for (const r of data ?? []) if (r.email) set.add(r.email.toLowerCase());
} catch { /* table may not exist yet — env allowlist still applies */ }
return [...set];
const { rows, error, truncated } = await readAll<{ email: string | null }>((from, to) => admin
.from('super_admins').select('email').order('email').range(from, to), { max: SUPER_ADMIN_READ_MAX });
if (error) return { emails: null, failure: truncated ? 'truncated' : 'read_failed', cause: error };
for (const r of rows) if (r.email) set.add(r.email.toLowerCase());
} catch (cause) {
return { emails: null, failure: 'read_failed', cause };
}
return { emails: [...set], failure: null };
}

/**
* Best-effort form for alerts (notifySuperAdmins): when the table cannot be read
* it still reaches the allowlist rather than nobody, and says so in the log. A
* caller that must not send to a partial list uses readSuperAdminRecipients.
*/
export async function allSuperAdminEmails(admin: Admin): Promise<string[]> {
const read = await readSuperAdminRecipients(admin);
if (read.failure === null) return read.emails;
console.error('[feedback-notify] super_admins read failed; using the env allowlist only', read.failure, read.cause);
return envSuperAdminEmails();
}

/**
Expand Down
Loading
Loading