feat: add markdown preview with toggle to view raw source#863
feat: add markdown preview with toggle to view raw source#863Naresh-official wants to merge 5 commits intosourcebot-dev:mainfrom
Conversation
WalkthroughAdds markdown preview functionality to the code browser with a toggle between preview and code viewing modes. Introduces UI components for toggling views, rendering markdown with syntax highlighting and theme support, and API endpoints for serving raw repository files with proper MIME type handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SourcePreviewPanelClient
participant MarkdownPreview as PureMarkDownPreviewPanel
participant CodePreview as PureCodePreviewPanel
participant API as Raw File API
User->>SourcePreviewPanelClient: View markdown file
SourcePreviewPanelClient->>SourcePreviewPanelClient: Detect language = "Markdown"
SourcePreviewPanelClient->>User: Show Preview/Code toggle
User->>SourcePreviewPanelClient: Click "Preview" mode
SourcePreviewPanelClient->>MarkdownPreview: Render with markdown source
MarkdownPreview->>MarkdownPreview: Parse markdown & detect URLs
alt Internal Repository Link
MarkdownPreview->>API: Fetch raw file from /api/{domain}/raw/{path}
API->>API: Resolve Git reference
API->>User: Return file with Content-Type
MarkdownPreview->>MarkdownPreview: Render with resolved URL
else External Link
MarkdownPreview->>MarkdownPreview: Render with target="_blank"
end
MarkdownPreview->>User: Display styled markdown with syntax highlighting
User->>SourcePreviewPanelClient: Click "Code" mode
SourcePreviewPanelClient->>CodePreview: Render source code
CodePreview->>User: Display syntax-highlighted code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@packages/web/src/app/`[domain]/browse/[...path]/components/codePreviewPanelClient.tsx:
- Line 26: Remove the debugging console.log in the CodePreviewPanelClient
component: delete the line that calls console.log({language, path, repoName,
revisionName}) (or replace it with a dev-only guard using
NODE_ENV==='development' if you want to keep local debugging). Locate the
console.log call inside the CodePreviewPanelClient component in
codePreviewPanelClient.tsx and remove or guard it so it does not run in
production.
In
`@packages/web/src/app/`[domain]/browse/[...path]/components/pureMarkDownPreviewPanel.tsx:
- Around line 86-99: The video renderer in pureMarkDownPreviewPanel.tsx (the
video: ({ src = "", ...props }) => { ... } handler) currently passes src through
unchanged, so relative video paths break; update this handler to rewrite
relative src the same way the img and source handlers do (use the same
IMAGE_BASE_URL or the existing resolve logic used for img/source) before
rendering the <video>, ensuring the src is resolved for both absolute and
relative URLs while preserving {...props} and controls.
- Line 22: IMAGE_BASE_URL is currently hardcoded to GitHub's raw domain which
breaks image resolution for non-GitHub repos; update
pureMarkDownPreviewPanel.tsx to build the image base URL from the repository's
code host instead of assuming raw.githubusercontent.com: either accept a
precomputed imageBaseUrl prop from the server component (which knows the code
host) or add code-host-aware logic that inspects repoName/revisionName and
composes the correct raw file URL for GitHub, GitLab, Bitbucket, and ADO; change
the constant IMAGE_BASE_URL to use the new prop or host-aware builder and adjust
any references in the component to use that value.
- Around line 25-26: In pureMarkDownPreviewPanel.tsx replace the hardcoded
bg-white on the wrapper with bg-background and remove the unconditional "dark"
class on the <article>; instead wire the component to the app theme (e.g., use
next-themes or the existing theme context) and conditionally add the "dark"
class to the article only when the current theme is dark so github-markdown-css
toggles correctly, and ensure you use Tailwind tokens like
bg-background/border-border/text-foreground rather than CSS variable syntax;
update the JSX in the PureMarkDownPreviewPanel component to read the theme and
apply classes accordingly.
- Around line 27-29: The ReactMarkdown rendering currently uses rehypeRaw
without sanitization (see ReactMarkdown component and the rehypePlugins array
containing rehypeRaw and rehypeHighlight); import and add rehypeSanitize to the
rehypePlugins list immediately after rehypeRaw so raw HTML is sanitized before
rendering, and if your UI requires additional tags/attributes (e.g., video,
source, iframe attrs), extend rehypeSanitize's schema (starting from
defaultSchema) to allowlist those elements/attributes before passing it to
rehypeSanitize.
🧹 Nitpick comments (4)
packages/web/src/app/[domain]/browse/[...path]/components/pureMarkDownPreviewPanel.tsx (2)
150-160: Hardcodedtext-blue-600doesn't respect the app's theme.Use a semantic Tailwind color class (e.g.,
text-primaryortext-foreground) instead.As per coding guidelines, "Use Tailwind color classes directly (e.g.,
border-border,bg-card,text-foreground) instead of CSS variable syntax (e.g.,border-[var(--border)])".
42-49: The.github/prefix check is redundant.
srcSet.startsWith(".github/")is already covered by!srcSet.startsWith("http"). The same redundancy exists in theimghandler (Line 65). Consider simplifying:Simplified condition
- if ( - srcSet.startsWith(".github/") || - !srcSet.startsWith("http") - ) { + if (!srcSet.startsWith("http")) {packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanelClient.tsx (2)
32-35:defaultValueis redundant in a controlledToggleGroup.Since you're passing
value={viewMode}, the component is controlled anddefaultValueis ignored by Radix. Remove it to avoid confusion.Fix
<ToggleGroup type="single" - defaultValue="preview" value={viewMode} onValueChange={(value) => value && setViewMode(value)} >
23-24: ImproveisMarkdowndetection readability and type safety.
- The
"gcc machine description"check is a necessary workaround — linguist-languages sometimes misidentifies.mdfiles with this language. Add a comment explaining this.- The
viewModestate should use a union type ("preview" | "code") instead ofstringfor type safety.- The long boolean expression is hard to scan and should be refactored for clarity.
Suggested refactor
- const [viewMode, setViewMode] = useState<string>("preview"); - const isMarkdown = language.toLowerCase() === "gcc machine description" || language.toLowerCase() === "md" || path.toLocaleLowerCase().endsWith(".md") || path.toLocaleLowerCase().endsWith(".markdown"); + const [viewMode, setViewMode] = useState<"preview" | "code">("preview"); + const lang = language.toLowerCase(); + const lowerPath = path.toLowerCase(); + // "gcc machine description" is included because some language classifiers + // misidentify .md files as GCC Machine Description files. + const isMarkdown = + lang === "markdown" || + lang === "gcc machine description" || + lowerPath.endsWith(".md") || + lowerPath.endsWith(".markdown");Note: linguist-languages returns
"Markdown"as the canonical language name for.mdfiles, not"md", so thelanguage === "md"check may be redundant. Use"markdown"to match the library's actual output.
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanelClient.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/app/[domain]/browse/[...path]/components/pureMarkDownPreviewPanel.tsx
Outdated
Show resolved
Hide resolved
| <div className="w-full flex justify-center bg-white dark:bg-background"> | ||
| <article className="markdown-body dark dark:bg-background w-full max-w-4xl px-6 py-10"> |
There was a problem hiding this comment.
Hardcoded bg-white and unconditional dark class cause theme issues.
bg-whitedoesn't respect the app's theme system. Usebg-backgroundfor both modes.- The
darkclass on the<article>is applied unconditionally, which will causegithub-markdown-cssto always render dark-mode styles, even when the app is in light mode.
Proposed fix
- <div className="w-full flex justify-center bg-white dark:bg-background">
- <article className="markdown-body dark dark:bg-background w-full max-w-4xl px-6 py-10">
+ <div className="w-full flex justify-center bg-background">
+ <article className="markdown-body w-full max-w-4xl px-6 py-10">You'll need to integrate with the app's theme (e.g., via next-themes) to conditionally apply github-markdown-css's dark/light mode class.
As per coding guidelines, "Use Tailwind color classes directly (e.g., border-border, bg-card, text-foreground) instead of CSS variable syntax (e.g., border-[var(--border)])".
🤖 Prompt for AI Agents
In
`@packages/web/src/app/`[domain]/browse/[...path]/components/pureMarkDownPreviewPanel.tsx
around lines 25 - 26, In pureMarkDownPreviewPanel.tsx replace the hardcoded
bg-white on the wrapper with bg-background and remove the unconditional "dark"
class on the <article>; instead wire the component to the app theme (e.g., use
next-themes or the existing theme context) and conditionally add the "dark"
class to the article only when the current theme is dark so github-markdown-css
toggles correctly, and ensure you use Tailwind tokens like
bg-background/border-border/text-foreground rather than CSS variable syntax;
update the JSX in the PureMarkDownPreviewPanel component to read the theme and
apply classes accordingly.
There was a problem hiding this comment.
@brendan-kellam The Comment image shows the CHANGELOG.md file rendered in preview mode with proper markdown formatting—version sections, issue links, and structured headings are all displaying correctly in dark theme. The PR description images demonstrate the README preview working beautifully with the Sourcebot branding, badges, and features section.
Thanks for the screenshot and the +1! The markdown preview feature is working well. Just to clarify—are you planning to address the theme-switching concern I flagged (the hardcoded bg-white and unconditional dark class), or would you like me to help with that implementation? Happy to provide guidance on wiring this up to next-themes if needed.
🧠 Learnings used
Learnt from: CR
Repo: sourcebot-dev/sourcebot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-05T22:59:12.488Z
Learning: Applies to **/*.{jsx,tsx} : Use Tailwind color classes directly (e.g., `border-border`, `bg-card`, `text-foreground`) instead of CSS variable syntax (e.g., `border-[var(--border)]`)
| <ReactMarkdown | ||
| remarkPlugins={[remarkGfm]} | ||
| rehypePlugins={[rehypeRaw, rehypeHighlight]} |
There was a problem hiding this comment.
Security: rehype-raw without rehype-sanitize enables XSS.
rehypeRaw passes raw HTML through to the rendered output. Without rehypeSanitize (already a project dependency), any markdown source containing <script>, <iframe>, onerror handlers, etc., will execute in the user's browser. This is especially dangerous since the markdown content comes from external repositories.
🔒 Proposed fix: add rehype-sanitize
import rehypeHighlight from "rehype-highlight";
import "github-markdown-css/github-markdown.css";
import "highlight.js/styles/github-dark.css";
import rehypeRaw from "rehype-raw";
+import rehypeSanitize, { defaultSchema } from "rehype-sanitize";Then update the plugins array — rehypeSanitize must come after rehypeRaw:
- rehypePlugins={[rehypeRaw, rehypeHighlight]}
+ rehypePlugins={[rehypeRaw, rehypeSanitize, rehypeHighlight]}You may need to extend defaultSchema to allowlist elements like <video>, <source>, etc., that your custom components rely on.
🤖 Prompt for AI Agents
In
`@packages/web/src/app/`[domain]/browse/[...path]/components/pureMarkDownPreviewPanel.tsx
around lines 27 - 29, The ReactMarkdown rendering currently uses rehypeRaw
without sanitization (see ReactMarkdown component and the rehypePlugins array
containing rehypeRaw and rehypeHighlight); import and add rehypeSanitize to the
rehypePlugins list immediately after rehypeRaw so raw HTML is sanitized before
rendering, and if your UI requires additional tags/attributes (e.g., video,
source, iframe attrs), extend rehypeSanitize's schema (starting from
defaultSchema) to allowlist those elements/attributes before passing it to
rehypeSanitize.
There was a problem hiding this comment.
@brendan-kellam Thanks for acknowledging this! Given the critical nature of the XSS vulnerability, I'd recommend addressing this before merging the PR. The fix is straightforward—just adding rehype-sanitize to the pipeline—but it's essential for safely rendering markdown from external repositories.
| video: ({ src = "", ...props }) => { | ||
| return ( | ||
| <video | ||
| src={src} | ||
| controls | ||
| preload="metadata" | ||
| className="max-w-full h-auto my-4" | ||
| {...props} | ||
| > | ||
| Your browser does not support the video | ||
| tag. | ||
| </video> | ||
| ); | ||
| }, |
There was a problem hiding this comment.
Video src URLs are not resolved for relative paths.
The img and source handlers rewrite relative URLs using IMAGE_BASE_URL, but the video handler passes src through as-is. Relative video paths in markdown will be broken.
🤖 Prompt for AI Agents
In
`@packages/web/src/app/`[domain]/browse/[...path]/components/pureMarkDownPreviewPanel.tsx
around lines 86 - 99, The video renderer in pureMarkDownPreviewPanel.tsx (the
video: ({ src = "", ...props }) => { ... } handler) currently passes src through
unchanged, so relative video paths break; update this handler to rewrite
relative src the same way the img and source handlers do (use the same
IMAGE_BASE_URL or the existing resolve logic used for img/source) before
rendering the <video>, ensuring the src is resolved for both absolute and
relative URLs while preserving {...props} and controls.
- Implement dynamic theme switching for markdown rendering using next-themes - Load GitHub markdown CSS and highlight.js themes based on dark/light mode - Enhance image handling with theme-aware source resolution
- Add internal API to serve raw files directly from git repositories. - Update markdown preview to resolve images and links using the new API. - Remove dependency on external GitHub raw URLs. - Fix local backend development script.
|
Thanks for the review and suggestions. I’ve now incorporated the changes you mentioned and updated the implementation accordingly. |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/web/src/app/[domain]/browse/[...path]/components/sourcePreviewPanel.tsx (1)
77-83:⚠️ Potential issue | 🔴 CriticalMissing
codeHostType,domain, andexternalWebUrlprops passed toSourcePreviewPanelClient.As noted in the
codePreviewPanelClient.tsxreview, this call site is missing required props.codeHostTypeis available fromrepoInfoResponse.codeHostType(line 34) andexternalWebUrlfromrepoInfoResponse.externalWebUrl. However,domainis not in scope here — it needs to be threaded from the page viaSourcePreviewPanelProps.Proposed fix (assuming `domain` is added to `SourcePreviewPanelProps`)
interface SourcePreviewPanelProps { path: string; repoName: string; revisionName?: string; + domain: string; } -export const SourcePreviewPanel = async ({ path, repoName, revisionName }: SourcePreviewPanelProps) => { +export const SourcePreviewPanel = async ({ path, repoName, revisionName, domain }: SourcePreviewPanelProps) => {<SourcePreviewPanelClient source={fileSourceResponse.source} language={fileSourceResponse.language} repoName={repoName} path={path} revisionName={revisionName ?? 'HEAD'} + codeHostType={repoInfoResponse.codeHostType} + externalWebUrl={repoInfoResponse.externalWebUrl ?? undefined} + domain={domain} />packages/web/src/app/[domain]/browse/[...path]/page.tsx (1)
92-97:⚠️ Potential issue | 🔴 Critical
domainshould be passed toSourcePreviewPanelto complete the prop chain.The
domainparameter is available fromparams(it's part of the[domain]route segment), but it's not destructured or forwarded. OnceSourcePreviewPanelPropsis updated to requiredomain, this call site needs to pass it.Proposed fix
const params = await props.params; const { path: _rawPath, + domain, } = params;<SourcePreviewPanel path={path} repoName={repoName} revisionName={revisionName} + domain={domain} />Note:
domainis missing from theBrowsePagePropstype at line 69 — it should be added there as well (it's already part of the Next.js route[domain]segment type at line 46).
🤖 Fix all issues with AI agents
In
`@packages/web/src/app/`[domain]/browse/[...path]/components/codePreviewPanelClient.tsx:
- Around line 8-17: SourcePreviewPanelClient is missing required props
codeHostType and domain at its call site; thread these from page.tsx through
SourcePreviewPanel to SourcePreviewPanelClient instead of making them optional:
add domain and codeHostType to SourcePreviewPanelProps, accept them in
SourcePreviewPanel (pull codeHostType from repoInfoResponse there), pass both
props into the <SourcePreviewPanelClient .../> invocation, and update
SourcePreviewPanelClientProps to include and use them when rendering
PureMarkDownPreviewPanel; also update page.tsx to supply params.domain into the
top-level SourcePreviewPanel.
In
`@packages/web/src/app/`[domain]/browse/[...path]/components/pureMarkDownPreviewPanel.tsx:
- Around line 36-65: The component's useEffect currently injects external CDN
links for GitHub markdown and Highlight.js styles which breaks
offline/self-hosted deployments and uses a terse arrow callback in the .forEach
that triggers a Biome lint warning; replace the runtime CDN injection in the
useEffect of PureMarkDownPreviewPanel (and any helper that creates
markdownLink/highlightLink) by bundling or importing the two CSS files into the
app (e.g., add them to static assets or import them as modules) and point the
injected link.href to the local asset paths or simply import the styles and
conditionally apply a CSS class based on currentTheme, and update the
.forEach((el) => el.remove()) to use an explicit block .forEach((el) => {
el.remove(); }) to satisfy the linter.
In `@packages/web/src/app/api/`[domain]/raw/[...path]/route.ts:
- Around line 6-35: The getContentType function currently returns
"image/svg+xml" for .svg files which allows inline SVG execution; update the
route handling that serves files so that responses for SVG files do not render
in-page: detect SVG responses using getContentType(filename) or by checking the
file extension and add either a strict Content-Security-Policy: sandbox header
(e.g. "sandbox;") or set Content-Disposition: attachment to force download for
.svg, and ensure other content types are unchanged; adjust any response-building
logic that references getContentType to include this special-case header for
SVGs.
In `@packages/web/src/features/git/getRawFileSourceApi.ts`:
- Around line 40-48: Remove the exploratory/dev comments around binary handling
and use the proper simple-git v3 API call: replace the incorrect
git.binaryCatFile(["blob", sha]) invocation with git.binaryCatFile(["-p", sha])
to match git cat-file -p semantics and retrieve the blob content; keep the
single call site in getRawFileSourceApi.ts and ensure binaryCatFile is used
directly (no extra commentary about availability).
🧹 Nitpick comments (6)
packages/web/src/features/git/getRawFileSourceApi.ts (1)
50-68: Fragile error-message matching for git failures.String matching on error messages (e.g.,
"exists on disk","fatal: path") is brittle across git versions and locales. This is acceptable for an MVP but consider a more robust approach long-term (e.g., checking git exit codes if simple-git exposes them via the error object).packages/web/src/app/api/[domain]/raw/[...path]/route.ts (2)
59-93: The"-"separator approach relies on no repo path segment being exactly"-".This works because
path.indexOf("-")matches only segments that are the literal string"-", not segments containing a hyphen (like"my-repo"). However, this means a repository named literally-(while unlikely) would break the parsing. Worth a brief comment in the code to document this convention.Additionally, in the legacy (no-separator) branch, the
path.length < 3check at line 74 is a duplicate of line 45 — both branches reach the same guard. It's harmless but redundant.
108-114:immutablecache directive is semantically misleading withmax-age=3600.
immutabletells the browser to never revalidate during the freshness window, but 1 hour is short for content addressed by git ref. Ifrefis a commit SHA,immutablewith a longermax-agewould be correct. Ifrefcan beHEADor a branch name (mutable),immutableis incorrect since the content can change. Consider varying the caching strategy based on whetherrefis a mutable reference.packages/web/src/app/[domain]/browse/[...path]/components/pureMarkDownPreviewPanel.tsx (1)
27-28: Unused propscodeHostTypeandexternalWebUrl.These are destructured and immediately aliased to underscore-prefixed variables (
_codeHostType,_externalWebUrl), indicating they're unused. Since the component now uses the internal API for URL resolution (as per the learnings), consider removing them from both the interface and destructuring to reduce API surface confusion. If they're being kept for future use, a brief comment would clarify intent.packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanelClient.tsx (1)
29-29: Consider a narrower type forviewMode.Using
useState<string>("preview")is overly broad. A union type makes the intent clearer and prevents invalid states.♻️ Proposed fix
- const [viewMode, setViewMode] = useState<string>("preview"); + const [viewMode, setViewMode] = useState<"preview" | "code">("preview");packages/web/src/app/[domain]/browse/[...path]/components/sourcePreviewPanel.tsx (1)
6-6: File namecodePreviewPanelClientis inconsistent with the exported componentSourcePreviewPanelClient.The import
from "./codePreviewPanelClient"references a file that exportsSourcePreviewPanelClient. The file should be renamed tosourcePreviewPanelClient.tsxto match the export and avoid confusion during maintenance.
| interface SourcePreviewPanelClientProps { | ||
| path: string; | ||
| repoName: string; | ||
| revisionName: string; | ||
| source: string; | ||
| language: string; | ||
| codeHostType: string; | ||
| externalWebUrl?: string; | ||
| domain: string; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the TypeScript props issue
echo "=== SourcePreviewPanelClientProps interface ==="
rg -n "interface SourcePreviewPanelClientProps" -g '*.tsx' -g '*.ts' -A 15
echo ""
echo "=== Call sites of SourcePreviewPanelClient ==="
rg -n "<SourcePreviewPanelClient" -g '*.tsx' -g '*.ts' -B 2 -A 10
echo ""
echo "=== SourcePreviewPanelProps interface ==="
rg -n "interface SourcePreviewPanelProps" -g '*.tsx' -g '*.ts' -A 15Repository: sourcebot-dev/sourcebot
Length of output: 5066
🏁 Script executed:
#!/bin/bash
# Check for type overrides around the call site and implementation
echo "=== Check for type overrides or 'as any' ==="
rg -n "SourcePreviewPanelClient" -g '*.tsx' -g '*.ts' -B 5 -A 12 | grep -E "(as any|as const|type |@ts-)"
echo ""
echo "=== SourcePreviewPanelClient implementation (full destructure) ==="
sed -n '19,35p' packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanelClient.tsx
echo ""
echo "=== Check if domain or codeHostType are used in the component ==="
rg -n "domain|codeHostType" packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanelClient.tsxRepository: sourcebot-dev/sourcebot
Length of output: 810
🏁 Script executed:
#!/bin/bash
# Verify that domain and codeHostType are actually required by child components
echo "=== Lines around 66-68 where domain and codeHostType are passed ==="
sed -n '60,75p' packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanelClient.tsx
echo ""
echo "=== Check SourcePreviewPanel to see if it has access to domain ==="
head -50 packages/web/src/app/[domain]/browse/[...path]/components/sourceReviewPanel.tsx
echo ""
echo "=== Find where SourcePreviewPanel is called from ==="
rg -n "<SourcePreviewPanel" -g '*.tsx' -g '*.ts' -B 5 -A 5Repository: sourcebot-dev/sourcebot
Length of output: 3430
🏁 Script executed:
#!/bin/bash
# Check the full SourcePreviewPanel component to see if it should receive domain/codeHostType
echo "=== Full SourcePreviewPanel component ==="
cat packages/web/src/app/[domain]/browse/[...path]/components/sourcePreviewPanel.tsx
echo ""
echo "=== page.tsx context - does it have domain/codeHostType? ==="
sed -n '1,100p' packages/web/src/app/[domain]/browse/[...path]/page.tsx | head -50Repository: sourcebot-dev/sourcebot
Length of output: 5310
🏁 Script executed:
#!/bin/bash
# Confirm: Check if repoInfoResponse has the data needed
echo "=== Does SourcePreviewPanel have access to codeHostType/externalWebUrl? ==="
grep -n "repoInfoResponse\." packages/web/src/app/[domain]/browse/[...path]/components/sourcePreviewPanel.tsx
echo ""
echo "=== Check page.tsx params structure ==="
sed -n '40,100p' packages/web/src/app/[domain]/browse/[...path]/page.tsxRepository: sourcebot-dev/sourcebot
Length of output: 2351
Missing required props codeHostType and domain at the call site will cause a TypeScript compilation error.
SourcePreviewPanelClientProps declares codeHostType: string and domain: string as required, but the parent SourcePreviewPanel (in sourcePreviewPanel.tsx, lines 77-82) does not pass them:
<SourcePreviewPanelClient
source={fileSourceResponse.source}
language={fileSourceResponse.language}
repoName={repoName}
path={path}
revisionName={revisionName ?? 'HEAD'}
/>SourcePreviewPanel has access to codeHostType from repoInfoResponse, but neither domain nor codeHostType are available in SourcePreviewPanelProps. domain needs to be threaded from page.tsx (which has it in params) through to SourcePreviewPanel, and both codeHostType and domain must be passed to SourcePreviewPanelClient, which uses them when rendering PureMarkDownPreviewPanel.
You need to either:
- Thread
domainandcodeHostTypethrough the component hierarchy frompage.tsx→SourcePreviewPanel→SourcePreviewPanelClient, or - Make these props optional and handle their absence in
PureMarkDownPreviewPanel.
🤖 Prompt for AI Agents
In
`@packages/web/src/app/`[domain]/browse/[...path]/components/codePreviewPanelClient.tsx
around lines 8 - 17, SourcePreviewPanelClient is missing required props
codeHostType and domain at its call site; thread these from page.tsx through
SourcePreviewPanel to SourcePreviewPanelClient instead of making them optional:
add domain and codeHostType to SourcePreviewPanelProps, accept them in
SourcePreviewPanel (pull codeHostType from repoInfoResponse there), pass both
props into the <SourcePreviewPanelClient .../> invocation, and update
SourcePreviewPanelClientProps to include and use them when rendering
PureMarkDownPreviewPanel; also update page.tsx to supply params.domain into the
top-level SourcePreviewPanel.
| useEffect(() => { | ||
| const isDark = currentTheme === "dark"; | ||
|
|
||
| document | ||
| .querySelectorAll( | ||
| "link[data-markdown-theme], link[data-highlight-theme]", | ||
| ) | ||
| .forEach((el) => el.remove()); | ||
|
|
||
| const markdownLink = document.createElement("link"); | ||
| markdownLink.rel = "stylesheet"; | ||
| markdownLink.href = isDark | ||
| ? "https://cdnjs.cloudflare.com/ajax/libs/github-markdown-css/5.5.1/github-markdown-dark.min.css" | ||
| : "https://cdnjs.cloudflare.com/ajax/libs/github-markdown-css/5.5.1/github-markdown-light.min.css"; | ||
| markdownLink.setAttribute("data-markdown-theme", "true"); | ||
| document.head.appendChild(markdownLink); | ||
|
|
||
| const highlightLink = document.createElement("link"); | ||
| highlightLink.rel = "stylesheet"; | ||
| highlightLink.href = isDark | ||
| ? "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/github-dark.min.css" | ||
| : "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/github.min.css"; | ||
| highlightLink.setAttribute("data-highlight-theme", "true"); | ||
| document.head.appendChild(highlightLink); | ||
|
|
||
| return () => { | ||
| markdownLink.remove(); | ||
| highlightLink.remove(); | ||
| }; | ||
| }, [currentTheme]); |
There was a problem hiding this comment.
External CDN dependency for theme CSS breaks offline/air-gapped deployments.
The component dynamically loads CSS from cdnjs.cloudflare.com at runtime. For a self-hosted tool (as Sourcebot is), this means the markdown preview will be unstyled if the instance has no internet access. Consider bundling these CSS files or importing them statically.
Also, per the Biome hint: the arrow in .forEach((el) => el.remove()) at line 43 implicitly returns the result of el.remove(). Use braces to satisfy the linter:
Fix for Biome lint warning
- .forEach((el) => el.remove());
+ .forEach((el) => { el.remove(); });📝 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.
| useEffect(() => { | |
| const isDark = currentTheme === "dark"; | |
| document | |
| .querySelectorAll( | |
| "link[data-markdown-theme], link[data-highlight-theme]", | |
| ) | |
| .forEach((el) => el.remove()); | |
| const markdownLink = document.createElement("link"); | |
| markdownLink.rel = "stylesheet"; | |
| markdownLink.href = isDark | |
| ? "https://cdnjs.cloudflare.com/ajax/libs/github-markdown-css/5.5.1/github-markdown-dark.min.css" | |
| : "https://cdnjs.cloudflare.com/ajax/libs/github-markdown-css/5.5.1/github-markdown-light.min.css"; | |
| markdownLink.setAttribute("data-markdown-theme", "true"); | |
| document.head.appendChild(markdownLink); | |
| const highlightLink = document.createElement("link"); | |
| highlightLink.rel = "stylesheet"; | |
| highlightLink.href = isDark | |
| ? "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/github-dark.min.css" | |
| : "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/github.min.css"; | |
| highlightLink.setAttribute("data-highlight-theme", "true"); | |
| document.head.appendChild(highlightLink); | |
| return () => { | |
| markdownLink.remove(); | |
| highlightLink.remove(); | |
| }; | |
| }, [currentTheme]); | |
| useEffect(() => { | |
| const isDark = currentTheme === "dark"; | |
| document | |
| .querySelectorAll( | |
| "link[data-markdown-theme], link[data-highlight-theme]", | |
| ) | |
| .forEach((el) => { el.remove(); }); | |
| const markdownLink = document.createElement("link"); | |
| markdownLink.rel = "stylesheet"; | |
| markdownLink.href = isDark | |
| ? "https://cdnjs.cloudflare.com/ajax/libs/github-markdown-css/5.5.1/github-markdown-dark.min.css" | |
| : "https://cdnjs.cloudflare.com/ajax/libs/github-markdown-css/5.5.1/github-markdown-light.min.css"; | |
| markdownLink.setAttribute("data-markdown-theme", "true"); | |
| document.head.appendChild(markdownLink); | |
| const highlightLink = document.createElement("link"); | |
| highlightLink.rel = "stylesheet"; | |
| highlightLink.href = isDark | |
| ? "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/github-dark.min.css" | |
| : "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/github.min.css"; | |
| highlightLink.setAttribute("data-highlight-theme", "true"); | |
| document.head.appendChild(highlightLink); | |
| return () => { | |
| markdownLink.remove(); | |
| highlightLink.remove(); | |
| }; | |
| }, [currentTheme]); |
🧰 Tools
🪛 Biome (2.3.14)
[error] 43-43: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In
`@packages/web/src/app/`[domain]/browse/[...path]/components/pureMarkDownPreviewPanel.tsx
around lines 36 - 65, The component's useEffect currently injects external CDN
links for GitHub markdown and Highlight.js styles which breaks
offline/self-hosted deployments and uses a terse arrow callback in the .forEach
that triggers a Biome lint warning; replace the runtime CDN injection in the
useEffect of PureMarkDownPreviewPanel (and any helper that creates
markdownLink/highlightLink) by bundling or importing the two CSS files into the
app (e.g., add them to static assets or import them as modules) and point the
injected link.href to the local asset paths or simply import the styles and
conditionally apply a CSS class based on currentTheme, and update the
.forEach((el) => el.remove()) to use an explicit block .forEach((el) => {
el.remove(); }) to satisfy the linter.
| const getContentType = (filename: string): string => { | ||
| const ext = filename.split(".").pop()?.toLowerCase(); | ||
| switch (ext) { | ||
| case "png": | ||
| return "image/png"; | ||
| case "jpg": | ||
| case "jpeg": | ||
| return "image/jpeg"; | ||
| case "gif": | ||
| return "image/gif"; | ||
| case "svg": | ||
| return "image/svg+xml"; | ||
| case "webp": | ||
| return "image/webp"; | ||
| case "ico": | ||
| return "image/x-icon"; | ||
| case "bmp": | ||
| return "image/bmp"; | ||
| case "tiff": | ||
| return "image/tiff"; | ||
| case "pdf": | ||
| return "application/pdf"; | ||
| case "mp4": | ||
| return "video/mp4"; | ||
| case "webm": | ||
| return "video/webm"; | ||
| default: | ||
| return "application/octet-stream"; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Serving image/svg+xml without sandboxing is an XSS vector.
SVGs can embed <script> tags and event handlers. If a user navigates directly to /api/{domain}/raw/{repo}/-/malicious.svg, the browser will execute embedded JavaScript in the origin's context. Since this endpoint serves repo content from potentially untrusted sources, this is exploitable.
Consider mitigating with a Content-Security-Policy: sandbox header or Content-Disposition: attachment for SVG responses:
🔒 Proposed mitigation
// eslint-disable-next-line `@typescript-eslint/no-explicit-any`
return new Response(new Blob([buffer as any]), {
headers: {
"Content-Type": contentType,
"Cache-Control": "public, max-age=3600, immutable",
+ // Sandbox SVG and HTML to prevent script execution
+ ...(contentType.includes("svg") || contentType.includes("html")
+ ? { "Content-Security-Policy": "sandbox" }
+ : {}),
},
});🤖 Prompt for AI Agents
In `@packages/web/src/app/api/`[domain]/raw/[...path]/route.ts around lines 6 -
35, The getContentType function currently returns "image/svg+xml" for .svg files
which allows inline SVG execution; update the route handling that serves files
so that responses for SVG files do not render in-page: detect SVG responses
using getContentType(filename) or by checking the file extension and add either
a strict Content-Security-Policy: sandbox header (e.g. "sandbox;") or set
Content-Disposition: attachment to force download for .svg, and ensure other
content types are unchanged; adjust any response-building logic that references
getContentType to include this special-case header for SVGs.
| // simple-git's binaryCatFile returns the raw buffer from git cat-file -p <sha> | ||
| // Note: binaryCatFile is not available in all simple-git versions, checking typing... | ||
| // If simple-git doesn't support binaryCatFile directly in types, we use cat-file with buffer encoding. | ||
| // Assuming simple-git v3+ supports this via custom options or command execution. | ||
|
|
||
| // Using 'buffer' encoding with show/cat-file is tricky with simple-git's high-level API which returns string by default. | ||
| // The most reliable way for binary data with simple-git is using `binaryCatFile` if available or `raw` with buffer handling. | ||
| // Let's use `git.binaryCatFile` which is designed for this. | ||
| const buffer = await git.binaryCatFile(["blob", sha]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check simple-git version in package.json
cat packages/web/package.json | jq '.dependencies["simple-git"] // .devDependencies["simple-git"]'Repository: sourcebot-dev/sourcebot
Length of output: 76
🏁 Script executed:
# Read the actual file to confirm code state
cat -n packages/web/src/features/git/getRawFileSourceApi.ts | head -70Repository: sourcebot-dev/sourcebot
Length of output: 3632
🏁 Script executed:
# Check if simple-git is installed and search for binaryCatFile
rg "binaryCatFile" --type ts --type tsx -iRepository: sourcebot-dev/sourcebot
Length of output: 94
🏁 Script executed:
# Search for binaryCatFile usage/definition in the codebase
rg "binaryCatFile" --type ts -iRepository: sourcebot-dev/sourcebot
Length of output: 993
🏁 Script executed:
# Check simple-git type definitions for binaryCatFile
fd "index.d.ts" node_modules/simple-git --exec cat {} \; 2>/dev/null | head -300Repository: sourcebot-dev/sourcebot
Length of output: 49
🏁 Script executed:
# Find simple-git type definition files
fd -t f "*.d.ts" node_modules/simple-git 2>/dev/null | head -10Repository: sourcebot-dev/sourcebot
Length of output: 49
🏁 Script executed:
# Alternative: check simple-git package.json for main entry
cat node_modules/simple-git/package.json | jq '.types, .main, .exports'Repository: sourcebot-dev/sourcebot
Length of output: 135
🏁 Script executed:
# Search for binaryCatFile in simple-git package
find node_modules/simple-git -type f \( -name "*.d.ts" -o -name "*.ts" \) -exec grep -l "binaryCatFile" {} \; 2>/dev/nullRepository: sourcebot-dev/sourcebot
Length of output: 49
🏁 Script executed:
# Since node_modules isn't available in sandbox, let's check the package-lock or yarn.lock
# for the exact version that will be installed
cat packages/web/package-lock.json | jq '.dependencies["simple-git"] // .packages["node_modules/simple-git"]' 2>/dev/null | head -20Repository: sourcebot-dev/sourcebot
Length of output: 129
🌐 Web query:
simple-git v3.27.0 binaryCatFile API documentation
💡 Result:
binaryCatFile (simple-git v3.27.0) — API
What it does: Same as catFile, but returns the native Buffer from git cat-file’s stdout (ie, binary-safe output). [1]
Signature (TypeScript):
binaryCatFile(options: string[], callback?: SimpleGitTaskCallback<any>): Response<any>;[1]
Parameters
options: string[]— arguments passed togit cat-file(for example['-p', '<object>']). [1]
Return
Response<any>(promise-like / thenable returned by simple-git). The resolved value is the raw binary output (Buffer). [1]
Example
import simpleGit from 'simple-git';
const git = simpleGit();
const buf = await git.binaryCatFile(['-p', 'HEAD:path/to/image.png']);
// buf is a Buffer[1]
Related
catFile(options)returns string output (non-binary-focused), whereasbinaryCatFileis explicitly forBufferoutput. [1]
Sources
[1] simple-git v3.27.0 type definitions (dist/typings/simple-git.d.ts) on UNPKG. (app.unpkg.com)
Citations:
Remove leftover development comments and fix incorrect binaryCatFile API usage.
Lines 40-47 contain exploratory comments expressing uncertainty about binaryCatFile availability. These should be removed—binaryCatFile is a standard API in simple-git v3.27.0 (your project's version).
However, there's a critical functional issue: Line 48 uses git.binaryCatFile(["blob", sha]), which translates to git cat-file blob <sha>. This is invalid git syntax. The correct usage is git.binaryCatFile(["-p", sha]) to print/retrieve the blob content, matching the git cat-file -p option for reading object contents.
Fix both the comments and the API arguments:
Suggested fix
- // 2. Read the binary content using cat-file
- // simple-git's binaryCatFile returns the raw buffer from git cat-file -p <sha>
- // Note: binaryCatFile is not available in all simple-git versions, checking typing...
- // If simple-git doesn't support binaryCatFile directly in types, we use cat-file with buffer encoding.
- // Assuming simple-git v3+ supports this via custom options or command execution.
-
- // Using 'buffer' encoding with show/cat-file is tricky with simple-git's high-level API which returns string by default.
- // The most reliable way for binary data with simple-git is using `binaryCatFile` if available or `raw` with buffer handling.
- // Let's use `git.binaryCatFile` which is designed for this.
- const buffer = await git.binaryCatFile(["blob", sha]);
+ // Read the binary blob content
+ const buffer = await git.binaryCatFile(["-p", sha]);🤖 Prompt for AI Agents
In `@packages/web/src/features/git/getRawFileSourceApi.ts` around lines 40 - 48,
Remove the exploratory/dev comments around binary handling and use the proper
simple-git v3 API call: replace the incorrect git.binaryCatFile(["blob", sha])
invocation with git.binaryCatFile(["-p", sha]) to match git cat-file -p
semantics and retrieve the blob content; keep the single call site in
getRawFileSourceApi.ts and ensure binaryCatFile is used directly (no extra
commentary about availability).
2 participants
Fix: Add toggle for Markdown preview #794
Summary
This PR adds a toggle to control the visibility of the Markdown preview panel. The preview is now rendered conditionally based on user intent.
Details
Results
Summary by CodeRabbit
Release Notes