Repository navigation
release: v2026.10.11 - #61
Merged
Merged
Conversation
…env 18 (#33) ## Summary - **Security:** 1 high → **0**. `brace-expansion` DoS (GHSA-q2hr-2g5m-vwhr, GHSA-qhr7-859c-m2p7, GHSA-6j4f-fj2g-mc7p), reached via `eslint` and `mjml` → `minimatch`. Cleared by in-range updates alone. Low real exposure: every path matches config-file globs, never request input. - **In-range:** `next` 16.3.7, `better-auth` 1.7.6, `zod` 4.6.5, `vitest` 5.0.2, `jsdom` 30.1.1, plus minor bumps. - **Major adopted:** `dotenv` 17 → 18. Used only by the MP codegen scripts; both verified with `--help` (loads `.env.local`, exit 0). - **Holds re-checked, all still blocked:** TypeScript 7, ESLint 10, GrapesJS 0.23, `@types/node` 25+. - **Removed:** none. Every package without a direct import is reached implicitly. OSV.dev sweep: 472 packages, 1 hit (the known `grapesjs` false positive). Full record: `.claude/packages/2026-09-30.md` ## Verification | | Baseline | After | |---|---|---| | Tests | 1563 / 116 files | 1563 / 116 files | | Lint | clean | clean | | Type errors | 0 | 0 | | `next build` | — | passes | | `coverage/coverage-final.json` | — | present | Commits are separable: in-range / dotenv 18 / docs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Adds 14 TODOs from auditing the five MPNext port playbooks: 9 ordered remediation steps (one PR each), 2 ops actions (rotate \`BETTER_AUTH_SECRET\`, set \`MP_SECURITY_ROLES=*\`), 3 held items (CSP for customer clones, stored-proc allowlist, GitHub security settings). Docs only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…bypass (F3b), harden /sign-in/social (#35) ## What / why Ports two unported findings from upstream MPNext's 2026-09-25 sign-in advisory, plus the `/sign-in/social` boundary from the 2026-09-28 security review. Closes the critical TODO `.claude/TODO/2026-09-30-step1-close-live-auth-holes-f12-f3b.md`. **F12 — account takeover via id_token sign-in.** `discoveryUrl` is set, so better-auth 1.7's `idToken` branch of `POST /sign-in/social` was live. It calls `getUserInfo` with a caller-supplied access token, and nothing binds `id_token.sub` to userinfo `sub`. An attacker's own id_token plus a victim's MP access token gave a session as the victim. (MP's discovery doc advertises implicit/hybrid flows, so this is Low–Medium.) - `hooks.before: refuseIdTokenSignIn` → 404 `ID_TOKEN_SIGN_IN_DISABLED` whenever the body has an `idToken` key. It checks that the key is present, not that it is truthy. It covers HTTP and in-process `auth.api` calls. - `/link-social` added to `disabledAuthPaths`. - `requireIdTokenVerification: true`: a discovery doc missing `issuer`/`jwks_uri` now skips the provider instead of leaving the id_token unverified. **Route boundary** (`api/auth/[...all]/route.ts`). `/sign-in/social` returns a plain 404, and Better Auth is never called, unless all of these hold: - the raw Content-Type is `application/json` with no comma - the body is ≤ 4096 bytes, both as declared and as actually streamed - the keys are a subset of `provider`/`callbackURL` - `provider === "ministryplatform"` - `callbackURL` is ≤ 2048 chars Every `/api/auth` response, 404s included, now gets `Cache-Control: no-store`. **F3b — open redirect bypass.** `/signin?callbackUrl=/%09/example.com` sent a signed-in user off-site. The sanitizer now mirrors better-auth's `isSafeRelativeURL`: it refuses control chars, any backslash, `%2F`/`%5C` in the path, and anything that resolves off-origin, and it returns the raw value. `?error=` is now only a lookup key into a fixed message map. Free text is never rendered or logged. ## Tests - **New `src/auth.id-token-sign-in.test.ts`.** Runs the real `auth` instance against a mock MP OIDC provider with really-signed id_tokens. Any other `fetch` throws. - **`route.test.ts`.** Runs the body filter with Better Auth mocked out, and asserts the handler is never called on any refusal. Also covers no-store on every path. - **`page.test.tsx`.** F3b cases both as strings and through a real query string, on both sinks (signed-in redirect and signed-out `callbackURL`). - All 17 mutation checks turned tests red and were reverted. The full list is in the TODO's Resolution section. It includes deleting the hook, dropping the comma check, removing `CONTROL_CHARS`, and returning the normalised URL. - Local: `lint`, `typecheck`, `test:coverage` (1673 tests; 98.79% statements / 99.68% lines) and `build` all pass. - **Live check.** Ran `next start` against non-prod `mpi.ministryplatform.com`: - attack body, comma Content-Type and a 3000-char `callbackURL` all → 404 no-store - a normal sign-in → 200 with the MP authorize URL, so `requireIdTokenVerification` is compatible with MP's discovery doc ## Deliberately deferred - **Step 4:** the `sub` binding in `getUserInfo`, and removing `discoveryUrl`. A test named "KNOWN GAP until Step 4" shows that with the hook removed, the attack still succeeds. That test must be flipped when the binding lands. - **Step 3:** the client-IP / trusted-proxy rate-limit config. ## Deploy notes - No env or config changes. `providerId` stays `ministryplatform`, so the redirect URI is unchanged. - If MP's discovery doc ever loses `issuer`/`jwks_uri`, sign-in now fails closed (`PROVIDER_NOT_FOUND`) instead of skipping id_token verification. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ment, page-metadata injection, F11, page self-gating (#36) > ##⚠️ Deploy note: set `MP_SECURITY_ROLES=*` in every environment BEFORE this deploys > `MP_SECURITY_ROLES` now **fails closed**. If it's unset, blank or `","`, **every user is locked out** of every tool (they get redirected to `/no-access`). Set `MP_SECURITY_ROLES=*` in Vercel **Production, Preview and Development**, and in local `.env.local` files. `*` means any MP security role. A user with no role is still refused. See `.claude/TODO/2026-09-30-ops-set-mp-security-roles-env.md`. ## What / why Step 2 of the 2026-09-30 playbook remediation (`.claude/TODO/2026-09-30-step2-authorization-and-input-validation.md`). This PR has one commit per item: 1. **Fail-closed roles** (`authorizationService.ts`): unset, blank or `","` now means nobody gets in (denial reason `roles_not_configured`). `*` means any role, but only when it is the whole value. A comma list means only those roles. Before this change, a blank value silently meant "any role". `.env.example` now ships `*` with a comment explaining the control. The setup wizard treats the variable as required, with `*` as the default. CLAUDE.md and the security reference are updated. 2. **Group-wizard mass assignment** (fork-only): the actions spread the unparsed client payload into the Groups POST/PUT. That let any role holder write *any* Groups column, and a payload `Group_ID` could retarget an update at a different record. Now: - the action parses the payload with `groupWizardSchema` on the server; - the service keeps only an explicit `GROUP_WRITABLE_FIELDS` allowlist; - `groupId` is validated and `Group_ID` is written last. 3. **`resolveContactIdFromPage` query injection**: - A new `validateFkPath` check applies a strict regex to each `_TABLE` segment and to the final column. - The service validates every identifier before any MP call. - The action now takes `{ pageId, recordId }` and looks up the page metadata on the server, instead of accepting table and column names from the caller. 4. **F11**: `getMpTimezone` now requires a session. The file explains in writing why it is a carve-out. 5. **Pages gate themselves**: all six `tools/*/page.tsx` await `requireToolAccess()` before doing anything else. A refusal redirects to `/no-access`; infrastructure errors still throw. The layout comment wrongly claimed its redirect stopped the page from running (it doesn't in Next 16), so that is corrected. A structural test pins the gate on every page. 6. **Filter / error hygiene**: - `escapeFilterString` now escapes `[`, refuses non-strings and control characters, caps input at 100 characters, and maps quote look-alikes to `_`. - `listRoles` uses `escapeFilterString`. - Validators name the field and never echo the value. - The `domainTimezoneService` messages are ported from upstream, along with its two injection tests. ### Derived group-wizard writable allowlist (40 fields) I derived this list by probing the five editing step components (one `FormField name=` each). It equals the union of `STEP_FIELDS` and the schema keys, and a test pins that: `Group_Name, Group_Type_ID, Description, Start_Date, End_Date, Reason_Ended, Congregation_ID, Ministry_ID, Primary_Contact, Parent_Group, Priority_ID, Meeting_Day_ID, Meeting_Time, Meeting_Frequency_ID, Meeting_Duration_ID, Meets_Online, Default_Meeting_Room, Offsite_Meeting_Address, Target_Size, Life_Stage_ID, Group_Focus_ID, Required_Book, SMS_Number, Group_Is_Full, Available_Online, Available_On_App, Enable_Discussion, Send_Attendance_Notification, Send_Service_Notification, Create_Next_Meeting, Secure_Check-in, Suppress_Nametag, Suppress_Care_Note, On_Classroom_Manager, Promote_to_Group, Age_in_Months_to_Promote, Promote_Weekly, Promote_Participants_Only, Promotion_Date, Descended_From` `Group_ID`, `Domain_ID`, `_`-prefixed system columns and everything else are dropped. ### Behaviour changes worth knowing - An `updateGroup` payload that leaves out a date field no longer sends it as `null`. - Dotted `Participant_ID_Table.Contact_ID` paths from `api_Tools_GetPageData` now resolve. The old case-sensitive `_TABLE` check rejected them. - A search term longer than 100 characters now throws `Invalid search value: longer than 100 characters`. ## Tests - Local gates all pass: `npm run lint`, `npm run typecheck`, `npm run test:coverage` (118 files, 1,787 tests; 98.82% statements / 99.68% lines; thresholds 97/98 hold), and `npm run build`. - Adversarial tests cover: - `MP_SECURITY_ROLES` unset, blank, whitespace, `","` and `" , , "`, plus `*` and `*`-inside-a-list; - smuggled `Group_ID`, unknown columns, `$userId` and prototype keys; - `groupId` of 0, -1, 1.5, NaN or a string; - create and update each write nothing when the gate throws; - nine hostile FK paths and several hostile table names; - `getMpTimezone` with a missing session; - gate-before-parse on all six pages; - validator messages never echo the value. ## Mutation checks (break → red → revert → green) | Mutation | Red | |---|---| | role policy: blank → `any` | 8 | | allowlist pick → spread whole payload | 2 | | `Group_ID` spread first | 1 | | `validatePositiveInt(groupId)` removed | 5 | | action passes raw (unparsed) payload | 1 | | old `_TABLE` passthrough in `resolveContactIdFromPage` | all hostile-path tests | | `getMpTimezone` session check removed | 3 | | group-wizard page gate removed | 2 | | `toMpSqlDatetime` echoes the value | 1 | ## Deferred - `ToolService.resolveContactIds` (dev panel) still takes table and column names from its caller. They are identifier-validated, and that path is dev-only. - The `(web)/layout.tsx` session redirect has the same segment limitation as the tools layout. Tool pages are now covered by `requireToolAccess`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ot guards, secrets hygiene, client-IP header (#37) Security remediation **Step 3** — `.claude/TODO/2026-09-30-step3-session-lifetime-and-secrets-hygiene.md` (playbook `port-security-review-2026-09-28.md` Phases 2, 3 and the client-IP part of 6). ## What and why ### Sessions (P3), in `src/lib/auth.ts` Stateless better-auth 1.7.6 quietly defaults `cookieCache.refreshCache` to **true**. Because of that, a copied or forged `session_data` cookie kept re-signing itself for **up to 7 days**, sign-out or not. The cookie was also a readable JWT, and the user's MP access and refresh tokens (full `dataplatform/scopes/all`) sat in an `account_data` cookie. - `expiresIn` 12 h, `disableSessionRefresh: true`, `strategy: "jwe"`, `refreshCache: false` (set explicitly). A cookie with no live in-memory row behind it now dies within 1 h. - `storeAccountCookie: false`. `databaseHooks` null the access and refresh tokens in the in-memory account row. The `idToken` is kept for Step 4's `id_token_hint`. - `offline_access` is no longer requested. - `customSession` → `enrichSession`: it keeps `firstName`/`lastName` (the fork's documented behaviour) and withholds `token`, `ipAddress` and `userAgent`. ### Boot guards (P2) - `assertAuthEnvironment` refuses to start when the secret is missing, is better-auth's public default, or is **shorter than 32 characters**. It also refuses when `BETTER_AUTH_SECRETS` is set or when `TEST` is set in production. It runs everywhere except Vitest, and that includes `next build`. - `advanced.disableOriginCheck: false` is pinned. - New `src/lib/env.ts` (`getMpBaseUrl`, `getAuthBaseUrl`) validates the two URLs (https, no credentials, query or fragment, app URL origin-only, `NEXTAUTH_URL` fallback kept). `auth.ts`, the MP client, client-credentials and the sign-out action now go through it. ### Secrets hygiene - `.gitignore` now covers `.env*` (except `.env.example`), `.vercel`, `.claude/settings.local.json` and `.claude/worktrees/`. - `.claude/settings.local.json` is untracked. It granted `Bash(sed:*)` and `rm -rf .claude/*` to anyone who cloned. - New `.githooks/pre-commit` refuses staged env files. It is installed by a `prepare` script and adds no dependency. - New `SECURITY.md`, which includes what downstream clones should do: port `disabledPaths`, **rotate their own `BETTER_AUTH_SECRET`**, and review `dp_Audit_Log`. ### Client IP (P6): recommendation `AUTH_IP_ADDRESS_HEADERS` and `AUTH_TRUSTED_PROXIES` feed `advanced.ipAddress`. Invalid entries refuse startup. **Default:** `x-vercel-forwarded-for`, then `x-real-ip`, **applied only when `VERCEL` is set**. - Checked against <https://vercel.com/docs/headers/request-headers> on 2026-09-30. Vercel overwrites `x-forwarded-for` and "does not forward external IPs … to prevent IP spoofing". `x-vercel-forwarded-for` and `x-real-ip` are "identical to" it. - `x-vercel-forwarded-for` comes first because it survives a proxy placed in front of Vercel, which may overwrite `x-forwarded-for`. - Off Vercel, those same headers are just values any client can send. Defaulting to them there would let a client dodge the limit or lock out someone else. So an unconfigured non-Vercel clone keeps better-auth's default (single-value `x-forwarded-for`). - This is a slight refinement of "always default to them". For this Vercel deployment the behaviour is identical. - `.env.example` lists what other edges should set: Cloudflare `cf-connecting-ip`, Azure Front Door `x-azure-clientip`, App Service `x-client-ip`, and for nginx the header it *sets*. ## Tests New suites drive the real `auth` instance through a mock MP OIDC server with RS256-signed id_tokens (`src/test-utils/mock-oidc.ts`, excluded from coverage). Every unserved fetch throws. - `auth.session-lifetime.test.ts`: clock walks. The 12 h ceiling holds on both paths. A cookie copied before sign-out dies within 1 h, and so does one replayed at a serverless instance that never saw the sign-in. A negative control with the pre-fix config survives ~7 days. - `auth.session-config.test.ts`: the only cookies are `session_token` + `session_data`, and no `account_data`. **`session_data` is decrypted with `symmetricDecodeJWT`** and holds no token material. The response withholds its fields over HTTP and via `auth.api.getSession`. - `auth.user-oauth-tokens.test.ts`: no `offline_access`; the in-memory row has null access and refresh tokens; the idToken is kept; sign-out on the same instance returns MP endsession with `id_token_hint`. - `auth.secret-guard.test.ts`: import-time refusals, including with `VITEST` cleared. - `lib/env.test.ts` and `auth.ip-address.test.ts` (parsing, per-edge resolution, rate-limit buckets). **Mutation checks** (break it → red → revert). Each one turned the suite red: - `refreshCache: false` removed - `storeAccountCookie: true` - token stripping removed - secret-length guard weakened - module-level guard removed - `jwe`→`jwt` - `expiresIn` removed - `offline_access` re-added - Vercel default dropped - withheld fields kept **Local gates:** lint, typecheck and `test:coverage` all pass (1896 tests; 98.87% statements, 99.72% lines). `npm run build` passes, and its `_INSTALL`/`next-env.d.ts` side effects were reverted. ## Deploy note - **Every user is signed out once**, because the cookie strategy and session config change. - **Rotate `BETTER_AUTH_SECRET` at this deploy** in Production, Preview and Development on Vercel, and in every `.env.local`: `openssl rand -base64 32`. It must be **≥ 32 characters**, or the app (and the Vercel build) refuses to start. Ops TODO `2026-09-30-ops-rotate-better-auth-secret.md` stays open for this and the audit-log review. - `BETTER_AUTH_URL` must be an https origin with no path (loopback http is fine). `MINISTRY_PLATFORM_BASE_URL` must be https. `BETTER_AUTH_SECRETS` and a production `TEST` must not be set. - There are no new *required* variables. `AUTH_IP_ADDRESS_HEADERS` and `AUTH_TRUSTED_PROXIES` are optional, and the default is right for Vercel. - Trade-off: on serverless, after 1 h a request can land on an instance without the in-memory row. That user goes back through MP sign-in, which is also an hourly check against MP. ## Owner follow-ups (not doable from here) - GitHub private vulnerability reporting, secret scanning, push protection and Dependabot security updates are all **disabled** on this public repo. `SECURITY.md` points reporters at private reporting (held TODO `2026-09-30-held-github-repo-security-settings.md`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ted OIDC client, sign-out hint (#38) ## What Security **Step 4** (`.claude/TODO/2026-09-30-step4-oidc-lazy-discovery-and-sub-binding.md`), ported from MPNext `port-oidc-lazy-discovery.md` (Phases 2–7) and `port-security-review-2026-09-28.md` Phases 4–5. The repo was in **state B**: `discoveryUrl` set, no rebuild facade. - **Fixes the boot-time discovery outage (MPNext #101).** The provider now has explicit `authorizationUrl` / `tokenUrl` / `endSessionEndpoint` and no `discoveryUrl`. Building the auth instance makes **no MP call**, so `next build`, cold starts and session checks never contact MP, and the provider always registers. - **The app still verifies the id_token itself**, in `getUserInfo`. RS256 is pinned, and `iss` and `aud` (the OIDC client id) are checked. The issuer and `jwks_uri` load lazily at the first callback (`lazyIdTokenVerifier`): 5 s timeout, `redirect:"error"`, single-flight, cached on success, **never cached on failure**, and it fails closed on a partial document. - **`getUserInfo` rewritten.** Steps, in order: refuse a missing id_token → verify → check `sub`, `exp` and `azp` → fetch userinfo (10 s timeout, `redirect:"error"`, never throws) → `extractUserGuid` → **case-insensitive `sub` binding** (`auth.userinfo.sub_mismatch`). This is F12's third layer. Kept unchanged: the synthetic email, `mpEmail`, `emailVerified` taken from the claim, `accountLinking:false`, `disabledPaths` and the `refuseIdTokenSignIn` hook. - **Removed:** `requireIdTokenVerification`, which genericOAuth throws for when there is no discovery, and `disableIdTokenNonceBinding`, which is now a no-op. `accountSubject` uses the `typeof` form. `providerId` stays `"ministryplatform"` (it is the registered redirect URI). - **Dedicated OIDC client (owner decision).** Sign-in now uses `MP_OIDC_CLIENT_ID` / `MP_OIDC_CLIENT_SECRET` (`getOidcClient()` in `src/lib/env.ts`) as both the OAuth client and the id_token audience. If they are unset it falls back to `MINISTRY_PLATFORM_CLIENT_ID/SECRET` and logs a one-time `auth.oidc.shared_client` warning per process (not during `next build`). Setting only one of the pair refuses to start. A follow-up TODO makes them required. - **One auth instance per process** (`sharedInstance`). Next loads `auth.ts` once per bundle layer, so without it the sign-out action never saw the callback's in-memory rows. - **Sign-out.** `auth.api.signOut({ body: { disableRedirect: true } })` runs first. `id_token_hint` is taken only from an MP-origin URL. The URL is rebuilt with the exact `post_logout_redirect_uri` and **always** includes `client_id`. `user-menu.tsx` now uses try/catch with `unstable_rethrow` first, then shows an alert; before, it swallowed failures. - **`/auth-error`.** `unable_to_get_user_info` now tells the user to retry, since that is what an MP blip at the callback looks like. `oauth_provider_not_found` now reads as a configuration fault. `nonce_binding_missing` is removed. ## Discovery document (mpi, read-only GET, 2026-09-30) | key | value | |---|---| | issuer | `https://mpi.ministryplatform.com/ministryplatformapi/oauth` | | authorize / token / userinfo / endsession | `…/oauth/connect/{authorize,token,userinfo,endsession}` (match the hardcoded paths) | | `jwks_uri` | `…/oauth/.well-known/jwks` (**not** hardcoded) | | `id_token_signing_alg_values_supported` | `["RS256"]` | | `authorization_response_iss_parameter_supported` | absent, so nothing is lost by skipping better-auth's RFC 9207 check | | `response_types_supported` | includes implicit/hybrid | | `code_challenge_methods_supported` | `["plain","S256"]` (still not honoured; PKCE stays off) | ## Log events - **Added:** `auth.oidc.discovery_failed` (`http_status`/`request_failed`/`invalid_json`/`invalid_document`+`field`), `auth.userinfo.id_token_unverified` (`verifier_unavailable` | `verification_failed` + jose `code`/`claim`/`errName`), `auth.userinfo.id_token_claims_invalid`, `auth.userinfo.sub_mismatch`, `auth.oidc.shared_client` (warn). - **Changed:** `auth.userinfo.fetch_failed` and `auth.userinfo.invalid_sub` are now single JSON lines. - All events carry identifiers only. **What an MP outage looks like now:** `/signin` always starts. A callback during the outage lands on `/auth-error?error=unable_to_get_user_info`, and the next sign-in recovers on the same process. `PROVIDER_NOT_FOUND` now means misconfiguration only. ## Tests 1986 tests pass (127 files). Coverage is 98.9 % statements and 99.72 % lines; the thresholds are unchanged. - New `src/auth.oidc-discovery.test.ts` (28 tests): - MP `reject`/`http_500`/`hang` at boot → sign-in starts and sessions answer with **0** MP calls - discovery or JWKS failure at the callback → refused, then the same instance recovers - discovery timeout and `redirect:"error"`; a hung discovery is given up - single-flight across 5 concurrent callbacks - a partial document fails closed - nonce negative control with `discoveryUrl` re-added - `lazyIdTokenVerifier` unit tests - New `src/auth.user-info.test.ts` (43 tests), using genuinely signed tokens: - alg confusion: HS256 keyed with either client secret, and `alg:none` → `ERR_JOSE_ALG_NOT_ALLOWED` - foreign key, wrong iss/aud, the service-account id as aud, expired or non-numeric exp - missing_exp, azp - sub binding: match, case-only difference, mismatch, missing/empty/non-string sub, missing id_token - userinfo robustness, emailVerified, mpEmail - New `src/auth.shared-instance.test.ts`, with a negative control. - `src/auth.id-token-sign-in.test.ts`: the **"KNOWN GAP until Step 4" test now expects refusal via the sub binding** (hook removed and `discoveryUrl` re-added: 401, `reason:"mismatch"`). A matched pair still signs in as a control. With only the hook removed, better-auth returns `404 ID_TOKEN_NOT_SUPPORTED`. - `src/auth.test.ts`: the old discovery/nonce/`requireIdTokenVerification` pins are replaced with explicit-endpoint pins and a check that the live provider has `idToken` undefined and `requiresIdTokenNonce:false`. The getUserInfo tests moved to the node-env suite. - `src/test-utils/mock-oidc.ts` gains per-endpoint failure modes, `omitDiscovery`/`claims`/`foreignKey`/`userinfo` state, and `signIdToken`. ## Mutation checks (each turned the suite red, then reverted) - `discoveryUrl` restored, run with `--testTimeout=3000`: 14/28 discovery tests failed (35 across `src/auth*`), and the hang cases failed by timeout. - The sub comparison forced true: the F12 substitution test and the user-info mismatch test failed. - Each of these also failed at least one test: - removing the algorithms pin - removing userinfo or discovery `redirect:"error"` - removing the missing_exp or azp check - caching a failure - setting the audience to the service account - accepting a missing id_token - removing the shared-client warning - removing the `sharedInstance` cache - removing `endSessionEndpoint` - removing `accountSubject` ## Verification - `npm run lint`, `npm run typecheck`, `npm run test:coverage` and `npm run build` all pass. - `MINISTRY_PLATFORM_BASE_URL=https://mp.invalid npm run build` logs no discovery error. - `next start` replay with process-env overrides (`.env.local` untouched), against three MP targets: unresolvable, **hanging** (a TCP listener that never answers) and mpi. - `/api/auth/get-session`, `/sign-in/social` (200, authorize URL with `client_id` = OIDC client, no nonce/PKCE) and `/signin` all answer in ≤ 35 ms. - The hanging MP received 0 connections. - Against mpi, `lazyIdTokenVerifier` loads the discovery doc and JWKS and resolves an RS256 key (read-only GETs). - **Not done:** the real-MP sign-in smoke test. It needs a person and a real login: sign in, check `userGuid`, confirm no discovery/unverified events, and confirm sign-out ends the MP session. - `jose ^6.2.12` is now a direct dependency. The lockfile root lists it (a one-line diff); this repo has no lockfile script to fix. ## Deploy note - **New optional env vars `MP_OIDC_CLIENT_ID` / `MP_OIDC_CLIENT_SECRET`.** Nothing breaks without them: sign-in falls back to the service-account client and logs `auth.oidc.shared_client`. - **Owner action:** register a dedicated MP OIDC client for this app in each environment, then set both vars in Vercel (Preview and Production) and `.env.local`. Register on it: - redirect URI `<BETTER_AUTH_URL>/api/auth/callback/ministryplatform` - post-logout redirect URI exactly `<BETTER_AUTH_URL>` - Follow-up TODO: `.claude/TODO/2026-09-30-require-dedicated-oidc-client.md`. - **Sign-out limitation:** the `id_token_hint` exists only on the process that handled the callback. On another serverless instance MP gets `client_id` + `post_logout_redirect_uri` and may show its "log out?" prompt once. - Any alerts should key on the new events above. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
## What / why
Step 5 of the 2026-09-30 playbook-audit remediation
(`port-security-review-2026-09-28.md` Phases 8–9). Every MP request goes
out with the admin-level service-account bearer
(`dataplatform/scopes/all`), so the client and provider services are
hardened:
**Phase 8 — HTTP client** (`utils/http-client.ts`, `client.ts`,
`auth/client-credentials.ts`, `utils/logger.ts`)
- `AbortSignal.timeout` on every MP fetch (20 s JSON, 60 s multipart, 10
s token; fresh per attempt) and `redirect: "error"` (a 307/308 would
re-send member data or `client_secret`).
- Path guard in `buildUrl`: refuses `..`, `?`/`#`/`\`,
`%2e`/`%2f`/`%5c`, control chars, no leading `/`, and any resolved URL
outside the API root. Query **keys** now encoded (`$`/`@` literal).
- `readJsonResponse`: 204 / `Content-Length: 0` → `undefined`; JSON
content-type required; a malformed body never leaks via `SyntaxError`.
- Token response validated (non-empty `access_token`, `token_type`
bearer).
- Default and dev pipelines are one `TokenPipeline` each: single-flight
refresh, 5–30 s jittered negative cache after a failure, 401 →
invalidate-if-current → refresh → **one** retry.
- Logs: `mp.request.failed` / `mp.token.*` events with method,
GUID-redacted path, status or `errorName`.
**Phase 9 — provider services**
- `services/guards.ts` (`sanitizeIdentifier`,
`sanitizeRecordId`/`sanitizeUniqueId` reusing
`validatePositiveInt`/`validateGuid`, `tableEndpoint`).
Table/File/Procedure services validate every path segment before any
token work; `deleteTableRecords` refuses non-array `ids`.
- `/files/{uniqueId}` download: own timeout + `redirect: "error"`;
message says `{uniqueId}`.
- `$ignorePermissions` removed from `GlobalFilterParams`;
`getGlobalFilters` forwards only a validated `$userId`.
- ~25 `logger.error(msg, error)` sites → `logger.error(event, { ids,
error: errorName(e) })` (CLAUDE.md rule 14).
**Out of scope (owner decision):** deny-all stored-procedure allowlist —
still held in
`.claude/TODO/2026-09-30-held-stored-procedure-allowlist.md`.
Public `MPHelper` / `MinistryPlatformClient` APIs are unchanged; no
`src/services/` changes needed.
## Behavior changes callers will notice
- Table/procedure names that aren't plain identifiers, and
non-positive-integer record/file IDs, now throw (`Invalid table name`,
`Expected positive integer for record ID`, ...) before any request.
- A 2xx without a JSON content-type throws `... unexpected
content-type`.
- After a failed token refresh, calls fail fast (`MP access token
unavailable: recent refresh failed`) for 5–30 s.
- A 401 the caller sees has already been retried once with a fresh
token.
## Tests
- Ported upstream `http-client`, `client-credentials`, `guards` tests;
added client single-flight / negative-cache / 401 / secret-handling /
dev-isolation tests and table/file/domain/procedure guard + log-shape
tests. Provider folder at 100% coverage.
- Local gates: `npm run lint` ✓, `npm run typecheck` ✓, `npm run
test:coverage` ✓ (2178 tests; 98.96% stmts / 99.73% lines), `npm run
build` ✓.
- Live read-only check against non-prod `mpi.ministryplatform.com`:
token validation, `/domain`, `/tables/Congregations`, `/domain/filters`
all succeed; `../procs` table name refused.
## Mutation checks (break → red → revert)
| Mutation | Failing tests |
|---|---|
| Remove `AbortSignal.timeout` (http-client) | 9 |
| Remove token timeout | 2 |
| `redirect: 'follow'` (http-client) | 8 |
| `redirect: 'follow'` (token) | 2 |
| Disable `assertSafeEndpoint` | 26 |
| Disable 401 retry | 9 |
| Disable token-response validation | 11 |
| Disable negative cache | 3 |
| Drop table-name guard | 9 |
| Drop `recordId` guard (copy methods) | 9 |
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
## Summary The template editor was a demo, not a supported tool. This removes it entirely. - **Code:** `src/components/template-editor/`, `src/app/(web)/tools/templateeditor/`, `src/styles/grapesjs-overrides.css`, the home-page card, and the editor entries in `page.test.tsx` / `require-tool-access.test.ts`. - **Dependencies:** `grapesjs`, `@grapesjs/react`, `grapesjs-mjml`, `mjml`, `@types/mjml`, and the mjml `serverExternalPackages` entry in `next.config.ts`. About 200 lockfile entries removed; no other package versions changed (diffed against `HEAD`). - **Docs:** README, CLAUDE.md (component tree, `grapesjs` 0.23 hold dropped), `/update-deps` and `/newtool` commands, reference docs. ADR-023 marked Superseded, GOTCHA-038/039 Retired (numbering kept stable). Dated history records untouched. - **TODOs:** both open template-editor TODOs closed as won't-fix; held CSP item and Step 6 no longer carry the GrapesJS Google Fonts question; `INDEX.md` counts regenerated (12 open). ## Test plan - [x] `npm run lint` - [x] `npm run typecheck` (after clearing stale `.next/` generated types) - [x] `npm run test:coverage` — 116 files, 2,084 tests; 98.97% statements / 99.71% lines (thresholds hold) ## Notes - `npm audit` reports high advisories against `next` 16.3.7 and `braces` (via `eslint-config-next`). They predate this change and are unrelated; next up is `/update-deps`. - `src/auth.shared-instance.test.ts` failed once under full-suite load, then passed alone and in the next full run. Looks timing-sensitive. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… MP_OIDC_* (#41) ## Summary Owner decision ([ADR-027](.claude/references/DECISIONS.md)): the app runs on **one** MP OAuth client. This reverses Step 4's dedicated sign-in client. - **Code:** `getOidcClient()` / `OidcClient` replaced by `getMpClient()` in `src/lib/env.ts`, which reads `MINISTRY_PLATFORM_CLIENT_ID` / `_SECRET` and refuses to start when either is unset, without echoing a value. `auth.ts` (OAuth client, id_token audience) and `user-menu/actions.ts` (sign-out `client_id`) use it. `warnIfSharedOidcClient` and the `auth.oidc.shared_client` event are removed. - **Config:** `MP_OIDC_*` removed from `.env.example` and `scripts/setup.ts`. Both now say the one client needs the sign-in redirect URIs registered in MP. - **Tests:** fallback, half-pair and "audience must not be the service account" cases removed. New `getMpClient` refusal tests, plus regression tests that a leftover `MP_OIDC_*` pair is ignored. `test-setup.ts` stubs a single client. - **Docs:** CLAUDE.md, auth/security references, GLOSSARY, and ADR-027 with the accepted risk. - **TODOs:** `require-dedicated-oidc-client` closed as superseded, Step 4 annotated, INDEX regenerated (11 open). ## Accepted risk One secret now covers both full MP data access and user sign-in, and the id_token audience is the service account's client id. This is recorded in ADR-027 and the security README. ## Before deploying - [ ] In each environment, confirm the `MINISTRY_PLATFORM_CLIENT_ID` client's MP record has redirect URI `<BETTER_AUTH_URL>/api/auth/callback/ministryplatform` and post-logout URI `<BETTER_AUTH_URL>`. Without these, sign-in fails after deploy for environments that used a dedicated client. - [ ] Delete any `MP_OIDC_CLIENT_ID` / `_SECRET` values left in Vercel or `.env.local`. They're ignored now, but they're stale secrets. ## Test plan - [x] `npm run lint` - [x] `npm run typecheck` - [x] `npm run test:coverage`: 116 files, 2,076 tests; 98.97% statements / 99.71% lines 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Adds `.claude/TODO/2026-10-10-owner-ops-checklist-before-release.md`, a single checklist of the non-code work: `MP_SECURITY_ROLES=*`, MP client redirect URIs (after #41), deleting stale `MP_OIDC_*` secrets, `BETTER_AUTH_SECRET` rotation and audit-log review, the post-deploy real-MP smoke test, Security Role naming, and GitHub code-security toggles. Items are grouped into before release / right after / anytime, and existing ops and held TODOs are linked rather than duplicated. INDEX updated (12 open). Docs only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s) (#43) ## Summary - **Security:** `npm audit` 7 high → 5 high. `next` 16.3.7 → 16.4.0 fixes six high advisories, including Image Optimization SSRF (GHSA-cjq9-62q9-8jv4) and SSG/ISR cache poisoning. A `source-map-js` DoS is cleared transitively. - **Still open:** the remaining 5 are a single `braces` DoS (GHSA-vfj7-8cjw-p6xm) counted once per chain node. No fixed `braces` has been published (3.0.3 is latest and last-affected). It is reached only by `@next/eslint-plugin-next` at lint time and is never in the app bundle. Accepted and recorded with a re-check command. - In-range updates otherwise: better-auth, docx, dotenv, lucide-react, vitest, jsdom, radix-ui, postcss, and others (full table in the record). - No removals and no majors adopted. The TS 7, ESLint 10 and `@types/node` 25+ holds were re-checked and are still blocked. The GrapesJS hold is retired (package removed in #40). Record: `.claude/packages/2026-10-10.md` ## Verification - Tests 2076 / 116 files: identical to baseline - `eslint .` clean, `tsc --noEmit` 0 errors, `next build` passes - `npm run test:coverage` passes thresholds; `coverage/coverage-final.json` present - OSV.dev sweep of 431 packages: only the `braces` hit 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
## Summary `.claude/TODO/` now holds **open work only**. When an item is done or declined, its file is deleted and the deleting commit becomes the record. The rule is in `SCHEMA.md` § Lifecycle. - **Deleted 17 resolved TODOs**: 13 done (security Steps 1–5, the 2026-09-13 coverage/CI/logging fixes), 2 won't-do (template editor, removed in #40), 1 superseded (dedicated OIDC client, #41 / ADR-027), and the template-editor tests item. - **Salvaged first:** two new low TODOs for follow-ups that only lived inside closed files (Select id-0 check, dev-panel `resolveContactIds`), plus GOTCHA-044 for the `removeGroup` setState-updater lesson. - **Links fixed:** removed "See .claude/TODO/…" from 4 test comments and a facts snapshot. Six GOTCHAs and the group-wizard reference pointed at TODO files deleted back in April. I checked each bug against the code, found all fixed, and updated those entries to point at the code that enforces the fix. No link to a missing TODO remains. - **INDEX.md** rebuilt: 14 open, no resolved section. ## Test plan - [x] `npm run lint` - [x] Vitest on the 4 edited test files (10 files, 253 tests). Only comments changed there. - [x] Scripted check: no reference to a non-existent `.claude/TODO/*.md` anywhere in the repo 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
One-line VS Code Peacock setting (`statusBar.inactiveBackground: #1857a4`). A direct push to `dev` was rejected by the required `test` check, so this lands through a PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Closes the `select-falsy-id-zero-predicate` TODO as won't-do: MP primary keys are identity columns starting at 1, so a lookup id of 0 can't occur and the `field.value ? … : ""` check is correct in practice. - Deletes `.claude/TODO/2026-10-10-select-falsy-id-zero-predicate.md` - Updates `.claude/TODO/INDEX.md` counts (14 → 13 open) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
#47) Both owner ops items are done (2026-10-10): - `BETTER_AUTH_SECRET` rotated in every environment, `dp_Audit_Log` reviewed for the exposure window - `MP_SECURITY_ROLES=*` set in every environment Changes: - Deletes `2026-09-30-ops-rotate-better-auth-secret.md` and `2026-09-30-ops-set-mp-security-roles-env.md` - Ticks items 1 and 4 in the owner checklist and drops their dead links - Marks the rotation done in `.claude/references/security/README.md` - `INDEX.md`: 13 → 11 open 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Removes five TODOs that are either done or won't be done in this repo: - **Done:** `owner-ops-checklist-before-release` - **Won't do (template repo; downstream clones decide, or the change causes more problems than it solves):** `held-github-repo-security-settings`, `held-stored-procedure-allowlist`, `step7-write-audit-and-datetime-cleanup`, `step9-ci-setup-and-docs` Reference cleanup: - `INDEX.md` rebuilt: 11 → 6 open, no high items - `security/README.md` "Still open — needs a human" trimmed to the enforced-CSP walk (still tracked by step6 / held-csp) - `mp-provider/services/procedure.md` no longer links the deleted allowlist TODO; it notes the missing allowlist is by design and that a clone can add one 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…mes (#49) ## Summary - `ToolService.resolveContactIds(pageId, recordIds)` now looks up `Table_Name`, `Primary_Key` and `Contact_ID_Field` server-side via `getPageData`, validating them with `validateColumnName` / `validateFkPath` before interpolation. Returns `null` when the page is unknown or has no contact metadata. - The dev-panel action `resolveContactRecords` and `ContactRecordsPanel` pass `params.pageID` instead of table/column names, matching what `FamilyService.resolveContactIdFromPage`'s action already does. - Hostile-identifier tests (SQL in table name/PK, extra column, alias, non-FK segment) and null/invalid-page tests. - Updated reference docs; deleted the closed TODO and updated `INDEX.md`. ## Test plan - [x] `npm run lint` - [x] `npm run typecheck` - [x] `npm run test:coverage` (2095 passed) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…mpty (#50) ## Summary - `FamilySearchBar`'s `<CommandEmpty>` never rendered: cmdk shows it only when the **whole** `Command` has zero items, and the "+ New Family" item is always registered once the query is >= 2 chars. - Replaced it with a plain `<div>` matching the "Searching…" / "Type at least 2 characters" states. - Gated on a new `searchedTerm` state, so the message appears only after the term on screen has been searched. Without that it would flash during the 300 ms debounce while `results` still holds the previous term's answer. A failed search shows nothing, as before. - Tests: replaced the test that asserted the broken behavior; added a debounce/stale-term test; the error test now also asserts no false "not found". - Added GOTCHA-045; deleted the closed TODO and updated `INDEX.md`. ## Test plan - [x] `npm run lint` - [x] `npm run typecheck` - [x] `npm run test:coverage` (2096 passed) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…Next (#51) ## Summary - Removed the `!fields || fields.length === 0` special case from `GroupWizard.handleNext`. Only the Review step's `STEP_FIELDS` entry is empty, and Review renders Submit (`handleSubmit`), never Next, so the branch could not run. - `handleNext` now always calls `form.trigger(STEP_FIELDS[currentStep])`. In react-hook-form 7.89 with a resolver, `trigger([])` resolves `true` and updates no field errors. A missing entry (`undefined`) validates the whole form, so a future misconfiguration fails safe instead of skipping validation. - New `schema.test.ts` case: `STEP_FIELDS` has an entry per `WIZARD_STEPS`, every step before Review lists fields, and only Review is empty. - `group-wizard.tsx` goes from 94.5% to 100% statements. Deleted the closed TODO and updated `INDEX.md`. ## Test plan - [x] `npm run lint` - [x] `npm run typecheck` - [x] `npm run test:coverage` (2097 passed) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…de, codegen escaping, @domainID, trimmed profile (#52) Closes the Step 8 TODO from the 2026-09-30 playbook audit. One commit per item. ## Summary **Address labels** (`5f20cbd`, `9cc2879`) - Labels and config sent by the client are now validated with Zod (`schemas.ts` → `parseLabelInput`): at most 10,000 labels, bounded strings, `serviceType` limited to `SERVICE_TYPES`, and `barStates`/`barType` **dropped** (only the server computes those). A shared `prepareLabels` helper also caps `startPosition` at the stock's labels per sheet. It's a count of blank cells, so before this it was an unbounded page count. - **Zip-bomb guard** (`src/lib/zip-guard.ts`): reads the central directory itself before PizZip sees the file. It caps the number of entries (1,000) and the declared uncompressed total (64 MB), then inflates each entry with `maxOutputLength` set to its declared size, so a header that lies about its size fails fast. PizZip only checks sizes *after* inflating each file fully into memory. - **Body size:** the template limit drops from 5 MB to **2 MB**, and `experimental.serverActions.bodySizeLimit` goes from the 1 MB default to **4 MB**, just under Vercel's 4.5 MB request cap. The old 5 MB check could never fire. All limits now live in `address-labels/limits.ts`, shared by client and server. - Also fixed (rule 14): `preEncodeBarcodes` was logging label names and ZIP/routing digits on encode fallbacks. It now logs `{ labelIndex, error: <name> }`. **App** (`5f3992b`) - `global-error.tsx`: adds a plain `<a href="/">` that works without JS. "Try again" stays disabled until hydration (`useSyncExternalStore`). - `tool-params.server.ts`: a stray `%` in `recordDescription` threw `URIError` and crashed the tool page. It now falls back to the raw value. **Codegen** (`efa6923`): every MP metadata value interpolated into generated types goes through `codegen-escape.ts` (`propertyKey`, `commentText`, `positiveInt`). Before this, `col.Size` went into the Zod `.max(...)` calls as raw code. Output is unchanged for ordinary metadata. **SQL** (`e181994`): `api_dev_GetProcedureDefinition` now takes `@DomainID INT` first; `_INSTALL` regenerated. **It must be re-deployed to MP.** **Profile** (`7f22608`): the `MPUserProfile` sent to the client is now `First_Name`, `Nickname`, `Last_Name`, `Email_Address`. That drops `User_ID`, `User_GUID`, `Contact_ID`, `Mobile_Phone`, `Image_GUID`, roles and user groups, none of which were displayed. `UserService` runs one query instead of three. **Docs** (`docs:` commit): reference docs updated; TODO deleted and `INDEX.md` updated. ## Notes / follow-ups - Responses aren't covered: Vercel's 4.5 MB cap applies to response bodies too, and a very large PDF/merged `.docx` returned as base64 could exceed it. This PR doesn't address that. - `UserMenu` isn't rendered anywhere at the moment, so the profile trim has no visible effect today. ## Test plan - [x] `npm run lint` - [x] `npm run typecheck` - [x] `npm run test:coverage`: 2161 passed, 99.12% statements. New: zip-guard unit tests against real archives, a merge integration test with real PizZip/docxtemplater (sample template merges, forged-size bomb rejected), label-validation tests, global-error SSR test, codegen-escape tests - [x] `npm run build` (production build succeeds with the new `next.config`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…der check + build in CI (#53) Closes the last two open TODOs: **step 6** of the 2026-09-30 playbook audit and the **held CSP notes for customer clones**. ## Headers and proxy - `next.config.ts`: COOP/CORP `same-origin`, `poweredByHeader: false`, `images.unoptimized` (nothing uses `next/image`, so the optimizer was only SSRF surface), `logging.serverFunctions: false` (stops `next dev` printing server-action arguments such as household records). - `proxy.ts`: `/api/auth` is public only as a whole path segment (`/api/authz` now gets the session check). Matcher exclusions are anchored, so look-alike paths like `/favicon.ico-admin` are still gated. - CSP: MP origins are accepted only over https (http to localhost only in dev). `data:`, `javascript:`, plain-http and credentialed URLs are dropped. `base-uri 'none'`. - **Opt-in `CSP_REPORT_URI`**: adds `report-uri` + `report-to` and a `Reporting-Endpoints` header. Unset means no reporting, and the repo ships no collector. ## Build guards - `import 'server-only'` in every service, `auth.ts`, `env.ts`, `tool-params.server.ts` and the MP client/credentials modules. Tested by importing `domainTimezoneService` from a client component, which made `next build` fail with the `server-only` trace. - Vitest aliases `server-only` to its no-op entry. The `mp:generate*` scripts now run `tsx --conditions=react-server`. - `scripts/check-prerender.mjs` (`npm run check:prerender`) fails when any route other than `/_not-found` or `/_global-error` is statically prerendered. Tested with a planted static page. - CI `test` job: new **Build** step (placeholder env, no MP call) and **Check prerendered routes** step. ## Verification - `lint`, `typecheck` and `test:coverage` pass: 2191 tests, 99.13% statements. - A build with CI placeholder env and no `.env.local` passes. - **Enforced-CSP walk against a production build (2026-10-10):** sign-in, every tool, Radix select/popover/command and dialog (scroll-lock `<style>` applied), contact search and household load. Zero CSP violations. Headers checked with curl on the real server. - Not walked: sign-out. `UserMenu` isn't mounted anywhere, so a production build has no sign-out control. This was already the case before this PR. ## Docs Security reference (headers, matching, reporting, customer-clone roll-out), GOTCHA-046/047, CLAUDE.md (CI builds now, server-only practice). Both TODOs deleted; the index shows 0 open. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…codegen docs (#54) ## UserMenu removal The project won't use the dropdown `UserMenu`, which was never mounted anywhere. - Deleted `src/components/user-menu/` (component, barrel, component test). - `handleSignOut` is still used by the `/session-error` form, so it moved with its tests to `src/components/shared-actions/sign-out.ts`. Imports and code comments are updated. - Removed `@heroicons/react`, which only the menu used. The vendored shadcn `dropdown-menu` primitive stays. - Docs: deleted `components/user-menu.md`; updated auth, data-flow (sign-out call graph rewritten to match the current action), GLOSSARY, GOTCHAS, security, testing inventory, CLAUDE.md and README. - The security reference now says plainly that **`/session-error` holds the app's only sign-out control** and `/no-access` has none. ## Codegen docs Every direct `tsx` command for `generate-types` and `generate-storedprocs` (README, scripts README, the mp-provider reference) now includes `--conditions=react-server`, with a note on why: the MP client modules use `server-only`. The CLIs' `--help` text now points to the npm scripts, which already pass the flag. ## Verification `lint`, `typecheck` and `test:coverage` pass (2184 tests; the 7 `UserMenu` component tests were removed with it). `npm run build` and `check:prerender` pass. `npm run mp:generate:storedprocs -- --help` runs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign-in isn't role-gated, so a user with no MP security role gets a session and lands on `/no-access`. With `UserMenu` removed (#54), nothing let them end that session before the 12-hour limit. - `/no-access` now has a **Sign out** form wired to `handleSignOut`, the same pattern as `/session-error`. The page stays dynamic because it renders under AuthWrapper, so the CSP nonce is present. - Added a test that pins the submit button and form. - Updated the docs (security, oauth-flow, the sign-out call graph, shared-actions README, README) to name both sign-out forms. **Verified** against a production build with the CSP enforced. Clicking Sign out on `/no-access` went to MP's logout page, back to the app, through `/signin`, and stopped at the MP login prompt, so both sessions ended. No CSP violations. `lint`, `typecheck` and `test:coverage` pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
## Summary - Moves `SECURITY.md` → `.github/SECURITY.md` (still detected by GitHub as the security policy) and updates every reference. - Rewrites `README.md` against the current code. **Fixes the documented OAuth redirect URI** (`/api/auth/callback/ministryplatform`; the old `/oauth2/callback/...` path is blocked by the route allowlist). Adds the required `MP_SECURITY_ROLES` and the security env vars, all 5 tools / 9 services, the real setup wizard steps (Node 24), the 4 existing Claude Code commands, current dependency versions, and examples that follow the auth-gate and datetime rules. - Fixes `CLAUDE.md`: the `Congregations` and `ContactLogSchema` imports didn't compile, and the `$userId` example contradicted rule 13. Also completes the service list and corrects stale versions and file names. - Verifies and corrects every `.claude/references` domain plus INDEX/GLOSSARY/DECISIONS/GOTCHAS, commands and `_meta`. Adds `services/family-service.md` and a 2026-10-10 facts snapshot. - Code changes are comments only, plus one stray whitespace line in `toolService.test.ts`: `error.tsx`, `auth-wrapper.tsx`, `helper.ts` JSDoc. ## Code fixes merged into this branch (found during the review) - #57 — `GooglePlacesService` gated on `dp_Configuration_Settings`/read on every public method (a cached API key can't be used without the role). - #58 — `UserProvider` fetches the profile lazily on the first `useUser()` call (no MP call per page load with no consumer). - #59 — `FieldManagementService.getTableMetadata` matches on `Name` (confirmed with a live read-only call: the API never returns `Table_Name`); no `tables[0]` fallback. `TableMetadata` type corrected. - #60 — Generated Zod schemas accept MP wall-clock and ISO datetimes; generators write straight to `mp-schema/`; the `--search` run no longer clobbers the barrel or catalog. **Models regenerated from live MP** (301 → 307 tables, 532 → 539 procs). All TODOs closed (0 open). ## Open question The repo is public with no LICENSE file. The README now says so; previously it said "Private". ## Test plan - [x] `npm run lint` - [x] `npm run typecheck` - [x] `npx vitest run`: 120 files / 2,237 tests pass - [x] Relative-link check across all markdown: 0 broken 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes
dev→main: 24 PRs (#33–#56), mostly the 2026-09-30 security playbook remediation.Before deploying: set
MP_SECURITY_ROLESin every environment;BETTER_AUTH_SECRETmust be ≥ 32 characters; register the sign-in callback and post-logout URIs on theMINISTRY_PLATFORM_CLIENT_IDclient in MP; re-deployapi_dev_GetProcedureDefinition.Merge with a merge commit (never squash).
🤖 Generated with Claude Code