Skip to content

Fix/system hardening#92

Merged
mind-murtaza merged 15 commits into
devfrom
fix/system-hardening
Jun 3, 2026
Merged

Fix/system hardening#92
mind-murtaza merged 15 commits into
devfrom
fix/system-hardening

Conversation

@Ashminita28
Copy link
Copy Markdown
Collaborator

@Ashminita28 Ashminita28 commented Jun 3, 2026

Description

This PR solves

Type of Change

  • Bug fix
  • New feature
  • Refactor (no functional change)
  • Documentation update
  • Style / UI change
  • Build / CI change

Checklist

  • My code follows the project's coding guidelines
  • I have tested my changes locally
  • I have used conventional commit messages
  • I have updated documentation if needed
  • My changes do not introduce new warnings or errors

Electron main/preload

  • exportPDF: write generated HTML to a uniquely-named temp .html file and load with pdfWindow.loadFile(tempFilePath); close/wait-for-window-closed in finally before best-effort rm(tempFilePath), logging failures
  • autoUpdater: clear previous listeners before re-registering to prevent duplicate event handling
  • IPC/SEARCH_FOLDER: use platform-correct path separator (path.sep) when checking folder authorization
  • validatePath: build Windows forbidden-system-folder prefixes from SystemDrive (defaulting to C:) instead of hardcoding c:\...
  • Menu/Theme: register application menu with github-light on startup; buildMenuTemplate(currentTheme) and registerMenu(currentTheme) added so theme radio shows active selection
  • Settings: getSettings now rejects arrays by guarding with !Array.isArray(parsed) before merging with defaults

Renderer UI

  • useFileActions: guard for missing window.api before calling openFileDialog
  • Sidebar/App: stop deriving TOC inline from activeTab; pass precomputed activeToc into Sidebar
  • useFilePersistence: track isMounted, clear debounce/scroll timers on unmount, and guard against state updates after unmount; include toc conditionally in loaded tab payload; tighten scroll timer guards and dependencies

Markdown rendering

  • Heading IDs/TOC: introduce registry-driven heading utilities (createHeadingRegistry and getHeadingId) and update heading renderer to accept a registry to avoid shared global heading-id state across render passes
  • getMarkdown: now requires a registry parameter and constructs a Marked instance per call (module-level cached Marked removed)
  • renderMarkdown/toc: create a per-call heading registry so heading ID state is scoped to each render; extractTOC uses getHeadingId(registry) for deduped IDs
  • Shiki: attach .catch to highlighter initialization to reset cached highlighter to null on failure and rethrow

Shared packages

  • .coderabbit.yaml: fix path filter for shared package TypeScript files from packages/shared-* /src/**/*.tspackages/shared-*/src/**/*.ts

Tooling/CI

  • (covered above: .coderabbit.yaml path-filter fix)

Docs

  • Remove duplicate top-level # Architecture heading from docs/docs/architecture.md and versioned docs/version-1.0.0/architecture.md

Breaking changes: None

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

Walkthrough

This 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.

Changes

Core Functionality Improvements

