refactor: flatten monorepo packages into src/#146
Conversation
…gs found Thorough Playwright MCP testing of all 14 screens at iPhone 14 Pro viewport. Found infinite re-render loops in AgentDetail and Chat pages caused by Zustand selectors returning new array references. Report includes design alignment verification, navigation flow testing, and prioritized fix list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add useShallow from zustand/react/shallow to stabilize array selectors that return new references via filter()/sort() on every render - Fix useAgentDetail hook tasks selector (BUG-001) - Fix ThreadList pinned/recent thread selectors (BUG-002) - Fix ChatThread messages/typing selectors (preventive) - Fix chat empty state layout (missing flex flex-col) - Fix agent filter button overflow on mobile (add shrink-0 + flex-nowrap) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… 17 files Add stable empty array constants to all 4 stores (agent, chat, credential, project) preventing ?? [] from creating new references each render. Fix 25+ flex-row usages missing display:flex (React Native migration artifact) and add flex to 5 icon wrapper divs missing items-center alignment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rts) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Rewrite AGENTS.md with agent roles, tools, orchestration architecture, workflow modes, approval gates, inter-agent communication, and metrics - Update CLAUDE.md stack table: Vite 7, Capacitor 8, react-router-dom 7, Tailwind CSS, Zustand 5, pnpm workspaces (replace stale Expo/NativeWind/ expo-router/gluestack references) - Update file structure to reflect current layout (pages/ not app/) - Add references to AGENTS.md and docs/memory-bank/ - Delete stale root docs migrated to docs/: ARCHITECTURE.md, DECISIONS.md, DEVELOPMENT-LOG.md, PROJECT-STATUS.md, thumbcode-agent-playbook.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…yout pages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add multi-agent pipeline system enabling sequential agent execution (Architect -> Implementer -> Reviewer -> Tester) with approval gates between stages. Orchestrator changes (packages/agent-intelligence): - Add Pipeline, PipelineStage, PipelineStatus types to orchestrator - Add createPipeline() with dependency-chained tasks per stage - Add advancePipeline(), approvePipelineStage(), cancelPipeline(), failPipeline() for lifecycle management - Pipeline events: created, stage_started/completed, awaiting_approval, approval_received, completed, failed, cancelled - Add pipelines Map to OrchestratorState Chat service changes (src/services/chat): - Add isMultiStepRequest() regex detection for pipeline-triggering user messages (build, create, implement + app/feature/page etc.) - Add requestPipelineResponse() for full pipeline execution with system handoff messages and approval gates between stages - Add executePipelineStage() for per-stage agent execution - Integrate pipeline detection into ChatService.sendMessage() - Add pipeline event types to ChatEventType Tests (46 new tests, all passing): - 28 orchestrator pipeline tests: creation, stage progression, approval gates, cancellation, failure, full progression - 18 routing tests: multi-step detection, pipeline creation, system messages, agent store tasks, error handling Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…xecutionBridge Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…gistry Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cument, audio) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…th design system awareness - Add AgentSkill interface with tiered context injection and composable tool system - Add SkillToolDefinition type for skill-provided tools - Modify BaseAgent to support skills: addSkill(), getSystemPromptWithSkills(), getToolsWithSkills(), executeToolWithSkills() with automatic routing - Create FrontendSkill with P3 "Warm Technical" design system context (4 tiers) and 5 tools: list_components, generate_component, analyze_ui_screenshot, compare_ui, preview_component - Attach FrontendSkill to ImplementerAgent by default - Add barrel exports and package.json export path for skills module - 29 tests covering interface compliance, context tiers, all tools, BaseAgent skill integration, and ImplementerAgent default skill attachment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…th design system awareness - Add AgentSkill interface with tiered context injection and composable tool system - Add SkillToolDefinition type for skill-provided tools - Modify BaseAgent to support skills: addSkill(), getSystemPromptWithSkills(), getToolsWithSkills(), executeToolWithSkills() with automatic routing - Create FrontendSkill with P3 "Warm Technical" design system context (4 tiers) and 5 tools: list_components, generate_component, analyze_ui_screenshot, compare_ui, preview_component - Attach FrontendSkill to ImplementerAgent by default - Add barrel exports and package.json export path for skills module - 29 tests covering interface compliance, context tiers, all tools, BaseAgent skill integration, and ImplementerAgent default skill attachment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…generation Add generate_variants tool to FrontendSkill that generates 3-4 design variants of a component (like 21st.dev), each with a different visual approach while remaining brand-compliant with ThumbCode P3 design system. Built-in variant presets: - Minimal: spacious, subtle accents, no shadows - Rich: full-featured with coral/gold/teal accents and layered shadows - Compact: dense, information-rich, optimized for lists - Playful: bold rotations, larger organic shapes, warm gold accents Each variant includes: unique component code, preview HTML, file name, style description. Users pick their favorite rather than getting a single take-it-or-leave-it output. 14 new tests covering variant count, style hints, brand compliance, shared props, unique file names, preview HTML, and per-preset styles. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…upport Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… cloud save foundation - Create DocumentEngine service with format-specific generators for Word, PowerPoint, Excel, and PDF - Add document tools (create_document, create_presentation, create_spreadsheet, create_pdf) to ToolExecutionBridge - Add DocumentEngineLike interface for dependency injection in agent-intelligence package - Create DocumentCard chat component with organic daube styling and format-specific icons - Create CloudSaveService foundation with local download support - Add 40 new tests across 4 test files (DocumentEngine, DocumentTools, DocumentCard, CloudSaveService) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…t coverage, and cleanup dead code - Rewrite ai-client.test.ts and VariantGeneration.test.ts to mock Vercel AI SDK (generateText/streamText) instead of old @anthropic-ai/sdk/@openai mocks - Fix OrchestrationState.test.ts type errors: update TaskAssignment mocks with required fields (title, acceptanceCriteria, references, updatedAt), fix OrchestratorConfig shape, fix OrchestratorEvent structure, fix TaskResult mocks - Fix ProjectActions.test.tsx, AgentHistory.test.tsx, ThreadList.test.tsx, RepoSelector.test.tsx type errors for evolved interfaces - Add AIProviderFactory with provider-factory.ts and tests - Add 50+ new test files across all packages for comprehensive coverage - Remove dead components (Tooltip, BottomSheet, Pagination, Spacer, workspace) - Remove stale archives and ralph directory - Rename useAppRouter to use-app-router (kebab-case convention) All 155 test files pass (1886 tests), typecheck clean. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add LivePreview and PreviewPanel components for rendering component previews in sandboxed iframes, with a preview-sandbox utility for generating self-contained HTML with ThumbCode design tokens. Wire preview capability into VariantSelector for inline variant previews. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add LivePreview and PreviewPanel components for rendering component previews in sandboxed iframes, with a preview-sandbox utility for generating self-contained HTML with ThumbCode design tokens. Wire preview capability into VariantSelector for inline variant previews. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- ImageMessage: thumbnail with click-to-expand lightbox, loading skeleton, error state - AudioMessage: custom player with play/pause, waveform visualization, transcript display - MixedMediaMessage: grid layout for multiple attachments with type-specific rendering - ChatMessage routing: image → ImageMessage, voice_transcript → AudioMessage, mixed_media → MixedMediaMessage - 29 new tests across 3 test files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 297 files, which is 147 over the limit of 150. ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (297)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This is an ambitious production launch PR delivering 24 commits across 18 user stories with comprehensive features. All tests pass (2064 tests) and TypeScript typechecks cleanly. However, I've identified 6 critical security and logic issues that must be addressed before merge:
Critical Security Issues 🔴
- XSS Vulnerability in
preview-sandbox.ts: ThecustomCssparameter is injected without sanitization, allowing CSS injection attacks - Incomplete XSS Protection: The
sanitizeHtmlfunction has regex gaps that miss unquoted attributes and edge cases - Color Injection Risk: Color values lack validation before HTML interpolation
Logic Errors 🟡
- Race Condition in MCP Client: The
disconnect()method updates connection state before deleting it, creating unnecessary intermediate state - Poor Error Handling: Variant generation silently swallows all errors without logging, making debugging impossible
- Incorrect Message Filtering: The
toAIMessagesfilter uses redundant logic that doesn't properly exclude system messages
Impact Assessment
The security vulnerabilities in the preview sandbox are high severity - they could allow attackers to inject malicious code through CSS parameters or bypass HTML sanitization. The logic errors reduce reliability and debuggability but don't prevent core functionality.
Architecture Strengths
✅ Unified AI SDK with 10 providers
✅ Comprehensive test coverage (2064 tests passing)
✅ Clean TypeScript with 0 type errors
✅ Well-structured multi-agent orchestration
✅ Proper credential management with SecureStore
Recommendation
Request changes - The security vulnerabilities must be fixed before production deployment. Once the 6 issues are addressed, this PR will be ready to merge.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Summary of ChangesHello @jbdevprimary, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request marks a significant milestone, delivering a production-ready application by implementing a wide array of features across core functionalities. It establishes a sophisticated multi-agent system capable of dynamic task orchestration and intelligent routing based on AI provider capabilities. The user experience is enhanced with rich multimodal interactions and specialized agent skills for frontend development, supported by a robust testing framework and updated documentation. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Remove unused isRecordMode variable, fix import ordering, and apply Biome formatter fixes to pass CI lint check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The execute() method returned promises without awaiting them inside the try block, making the catch block unreachable for async rejections. Fixes SonarCloud S4822 reliability bug (Quality Gate C → A). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All 4 agents (Architect, Implementer, Reviewer, Tester) now route file I/O tools through ToolExecutionBridge for real workspace operations. Agent-specific tools remain as orchestration-level handlers. Updated AGENTS.md to reflect production-ready state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Approved design + 16-task implementation plan to dissolve all 6 runtime packages into src/ and decompose agent-intelligence into src/services/. Removes all workspace ceremony. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace stub MCP transport with real Vercel AI SDK integration. McpClient now creates stdio/HTTP/SSE connections, discovers tools via the SDK, and executes them natively. McpToolBridge converts SDK tool definitions to agent ToolDefinition format with name prefixing for collision avoidance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rvices/ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jbdevprimary
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Fix 126 noExplicitAny warnings across 37 test/production files - Fix noNonNullAssertion, noExcessiveCognitiveComplexity, unused params - Address all 18 PR review threads (security, CSS, error handling) - Harden preview-sandbox.ts with iterative sanitization loops - Use LucideIcon type for Toast.tsx icon prop - Use GitHubContent type in ProjectFileExplorer test mocks - Fix Zustand store selector types in test mocks - Remove unused imports and biome-ignore suppressions - Auto-fix all Biome formatting violations Result: 0 TS errors, 0 lint errors, 0 warnings, 2076 tests passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| let sanitized = input; | ||
|
|
||
| // Remove <style> and </style> tags (with optional whitespace/attributes) | ||
| sanitized = sanitized.replace(/<\/?style\s*[^>]*>/gi, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
General approach: Ensure that any occurrence of the substring <style (regardless of whether it forms a syntactically complete tag) is removed or neutralized, so an attacker cannot rely on partial or malformed <style markup surviving. This addresses the “incomplete multi-character sanitization” concern by matching the dangerous prefix itself, not only full tags.
Best targeted fix: Extend the existing “Remove <style> and </style> tags” step to also remove any residual <style sequences. We can do this by adding one more replacement call that strips <style prefixes regardless of whether they have a closing >. This keeps behavior aligned with current intent (you already intend to strip styles entirely) and minimizes changes.
Concretely, in sanitizeCss in src/lib/preview-sandbox.ts:
- Keep the existing line 118 that removes
<style>/</style>tags. - Immediately after it, add a second replacement, e.g.
sanitized = sanitized.replace(/<\s*style\b/gi, '');
This removes any<styleopening sequence (with optional whitespace) that could remain or be (re)created by prior transformations, even without a terminating>or attributes. - No new imports or helpers are needed; this is a simple regex replacement.
This solves the CodeQL concern by ensuring the sensitive multi-character pattern <style itself cannot persist, independent of tag completeness.
| @@ -116,6 +116,8 @@ | ||
|
|
||
| // Remove <style> and </style> tags (with optional whitespace/attributes) | ||
| sanitized = sanitized.replace(/<\/?style\s*[^>]*>/gi, ''); | ||
| // Additionally remove any remaining "<style" openings, even if the tag is malformed/incomplete | ||
| sanitized = sanitized.replace(/<\s*style\b/gi, ''); | ||
|
|
||
| // Remove <script> tags and their content (handles whitespace in closing tag) | ||
| sanitized = sanitized.replace( |
| sanitized = sanitized.replace( | ||
| /<script\b[^<]*(?:(?!<\/\s*script\s*>)<[^<]*)*<\/\s*script\s*>/gi, | ||
| '' | ||
| ); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
General approach: Avoid the fragile, complex multi-character <script>...</script> regex that can leave <script behind when used in isolation. Since we already run this in an iterative sanitizer and then strip all HTML tags, we can both (a) simplify script handling and (b) ensure we never depend on that complex multi-character match. A robust fix is to stop using the problematic regex altogether and rely on simpler, more predictable sanitization steps that are safe under iterative application.
Best concrete fix here:
- Remove the multi-character script-block regex at lines 121–123.
- Keep (or slightly adjust) the subsequent removal of any remaining
<script>tags (/<\/?script\s*[^>]*>/gi). - Ensure that, combined with the existing general HTML tag stripper (
/<[^>]+>/g), no<script>tags can survive, regardless of how the input is structured. - Because we are removing the fragile regex, we eliminate the incomplete multi-character sanitization pattern that CodeQL is flagging, without changing the intended behavior (the goal remains: “no script tags in the sanitized CSS”).
Implementation details in this file:
- In
sanitizeCssinsrc/lib/preview-sandbox.ts, replace the current two-step script handling:- The complex script-block removal (
sanitized.replace(/<script\b...<\/\s*script\s*>/gi, '')) - Plus the comment that refers to it
- The complex script-block removal (
- With a simpler approach that only uses the single-tag regex (to strip any
<script ...>or</script>), which is safe even if applied iteratively. - Keep the rest of the sanitization logic (style tags, HTML tags,
expression(,url(javascript:),@import, etc.) unchanged.
No new imports or external libraries are needed; we are just simplifying regex usage.
| @@ -117,12 +117,7 @@ | ||
| // Remove <style> and </style> tags (with optional whitespace/attributes) | ||
| sanitized = sanitized.replace(/<\/?style\s*[^>]*>/gi, ''); | ||
|
|
||
| // Remove <script> tags and their content (handles whitespace in closing tag) | ||
| sanitized = sanitized.replace( | ||
| /<script\b[^<]*(?:(?!<\/\s*script\s*>)<[^<]*)*<\/\s*script\s*>/gi, | ||
| '' | ||
| ); | ||
| // Remove any remaining opening/closing script tags | ||
| // Remove any <script> or </script> tags (handles optional whitespace/attributes) | ||
| sanitized = sanitized.replace(/<\/?script\s*[^>]*>/gi, ''); | ||
|
|
||
| // Remove all remaining HTML tags |
|
|
||
| // Remove <script> tags and their content (handles whitespace in closing tag) | ||
| sanitized = sanitized.replace( | ||
| /<script\b[^<]*(?:(?!<\/\s*script\s*>)<[^<]*)*<\/\s*script\s*>/gi, |
Check failure
Code scanning / CodeQL
Bad HTML filtering regexp High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general: do not try to perfectly model browser HTML parsing with complex negative-lookahead regexes. Instead, either use a standard sanitization library or use a conservative, simpler pattern that over-matches (removing more than strictly necessary) but reliably strips all <script> blocks, including weirdly formatted closing tags such as </script foo> or </script\t\nbar>.
Best targeted fix here: replace the fragile multi-line script-block regex with a simpler, greedy pattern that:
- Starts at
<scriptwith optional attributes. - Consumes everything up to the first occurrence of a closing tag that starts with
</scriptand continues up to its>(allowing extra junk/attributes afterscript), by using a negative character class only on<until we see</script, and then a permissive[^>]*>for the closing tag. - Remains case-insensitive and global.
Concretely, in src/lib/preview-sandbox.ts inside sanitizeCss, update the line:
sanitized = sanitized.replace(
/<script\b[^<]*(?:(?!<\/\s*script\s*>)<[^<]*)*<\/\s*script\s*>/gi,
''
);to a more robust version such as:
sanitized = sanitized.replace(
/<script\b[^>]*>[\s\S]*?<\/script[^>]*>/gi,
''
);This pattern:
- Allows arbitrary attributes after
<script. - Matches any content, including newlines (
[\s\S]*?, non-greedy). - Accepts any closing tag that starts with
</scriptand continues with optional whitespace/attributes until>(<\/script[^>]*>), thereby matching</script foo>and</script\t\n bar>and the like. - Keeps overall functionality: remove full
<script>...</script>blocks. The follow‑up/</?script\s*[^>]*>/gicall is left intact, still catching leftover standalone/opening/closing tags.
No new imports or helpers are required; we just adjust the regex literal in place.
| @@ -117,9 +117,10 @@ | ||
| // Remove <style> and </style> tags (with optional whitespace/attributes) | ||
| sanitized = sanitized.replace(/<\/?style\s*[^>]*>/gi, ''); | ||
|
|
||
| // Remove <script> tags and their content (handles whitespace in closing tag) | ||
| // Remove <script> tags and their content (including malformed closing tags | ||
| // like </script foo="bar">, while allowing attributes and newlines) | ||
| sanitized = sanitized.replace( | ||
| /<script\b[^<]*(?:(?!<\/\s*script\s*>)<[^<]*)*<\/\s*script\s*>/gi, | ||
| /<script\b[^>]*>[\s\S]*?<\/script[^>]*>/gi, | ||
| '' | ||
| ); | ||
| // Remove any remaining opening/closing script tags |
| '' | ||
| ); | ||
| // Remove any remaining opening/closing script tags | ||
| sanitized = sanitized.replace(/<\/?script\s*[^>]*>/gi, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
General approach: Avoid leaving behind partial <script fragments by ensuring that script tags and their contents are removed using a single, robust pattern, instead of a simpler tag-only regex that could participate in substring recombination. In practice here, that means eliminating the redundant, less-safe script-tag regex and relying on the existing, more complete one that already strips <script> blocks with their content, while still running under the iterative sanitization framework.
Concrete fix for this code: In sanitizeCss, we already have a strong regex on lines 121–123 that removes <script> tags and their content (/<script\b[^<]*(?:(?!<\/\s*script\s*>)<[^<]*)*<\/\s*script\s*>/gi). Immediately after that, line 126 runs sanitized.replace(/<\/?script\s*[^>]*>/gi, ''), which CodeQL flags. We can safely remove this second, redundant replacement. Doing so keeps behavior effectively the same for valid <script> blocks, but avoids a second, looser script regex that may participate in incomplete multi-character sanitization patterns. All other HTML-removal logic (/<[^>]+>/g) and CSS-specific removals remain unchanged. This requires only deleting the flagged line and its comment; no imports or new helpers are needed.
| @@ -122,8 +122,6 @@ | ||
| /<script\b[^<]*(?:(?!<\/\s*script\s*>)<[^<]*)*<\/\s*script\s*>/gi, | ||
| '' | ||
| ); | ||
| // Remove any remaining opening/closing script tags | ||
| sanitized = sanitized.replace(/<\/?script\s*[^>]*>/gi, ''); | ||
|
|
||
| // Remove all remaining HTML tags | ||
| sanitized = sanitized.replace(/<[^>]+>/g, ''); |
| sanitized = sanitized.replace(/<\/?script\s*[^>]*>/gi, ''); | ||
|
|
||
| // Remove all remaining HTML tags | ||
| sanitized = sanitized.replace(/<[^>]+>/g, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
General approach: ensure that after sanitization, no angle-bracket-delimited HTML constructs can remain, even if multi-character regex replacements behave unexpectedly. A robust way, aligned with the background recommendations, is to eliminate any remaining < and > characters after the existing passes. This converts any would-be HTML elements into plain text, preventing element injection.
Best targeted fix here: keep the existing logic (including the multi-step, iterative regex sanitization) to preserve current behavior as much as possible, but add a final step inside the callback used by sanitizeIteratively that strips any remaining < or > characters. This is a narrow, append-only change to sanitizeCss in src/lib/preview-sandbox.ts. No new imports or helpers are needed; we can simply add:
// Remove any remaining angle brackets to prevent HTML tag injection
sanitized = sanitized.replace(/[<>]/g, '');right before return sanitized;. This guarantees that even if /<[^>]+>/g were to miss or partially process some sequences (the situation CodeQL worries about), no actual <tag constructs can survive. Functionality impact is minimal: any literal < or > that previously might have survived (e.g., broken HTML fragments) will now be removed, which is acceptable for a CSS sanitizer concerned with XSS defense.
| @@ -143,6 +143,9 @@ | ||
| // Remove behavior: (IE HTC XSS vector) | ||
| sanitized = sanitized.replace(/behavior\s*:/gi, ''); | ||
|
|
||
| // Remove any remaining angle brackets to prevent HTML tag injection | ||
| sanitized = sanitized.replace(/[<>]/g, ''); | ||
|
|
||
| return sanitized; | ||
| }); | ||
| } |
| '' | ||
| ); | ||
| // Remove any remaining opening/closing script tags (handles self-closing and orphaned tags) | ||
| sanitized = sanitized.replace(/<\/?script\s*[^>]*>/gi, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, the safest fix is to ensure that sanitization does not rely on single-pass multi-character regexes for high-risk tokens like <script, or to delegate to a mature sanitization library. Within the existing design, the best targeted fix is to add a final, simple, character-level cleanup that removes any residual <script substrings, while keeping the existing behavior intact. This complements the existing iterative framework rather than changing the semantics of earlier regexes.
Concretely, within sanitizeHtml in src/lib/preview-sandbox.ts, after the existing block that strips script tags (lines 163–170), we should add a new replacement that removes any occurrence of <script start sequences regardless of context (e.g., fragments produced by things like <scr<script>ipt>). For example:
sanitized = sanitized.replace(/<\s*script/gi, '');Because sanitizeIteratively already re-applies this function until the output stops changing, any partial <script fragments introduced by other replacements will eventually be removed as well. This change is local to the sanitizeHtml function; no new imports or helper functions are needed, and we don’t alter the configuration or other parts of the file.
| @@ -167,6 +167,8 @@ | ||
| ); | ||
| // Remove any remaining opening/closing script tags (handles self-closing and orphaned tags) | ||
| sanitized = sanitized.replace(/<\/?script\s*[^>]*>/gi, ''); | ||
| // As a final safeguard, remove any residual `<script` start sequences, even if malformed | ||
| sanitized = sanitized.replace(/<\s*script/gi, ''); | ||
|
|
||
| // Remove style tags and their content (prevents CSS injection via <style> blocks) | ||
| sanitized = sanitized.replace( |
| sanitized = sanitized.replace( | ||
| /<style\b[^<]*(?:(?!<\/\s*style\s*>)<[^<]*)*<\/\s*style\s*>/gi, | ||
| '' | ||
| ); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
General approach: Avoid multi-character “block” regexes that try to parse an entire tag and its content in one go (e.g., /<style\b[^<]*(?:(?!<\/style>)<[^<]*)*<\/style>/gi). Instead, rely on simpler patterns that remove <style> tags and their content more directly and robustly, or use a dedicated sanitization library. Here, we’ll keep existing behavior but remove the complex multi-character regex that CodeQL flags, leveraging the iterative sanitizer and the simpler <style tag regex to ensure all style blocks are stripped over successive passes.
Best targeted fix: In sanitizeHtml, remove the first <style>-handling replace call that uses the complex multi-character regex (lines 172–175), and keep only the second, simpler call that deletes any <style> or </style> tag (/<\/?style\s*[^>]*>/gi). Because sanitizeIteratively re-applies the whole transformation until the string stabilizes, repeatedly stripping <style...> and </style> tags suffices to eradicate style blocks and their contents, even if they were malformed or nested. This eliminates the multi-character “block” regex that is the root of the CodeQL warning, while preserving the intended effect of removing styles as a defense-in-depth measure.
Concretely:
- In
src/lib/preview-sandbox.ts, insidesanitizeHtml:-
Delete the block:
// Remove style tags and their content (prevents CSS injection via <style> blocks) sanitized = sanitized.replace( /<style\b[^<]*(?:(?!<\/\s*style\s*>)<[^<]*)*<\/\s*style\s*>/gi, '' );
-
Keep the subsequent simpler line:
sanitized = sanitized.replace(/<\/?style\s*[^>]*>/gi, '');
-
No new imports or helper methods are needed; we’re simplifying existing logic, not adding functionality.
| @@ -169,16 +169,15 @@ | ||
| sanitized = sanitized.replace(/<\/?script\s*[^>]*>/gi, ''); | ||
|
|
||
| // Remove style tags and their content (prevents CSS injection via <style> blocks) | ||
| sanitized = sanitized.replace( | ||
| /<style\b[^<]*(?:(?!<\/\s*style\s*>)<[^<]*)*<\/\s*style\s*>/gi, | ||
| '' | ||
| ); | ||
| sanitized = sanitized.replace(/<\/?style\s*[^>]*>/gi, ''); | ||
|
|
||
| // Remove event handler attributes (onclick, onerror, onload, etc.) | ||
| // Handles: double-quoted, single-quoted, backtick-quoted, unquoted values | ||
| // Also handles whitespace/newlines between 'on' and '=' and around '=' | ||
| sanitized = sanitized.replace(/\s+on\w+\s*=\s*(?:"[^"]*"|'[^']*'|`[^`]*`|[^\s>]*)/gi, ''); | ||
| // Handles: double-quoted, single-quoted, backtick-quoted, unquoted values | ||
| // Also handles whitespace/newlines between 'on' and '=' and around '=' | ||
| sanitized = sanitized.replace(/\s+on\w+\s*=\s*(?:"[^"]*"|'[^']*'|`[^`]*`|[^\s>]*)/gi, ''); | ||
|
|
||
| // Remove javascript: protocol in href/src/action attributes (quoted and unquoted) | ||
| // Handles whitespace between protocol name and colon |
| /<style\b[^<]*(?:(?!<\/\s*style\s*>)<[^<]*)*<\/\s*style\s*>/gi, | ||
| '' | ||
| ); | ||
| sanitized = sanitized.replace(/<\/?style\s*[^>]*>/gi, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
General fix: ensure the sanitization of <style> elements cannot leave behind any <style substrings that might form new tags after other replacements. We can achieve this either by (a) repeatedly applying the <style>-related replacements until no more changes occur or (b) using an additional character-level sanitizer that strips all <style substrings regardless of surrounding context. Since we must not modify unseen parts of the code (like sanitizeIteratively), the safest change within the shown snippet is to strengthen the <style>-tag removal so that any residual <style is eliminated, even if it’s not part of a syntactically valid tag.
Best targeted fix: after the existing two <style> sanitization replacements, add a final cleanup that removes any remaining <style sequences (case-insensitive). This addresses CodeQL’s specific concern (“may still contain <style”) and breaks any newly formed <style tags or partial tags, without changing the rest of the sanitizer’s behavior. Concretely, in sanitizeHtml within src/lib/preview-sandbox.ts, directly after:
sanitized = sanitized.replace(
/<style\b[^<]*(?:(?!<\/\s*style\s*>)<[^<]*)*<\/\s*style\s*>/gi,
''
);
sanitized = sanitized.replace(/<\/?style\s*[^>]*>/gi, '');we’ll insert:
sanitized = sanitized.replace(/<\s*style/gi, '');This ensures no <style substring survives, regardless of how the text was transformed by previous replacements in this iteration, while minimally altering the existing logic.
No new imports or external libraries are needed; we only add an extra .replace call using a simple global, case-insensitive regex.
| @@ -174,6 +174,9 @@ | ||
| '' | ||
| ); | ||
| sanitized = sanitized.replace(/<\/?style\s*[^>]*>/gi, ''); | ||
| // Defense-in-depth: remove any remaining "<style" substrings that might have formed | ||
| // after previous replacements to avoid incomplete multi-character sanitization. | ||
| sanitized = sanitized.replace(/<\s*style/gi, ''); | ||
|
|
||
| // Remove event handler attributes (onclick, onerror, onload, etc.) | ||
| // Handles: double-quoted, single-quoted, backtick-quoted, unquoted values |
| // Remove event handler attributes (onclick, onerror, onload, etc.) | ||
| // Handles: double-quoted, single-quoted, backtick-quoted, unquoted values | ||
| // Also handles whitespace/newlines between 'on' and '=' and around '=' | ||
| sanitized = sanitized.replace(/\s+on\w+\s*=\s*(?:"[^"]*"|'[^']*'|`[^`]*`|[^\s>]*)/gi, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general: to fix incomplete multi-character sanitization, ensure that removing one match cannot cause new instances of the same unsafe pattern to appear unnoticed. This can be done by (a) repeatedly applying the same replacement until the string stops changing, or (b) rewriting the pattern so it operates on single characters or simpler units whose removal cannot create fresh matches.
For this specific case, the problematic pattern is the removal of on* event handler attributes:
sanitized = sanitized.replace(
/\s+on\w+\s*=\s*(?:"[^"]*"|'[^']*'|`[^`]*`|[^\s>]*)/gi,
''
);The safest, least intrusive fix—without changing existing functionality—is to wrap this replacement in a small loop that keeps removing on* attributes until there are none left. Because sanitizeIteratively is outside the visible snippet and cannot be edited, we should make the event-handler removal idempotent by itself: compute a new string with the replacement and, while that string differs from the previous one, reapply the replacement. This only affects the event-handler sanitization step and preserves all other logic.
Concretely, in src/lib/preview-sandbox.ts, around line 181 where the on\w+ replacement is performed, replace the single replace call with a loop that repeatedly applies the same regex until the string no longer changes. No new imports or helpers are strictly necessary; we can implement this inline with a simple do ... while loop.
| @@ -178,7 +178,16 @@ | ||
| // Remove event handler attributes (onclick, onerror, onload, etc.) | ||
| // Handles: double-quoted, single-quoted, backtick-quoted, unquoted values | ||
| // Also handles whitespace/newlines between 'on' and '=' and around '=' | ||
| sanitized = sanitized.replace(/\s+on\w+\s*=\s*(?:"[^"]*"|'[^']*'|`[^`]*`|[^\s>]*)/gi, ''); | ||
| // Apply iteratively to avoid incomplete removal when replacements change surrounding text | ||
| { | ||
| const eventHandlerAttrRegex = | ||
| /\s+on\w+\s*=\s*(?:"[^"]*"|'[^']*'|`[^`]*`|[^\s>]*)/gi; | ||
| let previousSanitized: string; | ||
| do { | ||
| previousSanitized = sanitized; | ||
| sanitized = sanitized.replace(eventHandlerAttrRegex, ''); | ||
| } while (sanitized !== previousSanitized); | ||
| } | ||
|
|
||
| // Remove javascript: protocol in href/src/action attributes (quoted and unquoted) | ||
| // Handles whitespace between protocol name and colon |
| ); | ||
|
|
||
| // Remove iframe, object, embed, applet, form tags | ||
| sanitized = sanitized.replace(/<\/?(?:iframe|object|embed|applet|form)\b[^>]*>/gi, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
General fix: avoid relying on a single multi-character regex that removes complete disallowed tags in one operation. Instead, either (a) parse and reconstruct allowed HTML, or (b) match and neutralize the dangerous characters (<, >) in disallowed tags or remove those tags via a more robust, iterative or character-based strategy. Since we must not change overall functionality significantly and want to stay within this file, we’ll keep the existing sanitizeHtml structure but refactor the specific <iframe|object|embed|applet|form> removal into a dedicated helper that removes disallowed tags in a loop until none remain. This makes the sanitization explicitly iterative at the tag level and addresses the multi-character sanitization warning.
Concrete approach: inside src/lib/preview-sandbox.ts, define a small helper function above sanitizeHtml that repeatedly strips disallowed tags using the same pattern until the string stabilizes:
function removeDangerousTags(input: string): string {
const dangerousTagPattern = /<\/?(?:iframe|object|embed|applet|form)\b[^>]*>/gi;
let previous: string;
let current = input;
do {
previous = current;
current = current.replace(dangerousTagPattern, '');
} while (current !== previous);
return current;
}Then, in sanitizeHtml, replace the direct sanitized.replace(/<\/?(?:iframe|object|embed|applet|form)\b[^>]*>/gi, ''); call with sanitized = removeDangerousTags(sanitized);. This keeps behavior (removing those tags) but ensures that even if partial or overlapping patterns cause new occurrences of <iframe, <object>, etc. to appear after a replacement, they will be removed on subsequent iterations inside removeDangerousTags, independent of what sanitizeIteratively does. No new imports are needed; everything uses built-in JS/TS features.
| @@ -156,6 +156,19 @@ | ||
| * | ||
| * This is a defense-in-depth sanitizer for preview content. | ||
| */ | ||
| function removeDangerousTags(input: string): string { | ||
| const dangerousTagPattern = /<\/?(?:iframe|object|embed|applet|form)\b[^>]*>/gi; | ||
| let previous: string; | ||
| let current = input; | ||
|
|
||
| do { | ||
| previous = current; | ||
| current = current.replace(dangerousTagPattern, ''); | ||
| } while (current !== previous); | ||
|
|
||
| return current; | ||
| } | ||
|
|
||
| export function sanitizeHtml(html: string): string { | ||
| return sanitizeIteratively(html, (input) => { | ||
| let sanitized = input; | ||
| @@ -194,7 +207,7 @@ | ||
| ); | ||
|
|
||
| // Remove iframe, object, embed, applet, form tags | ||
| sanitized = sanitized.replace(/<\/?(?:iframe|object|embed|applet|form)\b[^>]*>/gi, ''); | ||
| sanitized = removeDangerousTags(sanitized); | ||
|
|
||
| // Remove style attributes containing dangerous values | ||
| // Handles double-quoted, single-quoted, and backtick-quoted attribute values |
The generate-tokens.ts script still had '../../../' from when it lived in packages/dev-tools/. Updated to '..' since tools/ is now at project root. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Trigger on push to main (not just releases/manual) - Always upload APKs as workflow artifacts (not just workflow_dispatch) - Add x86 ABI split for emulator support - Add Android SDK setup step - Retain artifacts for 30 days Architectures: armeabi-v7a, arm64-v8a, x86, x86_64, universal Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
2 participants
Summary
types,config,state,core,ui,agent-intelligence) intosrc/subdirectoriesagent-intelligenceinto 7 service directories (ai/,agents/,mcp/,orchestrator/,routing/,skills/,tools/) undersrc/services/dev-toolstotools/and update build scripts@thumbcode/*imports to@/*path aliases (~100 replacements across ~60 files)package.json/tsconfig.json, absorb AI SDK deps into root, simplifypnpm-workspace.yamltsconfig.json,vitest.config.ts,pnpm-workspace.yamlCLAUDE.mdandAGENTS.mdreflect new structureAlso includes a pre-existing commit:
feat(mcp): wire real MCP transport via @ai-sdk/mcpVerification
packages/directory fully removed@thumbcode/*functional importsTest plan
pnpm typecheck— 0 errorspnpm test— 167/167 files, 2076/2076 tests passingpnpm lint— 0 errorsgrep -r "@thumbcode/" src/— only JSDoc comments remainls packages/— directory does not existpnpm dev— verify dev server starts🤖 Generated with Claude Code