From 24a78a71149466a6e94d13a42c3abc0fbb674a53 Mon Sep 17 00:00:00 2001 From: bendsp Date: Sat, 9 May 2026 16:33:19 +0200 Subject: [PATCH 1/2] fix: harden ipc payload validation --- src/main/index.ts | 51 ++++++++------------- src/main/ipcValidation.ts | 91 +++++++++++++++++++++++++++++++++++++ tests/index.ts | 1 + tests/ipcValidation.test.ts | 55 ++++++++++++++++++++++ 4 files changed, 167 insertions(+), 31 deletions(-) create mode 100644 src/main/ipcValidation.ts create mode 100644 tests/ipcValidation.test.ts diff --git a/src/main/index.ts b/src/main/index.ts index a262d1b..3fbc9ba 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -14,12 +14,16 @@ import { BedrockTestConfig, BedrockTestState, DiscardAction, - SaveFilePayload, OpenFileResult, OpenSpecificFilePayload, SaveFileResult, - ExportFilePayload, } from "../shared/types"; +import { + MAX_MARKDOWN_FILE_BYTES, + normalizeExportFilePayload, + normalizeSaveFilePayload, + safeExportBaseName, +} from "./ipcValidation"; import { buildRuntimeInfo, captureMainTelemetryException, @@ -32,8 +36,6 @@ const MARKDOWN_DIALOG_FILTER = { name: "Markdown Files", extensions: ["md"], }; -const MAX_MARKDOWN_FILE_BYTES = 10 * 1024 * 1024; -const MAX_EXPORT_HTML_BYTES = 25 * 1024 * 1024; const ensureMarkdownExtension = (filePath: string): string => { return filePath.toLowerCase().endsWith(".md") ? filePath : `${filePath}.md`; @@ -133,16 +135,6 @@ const normalizeMarkdownFilePath = (filePath: unknown): string | null => { return isMarkdownFilePath(resolvedPath) ? resolvedPath : null; }; -const assertReasonableContentSize = ( - content: unknown, - maxBytes: number -): content is string => { - return ( - typeof content === "string" && - Buffer.byteLength(content, "utf-8") <= maxBytes - ); -}; - const readMarkdownFile = async ( filePath: string ): Promise => { @@ -285,15 +277,16 @@ ipcMain.handle("file:consume-pending-external-open", () => { ipcMain.handle( "file:save", - async (event, args: SaveFilePayload): Promise => { + async (event, args: unknown): Promise => { try { - if (!assertReasonableContentSize(args.content, MAX_MARKDOWN_FILE_BYTES)) { - throw new Error("Markdown content is too large to save."); + const payload = normalizeSaveFilePayload(args); + if (!payload) { + throw new Error("Invalid save payload."); } - let targetPath = args.filePath - ? ensureMarkdownExtension(path.resolve(args.filePath)) - : args.filePath; + let targetPath = payload.filePath + ? ensureMarkdownExtension(path.resolve(payload.filePath)) + : payload.filePath; if (!targetPath) { const nextSavePath = resolveNextSavePath(); @@ -316,14 +309,13 @@ ipcMain.handle( } } - await fs.writeFile(targetPath, args.content, "utf-8"); + await fs.writeFile(targetPath, payload.content, "utf-8"); return { filePath: targetPath }; } catch (error) { const message = error instanceof Error ? error.message : "An unknown error occurred."; captureMainTelemetryException(error, { operation: "file:save", - filePath: args?.filePath, }); dialog.showErrorBox("Unable to save file", message); return null; @@ -402,15 +394,13 @@ ipcMain.handle("test:simulate-external-open", (_event, filePath: string) => { ipcMain.handle( "file:export", - async (event, args: ExportFilePayload): Promise => { + async (event, args: unknown): Promise => { try { - const { content, format, defaultFileName } = args; - if (format !== "html" && format !== "pdf") { - throw new Error("Unsupported export format."); - } - if (!assertReasonableContentSize(content, MAX_EXPORT_HTML_BYTES)) { - throw new Error("Export content is too large."); + const payload = normalizeExportFilePayload(args); + if (!payload) { + throw new Error("Invalid export payload."); } + const { content, format, defaultFileName } = payload; const extension = format === "html" ? "html" : "pdf"; const filters = @@ -418,7 +408,7 @@ ipcMain.handle( ? [{ name: "HTML Files", extensions: ["html"] }] : [{ name: "PDF Files", extensions: ["pdf"] }]; - const baseName = defaultFileName || "Exported"; + const baseName = safeExportBaseName(defaultFileName); const { canceled, filePath } = await dialog.showSaveDialog( BrowserWindow.fromWebContents(event.sender) ?? undefined, @@ -507,7 +497,6 @@ ipcMain.handle( error instanceof Error ? error.message : "An unknown error occurred."; captureMainTelemetryException(error, { operation: "file:export", - format: args?.format, }); dialog.showErrorBox("Unable to export file", message); return false; diff --git a/src/main/ipcValidation.ts b/src/main/ipcValidation.ts new file mode 100644 index 0000000..5c439b4 --- /dev/null +++ b/src/main/ipcValidation.ts @@ -0,0 +1,91 @@ +import { ExportFilePayload, SaveFilePayload } from "../shared/types"; + +export const MAX_MARKDOWN_FILE_BYTES = 10 * 1024 * 1024; +export const MAX_EXPORT_HTML_BYTES = 25 * 1024 * 1024; + +const isRecord = (value: unknown): value is Record => { + return typeof value === "object" && value !== null; +}; + +export const hasReasonableContentSize = ( + content: unknown, + maxBytes: number +): content is string => { + return ( + typeof content === "string" && + Buffer.byteLength(content, "utf-8") <= maxBytes + ); +}; + +export const normalizeSaveFilePayload = ( + payload: unknown +): SaveFilePayload | null => { + if (!isRecord(payload)) { + return null; + } + if (!hasReasonableContentSize(payload.content, MAX_MARKDOWN_FILE_BYTES)) { + return null; + } + if ( + "filePath" in payload && + payload.filePath !== undefined && + (typeof payload.filePath !== "string" || payload.filePath.trim() === "") + ) { + return null; + } + + const filePath = + typeof payload.filePath === "string" ? payload.filePath : undefined; + + return { + content: payload.content, + filePath, + }; +}; + +export const normalizeExportFilePayload = ( + payload: unknown +): ExportFilePayload | null => { + if (!isRecord(payload)) { + return null; + } + if (payload.format !== "html" && payload.format !== "pdf") { + return null; + } + if (!hasReasonableContentSize(payload.content, MAX_EXPORT_HTML_BYTES)) { + return null; + } + if ( + "defaultFileName" in payload && + payload.defaultFileName !== undefined && + typeof payload.defaultFileName !== "string" + ) { + return null; + } + + const defaultFileName = + typeof payload.defaultFileName === "string" + ? payload.defaultFileName + : undefined; + + return { + content: payload.content, + format: payload.format, + defaultFileName, + }; +}; + +export const safeExportBaseName = ( + defaultFileName: string | undefined +): string => { + const rawName = defaultFileName || "Exported"; + const lastSegment = rawName.split(/[\\/]/).pop() ?? ""; + const withoutExtension = lastSegment.replace(/\.(?:html|pdf)$/i, "").trim(); + const reservedCharacters = '<>:"|?*'; + const sanitized = [...withoutExtension] + .map((char) => + char.charCodeAt(0) < 32 || reservedCharacters.includes(char) ? "-" : char + ) + .join(""); + return sanitized || "Exported"; +}; diff --git a/tests/index.ts b/tests/index.ts index c576031..ec0d415 100644 --- a/tests/index.ts +++ b/tests/index.ts @@ -1,2 +1,3 @@ import "./controllerShortcuts.test"; +import "./ipcValidation.test"; import "./themeSettings.test"; diff --git a/tests/ipcValidation.test.ts b/tests/ipcValidation.test.ts new file mode 100644 index 0000000..25f3726 --- /dev/null +++ b/tests/ipcValidation.test.ts @@ -0,0 +1,55 @@ +import { strict as assert } from "assert"; +import { + MAX_MARKDOWN_FILE_BYTES, + normalizeExportFilePayload, + normalizeSaveFilePayload, + safeExportBaseName, +} from "../src/main/ipcValidation"; + +const runTest = (name: string, fn: () => void) => { + try { + fn(); + console.log(`✓ ${name}`); + } catch (error) { + console.error(`✗ ${name}`); + console.error(error); + process.exitCode = 1; + } +}; + +runTest("save payload validation rejects malformed file paths", () => { + assert.equal(normalizeSaveFilePayload({ content: "ok", filePath: "" }), null); + assert.equal( + normalizeSaveFilePayload({ content: "ok", filePath: 123 }), + null + ); +}); + +runTest("save payload validation rejects oversized content", () => { + const oversized = "x".repeat(MAX_MARKDOWN_FILE_BYTES + 1); + + assert.equal(normalizeSaveFilePayload({ content: oversized }), null); +}); + +runTest("save payload validation accepts valid content", () => { + assert.deepEqual(normalizeSaveFilePayload({ content: "ok" }), { + content: "ok", + filePath: undefined, + }); +}); + +runTest("export payload validation rejects unsupported formats", () => { + assert.equal( + normalizeExportFilePayload({ content: "

ok

", format: "docx" }), + null + ); +}); + +runTest("export default names are sanitized to a base filename", () => { + assert.equal(safeExportBaseName("../notes:bad?.html"), "notes-bad-"); + assert.equal(safeExportBaseName(""), "Exported"); +}); + +if (process.exitCode && process.exitCode !== 0) { + throw new Error("One or more tests failed."); +} From ca9754a4d97e480caaacdcb253fe187ed362a904 Mon Sep 17 00:00:00 2001 From: bendsp Date: Sat, 9 May 2026 16:42:07 +0200 Subject: [PATCH 2/2] fix: improve ipc validation feedback --- src/main/index.ts | 24 ++++++---- src/main/ipcValidation.ts | 91 +++++++++++++++++++++++++++++-------- tests/ipcValidation.test.ts | 55 ++++++++++++++++++++++ 3 files changed, 144 insertions(+), 26 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index 3fbc9ba..9c8ea5f 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -20,9 +20,9 @@ import { } from "../shared/types"; import { MAX_MARKDOWN_FILE_BYTES, - normalizeExportFilePayload, - normalizeSaveFilePayload, safeExportBaseName, + validateExportFilePayload, + validateSaveFilePayload, } from "./ipcValidation"; import { buildRuntimeInfo, @@ -278,11 +278,14 @@ ipcMain.handle("file:consume-pending-external-open", () => { ipcMain.handle( "file:save", async (event, args: unknown): Promise => { + let telemetryFilePath: string | undefined; try { - const payload = normalizeSaveFilePayload(args); - if (!payload) { - throw new Error("Invalid save payload."); + const validation = validateSaveFilePayload(args); + if (validation.ok === false) { + throw new Error(validation.message); } + const payload = validation.payload; + telemetryFilePath = payload.filePath; let targetPath = payload.filePath ? ensureMarkdownExtension(path.resolve(payload.filePath)) @@ -316,6 +319,7 @@ ipcMain.handle( error instanceof Error ? error.message : "An unknown error occurred."; captureMainTelemetryException(error, { operation: "file:save", + filePath: telemetryFilePath, }); dialog.showErrorBox("Unable to save file", message); return null; @@ -395,12 +399,15 @@ ipcMain.handle("test:simulate-external-open", (_event, filePath: string) => { ipcMain.handle( "file:export", async (event, args: unknown): Promise => { + let telemetryFormat: string | undefined; try { - const payload = normalizeExportFilePayload(args); - if (!payload) { - throw new Error("Invalid export payload."); + const validation = validateExportFilePayload(args); + if (validation.ok === false) { + throw new Error(validation.message); } + const payload = validation.payload; const { content, format, defaultFileName } = payload; + telemetryFormat = format; const extension = format === "html" ? "html" : "pdf"; const filters = @@ -497,6 +504,7 @@ ipcMain.handle( error instanceof Error ? error.message : "An unknown error occurred."; captureMainTelemetryException(error, { operation: "file:export", + format: telemetryFormat, }); dialog.showErrorBox("Unable to export file", message); return false; diff --git a/src/main/ipcValidation.ts b/src/main/ipcValidation.ts index 5c439b4..cc6024d 100644 --- a/src/main/ipcValidation.ts +++ b/src/main/ipcValidation.ts @@ -7,6 +7,14 @@ const isRecord = (value: unknown): value is Record => { return typeof value === "object" && value !== null; }; +const hasOwn = (value: Record, key: string): boolean => { + return Object.prototype.hasOwnProperty.call(value, key); +}; + +type ValidationResult = + | { ok: true; payload: T } + | { ok: false; message: string }; + export const hasReasonableContentSize = ( content: unknown, maxBytes: number @@ -20,58 +28,97 @@ export const hasReasonableContentSize = ( export const normalizeSaveFilePayload = ( payload: unknown ): SaveFilePayload | null => { + const result = validateSaveFilePayload(payload); + return result.ok ? result.payload : null; +}; + +export const validateSaveFilePayload = ( + payload: unknown +): ValidationResult => { if (!isRecord(payload)) { - return null; + return { ok: false, message: "Invalid save payload." }; } if (!hasReasonableContentSize(payload.content, MAX_MARKDOWN_FILE_BYTES)) { - return null; + return { + ok: false, + message: + typeof payload.content === "string" + ? "Markdown content is too large to save." + : "Save content must be text.", + }; } + const hasFilePath = hasOwn(payload, "filePath"); if ( - "filePath" in payload && + hasFilePath && payload.filePath !== undefined && (typeof payload.filePath !== "string" || payload.filePath.trim() === "") ) { - return null; + return { ok: false, message: "Save file path must be a non-empty string." }; } const filePath = - typeof payload.filePath === "string" ? payload.filePath : undefined; + hasFilePath && typeof payload.filePath === "string" + ? payload.filePath + : undefined; return { - content: payload.content, - filePath, + ok: true, + payload: { + content: payload.content, + filePath, + }, }; }; export const normalizeExportFilePayload = ( payload: unknown ): ExportFilePayload | null => { + const result = validateExportFilePayload(payload); + return result.ok ? result.payload : null; +}; + +export const validateExportFilePayload = ( + payload: unknown +): ValidationResult => { if (!isRecord(payload)) { - return null; + return { ok: false, message: "Invalid export payload." }; } if (payload.format !== "html" && payload.format !== "pdf") { - return null; + return { ok: false, message: "Unsupported export format." }; } if (!hasReasonableContentSize(payload.content, MAX_EXPORT_HTML_BYTES)) { - return null; + return { + ok: false, + message: + typeof payload.content === "string" + ? "Export content is too large." + : "Export content must be text.", + }; } + const hasDefaultFileName = hasOwn(payload, "defaultFileName"); if ( - "defaultFileName" in payload && + hasDefaultFileName && payload.defaultFileName !== undefined && typeof payload.defaultFileName !== "string" ) { - return null; + return { + ok: false, + message: "Export default filename must be a string.", + }; } const defaultFileName = - typeof payload.defaultFileName === "string" + hasDefaultFileName && typeof payload.defaultFileName === "string" ? payload.defaultFileName : undefined; return { - content: payload.content, - format: payload.format, - defaultFileName, + ok: true, + payload: { + content: payload.content, + format: payload.format, + defaultFileName, + }, }; }; @@ -86,6 +133,14 @@ export const safeExportBaseName = ( .map((char) => char.charCodeAt(0) < 32 || reservedCharacters.includes(char) ? "-" : char ) - .join(""); - return sanitized || "Exported"; + .join("") + .replace(/[ .]+$/g, ""); + + if (!sanitized) { + return "Exported"; + } + if (/^(?:con|prn|aux|nul|com[1-9]|lpt[1-9])$/i.test(sanitized)) { + return `${sanitized}-file`; + } + return sanitized; }; diff --git a/tests/ipcValidation.test.ts b/tests/ipcValidation.test.ts index 25f3726..93811a5 100644 --- a/tests/ipcValidation.test.ts +++ b/tests/ipcValidation.test.ts @@ -1,9 +1,12 @@ import { strict as assert } from "assert"; import { + MAX_EXPORT_HTML_BYTES, MAX_MARKDOWN_FILE_BYTES, normalizeExportFilePayload, normalizeSaveFilePayload, safeExportBaseName, + validateExportFilePayload, + validateSaveFilePayload, } from "../src/main/ipcValidation"; const runTest = (name: string, fn: () => void) => { @@ -31,6 +34,17 @@ runTest("save payload validation rejects oversized content", () => { assert.equal(normalizeSaveFilePayload({ content: oversized }), null); }); +runTest("save payload validation reports specific errors", () => { + assert.deepEqual(validateSaveFilePayload({ content: 12 }), { + ok: false, + message: "Save content must be text.", + }); + assert.deepEqual(validateSaveFilePayload({ content: "ok", filePath: "" }), { + ok: false, + message: "Save file path must be a non-empty string.", + }); +}); + runTest("save payload validation accepts valid content", () => { assert.deepEqual(normalizeSaveFilePayload({ content: "ok" }), { content: "ok", @@ -45,8 +59,49 @@ runTest("export payload validation rejects unsupported formats", () => { ); }); +runTest("export payload validation rejects oversized content", () => { + const oversized = "x".repeat(MAX_EXPORT_HTML_BYTES + 1); + + assert.equal( + normalizeExportFilePayload({ content: oversized, format: "html" }), + null + ); + assert.deepEqual( + validateExportFilePayload({ content: oversized, format: "html" }), + { ok: false, message: "Export content is too large." } + ); +}); + +runTest("export payload validation rejects non-string default names", () => { + assert.equal( + normalizeExportFilePayload({ + content: "

ok

", + format: "html", + defaultFileName: 12, + }), + null + ); +}); + +runTest("export payload validation accepts valid payloads", () => { + assert.deepEqual( + normalizeExportFilePayload({ + content: "

ok

", + format: "pdf", + defaultFileName: "Notes", + }), + { + content: "

ok

", + format: "pdf", + defaultFileName: "Notes", + } + ); +}); + runTest("export default names are sanitized to a base filename", () => { assert.equal(safeExportBaseName("../notes:bad?.html"), "notes-bad-"); + assert.equal(safeExportBaseName("CON.pdf"), "CON-file"); + assert.equal(safeExportBaseName("notes. "), "notes"); assert.equal(safeExportBaseName(""), "Exported"); });