Layer / File(s) Summary
Per-render heading registry & TOC integration
apps/renderer/src/utils/helpers/heading-helper.ts, apps/renderer/src/config/marked.ts, apps/renderer/src/renderer/markdown.ts, apps/renderer/src/renderer/toc.ts, apps/renderer/src/App.tsx
Introduce createHeadingRegistry() and pass a registry through getMarkdown(registry)marked renderer → extractTOC/getHeadingId to produce stable, per-render unique heading IDs and stop deriving TOC inline in App.
Marked per-call instance
apps/renderer/src/config/marked.ts
Remove module singleton; getMarkdown(registry) constructs a fresh Marked instance bound to the provided registry.
PDF export temp-file lifecycle
apps/main-processor/src/export/exportPdf.ts
Generate unique temp .html in OS temp dir, write sanitized HTML, use pdfWindow.loadFile(tempFilePath), then close window and best-effort rm(tempFilePath, { force: true }) with errors logged.
Menu theme propagation & startup
apps/main-processor/src/menu.ts, apps/main-processor/src/register-menu.ts, apps/main-processor/src/index.ts
buildMenuTemplate(currentTheme) and registerMenu(currentTheme) added; theme radio items set checked when matching currentTheme; startup calls registerMenu('github-light').
IPC path validation & settings parsing
apps/main-processor/src/ipc.ts, apps/main-processor/src/settings/get-settings.ts, apps/main-processor/src/utils/constants/ipc-validation.ts
Use path.sep for SEARCH_FOLDER prefix checks (${root}${sep}) for cross-platform correctness; getSettings rejects arrays (!Array.isArray(parsed)); Windows forbidden prefixes derived from process.env.SystemDrive.
Renderer initialization & API guards
apps/renderer/src/renderer/shiki.ts, apps/renderer/src/hooks/useFileActions.ts
Attach .catch() to createHighlighterCore() to reset cached highlighter on failure and rethrow; openFileDialog now early-returns if window.api is absent.
File persistence timers and unmount safety
apps/renderer/src/hooks/useFilePersistence.ts
Track mount-state with isMounted, clear debounce/scroll timers on unmount, guard debounced updates against unmounted state, include toc conditionally in payload, and tighten scroll restoration guards and deps.
Auto-updater listener reset
apps/main-processor/src/updater.ts
Call removeAllListeners for updater events before (re)registering IPC-emitting handlers.
Docs and CI pattern fixes
.coderabbit.yaml, docs/docs/architecture.md, docs/versioned_docs/version-1.0.0/architecture.md, apps/renderer/index.html
Fix shared package glob in .coderabbit.yaml; remove redundant top-level Architecture headings in docs; update CSP meta content string.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes


Possibly related PRs


Suggested reviewers

  • mind-murtaza
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive Title is vague and generic, using broad terms like 'Fix/system hardening' that don't convey the specific nature of the changes across PDF export, heading IDs, path validation, and other areas. Revise to a more specific title reflecting the main change, such as 'Refactor heading ID generation to use per-render registry' or 'Fix path validation and PDF temp file cleanup across app' depending on the primary focus.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/system-hardening

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Harden preload API guard and handle dialog rejection.

Line 35 only checks window.api; if the bridge shape is partial, openFileDialog can 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

📥 Commits

Reviewing files that changed from the base of the PR and between 09ce480 and 3930380.

📒 Files selected for processing (15)
  • .coderabbit.yaml
  • apps/main-processor/src/export/exportPdf.ts
  • apps/main-processor/src/index.ts
  • apps/main-processor/src/ipc.ts
  • apps/main-processor/src/menu.ts
  • apps/main-processor/src/register-menu.ts
  • apps/main-processor/src/settings/get-settings.ts
  • apps/renderer/src/App.tsx
  • apps/renderer/src/hooks/useFileActions.ts
  • apps/renderer/src/renderer/markdown.ts
  • apps/renderer/src/renderer/shiki.ts
  • apps/renderer/src/renderer/toc.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • docs/docs/architecture.md
  • docs/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.ts
  • apps/renderer/src/renderer/toc.ts
  • apps/renderer/src/hooks/useFileActions.ts
  • apps/renderer/src/App.tsx
  • apps/main-processor/src/menu.ts
  • apps/main-processor/src/settings/get-settings.ts
  • apps/renderer/src/renderer/shiki.ts
  • apps/main-processor/src/index.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • apps/main-processor/src/ipc.ts
  • apps/renderer/src/renderer/markdown.ts
  • apps/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.ts
  • apps/main-processor/src/menu.ts
  • apps/main-processor/src/settings/get-settings.ts
  • apps/main-processor/src/index.ts
  • apps/main-processor/src/ipc.ts
  • apps/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.ts
  • apps/main-processor/src/menu.ts
  • apps/main-processor/src/settings/get-settings.ts
  • apps/main-processor/src/index.ts
  • apps/main-processor/src/ipc.ts
  • apps/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.ts
  • apps/renderer/src/hooks/useFileActions.ts
  • apps/renderer/src/App.tsx
  • apps/renderer/src/renderer/shiki.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • apps/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.ts
  • apps/renderer/src/hooks/useFileActions.ts
  • apps/renderer/src/App.tsx
  • apps/renderer/src/renderer/shiki.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • apps/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.ts
  • apps/renderer/src/hooks/useFileActions.ts
  • apps/renderer/src/App.tsx
  • apps/renderer/src/renderer/shiki.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • apps/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.ts
  • apps/renderer/src/renderer/shiki.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • apps/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.yaml
  • apps/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.yaml
  • apps/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.ts
  • apps/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.yaml
  • apps/main-processor/src/index.ts
  • apps/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.ts
  • apps/main-processor/src/ipc.ts
  • apps/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.ts
  • apps/main-processor/src/ipc.ts
  • apps/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 into pdfWindow.
