fix(resume): reject invalid submitted write values (#3413)

This commit is contained in:
Amruth Pillai
2026-09-05 08:51:55 -07:00
committed by GitHub
parent 5a6f5d4d68
commit 779ea5cb4a
12 changed files with 301 additions and 10 deletions
+13
View File
@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
import z from "zod";
import { defaultResumeData } from "@reactive-resume/schema/resume/default";
import { createResumeDataJsonSchema } from "@reactive-resume/schema/resume/json-schema";
import { writableResumeDataSchema } from "@reactive-resume/schema/resume/write";
type GeneratedSpecView = {
components?: { schemas?: Record<string, unknown> };
@@ -138,6 +139,18 @@ describe("generateOpenApiSpec", () => {
expect(schema.safeParse(mismatched).success).toBe(false);
});
it("enforces the same submitted bounds as the published request schema", async () => {
const spec = (await generateSpec()) as GeneratedSpecView;
const published = z.fromJSONSchema(spec.components?.schemas?.ResumeData as Parameters<typeof z.fromJSONSchema>[0]);
for (const marginX of [0, 100, -1, 500]) {
const data = structuredClone(defaultResumeData);
data.metadata.page.marginX = marginX;
const expected = marginX === 0 || marginX === 100;
expect(published.safeParse(data).success).toBe(expected);
expect(writableResumeDataSchema.safeParse(data).success).toBe(expected);
}
});
it("does not publish impossible request schemas", async () => {
const spec = (await generateSpec()) as GeneratedSpecView;
+3 -1
View File
@@ -5,6 +5,7 @@ import { downloadResumePdfProcedure } from "@reactive-resume/api/features/resume
import router from "@reactive-resume/api/routers";
import { resumeDataSchema } from "@reactive-resume/schema/resume/data";
import { createResumeDataJsonSchema } from "@reactive-resume/schema/resume/json-schema";
import { writableResumeDataSchema } from "@reactive-resume/schema/resume/write";
export const openAPIRouter = {
...router,
@@ -17,6 +18,7 @@ export const openAPIRouter = {
const { $schema: _dialect, ...resumeDataInputSchema } = createResumeDataJsonSchema();
type ResumeDataInputJsonSchema = Parameters<typeof JSON_SCHEMA_INPUT_REGISTRY.add<typeof resumeDataSchema>>[1];
JSON_SCHEMA_INPUT_REGISTRY.add(resumeDataSchema, resumeDataInputSchema as unknown as ResumeDataInputJsonSchema);
JSON_SCHEMA_INPUT_REGISTRY.add(writableResumeDataSchema, resumeDataInputSchema as unknown as ResumeDataInputJsonSchema);
const importResumeInputSchema = openAPIRouter.resume.import["~orpc"].inputSchema;
if (importResumeInputSchema) {
JSON_SCHEMA_INPUT_REGISTRY.add(importResumeInputSchema, {
@@ -110,7 +112,7 @@ export async function generateOpenApiSpec({ appUrl, version }: GenerateOpenApiSp
},
externalDocs: { url: "https://docs.rxresu.me", description: "Reactive Resume Documentation" },
commonSchemas: {
ResumeData: { schema: resumeDataSchema, strategy: "input" },
ResumeData: { schema: writableResumeDataSchema, strategy: "input" },
},
components: {
securitySchemes: {
+14
View File
@@ -155,3 +155,17 @@ describe("resume DTO output validation", () => {
expect(resumeDto.restoreVersion.output.parse(resume)).toEqual(resume);
});
});
describe("resume DTO write bounds", () => {
it.each(["update", "import"] as const)("rejects invalid template in %s input before normalization", (operation) => {
const data = { ...defaultResumeData, metadata: { ...defaultResumeData.metadata, template: "unknown-template" } };
expect(resumeDto[operation].input.safeParse({ id: "resume-id", data }).success).toBe(false);
});
it("continues to accept metadata-only updates without resume data", () => {
expect(resumeDto.update.input.parse({ id: "resume-id", name: "Renamed" })).toEqual({
id: "resume-id",
name: "Renamed",
});
});
});
+3 -2
View File
@@ -3,6 +3,7 @@ import z from "zod";
import * as schema from "@reactive-resume/db/schema";
import { jsonPatchOperationSchema } from "@reactive-resume/resume/patch";
import { resumeDataSchema } from "@reactive-resume/schema/resume/data";
import { writableResumeDataSchema } from "@reactive-resume/schema/resume/write";
const resumeSchema = createSelectSchema(schema.resume, {
id: z.string().describe("The ID of the resume."),
@@ -54,7 +55,7 @@ export const resumeDto = {
},
import: {
input: z.object({ data: resumeDataSchema }),
input: z.object({ data: writableResumeDataSchema }),
output: z.string().describe("The ID of the imported resume."),
},
@@ -62,7 +63,7 @@ export const resumeDto = {
input: resumeSchema
.pick({ name: true, slug: true, tags: true, data: true, isPublic: true })
.partial()
.extend({ id: z.string() }),
.extend({ id: z.string(), data: writableResumeDataSchema.optional() }),
output: resumeSchema.omit({ password: true, userId: true, createdAt: true }).extend({ hasPassword: z.boolean() }),
},
@@ -6,6 +6,8 @@ import { defaultResumeData } from "@reactive-resume/schema/resume/default";
const mocks = vi.hoisted(() => ({
create: vi.fn(),
getById: vi.fn(),
update: vi.fn(),
snapshot: vi.fn(),
}));
vi.mock("../../context", async () => {
@@ -23,6 +25,8 @@ vi.mock("./service", () => ({
resumeService: {
create: mocks.create,
getById: mocks.getById,
update: mocks.update,
versions: { snapshot: mocks.snapshot },
},
}));
@@ -73,3 +77,29 @@ describe("resume duplicate route", () => {
expect(mocks.create).not.toHaveBeenCalled();
});
});
describe("resume write route validation", () => {
beforeEach(() => {
vi.clearAllMocks();
mocks.create.mockResolvedValue("resume-id");
mocks.update.mockResolvedValue({});
mocks.snapshot.mockResolvedValue(undefined);
});
it.each(["import", "update"] as const)("rejects invalid %s input before calling persistence", async (operation) => {
const client = createRouterClient(crudRouter, {
context: { locale: "en-US", reqHeaders: new Headers(), user: { id: "user-id" } } as never,
});
const data = structuredClone(defaultResumeData);
data.metadata.page.marginX = 500;
const result = operation === "import" ? client.import({ data }) : client.update({ id: "resume-id", data });
const error = await result.catch((caught: unknown) => caught);
expect(error).toMatchObject({ code: "BAD_REQUEST", status: 400 });
expect(error).toHaveProperty("cause.issues.0.path", ["data", "metadata", "page", "marginX"]);
expect(mocks.create).not.toHaveBeenCalled();
expect(mocks.update).not.toHaveBeenCalled();
expect(mocks.snapshot).not.toHaveBeenCalled();
});
});
@@ -1,7 +1,9 @@
import { describe, expect, it } from "vitest";
import { set } from "es-toolkit/compat";
import { SEMANTIC_CSS_LIMITS_V1 } from "@reactive-resume/resume/stylesheet";
import { defaultResumeData } from "@reactive-resume/schema/resume/default";
import { parseWritableResumeData } from "./resume-data-validation";
import { sampleResumeData } from "@reactive-resume/schema/resume/sample";
import { parseStoredResumeData, parseWritableResumeData } from "./resume-data-validation";
describe("parseWritableResumeData", () => {
it("rejects stylesheet source above the Semantic CSS byte limit", () => {
@@ -16,3 +18,48 @@ describe("parseWritableResumeData", () => {
);
});
});
const invalidBounds = [
["metadata.template", "unknown-template"],
["metadata.page.format", "a3"],
["metadata.page.marginX", 500],
["metadata.page.marginY", -1],
["metadata.typography.body.fontSize", 999],
["metadata.typography.heading.fontSize", 5],
["metadata.typography.body.lineHeight", 5],
["metadata.typography.heading.lineHeight", 0.1],
["metadata.typography.body.fontWeights", ["950"]],
["metadata.layout.sidebarWidth", 51],
["summary.columns", 7],
["sections.experience.columns", 1.5],
["sections.skills.items.0.level", 6],
["sections.languages.items.0.level", -1],
] as const;
describe("strict write bounds", () => {
it.each(invalidBounds)("rejects %s instead of applying a fallback", (path, value) => {
const data = structuredClone(sampleResumeData);
set(data, path, value);
expect(() => parseWritableResumeData(data)).toThrowError(
expect.objectContaining({ code: "BAD_REQUEST", status: 400 }),
);
});
it("keeps tolerant normalization for stored documents", () => {
const data = structuredClone(defaultResumeData);
set(data, "metadata.template", "retired-template");
data.metadata.page.marginX = 500;
expect(parseStoredResumeData(data).metadata).toMatchObject({ template: "onyx", page: { marginX: 14 } });
});
it("preserves defaults for omitted fields from older clients", () => {
const data = structuredClone(defaultResumeData);
Reflect.deleteProperty(data.summary, "keepTogether");
Reflect.deleteProperty(data.metadata.page, "hideSectionIcons");
Reflect.deleteProperty(data.metadata.typography.body, "fontSize");
expect(parseWritableResumeData(data)).toMatchObject({
summary: { keepTogether: false },
metadata: { page: { hideSectionIcons: true }, typography: { body: { fontSize: 11 } } },
});
});
});
@@ -2,10 +2,11 @@ import type { ResumeData } from "@reactive-resume/schema/resume/data";
import { ORPCError } from "@orpc/client";
import { SEMANTIC_CSS_LIMITS_V1 } from "@reactive-resume/resume/stylesheet";
import { parseResumeData } from "@reactive-resume/schema/resume/data";
import { parseResumeDataForWrite } from "@reactive-resume/schema/resume/write";
function parseApiResumeData(data: unknown, code: "BAD_REQUEST" | "INTERNAL_SERVER_ERROR", message: string): ResumeData {
try {
const parsed = parseResumeData(data);
const parsed = code === "BAD_REQUEST" ? parseResumeDataForWrite(data) : parseResumeData(data);
const source = parsed.metadata.stylesheet?.source.text;
if (source !== undefined && new TextEncoder().encode(source).byteLength > SEMANTIC_CSS_LIMITS_V1.maxSourceBytes) {
throw new Error("The stylesheet source exceeds the Semantic CSS byte limit.");
@@ -237,6 +237,17 @@ it("imports", () => {
});
describe("create", () => {
it("rejects out-of-range values before creating any record", async () => {
const data = structuredClone(defaultResumeData);
data.metadata.page.marginX = 500;
dbMock.insert.mockReturnValue({ values: vi.fn(() => Promise.resolve()) });
await expect(
resumeService.create({ userId: "u1", name: "Resume", slug: "resume", tags: [], locale: "en-US", data }),
).rejects.toMatchObject({ code: "BAD_REQUEST", status: 400 });
expect(dbMock.insert).not.toHaveBeenCalled();
expect(publishResumeUpdatedMock).not.toHaveBeenCalled();
});
it("copies stylesheet content", async () => {
const data = createSemanticResumeData();
const values = vi.fn((_input: unknown) => Promise.resolve());
@@ -545,6 +556,22 @@ describe("update", () => {
);
});
it("rejects out-of-range PUT data before any update or notification", async () => {
const data = structuredClone(defaultResumeData);
data.metadata.typography.body.fontSize = 999;
const select = createLockedSelectChain([{ data: defaultResumeData, isLocked: false }]);
const update = createUpdateChain([createResumeRow(defaultResumeData)]);
dbMock.transaction.mockImplementationOnce(async (callback: (tx: unknown) => Promise<unknown>) =>
callback({ select: () => select.chain, update: () => update.chain }),
);
await expect(resumeService.update({ id: "r1", userId: "u1", data, skipAutoSnapshot: true })).rejects.toMatchObject({
code: "BAD_REQUEST",
status: 400,
});
expect(update.set).not.toHaveBeenCalled();
expect(publishResumeUpdatedMock).not.toHaveBeenCalled();
});
it("rejects renderer-unsafe data before updating the JSONB column", async () => {
const select = createLockedSelectChain([{ data: defaultResumeData, isLocked: false, updatedAt: new Date() }]);
const update = createUpdateChain([createResumeRow(defaultResumeData)]);
@@ -608,6 +635,10 @@ describe("patch", () => {
const lockedSelect = createLockedSelectChain([existing]);
const row = createResumeRow(existing.data, existing.updatedAt);
const update = createUpdateChain([row]);
update.returning.mockImplementation(() => {
const written = update.set.mock.calls.at(-1)?.[0] as { data: ResumeData };
return Promise.resolve([{ ...row, data: written.data }]);
});
const versionSelect = {
from: () => ({ where: () => ({ orderBy: () => ({ limit: () => [] }) }) }),
};
@@ -621,6 +652,49 @@ describe("patch", () => {
return { tx, update };
};
it.each([
["/metadata/template", "unknown-template"],
["/metadata/page/format", "a3"],
["/metadata/page/marginX", 500],
["/metadata/typography/body/fontSize", 999],
] as const)("rejects an invalid patch at %s atomically", async (path, value) => {
const data = structuredClone(defaultResumeData);
data.metadata.template = "chikorita";
data.metadata.page.marginX = 40;
const before = structuredClone(data);
const { tx, update } = createPatchTx({ data, isLocked: false, updatedAt: new Date() });
await expect(
resumeService.patchInTransaction(tx as never, {
id: "r1",
userId: "u1",
operations: [
{ op: "replace", path: "/basics/name", value: "Must not persist" },
{ op: "replace", path, value },
],
}),
).rejects.toMatchObject({ code: "INVALID_PATCH_OPERATIONS", status: 400 });
expect(update.set).not.toHaveBeenCalled();
expect(tx.insert).not.toHaveBeenCalled();
expect(data).toEqual(before);
});
it("normalizes stored legacy data before validating newly submitted patch values", async () => {
const data = structuredClone(defaultResumeData);
data.metadata.page.marginX = 500;
const { tx, update } = createPatchTx({ data, isLocked: false, updatedAt: new Date() });
await resumeService.patchInTransaction(tx as never, {
id: "r1",
userId: "u1",
operations: [{ op: "replace", path: "/basics/name", value: "Ada" }],
});
expect(update.set).toHaveBeenCalledWith({
data: expect.objectContaining({
basics: expect.objectContaining({ name: "Ada" }),
metadata: expect.objectContaining({ page: expect.objectContaining({ marginX: 14 }) }),
}),
});
});
it("persists stylesheet source through the ordinary patch path", async () => {
const data = createSemanticResumeData();
const { tx, update } = createPatchTx({
+1 -1
View File
@@ -142,7 +142,7 @@ async function applyResumePatchTx(
let patchedData: ResumeData;
try {
patchedData = applyResumePatches(existing.data, input.operations);
patchedData = applyResumePatches(parseStoredResumeData(existing.data), input.operations);
} catch (error) {
if (error instanceof ResumePatchError) {
throw new ORPCError("INVALID_PATCH_OPERATIONS", {
+4 -4
View File
@@ -2,7 +2,7 @@ import type { ResumeData } from "@reactive-resume/schema/resume/data";
import type { JsonPatchError, Operation } from "fast-json-patch";
import jsonpatch from "fast-json-patch";
import z from "zod";
import { parseResumeData } from "@reactive-resume/schema/resume/data";
import { parseResumeDataForWrite } from "@reactive-resume/schema/resume/write";
/**
* A Zod schema that models JSON Patch (RFC 6902) operations as a discriminated union on `op`.
@@ -86,8 +86,8 @@ function toResumePatchError(error: JsonPatchError): ResumePatchError {
* Applies an array of JSON Patch (RFC 6902) operations to a `ResumeData` object.
*
* This function validates the operations before applying them, then validates the
* resulting document against the `resumeDataSchema` to ensure the patched data is
* still a valid resume.
* resulting document against the published write constraints, rejecting invalid
* values rather than replacing them with read-time defaults.
*
* The original `data` object is not mutated; a deep clone is created internally.
*
@@ -118,7 +118,7 @@ export function applyResumePatches(data: ResumeData, operations: Operation[]): R
}
try {
return parseResumeData(patched);
return parseResumeDataForWrite(patched);
} catch (error) {
throw new Error(`Patch produced invalid resume data: ${error instanceof Error ? error.message : String(error)}`, {
cause: error,
+72
View File
@@ -0,0 +1,72 @@
import type { z } from "zod";
import { describe, expect, expectTypeOf, it } from "vitest";
import { resumeDataSchema } from "./data";
import { defaultResumeData } from "./default";
import { sampleResumeData } from "./sample";
import { parseResumeDataForWrite, writableResumeDataSchema } from "./write";
describe("writableResumeDataSchema", () => {
it("keeps canonical client input and output types", () => {
expectTypeOf<z.input<typeof writableResumeDataSchema>>().toEqualTypeOf<z.input<typeof resumeDataSchema>>();
expectTypeOf<z.output<typeof writableResumeDataSchema>>().toEqualTypeOf<z.output<typeof resumeDataSchema>>();
});
it.each([defaultResumeData, sampleResumeData])("accepts canonical initial resume data", (data) => {
expect(parseResumeDataForWrite(data)).toEqual(resumeDataSchema.parse(data));
});
it("defaults absent legacy style rules while rejecting an explicitly invalid value", () => {
const { styleRules: _, ...metadata } = defaultResumeData.metadata;
const data = { ...defaultResumeData, metadata };
expect(parseResumeDataForWrite(data).metadata.styleRules).toEqual([]);
expect(
writableResumeDataSchema.safeParse({
...data,
metadata: { ...metadata, styleRules: null },
}).success,
).toBe(false);
});
it("preserves historical stylesheet normalization", () => {
const source = { languageVersion: 1, text: "@version 1;\nname { color: red; }\n" };
const data = {
...defaultResumeData,
metadata: { ...defaultResumeData.metadata, stylesheet: { mode: "semantic", source, applied: source } },
};
expect(parseResumeDataForWrite(data).metadata.stylesheet).toEqual({ mode: "semantic", source });
});
it("does not change existing legacy style-rule filtering", () => {
const data = {
...defaultResumeData,
metadata: {
...defaultResumeData.metadata,
styleRules: [
{
id: "legacy",
label: "",
enabled: true,
target: { scope: "global" },
slots: { heading: { lineHeight: 5 } },
},
],
},
};
expect(parseResumeDataForWrite(data).metadata.styleRules).toEqual([]);
});
it("rejects bounds within custom-section item arrays", () => {
const data = {
...defaultResumeData,
customSections: [
{
...sampleResumeData.sections.skills,
id: "custom-skills",
type: "skills",
items: [{ ...sampleResumeData.sections.skills.items[0], level: 6 }],
},
],
};
expect(writableResumeDataSchema.safeParse(data).success).toBe(false);
});
});
+37
View File
@@ -0,0 +1,37 @@
import type { ResumeData } from "./data";
import z from "zod";
import { resumeDataSchema } from "./data";
import { createResumeDataJsonSchema } from "./json-schema";
const publishedInputSchema = z.fromJSONSchema(createResumeDataJsonSchema());
/** Validate submitted values before tolerant read-time fallbacks can replace them. */
export const writableResumeDataSchema = z.transform<z.input<typeof resumeDataSchema>, ResumeData>((input, ctx) => {
// Keep canonical diagnostics and migrations, including historical stylesheet metadata.
const canonical = resumeDataSchema.safeParse(input);
if (!canonical.success) {
for (const issue of canonical.error.issues) ctx.addIssue({ ...issue });
return z.NEVER;
}
// The stylesheet parser already validates strictly and removes the historical `applied` field.
// Preserve that migration without normalizing any submitted values with catch fallbacks.
const submitted = publishedInputSchema.safeParse({
...input,
metadata: {
...input.metadata,
// JSON Schema cannot represent the fallback on this transformed historical field.
styleRules: input.metadata.styleRules === undefined ? [] : input.metadata.styleRules,
stylesheet: canonical.data.metadata.stylesheet,
},
});
if (!submitted.success) {
for (const issue of submitted.error.issues) ctx.addIssue({ ...issue });
return z.NEVER;
}
// Use the canonical result so JSON Schema conversion cannot strip compatible extra data.
return canonical.data;
});
export const parseResumeDataForWrite = (data: unknown): ResumeData => writableResumeDataSchema.parse(data);