From b6adebc1deeb73c439c6c4b8323431f25b442953 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 13 Jun 2026 04:08:34 +0000 Subject: [PATCH] fix(security): enforce note edit access on PDF derive-page POST /api/sources/pdf/.../derive-page accepted an arbitrary noteId without checking membership, allowing authenticated users to inject pages into notes they cannot edit. Mirror POST /api/pages permission checks when noteId is set. Also correct zedi_remove_page_from_note MCP description: after issue #823 the endpoint soft-deletes the page; there is no unlink-only mode. Co-authored-by: akimasa.sugai --- .../src/__tests__/routes/pdfSources.test.ts | 116 ++++++++++++++++++ server/api/src/routes/pdfSources.ts | 20 ++- server/mcp/src/tools/index.ts | 4 +- 3 files changed, 135 insertions(+), 5 deletions(-) create mode 100644 server/api/src/__tests__/routes/pdfSources.test.ts diff --git a/server/api/src/__tests__/routes/pdfSources.test.ts b/server/api/src/__tests__/routes/pdfSources.test.ts new file mode 100644 index 00000000..a6c440e5 --- /dev/null +++ b/server/api/src/__tests__/routes/pdfSources.test.ts @@ -0,0 +1,116 @@ +/** + * PDF ソース derive-page の権限テスト。 + * Permission tests for POST /api/sources/pdf/:sourceId/highlights/:highlightId/derive-page. + */ +import { describe, it, expect, vi } from "vitest"; +import type { Context, Next } from "hono"; +import type { AppEnv } from "../../types/index.js"; + +vi.mock("../../middleware/auth.js", () => ({ + authRequired: async (c: Context, next: Next) => { + const userId = c.req.header("x-test-user-id"); + if (!userId) return c.json({ message: "Unauthorized" }, 401); + c.set("userId", userId); + c.set("userEmail", "tester@example.com"); + await next(); + }, +})); + +vi.mock("../../middleware/rateLimit.js", () => ({ + rateLimit: () => async (_c: Context, next: Next) => { + await next(); + }, +})); + +vi.mock("../../services/defaultNoteService.js", () => ({ + ensureDefaultNote: vi.fn(async (_db: unknown, userId: string) => ({ + id: "default-note-mock", + ownerId: userId, + title: "Mock note", + visibility: "private" as const, + editPermission: "owner_only" as const, + isOfficial: false, + isDefault: true, + viewCount: 0, + createdAt: new Date(), + updatedAt: new Date(), + isDeleted: false, + })), +})); + +import { Hono } from "hono"; +import { errorHandler } from "../../middleware/errorHandler.js"; +import pdfSourcesRoutes from "../../routes/pdfSources.js"; +import { createMockDb } from "../createMockDb.js"; + +const TEST_USER_ID = "user-pdf-derive-1"; +const SOURCE_ID = "pdf-source-1"; +const HIGHLIGHT_ID = "highlight-1"; +const FOREIGN_NOTE_ID = "foreign-note-id"; + +const PDF_SOURCE_ROW = { + id: SOURCE_ID, + kind: "pdf_local" as const, + ownerId: TEST_USER_ID, + displayName: "Test PDF", +}; + +const HIGHLIGHT_ROW = { + id: HIGHLIGHT_ID, + sourceId: SOURCE_ID, + ownerId: TEST_USER_ID, + text: "Highlighted text", + derivedPageId: null, +}; + +function authHeaders(): Record { + return { + "x-test-user-id": TEST_USER_ID, + "Content-Type": "application/json", + }; +} + +function createPdfSourcesApp(dbResults: unknown[]) { + const { db } = createMockDb(dbResults); + const app = new Hono(); + app.onError(errorHandler); + app.use("*", async (c, next) => { + c.set("db", db as unknown as AppEnv["Variables"]["db"]); + await next(); + }); + app.route("/api/sources", pdfSourcesRoutes); + return app; +} + +describe("POST /api/sources/pdf/:sourceId/highlights/:highlightId/derive-page", () => { + it("returns 403 when noteId points to a note the caller cannot edit", async () => { + const foreignNote = { + id: FOREIGN_NOTE_ID, + ownerId: "other-user", + title: "Someone else's note", + visibility: "private" as const, + editPermission: "owner_only" as const, + isOfficial: false, + isDefault: false, + viewCount: 0, + createdAt: new Date(), + updatedAt: new Date(), + isDeleted: false, + memberRole: null, + domainRole: null, + }; + + const app = createPdfSourcesApp([[PDF_SOURCE_ROW], [HIGHLIGHT_ROW], [foreignNote]]); + + const res = await app.request( + `/api/sources/pdf/${SOURCE_ID}/highlights/${HIGHLIGHT_ID}/derive-page`, + { + method: "POST", + headers: authHeaders(), + body: JSON.stringify({ noteId: FOREIGN_NOTE_ID, title: "Injected page" }), + }, + ); + + expect(res.status).toBe(403); + }); +}); diff --git a/server/api/src/routes/pdfSources.ts b/server/api/src/routes/pdfSources.ts index b19927e2..d4632891 100644 --- a/server/api/src/routes/pdfSources.ts +++ b/server/api/src/routes/pdfSources.ts @@ -26,6 +26,7 @@ import type { PdfSourceMetadata } from "../schema/sources.js"; import { pageSources } from "../schema/pageSources.js"; import { pages } from "../schema/pages.js"; import { ensureDefaultNote } from "../services/defaultNoteService.js"; +import { canEdit, getNoteRole } from "./notes/helpers.js"; import type { AppEnv } from "../types/index.js"; const app = new Hono(); @@ -468,10 +469,21 @@ app.post( ? body.contentPreview.slice(0, 240) : highlightRow.text.slice(0, 240); - // ノート所属を解決（default に寄せる）。ノート権限の細かなチェックは - // /api/pages POST と同等のロジックを将来 import で共有してもよい。 - // Resolve note membership; default to the user's default note. - const resolvedNoteId: string = requestedNoteId ?? (await ensureDefaultNote(db, userId)).id; + // ノート所属を解決（default に寄せる）。明示的な noteId は POST /api/pages と + // 同様に編集権限を検証する（他ユーザーのノートへのページ注入を防ぐ）。 + // Resolve note membership; when noteId is explicit, require edit access like POST /api/pages. + let resolvedNoteId: string; + if (requestedNoteId) { + const userEmail = c.get("userEmail"); + const { role, note } = await getNoteRole(requestedNoteId, userId, userEmail, db); + if (!note) throw new HTTPException(404, { message: "Note not found" }); + if (!role || !canEdit(role, note)) { + throw new HTTPException(403, { message: "Forbidden" }); + } + resolvedNoteId = requestedNoteId; + } else { + resolvedNoteId = (await ensureDefaultNote(db, userId)).id; + } const sectionAnchor = `pdf:v1:${highlightRow.id}`; diff --git a/server/mcp/src/tools/index.ts b/server/mcp/src/tools/index.ts index f154eeab..c8d5cacf 100644 --- a/server/mcp/src/tools/index.ts +++ b/server/mcp/src/tools/index.ts @@ -225,7 +225,9 @@ export function registerAllTools(server: McpServer, client: ZediClient): void { { title: "Remove page from note", description: - "Removes a page from a note. The page itself is not deleted, only the linkage is removed.", + "Soft-deletes a page that belongs to the note (same as DELETE /api/notes/:noteId/pages/:pageId). " + + "There is no unlink-only mode after issue #823 — the page is marked deleted. " + + "Use zedi_delete_page when you intend to delete a page by id.", inputSchema: { note_id: z.string().min(1), page_id: z.string().min(1) }, }, async (args) =>