fix(logging): remove PII/pastoral-note debug logging from src (F5)

.claude/references/auth.md

@@ -629,6 +629,26 @@ and a `reason` of `no_mp_user` / `no_security_role` / `role_not_permitted` — s
 operations are greppable in production logs. `hasSecurityRole` logs nothing: it runs on
 every profile load, and the UI asking "may they?" is not an incident.
 
+#### Logging policy (F5, closed 2026-09-12)
+
+No debug/info logging (`console.log`/`.debug`/`.info`) is allowed in `src/` outside
+`src/lib/providers/ministry-platform/scripts/` (dev-only CLI tools). Contact logs carry
+pastoral notes and every MP read/write can carry member PII (names, emails, phones); a
+hosting or log-aggregation platform retains `console.*` output with broader access and
+longer retention than the MP database itself, so none of that content may reach a log
+line.
+
+`console.error`/`console.warn` in catch blocks may stay, but must log **identifiers and
+shape, never content**: table name, record IDs/counts, HTTP status, and an error's
+`name`/`message` — never an MP result set, a request body, `Notes`, emails, phones,
+names, or a URL/query string containing `$filter`. The HTTP client's failure logs are the
+canonical shape: `{ method, endpoint (path only, no query string), status, statusText }`,
+and the thrown `Error`'s message keeps only `status`/`statusText`/`endpoint` — no
+response body. The four structured events above (`mp.read.unauthorized`,
+`mp.write.unauthorized`, `mp.write.non_user`, and `auth.userinfo.invalid_sub` in
+`src/lib/auth.ts`) are the greppable contract this policy exists alongside; they already
+log identifiers only and are unaffected by it.
+
 ### Caching: per request, never across requests
 
 The gate now runs at up to three layers per request, so the `dp_User_Roles` read is
@@ -697,6 +717,7 @@ server-side; defence in depth).
 | **F3** (Medium) — open redirect via `callbackUrl` on `/signin` | 2026-09-12 | `sanitizeCallbackUrl`, applied to both redirect sinks |
 | **F10** (Low) — `ContactService.updateContact` wrote with no authorization | 2026-09-12 | Calls `requireSecurityRole({ table: "Contacts", operation: "update" })` and uses its `User_ID` for `$userId` |
 | **F11** (Low) — `getMpTimezone` had no check at all | 2026-09-12 | Authenticated-session check (its only consumer is the role-gated contact page) |
+| **F5** (Medium) — member PII and pastoral notes written to server logs at info level | 2026-09-12 | Removed all `console.log`/`.debug`/`.info` from non-script `src/`; error logs now carry identifiers/shape only (no request bodies, result sets, `Notes`, or `$filter`/full URLs); see § Logging policy above |
 
 ## Environment Variables
 

CLAUDE.md

@@ -185,6 +185,7 @@ export default MyComponent;            // ❌ Avoid
 9. **Use service classes in server actions** - call services from `src/services/`, not MPHelper directly from components or actions
 10. **Authorize, don't just authenticate** - feature server actions AND service methods that touch Ministry Platform data must call `AuthorizationService` (`requireSecurityRole`, for reads as well as writes), never a bare `auth.api.getSession()` check. A session proves only that some MP user signed in; all MP data is fetched with the app's service account, so the role gate is the only thing that decides who may see or change it. See **[Auth Reference](.claude/references/auth.md)** § Authorization.
 11. **Convert all date/time values at the MP boundary** - use `DomainTimezoneService` (never raw `new Date(x).toISOString()` or `getFullYear()`) when sending or receiving datetime fields, since MP stores wall-clock values in the domain's time zone, not UTC. See **[Date/Time Handling Reference](.claude/references/ministryplatform.datetimehandling.md)**.
+12. **No debug logging in `src/`** - `console.log`/`.info`/`.debug` are not allowed outside `scripts/`; log errors with identifiers (table, IDs, status), never record content, `$filter` strings, or request bodies. MP data is member PII and pastoral notes.
 
 ## Validation Best Practices
 

eslint.config.mjs

@@ -6,6 +6,21 @@ const eslintConfig = defineConfig([
   ...nextVitals,
   ...nextTs,
   globalIgnores([".next/**", "out/**", "build/**", "coverage/**", "next-env.d.ts"]),
+  // F5 (2026-09-12): member PII and pastoral notes must never reach info-level
+  // logs. `console.log`/`.debug`/`.info` are disallowed in application source;
+  // `console.warn`/`.error` remain for structured/error logging. Generator
+  // scripts (dev-only CLI tools) and test files are exempt. See
+  // .claude/references/auth.md § Logging policy.
+  {
+    files: ["src/**/*.{ts,tsx}"],
+    ignores: [
+      "src/lib/providers/ministry-platform/scripts/**",
+      "**/*.test.{ts,tsx}",
+    ],
+    rules: {
+      "no-console": ["error", { allow: ["warn", "error"] }],
+    },
+  },
 ]);
 
 export default eslintConfig;

