feat(share): point the app at a self-hosted share/collab server at runtime - #1687
Conversation
…ntime
The web container could not be pointed at a self-hosted sharing server or
collaboration relay. `resolveShareBaseUrl()` and `resolveCollabBaseUrl()` already
honored `VITE_GEOLIBRE_SHARE_URL` / `VITE_GEOLIBRE_COLLAB_URL`, but both read
`import.meta.env` (build time only) and neither variable reached `Dockerfile` or
`docker/entrypoint.sh` — so repointing a published image meant forking it.
The entrypoint already writes `geolibre-runtime-config.js` on every boot for the
AI proxy and embed origins, so this wires both hosts through that same channel:
- `lib/deployment-env.ts` reads one `VITE_*` value, deployment env before build
env, matching the precedence `readEmbedOrigins`/`readDeploymentAssistantEnv`
already use.
- `resolveShareHost()` returns a status (`default` / `configured` / `disabled` /
`invalid`) plus the base URL. `GEOLIBRE_SHARE_URL=off` removes Share and the
Project Gallery entirely.
- `resolveCollabBaseUrl()` reads the deployment env too.
- The entrypoint validates both and exits on a malformed value, as the
`GEOLIBRE_EMBED_ORIGINS` block already does.
A rejected value no longer falls back to `share.geolibre.app`. Previously a
self-hosted deployment with a typo'd or plaintext host silently uploaded its
users' projects to the public hosted service; `resolveShareBaseUrl()` now returns
null, the menu entries disable with a reason, and the gallery reports a new
`not-configured` error instead.
The hostname in UI copy is also no longer hardcoded: 11 keys across all 16
catalogues take a `{{shareHost}}` interpolation, and the account-settings link in
SettingsDialog/ShareProjectDialog derives from the resolved host, so a
self-hosted instance no longer names or links to share.geolibre.app.
Verified in the built web app against a preview server with the runtime config
set four ways: unset (unchanged — Share enabled, copy names share.geolibre.app),
`off` (Share and Gallery absent), an invalid host (both present but disabled with
the reason), and `https://maps.example.org` (copy and token link both name the
self-hosted host). Also exercised the entrypoint's validation across 12 inputs.
Desktop note: the Tauri `http:default` capability scope still pins the share
host, so self-hosting remains a web/Docker capability for now.
Refs #1684, #1665
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds runtime and build-time configuration for self-hosted sharing and collaboration services. It validates service URLs, resolves deployment-specific hosts, updates sharing behavior and UI, and replaces fixed sharing-host text across supported locales. ChangesSelf-hosted service configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Admin
participant Docker
participant Frontend
participant ShareService
Admin->>Docker: configure GEOLIBRE_SHARE_URL
Docker->>Docker: validate and write runtime configuration
Frontend->>Frontend: resolve share host status
Frontend->>ShareService: upload or fetch using configured host
ShareService-->>Frontend: return sharing response
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 PR preview
|
There was a problem hiding this comment.
Pull request overview
Implements runtime (container-startup) configuration for GeoLibre’s Share and Collaboration hosts in the Docker/web deployment, eliminating the prior silent fallback to the public hosted share service on misconfiguration and enabling GEOLIBRE_SHARE_URL=off to remove sharing UI entirely.
Changes:
- Adds a shared deployment-env reader (
window.__GEOLIBRE_DEPLOYMENT_ENV__precedence overimport.meta.env) and wires Share/Collab URL resolution through it. - Reworks share host resolution to return a status (
default/configured/disabled/invalid) and makes callers handlestring | nullfor share base URLs. - Updates Docker entrypoint + docs + UI + tests to validate, surface, and explain disabled/invalid configurations; updates i18n strings to interpolate
{{shareHost}}.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/share-geolibre.test.ts | Updates share URL tests for “refuse instead of fallback”, adds resolveShareHost coverage and deployment-env precedence assertions. |
| tests/share-gallery.test.ts | Adds tests ensuring gallery throws not-configured when sharing is disabled/invalid in deployment env. |
| tests/deployment-env.test.ts | New unit tests for deployment-env precedence and blank-value handling. |
| tests/collab-protocol.test.ts | Adds tests for collab URL deployment-env precedence and validation. |
| docs/getting-started.md | Documents GEOLIBRE_SHARE_URL / GEOLIBRE_COLLAB_URL runtime configuration and TLS policy. |
| docs/collaboration.md | Documents Docker runtime configuration path for the collab relay and failure behavior. |
| Dockerfile | Adds documented ARG/ENV for VITE_GEOLIBRE_SHARE_URL and VITE_GEOLIBRE_COLLAB_URL. |
| docker/entrypoint.sh | Validates share/collab URLs at boot, writes them into runtime config, and logs chosen endpoints. |
| apps/geolibre-desktop/src/lib/share-geolibre.ts | Introduces resolveShareHost + status, makes resolveShareBaseUrl return `string |
| apps/geolibre-desktop/src/lib/share-gallery.ts | Throws GalleryError("not-configured") when share base URL is unavailable, preventing unintended hosted listing usage. |
| apps/geolibre-desktop/src/lib/share-fetch.ts | Skips installing native fetch override when share base URL resolves to null. |
| apps/geolibre-desktop/src/lib/deployment-env.ts | New helper to read deployment env from window and prefer it over build-time env. |
| apps/geolibre-desktop/src/lib/collab-client.ts | Reads collab base URL from deployment env first; keeps validation and returns null on invalid/unset. |
| apps/geolibre-desktop/src/hooks/useProjectFileActions.ts | Avoids attaching share token auth when no share base URL is configured. |
| apps/geolibre-desktop/src/components/layout/TopToolbar.tsx | Resolves share host once per render and propagates share availability/status to menus/commands. |
| apps/geolibre-desktop/src/components/layout/toolbar/ProjectMenu.tsx | Hides Share/Gallery when disabled; disables with reason when invalid. |
| apps/geolibre-desktop/src/components/layout/ShareProjectDialog.tsx | Derives account settings link from resolved share base URL and interpolates shareHost in copy. |
| apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx | Derives token help link from resolved share host and interpolates shareHost in copy. |
| apps/geolibre-desktop/src/components/layout/ProjectGalleryDialog.tsx | Interpolates shareHost in gallery copy/error messages and maps not-configured to a dedicated string. |
| apps/geolibre-desktop/src/i18n/locales/en.json | Switches Share/Gallery copy to {{shareHost}} interpolation; adds new error/tooltip strings. |
| apps/geolibre-desktop/src/i18n/locales/ar.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/de.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/es.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/fr.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/hi.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/id.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/it.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/ja.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/ka.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/ko.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/nl.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/pt.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/ru.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/tr.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
| apps/geolibre-desktop/src/i18n/locales/zh.json | Switches Share/Gallery copy to {{shareHost}} interpolation. |
Suppressed comments (1)
apps/geolibre-desktop/src/lib/collab-client.ts:66
resolveCollabBaseUrlacceptsws(s)://URLs with embedded credentials (userinfo). Even if uncommon for WebSocket URLs, it should be rejected for consistency with the container entrypoint validator and to avoid accidentally enabling Basic Auth / leaking secrets via configuration.
const value =
configured !== undefined ? configured : readDeploymentEnvValue(COLLAB_URL_ENV, deploymentEnv);
if (typeof value !== "string" || !value.trim()) return null;
const trimmed = value.trim().replace(/\/+$/, "");
try {
const url = new URL(trimmed);
if (
url.protocol === "wss:" ||
(url.protocol === "ws:" &&
(url.hostname === "localhost" || url.hostname === "127.0.0.1" || url.hostname === "[::1]"))
) {
return trimmed;
}
} catch {
// Invalid URL; treat as unconfigured.
}
return null;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // deployment env does not change while the app is running) and passed down so | ||
| // the menu, the command palette, and the dialogs agree. | ||
| const shareHost = resolveShareHost(); | ||
| const shareAvailable = shareHost.baseUrl != null; |
There was a problem hiding this comment.
Minor UX inconsistency: when shareHostStatus === "invalid", ProjectMenu keeps the Share/Gallery items visible but disabled with a reason (shareBrokenReason) so the misconfiguration is discoverable. Here, shareAvailable (baseUrl != null) is false for both disabled and invalid, so the command-palette entry is silently omitted in the invalid case too — a user relying on the command palette gets no hint that sharing is broken vs. simply off. Not a functional bug, just a discoverability gap between the two entry points. Confidence: low.
There was a problem hiding this comment.
Not fixing this one — leaving the thread open for a maintainer call.
The gap is real, but the Command type in lib/commands.ts has no disabled state: it is { id, title, group, keywords?, shortcut?, icon?, run }. Surfacing a broken-config hint in the palette means adding one and teaching both the palette renderer and the global shortcut handler to honour it, which is a lot of surface for a state an operator hits once at deploy time — and one the container now refuses to boot into at all, since the entrypoint exits on a malformed GEOLIBRE_SHARE_URL. The only way to reach invalid in practice is a bad build-time var or a hand-edited runtime config.
The menu keeps the discoverable version (visible reason + aria-describedby, tightened in 8a879a4), so the hint exists in the UI. Happy to add disabled to Command if you'd rather have parity.
|
All five inline comments posted successfully. Code reviewBugs
Security
Quality
CLAUDE.md
Overall the core logic ( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
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-geolibre.ts (1)
255-264: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the resolved share host in these error messages, not the hardcoded hosted-service name.
Line 219 in this same function was reworded to drop a hardcoded host reference, but Line 263 and Line 274 still hardcode
share.geolibre.app. For a self-hosted deployment, a network failure or malformed response reports the wrong server name to the user, which contradicts the goal of dynamically naming the configured host throughout the UI.shareHostLabel()in this file already derives the correct host string; reuse it (orbase) here.🛠️ Proposed fix to name the actual configured host
} catch (error) { if (error instanceof DOMException) { // Caller-initiated abort (dialog closed): propagate so the UI ignores it. if (error.name === "AbortError") throw error; if (error.name === "TimeoutError") { throw new Error("Upload timed out. Please try again."); } } - throw new Error("Could not reach share.geolibre.app. Check your internet connection."); + throw new Error(`Could not reach ${shareHostLabel()}. Check your internet connection.`); } if (!response.ok) { const { message, code } = await uploadErrorInfo(response); throw new ShareUploadError(message, code); } const payload = (await response.json().catch(() => ({}))) as ShareProjectResponse; const project = payload.project; if (!project?.projectUrl || !project.rawJsonUrl) { - throw new Error("share.geolibre.app returned an unexpected response."); + throw new Error(`${shareHostLabel()} returned an unexpected response.`); }Also applies to: 271-275
🤖 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-geolibre.ts` around lines 255 - 264, Update the error messages in the catch handling around the share upload function, including the malformed-response path near lines 271–275, to use the resolved host from shareHostLabel() or base instead of the hardcoded share.geolibre.app name. Preserve the existing timeout and abort behavior while ensuring network and response errors identify the configured server.
🤖 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 `@apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx`:
- Around line 382-388: Hide the share-token guidance and input in
apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx at lines 382-388
when shareBaseUrl is null, or show an unavailable-state message without using
shareHostLabel(). In
apps/geolibre-desktop/src/components/layout/ShareProjectDialog.tsx at lines
62-66, guard the dialog with the same unavailable state and do not render setup
or upload guidance based on shareHostLabel() when the resolved share URL is
null.
In `@apps/geolibre-desktop/src/components/layout/toolbar/ProjectMenu.tsx`:
- Around line 156-165: Make the invalid-host reason accessible for both the
Gallery item at
apps/geolibre-desktop/src/components/layout/toolbar/ProjectMenu.tsx:156-165 and
the Share item at
apps/geolibre-desktop/src/components/layout/toolbar/ProjectMenu.tsx:279-280 by
wrapping each disabled DropdownMenuItem in an enabled tooltip trigger or adding
an equivalent accessible explanatory element; do not rely on the native title
attribute.
- Around line 102-106: Update the showSaveGroup calculation to count the Share
item only when shareHidden is false, so a group containing only disabled sharing
is not rendered without items. Preserve the existing group visibility behavior
for the other save-related items and use the shareHidden symbol defined near
shareBroken.
In `@apps/geolibre-desktop/src/hooks/useProjectFileActions.ts`:
- Around line 521-525: In the project-loading flow around fetchProjectFromUrl
and loadProject, derive a single usesShareAuth flag from the effective
shareBaseUrl, auth token, and the same-origin validation used by
shareAuthorizedFetch. Use this flag both to select shareAuthorizedFetch versus
the unauthenticated fetch and to determine the path stored by loadProject,
replacing the independent options.authToken check so recent-project persistence
matches the actual authentication decision.
In `@apps/geolibre-desktop/src/lib/share-fetch.ts`:
- Around line 57-73: Update installNativeShareFetch to derive the native-fetch
host guard from the configured URL.origin rather than only URL.host, preserving
HTTPS origin boundaries when deciding whether to intercept requests. Add
coverage for an HTTPS base URL receiving an HTTP request and verify that request
remains on the browser fetch path.
---
Outside diff comments:
In `@apps/geolibre-desktop/src/lib/share-geolibre.ts`:
- Around line 255-264: Update the error messages in the catch handling around
the share upload function, including the malformed-response path near lines
271–275, to use the resolved host from shareHostLabel() or base instead of the
hardcoded share.geolibre.app name. Preserve the existing timeout and abort
behavior while ensuring network and response errors identify the configured
server.
🪄 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: d960b90d-ed10-49ac-a004-bc26925f09fd
📒 Files selected for processing (35)
Dockerfileapps/geolibre-desktop/src/components/layout/ProjectGalleryDialog.tsxapps/geolibre-desktop/src/components/layout/SettingsDialog.tsxapps/geolibre-desktop/src/components/layout/ShareProjectDialog.tsxapps/geolibre-desktop/src/components/layout/TopToolbar.tsxapps/geolibre-desktop/src/components/layout/toolbar/ProjectMenu.tsxapps/geolibre-desktop/src/hooks/useProjectFileActions.tsapps/geolibre-desktop/src/i18n/locales/ar.jsonapps/geolibre-desktop/src/i18n/locales/de.jsonapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/i18n/locales/es.jsonapps/geolibre-desktop/src/i18n/locales/fr.jsonapps/geolibre-desktop/src/i18n/locales/hi.jsonapps/geolibre-desktop/src/i18n/locales/id.jsonapps/geolibre-desktop/src/i18n/locales/it.jsonapps/geolibre-desktop/src/i18n/locales/ja.jsonapps/geolibre-desktop/src/i18n/locales/ka.jsonapps/geolibre-desktop/src/i18n/locales/ko.jsonapps/geolibre-desktop/src/i18n/locales/nl.jsonapps/geolibre-desktop/src/i18n/locales/pt.jsonapps/geolibre-desktop/src/i18n/locales/ru.jsonapps/geolibre-desktop/src/i18n/locales/tr.jsonapps/geolibre-desktop/src/i18n/locales/zh.jsonapps/geolibre-desktop/src/lib/collab-client.tsapps/geolibre-desktop/src/lib/deployment-env.tsapps/geolibre-desktop/src/lib/share-fetch.tsapps/geolibre-desktop/src/lib/share-gallery.tsapps/geolibre-desktop/src/lib/share-geolibre.tsdocker/entrypoint.shdocs/collaboration.mddocs/getting-started.mdtests/collab-protocol.test.tstests/deployment-env.test.tstests/share-gallery.test.tstests/share-geolibre.test.ts
🔍 GitHub Pages PR preview
|
|
All four findings posted. Final summary below. Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
- share-geolibre.ts / collab-client.ts: reject URLs with embedded credentials in `isSafeShareUrl` and `resolveCollabBaseUrl`, so the client matches `service_url()` in the entrypoint. A `https://user:pass@host` base would send Basic Auth alongside the Bearer token (Copilot, Claude). - docker/entrypoint.sh: move the credential check ahead of the loopback early-return, which previously let `http://user:pass@localhost` through despite the docstring — and it is echoed to the boot log a few lines down (Claude, x2). - docker/entrypoint.sh: make the disabled-sharing boot log case-insensitive (`[oO][fF][fF]`) to match the Python validator and `resolveShareHost`, both of which lowercase before comparing (Claude). - share-geolibre.ts: the network-failure and unexpected-response upload errors still named share.geolibre.app; they now name the resolved host via a new `hostOf` helper that `shareHostLabel` also uses (Claude). - share-fetch.ts: scope the CORS-exempt native fetch by `URL.origin` rather than `URL.host`, so a plaintext request to a host configured over HTTPS is not routed through it. `requestOrigin` is exported and unit-tested (CodeRabbit). - useProjectFileActions.ts: derive one `shareAuth` value for both the fetch choice and whether the URL is remembered as recent. A token set on a deployment with no share host previously loaded unauthenticated yet was still not remembered (CodeRabbit). - ProjectMenu.tsx: count Share in `showSaveGroup` only when it is not hidden, so a profile showing only Share cannot leave an orphaned separator (CodeRabbit). - ProjectMenu.tsx: a disabled DropdownMenuItem carries `pointer-events-none`, so the native `title` holding the invalid-host reason could never be hovered. The reason is now a rendered line, referenced by `aria-describedby` with a distinct id per item (CodeRabbit). - SettingsDialog.tsx: hide the share-token description, input, and storage note when no share host is usable, replacing them with `settings.env.tokenUnavailable`. A deployment with `GEOLIBRE_SHARE_URL=off` was still telling users to get a token from the public hosted service (Claude, CodeRabbit). - ShareProjectDialog.tsx: guard the dialog on the same state defensively, so a future caller cannot render host-bearing setup guidance with no host (CodeRabbit). - Locales: add `gallery.errorNotConfigured`, `toolbar.item.shareHostUnavailable`, and the new `settings.env.tokenUnavailable` to all 15 non-English catalogues, translated and inserted in en.json key order (Copilot x2, Claude x2). Verified: the entrypoint validator now rejects credentials on the loopback path too (9 inputs re-checked), the boot log treats any casing of "off" as disabled, and in the built app an invalid host renders the reason as visible text with `aria-describedby` wired up while the Settings token input is replaced by the unavailable message. 5026 frontend tests pass; lint has 0 errors.
Code reviewReviewed the full diff: the new Bugs: None found. The client-side ( Security: None found. Credentials-in-URL are rejected on both the client and the entrypoint for both share and collab URLs; HTTPS/WSS-except-loopback is enforced consistently; the "no silent fallback to the public host" behavior (the PR's core stated goal) is correctly implemented and tested ( Performance: No issues; all changes are cheap, render-time string/URL parsing. Quality:
CLAUDE.md: No violations — new user-facing strings use |
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 (2)
docker/entrypoint.sh (2)
196-224: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject whitespace-only deployment values instead of treating them as unset.
share_urlandcollab_urlare trimmed before their presence checks. ForGEOLIBRE_SHARE_URL=" ", no runtime key is emitted, so a build-time value can become active again. This can route sharing to the public service instead of rejecting the deployment value. The shell log also checks the untrimmed value, so" off "is disabled by Python but reported as a configured server.Track environment-variable presence separately. Reject non-empty values that become empty after trimming. Use the normalized value for the startup message.
Suggested fix
- share_url = os.environ.get("GEOLIBRE_SHARE_URL", "").strip() - if share_url: + raw_share_url = os.environ.get("GEOLIBRE_SHARE_URL") + share_url = raw_share_url.strip() if raw_share_url is not None else "" + if raw_share_url not in (None, "") and not share_url: + raise SystemExit("ERROR: GEOLIBRE_SHARE_URL must not be whitespace-only.") + if share_url: ... - collab_url = os.environ.get("GEOLIBRE_COLLAB_URL", "").strip() - if collab_url: + raw_collab_url = os.environ.get("GEOLIBRE_COLLAB_URL") + collab_url = raw_collab_url.strip() if raw_collab_url is not None else "" + if raw_collab_url not in (None, "") and not collab_url: + raise SystemExit("ERROR: GEOLIBRE_COLLAB_URL must not be whitespace-only.") + if collab_url: ... +share_url_log=$(printf '%s' "${GEOLIBRE_SHARE_URL:-}" | + sed 's/^[[:space:]]*//; s/[[:space:]]*$//') - case "$GEOLIBRE_SHARE_URL" in + case "$share_url_log" in ... - *) echo "Project sharing server: $GEOLIBRE_SHARE_URL" ;; + *) echo "Project sharing server: $share_url_log" ;;🤖 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 `@docker/entrypoint.sh` around lines 196 - 224, Update the Python deployment-config logic for share_url and collab_url to distinguish an unset variable from a present-but-whitespace-only value, rejecting the latter instead of omitting its runtime key. Preserve normalized trimmed values for valid configuration, and update the GEOLIBRE_SHARE_URL startup logging to use the same trimmed value so values such as “ off ” are reported as disabled consistently.
169-190: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate the service URL hostname and explicit port before accepting it.
urlsplit()acceptshttps://:443with no hostname andhttps://collab.example:badwith an invalid port. These values can passservice_url()and reach configuration as a malformed service URL, or disable self-hosted features when the app rejects them.Suggested fix
- parsed = urlsplit(value) + try: + parsed = urlsplit(value) + hostname = parsed.hostname + parsed.port # Force validation of an explicit port. + except ValueError: + raise SystemExit(f"ERROR: {name} must be a valid URL, not {value!r}.") if parsed.username or parsed.password: raise SystemExit(f"ERROR: {name} must not embed credentials.") - if parsed.scheme in loopback_schemes and parsed.hostname in loopback_hosts: + if parsed.scheme in loopback_schemes and hostname in loopback_hosts: return value - if parsed.scheme not in schemes or not parsed.netloc: + if parsed.scheme not in schemes or not parsed.netloc or not hostname:🤖 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 `@docker/entrypoint.sh` around lines 169 - 190, Update service_url to validate parsed.hostname and the explicit port before accepting either the loopback shortcut or the general URL path. Reject URLs with a missing hostname or an invalid/out-of-range port, while preserving the existing credential, scheme, netloc, and loopback validation behavior.
🤖 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 `@docker/entrypoint.sh`:
- Around line 196-224: Update the Python deployment-config logic for share_url
and collab_url to distinguish an unset variable from a
present-but-whitespace-only value, rejecting the latter instead of omitting its
runtime key. Preserve normalized trimmed values for valid configuration, and
update the GEOLIBRE_SHARE_URL startup logging to use the same trimmed value so
values such as “ off ” are reported as disabled consistently.
- Around line 169-190: Update service_url to validate parsed.hostname and the
explicit port before accepting either the loopback shortcut or the general URL
path. Reject URLs with a missing hostname or an invalid/out-of-range port, while
preserving the existing credential, scheme, netloc, and loopback validation
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 20bcd4c5-a9f4-4755-88f0-cca636f84bbb
📒 Files selected for processing (27)
apps/geolibre-desktop/src/components/layout/SettingsDialog.tsxapps/geolibre-desktop/src/components/layout/ShareProjectDialog.tsxapps/geolibre-desktop/src/components/layout/toolbar/ProjectMenu.tsxapps/geolibre-desktop/src/hooks/useProjectFileActions.tsapps/geolibre-desktop/src/i18n/locales/ar.jsonapps/geolibre-desktop/src/i18n/locales/de.jsonapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/i18n/locales/es.jsonapps/geolibre-desktop/src/i18n/locales/fr.jsonapps/geolibre-desktop/src/i18n/locales/hi.jsonapps/geolibre-desktop/src/i18n/locales/id.jsonapps/geolibre-desktop/src/i18n/locales/it.jsonapps/geolibre-desktop/src/i18n/locales/ja.jsonapps/geolibre-desktop/src/i18n/locales/ka.jsonapps/geolibre-desktop/src/i18n/locales/ko.jsonapps/geolibre-desktop/src/i18n/locales/nl.jsonapps/geolibre-desktop/src/i18n/locales/pt.jsonapps/geolibre-desktop/src/i18n/locales/ru.jsonapps/geolibre-desktop/src/i18n/locales/tr.jsonapps/geolibre-desktop/src/i18n/locales/zh.jsonapps/geolibre-desktop/src/lib/collab-client.tsapps/geolibre-desktop/src/lib/share-fetch.tsapps/geolibre-desktop/src/lib/share-geolibre.tsdocker/entrypoint.shtests/collab-protocol.test.tstests/share-fetch.test.tstests/share-geolibre.test.ts
- SettingsDialog.tsx: distinguish the two unusable share-host states. `disabled` keeps "no project sharing server configured"; `invalid` now gets a new `settings.env.tokenHostInvalid` string saying the configured address is not valid, so an operator who typo'd GEOLIBRE_SHARE_URL is not told they forgot to set it. Switched the component to `resolveShareHost()` so it has the status, and translated the new key into all 15 non-English catalogues. Chose a settings-namespaced key over reusing `toolbar.item.shareHostUnavailable` so a later edit to the menu string cannot silently change the Settings copy.
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 `@apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx`:
- Around line 386-389: Align the settings link with the displayed host by
updating resolveShareHost to reject safe base URLs containing a path, preserving
only host-root URLs for shareBaseUrl and shareHostLabel; alternatively, make the
displayed host use the resolved settings URL rather than appending /settings to
a path-bearing base URL.
🪄 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: d9edea98-7445-42f1-86d0-5643f2e5fcfe
📒 Files selected for processing (17)
apps/geolibre-desktop/src/components/layout/SettingsDialog.tsxapps/geolibre-desktop/src/i18n/locales/ar.jsonapps/geolibre-desktop/src/i18n/locales/de.jsonapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/i18n/locales/es.jsonapps/geolibre-desktop/src/i18n/locales/fr.jsonapps/geolibre-desktop/src/i18n/locales/hi.jsonapps/geolibre-desktop/src/i18n/locales/id.jsonapps/geolibre-desktop/src/i18n/locales/it.jsonapps/geolibre-desktop/src/i18n/locales/ja.jsonapps/geolibre-desktop/src/i18n/locales/ka.jsonapps/geolibre-desktop/src/i18n/locales/ko.jsonapps/geolibre-desktop/src/i18n/locales/nl.jsonapps/geolibre-desktop/src/i18n/locales/pt.jsonapps/geolibre-desktop/src/i18n/locales/ru.jsonapps/geolibre-desktop/src/i18n/locales/tr.jsonapps/geolibre-desktop/src/i18n/locales/zh.json
Code reviewBugs: None found. The Security: None found. Performance: No concerns; this is UI/config plumbing with no hot paths touched. Quality:
CLAUDE.md adherence: Consistent with repo conventions — i18n strings use |
- share-geolibre.ts: `hostOf` now keeps a base URL's path, so a server hosted under a subpath (`https://example.test/geolibre`) is named the way the links built from the same base resolve. Previously the copy read `example.test` beside a link to `https://example.test/geolibre/settings` (CodeRabbit). Rejecting paths outright was the other option offered, but that would refuse a legitimate subpath deployment. - ShareProjectDialog.tsx: drop the two `settingsUrl &&` checks, dead since the early guard added in 8a879a4 makes it non-null past that point (Claude). - docker/entrypoint.sh: trim the logged values the way the Python block trims the validated ones, so `" off "` logs as disabled instead of printing a "sharing server" line that disagrees with the config actually written (Claude). Added `shareHostLabel` tests covering the subpath case (with and without a trailing slash) and the unusable-host fallback. Re-checked the boot log across `off`/`" off "`/`oFF`/a padded URL, and `sh -n`/`bash -n` on the entrypoint.
Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
- docker/nginx.conf + entrypoint.sh: allow a self-hosted collaboration relay in the CSP. `connect-src` has a bare `https:` (so any share host works) but no bare `wss:` — only `wss://collab.geolibre.app` was listed. A container started with `-e GEOLIBRE_COLLAB_URL=wss://relay.example.org` (the example this PR added to the docs) passed validation, advertised collaboration in the UI, and then had its WebSocket blocked by the browser. The entrypoint now substitutes that relay's origin into a `__GEOLIBRE_COLLAB_CONNECT_SRC__` placeholder while rendering the nginx template, alongside the existing sidecar-token substitution. - SettingsDialog.tsx: drop the unreachable `<span />` fallback for `tokenLink`; the branch requires `shareBaseUrl`, which `shareSettingsUrl` is derived from. - docs: note that the CSP is handled automatically for the web/Docker path, in contrast to the desktop build's manual `connect-src` edit. Verified the render step over four inputs: unset, a wss relay, a relay with a port and path (origin only, path stripped — CSP source expressions take no path), and a loopback ws relay. With nothing configured the emitted `connect-src` is byte-identical to the one on `main`.
Code reviewSecurity
Bugs
Performance
Quality
CLAUDE.md
|
Harden the value that now reaches the nginx CSP. - docker/entrypoint.sh: `service_url()` rejects a netloc containing anything outside `[A-Za-z0-9.\-:\[\]]`. `urlsplit()` puts everything up to the next `/`, `?` or `#` into netloc — quotes and semicolons included — and 2153097 started substituting that unescaped into the double-quoted `add_header Content-Security-Policy` value, so `wss://x"; add_header X-Pwned "1` could have injected nginx directives at boot. - The check runs *before* the loopback early-return, which is load-bearing: `urlsplit('ws://localhost:8080"; …')` reports hostname `localhost`, so the payload would otherwise have matched the loopback allowlist and returned early. (Same ordering lesson as the credentials check.) - The substitution site re-validates the composed origin with `re.fullmatch` and exits rather than emitting anything that is not a plain origin, so a future edit loosening the validator cannot reach the config. - Added the missing `import re` to the nginx-render block. Verified: four injection shapes (quote-escape, the loopback-shaped variant, a bare `;`, and an embedded newline) are rejected by both the validator and the render step, with no `X-Pwned` header reaching the output; `wss://relay.example.org`, `:8443`, `ws://127.0.0.1:8787`, and `wss://[::1]:8443` still pass. With nothing configured the emitted `connect-src` is still byte-identical to `main`.
Code reviewReviewed the runtime share/collab host resolution ( Bugs: None found. Traced every Security: No issues. The credential-stripping, hostname-exact-match (vs. prefix) checks, and the netloc character-allowlist that guards against breaking out of the double-quoted CSP Performance: No concerns. Quality: Minor, low-confidence observation — the nginx CSP's CLAUDE.md: No violations. New user-facing strings use |
Closes #1684. First step of #1665's self-hosting track.
Problem
The web container could not be pointed at a self-hosted sharing server or collaboration relay.
resolveShareBaseUrl()andresolveCollabBaseUrl()already honoredVITE_GEOLIBRE_SHARE_URL/VITE_GEOLIBRE_COLLAB_URL— but both readimport.meta.env, so they were build-time only, and neither variable appeared inDockerfileordocker/entrypoint.sh. Repointing the published image meant forking and rebuilding it.There was also a worse failure mode.
resolveShareBaseUrl()fell back tohttps://share.geolibre.appwhenever the configured value failed to parse or was HTTP on a non-loopback host. A self-hosted deployment that sethttp://geolibre.lan:8080, or typo'd its own hostname, got a working Share button that uploaded its users' projects to the public hosted service.What changed
The entrypoint already writes
geolibre-runtime-config.json every boot (for the AI proxy and embed origins), so both hosts now flow through that same channel:lib/deployment-env.tsVITE_*value, deployment env before build env — the precedencereadEmbedOriginsandreadDeploymentAssistantEnvalready use.resolveShareHost()default/configured/disabled/invalid— plus the base URL and the configured value.resolveShareBaseUrl()is now a thin wrapper returningstring | null.GEOLIBRE_SHARE_URL=offresolveCollabBaseUrl()docker/entrypoint.shGEOLIBRE_EMBED_ORIGINSblock already applies.DockerfileARG/ENVpairs, so the build-time path is documented rather than accidental.No more silent fallback. A rejected value resolves to null; the hosted default applies only when nothing is configured. Menu entries disable with a reason, and the gallery throws a new
not-configuredGalleryErrorCode.The hostname is no longer hardcoded in the UI. 11 catalogue keys across all 16 locales now take a
{{shareHost}}interpolation (the hostname isn't translated, so this was a mechanical substitution), and the account-settings link inSettingsDialog/ShareProjectDialogderives from the resolved host. A self-hosted instance previously read "Sign in to share.geolibre.app" while linking somewhere else.TLS policy (decision on item 6 of the issue)
Kept as-is and now documented: HTTPS/WSS required, plaintext only on loopback. These URLs carry a Bearer token. The change is that a value failing this now fails the container boot with an error naming the variable, instead of quietly using the public service. Self-hosters put the server behind a TLS-terminating proxy. I did not add the
GEOLIBRE_SHARE_ALLOW_INSECUREescape hatch the issue floated — happy to if you'd rather have it.Verification
Beyond the unit tests, I drove the built web app under
vite previewwith the runtime config set four ways (clearing the service worker between each, since it precaches the config):share.geolibre.appoffhttp://internal.corparia-disabled=true, title "Unavailable: this deployment's sharing server address is not valid."https://maps.example.orghttps://maps.example.org/settingsAlso exercised the entrypoint's validator across 12 inputs —
https,off/OFF, loopbackhttp,wss, loopbackwsall accepted; plaintext LAN, unparseable, embedded credentials, and wrong-scheme all exit with a message naming the variable.Gate:
npm run build,npm run test:frontend(4959 pass / 0 fail),npm run test:worker,npm run lint(0 errors; the two TopToolbar warnings are pre-existing onmain, verified by stashing).Notes for review
resolveShareBaseUrl()changing fromstringtostring | nullis the deliberate core of this. Four call sites handle null; the tests that asserted the old fallback now assert refusal.installNativeShareFetchskips the override when null, but the Taurihttp:defaultcapability scope still pins the share host, so self-hosting stays a web/Docker capability. Called out inshare-fetch.tsand the docs.pre-commit runcould not install its node hook env locally (npm error Unknown cli flag: --ignore-prepublish— a toolchain issue on my machine, not this branch), so I ran the hook equivalents directly:oxfmt@0.59.0over every changed file, plus eslint and the build via the npm scripts. pre-commit.ci will run the rest on the PR.Summary by CodeRabbit
New Features
Bug Fixes
Documentation