diff --git a/.env.example b/.env.example index 4c1e940..39b3952 100644 --- a/.env.example +++ b/.env.example @@ -71,6 +71,16 @@ CFP_JWT_SIGNING_KEY=change-me-to-a-random-string-at-least-32-chars # PEM-encoded certificate matching SAML_PRIVATE_KEY. # SAML_CERTIFICATE=-----BEGIN CERTIFICATE-----\n...\n-----END CERTIFICATE----- +# Stable IdP entity ID — also the on every assertion. Slack stores +# this at setup, so it must NOT change when CFP_SITE_HOST flips at cutover. +# Leave unset unless registering a separate IdP with a different workspace. +# See specs/api/saml.md#idp-identity-and-hosts. +# SAML_ENTITY_ID=https://codeforphilly.org/api/saml/slack/metadata + +# Slack workspace host. Drives the ACS URL, NameID NameQualifier, and the +# /chat + /launch redirects. Never used for our own entity ID or endpoints. +# SLACK_TEAM_HOST=codeforphilly.slack.com + # --------------------------------------------------------------------------- # Static SPA serving (production only) # --------------------------------------------------------------------------- @@ -92,14 +102,19 @@ CFP_JWT_SIGNING_KEY=change-me-to-a-random-string-at-least-32-chars # CFP_SITE_HOST=codeforphilly.org # --------------------------------------------------------------------------- -# Outbound notifications (Resend) +# Outbound notifications (Postmark) # --------------------------------------------------------------------------- -# Resend API key for the help-wanted email notifier. When unset, the -# notifier falls back to a no-op LoggingNotifier so dev + tests work -# without an account. See plans/notifier-email.md. -# RESEND_API_KEY=re_… +# Postmark server token for the email notifier (help-wanted, welcome, +# password-reset). When unset, the notifier falls back to a no-op +# LoggingNotifier so dev + tests work without an account. See +# plans/postmark-notifier.md and docs/operations/secrets.md. +# POSTMARK_SERVER_TOKEN=… + +# Postmark message stream to send on. Defaults to `outbound`, the +# transactional stream every Postmark server ships with. +# POSTMARK_MESSAGE_STREAM=outbound # From-address for outbound notifications. RFC 5322 form. -# Only used when RESEND_API_KEY is set. +# Only used when POSTMARK_SERVER_TOKEN is set. # CFP_NOTIFICATION_FROM="Code for Philly " diff --git a/apps/api/package.json b/apps/api/package.json index b5425ed..85d4050 100644 --- a/apps/api/package.json +++ b/apps/api/package.json @@ -35,9 +35,9 @@ "bcryptjs": "^3.0.3", "better-sqlite3": "^12.10.0", "fastify": "^5.8.5", - "gitsheets": "^1.4.1", + "gitsheets": "^2.2.0", "jose": "^6.2.3", - "resend": "^6.12.4", + "postmark": "^5.1.0", "samlify": "^2.13.0", "sharp": "^0.34.5", "uuidv7": "^1.2.1", diff --git a/apps/api/scripts/cutover-mailout.ts b/apps/api/scripts/cutover-mailout.ts index 93a72a8..032f674 100644 --- a/apps/api/scripts/cutover-mailout.ts +++ b/apps/api/scripts/cutover-mailout.ts @@ -6,7 +6,7 @@ * them to sign in and claim their account. Run manually at T+90 per * specs/behaviors/account-migration.md#cutover-window-policy. * - * --dry-run prints the would-be send list and exits — no Resend calls, no + * --dry-run prints the would-be send list and exits — no Postmark calls, no * disk writes. The CI test exercises only --dry-run. * * Usage: @@ -14,7 +14,8 @@ * npm run -w apps/api script:cutover-mailout -- --send --from=hello@codeforphilly.org * * Env: - * RESEND_API_KEY — required for actual sends (otherwise --send refuses) + * POSTMARK_SERVER_TOKEN — required for actual sends (otherwise --send refuses) + * POSTMARK_MESSAGE_STREAM — optional; defaults to `outbound` * CFP_PUBLIC_URL — base URL used in the email body (defaults to * https://codeforphilly.org) * CFP_DATA_REPO_PATH + STORAGE_BACKEND + bucket envs — same shape as the API @@ -22,6 +23,9 @@ import { writeFile } from 'node:fs/promises'; import { resolve } from 'node:path'; +import { ServerClient } from 'postmark'; + +import { PostmarkTransport } from '../src/notify/postmark-transport.js'; import { openPublicStore, type PublicStore } from '../src/store/public.js'; import { FilesystemPrivateStore, @@ -195,7 +199,7 @@ export async function runMailout(opts: MailoutOptions): Promise { } // --------------------------------------------------------------------------- -// Env wiring + Resend send +// Env wiring + Postmark send // --------------------------------------------------------------------------- function requireEnv(name: string): string { @@ -220,33 +224,19 @@ function buildPrivateStore(): PrivateStore { }); } -/** Resend HTTP send. Fetch-based to avoid adding a new dep at this stage. */ -async function resendSend(input: { - to: string; - from: string; - subject: string; - html: string; - text: string; -}): Promise { - const apiKey = requireEnv('RESEND_API_KEY'); - const res = await fetch('https://api.resend.com/emails', { - method: 'POST', - headers: { - 'authorization': `Bearer ${apiKey}`, - 'content-type': 'application/json', - }, - body: JSON.stringify({ - from: input.from, - to: input.to, - subject: input.subject, - html: input.html, - text: input.text, - }), +/** + * Postmark send via the same transport the API's notifier uses. The SDK + * throws on any non-2xx, which runMailout() records per-recipient in + * `failed` rather than aborting the run. + */ +function buildPostmarkSend(): NonNullable { + const transport = new PostmarkTransport({ + client: new ServerClient(requireEnv('POSTMARK_SERVER_TOKEN')), + messageStream: process.env['POSTMARK_MESSAGE_STREAM'] || undefined, }); - if (!res.ok) { - const body = await res.text(); - throw new Error(`Resend ${res.status}: ${body.slice(0, 200)}`); - } + return async (input) => { + await transport.send(input); + }; } // --------------------------------------------------------------------------- @@ -299,7 +289,7 @@ async function main(): Promise { mode: args.dryRun ? 'dry-run' : 'send', from: args.from, publicUrl: args.publicUrl ?? process.env['CFP_PUBLIC_URL'], - send: args.send ? resendSend : undefined, + send: args.send ? buildPostmarkSend() : undefined, }); process.stderr.write( diff --git a/apps/api/scripts/import-laddr/importer.ts b/apps/api/scripts/import-laddr/importer.ts index 80b4386..d4f7553 100644 --- a/apps/api/scripts/import-laddr/importer.ts +++ b/apps/api/scripts/import-laddr/importer.ts @@ -92,7 +92,7 @@ import { type TranslateCtx, type Warnings, } from './translators.js'; -import { BlobObject } from 'hologit'; +import type { BlobHandle } from 'gitsheets'; // --------------------------------------------------------------------------- // Public types @@ -187,7 +187,7 @@ export async function importLaddrFromJson(opts: ImportOptions): Promise>['repo'] | null = null; let existingIds: ExistingIds; @@ -500,7 +500,6 @@ export async function importLaddrFromJson(opts: ImportOptions): Promise 0) { - const blobs: Record = {}; + const blobs: Record = {}; for (const a of artifacts) { - // BlobObject.write hashes the buffer into the git object DB. - // Same `as unknown as string` cast as the avatar route — the - // declared signature is too narrow; the underlying - // git-client `$putBlob` accepts Buffer at runtime. - blobs[a.filename] = await BlobObject.write( - hologit, - a.bytes as unknown as string, - ); + // repo.writeBlob hashes the Buffer into the git object DB. + blobs[a.filename] = await publicRepo.writeBlob(a.bytes); } await tx['blog-posts'].setAttachments(record, blobs); } diff --git a/apps/api/src/env.ts b/apps/api/src/env.ts index b171799..6c2908f 100644 --- a/apps/api/src/env.ts +++ b/apps/api/src/env.ts @@ -6,6 +6,12 @@ */ import { z } from 'zod'; +/** + * Default SAML IdP entity ID. Stable across hosts — see the SAML_ENTITY_ID + * field below and specs/api/saml.md#idp-identity-and-hosts. + */ +export const SAML_ENTITY_ID_DEFAULT = 'https://codeforphilly.org/api/saml/slack/metadata'; + export const EnvSchema = z.object({ /** TCP port the Fastify server listens on. */ PORT: z.coerce.number().default(3001), @@ -49,8 +55,17 @@ export const EnvSchema = z.object({ /** SAML IdP certificate (PEM) for the Slack SAML integration. */ SAML_CERTIFICATE: z.string().optional(), /** - * Slack workspace host. Used as the SAML `NameQualifier` per - * specs/api/saml.md and shared with the `/chat` redirect handler. + * SAML IdP entity ID — the metadata `entityID` and the `` on every + * assertion. A stable logical identifier Slack stores at setup time, so it + * deliberately does NOT follow CFP_SITE_HOST: the pre-cutover + * `next.codeforphilly.org` deploy and the post-cutover `codeforphilly.org` + * deploy present the same issuer. Per specs/api/saml.md#idp-identity-and-hosts. + */ + SAML_ENTITY_ID: z.url().default(SAML_ENTITY_ID_DEFAULT), + /** + * Slack workspace host. Used for the SAML ACS URL and `NameQualifier` per + * specs/api/saml.md and shared with the `/chat` redirect handler. Never + * used for our own IdP entity ID or endpoint URLs. */ SLACK_TEAM_HOST: z.string().default('codeforphilly.slack.com'), /** @@ -64,19 +79,27 @@ export const EnvSchema = z.object({ * `next-v2.codeforphilly.org` in sandbox). Used by the server-side * markdown renderer to distinguish internal from external links — anchors * with a host different from this one get `target="_blank" rel="noopener - * nofollow"`. Per specs/behaviors/markdown-rendering.md. + * nofollow"`. Per specs/behaviors/markdown-rendering.md. Also the host the + * SAML IdP metadata advertises for its SSO endpoint Locations (per + * specs/api/saml.md#idp-identity-and-hosts). */ CFP_SITE_HOST: z.string().default('codeforphilly.org'), /** - * Resend API key for the email notifier. When unset, the services plugin - * falls back to LoggingNotifier so dev + test runs don't need a real key. - * See plans/notifier-email.md. + * Postmark server token for the email notifier. When unset, the services + * plugin falls back to LoggingNotifier so dev + test runs don't need a + * real token. See plans/postmark-notifier.md. + */ + POSTMARK_SERVER_TOKEN: z.string().optional(), + /** + * Postmark message stream outbound mail is sent on. `outbound` is the + * transactional default stream every Postmark server ships with. Only + * relevant when POSTMARK_SERVER_TOKEN is set. */ - RESEND_API_KEY: z.string().optional(), + POSTMARK_MESSAGE_STREAM: z.string().default('outbound'), /** * From-address for outbound notifications. RFC 5322 form * (e.g. `"Code for Philly "`). Only - * relevant when RESEND_API_KEY is set. + * relevant when POSTMARK_SERVER_TOKEN is set. */ CFP_NOTIFICATION_FROM: z .string() @@ -115,10 +138,12 @@ export const envJsonSchema = { CFP_JWT_SIGNING_KEY: { type: 'string', minLength: 1 }, SAML_PRIVATE_KEY: { type: 'string' }, SAML_CERTIFICATE: { type: 'string' }, + SAML_ENTITY_ID: { type: 'string', default: SAML_ENTITY_ID_DEFAULT }, SLACK_TEAM_HOST: { type: 'string', default: 'codeforphilly.slack.com' }, CFP_WEB_DIST_PATH: { type: 'string' }, CFP_SITE_HOST: { type: 'string', default: 'codeforphilly.org' }, - RESEND_API_KEY: { type: 'string' }, + POSTMARK_SERVER_TOKEN: { type: 'string' }, + POSTMARK_MESSAGE_STREAM: { type: 'string', default: 'outbound' }, CFP_NOTIFICATION_FROM: { type: 'string', default: 'Code for Philly ', diff --git a/apps/api/src/notify/email-notifier.ts b/apps/api/src/notify/email-notifier.ts index 40144b1..0b6cdcd 100644 --- a/apps/api/src/notify/email-notifier.ts +++ b/apps/api/src/notify/email-notifier.ts @@ -1,18 +1,18 @@ /** - * EmailNotifier — Resend-backed implementation of the Notifier interface. + * EmailNotifier — transport-backed implementation of the Notifier interface. * - * Sends help-wanted notifications via the Resend HTTPS API. Delivery - * failures are logged but never thrown — per - * `specs/api/projects-help-wanted.md`, the express-interest endpoint - * returns 202 to the caller regardless of downstream notification - * outcome. + * Renders each notification through the templates module and hands the + * result to an `EmailTransport` (Postmark in production — see + * `postmark-transport.ts`). Delivery failures are logged but never thrown — + * per `specs/api/projects-help-wanted.md`, the express-interest endpoint + * returns 202 to the caller regardless of downstream notification outcome, + * and the auth routes fire-and-forget for the same reason. * * Slack DM is deliberately out of scope here (tracked at #95); this is * the email-only first cut. The Notifier interface still accepts * `maintainerSlackHandle` so the data flow is ready when Slack lands. */ import type { FastifyBaseLogger } from 'fastify'; -import type { Resend } from 'resend'; import type { HelpWantedFillNotification, @@ -27,10 +27,18 @@ import { renderPasswordResetEmail, renderWelcomeEmail, } from './templates.js'; +import type { EmailTransport } from './transport.js'; + +/** Common shape of every template renderer's output. */ +interface RenderedEmail { + readonly subject: string; + readonly text: string; + readonly html: string; +} export interface EmailNotifierOptions { - /** Resend client (constructed at boot with the API key from env). */ - readonly resend: Resend; + /** Provider adapter (constructed at boot from env; a stub in tests). */ + readonly transport: EmailTransport; /** Sender address — RFC 5322 form, e.g. `"Code for Philly "`. */ readonly fromAddress: string; /** Public site host (no scheme), used to construct absolute URLs in email bodies. */ @@ -40,13 +48,13 @@ export interface EmailNotifierOptions { } export class EmailNotifier implements Notifier { - readonly #resend: Resend; + readonly #transport: EmailTransport; readonly #from: string; readonly #siteHost: string; readonly #log: FastifyBaseLogger; constructor(opts: EmailNotifierOptions) { - this.#resend = opts.resend; + this.#transport = opts.transport; this.#from = opts.fromAddress; this.#siteHost = opts.siteHost; this.#log = opts.logger; @@ -55,174 +63,82 @@ export class EmailNotifier implements Notifier { async notifyHelpWantedInterest( n: HelpWantedInterestNotification, ): Promise<{ delivered: boolean }> { + const ctx = { kind: 'help-wanted.interest', projectSlug: n.projectSlug, roleId: n.roleId }; if (!n.maintainerEmail) { - this.#log.warn( - { kind: 'help-wanted.interest', projectSlug: n.projectSlug, roleId: n.roleId }, - 'help-wanted interest: no maintainer email; skipped', - ); - return { delivered: false }; - } - const tpl = renderInterestEmail(n, this.#siteHost); - try { - const result = await this.#resend.emails.send({ - from: this.#from, - to: n.maintainerEmail, - subject: tpl.subject, - text: tpl.text, - html: tpl.html, - }); - if (result.error) { - this.#log.error( - { - kind: 'help-wanted.interest', - err: result.error, - projectSlug: n.projectSlug, - roleId: n.roleId, - }, - 'help-wanted interest: Resend reported delivery failure', - ); - return { delivered: false }; - } - this.#log.info( - { - kind: 'help-wanted.interest', - projectSlug: n.projectSlug, - roleId: n.roleId, - resendId: result.data?.id, - }, - 'help-wanted interest: email queued for delivery', - ); - return { delivered: true }; - } catch (err) { - this.#log.error( - { - kind: 'help-wanted.interest', - err, - projectSlug: n.projectSlug, - roleId: n.roleId, - }, - 'help-wanted interest: email send threw', - ); + this.#log.warn(ctx, 'help-wanted interest: no maintainer email; skipped'); return { delivered: false }; } + return this.#deliver( + 'help-wanted interest', + ctx, + n.maintainerEmail, + renderInterestEmail(n, this.#siteHost), + ); } async notifyWelcomeOnSignup(n: WelcomeNotification): Promise<{ delivered: boolean }> { + const ctx = { kind: 'auth.welcome', slug: n.slug }; if (!n.email) { - this.#log.warn( - { kind: 'auth.welcome', slug: n.slug }, - 'welcome: no email address; skipped', - ); - return { delivered: false }; - } - const tpl = renderWelcomeEmail(n, this.#siteHost); - try { - const result = await this.#resend.emails.send({ - from: this.#from, - to: n.email, - subject: tpl.subject, - text: tpl.text, - html: tpl.html, - }); - if (result.error) { - this.#log.error( - { kind: 'auth.welcome', err: result.error, slug: n.slug }, - 'welcome: Resend reported delivery failure', - ); - return { delivered: false }; - } - this.#log.info( - { kind: 'auth.welcome', slug: n.slug, resendId: result.data?.id }, - 'welcome: email queued for delivery', - ); - return { delivered: true }; - } catch (err) { - this.#log.error( - { kind: 'auth.welcome', err, slug: n.slug }, - 'welcome: email send threw', - ); + this.#log.warn(ctx, 'welcome: no email address; skipped'); return { delivered: false }; } + return this.#deliver('welcome', ctx, n.email, renderWelcomeEmail(n, this.#siteHost)); } async notifyPasswordReset(n: PasswordResetNotification): Promise<{ delivered: boolean }> { + const ctx = { kind: 'auth.password-reset', slug: n.slug }; if (!n.email) { - this.#log.warn( - { kind: 'auth.password-reset', slug: n.slug }, - 'password-reset: no email address; skipped', - ); - return { delivered: false }; - } - const tpl = renderPasswordResetEmail(n, this.#siteHost); - try { - const result = await this.#resend.emails.send({ - from: this.#from, - to: n.email, - subject: tpl.subject, - text: tpl.text, - html: tpl.html, - }); - if (result.error) { - this.#log.error( - { kind: 'auth.password-reset', err: result.error, slug: n.slug }, - 'password-reset: Resend reported delivery failure', - ); - return { delivered: false }; - } - this.#log.info( - { kind: 'auth.password-reset', slug: n.slug, resendId: result.data?.id }, - 'password-reset: email queued for delivery', - ); - return { delivered: true }; - } catch (err) { - this.#log.error( - { kind: 'auth.password-reset', err, slug: n.slug }, - 'password-reset: email send threw', - ); + this.#log.warn(ctx, 'password-reset: no email address; skipped'); return { delivered: false }; } + return this.#deliver( + 'password-reset', + ctx, + n.email, + renderPasswordResetEmail(n, this.#siteHost), + ); } async notifyHelpWantedFilled( n: HelpWantedFillNotification, ): Promise<{ delivered: boolean }> { + const ctx = { kind: 'help-wanted.filled', projectTitle: n.projectTitle }; if (!n.maintainerEmail) { - this.#log.warn( - { kind: 'help-wanted.filled', projectTitle: n.projectTitle }, - 'help-wanted fill: no maintainer email; skipped', - ); + this.#log.warn(ctx, 'help-wanted fill: no maintainer email; skipped'); return { delivered: false }; } - const tpl = renderFilledEmail(n, this.#siteHost); + return this.#deliver( + 'help-wanted fill', + ctx, + n.maintainerEmail, + renderFilledEmail(n, this.#siteHost), + ); + } + + /** + * Shared send path. The transport's only failure shape is a throw (the + * Postmark SDK raises on every non-2xx), so one catch covers network + * blips and provider rejections alike; the `err` field carries the + * provider's code/status for operators to tell them apart. + */ + async #deliver( + label: string, + ctx: Record, + to: string, + tpl: RenderedEmail, + ): Promise<{ delivered: boolean }> { try { - const result = await this.#resend.emails.send({ + const { messageId } = await this.#transport.send({ from: this.#from, - to: n.maintainerEmail, + to, subject: tpl.subject, text: tpl.text, html: tpl.html, }); - if (result.error) { - this.#log.error( - { kind: 'help-wanted.filled', err: result.error, projectTitle: n.projectTitle }, - 'help-wanted fill: Resend reported delivery failure', - ); - return { delivered: false }; - } - this.#log.info( - { - kind: 'help-wanted.filled', - projectTitle: n.projectTitle, - resendId: result.data?.id, - }, - 'help-wanted fill: email queued for delivery', - ); + this.#log.info({ ...ctx, messageId }, `${label}: email queued for delivery`); return { delivered: true }; } catch (err) { - this.#log.error( - { kind: 'help-wanted.filled', err, projectTitle: n.projectTitle }, - 'help-wanted fill: email send threw', - ); + this.#log.error({ ...ctx, err }, `${label}: email send failed`); return { delivered: false }; } } diff --git a/apps/api/src/notify/index.ts b/apps/api/src/notify/index.ts index 2d98fcc..85035d7 100644 --- a/apps/api/src/notify/index.ts +++ b/apps/api/src/notify/index.ts @@ -5,7 +5,7 @@ * Slack integration exists. Failures are logged but never fail the request — * the spec says express-interest returns 202 to the caller regardless. * - * The Resend / email transport is also stubbed; this module exists so the + * The email transport is also stubbed; this module exists so the * surface is in place for write-api to call and for tests to spy on. */ import type { FastifyBaseLogger } from 'fastify'; @@ -66,7 +66,8 @@ export interface Notifier { /** * Default no-op notifier — logs the intent and returns delivered:true. - * Replace with a real notifier once the Resend / Slack transports land. + * Replaced at boot by EmailNotifier when POSTMARK_SERVER_TOKEN is set; the + * Slack transport is still to come (#95). */ export class LoggingNotifier implements Notifier { readonly #log: FastifyBaseLogger; diff --git a/apps/api/src/notify/postmark-transport.ts b/apps/api/src/notify/postmark-transport.ts new file mode 100644 index 0000000..29ed5de --- /dev/null +++ b/apps/api/src/notify/postmark-transport.ts @@ -0,0 +1,48 @@ +/** + * PostmarkTransport — EmailTransport backed by the official `postmark` SDK. + * + * Postmark is the provider the legacy site already sends through, so the + * `codeforphilly.org` sender signature is verified there (see + * specs/architecture.md and docs/operations/secrets.md). The SDK throws a + * `PostmarkError` subclass on every non-2xx response (bad token, inactive + * recipient, rate limit, 5xx) and resolves with `{ MessageID, ... }` on + * success — so this adapter needs no `{ error }` branch; a throw is the + * only failure shape and the notifier catches it. + */ +import type { Message, Models } from 'postmark'; + +import type { EmailTransport, OutboundEmail } from './transport.js'; + +/** The slice of `postmark.ServerClient` this adapter touches. */ +export interface PostmarkSender { + sendEmail(email: Message): Promise; +} + +export interface PostmarkTransportOptions { + /** `new ServerClient(POSTMARK_SERVER_TOKEN)` at boot; anything with `sendEmail` in tests. */ + readonly client: PostmarkSender; + /** Postmark message stream. Defaults to `outbound` (the transactional default stream). */ + readonly messageStream?: string; +} + +export class PostmarkTransport implements EmailTransport { + readonly #client: PostmarkSender; + readonly #messageStream: string; + + constructor(opts: PostmarkTransportOptions) { + this.#client = opts.client; + this.#messageStream = opts.messageStream ?? 'outbound'; + } + + async send(email: OutboundEmail): Promise<{ messageId: string }> { + const result = await this.#client.sendEmail({ + From: email.from, + To: email.to, + Subject: email.subject, + TextBody: email.text, + HtmlBody: email.html, + MessageStream: this.#messageStream, + }); + return { messageId: result.MessageID }; + } +} diff --git a/apps/api/src/notify/transport.ts b/apps/api/src/notify/transport.ts new file mode 100644 index 0000000..1dc9448 --- /dev/null +++ b/apps/api/src/notify/transport.ts @@ -0,0 +1,25 @@ +/** + * EmailTransport — the one-method seam between the Notifier and whichever + * provider actually delivers mail. + * + * `EmailNotifier` composes subject/text/html from templates and hands the + * result here. The transport either resolves with a provider message id + * or throws; it never swallows failures — the notifier owns the + * log-and-return-`delivered: false` contract. Keeping the seam this narrow + * means tests exercise the notifier with a `vi.fn()` and the provider + * adapter (`postmark-transport.ts`) is the only file that knows a vendor. + */ +export interface OutboundEmail { + /** RFC 5322 sender, e.g. `"Code for Philly "`. */ + readonly from: string; + /** Single recipient address. */ + readonly to: string; + readonly subject: string; + readonly text: string; + readonly html: string; +} + +export interface EmailTransport { + /** Deliver one message. Resolves with the provider's message id; throws on any failure. */ + send(email: OutboundEmail): Promise<{ messageId: string }>; +} diff --git a/apps/api/src/plugins/services.ts b/apps/api/src/plugins/services.ts index 92bcbd5..f78d828 100644 --- a/apps/api/src/plugins/services.ts +++ b/apps/api/src/plugins/services.ts @@ -30,7 +30,8 @@ import { GitHubAccountService } from '../services/github-account.js'; import { AccountClaimService } from '../services/account-claim.js'; import { LoggingNotifier, type Notifier } from '../notify/index.js'; import { EmailNotifier } from '../notify/email-notifier.js'; -import { Resend } from 'resend'; +import { PostmarkTransport } from '../notify/postmark-transport.js'; +import { ServerClient } from 'postmark'; declare module 'fastify' { interface FastifyInstance { @@ -67,13 +68,17 @@ async function servicesPlugin(fastify: FastifyInstance): Promise { // (relevant in tests where multiple buildApp() runs share the module). invalidateFacets(); const fts = buildFtsEngine(state); - // Email notifier when RESEND_API_KEY is configured; otherwise fall back to - // the no-op LoggingNotifier so tests + dev runs work without a real key. + // Email notifier when POSTMARK_SERVER_TOKEN is configured; otherwise fall + // back to the no-op LoggingNotifier so tests + dev runs work without a + // real token. // Slack DM is deferred (#95) — when it lands it'll compose alongside email // here or via a CompoundNotifier wrapper. - const notifier: Notifier = fastify.config.RESEND_API_KEY + const notifier: Notifier = fastify.config.POSTMARK_SERVER_TOKEN ? new EmailNotifier({ - resend: new Resend(fastify.config.RESEND_API_KEY), + transport: new PostmarkTransport({ + client: new ServerClient(fastify.config.POSTMARK_SERVER_TOKEN), + messageStream: fastify.config.POSTMARK_MESSAGE_STREAM, + }), fromAddress: fastify.config.CFP_NOTIFICATION_FROM, siteHost: fastify.config.CFP_SITE_HOST, logger: fastify.log, diff --git a/apps/api/src/routes/auth.ts b/apps/api/src/routes/auth.ts index c249c0c..88d37d2 100644 --- a/apps/api/src/routes/auth.ts +++ b/apps/api/src/routes/auth.ts @@ -541,7 +541,7 @@ export async function authRoutes(fastify: FastifyInstance): Promise { }; await fastify.store.private.putPasswordToken(tokenRecord); - // Fire-and-forget — never block the response on Resend latency. + // Fire-and-forget — never block the response on email-provider latency. void fastify.notifier .notifyPasswordReset({ email: profile.email, diff --git a/apps/api/src/routes/people.ts b/apps/api/src/routes/people.ts index 7bff95a..ccd4dae 100644 --- a/apps/api/src/routes/people.ts +++ b/apps/api/src/routes/people.ts @@ -16,7 +16,6 @@ import { computePersonPermissions, getCallerSession } from '../services/permissi import { buildTransactionOptions } from '../store/commit-meta.js'; import type { UpdatePersonInput } from '../services/person.write.js'; import { AVATAR_ALLOWED_MIME, processAvatar } from '../lib/avatar.js'; -import { BlobObject } from 'hologit'; import type { Person } from '@cfp/shared/schemas'; import { PersonSchema } from '@cfp/shared/schemas'; import { StateApply } from '../store/state-apply.js'; @@ -404,7 +403,6 @@ export async function peopleRoutes(fastify: FastifyInstance): Promise { const newAvatarKey = `people/${person.slug}/avatar.jpg`; const stateApply = new StateApply(); - const hologit = fastify.publicRepo.hologitRepo; let updatedPerson: Person = person; await fastify.store.transact( @@ -418,17 +416,11 @@ export async function peopleRoutes(fastify: FastifyInstance): Promise { }), async (tx) => { // Write the two attachment blobs into the gitsheets transaction - // tree. BlobObject.write hashes the buffer into the git object DB - // via `git hash-object -w`; the tx-level setAttachments then wires - // the blob refs into the post-commit tree at the conventional path. - // - // BlobObject.write's TypeScript signature declares `content: string` - // but the underlying `git-client` `$putBlob` spawns `git hash-object - // --stdin -w` and pipes `content` to stdin, which accepts both - // strings and Buffers at runtime. Cast to match the declared shape; - // hologit's type would tighten upstream eventually. - const originalBlob = await BlobObject.write(hologit, processed.original as unknown as string); - const thumbnailBlob = await BlobObject.write(hologit, processed.thumbnail as unknown as string); + // tree. repo.writeBlob hashes the buffer into the git object DB; + // the tx-level setAttachments then wires the blob refs into the + // post-commit tree at the conventional path. + const originalBlob = await fastify.publicRepo.writeBlob(processed.original); + const thumbnailBlob = await fastify.publicRepo.writeBlob(processed.thumbnail); await tx.public.people.setAttachments(person, { 'avatar.jpg': originalBlob, 'avatar-128.jpg': thumbnailBlob, diff --git a/apps/api/src/routes/saml.ts b/apps/api/src/routes/saml.ts index d07c7d0..e565398 100644 --- a/apps/api/src/routes/saml.ts +++ b/apps/api/src/routes/saml.ts @@ -128,18 +128,21 @@ function getSamlContext(fastify: FastifyInstance): SamlContext { throw new ApiValidationError('SAML IdP is not configured'); } - const base = `https://${cfg.SLACK_TEAM_HOST}`.replace('https://', ''); - const issuerHost = base; - // Fallback to the team host for the metadata entity ID if we can't see - // the inbound request origin. Per spec the entityID is our own URL — - // we'll prefer the request origin when building responses. + // Three distinct sources, per specs/api/saml.md#idp-identity-and-hosts: + // - entityId (metadata entityID + assertion Issuer) is the stable + // SAML_ENTITY_ID — it must NOT track the serving host, because Slack + // stored it at setup and the host flips at cutover; + // - the SSO endpoint Locations follow CFP_SITE_HOST so the metadata + // points Slack at whatever host this deployment answers on; + // - SLACK_TEAM_HOST is Slack's side only (ACS URL, NameQualifier). + const ssoUrl = `https://${cfg.CFP_SITE_HOST}/api/saml/slack/sso`; const ctx: SamlContext = { entities: buildSlackSamlEntities({ privateKey: cfg.SAML_PRIVATE_KEY, certificate: cfg.SAML_CERTIFICATE, - entityId: `https://${issuerHost}/api/saml/slack/metadata`, - ssoLoginPostUrl: `https://${issuerHost}/api/saml/slack/sso`, - ssoLoginRedirectUrl: `https://${issuerHost}/api/saml/slack/sso`, + entityId: cfg.SAML_ENTITY_ID, + ssoLoginPostUrl: ssoUrl, + ssoLoginRedirectUrl: ssoUrl, slackTeamHost: cfg.SLACK_TEAM_HOST, }), }; diff --git a/apps/api/src/saml/config.ts b/apps/api/src/saml/config.ts index caa67a7..7038188 100644 --- a/apps/api/src/saml/config.ts +++ b/apps/api/src/saml/config.ts @@ -36,13 +36,21 @@ export interface SamlIdpSettings { readonly privateKey: string; /** PEM-encoded X.509 certificate (the public half). */ readonly certificate: string; - /** The IdP entity ID — also the metadata URL. */ + /** + * The IdP entity ID — becomes the metadata `entityID` AND the `` + * on every Response/Assertion (via `SlackSamlEntities.entityId` → + * `issuerEntityId`). A stable logical identifier (`SAML_ENTITY_ID`), not + * necessarily a URL that resolves on the serving host. + */ readonly entityId: string; - /** The IdP SSO POST binding location (the /launch endpoint). */ + /** The IdP SSO POST binding location — `https:///api/saml/slack/sso`. */ readonly ssoLoginPostUrl: string; - /** The IdP SSO Redirect binding location. */ + /** The IdP SSO Redirect binding location — same URL as the POST binding. */ readonly ssoLoginRedirectUrl: string; - /** Slack team host (e.g. `codeforphilly.slack.com`). */ + /** + * Slack team host (e.g. `codeforphilly.slack.com`). Slack-side only: the + * ACS URL and the NameID `NameQualifier`. Never part of our own identity. + */ readonly slackTeamHost: string; } diff --git a/apps/api/src/store/memory/reload.ts b/apps/api/src/store/memory/reload.ts index 3e53ec3..bb1c430 100644 --- a/apps/api/src/store/memory/reload.ts +++ b/apps/api/src/store/memory/reload.ts @@ -85,42 +85,31 @@ export async function reloadInMemoryStateAndFts( } /** - * Synchronously replace the contents of every Map on `live` with the - * contents from `fresh`. Object identity of `live` is preserved. + * Synchronously replace the contents of every collection on `live` with + * the contents from `fresh`. Object identity of `live` — and of every Map + * hanging off it — is preserved. + * + * Enumerates `fresh`'s own properties rather than naming each field: a + * hand-maintained list silently skipped three secondary indices + * (`projectIdByLegacyId`, `buzzIdBySlug`, `slugHistory`) and left legacy + * and slug-history redirects pointing at ids that no longer existed after + * a re-import + hot reload. Every own property of `InMemoryState` is a Map + * today; if a future field is anything else this throws so the author has + * to decide how it's swapped, instead of it being skipped again. Per + * specs/behaviors/storage.md#hot-reload → Atomicity. * * Exported for testability — production code should call * `reloadInMemoryStateAndFts`. */ export function swapInPlace(live: InMemoryState, fresh: InMemoryState): void { - // Primary entity maps. - replaceMapContents(live.projects, fresh.projects); - replaceMapContents(live.people, fresh.people); - replaceMapContents(live.tags, fresh.tags); - replaceMapContents(live.tagAssignments, fresh.tagAssignments); - replaceMapContents(live.projectMemberships, fresh.projectMemberships); - replaceMapContents(live.projectUpdates, fresh.projectUpdates); - replaceMapContents(live.projectBuzz, fresh.projectBuzz); - replaceMapContents(live.blogPosts, fresh.blogPosts); - replaceMapContents(live.helpWantedRoles, fresh.helpWantedRoles); - replaceMapContents(live.helpWantedInterest, fresh.helpWantedInterest); - - // Secondary indices. - replaceMapContents(live.projectSlugById, fresh.projectSlugById); - replaceMapContents(live.projectIdBySlug, fresh.projectIdBySlug); - replaceMapContents(live.personSlugById, fresh.personSlugById); - replaceMapContents(live.personIdBySlug, fresh.personIdBySlug); - replaceMapContents(live.tagIdByHandle, fresh.tagIdByHandle); - replaceMapContents(live.membershipsByProject, fresh.membershipsByProject); - replaceMapContents(live.membershipsByPerson, fresh.membershipsByPerson); - replaceMapContents(live.updatesByProject, fresh.updatesByProject); - replaceMapContents(live.updateByProjectAndNumber, fresh.updateByProjectAndNumber); - replaceMapContents(live.buzzByProject, fresh.buzzByProject); - replaceMapContents(live.buzzByProjectAndSlug, fresh.buzzByProjectAndSlug); - replaceMapContents(live.blogPostIdBySlug, fresh.blogPostIdBySlug); - replaceMapContents(live.blogPostIdByLegacyId, fresh.blogPostIdByLegacyId); - replaceMapContents(live.helpWantedByProject, fresh.helpWantedByProject); - replaceMapContents(live.tagAssignmentsByTaggable, fresh.tagAssignmentsByTaggable); - replaceMapContents(live.tagAssignmentsByTag, fresh.tagAssignmentsByTag); - replaceMapContents(live.interestByRoleAndPerson, fresh.interestByRoleAndPerson); - replaceMapContents(live.interestByRole, fresh.interestByRole); + for (const key of Object.keys(fresh) as (keyof InMemoryState)[]) { + const target: unknown = live[key]; + const source: unknown = fresh[key]; + if (!(target instanceof Map) || !(source instanceof Map)) { + throw new Error( + `swapInPlace: InMemoryState.${key} is not a Map — extend swapInPlace to handle it`, + ); + } + replaceMapContents(target, source); + } } diff --git a/apps/api/src/store/public.ts b/apps/api/src/store/public.ts index 348caac..3d752a7 100644 --- a/apps/api/src/store/public.ts +++ b/apps/api/src/store/public.ts @@ -33,7 +33,49 @@ import type { import type { Project } from '@cfp/shared/schemas'; /** - * Cast a Zod v4 schema to gitsheets' StandardSchemaV1. + * Recursively drop `null` / `undefined`-valued keys from a record. + * + * gitsheets 2.x (Rust core) refuses to marshal `null` or `undefined` field + * values — `serializeRecords`/`upsert` throw `cannot marshal JS value of type + * Null/Undefined to a TOML value`. gitsheets 1.4.1 (`@iarna/toml`) silently + * dropped such keys instead, so they were never written to disk: an absent + * optional field is simply an absent TOML key (verified against the on-disk + * `published` snapshot — no record carries a `null`-valued key). + * + * Our Zod schemas mark optional fields `.nullable().optional()` and the write + * services normalize "cleared" fields to `?? null`. To keep the on-disk form + * byte-identical to 1.4.1 (and to keep those `?? null` write paths working), + * we strip null/undefined keys here, at the single write boundary, before the + * record reaches the core marshaller. Nested tables (objects) are cleaned + * recursively; arrays are passed through untouched (TOML has no null in + * arrays, and our schemas never emit sparse arrays). + * + * `null` as an explicit "delete this field" signal only exists for + * `Sheet.patch` (RFC 7396 merge-patch); we don't use `patch`, so stripping on + * the full-record `upsert` path is unambiguous. + */ +function stripNullish(value: unknown): unknown { + if (value === null || value === undefined) return undefined; + if (Array.isArray(value)) return value; + if (typeof value === 'object' && !(value instanceof Date)) { + const out: Record = {}; + for (const [k, v] of Object.entries(value as Record)) { + const cleaned = stripNullish(v); + if (cleaned !== undefined) out[k] = cleaned; + } + return out; + } + return value; +} + +/** + * Cast a Zod v4 schema to gitsheets' StandardSchemaV1, wrapping its validator + * so the validated record has null/undefined-valued keys stripped before it + * reaches gitsheets' core marshaller. + * + * gitsheets runs the Standard Schema validator host-side and marshals the + * validator's *output* — so stripping here (rather than at every upsert call + * site) is the single, authoritative write boundary. See `stripNullish`. * * Zod v4 implements the Standard Schema interface at runtime, but TypeScript * cannot prove that Zod's Result type is assignable to gitsheets' narrow @@ -41,7 +83,24 @@ import type { Project } from '@cfp/shared/schemas'; * shape. Both are correct at runtime; the cast is safe. */ function asValidator>(schema: unknown): StandardSchemaV1 { - return schema as StandardSchemaV1; + const inner = schema as StandardSchemaV1; + const innerValidate = inner['~standard'].validate; + return { + ...inner, + '~standard': { + ...inner['~standard'], + validate: (value: unknown) => { + const result = innerValidate(value); + const strip = ( + r: Awaited>, + ): Awaited> => + r.issues === undefined + ? { value: stripNullish(r.value) as T } + : r; + return result instanceof Promise ? result.then(strip) : strip(result); + }, + }, + }; } /** Typed validator map for openStore. */ diff --git a/apps/api/tests/cutover-mailout.test.ts b/apps/api/tests/cutover-mailout.test.ts index 2c57648..5983426 100644 --- a/apps/api/tests/cutover-mailout.test.ts +++ b/apps/api/tests/cutover-mailout.test.ts @@ -190,13 +190,13 @@ describe('cutover-mailout', () => { privateStore, mode: 'send', send: async () => { - throw new Error('Resend 429'); + throw new Error('Postmark 429'); }, now: NOW, }); expect(report.sent).toBe(0); expect(report.failed).toHaveLength(1); - expect(report.failed[0]?.error).toContain('Resend 429'); + expect(report.failed[0]?.error).toContain('Postmark 429'); } finally { await repo.cleanup(); await priv.cleanup(); diff --git a/apps/api/tests/email-notifier.test.ts b/apps/api/tests/email-notifier.test.ts index 69629a1..1e59f7a 100644 --- a/apps/api/tests/email-notifier.test.ts +++ b/apps/api/tests/email-notifier.test.ts @@ -1,9 +1,11 @@ /** - * Tests for the Resend-backed EmailNotifier (apps/api/src/notify/email-notifier.ts). + * Tests for the transport-backed EmailNotifier (apps/api/src/notify/email-notifier.ts). * - * Mocks the Resend SDK at the `emails.send` boundary — verifies that the + * Stubs the `EmailTransport` seam with a `vi.fn()` — verifies that the * notifier composes the right payload + handles delivery success/failure * per the spec (express-interest must return 202 to the caller regardless). + * The Postmark adapter behind that seam has its own test + * (postmark-transport.test.ts). * * Template renderers are also exercised here with snapshot-style asserts * on the interpolated fields, since they're pure functions with simple @@ -62,10 +64,9 @@ const baseWelcome: WelcomeNotification = { slug: 'new-user', }; -function makeNotifier(emails: { send: ReturnType }): EmailNotifier { +function makeNotifier(transport: { send: ReturnType }): EmailNotifier { return new EmailNotifier({ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - resend: { emails } as any, + transport, fromAddress: 'Code for Philly ', siteHost: 'codeforphilly.org', logger: noopLogger, @@ -120,8 +121,8 @@ describe('renderFilledEmail', () => { }); describe('EmailNotifier.notifyHelpWantedInterest', () => { - it('sends via Resend and returns delivered:true', async () => { - const send = vi.fn().mockResolvedValue({ data: { id: 'msg-123' }, error: null }); + it('sends via the transport and returns delivered:true', async () => { + const send = vi.fn().mockResolvedValue({ messageId: 'msg-123' }); const notifier = makeNotifier({ send }); const result = await notifier.notifyHelpWantedInterest(baseInterest); @@ -135,7 +136,7 @@ describe('EmailNotifier.notifyHelpWantedInterest', () => { expect(arg.html).toContain('Jane Doe'); }); - it('returns delivered:false when maintainerEmail is null (no Resend call)', async () => { + it('returns delivered:false when maintainerEmail is null (no transport call)', async () => { const send = vi.fn(); const notifier = makeNotifier({ send }); @@ -147,17 +148,20 @@ describe('EmailNotifier.notifyHelpWantedInterest', () => { expect(send).not.toHaveBeenCalled(); }); - it('returns delivered:false when Resend reports an error', async () => { + it('returns delivered:false when the provider rejects the send', async () => { + // Postmark surfaces API rejections (unverified sender, inactive + // recipient, bad token) as thrown errors carrying code + statusCode. const send = vi .fn() - .mockResolvedValue({ data: null, error: { message: 'Sender domain unverified' } }); + .mockRejectedValue(Object.assign(new Error('Sender signature not found'), { code: 400, statusCode: 422 })); const notifier = makeNotifier({ send }); const result = await notifier.notifyHelpWantedInterest(baseInterest); expect(result).toEqual({ delivered: false }); + expect(noopLogger.error).toHaveBeenCalled(); }); - it('returns delivered:false when the Resend SDK throws', async () => { + it('returns delivered:false when the transport throws', async () => { const send = vi.fn().mockRejectedValue(new Error('network blip')); const notifier = makeNotifier({ send }); @@ -196,8 +200,8 @@ describe('renderWelcomeEmail', () => { }); describe('EmailNotifier.notifyWelcomeOnSignup', () => { - it('sends via Resend and returns delivered:true', async () => { - const send = vi.fn().mockResolvedValue({ data: { id: 'msg-welcome' }, error: null }); + it('sends via the transport and returns delivered:true', async () => { + const send = vi.fn().mockResolvedValue({ messageId: 'msg-welcome' }); const notifier = makeNotifier({ send }); const result = await notifier.notifyWelcomeOnSignup(baseWelcome); @@ -210,11 +214,10 @@ describe('EmailNotifier.notifyWelcomeOnSignup', () => { expect(arg.html).toContain('New User'); }); - it('returns delivered:false when Resend reports an error', async () => { - const send = vi.fn().mockResolvedValue({ - data: null, - error: { message: 'Sender domain unverified' }, - }); + it('returns delivered:false when the provider rejects the send', async () => { + const send = vi + .fn() + .mockRejectedValue(Object.assign(new Error('Inactive recipient'), { code: 406, statusCode: 422 })); const notifier = makeNotifier({ send }); const result = await notifier.notifyWelcomeOnSignup(baseWelcome); expect(result).toEqual({ delivered: false }); @@ -237,8 +240,8 @@ describe('EmailNotifier.notifyWelcomeOnSignup', () => { }); describe('EmailNotifier.notifyHelpWantedFilled', () => { - it('sends via Resend and returns delivered:true', async () => { - const send = vi.fn().mockResolvedValue({ data: { id: 'msg-456' }, error: null }); + it('sends via the transport and returns delivered:true', async () => { + const send = vi.fn().mockResolvedValue({ messageId: 'msg-456' }); const notifier = makeNotifier({ send }); const result = await notifier.notifyHelpWantedFilled(baseFill); diff --git a/apps/api/tests/github-oauth.test.ts b/apps/api/tests/github-oauth.test.ts index 34226a7..609ae14 100644 --- a/apps/api/tests/github-oauth.test.ts +++ b/apps/api/tests/github-oauth.test.ts @@ -519,7 +519,7 @@ describe('GET /api/auth/github/callback — fresh user outcome', () => { const ip = nextTestIp(); const flow = await startFlow(app, '/', ip); - // Spy on the boot-installed LoggingNotifier (no Resend in tests). + // Spy on the boot-installed LoggingNotifier (no Postmark in tests). // The notifier call is fire-and-forget — we await the OAuth response // first, then assert the spy. The notifier's spawn is synchronous up // to the await inside it, so it's guaranteed to have been called by diff --git a/apps/api/tests/helpers/mocks.ts b/apps/api/tests/helpers/mocks.ts index f7b4786..5dcad22 100644 --- a/apps/api/tests/helpers/mocks.ts +++ b/apps/api/tests/helpers/mocks.ts @@ -22,12 +22,14 @@ export interface GitHubEmail { /** * A captured outbound email send — inspectable in tests. */ +/** Postmark `POST /email` body — PascalCase fields as the API wants them. */ export interface CapturedEmail { - readonly to: string | string[]; - readonly from: string; - readonly subject: string; - readonly html?: string; - readonly text?: string; + readonly To: string; + readonly From: string; + readonly Subject: string; + readonly HtmlBody?: string; + readonly TextBody?: string; + readonly MessageStream?: string; } /** @@ -92,23 +94,32 @@ export function createGitHubMock(defaults?: { } /** - * No-op Resend mock. Intercepts POST /emails via MSW and collects sends - * into an in-memory array for inspection. Does not call the real Resend API. + * No-op Postmark mock. Intercepts POST /email via MSW and collects sends + * into an in-memory array for inspection. Does not call the real Postmark API. * * Usage: - * const { server, sentEmails } = createResendMock(); + * const { server, sentEmails } = createPostmarkMock(); * beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); * afterEach(() => { server.resetHandlers(); sentEmails.length = 0; }); * afterAll(() => server.close()); */ -export function createResendMock() { +export function createPostmarkMock() { const sentEmails: CapturedEmail[] = []; const server = setupServer( - http.post('https://api.resend.com/emails', async ({ request }) => { + http.post('https://api.postmarkapp.com/email', async ({ request }) => { const body = (await request.json()) as CapturedEmail; sentEmails.push(body); - return HttpResponse.json({ id: `mock-${Date.now()}` }, { status: 200 }); + return HttpResponse.json( + { + To: body.To, + SubmittedAt: new Date().toISOString(), + MessageID: `mock-${Date.now()}`, + ErrorCode: 0, + Message: 'OK', + }, + { status: 200 }, + ); }), ); diff --git a/apps/api/tests/internal-reload.test.ts b/apps/api/tests/internal-reload.test.ts index fffd5c5..85e754d 100644 --- a/apps/api/tests/internal-reload.test.ts +++ b/apps/api/tests/internal-reload.test.ts @@ -15,6 +15,12 @@ * record introduced on the "remote" must be visible via a service * call AFTER the reload completes (proves the in-memory state + * FTS index actually got rebuilt against the new tree). + * - Re-import scenario: a project is replaced on the remote by a + * record with a fresh id and slug (what the laddr importer does on + * every run). After the reload the legacy `/projects?ID=` redirect, + * the `/project-buzz/` redirect, and the slug-history 301 + * must all resolve against the NEW records — the secondary indices + * behind them were once skipped by the in-place swap. */ import { execFile } from 'node:child_process'; import { mkdtemp, rm, writeFile } from 'node:fs/promises'; @@ -117,14 +123,14 @@ async function createRig(): Promise { } /** - * Advance the bare remote by one commit on `main` via an ephemeral - * clone. Used to put the local working tree behind so a hot reload - * fast-forwards. The new commit introduces a fresh project record at - * `projects/.toml`. + * Advance the bare remote by one commit on `main` via an ephemeral clone. + * `mutate` edits the clone's working tree and stages whatever it changed + * (paths are relative to the clone root). Returns the new remote HEAD. */ -async function advanceRemoteWithProject( +async function advanceRemote( rig: Rig, - fields: { id: string; slug: string; title: string; summary?: string }, + message: string, + mutate: (wt: string) => Promise, ): Promise { const wt = `${rig.local}-advance-${Date.now()}-${Math.random() .toString(36) @@ -135,12 +141,31 @@ async function advanceRemoteWithProject( await git(wt, 'config', 'commit.gpgsign', 'false'); await git(wt, 'config', 'core.hooksPath', '/dev/null'); - // Minimal Project TOML the gitsheets reader will accept + the Zod - // schema will validate at load time. The schema allows a lot of - // optional fields; we provide only the required ones plus a couple - // for the assertion. - const toml = [ + await mutate(wt); + await git(wt, 'commit', '-m', message); + await git(wt, 'push', 'origin', 'main'); + const head = await git(wt, 'rev-parse', 'HEAD'); + await rm(wt, { recursive: true, force: true }); + return head; +} + +interface ProjectFields { + id: string; + slug: string; + title: string; + summary?: string; + legacyId?: number; +} + +/** + * Minimal Project TOML the gitsheets reader will accept + the Zod schema + * will validate at load time. The schema allows a lot of optional fields; + * we provide only the required ones plus a couple for the assertions. + */ +function projectToml(fields: ProjectFields): string { + return [ `id = '${fields.id}'`, + ...(fields.legacyId !== undefined ? [`legacyId = ${fields.legacyId}`] : []), `slug = '${fields.slug}'`, `title = '${fields.title}'`, ...(fields.summary ? [`summary = '${fields.summary}'`] : []), @@ -150,14 +175,66 @@ async function advanceRemoteWithProject( `updatedAt = '2026-05-19T00:00:00Z'`, '', ].join('\n'); +} + +async function writeProject(wt: string, fields: ProjectFields): Promise { await exec('mkdir', ['-p', join(wt, 'projects')]); - await writeFile(join(wt, 'projects', `${fields.slug}.toml`), toml); + await writeFile(join(wt, 'projects', `${fields.slug}.toml`), projectToml(fields)); await git(wt, 'add', `projects/${fields.slug}.toml`); - await git(wt, 'commit', '-m', `seed: project ${fields.slug}`); - await git(wt, 'push', 'origin', 'main'); - const head = await git(wt, 'rev-parse', 'HEAD'); - await rm(wt, { recursive: true, force: true }); - return head; +} + +async function writeBuzz( + wt: string, + fields: { id: string; projectId: string; projectSlug: string; slug: string }, +): Promise { + const rel = `project-buzz/${fields.projectSlug}/${fields.slug}.toml`; + await exec('mkdir', ['-p', join(wt, 'project-buzz', fields.projectSlug)]); + await writeFile( + join(wt, rel), + [ + `id = '${fields.id}'`, + `projectId = '${fields.projectId}'`, + `slug = '${fields.slug}'`, + `headline = 'Buzz ${fields.slug}'`, + `url = 'https://example.test/${fields.slug}'`, + `publishedAt = '2026-05-19T00:00:00Z'`, + `createdAt = '2026-05-19T00:00:00Z'`, + `updatedAt = '2026-05-19T00:00:00Z'`, + '', + ].join('\n'), + ); + await git(wt, 'add', rel); +} + +async function writeSlugHistory( + wt: string, + fields: { id: string; entityId: string; oldSlug: string; newSlug: string }, +): Promise { + const rel = `slug-history/project/${fields.oldSlug}.toml`; + await exec('mkdir', ['-p', join(wt, 'slug-history', 'project')]); + await writeFile( + join(wt, rel), + [ + `id = '${fields.id}'`, + `entityType = 'project'`, + `oldSlug = '${fields.oldSlug}'`, + `newSlug = '${fields.newSlug}'`, + `entityId = '${fields.entityId}'`, + `changedAt = '2026-05-19T00:00:00Z'`, + `expiresAt = '2099-01-01T00:00:00Z'`, + '', + ].join('\n'), + ); + await git(wt, 'add', rel); +} + +/** + * Advance the remote by one commit that introduces a fresh project record + * at `projects/.toml`. Used to put the local clone behind so a hot + * reload fast-forwards. + */ +async function advanceRemoteWithProject(rig: Rig, fields: ProjectFields): Promise { + return advanceRemote(rig, `seed: project ${fields.slug}`, (wt) => writeProject(wt, fields)); } // --------------------------------------------------------------------------- @@ -390,4 +467,85 @@ describe('POST /api/_internal/reload-data — short-circuit + reconcile', () => const contents = await git(rig.local, 'show', 'HEAD:projects/lazyloader.toml'); expect(contents).toContain("slug = 'lazyloader'"); }); + + it('re-points legacy, buzz, and slug-history redirects after a re-import mints fresh ids', async () => { + // Seed the remote with a project carrying a laddr legacy id plus one + // buzz item, then bring the local clone up to date so the app boots + // in-sync with those records already indexed (production pods + // bare-clone fresh on every boot, so in-sync at boot is the norm). + const oldProjectId = '01951a3c-0000-7000-8000-000000000101'; + await advanceRemote(rig, 'seed: alpha-v1 + buzz', async (wt) => { + await writeProject(wt, { id: oldProjectId, slug: 'alpha-v1', title: 'Alpha', legacyId: 42 }); + await writeBuzz(wt, { + id: '01951a3c-0000-7000-8000-000000000103', + projectId: oldProjectId, + projectSlug: 'alpha-v1', + slug: 'alpha-launch', + }); + }); + await git(rig.local, 'fetch', 'origin', `${rig.branch}:${rig.branch}`); + app = await buildTestApp({ CFP_DATA_RELOAD_SECRET: VALID_SECRET }); + + const legacyBefore = await app.inject({ method: 'GET', url: '/projects?ID=42' }); + expect(legacyBefore.statusCode).toBe(301); + expect(legacyBefore.headers.location).toBe('/projects/alpha-v1'); + + const buzzBefore = await app.inject({ method: 'GET', url: '/project-buzz/alpha-launch' }); + expect(buzzBefore.statusCode).toBe(301); + expect(buzzBefore.headers.location).toBe('/projects/alpha-v1/buzz/alpha-launch'); + + // Re-import: the importer replaces the tree wholesale, minting fresh + // ids. Same legacy id, new project id + slug, new buzz id + slug, and + // a slug-history record so the old slug keeps resolving. + const newProjectId = '01951a3c-0000-7000-8000-000000000201'; + const newRemoteHead = await advanceRemote(rig, 're-import: alpha-v2', async (wt) => { + await git(wt, 'rm', '-q', 'projects/alpha-v1.toml', 'project-buzz/alpha-v1/alpha-launch.toml'); + await writeProject(wt, { id: newProjectId, slug: 'alpha-v2', title: 'Alpha', legacyId: 42 }); + await writeBuzz(wt, { + id: '01951a3c-0000-7000-8000-000000000203', + projectId: newProjectId, + projectSlug: 'alpha-v2', + slug: 'alpha-relaunch', + }); + await writeSlugHistory(wt, { + id: '01951a3c-0000-7000-8000-000000000206', + entityId: newProjectId, + oldSlug: 'alpha-v1', + newSlug: 'alpha-v2', + }); + }); + + const res = await app.inject({ + method: 'POST', + url: '/api/_internal/reload-data', + headers: { authorization: `Bearer ${VALID_SECRET}` }, + payload: { branch: rig.branch, commitHash: newRemoteHead }, + }); + expect(res.statusCode).toBe(200); + expect(res.json<{ data: { rebuilt: boolean } }>().data.rebuilt).toBe(true); + + // Legacy id → the NEW project. Before the fix, projectIdByLegacyId + // still held the old id, projectSlugById no longer knew it, and the + // request fell through to the SPA. + const legacyAfter = await app.inject({ method: 'GET', url: '/projects?ID=42' }); + expect(legacyAfter.statusCode).toBe(301); + expect(legacyAfter.headers.location).toBe('/projects/alpha-v2'); + + const updatesAfter = await app.inject({ method: 'GET', url: '/project-updates?ProjectID=42' }); + expect(updatesAfter.statusCode).toBe(301); + expect(updatesAfter.headers.location).toBe('/projects/alpha-v2'); + + // Buzz slug → the NEW buzz under the NEW project slug; the retired + // buzz slug no longer redirects. + const buzzAfter = await app.inject({ method: 'GET', url: '/project-buzz/alpha-relaunch' }); + expect(buzzAfter.statusCode).toBe(301); + expect(buzzAfter.headers.location).toBe('/projects/alpha-v2/buzz/alpha-relaunch'); + const buzzRetired = await app.inject({ method: 'GET', url: '/project-buzz/alpha-launch' }); + expect(buzzRetired.statusCode).not.toBe(301); + + // Slug history → the old project URL 301s to the new slug. + const slugAfter = await app.inject({ method: 'GET', url: '/projects/alpha-v1' }); + expect(slugAfter.statusCode).toBe(301); + expect(slugAfter.headers.location).toBe('/projects/alpha-v2'); + }); }); diff --git a/apps/api/tests/postmark-transport.test.ts b/apps/api/tests/postmark-transport.test.ts new file mode 100644 index 0000000..a34c308 --- /dev/null +++ b/apps/api/tests/postmark-transport.test.ts @@ -0,0 +1,89 @@ +/** + * Tests for PostmarkTransport (apps/api/src/notify/postmark-transport.ts). + * + * Two layers: + * - unit: a stub `sendEmail` proves the OutboundEmail → Postmark Message + * field mapping and that SDK errors propagate (the notifier owns + * catch-and-log, so the transport must not swallow them). + * - integration: the real `ServerClient` against an MSW intercept of + * `POST https://api.postmarkapp.com/email`, so a change in the SDK's + * wire format or auth header would surface here rather than in prod. + */ +import { ServerClient } from 'postmark'; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; + +import { PostmarkTransport } from '../src/notify/postmark-transport.js'; +import { createPostmarkMock } from './helpers/mocks.js'; + +const email = { + from: 'Code for Philly ', + to: 'maintainer@example.com', + subject: 'Hello', + text: 'plain body', + html: '

