From b830382e86031733d1e4f83df95e4e24c15369c4 Mon Sep 17 00:00:00 2001 From: nikit Date: Mon, 6 Jul 2026 21:07:26 -0400 Subject: [PATCH] =?UTF-8?q?security(sections):=20fix=20IDOR=20=E2=80=94=20?= =?UTF-8?q?private=20templates=20leaked/writable=20by=20slug?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every custom/AI-generated SectionTemplate defaulted to isShared:true (schema default), and neither buildTemplateDoc (profile-generated sections) nor the manual templates POST overrode it — so every personal section a user creates (including ones whose name/description describe their actual life) was silently public. On top of that, several per-slug endpoints applied no ownership or usageCount gate at all, unlike the list endpoint (which requires isShared + usageCount >= 3): - GET /api/sections/templates/[slug] — had isShared:true with no usageCount check, letting a template become public the instant a single other user's search happened to hit it - GET/POST /api/sections/[slug]/entries — no gate whatsoever - GET /api/sections/[slug]/entries/[id] (PATCH) — no gate whatsoever - GET /api/export/[section] (custom: section export) — no gate, leaked field labels via exported column headers - POST /api/sections/templates/[slug]/edit-layout — no gate, and worse: the save:true path let ANY user overwrite another user's template's layoutHtml by slug (write IDOR, not just a read leak) Fixes: - Schema default isShared: false (private unless explicitly shared) - buildTemplateDoc and the manual template POST now set isShared: false explicitly - All five endpoints above now apply the same ownership/shared gate used by the list endpoint: createdBy === user, or createdBy null (built-in), or isShared + usageCount >= 3 (proven shared template) - edit-layout's save path additionally requires strict ownership (createdBy === userId) even for templates matched via the shared/ built-in branches of that gate, since read-for-AI-context and write-back are different trust levels Updated a pre-existing unit test that had encoded the old insecure default (isShared: true) as expected schema behavior. Verified live with two real registered users: created a private section as User A, then as User B attempted (1) reading A's template definition, (2) reading/(3) posting A's entries, (4) exporting A's section, (5) PATCHing A's entry via the slug route, (6) overwriting A's layoutHtml via edit-layout save:true — all six blocked with 404 (layoutHtml confirmed unchanged in the DB for #6). Confirmed no regression: A's own access to all of the above still works, and a genuinely proven shared template (isShared:true, usageCount>=3, different owner) is still correctly readable by other users, while an unproven one (usageCount<3) is correctly still blocked. Fixes #44 --- src/app/api/export/[section]/route.ts | 9 ++++++++- .../api/sections/[slug]/entries/[id]/route.ts | 9 ++++++++- src/app/api/sections/[slug]/entries/route.ts | 18 ++++++++++++++++-- .../templates/[slug]/edit-layout/route.ts | 14 +++++++++++--- src/app/api/sections/templates/[slug]/route.ts | 2 +- src/app/api/sections/templates/route.ts | 1 + src/lib/__tests__/section-template.test.ts | 4 ++-- src/lib/models/section-template.ts | 4 +++- src/lib/profile/persist-sections.ts | 4 ++++ 9 files changed, 54 insertions(+), 11 deletions(-) diff --git a/src/app/api/export/[section]/route.ts b/src/app/api/export/[section]/route.ts index d8d7d45..b32898c 100644 --- a/src/app/api/export/[section]/route.ts +++ b/src/app/api/export/[section]/route.ts @@ -367,7 +367,14 @@ export async function buildExport( default: { if (section.startsWith("custom:")) { const slug = section.slice("custom:".length); - const template = await SectionTemplate.findOne({ slug }).lean(); + const template = await SectionTemplate.findOne({ + slug, + $or: [ + { createdBy: userId }, + { createdBy: null }, + { isShared: true, usageCount: { $gte: 3 } }, + ], + }).lean(); if (!template) { return { name: "Unknown", columns: [], rows: [] }; } diff --git a/src/app/api/sections/[slug]/entries/[id]/route.ts b/src/app/api/sections/[slug]/entries/[id]/route.ts index d308d46..e54658c 100644 --- a/src/app/api/sections/[slug]/entries/[id]/route.ts +++ b/src/app/api/sections/[slug]/entries/[id]/route.ts @@ -46,7 +46,14 @@ export async function PATCH( return NextResponse.json({ error: "Not found" }, { status: 404 }); } - const template = await SectionTemplate.findOne({ slug }).lean(); + const template = await SectionTemplate.findOne({ + slug, + $or: [ + { createdBy: userId }, + { createdBy: null }, + { isShared: true, usageCount: { $gte: 3 } }, + ], + }).lean(); if (!template) { return NextResponse.json({ error: "Section not found" }, { status: 404 }); } diff --git a/src/app/api/sections/[slug]/entries/route.ts b/src/app/api/sections/[slug]/entries/route.ts index 7197801..dd8b8d2 100644 --- a/src/app/api/sections/[slug]/entries/route.ts +++ b/src/app/api/sections/[slug]/entries/route.ts @@ -20,7 +20,14 @@ export async function GET( await connectDB(); const { slug } = await params; - const template = await SectionTemplate.findOne({ slug }).lean(); + const template = await SectionTemplate.findOne({ + slug, + $or: [ + { createdBy: userId }, + { createdBy: null }, + { isShared: true, usageCount: { $gte: 3 } }, + ], + }).lean(); if (!template) { return NextResponse.json({ error: "Section not found" }, { status: 404 }); } @@ -89,7 +96,14 @@ export async function POST( await connectDB(); const { slug } = await params; - const template = await SectionTemplate.findOne({ slug }).lean(); + const template = await SectionTemplate.findOne({ + slug, + $or: [ + { createdBy: userId }, + { createdBy: null }, + { isShared: true, usageCount: { $gte: 3 } }, + ], + }).lean(); if (!template) { return NextResponse.json({ error: "Section not found" }, { status: 404 }); } diff --git a/src/app/api/sections/templates/[slug]/edit-layout/route.ts b/src/app/api/sections/templates/[slug]/edit-layout/route.ts index 5a2fffa..aa6c714 100644 --- a/src/app/api/sections/templates/[slug]/edit-layout/route.ts +++ b/src/app/api/sections/templates/[slug]/edit-layout/route.ts @@ -338,7 +338,14 @@ export async function POST( await connectDB(); const { slug } = await params; - const template = await SectionTemplate.findOne({ slug }); + const template = await SectionTemplate.findOne({ + slug, + $or: [ + { createdBy: userId }, + { createdBy: null }, + { isShared: true, usageCount: { $gte: 3 } }, + ], + }); if (!template) { return NextResponse.json({ error: "Template not found" }, { status: 404 }); } @@ -381,8 +388,9 @@ Output ONLY the complete updated HTML:`; .replace(/\n?```$/i, "") .trim(); - // Optionally save to template - if (body.save) { + // Optionally save to template — only the owner may persist changes, even + // though a shared/built-in template can be read here for AI context. + if (body.save && String(template.createdBy) === String(userId)) { template.layoutHtml = cleaned; await template.save(); } diff --git a/src/app/api/sections/templates/[slug]/route.ts b/src/app/api/sections/templates/[slug]/route.ts index 9d20be7..d57b8aa 100644 --- a/src/app/api/sections/templates/[slug]/route.ts +++ b/src/app/api/sections/templates/[slug]/route.ts @@ -25,7 +25,7 @@ export async function GET( $or: [ { createdBy: userId }, { createdBy: null }, - { isShared: true }, + { isShared: true, usageCount: { $gte: 3 } }, ], }).lean(); diff --git a/src/app/api/sections/templates/route.ts b/src/app/api/sections/templates/route.ts index 5a2497c..7c55624 100644 --- a/src/app/api/sections/templates/route.ts +++ b/src/app/api/sections/templates/route.ts @@ -88,6 +88,7 @@ export async function POST(req: NextRequest) { isBuiltIn: false, createdBy: userId, usageCount: 1, + isShared: false, }); // Add to user's customSections diff --git a/src/lib/__tests__/section-template.test.ts b/src/lib/__tests__/section-template.test.ts index 5bf86ef..5e088cb 100644 --- a/src/lib/__tests__/section-template.test.ts +++ b/src/lib/__tests__/section-template.test.ts @@ -32,11 +32,11 @@ describe("SectionTemplate schema", () => { expect(instance.forkCount).toBe(0); }); - it("has isShared field as Boolean defaulting to true", () => { + it("has isShared field as Boolean defaulting to false (private unless explicitly shared)", () => { const schemaPaths = SectionTemplate.schema.paths; expect(schemaPaths["isShared"]).toBeDefined(); const instance = new SectionTemplate({ name: "Test", slug: "test", icon: "Star" }); - expect(instance.isShared).toBe(true); + expect(instance.isShared).toBe(false); }); it("has isShared + usageCount compound index defined", () => { diff --git a/src/lib/models/section-template.ts b/src/lib/models/section-template.ts index bc204f3..c1f2ab0 100644 --- a/src/lib/models/section-template.ts +++ b/src/lib/models/section-template.ts @@ -93,7 +93,9 @@ const SectionTemplateSchema = new Schema( sourcePrompt: { type: String, default: "" }, forkedFrom: { type: Schema.Types.ObjectId, ref: "SectionTemplate", default: null }, forkCount: { type: Number, default: 0 }, - isShared: { type: Boolean, default: true }, + // Private by default — only the shared-template-pool save path + // (src/lib/embeddings.ts) should set this true explicitly. + isShared: { type: Boolean, default: false }, }, { timestamps: true } ); diff --git a/src/lib/profile/persist-sections.ts b/src/lib/profile/persist-sections.ts index c1f7d51..2b9b242 100644 --- a/src/lib/profile/persist-sections.ts +++ b/src/lib/profile/persist-sections.ts @@ -22,6 +22,7 @@ export interface TemplateDoc { isBuiltIn: boolean; createdBy: string; usageCount: number; + isShared: boolean; sourceDimension?: string; sourceFacetKey?: string; } @@ -57,6 +58,9 @@ export function buildTemplateDoc( isBuiltIn: false, createdBy: userId, usageCount: 1, + // Personal, profile-generated sections describe the user's actual life + // (name/description/layout) — never share them by default. + isShared: false, ...(source?.dimension ? { sourceDimension: source.dimension } : {}), ...(source?.facetKey ? { sourceFacetKey: source.facetKey } : {}), };