Daily branch 2025 08 15 #8
Conversation
- Add /privacy-policy page with comprehensive privacy policy content - Add /terms-of-service page with terms and conditions - Update middleware to allow unauthenticated access to /, /privacy-policy, and /terms-of-service - Update middleware matcher to exclude apple-touch-icon.png from authentication checks - Add new favicon.ico and apple-touch-icon.png assets - Remove unused SVG assets (file.svg, globe.svg, next.svg, vercel.svg, window.svg)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRefactors terminal ANSI rendering into a new AnsiCodeBlock using shiki/codeToHtml with caching, debounced streaming renders, and fallbacks; adds FileToolsHandler and TerminalToolHandler and updates MessagePartHandler to delegate tooling UI; supports multi-edit file-creation and local-file changes; adds static Privacy Policy and Terms pages; updates middleware routes, sandbox template, and system-prompt text. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as MessagePartHandler
participant FT as FileToolsHandler
participant TT as TerminalToolHandler
participant Msg as Message parts
UI->>Msg: receives message with parts
UI->>FT: delegate tool-* parts (file tools)
UI->>TT: delegate terminal-related parts
FT-->>UI: renders file tool UI, actions openFileInSidebar
TT-->>UI: aggregates streaming data, renders TerminalCodeBlock
sequenceDiagram
participant TerminalUI as TerminalCodeBlock
participant Ansi as AnsiCodeBlock
participant Cache as ansiCache
participant Shiki as shiki.codeToHtml
TerminalUI->>Ansi: render(code, isStreaming, theme, delay)
Ansi->>Cache: lookup(cacheKey)
alt cache hit
Cache-->>Ansi: html
Ansi-->>TerminalUI: set innerHTML
else cache miss
Ansi->>Shiki: codeToHtml(code,{lang:"ansi",theme})
alt success
Shiki-->>Ansi: html
Ansi->>Cache: store(cacheKey, html)
Ansi-->>TerminalUI: set innerHTML
else error
Ansi-->>TerminalUI: render escaped <pre><code>
Ansi->>Cache: store(cacheKey, fallback)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (2)
lib/ai/tools/search-replace.ts (1)
66-76: Guard against empty old_string to avoid catastrophic replacements.If old_string is empty, the current logic will create a zero-width global regex and insert new_string between every character, corrupting files. Add a validation to reject empty old_string.
Apply this patch:
// Validate that old_string and new_string are different if (old_string === new_string) { return { result: "Invalid: old_string and new_string are exactly the same", }; } + + // Disallow empty old_string to prevent zero-width global matches + if (old_string.length === 0) { + return { + result: "Invalid: old_string cannot be empty", + }; + }lib/ai/tools/multi-edit.ts (1)
111-116: Disallow empty old_string to prevent zero-width replacements.Same risk as in search-replace: an empty old_string would produce a zero-width match and corrupt content when used with replace_all, and also violates your own doc constraints.
Apply this patch:
// Validate that old_string and new_string are different if (old_string === new_string) { return { result: `Edit ${i + 1}: Invalid - old_string and new_string are exactly the same`, }; } + + // Disallow empty old_string to prevent zero-width global matches + if (old_string.length === 0) { + return { + result: `Edit ${i + 1}: Invalid - old_string cannot be empty`, + }; + }
🧹 Nitpick comments (12)
lib/ai/tools/search-replace.ts (1)
31-36: Typo: “occurences” → “occurrences”.Minor spelling fix in the schema description.
Apply this patch:
- .describe("Replace all occurences of old_string (default false)"), + .describe("Replace all occurrences of old_string (default false)"),lib/ai/tools/multi-edit.ts (2)
20-21: Inconsistent path requirement: description vs input schema.The description says file_path must be absolute, while the inputSchema description allows relative or absolute. Align them; either enforce absolute in code or update the description to match current behavior.
Apply this patch to the description for consistency with the schema:
-- file_path: The absolute path to the file to modify (must be absolute, not relative) +- file_path: The path to the file to modify (relative to the workspace or absolute)
24-24: Typo: “occurences” → “occurrences”.Fix spelling in both locations.
Apply these patches:
- - replace_all: Replace all occurences of old_string. This parameter is optional and defaults to false. + - replace_all: Replace all occurrences of old_string. This parameter is optional and defaults to false.- .describe("Replace all occurences of old_string (default false)"), + .describe("Replace all occurrences of old_string (default false)"),Also applies to: 70-70
lib/ai/tools/utils/sandbox-utils.ts (3)
28-31: Clarify error message and preserve original causeImprove diagnosability by including the template in logs/messages and preserving the original error as the cause.
- console.error("Error creating persistent sandbox:", error); - throw new Error( - `Failed creating persistent sandbox: ${error instanceof Error ? error.message : "Unknown error"}`, - ); + console.error( + `Error creating or connecting terminal sandbox (template=${SANDBOX_TEMPLATE}):`, + error, + ); + throw new Error( + `Failed creating or connecting terminal sandbox (template=${SANDBOX_TEMPLATE}): ${ + error instanceof Error ? error.message : "Unknown error" + }`, + { cause: error }, + );
6-6: Nit: make timeout units more readableTiny readability win by using numeric separators to highlight units.
-const BASH_SANDBOX_TIMEOUT = 15 * 60 * 1000; +const BASH_SANDBOX_TIMEOUT = 15 * 60_000;
5-5: MakeSANDBOX_TEMPLATEconfigurable via an env var for safe rolloutWe searched for all occurrences of the hard-coded template name and call sites and confirmed there are no other references in the codebase. This change lets you stage the template rename and quickly roll back if needed.
• File to update:
- lib/ai/tools/utils/sandbox-utils.ts (line 5)
• Apply this minimal diff:
- const SANDBOX_TEMPLATE = "terminal-agent-sandbox"; + const SANDBOX_TEMPLATE = process.env.SANDBOX_TEMPLATE ?? "terminal-agent-sandbox";• Next steps:
- Document the new
SANDBOX_TEMPLATEenv var in your deployment/runbook.- Plan a staged rollout (e.g., set the env var in a subset of environments/users, monitor resource usage and orphaned sandboxes).
- Roll back by unsetting or reverting the env var if any issues arise.
[optional_refactors_recommended]
middleware.ts (1)
30-32: Extend asset and well-known file exclusions in matcher (avoid auth for public assets) [optional]Consider excluding other common public assets to prevent unnecessary middleware/auth work on them (e.g., robots.txt, sitemap.xml, site.webmanifest/manifest.webmanifest, and opengraph/icon routes if present).
Apply this diff to extend the matcher string:
- matcher: [ - "/((?!_next/static|_next/image|favicon.ico|apple-touch-icon.png).*)", - ], + matcher: [ + "/((?!_next/static|_next/image|favicon.ico|apple-touch-icon.png|robots.txt|sitemap.xml|site.webmanifest|manifest.webmanifest|icon.png|icon.jpg|opengraph-image|twitter-image).*)", + ],If some of these files don’t exist in this repo, it’s still safe—they’ll just match nothing.
app/privacy-policy/page.tsx (2)
42-42: Use ordered list semantics for a decimal listYou’re using a decimal list style on a
. For semantics and accessibility, this should be an
with list-decimal.
Apply this diff:
- <ul className="list-inside list-decimal"> + <ol className="list-inside list-decimal"> ... - </ul> + </ol>Also applies to: 118-118
3-19: SEO metadata is solid; consider canonical URL and images (optional)Title/description/OpenGraph/Twitter are correct. Optionally add a canonical URL and OG/Twitter images for richer sharing.
app/components/TerminalCodeBlock.tsx (3)
118-135: Escape fallback is good; also sanitize and cache after cleanup consistentlyEven though the fallback escapes characters, sanitize the composed HTML string and clean cache before insert to keep consistency.
Apply this diff:
const fallbackHtml = `<pre><code>${escapedCode}</code></pre>`; if (cacheKeyRef.current === cacheKey) { - setHtmlContent(fallbackHtml); - cleanCache(); - ansiCache.set(cacheKey, fallbackHtml); + const sanitized = DOMPurify.sanitize(fallbackHtml); + cleanCache(); + ansiCache.set(cacheKey, sanitized); + setHtmlContent(sanitized); lastRenderedCodeRef.current = codeToRender; }
90-99: Improve cache key and re-render logic (avoid stale renders, boost dedupe)
- Early-return check uses only code string; if theme changes, it incorrectly skips re-render. Include theme in the check or remove the early return.
- Cache key includes isWrapped, but wrapping is handled via containerClassName, not shiki HTML. Including isWrapped reduces cache hit rate without benefit.
Apply this diff:
- // Skip if we've already rendered this exact content - if (lastRenderedCodeRef.current === codeToRender) { + // Skip only if both code and theme match last render + if (lastRenderedCodeRef.current === `${codeToRender}-${theme}`) { return; } // Create cache key including theme for proper caching - const cacheKey = `${codeToRender}-${isWrapped}-${theme}`; + const cacheKey = `${codeToRender}-${theme}`; cacheKeyRef.current = cacheKey;And when updating lastRenderedCodeRef:
- lastRenderedCodeRef.current = codeToRender; + lastRenderedCodeRef.current = `${codeToRender}-${theme}`;Update both occurrences.
152-158: Avoid setState after unmount (memory leak on async render completion)If the timeout fires and the async render resolves after unmount, setState calls can warn. Track mounted state to skip state updates.
Example minimal change:
+ const mountedRef = useRef(true); useEffect(() => { + mountedRef.current = true; if (code) { debouncedRender(code); } else { setHtmlContent(""); lastRenderedCodeRef.current = ""; } // Cleanup timeout on unmount or code change return () => { + mountedRef.current = false; if (renderTimeoutRef.current) { clearTimeout(renderTimeoutRef.current); } }; }, [code, debouncedRender]);Then guard state updates:
- setHtmlContent(sanitized); + if (mountedRef.current) setHtmlContent(sanitized); - setIsRendering(false); + if (mountedRef.current) setIsRendering(false);Apply similar guards at other setState calls within the debounce.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (8)
app/favicon.icois excluded by!**/*.icopublic/apple-touch-icon.pngis excluded by!**/*.pngpublic/favicon.icois excluded by!**/*.icopublic/file.svgis excluded by!**/*.svgpublic/globe.svgis excluded by!**/*.svgpublic/next.svgis excluded by!**/*.svgpublic/vercel.svgis excluded by!**/*.svgpublic/window.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
app/components/TerminalCodeBlock.tsx(3 hunks)app/privacy-policy/page.tsx(1 hunks)app/terms-of-service/page.tsx(1 hunks)lib/ai/tools/multi-edit.ts(1 hunks)lib/ai/tools/search-replace.ts(1 hunks)lib/ai/tools/utils/sandbox-utils.ts(1 hunks)lib/system-prompt.ts(2 hunks)middleware.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
app/terms-of-service/page.tsx (1)
app/privacy-policy/page.tsx (2)
metadata(3-19)dynamic(21-21)
app/privacy-policy/page.tsx (2)
app/terms-of-service/page.tsx (2)
metadata(3-16)dynamic(18-18)app/components/MemoizedMarkdown.tsx (3)
h1(26-32)p(89-93)a(69-88)
app/components/TerminalCodeBlock.tsx (1)
app/components/ShimmerText.tsx (1)
ShimmerText(12-39)
🪛 ast-grep (0.38.6)
app/components/TerminalCodeBlock.tsx
[warning] 189-189: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
app/components/TerminalCodeBlock.tsx
[error] 190-190: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (8)
lib/ai/tools/search-replace.ts (1)
17-18: Doc update aligns with write-tool guidance.The added guideline to prefer the write tool for create/overwrite is a good clarification and reduces misuse of search-replace for file creation.
lib/ai/tools/multi-edit.ts (1)
13-16: Nice doc expansion and pre-flight checks.Pre-use guidance to read the file and verify paths is helpful and reduces failed operations.
middleware.ts (1)
11-17: Public routes list looks correct and aligns with new static pagesAllowing "/", "/privacy-policy", and "/terms-of-service" unauthenticated is consistent with adding public legal pages and a public landing. Good alignment with the expanded static content in this PR.
lib/system-prompt.ts (3)
36-45: Tool calling rules are clear and consistent with agent behaviorThe constraints around tool schemas, not naming tools to the user, and immediate execution are crisp. No issues here.
78-93: Verify sandbox toolchain versions for accuracyThe specific versions (e.g., Golang 1.24.2, Node 20.19.2) should reflect the actual sandbox; otherwise users will be misled.
If these are surfaced to users, confirm they match reality. If they’re just illustrative, consider softening to major versions.
23-25: Math delimiters are supported by the markdown pipelineThe codebase already imports and uses both
remark-mathandrehype-mathjaxinapp/components/MemoizedMarkdown.tsx, so\(...\)and\[...\]will render correctly.app/terms-of-service/page.tsx (1)
3-16: Metadata mirrors the privacy page wellConsistent SEO config with the privacy page. Looks good.
app/components/TerminalCodeBlock.tsx (1)
350-364: Good UX on streaming and wrapping; API remains stableThe swap to the internal AnsiCodeBlock keeps TerminalCodeBlock’s external API stable, improves perceived performance with debounced rendering, and adds a loading affordance via ShimmerText. Nice upgrade.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
app/components/TerminalCodeBlock.tsx (2)
1-1: Resolved: Client component directive present"use client" is correctly added at the top. This addresses the earlier concern.
64-68: Theme/grammar registration for Shiki likely missing; consider built-ins or explicit registrationUsing theme="houston" and lang="ansi" without explicitly registering them will cause codeToHtml to throw and always hit the fallback. Either register the custom theme and the ANSI grammar up-front, or switch to built-in theme/grammar.
Two options:
- Register: load the custom theme and ansi language before calling codeToHtml (e.g., via setCDN/getHighlighter/loadTheme).
- Switch to built-ins: e.g., theme="github-dark" (or other bundled theme). Verify ansi grammar availability in your Shiki version.
Run to check if registration exists anywhere:
#!/bin/bash # Look for custom shiki theme/lang wiring and any CDN/WASM setup rg -nP -C2 '\b(houston)\b' rg -nP -C2 '\b(lang:\s*["'\'']ansi["'\'']|\bansi\b)' rg -nP -C3 '\b(codeToHtml|getHighlighter|setCDN|setWasm|loadTheme|loadLanguage|langs|themes)\b'If you prefer switching to a built-in theme, minimal diffs:
- theme = "houston", + theme = "github-dark",- <AnsiCodeBlock + <AnsiCodeBlock code={output || ""} isWrapped={isWrapped} isStreaming={status === "streaming" || isExecuting} - theme="houston" + theme="github-dark" delay={150} />Also applies to: 360-366
lib/ai/tools/multi-edit.ts (1)
47-50: Documentation still suggests unsupported file creation workflow.Based on past review comments, the documentation here is misleading. While the code now attempts to support file creation (Lines 101-117), the description still suggests using an empty
old_stringfor the first edit, but the actual implementation reads the file first and only handles creation as a fallback when the file doesn't exist.The current implementation (Lines 105-117) does attempt to support file creation, but the workflow is fragile:
- It tries to read the file first
- Only if reading fails does it check for the creation case
- This creates a race condition if the file is created between the check and write
Consider either:
- Option 1: Remove the file creation feature and direct users to use the write tool
- Option 2: Check if the file exists before attempting to read it
-If you want to create a new file, use: -- A new file path, including dir name if needed -- First edit: empty old_string and the new file's contents as new_string -- Subsequent edits: normal edit operations on the created content`, +Note: To create a new file, use the write tool. This tool is designed for editing existing files. +If you need to create a file and then edit it, first use the write tool, then use this multi-edit tool.`,
🧹 Nitpick comments (12)
components/ui/tool-block.tsx (1)
31-37: Prefer non-interactive semantics when not clickable; add type="button"As rendered, this is always a and remains focusable even when not clickable. Use type="button" and disable semantics for a11y and to avoid accidental form submission in nested forms.
Outside-range snippet showing the intent:
<button type="button" className={`${baseClasses} ${clickableClasses}`} onClick={isClickable ? onClick : undefined} onKeyDown={isClickable ? onKeyDown : undefined} tabIndex={isClickable ? 0 : -1} aria-disabled={!isClickable || undefined} disabled={!isClickable || undefined} // role not needed on a real button >app/components/TerminalCodeBlock.tsx (1)
71-71: Use browser-safe timeout typesNodeJS.Timeout can conflict in browser builds. Prefer ReturnType.
- const renderTimeoutRef = useRef<NodeJS.Timeout | null>(null); + const renderTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);lib/ai/tools/utils/local-file-operations.ts (2)
257-271: Solid split between creation vs. modification pathsThe preflight existence check and gating new-file creation on an empty old_string in the first edit is clear and predictable. Minor: validateFileAccess(filePath) after fileExists is slightly redundant since you just derived existence; you could use resolvedPath directly for a tiny win.
283-291: Creation rule is strict; consider optional “append new file” supportRejecting empty old_string for any edit beyond the first is consistent, but you might want to support additional empty-old_string edits in the same creation flow to allow staged concatenations (e.g., header + body). If desired, gate it to when !fileExists and i >= 0, or add an explicit mode flag.
I can adjust the logic and add unit tests for multi-insert creation if you want that behavior—say the word.
app/components/MessagePartHandler.tsx (1)
6-11: Tighten part typing to a discriminated unionpart is any; consider a union of the specific tool/data shapes to get exhaustiveness checking in the switch and better prop safety for the handlers.
Example shape sketch:
type ToolPart = | { type: "tool-readFile"; /* ... */ } | { type: "tool-writeFile"; /* ... */ } | { type: "tool-deleteFile"; /* ... */ } | { type: "tool-searchReplace"; /* ... */ } | { type: "tool-multiEdit"; /* ... */ } | { type: "data-terminal"; /* ... */ } | { type: "tool-runTerminalCmd"; /* ... */ } | { type: "text"; text?: string }; interface MessagePartHandlerProps { message: UIMessage; part: ToolPart; partIndex: number; status: "ready" | "submitted" | "streaming" | "error"; }app/components/tools/TerminalToolHandler.tsx (2)
4-4: Import CommandResult as a type-only import to avoid bundlingThis component may render on the client via a client child; avoid shipping @e2b/code-interpreter to the browser.
-import { CommandResult } from "@e2b/code-interpreter"; +import type { CommandResult } from "@e2b/code-interpreter";
52-57: Join stdout and stderr with a newline for readabilityCurrently concatenated without a separator; output lines can run together.
- const combinedOutput = stdout + stderr; - const terminalOutputContent = - combinedOutput || (terminalOutput.result?.error ?? ""); + const combinedOutput = + stdout && stderr ? `${stdout}\n${stderr}` : (stdout || stderr); + const terminalOutputContent = + combinedOutput || (terminalOutput.result?.error ?? "");app/components/tools/FileToolsHandler.tsx (3)
1-4: Remove unused import.The
UIMessageimport from@ai-sdk/reactis not used anywhere in the file.-import { UIMessage } from "@ai-sdk/react"; import ToolBlock from "@/components/ui/tool-block"; import { FilePlus, FileText, FilePen, FileMinus } from "lucide-react"; import { useGlobalState } from "../../contexts/GlobalState";
22-30: Simplify range calculation logic.The current implementation has redundant checks. When
offsetis undefined, it can be treated as 1 for simplicity.const getFileRange = () => { - if (readInput.offset && readInput.limit) { - return ` L${readInput.offset}-${readInput.offset + readInput.limit - 1}`; - } - if (!readInput.offset && readInput.limit) { - return ` L1-${readInput.limit}`; - } - return ""; + if (!readInput.limit) return ""; + const start = readInput.offset || 1; + const end = start + readInput.limit - 1; + return ` L${start}-${end}`; };
284-286: Use more specific success detection for multi-edit operations.The current check
includes("Successfully applied")could match partial strings or errors that contain this phrase. Consider checking for an exact prefix match or using a more specific pattern.const multiEditOutput = output as { result: string }; -const isSuccess = multiEditOutput.result.includes( - "Successfully applied", -); +const isSuccess = multiEditOutput.result.startsWith("Successfully applied");lib/ai/tools/multi-edit.ts (2)
101-117: Potential race condition in file existence check.The current approach of trying to read and catching the error could lead to race conditions. A file might be created between the read attempt and the write operation.
Consider using a file existence check before attempting operations:
// Check if file exists and handle file creation let currentContent: string; let fileExists = true; try { - currentContent = await sandbox.files.read(file_path); + // First check if file exists + fileExists = await sandbox.files.exists?.(file_path) ?? true; + if (fileExists) { + currentContent = await sandbox.files.read(file_path); + } else { + currentContent = ""; + } } catch (error) { // File doesn't exist - check if this is a file creation case fileExists = false; const firstEdit = edits[0]; if (firstEdit.old_string !== "") { return { result: `File not found: ${file_path}. For new file creation, the first edit must have an empty old_string and the file contents as new_string.`, }; } currentContent = ""; }Note: This assumes the sandbox has a
files.existsmethod. If not available, the current approach is acceptable but should be documented as having this limitation.
128-140: Inconsistent file creation logic.The logic allows empty
old_stringonly for the first edit when creating a new file, but this creates an asymmetry: if a file exists and you want to prepend content, you cannot use an emptyold_string. This might be confusing for users.Consider allowing empty
old_stringfor prepending content to existing files as well:// Handle file creation case (empty old_string means insert content) if (old_string === "") { if (i === 0 && !fileExists) { // First edit for new file creation currentContent = new_string; totalReplacements += 1; editResults.push(`Edit ${i + 1}: Created file with content`); continue; + } else if (fileExists && i === 0) { + // Prepend to existing file + currentContent = new_string + currentContent; + totalReplacements += 1; + editResults.push(`Edit ${i + 1}: Prepended content to file`); + continue; } else { return { - result: `Edit ${i + 1}: Empty old_string is only allowed for the first edit when creating a new file`, + result: `Edit ${i + 1}: Empty old_string is only allowed for the first edit`, }; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
app/components/MessagePartHandler.tsx(2 hunks)app/components/TerminalCodeBlock.tsx(3 hunks)app/components/tools/FileToolsHandler.tsx(1 hunks)app/components/tools/TerminalToolHandler.tsx(1 hunks)app/terms-of-service/page.tsx(1 hunks)components/ui/tool-block.tsx(1 hunks)lib/ai/tools/multi-edit.ts(3 hunks)lib/ai/tools/utils/local-file-operations.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/terms-of-service/page.tsx
🧰 Additional context used
🧬 Code Graph Analysis (4)
app/components/tools/TerminalToolHandler.tsx (2)
app/components/ShimmerText.tsx (1)
ShimmerText(12-39)app/components/TerminalCodeBlock.tsx (1)
TerminalCodeBlock(198-373)
app/components/tools/FileToolsHandler.tsx (1)
app/contexts/GlobalState.tsx (1)
useGlobalState(92-98)
app/components/TerminalCodeBlock.tsx (1)
app/components/ShimmerText.tsx (1)
ShimmerText(12-39)
app/components/MessagePartHandler.tsx (2)
app/components/tools/FileToolsHandler.tsx (1)
FileToolsHandler(11-321)app/components/tools/TerminalToolHandler.tsx (1)
TerminalToolHandler(12-71)
🪛 ast-grep (0.38.6)
app/components/TerminalCodeBlock.tsx
[warning] 192-192: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
app/components/TerminalCodeBlock.tsx
[error] 193-193: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (4)
components/ui/tool-block.tsx (1)
24-24: Sizing tweak looks good and aligns with taller tool rowsThe increased padding/height will better match the new tool wrappers’ visual rhythm.
app/components/MessagePartHandler.tsx (1)
3-4: Delegating tool UIs to handlers is a good separationMoving file/terminal tool rendering into FileToolsHandler and TerminalToolHandler simplifies this component and improves cohesion.
Also applies to: 36-37, 40-42
app/components/tools/TerminalToolHandler.tsx (1)
24-31: Efficient streaming aggregationCollecting data-terminal parts by toolCallId and concatenating is straightforward and correct for streaming.
app/components/tools/FileToolsHandler.tsx (1)
56-56: Verify line‐number cleanup regex robustnessThe current pattern
readOutput.result.replace(/^\s*\d+\|/gm, "")only strips digits followed immediately by a pipe (
|). If the CLI or backend tool ever emits line numbers with other delimiters (e.g.:,>,-) or just a space, leftover artifacts will remain. Please:
- Confirm the exact format your file-reading tool outputs (pipe vs. colon vs. tab, etc.).
- Broaden the regex to catch other delimiters or optional spacing. For example:
const cleanContent = readOutput.result.replace(/^\s*\d+\s*[\|:>\-]?\s*/gm, "");- Update in:
• File:app/components/tools/FileToolsHandler.tsx
• Method:handleOpenInSidebar(line around thecleanContentreplace)This ensures all numbered prefixes are reliably removed.
| try { | ||
| const html = await codeToHtml(codeToRender, { | ||
| lang: "ansi", | ||
| theme: theme, | ||
| }); | ||
|
|
||
| // Only update if this is still the current render request | ||
| if (cacheKeyRef.current === cacheKey) { | ||
| setHtmlContent(html); | ||
| cleanCache(); | ||
| ansiCache.set(cacheKey, html); | ||
| lastRenderedCodeRef.current = codeToRender; | ||
| } |
There was a problem hiding this comment.
Sanitize HTML before injecting (XSS risk with dangerouslySetInnerHTML)
Shiki output and the fallback HTML should be sanitized before setting state and caching.
@@
- try {
- const html = await codeToHtml(codeToRender, {
+ try {
+ const html = await codeToHtml(codeToRender, {
lang: "ansi",
theme: theme,
});
// Only update if this is still the current render request
if (cacheKeyRef.current === cacheKey) {
- setHtmlContent(html);
- cleanCache();
- ansiCache.set(cacheKey, html);
+ const sanitized = DOMPurify.sanitize(html);
+ cleanCache();
+ ansiCache.set(cacheKey, sanitized);
+ setHtmlContent(sanitized);
lastRenderedCodeRef.current = codeToRender;
}
} catch (error) {
@@
- const fallbackHtml = `<pre><code>${escapedCode}</code></pre>`;
+ const fallbackHtml = `<pre><code>${escapedCode}</code></pre>`;
if (cacheKeyRef.current === cacheKey) {
- setHtmlContent(fallbackHtml);
- cleanCache();
- ansiCache.set(cacheKey, fallbackHtml);
+ const sanitized = DOMPurify.sanitize(fallbackHtml);
+ cleanCache();
+ ansiCache.set(cacheKey, sanitized);
+ setHtmlContent(sanitized);
lastRenderedCodeRef.current = codeToRender;
}
} finally {
setIsRendering(false);
}
@@
return (
<div
className={containerClassName}
style={style}
dangerouslySetInnerHTML={{ __html: htmlContent }}
/>
);Outside-range import to add at the top:
import DOMPurify from "dompurify";Also applies to: 121-137, 189-194
🤖 Prompt for AI Agents
In app/components/TerminalCodeBlock.tsx around lines 105-117 (and also apply
same changes to 121-137 and 189-194), the generated HTML from shiki/fallback is
being stored and injected without sanitization; import DOMPurify at the top of
the file and run DOMPurify.sanitize on the html string before calling
setHtmlContent and before saving to ansiCache to remove any malicious scripts or
attributes, ensuring you sanitize both the primary and fallback HTML paths and
cache/store the sanitized result only.
| interface FileToolsHandlerProps { | ||
| part: any; | ||
| status: "ready" | "submitted" | "streaming" | "error"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace any type with a proper interface.
Using any for the part prop defeats TypeScript's type safety. Based on the code usage, this should be a properly typed interface that includes fields like type, toolCallId, state, input, and output.
+interface ToolPart {
+ type: string;
+ toolCallId: string;
+ state: "input-streaming" | "input-available" | "output-available";
+ input: unknown;
+ output?: unknown;
+}
+
interface FileToolsHandlerProps {
- part: any;
+ part: ToolPart;
status: "ready" | "submitted" | "streaming" | "error";
}📝 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.
| interface FileToolsHandlerProps { | |
| part: any; | |
| status: "ready" | "submitted" | "streaming" | "error"; | |
| } | |
| interface ToolPart { | |
| type: string; | |
| toolCallId: string; | |
| state: "input-streaming" | "input-available" | "output-available"; | |
| input: unknown; | |
| output?: unknown; | |
| } | |
| interface FileToolsHandlerProps { | |
| part: ToolPart; | |
| status: "ready" | "submitted" | "streaming" | "error"; | |
| } |
🤖 Prompt for AI Agents
In app/components/tools/FileToolsHandler.tsx around lines 6 to 9, replace the
liberal use of "any" for the prop "part" with a concrete TypeScript interface
describing the properties the component accesses (at minimum: type, toolCallId,
state, input, output). Add an interface (e.g. FilePart) with explicit types such
as type: string; toolCallId?: string; state?: string; input?: Record<string,
any> | unknown; output?: unknown (tighten union/string literal types if you can
from usage), export it if used elsewhere, then update the FileToolsHandlerProps
to use that interface instead of any and update any local usages/refs to respect
the new property types.
Summary by CodeRabbit
New Features
Improvements