html body

', +}; + +describe('PostmarkTransport (unit)', () => { + it('maps OutboundEmail onto the Postmark Message shape and returns MessageID', async () => { + const sendEmail = vi.fn().mockResolvedValue({ + To: email.to, + SubmittedAt: '2026-09-08T00:00:00Z', + MessageID: 'pm-123', + ErrorCode: 0, + Message: 'OK', + }); + const transport = new PostmarkTransport({ client: { sendEmail } }); + + const result = await transport.send(email); + expect(result).toEqual({ messageId: 'pm-123' }); + expect(sendEmail).toHaveBeenCalledTimes(1); + expect(sendEmail.mock.calls[0]![0]).toEqual({ + From: email.from, + To: email.to, + Subject: 'Hello', + TextBody: 'plain body', + HtmlBody: '

html body

', + MessageStream: 'outbound', + }); + }); + + it('honours an explicit messageStream', async () => { + const sendEmail = vi.fn().mockResolvedValue({ MessageID: 'pm-1', SubmittedAt: '', ErrorCode: 0, Message: 'OK' }); + const transport = new PostmarkTransport({ client: { sendEmail }, messageStream: 'notifications' }); + await transport.send(email); + expect(sendEmail.mock.calls[0]![0].MessageStream).toBe('notifications'); + }); + + it('propagates SDK errors untouched', async () => { + const boom = Object.assign(new Error('Inactive recipient'), { code: 406, statusCode: 422 }); + const sendEmail = vi.fn().mockRejectedValue(boom); + const transport = new PostmarkTransport({ client: { sendEmail } }); + await expect(transport.send(email)).rejects.toBe(boom); + }); +}); + +describe('PostmarkTransport (real ServerClient over MSW)', () => { + const mock = createPostmarkMock(); + beforeAll(() => mock.server.listen({ onUnhandledRequest: 'error' })); + afterEach(() => { + mock.server.resetHandlers(); + mock.sentEmails.length = 0; + }); + afterAll(() => mock.server.close()); + + it('POSTs the expected JSON body to /email', async () => { + const transport = new PostmarkTransport({ client: new ServerClient('test-server-token') }); + const result = await transport.send(email); + + expect(result.messageId).toMatch(/^mock-/); + expect(mock.sentEmails).toHaveLength(1); + expect(mock.sentEmails[0]).toEqual({ + From: email.from, + To: email.to, + Subject: 'Hello', + TextBody: 'plain body', + HtmlBody: '

