Dev - #91
Dev#91
Conversation
Feature/drag drop
…g the main window
Fix/system hardening
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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/utils/helpers/heading-helper.ts (1)
36-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDecode HTML entities before building TOC text and ids.
parseInline(token.text)escapes entities first, so a heading like## A & BbecomesA & Bhere.getHeadingId()then slugs that toa-amp-b, while the rendered heading ID is still derived from the raw text and staysa-b, so the sidebar TOC link points at the wrong anchor.💡 Proposed fix
export function headingText(token: Tokens.Heading): string { const inlineHtml = parseInline(token.text) as string; - return stripHtml(inlineHtml).trim(); + return decodeHtml(stripHtml(inlineHtml)).trim(); }🤖 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/utils/helpers/heading-helper.ts` around lines 36 - 38, headingText() is using parseInline(token.text) which returns escaped HTML entities, causing mismatched TOC IDs vs rendered headings; update headingText (and ensure getHeadingId uses the same source) to decode HTML entities after parseInline and before stripHtml/slugging so "A & B" becomes "A & B"; use your project's HTML-decoding utility (or DOMParser/textarea/unescape helper) on the parsed inline string, then call stripHtml(...).trim() and pass that decoded string to getHeadingId to keep TOC links consistent with rendered heading IDs.
🤖 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/renderer/src/utils/helpers/heading-helper.ts`:
- Around line 23-27: The heading renderer currently escapes the stripped text
and loses inline markdown while headingText builds plain text differently,
causing TOC ID drift; update heading({ text, depth }, registry) to render the
inner HTML from the inline tokens via this.parser.parseInline(tokens) (i.e., use
the parser’s inline-rendered HTML for the hN content) and compute the id by
decoding entities from the same plain source passed to getHeadingId so both
paths normalize identically; in practice use the parser to produce safe HTML for
the element body and run decodeHtml(...) (or the same decode path used by
headingText) on the plain-result before calling getHeadingId (keeping
getHeadingId usage intact) to keep TOC slugs and emitted IDs synchronized.
---
Outside diff comments:
In `@apps/renderer/src/utils/helpers/heading-helper.ts`:
- Around line 36-38: headingText() is using parseInline(token.text) which
returns escaped HTML entities, causing mismatched TOC IDs vs rendered headings;
update headingText (and ensure getHeadingId uses the same source) to decode HTML
entities after parseInline and before stripHtml/slugging so "A & B" becomes
"A & B"; use your project's HTML-decoding utility (or
DOMParser/textarea/unescape helper) on the parsed inline string, then call
stripHtml(...).trim() and pass that decoded string to getHeadingId to keep TOC
links consistent with rendered heading IDs.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0c16d786-ac72-45e6-a54d-ac359bb6f055
📒 Files selected for processing (20)
.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/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/useFileActions.tsapps/renderer/src/hooks/useFilePersistence.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/docs/architecture.md
- docs/versioned_docs/version-1.0.0/architecture.md
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/main-processor/src/updater.ts
- apps/renderer/src/renderer/shiki.ts
- apps/main-processor/src/menu.ts
- apps/main-processor/src/export/exportPdf.ts
- apps/renderer/index.html
- apps/renderer/src/hooks/useFileActions.ts
- apps/renderer/src/hooks/useFilePersistence.ts
- apps/main-processor/src/settings/get-settings.ts
- apps/main-processor/src/index.ts
- apps/renderer/src/App.tsx
- apps/main-processor/src/ipc.ts
- .coderabbit.yaml
| export function heading({ text, depth }: HeadingProps, registry: Map<string, number>) { | ||
| const plainText = stripHtml(text); | ||
| const id = getHeadingId(plainText, registry); | ||
| const safeText = escapeHtml(plainText); | ||
| return `<h${depth} id="${id}">${safeText}</h${depth}>\n`; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Official Marked docs for custom heading renderers: should renderer.headingrender heading content viathis.parser.parseInline(tokens) to preserve inline markdown formatting?
💡 Result:
Yes. In Marked’s official extensibility docs, the renderer.heading override example renders the heading’s inline content by calling this.parser.parseInline(tokens) (where tokens is the heading token’s inline token array), specifically so inline markdown inside the heading (e.g., **bold**, links, code spans) is preserved in the generated HTML. [1]
Source
- [1] Marked Documentation – “Extending Marked” / Renderer example (
heading({ tokens, depth }) { const text = this.parser.parseInline(tokens); ... }) (marked.js.org)
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and show the relevant sections (small excerpts)
FILE="apps/renderer/src/utils/helpers/heading-helper.ts"
if [ -f "$FILE" ]; then
echo "== $FILE =="
wc -l "$FILE"
sed -n '1,120p' "$FILE" | nl -ba | sed -n '1,120p'
else
echo "Missing $FILE"
fi
# Find where heading-helper's functions are used (especially for TOC)
rg -n "headingHelper|headingText|getHeadingId|createHeadingRegistry|decodeHtml|heading\(" apps/renderer/src -S || true
rg -n "toc|table of contents|TOC" apps/renderer/src -S || true
rg -n "getHeadingId\(" -S . || trueRepository: mindfiredigital/markdown-reader
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/renderer/src/utils/helpers/heading-helper.ts"
if [ -f "$FILE" ]; then
echo "== $FILE =="
wc -l "$FILE"
sed -n '1,120p' "$FILE" | cat -n
else
echo "Missing $FILE"
fi
echo "== usages in apps/renderer/src =="
rg -n "heading-helper|headingText|getHeadingId|createHeadingRegistry|decodeHtml|renderer\.heading|heading\(" apps/renderer/src -S || true
echo "== TOC related =="
rg -n "(toc|table of contents|TOC)" apps/renderer/src -S || true
echo "== getHeadingId references across repo =="
rg -n "getHeadingId\(" -S . || trueRepository: mindfiredigital/markdown-reader
Length of output: 9849
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' apps/renderer/src/config/marked.ts | cat -n
echo "----"
sed -n '1,120p' apps/renderer/src/renderer/toc.ts | cat -n
echo "----"
sed -n '1,120p' apps/renderer/src/renderer/markdown.ts | cat -nRepository: mindfiredigital/markdown-reader
Length of output: 3821
🏁 Script executed:
sed -n '1,120p' apps/renderer/src/hooks/useFile.ts | cat -nRepository: mindfiredigital/markdown-reader
Length of output: 2538
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print the HeadingProps type definition
rg -n "export (type|interface) HeadingProps|HeadingProps" apps/renderer/src/types/component-types.ts apps/renderer/src -S || true
# Show around the match
MATCH_FILE="apps/renderer/src/types/component-types.ts"
if [ -f "$MATCH_FILE" ]; then
# Print entire file if small enough; otherwise print around likely area
wc -l "$MATCH_FILE"
sed -n '1,220p' "$MATCH_FILE" | cat -n
fiRepository: mindfiredigital/markdown-reader
Length of output: 5948
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json =="
ls -la package.json apps/renderer/package.json 2>/dev/null || true
cat package.json 2>/dev/null | head -n 120 || true
cat apps/renderer/package.json 2>/dev/null | head -n 120 || true
echo "== marked version from lockfile (if present) =="
ls -ლა package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || true
rg -n "\"marked\"\\s*:" package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || true
rg -n "marked@" pnpm-lock.yaml yarn.lock 2>/dev/null || true
echo "== check for marked package availability =="
node -e "try{const m=require('marked'); console.log('marked loaded', typeof m, Object.keys(m).slice(0,5));}catch(e){console.error('marked require failed:', e.message); process.exit(0)}"Repository: mindfiredigital/markdown-reader
Length of output: 3763
🌐 Web query:
Marked v18 renderer.heading props shape { tokens, depth } or { text, depth } and recommended usage to preserve inline markdown
💡 Result:
In Marked v18, renderer methods receive the token object itself rather than a list of individual parameters [1][2]. For a heading renderer, the expected prop shape is { tokens, depth, raw } (where tokens is an array of inline tokens) [3][4][1]. Recommended usage to preserve inline markdown involves using the provided tokens and the parser's built-in methods [1][5]. Specifically, you should use this.parser.parseInline(tokens) to convert the heading's inline tokens into HTML, which ensures that markdown formatting (like bold, italics, or links) within the heading text is correctly processed [3][1][5]. Example of correct implementation: renderer: { heading({ tokens, depth }) { // Correctly parse the inline tokens to preserve markdown formatting const text = this.parser.parseInline(tokens); return <h${depth}>${text}</h${depth}>\n; } } Key points for v18 renderers: - Token Object: The renderer receives a single token object of type Tokens.Heading [4][1]. - Parser Access: The renderer context (this) provides access to the parser, specifically this.parser.parseInline for inline elements and this.parser.parse for block elements [3][5]. - Avoiding Deprecation: Ensure you are accepting the token object instead of positional arguments, as older signatures may no longer be supported or are deprecated [1][2].
Citations:
- 1: Linking back to the source markedjs/marked#3591
- 2: feat: Send token objects to renderers markedjs/marked#3291
- 3: https://github.com/markedjs/marked/blob/v18.0.0/docs/USING_PRO.md
- 4: https://github.com/markedjs/marked/blob/0ccf4769/src/Renderer.ts
- 5: https://github.com/markedjs/marked/blob/0ccf4769/docs/USING_PRO.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/renderer/src/utils/constants/regex-constants.ts"
if [ -f "$FILE" ]; then
wc -l "$FILE"
sed -n '1,200p' "$FILE" | cat -n
else
echo "Missing $FILE"
fiRepository: mindfiredigital/markdown-reader
Length of output: 1267
🌐 Web query:
Marked v18 Tokens.Heading properties: what is token.text and what is renderer.heading props.text vs tokens
💡 Result:
In Marked v18, the distinction between token properties and renderer arguments centers on how the library handles raw markdown versus parsed content [1][2]. Token.text The text property on a Heading token (Tokens.Heading) contains the raw markdown text of the heading, excluding the leading hashes (#) and surrounding whitespace that define the heading level [1][3]. It represents the content that needs to be further parsed to handle inline markdown elements like bold, italic, or links [2]. Renderer.heading Props In recent versions of Marked, the renderer methods (including heading) receive a single token object as an argument, rather than a list of individual parameters [2][4]. Heading Renderer Signature: The heading renderer expects a Tokens.Heading object, which contains: - tokens: An array of parsed inline tokens generated from the heading text [1][3]. This should be passed to this.parser.parseInline(tokens) to convert the heading's inner markdown into HTML [2]. - depth: The numeric level of the heading (e.g., 1 for h1, 2 for h2) [1][3]. - raw: The original, full markdown string that produced the token [2]. - text: The raw text content of the heading [1]. Key Difference When customizing the renderer, you should use token.tokens to render the content to HTML, as it allows Marked to correctly process inline elements [2]. Using token.text directly would treat the heading content as plain text, bypassing the parser and failing to render inline markdown (like formatting) correctly [2].
Citations:
- 1: https://github.com/markedjs/marked/blob/0ccf4769/src/Tokens.ts
- 2: Linking back to the source markedjs/marked#3591
- 3: https://github.com/markedjs/marked/blob/0ccf4769/src/Tokenizer.ts
- 4: feat: Send token objects to renderers markedjs/marked#3291
Fix heading rendering to preserve inline markdown and keep TOC IDs in sync
apps/renderer/src/utils/helpers/heading-helper.ts:heading()renders<h*>fromstripHtml(text)+escapeHtml(...), which outputs escaped raw heading content and drops inline markdown formatting (e.g.,*emphasis*,code,[link](...)). Update the Markedrenderer.headingimplementation to generate the heading HTML from the heading’s inline tokens viathis.parser.parseInline(tokens)(as Marked’s extension API intends), rather than escaping the raw text.apps/renderer/src/utils/helpers/heading-helper.ts:headingText()runsparseInline(token.text)and strips tags, but it never decodes HTML entities, sogetHeadingId()can produce different slugs vs the IDs emitted byheading()(the file’sdecodeHtml()is currently unused). Decode entities before slug generation, or normalize both code paths from the same plain-text source so TOC anchors don’t drift.
🤖 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/utils/helpers/heading-helper.ts` around lines 23 - 27, The
heading renderer currently escapes the stripped text and loses inline markdown
while headingText builds plain text differently, causing TOC ID drift; update
heading({ text, depth }, registry) to render the inner HTML from the inline
tokens via this.parser.parseInline(tokens) (i.e., use the parser’s
inline-rendered HTML for the hN content) and compute the id by decoding entities
from the same plain source passed to getHeadingId so both paths normalize
identically; in practice use the parser to produce safe HTML for the element
body and run decodeHtml(...) (or the same decode path used by headingText) on
the plain-result before calling getHeadingId (keeping getHeadingId usage intact)
to keep TOC slugs and emitted IDs synchronized.
mind-murtaza
left a comment
mind-murtaza
left a comment
There was a problem hiding this comment.
Overall, this looks like a solid update with several good improvements around IPC validation, file handling, Shiki initialization, and TOC generation. However, I found a few items that should be addressed before merging.
1. Path Traversal / Arbitrary File Read in Export
File: apps/main-processor/src/export/inlineImage.ts
The export flow reads image paths directly from rendered HTML and inlines them without validating the resolved path. This could allow a crafted markdown document to reference arbitrary local files, which would then be embedded into exported PDF/HTML/DOCX outputs.
Recommendation: Validate image paths before reading them and ensure they resolve within approved directories (e.g., markdown workspace roots). Using realpath() and verifying the resolved path is contained within an allowed root would provide stronger protection.
2. CSS Sanitization Coverage
File: apps/main-processor/src/utils/constants/export-constants.ts
The current CSS sanitization rules cover several legacy injection vectors but do not block external resource loading through @import or remote url(...) references.
Recommendation: Extend the sanitization patterns to block:
@importurl(http://...)url(https://...)- protocol-relative URLs (
url(//...))
This will reduce the risk of exported HTML loading remote resources when opened in a browser.
3. Marked Instance Recreation
File: apps/renderer/src/config/marked.ts
The previous implementation cached the Marked instance, whereas the new implementation creates a fresh instance and re-registers extensions on every render. This introduces unnecessary allocations and setup work for large documents.
Recommendation: Consider extracting reusable configuration and caching the shared portions of the setup to avoid repeated initialization overhead.
4. Theme Selection via Array Index
File: apps/renderer/src/config/marked.ts
Using THEMES[1] creates a hidden dependency on array ordering and can silently break if the list changes in the future.
Recommendation: Use an explicit theme constant (e.g. 'github-dark') or introduce a named default theme constant.
5. Unbounded customCss Length
File: apps/main-processor/src/utils/helper/setting-helper.ts
customCss currently accepts strings of any size. Extremely large values could negatively impact export performance and increase processing overhead.
Recommendation: Introduce a reasonable maximum size limit consistent with the validation approach used for other settings.
Minor / Follow-up Items
- Remove the redundant guard inside the debounce callback in
useFilePersistence.ts. - Add an
isMountedcheck inside the scroll persistence timeout callback. - Consider removing the silent
realpath()fallback infolder.tssince the current flow already resolves paths earlier. - Tighten
validateSender()by restricting acceptedfile://origins to known renderer locations. - Consider moving the
ErrorBoundaryhigher in the component tree to provide broader crash protection.
Overall, I think the merge is very close. The export-related security concerns should be resolved before merging, while the remaining items can be addressed either in this PR or as follow-up improvements.
…nown renderer locations
Fix/system hardening
2 participants
Description
This PR raised to merge dev into main
Type of Change
Checklist
Summary by CodeRabbit
New Features
Improvements
Documentation