src/app/signin/page.tsx

@@ -28,18 +28,14 @@ function SignInContent() {
   const callbackUrl = sanitizeCallbackUrl(searchParams?.get("callbackUrl"));
   const [isRedirecting, setIsRedirecting] = useState(false);
 
-  console.log("SignIn Page rendered with callbackUrl:", callbackUrl);
-
   useEffect(() => {
     // Check if user is already signed in
     authClient.getSession().then(({ data: session }) => {
       if (session) {
         // User is already signed in, redirect to callback URL
-        console.log("User is already signed in, redirecting to callback URL:", callbackUrl);
         window.location.href = callbackUrl;
       } else if (!isRedirecting) {
         // User is not signed in, initiate sign in
-        console.log("Redirecting to SignIn API");
         setIsRedirecting(true);
         // better-auth 1.7 routes generic OAuth providers through the standard
         // social sign-in path; `signIn.oauth2()` was removed.

src/components/contact-logs/actions.test.ts

@@ -558,4 +558,64 @@ describe('contact-logs actions', () => {
       expect(mockGetContactLogById).toHaveBeenCalledWith(42);
     });
   });
+
+  describe('Logging safety (F5)', () => {
+    // Pastoral notes and record content must never reach info-level logs, and
+    // an error log must never echo them either. See
+    // .claude/references/auth.md § Logging policy.
+    let logSpy: ReturnType<typeof vi.fn>;
+    let errorSpy: ReturnType<typeof vi.fn>;
+
+    beforeEach(() => {
+      logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+      errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+    });
+
+    const sensitiveNotes = 'Confidential: disclosed a personal crisis in confidence';
+
+    it('should not log Notes when creating a contact log succeeds', async () => {
+      mockCreateContactLog.mockResolvedValueOnce({ Contact_Log_ID: 1 });
+
+      await createContactLog({ ...validCreateInput, Notes: sensitiveNotes });
+
+      expect(logSpy).not.toHaveBeenCalled();
+      expect(errorSpy).not.toHaveBeenCalled();
+    });
+
+    it('should not log Notes when creating a contact log fails', async () => {
+      mockCreateContactLog.mockRejectedValueOnce(new Error('MP write failed'));
+
+      await expect(
+        createContactLog({ ...validCreateInput, Notes: sensitiveNotes })
+      ).rejects.toThrow('MP write failed');
+
+      expect(logSpy).not.toHaveBeenCalled();
+      expect(errorSpy).toHaveBeenCalledTimes(1);
+      const loggedArgs = errorSpy.mock.calls[0].map(String).join(' ');
+      expect(loggedArgs).not.toContain(sensitiveNotes);
+    });
+
+    it('should not log Notes when updating a contact log fails', async () => {
+      mockUpdateContactLog.mockRejectedValueOnce(new Error('MP write failed'));
+
+      await expect(
+        updateContactLog(1, { Notes: sensitiveNotes })
+      ).rejects.toThrow('MP write failed');
+
+      expect(logSpy).not.toHaveBeenCalled();
+      const loggedArgs = errorSpy.mock.calls
+        .map((args: unknown[]) => args.map(String).join(' '))
+        .join(' ');
+      expect(loggedArgs).not.toContain(sensitiveNotes);
+    });
+
+    it('should not log anything when deleting a contact log succeeds', async () => {
+      mockDeleteContactLog.mockResolvedValueOnce(undefined);
+
+      await deleteContactLog(1);
+
+      expect(logSpy).not.toHaveBeenCalled();
+      expect(errorSpy).not.toHaveBeenCalled();
+    });
+  });
 });

src/components/contact-logs/actions.ts

@@ -78,12 +78,9 @@ export async function createContactLog(
       Made_By: userId,
     };
 
-    console.log("createContactLog action - Creating with data:", JSON.stringify(logDataWithUser, null, 2));
-
     const contactLogService = await ContactLogService.getInstance();
     const contactLog = await contactLogService.createContactLog(logDataWithUser);
 
-    console.log("createContactLog action - Successfully created");
     return contactLog;
   } catch (error) {
     console.error("Error creating contact log:", error);
@@ -109,13 +106,9 @@ export async function updateContactLog(
     // Under this policy any role-holder may edit anyone's log, so stamping the
     // editor would rewrite the pastoral record's authorship. MP's audit trail
     // already captures the editor via `$userId` in ContactLogService.
-    console.log("updateContactLog action - Updating log:", logId);
-    console.log("updateContactLog action - Update data:", JSON.stringify(contactLogData, null, 2));
-
     const contactLogService = await ContactLogService.getInstance();
     const contactLog = await contactLogService.updateContactLog(logId, contactLogData);
 
-    console.log("updateContactLog action - Successfully updated");
     return contactLog;
   } catch (error) {
     console.error("Error updating contact log:", error);
@@ -129,12 +122,8 @@ export async function deleteContactLog(contactLogId: number): Promise<void> {
 
     const logId = sanitizeNumericId(contactLogId, "Contact Log ID");
 
-    console.log("deleteContactLog action - Deleting log:", logId);
-
     const contactLogService = await ContactLogService.getInstance();
     await contactLogService.deleteContactLog(logId);
-
-    console.log("deleteContactLog action - Successfully deleted");
   } catch (error) {
     console.error("Error deleting contact log:", error);
     throw error instanceof Error ? error : new Error("Failed to delete contact log");

src/components/contact-logs/contact-logs.tsx

@@ -199,8 +199,6 @@ export function ContactLogs({
         Feedback_Entry_ID: null,
       };
 
-      console.log("Creating contact log with data:", contactLogData);
-
       await createContactLog(contactLogData);
 
       setIsCreateModalOpen(false);
@@ -211,8 +209,8 @@ export function ContactLogs({
       }
 
     } catch (err) {
-      console.error("Error creating contact log:", err);
       const errorMessage = err instanceof Error ? err.message : "Failed to create contact log";
+      console.error("Error creating contact log:", errorMessage);
       alert(`Error: ${errorMessage}`);
     } finally {
       setIsCreating(false);
@@ -233,8 +231,6 @@ export function ContactLogs({
         Contact_Log_Type_ID: selectedLogType?.Contact_Log_Type_ID || null,
       };
 
-      console.log("Updating contact log with data:", contactLogData);
-
       await updateContactLog(editingLog.Contact_Log_ID, contactLogData);
 
       setIsEditModalOpen(false);
@@ -245,8 +241,8 @@ export function ContactLogs({
         onRefresh();
       }
     } catch (err) {
-      console.error("Error updating contact log:", err);
       const errorMessage = err instanceof Error ? err.message : "Failed to update contact log";
+      console.error("Error updating contact log:", errorMessage);
       alert(`Error: ${errorMessage}`);
     } finally {
       setIsEditing(false);
@@ -281,8 +277,8 @@ export function ContactLogs({
         onRefresh();
       }
     } catch (err) {
-      console.error("Error deleting contact log:", err);
       const errorMessage = err instanceof Error ? err.message : "Failed to delete contact log";
+      console.error("Error deleting contact log:", errorMessage);
       alert(`Error: ${errorMessage}`);
     } finally {
       setIsDeleting(false);

src/lib/auth.ts

@@ -84,7 +84,10 @@ async function resolveMpUserId(userGuid: string): Promise<number | null> {
   } catch (err) {
     // Never block session creation on this — the NonUser Write warning at
     // write time will surface the missing attribution.
-    console.error("[customSession] resolveMpUserId failed", { userGuid, err });
+    console.error("[customSession] resolveMpUserId failed", {
+      userGuid,
+      err: err instanceof Error ? err.message : String(err),
+    });
     return null;
   }
 }

src/lib/providers/ministry-platform/client.ts

@@ -44,14 +44,8 @@ export class MinistryPlatformClient {
      * @throws Error if token refresh fails
      */
     public async ensureValidToken(): Promise<void> {
-        console.log("Checking token validity...");
-        console.log("Expires at: ", this.expiresAt);
-        console.log("Current time: ", new Date());
-
         // Check if token is expired or about to expire
         if (this.expiresAt < new Date()) {
-            console.log("Token expired, refreshing...");
-
             try {
                 // Get new access token using client credentials flow
                 const creds = await getClientCredentialsToken();
@@ -67,10 +61,8 @@ export class MinistryPlatformClient {
                 this.expiresAt = new Date(
                     Date.now() + Math.max(lifetimeMs - TOKEN_SAFETY_MARGIN, MIN_TOKEN_LIFETIME)
                 );
-
-                console.log("Token refreshed. Expires at: ", this.expiresAt);
             } catch (error) {
-                console.error("Failed to refresh token:", error);
+                console.error("Failed to refresh MP access token:", error);
                 throw error;
             }
         }

src/lib/providers/ministry-platform/services/procedure.service.ts

@@ -27,19 +27,15 @@ export class ProcedureService {
      * Executes the requested stored procedure retrieving parameters from the query string.
      */
     public async executeProcedure(
-        procedure: string, 
+        procedure: string,
         params?: QueryParams
     ): Promise<unknown[][]> {
         try {
             await this.client.ensureValidToken();
 
-            console.log('Executing procedure:', procedure);
-            console.log('Query Params:', params);
-
             const endpoint = `/procs/${encodeURIComponent(procedure)}`;
             const data = await this.client.getHttpClient().get<unknown[][]>(endpoint, params);
-
-            console.log('Procedure results:', data);
+
             return data;
         } catch (error) {
             console.error(`Error executing procedure ${procedure}:`, error);
@@ -51,23 +47,19 @@ export class ProcedureService {
      * Executes the requested stored procedure with provided parameters in the request body.
      */
     public async executeProcedureWithBody(
-        procedure: string, 
+        procedure: string,
         parameters: Record<string, unknown>
     ): Promise<unknown[][]> {
         try {
             await this.client.ensureValidToken();
 
-            console.log('Executing procedure with body:', procedure);
-            console.log('Parameters:', parameters);
-
             const endpoint = `/procs/${encodeURIComponent(procedure)}`;
             const data = await this.client.getHttpClient().post<unknown[][]>(endpoint, parameters);
-
-            console.log('Procedure results:', data);
+
             return data;
         } catch (error) {
             console.error(`Error executing procedure ${procedure}:`, error);
             throw error;
         }
     }
-}
+}

src/lib/providers/ministry-platform/services/table.service.test.ts

@@ -346,4 +346,67 @@ describe('TableService', () => {
       expect(mockClient.ensureValidToken).toHaveBeenCalledTimes(4);
     });
   });
+
+  describe('Logging safety (F5)', () => {
+    // Member PII and pastoral notes must never reach info-level logs. See
+    // .claude/references/auth.md § Logging policy.
+    let logSpy: ReturnType<typeof vi.spyOn>;
+    let errorSpy: ReturnType<typeof vi.spyOn>;
+
+    beforeEach(() => {
+      logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+      errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+    });
+
+    it('should not log anything when fetching records succeeds', async () => {
+      const sensitiveRecords = [
+        { Contact_ID: 1, Display_Name: 'Jane Doe', Notes: 'Struggling with grief after a loss' },
+      ];
+      (mockHttpClient.get as ReturnType<typeof vi.fn>).mockResolvedValueOnce(sensitiveRecords);
+
+      await tableService.getTableRecords('Contact_Log', {
+        $filter: "Notes LIKE '%grief%'",
+      });
+
+      expect(logSpy).not.toHaveBeenCalled();
+      expect(errorSpy).not.toHaveBeenCalled();
+    });
+
+    it('should not include the $filter value or record content in the failure log', async () => {
+      (mockHttpClient.get as ReturnType<typeof vi.fn>).mockRejectedValueOnce(
+        new Error('GET /tables/Contact_Log failed: 500 Internal Server Error')
+      );
+
+      await expect(
+        tableService.getTableRecords('Contact_Log', {
+          $filter: "Notes LIKE '%grief%' AND Display_Name = 'Jane Doe'",
+        })
+      ).rejects.toThrow();
+
+      expect(logSpy).not.toHaveBeenCalled();
+      expect(errorSpy).toHaveBeenCalledTimes(1);
+      const loggedArgs = errorSpy.mock.calls[0].map(String).join(' ');
+      expect(loggedArgs).not.toContain('grief');
+      expect(loggedArgs).not.toContain('Jane Doe');
+      expect(loggedArgs).not.toContain('$filter');
+    });
+
+    it('should not log request records when creating fails', async () => {
+      (mockHttpClient.post as ReturnType<typeof vi.fn>).mockRejectedValueOnce(
+        new Error('POST /tables/Contact_Log failed: 400 Bad Request')
+      );
+
+      await expect(
+        tableService.createTableRecords('Contact_Log', [
+          { Notes: 'Confidential pastoral note' },
+        ])
+      ).rejects.toThrow();
+
+      expect(logSpy).not.toHaveBeenCalled();
+      const loggedArgs = errorSpy.mock.calls
+        .map((args: unknown[]) => args.map(String).join(' '))
+        .join(' ');
+      expect(loggedArgs).not.toContain('Confidential pastoral note');
+    });
+  });
 });