From 8bfde538bfebadc7085374909ba2eb2aad3a1c8c Mon Sep 17 00:00:00 2001 From: Francisco de Guzman <17106076+franciszver@users.noreply.github.com> Date: Fri, 24 Jul 2026 06:00:10 -0700 Subject: [PATCH 1/2] =?UTF-8?q?test(security):=20red=20=E2=80=94=20trust-f?= =?UTF-8?q?lag=20mass=20assignment=20+=20favorite=20IDOR=20(#49)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds failing tests against current repository/route behavior: - updateCitation/updateClause/updateTemplate must not let clients set isVerified/isPublished (and createCitation/createClause/createTemplate must not let clients set them at creation either). - updateDraft must not let clients reassign userId. - removeClauseFavorite/removeCitationFavorite must not delete a favorite row owned by a different user (IDOR). Also extends the fake Prisma test client with deleteMany, needed by the upcoming userId-scoped favorite delete implementation. Assisted-by: Claude Code (Sonnet) Co-Authored-By: Claude Fable 5 --- .../repositories/citationRepository.test.js | 30 +++++++++++++++- .../src/repositories/clauseRepository.test.js | 35 +++++++++++++++++-- .../src/repositories/draftRepository.test.js | 7 ++++ .../repositories/templateRepository.test.js | 20 +++++++++-- server/src/routes/citations.test.js | 18 ++++++++++ server/src/routes/clauses.test.js | 18 ++++++++++ server/src/routes/drafts.test.js | 11 ++++++ server/test-utils/fakePrismaClient.js | 12 +++++-- 8 files changed, 143 insertions(+), 8 deletions(-) diff --git a/server/src/repositories/citationRepository.test.js b/server/src/repositories/citationRepository.test.js index 69a516d..4a1dec9 100644 --- a/server/src/repositories/citationRepository.test.js +++ b/server/src/repositories/citationRepository.test.js @@ -67,6 +67,24 @@ describe('citationRepository', () => { expect(await getCitation(prisma, created.id)).toBeNull(); }); + it('does not allow isVerified to be set via update (trust flag mass assignment)', async () => { + const created = await createCitation(prisma, { title: 'X', citation: 'x', type: 'case' }); + expect(created.isVerified).toBe(false); + + const updated = await updateCitation(prisma, created.id, { isVerified: true }); + expect(updated.isVerified).toBe(false); + }); + + it('does not allow isVerified to be set via create (trust flag mass assignment)', async () => { + const created = await createCitation(prisma, { + title: 'X', + citation: 'x', + type: 'case', + isVerified: true, + }); + expect(created.isVerified).toBe(false); + }); + it('searches citations by type', async () => { await seedCitations(prisma); @@ -106,10 +124,20 @@ describe('citationRepository', () => { const list = await listCitationFavoritesByUser(prisma, 'user-1'); expect(list).toHaveLength(1); - await removeCitationFavorite(prisma, favorite.id); + await removeCitationFavorite(prisma, favorite.id, 'user-1'); expect(await listCitationFavoritesByUser(prisma, 'user-1')).toHaveLength(0); }); + it('does not remove a favorite owned by a different user (IDOR)', async () => { + const citation = await createCitation(prisma, { title: 'X', citation: 'x', type: 'case' }); + const favorite = await addCitationFavorite(prisma, 'user-1', citation.id); + + const removed = await removeCitationFavorite(prisma, favorite.id, 'user-2'); + + expect(removed).toBe(false); + expect(await listCitationFavoritesByUser(prisma, 'user-1')).toHaveLength(1); + }); + it('joins favorites with their citation records', async () => { const citation = await createCitation(prisma, { title: 'X', citation: 'x', type: 'case' }); await addCitationFavorite(prisma, 'user-1', citation.id); diff --git a/server/src/repositories/clauseRepository.test.js b/server/src/repositories/clauseRepository.test.js index 5fd451b..9f73bf9 100644 --- a/server/src/repositories/clauseRepository.test.js +++ b/server/src/repositories/clauseRepository.test.js @@ -35,8 +35,10 @@ async function seedClauses(prisma) { content: '

...

', category: 'Confidentiality', jurisdiction: 'California', - isPublished: false, }); + // isPublished is not a client-writable field (see trust-flag test below), + // so flip it directly on the fake store to seed an unpublished fixture. + await prisma.clause.update({ where: { id: unpublished.id }, data: { isPublished: false } }); return { indemnification, confidentiality, unpublished }; } @@ -71,6 +73,25 @@ describe('clauseRepository', () => { expect(await getClause(prisma, created.id)).toBeNull(); }); + it('does not allow isPublished to be set via update (trust flag mass assignment)', async () => { + const created = await createClause(prisma, { title: 'X', content: 'c', category: 'Notices' }); + expect(created.isPublished).toBe(true); + await prisma.clause.update({ where: { id: created.id }, data: { isPublished: false } }); + + const updated = await updateClause(prisma, created.id, { isPublished: true }); + expect(updated.isPublished).toBe(false); + }); + + it('does not allow isPublished to be set via create (trust flag mass assignment)', async () => { + const created = await createClause(prisma, { + title: 'X', + content: 'c', + category: 'Notices', + isPublished: false, + }); + expect(created.isPublished).toBe(true); + }); + it('searches published clauses by category', async () => { const { indemnification } = await seedClauses(prisma); @@ -138,11 +159,21 @@ describe('clauseRepository', () => { const clause = await createClause(prisma, { title: 'X', content: 'c', category: 'Notices' }); const favorite = await addClauseFavorite(prisma, 'user-1', clause.id); - await removeClauseFavorite(prisma, favorite.id); + await removeClauseFavorite(prisma, favorite.id, 'user-1'); expect(await findClauseFavorite(prisma, 'user-1', clause.id)).toBeNull(); }); + it('does not remove a favorite owned by a different user (IDOR)', async () => { + const clause = await createClause(prisma, { title: 'X', content: 'c', category: 'Notices' }); + const favorite = await addClauseFavorite(prisma, 'user-1', clause.id); + + const removed = await removeClauseFavorite(prisma, favorite.id, 'user-2'); + + expect(removed).toBe(false); + expect(await findClauseFavorite(prisma, 'user-1', clause.id)).not.toBeNull(); + }); + it('lists favorite ids for a user', async () => { const clauseA = await createClause(prisma, { title: 'A', content: 'c', category: 'Notices' }); const clauseB = await createClause(prisma, { title: 'B', content: 'c', category: 'Notices' }); diff --git a/server/src/repositories/draftRepository.test.js b/server/src/repositories/draftRepository.test.js index 01dc8d4..f297b5a 100644 --- a/server/src/repositories/draftRepository.test.js +++ b/server/src/repositories/draftRepository.test.js @@ -47,6 +47,13 @@ describe('draftRepository', () => { expect(updated.title).toBe('New'); }); + it('does not allow userId to be reassigned via update (mass assignment)', async () => { + const created = await createDraft(prisma, { userId: 'user-1', title: 'Old' }); + const updated = await updateDraft(prisma, created.id, { userId: 'user-2', title: 'New' }); + expect(updated.userId).toBe('user-1'); + expect(updated.title).toBe('New'); + }); + it('deletes a draft', async () => { const created = await createDraft(prisma, { userId: 'user-1', title: 'Gone' }); await deleteDraft(prisma, created.id); diff --git a/server/src/repositories/templateRepository.test.js b/server/src/repositories/templateRepository.test.js index d5ab3f7..4b1c634 100644 --- a/server/src/repositories/templateRepository.test.js +++ b/server/src/repositories/templateRepository.test.js @@ -30,14 +30,30 @@ describe('templateRepository', () => { expect((await getTemplate(prisma, created.id)).name).toBe('X'); - const updated = await updateTemplate(prisma, created.id, { name: 'Y', isPublished: true }); + const updated = await updateTemplate(prisma, created.id, { name: 'Y' }); expect(updated.name).toBe('Y'); - expect(updated.isPublished).toBe(true); await deleteTemplate(prisma, created.id); expect(await getTemplate(prisma, created.id)).toBeNull(); }); + it('does not allow isPublished to be set via update (trust flag mass assignment)', async () => { + const created = await createTemplate(prisma, { category: 'Demand Letter', name: 'X' }); + expect(created.isPublished).toBe(false); + + const updated = await updateTemplate(prisma, created.id, { isPublished: true }); + expect(updated.isPublished).toBe(false); + }); + + it('does not allow isPublished to be set via create (trust flag mass assignment)', async () => { + const created = await createTemplate(prisma, { + category: 'Demand Letter', + name: 'X', + isPublished: true, + }); + expect(created.isPublished).toBe(false); + }); + it('lists all templates when no category given', async () => { await createTemplate(prisma, { category: 'Demand Letter', name: 'A' }); await createTemplate(prisma, { category: 'NDA', name: 'B' }); diff --git a/server/src/routes/citations.test.js b/server/src/routes/citations.test.js index 0c08534..557be34 100644 --- a/server/src/routes/citations.test.js +++ b/server/src/routes/citations.test.js @@ -8,11 +8,13 @@ describe('citations API', () => { let app; let prisma; let user; + let other; beforeEach(async () => { prisma = createFakePrismaClient(); app = createApp({ prisma }); user = await registerUser(app); + other = await registerUser(app); }); function auth(token) { @@ -149,5 +151,21 @@ describe('citations API', () => { const res = await request(app).delete('/citations/favorites/no-such-id').set(auth(user.accessToken)); expect(res.status).toBe(404); }); + + it('does not let another user remove your favorite (IDOR)', async () => { + const created = await createCitation(); + const addRes = await request(app) + .post('/citations/favorites') + .set(auth(user.accessToken)) + .send({ citationId: created.body.id }); + + const res = await request(app) + .delete(`/citations/favorites/${addRes.body.id}`) + .set(auth(other.accessToken)); + expect(res.status).toBe(404); + + const listRes = await request(app).get('/citations/favorites/mine').set(auth(user.accessToken)); + expect(listRes.body).toHaveLength(1); + }); }); }); diff --git a/server/src/routes/clauses.test.js b/server/src/routes/clauses.test.js index 8af5c06..fed6b02 100644 --- a/server/src/routes/clauses.test.js +++ b/server/src/routes/clauses.test.js @@ -8,11 +8,13 @@ describe('clauses API', () => { let app; let prisma; let user; + let other; beforeEach(async () => { prisma = createFakePrismaClient(); app = createApp({ prisma }); user = await registerUser(app); + other = await registerUser(app); }); function auth(token) { @@ -165,5 +167,21 @@ describe('clauses API', () => { const res = await request(app).delete('/clauses/favorites/no-such-id').set(auth(user.accessToken)); expect(res.status).toBe(404); }); + + it('does not let another user remove your favorite (IDOR)', async () => { + const created = await createClause(); + const addRes = await request(app) + .post('/clauses/favorites') + .set(auth(user.accessToken)) + .send({ clauseId: created.body.id }); + + const res = await request(app) + .delete(`/clauses/favorites/${addRes.body.id}`) + .set(auth(other.accessToken)); + expect(res.status).toBe(404); + + const listRes = await request(app).get('/clauses/favorites/mine').set(auth(user.accessToken)); + expect(listRes.body).toHaveLength(1); + }); }); }); diff --git a/server/src/routes/drafts.test.js b/server/src/routes/drafts.test.js index 7ccd59c..adedb76 100644 --- a/server/src/routes/drafts.test.js +++ b/server/src/routes/drafts.test.js @@ -101,6 +101,17 @@ describe('drafts API', () => { const res = await request(app).patch('/drafts/no-such-id').set(auth(owner.accessToken)).send({ title: 'x' }); expect(res.status).toBe(404); }); + + it('ignores an attempt to reassign userId in the body (mass assignment)', async () => { + const created = await request(app).post('/drafts').set(auth(owner.accessToken)).send({ title: 'x' }); + const res = await request(app) + .patch(`/drafts/${created.body.id}`) + .set(auth(owner.accessToken)) + .send({ userId: other.user.id, title: 'still mine' }); + expect(res.status).toBe(200); + expect(res.body.userId).toBe(owner.user.id); + expect(res.body.title).toBe('still mine'); + }); }); describe('DELETE /drafts/:id', () => { diff --git a/server/test-utils/fakePrismaClient.js b/server/test-utils/fakePrismaClient.js index 999ada3..f369a4d 100644 --- a/server/test-utils/fakePrismaClient.js +++ b/server/test-utils/fakePrismaClient.js @@ -5,9 +5,9 @@ // types to be meaningful here). // // Supports the subset of Prisma's query API the repositories rely on: -// create / findUnique / findFirst / findMany / update / delete / count, -// with `where` clauses of plain equality plus `{ not }` and `{ in }` -// operators, and `orderBy` / `take` on findMany. +// create / findUnique / findFirst / findMany / update / delete / count / +// deleteMany, with `where` clauses of plain equality plus `{ not }` and +// `{ in }` operators, and `orderBy` / `take` on findMany. import { randomUUID } from 'node:crypto'; @@ -112,6 +112,12 @@ function createFakeModel({ uniqueFields = [] } = {}) { async count({ where } = {}) { return [...rows.values()].filter((r) => matchesWhere(r, where)).length; }, + + async deleteMany({ where } = {}) { + const matches = [...rows.values()].filter((r) => matchesWhere(r, where)); + for (const record of matches) rows.delete(record.id); + return { count: matches.length }; + }, }; } From 311cfb5af0e20da1988b0881619d28c47347c24a Mon Sep 17 00:00:00 2001 From: Francisco de Guzman <17106076+franciszver@users.noreply.github.com> Date: Fri, 24 Jul 2026 06:01:26 -0700 Subject: [PATCH 2/2] fix(security): allowlist library/draft writes + scope favorite deletes by userId (#49) Clause/citation/template libraries stay user-editable by any authenticated user (owner decision: no role gate), but clients could previously mass- assign system/trust fields by spreading raw req.body into prisma create/ update calls: - Citation.isVerified, Clause.isPublished, Template.isPublished/version could be set directly by any client on create or update. - Draft.userId could be reassigned via PATCH /drafts/:id. - removeClauseFavorite/removeCitationFavorite deleted by row id with no ownership check (IDOR: any authenticated user could delete any other user's favorite by guessing/enumerating ids). Adds a small pick(obj, keys) helper (server/src/repositories/pick.js) and per-model allowlists of client-writable content fields, built from prisma/schema.prisma: - Citation: title, citation, type, court, year, volume, reporter, page, pinpoint, jurisdiction, codeTitle, section, subdivision, shortForm, parenthetical, url, category, tags, notes. - Clause: title, content, description, category, subcategory, tags, jurisdiction, documentTypes, variations, author, isFavorite, notes, placeholders. - Template: category, name, skeletonContent, defaultMetadata, placeholders, sections, variables. - Draft (update only): title, content, metadata, intakeData, status. usageCount/lastUsedAt (managed by the dedicated /usage endpoints), isVerified/isPublished/version/publishedAt/parentTemplateId (trust/version fields), createdBy, and all ids/timestamps/relations are excluded from client writes on all three library models. removeClauseFavorite/removeCitationFavorite now take a userId and delete via deleteMany({ where: { id, userId } }), so a mismatched user deletes nothing; the DELETE /favorites/:id routes now pass req.user.id. Added deleteMany to the fake Prisma test client to support this. Adjusted templateRepository.test.js and clauseRepository.test.js fixtures that previously set isPublished via createClause/updateTemplate directly (now seeded via a raw prisma.*.update call instead, since that field is no longer client-writable through the repository functions). Assisted-by: Claude Code (Sonnet) Co-Authored-By: Claude Fable 5 --- server/src/repositories/citationRepository.js | 40 ++++++++++++++++--- server/src/repositories/clauseRepository.js | 34 +++++++++++++--- server/src/repositories/draftRepository.js | 8 +++- server/src/repositories/pick.js | 13 ++++++ server/src/repositories/templateRepository.js | 23 +++++++++-- server/src/routes/citations.js | 3 +- server/src/routes/clauses.js | 3 +- 7 files changed, 103 insertions(+), 21 deletions(-) create mode 100644 server/src/repositories/pick.js diff --git a/server/src/repositories/citationRepository.js b/server/src/repositories/citationRepository.js index 9feb903..4669461 100644 --- a/server/src/repositories/citationRepository.js +++ b/server/src/repositories/citationRepository.js @@ -1,12 +1,39 @@ // Repository for the Citation aggregate, including each user's favorites // (UserCitationFavorite). +import { pick } from './pick.js'; + +// Client-writable content fields (see prisma/schema.prisma Citation model). +// Excludes id, usageCount/lastUsedAt (managed by incrementCitationUsage), +// isVerified (trust flag), createdBy (identity field), createdAt/updatedAt. +const WRITABLE_FIELDS = [ + 'title', + 'citation', + 'type', + 'court', + 'year', + 'volume', + 'reporter', + 'page', + 'pinpoint', + 'jurisdiction', + 'codeTitle', + 'section', + 'subdivision', + 'shortForm', + 'parenthetical', + 'url', + 'category', + 'tags', + 'notes', +]; + export async function createCitation(prisma, data) { return prisma.citation.create({ data: { - ...data, - usageCount: data.usageCount ?? 0, - isVerified: data.isVerified ?? false, + ...pick(data, WRITABLE_FIELDS), + usageCount: 0, + isVerified: false, }, }); } @@ -16,7 +43,7 @@ export async function getCitation(prisma, id) { } export async function updateCitation(prisma, id, data) { - return prisma.citation.update({ where: { id }, data }); + return prisma.citation.update({ where: { id }, data: pick(data, WRITABLE_FIELDS) }); } export async function deleteCitation(prisma, id) { @@ -59,8 +86,9 @@ export async function addCitationFavorite(prisma, userId, citationId, notes) { }); } -export async function removeCitationFavorite(prisma, id) { - return prisma.userCitationFavorite.delete({ where: { id } }); +export async function removeCitationFavorite(prisma, id, userId) { + const { count } = await prisma.userCitationFavorite.deleteMany({ where: { id, userId } }); + return count > 0; } export async function listCitationFavoritesByUser(prisma, userId) { diff --git a/server/src/repositories/clauseRepository.js b/server/src/repositories/clauseRepository.js index c20bcb5..55f459d 100644 --- a/server/src/repositories/clauseRepository.js +++ b/server/src/repositories/clauseRepository.js @@ -1,12 +1,33 @@ // Repository for the Clause aggregate, including each user's favorites // (UserClauseFavorite). +import { pick } from './pick.js'; + +// Client-writable content fields (see prisma/schema.prisma Clause model). +// Excludes id, usageCount/lastUsedAt (managed by incrementClauseUsage), +// isPublished (trust flag), createdAt/updatedAt. +const WRITABLE_FIELDS = [ + 'title', + 'content', + 'description', + 'category', + 'subcategory', + 'tags', + 'jurisdiction', + 'documentTypes', + 'variations', + 'author', + 'isFavorite', + 'notes', + 'placeholders', +]; + export async function createClause(prisma, data) { return prisma.clause.create({ data: { - ...data, - usageCount: data.usageCount ?? 0, - isPublished: data.isPublished ?? true, + ...pick(data, WRITABLE_FIELDS), + usageCount: 0, + isPublished: true, }, }); } @@ -16,7 +37,7 @@ export async function getClause(prisma, id) { } export async function updateClause(prisma, id, data) { - return prisma.clause.update({ where: { id }, data }); + return prisma.clause.update({ where: { id }, data: pick(data, WRITABLE_FIELDS) }); } export async function deleteClause(prisma, id) { @@ -65,8 +86,9 @@ export async function addClauseFavorite(prisma, userId, clauseId, notes) { }); } -export async function removeClauseFavorite(prisma, id) { - return prisma.userClauseFavorite.delete({ where: { id } }); +export async function removeClauseFavorite(prisma, id, userId) { + const { count } = await prisma.userClauseFavorite.deleteMany({ where: { id, userId } }); + return count > 0; } export async function findClauseFavorite(prisma, userId, clauseId) { diff --git a/server/src/repositories/draftRepository.js b/server/src/repositories/draftRepository.js index 8cddb99..f6d53a6 100644 --- a/server/src/repositories/draftRepository.js +++ b/server/src/repositories/draftRepository.js @@ -1,5 +1,11 @@ // Repository for the Draft aggregate (a user's document). +import { pick } from './pick.js'; + +// Client-writable fields for updates. Excludes id and userId (ownership must +// not be reassignable via the body) and createdAt/updatedAt. +const WRITABLE_FIELDS = ['title', 'content', 'metadata', 'intakeData', 'status']; + export async function createDraft(prisma, { userId, title, content, metadata, intakeData, status }) { return prisma.draft.create({ data: { @@ -18,7 +24,7 @@ export async function getDraft(prisma, id) { } export async function updateDraft(prisma, id, data) { - return prisma.draft.update({ where: { id }, data }); + return prisma.draft.update({ where: { id }, data: pick(data, WRITABLE_FIELDS) }); } export async function deleteDraft(prisma, id) { diff --git a/server/src/repositories/pick.js b/server/src/repositories/pick.js new file mode 100644 index 0000000..d66c460 --- /dev/null +++ b/server/src/repositories/pick.js @@ -0,0 +1,13 @@ +// Small allowlist helper: returns a new object containing only the keys in +// `keys` that are present on `obj`. Used by the repository create/update +// functions to keep server/system fields (trust flags, ids, timestamps, FKs) +// out of client-controlled writes. +export function pick(obj, keys) { + const result = {}; + for (const key of keys) { + if (obj && Object.prototype.hasOwnProperty.call(obj, key)) { + result[key] = obj[key]; + } + } + return result; +} diff --git a/server/src/repositories/templateRepository.js b/server/src/repositories/templateRepository.js index 736447a..a6adee4 100644 --- a/server/src/repositories/templateRepository.js +++ b/server/src/repositories/templateRepository.js @@ -1,11 +1,26 @@ // Repository for the Template aggregate. +import { pick } from './pick.js'; + +// Client-writable content fields (see prisma/schema.prisma Template model). +// Excludes id, version/isPublished/publishedAt (version & trust fields), +// parentTemplateId (relation-like reference), createdAt/updatedAt. +const WRITABLE_FIELDS = [ + 'category', + 'name', + 'skeletonContent', + 'defaultMetadata', + 'placeholders', + 'sections', + 'variables', +]; + export async function createTemplate(prisma, data) { return prisma.template.create({ data: { - ...data, - version: data.version ?? 1, - isPublished: data.isPublished ?? false, + ...pick(data, WRITABLE_FIELDS), + version: 1, + isPublished: false, }, }); } @@ -15,7 +30,7 @@ export async function getTemplate(prisma, id) { } export async function updateTemplate(prisma, id, data) { - return prisma.template.update({ where: { id }, data }); + return prisma.template.update({ where: { id }, data: pick(data, WRITABLE_FIELDS) }); } export async function deleteTemplate(prisma, id) { diff --git a/server/src/routes/citations.js b/server/src/routes/citations.js index c4585c7..ad98c5f 100644 --- a/server/src/routes/citations.js +++ b/server/src/routes/citations.js @@ -1,6 +1,5 @@ import { Router } from 'express'; import { asyncHandler } from './asyncHandler.js'; -import { withNotFound } from './helpers.js'; import { createCitation, getCitation, @@ -50,7 +49,7 @@ export function createCitationsRouter({ prisma }) { router.delete( '/favorites/:id', asyncHandler(async (req, res) => { - const removed = await withNotFound(removeCitationFavorite(prisma, req.params.id)); + const removed = await removeCitationFavorite(prisma, req.params.id, req.user.id); if (!removed) return res.status(404).json({ error: 'Favorite not found' }); res.status(204).send(); }) diff --git a/server/src/routes/clauses.js b/server/src/routes/clauses.js index bd1bd54..407c07c 100644 --- a/server/src/routes/clauses.js +++ b/server/src/routes/clauses.js @@ -1,6 +1,5 @@ import { Router } from 'express'; import { asyncHandler } from './asyncHandler.js'; -import { withNotFound } from './helpers.js'; import { createClause, getClause, @@ -51,7 +50,7 @@ export function createClausesRouter({ prisma }) { router.delete( '/favorites/:id', asyncHandler(async (req, res) => { - const removed = await withNotFound(removeClauseFavorite(prisma, req.params.id)); + const removed = await removeClauseFavorite(prisma, req.params.id, req.user.id); if (!removed) return res.status(404).json({ error: 'Favorite not found' }); res.status(204).send(); })