From 31b8e85c44f2bb2547d8e5c541e98102d6c66e32 Mon Sep 17 00:00:00 2001
From: Shawn Blackmore <blackmore.technology.group@gmail.com>
Date: Wed, 30 Sep 2026 08:59:47 -0700
Subject: [PATCH] fix(users): validate profile updates at service boundary

---
 src/modules/users/user.service.ts             | 16 +++++-
 .../users/profile-service-validation.test.ts  | 55 +++++++++++++++++++
 2 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 tests/unit/users/profile-service-validation.test.ts

diff --git a/src/modules/users/user.service.ts b/src/modules/users/user.service.ts
index 75ad89a..f4af9a6 100644
--- a/src/modules/users/user.service.ts
+++ b/src/modules/users/user.service.ts
@@ -31,6 +31,7 @@ import {
   type CandidateCourse,
   type ScoredCourse,
 } from "./recommendations.js";
+import { updateProfileSchema } from "./user.types.js";
 import type {
   ActivityQuery,
   AvatarUpload,
@@ -86,9 +87,22 @@ export class UserService {
     userId: string,
     data: UpdateProfileBody,
   ): Promise<UserProfile> {
+    // Re-validate at the service boundary so direct/internal callers receive
+    // the same constraints and sanitization as HTTP callers.
+    const parsed = updateProfileSchema.safeParse(data);
+    if (!parsed.success) {
+      const errors: Record<string, string[]> = {};
+      for (const issue of parsed.error.issues) {
+        const field = issue.path[0];
+        const key = typeof field === "string" ? field : "profile";
+        (errors[key] ??= []).push(issue.message);
+      }
+      throw new ValidationError(errors);
+    }
+
     const [updated] = await db
       .update(users)
-      .set({ ...data, updatedAt: new Date() })
+      .set({ ...parsed.data, updatedAt: new Date() })
       .where(eq(users.id, userId))
       .returning();
 
diff --git a/tests/unit/users/profile-service-validation.test.ts b/tests/unit/users/profile-service-validation.test.ts
new file mode 100644
index 0000000..295ea76
--- /dev/null
+++ b/tests/unit/users/profile-service-validation.test.ts
@@ -0,0 +1,55 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const { mockUpdate } = vi.hoisted(() => ({ mockUpdate: vi.fn() }));
+
+vi.mock("../../../src/config/database.js", () => ({
+  db: {
+    update: mockUpdate,
+    query: { users: { findFirst: vi.fn() } },
+  },
+}));
+
+vi.mock("../../../src/cache/index.js", () => ({
+  cacheGet: vi.fn(),
+  cacheSet: vi.fn(),
+  cacheDel: vi.fn(),
+  cacheInvalidatePattern: vi.fn(),
+  cacheKey: vi.fn((...parts: string[]) => parts.join(":")),
+  cacheKeyPattern: vi.fn((...parts: string[]) => parts.join(":")),
+}));
+
+vi.mock("../../../src/config/index.js", () => ({
+  config: { AVATAR_UPLOAD_MAX_BYTES: 5_000_000, AVATAR_UPLOAD_DIR: "uploads" },
+}));
+
+import { userService } from "../../../src/modules/users/user.service.js";
+import { ValidationError } from "../../../src/utils/errors.js";
+
+describe("UserService.updateProfile service-boundary validation (#534)", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+  it("rejects over-length profile data before touching the database", async () => {
+    const result = userService.updateProfile("user-1", {
+      displayName: "a".repeat(101),
+    } as never);
+
+    await expect(result).rejects.toBeInstanceOf(ValidationError);
+    await expect(result).rejects.toMatchObject({
+      errors: { displayName: expect.any(Array) },
+    });
+    expect(mockUpdate).not.toHaveBeenCalled();
+  });
+
+  it("rejects null for string fields when called outside the route layer", async () => {
+    const result = userService.updateProfile("user-1", {
+      language: null,
+    } as never);
+
+    await expect(result).rejects.toBeInstanceOf(ValidationError);
+    await expect(result).rejects.toMatchObject({
+      errors: { language: expect.any(Array) },
+    });
+    expect(mockUpdate).not.toHaveBeenCalled();
+  });
+});
\ No newline at end of file