Skip to content

[codex] Harden desktop image data URL handling#3003

Merged
Kitenite merged 1 commit into
mainfrom
debug-project-icon-image-upload-issue
Mar 30, 2026
Merged

[codex] Harden desktop image data URL handling#3003
Kitenite merged 1 commit into
mainfrom
debug-project-icon-image-upload-issue

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Mar 30, 2026

Summary

  • centralize desktop image MIME normalization and base64 data URL parsing in a shared helper
  • route project icon parsing through the shared helper while keeping the icon allowlist limited to PNG, JPEG, SVG, and ICO
  • reuse the shared parsing in account and organization uploads and extend project settings to accept .ico uploads with surfaced mutation errors

Why

Project icon uploads were relying on a narrow regex in the main-process save path. That broke valid browser-generated data URLs for formats like ICO variants and SVG data URLs with extra MIME parameters. The same MIME parsing pattern also existed in other desktop upload flows.

Impact

  • project icon uploads now handle both image/x-icon and image/vnd.microsoft.icon
  • SVG data URLs with extra parameters are parsed correctly
  • desktop image upload code has one source of truth for MIME-to-extension normalization

Validation

  • bun test apps/desktop/src/shared/file-types.test.ts apps/desktop/src/main/lib/project-icons.test.ts
  • bunx biome check --write 'apps/desktop/src/main/lib/project-icons.ts' 'apps/desktop/src/main/lib/project-icons.test.ts' 'apps/desktop/src/shared/file-types.ts' 'apps/desktop/src/shared/file-types.test.ts' 'apps/desktop/src/lib/trpc/routers/window.ts' 'apps/desktop/src/renderer/routes/_authenticated/settings/account/components/AccountSettings/AccountSettings.tsx' 'apps/desktop/src/renderer/routes/_authenticated/settings/organization/components/OrganizationSettings/OrganizationSettings.tsx' 'apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx'
  • bunx tsc -p apps/desktop/tsconfig.json --noEmit --ignoreDeprecations 6.0 currently fails on an existing unrelated error in apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx(62,4): error TS2871: This expression is always nullish.

Summary by cubic

Centralized and hardened desktop image data URL parsing/normalization to fix project icon uploads and make desktop image uploads consistent. Icons now accept ICO variants and SVG data URLs with extra parameters, and errors surface in the UI.

  • Bug Fixes

    • Project icons accept image/x-icon and image/vnd.microsoft.icon; JPEG normalized to .jpg; SVG with extra params parsed correctly.
    • Data URLs are built with the full MIME type in the file picker flow.
    • Project settings accept .ico and image/vnd.microsoft.icon; show toast on mutation errors.
  • Refactors

    • Introduced shared/file-types helpers: getImageMimeType, getImageExtensionFromMimeType, parseBase64DataUrl; reused across project, account, and org uploads.
    • Centralized icon parsing via parseProjectIconDataUrl with a PNG/JPEG/SVG/ICO allowlist.
    • Added unit tests for shared/file-types and project icon parsing.

Written for commit 83baf82. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Added support for Windows ICO format when uploading project icons.
    • Improved error messaging when project icon updates fail—users now receive clear feedback instead of silent failures.
  • Bug Fixes

    • Enhanced image format handling for avatars, logos, and project icons to ensure consistent MIME type detection and validation across the app.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ac8688c6-844c-4d8b-8a2c-bd22ea83f18f

📥 Commits

Reviewing files that changed from the base of the PR and between ce88250 and 83baf82.

📒 Files selected for processing (8)
  • apps/desktop/src/lib/trpc/routers/window.ts
  • apps/desktop/src/main/lib/project-icons.test.ts
  • apps/desktop/src/main/lib/project-icons.ts
  • apps/desktop/src/renderer/routes/_authenticated/settings/account/components/AccountSettings/AccountSettings.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/organization/components/OrganizationSettings/OrganizationSettings.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx
  • apps/desktop/src/shared/file-types.test.ts
  • apps/desktop/src/shared/file-types.ts

📝 Walkthrough

Walkthrough

Centralizes image MIME type and extension handling by introducing shared utility functions (getImageExtensionFromMimeType, parseBase64DataUrl). Replaces local regex-based and path-based parsing across multiple components (window router, account settings, organization settings, project settings) and main process icon handling. Adds comprehensive test coverage for new utilities.

