fix: Normalize IP addresses to avoid validation errors (#12500)

* fix: Normalize IP addresses to avoid validation errors on audit columns

Koa's `ctx.request.ip` can yield values that fail Sequelize's `isIP`
validation (X-Forwarded-For chains, IPv6 zone identifiers, "unknown"
from misconfigured proxies). This drops the IP metadata silently
instead of raising a 500 on Event/User writes.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* test: Cover IP normalization on User setters

Reviewer feedback. Also switches the column-options `set` to TypeScript
get/set accessors — the original approach was shadowed by the class
field declaration and never actually fired, which the new tests would
have caught.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

server/models/Event.ts

@@ -20,6 +20,7 @@ import {
 import { globalEventQueue } from "../queues";
 import type { APIContext } from "../types";
 import { AuthenticationType } from "../types";
+import { normalizeIp } from "../utils/ip";
 import Collection from "./Collection";
 import Document from "./Document";
 import Team from "./Team";
@@ -75,10 +76,7 @@ class Event extends IdModel<
 
   @BeforeCreate
   static cleanupIp(model: Event) {
-    if (model.ip) {
-      // cleanup IPV6 representations of IPV4 addresses
-      model.ip = model.ip.replace(/^::ffff:/, "");
-    }
+    model.ip = normalizeIp(model.ip);
   }
 
   @AfterSave

server/models/User.test.ts

@@ -76,6 +76,30 @@ describe("user model", () => {
     });
   });
 
+  describe("ip setters", () => {
+    it("normalizes lastActiveIp and lastSignedInIp on assignment", async () => {
+      const user = await buildUser();
+
+      user.lastActiveIp = "::ffff:127.0.0.1";
+      user.lastSignedInIp = "203.0.113.1, 70.41.3.18";
+      await user.save({ hooks: false });
+
+      expect(user.lastActiveIp).toBe("127.0.0.1");
+      expect(user.lastSignedInIp).toBe("203.0.113.1");
+    });
+
+    it("nulls out invalid IP values without failing validation", async () => {
+      const user = await buildUser();
+
+      user.lastActiveIp = "unknown";
+      user.lastSignedInIp = "not-an-ip";
+      await expect(user.save({ hooks: false })).resolves.toBeDefined();
+
+      expect(user.lastActiveIp).toBeNull();
+      expect(user.lastSignedInIp).toBeNull();
+    });
+  });
+
   describe("destroy", () => {
     it("should clear PII", async () => {
       const user = await buildUser();

server/models/User.ts

@@ -54,6 +54,7 @@ import type { APIContext } from "@server/types";
 import { VerificationCode } from "@server/utils/VerificationCode";
 import parseAttachmentIds from "@server/utils/parseAttachmentIds";
 import { CacheHelper } from "@server/utils/CacheHelper";
+import { normalizeIp } from "@server/utils/ip";
 import { RedisPrefixHelper } from "@server/utils/RedisPrefixHelper";
 import { ValidationError } from "../errors";
 import Attachment from "./Attachment";
@@ -169,9 +170,15 @@ class User extends ParanoidModel<
   lastActiveAt: Date | null;
 
   @IsIP
-  @Column
   @SkipChangeset
-  lastActiveIp: string | null;
+  @Column(DataType.STRING)
+  get lastActiveIp(): string | null {
+    return this.getDataValue("lastActiveIp");
+  }
+
+  set lastActiveIp(value: string | null) {
+    this.setDataValue("lastActiveIp", normalizeIp(value));
+  }
 
   @IsDate
   @Column
@@ -179,9 +186,15 @@ class User extends ParanoidModel<
   lastSignedInAt: Date | null;
 
   @IsIP
-  @Column
   @SkipChangeset
-  lastSignedInIp: string | null;
+  @Column(DataType.STRING)
+  get lastSignedInIp(): string | null {
+    return this.getDataValue("lastSignedInIp");
+  }
+
+  set lastSignedInIp(value: string | null) {
+    this.setDataValue("lastSignedInIp", normalizeIp(value));
+  }
 
   @IsDate
   @Column

server/utils/ip.test.ts

@@ -0,0 +1,38 @@
+import { normalizeIp } from "./ip";
+
+describe("normalizeIp", () => {
+  it("returns null for nullish or empty input", () => {
+    expect(normalizeIp(null)).toBeNull();
+    expect(normalizeIp(undefined)).toBeNull();
+    expect(normalizeIp("")).toBeNull();
+    expect(normalizeIp("   ")).toBeNull();
+  });
+
+  it("returns valid IPv4 unchanged", () => {
+    expect(normalizeIp("192.168.1.1")).toBe("192.168.1.1");
+  });
+
+  it("returns valid IPv6 unchanged", () => {
+    expect(normalizeIp("2001:db8::1")).toBe("2001:db8::1");
+  });
+
+  it("strips ::ffff: IPv4-mapped IPv6 prefix", () => {
+    expect(normalizeIp("::ffff:127.0.0.1")).toBe("127.0.0.1");
+  });
+
+  it("strips IPv6 zone identifier", () => {
+    expect(normalizeIp("fe80::1%eth0")).toBe("fe80::1");
+  });
+
+  it("takes the first entry from an X-Forwarded-For chain", () => {
+    expect(normalizeIp("203.0.113.1, 70.41.3.18, 150.172.238.178")).toBe(
+      "203.0.113.1"
+    );
+  });
+
+  it("returns null for non-IP values", () => {
+    expect(normalizeIp("unknown")).toBeNull();
+    expect(normalizeIp("not-an-ip")).toBeNull();
+    expect(normalizeIp("999.999.999.999")).toBeNull();
+  });
+});

server/utils/ip.ts

@@ -0,0 +1,27 @@
+import net from "node:net";
+
+/**
+ * Normalize an IP address string for storage in audit columns.
+ *
+ * Handles common upstream-proxy artifacts that would otherwise fail
+ * Sequelize's `isIP` validation: IPv4-mapped IPv6 prefixes, IPv6 zone
+ * identifiers, and `X-Forwarded-For` chains. Returns `null` for any
+ * value that is not a valid IPv4 or IPv6 address after normalization.
+ *
+ * @param value the raw IP string (e.g. from `ctx.request.ip`).
+ * @returns a valid IP string, or `null`.
+ */
+export function normalizeIp(value: string | null | undefined): string | null {
+  if (!value || typeof value !== "string") {
+    return null;
+  }
+
+  let ip = value.split(",")[0]?.trim() ?? "";
+  ip = ip.replace(/^::ffff:/i, "");
+  const zoneIndex = ip.indexOf("%");
+  if (zoneIndex !== -1) {
+    ip = ip.slice(0, zoneIndex);
+  }
+
+  return net.isIP(ip) !== 0 ? ip : null;
+}