From 629da55a96368ad7a91be3b91787f235485e0357 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 10:58:41 -0400 Subject: [PATCH 01/11] fix: remember project save choices --- .../src/hooks/useProjectFileActions.ts | 32 +++++++++++- .../src/lib/project-save-choices.ts | 51 +++++++++++++++++++ tests/project-save-choices.test.ts | 35 +++++++++++++ 3 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 apps/geolibre-desktop/src/lib/project-save-choices.ts create mode 100644 tests/project-save-choices.test.ts diff --git a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts index be3b0fe881..03fb7ff219 100644 --- a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts +++ b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts @@ -44,6 +44,11 @@ import { resolveShareBaseUrl } from "../lib/share-geolibre"; import { shareAuthorizedFetch } from "../lib/share-gallery"; import { normalizeProjectUrl } from "../lib/urls"; import { recordExplicitProjectSave } from "../lib/project-history-session"; +import { + rememberProjectSaveChoices, + saveChoicesForProject, + type ProjectSaveChoices, +} from "../lib/project-save-choices"; import { resolveProjectXyzLayers } from "../lib/xyz-url"; import { importQgisProject, @@ -264,6 +269,10 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { // Separate from projectUrlAbortRef so a gallery open and an Open-from-URL // submit can't abort each other's in-flight fetch. const shareUrlAbortRef = useRef(null); + // Retain explicit, non-cancel save decisions for this project only. The + // generation check clears them synchronously when newProject/loadProject + // switches the store, including before React has rendered the new project. + const saveChoicesRef = useRef(null); // Guards against overlapping saves: a second save started while a prompt // dialog is open would overwrite the pending prompt and strand the first // call's unresolved promise. @@ -815,8 +824,18 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const count = embeddable.size + localFileLayers.length; const bytes = estimateEmbedBytes(state.layers, embeddable); - const choice = await askEmbedVectorData(count, bytes, isTauri()); + const remembered = saveChoicesForProject(saveChoicesRef.current, state.projectGeneration); + saveChoicesRef.current = remembered; + const choice = remembered.vectorData ?? (await askEmbedVectorData(count, bytes, isTauri())); if (choice === "cancel") return "cancel"; + // A project can be opened while a prompt is visible. Do not apply that + // prompt's answer to the replacement project or continue saving stale data. + if (useAppStore.getState().projectGeneration !== state.projectGeneration) return "cancel"; + saveChoicesRef.current = rememberProjectSaveChoices( + saveChoicesRef.current, + state.projectGeneration, + { vectorData: choice }, + ); if (choice === "embed") { // Reuse the map already materialized for the size estimate. @@ -905,8 +924,17 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const projectToEgress = excludeHiddenFieldsFromProject(project); const redacted = redactProjectCredentials(projectToEgress); if (redacted.redactedPaths.length > 0) { - const choice = await askStripCredentials(redacted.redactedCount); + const projectGeneration = useAppStore.getState().projectGeneration; + const remembered = saveChoicesForProject(saveChoicesRef.current, projectGeneration); + saveChoicesRef.current = remembered; + const choice = remembered.credentials ?? (await askStripCredentials(redacted.redactedCount)); if (choice === "cancel") return false; + if (useAppStore.getState().projectGeneration !== projectGeneration) return false; + saveChoicesRef.current = rememberProjectSaveChoices( + saveChoicesRef.current, + projectGeneration, + { credentials: choice }, + ); contentToSave = serializeForSave(choice === "strip" ? redacted.project : projectToEgress); } else { contentToSave = serializeForSave(projectToEgress); diff --git a/apps/geolibre-desktop/src/lib/project-save-choices.ts b/apps/geolibre-desktop/src/lib/project-save-choices.ts new file mode 100644 index 0000000000..964088c8e6 --- /dev/null +++ b/apps/geolibre-desktop/src/lib/project-save-choices.ts @@ -0,0 +1,51 @@ +/** How credentials should be handled when the current project is saved. */ +export type CredentialSaveChoice = "strip" | "keep"; + +/** How local vector data should be handled when the current project is saved. */ +export type VectorDataSaveChoice = "embed" | "noembed"; + +/** Save choices remembered for one loaded project during the current session. */ +export interface ProjectSaveChoices { + projectGeneration: number; + credentials?: CredentialSaveChoice; + vectorData?: VectorDataSaveChoice; +} + +/** + * Returns remembered choices only when they belong to the current project. + * + * The store increments `projectGeneration` for every new or loaded project, so + * this keeps potentially sensitive save decisions from leaking into another + * project while allowing repeated saves of the same project to stay silent. + * + * @param remembered - Choices retained by the project-file hook, if any. + * @param projectGeneration - Generation of the currently loaded project. + * @returns Existing choices for this project, or an empty choice set. + */ +export function saveChoicesForProject( + remembered: ProjectSaveChoices | null, + projectGeneration: number, +): ProjectSaveChoices { + return remembered?.projectGeneration === projectGeneration + ? remembered + : { projectGeneration }; +} + +/** + * Remembers one or more save choices for the current project. + * + * @param remembered - Choices retained by the project-file hook, if any. + * @param projectGeneration - Generation of the currently loaded project. + * @param choices - New choices to retain. + * @returns Updated choices scoped to the supplied project generation. + */ +export function rememberProjectSaveChoices( + remembered: ProjectSaveChoices | null, + projectGeneration: number, + choices: Partial>, +): ProjectSaveChoices { + return { + ...saveChoicesForProject(remembered, projectGeneration), + ...choices, + }; +} diff --git a/tests/project-save-choices.test.ts b/tests/project-save-choices.test.ts new file mode 100644 index 0000000000..2d8c6aa21f --- /dev/null +++ b/tests/project-save-choices.test.ts @@ -0,0 +1,35 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { + rememberProjectSaveChoices, + saveChoicesForProject, +} from "../apps/geolibre-desktop/src/lib/project-save-choices"; + +describe("project save choices", () => { + it("remembers credential and vector-data choices for the current project", () => { + const credentials = rememberProjectSaveChoices(null, 4, { credentials: "strip" }); + const complete = rememberProjectSaveChoices(credentials, 4, { vectorData: "embed" }); + + assert.deepEqual(saveChoicesForProject(complete, 4), { + projectGeneration: 4, + credentials: "strip", + vectorData: "embed", + }); + }); + + it("clears remembered choices when the project generation changes", () => { + const remembered = rememberProjectSaveChoices(null, 4, { + credentials: "keep", + vectorData: "noembed", + }); + + assert.deepEqual(saveChoicesForProject(remembered, 5), { projectGeneration: 5 }); + }); + + it("does not restore choices from a previously loaded project", () => { + const firstProject = rememberProjectSaveChoices(null, 4, { credentials: "strip" }); + const secondProject = saveChoicesForProject(firstProject, 5); + + assert.deepEqual(saveChoicesForProject(secondProject, 4), { projectGeneration: 4 }); + }); +}); From d91f757cb58ec29ca3a09cd36eddb526c092074a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 15 Aug 2026 14:59:49 +0000 Subject: [PATCH 02/11] style: auto-format (ruff + oxfmt) [pre-commit.ci] --- apps/geolibre-desktop/src/lib/project-save-choices.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/geolibre-desktop/src/lib/project-save-choices.ts b/apps/geolibre-desktop/src/lib/project-save-choices.ts index 964088c8e6..06853564c2 100644 --- a/apps/geolibre-desktop/src/lib/project-save-choices.ts +++ b/apps/geolibre-desktop/src/lib/project-save-choices.ts @@ -26,9 +26,7 @@ export function saveChoicesForProject( remembered: ProjectSaveChoices | null, projectGeneration: number, ): ProjectSaveChoices { - return remembered?.projectGeneration === projectGeneration - ? remembered - : { projectGeneration }; + return remembered?.projectGeneration === projectGeneration ? remembered : { projectGeneration }; } /** From 12a382f2d829ae1e61fe89c09b68891969623635 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 11:06:28 -0400 Subject: [PATCH 03/11] fix: cancel stale save prompts on project switch --- .../src/hooks/useProjectFileActions.ts | 48 ++++++++++++++++--- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts index 03fb7ff219..246524f109 100644 --- a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts +++ b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts @@ -16,7 +16,7 @@ import { materializeEmbeddableVectorLayers, } from "@geolibre/plugins"; import type { FeatureCollection } from "geojson"; -import { type FormEvent, useRef, useState } from "react"; +import { type FormEvent, useEffect, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { createAppAPI, getPluginManager } from "./usePlugins"; import { pluginManifestUrlsForIds } from "../lib/external-plugins"; @@ -61,6 +61,8 @@ import type { MapControllerRef } from "../components/layout/toolbar/constants"; /** A pending "strip credentials before saving?" prompt. */ export interface CredentialStripPrompt { count: number; + /** Project generation that opened the prompt. */ + projectGeneration: number; resolve: (choice: "strip" | "keep" | "cancel") => void; } @@ -108,6 +110,8 @@ export interface EmbedVectorDataPrompt { * described differently than on the web (where it discards the data). */ desktop: boolean; + /** Project generation that opened the prompt. */ + projectGeneration: number; resolve: (choice: "embed" | "noembed" | "cancel") => void; } @@ -244,6 +248,7 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const rememberRecentProject = useAppStore((s) => s.rememberRecentProject); const forgetRecentProject = useAppStore((s) => s.forgetRecentProject); const markSaved = useAppStore((s) => s.markSaved); + const projectGeneration = useAppStore((s) => s.projectGeneration); const [actionError, setActionError] = useState(null); const [qgisImportWarnings, setQgisImportWarnings] = useState( @@ -278,6 +283,20 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { // call's unresolved promise. const isSavingRef = useRef(false); + // A project can be replaced by an external open action while a modal save + // prompt is visible. Cancel the stale promise immediately so its dialog does + // not cover the replacement project and its save guard is released. + useEffect(() => { + if (credentialStripPrompt && credentialStripPrompt.projectGeneration !== projectGeneration) { + credentialStripPrompt.resolve("cancel"); + setCredentialStripPrompt(null); + } + if (embedVectorDataPrompt && embedVectorDataPrompt.projectGeneration !== projectGeneration) { + embedVectorDataPrompt.resolve("cancel"); + setEmbedVectorDataPrompt(null); + } + }, [credentialStripPrompt, embedVectorDataPrompt, projectGeneration]); + const handleOpenFromFile = async () => { const result = await openProjectFile(); if (result) { @@ -733,9 +752,9 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { // Ask whether to strip credentials (environment variables, geocoder keys, // layer tokens) before writing the file. The promise resolves when the user // picks an option in the dialog. - const askStripCredentials = (count: number) => + const askStripCredentials = (count: number, promptProjectGeneration: number) => new Promise<"strip" | "keep" | "cancel">((resolve) => { - setCredentialStripPrompt({ count, resolve }); + setCredentialStripPrompt({ count, projectGeneration: promptProjectGeneration, resolve }); }); const resolveCredentialStripPrompt = (choice: "strip" | "keep" | "cancel") => { @@ -746,9 +765,20 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { // Ask whether to embed local vector layers' data in the saved file. Resolves // when the user picks an option in the dialog. - const askEmbedVectorData = (count: number, bytes: number, desktop: boolean) => + const askEmbedVectorData = ( + count: number, + bytes: number, + desktop: boolean, + promptProjectGeneration: number, + ) => new Promise<"embed" | "noembed" | "cancel">((resolve) => { - setEmbedVectorDataPrompt({ count, bytes, desktop, resolve }); + setEmbedVectorDataPrompt({ + count, + bytes, + desktop, + projectGeneration: promptProjectGeneration, + resolve, + }); }); const resolveEmbedVectorDataPrompt = (choice: "embed" | "noembed" | "cancel") => { @@ -826,7 +856,9 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const bytes = estimateEmbedBytes(state.layers, embeddable); const remembered = saveChoicesForProject(saveChoicesRef.current, state.projectGeneration); saveChoicesRef.current = remembered; - const choice = remembered.vectorData ?? (await askEmbedVectorData(count, bytes, isTauri())); + const choice = + remembered.vectorData ?? + (await askEmbedVectorData(count, bytes, isTauri(), state.projectGeneration)); if (choice === "cancel") return "cancel"; // A project can be opened while a prompt is visible. Do not apply that // prompt's answer to the replacement project or continue saving stale data. @@ -927,7 +959,9 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const projectGeneration = useAppStore.getState().projectGeneration; const remembered = saveChoicesForProject(saveChoicesRef.current, projectGeneration); saveChoicesRef.current = remembered; - const choice = remembered.credentials ?? (await askStripCredentials(redacted.redactedCount)); + const choice = + remembered.credentials ?? + (await askStripCredentials(redacted.redactedCount, projectGeneration)); if (choice === "cancel") return false; if (useAppStore.getState().projectGeneration !== projectGeneration) return false; saveChoicesRef.current = rememberProjectSaveChoices( From 948fee243a2e64212bb93d2a75d88a054e1b7f8f Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 11:15:19 -0400 Subject: [PATCH 04/11] fix: invalidate suspended saves on project switch --- .../src/hooks/useProjectFileActions.ts | 74 +++++++++++++------ 1 file changed, 53 insertions(+), 21 deletions(-) diff --git a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts index 246524f109..f32c1e0ae1 100644 --- a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts +++ b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts @@ -122,6 +122,8 @@ export interface EmbedVectorDataPrompt { * same component serves both. */ export interface SaveNamePrompt { + /** Project generation that opened the prompt. */ + projectGeneration: number; resolve: (name: string | null) => void; /** Dialog title. */ title: string; @@ -295,7 +297,12 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { embedVectorDataPrompt.resolve("cancel"); setEmbedVectorDataPrompt(null); } - }, [credentialStripPrompt, embedVectorDataPrompt, projectGeneration]); + if (saveNamePrompt && saveNamePrompt.projectGeneration !== projectGeneration) { + saveNamePrompt.resolve(null); + setSaveNamePrompt(null); + setSaveNameInput(""); + } + }, [credentialStripPrompt, embedVectorDataPrompt, projectGeneration, saveNamePrompt]); const handleOpenFromFile = async () => { const result = await openProjectFile(); @@ -849,6 +856,7 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const resolveLayersForSave = async (): Promise<{ layers?: GeoLibreLayer[] } | "cancel"> => { const state = useAppStore.getState(); const embeddable = await materializeEmbeddableVectorLayers(state.layers); + if (useAppStore.getState().projectGeneration !== state.projectGeneration) return "cancel"; const localFileLayers = isTauri() ? state.layers.filter(isReloadableLocalFileLayer) : []; if (embeddable.size === 0 && localFileLayers.length === 0) return {}; @@ -921,10 +929,14 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { // the user can control. The caller supplies the dialog copy so the same prompt // serves both project saves and HTML exports. Resolves with the name, or null // if cancelled. - const askSaveName = (defaultName: string, labels: Omit) => + const askSaveName = ( + defaultName: string, + labels: Omit, + promptProjectGeneration: number, + ) => new Promise((resolve) => { setSaveNameInput(defaultName); - setSaveNamePrompt({ resolve, ...labels }); + setSaveNamePrompt({ projectGeneration: promptProjectGeneration, resolve, ...labels }); }); const submitSaveNamePrompt = (event?: FormEvent) => { @@ -941,10 +953,16 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { }; const runSaveProject = async (options?: { saveAs?: boolean }): Promise => { + const saveProjectGeneration = useAppStore.getState().projectGeneration; // Offer to embed local vector data (or, on desktop, save file references) // first, so the serialized content below reflects the user's choice. const layersForSave = await resolveLayersForSave(); - if (layersForSave === "cancel") return false; + if ( + layersForSave === "cancel" || + useAppStore.getState().projectGeneration !== saveProjectGeneration + ) { + return false; + } const { project, defaultProjectName, projectPath } = buildCurrentProject( undefined, layersForSave.layers, @@ -956,17 +974,16 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const projectToEgress = excludeHiddenFieldsFromProject(project); const redacted = redactProjectCredentials(projectToEgress); if (redacted.redactedPaths.length > 0) { - const projectGeneration = useAppStore.getState().projectGeneration; - const remembered = saveChoicesForProject(saveChoicesRef.current, projectGeneration); + const remembered = saveChoicesForProject(saveChoicesRef.current, saveProjectGeneration); saveChoicesRef.current = remembered; const choice = remembered.credentials ?? - (await askStripCredentials(redacted.redactedCount, projectGeneration)); + (await askStripCredentials(redacted.redactedCount, saveProjectGeneration)); if (choice === "cancel") return false; - if (useAppStore.getState().projectGeneration !== projectGeneration) return false; + if (useAppStore.getState().projectGeneration !== saveProjectGeneration) return false; saveChoicesRef.current = rememberProjectSaveChoices( saveChoicesRef.current, - projectGeneration, + saveProjectGeneration, { credentials: choice }, ); contentToSave = serializeForSave(choice === "strip" ? redacted.project : projectToEgress); @@ -985,15 +1002,20 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const promptForName = browserSaveFallsBackToDownload() && (options?.saveAs === true || !existingLocalPath); if (promptForName) { - const chosen = await askSaveName(saveName, { - title: t("toolbar.item.saveProjectAsTitle"), - description: t("toolbar.item.saveProjectAsDesc"), - label: t("toolbar.item.saveProjectFileName"), - placeholder: t("toolbar.item.saveProjectFileNamePlaceholder"), - }); + const chosen = await askSaveName( + saveName, + { + title: t("toolbar.item.saveProjectAsTitle"), + description: t("toolbar.item.saveProjectAsDesc"), + label: t("toolbar.item.saveProjectFileName"), + placeholder: t("toolbar.item.saveProjectFileNamePlaceholder"), + }, + saveProjectGeneration, + ); if (chosen === null) return false; saveName = ensureProjectFileName(chosen); } + if (useAppStore.getState().projectGeneration !== saveProjectGeneration) return false; let path: string | null; try { path = @@ -1011,6 +1033,10 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { return false; } if (!path) return false; + // A native picker can remain open while another project arrives through an + // external action. The old project may have been written successfully, but + // never attach its path or saved state to the replacement project. + if (useAppStore.getState().projectGeneration !== saveProjectGeneration) return false; setProjectPath(path); rememberRecentProject({ path, @@ -1042,6 +1068,7 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { if (isSavingRef.current) return false; isSavingRef.current = true; try { + const exportProjectGeneration = useAppStore.getState().projectGeneration; // Derive the default file name from the project name in the store first, // without materializing embedded data, so the prompt can appear right away // and a cancel discards no work. This snapshot is passed to @@ -1061,12 +1088,16 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { // saveTextFileWithFallback below instead. let defaultName = `${slug}.html`; if (browserSaveFallsBackToDownload()) { - const chosen = await askSaveName(defaultName, { - title: t("toolbar.item.exportHtmlAsTitle"), - description: t("toolbar.item.exportHtmlAsDesc"), - label: t("toolbar.item.exportHtmlFileName"), - placeholder: t("toolbar.item.exportHtmlFileNamePlaceholder"), - }); + const chosen = await askSaveName( + defaultName, + { + title: t("toolbar.item.exportHtmlAsTitle"), + description: t("toolbar.item.exportHtmlAsDesc"), + label: t("toolbar.item.exportHtmlFileName"), + placeholder: t("toolbar.item.exportHtmlFileNamePlaceholder"), + }, + exportProjectGeneration, + ); if (chosen === null) return false; defaultName = ensureHtmlFileName(chosen, slug); } @@ -1077,6 +1108,7 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { // serve no purpose in a static viewer and are removed inside // buildProjectHtml, which runs the central redaction pass. const { project, defaultProjectName } = await buildEmbeddedProject(projectName); + if (useAppStore.getState().projectGeneration !== exportProjectGeneration) return false; const html = buildProjectHtml({ project, title: defaultProjectName, From 80ac7efbf69a3f0525b68998d6ae64fdabe88da3 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 11:17:27 -0400 Subject: [PATCH 05/11] fix: preserve save safety across project changes --- .../src/hooks/useProjectFileActions.ts | 18 +++++++++++++++-- .../src/lib/project-save-choices.ts | 20 +++++++++++++++++++ tests/project-save-choices.test.ts | 19 ++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts index f32c1e0ae1..6151b3225d 100644 --- a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts +++ b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts @@ -46,6 +46,7 @@ import { normalizeProjectUrl } from "../lib/urls"; import { recordExplicitProjectSave } from "../lib/project-history-session"; import { rememberProjectSaveChoices, + reusableVectorDataChoice, saveChoicesForProject, type ProjectSaveChoices, } from "../lib/project-save-choices"; @@ -864,8 +865,15 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const bytes = estimateEmbedBytes(state.layers, embeddable); const remembered = saveChoicesForProject(saveChoicesRef.current, state.projectGeneration); saveChoicesRef.current = remembered; + // A remembered Embed choice stays silent until the project first crosses + // the large-data warning threshold. That material risk deserves one fresh + // confirmation even though the ordinary per-project choice is remembered. + const rememberedVectorChoice = reusableVectorDataChoice( + remembered, + bytes >= LARGE_EMBED_WARNING_BYTES, + ); const choice = - remembered.vectorData ?? + rememberedVectorChoice ?? (await askEmbedVectorData(count, bytes, isTauri(), state.projectGeneration)); if (choice === "cancel") return "cancel"; // A project can be opened while a prompt is visible. Do not apply that @@ -874,7 +882,13 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { saveChoicesRef.current = rememberProjectSaveChoices( saveChoicesRef.current, state.projectGeneration, - { vectorData: choice }, + { + vectorData: choice, + largeEmbedWarningAcknowledged: + choice === "embed" && bytes >= LARGE_EMBED_WARNING_BYTES + ? true + : remembered.largeEmbedWarningAcknowledged, + }, ); if (choice === "embed") { diff --git a/apps/geolibre-desktop/src/lib/project-save-choices.ts b/apps/geolibre-desktop/src/lib/project-save-choices.ts index 06853564c2..f0276807f1 100644 --- a/apps/geolibre-desktop/src/lib/project-save-choices.ts +++ b/apps/geolibre-desktop/src/lib/project-save-choices.ts @@ -9,6 +9,8 @@ export interface ProjectSaveChoices { projectGeneration: number; credentials?: CredentialSaveChoice; vectorData?: VectorDataSaveChoice; + /** Whether the user accepted embedding after seeing the large-data warning. */ + largeEmbedWarningAcknowledged?: boolean; } /** @@ -47,3 +49,21 @@ export function rememberProjectSaveChoices( ...choices, }; } + +/** + * Returns a remembered vector-data choice when no new size warning is needed. + * + * @param remembered - Choices scoped to the current project. + * @param largeEmbedWarningRequired - Whether the current data crosses the warning threshold. + * @returns The reusable choice, or undefined when the user must confirm a large embed. + */ +export function reusableVectorDataChoice( + remembered: ProjectSaveChoices, + largeEmbedWarningRequired: boolean, +): VectorDataSaveChoice | undefined { + return remembered.vectorData === "embed" && + largeEmbedWarningRequired && + remembered.largeEmbedWarningAcknowledged !== true + ? undefined + : remembered.vectorData; +} diff --git a/tests/project-save-choices.test.ts b/tests/project-save-choices.test.ts index 2d8c6aa21f..88e4c7cf5f 100644 --- a/tests/project-save-choices.test.ts +++ b/tests/project-save-choices.test.ts @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; import { rememberProjectSaveChoices, + reusableVectorDataChoice, saveChoicesForProject, } from "../apps/geolibre-desktop/src/lib/project-save-choices"; @@ -21,6 +22,7 @@ describe("project save choices", () => { const remembered = rememberProjectSaveChoices(null, 4, { credentials: "keep", vectorData: "noembed", + largeEmbedWarningAcknowledged: true, }); assert.deepEqual(saveChoicesForProject(remembered, 5), { projectGeneration: 5 }); @@ -32,4 +34,21 @@ describe("project save choices", () => { assert.deepEqual(saveChoicesForProject(secondProject, 4), { projectGeneration: 4 }); }); + + it("retains a large-embed warning acknowledgement with the project choices", () => { + const vectorChoice = rememberProjectSaveChoices(null, 4, { vectorData: "embed" }); + assert.equal(reusableVectorDataChoice(vectorChoice, false), "embed"); + assert.equal(reusableVectorDataChoice(vectorChoice, true), undefined); + + const acknowledged = rememberProjectSaveChoices(vectorChoice, 4, { + largeEmbedWarningAcknowledged: true, + }); + + assert.deepEqual(acknowledged, { + projectGeneration: 4, + vectorData: "embed", + largeEmbedWarningAcknowledged: true, + }); + assert.equal(reusableVectorDataChoice(acknowledged, true), "embed"); + }); }); From 76102dcfe655e44ea9fc8b4aca736de41e57fc47 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 11:21:52 -0400 Subject: [PATCH 06/11] fix: reject stale HTML export results --- apps/geolibre-desktop/src/hooks/useProjectFileActions.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts index 6151b3225d..c1efc90f85 100644 --- a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts +++ b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts @@ -1140,6 +1140,7 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { ], mimeType: "text/html", }); + if (useAppStore.getState().projectGeneration !== exportProjectGeneration) return false; return savedPath !== null; } catch (error) { setActionError( From 5ffe3472bbe21a3aadfd2bb9942a6a736a3acc63 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 11:31:15 -0400 Subject: [PATCH 07/11] fix: reconfirm expanded credential saves --- .../src/hooks/useProjectFileActions.ts | 9 ++++++-- .../src/lib/project-save-choices.ts | 23 +++++++++++++++++++ tests/project-save-choices.test.ts | 14 +++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts index c1efc90f85..259cc6fbfa 100644 --- a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts +++ b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts @@ -46,6 +46,7 @@ import { normalizeProjectUrl } from "../lib/urls"; import { recordExplicitProjectSave } from "../lib/project-history-session"; import { rememberProjectSaveChoices, + reusableCredentialChoice, reusableVectorDataChoice, saveChoicesForProject, type ProjectSaveChoices, @@ -991,14 +992,18 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const remembered = saveChoicesForProject(saveChoicesRef.current, saveProjectGeneration); saveChoicesRef.current = remembered; const choice = - remembered.credentials ?? + reusableCredentialChoice(remembered, redacted.redactedCount) ?? (await askStripCredentials(redacted.redactedCount, saveProjectGeneration)); if (choice === "cancel") return false; if (useAppStore.getState().projectGeneration !== saveProjectGeneration) return false; saveChoicesRef.current = rememberProjectSaveChoices( saveChoicesRef.current, saveProjectGeneration, - { credentials: choice }, + { + credentials: choice, + keptCredentialCount: + choice === "keep" ? redacted.redactedCount : remembered.keptCredentialCount, + }, ); contentToSave = serializeForSave(choice === "strip" ? redacted.project : projectToEgress); } else { diff --git a/apps/geolibre-desktop/src/lib/project-save-choices.ts b/apps/geolibre-desktop/src/lib/project-save-choices.ts index f0276807f1..2fc37dcc75 100644 --- a/apps/geolibre-desktop/src/lib/project-save-choices.ts +++ b/apps/geolibre-desktop/src/lib/project-save-choices.ts @@ -8,6 +8,8 @@ export type VectorDataSaveChoice = "embed" | "noembed"; export interface ProjectSaveChoices { projectGeneration: number; credentials?: CredentialSaveChoice; + /** Credential count covered by the last explicit Keep choice. */ + keptCredentialCount?: number; vectorData?: VectorDataSaveChoice; /** Whether the user accepted embedding after seeing the large-data warning. */ largeEmbedWarningAcknowledged?: boolean; @@ -67,3 +69,24 @@ export function reusableVectorDataChoice( ? undefined : remembered.vectorData; } + +/** + * Returns a remembered credential choice when it covers the current risk. + * + * Stripping remains safe as more credentials are added. Keeping credentials is + * reused only while the project has no more credential-bearing fields than the + * user explicitly accepted. + * + * @param remembered - Choices scoped to the current project. + * @param credentialCount - Current number of credential-bearing fields. + * @returns The reusable choice, or undefined when Keep must be confirmed again. + */ +export function reusableCredentialChoice( + remembered: ProjectSaveChoices, + credentialCount: number, +): CredentialSaveChoice | undefined { + return remembered.credentials === "keep" && + (remembered.keptCredentialCount == null || credentialCount > remembered.keptCredentialCount) + ? undefined + : remembered.credentials; +} diff --git a/tests/project-save-choices.test.ts b/tests/project-save-choices.test.ts index 88e4c7cf5f..372d0e826e 100644 --- a/tests/project-save-choices.test.ts +++ b/tests/project-save-choices.test.ts @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; import { rememberProjectSaveChoices, + reusableCredentialChoice, reusableVectorDataChoice, saveChoicesForProject, } from "../apps/geolibre-desktop/src/lib/project-save-choices"; @@ -51,4 +52,17 @@ describe("project save choices", () => { }); assert.equal(reusableVectorDataChoice(acknowledged, true), "embed"); }); + + it("reconfirms Keep when the project gains more credential-bearing fields", () => { + const keep = rememberProjectSaveChoices(null, 4, { + credentials: "keep", + keptCredentialCount: 2, + }); + assert.equal(reusableCredentialChoice(keep, 2), "keep"); + assert.equal(reusableCredentialChoice(keep, 1), "keep"); + assert.equal(reusableCredentialChoice(keep, 3), undefined); + + const strip = rememberProjectSaveChoices(null, 4, { credentials: "strip" }); + assert.equal(reusableCredentialChoice(strip, 10), "strip"); + }); }); From 160d0ea9d7ee4b9c29dacdeea5499fe0b1ef357d Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 11:47:29 -0400 Subject: [PATCH 08/11] Address Claude review feedback - Track which credentials an explicit Keep covered, not how many. The redaction pass now returns a `path=hash` fingerprint per redacted path, and a remembered Keep is reused only when every credential this save would write is one the user was shown. Swapping one credentialed layer for another leaves the count unchanged, so the previous count check silently wrote a secret the user never approved. - Re-warn when embedded vector data outgrows the acknowledged size. `largeEmbedWarningAcknowledged` became `acknowledgedEmbedBytes`, and the remembered Embed choice is reused only up to twice the size the user actually saw. Only a prompted answer records the size, so a silent reuse cannot ratchet the allowance in either direction. --- .../src/hooks/useProjectFileActions.ts | 34 ++-- .../src/lib/project-save-choices.ts | 60 ++++--- packages/core/src/credentials.ts | 151 +++++++++++------- tests/project-credentials.test.ts | 23 +++ tests/project-save-choices.test.ts | 66 ++++++-- 5 files changed, 236 insertions(+), 98 deletions(-) diff --git a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts index 259cc6fbfa..e37d3d26b9 100644 --- a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts +++ b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts @@ -866,12 +866,14 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const bytes = estimateEmbedBytes(state.layers, embeddable); const remembered = saveChoicesForProject(saveChoicesRef.current, state.projectGeneration); saveChoicesRef.current = remembered; - // A remembered Embed choice stays silent until the project first crosses - // the large-data warning threshold. That material risk deserves one fresh + // A remembered Embed choice stays silent until the project crosses the + // large-data warning threshold, and again whenever the data outgrows the + // size that was acknowledged. That material risk deserves a fresh // confirmation even though the ordinary per-project choice is remembered. const rememberedVectorChoice = reusableVectorDataChoice( remembered, - bytes >= LARGE_EMBED_WARNING_BYTES, + bytes, + LARGE_EMBED_WARNING_BYTES, ); const choice = rememberedVectorChoice ?? @@ -885,10 +887,14 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { state.projectGeneration, { vectorData: choice, - largeEmbedWarningAcknowledged: - choice === "embed" && bytes >= LARGE_EMBED_WARNING_BYTES - ? true - : remembered.largeEmbedWarningAcknowledged, + // Only a size the user was actually shown extends the allowance; a + // silent reuse must not ratchet it up (or down) on its own. + acknowledgedEmbedBytes: + rememberedVectorChoice === undefined && + choice === "embed" && + bytes >= LARGE_EMBED_WARNING_BYTES + ? bytes + : remembered.acknowledgedEmbedBytes, }, ); @@ -991,8 +997,12 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { if (redacted.redactedPaths.length > 0) { const remembered = saveChoicesForProject(saveChoicesRef.current, saveProjectGeneration); saveChoicesRef.current = remembered; + const rememberedCredentialChoice = reusableCredentialChoice( + remembered, + redacted.redactedFingerprints, + ); const choice = - reusableCredentialChoice(remembered, redacted.redactedCount) ?? + rememberedCredentialChoice ?? (await askStripCredentials(redacted.redactedCount, saveProjectGeneration)); if (choice === "cancel") return false; if (useAppStore.getState().projectGeneration !== saveProjectGeneration) return false; @@ -1001,8 +1011,12 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { saveProjectGeneration, { credentials: choice, - keptCredentialCount: - choice === "keep" ? redacted.redactedCount : remembered.keptCredentialCount, + // Keep covers exactly the credentials the user was asked about, so a + // later save that would write a different secret asks again. + keptCredentialFingerprints: + rememberedCredentialChoice === undefined && choice === "keep" + ? redacted.redactedFingerprints + : remembered.keptCredentialFingerprints, }, ); contentToSave = serializeForSave(choice === "strip" ? redacted.project : projectToEgress); diff --git a/apps/geolibre-desktop/src/lib/project-save-choices.ts b/apps/geolibre-desktop/src/lib/project-save-choices.ts index 2fc37dcc75..e96b659cb2 100644 --- a/apps/geolibre-desktop/src/lib/project-save-choices.ts +++ b/apps/geolibre-desktop/src/lib/project-save-choices.ts @@ -4,15 +4,23 @@ export type CredentialSaveChoice = "strip" | "keep"; /** How local vector data should be handled when the current project is saved. */ export type VectorDataSaveChoice = "embed" | "noembed"; +/** + * How far embedded data may grow past an acknowledged size before the + * large-embed warning is shown again. A project gains features between saves, + * so re-prompting on any growth would defeat the point of remembering the + * choice; doubling is a change of scale the user has not actually agreed to. + */ +export const EMBED_REACKNOWLEDGE_GROWTH_FACTOR = 2; + /** Save choices remembered for one loaded project during the current session. */ export interface ProjectSaveChoices { projectGeneration: number; credentials?: CredentialSaveChoice; - /** Credential count covered by the last explicit Keep choice. */ - keptCredentialCount?: number; + /** Credential fingerprints covered by the last explicit Keep choice. */ + keptCredentialFingerprints?: readonly string[]; vectorData?: VectorDataSaveChoice; - /** Whether the user accepted embedding after seeing the large-data warning. */ - largeEmbedWarningAcknowledged?: boolean; + /** Embedded size, in bytes, the user accepted after seeing the large-data warning. */ + acknowledgedEmbedBytes?: number; } /** @@ -55,38 +63,50 @@ export function rememberProjectSaveChoices( /** * Returns a remembered vector-data choice when no new size warning is needed. * + * An acknowledgement covers the size the user actually saw, plus the ordinary + * growth of a project being edited. Data that balloons past + * {@link EMBED_REACKNOWLEDGE_GROWTH_FACTOR} times that size is a materially + * different write and is confirmed again. + * * @param remembered - Choices scoped to the current project. - * @param largeEmbedWarningRequired - Whether the current data crosses the warning threshold. + * @param embedBytes - Estimated size of the data this save would embed. + * @param warningBytes - Threshold at which the large-embed warning applies. * @returns The reusable choice, or undefined when the user must confirm a large embed. */ export function reusableVectorDataChoice( remembered: ProjectSaveChoices, - largeEmbedWarningRequired: boolean, + embedBytes: number, + warningBytes: number, ): VectorDataSaveChoice | undefined { - return remembered.vectorData === "embed" && - largeEmbedWarningRequired && - remembered.largeEmbedWarningAcknowledged !== true - ? undefined - : remembered.vectorData; + if (remembered.vectorData !== "embed" || embedBytes < warningBytes) return remembered.vectorData; + const acknowledged = remembered.acknowledgedEmbedBytes; + return acknowledged != null && embedBytes <= acknowledged * EMBED_REACKNOWLEDGE_GROWTH_FACTOR + ? remembered.vectorData + : undefined; } /** * Returns a remembered credential choice when it covers the current risk. * - * Stripping remains safe as more credentials are added. Keeping credentials is - * reused only while the project has no more credential-bearing fields than the - * user explicitly accepted. + * Stripping remains safe however the project changes. Keeping credentials is + * reused only while every credential this save would write is one the user + * explicitly accepted. Fingerprints rather than a count, because swapping one + * credentialed layer for another leaves the count unchanged while putting a + * secret the user never saw on disk. * * @param remembered - Choices scoped to the current project. - * @param credentialCount - Current number of credential-bearing fields. + * @param credentialFingerprints - Fingerprints of the credentials this save would keep. * @returns The reusable choice, or undefined when Keep must be confirmed again. */ export function reusableCredentialChoice( remembered: ProjectSaveChoices, - credentialCount: number, + credentialFingerprints: readonly string[], ): CredentialSaveChoice | undefined { - return remembered.credentials === "keep" && - (remembered.keptCredentialCount == null || credentialCount > remembered.keptCredentialCount) - ? undefined - : remembered.credentials; + if (remembered.credentials !== "keep") return remembered.credentials; + const acknowledged = remembered.keptCredentialFingerprints; + if (acknowledged == null) return undefined; + const covered = new Set(acknowledged); + return credentialFingerprints.every((fingerprint) => covered.has(fingerprint)) + ? remembered.credentials + : undefined; } diff --git a/packages/core/src/credentials.ts b/packages/core/src/credentials.ts index 3dbc41a1d4..41c988f737 100644 --- a/packages/core/src/credentials.ts +++ b/packages/core/src/credentials.ts @@ -57,10 +57,63 @@ export interface CredentialRedactionResult { project: GeoLibreProject; /** Stable project paths removed or rewritten by the redaction pass. */ redactedPaths: string[]; + /** + * One opaque `path=hash` fingerprint per redacted path, identifying *which* + * secret sat there. A caller that remembers an explicit "keep credentials" + * decision compares these between saves, so a different credential, such as + * a swapped layer or rotated token, is confirmed again instead of being + * written to disk under the earlier answer. Paths alone cannot carry that: + * they are positional, so a replacement layer inherits the path of the one it + * replaced. + */ + redactedFingerprints: string[]; /** Number of individual credential-bearing fields removed or rewritten. */ redactedCount: number; } +/** Collector threaded through the redaction pass. */ +interface RedactionAccumulator { + paths: string[]; + fingerprints: string[]; + count: number; +} + +/** + * Hash a credential value to a short, stable token (FNV-1a). + * + * The digest never leaves memory and is only ever compared for equality, so it + * needs to be stable and cheap rather than cryptographic. Hashing rather than + * retaining the value keeps the secret itself out of the remembered choices. + */ +function fingerprintValue(value: unknown): string { + let serialized: string; + try { + serialized = JSON.stringify(value) ?? String(value); + } catch { + // A circular or otherwise unserializable blob still needs a marker; that it + // collides with other unserializable blobs only costs an extra prompt. + serialized = String(value); + } + let hash = 0x811c9dc5; + for (let index = 0; index < serialized.length; index += 1) { + hash ^= serialized.charCodeAt(index); + hash = Math.imul(hash, 0x01000193); + } + return (hash >>> 0).toString(36); +} + +/** Record one redaction, with the fingerprint of the value being removed. */ +function recordRedaction( + accumulator: RedactionAccumulator, + path: string, + value: unknown, + count = 1, +): void { + accumulator.paths.push(path); + accumulator.fingerprints.push(`${path}=${fingerprintValue(value)}`); + accumulator.count += count; +} + /** * Fold the spellings of one credential name together, so `apiKey`, `api_key`, * `api-key`, and `APIKEY` are a single registry entry on both the object-key @@ -175,29 +228,26 @@ function isGeoJsonPayload(value: Record): boolean { function redactConfigurationValue( value: unknown, path: string, - redactedPaths: string[], - redactedCount: { value: number }, + accumulator: RedactionAccumulator, depth = 0, ): unknown { if (depth >= MAX_REDACT_DEPTH) { // Fail closed. A deeply nested configuration shape is not needed to render // any built-in layer, and returning it unchanged would let a credential // bypass the invariant merely by exceeding the traversal cap. - redactedPaths.push(path); - redactedCount.value += 1; + recordRedaction(accumulator, path, value); return undefined; } if (typeof value === "string") { const redacted = redactUrlCredentials(value); if (redacted !== value) { - redactedPaths.push(path); - redactedCount.value += 1; + recordRedaction(accumulator, path, value); } return redacted; } if (Array.isArray(value)) { return value.map((item, index) => - redactConfigurationValue(item, `${path}[${index}]`, redactedPaths, redactedCount, depth + 1), + redactConfigurationValue(item, `${path}[${index}]`, accumulator, depth + 1), ); } if (!isPlainObject(value)) return value; @@ -207,17 +257,10 @@ function redactConfigurationValue( for (const [key, nested] of Object.entries(value)) { const nestedPath = path ? `${path}.${key}` : key; if (isCredentialFieldName(key)) { - redactedPaths.push(nestedPath); - redactedCount.value += 1; + recordRedaction(accumulator, nestedPath, nested); continue; } - result[key] = redactConfigurationValue( - nested, - nestedPath, - redactedPaths, - redactedCount, - depth + 1, - ); + result[key] = redactConfigurationValue(nested, nestedPath, accumulator, depth + 1); } return result; } @@ -248,15 +291,13 @@ function countLeafValues(value: unknown): number { * themselves. */ export function redactProjectCredentials(project: GeoLibreProject): CredentialRedactionResult { - const redactedPaths: string[] = []; - const redactedCount = { value: 0 }; + const accumulator: RedactionAccumulator = { paths: [], fingerprints: [], count: 0 }; const basemapStyleUrl = typeof project.basemapStyleUrl === "string" ? redactUrlCredentials(project.basemapStyleUrl) : project.basemapStyleUrl; if (basemapStyleUrl !== project.basemapStyleUrl) { - redactedPaths.push("basemapStyleUrl"); - redactedCount.value += 1; + recordRedaction(accumulator, "basemapStyleUrl", project.basemapStyleUrl); } const geocoding = project.preferences?.geocoding ? { ...project.preferences.geocoding, apiKeys: {} } @@ -268,8 +309,7 @@ export function redactProjectCredentials(project: GeoLibreProject): CredentialRe const redacted = redactUrlCredentials(endpoint); if (redacted !== endpoint) { geocoding[field] = redacted; - redactedPaths.push(`preferences.geocoding.${field}`); - redactedCount.value += 1; + recordRedaction(accumulator, `preferences.geocoding.${field}`, endpoint); } } } @@ -277,15 +317,23 @@ export function redactProjectCredentials(project: GeoLibreProject): CredentialRe ? { ...project.preferences, environmentVariables: [], geocoding } : project.preferences; const populatedEnvironmentVariables = - project.preferences?.environmentVariables?.filter((variable) => variable.key.trim()).length ?? - 0; - if (populatedEnvironmentVariables > 0) { - redactedPaths.push("preferences.environmentVariables"); - redactedCount.value += populatedEnvironmentVariables; + project.preferences?.environmentVariables?.filter((variable) => variable.key.trim()) ?? []; + if (populatedEnvironmentVariables.length > 0) { + recordRedaction( + accumulator, + "preferences.environmentVariables", + populatedEnvironmentVariables, + populatedEnvironmentVariables.length, + ); } - if (Object.keys(project.preferences?.geocoding?.apiKeys ?? {}).length > 0) { - redactedPaths.push("preferences.geocoding.apiKeys"); - redactedCount.value += Object.keys(project.preferences?.geocoding?.apiKeys ?? {}).length; + const geocodingApiKeys = project.preferences?.geocoding?.apiKeys ?? {}; + if (Object.keys(geocodingApiKeys).length > 0) { + recordRedaction( + accumulator, + "preferences.geocoding.apiKeys", + geocodingApiKeys, + Object.keys(geocodingApiKeys).length, + ); } const layers = (project.layers ?? []).map((layer, index) => ({ @@ -293,22 +341,19 @@ export function redactProjectCredentials(project: GeoLibreProject): CredentialRe source: redactConfigurationValue( layer.source, `layers[${index}].source`, - redactedPaths, - redactedCount, + accumulator, ) as Record, metadata: redactConfigurationValue( layer.metadata, `layers[${index}].metadata`, - redactedPaths, - redactedCount, + accumulator, ) as Record, ...(typeof layer.sourcePath === "string" ? { sourcePath: redactConfigurationValue( layer.sourcePath, `layers[${index}].sourcePath`, - redactedPaths, - redactedCount, + accumulator, ) as string, } : {}), @@ -321,8 +366,7 @@ export function redactProjectCredentials(project: GeoLibreProject): CredentialRe connection: redactConfigurationValue( layer.connection, `layers[${index}].connection`, - redactedPaths, - redactedCount, + accumulator, ) as LayerConnection, } : {}), @@ -333,8 +377,7 @@ export function redactProjectCredentials(project: GeoLibreProject): CredentialRe const manifestUrls = plugins.manifestUrls.map((url, index) => { const redacted = redactUrlCredentials(url); if (redacted !== url) { - redactedPaths.push(`plugins.manifestUrls[${index}]`); - redactedCount.value += 1; + recordRedaction(accumulator, `plugins.manifestUrls[${index}]`, url); } return redacted; }); @@ -368,20 +411,17 @@ export function redactProjectCredentials(project: GeoLibreProject): CredentialRe if (Object.keys(rest).length > 0) dropped[id] = rest; } if (Object.keys(dropped).length > 0) { - redactedPaths.push("plugins.settings"); - redactedCount.value += countLeafValues(dropped); + recordRedaction(accumulator, "plugins.settings", dropped, countLeafValues(dropped)); } // What survives is still swept by the same pass layer configuration gets, // so a credentialed URL inside a kept blob is scrubbed rather than trusted. plugins = { ...plugins, manifestUrls, - settings: redactConfigurationValue( - kept, - "plugins.settings", - redactedPaths, - redactedCount, - ) as Record, + settings: redactConfigurationValue(kept, "plugins.settings", accumulator) as Record< + string, + unknown + >, }; } @@ -397,17 +437,16 @@ export function redactProjectCredentials(project: GeoLibreProject): CredentialRe ...(plugins ? { plugins } : {}), ...(project.metadata ? { - metadata: redactConfigurationValue( - project.metadata, - "metadata", - redactedPaths, - redactedCount, - ) as Record, + metadata: redactConfigurationValue(project.metadata, "metadata", accumulator) as Record< + string, + unknown + >, } : {}), }, - redactedPaths: [...new Set(redactedPaths)], - redactedCount: redactedCount.value, + redactedPaths: [...new Set(accumulator.paths)], + redactedFingerprints: [...new Set(accumulator.fingerprints)], + redactedCount: accumulator.count, }; } diff --git a/tests/project-credentials.test.ts b/tests/project-credentials.test.ts index 774fb4bf6d..9f1e2d3156 100644 --- a/tests/project-credentials.test.ts +++ b/tests/project-credentials.test.ts @@ -206,6 +206,29 @@ describe("project credential redaction", () => { assert.ok(redactedPaths.includes("layers[0].connection.lastError")); }); + it("fingerprints the credential values, not just their paths", () => { + // The save prompt reuses a remembered Keep only while the fingerprints + // match, so a different secret at an unchanged path has to change one. + const original = credentialProject(); + const baseline = redactProjectCredentials(original); + assert.equal(baseline.redactedFingerprints.length, baseline.redactedPaths.length); + assert.deepEqual( + redactProjectCredentials(credentialProject()).redactedFingerprints, + baseline.redactedFingerprints, + ); + assert.ok( + !baseline.redactedFingerprints.some((fingerprint) => fingerprint.includes("header-secret")), + ); + + const rotated = credentialProject(); + ( + rotated.layers[0].source.nested as { headers: { Authorization: string } } + ).headers.Authorization = "Bearer rotated-secret"; + const after = redactProjectCredentials(rotated); + assert.deepEqual(after.redactedPaths, baseline.redactedPaths); + assert.notDeepEqual(after.redactedFingerprints, baseline.redactedFingerprints); + }); + it("fails closed when configuration exceeds the traversal depth", () => { let nested: Record = { arbitrary: "too-deep-secret" }; for (let index = 0; index < 12; index += 1) nested = { child: nested }; diff --git a/tests/project-save-choices.test.ts b/tests/project-save-choices.test.ts index 372d0e826e..3392582716 100644 --- a/tests/project-save-choices.test.ts +++ b/tests/project-save-choices.test.ts @@ -23,7 +23,7 @@ describe("project save choices", () => { const remembered = rememberProjectSaveChoices(null, 4, { credentials: "keep", vectorData: "noembed", - largeEmbedWarningAcknowledged: true, + acknowledgedEmbedBytes: 60_000_000, }); assert.deepEqual(saveChoicesForProject(remembered, 5), { projectGeneration: 5 }); @@ -37,32 +37,74 @@ describe("project save choices", () => { }); it("retains a large-embed warning acknowledgement with the project choices", () => { + const warning = 50_000_000; const vectorChoice = rememberProjectSaveChoices(null, 4, { vectorData: "embed" }); - assert.equal(reusableVectorDataChoice(vectorChoice, false), "embed"); - assert.equal(reusableVectorDataChoice(vectorChoice, true), undefined); + assert.equal(reusableVectorDataChoice(vectorChoice, 1_000, warning), "embed"); + assert.equal(reusableVectorDataChoice(vectorChoice, warning, warning), undefined); const acknowledged = rememberProjectSaveChoices(vectorChoice, 4, { - largeEmbedWarningAcknowledged: true, + acknowledgedEmbedBytes: warning, }); assert.deepEqual(acknowledged, { projectGeneration: 4, vectorData: "embed", - largeEmbedWarningAcknowledged: true, + acknowledgedEmbedBytes: warning, }); - assert.equal(reusableVectorDataChoice(acknowledged, true), "embed"); + assert.equal(reusableVectorDataChoice(acknowledged, warning, warning), "embed"); }); - it("reconfirms Keep when the project gains more credential-bearing fields", () => { + it("re-warns when embedded data outgrows the acknowledged size", () => { + const warning = 50_000_000; + const acknowledged = rememberProjectSaveChoices(null, 4, { + vectorData: "embed", + acknowledgedEmbedBytes: 51_000_000, + }); + + // Ordinary growth within the acknowledged scale stays silent. + assert.equal(reusableVectorDataChoice(acknowledged, 80_000_000, warning), "embed"); + assert.equal(reusableVectorDataChoice(acknowledged, 102_000_000, warning), "embed"); + // An order-of-magnitude larger write is confirmed again. + assert.equal(reusableVectorDataChoice(acknowledged, 500_000_000, warning), undefined); + + // "noembed" writes no data, so its size never matters. + const noembed = rememberProjectSaveChoices(null, 4, { vectorData: "noembed" }); + assert.equal(reusableVectorDataChoice(noembed, 900_000_000, warning), "noembed"); + }); + + it("reconfirms Keep when the project would write a credential the user has not seen", () => { const keep = rememberProjectSaveChoices(null, 4, { credentials: "keep", - keptCredentialCount: 2, + keptCredentialFingerprints: ["layers[0].source.token=a1", "layers[1].source.token=b2"], }); - assert.equal(reusableCredentialChoice(keep, 2), "keep"); - assert.equal(reusableCredentialChoice(keep, 1), "keep"); - assert.equal(reusableCredentialChoice(keep, 3), undefined); + assert.equal(reusableCredentialChoice(keep, ["layers[0].source.token=a1"]), "keep"); + assert.equal( + reusableCredentialChoice(keep, ["layers[0].source.token=a1", "layers[1].source.token=b2"]), + "keep", + ); + // A third credential was never acknowledged. + assert.equal( + reusableCredentialChoice(keep, [ + "layers[0].source.token=a1", + "layers[1].source.token=b2", + "layers[2].source.token=c3", + ]), + undefined, + ); + // Swapping one credentialed layer for another leaves the count unchanged, + // but puts a secret on disk that the user never approved. + assert.equal( + reusableCredentialChoice(keep, ["layers[0].source.token=b2", "layers[1].source.token=c3"]), + undefined, + ); + // A rotated token at an acknowledged path is confirmed again too. + assert.equal(reusableCredentialChoice(keep, ["layers[0].source.token=z9"]), undefined); + + // Keep without a recorded acknowledgement cannot cover anything. + const bare = rememberProjectSaveChoices(null, 4, { credentials: "keep" }); + assert.equal(reusableCredentialChoice(bare, []), undefined); const strip = rememberProjectSaveChoices(null, 4, { credentials: "strip" }); - assert.equal(reusableCredentialChoice(strip, 10), "strip"); + assert.equal(reusableCredentialChoice(strip, ["basemapStyleUrl=q7"]), "strip"); }); }); From 6a564b18b10eb15467c786556739b98d684253ba Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 12:03:44 -0400 Subject: [PATCH 09/11] Address Claude review feedback - Reconfirm "Save without data" for layers the user never agreed to lose. A remembered noembed was reused unconditionally, so one click for a throwaway layer silently discarded every local vector layer added afterwards on the web build. The choice now records the layer ids it covered and re-asks when the save would drop one outside that set. Desktop discards nothing (it writes file references), so it stays silent. - Fail closed when a credential cannot be fingerprinted. A collision costs a missed confirmation rather than an extra one, so the lossy String(value) fallback (every unserializable value collapsing onto "[object Object]") is gone: such values are reported through hasUnfingerprintableCredential and force a fresh Keep confirmation. The digest is also 64 bits now rather than 32, and the comment that had the collision impact backwards is corrected. --- .../src/hooks/useProjectFileActions.ts | 32 ++++--- .../src/lib/project-save-choices.ts | 74 +++++++++++---- packages/core/src/credentials.ts | 54 ++++++++--- tests/project-credentials.test.ts | 18 ++++ tests/project-save-choices.test.ts | 90 ++++++++++++++----- 5 files changed, 206 insertions(+), 62 deletions(-) diff --git a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts index e37d3d26b9..bd858539ad 100644 --- a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts +++ b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts @@ -868,13 +868,17 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { saveChoicesRef.current = remembered; // A remembered Embed choice stays silent until the project crosses the // large-data warning threshold, and again whenever the data outgrows the - // size that was acknowledged. That material risk deserves a fresh - // confirmation even though the ordinary per-project choice is remembered. - const rememberedVectorChoice = reusableVectorDataChoice( - remembered, - bytes, - LARGE_EMBED_WARNING_BYTES, - ); + // size that was acknowledged. A remembered Save without data stays silent + // only for the layers whose data the user accepted losing. Both are + // material risks that deserve a fresh confirmation even though the ordinary + // per-project choice is remembered. On desktop that second case cannot + // arise: "without data" writes file references, so nothing is discarded. + const discardedLayerIds = isTauri() ? [] : [...embeddable.keys()]; + const rememberedVectorChoice = reusableVectorDataChoice(remembered, { + embedBytes: bytes, + warningBytes: LARGE_EMBED_WARNING_BYTES, + discardedLayerIds, + }); const choice = rememberedVectorChoice ?? (await askEmbedVectorData(count, bytes, isTauri(), state.projectGeneration)); @@ -895,6 +899,12 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { bytes >= LARGE_EMBED_WARNING_BYTES ? bytes : remembered.acknowledgedEmbedBytes, + // Likewise, only an answered prompt widens the set of layers the user + // has agreed to lose. + discardedVectorLayerIds: + rememberedVectorChoice === undefined && choice === "noembed" + ? discardedLayerIds + : remembered.discardedVectorLayerIds, }, ); @@ -997,10 +1007,10 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { if (redacted.redactedPaths.length > 0) { const remembered = saveChoicesForProject(saveChoicesRef.current, saveProjectGeneration); saveChoicesRef.current = remembered; - const rememberedCredentialChoice = reusableCredentialChoice( - remembered, - redacted.redactedFingerprints, - ); + const rememberedCredentialChoice = reusableCredentialChoice(remembered, { + fingerprints: redacted.redactedFingerprints, + hasUnfingerprintable: redacted.hasUnfingerprintableCredential, + }); const choice = rememberedCredentialChoice ?? (await askStripCredentials(redacted.redactedCount, saveProjectGeneration)); diff --git a/apps/geolibre-desktop/src/lib/project-save-choices.ts b/apps/geolibre-desktop/src/lib/project-save-choices.ts index e96b659cb2..20ddd36f87 100644 --- a/apps/geolibre-desktop/src/lib/project-save-choices.ts +++ b/apps/geolibre-desktop/src/lib/project-save-choices.ts @@ -21,6 +21,22 @@ export interface ProjectSaveChoices { vectorData?: VectorDataSaveChoice; /** Embedded size, in bytes, the user accepted after seeing the large-data warning. */ acknowledgedEmbedBytes?: number; + /** Layers whose data the user accepted discarding with an explicit Save without data. */ + discardedVectorLayerIds?: readonly string[]; +} + +/** What the current save would write, measured against a remembered choice. */ +export interface VectorDataSaveRisk { + /** Estimated size of the data this save would embed. */ + embedBytes: number; + /** Threshold at which the large-embed warning applies. */ + warningBytes: number; + /** + * Ids of local vector layers whose data this save would drop outright. Only + * the web build can lose data this way: on desktop "Save without data" writes + * file references that reload from disk, so nothing is discarded there. + */ + discardedLayerIds: readonly string[]; } /** @@ -61,28 +77,50 @@ export function rememberProjectSaveChoices( } /** - * Returns a remembered vector-data choice when no new size warning is needed. + * Returns a remembered vector-data choice when it still covers what this save + * would do. * - * An acknowledgement covers the size the user actually saw, plus the ordinary - * growth of a project being edited. Data that balloons past + * An Embed acknowledgement covers the size the user actually saw, plus the + * ordinary growth of a project being edited. Data that balloons past * {@link EMBED_REACKNOWLEDGE_GROWTH_FACTOR} times that size is a materially * different write and is confirmed again. * + * Save without data is reused only for the layers the user accepted losing. + * Dismissing the prompt for one throwaway layer must not silently discard a + * layer added afterwards, which is the one branch here that destroys data. + * * @param remembered - Choices scoped to the current project. - * @param embedBytes - Estimated size of the data this save would embed. - * @param warningBytes - Threshold at which the large-embed warning applies. - * @returns The reusable choice, or undefined when the user must confirm a large embed. + * @param risk - What the current save would embed or discard. + * @returns The reusable choice, or undefined when the user must confirm again. */ export function reusableVectorDataChoice( remembered: ProjectSaveChoices, - embedBytes: number, - warningBytes: number, + risk: VectorDataSaveRisk, ): VectorDataSaveChoice | undefined { - if (remembered.vectorData !== "embed" || embedBytes < warningBytes) return remembered.vectorData; - const acknowledged = remembered.acknowledgedEmbedBytes; - return acknowledged != null && embedBytes <= acknowledged * EMBED_REACKNOWLEDGE_GROWTH_FACTOR - ? remembered.vectorData - : undefined; + if (remembered.vectorData === "embed") { + if (risk.embedBytes < risk.warningBytes) return remembered.vectorData; + const acknowledged = remembered.acknowledgedEmbedBytes; + return acknowledged != null && + risk.embedBytes <= acknowledged * EMBED_REACKNOWLEDGE_GROWTH_FACTOR + ? remembered.vectorData + : undefined; + } + if (remembered.vectorData === "noembed") { + if (risk.discardedLayerIds.length === 0) return remembered.vectorData; + const acknowledged = new Set(remembered.discardedVectorLayerIds ?? []); + return risk.discardedLayerIds.every((id) => acknowledged.has(id)) + ? remembered.vectorData + : undefined; + } + return remembered.vectorData; +} + +/** Which credentials the current save would write, from the redaction pass. */ +export interface CredentialSaveRisk { + /** Fingerprints of the credentials this save would keep. */ + fingerprints: readonly string[]; + /** Whether any credential could not be fingerprinted, and so cannot be compared. */ + hasUnfingerprintable: boolean; } /** @@ -92,21 +130,23 @@ export function reusableVectorDataChoice( * reused only while every credential this save would write is one the user * explicitly accepted. Fingerprints rather than a count, because swapping one * credentialed layer for another leaves the count unchanged while putting a - * secret the user never saw on disk. + * secret the user never saw on disk. A credential that could not be + * fingerprinted cannot be shown to be unchanged, so it is confirmed again. * * @param remembered - Choices scoped to the current project. - * @param credentialFingerprints - Fingerprints of the credentials this save would keep. + * @param risk - The credentials this save would keep. * @returns The reusable choice, or undefined when Keep must be confirmed again. */ export function reusableCredentialChoice( remembered: ProjectSaveChoices, - credentialFingerprints: readonly string[], + risk: CredentialSaveRisk, ): CredentialSaveChoice | undefined { if (remembered.credentials !== "keep") return remembered.credentials; + if (risk.hasUnfingerprintable) return undefined; const acknowledged = remembered.keptCredentialFingerprints; if (acknowledged == null) return undefined; const covered = new Set(acknowledged); - return credentialFingerprints.every((fingerprint) => covered.has(fingerprint)) + return risk.fingerprints.every((fingerprint) => covered.has(fingerprint)) ? remembered.credentials : undefined; } diff --git a/packages/core/src/credentials.ts b/packages/core/src/credentials.ts index 41c988f737..415a310ab5 100644 --- a/packages/core/src/credentials.ts +++ b/packages/core/src/credentials.ts @@ -58,8 +58,8 @@ export interface CredentialRedactionResult { /** Stable project paths removed or rewritten by the redaction pass. */ redactedPaths: string[]; /** - * One opaque `path=hash` fingerprint per redacted path, identifying *which* - * secret sat there. A caller that remembers an explicit "keep credentials" + * Opaque `path=hash` fingerprints identifying *which* secret sat at each + * redacted path. A caller that remembers an explicit "keep credentials" * decision compares these between saves, so a different credential, such as * a swapped layer or rotated token, is confirmed again instead of being * written to disk under the earlier answer. Paths alone cannot carry that: @@ -67,6 +67,13 @@ export interface CredentialRedactionResult { * replaced. */ redactedFingerprints: string[]; + /** + * Whether any redacted value could not be serialized, and so has no entry in + * {@link redactedFingerprints}. Such a value cannot be compared with what the + * user accepted, so a caller reusing a remembered decision must ask again + * rather than assume the unfingerprintable credential is unchanged. + */ + hasUnfingerprintableCredential: boolean; /** Number of individual credential-bearing fields removed or rewritten. */ redactedCount: number; } @@ -76,30 +83,40 @@ interface RedactionAccumulator { paths: string[]; fingerprints: string[]; count: number; + /** Whether some redacted value could not be serialized, and so not fingerprinted. */ + unfingerprintable: boolean; } /** - * Hash a credential value to a short, stable token (FNV-1a). + * Hash a credential value to a short, stable token (two FNV-1a lanes, 64 bits). * * The digest never leaves memory and is only ever compared for equality, so it * needs to be stable and cheap rather than cryptographic. Hashing rather than * retaining the value keeps the secret itself out of the remembered choices. + * + * A collision costs a *missed* confirmation, not a spurious one: a caller + * comparing fingerprints would treat a different secret as one the user already + * accepted. That is why this is 64 bits rather than 32, and why a value that + * cannot be serialized returns null rather than hashing a lossy `String(value)` + * that would collapse every such value onto `[object Object]`. + * + * @returns The digest, or null when the value cannot be serialized. */ -function fingerprintValue(value: unknown): string { +function fingerprintValue(value: unknown): string | null { let serialized: string; try { serialized = JSON.stringify(value) ?? String(value); } catch { - // A circular or otherwise unserializable blob still needs a marker; that it - // collides with other unserializable blobs only costs an extra prompt. - serialized = String(value); + return null; } - let hash = 0x811c9dc5; + let low = 0x811c9dc5; + let high = 0x27220a95; for (let index = 0; index < serialized.length; index += 1) { - hash ^= serialized.charCodeAt(index); - hash = Math.imul(hash, 0x01000193); + const code = serialized.charCodeAt(index); + low = Math.imul(low ^ code, 0x01000193); + high = Math.imul(high ^ code, 0x85ebca6b); } - return (hash >>> 0).toString(36); + return `${(low >>> 0).toString(36)}.${(high >>> 0).toString(36)}`; } /** Record one redaction, with the fingerprint of the value being removed. */ @@ -109,8 +126,13 @@ function recordRedaction( value: unknown, count = 1, ): void { + const digest = fingerprintValue(value); accumulator.paths.push(path); - accumulator.fingerprints.push(`${path}=${fingerprintValue(value)}`); + // A value that cannot be serialized fails closed: it is reported through + // `hasUnfingerprintableCredential` rather than fingerprinted, so a caller + // asks again instead of reusing an answer it cannot verify still applies. + if (digest === null) accumulator.unfingerprintable = true; + else accumulator.fingerprints.push(`${path}=${digest}`); accumulator.count += count; } @@ -291,7 +313,12 @@ function countLeafValues(value: unknown): number { * themselves. */ export function redactProjectCredentials(project: GeoLibreProject): CredentialRedactionResult { - const accumulator: RedactionAccumulator = { paths: [], fingerprints: [], count: 0 }; + const accumulator: RedactionAccumulator = { + paths: [], + fingerprints: [], + count: 0, + unfingerprintable: false, + }; const basemapStyleUrl = typeof project.basemapStyleUrl === "string" ? redactUrlCredentials(project.basemapStyleUrl) @@ -446,6 +473,7 @@ export function redactProjectCredentials(project: GeoLibreProject): CredentialRe }, redactedPaths: [...new Set(accumulator.paths)], redactedFingerprints: [...new Set(accumulator.fingerprints)], + hasUnfingerprintableCredential: accumulator.unfingerprintable, redactedCount: accumulator.count, }; } diff --git a/tests/project-credentials.test.ts b/tests/project-credentials.test.ts index 9f1e2d3156..058bf9b0d3 100644 --- a/tests/project-credentials.test.ts +++ b/tests/project-credentials.test.ts @@ -229,6 +229,24 @@ describe("project credential redaction", () => { assert.notDeepEqual(after.redactedFingerprints, baseline.redactedFingerprints); }); + it("fails closed for a credential it cannot fingerprint", () => { + // A fingerprint match skips the Keep confirmation, so a value that cannot + // be serialized is reported rather than hashed into something that could + // collide with an unrelated one. + const project = credentialProject(); + assert.equal(redactProjectCredentials(project).hasUnfingerprintableCredential, false); + + const circular: Record = {}; + circular.self = circular; + project.layers[0].source = { token: circular }; + const result = redactProjectCredentials(project); + + assert.equal(result.hasUnfingerprintableCredential, true); + assert.ok(result.redactedPaths.includes("layers[0].source.token")); + assert.ok(!result.redactedFingerprints.some((entry) => entry.startsWith("layers[0].source."))); + assert.ok(!serializeProject(result.project).includes("self")); + }); + it("fails closed when configuration exceeds the traversal depth", () => { let nested: Record = { arbitrary: "too-deep-secret" }; for (let index = 0; index < 12; index += 1) nested = { child: nested }; diff --git a/tests/project-save-choices.test.ts b/tests/project-save-choices.test.ts index 3392582716..cbbbadd01f 100644 --- a/tests/project-save-choices.test.ts +++ b/tests/project-save-choices.test.ts @@ -38,9 +38,14 @@ describe("project save choices", () => { it("retains a large-embed warning acknowledgement with the project choices", () => { const warning = 50_000_000; + const risk = (embedBytes: number) => ({ + embedBytes, + warningBytes: warning, + discardedLayerIds: [], + }); const vectorChoice = rememberProjectSaveChoices(null, 4, { vectorData: "embed" }); - assert.equal(reusableVectorDataChoice(vectorChoice, 1_000, warning), "embed"); - assert.equal(reusableVectorDataChoice(vectorChoice, warning, warning), undefined); + assert.equal(reusableVectorDataChoice(vectorChoice, risk(1_000)), "embed"); + assert.equal(reusableVectorDataChoice(vectorChoice, risk(warning)), undefined); const acknowledged = rememberProjectSaveChoices(vectorChoice, 4, { acknowledgedEmbedBytes: warning, @@ -51,60 +56,103 @@ describe("project save choices", () => { vectorData: "embed", acknowledgedEmbedBytes: warning, }); - assert.equal(reusableVectorDataChoice(acknowledged, warning, warning), "embed"); + assert.equal(reusableVectorDataChoice(acknowledged, risk(warning)), "embed"); }); it("re-warns when embedded data outgrows the acknowledged size", () => { - const warning = 50_000_000; + const risk = (embedBytes: number) => ({ + embedBytes, + warningBytes: 50_000_000, + discardedLayerIds: [], + }); const acknowledged = rememberProjectSaveChoices(null, 4, { vectorData: "embed", acknowledgedEmbedBytes: 51_000_000, }); // Ordinary growth within the acknowledged scale stays silent. - assert.equal(reusableVectorDataChoice(acknowledged, 80_000_000, warning), "embed"); - assert.equal(reusableVectorDataChoice(acknowledged, 102_000_000, warning), "embed"); + assert.equal(reusableVectorDataChoice(acknowledged, risk(80_000_000)), "embed"); + assert.equal(reusableVectorDataChoice(acknowledged, risk(102_000_000)), "embed"); // An order-of-magnitude larger write is confirmed again. - assert.equal(reusableVectorDataChoice(acknowledged, 500_000_000, warning), undefined); + assert.equal(reusableVectorDataChoice(acknowledged, risk(500_000_000)), undefined); + }); + + it("reconfirms Save without data for a layer the user has not agreed to lose", () => { + const risk = (discardedLayerIds: string[]) => ({ + embedBytes: 900_000_000, + warningBytes: 50_000_000, + discardedLayerIds, + }); + const noembed = rememberProjectSaveChoices(null, 4, { + vectorData: "noembed", + discardedVectorLayerIds: ["scratch"], + }); + + // The size of data that is never written does not matter. + assert.equal(reusableVectorDataChoice(noembed, risk(["scratch"])), "noembed"); + assert.equal(reusableVectorDataChoice(noembed, risk([])), "noembed"); + // A layer added after the choice would be discarded without ever being + // mentioned, so the prompt comes back. + assert.equal(reusableVectorDataChoice(noembed, risk(["scratch", "survey"])), undefined); + assert.equal(reusableVectorDataChoice(noembed, risk(["survey"])), undefined); - // "noembed" writes no data, so its size never matters. - const noembed = rememberProjectSaveChoices(null, 4, { vectorData: "noembed" }); - assert.equal(reusableVectorDataChoice(noembed, 900_000_000, warning), "noembed"); + // Desktop discards nothing (it writes file references), so it stays silent. + const bare = rememberProjectSaveChoices(null, 4, { vectorData: "noembed" }); + assert.equal(reusableVectorDataChoice(bare, risk([])), "noembed"); + assert.equal(reusableVectorDataChoice(bare, risk(["survey"])), undefined); }); it("reconfirms Keep when the project would write a credential the user has not seen", () => { + const risk = (fingerprints: string[]) => ({ fingerprints, hasUnfingerprintable: false }); const keep = rememberProjectSaveChoices(null, 4, { credentials: "keep", keptCredentialFingerprints: ["layers[0].source.token=a1", "layers[1].source.token=b2"], }); - assert.equal(reusableCredentialChoice(keep, ["layers[0].source.token=a1"]), "keep"); + assert.equal(reusableCredentialChoice(keep, risk(["layers[0].source.token=a1"])), "keep"); assert.equal( - reusableCredentialChoice(keep, ["layers[0].source.token=a1", "layers[1].source.token=b2"]), + reusableCredentialChoice( + keep, + risk(["layers[0].source.token=a1", "layers[1].source.token=b2"]), + ), "keep", ); // A third credential was never acknowledged. assert.equal( - reusableCredentialChoice(keep, [ - "layers[0].source.token=a1", - "layers[1].source.token=b2", - "layers[2].source.token=c3", - ]), + reusableCredentialChoice( + keep, + risk([ + "layers[0].source.token=a1", + "layers[1].source.token=b2", + "layers[2].source.token=c3", + ]), + ), undefined, ); // Swapping one credentialed layer for another leaves the count unchanged, // but puts a secret on disk that the user never approved. assert.equal( - reusableCredentialChoice(keep, ["layers[0].source.token=b2", "layers[1].source.token=c3"]), + reusableCredentialChoice( + keep, + risk(["layers[0].source.token=b2", "layers[1].source.token=c3"]), + ), undefined, ); // A rotated token at an acknowledged path is confirmed again too. - assert.equal(reusableCredentialChoice(keep, ["layers[0].source.token=z9"]), undefined); + assert.equal(reusableCredentialChoice(keep, risk(["layers[0].source.token=z9"])), undefined); + // So is a credential that could not be fingerprinted at all. + assert.equal( + reusableCredentialChoice(keep, { + fingerprints: ["layers[0].source.token=a1"], + hasUnfingerprintable: true, + }), + undefined, + ); // Keep without a recorded acknowledgement cannot cover anything. const bare = rememberProjectSaveChoices(null, 4, { credentials: "keep" }); - assert.equal(reusableCredentialChoice(bare, []), undefined); + assert.equal(reusableCredentialChoice(bare, risk([])), undefined); const strip = rememberProjectSaveChoices(null, 4, { credentials: "strip" }); - assert.equal(reusableCredentialChoice(strip, ["basemapStyleUrl=q7"]), "strip"); + assert.equal(reusableCredentialChoice(strip, risk(["basemapStyleUrl=q7"])), "strip"); }); }); From a6f255e9d13fa426e8149b04f36eb09ce442829c Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 12:16:04 -0400 Subject: [PATCH 10/11] Address Claude review feedback - Describe the fingerprint hash accurately. The second lane uses a MurmurHash3 mixing constant, not the FNV prime, so calling both lanes FNV-1a was wrong. Comment only, no behavior change. --- packages/core/src/credentials.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/src/credentials.ts b/packages/core/src/credentials.ts index 415a310ab5..2522ba20e4 100644 --- a/packages/core/src/credentials.ts +++ b/packages/core/src/credentials.ts @@ -88,7 +88,9 @@ interface RedactionAccumulator { } /** - * Hash a credential value to a short, stable token (two FNV-1a lanes, 64 bits). + * Hash a credential value to a short, stable 64-bit token: two independent + * FNV-1a-style lanes, the first with the canonical FNV prime and the second + * with a MurmurHash3 mixing constant so the lanes do not move together. * * The digest never leaves memory and is only ever compared for equality, so it * needs to be stable and cheap rather than cryptographic. Hashing rather than From ce59232785a73b44eab42cd4d0609b0c0499fbf3 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 13:09:47 -0400 Subject: [PATCH 11/11] Address Claude review feedback - Cancel stale save prompts in `useLayoutEffect` instead of `useEffect`, so a dialog belonging to a replaced project is gone in the same commit that swapped the project rather than lingering for one paint. - Extract `settleCredentialStripPrompt` / `settleEmbedVectorDataPrompt` / `settleSaveNamePrompt` so the resolve-and-clear pattern lives in one place. The dialog handlers and the generation-change cancellation now share it and cannot drift apart; the helpers close over only their setters, so the effect still re-runs only when a prompt or the generation changes. --- .../src/hooks/useProjectFileActions.ts | 75 ++++++++++++------- 1 file changed, 48 insertions(+), 27 deletions(-) diff --git a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts index bd858539ad..539b73a67a 100644 --- a/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts +++ b/apps/geolibre-desktop/src/hooks/useProjectFileActions.ts @@ -16,7 +16,7 @@ import { materializeEmbeddableVectorLayers, } from "@geolibre/plugins"; import type { FeatureCollection } from "geojson"; -import { type FormEvent, useEffect, useRef, useState } from "react"; +import { type FormEvent, useCallback, useLayoutEffect, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { createAppAPI, getPluginManager } from "./usePlugins"; import { pluginManifestUrlsForIds } from "../lib/external-plugins"; @@ -287,24 +287,56 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { // call's unresolved promise. const isSavingRef = useRef(false); + // Settling a prompt means resolving its promise and clearing the dialog + // state. Each pattern lives here once so the dialog handlers further down and + // the generation-change cancellation below cannot drift apart. They close over + // nothing but their setters, so their identity is stable and the effect below + // still re-runs only when a prompt or the generation changes. + const settleCredentialStripPrompt = useCallback( + (prompt: CredentialStripPrompt | null, choice: "strip" | "keep" | "cancel") => { + // Resolve outside the state updater (updaters must be side-effect free). + prompt?.resolve(choice); + setCredentialStripPrompt(null); + }, + [], + ); + const settleEmbedVectorDataPrompt = useCallback( + (prompt: EmbedVectorDataPrompt | null, choice: "embed" | "noembed" | "cancel") => { + prompt?.resolve(choice); + setEmbedVectorDataPrompt(null); + }, + [], + ); + const settleSaveNamePrompt = useCallback((prompt: SaveNamePrompt | null, name: string | null) => { + prompt?.resolve(name); + setSaveNamePrompt(null); + setSaveNameInput(""); + }, []); + // A project can be replaced by an external open action while a modal save // prompt is visible. Cancel the stale promise immediately so its dialog does // not cover the replacement project and its save guard is released. - useEffect(() => { + // useLayoutEffect (not useEffect) so the stale dialog is gone in the same + // commit that swapped the project, rather than lingering for one paint. + useLayoutEffect(() => { if (credentialStripPrompt && credentialStripPrompt.projectGeneration !== projectGeneration) { - credentialStripPrompt.resolve("cancel"); - setCredentialStripPrompt(null); + settleCredentialStripPrompt(credentialStripPrompt, "cancel"); } if (embedVectorDataPrompt && embedVectorDataPrompt.projectGeneration !== projectGeneration) { - embedVectorDataPrompt.resolve("cancel"); - setEmbedVectorDataPrompt(null); + settleEmbedVectorDataPrompt(embedVectorDataPrompt, "cancel"); } if (saveNamePrompt && saveNamePrompt.projectGeneration !== projectGeneration) { - saveNamePrompt.resolve(null); - setSaveNamePrompt(null); - setSaveNameInput(""); + settleSaveNamePrompt(saveNamePrompt, null); } - }, [credentialStripPrompt, embedVectorDataPrompt, projectGeneration, saveNamePrompt]); + }, [ + credentialStripPrompt, + embedVectorDataPrompt, + projectGeneration, + saveNamePrompt, + settleCredentialStripPrompt, + settleEmbedVectorDataPrompt, + settleSaveNamePrompt, + ]); const handleOpenFromFile = async () => { const result = await openProjectFile(); @@ -766,11 +798,8 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { setCredentialStripPrompt({ count, projectGeneration: promptProjectGeneration, resolve }); }); - const resolveCredentialStripPrompt = (choice: "strip" | "keep" | "cancel") => { - // Resolve outside the state updater (updaters must be side-effect free). - credentialStripPrompt?.resolve(choice); - setCredentialStripPrompt(null); - }; + const resolveCredentialStripPrompt = (choice: "strip" | "keep" | "cancel") => + settleCredentialStripPrompt(credentialStripPrompt, choice); // Ask whether to embed local vector layers' data in the saved file. Resolves // when the user picks an option in the dialog. @@ -790,10 +819,8 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { }); }); - const resolveEmbedVectorDataPrompt = (choice: "embed" | "noembed" | "cancel") => { - embedVectorDataPrompt?.resolve(choice); - setEmbedVectorDataPrompt(null); - }; + const resolveEmbedVectorDataPrompt = (choice: "embed" | "noembed" | "cancel") => + settleEmbedVectorDataPrompt(embedVectorDataPrompt, choice); // Builds the embed-mode layers: every local vector layer carries its own // features so the project is self-contained (portable to another machine or @@ -972,16 +999,10 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { const submitSaveNamePrompt = (event?: FormEvent) => { event?.preventDefault(); - saveNamePrompt?.resolve(saveNameInput); - setSaveNamePrompt(null); - setSaveNameInput(""); + settleSaveNamePrompt(saveNamePrompt, saveNameInput); }; - const cancelSaveNamePrompt = () => { - saveNamePrompt?.resolve(null); - setSaveNamePrompt(null); - setSaveNameInput(""); - }; + const cancelSaveNamePrompt = () => settleSaveNamePrompt(saveNamePrompt, null); const runSaveProject = async (options?: { saveAs?: boolean }): Promise => { const saveProjectGeneration = useAppStore.getState().projectGeneration;