File: apps/main-processor/src/export/exportPdf.ts (lines 24-25)

await writeFile(tempFilePath, html, 'utf8');
await pdfWindow.loadFile(tempFilePath);
  • Confirm the exact html written to tempFilePath is sanitized using the same rules expected by SECURITY.md for Markdown sanitization (remove <script>/executable content, strip inline event handlers like on*, and block dangerous URL schemes like javascript:).
  • 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 of script, on*= handlers, or javascript:.
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!

Comment thread apps/main-processor/src/export/exportPdf.ts Outdated
Comment thread apps/renderer/src/utils/helpers/heading-helper.ts Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 3930380 and 0c5992f.

📒 Files selected for processing (10)
  • apps/main-processor/src/export/exportPdf.ts
  • apps/main-processor/src/updater.ts
  • apps/main-processor/src/utils/constants/ipc-validation.ts
  • apps/renderer/index.html
  • apps/renderer/src/App.tsx
  • apps/renderer/src/config/marked.ts
  • apps/renderer/src/hooks/useFilePersistence.ts
  • apps/renderer/src/renderer/markdown.ts
  • apps/renderer/src/renderer/toc.ts
  • apps/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.ts
  • apps/renderer/src/renderer/markdown.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • apps/main-processor/src/utils/constants/ipc-validation.ts
  • apps/renderer/src/config/marked.ts
  • apps/renderer/src/hooks/useFilePersistence.ts
  • apps/renderer/src/renderer/toc.ts
  • apps/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.ts
  • apps/main-processor/src/utils/constants/ipc-validation.ts
  • apps/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.ts
  • apps/main-processor/src/utils/constants/ipc-validation.ts
  • apps/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.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • apps/renderer/src/config/marked.ts
  • apps/renderer/src/hooks/useFilePersistence.ts
  • apps/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.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • apps/renderer/src/config/marked.ts
  • apps/renderer/src/hooks/useFilePersistence.ts
  • apps/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.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • apps/renderer/src/config/marked.ts
  • apps/renderer/src/hooks/useFilePersistence.ts
  • apps/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.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • apps/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.ts
  • apps/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.ts
  • apps/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.ts
  • apps/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.ts
  • apps/renderer/src/utils/helpers/heading-helper.ts
  • apps/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 win

Guard stale side-effects during in-flight loadFile when switching tabs

In apps/renderer/src/hooks/useFilePersistence.ts (debounced reload callback), after the await loadFile(...) you only check result and isMounted. If the user switches tabs mid-flight, the dispatch targets the old tabId, but the subsequent requestAnimationFrame scroll restore and setShowToast(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 an activeTabRef).

🛟 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.

Comment on lines +9 to +11
autoUpdater.removeAllListeners('update-available');
autoUpdater.removeAllListeners('update-downloaded');
autoUpdater.removeAllListeners('error');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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' . || true

Repository: 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' . || true

Repository: 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' . || true

Repository: 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*\(" . || true

Repository: 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 || true

Repository: 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).

Comment on lines +42 to +48
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\\`,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Comment thread apps/renderer/index.html
<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:"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Suggested change
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.

@mind-murtaza mind-murtaza merged commit e621a5f into dev Jun 3, 2026
3 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Jun 5, 2026
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants