feat(cloud): ✨ Add cloud API integration with authentication and credits

[jorben]

Summary

• Implement full cloud API integration with device flow OAuth authentication
• Add cloud task management (create, list, delete, detail with page images)
• Integrate credits system with real backend (free/paid balance, daily limits, history)
• Add SSE real-time updates for cloud task status with reconnection handling
• Support Office file uploads and multi-tier cloud model selection
• Add internationalization for all cloud features across 6 locales

Test Plan

• Verify device flow login/logout works correctly
• Test cloud task creation with PDF and Office files
• Verify credit balance display and history pagination
• Test SSE reconnection and event deduplication
• Confirm recharge button is disabled with "Coming soon" tooltip
• Run full test suite: npm test

🤖 Generated with Claude Code

[github-actions]

Summary

This PR introduces a substantial cloud integration surface (auth/device flow, cloud task APIs, SSE updates, credits, and renderer wiring) and is generally well-structured for a large feature. However, I found a couple of correctness/security issues that should be fixed before merge, plus a few important contract/perf gaps. As-is, I don’t think it’s fully ready to merge yet.

Critical

• src/core/infrastructure/services/AuthManager.ts:537 — Refresh token may be persisted in plaintext fallback: persistRefreshToken writes the refresh token unencrypted when safeStorage.isEncryptionAvailable() is false. This is a credential-at-rest leak risk on some platforms/environments.
  Fix: Do not persist the refresh token unless encryption is available (keep session memory-only in that case), or encrypt with an app-managed key from OS keychain/credential vault. At minimum, gate persistence behind a strict security check and log a warning.

