Fix/system hardening#92
Conversation
WalkthroughThis PR tightens Electron main-process behaviors (menu theme, IPC validation, updater listeners, PDF export temp-file handling) and refactors renderer heading ID handling to use per-render registries while adding defensive guards for shiki, file actions, and persistence timers. ChangesCore Functionality Improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/renderer/src/hooks/useFileActions.ts (1)
35-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHarden preload API guard and handle dialog rejection.
Line 35 only checks
window.api; if the bridge shape is partial,openFileDialogcan still throw. The promise chain also lacks.catch(), so IPC failures can surface as unhandled rejections.Suggested patch
const openFileDialog = useCallback(() => { - if (!window.api) return; - void window.api.openFileDialog().then((chosenPath) => { + if (!window.api?.openFileDialog) return; + void window.api.openFileDialog().then((chosenPath) => { if (chosenPath) { void loadFileInTab(chosenPath); } - }); + }).catch(() => { + // optionally surface a toast/log event for dialog failure + }); }, [loadFileInTab]);As per coding guidelines: “Keep components typed, accessible, keyboard-friendly, and resilient to missing preload APIs.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/renderer/src/hooks/useFileActions.ts` around lines 35 - 40, The current handler in useFileActions uses a weak guard (if (!window.api) return) and calls window.api.openFileDialog() without ensuring the method exists or handling rejections; update the guard to verify window.api?.openFileDialog is a function before calling, and handle errors from the IPC call by using async/await with try/catch (or adding .catch()) so failures don't produce unhandled rejections; ensure you still call loadFileInTab(chosenPath) only when chosenPath is truthy and propagate or log errors via the existing logger or UI feedback instead of letting them bubble.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/main-processor/src/export/exportPdf.ts`:
- Around line 50-55: pdfWindow.close() returns before the window is actually
destroyed, so move the temp-file deletion to run after the BrowserWindow emits
its 'closed' event: call pdfWindow.close(), attach a one-time listener like
pdfWindow.once('closed', async () => { await rm(tempFilePath, { force: true })
... }), and keep the existing try/catch to log errors; reference pdfWindow,
tempFilePath, and rm to locate where to apply the change and ensure the file is
removed only after the window is fully closed.
In `@apps/renderer/src/utils/helpers/heading-helper.ts`:
- Around line 5-10: The module-level idRegistry and resetHeadingRegistry create
shared global state that causes cross-request coupling; replace this with a
per-document heading ID generator by removing the module-level const idRegistry
and instead provide a factory like createHeadingRegistry() (or change functions
that currently reference idRegistry such as resetHeadingRegistry, any
generateHeadingId/getHeadingId helpers) to accept or return a registry instance
(Map<string, number>) so each markdown/TOC parse uses its own registry; update
all call sites to create and pass the registry for that parse and remove the
global resetHeadingRegistry usage.
---
Outside diff comments:
In `@apps/renderer/src/hooks/useFileActions.ts`:
- Around line 35-40: The current handler in useFileActions uses a weak guard (if
(!window.api) return) and calls window.api.openFileDialog() without ensuring the
method exists or handling rejections; update the guard to verify
window.api?.openFileDialog is a function before calling, and handle errors from
the IPC call by using async/await with try/catch (or adding .catch()) so
failures don't produce unhandled rejections; ensure you still call
loadFileInTab(chosenPath) only when chosenPath is truthy and propagate or log
errors via the existing logger or UI feedback instead of letting them bubble.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 44989606-9141-40d8-941f-68184561ca32
📒 Files selected for processing (15)
.coderabbit.yamlapps/main-processor/src/export/exportPdf.tsapps/main-processor/src/index.tsapps/main-processor/src/ipc.tsapps/main-processor/src/menu.tsapps/main-processor/src/register-menu.tsapps/main-processor/src/settings/get-settings.tsapps/renderer/src/App.tsxapps/renderer/src/hooks/useFileActions.tsapps/renderer/src/renderer/markdown.tsapps/renderer/src/renderer/shiki.tsapps/renderer/src/renderer/toc.tsapps/renderer/src/utils/helpers/heading-helper.tsdocs/docs/architecture.mddocs/versioned_docs/version-1.0.0/architecture.md
💤 Files with no reviewable changes (2)
- docs/versioned_docs/version-1.0.0/architecture.md
- docs/docs/architecture.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Use TypeScript as the primary language for the application
Files:
apps/main-processor/src/register-menu.tsapps/renderer/src/renderer/toc.tsapps/renderer/src/hooks/useFileActions.tsapps/renderer/src/App.tsxapps/main-processor/src/menu.tsapps/main-processor/src/settings/get-settings.tsapps/renderer/src/renderer/shiki.tsapps/main-processor/src/index.tsapps/renderer/src/utils/helpers/heading-helper.tsapps/main-processor/src/ipc.tsapps/renderer/src/renderer/markdown.tsapps/main-processor/src/export/exportPdf.ts
apps/main-processor/**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Use Marked for parsing Markdown content
Files:
apps/main-processor/src/register-menu.tsapps/main-processor/src/menu.tsapps/main-processor/src/settings/get-settings.tsapps/main-processor/src/index.tsapps/main-processor/src/ipc.tsapps/main-processor/src/export/exportPdf.ts
apps/main-processor/src/**/*.ts
⚙️ CodeRabbit configuration file
apps/main-processor/src/**/*.ts: Review as Electron main-process code.
- IPC handlers must use shared constants and validate renderer input.
- File/folder access must guard path traversal, missing files, permissions, symlinks, and deleted watched files.
- Watchers, menus, dialogs, and IPC listeners must be cleaned up.
- Do not expose Node/Electron internals or unrestricted filesystem access.
- Export/update/download flows must sanitize content, close resources, and avoid executing embedded scripts.
Files:
apps/main-processor/src/register-menu.tsapps/main-processor/src/menu.tsapps/main-processor/src/settings/get-settings.tsapps/main-processor/src/index.tsapps/main-processor/src/ipc.tsapps/main-processor/src/export/exportPdf.ts
apps/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
apps/renderer/**/*.{ts,tsx}: Use React for frontend UI components
Use Shiki for syntax highlighting in code blocks
Use KaTeX for mathematical equation rendering
Use Mermaid for diagram rendering
Files:
apps/renderer/src/renderer/toc.tsapps/renderer/src/hooks/useFileActions.tsapps/renderer/src/App.tsxapps/renderer/src/renderer/shiki.tsapps/renderer/src/utils/helpers/heading-helper.tsapps/renderer/src/renderer/markdown.ts
apps/renderer/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (README.md)
Use Tailwind CSS for styling
Files:
apps/renderer/src/renderer/toc.tsapps/renderer/src/hooks/useFileActions.tsapps/renderer/src/App.tsxapps/renderer/src/renderer/shiki.tsapps/renderer/src/utils/helpers/heading-helper.tsapps/renderer/src/renderer/markdown.ts
apps/renderer/src/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/*.{ts,tsx}: Review as React renderer code.
- Keep components typed, accessible, keyboard-friendly, and resilient to missing preload APIs.
- Effects must have correct dependencies and cleanup.
- Handle loading, empty, error, stale-response, and rejected-promise states.
- Do not import Node-only modules into renderer code.
- Avoid unnecessary derived state, unsafe globals, and broad any types.
Files:
apps/renderer/src/renderer/toc.tsapps/renderer/src/hooks/useFileActions.tsapps/renderer/src/App.tsxapps/renderer/src/renderer/shiki.tsapps/renderer/src/utils/helpers/heading-helper.tsapps/renderer/src/renderer/markdown.ts
apps/renderer/src/**/{renderer,markdown,utils}/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/{renderer,markdown,utils}/**/*.{ts,tsx}: Review markdown rendering carefully.
- Sanitize raw HTML, links, images, Mermaid, KaTeX, anchors, and exported content.
- Block script execution, javascript: URLs, unsafe inline handlers, and unsafe local file references.
- Heading IDs and TOC entries must be stable and collision-safe.
- Mermaid/KaTeX/code highlighting failures should not break the whole document.
- Add tests for unsafe HTML, malformed markdown, links, images, code blocks, Mermaid, and KaTeX when changed.
Files:
apps/renderer/src/renderer/toc.tsapps/renderer/src/renderer/shiki.tsapps/renderer/src/utils/helpers/heading-helper.tsapps/renderer/src/renderer/markdown.ts
apps/renderer/src/**/*.{css,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/*.{css,tsx}: Review UI, theme, and accessibility.
- Interactive controls need semantic elements, visible focus, and keyboard access.
- Theme changes must preserve readable contrast in light and dark modes.
- Markdown prose must remain readable for tables, code, blockquotes, links, lists, and images.
- Prefer existing tokens/classes over ad hoc inline styling.
Files:
apps/renderer/src/App.tsx
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: docs/versioned_docs/version-1.0.0/contribution-guide/how-to-contribute.md:0-0
Timestamp: 2026-05-29T10:14:10.971Z
Learning: Bug-fix PRs must address only one bug or issue
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: docs/versioned_docs/version-1.0.0/release-notes.md:0-0
Timestamp: 2026-05-29T10:15:09.313Z
Learning: Applies to docs/versioned_docs/version-1.0.0/**/RELEASE*NOTES*.md|**/release*notes*.md|**/CHANGELOG*.md|**/changelog*.md|**/releases/**/*.md|**/docs/**/release*.md : Document release highlights in release notes, including features like Markdown rendering with GFM support, Mermaid diagram rendering, KaTeX math rendering, syntax highlighting, tabs, search, themes, font zoom, file watching and live reload, recent files, and file association support
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {main.{js,ts},electron.{js,ts},preload.{js,ts},src/main/**/*.{js,ts}} : Limit IPC exposure to only necessary communication channels
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {main.{js,ts},electron.{js,ts},preload.{js,ts},src/main/**/*.{js,ts}} : Enable `contextIsolation` in Electron application configuration
Learnt from: CR
Repo: mindfiredigital/markdown-reader
Timestamp: 2026-06-03T11:39:07.260Z
Learning: File open operations must follow the 7-step sequential flow: Renderer initiates -> Preload Bridge translates to IPC READ_FILE token -> Main Process validates -> Filesystem reads -> Filesystem returns bytes -> Main Process forwards response -> Preload Bridge delivers to Renderer state
Learnt from: CR
Repo: mindfiredigital/markdown-reader
Timestamp: 2026-06-03T11:39:14.092Z
Learning: File read operations must follow the 7-step sequential flow: Renderer calls `readFile(path)` → Preload Bridge converts to `IPC READ_FILE` token → Main Process validates and checks security → Filesystem reads file → Filesystem returns content → Main Process forwards to Preload Bridge → Preload Bridge delivers to Renderer state
Learnt from: CR
Repo: mindfiredigital/markdown-reader
Timestamp: 2026-06-03T11:39:14.092Z
Learning: Subsystem modules including Local Filesystem, Export Modules, and File Watcher must be managed directly by the Main Process
Learnt from: CR
Repo: mindfiredigital/markdown-reader
Timestamp: 2026-06-03T11:39:14.092Z
Learning: The File Watcher subsystem must monitor project directories for automated file changes and send real-time update notifications back through the Main Process to the Renderer
📚 Learning: 2026-05-29T10:12:19.223Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {src/components/**/*.{ts,tsx},src/renderer/**/*.{ts,tsx},src/utils/**/*.{ts,tsx}} : Filter dangerous HTML elements and event handlers during Markdown sanitization
Applied to files:
apps/renderer/src/renderer/toc.ts.coderabbit.yamlapps/renderer/src/renderer/markdown.ts
📚 Learning: 2026-05-29T10:13:28.986Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: docs/docs/product/features.md:0-0
Timestamp: 2026-05-29T10:13:28.986Z
Learning: Implement auto-generated table of contents from headings in rendered Markdown
Applied to files:
apps/renderer/src/renderer/toc.ts
📚 Learning: 2026-05-29T10:12:02.313Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: README.md:0-0
Timestamp: 2026-05-29T10:12:02.313Z
Learning: Applies to apps/main-processor/**/*.{ts,tsx} : Use Marked for parsing Markdown content
Applied to files:
apps/renderer/src/renderer/toc.ts.coderabbit.yamlapps/renderer/src/renderer/markdown.ts
📚 Learning: 2026-05-29T10:12:02.313Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: README.md:0-0
Timestamp: 2026-05-29T10:12:02.313Z
Learning: Generate table of contents automatically in the sidebar based on document headings
Applied to files:
apps/renderer/src/renderer/toc.tsapps/renderer/src/App.tsx
📚 Learning: 2026-05-29T10:14:44.000Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: docs/versioned_docs/version-1.0.0/product/features.md:0-0
Timestamp: 2026-05-29T10:14:44.000Z
Learning: Implement active heading highlighting while scrolling through Markdown content
Applied to files:
apps/renderer/src/App.tsx
📚 Learning: 2026-05-29T10:12:41.254Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: docs/docs/contribution-guide/how-to-contribute.md:0-0
Timestamp: 2026-05-29T10:12:41.254Z
Learning: Applies to docs/docs/contribution-guide/**/*.{test,spec}.{ts,tsx,js,jsx} : Write unit tests for all new features and ensure all tests pass before submitting changes using `pnpm vitest`
Applied to files:
.coderabbit.yaml
📚 Learning: 2026-05-29T10:12:10.468Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-05-29T10:12:10.468Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Write unit tests for all new features to maintain code quality, adding test cases alongside component source code and ensuring all tests pass via `pnpm vitest`
Applied to files:
.coderabbit.yaml
📚 Learning: 2026-05-29T10:12:02.313Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: README.md:0-0
Timestamp: 2026-05-29T10:12:02.313Z
Learning: Use shared-types and shared-constants packages for code reuse across app layers
Applied to files:
.coderabbit.yaml
📚 Learning: 2026-05-29T10:12:41.254Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: docs/docs/contribution-guide/how-to-contribute.md:0-0
Timestamp: 2026-05-29T10:12:41.254Z
Learning: Avoid touching unrelated files for formatting, refactors, renames, or cleanups - only include files required for your component or fix
Applied to files:
.coderabbit.yaml
📚 Learning: 2026-05-29T10:12:19.223Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {main.{js,ts},electron.{js,ts},preload.{js,ts},src/main/**/*.{js,ts}} : Limit IPC exposure to only necessary communication channels
Applied to files:
.coderabbit.yamlapps/main-processor/src/index.tsapps/main-processor/src/ipc.ts
📚 Learning: 2026-05-29T10:12:02.313Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: README.md:0-0
Timestamp: 2026-05-29T10:12:02.313Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Use Vitest for testing
Applied to files:
.coderabbit.yaml
📚 Learning: 2026-05-29T10:12:02.313Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: README.md:0-0
Timestamp: 2026-05-29T10:12:02.313Z
Learning: Applies to apps/renderer/**/*.{ts,tsx} : Use Shiki for syntax highlighting in code blocks
Applied to files:
apps/renderer/src/renderer/shiki.ts
📚 Learning: 2026-05-29T10:14:58.167Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: docs/versioned_docs/version-1.0.0/product/markdown-support.md:0-0
Timestamp: 2026-05-29T10:14:58.167Z
Learning: Applies to docs/versioned_docs/version-1.0.0/product/**/*.md : Use Shiki for code block syntax highlighting with language-aware highlighting for 100+ languages, controlled by the active app theme
Applied to files:
apps/renderer/src/renderer/shiki.ts
📚 Learning: 2026-05-29T10:12:19.223Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {main.{js,ts},electron.{js,ts},preload.{js,ts},src/main/**/*.{js,ts}} : Disable `nodeIntegration` in Electron application configuration
Applied to files:
apps/main-processor/src/index.tsapps/main-processor/src/ipc.tsapps/main-processor/src/export/exportPdf.ts
📚 Learning: 2026-05-29T10:12:19.223Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {main.{js,ts},electron.{js,ts},preload.{js,ts},src/main/**/*.{js,ts}} : Enable `contextIsolation` in Electron application configuration
Applied to files:
apps/main-processor/src/index.tsapps/main-processor/src/ipc.tsapps/main-processor/src/export/exportPdf.ts
📚 Learning: 2026-05-29T10:12:19.223Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {src/components/**/*.{ts,tsx},src/renderer/**/*.{ts,tsx},src/utils/**/*.{ts,tsx}} : Remove script tags from Markdown content during sanitization
Applied to files:
apps/renderer/src/renderer/markdown.ts
📚 Learning: 2026-05-29T10:12:19.223Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {src/components/**/*.{ts,tsx},src/renderer/**/*.{ts,tsx}} : Sanitize user-rendered Markdown content before rendering using DOMPurify
Applied to files:
apps/renderer/src/renderer/markdown.ts
📚 Learning: 2026-05-29T10:14:44.000Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: docs/versioned_docs/version-1.0.0/product/features.md:0-0
Timestamp: 2026-05-29T10:14:44.000Z
Learning: Render Markdown using GitHub Flavored Markdown specification
Applied to files:
apps/renderer/src/renderer/markdown.ts
📚 Learning: 2026-05-29T10:14:30.340Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: docs/versioned_docs/version-1.0.0/introduction.md:0-0
Timestamp: 2026-05-29T10:14:30.340Z
Learning: Ensure comfortable reading experience for Markdown content, prioritizing legibility and visual presentation of rendered Markdown
Applied to files:
apps/renderer/src/renderer/markdown.ts
🔇 Additional comments (11)
.coderabbit.yaml (1)
116-116: LGTM!apps/renderer/src/renderer/markdown.ts (1)
2-2: LGTM!Also applies to: 7-7
apps/renderer/src/App.tsx (1)
201-201: LGTM!apps/renderer/src/renderer/shiki.ts (1)
21-23: LGTM!apps/main-processor/src/menu.ts (1)
5-5: LGTM!Also applies to: 78-78
apps/main-processor/src/register-menu.ts (1)
4-5: LGTM!apps/main-processor/src/index.ts (1)
100-100: LGTM!apps/main-processor/src/export/exportPdf.ts (2)
2-4: LGTM!Also applies to: 16-19
24-25: Ensure export-to-PDF HTML is sanitized before loading it intopdfWindow.
File:apps/main-processor/src/export/exportPdf.ts(lines 24-25)await writeFile(tempFilePath, html, 'utf8'); await pdfWindow.loadFile(tempFilePath);
- Confirm the exact
htmlwritten totempFilePathis sanitized using the same rules expected bySECURITY.mdfor Markdown sanitization (remove<script>/executable content, strip inline event handlers likeon*, and block dangerous URL schemes likejavascript:).- Add a regression test exporting PDF from malicious Markdown (e.g.
<img src=x onerror=alert(1)>,<a href="javascript:alert(1)">x</a>,<script>alert(1)</script>) and assert the generated/written HTML contains none ofscript,on*=handlers, orjavascript:.apps/main-processor/src/ipc.ts (1)
2-2: LGTM!Also applies to: 213-213
apps/main-processor/src/settings/get-settings.ts (1)
12-12: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/main-processor/src/updater.ts`:
- Around line 9-11: Currently the code calls
autoUpdater.removeAllListeners('update-available'|'update-downloaded'|'error'),
which can clobber other feature listeners; instead declare module-scoped handler
functions (e.g. onUpdateAvailable, onUpdateDownloaded, onUpdateError) and use
autoUpdater.off('update-available', onUpdateAvailable) /
off('update-downloaded', onUpdateDownloaded) / off('error', onUpdateError) (or
removeListener) before re-registering them in setupAutoUpdater so only these
specific handlers are removed and re-bound (refer to autoUpdater and
setupAutoUpdater in updater.ts).
In `@apps/main-processor/src/utils/constants/ipc-validation.ts`:
- Around line 42-48: The current forbiddenPrefixes array in ipc-validation.ts
uses sysDrive and only blocks protected folders on the system drive; update the
logic so these protected folder checks are drive-agnostic: change
forbiddenPrefixes to use drive-independent names (e.g., "\\windows\\",
"\\winnt\\", "\\boot\\", "\\system volume information\\", "\\$recycle.bin\\")
and update the validatePath check to normalize the input with
path.win32.normalize().toLowerCase() and compare using either a regex that
allows any drive letter prefix (/^[a-z]:\\(windows|winnt|boot|system volume
information|\\$recycle\\.bin)\\/) or by stripping path.parse(p).root and
checking the remainder startsWith these folder names with a trailing backslash;
reference sysDrive, forbiddenPrefixes and validatePath in ipc-validation.ts when
making the change.
In `@apps/renderer/index.html`:
- Line 12: Update the Content-Security-Policy meta string in
apps/renderer/index.html to explicitly deny unused sinks: add directives
object-src 'none', base-uri 'none', frame-ancestors 'none', and form-action
'none' to the existing policy (the meta tag whose content currently starts with
"default-src 'self'; script-src 'self'; ..."). Keep existing directives
(style-src, img-src, font-src) and preserve any 'unsafe-inline' or data
allowances intentionally used; simply append the new deny directives separated
by semicolons to harden the policy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b6ec815d-5189-44fc-b642-1abc9e9c08e7
📒 Files selected for processing (10)
apps/main-processor/src/export/exportPdf.tsapps/main-processor/src/updater.tsapps/main-processor/src/utils/constants/ipc-validation.tsapps/renderer/index.htmlapps/renderer/src/App.tsxapps/renderer/src/config/marked.tsapps/renderer/src/hooks/useFilePersistence.tsapps/renderer/src/renderer/markdown.tsapps/renderer/src/renderer/toc.tsapps/renderer/src/utils/helpers/heading-helper.ts
💤 Files with no reviewable changes (1)
- apps/renderer/src/App.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Use TypeScript as the primary language for the application
Files:
apps/main-processor/src/updater.tsapps/renderer/src/renderer/markdown.tsapps/renderer/src/utils/helpers/heading-helper.tsapps/main-processor/src/utils/constants/ipc-validation.tsapps/renderer/src/config/marked.tsapps/renderer/src/hooks/useFilePersistence.tsapps/renderer/src/renderer/toc.tsapps/main-processor/src/export/exportPdf.ts
apps/main-processor/**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Use Marked for parsing Markdown content
Files:
apps/main-processor/src/updater.tsapps/main-processor/src/utils/constants/ipc-validation.tsapps/main-processor/src/export/exportPdf.ts
apps/main-processor/src/**/*.ts
⚙️ CodeRabbit configuration file
apps/main-processor/src/**/*.ts: Review as Electron main-process code.
- IPC handlers must use shared constants and validate renderer input.
- File/folder access must guard path traversal, missing files, permissions, symlinks, and deleted watched files.
- Watchers, menus, dialogs, and IPC listeners must be cleaned up.
- Do not expose Node/Electron internals or unrestricted filesystem access.
- Export/update/download flows must sanitize content, close resources, and avoid executing embedded scripts.
Files:
apps/main-processor/src/updater.tsapps/main-processor/src/utils/constants/ipc-validation.tsapps/main-processor/src/export/exportPdf.ts
apps/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
apps/renderer/**/*.{ts,tsx}: Use React for frontend UI components
Use Shiki for syntax highlighting in code blocks
Use KaTeX for mathematical equation rendering
Use Mermaid for diagram rendering
Files:
apps/renderer/src/renderer/markdown.tsapps/renderer/src/utils/helpers/heading-helper.tsapps/renderer/src/config/marked.tsapps/renderer/src/hooks/useFilePersistence.tsapps/renderer/src/renderer/toc.ts
apps/renderer/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (README.md)
Use Tailwind CSS for styling
Files:
apps/renderer/src/renderer/markdown.tsapps/renderer/src/utils/helpers/heading-helper.tsapps/renderer/src/config/marked.tsapps/renderer/src/hooks/useFilePersistence.tsapps/renderer/src/renderer/toc.ts
apps/renderer/src/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/*.{ts,tsx}: Review as React renderer code.
- Keep components typed, accessible, keyboard-friendly, and resilient to missing preload APIs.
- Effects must have correct dependencies and cleanup.
- Handle loading, empty, error, stale-response, and rejected-promise states.
- Do not import Node-only modules into renderer code.
- Avoid unnecessary derived state, unsafe globals, and broad any types.
Files:
apps/renderer/src/renderer/markdown.tsapps/renderer/src/utils/helpers/heading-helper.tsapps/renderer/src/config/marked.tsapps/renderer/src/hooks/useFilePersistence.tsapps/renderer/src/renderer/toc.ts
apps/renderer/src/**/{renderer,markdown,utils}/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/{renderer,markdown,utils}/**/*.{ts,tsx}: Review markdown rendering carefully.
- Sanitize raw HTML, links, images, Mermaid, KaTeX, anchors, and exported content.
- Block script execution, javascript: URLs, unsafe inline handlers, and unsafe local file references.
- Heading IDs and TOC entries must be stable and collision-safe.
- Mermaid/KaTeX/code highlighting failures should not break the whole document.
- Add tests for unsafe HTML, malformed markdown, links, images, code blocks, Mermaid, and KaTeX when changed.
Files:
apps/renderer/src/renderer/markdown.tsapps/renderer/src/utils/helpers/heading-helper.tsapps/renderer/src/renderer/toc.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: docs/versioned_docs/version-1.0.0/contribution-guide/how-to-contribute.md:0-0
Timestamp: 2026-05-29T10:14:10.971Z
Learning: Bug-fix PRs must address only one bug or issue
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {main.{js,ts},electron.{js,ts},preload.{js,ts},src/main/**/*.{js,ts}} : Limit IPC exposure to only necessary communication channels
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {main.{js,ts},electron.{js,ts},preload.{js,ts},src/main/**/*.{js,ts}} : Enable `contextIsolation` in Electron application configuration
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {main.{js,ts},electron.{js,ts},preload.{js,ts},src/main/**/*.{js,ts}} : Disable `nodeIntegration` in Electron application configuration
Learnt from: CR
Repo: mindfiredigital/markdown-reader
Timestamp: 2026-06-03T13:07:30.861Z
Learning: The application architecture consists of four distinct sequential layers: Renderer (React), Preload Bridge, Main Process IPC, and Subsystems (Local Filesystem, Export Modules, File Watcher)
Learnt from: CR
Repo: mindfiredigital/markdown-reader
Timestamp: 2026-06-03T13:07:30.861Z
Learning: File read operations must follow the sequential 7-step flow: Renderer calls readFile() → Preload Bridge converts to IPC READ_FILE token → Main Process validates sender and path → Filesystem reads file → response flows back through Main Process → Preload Bridge → Renderer
Learnt from: CR
Repo: mindfiredigital/markdown-reader
Timestamp: 2026-06-03T13:07:30.861Z
Learning: The File Watcher subsystem must monitor project directories for automated file changes and send real-time update notifications back through the Main Process to the Renderer
Learnt from: CR
Repo: mindfiredigital/markdown-reader
Timestamp: 2026-06-03T13:07:36.314Z
Learning: IPC communication flow: Renderer calls `readFile(path)` → Preload Bridge translates to `IPC READ_FILE` token → Main Process validates and executes → Filesystem reads file → Main Process forwards content back → Preload Bridge delivers to Renderer state
📚 Learning: 2026-05-29T10:12:02.313Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: README.md:0-0
Timestamp: 2026-05-29T10:12:02.313Z
Learning: Applies to apps/main-processor/**/*.{ts,tsx} : Use Marked for parsing Markdown content
Applied to files:
apps/renderer/src/renderer/markdown.tsapps/renderer/src/config/marked.ts
📚 Learning: 2026-05-29T10:12:19.223Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {src/components/**/*.{ts,tsx},src/renderer/**/*.{ts,tsx},src/utils/**/*.{ts,tsx}} : Remove script tags from Markdown content during sanitization
Applied to files:
apps/renderer/src/renderer/markdown.tsapps/renderer/src/config/marked.ts
📚 Learning: 2026-05-29T10:12:19.223Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {src/components/**/*.{ts,tsx},src/renderer/**/*.{ts,tsx}} : Sanitize user-rendered Markdown content before rendering using DOMPurify
Applied to files:
apps/renderer/src/renderer/markdown.tsapps/renderer/src/config/marked.ts
📚 Learning: 2026-05-29T10:12:19.223Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: SECURITY.md:0-0
Timestamp: 2026-05-29T10:12:19.223Z
Learning: Applies to {src/components/**/*.{ts,tsx},src/renderer/**/*.{ts,tsx},src/utils/**/*.{ts,tsx}} : Filter dangerous HTML elements and event handlers during Markdown sanitization
Applied to files:
apps/renderer/src/renderer/markdown.tsapps/renderer/src/utils/helpers/heading-helper.tsapps/renderer/src/config/marked.ts
📚 Learning: 2026-05-29T10:12:02.313Z
Learnt from: CR
Repo: mindfiredigital/markdown-reader PR: 0
File: README.md:0-0
Timestamp: 2026-05-29T10:12:02.313Z
Learning: Generate table of contents automatically in the sidebar based on document headings
Applied to files:
apps/renderer/src/renderer/toc.ts
🔇 Additional comments (7)
apps/main-processor/src/export/exportPdf.ts (1)
1-1: LGTM!Also applies to: 50-62
apps/renderer/src/utils/helpers/heading-helper.ts (1)
5-28: LGTM!apps/renderer/src/config/marked.ts (1)
8-20: LGTM!apps/renderer/src/renderer/markdown.ts (1)
6-13: LGTM!apps/renderer/src/renderer/toc.ts (1)
11-26: LGTM!apps/renderer/src/hooks/useFilePersistence.ts (2)
17-30: LGTM!
37-57: ⚡ Quick winGuard stale side-effects during in-flight
loadFilewhen switching tabsIn
apps/renderer/src/hooks/useFilePersistence.ts(debounced reload callback), after theawait loadFile(...)you only checkresultandisMounted. If the user switches tabs mid-flight, thedispatchtargets the oldtabId, but the subsequentrequestAnimationFramescroll restore andsetShowToast(true)can still run for the now-visible tab, causing an incorrect scroll jump and a toast for a background update. Gate the scroll restore + toast on the tab identity after the await (e.g., via anactiveTabRef).🛟 Sketch
- if (!result || !isMounted.current) return; + if (!result || !isMounted.current || activeTabRef.current?.id !== activeTab.id) return;Please confirm the intended behavior when switching tabs while a watched file reload is in flight.
| autoUpdater.removeAllListeners('update-available'); | ||
| autoUpdater.removeAllListeners('update-downloaded'); | ||
| autoUpdater.removeAllListeners('error'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether other modules also register autoUpdater listeners
# (if yes, removeAllListeners here is unsafe and can remove them).
rg -n -C3 --type=ts '\bautoUpdater\.(on|once)\s*\('Repository: mindfiredigital/markdown-reader
Length of output: 1112
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the current updater.ts context around the removeAllListeners calls
sed -n '1,120p' apps/main-processor/src/updater.ts | nl -ba | sed -n '1,120p'
# Find all uses of autoUpdater across the repo
rg -n --type=ts '\bautoUpdater\.' apps/main-processor/src || true
rg -n --type=ts '\bautoUpdater\.' . || true
# Find any other listener registrations for these event names
rg -n --type=ts "autoUpdater\.(on|once)\('update-(available|downloaded)'" . || true
rg -n --type=ts "autoUpdater\.(on|once)\('error'" . || true
# Find any removeAllListeners/off usage on autoUpdater
rg -n --type=ts '\bautoUpdater\.removeAllListeners\s*\(' . || true
rg -n --type=ts '\bautoUpdater\.(off|removeListener|removeListeners)\b' . || trueRepository: mindfiredigital/markdown-reader
Length of output: 118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the current updater.ts context around the removeAllListeners calls
sed -n '1,120p' apps/main-processor/src/updater.ts | nl -ba | sed -n '1,120p'
# Find all uses of autoUpdater across the repo
rg -n --type=ts '\bautoUpdater\.' apps/main-processor/src || true
rg -n --type=ts '\bautoUpdater\.' . || true
# Find any other listener registrations for these event names
rg -n --type=ts "autoUpdater\.(on|once)\('update-(available|downloaded)'" . || true
rg -n --type=ts "autoUpdater\.(on|once)\('error'" . || true
# Find any removeAllListeners/off usage on autoUpdater
rg -n --type=ts '\bautoUpdater\.removeAllListeners\s*\(' . || true
rg -n --type=ts '\bautoUpdater\.(off|removeListener|removeListeners)\b' . || trueRepository: mindfiredigital/markdown-reader
Length of output: 118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show updater.ts with line numbers (nl isn't available)
cat -n apps/main-processor/src/updater.ts | sed -n '1,140p'
# Find all uses of autoUpdater across the repo
rg -n --type=ts '\bautoUpdater\.' apps/main-processor/src || true
rg -n --type=ts '\bautoUpdater\.' . || true
# Find any other listener registrations for these event names
rg -n --type=ts "autoUpdater\.(on|once)\('update-(available|downloaded)'" . || true
rg -n --type=ts "autoUpdater\.(on|once)\('error'" . || true
# Find any removeAllListeners/off usage on autoUpdater
rg -n --type=ts '\bautoUpdater\.removeAllListeners\s*\(' . || true
rg -n --type=ts '\bautoUpdater\.(off|removeListener|removeListeners)\b' . || trueRepository: mindfiredigital/markdown-reader
Length of output: 3377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Where setupAutoUpdater is called
rg -n --type=ts "setupAutoUpdater\(" .
# Any autoUpdater listener registrations (broader patterns) elsewhere
rg -n --type=ts "\bautoUpdater\.(on|once)\s*\(" . || true
# Any autoUpdater listener cleanup elsewhere
rg -n --type=ts "\bautoUpdater\.(off|removeListener|removeAllListeners)\s*\(" . || trueRepository: mindfiredigital/markdown-reader
Length of output: 792
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant portion of index.ts around setupAutoUpdater call
cat -n apps/main-processor/src/index.ts | sed -n '1,140p'
# Check if main window creation is potentially re-invoked (search for createMainWindow or BrowserWindow instantiation)
rg -n --type=ts "new BrowserWindow|createMainWindow|mainWindow|setupAutoUpdater\(" apps/main-processor/src || trueRepository: mindfiredigital/markdown-reader
Length of output: 5615
Avoid autoUpdater.removeAllListeners(...) clobbering other handlers (apps/main-processor/src/updater.ts lines 9-11): those calls remove all listeners for the given events; even though this repo currently only registers these handlers in setupAutoUpdater, a second feature (or another createWindow() run) could silently wipe it. Keep module-scoped handler references and use autoUpdater.off('update-available', handler) / off('update-downloaded', handler) / off('error', handler) (or removeListener) before re-registering.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/main-processor/src/updater.ts` around lines 9 - 11, Currently the code
calls
autoUpdater.removeAllListeners('update-available'|'update-downloaded'|'error'),
which can clobber other feature listeners; instead declare module-scoped handler
functions (e.g. onUpdateAvailable, onUpdateDownloaded, onUpdateError) and use
autoUpdater.off('update-available', onUpdateAvailable) /
off('update-downloaded', onUpdateDownloaded) / off('error', onUpdateError) (or
removeListener) before re-registering them in setupAutoUpdater so only these
specific handlers are removed and re-bound (refer to autoUpdater and
setupAutoUpdater in updater.ts).
| const sysDrive = (process.env.SystemDrive ?? 'C:').toLowerCase(); | ||
| const forbiddenPrefixes = [ | ||
| 'c:\\windows\\', | ||
| 'c:\\winnt\\', | ||
| 'c:\\boot\\', | ||
| 'c:\\system volume information\\', | ||
| 'c:\\$recycle.bin\\', | ||
| `${sysDrive}\\windows\\`, | ||
| `${sysDrive}\\winnt\\`, | ||
| `${sysDrive}\\boot\\`, | ||
| `${sysDrive}\\system volume information\\`, | ||
| `${sysDrive}\\$recycle.bin\\`, |
There was a problem hiding this comment.
Block protected Windows folders on all volumes, not only %SystemDrive%.
On Line 42 and Lines 44-48, forbidden prefixes are derived only from %SystemDrive%. Volume-scoped protected directories (notably \$Recycle.Bin and \System Volume Information) on other drives can still pass validatePath (e.g., D:\$Recycle.Bin\...), and those paths flow into export/directory IPC checks.
Suggested fix
- const sysDrive = (process.env.SystemDrive ?? 'C:').toLowerCase();
+ const sysDrive = (process.env.SystemDrive ?? 'C:').toLowerCase();
+ const driveMatch = lower.match(/^([a-z]:)\\/);
+ if (!driveMatch) return false;
+ const inputDrive = driveMatch[1];
const forbiddenPrefixes = [
`${sysDrive}\\windows\\`,
`${sysDrive}\\winnt\\`,
`${sysDrive}\\boot\\`,
- `${sysDrive}\\system volume information\\`,
- `${sysDrive}\\$recycle.bin\\`,
+ `${inputDrive}\\system volume information\\`,
+ `${inputDrive}\\$recycle.bin\\`,
];As per coding guidelines: "File/folder access must guard path traversal, missing files, permissions, symlinks, and deleted watched files."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/main-processor/src/utils/constants/ipc-validation.ts` around lines 42 -
48, The current forbiddenPrefixes array in ipc-validation.ts uses sysDrive and
only blocks protected folders on the system drive; update the logic so these
protected folder checks are drive-agnostic: change forbiddenPrefixes to use
drive-independent names (e.g., "\\windows\\", "\\winnt\\", "\\boot\\", "\\system
volume information\\", "\\$recycle.bin\\") and update the validatePath check to
normalize the input with path.win32.normalize().toLowerCase() and compare using
either a regex that allows any drive letter prefix
(/^[a-z]:\\(windows|winnt|boot|system volume information|\\$recycle\\.bin)\\/)
or by stripping path.parse(p).root and checking the remainder startsWith these
folder names with a trailing backslash; reference sysDrive, forbiddenPrefixes
and validatePath in ipc-validation.ts when making the change.
| <meta | ||
| http-equiv="Content-Security-Policy" | ||
| content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:" | ||
| content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self' data:" |
There was a problem hiding this comment.
Harden CSP further with explicit deny directives.
This policy still omits object-src, base-uri, frame-ancestors, and form-action, which leaves unnecessary surface open. Add explicit deny rules for defense-in-depth.
Suggested CSP tightening
- content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self' data:"
+ content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self' data:; object-src 'none'; base-uri 'none'; frame-ancestors 'none'; form-action 'none'"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self' data:" | |
| content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self' data:; object-src 'none'; base-uri 'none'; frame-ancestors 'none'; form-action 'none'" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/renderer/index.html` at line 12, Update the Content-Security-Policy meta
string in apps/renderer/index.html to explicitly deny unused sinks: add
directives object-src 'none', base-uri 'none', frame-ancestors 'none', and
form-action 'none' to the existing policy (the meta tag whose content currently
starts with "default-src 'self'; script-src 'self'; ..."). Keep existing
directives (style-src, img-src, font-src) and preserve any 'unsafe-inline' or
data allowances intentionally used; simply append the new deny directives
separated by semicolons to harden the policy.
Description
This PR solves
Type of Change
Checklist
Electron main/preload
c:\...github-lighton startup; buildMenuTemplate(currentTheme) and registerMenu(currentTheme) added so theme radio shows active selection!Array.isArray(parsed)before merging with defaultsRenderer UI
Markdown rendering
Shared packages
packages/shared-* /src/**/*.ts→packages/shared-*/src/**/*.tsTooling/CI
Docs
# Architectureheading from docs/docs/architecture.md and versioned docs/version-1.0.0/architecture.mdBreaking changes: None