Skip to content

fix(images): make "Save image as" work for local images and refuse honestly for remote ones - #408

Merged
PathGao merged 1 commit into
fix/editor-mode-pdf-exportfrom
fix/save-image-as-asset-urls
Aug 3, 2026
Merged

fix(images): make "Save image as" work for local images and refuse honestly for remote ones#408
PathGao merged 1 commit into
fix/editor-mode-pdf-exportfrom
fix/save-image-as-asset-urls

Conversation

@PathGao

@PathGao PathGao commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

4 / 5 of a stack. Base: #407.

The defect

The remote branch fetched the bytes with fetch(src) — and connect-src 'self' refuses every cross-origin fetch, so that branch was unreachable on every platform, not merely failing. (img-src allowing https: only affects <img> loading, not fetch.)

Windows fell into that same unreachable branch for local images too: the local test was src.startsWith('asset:'), while Windows asset URLs are http://asset.localhost/….

The fix reuses #363

normalizeAssetPath handles both URL shapes and — the part a hand-rolled prefix test gets wrong — rejects http://asset.localhost.evil.test/C:/secret.png, which startsWith('http://asset.localhost') accepts and would have handed to copy_file. That hole is real in the original implementation; the test asserts it.

The dead fetch path is deleted. The remote case now bails before opening a save dialog it cannot honour, and says why, rather than leaving code that reads like a feature. Supporting it needs a download command in Rust — out of scope here.

save_file_binary is now unreferenced from the frontend; left registered in Rust.

Tests

scripts/saveImageAsAssetUrl.test.ts, including a pin on "connect-src": "'self'" so the explanation in the source cannot go stale silently.

vs #407 3 red / 3 green → 6 / 6
npm run check   0 errors
npm test        458 / 458
cargo test      131 / 131

Not covered

  • data: image sources — decodable in pure JS, but scope creep.
  • The context-menu item is still offered for remote images; it now explains itself instead of being hidden.

🤖 Generated with Claude Code

@PathGao
PathGao force-pushed the fix/save-image-as-asset-urls branch 2 times, most recently from b3c457d to 29a7417 Compare August 2, 2026 23:11
…nestly for remote ones

The remote branch fetched the image bytes with `fetch(src)`, which
`connect-src 'self'` refuses - so it was unreachable on every platform,
not merely failing. Windows fell into that same unreachable branch for
*local* images too, because the local test was `src.startsWith('asset:')`
while Windows asset URLs are `http://asset.localhost/...`.

Reuses `normalizeAssetPath` from #363 rather than another hand-rolled
prefix test. It handles both URL shapes and rejects
`http://asset.localhost.evil.test/...`, which a `startsWith` check accepts
and would have handed to `copy_file`.

The dead fetch path is deleted. The remote case now bails before opening a
save dialog it cannot honour, and says why, instead of leaving code that
reads like a feature. Supporting it needs a download command in Rust.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@PathGao
PathGao force-pushed the fix/save-image-as-asset-urls branch from 29a7417 to 03d3c24 Compare August 2, 2026 23:13
@PathGao
PathGao merged commit ddba699 into master Aug 3, 2026
4 checks passed
@PathGao
PathGao deleted the fix/save-image-as-asset-urls branch August 3, 2026 02:05
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