Merge dev: app-review fixes (security, env validation, docs, licensing)#9
Merged
Conversation
MP stores and interprets datetime values as wall-clock in the domain's time zone, not UTC. Converting with raw new Date().toISOString() shifted date-boundary queries by the UTC offset. - Add DomainTimezoneService singleton (domain TZ -> IANA, toMpSqlDatetime, parseMpDatetime) with unit tests - Add getMpTimezone() server action for client-side Intl.DateTimeFormat rendering - Route fullCalendar getEvents \ literals through toMpSqlDatetime (+ regression tests) - Document the boundary-conversion rule and anti-patterns (CLAUDE.md, references, playbook) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove all hardcoded 'northwoods' tenant identifiers, my.northwoods.church hosts, and Northwoods-specific group ID defaults so the codebase is tenant-neutral. - MP host: server resolves via getMpHost() (throws if MINISTRY_PLATFORM_BASE_URL unset); docs/snippets use neutral placeholder; widget falls back soft with a warning - SDK build: drop church default, inject org name via VITE_ORG_NAME build define - profile widget SMS consent text uses configurable org name - Demo HTML: use existing __MP_BASE_URL__ placeholder; delete dead tid/initToken (session auth is origin-based) - Group gates fail closed: remove '22'/'73' defaults; unset = no access + warning - Docs: drop NorthwoodsNext/Northwoods-specific wording; fix stale CLAUDE.md tenant note - Tests: neutral fixtures + fail-closed coverage Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two pre-existing tooling failures discovered during the Northwoods refactor, both unrelated to it: #1 pnpm build:sdk: the SDK tsc step type-checked .test.ts files and vitest v4's vi.fn() mocks didn't satisfy ApiClientConfig's signatures. Give the api-client mocks precise Mock<T> types, and exclude *.test.ts from the production build tsconfig (tests don't belong in dist). #2 pnpm lint crashed with 'TypeError: expand is not a function'. The pnpm override 'brace-expansion: >=5.0.5' forced 5.x (ESM named export) onto minimatch@3.1.5, which calls brace-expansion as a default function. Scope the override (minimatch@3>brace-expansion: 1.1.12) so minimatch@3 gets a patched v1 while minimatch@10 keeps 5.x. Both remain patched against the ReDoS advisory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With ESLint running again, 4 errors + 4 warnings (previously masked by the crash) surfaced. None originated from the Northwoods or datetime work. - eslint config: ignore coverage/ (generated output should not be linted) - (app)/layout.tsx: remove dead, unused mpBaseUrl - access-denied.tsx: use next/link for the internal '/' page link - demo/page.tsx: keep <a> for the /api/auth/sign-out API route (Link would prefetch and sign out early) with a scoped rule disable - add-to-calendar route: drop unused 'claims' binding, keep the auth-check side effect - fullCalendarService: remove unused EventParticipantCount interface; let -> const for ministryMap; justify the heterogeneous Promise<any>[] with a scoped disable Verified: pnpm build, pnpm lint, and pnpm test:run (530 tests) all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Security (pnpm.overrides): - brace-expansion: override pinned the vulnerable 1.1.12; bump to 1.1.15 (CVE-2026-33750, ReDoS/memory exhaustion via eslint>minimatch) - postcss: add >=8.5.10 to float Next's bundled copy past the fix (CVE-2026-41305, XSS via unescaped </style> in stringify output) pnpm audit now reports 0 vulnerabilities. Safe updates: @types/react 19.2.15, vitest/@vitest/coverage-v8 4.1.7, better-auth 1.6.12, postcss 8.5.15, react-hook-form 7.76.1, tsx 4.22.3, typescript 6.0.3, vite 8.0.14, @hookform/resolvers 5.4.0, @inquirer/prompts 8.5.1, @types/node 25.9.1, lucide-react 1.17.0, zod aligned to 4.4.3 in @mpnext/types. Workaround: pin kysely ^0.28.17 in overrides. better-auth 1.6.12 widened its kysely peer range to ^0.29.0, but @better-auth/kysely-adapter's dist still imports DEFAULT_MIGRATION_TABLE/DEFAULT_MIGRATION_LOCK_TABLE, which kysely 0.29 removed — broke the Next build. Remove once upstream ships a 0.29-compatible adapter. Verified: pnpm build (SDK + Next) green, pnpm lint clean, pnpm test:run 530/530 passing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Raise the SDK's @types/node from ^20.19.37 to ^22.19.19 (latest 22.x, Node 22 LTS), closing the gap with the Node 20 baseline. Intentionally kept on the 22.x line rather than matching the root app's @types/node 25, since the SDK targets a Node 22 LTS toolchain. Dev-only type dependency for the Vite/TS toolchain; SDK source uses DOM/Web Component APIs, not Node globals, so no type surface is affected. Verified: tsc typecheck clean, vite build green (bundle byte-identical, 161.81 kB / 33.95 kB gzip), pnpm test:run 530/530 passing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Align @mpnext/embed-sdk and @mpnext/types from ^6 (resolved 6.0.2) to 6.0.3, matching the root app. Patch-level, dev-only. Verified: pnpm build (SDK + Next) green, pnpm test:run 530/530 passing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dev-only task runner used by the dev and test:widget scripts. v10's only notable breaking change is dropping Node <20 (CI/dev run Node 24). Verified: smoke-tested the exact flags the scripts use (concurrently -n next,widget -c blue,green ...) — both processes labeled and exit 0. pnpm build (SDK + Next) green, pnpm test:run 530/530 passing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Audited the codebase for every env reference (process.env, import.meta.env, VITE_*, NEXT_PUBLIC_*) and updated .env.example so all required and optional variables are present with explanatory comments. Added notes for the OIDC -> MINISTRY_PLATFORM_CLIENT_* fallback, Vercel auto-detected origins, and the auto-derived NEXT_PUBLIC_APP_VERSION. Removed the legacy NEXTAUTH_URL / NEXTAUTH_SECRET fallbacks in favor of the canonical BETTER_AUTH_* names across auth, logout, the SDK Vite config, and the demo page. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Align README with the actual implementation: drop the unimplemented multi-tenant/idempotency claims, fix the SDK loader description (static hashed loader, not an x-sdk-hash header), bump the Node requirement to 20.9+, remove the nonexistent src/contexts/ entry, add the logout/session-tokens routes and domainTimezoneService, fix the ProfileService example, and correct the Claude Code commands table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…curity) The put() method logged the full request body via console.log on every PUT, which could leak personal/sensitive data into production logs. PUT flows through profile updates and other write paths. Removed the log; remaining console.error calls only fire on failed responses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove debug console.log noise on hot paths in the proxy and MP provider client/table/procedure services. Intentional console.error calls are retained. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add src/lib/env.ts with getEnv()/getEnvOptional() that throw a clear, actionable error naming the missing variable, mirroring getMpHost(). Replace process.env.X! non-null assertions across the MP client, client-credentials, auth, invoice service, and logout route so that missing config fails fast instead of surfacing as cryptic later errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… perpetual spinner The sign-in effect called authClient.getSession().then(...) with no rejection handling. A network failure or auth-service outage left the user on a permanent "Redirecting to sign in..." spinner with no feedback or recovery path. Convert the effect to an inline async function with try/catch. On failure it now surfaces a visible "Sign-in unavailable" message with a "Retry sign-in" button that re-runs the flow via an attempt counter. A cancelled flag guards against state updates / redirects after unmount. Also drops the previous isRedirecting-in-deps pattern that could re-trigger OAuth. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mented) No embed route reads Idempotency-Key and no jti-based deduplication exists, so advertising the header in Access-Control-Allow-Headers implied unsupported behavior. Remove it from the three CORS allow-header lists in auth.ts, update the matching test assertions, and drop the "idempotency keys" design claim from CLAUDE.md. The standard jti JWT claim is retained (benign, unrelated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… services list Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Align all project-name references to the canonical MPNext-Widgets (matching the GitHub repo and git remote). Updates root package.json name, CLAUDE.md title, .env.example NEXT_PUBLIC_APP_NAME, the app metadata title, and setup script banners. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an engines field (node >=20.9) to the root package.json, bump the setup.ts Node check from major 18 to 20 (updating user-facing v18 message strings to v20.9), and add a root .npmrc with engine-strict=true so pnpm/npm hard-fail installs on unsupported Node. Aligns the Node policy across package.json, setup.ts, CI (Node 22), and the README (v20.9+). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…file Set license: UNLICENSED in root package.json, add a root LICENSE file with a proprietary/confidential notice (ACS Technologies, 2026), and align the README License section to reference the LICENSE file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
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
2 participants
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.
@
Summary
Merges
devintomain. Includes the 9 application-review TODO fixes plus prior dev work (MP datetime handling at the boundary, dependency/CVE updates, README/env documentation).Application-review fixes (this batch)
console.logfrom the MP HTTP client; removed leftover debugconsole.logs across proxy/client/services (keptconsole.error).getEnv()fail-fast env helper replacingprocess.env.X!assertions; sign-ingetSession()now surfaces an error + retry instead of a perpetual spinner.Idempotency-KeyCORS header (no dedup implemented; standardjticlaim retained for revocation).next-embed.es.js, ~33KB gzip, full services list); canonicalized project name toMPNext-Widgets; enforced Node>=20.9viaengines+.npmrc engine-strict; addedUNLICENSEDlicense field and proprietaryLICENSE.Prior dev work also included
DomainTimezoneServiceat the boundary (+ tests, reference docs).Verification
Each fix passed
pnpm lintand a Next.js build with no new failures.🤖 Generated with Claude Code
@