• src/main/index.ts:114 — Protocol callback URL is not validated beyond scheme prefix: handleProtocolUrl accepts any markpdfdown://... URL and triggers auth token check. While you’re not parsing params now, this still creates a broad attack surface for external URL triggers and future callback parsing bugs.
  Fix: Strict-parse the URL and only accept expected host/path patterns for OAuth callback (e.g., markpdfdown://auth/callback), reject everything else, and avoid logging full callback URLs if tokens/codes may appear in query params in future.

Important

• src/preload/electron.d.ts:89 — IPC type contract mismatch for cloud.convert: d.ts omits page_range but preload implementation includes it (src/preload/index.ts:93). This can cause renderer compile-time mismatch and hidden runtime usage drift.
  Fix: Update electron.d.ts to include page_range?: string in cloud.convert signature (and ideally share a single type from shared/types).

• src/core/infrastructure/services/CloudSSEManager.ts:228 — Unbounded verbose logging in stream loop: per-chunk logging (Chunk #...) and frequent event logging can heavily degrade main-process performance and bloat logs during long sessions.
  Fix: Gate chunk/event debug logs behind a debug flag or environment check; keep only warnings/errors in production.

• src/core/infrastructure/services/CloudService.ts:150 — Path parameters are not URL-encoded: IDs are interpolated directly in multiple endpoints (/tasks/${id}, /tasks/${taskId}/...). If malformed IDs ever pass through IPC, requests can break or hit unintended routes.
  Fix: Wrap all path params with encodeURIComponent(...) before interpolation.

Suggestion

• src/main/ipc/handlers/cloud.handler.ts:12 — Missing input validation at IPC boundary: handlers trust renderer-provided payloads (taskId, pageNumber, fileData.path) without shape/range checks.
  Suggestion: Add lightweight runtime validation (e.g., zod/manual guards) in handlers before calling services to reduce malformed input risks and improve error clarity.

Praise

• src/core/infrastructure/services/AuthManager.ts:83 — Good transient-vs-definitive auth failure handling: distinguishing AuthTokenInvalidError from network/server errors and scheduling retry is a solid resilience pattern that avoids unnecessary forced logouts.

• src/core/infrastructure/services/CloudSSEManager.ts:46 — Strong reconnection lifecycle control: setting connected before await, aborting old streams, heartbeat timeout, and exponential backoff are well thought out and reduce race/reconnect duplication bugs.

• src/main/ipc/handlers/file.handler.ts:101 — Nice UX/security balance on file filters: conditional Office support plus backend-side revalidation in upload flow is a good defense-in-depth approach against file-type bypasses.

[github-actions]

Summary

This PR adds substantial cloud integration across main/preload/renderer, including OAuth device flow, authenticated cloud task APIs, and SSE task event streaming. Overall, the architecture is solid and most flows are thoughtfully implemented (especially retries/backoff and state broadcasting), but there are a couple of correctness/security issues that should be fixed before merge. I would not merge yet until the critical items below are addressed.

Critical

• src/main/ipc/handlers/cloud.handler.ts:13 — Unbounded IPC input can trigger memory exhaustion via content uploads: The cloud:convert handler accepts arbitrary ArrayBuffer from renderer (fileData.content) and forwards it to FormData without size/type validation. A compromised renderer (or XSS in renderer) can send huge payloads and exhaust main-process memory (DoS).
  Fix: Validate IPC payload in main before passing to CloudService: enforce max byte size, require either path or content (not both), validate filename extension/MIME allowlist, and reject oversized/invalid payloads.

• src/main/index.ts:133 — Protocol callback path validation can be bypassed for malformed but accepted URLs: Path allowlisting is based on string concat of host + pathname and only trims trailing slashes. This may accept unexpected variants (markpdfdown://auth//callback, encoded slashes, case variants) depending on URL normalization and upstream callback generation. For auth callbacks, strict parsing is security-sensitive.
  Fix: Normalize and validate components explicitly: lowercase host, decode/normalize pathname, reject duplicate slashes, and match exact allowed tuples like (host==='auth' && pathname==='/callback') || (host==='auth' && pathname==='').

Important

• src/core/infrastructure/services/AuthManager.ts:308 — Timeout/retry race can duplicate refresh attempts under concurrent requests: fetchWithAuth and getAccessToken can both call refreshAccessToken() concurrently from different callers when token is near expiry or 401 responses happen in parallel. This can cause redundant refresh calls and state races (especially around token rotation).
  Fix: Add a refresh mutex/promise deduplication (this.refreshInFlight) so concurrent callers await one refresh operation.

• src/core/infrastructure/services/AuthManager.ts:150 — shell.openExternal not awaited/checked: Browser open failures are ignored; user can be stuck in polling without browser launched.
  Fix: await shell.openExternal(...) and handle false/rejection by setting deviceFlowStatus='error' with actionable message.

• src/core/infrastructure/services/CloudSSEManager.ts:223 — SSE parser only splits on \n\n, misses CRLF framing: Many SSE servers delimit with \r\n\r\n. Current parsing can delay or break event handling.
  Fix: Normalize line endings (buffer.replace(/\r\n/g, '\n')) before split, or split on /\r?\n\r?\n/.

• src/core/infrastructure/services/CloudService.ts:382 — Filename from Content-Disposition insufficiently sanitized: Download filename is derived from header and directly used in path.join(downloadsPath, fileName). Even with save dialog, odd names can degrade UX and potentially cause path oddities on some platforms.
  Fix: Sanitize filename to basename-safe characters (path.basename, strip control chars/reserved chars), and fallback to safe default.

• src/preload/electron.d.ts:85 — any-heavy API contracts weaken IPC type safety: New auth/cloud APIs and events are all Promise<any>/(event:any), increasing risk of contract drift between main/preload/renderer.
  Fix: Reuse shared cloud/auth response/event types in preload typings and expose strict return types.

Praise

• src/core/infrastructure/services/AuthManager.ts:90 — Good transient vs permanent auth failure handling: Distinguishing AuthTokenInvalidError from transient network/server errors avoids unnecessary logout and improves resilience.
• src/core/infrastructure/services/CloudSSEManager.ts:119 — Clean reconnect strategy: Backoff + heartbeat timeout + timer cleanup is well-structured and reduces duplicate stream risk.
• src/main/ipc/handlers/file.handler.ts:101 — Nice UX/security balance on file filters: Conditional Office support plus dialog filter ordering is a practical improvement and integrates cleanly with existing file flow.

[github-actions]

Summary

This PR adds substantial cloud/auth infrastructure (device-flow OAuth, cloud task CRUD, credits APIs, SSE streaming, protocol callback wiring, and renderer/preload IPC exposure). Overall direction is solid and many reliability concerns were considered, but there are a few correctness/security issues that should be fixed before merge. As-is, I don’t think it is ready to merge yet.

Critical

• src/main/ipc/handlers/cloud.handler.ts:17 — IPC handler accepts arbitrary renderer-provided file paths without path trust boundary: cloud:convert will read fileData.path directly in main process (fs.statSync then eventually fs.readFileSync in CloudService.convert). Any renderer compromise (or untrusted web content loaded by mistake) can exfiltrate arbitrary local files by passing crafted absolute paths.
  Fix: Restrict path inputs to files selected via trusted dialog.showOpenDialog flow (e.g., mint short-lived file tokens/handles in main and only accept those), or disallow path input entirely and require content passed from preload-controlled selection flow.

• src/core/infrastructure/services/CloudService.ts:53 — Synchronous full-file read in main process can block UI/event loop: fs.readFileSync(fileData.path) is used for uploads. For large files (up to 100MB allowed), this can noticeably freeze Electron main process and impact IPC responsiveness.
  Fix: Use async IO (fs.promises.readFile) or stream/multipart upload to avoid blocking main thread. If full buffering is required, do it async and consider worker/off-main process.

Important

• src/main/index.ts:121 — Protocol callback path normalization can throw on malformed percent-encoding: decodeURIComponent(parsed.host) / decodeURIComponent(parsed.pathname) can throw for malformed escape sequences, which is caught, but then valid-but-unusual encoded paths may be rejected inconsistently after decode/recombine.
  Fix: Normalize using URL components without manual decode where possible, and compare against canonical expected forms more defensively (or decode each segment safely).

• src/preload/electron.d.ts:85 — Weak API contracts (Promise<any>) for new auth/cloud IPC surface: Most newly added APIs are typed as any, which undermines compile-time contract checks across preload/renderer and can hide breaking changes.
  Fix: Replace any with shared response/request types from shared/types/cloud-api.ts (and equivalent auth types), especially for auth:*, cloud:*, and event callbacks.

• src/core/infrastructure/services/AuthManager.ts:313 — Download timeout detection via URL substring is brittle: isDownload = url.includes('/result') || url.includes('/download') can disable timeout for unintended endpoints or miss real download endpoints (/pdf currently not included).
  Fix: Make timeout behavior explicit via options flag (fetchWithAuth(url, { ..., meta: { timeoutMs } }) pattern) or centralize endpoint-specific configuration.

Suggestion

• src/core/infrastructure/services/CloudSSEManager.ts:155 — No content-type validation for SSE stream: You log content-type but don’t enforce text/event-stream, so HTML/JSON error bodies might be parsed as stream data and generate noisy reconnect loops.
  Suggestion: Validate content-type includes text/event-stream; otherwise treat as non-retriable (or retry with capped attempts + explicit auth check).

Praise

• src/core/infrastructure/services/AuthManager.ts:498 — Refresh de-duplication is well implemented: refreshInFlight avoids concurrent refresh races and token stomp issues; this is a strong correctness safeguard.
• src/core/infrastructure/services/CloudService.ts:404 — Safe filename sanitization on PDF download: Using path.basename plus reserved/control character replacement is a good defense against path/header-based filename abuse.
• src/core/infrastructure/services/CloudSSEManager.ts:118 — Reconnect cleanup logic prevents duplicate reconnect storms: Clearing existing reconnect and heartbeat timers before scheduling reconnect is a solid resilience pattern.

[github-actions]

Summary

This PR introduces substantial cloud integration across main/preload/renderer, including OAuth device flow auth, cloud task APIs, credits, and SSE updates. Overall the structure is solid and the implementation shows good attention to retry logic and token lifecycle, but there are a few correctness and contract issues that should be fixed before merge. I would not merge yet until the critical items are addressed.

Critical

• src/core/infrastructure/services/CloudSSEManager.ts:177 — SSE abort signal is dropped due to fetchWithAuth overwrite: startStream() passes signal: this.abortController.signal, but AuthManager.fetchWithAuth() always creates its own AbortController and overwrites options.signal. This means disconnect()/reconnect() won’t reliably abort the in-flight SSE fetch, causing leaked/duplicate streams and racey reconnect behavior.
  Fix: Update fetchWithAuth to respect a caller-provided options.signal (compose with timeout signal via AbortSignal.any where available, or skip creating timeout controller when options.signal exists).

• src/main/index.ts:126 — OAuth callback trigger can be spoofed by any markpdfdown://auth... URL: handleProtocolUrl validates only scheme/path and then immediately calls authManager.checkDeviceTokenStatus() with no state/nonce validation. Any external app/site can force callback handling and authentication polling transitions (CSRF-like trigger on local app state).
  Fix: Require a short-lived random state generated at login initiation and validate it in protocol callback before invoking token check. Ignore callbacks without matching state.

Important

• src/preload/electron.d.ts:85 — Unsafe any API contracts for new IPC surface: all newly added auth/cloud methods and event payloads are typed as Promise<any> / any, which breaks shared contract safety and makes main/renderer drift likely.
  Fix: Reuse shared/types/cloud-api.ts types in preload typings and enforce strongly-typed return envelopes ({ success, data, error }) per method.

• src/main/ipc/handlers/cloud.handler.ts:16 — Input validation incomplete for untrusted renderer payload: cloud:convert validates size but not shape/content of fileData.name, model, or page_range. Since IPC is an attack surface, malformed values can still hit backend/API unexpectedly.
  Fix: Add strict runtime validation (type/length/pattern), especially for page_range and model allowlist, and reject invalid payloads early.

• src/core/infrastructure/services/AuthManager.ts:312 — Timeout abort error not normalized for UX: fetchWithAuth uses abort for timeout but returns raw AbortError, which will surface as generic failure in many callers.
  Fix: Catch AbortError in fetchWithAuth and throw a normalized timeout error message (e.g., Request timeout) so renderer can show actionable feedback.

Suggestion

• src/core/infrastructure/services/AuthManager.ts:1 — Use async fs APIs for token persistence in main process: sync fs operations (readFileSync, writeFileSync, etc.) on auth paths can block the main thread.
  Suggestion: Switch to fs/promises equivalents and keep initialization non-blocking.

• src/renderer/components/UploadPanel.tsx:119 — Duplicate comment line: “Inject Cloud Models...” comment is duplicated consecutively, which adds noise.
  Suggestion: Remove redundant line.

Praise

• src/core/infrastructure/services/AuthManager.ts:483 — Good refresh deduplication: refreshInFlight guard is a strong concurrency control that prevents token refresh stampedes.

• src/core/infrastructure/services/CloudService.ts:402 — Safe filename handling for downloads: sanitizing Content-Disposition filename with basename and reserved/control char replacement is a solid defense against path/file-name abuse.

• src/main/index.ts:113 — Protocol path allowlist validation: validating callback URL against explicit allowed paths is a good baseline hardening step for custom protocol handling.

[github-actions]

Summary

This PR adds a substantial cloud integration surface (auth/device flow, cloud task APIs, SSE updates, credits, protocol callback wiring, and renderer IPC/context integration). Overall the implementation is solid and thoughtful (notably around token refresh dedup and SSE reconnect handling), but there are a few correctness and security issues that should be fixed before merge. I would not merge yet until the critical items below are addressed.

Critical

• src/main/ipc/handlers/cloud.handler.ts:19 — IPC allows arbitrary filesystem reads via untrusted fileData.path: The renderer can pass any absolute path to cloud:convert, and main will stat it then CloudService.convert() reads it and uploads contents. If renderer is compromised (XSS/dependency compromise), this becomes local file exfiltration. Restrict path inputs to a trusted file-selection flow (e.g., issue opaque file tokens from file:selectDialog and only allow those), or at minimum validate against a recent allowlist of user-selected paths and block everything else.

• src/main/index.ts:126 — OAuth callback URL validation can be bypassed with encoded slash in host: In handleProtocolUrl, decodeURIComponent(parsed.host) is applied before path checks. A URL like markpdfdown://auth%2Fcallback can decode host to auth/callback and pass ALLOWED_PROTOCOL_PATHS. Host should not be decoded this way for path authorization logic. Validate using structural URL parts directly (parsed.host exact match + normalized parsed.pathname) and reject percent-encoded / in host.

Important

• src/core/infrastructure/services/AuthManager.ts:357 — Potential runtime incompatibility with AbortSignal.any: fetchWithAuth uses AbortSignal.any(signals), which may be unavailable depending on Electron/Node runtime. If unsupported, auth/network calls will throw at runtime. Add a compatibility fallback (manual composite AbortController wiring) when AbortSignal.any is undefined.

• src/core/infrastructure/services/CloudSSEManager.ts:165 — Connection lock can deadlock on early startStream errors: connect() sets this.connected = true before await this.startStream(). If startStream() throws before scheduling reconnect (e.g., synchronous runtime failure), connected remains true and future connect() calls no-op with “Already connected”. Wrap startStream() in try/catch in connect() and reset connected=false on unrecoverable startup error.

• src/main/ipc/handlers/cloud.handler.ts:53 — Missing input bounds validation for pagination parameters: cloud:getTasks, cloud:getTaskPages, and cloud:getCreditHistory forward renderer-provided page/pageSize directly. A malicious renderer can request huge page sizes causing backend and memory pressure. Clamp and validate numeric ranges in IPC layer (e.g., page >= 1, pageSize 1..100).

• src/core/infrastructure/config.ts:1 — Unvalidated API_BASE_URL contract: Base URL is taken from environment without validation. A malformed or non-HTTPS value could break auth flow or downgrade security. Validate at startup (must be https: in production, allow http://localhost only in dev), and fail fast with explicit error.

Praise

• src/core/infrastructure/services/AuthManager.ts:530 — Refresh deduplication is well implemented: The refreshInFlight guard prevents concurrent refresh stampedes and racey token overwrites, which is critical under high request concurrency.

• src/core/infrastructure/services/CloudSSEManager.ts:112 — Reconnect/backoff and heartbeat management are robust: Cleanup of timers/controllers plus exponential backoff and heartbeat timeout handling demonstrates good resilience design for long-lived SSE streams.

• src/main/index.ts:157 — Single-instance + protocol callback handling is thoughtfully integrated: Handling open-url (macOS) and second-instance (Windows/Linux) with window focusing improves cross-platform correctness for OAuth callback UX.

[github-actions]

Summary

This PR introduces a substantial cloud integration surface (OAuth device flow auth, cloud task APIs, credits, SSE updates, and renderer wiring) and overall follows a solid architecture split. The core service structure is good, but there are a few correctness/security issues around unvalidated renderer input and protocol URL handling that should be fixed before merge. With those addressed, this is close to merge-ready.

Critical

• src/main/ipc/handlers/cloud.handler.ts:17 — Unvalidated fileData.path allows arbitrary main-process file reads/uploads: cloud:convert accepts a raw path from the renderer and reads it (fs.promises.stat + later fs.readFile in CloudService) without constraining source. If renderer-side JS is compromised, this can exfiltrate arbitrary local files to cloud.
  Fix: Only allow uploads from:

  1. paths returned by trusted file dialog in this process (track allowlisted selections), or
  2. direct file bytes from renderer (content) while disabling arbitrary path mode for IPC callers.
     Also validate extension/MIME in main before upload, not only in renderer.

• src/main/ipc/handlers/cloud.handler.ts:203 — Unvalidated task/page parameters in privileged IPC endpoints: task IDs/page numbers from renderer are passed directly into authenticated backend requests (getTaskById, retryPage, etc.). While encodeURIComponent helps URL safety, this still exposes a broad authenticated proxy surface to any renderer compromise.
  Fix: Add strict schema validation in each IPC handler (e.g., zod/yup/manual): taskId pattern/length, pageNumber positive integer bounds, pagination limits, and allowed enums (type in history). Reject invalid payloads early.

• src/main/index.ts:116 — Custom protocol URL validation bypass risk due to host parsing assumptions: validation relies on parsed.host and pathname equality with limited normalization. markpdfdown:///auth/callback and other URL forms can shift expected components and may be mishandled.
  Fix: Validate against explicit accepted URL shapes using both hostname, pathname, and possibly origin, with a strict parser helper + exhaustive allowed patterns. Reject anything not exactly matching documented callback URL(s).

Important

• src/core/infrastructure/services/CloudService.ts:53 — Node/Electron runtime compatibility risk for FormData/Blob: service uses web FormData and Blob in main process. Depending on Electron/Node target, this can fail or behave inconsistently.
  Fix: Ensure runtime support is guaranteed by project engine version, or switch to a Node-safe multipart implementation (form-data/undici-native path) and add an integration test in main context.

• src/core/infrastructure/services/AuthManager.ts:34 — Potential credential/user data leakage in logs: multiple logs print auth flow internals and raw error bodies (Token check error: ... body, session restoration errors). Backend error bodies can contain sensitive context.
  Fix: Sanitize/redact logs (status codes + stable error codes only), and avoid printing response bodies from auth/token endpoints in production.

• src/preload/electron.d.ts:85 — Type contract too weak (Promise<any>) for new high-impact IPC API: new auth/cloud APIs are all any, which removes compile-time guarantees between renderer/main and increases runtime mismatch risk.
  Fix: Replace with shared response/request types from shared/types/cloud-api.ts and consistent IPC result envelopes.

Suggestion

• src/core/infrastructure/services/CloudSSEManager.ts:236 — SSE buffer growth guard: parsing loop accumulates buffer until delimiter appears. Malformed stream/chunk behavior could grow memory.
  Suggestion: Add max buffer size guard and reset/reconnect if exceeded.

• src/core/infrastructure/services/AuthManager.ts:492 — Auto-refresh retry scheduling is hard to reason about: recursive scheduleTokenRefresh(retryDelayMs / 1000) during retry path mixes “expiry-driven schedule” and “retry-driven schedule”.
  Suggestion: Split into dedicated scheduleRefreshAtExpiry and scheduleRefreshRetry methods to simplify control flow and avoid accidental timing regressions.

Praise

• src/core/infrastructure/services/AuthManager.ts:531 — Good concurrent refresh deduplication: refreshInFlight prevents token refresh stampedes across concurrent requests, which is important for correctness and backend load.
• src/core/infrastructure/services/CloudSSEManager.ts:140 — Thoughtful reconnect lifecycle cleanup: aborting streams, clearing heartbeat/reconnect timers, and preserving lastEventId is robust and minimizes duplicate event processing.
• src/main/index.ts:394 — Startup sequencing is well integrated: auth restoration is explicitly initialized during background service setup, improving user session continuity and reducing renderer-side complexity.