html body

', + MessageStream: 'outbound', + }); + }); +}); diff --git a/apps/api/tests/reload-swap.test.ts b/apps/api/tests/reload-swap.test.ts new file mode 100644 index 0000000..b018837 --- /dev/null +++ b/apps/api/tests/reload-swap.test.ts @@ -0,0 +1,247 @@ +/** + * Unit tests for `swapInPlace` — the in-place Map replacement behind the + * hot-reload webhook (specs/behaviors/storage.md#hot-reload → Atomicity). + * + * The regression this guards: `swapInPlace` used to name each field of + * `InMemoryState` by hand and skipped `projectIdByLegacyId`, + * `buzzIdBySlug`, and `slugHistory`. Because the laddr importer mints + * fresh ids every run, a re-import + hot reload left legacy redirects + * pointing at project ids that no longer existed. These tests enumerate + * every own property of a fresh state so a newly added collection can't + * be silently skipped again. + */ +import { describe, expect, it } from 'vitest'; +import type { + BlogPost, + HelpWantedInterestExpression, + HelpWantedRole, + Person, + Project, + ProjectBuzz, + ProjectMembership, + ProjectUpdate, + SlugHistory, + Tag, + TagAssignment, +} from '@cfp/shared/schemas'; + +import { swapInPlace } from '../src/store/memory/reload.js'; +import { + createEmptyState, + indexBlogPost, + indexHelpWantedInterest, + indexHelpWantedRole, + indexMembership, + indexPerson, + indexProject, + indexProjectBuzz, + indexProjectUpdate, + indexSlugHistory, + indexTag, + indexTagAssignment, + slugHistoryKey, + type InMemoryState, +} from '../src/store/memory/state.js'; + +const NOW = '2026-06-01T00:00:00Z'; +const FAR_FUTURE = '2099-01-01T00:00:00Z'; + +function uuid(n: number): string { + return `01951a3c-0000-7000-8000-${String(n).padStart(12, '0')}`; +} + +function makeProject(n: number, slug: string, legacyId: number): Project { + return { + id: uuid(n), + legacyId, + slug, + title: slug, + summary: null, + overview: null, + stage: 'prototyping', + maintainerId: null, + featured: false, + deletedAt: null, + createdAt: NOW, + updatedAt: NOW, + }; +} + +function makePerson(n: number, slug: string): Person { + return { + id: uuid(n), + slug, + fullName: slug, + accountLevel: 'user', + createdAt: NOW, + updatedAt: NOW, + } as Person; +} + +function makeBuzz(n: number, projectId: string, slug: string): ProjectBuzz { + return { + id: uuid(n), + projectId, + slug, + headline: slug, + url: `https://example.test/${slug}`, + publishedAt: NOW, + createdAt: NOW, + updatedAt: NOW, + }; +} + +function makeTag(n: number, slug: string): Tag { + return { id: uuid(n), namespace: 'tech', slug, title: slug, createdAt: NOW, updatedAt: NOW }; +} + +function makeAssignment(n: number, tagId: string, projectId: string): TagAssignment { + return { id: uuid(n), tagId, taggableType: 'project', taggableId: projectId, createdAt: NOW }; +} + +/** + * The remaining entity types only need the fields their index helpers read + * (ids + foreign keys). Cast rather than spell out every schema field — + * this test is about index bookkeeping, not record validation. + */ +function makeMembership(n: number, projectId: string, personId: string): ProjectMembership { + return { id: uuid(n), projectId, personId, role: 'member', createdAt: NOW, updatedAt: NOW } as unknown as ProjectMembership; +} + +function makeUpdate(n: number, projectId: string, number: number): ProjectUpdate { + return { id: uuid(n), projectId, number, createdAt: NOW, updatedAt: NOW } as unknown as ProjectUpdate; +} + +function makeBlogPost(n: number, slug: string, legacyId: number): BlogPost { + return { id: uuid(n), slug, legacyId, createdAt: NOW, updatedAt: NOW } as unknown as BlogPost; +} + +function makeRole(n: number, projectId: string): HelpWantedRole { + return { id: uuid(n), projectId, createdAt: NOW, updatedAt: NOW } as unknown as HelpWantedRole; +} + +function makeInterest(n: number, roleId: string, personId: string): HelpWantedInterestExpression { + return { id: uuid(n), roleId, personId, createdAt: NOW } as unknown as HelpWantedInterestExpression; +} + +function makeSlugHistory(n: number, entityId: string, oldSlug: string, newSlug: string): SlugHistory { + return { + id: uuid(n), + entityType: 'project', + entityId, + oldSlug, + newSlug, + changedAt: NOW, + expiresAt: FAR_FUTURE, + }; +} + +/** + * Build a state holding one record of every entity type, with ids drawn + * from `base + n`. Two calls with different bases model "before" and + * "after a re-import that minted fresh ids": every collection differs. + */ +function buildState(base: number, slugs: { project: string; buzz: string; oldSlug: string }): InMemoryState { + const state = createEmptyState(); + const project = makeProject(base + 1, slugs.project, 42); + const person = makePerson(base + 2, 'jane'); + const tag = makeTag(base + 4, 'flutter'); + const role = makeRole(base + 9, project.id); + + indexProject(state, project); + indexPerson(state, person); + indexProjectBuzz(state, makeBuzz(base + 3, project.id, slugs.buzz)); + indexTag(state, tag); + indexTagAssignment(state, makeAssignment(base + 5, tag.id, project.id)); + indexMembership(state, makeMembership(base + 6, project.id, person.id)); + indexProjectUpdate(state, makeUpdate(base + 7, project.id, 1)); + indexBlogPost(state, makeBlogPost(base + 8, `${slugs.project}-post`, 7)); + indexHelpWantedRole(state, role); + indexHelpWantedInterest(state, makeInterest(base + 10, role.id, person.id)); + indexSlugHistory(state, makeSlugHistory(base + 11, project.id, slugs.oldSlug, slugs.project)); + return state; +} + +/** "Before" state: ids in the 1xx range, project slug alpha-v1. */ +function buildLiveState(): InMemoryState { + return buildState(100, { project: 'alpha-v1', buzz: 'alpha-launch', oldSlug: 'alpha-v0' }); +} + +/** + * "After re-import" state: freshly minted ids (2xx range), renamed project + * slug, a different buzz slug, and a slug-history entry pointing at the new + * slug. Same legacy ids as the live state — that's the real-world shape. + */ +function buildFreshState(): InMemoryState { + return buildState(200, { project: 'alpha-v2', buzz: 'alpha-relaunch', oldSlug: 'alpha-v1' }); +} + +describe('swapInPlace', () => { + it('replaces every collection on the live state with the fresh contents', () => { + const live = buildLiveState(); + const fresh = buildFreshState(); + const keys = Object.keys(fresh) as (keyof InMemoryState)[]; + + // Sanity: the fixture must actually exercise every field, otherwise a + // skipped field would trivially "match". + expect(keys.length).toBeGreaterThan(0); + for (const key of keys) { + expect(fresh[key], `fresh.${key} is empty — extend the fixture`).not.toEqual(live[key]); + } + + swapInPlace(live, fresh); + + for (const key of keys) { + expect(live[key], `live.${key} was not replaced`).toEqual(fresh[key]); + } + // Also catch fields present on live but somehow absent on fresh. + expect(Object.keys(live).sort()).toEqual(keys.sort()); + }); + + it('preserves the identity of the state object and of every Map', () => { + const live = buildLiveState(); + const fresh = buildFreshState(); + const before = new Map( + (Object.keys(live) as (keyof InMemoryState)[]).map((k) => [k, live[k]]), + ); + + swapInPlace(live, fresh); + + for (const [key, map] of before) { + expect(live[key], `live.${key} Map identity changed`).toBe(map); + } + }); + + it('re-points the legacy-id, buzz-by-slug, and slug-history indices at the new records', () => { + const live = buildLiveState(); + const fresh = buildFreshState(); + const oldProjectId = uuid(101); + const newProjectId = uuid(201); + + expect(live.projectIdByLegacyId.get(42)).toBe(oldProjectId); + expect(live.buzzIdBySlug.get('alpha-launch')).toBe(uuid(103)); + expect(live.slugHistory.get(slugHistoryKey('project', 'alpha-v0'))?.newSlug).toBe('alpha-v1'); + + swapInPlace(live, fresh); + + // Legacy redirect path: legacyId → projectId → slug must resolve + // end-to-end against the new records. + expect(live.projectIdByLegacyId.get(42)).toBe(newProjectId); + expect(live.projectSlugById.get(live.projectIdByLegacyId.get(42) as string)).toBe('alpha-v2'); + + expect(live.buzzIdBySlug.get('alpha-launch')).toBeUndefined(); + expect(live.buzzIdBySlug.get('alpha-relaunch')).toBe(uuid(203)); + + expect(live.slugHistory.get(slugHistoryKey('project', 'alpha-v0'))).toBeUndefined(); + expect(live.slugHistory.get(slugHistoryKey('project', 'alpha-v1'))?.newSlug).toBe('alpha-v2'); + }); + + it('throws if a field on InMemoryState is not a Map instead of skipping it', () => { + const live = buildLiveState(); + const fresh = buildFreshState(); + (fresh as unknown as Record).someFutureIndex = new Set(['x']); + (live as unknown as Record).someFutureIndex = new Set(); + + expect(() => swapInPlace(live, fresh)).toThrow(/someFutureIndex/); + }); +}); diff --git a/apps/api/tests/saml.test.ts b/apps/api/tests/saml.test.ts index 1352ca2..c2ce141 100644 --- a/apps/api/tests/saml.test.ts +++ b/apps/api/tests/saml.test.ts @@ -26,6 +26,36 @@ import { getSamlTestKeyPair, type SamlTestKeyPair } from './helpers/saml-cert.js const JWT_KEY = 'test-jwt-signing-key-at-least-32-chars!!'; const SLACK_TEAM_HOST = 'codeforphilly.slack.com'; +/** Default SAML_ENTITY_ID per specs/api/saml.md#idp-identity-and-hosts. */ +const DEFAULT_ENTITY_ID = 'https://codeforphilly.org/api/saml/slack/metadata'; +/** Default CFP_SITE_HOST — the SSO endpoint Locations are built on it. */ +const DEFAULT_SITE_HOST = 'codeforphilly.org'; + +const MD_NS = 'urn:oasis:names:tc:SAML:2.0:metadata'; +const ASSERTION_NS = 'urn:oasis:names:tc:SAML:2.0:assertion'; + +function ssoLocations(metadataXml: string): { entityId: string | null; locations: string[] } { + const doc = new DOMParser().parseFromString(metadataXml, 'application/xml'); + const root = doc.documentElement; + const locations = Array.from(root?.getElementsByTagNameNS(MD_NS, 'SingleSignOnService') ?? []) + .map((el) => el.getAttribute('Location')) + .filter((v): v is string => typeof v === 'string'); + return { entityId: root?.getAttribute('entityID') ?? null, locations }; +} + +/** Every `` text in a decoded SAMLResponse (Response + Assertion). */ +function issuers(responseXml: string): string[] { + const doc = new DOMParser().parseFromString(responseXml, 'application/xml'); + return Array.from(doc.documentElement?.getElementsByTagNameNS(ASSERTION_NS, 'Issuer') ?? []).map( + (el) => el.textContent ?? '', + ); +} + +function decodeSamlResponse(html: string): string { + const match = /name="SAMLResponse" value="([^"]+)"/.exec(html); + expect(match).not.toBeNull(); + return Buffer.from(match![1]!, 'base64').toString('utf8'); +} async function seedPerson( repoDir: string, @@ -129,10 +159,17 @@ describe('SAML IdP — Slack', () => { const root = doc.documentElement; expect(root?.localName).toBe('EntityDescriptor'); - // entityID present - expect(root?.getAttribute('entityID')).toBe( - `https://${SLACK_TEAM_HOST}/api/saml/slack/metadata`, - ); + // entityID is the stable SAML_ENTITY_ID default — NOT built on + // SLACK_TEAM_HOST (Slack's host) and NOT on the serving host. + expect(root?.getAttribute('entityID')).toBe(DEFAULT_ENTITY_ID); + + // Both SSO bindings point at our own site host. + const { locations } = ssoLocations(res.body); + expect(locations).toHaveLength(2); + for (const loc of locations) { + expect(loc).toBe(`https://${DEFAULT_SITE_HOST}/api/saml/slack/sso`); + } + expect(res.body).not.toContain(`https://${SLACK_TEAM_HOST}/api/saml`); // IDPSSODescriptor + at least one SingleSignOnService and an X509Certificate. const idpDescriptors = root?.getElementsByTagNameNS( @@ -190,6 +227,10 @@ describe('SAML IdP — Slack', () => { const root = doc.documentElement; expect(root?.localName).toBe('Response'); + // Issuer on both the Response and the Assertion is the entity ID — the + // same value the metadata advertises as entityID. + expect(issuers(xml)).toEqual([DEFAULT_ENTITY_ID, DEFAULT_ENTITY_ID]); + // NameID is the slackSamlNameId, format persistent const nameIdEl = root?.getElementsByTagNameNS( 'urn:oasis:names:tc:SAML:2.0:assertion', @@ -319,6 +360,72 @@ describe('SAML IdP — Slack', () => { }); }); +describe('SAML IdP — entity ID vs. site host', () => { + let dataRepo: { path: string; cleanup: () => Promise }; + let privateStore: { path: string; cleanup: () => Promise }; + let keyPair: SamlTestKeyPair; + const personId = '01951a3c-0000-7000-8000-000000000002'; + const slug = 'sam'; + + beforeAll(async () => { + keyPair = await getSamlTestKeyPair(); + dataRepo = await createFullDataRepo(); + privateStore = await createPrivateStorageDir(); + await seedPerson(dataRepo.path, { id: personId, slug, slackSamlNameId: slug }); + await seedPrivateProfile(privateStore.path, { personId, email: 'sam@example.com' }); + }); + + afterAll(async () => { + await dataRepo.cleanup(); + await privateStore.cleanup(); + }); + + it('CFP_SITE_HOST moves the SSO Locations but leaves entityID alone', async () => { + const app = await buildTestApp(dataRepo.path, privateStore.path, keyPair, { + CFP_SITE_HOST: 'next.example.org', + }); + try { + const res = await app.inject({ method: 'GET', url: '/api/saml/slack/metadata' }); + expect(res.statusCode).toBe(200); + const { entityId, locations } = ssoLocations(res.body); + expect(locations).toHaveLength(2); + for (const loc of locations) { + expect(loc).toBe('https://next.example.org/api/saml/slack/sso'); + } + // The pre-cutover host does not leak into the identifier Slack stores. + expect(entityId).toBe(DEFAULT_ENTITY_ID); + } finally { + await app.close(); + } + }); + + it('SAML_ENTITY_ID overrides both the metadata entityID and the assertion Issuer', async () => { + const entityId = 'https://idp.example.org/saml/slack'; + const app = await buildTestApp(dataRepo.path, privateStore.path, keyPair, { + SAML_ENTITY_ID: entityId, + CFP_SITE_HOST: 'next.example.org', + }); + try { + const meta = await app.inject({ method: 'GET', url: '/api/saml/slack/metadata' }); + expect(meta.statusCode).toBe(200); + expect(ssoLocations(meta.body).entityId).toBe(entityId); + + const { accessToken } = await mintSessionFor(personId, 'user', JWT_KEY); + const launch = await app.inject({ + method: 'GET', + url: '/api/saml/slack/launch', + cookies: { cfp_session: accessToken }, + }); + expect(launch.statusCode).toBe(200); + expect(issuers(decodeSamlResponse(launch.body))).toEqual([entityId, entityId]); + // Slack-side values still come from SLACK_TEAM_HOST. + expect(launch.body).toContain(`action="https://${SLACK_TEAM_HOST}/sso/saml"`); + } finally { + await app.close(); + } + }); +}); + describe('SAML IdP — without configured cert/key', () => { let dataRepo: { path: string; cleanup: () => Promise }; let privateStore: { path: string; cleanup: () => Promise }; diff --git a/apps/api/tests/store.test.ts b/apps/api/tests/store.test.ts index 83b884e..49423a8 100644 --- a/apps/api/tests/store.test.ts +++ b/apps/api/tests/store.test.ts @@ -11,10 +11,17 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { openStore } from 'gitsheets'; +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; + import { PersonSchema, ProjectSchema } from '@cfp/shared/schemas'; import { FilesystemPrivateStore } from '../src/store/private/filesystem.js'; import { Store } from '../src/store/store.js'; +import { openPublicStore } from '../src/store/public.js'; import { createTestRepo } from './helpers/test-repo.js'; +import { createFullDataRepo } from './helpers/test-full-repo.js'; + +const exec = promisify(execFile); const now = '2026-05-16T00:00:00Z'; const uuid = (n: number) => `01951a3c-0000-7000-8000-${String(n).padStart(12, '0')}`; @@ -99,6 +106,61 @@ describe('public store (gitsheets)', () => { await cleanup(); } }); + + it('drops null/undefined-valued keys before writing (gitsheets 2.x marshal contract)', async () => { + // gitsheets 2.x (Rust core) throws when asked to marshal a null- or + // undefined-valued field to TOML; 1.4.1 silently dropped such keys. Our + // Zod schemas use `.nullable().optional()` and write services normalize + // cleared fields to `?? null`, so openPublicStore's validator wrapper must + // strip those keys — keeping the on-disk form byte-identical to 1.4.1 + // (an absent optional field is simply an absent TOML key). See + // apps/api/src/store/public.ts → stripNullish / asValidator. + const repo = await createFullDataRepo(); + try { + const { store } = await openPublicStore(repo.path); + + await store.transact( + { message: 'test: person with nullish fields', author: { name: 'test', email: 'test@cfp.test' } }, + async (tx) => { + await tx.people.upsert( + PersonSchema.parse({ + id: uuid(70), + slug: 'nullish-person', + fullName: 'Nullish Person', + accountLevel: 'user', + legacyId: 31618, // integer w/o underscore (2.x re-baseline) + bio: null, // explicit null — must be dropped, not written + avatarKey: null, + deletedAt: null, + createdAt: now, + updatedAt: now, + }), + ); + }, + ); + + const { stdout: toml } = await exec( + 'git', + ['show', 'HEAD:people/nullish-person.toml'], + { cwd: repo.path }, + ); + + // Present fields survive. + expect(toml).toContain('slug = "nullish-person"'); + expect(toml).toContain('fullName = "Nullish Person"'); + // Integer re-baseline: no underscore separator under the Rust core. + expect(toml).toContain('legacyId = 31618'); + // Null-valued keys are absent from disk (never serialized as `null`). + expect(toml).not.toMatch(/^bio\s*=/m); + expect(toml).not.toMatch(/^avatarKey\s*=/m); + expect(toml).not.toMatch(/^deletedAt\s*=/m); + // No field is assigned a bare `null` value. (Substring 'null' on its own + // would false-match the fixture's "Nullish Person" / "nullish-person".) + expect(toml).not.toMatch(/=\s*null\b/); + } finally { + await repo.cleanup(); + } + }); }); // ------------------------------------------------------------------------- diff --git a/apps/web/src/components/ActivityCard.tsx b/apps/web/src/components/ActivityCard.tsx index 409c45f..3d65587 100644 --- a/apps/web/src/components/ActivityCard.tsx +++ b/apps/web/src/components/ActivityCard.tsx @@ -33,9 +33,13 @@ function UpdateCard({ update }: { update: ProjectUpdateResponse }) { Update #{update.number} - + + {update.author && ( @@ -69,9 +73,9 @@ function BuzzCard({ buzz }: { buzz: ProjectBuzzResponse }) { {buzz.project.title} · Buzz · - + + @@ -89,6 +93,7 @@ function BuzzCard({ buzz }: { buzz: ProjectBuzzResponse }) {

{buzz.headline} + (opens in new tab)

{hostname}

diff --git a/apps/web/src/components/AppFooter.tsx b/apps/web/src/components/AppFooter.tsx index 620b03f..b381912 100644 --- a/apps/web/src/components/AppFooter.tsx +++ b/apps/web/src/components/AppFooter.tsx @@ -182,7 +182,7 @@ export function AppFooter() { Open source — view this site on GitHub + (opens in new tab) diff --git a/apps/web/src/components/AppHeader.tsx b/apps/web/src/components/AppHeader.tsx index c0d6516..2a303e3 100644 --- a/apps/web/src/components/AppHeader.tsx +++ b/apps/web/src/components/AppHeader.tsx @@ -1,5 +1,5 @@ import { useState } from 'react'; -import { Link, NavLink } from 'react-router'; +import { Link, NavLink, useLocation } from 'react-router'; import { Button } from '@/components/ui/button'; import { DropdownMenu, @@ -8,11 +8,21 @@ import { DropdownMenuSeparator, DropdownMenuTrigger, } from '@/components/ui/dropdown-menu'; -import { Sheet, SheetContent, SheetTrigger } from '@/components/ui/sheet'; +import { + Sheet, + SheetContent, + SheetDescription, + SheetHeader, + SheetTitle, + SheetTrigger, +} from '@/components/ui/sheet'; import { Separator } from '@/components/ui/separator'; import { SearchBox } from '@/components/SearchBox'; +import { GitHubIcon } from '@/components/icons/GitHubIcon'; import { useAuth } from '@/hooks/useAuth'; +const GITHUB_URL = 'https://github.com/CodeForPhilly'; + function ChevronDownIcon() { return ( ); } @@ -143,12 +153,8 @@ function AboutDropdown() { return ( - @@ -176,13 +182,41 @@ function AboutDropdown() { ); } +// `block` so each link fills its row: inside the sheet's
  • s an inline +// anchor would shrink the tap target to the width of its text. const navLinkClass = ({ isActive }: { isActive: boolean }) => - `text-sm font-medium transition-colors hover:text-primary ${ + `block text-sm font-medium transition-colors hover:text-primary ${ isActive ? 'text-primary' : 'text-muted-foreground' }`; +function GitHubLink() { + return ( + // Desktop-only: between md and lg the header has no room for it (the + // utility cluster would push "Help Wanted" onto two lines); the mobile + // sheet carries its own GitHub row. + + ); +} + export function AppHeader() { - const [mobileOpen, setMobileOpen] = useState(false); + const location = useLocation(); + // The sheet is open only for the location it was opened at, so any + // client-side navigation — a NavLink or Enter in the inline search — + // closes it without per-item onClick closers. Derived during render + // rather than synced in an effect (react-hooks/set-state-in-effect). + const [openedAtKey, setOpenedAtKey] = useState(null); + const mobileOpen = openedAtKey === location.key; + const setMobileOpen = (open: boolean) => + setOpenedAtKey(open ? location.key : null); return (
    @@ -201,135 +235,173 @@ export function AppHeader() { /> - {/* Desktop nav */} + {/* Desktop content cluster. The parent gap is the only source of + spacing between children — no per-child margins. */} + {/* A nav is a list of destinations — the
      /
    • is what tells a + screen reader how many there are and where you are in them. The + flex/gap spacing moves to the
        ; the
      • s contribute none. */} - {/* Desktop: search + auth */} -
        + {/* Desktop utility cluster: GitHub, search, auth, then the Volunteer + CTA pinned rightmost (specs/behaviors/app-shell.md). */} +
        + + {/* Mobile: auth + hamburger */}
        + {/* No aria-expanded here — Radix's Dialog.Trigger supplies it. */} - + + {/* SheetHeader/SheetTitle carry the panel's own padding and give + the underlying Radix dialog its accessible name; the + visually-hidden description satisfies aria-describedby. */} + + Menu + + Site navigation + + + {/* min-h-0 + overflow-y-auto so the list stays reachable on + short viewports instead of overflowing the panel. */} + {/* Three lists with the separators and the group heading + between them, rather than one list interrupted by + non-list children. The nav keeps the flex column so the + gap-2 rhythm between groups is unchanged. */} - +
        + +
        diff --git a/apps/web/src/components/AppShell.tsx b/apps/web/src/components/AppShell.tsx index a7c6bd6..cb73a1c 100644 --- a/apps/web/src/components/AppShell.tsx +++ b/apps/web/src/components/AppShell.tsx @@ -11,7 +11,7 @@ export function AppShell() { {/* Skip to main content — must be the first focusable element */} Skip to main content diff --git a/apps/web/src/components/Breadcrumbs.tsx b/apps/web/src/components/Breadcrumbs.tsx index 31e5468..fd575a5 100644 --- a/apps/web/src/components/Breadcrumbs.tsx +++ b/apps/web/src/components/Breadcrumbs.tsx @@ -21,7 +21,7 @@ export function Breadcrumbs({ items }: BreadcrumbsProps) { {items.map((item, index) => { const isLast = index === items.length - 1; return ( -
      • +
      • {index > 0 && (