Changes

Cohort / File(s) Summary
Shared Image Type Utilities
apps/desktop/src/shared/file-types.ts, apps/desktop/src/shared/file-types.test.ts
Added getImageExtensionFromMimeType() and parseBase64DataUrl() functions with MIME type-to-extension mapping (IMAGE_MIME_TYPE_EXTENSIONS). New test suite validates MIME→extension mapping, extension→MIME mapping, base64 data URL parsing with extra parameters, and error handling for invalid formats.
TRPC Window Router
apps/desktop/src/lib/trpc/routers/window.ts
Replaced node:path.extname usage with getImageMimeType from shared utilities. Updated data URL format from data:image/{mimeType} to data:{mimeType} to align with new MIME-type parsing approach.
Project Icon Handling
apps/desktop/src/main/lib/project-icons.ts, apps/desktop/src/main/lib/project-icons.test.ts
Added parseProjectIconDataUrl() function to extract base64 data and MIME type, validate extension against allowed set (png, jpg, svg, ico). Updated saveProjectIconFromDataUrl to use new parser. New test suite validates parsing of PNG, JPEG, SVG, and ICO variants with MIME parameter handling and error cases.
UI Settings Components
apps/desktop/src/renderer/routes/_authenticated/settings/account/components/AccountSettings/AccountSettings.tsx, apps/desktop/src/renderer/routes/_authenticated/settings/organization/components/OrganizationSettings/OrganizationSettings.tsx
Updated avatar and logo upload handlers to use parseBase64DataUrl() and getImageExtensionFromMimeType() instead of local regex extraction and manual string manipulation.
Project Settings Component
apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx
Added UI error notification for icon update failures via toast message. Expanded file input accept attribute to include Windows ICO support (image/vnd.microsoft.icon, .ico).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 From scattered regexes, a pattern emerged—
MIME types and extensions converged.
Shared helpers now sing in harmony true,
Icons and avatars, logos anew!
One home for the parsing, one place for the rules.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: hardening desktop image data URL handling through centralized MIME normalization and parsing helpers.
Description check ✅ Passed The description fully covers all required sections: clear summary, motivation (Why), impact, validation steps, and additional context via auto-generated summary. All key information is present.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch debug-project-icon-image-upload-issue

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@Kitenite Kitenite marked this pull request as ready for review March 30, 2026 03:13
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/desktop/src/main/lib/project-icons.ts">

<violation number="1" location="apps/desktop/src/main/lib/project-icons.ts:116">
P2: Reject empty base64 payloads before saving; the new parser path allows `data:image/...;base64,` and writes a 0-byte icon file.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.


const ext = match[1] === "jpeg" ? "jpg" : match[1];
const buffer = Buffer.from(match[2], "base64");
const { buffer, ext } = parseProjectIconDataUrl(dataUrl);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 30, 2026

Choose a reason for hiding this comment

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

P2: Reject empty base64 payloads before saving; the new parser path allows data:image/...;base64, and writes a 0-byte icon file.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/main/lib/project-icons.ts, line 116:

<comment>Reject empty base64 payloads before saving; the new parser path allows `data:image/...;base64,` and writes a 0-byte icon file.</comment>

<file context>
@@ -89,14 +113,7 @@ export async function saveProjectIconFromDataUrl({
-
-	const ext = match[1] === "jpeg" ? "jpg" : match[1];
-	const buffer = Buffer.from(match[2], "base64");
+	const { buffer, ext } = parseProjectIconDataUrl(dataUrl);
 
 	if (buffer.length > MAX_ICON_SIZE) {
</file context>
Suggested change
const { buffer, ext } = parseProjectIconDataUrl(dataUrl);
const { buffer, ext } = parseProjectIconDataUrl(dataUrl);
if (buffer.length === 0) {
throw new Error("Invalid data URL format");
}
Fix with Cubic

@Kitenite Kitenite merged commit 5839970 into main Mar 30, 2026
7 checks passed
@Kitenite Kitenite deleted the debug-project-icon-image-upload-issue branch March 30, 2026 03:20
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch
  • ⚠️ Electric Fly.io app

Thank you for your contribution! 🎉

siarhei-belavus pushed a commit to siarhei-belavus/localset that referenced this pull request Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant