diff --git a/packages/api/src/dto/application.test.ts b/packages/api/src/dto/application.test.ts index b91108b73..ed3ad7410 100644 --- a/packages/api/src/dto/application.test.ts +++ b/packages/api/src/dto/application.test.ts @@ -83,3 +83,24 @@ describe("applicationDto zero-argument inputs", () => { expect(applicationDto.tags.input.parse(undefined)).toEqual({}); }); }); + +// Bulk operations cap `ids` at 200 to bound memory/DB work from a single call. +describe("applicationDto bulk id caps", () => { + const idsOfLength = (n: number) => Array.from({ length: n }, (_, i) => String(i)); + + it("rejects a bulkDelete ids array over the cap", () => { + expect(applicationDto.bulkDelete.input.safeParse({ ids: idsOfLength(201) }).success).toBe(false); + }); + + it("accepts a bulkDelete ids array at the cap", () => { + expect(applicationDto.bulkDelete.input.safeParse({ ids: idsOfLength(200) }).success).toBe(true); + }); + + it("rejects a bulkUpdate ids array over the cap", () => { + expect(applicationDto.bulkUpdate.input.safeParse({ ids: idsOfLength(201) }).success).toBe(false); + }); + + it("accepts a bulkUpdate ids array at the cap", () => { + expect(applicationDto.bulkUpdate.input.safeParse({ ids: idsOfLength(200) }).success).toBe(true); + }); +}); diff --git a/packages/api/src/dto/application.ts b/packages/api/src/dto/application.ts index 0fc0753fb..9f9e26255 100644 --- a/packages/api/src/dto/application.ts +++ b/packages/api/src/dto/application.ts @@ -162,7 +162,7 @@ export const applicationDto = { // Table bulk actions: move stage, archive/unarchive, add tags across a selection. bulkUpdate: { input: z.object({ - ids: z.array(z.string()).min(1), + ids: z.array(z.string()).min(1).max(200, "Too many items in a single bulk operation"), status: applicationStatusSchema.optional(), archived: z.boolean().optional(), addTags: z.array(z.string()).optional(), @@ -171,7 +171,7 @@ export const applicationDto = { }, bulkDelete: { - input: z.object({ ids: z.array(z.string()).min(1) }), + input: z.object({ ids: z.array(z.string()).min(1).max(200, "Too many items in a single bulk operation") }), output: z.object({ deleted: z.number() }), }, diff --git a/packages/api/src/features/resume/service.test.ts b/packages/api/src/features/resume/service.test.ts new file mode 100644 index 000000000..016b7407a --- /dev/null +++ b/packages/api/src/features/resume/service.test.ts @@ -0,0 +1,341 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// Characterization tests for the resume service. The goal is to pin down CURRENT behavior +// (CRUD / lock / password / statistics branching) so later changes are deliberate. The DB +// layer and side-effecting helpers are mocked; the branching in service.ts is what's under test. + +const dbMock = vi.hoisted(() => ({ + select: vi.fn(), + insert: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + transaction: vi.fn(), +})); +const hashMock = vi.hoisted(() => vi.fn()); +const compareMock = vi.hoisted(() => vi.fn()); +const publishResumeUpdatedMock = vi.hoisted(() => vi.fn()); +const grantResumeAccessMock = vi.hoisted(() => vi.fn()); +const hasResumeAccessMock = vi.hoisted(() => vi.fn()); +const storageDeleteMock = vi.hoisted(() => vi.fn()); + +vi.mock("@reactive-resume/db/client", () => ({ db: dbMock })); +vi.mock("@reactive-resume/db/schema", () => ({ + resume: { + id: "id", + userId: "user_id", + slug: "slug", + name: "name", + tags: "tags", + data: "data", + isPublic: "is_public", + isLocked: "is_locked", + password: "password", + updatedAt: "updated_at", + createdAt: "created_at", + }, + resumeStatistics: { + resumeId: "resume_id", + views: "views", + downloads: "downloads", + lastViewedAt: "last_viewed_at", + lastDownloadedAt: "last_downloaded_at", + }, + resumeStatisticsDaily: { + resumeId: "resume_id", + date: "date", + views: "views", + downloads: "downloads", + }, + resumeVersion: { + id: "id", + resumeId: "resume_id", + userId: "user_id", + data: "data", + label: "label", + createdAt: "created_at", + }, + resumeAnalysis: { resumeId: "resume_id", analysis: "analysis" }, + user: { id: "id", username: "username" }, +})); +vi.mock("drizzle-orm", () => ({ + and: (...a: unknown[]) => a, + arrayContains: (...a: unknown[]) => a, + asc: (x: unknown) => x, + desc: (x: unknown) => x, + eq: (...a: unknown[]) => a, + gte: (...a: unknown[]) => a, + isNotNull: (...a: unknown[]) => a, + notInArray: (...a: unknown[]) => a, + sql: Object.assign((strings: TemplateStringsArray, ...values: unknown[]) => ({ strings, values }), { + join: (values: unknown[]) => values, + }), +})); +vi.mock("bcrypt", () => ({ hash: hashMock, compare: compareMock })); +vi.mock("./events", () => ({ publishResumeUpdated: publishResumeUpdatedMock })); +vi.mock("./access", () => ({ + grantResumeAccess: grantResumeAccessMock, + hasResumeAccess: hasResumeAccessMock, +})); +vi.mock("../storage/service", () => ({ + getStorageService: () => ({ delete: storageDeleteMock }), +})); + +const { resumeService } = await import("./service"); + +// A `db.update(...).set(...).where(...).returning(...)` chain that resolves to `rows`. +const createUpdateChain = (rows: unknown[]) => { + const returning = vi.fn(() => Promise.resolve(rows)); + const where = vi.fn(() => ({ returning })); + const set = vi.fn(() => ({ where })); + return { chain: { set }, set, where, returning }; +}; + +// A `db.select(...).from(...).where(...)` chain that resolves to `rows`. +const createSelectChain = (rows: unknown[]) => ({ + from: () => ({ where: () => Promise.resolve(rows) }), +}); + +beforeEach(() => { + dbMock.select.mockReset(); + dbMock.insert.mockReset(); + dbMock.update.mockReset(); + dbMock.delete.mockReset(); + dbMock.transaction.mockReset(); + hashMock.mockReset(); + compareMock.mockReset(); + publishResumeUpdatedMock.mockReset(); + grantResumeAccessMock.mockReset(); + hasResumeAccessMock.mockReset(); + storageDeleteMock.mockReset(); + hashMock.mockResolvedValue("hashed-password"); + publishResumeUpdatedMock.mockResolvedValue(undefined); + storageDeleteMock.mockResolvedValue(true); +}); + +it("imports", () => { + expect(resumeService).toBeDefined(); +}); + +describe("update", () => { + it("throws RESUME_LOCKED when the pre-read reports the resume is locked", async () => { + dbMock.select.mockReturnValueOnce(createSelectChain([{ isLocked: true }])); + + await expect(resumeService.update({ id: "r1", userId: "u1", name: "New" })).rejects.toMatchObject({ + code: "RESUME_LOCKED", + }); + }); + + it("returns the updated row on success", async () => { + dbMock.select.mockReturnValueOnce(createSelectChain([{ isLocked: false }])); + const row = { + id: "r1", + name: "New", + slug: "slug", + tags: [], + data: {}, + isPublic: false, + isLocked: false, + updatedAt: new Date("2026-01-01T00:00:00Z"), + hasPassword: false, + }; + dbMock.update.mockReturnValueOnce(createUpdateChain([row]).chain); + + const result = await resumeService.update({ id: "r1", userId: "u1", name: "New" }); + + expect(result).toEqual(row); + expect(publishResumeUpdatedMock).toHaveBeenCalledTimes(1); + }); + + it("throws NOT_FOUND when the UPDATE ... RETURNING matches no row", async () => { + dbMock.select.mockReturnValueOnce(createSelectChain([{ isLocked: false }])); + dbMock.update.mockReturnValueOnce(createUpdateChain([]).chain); + + await expect(resumeService.update({ id: "r1", userId: "u1", name: "New" })).rejects.toMatchObject({ + code: "NOT_FOUND", + }); + }); + + it("maps a resume_slug_user_id_unique violation to RESUME_SLUG_ALREADY_EXISTS", async () => { + dbMock.select.mockReturnValueOnce(createSelectChain([{ isLocked: false }])); + dbMock.update.mockReturnValueOnce({ + set: () => ({ + where: () => ({ + returning: () => { + const error = new Error("duplicate key") as Error & { cause: { constraint: string } }; + error.cause = { constraint: "resume_slug_user_id_unique" }; + return Promise.reject(error); + }, + }), + }), + }); + + await expect(resumeService.update({ id: "r1", userId: "u1", slug: "taken" })).rejects.toMatchObject({ + code: "RESUME_SLUG_ALREADY_EXISTS", + }); + }); +}); + +describe("setLocked", () => { + it("resolves and notifies on success (mutation: lock)", async () => { + dbMock.update.mockReturnValueOnce( + createUpdateChain([{ id: "r1", updatedAt: new Date("2026-01-01T00:00:00Z") }]).chain, + ); + + await expect(resumeService.setLocked({ id: "r1", userId: "u1", isLocked: true })).resolves.toBeUndefined(); + + expect(publishResumeUpdatedMock).toHaveBeenCalledTimes(1); + expect(publishResumeUpdatedMock).toHaveBeenCalledWith(expect.objectContaining({ mutation: "lock" })); + }); + + // Plan 003: no matching row now rejects with NOT_FOUND (previously a silent resolve). + it("throws NOT_FOUND when no row matches, without notifying", async () => { + dbMock.update.mockReturnValueOnce(createUpdateChain([]).chain); + + await expect(resumeService.setLocked({ id: "r1", userId: "u1", isLocked: true })).rejects.toMatchObject({ + code: "NOT_FOUND", + }); + expect(publishResumeUpdatedMock).not.toHaveBeenCalled(); + }); +}); + +describe("setPassword", () => { + it("hashes the password then resolves and notifies on success (mutation: password)", async () => { + dbMock.update.mockReturnValueOnce( + createUpdateChain([{ id: "r1", updatedAt: new Date("2026-01-01T00:00:00Z") }]).chain, + ); + + await expect(resumeService.setPassword({ id: "r1", userId: "u1", password: "secret" })).resolves.toBeUndefined(); + + expect(hashMock).toHaveBeenCalledWith("secret", 10); + expect(publishResumeUpdatedMock).toHaveBeenCalledTimes(1); + expect(publishResumeUpdatedMock).toHaveBeenCalledWith(expect.objectContaining({ mutation: "password" })); + }); + + // Plan 003: no matching row now rejects with NOT_FOUND (previously a silent resolve). + it("throws NOT_FOUND when no row matches, without notifying", async () => { + dbMock.update.mockReturnValueOnce(createUpdateChain([]).chain); + + await expect(resumeService.setPassword({ id: "r1", userId: "u1", password: "secret" })).rejects.toMatchObject({ + code: "NOT_FOUND", + }); + expect(publishResumeUpdatedMock).not.toHaveBeenCalled(); + }); +}); + +describe("removePassword", () => { + it("resolves and notifies on success (mutation: password)", async () => { + dbMock.update.mockReturnValueOnce( + createUpdateChain([{ id: "r1", updatedAt: new Date("2026-01-01T00:00:00Z") }]).chain, + ); + + await expect(resumeService.removePassword({ id: "r1", userId: "u1" })).resolves.toBeUndefined(); + + expect(publishResumeUpdatedMock).toHaveBeenCalledTimes(1); + expect(publishResumeUpdatedMock).toHaveBeenCalledWith(expect.objectContaining({ mutation: "password" })); + }); + + // Plan 003: no matching row now rejects with NOT_FOUND (previously a silent resolve). + it("throws NOT_FOUND when no row matches, without notifying", async () => { + dbMock.update.mockReturnValueOnce(createUpdateChain([]).chain); + + await expect(resumeService.removePassword({ id: "r1", userId: "u1" })).rejects.toMatchObject({ + code: "NOT_FOUND", + }); + expect(publishResumeUpdatedMock).not.toHaveBeenCalled(); + }); +}); + +describe("verifyPassword", () => { + it("throws INVALID_PASSWORD when no matching row is found", async () => { + dbMock.select.mockReturnValueOnce({ + from: () => ({ innerJoin: () => ({ where: () => Promise.resolve([]) }) }), + }); + + await expect(resumeService.verifyPassword({ slug: "s", username: "u", password: "p" })).rejects.toMatchObject({ + code: "INVALID_PASSWORD", + }); + }); + + it("throws INVALID_PASSWORD when bcrypt.compare returns false", async () => { + dbMock.select.mockReturnValueOnce({ + from: () => ({ innerJoin: () => ({ where: () => Promise.resolve([{ id: "r1", password: "hash" }]) }) }), + }); + compareMock.mockResolvedValueOnce(false); + + await expect(resumeService.verifyPassword({ slug: "s", username: "u", password: "p" })).rejects.toMatchObject({ + code: "INVALID_PASSWORD", + }); + }); + + it("returns true and grants access when bcrypt.compare returns true", async () => { + dbMock.select.mockReturnValueOnce({ + from: () => ({ innerJoin: () => ({ where: () => Promise.resolve([{ id: "r1", password: "hash" }]) }) }), + }); + compareMock.mockResolvedValueOnce(true); + const responseHeaders = new Headers(); + + const result = await resumeService.verifyPassword({ + slug: "s", + username: "u", + password: "p", + responseHeaders, + }); + + expect(result).toBe(true); + expect(grantResumeAccessMock).toHaveBeenCalledWith(responseHeaders, "r1", "hash"); + }); +}); + +describe("delete", () => { + const runTransaction = (tx: unknown) => { + dbMock.transaction.mockImplementationOnce(async (cb: (tx: unknown) => Promise) => cb(tx)); + }; + + it("throws NOT_FOUND when the row is missing", async () => { + runTransaction({ + select: () => createSelectChain([]), + }); + + await expect(resumeService.delete({ id: "r1", userId: "u1" })).rejects.toMatchObject({ code: "NOT_FOUND" }); + }); + + it("throws RESUME_LOCKED when the row is locked", async () => { + runTransaction({ + select: () => createSelectChain([{ isLocked: true }]), + }); + + await expect(resumeService.delete({ id: "r1", userId: "u1" })).rejects.toMatchObject({ + code: "RESUME_LOCKED", + }); + }); + + it("deletes storage for screenshot and pdf keys on success", async () => { + const deleteWhere = vi.fn(() => Promise.resolve()); + runTransaction({ + select: () => createSelectChain([{ isLocked: false }]), + delete: () => ({ where: deleteWhere }), + }); + + await resumeService.delete({ id: "r1", userId: "u1" }); + + expect(deleteWhere).toHaveBeenCalledTimes(1); + expect(storageDeleteMock).toHaveBeenCalledWith("uploads/u1/screenshots/r1"); + expect(storageDeleteMock).toHaveBeenCalledWith("uploads/u1/pdfs/r1"); + expect(publishResumeUpdatedMock).toHaveBeenCalledWith(expect.objectContaining({ mutation: "delete" })); + }); +}); + +describe("statistics.increment", () => { + it("writes both resumeStatistics and resumeStatisticsDaily inside one transaction", async () => { + const values = vi.fn(() => ({ onConflictDoUpdate: vi.fn(() => Promise.resolve()) })); + const txInsert = vi.fn(() => ({ values })); + dbMock.transaction.mockImplementationOnce(async (cb: (tx: unknown) => Promise) => + cb({ insert: txInsert }), + ); + + await resumeService.statistics.increment({ id: "r1", views: true }); + + expect(dbMock.transaction).toHaveBeenCalledTimes(1); + expect(txInsert).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/api/src/features/resume/service.ts b/packages/api/src/features/resume/service.ts index 36d847f49..909b13259 100644 --- a/packages/api/src/features/resume/service.ts +++ b/packages/api/src/features/resume/service.ts @@ -17,6 +17,7 @@ import { getStorageService } from "../storage/service"; import { grantResumeAccess, hasResumeAccess } from "./access"; import { assertCanView, isOwner, redactResumeForViewer, shouldCountForStatistics } from "./access-policy"; import { publishResumeUpdated } from "./events"; +import { clientKeyFromHeaders, shouldCountView } from "./view-dedup"; type DbOrTx = typeof db | Parameters[0]>[0]; @@ -503,7 +504,10 @@ export const resumeService = { } if (shouldCountForStatistics(resume, viewer)) { - await resumeService.statistics.increment({ id: resume.id, views: true }); + const key = `${resume.id}:${clientKeyFromHeaders(input.requestHeaders)}`; + if (shouldCountView(key, Date.now())) { + await resumeService.statistics.increment({ id: resume.id, views: true }); + } } return toSharedResumeResponse(redactResumeForViewer(resume, isOwner(resume, viewer)), resume.hasPassword); @@ -667,7 +671,7 @@ export const resumeService = { .where(and(eq(schema.resume.id, input.id), eq(schema.resume.userId, input.userId))) .returning({ id: schema.resume.id, updatedAt: schema.resume.updatedAt }); - if (!resume) return; + if (!resume) throw new ORPCError("NOT_FOUND"); await notifyResumeUpdated({ type: "resume.updated", @@ -687,7 +691,7 @@ export const resumeService = { .where(and(eq(schema.resume.id, input.id), eq(schema.resume.userId, input.userId))) .returning({ id: schema.resume.id, updatedAt: schema.resume.updatedAt }); - if (!resume) return; + if (!resume) throw new ORPCError("NOT_FOUND"); await notifyResumeUpdated({ type: "resume.updated", @@ -730,7 +734,7 @@ export const resumeService = { .where(and(eq(schema.resume.id, input.id), eq(schema.resume.userId, input.userId))) .returning({ id: schema.resume.id, updatedAt: schema.resume.updatedAt }); - if (!resume) return; + if (!resume) throw new ORPCError("NOT_FOUND"); await notifyResumeUpdated({ type: "resume.updated", diff --git a/packages/api/src/features/resume/view-dedup.test.ts b/packages/api/src/features/resume/view-dedup.test.ts new file mode 100644 index 000000000..a823689c7 --- /dev/null +++ b/packages/api/src/features/resume/view-dedup.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from "vitest"; +import { clientKeyFromHeaders, shouldCountView } from "./view-dedup"; + +// `seen` is module-level state shared across tests, so each test uses a unique key. +const WINDOW_MS = 60 * 60 * 1000; + +describe("shouldCountView", () => { + it("counts the first view, skips repeats within the window, counts again after it", () => { + const key = "resume-1:viewer-a"; + const t0 = 1_000_000; + + expect(shouldCountView(key, t0)).toBe(true); + expect(shouldCountView(key, t0 + 1)).toBe(false); + expect(shouldCountView(key, t0 + WINDOW_MS - 1)).toBe(false); + // Once now is past the window, the same key counts again. + expect(shouldCountView(key, t0 + WINDOW_MS + 1)).toBe(true); + }); + + it("treats different keys independently", () => { + const t0 = 2_000_000; + + expect(shouldCountView("resume-2:viewer-a", t0)).toBe(true); + expect(shouldCountView("resume-2:viewer-b", t0)).toBe(true); + expect(shouldCountView("resume-2:viewer-a", t0 + 1)).toBe(false); + }); +}); + +describe("clientKeyFromHeaders", () => { + it("derives distinct keys from distinct trusted-IP headers", () => { + const a = clientKeyFromHeaders(new Headers({ "X-Forwarded-For": "1.1.1.1" })); + const b = clientKeyFromHeaders(new Headers({ "X-Forwarded-For": "2.2.2.2" })); + + expect(a).toBe("ip:1.1.1.1"); + expect(a).not.toBe(b); + }); + + it("uses the first IP from a comma-delimited proxy chain", () => { + const key = clientKeyFromHeaders(new Headers({ "X-Forwarded-For": "3.3.3.3, 10.0.0.1" })); + expect(key).toBe("ip:3.3.3.3"); + }); + + it("falls back to a stable user-agent fingerprint when no trusted IP header is present", () => { + const headers = new Headers({ "user-agent": "Mozilla/5.0", "accept-language": "en-US,en" }); + + const first = clientKeyFromHeaders(headers); + const second = clientKeyFromHeaders(headers); + + expect(first).toBe(second); + expect(first.startsWith("fp:")).toBe(true); + // A different UA yields a different fallback key. + expect(clientKeyFromHeaders(new Headers({ "user-agent": "curl/8" }))).not.toBe(first); + }); +}); diff --git a/packages/api/src/features/resume/view-dedup.ts b/packages/api/src/features/resume/view-dedup.ts new file mode 100644 index 000000000..a699931a7 --- /dev/null +++ b/packages/api/src/features/resume/view-dedup.ts @@ -0,0 +1,45 @@ +import { TRUSTED_IP_HEADERS } from "@reactive-resume/utils/rate-limit"; + +// ponytail: in-memory per-process dedup window. Single-instance is the default deploy; for +// multi-instance, swap the Map for a Redis SETNX+EXPIRE keyed the same way (REDIS_URL already +// exists in env). Upgrade only if you scale out — each instance dedups independently otherwise. +const WINDOW_MS = 60 * 60 * 1000; // 1 hour +const MAX_ENTRIES = 50_000; // bound the Map; prune expired entries before growing past this. + +const seen = new Map(); // key -> expiry timestamp + +/** + * Returns `true` at most once per `key` per window. `now` is a parameter (not `Date.now()`) + * so callers stay testable with a driven clock. + */ +export function shouldCountView(key: string, now: number): boolean { + const expiry = seen.get(key); + if (expiry !== undefined && expiry > now) return false; + + if (seen.size >= MAX_ENTRIES) { + for (const [k, exp] of seen) { + if (exp <= now) seen.delete(k); + } + } + + seen.set(key, now + WINDOW_MS); + return true; +} + +// Mirrors the rate-limit middleware's client-key derivation so dedup and rate limiting agree on +// "who is this viewer": trusted proxy IP first, then a user-agent + language fingerprint fallback. +export function clientKeyFromHeaders(headers: Headers): string { + for (const headerName of TRUSTED_IP_HEADERS) { + const raw = headers.get(headerName)?.trim(); + if (!raw) continue; + + // Some proxies provide a comma-delimited chain; the first item is the original client. + const ip = raw.split(",")[0]?.trim(); + if (ip) return `ip:${ip}`; + } + + const userAgent = headers.get("user-agent")?.trim() ?? "unknown"; + const language = headers.get("accept-language")?.split(",")[0]?.trim() ?? "none"; + + return `fp:${userAgent.slice(0, 64)}:${language.slice(0, 16)}`; +}