Hotfix/mac sandbox export bug#19
Merged
Merged
Conversation
…arget file without tmp
…for WebKit fetch compatibility
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbitNew Features
Improvements
WalkthroughCanvas 由来の画像をフロントで Blob→ArrayBuffer に変換して /api/save に POST し、サーバは受信をシステムテンポラリへストリームして先頭バイトで MIME を検証、検証後に os.Rename を試行し失敗時はコピー＋Sync で保存するように変更されました。 ChangesCanvas Export API ペイロード形式とバックエンド検証・保存フロー
Sequence DiagramsequenceDiagram
participant Frontend
participant Server as handleSave
participant FileSystem
Frontend->>Server: POST /api/save<br/>(ArrayBuffer body)
Server->>Server: Create temp file (os.CreateTemp)<br/>Stream request body into temp (io.Copy)
Server->>Server: Read up to 512 bytes from temp<br/>http.DetectContentType
alt validation success
Server->>FileSystem: Rename temp → savePath (os.Rename)
alt rename fails
Server->>FileSystem: Create savePath, copy from temp (io.Copy)
Server->>FileSystem: out.Sync()
end
else validation failure
Server->>Frontend: HTTP 400 / 500
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@handler.go`:
- Around line 182-201: The current flow creates saveFile (os.Create) at savePath
and may leave a partial file if writing header (header, n) or streaming
(io.Copy, r.Body) fails; fix by either writing to a temporary file and renaming
on success (use os.CreateTemp -> write header and io.Copy -> close and
os.Rename(tempPath, savePath), remove temp on error) or, if keeping savePath,
ensure on any write or copy error you first close saveFile and then delete the
incomplete file with os.Remove(savePath) before returning; reference saveFile,
savePath, header, n, io.Copy and r.Body to locate the code to change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a8ba4a08-ceec-47aa-aafc-9349ffddb14a
📒 Files selected for processing (5)
frontend/src/App.tsxfrontend/wailsjs/runtime/package.jsonfrontend/wailsjs/runtime/runtime.d.tsfrontend/wailsjs/runtime/runtime.jshandler.go
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses the macOS export failure by changing the frontend upload payload and adjusting the Go save handler to avoid sandbox-incompatible temp-file creation.
Changes:
- Converts the exported Blob to an ArrayBuffer before POSTing to
/api/save. - Updates
handleSaveto validate the request body header before writing directly to the selected save path. - Adds Wails runtime JS/TypeScript files required by frontend imports.
Reviewed changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
handler.go |
Changes save handling to validate streamed content before creating the output file. |
frontend/src/App.tsx |
Sends an ArrayBuffer instead of a Blob for macOS WebKit compatibility. |
frontend/wailsjs/runtime/runtime.js |
Adds Wails runtime JavaScript wrappers. |
frontend/wailsjs/runtime/runtime.d.ts |
Adds TypeScript declarations for Wails runtime APIs. |
frontend/wailsjs/runtime/package.json |
Adds package metadata for the Wails runtime module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
172
to
178
| if expectedMime != "" && n > 0 { | ||
| actualMime := http.DetectContentType(header[:n]) | ||
| if actualMime != expectedMime { | ||
| http.Error(w, "Security Error: saved file content does not match expected type", http.StatusBadRequest) | ||
| return | ||
| } | ||
| } |
Comment on lines
+180
to
+184
| // Validation passed. Open the target file directly. | ||
| // The app has explicit permission to write to this path granted by the native Save Dialog. | ||
| saveFile, err := os.Create(savePath) | ||
| if err != nil { | ||
| http.Error(w, "Failed to create target file: "+err.Error(), http.StatusInternalServerError) |
…ejection, and EXDEV fallback
| } | ||
| if err := tmpFile.Close(); err != nil { | ||
| http.Error(w, "Failed to close temp file: "+err.Error(), http.StatusInternalServerError) | ||
| tmpFile.Close() |
Comment on lines
+221
to
+228
| out, err := os.Create(savePath) | ||
| if err != nil { | ||
| http.Error(w, "Failed to create final file: "+err.Error(), http.StatusInternalServerError) | ||
| return | ||
| } | ||
| defer out.Close() | ||
|
|
||
| if _, err := io.Copy(out, in); err != nil { |
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@handler.go`:
- Around line 221-237: When creating the fallback file with os.Create(savePath)
and writing to it (io.Copy) or syncing (out.Sync), ensure any error during write
or sync removes the partial file to avoid leaving corrupt artifacts: after
detecting errors from io.Copy or out.Sync (and before returning), close the file
handle (out.Close() if not already deferred/closed) and call os.Remove(savePath)
to delete the incomplete file; keep or adjust the existing defer out.Close() so
it doesn't conflict with explicit close-on-error, and perform Remove only on
error paths that occur after successful os.Create.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6975af41-9f34-4d83-9dcf-3b2778bf70b1
📒 Files selected for processing (1)
handler.go
| } | ||
| defer in.Close() | ||
|
|
||
| out, err := os.Create(savePath) |
| } | ||
| if err := tmpFile.Close(); err != nil { | ||
| http.Error(w, "Failed to close temp file: "+err.Error(), http.StatusInternalServerError) | ||
| tmpFile.Close() |
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
| } | ||
| defer in.Close() | ||
|
|
||
| out, err := os.Create(savePath) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolved #18