fix: route share.geolibre.app through native HTTP on desktop - #1109
Conversation
The Share action and project gallery use the WebView fetch, which the share server's CORS policy blocks from the Tauri origin, so desktop users hit "Could not reach share.geolibre.app." Route the share host through the native HTTP client (as geocoding already does) to bypass WebView CORS.
✅ Deploy Preview for geolibre-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds a ChangesNative share fetch integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MainTsx as main.tsx
participant ShareFetch as share-fetch.ts
participant TauriHttp as "Tauri plugin-http"
participant Browser as "browser fetch"
participant ShareApp as "share.geolibre.app"
MainTsx->>ShareFetch: installNativeShareFetch() (if isTauri)
ShareFetch->>ShareFetch: resolveShareBaseUrl()
ShareFetch->>TauriHttp: dynamic import plugin-http
ShareFetch->>ShareFetch: setShareFetch(wrapped fetch)
Note over ShareFetch: request made via uploadProjectToShare / fetchSharedProjects / fetchMyProjects
ShareFetch->>ShareFetch: requestHost(request)
alt host matches share host
ShareFetch->>TauriHttp: native fetch(request)
TauriHttp->>ShareApp: HTTP request (bypasses WebView CORS)
else other host
ShareFetch->>Browser: fetch(request)
end
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
⚡ Cloudflare Pages preview
|
There was a problem hiding this comment.
Pull request overview
This PR fixes the desktop (Tauri) Share + gallery CORS failure by routing requests to share.geolibre.app through Tauri’s native HTTP client (@tauri-apps/plugin-http) instead of the WebView fetch, while keeping the default web behavior unchanged.
Changes:
- Added a centralized, overridable share fetch layer (
lib/share-fetch.ts) with a desktop-only installer that swaps in the native HTTP client only for the resolved share host. - Updated the share upload and gallery listing clients to use the centralized share fetch by default (
getShareFetch()), ensuring the desktop override is actually exercised. - Expanded the Tauri HTTP capability scope to allow
https://share.geolibre.app/*, and added new unit tests covering override/reset and routing through the installed share fetch.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/share-fetch.test.ts | Adds tests ensuring share upload and gallery calls go through the installed share fetch (and that auth is preserved). |
| apps/geolibre-desktop/src/main.tsx | Desktop-only lazy import to install the native share fetch at startup, mirroring the geocoding native-fetch pattern. |
| apps/geolibre-desktop/src/lib/share-geolibre.ts | Routes Share upload through getShareFetch() by default (instead of direct browser fetch). |
| apps/geolibre-desktop/src/lib/share-gallery.ts | Routes gallery reads (shared + my projects) through getShareFetch() by default, preserving same-origin token gating. |
| apps/geolibre-desktop/src/lib/share-fetch.ts | Introduces the centralized share fetch + native installer with host scoping to limit native HTTP usage to the share host. |
| apps/geolibre-desktop/src-tauri/capabilities/default.json | Adds https://share.geolibre.app/* to the http:default allowlist and updates the rationale comment accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * The host is resolved from {@link resolveShareBaseUrl} (the configured or | ||
| * production share URL) at install time, so a `VITE_GEOLIBRE_SHARE_URL` override | ||
| * is honored. | ||
| * | ||
| * Loaded lazily and only in the desktop build so the web/embedded bundles never | ||
| * pull in `@tauri-apps/plugin-http`. | ||
| */ | ||
| export async function installNativeShareFetch(): Promise<void> { | ||
| let shareHost: string | null; | ||
| try { | ||
| shareHost = new URL(resolveShareBaseUrl()).host; |
There was a problem hiding this comment.
The routed host is derived from resolveShareBaseUrl(), which honors the runtime-configurable VITE_GEOLIBRE_SHARE_URL env var (including http://localhost/127.0.0.1 overrides for local dev). But the http:default capability scope in default.json hard-codes only https://share.geolibre.app/*. If that env var is ever set to anything else, shareHost will match a host that Tauri's native HTTP plugin isn't permitted to reach, so tauriFetch(...) will fail with a permission error — a regression versus today's behavior, where a plain browser fetch would at least attempt the request (subject to the dev server's own CORS config). The doc comment's claim that the override "is honored" is misleading in the desktop build; consider scoping native routing to the production host only (mirroring how geocoding-fetch.ts fixes NATIVE_FETCH_HOSTS to the known provider list) or noting that overriding the share URL for a Tauri build also requires updating the capability file.
Confidence: medium — this only manifests for non-default builds (self-hosted/dev share servers), which may not be a supported/exercised configuration today.
| /** The request URL's host, or null when it cannot be parsed. */ | ||
| function requestHost(input: RequestInfo | URL): string | null { |
There was a problem hiding this comment.
Minor duplication: requestHost is byte-for-byte identical to the helper of the same name in geocoding-fetch.ts. Not a functional issue, but if a third native-fetch host gets added later this logic will likely be copy-pasted a third time — could be worth hoisting into a small shared util alongside the two lazily-installed fetch overrides.
Confidence: low — pure style nit.
| @@ -0,0 +1,115 @@ | |||
| import assert from "node:assert/strict"; | |||
There was a problem hiding this comment.
Coverage gap: these tests cover getShareFetch/setShareFetch/resetShareFetch and that the share client functions call getShareFetch(), but none exercise installNativeShareFetch() itself — the host-matching logic that decides whether a request goes through tauriFetch or falls back to the browser fetch (share-fetch.ts:76-83). A regression there (e.g. a host mismatch due to a resolveShareBaseUrl() change) wouldn't be caught by this suite. (Note: installNativeGeocodingFetch has the same gap already, so this isn't a new pattern — just flagging since it's easy to miss.)
Confidence: low-medium — not blocking, but the core new CORS-bypass logic is currently untested.
Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/geolibre-desktop/src/lib/share-gallery.ts (1)
68-80: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale doc comment:
fetchImplno longer defaults to the global fetch.
FetchSharedProjectsOptions.fetchImpl's doc still says "defaults to the global fetch," but Line 196 now defaults togetShareFetch(). The siblingShareUploadOptions.fetchImpldoc inshare-geolibre.tswas correctly updated in this same PR to reflect this.📝 Proposed fix
- /** Injected for testing; defaults to the global fetch. */ + /** Injected for testing; defaults to the share fetch (see share-fetch.ts). */ fetchImpl?: typeof fetch;🤖 Prompt for 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. In `@apps/geolibre-desktop/src/lib/share-gallery.ts` around lines 68 - 80, Update the stale doc comment for FetchSharedProjectsOptions.fetchImpl in share-gallery.ts so it matches the actual behavior in fetchSharedProjects and no longer says it defaults to the global fetch; change it to reflect the getShareFetch() default used when no fetchImpl is provided, consistent with ShareUploadOptions.fetchImpl in share-geolibre.ts.
🤖 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.
Outside diff comments:
In `@apps/geolibre-desktop/src/lib/share-gallery.ts`:
- Around line 68-80: Update the stale doc comment for
FetchSharedProjectsOptions.fetchImpl in share-gallery.ts so it matches the
actual behavior in fetchSharedProjects and no longer says it defaults to the
global fetch; change it to reflect the getShareFetch() default used when no
fetchImpl is provided, consistent with ShareUploadOptions.fetchImpl in
share-geolibre.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 16c12967-e00d-49bf-8960-1eb8fcb21559
📒 Files selected for processing (6)
apps/geolibre-desktop/src-tauri/capabilities/default.jsonapps/geolibre-desktop/src/lib/share-fetch.tsapps/geolibre-desktop/src/lib/share-gallery.tsapps/geolibre-desktop/src/lib/share-geolibre.tsapps/geolibre-desktop/src/main.tsxtests/share-fetch.test.ts
Summary
fetch, and the share server's CORS policy allows the web origin but not the Tauri WebView origin, so the cross-origin request throws.@tauri-apps/plugin-http), which is not bound by CORS, exactly as the existing geocoding fix does. Added a centralized, overridable share fetch (lib/share-fetch.ts);installNativeShareFetch()runs desktop-only and lazily, so web and embedded bundles never import the plugin.https://share.geolibre.app/*to thehttp:defaultcapability scope.Test plan
tests/share-fetch.test.ts(new): default/override/reset of the share fetch, and that upload plus both gallery reads route through the installed fetch (with bearer token) - 43 share tests passfetch; only the desktop build swaps in the native pathSummary by CodeRabbit
New Features
Bug Fixes