From 76a3ba4e83ced4b65f26c7b401cbf6bd76e45e58 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 27 May 2026 22:52:05 -0400 Subject: [PATCH] fix: Normalize IP addresses to avoid validation errors (#12500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 --------- Co-authored-by: Claude Opus 4.7 --- server/models/Event.ts | 6 ++---- server/models/User.test.ts | 24 ++++++++++++++++++++++++ server/models/User.ts | 21 +++++++++++++++++---- server/utils/ip.test.ts | 38 ++++++++++++++++++++++++++++++++++++++ server/utils/ip.ts | 27 +++++++++++++++++++++++++++ 5 files changed, 108 insertions(+), 8 deletions(-) create mode 100644 server/utils/ip.test.ts create mode 100644 server/utils/ip.ts diff --git a/server/models/Event.ts b/server/models/Event.ts index bd4b6de4d6..5f4eaaba27 100644 --- a/server/models/Event.ts +++ b/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 diff --git a/server/models/User.test.ts b/server/models/User.test.ts index ddb0b9a2bd..410a29406e 100644 --- a/server/models/User.test.ts +++ b/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(); diff --git a/server/models/User.ts b/server/models/User.ts index 06ae43fd52..8234c36658 100644 --- a/server/models/User.ts +++ b/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 diff --git a/server/utils/ip.test.ts b/server/utils/ip.test.ts new file mode 100644 index 0000000000..f5dab06f44 --- /dev/null +++ b/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(); + }); +}); diff --git a/server/utils/ip.ts b/server/utils/ip.ts new file mode 100644 index 0000000000..b583f400cd --- /dev/null +++ b/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; +}