From 6f23636a3e92ea71976e4d0944f7428c27ed3234 Mon Sep 17 00:00:00 2001 From: Chris Kehayias Date: Sun, 13 Sep 2026 06:08:07 -0400 Subject: [PATCH 1/4] fix(security): authorize MP data access, scrub logs, add CSP and error boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applies the downstream hardening playbook (upstream MPNext 436466d..5bc505a) to the findings that affect this repo. Verified against a real `next start`, not only unit tests. F1/F10/F11 — authentication is not authorization MP's OIDC endpoint authenticates ANY dp_Users record, and this app reads MP with its own client-credentials service account, so MP's per-user record security never applies to what it returns. Every feature action was gated on "a session exists", which proves nothing. The sharpest surface was field management: updatePageFieldOrder rewrites dp_Page_Fields for the ENTIRE MP domain, not just the caller. Adds AuthorizationService with two entry points — requireSecurityRole() (throws, logs, returns the acting User_ID) and hasSecurityRole() (decision only, never enforcement). Gated at three independently-reachable layers: the tools layout, every feature server action, and every service method, reads included. Per-request memoization via React cache() keeps that to one MP read while ensuring a role revoked in MP takes effect on the very next request. Fails closed; infrastructure failures throw rather than reporting as a refusal, so "MP is down" can never be mistaken for "not allowed". Policy: any MP user may sign in and use the shell; the tools require a security role. Sign-in is deliberately NOT role-gated — a role-less user keeps a session, the header and a working sign-out, and is redirected to /no-access. Refusing at sign-in would strand them with no way out. Configured by MP_SECURITY_ROLES; blank means any MP role will do. Four carve-outs use a plain session check and justify it in-file. Write attribution now has exactly one source $userId comes from the gate's return value, applied in the service. No server action accepts a userId parameter. getSelectionRecordIds takes its @UserID from the gate too — a selection belongs to a specific MP user, so accepting one from the payload let any caller read someone else's. F5 — PII must not reach logs Removed 21 logger.debug calls dumping $filter params, stored-procedure parameters, PUT bodies and full result sets (names, emails, phones), plus 11 console.log sites. Being gated on NODE_ENV was not enough: dev machines and non-production deployments still wrote member PII to aggregators with broader access and longer retention than MP itself. Also strips the response body from THROWN error messages, not just logs — a thrown message reaches error reporters and client-visible action results, so it leaked record content and $filter strings everywhere at once. The logger now has no debug channel at all. no-console is enforced by ESLint (warn/error only), verified to fire against a negative control. F9 — security headers and a nonce-based CSP Static headers in next.config.ts (reaching /api and the paths the proxy matcher skips); CSP in the proxy, because the nonce must be fresh per request. Anti-framing is expressed twice on purpose — two CSP headers on one response are enforced as an intersection, which is miserable to debug. style-src keeps 'unsafe-inline' with NO nonce: Radix's dialog injects a