Skip to content

fix(updater): 🐛 sanitize update error details#38

Merged
jorben merged 4 commits intomasterfrom
fix/windows-updater
Feb 14, 2026
Merged

fix(updater): 🐛 sanitize update error details#38
jorben merged 4 commits intomasterfrom
fix/windows-updater

Conversation

@jorben
Copy link
Collaborator

@jorben jorben commented Feb 13, 2026

Summary

  • Truncate updater error messages before sending to the renderer to avoid leaking long/sensitive URLs
  • Remove response payload from updater error logs to prevent logging large/sensitive data
  • Validate required update manifests in the release workflow

Test Plan

  • Not run (not requested)

🤖 Generated with Claude Code

jorben and others added 3 commits February 13, 2026 23:04
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Summary

This PR improves security by sanitizing updater error messages to prevent leaking sensitive information like URLs or file paths. It also adds validation for required update manifests in the release workflow. The changes are generally well-implemented, though there are a few concerns worth noting.

Critical

  • None

Important

  • [src/main/services/UpdateService.ts:71] Stack trace logging: The code includes stack: error instanceof Error ? error.stack : undefined in the errorDetails object logged to console. Stack traces often contain file paths, function names, and other internal application details that could reveal sensitive information about the server environment or application structure. Consider removing the stack from the logged object to fully align with the PR's security goals.

  • [src/main/services/UpdateService.ts:66] Hardcoded truncation limit: The 200-character truncation limit is hardcoded without explanation. Consider extracting this to a named constant (e.g., MAX_ERROR_MESSAGE_LENGTH) with a comment explaining why this specific limit was chosen, to improve maintainability and make future adjustments easier.

Suggestion

  • [.github/workflows/release.yml:137] Manifest validation timing: The validation runs after filtering but before upload, which is correct. However, it would be more robust to validate that manifests not only exist but also contain valid YAML. Consider adding a basic YAML validity check or at least a comment explaining that manifest content validation happens elsewhere.

  • [src/renderer/components/UpdateChecker.tsx:120] Error message display: Displaying error messages to users can be helpful for debugging but ensure translations are in place for all error messages since users may see raw error text that wasn't intended for display.

Praise

  • [.github/workflows/release.yml:127-143] Manifest validation: Adding validation for required update manifests before release is an excellent improvement that prevents incomplete releases from being published.

  • [src/main/services/UpdateService.ts:64-65] Type-safe error handling: Properly checking error instanceof Error before accessing properties and using fallback String(error) is a robust pattern that handles edge cases well.

  • [src/main/services/UpdateService.ts:64-74] Comprehensive error details: Collecting multiple properties (message, name, stack, code, statusCode) into an object for logging provides good debuggability while keeping the renderer communication safe with just the truncated message.

  • [src/renderer/components/UpdateChecker.tsx:117-121] User-friendly error display: Adding error message display with proper styling (maxWidth, wordBreak: 'break-word') improves the user experience during update failures.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Summary

This PR addresses security concerns in the auto-updater by sanitizing error messages before sending them to the renderer, preventing potential leakage of long URLs or sensitive data. It also adds manifest validation in the release workflow. The changes are well-intentioned but have a significant security gap: the stack property is still being logged, which can leak file paths and other system information.

Critical

  • src/main/services/UpdateService.ts:72Stack trace still logged, leaking file paths: The code correctly truncates the message sent to the renderer, but errorDetails.stack is still passed to console.error. Stack traces typically include absolute file paths (e.g., /Users/username/project/src/...) which can reveal system information. Remove stack from errorDetails or use a safe stack representation.

  • src/main/services/UpdateService.ts:69Error code may contain sensitive data: The code property from the error is included in logged details without sanitization. Some errors include HTTP headers, tokens, or other sensitive info in their code property. Consider whether code should be included at all, or at least validate/filter it.

Important

  • src/main/services/UpdateService.ts:67Truncation may produce garbled output: Slicing at 200 characters without respecting word boundaries or URL structure can produce confusing partial URLs. Consider truncating at the first space after 200 chars, or using a regex to cut at path boundaries.

Suggestion

  • src/renderer/components/UpdateChecker.tsx:119-120Consistent error message handling: The renderer now displays error messages to users, which is good UX. However, consider adding a character limit in the UI display as well (CSS text-overflow: ellipsis with a title tooltip) to handle edge cases where the truncated message is still long.

  • src/main/services/UpdateService.ts:75Consider typed error handling: Using type casting (error as { code?: string | number }).code is fragile. Consider creating a type guard or extracting known safe error properties explicitly.

Praise

  • src/main/services/UpdateService.ts:64-65Defensive error handling: Properly handling both Error instances and primitive values with String(error) prevents runtime crashes.

  • .github/workflows/release.yml:136-147Comprehensive manifest validation: Adding explicit validation for all three platform manifests before release prevents incomplete releases from being published.

  • src/renderer/components/UpdateChecker.tsx:121Good UX improvement: Displaying the error message to users (even truncated) with proper word-break styling enables users to report meaningful issues.

@jorben jorben merged commit e15c8ed into master Feb 14, 2026
3 checks passed
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