diff --git a/servers/gateway/boot/post-listen.js b/servers/gateway/boot/post-listen.js index da7f4d0a9..182f147bd 100644 --- a/servers/gateway/boot/post-listen.js +++ b/servers/gateway/boot/post-listen.js @@ -343,6 +343,27 @@ export async function runPostListenSetup(server, app, deps) { }); } + // Redact local-only columns out of stored conflict snapshots (security audit + // N1b-7, fix round 2). A providers conflict saved before snapshots were + // stripped holds this device's api_key in winning_data (and possibly an old + // peer's in losing_data), and the settings page renders both in full. One-time + // and idempotent: it rewrites only rows that still carry a local-only column. + // Runs on every gateway, --no-auth included: the secret is in the LOCAL db, + // whether or not this instance serves a dashboard. Own db client, closed here, + // never throws, one log line with the count. + setImmediate(async () => { + let db; + try { + const { redactConflictSnapshotsAtBoot } = await import("../../sharing/sync-conflict-redact.js"); + db = createDbClient(); + await redactConflictSnapshotsAtBoot(db); + } catch (err) { + console.warn("[sync-conflicts] key redaction skipped:", err?.message || err); + } finally { + try { db?.close?.(); } catch {} + } + }); + // Start auto-update checker startAutoUpdate(createDbClient(), { noAuth }).catch((err) => { console.error("[auto-update] Failed to start:", err.message); diff --git a/servers/gateway/dashboard/settings/sections/llm/providers-tab.js b/servers/gateway/dashboard/settings/sections/llm/providers-tab.js index 6ae05bd90..3b2a997b1 100644 --- a/servers/gateway/dashboard/settings/sections/llm/providers-tab.js +++ b/servers/gateway/dashboard/settings/sections/llm/providers-tab.js @@ -53,6 +53,7 @@ import { } from "../../../../../shared/cloud-consent.js"; import { readLocalMarks, markLocal, unmarkLocal, isMarkableRow, isMarkCurrent, localMarkTokenFor, ownBoxMarkTokenFor } from "../../../../../shared/provider-local-marks.js"; import { readOwnBoxMarks, markOwnBox, unmarkOwnBox, isOwnBoxMarkableRow, isOwnBoxMarkCurrent } from "../../../../../shared/provider-local-marks.js"; +import { providerNeedsKey } from "../../../../../shared/provider-key-need.js"; const BACK = "?section=llm&tab=providers"; @@ -202,6 +203,23 @@ export function localMarkSwitch(p, { localMarks, salt, ownAddrs, lang = "en" }) `; } +/** + * "Needs an API key on this device." for a row that arrived from a paired + * device and holds no key here; "" otherwise (operator ruling 2026-11-05). + * Keys are never shared, so a synced row lands keyless — and this is the only + * place the operator is told, beside the row it applies to. Not for a row this + * instance wrote itself (it cannot be keyless by accident), not for a keyless + * provider (a llama.cpp on the LAN, provider-key-need.js), and not while a key + * is set here. + */ +export function needsKeyLine(p, { ownInstanceId, lang = "en" } = {}) { + if (!p || p.disabled || !ownInstanceId) return ""; + if (p.instance_id == null || p.instance_id === ownInstanceId) return ""; + if (!providerNeedsKey(p)) return ""; + if (typeof p.apiKey === "string" && p.apiKey.trim() !== "") return ""; + return `
${escapeHtml(t("settings.providers.needsKeyHere", lang))}
`; +} + /** markLocal refusals with their own line (?local=). */ const LOCAL_REFUSALS = new Set(["ip_literal", "public_address", "flagged_cloud", "known_cloud"]); @@ -360,7 +378,7 @@ export default { return ` ${statusDot(p, { external, lang })} ${idEsc}${dupBadge} - ${hostBadge(p, hostCtx)}${engineBadge(p, lang)}${botsSwitch(p, consentCtx)}${localMarkSwitch(p, localCtx)}${ownBoxSwitch(p, ownBoxCtx)} + ${hostBadge(p, hostCtx)}${engineBadge(p, lang)}${botsSwitch(p, consentCtx)}${localMarkSwitch(p, localCtx)}${ownBoxSwitch(p, ownBoxCtx)}${needsKeyLine(p, { ownInstanceId: hostCtx.ownInstanceId, lang })} ${escapeHtml(p.baseUrl || "—")} ${models} ${usedByCell(p, usedCtx)} @@ -428,6 +446,8 @@ export default { .llm-cell-id .llm-pill { margin-top:2px; } } .llm-bots-switch { margin-top:6px; } + .llm-needs-key { margin-top:6px; font-size:0.75rem; color:var(--crow-warning, #e0a000); } + .llm-key-note { font-size:0.72rem; color:var(--crow-text-muted); margin-top:4px; } .llm-bots-changed { margin:0 0 0.85rem; padding:0.6rem 0.8rem; border-left:3px solid var(--crow-error); background:var(--crow-bg-elevated); border-radius:4px; font-size:0.85rem; } .llm-bots-switch button { display:inline-flex; align-items:center; gap:8px; min-height:44px; padding:0 4px; @@ -519,9 +539,6 @@ export default { Add cloud provider
-
- Note: this API key will sync to your paired Crow instances (local-lab threat model). A per-row local-only toggle is planned for v2. -
@@ -539,6 +556,7 @@ export default {
+
${escapeHtml(t("settings.providers.keyStaysLocal", lang))}
diff --git a/servers/gateway/dashboard/settings/sections/sync-conflicts.js b/servers/gateway/dashboard/settings/sections/sync-conflicts.js index dcd489a3a..d757dd784 100644 --- a/servers/gateway/dashboard/settings/sections/sync-conflicts.js +++ b/servers/gateway/dashboard/settings/sections/sync-conflicts.js @@ -24,12 +24,20 @@ */ import { escapeHtml } from "../../shared/components.js"; -import { t } from "../../shared/i18n.js"; +import { fill, t } from "../../shared/i18n.js"; +import { + providerEndpointChanged, + providerEndpointKey, + providerOrigin, + clearProviderEndpointHeldNotice, + clearAllProviderEndpointHeldNotices, +} from "../../../../shared/provider-endpoint.js"; import { resolveConflict, restoreConflict, NATURAL_KEY_RESTORE_TABLES, } from "../../../../sharing/sync-conflict-resolve.js"; +import { stripConflictSnapshot } from "../../../../sharing/instance-sync.js"; import { getInstanceSyncManager } from "../../../../sharing/server.js"; import { EXPIRE_DAYS_DEFAULT, @@ -68,12 +76,71 @@ function opBadge(op) { return `${escapeHtml(op || "update")}`; } -function prettyJson(raw) { +/** + * A held providers conflict is one this device refused to apply because the + * peer's row MOVES the endpoint while the API key lives here (operator ruling + * 2026-11-06; instance-sync.js _holdProviderEndpointChange). Show the operator + * the one fact that matters before they press Restore. Every part is escaped. + * + * Only unresolved rows render it: a resolved row already had the decision made. + */ +function addressChangeLine(r, lang) { + let win; + let lose; + try { + win = JSON.parse(stripConflictSnapshot(r.table_name, r.winning_data)); + lose = JSON.parse(stripConflictSnapshot(r.table_name, r.losing_data)); + } catch { + return ""; + } + const before = win?.base_url; + const after = lose?.base_url; + if (!providerEndpointChanged(before, after)) return ""; + // The origin is what an operator recognises… except for a PATH-only move, + // where both origins read the same; then show the address itself so the line + // never says "https://x → https://x". + const beforeOrigin = providerOrigin(before); + const afterOrigin = providerOrigin(after); + const from = beforeOrigin === afterOrigin + ? (providerEndpointKey(before) || String(before ?? "")) : beforeOrigin; + const to = beforeOrigin === afterOrigin + ? (providerEndpointKey(after) || String(after ?? "")) : afterOrigin; + return `
+ ${escapeHtml(fill(t("syncConflicts.addressChange", lang), { from, to }))} +
`; +} + +/** Quiet the "another device wants to change its address" bell item once the + * operator has decided this conflict (kept the local address, or accepted it). */ +async function clearHoldNotice(db, conflictId) { + try { + const { rows } = await db.execute({ + sql: `SELECT row_id FROM sync_conflicts WHERE id = ?`, + args: [String(conflictId)], + }); + if (rows[0]) await clearProviderEndpointHeldNotice(db, String(rows[0].row_id)); + } catch { + // Best effort: the notice is a bell item, never a reason to fail the action. + } +} + +/** + * A stored snapshot as pretty JSON for the page, with `table`'s local-only + * columns stripped FIRST (instance-sync.js LOCAL_ONLY_COLUMNS): a pre-fix + * providers conflict row still holds this device's `api_key`, and this page is + * the only place it would be shown in full. A JSON null snapshot (`'null'`, an + * old row whose side was gone) and a blob we cannot parse render as "(none)" — + * an unreadable snapshot is exactly the one we cannot certify as redacted. + */ +function prettyJson(table, raw) { if (!raw) return "(none)"; + const stripped = stripConflictSnapshot(table, raw); try { - return JSON.stringify(JSON.parse(raw), null, 2); + const parsed = typeof stripped === "string" ? JSON.parse(stripped) : stripped; + if (!parsed || typeof parsed !== "object") return "(none)"; + return JSON.stringify(parsed, null, 2); } catch { - return String(raw); + return "(none)"; } } @@ -133,8 +200,8 @@ function renderConflictRow(r, lang, csrfToken) { const isNaturalKey = NATURAL_KEY_RESTORE_TABLES.has(r.table_name); const resolved = !!r.resolved; - const winJson = prettyJson(r.winning_data); - const loseJson = prettyJson(r.losing_data); + const winJson = prettyJson(r.table_name, r.winning_data); + const loseJson = prettyJson(r.table_name, r.losing_data); // Restore button is disabled for op='insert' conflicts per spec §6 // (precedence FIRST — matches the backend's refusal order), then for @@ -197,6 +264,7 @@ function renderConflictRow(r, lang, csrfToken) { ${escapeHtml(when)} ${opBadge(r.op)} + ${r.table_name === "providers" ? addressChangeLine(r, lang) : ""}
${escapeHtml(t("syncConflicts.showData", lang))} @@ -459,6 +527,7 @@ export default { } await resolveConflict(db, String(conflictId)); await markResolvedVia(db, conflictId, "operator"); + await clearHoldNotice(db, conflictId); res.redirectAfterPost(conflictPage); return true; } @@ -469,7 +538,10 @@ export default { return true; } const outcome = await restoreConflict(db, String(conflictId), { instanceSync }); - if (outcome.status === "applied") await markResolvedVia(db, conflictId, "operator"); + if (outcome.status === "applied") { + await markResolvedVia(db, conflictId, "operator"); + await clearHoldNotice(db, conflictId); + } // Pass only the status enum; render() maps it to an i18n'd flash. const status = ["applied", "stale", "refused"].includes(outcome.status) ? outcome.status @@ -485,6 +557,7 @@ export default { sql: `UPDATE sync_conflicts SET resolved = 1, resolved_at = datetime('now')${via} WHERE resolved = 0`, args: [], }); + await clearAllProviderEndpointHeldNotices(db); res.redirectAfterPost(conflictPage); return true; } diff --git a/servers/gateway/dashboard/shared/i18n.js b/servers/gateway/dashboard/shared/i18n.js index 0e384f4f0..4564298cc 100644 --- a/servers/gateway/dashboard/shared/i18n.js +++ b/servers/gateway/dashboard/shared/i18n.js @@ -2598,6 +2598,11 @@ export const translations = { "settings.providers.introTail": { en: "Bundle providers auto-appear on install. Instance-sync replicates this table across paired Crow instances.", es: "Los proveedores de paquetes aparecen solos al instalar. La sincronización entre instancias replica esta tabla en las instancias Crow emparejadas." }, "settings.providers.noneEnabled": { en: "No enabled providers", es: "Ningún proveedor activado" }, "settings.providers.noneRegistered": { en: "No providers registered.", es: "No hay proveedores registrados." }, + // Security audit N1b-7 (operator ruling 2026-11-05): provider rows sync, their + // API keys never do — said where the key is typed, and on a row that arrived + // from a paired device without one. + "settings.providers.keyStaysLocal": { en: "Keys stay on this device; they are not shared with paired devices.", es: "Las claves se quedan en este dispositivo; no se comparten con los dispositivos emparejados." }, + "settings.providers.needsKeyHere": { en: "Needs an API key on this device.", es: "Necesita una clave API en este dispositivo." }, "settings.roles.coordinator": { en: "Orchestrator", es: "Orquestador" }, "settings.roles.quick": { en: "Scout", es: "Explorador" }, "settings.roles.deep": { en: "Planner", es: "Planificador" }, @@ -2736,6 +2741,15 @@ export const translations = { "providers.ownBoxRefused.known_cloud": { en: "This address belongs to a known cloud provider, so it isn't your other box.", es: "Esta dirección pertenece a un proveedor en la nube conocido, así que no es tu otro equipo." }, "providers.ownBoxRefused.here": { en: "This address is this box, so it can't be your other box.", es: "Esta dirección es este equipo, así que no puede ser tu otro equipo." }, "providers.localNotice": { en: "These providers are no longer treated as local for voice and phone calls until you mark them \"This is local\": {names}", es: "Estos proveedores ya no se consideran locales para la voz y las llamadas hasta que los marques como \"Esto es local\": {names}" }, + // Raised ONCE per provider id when a paired device's keyless provider row + // lands here (provider-key-need.js). Keys never sync, so it must be entered here. + "providers.needsKeyTitle": { en: "{name} needs an API key on this device", es: "{name} necesita una clave API en este dispositivo" }, + "providers.needsKeyBody": { en: "This provider came from a paired device, and API keys are never shared. Enter a key here to use it.", es: "Este proveedor vino de un dispositivo emparejado y las claves API nunca se comparten. Introduce una clave aquí para usarlo." }, + // Raised ONCE per provider id when a paired device moves a provider's ENDPOINT + // while this device holds that provider's key: the change is held for review + // (provider-endpoint.js + instance-sync.js _holdProviderEndpointChange). + "providers.endpointHoldTitle": { en: "{name}: another device wants to change its address", es: "{name}: otro dispositivo quiere cambiar su dirección" }, + "providers.endpointHoldBody": { en: "Review it before your key is sent there — accepting sends this device's API key to the new address.", es: "Revísalo antes de que se envíe tu clave allí — aceptar envía la clave API de este dispositivo a la nueva dirección." }, "models.add.botsFailed": { en: "Bots can't use it yet. To allow them, open Models, then Advanced, then Providers, and turn on \"Let bots use this provider\".", es: "Los bots aún no pueden usarlo. Para permitirlo, abre Modelos, luego Avanzado, luego Proveedores, y activa \"Permitir que los bots usen este proveedor\"." }, "models.add.test.ok": { en: "That key works. {n} models available.", es: "Esa clave funciona. {n} modelos disponibles." }, "models.add.test.key_rejected": { en: "{provider} did not accept that key. Check that you copied all of it.", es: "{provider} no aceptó esa clave. Comprueba que la copiaste completa." }, @@ -3453,6 +3467,12 @@ export const translations = { "syncConflicts.msgStale": { en: "This item has changed since the conflict was recorded — please review the current version and confirm again.", es: "Este elemento ha cambiado desde que se registró el conflicto — revisa la versión actual y confirma de nuevo." }, "syncConflicts.msgRefused": { en: "This version can't be restored automatically; its data remains visible below.", es: "Esta versión no se puede restaurar automáticamente; sus datos siguen visibles abajo." }, "syncConflicts.msgFailed": { en: "Restore failed — the conflict stays open and its data remains visible below.", es: "La restauración falló — el conflicto sigue abierto y sus datos siguen visibles abajo." }, + // A held providers conflict: the peer moved the endpoint and this device holds + // the API key (operator ruling 2026-11-06), so it waited for this decision. + "syncConflicts.addressChange": { + en: "Address change: {from} → {to}. Accepting sends this device's API key to the new address.", + es: "Cambio de dirección: {from} → {to}. Aceptar envía la clave API de este dispositivo a la nueva dirección.", + }, // ─── Nav Groups Settings ─── "navGroups.description": { en: "Organize your sidebar by creating groups and moving panels between them.", es: "Organiza tu barra lateral creando grupos y moviendo paneles entre ellos." }, diff --git a/servers/shared/provider-endpoint.js b/servers/shared/provider-endpoint.js new file mode 100644 index 000000000..7332fd80a --- /dev/null +++ b/servers/shared/provider-endpoint.js @@ -0,0 +1,205 @@ +/** + * A provider's ENDPOINT — where this device would send its API key. + * + * Operator ruling (2026-11-06): an inbound sync change that moves a provider's + * endpoint, when this device holds a key for that provider, is HELD as a + * proposed change until the operator accepts it here. Without that, a + * compromised peer points "openai" at its own server and this device hands over + * the key it holds (security audit N1b-7, out-of-scope item 2). + * + * Two questions live here, so the hold decision and the conflict-review UI + * cannot disagree about what "the same endpoint" means: + * + * 1. providerEndpointKey(url) — the comparable identity of an address: + * scheme, host, port and path, with a trailing slash ignored. Pure. + * 2. providerEndpointChanged(before, after) — did the wire row move it? Pure. + * 3. providerOrigin(url) — the scheme://host:port shown to the operator. + * 4. noticeProviderEndpointHeld(db, row, heldConflictId) — one bell item per + * OPEN held conflict, by name, linking to the conflict review; and + * clearProviderEndpointHeldNotice(db, id) — quiet it once the operator has + * decided. + * + * Everything is per-DEVICE: the key never syncs (instance-sync.js's + * LOCAL_ONLY_COLUMNS), so "this device holds a key" is a local question, and + * the notice is a local notification (never emitted to a peer). + */ +import { providerDisplayName } from "./provider-key-need.js"; + +/** The conflict-review page a held endpoint change is decided on. */ +export const SYNC_CONFLICTS_URL = "/dashboard/settings?section=sync-conflicts"; + +/** + * The comparable identity of an endpoint: `origin + path`, trailing slash + * ignored (so `https://api.openai.com/v1/` and `https://api.openai.com/v1` are + * the same address). A port is part of it (it is part of `origin`), a query or + * fragment is not — nothing dials those. + * + * A string that is not a URL at all (a bare host, junk) is compared as written, + * also without a trailing slash: the operator's own row can hold one, and a + * peer changing it is still a change. + * + * @param {unknown} url + * @returns {string|null} null when there is no address at all + */ +export function providerEndpointKey(url) { + if (typeof url !== "string") return null; + const raw = url.trim(); + if (raw === "") return null; + try { + const u = new URL(raw); + return `${u.origin}${u.pathname.replace(/\/+$/, "")}`; + } catch { + return raw.replace(/\/+$/, ""); + } +} + +/** + * Did an inbound row move the endpoint? Pure. + * + * Only a row that CARRIES an address can move it: a models-only update (no + * `base_url` key, or an empty one) is never an endpoint change, which is what + * keeps ordinary edits flowing. + * + * @param {unknown} before — the local row's base_url + * @param {unknown} after — the incoming row's base_url + * @returns {boolean} + */ +export function providerEndpointChanged(before, after) { + const next = providerEndpointKey(after); + if (next === null) return false; + return providerEndpointKey(before) !== next; +} + +/** + * `scheme://host[:port]` for the operator-facing line — the part they can + * recognise at a glance. Falls back to the address as written. + * @param {unknown} url + * @returns {string} + */ +export function providerOrigin(url) { + if (typeof url !== "string") return ""; + const raw = url.trim(); + if (raw === "") return ""; + try { + return new URL(raw).origin; + } catch { + return raw.replace(/\/+$/, ""); + } +} + +/** The notification `source` for one provider id — the per-id dedupe key. */ +export function endpointHoldNoticeSource(id) { + return `providers:endpoint-held:${String(id)}`; +} + +/** + * The notice metadata key holding the id of the held conflict it announced. The + * bell is owed once per OPEN held conflict (fix round 2): a re-delivery of the + * same held change — read or dismissed — is silent, while a NEW held conflict + * formed after the previous one was resolved may ring again. + */ +export const HELD_CONFLICT_METADATA_KEY = "heldConflictId"; + +/** The held conflict id a notice was raised for, or null (pre-upgrade notice). Pure. */ +function heldConflictOf(metadataJson) { + if (!metadataJson) return null; + try { + const id = JSON.parse(metadataJson)?.[HELD_CONFLICT_METADATA_KEY]; + return Number.isFinite(Number(id)) ? Number(id) : null; + } catch { + return null; + } +} + +async function readLang(db) { + try { + const { rows } = await db.execute({ + sql: "SELECT value FROM dashboard_settings WHERE key = 'language'", + args: [], + }); + return rows[0]?.value === "es" ? "es" : "en"; + } catch { + return "en"; + } +} + +/** + * Ring the bell ONCE per OPEN held conflict: "another device wants to change its + * address". A newer held change for the same provider REPLACES the open conflict + * rather than adding one (instance-sync.js), so the same conflict id is in hand + * and it must not ring again — even after the operator read or dismissed the + * notice. Only a NEW held conflict (a new sync_conflicts row, made after the + * previous one was resolved) rings again. + * + * @param {object} db + * @param {object} row — this device's providers row + * @param {number|null} [heldConflictId] — the open sync_conflicts row this hold + * created; without it the notice falls back to the coarser per-provider dedupe + * (one open notice at a time). + * @returns {Promise<"raised"|"already"|"filtered">} + */ +export async function noticeProviderEndpointHeld(db, row, heldConflictId = null) { + const id = row?.id; + if (typeof id !== "string" || id === "") return "already"; + const source = endpointHoldNoticeSource(id); + const conflictId = Number.isFinite(Number(heldConflictId)) ? Number(heldConflictId) : null; + const { rows: prior } = await db.execute({ + sql: "SELECT id, metadata, is_read, is_dismissed FROM notifications WHERE source = ?", + args: [source], + }); + const alreadyRang = prior.some((n) => { + const recorded = heldConflictOf(n.metadata); + // A notice raised before this round carries no conflict id: it can only be + // attributed to the held change it named while it is still open. + if (recorded === null) return Number(n.is_read) === 0 && Number(n.is_dismissed) === 0; + return conflictId !== null && recorded === conflictId; + }); + if (alreadyRang) return "already"; + + // Loaded lazily: notifications.js and the i18n table are gateway-facing, and + // this runs from the sync manager (which the stdio memory server also hosts). + const [{ createNotification }, { t, fill }] = await Promise.all([ + import("./notifications.js"), + import("../gateway/dashboard/shared/i18n.js"), + ]); + const lang = await readLang(db); + const created = await createNotification(db, { + type: "system", + source, + priority: "high", + title: fill(t("providers.endpointHoldTitle", lang), { name: providerDisplayName(row) }), + body: t("providers.endpointHoldBody", lang), + action_url: SYNC_CONFLICTS_URL, + metadata: conflictId === null ? null : { [HELD_CONFLICT_METADATA_KEY]: conflictId }, + }); + return created ? "raised" : "filtered"; +} + +/** + * Quiet that notice because the operator decided the conflict (kept the local + * address, or accepted the new one). Marks it read rather than deleting it, so + * it stays in the bell's history; the NEXT held change raises a fresh one. + * @returns {Promise} rows touched + */ +export async function clearProviderEndpointHeldNotice(db, id) { + if (typeof id !== "string" || id === "") return 0; + const r = await db.execute({ + sql: `UPDATE notifications SET is_read = 1, updated_at = datetime('now') + WHERE source = ? AND is_read = 0`, + args: [endpointHoldNoticeSource(id)], + }); + return Number(r.rowsAffected ?? 0); +} + +/** + * As above, for "resolve all": every held endpoint change was just decided. + * @returns {Promise} rows touched + */ +export async function clearAllProviderEndpointHeldNotices(db) { + const r = await db.execute({ + sql: `UPDATE notifications SET is_read = 1, updated_at = datetime('now') + WHERE source LIKE 'providers:endpoint-held:%' AND is_read = 0`, + args: [], + }); + return Number(r.rowsAffected ?? 0); +} \ No newline at end of file diff --git a/servers/shared/provider-key-need.js b/servers/shared/provider-key-need.js new file mode 100644 index 000000000..f79fd27c8 --- /dev/null +++ b/servers/shared/provider-key-need.js @@ -0,0 +1,219 @@ +/** + * "This provider needs an API key on THIS device" (security audit N1b-7, + * operator ruling 2026-11-05). + * + * Provider rows sync between paired Crow instances; their `api_key` never does + * (servers/sharing/instance-sync.js's LOCAL_ONLY_COLUMNS). So a provider this + * operator added on one box arrives on the others with no key at all, and the + * only honest thing to say about it is: it needs one here, once. + * + * Three questions live here, in one place: + * + * 1. providerNeedsKey(row) — does this row's type or preset need a key? + * 2. providerDisplayName(row) — what to call it in the notice. + * 3. noticeProviderKeyNeeded(db, id) — raise it ONCE per provider id, as an + * in-app notification linking to the providers settings; and + * clearProviderKeyNeededNotice(db, id) — quiet it again when the operator + * enters a key (providers-db.js's local write path calls this), which is + * also the only thing that re-arms it for a later loss of that key. + * + * Everything is per-DEVICE by construction: the checks read this instance's own + * `providers` row, and neither the marker nor the notice is ever emitted to a + * peer (notifications are not a synced table, and the local key is never + * written by an inbound row). + * + * A KEYLESS provider is never nagged: a llama.cpp / Ollama on the LAN or the + * tailnet reaches a loopback, private, CGNAT/tailnet or bare-host name, and + * `provider_type` `openai-compat` (the generic OpenAI-compatible adapter a + * local server also uses) or `ollama` is not a cloud SDK's type. A row on a + * KNOWN cloud preset hostname always needs one; otherwise only the cloud SDK + * types (openai, anthropic, google, openrouter) do. + */ +import { isIP } from "node:net"; +import { hostnameOf } from "./provider-host.js"; +import { isLocalNetworkIp } from "./ip-classify.js"; +import { presetHostFor } from "./provider-recommendations.js"; + +/** The providers settings tab — where a key is entered. */ +export const PROVIDERS_SETTINGS_URL = "/dashboard/settings?section=llm&tab=providers"; + +/** + * provider_type values that name a hosted service with a mandatory key. NOT + * `openai-compat` (a locally-run llama.cpp / vLLM / LM Studio serves the same + * adapter and usually takes no key) and not `ollama` (none by default), and not + * NULL (a bundle- or models.json-registered local row leaves it empty). + */ +const KEYED_TYPES = Object.freeze(["openai", "anthropic", "google", "openrouter"]); + +/** Hostnames that can only resolve on this machine or the operator's own network. */ +const LOCAL_NAME_SUFFIXES = Object.freeze([".local", ".localhost", ".internal", ".lan", ".home.arpa", ".localdomain"]); + +/** A hostname that names this box or the LAN (never a public service). Pure. */ +function isLocalName(host) { + if (!host) return true; + if (host === "localhost") return true; + if (!host.includes(".")) return true; // a bare name is not publicly resolvable + return LOCAL_NAME_SUFFIXES.some((suffix) => host.endsWith(suffix)); +} + +/** + * Does this provider row need an API key to be usable? Pure; accepts either the + * DB row (`base_url` / `provider_type`) or the parsed panel object + * (`baseUrl` / `provider_type`, see provider-row.js). + * + * @param {object|null} row + * @returns {boolean} + */ +export function providerNeedsKey(row) { + if (!row) return false; + const url = row.baseUrl ?? row.base_url; + const host = hostnameOf(url); + if (!host) return false; // no endpoint to dial: nothing to key + // A local endpoint is keyless by definition (a llama.cpp on the LAN, a + // tailnet box, a loopback sidecar). + if (isIP(host) ? isLocalNetworkIp(host) : isLocalName(host)) return false; + // A known cloud preset's host (any path/port/scheme): every one of them + // requires a key, whatever the type column says. + if (presetHostFor({ baseUrl: url }) !== null) return true; + const type = String(row.provider_type ?? row.providerType ?? "").toLowerCase(); + return KEYED_TYPES.includes(type); +} + +/** + * The name the operator knows this provider by: the preset's label for a known + * cloud host ("OpenAI"), else its description, else its id. Display text only — + * the caller escapes/renders it. + */ +export function providerDisplayName(row) { + const url = row?.baseUrl ?? row?.base_url; + const presetId = presetHostFor({ baseUrl: url }); + if (presetId) { + const label = PRESET_LABELS[presetId]; + if (label) return label; + } + const description = typeof row?.description === "string" ? row.description.trim() : ""; + if (description) return description.slice(0, 80); + return String(row?.id ?? "(unknown provider)"); +} + +/** Preset id → English proper-noun label (a duplicate of provider-presets.js on purpose: no import cycle, and a label is not translatable text). */ +const PRESET_LABELS = Object.freeze({ + openrouter: "OpenRouter", + anthropic: "Anthropic", + openai: "OpenAI", + google: "Google AI Studio", + zai: "Z.AI", + groq: "Groq", + together: "Together AI", + modelstudio: "Alibaba Model Studio (Qwen)", + other: "Other (OpenAI-compatible)", +}); + +/** The notification `source` for one provider id — the dedupe key. */ +export function providerKeyNoticeSource(id) { + return `providers:key-needed:${String(id)}`; +} + +/** + * The notice metadata flag that says "the operator entered a key after this + * notice was raised" — i.e. the question this notice asked has been ANSWERED, so + * a later loss of that key may ring once more. Any notice without it (an open + * one, one the operator read, one they dismissed) is still the answer to the + * only question there is, and keeps the bell quiet (fix round 2: "needs a key" + * used to ring again after a read or a dismissal). + */ +export const KEY_ENTERED_METADATA_KEY = "keyEnteredAfterNotice"; + +/** Was this notice's question already answered by the operator entering a key? Pure. */ +function noticeWasAnswered(metadataJson) { + if (!metadataJson) return false; + try { + return JSON.parse(metadataJson)?.[KEY_ENTERED_METADATA_KEY] === true; + } catch { + return false; + } +} + +async function readLang(db) { + try { + const { rows } = await db.execute({ + sql: "SELECT value FROM dashboard_settings WHERE key = 'language'", + args: [], + }); + return rows[0]?.value === "es" ? "es" : "en"; + } catch { + return "en"; + } +} + +/** + * Raise the notice for provider `id`, if it is owed. ONCE per provider id: ANY + * existing notice for that id — open, read or dismissed — means it was already + * raised, so neither a re-delivered sync entry nor a dismissal can ring it + * again. It re-arms only through `clearProviderKeyNeededNotice` (the operator + * entered a key), so a later removal of that key may ring once more + * (fix round 2). + * + * Only fires for a row this instance HOLDS, that is enabled, that needs a key + * (providerNeedsKey) and that has none here. + * + * @param {object} db + * @param {string} id provider id + * @returns {Promise<"raised"|"already"|"has-key"|"keyless"|"disabled"|"no-row"|"no-id"|"filtered">} + * `filtered` = the operator switched this notification type off. + */ +export async function noticeProviderKeyNeeded(db, id) { + if (typeof id !== "string" || id === "") return "no-id"; + const { rows } = await db.execute({ sql: "SELECT * FROM providers WHERE id = ?", args: [id] }); + const row = rows[0]; + if (!row) return "no-row"; + if (Number(row.disabled)) return "disabled"; + if (!providerNeedsKey(row)) return "keyless"; + const key = typeof row.api_key === "string" ? row.api_key.trim() : ""; + if (key !== "") return "has-key"; + + const source = providerKeyNoticeSource(id); + // Any notice for this id counts, whatever the operator did with it: re-raising + // on a read or a dismissal is exactly the nag fix round 2 removed. Only a + // notice the operator ANSWERED with a key is spent (see clearProviderKeyNeededNotice). + const { rows: prior } = await db.execute({ + sql: "SELECT id, metadata FROM notifications WHERE source = ?", + args: [source], + }); + if (prior.some((n) => !noticeWasAnswered(n.metadata))) return "already"; + + // Loaded lazily: notifications.js and the i18n table are gateway-facing, and + // this runs from the sync manager (which the stdio memory server also hosts). + const [{ createNotification }, { t, fill }] = await Promise.all([ + import("./notifications.js"), + import("../gateway/dashboard/shared/i18n.js"), + ]); + const lang = await readLang(db); + const created = await createNotification(db, { + type: "system", + source, + priority: "normal", + title: fill(t("providers.needsKeyTitle", lang), { name: providerDisplayName(row) }), + body: t("providers.needsKeyBody", lang), + action_url: PROVIDERS_SETTINGS_URL, + }); + return created ? "raised" : "filtered"; +} + +/** + * Quiet the notice for provider `id` because a key is now set here. Called by + * the local write path (providers-db.js) whenever it stores a non-empty key. + * Marks it read rather than deleting it, so it stays in the bell's history, and + * stamps it as ANSWERED (KEY_ENTERED_METADATA_KEY): if the operator later removes + * that key, the next sync entry that leaves this row keyless may ring once more. + * @returns {Promise} rows touched + */ +export async function clearProviderKeyNeededNotice(db, id) { + if (typeof id !== "string" || id === "") return 0; + const r = await db.execute({ + sql: `UPDATE notifications SET is_read = 1, updated_at = datetime('now'), metadata = ? + WHERE source = ?`, + args: [JSON.stringify({ [KEY_ENTERED_METADATA_KEY]: true }), providerKeyNoticeSource(id)], + }); + return Number(r.rowsAffected ?? 0); +} \ No newline at end of file diff --git a/servers/shared/providers-db.js b/servers/shared/providers-db.js index ce5965728..9e51be18b 100644 --- a/servers/shared/providers-db.js +++ b/servers/shared/providers-db.js @@ -35,6 +35,7 @@ import { getOwnAddresses, isLocallyOrchestratable } from "./locality.js"; import { localizeNativeRow } from "./native-locality.js"; import { modelsJsonSearchPaths } from "./models-json-paths.js"; import { emitOrQueue } from "./sync-emit.js"; +import { clearProviderKeyNeededNotice } from "./provider-key-need.js"; import { providerRowToObject } from "./provider-row.js"; import { inferHost, repairHostDecision } from "./provider-host.js"; import { isExternalEngine, engineShapeError, externalEngineConflict } from "./provider-engine.js"; @@ -380,6 +381,8 @@ export async function upsertProvider(db, provider) { const newTs = Math.max(Number(currentTs), Number(provider.lamport_ts || 0)) + 1; + const storedKey = provider.apiKey ?? provider.api_key ?? null; + await db.execute({ sql: `INSERT INTO providers (id, base_url, api_key, host, bundle_id, description, models, disabled, lamport_ts, instance_id, provider_type, gpu_policy, updated_at) @@ -400,7 +403,7 @@ export async function upsertProvider(db, provider) { args: [ provider.id, provider.baseUrl || provider.base_url || "", - provider.apiKey ?? provider.api_key ?? null, + storedKey, host, provider.bundleId ?? provider.bundle_id ?? null, provider.description ?? null, @@ -413,10 +416,21 @@ export async function upsertProvider(db, provider) { ], }); + // A key is now set HERE, so the "needs an API key on this device" notice a + // paired peer's keyless row raised (provider-key-need.js) is answered. + // Best-effort: a notification-table problem must never fail the write. + if (typeof storedKey === "string" && storedKey.trim() !== "") { + try { await clearProviderKeyNeededNotice(db, provider.id); } + catch (err) { console.warn(`[providers] key-needed notice clear failed for ${provider.id}: ${err.message}`); } + } + await emitSync(db, existed ? "update" : "insert", { id: provider.id, base_url: provider.baseUrl || provider.base_url || "", - api_key: provider.apiKey ?? provider.api_key ?? null, + // api_key is deliberately ABSENT: it never leaves this instance + // (operator ruling 2026-11-05; instance-sync.js LOCAL_ONLY_COLUMNS strips + // it at every emit door anyway — not putting it here keeps that obvious + // at the call site that has the secret in hand). host, bundle_id: provider.bundleId ?? provider.bundle_id ?? null, description: provider.description ?? null, @@ -447,6 +461,9 @@ export async function disableProvider(db, id) { sql: "UPDATE providers SET disabled = 1, lamport_ts = ?, instance_id = ?, updated_at = datetime('now') WHERE id = ?", args: [newTs, instanceId, id], }); + // The SELECT * spread carries the row's api_key into this call — the emit + // doors (emitChange's EXCLUDED_COLUMNS / emitOrQueue's strip) drop it, so a + // soft-delete never ships the key even though the caller had the whole row. await emitSync(db, "update", { ...rows[0], disabled: 1, lamport_ts: newTs, instance_id: instanceId }); return { ok: true, lamport_ts: newTs }; } diff --git a/servers/shared/sync-emit.js b/servers/shared/sync-emit.js index 54735171c..88799f780 100644 --- a/servers/shared/sync-emit.js +++ b/servers/shared/sync-emit.js @@ -15,7 +15,7 @@ import { getOrCreateLocalInstanceId } from "../gateway/instance-registry.js"; import { readSetting } from "../gateway/dashboard/settings/registry.js"; -import { SYNCED_TABLES, shouldSyncRow, shouldInitInstanceSync } from "../sharing/instance-sync.js"; +import { SYNCED_TABLES, shouldSyncRow, shouldInitInstanceSync, stripLocalOnlyColumns } from "../sharing/instance-sync.js"; import { ensureSyncTables, seedCounterSql, @@ -202,6 +202,11 @@ export async function emitOrQueue(syncManager, db, table, op, row, opts = {}) { // wedges claimed/reclaimed forever and eventually cap-evicts real rows). if (!SYNCED_TABLES.includes(table)) return null; if (!shouldSyncRow(table, row)) return null; + // Local-only columns never enter the queue: the outbox row IS the emitted + // payload (`row_json`, replayed verbatim by the drain's emitChange), and a + // providers row's api_key belongs to this instance alone — the same strip + // emitChange applies on its way out (instance-sync.js LOCAL_ONLY_COLUMNS). + row = stripLocalOnlyColumns(table, row); const eligible = await isDeploymentEligible(db); if (!eligible) return null; diff --git a/servers/sharing/instance-sync.js b/servers/sharing/instance-sync.js index 1d3aea0a8..3290083d8 100644 --- a/servers/sharing/instance-sync.js +++ b/servers/sharing/instance-sync.js @@ -31,6 +31,8 @@ import { TRUSTED_PEER_SQL } from "../shared/peer-trust.js"; import { SYNC_PB_KEY_PREFIX, SYNC_PB_CLEARED_PREFIX } from "../shared/self-dial-address.js"; import { mintLamport, advanceCounter, stampSql, ensureLamportOriginColumn, incomingLosesLww } from "../shared/sync-stamp.js"; import { deleteContactArtifactComments } from "../shared/artifact-comment-delete.js"; +import { noticeProviderKeyNeeded } from "../shared/provider-key-need.js"; +import { providerEndpointChanged, noticeProviderEndpointHeld } from "../shared/provider-endpoint.js"; /** How long the re-pair fence waits to learn a peer feed's remote length (PT-1). */ const REPAIR_FENCE_UPDATE_TIMEOUT_MS = 15_000; @@ -102,6 +104,8 @@ export const SYNCED_TABLES = [ "crow_instances", // Phase 5-full: LLM provider registry. Operator-editable on any instance; // changes propagate to peers so model swaps/additions are one-and-done. + // The row syncs; its api_key NEVER does (LOCAL_ONLY_COLUMNS below — operator + // ruling 2026-11-05). A peer that needs a key is told once, by name. // Toggle off by removing this line if you want per-instance divergence. "providers", // Scoped-settings replication (2026-04-14): only rows with instance_id IS NULL @@ -194,17 +198,86 @@ export const DELETE_KEY_POLICY = Object.freeze({ relay_config: null, }); +/** + * Columns that belong to the instance that WROTE them and never cross the wire + * in either direction (operator ruling 2026-11-05: an API key never leaves the + * instance where it was entered, and an inbound row never writes or overwrites + * a local key). Per table, and the single source of truth for all four doors: + * + * - outbound emit — EXCLUDED_COLUMNS below AND OUTBOUND_TRANSFORMS.providers; + * - the stdio outbox (sync-emit.js emitOrQueue) — stripLocalOnlyColumns; + * - inbound apply — INBOUND_DROPPED_COLUMNS below (after the signature + * check, before _checkConflict, so an old peer's key can + * neither be written nor manufacture a conflict row); + * - the equivalence/conflict judgement — rowsEquivalent ignores them. + * + * A future local-only column goes here FIRST; every door reads this map. + */ +export const LOCAL_ONLY_COLUMNS = Object.freeze({ + providers: Object.freeze(["api_key"]), +}); + +/** Every local-only column name, whatever table it belongs to (rowsEquivalent). */ +const LOCAL_ONLY_COLUMN_NAMES = Object.freeze([...new Set(Object.values(LOCAL_ONLY_COLUMNS).flat())]); + +/** + * Pure: a copy of `row` with `table`'s local-only columns removed (the same + * object back when the table has none, so callers can pass it through + * unconditionally). Never mutates the input. + */ +export function stripLocalOnlyColumns(table, row) { + const cols = LOCAL_ONLY_COLUMNS[table]; + if (!cols || !row || typeof row !== "object") return row; + const out = { ...row }; + for (const col of cols) delete out[col]; + return out; +} + +/** + * A CONFLICT SNAPSHOT as it may be stored: `raw` (the JSON string callers already + * built) with `table`'s local-only columns removed. + * + * Every snapshot written to sync_conflicts goes through this (see + * _insertConflictRow, which no caller can bypass). A conflict row is rendered in + * FULL on the settings page and a prefix of it is returned by + * crow_list_sync_conflicts, so a snapshot is a publication of this device's data: + * `winning_data` used to be a raw `SELECT *` of a local providers row — this + * device's key — and `losing_data` can be an old peer's wire row that still + * carried one (security audit N1b-7). + * + * Idempotent, and never throws: a table with no local-only columns, a snapshot + * that holds no local-only column, a JSON null (`'null'`, a row that was gone) + * and an unparseable blob all come back as the SAME STRING that went in — a + * snapshot we cannot read cannot be redacted here either, and losing the + * conflict trace would be worse. Returning the caller's own string (not a + * re-serialization of an unchanged object) is what makes the boot repair's + * "write only when something changed" strictly true. + */ +export function stripConflictSnapshot(table, raw) { + const cols = LOCAL_ONLY_COLUMNS[table]; + if (!cols) return raw; + try { + const parsed = typeof raw === "string" ? JSON.parse(raw) : raw; + if (!parsed || typeof parsed !== "object") return raw; + if (!cols.some((col) => Object.hasOwn(parsed, col))) return raw; // nothing to redact + return JSON.stringify(stripLocalOnlyColumns(table, parsed)); + } catch { + return raw; + } +} + // Columns to exclude from sync payloads (security-sensitive or instance-local) export const EXCLUDED_COLUMNS = { crow_instances: ["auth_token_hash"], - // apiKey can be sensitive for some deployments; keep in sync by default for - // local-lab scenarios but flag here as the place to exclude if paranoid. - // created_at/updated_at are per-instance bookkeeping that ride the wire - // TODAY via disableProvider's SELECT * spread and manufacture the same - // spurious conflicts as contacts.created_at below. lamport_ts is sync - // metadata carried in the entry envelope, not the row (prior art: - // messages/contact_groups). - providers: ["created_at", "updated_at", "lamport_ts"], + // providers.api_key is LOCAL-ONLY (LOCAL_ONLY_COLUMNS above): the operator's + // ruling is that a provider row syncs (name/id, base_url, models, + // description, type, host, gpu_policy) but its key never does — a peer never + // receives it and can never write one back. created_at/updated_at are + // per-instance bookkeeping that ride the wire TODAY via disableProvider's + // SELECT * spread and manufacture the same spurious conflicts as + // contacts.created_at below. lamport_ts is sync metadata carried in the + // entry envelope, not the row (prior art: messages/contact_groups). + providers: [...LOCAL_ONLY_COLUMNS.providers, "created_at", "updated_at", "lamport_ts"], // Phase 3 (contacts follow the user): `verified` is a per-device attestation // ("I compared the safety number on THIS device") — never assert it on a // device that didn't check. `last_seen` is bumped on every inbound DM @@ -273,10 +346,16 @@ export const EXCLUDED_COLUMNS = { // memories.project_id is a local FK too (project_spaces ids are per-instance): an un-upgraded // peer still sends its own id, which would file the memory under an unrelated local project. // The receiver derives project_id from project_uuid instead (mapInboundMemoryProject). +// providers.api_key: an old (or hostile) peer still SENDS one, so it is dropped +// here — after the signature check, before dispatch and before _checkConflict — +// which leaves _applyInsert writing NULL for a new row and _applyUpdate +// (which only touches the keys present on the wire) leaving the local key +// untouched. A missing api_key key on the wire therefore never nulls it. export const INBOUND_DROPPED_COLUMNS = { memories: ["project_id"], research_notes: ["provenance", "quote_found", "locator", "source_id", "created_by", "withdrawn_at", "withdrawn_by", "withdrawn_reason"], + providers: [...LOCAL_ONLY_COLUMNS.providers], }; // A wire-row key may only reach SQL as an identifier if it matches this AND is @@ -396,18 +475,25 @@ const OUTBOUND_TRANSFORMS = { }, // source_id is a local FK too (research_sources rows are not synced). research_notes: (row) => ({ ...row, project_id: null, source_id: null }), - // providers: a null gpu_policy must not ride the wire — _applyUpdate writes - // wire nulls verbatim and would null a peer's locally-set policy, while the - // local write path already treats null as "keep" (COALESCE, - // providers-db.js:198). Drop the key when null; pass through otherwise. + // providers: the local-only key never rides the wire, in EITHER direction of + // this transform's two uses (emitChange's outbound row and _checkConflict's + // LOCAL comparison row) — so a peer holding a different key can never look + // divergent. api_key is also in EXCLUDED_COLUMNS/INBOUND_DROPPED_COLUMNS; + // stripping it here keeps the comparison like-for-like even for a caller that + // reaches this transform alone. + // Also: a null gpu_policy must not ride the wire — _applyUpdate writes wire + // nulls verbatim and would null a peer's locally-set policy, while the local + // write path already treats null as "keep" (COALESCE, providers-db.js:198). + // Drop the key when null; pass through otherwise. // NOTE: transforms are applied in emitChange AND to the local row in // _checkConflict — must be pure (never mutate the input). providers: (row) => { - if (row.gpu_policy == null) { - const { gpu_policy, ...rest } = row; - return rest; + const rest = stripLocalOnlyColumns("providers", row); + if (rest.gpu_policy == null) { + const { gpu_policy, ...out } = rest; + return out; } - return { ...row }; + return { ...rest }; }, }; @@ -417,7 +503,10 @@ const OUTBOUND_TRANSFORMS = { * * Per-key rules: both null/undefined → equal; exactly one nullish → NOT equal * (never alias null with ""); otherwise String(a) === String(b). - * lamport_ts and instance_id are ignored — sync metadata, not content. + * lamport_ts and instance_id are ignored — sync metadata, not content — and so + * is every LOCAL_ONLY_COLUMNS entry (api_key): two instances that hold + * different keys for the same provider are converged, not divergent, or they + * would re-send each other's rows forever. * * "a" is the LOCAL row (already passed through OUTBOUND_TRANSFORMS at the call * site). "b" is the INCOMING (wire) row. Only keys in "b" are checked: wire @@ -430,7 +519,7 @@ const OUTBOUND_TRANSFORMS = { */ export function rowsEquivalent(a, b) { if (!a || !b) return false; - const IGNORE = new Set(["lamport_ts", "instance_id"]); + const IGNORE = new Set(["lamport_ts", "instance_id", ...LOCAL_ONLY_COLUMN_NAMES]); for (const k of Object.keys(b)) { if (IGNORE.has(k)) continue; const av = a[k] ?? null; @@ -3033,6 +3122,17 @@ export class InstanceSyncManager { if (conflict === "skip") return; // Local version wins } + // Operator ruling (2026-11-06): an inbound change that MOVES a provider's + // endpoint, on a row whose key lives on THIS device, is held as a proposed + // change until the operator accepts it here. Placed AFTER the LWW conflict + // gate on purpose: an entry the local version already wins is an ordinary + // conflict (or noise), not a held proposal. An `insert` whose id collides + // with an existing local row reaches here too (the gate above is + // update/delete only) — that is the other way a peer moves an address. + if (table === "providers" && (op === "insert" || op === "update")) { + if (await this._holdProviderEndpointChange(row, lamport_ts, instance_id)) return; + } + // A peer edit to a note's text can break this instance's quote check: read the local note // first so the change (not a re-delivery) is what invalidates it. const notePrior = table === "research_notes" && op === "update" && row.id !== undefined @@ -3052,11 +3152,152 @@ export class InstanceSyncManager { await this._applyDelete(table, row); break; } + // Operator ruling (2026-11-05): a synced provider that needs a key and + // holds none HERE is surfaced once, by name. Only an inbound apply can + // reach this (a locally-created provider never passes through _applyEntry), + // and the check reads the LOCAL row, so it never fires while a key is set. + if (table === "providers" && op !== "delete" && typeof applyRow?.id === "string") { + await this._noticeProviderNeedsKey(applyRow); + } } catch (err) { console.warn(`[instance-sync] Failed to apply ${op} on ${table}:`, err.message); } } + /** + * Hold an inbound provider change that moves the endpoint while this device + * holds the key (operator ruling 2026-11-06, security audit N1b-7 item 2). + * + * Without this, a compromised peer repoints "openai" at its own server and + * this device sends its key there. With it the change becomes a PROPOSED + * change: recorded in sync_conflicts (the local row as winning_data, the + * incoming row as losing_data) exactly as _checkConflict records one, so the + * existing Settings review + "restore the other version" flow applies it — + * restoreConflict strips local-only columns, so the key stays here and only + * now follows the address the operator explicitly accepted. + * + * Held too: everything else in that entry (models, description). Splitting one + * peer update into "hold the address, apply the rest" would leave this device + * reporting a model list for an endpoint it does not use. + * + * Dedupe (two separate questions): + * - the SAME entry re-delivered (same peer, same losing lamport) raises + * nothing, whether its conflict is open or was dismissed — a dismissal is + * the operator's answer, and re-raising it on every replay of the same + * feed entry would nag forever; + * - a NEWER held change from that peer REPLACES the open row's losing_data + * and lamports instead of piling up one conflict per edit (the lookup is + * this table's own "unresolved for this row" query — _insertConflictRow + * only dedupes an identical (win, lose) pair and excludes resolved rows). + * + * Keyless rows (a llama.cpp on the LAN, a row nothing here has a key for), + * rows this device does not have yet, and address-preserving edits all update + * exactly as before. + * + * Fails CLOSED: if the hold cannot be recorded, the entry is not applied — + * the key must never follow an address this device could not vet. + * + * @param {object} row — the inbound wire row (api_key already dropped) + * @param {number} lamportTs + * @param {string} instanceId — the entry's origin + * @returns {Promise} true when the entry must NOT be applied + */ + async _holdProviderEndpointChange(row, lamportTs, instanceId) { + const rowId = row?.id; + if (typeof rowId !== "string" || rowId === "") return false; + try { + const { rows: localRows } = await this.db.execute({ + sql: "SELECT * FROM providers WHERE id = ?", + args: [rowId], + }); + const localRow = localRows[0]; + if (!localRow) return false; // no local row: nothing here holds a key for it + const localKey = typeof localRow.api_key === "string" ? localRow.api_key.trim() : ""; + if (localKey === "") return false; // keyless here (a LAN llama.cpp): update freely + if (!providerEndpointChanged(localRow.base_url, row.base_url)) return false; + + const { rows: prior } = await this.db.execute({ + sql: `SELECT id, losing_lamport_ts, resolved FROM sync_conflicts + WHERE table_name = 'providers' AND row_id = ? AND losing_instance_id = ? + ORDER BY id DESC`, + args: [rowId, String(instanceId)], + }); + if (prior.some((c) => Number(c.losing_lamport_ts) === Number(lamportTs))) { + return true; // this exact entry was already raised — silence, not a new row + } + + // The conflict row is shown on the sync-conflicts page and returned by + // crow_list_sync_conflicts, so neither snapshot may carry a key — not this + // device's (winning) and not an old peer's (losing). + const winningData = JSON.stringify(stripLocalOnlyColumns("providers", localRow)); + const losingData = JSON.stringify(stripLocalOnlyColumns("providers", row)); + const winningTs = Number(localRow.lamport_ts) || 0; + const openRow = prior.find((c) => Number(c.resolved) === 0); + if (openRow) { + await this.db.execute({ + sql: `UPDATE sync_conflicts + SET winning_lamport_ts = ?, losing_lamport_ts = ?, + winning_data = ?, losing_data = ? + WHERE id = ?`, + args: [winningTs, lamportTs, winningData, losingData, openRow.id], + }); + return true; + } + + // op 'update' even for an inbound INSERT collision: the review page and + // restoreConflict() treat op='insert' as unrestorable (D7), and accepting + // a held address IS the point — the rows are keyed by the same portable + // id, so applying the held version is an UPDATE either way. + const inserted = await this._insertConflictRow( + "providers", rowId, + localRow.instance_id || this.localInstanceId, instanceId, + winningTs, lamportTs, + winningData, losingData, + "update", + ); + if (inserted) { + // The notice is per OPEN held conflict, so it is stamped with the + // conflict row's id: a re-delivery of THIS held change stays silent + // (read or dismissed), while a new held conflict formed after this one + // was resolved rings again (provider-endpoint.js). + let heldConflictId = null; + try { + const { rows: created } = await this.db.execute({ + sql: `SELECT id FROM sync_conflicts + WHERE table_name = 'providers' AND row_id = ? AND losing_instance_id = ? + AND losing_lamport_ts = ? AND resolved = 0 + ORDER BY id DESC LIMIT 1`, + args: [rowId, String(instanceId), lamportTs], + }); + heldConflictId = created[0]?.id ?? null; + } catch { /* no id: the notice falls back to its per-provider dedupe */ } + try { + await noticeProviderEndpointHeld(this.db, localRow, heldConflictId); + } catch (err) { + console.warn(`[instance-sync] provider endpoint notice failed for ${rowId.slice(0, 64)}:`, err.message); + } + } + return true; + } catch (err) { + console.warn(`[instance-sync] could not hold an inbound provider endpoint change for ${String(rowId).slice(0, 64)} — entry NOT applied:`, err.message); + return true; + } + } + + /** + * Raise (at most once) the "needs an API key on this device" notice for a + * provider row that just arrived from a peer. provider-key-need.js owns the + * predicate (a keyless llama.cpp on the LAN is not nagged), the by-name text + * and the per-id dedupe; never throws into the apply loop. + */ + async _noticeProviderNeedsKey(row) { + try { + await noticeProviderKeyNeeded(this.db, row?.id); + } catch (err) { + console.warn(`[instance-sync] provider key notice failed for ${String(row?.id).slice(0, 64)}:`, err.message); + } + } + /** * Apply a dashboard_settings mutation. Only the global row is synced; * per-instance overrides live in dashboard_settings_overrides. @@ -4010,12 +4251,18 @@ export class InstanceSyncManager { // The verdict is already determined here (local is newer and the data // differs) — a failure to LOG must not flip it to "apply" and overwrite // newer local data, so logging gets its own guard and we skip regardless. + // Both snapshots are stripped of local-only columns: the local row here is + // a raw `SELECT *`, i.e. this device's providers.api_key, and the conflict + // row is shown in full on the settings page (security audit N1b-7). + // _insertConflictRow strips again — this call-site strip keeps the shape + // obvious where the raw row is in hand. try { const inserted = await this._insertConflictRow( table, String(rowId), localRow.instance_id || this.localInstanceId, incomingInstanceId, localTs, incomingTs, - JSON.stringify(localRow), JSON.stringify(loggedRow), + JSON.stringify(stripLocalOnlyColumns(table, localRow)), + JSON.stringify(stripLocalOnlyColumns(table, loggedRow)), op || "update", ); if (inserted) await this._notifyConflict(); @@ -4046,6 +4293,12 @@ export class InstanceSyncManager { * origin is INCLUDED (per-instance counters collide across peers; spec §3 C5). */ async _insertConflictRow(tableName, rowId, winInst, loseInst, winTs, loseTs, winData, loseData, conflictOp) { + // THE one place every conflict snapshot is stored, so the one place that + // must not be possible to forget: local-only columns (providers.api_key) are + // stripped here whatever the caller passed. Call sites strip too (the local + // raw row they hold), and a double strip is a no-op. + winData = stripConflictSnapshot(tableName, winData); + loseData = stripConflictSnapshot(tableName, loseData); const dedupeArgs = [tableName, rowId, winTs, loseTs, loseInst]; try { const { rows } = await this.db.execute({ diff --git a/servers/sharing/sync-conflict-redact.js b/servers/sharing/sync-conflict-redact.js new file mode 100644 index 000000000..d0cc24953 --- /dev/null +++ b/servers/sharing/sync-conflict-redact.js @@ -0,0 +1,116 @@ +/** + * Boot-time redaction of old conflict snapshots (security audit N1b-7, fix + * round 2). + * + * Before conflict snapshots were stripped, `sync_conflicts.winning_data` for a + * providers conflict was a raw `SELECT *` of THIS device's row — its `api_key` — + * and `losing_data` could be an old peer's wire row that still carried one. The + * settings page renders both in full and `crow_list_sync_conflicts` returns a + * prefix of them, so rows saved before the fix are a standing exposure. + * + * This is a one-time repair, run once per boot like the other boot repairs + * (servers/gateway/boot/post-listen.js): for every `sync_conflicts` row with + * `table_name = 'providers'`, parse both snapshots, drop the table's local-only + * columns (instance-sync.js LOCAL_ONLY_COLUMNS — the single source of truth) and + * write them back ONLY when something actually changed. Idempotent: a second run + * finds nothing to do. A row is never deleted and `resolved` is never touched — + * the operator's review history survives, minus the secret. + * + * Two bounds keep the one-time repair honest: + * - the SELECT only reads rows that still mention a local-only column (a boot + * on a clean DB reads nothing at all); + * - the UPDATE is conditional on the snapshots it read (`winning_data IS ?`), + * so a row that changed between the read and the write — a peer's entry + * applied concurrently, a restore's re-snapshot — is never overwritten with + * a stale redaction (the newer value is simply re-examined next boot). + * + * No schema change: this rewrites two TEXT columns in place. + * + * Never throws: a caller on the boot path must not be taken down by a corrupt + * snapshot. A blob that cannot be parsed is left exactly as it is (this repair + * cannot redact what it cannot read, and destroying a conflict is worse). + */ +import { LOCAL_ONLY_COLUMNS, stripConflictSnapshot } from "./instance-sync.js"; + +/** Tables this repair rewrites — the ones whose snapshots can hold a key. */ +const TABLES = Object.freeze(Object.keys(LOCAL_ONLY_COLUMNS)); + +/** + * Redact local-only columns from every stored conflict snapshot of `tables`. + * + * @param {object} db + * @param {{ tables?: string[] }} [opts] — test seam; defaults to every table that + * has local-only columns (today: `providers`). + * @returns {Promise} how many conflict rows were rewritten + */ +export async function redactConflictSnapshots(db, { tables = TABLES } = {}) { + const wanted = [...tables].filter((t) => LOCAL_ONLY_COLUMNS[t]); + if (wanted.length === 0) return 0; + const placeholders = wanted.map(() => "?").join(", "); + const { rows } = await db.execute({ + // `"api_key"` is the only local-only column today (LOCAL_ONLY_COLUMNS): + // extend this LIKE when a second one is added, or those rows go unread. + sql: `SELECT id, table_name, winning_data, losing_data FROM sync_conflicts + WHERE table_name IN (${placeholders}) + AND (winning_data LIKE '%"api_key"%' OR losing_data LIKE '%"api_key"%')`, + args: wanted, + }); + + let changed = 0; + for (const row of rows) { + const win = stripConflictSnapshot(row.table_name, row.winning_data); + const lose = stripConflictSnapshot(row.table_name, row.losing_data); + if (win === row.winning_data && lose === row.losing_data) continue; // nothing to redact + const written = await writeBackRedacted(db, row.id, { + winning: win, + losing: lose, + expectedWinning: row.winning_data, + expectedLosing: row.losing_data, + }); + if (written) changed += 1; + } + return changed; +} + +/** + * Write a redacted snapshot pair back, but ONLY if the row still holds the two + * values that were read (`IS ?` is null-safe, and this is a plain equality). The + * boot repair runs while sync may be live, so a snapshot rewritten between the + * read and this UPDATE (a peer's entry applied, the restore path's re-snapshot) + * must survive untouched: the conditional matches no row and nothing is written. + * + * Exported as the seam the tests drive stale values through directly. + * + * @returns {Promise} whether the row was actually written + */ +export async function writeBackRedacted( + db, + id, + { winning, losing, expectedWinning, expectedLosing }, +) { + const { rowsAffected } = await db.execute({ + sql: `UPDATE sync_conflicts SET winning_data = ?, losing_data = ? + WHERE id = ? AND winning_data IS ? AND losing_data IS ?`, + args: [winning, losing, id, expectedWinning, expectedLosing], + }); + return Number(rowsAffected) > 0; +} + +/** + * The boot entry point: run the repair, log exactly one line with the count, and + * never throw (see the module docblock). + * + * @param {object} db + * @param {{ log?: (msg: string) => void }} [opts] + * @returns {Promise} how many conflict rows were rewritten (0 on error) + */ +export async function redactConflictSnapshotsAtBoot(db, { log = console.log } = {}) { + try { + const changed = await redactConflictSnapshots(db); + log(`[sync-conflicts] key redaction: ${changed} stored conflict snapshot row(s) rewritten`); + return changed; + } catch (err) { + log(`[sync-conflicts] key redaction skipped: ${err?.message || err}`); + return 0; + } +} \ No newline at end of file diff --git a/servers/sharing/sync-conflict-resolve.js b/servers/sharing/sync-conflict-resolve.js index 1519e8fa2..598de6c7e 100644 --- a/servers/sharing/sync-conflict-resolve.js +++ b/servers/sharing/sync-conflict-resolve.js @@ -22,7 +22,7 @@ * on failure the conflict row stays unresolved (spec round-2 C2 / round-3 C2). */ -import { SYNCED_TABLES, rowsEquivalent, DELETE_KEY_POLICY } from "./instance-sync.js"; +import { SYNCED_TABLES, rowsEquivalent, DELETE_KEY_POLICY, stripLocalOnlyColumns } from "./instance-sync.js"; import { emitOrQueue } from "../shared/sync-emit.js"; // ── Natural-key restore refusals (2c C7 / 2c-F3) ───────────────────────────── @@ -220,7 +220,13 @@ export async function restoreConflict(db, conflictId, { instanceSync = null } = // When the row is gone, store the JSON string 'null' (not SQL NULL, which would // violate the winning_data NOT NULL constraint). The stale guard treats // JSON.parse('null') === null the same as storedWinningData === null below. - const newWinningData = liveSnapshot ? JSON.stringify(liveSnapshot) : "null"; + // The re-snapshot is a raw local `SELECT *`: local-only columns + // (providers.api_key) belong to this instance alone and the conflict row is + // rendered in full on the settings page, so they never go into it + // (security audit N1b-7). + const newWinningData = liveSnapshot + ? JSON.stringify(stripLocalOnlyColumns(table, liveSnapshot)) + : "null"; await db.execute({ sql: `UPDATE sync_conflicts SET winning_data = ? WHERE id = ?`, args: [newWinningData, conflictId], @@ -291,6 +297,14 @@ export async function restoreConflict(db, conflictId, { instanceSync = null } = }; } + // A losing version came from the WIRE, and a table's local-only columns are + // never the sender's to write back: providers.api_key belongs to this + // instance alone (operator ruling 2026-11-05; instance-sync.js + // LOCAL_ONLY_COLUMNS). An old, pre-upgrade conflict row can still carry one, + // so it is stripped before the restore touches SQL — and before the restore + // is emitted. + losingData = stripLocalOnlyColumns(table, losingData); + // ── 4. Intersect losing_data keys with live PRAGMA columns, exclude id ─── const cols = await liveColumns(db, table); diff --git a/servers/sharing/tools/instances.js b/servers/sharing/tools/instances.js index 40ef9fe36..05aa56b53 100644 --- a/servers/sharing/tools/instances.js +++ b/servers/sharing/tools/instances.js @@ -9,6 +9,34 @@ import { z } from "zod"; import { createNotification } from "../../shared/notifications.js"; +import { stripConflictSnapshot } from "../instance-sync.js"; + +/** + * A short, SAFE preview of one stored conflict snapshot, for the tool's text + * output. The snapshot is stripped of the table's local-only columns FIRST + * (instance-sync.js LOCAL_ONLY_COLUMNS) because this tool is another reading + * door onto `sync_conflicts`: a row saved before the fix can still hold an + * `api_key`, and this is the only redaction a stdio-only install ever gets. + * + * Returns "" ("show nothing") for a JSON null snapshot — an old row whose side + * was gone, `'null'` — which used to throw here by reading `.content` off null — + * and for a blob we cannot parse, which we must not print raw. + */ +function snapshotPreview(table, raw) { + if (raw == null) return ""; + const stripped = stripConflictSnapshot(table, raw); + let parsed; + try { + parsed = typeof stripped === "string" ? JSON.parse(stripped) : stripped; + } catch { + return ""; + } + if (!parsed || typeof parsed !== "object") return ""; + const text = typeof parsed.content === "string" + ? parsed.content + : JSON.stringify(parsed); + return text.substring(0, 80); +} export function registerInstancesTools(server, ctx) { const { db, identity, nostrManager, instanceSyncManager } = ctx; @@ -345,15 +373,13 @@ export function registerInstancesTools(server, ctx) { } const formatted = rows.map((r) => { - const winning = JSON.parse(r.winning_data); - const losing = JSON.parse(r.losing_data); - const winPreview = typeof winning.content === "string" ? winning.content.substring(0, 80) : JSON.stringify(winning).substring(0, 80); - const losePreview = typeof losing.content === "string" ? losing.content.substring(0, 80) : JSON.stringify(losing).substring(0, 80); + const winPreview = snapshotPreview(r.table_name, r.winning_data); + const losePreview = snapshotPreview(r.table_name, r.losing_data); return `Conflict #${r.id} (${r.table_name}, row: ${r.row_id}, ${r.resolved ? "resolved" : "unresolved"}) Winner: instance ${r.winning_instance_id} (ts: ${r.winning_lamport_ts}) - ${winPreview}... + ${winPreview || "(none)"} Loser: instance ${r.losing_instance_id} (ts: ${r.losing_lamport_ts}) - ${losePreview}... + ${losePreview || "(none)"} Created: ${r.created_at}`; }).join("\n\n"); diff --git a/tests/providers-key-locality.test.js b/tests/providers-key-locality.test.js new file mode 100644 index 000000000..605f90166 --- /dev/null +++ b/tests/providers-key-locality.test.js @@ -0,0 +1,1439 @@ +/** + * Security audit N1b-7 — provider settings sync to paired peers; API keys never + * leave the instance where they were entered (operator ruling 2026-11-05). + * + * What must hold, and what each test drives through the REAL sync code: + * + * 1. A providers row syncs (address, models, description, type...) and its + * `api_key` never appears on the wire, in ANY emit door. + * 2. A peer's row lands here with no key; an old (pre-upgrade) peer's raw + * inbound row that still carries `api_key` writes nothing — neither on an + * insert nor on an update, and a row that omits the key never nulls it. + * 3. Two instances holding DIFFERENT keys still converge: their rows are + * equivalent, no conflict row is manufactured, re-delivery is silent. + * 4. A synced provider that needs a key and has none here is surfaced once, + * by name; never for a keyless one (a llama.cpp on the LAN); never again + * once a key is entered here. + * 5. Fix round 1 (operator ruling 2026-11-06): an inbound change that MOVES a + * provider's endpoint while the key lives HERE is held as a proposed + * change (one sync_conflicts row, one named bell item) until the operator + * accepts it on this device; a keyless row still updates freely. Held in + * BOTH directions (whoever holds the key). + * 6. Fix round 2: NO conflict snapshot ever holds a key — not an ordinary + * Lamport conflict, not the endpoint hold, not the restore's stale + * re-snapshot — and old rows are redacted once at boot. + * 7. Fix round 2: "needs a key on this device" rings once per provider id + * (a read or a dismissed notice is an answer too), and the held-address + * bell rings once per OPEN held conflict. + * 8. Fix round 3: the READ doors strip old rows too — `crow_list_sync_conflicts` + * (a stdio-only install never runs the boot repair) and the sync-conflicts + * page — and neither throws on a `'null'` snapshot; + * the boot repair's write is conditional on the values it read, its SELECT + * reads only rows that mention a local-only column, and the snapshot strip + * returns the caller's own string when there is nothing to remove. + * + * Harness: real instance DBs (scripts/init-db.js per dir, like paired Crow + * boxes), real InstanceSyncManagers over stub Hypercore feeds that carry the + * SIGNED wire entries, one shared identity so sign/verify pass (pattern: + * tests/providers-war-sim.test.js). + */ +import { test, after } from "node:test"; +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createDbClient } from "../servers/db.js"; +import { InstanceSyncManager, rowsEquivalent, stripConflictSnapshot } from "../servers/sharing/instance-sync.js"; +import { upsertProvider, listProvidersAll, disableProvider, setProviderSyncManager } from "../servers/shared/providers-db.js"; +import { emitOrQueue, _setEligibilityForTest } from "../servers/shared/sync-emit.js"; +import { drainOnce } from "../servers/sharing/sync-outbox-drain.js"; +import { + providerNeedsKey, providerKeyNoticeSource, noticeProviderKeyNeeded, +} from "../servers/shared/provider-key-need.js"; +import { sign } from "../servers/sharing/identity.js"; +import { endpointHoldNoticeSource } from "../servers/shared/provider-endpoint.js"; +import * as ed from "../node_modules/@noble/ed25519/index.js"; +import { trustRemotesOnApply } from "./fixtures/trusted-apply.mjs"; + +const A_ID = "aaaa1111-0000-4000-8000-0000000a0001"; +const B_ID = "bbbb1111-0000-4000-8000-0000000b0001"; + +const KEY_A = "sk-test-key-on-a"; +const KEY_B = "sk-other-key-on-b"; + +const TEST_PRIV = Buffer.alloc(32, 0x7b); +const IDENTITY = { + ed25519Priv: TEST_PRIV, + ed25519Pubkey: Buffer.from(await ed.getPublicKey(TEST_PRIV)).toString("hex"), +}; + +// getOrCreateLocalInstanceId() (called by upsertProvider / emitOrQueue) reads +// CROW_DATA_DIR directly — pin it to a scratch dir so the real ~/.crow is never +// touched. Restored in after() because run-suite.mjs sets it for the process. +const idDir = mkdtempSync(join(tmpdir(), "crow-n1b7-instanceid-")); +const PREV_DATA_DIR = process.env.CROW_DATA_DIR; +process.env.CROW_DATA_DIR = idDir; + +const _dirs = []; +after(() => { + setProviderSyncManager(null); + _setEligibilityForTest(null); + if (PREV_DATA_DIR === undefined) delete process.env.CROW_DATA_DIR; + else process.env.CROW_DATA_DIR = PREV_DATA_DIR; + rmSync(idDir, { recursive: true, force: true }); + for (const d of _dirs) rmSync(d, { recursive: true, force: true }); + _dirs.length = 0; +}); + +/** A real, migrated instance DB + a real sync manager driving stub feeds. */ +function freshInstance(label, instanceId) { + const dir = mkdtempSync(join(tmpdir(), `crow-n1b7-${label}-`)); + _dirs.push(dir); + execFileSync(process.execPath, ["scripts/init-db.js"], { + env: { ...process.env, CROW_DATA_DIR: dir }, + stdio: "pipe", + cwd: join(import.meta.dirname, ".."), + }); + const db = createDbClient(join(dir, "crow.db")); + // PT-1: entries apply only from a trusted, live paired peer; the fixture + // pairs the remote for the duration of an apply run. + const mgr = trustRemotesOnApply(new InstanceSyncManager(IDENTITY, db, instanceId)); + mgr.feedsDisabled = false; + const feeds = new Map(); + mgr.outFeeds = feeds; + return { + dir, db, mgr, + /** The outbound side to one peer: what the manager appends IS the wire. */ + peerFeed(peerId) { + const feed = { + entries: [], + get length() { return feed.entries.length; }, + async get(seq) { return feed.entries[seq]; }, + async append(entry) { feed.entries.push(entry); return feed.entries.length - 1; }, + }; + feeds.set(peerId, feed); + return feed; + }, + async row(id) { + const { rows } = await db.execute({ sql: "SELECT * FROM providers WHERE id = ?", args: [id] }); + return rows[0] ?? null; + }, + async conflicts() { + const { rows } = await db.execute("SELECT COUNT(*) AS n FROM sync_conflicts"); + return Number(rows[0].n); + }, + async close() { + try { db.close(); } catch {} + }, + }; +} + +/** A signed entry exactly as emitChange would build one (what an OLD peer sends). */ +function signedEntry({ op, row, lamport_ts, instance_id }) { + const entry = { table: "providers", op, row, lamport_ts, instance_id }; + entry.signature = sign(JSON.stringify(entry), IDENTITY.ed25519Priv); + return entry; +} + +/** A stub in-feed over already-signed entries. */ +function stubFeed(entries) { + return { + entries, + get length() { return entries.length; }, + async get(seq) { return entries[seq]; }, + }; +} + +const wireHasKey = (feed) => feed.entries.some((e) => Object.hasOwn(e.row, "api_key")); + +const CLOUD = { baseUrl: "https://api.openai.com/v1", host: "cloud", providerType: "openai" }; +const MODELS_1 = [{ id: "gpt-4o-mini" }]; +const MODELS_2 = [{ id: "gpt-4o-mini" }, { id: "gpt-4o" }]; + +/* ══════════════════════════════ 1. two-instance, both directions ══════════════ */ + +test("two instances: the key stays where it was entered, in BOTH directions, while address/models converge", async () => { + const a = freshInstance("two-a", A_ID); + const b = freshInstance("two-b", B_ID); + const feedAtoB = a.peerFeed(B_ID); + const feedBtoA = b.peerFeed(A_ID); + + try { + // ── A adds a cloud provider WITH a key ────────────────────────────────── + setProviderSyncManager(a.mgr); + await upsertProvider(a.db, { id: "prov-cloud", ...CLOUD, models: MODELS_1, apiKey: KEY_A }); + + assert.equal(feedAtoB.entries.length, 1, "A emitted exactly one wire entry"); + const firstWire = feedAtoB.entries[0]; + assert.equal(Object.hasOwn(firstWire.row, "api_key"), false, + "the wire row must not carry api_key — this is the whole audit item"); + assert.equal(firstWire.row.base_url, CLOUD.baseUrl); + assert.deepEqual(JSON.parse(firstWire.row.models), MODELS_1); + + // ── B applies it: same provider, same endpoint/models, NO key ──────────── + await b.mgr._processNewEntries(A_ID, feedAtoB); + const bRow = await b.row("prov-cloud"); + assert.ok(bRow, "the provider row itself still syncs"); + assert.equal(bRow.base_url, CLOUD.baseUrl); + assert.deepEqual(JSON.parse(bRow.models), MODELS_1); + assert.equal(bRow.api_key, null, "B holds no key: keys never leave the instance that entered them"); + + // ── B enters its OWN key K2; A keeps K ────────────────────────────────── + setProviderSyncManager(b.mgr); + const bPanelRow = (await listProvidersAll(b.db)).find((p) => p.id === "prov-cloud"); + await upsertProvider(b.db, { ...bPanelRow, apiKey: KEY_B }); + assert.equal(feedBtoA.entries.length, 1, "B emitted its key write"); + assert.equal(Object.hasOwn(feedBtoA.entries[0].row, "api_key"), false, + "B's own key must not ride back to A"); + + await a.mgr._processNewEntries(B_ID, feedBtoA); + assert.equal((await a.row("prov-cloud")).api_key, KEY_A, "A keeps the key IT entered"); + assert.equal((await b.row("prov-cloud")).api_key, KEY_B, "B keeps the key IT entered"); + + // ── CONVERGENCE while the keys differ ─────────────────────────────────── + // The two instances now hold the same content and different keys. That must + // read as CONVERGED: the equivalence judgement ignores api_key, a re-delivery + // at an equal lamport is noise, and neither side logs a conflict. + const lastB = feedBtoA.entries[0]; + assert.equal(rowsEquivalent(await a.row("prov-cloud"), lastB.row), true, + "the equivalence/digest judgement ignores api_key (else differing keys look divergent forever)"); + const verdict = await a.mgr._checkConflict("providers", "prov-cloud", lastB.lamport_ts, B_ID, lastB.row, "update"); + assert.equal(verdict, "skip", "an equal-lamport re-delivery is noise, not a conflict"); + + // Full replay of BOTH feeds (checkpoints reset so the real apply path runs + // again, not just _checkConflict): silent on both sides. + await a.mgr._setLastAppliedSeq(B_ID, 0, null); + await b.mgr._setLastAppliedSeq(A_ID, 0, null); + await a.mgr._processNewEntries(B_ID, feedBtoA); + await b.mgr._processNewEntries(A_ID, feedAtoB); + assert.equal(await a.conflicts(), 0, "A logs no conflict while the keys differ"); + assert.equal(await b.conflicts(), 0, "B logs no conflict while the keys differ"); + assert.equal((await a.row("prov-cloud")).api_key, KEY_A); + assert.equal((await b.row("prov-cloud")).api_key, KEY_B); + + // ── A edits the models; B gets them and keeps K2 ──────────────────────── + setProviderSyncManager(a.mgr); + const aPanelRow = (await listProvidersAll(a.db)).find((p) => p.id === "prov-cloud"); + await upsertProvider(a.db, { ...aPanelRow, models: MODELS_2 }); + assert.equal(feedAtoB.entries.length, 2); + + await b.mgr._processNewEntries(A_ID, feedAtoB); + const bAfter = await b.row("prov-cloud"); + assert.deepEqual(JSON.parse(bAfter.models), MODELS_2, "the models edit propagates"); + assert.equal(bAfter.api_key, KEY_B, "…and never disturbs B's own key"); + assert.equal((await a.row("prov-cloud")).api_key, KEY_A); + + // ── No key on ANY wire entry, either direction ────────────────────────── + assert.equal(wireHasKey(feedAtoB), false); + assert.equal(wireHasKey(feedBtoA), false); + } finally { + setProviderSyncManager(null); + await a.close(); + await b.close(); + } +}); + +/* ══════════════════════════════ 2. an OLD peer still sends the key ═════════════ */ + +test("old peer: a raw inbound row carrying api_key writes nothing — insert", async () => { + const b = freshInstance("old-insert", B_ID); + try { + const entry = signedEntry({ + op: "insert", + lamport_ts: 500, + instance_id: A_ID, + row: { + id: "prov-old-insert", base_url: CLOUD.baseUrl, models: JSON.stringify(MODELS_1), + provider_type: "openai", api_key: "sk-leaked-by-old-peer", instance_id: A_ID, + }, + }); + await b.mgr._processNewEntries(A_ID, stubFeed([entry])); + + const inserted = await b.row("prov-old-insert"); + assert.ok(inserted, "the row itself is still applied (only the key is local)"); + assert.equal(inserted.base_url, CLOUD.baseUrl); + assert.equal(inserted.api_key, null, "a new row from a peer is inserted with NO local key"); + assert.equal(await b.conflicts(), 0); + } finally { + await b.close(); + } +}); + +test("old peer: a raw inbound row carrying api_key never overwrites the local key — update", async () => { + const b = freshInstance("old-update", B_ID); + try { + await b.db.execute({ + sql: "INSERT INTO providers (id, base_url, models, provider_type, api_key, lamport_ts, instance_id) VALUES (?, ?, ?, ?, ?, 100, ?)", + args: ["prov-old-update", CLOUD.baseUrl, JSON.stringify(MODELS_1), "openai", KEY_B, B_ID], + }); + const entry = signedEntry({ + op: "update", + lamport_ts: 900, + instance_id: A_ID, + row: { + id: "prov-old-update", base_url: CLOUD.baseUrl, models: JSON.stringify(MODELS_2), + provider_type: "openai", api_key: "sk-leaked-by-old-peer", + }, + }); + await b.mgr._processNewEntries(A_ID, stubFeed([entry])); + + const updated = await b.row("prov-old-update"); + assert.deepEqual(JSON.parse(updated.models), MODELS_2, "the peer's real content still applies"); + assert.equal(updated.api_key, KEY_B, "an inbound row never writes or overwrites the local key"); + // …and the peer's key was not even recorded as a conflict's losing version. + const { rows: conflicts } = await b.db.execute("SELECT losing_data FROM sync_conflicts"); + assert.equal(conflicts.length, 0, "a dropped local-only column manufactures no conflict row"); + } finally { + await b.close(); + } +}); + +test("old peer: an update that OMITS api_key leaves the local key alone", async () => { + const b = freshInstance("old-sparse", B_ID); + try { + await b.db.execute({ + sql: "INSERT INTO providers (id, base_url, models, provider_type, api_key, lamport_ts, instance_id) VALUES (?, ?, ?, ?, ?, 100, ?)", + args: ["prov-missing-key", CLOUD.baseUrl, JSON.stringify(MODELS_1), "openai", KEY_B, B_ID], + }); + const entry = signedEntry({ + op: "update", + lamport_ts: 1200, + instance_id: A_ID, + row: { id: "prov-missing-key", base_url: CLOUD.baseUrl, models: JSON.stringify(MODELS_2) }, + }); + await b.mgr._processNewEntries(A_ID, stubFeed([entry])); + + const sparse = await b.row("prov-missing-key"); + assert.equal(sparse.api_key, KEY_B, "an absent api_key on the wire never nulls the local one"); + assert.deepEqual(JSON.parse(sparse.models), MODELS_2); + } finally { + await b.close(); + } +}); + +test("conflict restore: a losing providers snapshot carrying api_key never writes it back", async () => { + const b = freshInstance("old-restore", B_ID); + try { + await b.db.execute({ + sql: "INSERT INTO providers (id, base_url, models, provider_type, api_key, lamport_ts, instance_id) VALUES (?, ?, ?, ?, ?, 100, ?)", + args: ["prov-restore", CLOUD.baseUrl, JSON.stringify(MODELS_1), "openai", KEY_B, B_ID], + }); + const live = await b.row("prov-restore"); + // A conflict row as the pre-upgrade code recorded one: the LOSING side is a + // peer's wire row that still carried its key. + await b.db.execute({ + sql: `INSERT INTO sync_conflicts + (table_name, row_id, winning_instance_id, losing_instance_id, + winning_lamport_ts, losing_lamport_ts, winning_data, losing_data, op) + VALUES ('providers', 'prov-restore', ?, ?, 100, 90, ?, ?, 'update')`, + args: [B_ID, A_ID, JSON.stringify(live), + JSON.stringify({ id: "prov-restore", base_url: CLOUD.baseUrl, provider_type: "openai", + models: JSON.stringify(MODELS_2), api_key: "sk-leaked-by-old-peer" })], + }); + const { rows } = await b.db.execute("SELECT id FROM sync_conflicts WHERE row_id = 'prov-restore'"); + const { restoreConflict } = await import("../servers/sharing/sync-conflict-resolve.js"); + const outcome = await restoreConflict(b.db, rows[0].id, { instanceSync: null }); + assert.equal(outcome.status, "applied"); + const after = await b.row("prov-restore"); + assert.deepEqual(JSON.parse(after.models), MODELS_2, "the restored content applies"); + assert.equal(after.api_key, KEY_B, "the losing snapshot's api_key is never written back"); + } finally { + await b.close(); + } +}); + +/* ═══════════════════════ 3. every emit door strips api_key ═══════════════════ */ + +test("every emit path emits no api_key: emitChange, upsert, disable, backfill, outbox queue and drain", async () => { + const inst = freshInstance("doors", A_ID); + const peerFeed = inst.peerFeed(B_ID); + + try { + await inst.db.execute({ + sql: "INSERT INTO providers (id, base_url, models, provider_type, api_key, lamport_ts, instance_id) VALUES (?, ?, ?, ?, ?, 10, ?)", + args: ["prov-door", CLOUD.baseUrl, JSON.stringify(MODELS_1), "openai", KEY_A, A_ID], + }); + const dbRow = await inst.row("prov-door"); + + // (a) emitChange directly — the door every manager-side emit uses. + setProviderSyncManager(null); + await inst.mgr.emitChange("providers", "update", dbRow); + assert.equal(peerFeed.entries.length, 1); + assert.equal(Object.hasOwn(peerFeed.entries[0].row, "api_key"), false, "emitChange strips api_key"); + + // (b) providers-db upsert (the panel / bundle / reconciler write path). + setProviderSyncManager(inst.mgr); + const panelRow = (await listProvidersAll(inst.db)).find((p) => p.id === "prov-door"); + await upsertProvider(inst.db, { ...panelRow, models: MODELS_2, apiKey: KEY_A }); + assert.equal(peerFeed.entries.length, 2); + assert.equal(Object.hasOwn(peerFeed.entries[1].row, "api_key"), false, "upsertProvider's emit carries no api_key"); + + // (c) disableProvider — its SELECT * spread hands the whole row to the emit. + await disableProvider(inst.db, "prov-door"); + assert.equal(peerFeed.entries.length, 3); + assert.equal(Object.hasOwn(peerFeed.entries[2].row, "api_key"), false, "a soft-delete emit carries no api_key"); + assert.equal(Number(peerFeed.entries[2].row.disabled), 1); + + // (d) the new-peer backfill (a SELECT * over the table, re-emitted as inserts). + setProviderSyncManager(null); + const before = peerFeed.entries.length; + const emitted = await inst.mgr.backfillProvidersForNewPeers(); + assert.ok(emitted >= 1, "the backfill emitted the row"); + const backfilled = peerFeed.entries.slice(before); + assert.ok(backfilled.length >= 1, "there is a wire payload to inspect"); + for (const e of backfilled) { + assert.equal(Object.hasOwn(e.row, "api_key"), false, "backfill carries no api_key"); + } + + // (e) the stdio outbox (no live manager): the QUEUED payload itself, which + // the drain later replays verbatim. + _setEligibilityForTest(() => true); + const stored = await inst.row("prov-door"); + try { + await emitOrQueue(null, inst.db, "providers", "update", { ...stored, api_key: KEY_A }); + } finally { + _setEligibilityForTest(null); + } + const { rows: boxed } = await inst.db.execute("SELECT row_json FROM sync_outbox ORDER BY id DESC LIMIT 1"); + assert.ok(boxed[0], "the write was queued"); + const queued = JSON.parse(boxed[0].row_json); + assert.equal(Object.hasOwn(queued, "api_key"), false, "the sync outbox row carries no api_key"); + assert.equal(queued.id, "prov-door"); + + // (f) the drain replays that queued row through emitChange onto the wire. + const drained = []; + inst.mgr.outFeeds.set("peer-drain", { append: async (e) => { drained.push(e); } }); + const drainedResult = await drainOnce(inst.mgr, inst.db); + assert.ok(drainedResult.emitted >= 1, "the drain delivered the queued row"); + assert.ok(drained.length >= 1, "the drain appended a real wire entry"); + for (const e of drained) { + assert.equal(Object.hasOwn(e.row, "api_key"), false, "the drained wire entry carries no api_key"); + } + } finally { + setProviderSyncManager(null); + _setEligibilityForTest(null); + await inst.close(); + } +}); + +/* ═══════════════════════════════════ 4. settings UI ═══════════════════════════ */ + +test("providers tab: the key line by the key field, and 'needs a key' on a peer row without one", async () => { + const { default: tab, needsKeyLine } = await import("../servers/gateway/dashboard/settings/sections/llm/providers-tab.js"); + const { getOrCreateLocalInstanceId } = await import("../servers/gateway/instance-registry.js"); + const ownId = getOrCreateLocalInstanceId(); + + const rows = [ + { id: "peer-cloud", base_url: CLOUD.baseUrl, api_key: null, host: "cloud", bundle_id: null, + provider_type: "openai", description: null, models: JSON.stringify(MODELS_1), disabled: 0, + instance_id: "peer-instance-id" }, + { id: "mine-own", base_url: CLOUD.baseUrl, api_key: null, host: "cloud", bundle_id: null, + provider_type: "openai", description: null, models: JSON.stringify(MODELS_1), disabled: 0, + instance_id: ownId }, + { id: "lan-llama", base_url: "http://100.64.20.9:8080/v1", api_key: null, host: "cloud", bundle_id: null, + provider_type: "openai-compat", description: null, models: JSON.stringify([{ id: "qwen3-4b" }]), disabled: 0, + instance_id: "peer-instance-id" }, + { id: "peer-with-key", base_url: "https://api.anthropic.com", api_key: "sk-not-rendered", host: "cloud", bundle_id: null, + provider_type: "anthropic", description: null, models: JSON.stringify(MODELS_1), disabled: 0, + instance_id: "peer-instance-id" }, + ]; + const db = { + execute: async (q) => { + const sql = typeof q === "string" ? q : q.sql; + if (sql.includes("FROM providers")) return { rows }; + return { rows: [] }; + }, + }; + + const html = await tab.render({ db, lang: "en" }); + assert.match(html, /Keys stay on this device; they are not shared with paired devices\./, + "the line by the key field says keys never leave this device"); + assert.doesNotMatch(html, /this API key will sync to your paired Crow instances/, + "the old, now-false claim is gone"); + // Exactly one row is told it needs a key: the peer's keyed provider. + assert.equal((html.match(/Needs an API key on this device\./g) || []).length, 1); + assert.match(needsKeyLine({ id: "x", baseUrl: CLOUD.baseUrl, apiKey: null, instance_id: "peer-instance-id" }, { ownInstanceId: ownId }), + /Needs an API key on this device\./); + assert.equal(needsKeyLine({ id: "x", baseUrl: CLOUD.baseUrl, apiKey: null, instance_id: ownId }, { ownInstanceId: ownId }), "", + "a locally created provider is never told to enter a key"); + assert.equal(needsKeyLine({ id: "x", baseUrl: "http://100.64.20.9:8080/v1", apiKey: null, instance_id: "peer-instance-id" }, { ownInstanceId: ownId }), "", + "a keyless provider (a llama.cpp on the tailnet) is not told to enter a key"); + assert.equal(needsKeyLine({ id: "x", baseUrl: CLOUD.baseUrl, apiKey: "sk-here", instance_id: "peer-instance-id" }, { ownInstanceId: ownId }), "", + "nothing to say once a key is here"); + // The Spanish line exists (i18n parity is enforced by tests/i18n-global-parity.test.js). + const { t } = await import("../servers/gateway/dashboard/shared/i18n.js"); + assert.notEqual(t("settings.providers.keyStaysLocal", "es"), t("settings.providers.keyStaysLocal", "en")); + assert.notEqual(t("settings.providers.needsKeyHere", "es"), t("settings.providers.needsKeyHere", "en")); +}); + + +/* ═════════ 5b. a peer may not repoint a provider that holds a key here ═══════ */ + +const HOLD_ID = "prov-hold"; +const URL_OLD = CLOUD.baseUrl; // https://api.openai.com/v1 +const URL_NEW = "https://openai.peer.example/v1"; // where the peer points it +const URL_NEWER = "https://openai.peer2.example/v1"; + +/** The held/conflict rows this device recorded for one providers row id. */ +async function providerConflicts(db, rowId = HOLD_ID) { + const { rows } = await db.execute({ + sql: `SELECT * FROM sync_conflicts WHERE table_name = 'providers' AND row_id = ? ORDER BY id`, + args: [rowId], + }); + return rows; +} + +const holdNoticeRows = async (db, id = HOLD_ID) => { + const { rows } = await db.execute({ + sql: "SELECT id, title, body, action_url, is_read FROM notifications WHERE source = ? ORDER BY id", + args: [endpointHoldNoticeSource(id)], + }); + return rows; +}; + +/** + * A and B share one cloud provider id. A's copy has NO key; B applies it, then + * enters its OWN key — the state under test: the key lives HERE, and A is about + * to want the address moved. + */ +async function keyedProviderOnBoth(label) { + const a = freshInstance(`${label}-a`, A_ID); + const b = freshInstance(`${label}-b`, B_ID); + const feedAtoB = a.peerFeed(B_ID); + const feedBtoA = b.peerFeed(A_ID); + + setProviderSyncManager(a.mgr); + await upsertProvider(a.db, { id: HOLD_ID, ...CLOUD, models: MODELS_1 }); + await b.mgr._processNewEntries(A_ID, feedAtoB); + + setProviderSyncManager(b.mgr); + const bPanel = (await listProvidersAll(b.db)).find((p) => p.id === HOLD_ID); + await upsertProvider(b.db, { ...bPanel, apiKey: KEY_B }); + await a.mgr._processNewEntries(B_ID, feedBtoA); + return { a, b, feedAtoB }; +} + +/** A repoints the provider — a plain local edit on A, which emits to B. */ +async function repointOnA(a, url) { + setProviderSyncManager(a.mgr); + const panel = (await listProvidersAll(a.db)).find((p) => p.id === HOLD_ID); + await upsertProvider(a.db, { ...panel, baseUrl: url }); +} + +test("endpoint hold: a peer's address change is held when the key lives here — one conflict, one bell, by name", async () => { + const { a, b, feedAtoB } = await keyedProviderOnBoth("hold-one"); + try { + assert.equal((await b.row(HOLD_ID)).base_url, URL_OLD, "both boxes start on the same address"); + assert.equal((await b.row(HOLD_ID)).api_key, KEY_B, "B holds the key on this device"); + + await repointOnA(a, URL_NEW); + await b.mgr._processNewEntries(A_ID, feedAtoB); + + const live = await b.row(HOLD_ID); + assert.equal(live.base_url, URL_OLD, "B's row is NOT repointed — the change is held"); + assert.equal(live.api_key, KEY_B, "and the key stays where it was entered"); + + const conflicts = await providerConflicts(b.db); + assert.equal(conflicts.length, 1, "exactly one conflict records the held change"); + assert.equal(Number(conflicts[0].resolved), 0, "it is unresolved: it waits for the operator"); + assert.equal(conflicts[0].op, "update", "it reads as a proposed update (Restore is available)"); + const losing = JSON.parse(conflicts[0].losing_data); + assert.equal(losing.base_url, URL_NEW, "the held version carries the peer's new address"); + assert.equal(Object.hasOwn(losing, "api_key"), false, "never a key in the held snapshot"); + const winning = JSON.parse(conflicts[0].winning_data); + assert.equal(winning.base_url, URL_OLD, "the kept version is this device's row"); + // The conflict row is shown on the sync-conflicts page and returned by + // crow_list_sync_conflicts: this device's key must not be stored in it. + assert.equal(Object.hasOwn(winning, "api_key"), false, "never this device's key in the kept snapshot"); + assert.equal(conflicts[0].winning_data.includes(KEY_B), false, "the key appears nowhere in the stored row"); + + const notices = await holdNoticeRows(b.db); + assert.equal(notices.length, 1, "one bell item, not one per entry"); + assert.match(notices[0].title, /OpenAI: another device wants to change its address/, + "the notice names the provider"); + assert.equal(notices[0].action_url, "/dashboard/settings?section=sync-conflicts", + "it links to the conflict review"); + } finally { + setProviderSyncManager(null); + await a.close(); + await b.close(); + } +}); + +test("endpoint hold: a newer address from the same peer REPLACES the held one — still one conflict", async () => { + const { a, b, feedAtoB } = await keyedProviderOnBoth("hold-replace"); + try { + await repointOnA(a, URL_NEW); + await b.mgr._processNewEntries(A_ID, feedAtoB); + await repointOnA(a, URL_NEWER); + await b.mgr._processNewEntries(A_ID, feedAtoB); + + assert.equal((await b.row(HOLD_ID)).base_url, URL_OLD, "B's row is still untouched"); + const conflicts = await providerConflicts(b.db); + assert.equal(conflicts.length, 1, "a second edit replaces the held version, it does not pile up"); + assert.equal(Number(conflicts[0].resolved), 0); + assert.equal(JSON.parse(conflicts[0].losing_data).base_url, URL_NEWER, + "the held version is the NEWEST address"); + assert.equal((await holdNoticeRows(b.db)).length, 1, "and the bell was not rung again"); + } finally { + setProviderSyncManager(null); + await a.close(); + await b.close(); + } +}); + +test("endpoint hold: accepting applies the address AND keeps this device's key", async () => { + const { a, b, feedAtoB } = await keyedProviderOnBoth("hold-accept"); + try { + await repointOnA(a, URL_NEW); + await b.mgr._processNewEntries(A_ID, feedAtoB); + const [conflict] = await providerConflicts(b.db); + + const { restoreConflict } = await import("../servers/sharing/sync-conflict-resolve.js"); + const outcome = await restoreConflict(b.db, conflict.id, { instanceSync: b.mgr }); + assert.equal(outcome.status, "applied", "the held version can be applied from the review page"); + + const live = await b.row(HOLD_ID); + assert.equal(live.base_url, URL_NEW, "the accepted address is now used here"); + assert.equal(live.api_key, KEY_B, "and it is B's own key that now goes there — the operator's informed choice"); + assert.equal(Number((await providerConflicts(b.db))[0].resolved), 1, "the conflict is closed"); + } finally { + setProviderSyncManager(null); + await a.close(); + await b.close(); + } +}); + +test("endpoint hold: dismissing keeps the address, and a re-delivery of the same entry raises nothing", async () => { + const { a, b, feedAtoB } = await keyedProviderOnBoth("hold-dismiss"); + try { + await repointOnA(a, URL_NEW); + await b.mgr._processNewEntries(A_ID, feedAtoB); + const [conflict] = await providerConflicts(b.db); + + const { resolveConflict } = await import("../servers/sharing/sync-conflict-resolve.js"); + await resolveConflict(b.db, conflict.id); + assert.equal((await b.row(HOLD_ID)).base_url, URL_OLD, "keeping the current version keeps the address"); + + // Re-deliver the SAME feed entry (fresh checkpoints, real apply path). + await b.mgr._setLastAppliedSeq(A_ID, 0, null); + await b.mgr._processNewEntries(A_ID, feedAtoB); + + assert.equal((await b.row(HOLD_ID)).base_url, URL_OLD, "still the local address"); + const after = await providerConflicts(b.db); + assert.equal(after.length, 1, "a re-delivery does not stack a second conflict on top of the dismissed one"); + assert.equal(Number(after[0].resolved), 1, "the dismissal stands"); + assert.equal((await holdNoticeRows(b.db)).length, 1, "and the bell does not ring again"); + } finally { + setProviderSyncManager(null); + await a.close(); + await b.close(); + } +}); + +test("endpoint hold: the same address change to a row with NO local key applies directly, with no conflict", async () => { + const a = freshInstance("hold-keyless-a", A_ID); + const b = freshInstance("hold-keyless-b", B_ID); + const feedAtoB = a.peerFeed(B_ID); + b.peerFeed(A_ID); + try { + // A llama.cpp on the tailnet (100.64.0.0/10, keyless like a LAN box): no key + // on either side (the ruling's keyless case). + setProviderSyncManager(a.mgr); + await upsertProvider(a.db, { + id: "prov-lan", baseUrl: "http://100.64.20.9:8080/v1", host: "cloud", + providerType: "openai-compat", models: [{ id: "qwen3-4b" }], + }); + await b.mgr._processNewEntries(A_ID, feedAtoB); + + const panel = (await listProvidersAll(a.db)).find((p) => p.id === "prov-lan"); + await upsertProvider(a.db, { ...panel, baseUrl: "http://100.64.20.10:8080/v1" }); + await b.mgr._processNewEntries(A_ID, feedAtoB); + + assert.equal((await b.row("prov-lan")).base_url, "http://100.64.20.10:8080/v1", + "a keyless row updates freely — the hold protects a key, and there is none here"); + assert.equal((await providerConflicts(b.db, "prov-lan")).length, 0, "and it records no conflict"); + } finally { + setProviderSyncManager(null); + await a.close(); + await b.close(); + } +}); + +test("endpoint hold: a models-only change to a keyed row applies directly, as before", async () => { + const { a, b, feedAtoB } = await keyedProviderOnBoth("hold-models"); + try { + setProviderSyncManager(a.mgr); + const panel = (await listProvidersAll(a.db)).find((p) => p.id === HOLD_ID); + await upsertProvider(a.db, { ...panel, models: MODELS_2 }); + await b.mgr._processNewEntries(A_ID, feedAtoB); + + const live = await b.row(HOLD_ID); + assert.deepEqual(JSON.parse(live.models), MODELS_2, "the models edit still propagates"); + assert.equal(live.base_url, URL_OLD); + assert.equal(live.api_key, KEY_B); + assert.equal((await providerConflicts(b.db)).length, 0, "no endpoint moved, so nothing is held"); + } finally { + setProviderSyncManager(null); + await a.close(); + await b.close(); + } +}); + +test("endpoint hold: a trailing-slash-only difference is not an endpoint change", async () => { + const { a, b, feedAtoB } = await keyedProviderOnBoth("hold-slash"); + try { + await repointOnA(a, `${URL_OLD}/`); + await b.mgr._processNewEntries(A_ID, feedAtoB); + + const live = await b.row(HOLD_ID); + assert.equal(live.base_url, `${URL_OLD}/`, "the row applies as any non-endpoint edit would"); + assert.equal(live.api_key, KEY_B); + assert.equal((await providerConflicts(b.db)).length, 0, + "`…/v1` and `…/v1/` are the same endpoint — nothing to hold"); + } finally { + setProviderSyncManager(null); + await a.close(); + await b.close(); + } +}); + +test("endpoint hold: an INSERT that collides with a keyed local row and carries another URL is held", async () => { + const b = freshInstance("hold-insert", B_ID); + try { + await b.db.execute({ + sql: "INSERT INTO providers (id, base_url, models, provider_type, api_key, lamport_ts, instance_id) VALUES (?, ?, ?, ?, ?, 100, ?)", + args: [HOLD_ID, URL_OLD, JSON.stringify(MODELS_1), "openai", KEY_B, B_ID], + }); + // A peer that never saw this row sends its own copy as an insert (the + // new-pairing backfill does exactly this) — at a URL of its choosing. + const entry = signedEntry({ + op: "insert", + lamport_ts: 900, + instance_id: A_ID, + row: { + id: HOLD_ID, base_url: URL_NEW, models: JSON.stringify(MODELS_1), + provider_type: "openai", instance_id: A_ID, + }, + }); + await b.mgr._processNewEntries(A_ID, stubFeed([entry])); + + const live = await b.row(HOLD_ID); + assert.equal(live.base_url, URL_OLD, "the colliding insert is held, not applied"); + assert.equal(live.api_key, KEY_B); + const conflicts = await providerConflicts(b.db); + assert.equal(conflicts.length, 1, "the held insert is recorded"); + assert.equal(conflicts[0].op, "update", "recorded as a proposed update so it CAN be accepted"); + assert.equal(JSON.parse(conflicts[0].losing_data).base_url, URL_NEW); + assert.equal((await holdNoticeRows(b.db)).length, 1, "one bell item, by name"); + } finally { + await b.close(); + } +}); + +test("sync-conflicts page: a held providers conflict shows the address change, escaped", async () => { + const inst = freshInstance("hold-ui", B_ID); + try { + const winning = JSON.stringify({ id: HOLD_ID, base_url: URL_OLD, api_key: KEY_B }); + // A value that is not a URL at all: rendered as written, so the line MUST escape it. + const hostile = 'evil { + const { rows } = await db.execute({ + sql: "SELECT id, title, body, action_url, is_read FROM notifications WHERE source = ? ORDER BY id", + args: [providerKeyNoticeSource(id)], + }); + return rows; +}; + +test("providerNeedsKey: a keyed cloud type/preset needs one; a LAN llama.cpp and a tailnet row do not", () => { + // Cloud preset hosts, whatever the type column says. + assert.equal(providerNeedsKey({ base_url: "https://api.openai.com/v1", provider_type: null }), true); + assert.equal(providerNeedsKey({ base_url: "https://api.anthropic.com", provider_type: "anthropic" }), true); + assert.equal(providerNeedsKey({ base_url: "https://dashscope-intl.aliyuncs.com/compatible-mode/v1" }), true); + // Cloud SDK types on an unknown public host: nothing local to reach, so a key. + assert.equal(providerNeedsKey({ base_url: "https://llm.example.com/v1", provider_type: "openai" }), true); + // Keyless: a tailnet/CGNAT box (100.64.0.0/10 — the repo's placeholder range), + // a loopback sidecar, a bare name, a LAN-ish name. + assert.equal(providerNeedsKey({ base_url: "http://100.64.20.9:8080/v1", provider_type: "openai-compat" }), false); + assert.equal(providerNeedsKey({ base_url: "http://100.64.20.9:8080/v1", provider_type: "openai" }), false, + "a tailnet address is keyless whatever the adapter type says"); + assert.equal(providerNeedsKey({ base_url: "http://100.64.20.1:8003/v1" }), false); + assert.equal(providerNeedsKey({ base_url: "http://127.0.0.1:8011/v1", provider_type: "openai" }), false); + assert.equal(providerNeedsKey({ base_url: "http://ollama:11434/v1" }), false); + assert.equal(providerNeedsKey({ base_url: "http://box.local:8000/v1" }), false); + // The panel shape (provider-row.js) is accepted too. + assert.equal(providerNeedsKey({ baseUrl: "https://api.openai.com/v1" }), true); + // No endpoint at all: nothing to key, never nag. + assert.equal(providerNeedsKey({ base_url: "", provider_type: "openai" }), false); + assert.equal(providerNeedsKey(null), false); +}); + +test("'needs a key' notice: raised ONCE by name for a keyed row, not for a keyless one, and quieted when a key is entered here", async () => { + const a = freshInstance("notice-a", A_ID); + const b = freshInstance("notice-b", B_ID); + const feedAtoB = a.peerFeed(B_ID); + + try { + setProviderSyncManager(a.mgr); + await upsertProvider(a.db, { id: "prov-keyed", ...CLOUD, models: MODELS_1, apiKey: KEY_A }); + await upsertProvider(a.db, { + id: "prov-lan-llama", baseUrl: "http://100.64.20.9:8080/v1", host: "cloud", + providerType: "openai-compat", models: [{ id: "qwen3-4b" }], + }); + assert.equal(feedAtoB.entries.length, 2, "both rows sync — the keyless one included"); + + await b.mgr._processNewEntries(A_ID, feedAtoB); + + // The keyed row: exactly ONE notice, by name, linking to the providers tab. + let keyed = await noticeRows(b.db, "prov-keyed"); + assert.equal(keyed.length, 1, "one notice per provider id"); + assert.match(keyed[0].title, /OpenAI needs an API key on this device/); + assert.equal(keyed[0].action_url, "/dashboard/settings?section=llm&tab=providers"); + assert.equal(Number(keyed[0].is_read), 0); + + // The keyless local model server: never nagged. + assert.equal((await noticeRows(b.db, "prov-lan-llama")).length, 0, + "a keyless provider (llama.cpp on the LAN) is not nagged"); + + // A re-delivery (A edits the models) does not raise a second one. + setProviderSyncManager(a.mgr); + const aPanel = (await listProvidersAll(a.db)).find((p) => p.id === "prov-keyed"); + await upsertProvider(a.db, { ...aPanel, models: MODELS_2 }); + await b.mgr._processNewEntries(A_ID, feedAtoB); + keyed = await noticeRows(b.db, "prov-keyed"); + assert.equal(keyed.length, 1, "still ONE notice after a re-delivery"); + assert.equal(await noticeProviderKeyNeeded(b.db, "prov-keyed"), "already", + "an open notice is the marker — asking again is a no-op"); + + // The operator enters a key HERE: the notice is answered, and never returns. + setProviderSyncManager(b.mgr); + const bPanel = (await listProvidersAll(b.db)).find((p) => p.id === "prov-keyed"); + await upsertProvider(b.db, { ...bPanel, apiKey: KEY_B }); + keyed = await noticeRows(b.db, "prov-keyed"); + assert.equal(Number(keyed[0].is_read), 1, "entering a key here quiets the notice"); + + setProviderSyncManager(a.mgr); + const again = (await listProvidersAll(a.db)).find((p) => p.id === "prov-keyed"); + await upsertProvider(a.db, { ...again, models: [{ id: "gpt-4o" }] }); + await b.mgr._processNewEntries(A_ID, feedAtoB); + keyed = await noticeRows(b.db, "prov-keyed"); + assert.equal(keyed.length, 1, "no new notice once a key is held here"); + assert.equal((await b.row("prov-keyed")).api_key, KEY_B); + + // A provider created LOCALLY with no key is never announced: the notice is + // about a key that could not travel, not about the operator's own form. + setProviderSyncManager(b.mgr); + await upsertProvider(b.db, { id: "prov-local-own", ...CLOUD, models: MODELS_1 }); + assert.equal((await noticeRows(b.db, "prov-local-own")).length, 0, + "a locally created provider is never nagged"); + } finally { + setProviderSyncManager(null); + await a.close(); + await b.close(); + } +}); + +/* ═══════════ 6. fix round 2 — no conflict snapshot ever holds a key ══════════ */ + +/** Every stored snapshot of one providers row, parsed. */ +async function conflictSnapshots(db, rowId) { + const rows = await providerConflicts(db, rowId); + return rows.map((r) => ({ + ...r, + winning: r.winning_data ? JSON.parse(r.winning_data) : null, + losing: r.losing_data ? JSON.parse(r.losing_data) : null, + })); +} + +test("conflict snapshot: an ordinary Lamport conflict on a keyed provider stores NO key in winning_data or losing_data", async () => { + const b = freshInstance("lamport-conflict", B_ID); + try { + await b.db.execute({ + sql: "INSERT INTO providers (id, base_url, models, provider_type, api_key, lamport_ts, instance_id) VALUES (?, ?, ?, ?, ?, 900, ?)", + args: ["prov-lamport", CLOUD.baseUrl, JSON.stringify(MODELS_2), "openai", KEY_B, B_ID], + }); + // Same address (so the endpoint hold is not what is under test), an OLDER + // lamport and different content: the ordinary LWW conflict path — the local + // row wins and both versions are recorded for review. The peer's row is one + // an OLD peer would send (it still carries its own key). + const entry = signedEntry({ + op: "update", + lamport_ts: 500, + instance_id: A_ID, + row: { + id: "prov-lamport", base_url: CLOUD.baseUrl, models: JSON.stringify(MODELS_1), + provider_type: "openai", api_key: "sk-leaked-by-old-peer", + }, + }); + await b.mgr._processNewEntries(A_ID, stubFeed([entry])); + + const live = await b.row("prov-lamport"); + assert.equal(live.api_key, KEY_B, "the local version wins, key untouched"); + assert.deepEqual(JSON.parse(live.models), MODELS_2); + + const [conflict, ...rest] = await conflictSnapshots(b.db, "prov-lamport"); + assert.equal(rest.length, 0); + assert.ok(conflict, "the stale edit is recorded for review"); + for (const side of ["winning", "losing"]) { + const snapshot = conflict[side]; + assert.equal(Object.hasOwn(snapshot, "api_key"), false, + `${side}_data must not carry an api_key column`); + assert.equal(conflict[`${side}_data`].includes(KEY_B), false, + `${side}_data holds no key from this device`); + assert.equal(conflict[`${side}_data`].includes("sk-leaked-by-old-peer"), false, + `${side}_data holds no key from the peer`); + } + assert.deepEqual(conflict.losing.models, JSON.stringify(MODELS_1), + "the losing content is still recorded in full — only the key is gone"); + assert.equal(conflict.winning.base_url, CLOUD.baseUrl); + } finally { + await b.close(); + } +}); + +test("conflict restore: the stale re-snapshot writes winning_data with no key either", async () => { + const b = freshInstance("stale-resnapshot", B_ID); + try { + await b.db.execute({ + sql: "INSERT INTO providers (id, base_url, models, provider_type, api_key, lamport_ts, instance_id) VALUES (?, ?, ?, ?, ?, 100, ?)", + args: ["prov-stale", CLOUD.baseUrl, JSON.stringify(MODELS_2), "openai", KEY_B, B_ID], + }); + const live = await b.row("prov-stale"); + // A conflict recorded when the local row still had MODELS_1 (key included, as + // the pre-fix code did). The live row has moved on, so the restore's stale + // guard fires and RE-SNAPSHOTS winning_data from the live row. + await b.db.execute({ + sql: `INSERT INTO sync_conflicts + (table_name, row_id, winning_instance_id, losing_instance_id, + winning_lamport_ts, losing_lamport_ts, winning_data, losing_data, op) + VALUES ('providers', 'prov-stale', ?, ?, 100, 90, ?, ?, 'update')`, + args: [B_ID, A_ID, + JSON.stringify({ ...live, models: JSON.stringify(MODELS_1) }), + JSON.stringify({ id: "prov-stale", base_url: CLOUD.baseUrl, models: JSON.stringify(MODELS_2) })], + }); + const { rows } = await b.db.execute("SELECT id FROM sync_conflicts WHERE row_id = 'prov-stale'"); + const { restoreConflict } = await import("../servers/sharing/sync-conflict-resolve.js"); + const outcome = await restoreConflict(b.db, rows[0].id, { instanceSync: null }); + assert.equal(outcome.status, "stale", "the guard fired: the snapshot was refreshed, nothing applied"); + + const { rows: after } = await b.db.execute({ + sql: "SELECT winning_data FROM sync_conflicts WHERE id = ?", + args: [rows[0].id], + }); + const resnapshot = JSON.parse(after[0].winning_data); + assert.deepEqual(resnapshot.models, JSON.stringify(MODELS_2), "winning_data is the current live row"); + assert.equal(Object.hasOwn(resnapshot, "api_key"), false, + "the re-snapshot is a raw SELECT * — the key must not be stored in it"); + assert.equal(after[0].winning_data.includes(KEY_B), false, "the key appears nowhere in the row"); + } finally { + await b.close(); + } +}); + +test("boot redaction: old conflict rows lose their keys, is idempotent, and leaves non-provider rows untouched", async () => { + const b = freshInstance("redact", B_ID); + try { + // A pre-fix providers conflict: winning_data holds THIS device's key, + // losing_data an old peer's. Plus a memories conflict, which must not move. + const winning = JSON.stringify({ + id: "prov-old", base_url: CLOUD.baseUrl, models: JSON.stringify(MODELS_1), + provider_type: "openai", api_key: KEY_B, lamport_ts: 100, instance_id: B_ID, + }); + const losing = JSON.stringify({ + id: "prov-old", base_url: URL_NEW, models: JSON.stringify(MODELS_2), + provider_type: "openai", api_key: "sk-leaked-by-old-peer", + }); + await b.db.execute({ + sql: `INSERT INTO sync_conflicts + (table_name, row_id, winning_instance_id, losing_instance_id, + winning_lamport_ts, losing_lamport_ts, winning_data, losing_data, op) + VALUES ('providers', 'prov-old', ?, ?, 100, 90, ?, ?, 'update')`, + args: [B_ID, A_ID, winning, losing], + }); + const memoriesWinning = JSON.stringify({ id: 7, content: "a", api_key: "not-a-providers-row" }); + const memoriesLosing = JSON.stringify({ id: 7, content: "b" }); + await b.db.execute({ + sql: `INSERT INTO sync_conflicts + (table_name, row_id, winning_instance_id, losing_instance_id, + winning_lamport_ts, losing_lamport_ts, winning_data, losing_data, op) + VALUES ('memories', '7', ?, ?, 2, 3, ?, ?, 'update')`, + args: [B_ID, A_ID, memoriesWinning, memoriesLosing], + }); + + const { redactConflictSnapshots, redactConflictSnapshotsAtBoot } = + await import("../servers/sharing/sync-conflict-redact.js"); + const lines = []; + assert.equal(await redactConflictSnapshotsAtBoot(b.db, { log: (m) => lines.push(m) }), 1, + "exactly one providers row needed redacting"); + assert.equal(lines.length, 1, "one log line"); + assert.match(lines[0], /1 stored conflict snapshot row\(s\) rewritten/); + + const [redacted] = await conflictSnapshots(b.db, "prov-old"); + assert.equal(Object.hasOwn(redacted.winning, "api_key"), false); + assert.equal(Object.hasOwn(redacted.losing, "api_key"), false); + assert.equal(redacted.winning_data.includes(KEY_B), false, "this device's key is gone"); + assert.equal(redacted.losing_data.includes("sk-leaked-by-old-peer"), false, "the peer's key is gone"); + assert.equal(redacted.winning.base_url, CLOUD.baseUrl, "everything else is preserved"); + assert.equal(redacted.losing.base_url, URL_NEW); + assert.equal(Number(redacted.resolved), 0, "the row is not resolved, not deleted: the operator still reviews it"); + + const { rows: memoriesAfter } = await b.db.execute( + "SELECT winning_data, losing_data FROM sync_conflicts WHERE table_name = 'memories'"); + assert.equal(memoriesAfter.length, 1, "the non-provider conflict row is still there"); + assert.equal(memoriesAfter[0].winning_data, memoriesWinning, "byte-identical: other tables are not rewritten"); + assert.equal(memoriesAfter[0].losing_data, memoriesLosing, "byte-identical"); + + // Second run: nothing left to do, nothing re-written. + assert.equal(await redactConflictSnapshots(b.db), 0, "idempotent"); + const [again] = await conflictSnapshots(b.db, "prov-old"); + assert.equal(again.winning_data, redacted.winning_data, "the snapshot is untouched by a second run"); + assert.equal(again.losing_data, redacted.losing_data); + } finally { + await b.close(); + } +}); + +/* ═════════ 7. fix round 2 — "needs a key" rings once, not once per state ════ */ + +test("'needs a key' notice: a read or a dismissed notice is still an answer — a re-delivery does not ring again", async () => { + const a = freshInstance("notice-once-a", A_ID); + const b = freshInstance("notice-once-b", B_ID); + const feedAtoB = a.peerFeed(B_ID); + try { + setProviderSyncManager(a.mgr); + await upsertProvider(a.db, { id: "prov-read", ...CLOUD, models: MODELS_1, apiKey: KEY_A }); + await upsertProvider(a.db, { id: "prov-dismissed", ...CLOUD, models: MODELS_1, apiKey: KEY_A }); + await b.mgr._processNewEntries(A_ID, feedAtoB); + assert.equal((await noticeRows(b.db, "prov-read")).length, 1); + assert.equal((await noticeRows(b.db, "prov-dismissed")).length, 1); + + // The operator reads one and dismisses the other. Both are answers to the + // only question the notice asks. + await b.db.execute({ + sql: "UPDATE notifications SET is_read = 1 WHERE source = ?", + args: [providerKeyNoticeSource("prov-read")], + }); + await b.db.execute({ + sql: "UPDATE notifications SET is_dismissed = 1 WHERE source = ?", + args: [providerKeyNoticeSource("prov-dismissed")], + }); + + // A re-delivery (A edits the models on both rows) must not ring again. + setProviderSyncManager(a.mgr); + for (const id of ["prov-read", "prov-dismissed"]) { + const panel = (await listProvidersAll(a.db)).find((p) => p.id === id); + await upsertProvider(a.db, { ...panel, models: MODELS_2 }); + } + await b.mgr._processNewEntries(A_ID, feedAtoB); + + assert.equal((await noticeRows(b.db, "prov-read")).length, 1, "still ONE notice after a read"); + assert.equal((await noticeRows(b.db, "prov-dismissed")).length, 1, "still ONE after a dismissal"); + assert.equal(await noticeProviderKeyNeeded(b.db, "prov-read"), "already"); + assert.equal(await noticeProviderKeyNeeded(b.db, "prov-dismissed"), "already"); + } finally { + setProviderSyncManager(null); + await a.close(); + await b.close(); + } +}); + +test("'needs a key' notice: it re-arms only after the operator enters a key and that key is later removed", async () => { + const b = freshInstance("notice-rearm", B_ID); + try { + await b.db.execute({ + sql: "INSERT INTO providers (id, base_url, models, provider_type, api_key, lamport_ts, instance_id) VALUES (?, ?, ?, ?, NULL, 10, ?)", + args: ["prov-rearm", CLOUD.baseUrl, JSON.stringify(MODELS_1), "openai", A_ID], + }); + assert.equal(await noticeProviderKeyNeeded(b.db, "prov-rearm"), "raised"); + assert.equal(await noticeProviderKeyNeeded(b.db, "prov-rearm"), "already"); + + // The operator enters a key HERE (the local write path clears the notice). + setProviderSyncManager(b.mgr); + const panel = (await listProvidersAll(b.db)).find((p) => p.id === "prov-rearm"); + await upsertProvider(b.db, { ...panel, apiKey: KEY_B }); + assert.equal(await noticeProviderKeyNeeded(b.db, "prov-rearm"), "has-key"); + + // …and that key is later removed on this box. + await b.db.execute({ sql: "UPDATE providers SET api_key = NULL WHERE id = ?", args: ["prov-rearm"] }); + assert.equal(await noticeProviderKeyNeeded(b.db, "prov-rearm"), "raised", + "a later loss of the key may ring once more"); + assert.equal(await noticeProviderKeyNeeded(b.db, "prov-rearm"), "already", "…and only once"); + assert.equal((await noticeRows(b.db, "prov-rearm")).length, 2, "one notice per key episode, never more"); + } finally { + setProviderSyncManager(null); + await b.close(); + } +}); + +/* ═══════ 8. fix round 2 — the endpoint hold, reverse direction (B → A) ═══════ */ + +/** B repoints the provider — the reverse of repointOnA. */ +async function repointOnB(b, url) { + setProviderSyncManager(b.mgr); + const panel = (await listProvidersAll(b.db)).find((p) => p.id === HOLD_ID); + await upsertProvider(b.db, { ...panel, baseUrl: url }); +} + +/** + * The mirror of keyedProviderOnBoth: B creates the provider (keyless), A applies + * it, then A enters its OWN key — the key lives on A, and B is about to want the + * address moved. + */ +async function keyedProviderOnA(label) { + const a = freshInstance(`${label}-a`, A_ID); + const b = freshInstance(`${label}-b`, B_ID); + const feedAtoB = a.peerFeed(B_ID); + const feedBtoA = b.peerFeed(A_ID); + + setProviderSyncManager(b.mgr); + await upsertProvider(b.db, { id: HOLD_ID, ...CLOUD, models: MODELS_1 }); + await a.mgr._processNewEntries(B_ID, feedBtoA); + + setProviderSyncManager(a.mgr); + const aPanel = (await listProvidersAll(a.db)).find((p) => p.id === HOLD_ID); + await upsertProvider(a.db, { ...aPanel, apiKey: KEY_A }); + await b.mgr._processNewEntries(A_ID, feedAtoB); + return { a, b, feedBtoA }; +} + +test("endpoint hold, reverse: B moves the address while A holds the key — held on A, no key in either snapshot", async () => { + const { a, b, feedBtoA } = await keyedProviderOnA("hold-reverse"); + try { + assert.equal((await a.row(HOLD_ID)).base_url, URL_OLD, "both boxes start on the same address"); + assert.equal((await a.row(HOLD_ID)).api_key, KEY_A, "A holds the key on this device"); + + await repointOnB(b, URL_NEW); + await a.mgr._processNewEntries(B_ID, feedBtoA); + + const live = await a.row(HOLD_ID); + assert.equal(live.base_url, URL_OLD, "A's row is NOT repointed — B's change is held"); + assert.equal(live.api_key, KEY_A, "and the key stays where it was entered"); + + const conflicts = await providerConflicts(a.db); + assert.equal(conflicts.length, 1, "exactly one conflict records the held change"); + assert.equal(Number(conflicts[0].resolved), 0, "it is unresolved: it waits for the operator"); + assert.equal(conflicts[0].op, "update", "it reads as a proposed update (Restore is available)"); + const losing = JSON.parse(conflicts[0].losing_data); + assert.equal(losing.base_url, URL_NEW, "the held version carries the peer's new address"); + assert.equal(Object.hasOwn(losing, "api_key"), false, "never a key in the held snapshot"); + const winning = JSON.parse(conflicts[0].winning_data); + assert.equal(winning.base_url, URL_OLD, "the kept version is this device's row"); + assert.equal(Object.hasOwn(winning, "api_key"), false, "never this device's key in the kept snapshot"); + assert.equal(conflicts[0].winning_data.includes(KEY_A), false, "the key appears nowhere in the stored row"); + + const notices = await holdNoticeRows(a.db); + assert.equal(notices.length, 1, "one bell item, not one per entry"); + assert.match(notices[0].title, /OpenAI: another device wants to change its address/, + "the notice names the provider"); + assert.equal(notices[0].action_url, "/dashboard/settings?section=sync-conflicts", + "it links to the conflict review"); + } finally { + setProviderSyncManager(null); + await a.close(); + await b.close(); + } +}); + +/* ═══ 9. fix round 2 — the held-address bell is per held CONFLICT, not per state ══ */ + +test("endpoint hold notice: one per OPEN held conflict — read or dismissed is not re-raised, a NEW conflict is", async () => { + const b = freshInstance("hold-notice-once", B_ID); + try { + await b.db.execute({ + sql: "INSERT INTO providers (id, base_url, models, provider_type, api_key, lamport_ts, instance_id) VALUES (?, ?, ?, ?, ?, 100, ?)", + args: [HOLD_ID, URL_OLD, JSON.stringify(MODELS_1), "openai", KEY_B, B_ID], + }); + const live = await b.row(HOLD_ID); + const { noticeProviderEndpointHeld, clearProviderEndpointHeldNotice } = + await import("../servers/shared/provider-endpoint.js"); + const insertConflict = async (winTs, loseTs) => { + await b.db.execute({ + sql: `INSERT INTO sync_conflicts + (table_name, row_id, winning_instance_id, losing_instance_id, + winning_lamport_ts, losing_lamport_ts, winning_data, losing_data, op) + VALUES ('providers', ?, ?, ?, ?, ?, ?, ?, 'update')`, + args: [HOLD_ID, B_ID, A_ID, winTs, loseTs, + JSON.stringify({ id: HOLD_ID, base_url: URL_OLD }), + JSON.stringify({ id: HOLD_ID, base_url: URL_NEW })], + }); + const { rows } = await b.db.execute("SELECT id FROM sync_conflicts ORDER BY id DESC LIMIT 1"); + return rows[0].id; + }; + + const first = await insertConflict(100, 90); + assert.equal(await noticeProviderEndpointHeld(b.db, live, first), "raised"); + assert.equal(await noticeProviderEndpointHeld(b.db, live, first), "already", + "the same open conflict never rings twice"); + + // Read, then dismissed: either way the operator has SEEN this conflict, and a + // newer held edit replacing it (same conflict row) must not ring again. + await b.db.execute({ + sql: "UPDATE notifications SET is_read = 1 WHERE source = ?", + args: [endpointHoldNoticeSource(HOLD_ID)], + }); + assert.equal(await noticeProviderEndpointHeld(b.db, live, first), "already", + "a read notice still answered THIS conflict"); + await b.db.execute({ + sql: "UPDATE notifications SET is_read = 0, is_dismissed = 1 WHERE source = ?", + args: [endpointHoldNoticeSource(HOLD_ID)], + }); + assert.equal(await noticeProviderEndpointHeld(b.db, live, first), "already", + "a dismissed notice still answered THIS conflict"); + assert.equal((await holdNoticeRows(b.db)).length, 1, "still exactly one bell item"); + + // The conflict is decided, and a NEW held change makes a NEW conflict row: + // that one may ring again. + await b.db.execute({ sql: "UPDATE sync_conflicts SET resolved = 1 WHERE id = ?", args: [first] }); + await clearProviderEndpointHeldNotice(b.db, HOLD_ID); + const second = await insertConflict(200, 150); + assert.equal(await noticeProviderEndpointHeld(b.db, live, second), "raised", + "a new held conflict rings once more"); + assert.equal((await holdNoticeRows(b.db)).length, 2, "one per held conflict, not one per provider id"); + } finally { + await b.close(); + } +}); + +/* ══ 10. fix round 3 — the READ doors strip old rows, and the repair's write ══ */ + +const OLD_KEY_WIN = "sk-still-in-an-old-row"; +const OLD_KEY_LOSE = "sk-leaked-by-old-peer"; +const OLD_ID = "prov-legacy"; + +/** Seed a pre-fix providers conflict: BOTH snapshots still carry an api_key. */ +async function seedOldProviderConflict(db, { rowId = OLD_ID, winning, losing, resolved = 0 } = {}) { + await db.execute({ + sql: `INSERT INTO sync_conflicts + (table_name, row_id, winning_instance_id, losing_instance_id, + winning_lamport_ts, losing_lamport_ts, winning_data, losing_data, resolved, op) + VALUES ('providers', ?, ?, ?, 100, 90, ?, ?, ?, 'update')`, + args: [rowId, B_ID, A_ID, + winning ?? JSON.stringify({ + id: rowId, base_url: CLOUD.baseUrl, models: JSON.stringify(MODELS_1), api_key: OLD_KEY_WIN, + }), + losing ?? JSON.stringify({ + id: rowId, base_url: URL_NEW, models: JSON.stringify(MODELS_2), api_key: OLD_KEY_LOSE, + }), + resolved], + }); + const { rows } = await db.execute({ + sql: "SELECT * FROM sync_conflicts WHERE row_id = ? ORDER BY id DESC LIMIT 1", + args: [rowId], + }); + return rows[0]; +} + +/** The registered crow_list_sync_conflicts handler, without spinning an MCP server. */ +async function listConflictsHandler(db) { + const handlers = {}; + const { registerInstancesTools } = await import("../servers/sharing/tools/instances.js"); + registerInstancesTools({ tool: (name, _desc, _schema, fn) => { handlers[name] = fn; } }, { db }); + return handlers["crow_list_sync_conflicts"]; +} + +const conflictPageHtml = async (db) => { + const section = (await import( + "../servers/gateway/dashboard/settings/sections/sync-conflicts.js" + )).default; + return section.render({ req: { csrfToken: "test-csrf", query: {} }, db, lang: "en" }); +}; + +test("read door: crow_list_sync_conflicts strips an old providers row's key — a stdio-only install never runs the boot repair", async () => { + const b = freshInstance("read-tool", B_ID); + try { + await seedOldProviderConflict(b.db); + const list = await listConflictsHandler(b.db); + // Called directly, so zod's defaults are not applied — pass them. + const res = await list({ unresolved_only: false, limit: 20 }); + const text = res.content.map((c) => c.text).join("\n"); + assert.match(text, /providers/, "the conflict is still listed"); + assert.match(text, /api\.openai\.com/, "and the rest of the snapshot is still shown"); + for (const key of [OLD_KEY_WIN, OLD_KEY_LOSE]) { + assert.equal(text.includes(key), false, `the key is not returned: ${key}`); + } + assert.equal(text.includes("api_key"), false, "the field is gone, not just the value"); + } finally { + await b.close(); + } +}); + +test("read door: the sync-conflicts page shows no key from an old providers row", async () => { + const b = freshInstance("read-page", B_ID); + try { + await seedOldProviderConflict(b.db); + const html = await conflictPageHtml(b.db); + assert.match(html, /api\.openai\.com/, "the conflict is still rendered in full otherwise"); + for (const key of [OLD_KEY_WIN, OLD_KEY_LOSE]) { + assert.equal(html.includes(key), false, `the page must not print the key: ${key}`); + } + assert.equal(html.includes("api_key"), false, "the field is gone, not just the value"); + } finally { + await b.close(); + } +}); + +test("read doors: a 'null' snapshot shows nothing and never throws", async () => { + const b = freshInstance("read-null", B_ID); + try { + await seedOldProviderConflict(b.db, { + rowId: "prov-null-win", + winning: "null", + losing: JSON.stringify({ id: "prov-null-win", base_url: URL_NEW }), + }); + await seedOldProviderConflict(b.db, { + rowId: "prov-null-lose", + winning: JSON.stringify({ id: "prov-null-lose", base_url: CLOUD.baseUrl }), + losing: "null", + }); + + const list = await listConflictsHandler(b.db); + const res = await list({ unresolved_only: false, limit: 20 }); + assert.equal(res.isError, undefined, "no throw on a 'null' snapshot"); + const text = res.content.map((c) => c.text).join("\n"); + assert.match(text, /prov-null-win/, "the row is still listed"); + assert.match(text, /\(none\)/, "the null side shows nothing"); + + const html = await conflictPageHtml(b.db); + assert.match(html, /prov-null-win/, "the row still renders on the page"); + assert.match(html, /\(none\)/, "and the null side renders as (none)"); + } finally { + await b.close(); + } +}); + +test("boot redaction: a snapshot that changed after the read is never overwritten by the stale redaction", async () => { + const b = freshInstance("redact-race", B_ID); + try { + const read = await seedOldProviderConflict(b.db, { rowId: "prov-race" }); + // Between the repair's read and its write, a newer snapshot lands on the row + // (a peer's entry applied, a restore's re-snapshot): keyless, new address. + const newer = JSON.stringify({ id: "prov-race", base_url: URL_NEWER, models: JSON.stringify(MODELS_2) }); + await b.db.execute({ + sql: "UPDATE sync_conflicts SET winning_data = ? WHERE id = ?", + args: [newer, read.id], + }); + + const { writeBackRedacted, redactConflictSnapshots } = + await import("../servers/sharing/sync-conflict-redact.js"); + const wrote = await writeBackRedacted(b.db, read.id, { + winning: stripConflictSnapshot("providers", read.winning_data), + losing: stripConflictSnapshot("providers", read.losing_data), + expectedWinning: read.winning_data, + expectedLosing: read.losing_data, + }); + assert.equal(wrote, false, "the conditional UPDATE matched no row, so nothing was written"); + + const { rows: after } = await b.db.execute({ + sql: "SELECT winning_data, losing_data FROM sync_conflicts WHERE id = ?", + args: [read.id], + }); + assert.equal(after[0].winning_data, newer, "the newer snapshot survives"); + assert.equal(after[0].losing_data, read.losing_data, "the side that did not move is unchanged too"); + + // The full repair still redacts what is left on the NEXT run — the guard + // skipped a stale write, it did not give up on the row. + assert.equal(await redactConflictSnapshots(b.db), 1, "the remaining snapshot is redacted next run"); + const { rows: done } = await b.db.execute({ + sql: "SELECT winning_data, losing_data FROM sync_conflicts WHERE id = ?", + args: [read.id], + }); + assert.equal(done[0].winning_data, newer, "the newer snapshot is byte-identical"); + assert.equal(done[0].losing_data.includes(OLD_KEY_LOSE), false, "the old side did lose its key"); + } finally { + await b.close(); + } +}); + +test("snapshot strip: the identical string comes back when there is nothing to remove", async () => { + const spaced = '{ "id": "prov-plain", "base_url": "https://api.openai.com/v1" }'; + assert.equal(stripConflictSnapshot("providers", spaced), spaced, + "byte-identical, not a re-serialization: 'writes only on change' stays strictly true"); + assert.equal(stripConflictSnapshot("providers", "null"), "null", "a JSON null comes back as it went in"); + assert.equal(stripConflictSnapshot("providers", "not json"), "not json", "an unreadable blob is left alone"); + assert.equal(stripConflictSnapshot("memories", spaced), spaced, "a table with no local-only columns is untouched"); + assert.equal( + stripConflictSnapshot("providers", JSON.stringify({ id: "prov-plain", api_key: "sk-x" })), + JSON.stringify({ id: "prov-plain" }), + "and a key IS removed when one is present", + ); +}); + +test("boot redaction: a keyless providers row is left byte-identical (the SELECT reads only rows that can hold a key)", async () => { + const b = freshInstance("redact-bound", B_ID); + try { + const spaced = '{ "id": "prov-plain", "base_url": "https://api.openai.com/v1", "models": "[]" }'; + await seedOldProviderConflict(b.db, { + rowId: "prov-plain", + winning: spaced, + losing: JSON.stringify({ id: "prov-plain", base_url: URL_NEW }), + }); + const { redactConflictSnapshots } = await import("../servers/sharing/sync-conflict-redact.js"); + assert.equal(await redactConflictSnapshots(b.db), 0, "nothing to redact"); + const { rows } = await b.db.execute({ + sql: "SELECT winning_data FROM sync_conflicts WHERE row_id = ?", + args: ["prov-plain"], + }); + assert.equal(rows[0].winning_data, spaced, "byte-identical: the row was never rewritten"); + } finally { + await b.close(); + } +}); \ No newline at end of file diff --git a/tests/providers-sync-wire.test.js b/tests/providers-sync-wire.test.js index 1d1a6c36d..0568002e0 100644 --- a/tests/providers-sync-wire.test.js +++ b/tests/providers-sync-wire.test.js @@ -202,10 +202,13 @@ test("local_only marker: also gates a LOOPBACK-shaped row (belt and braces, not // ── D3: excluded bookkeeping columns ───────────────────────────────────────── -test("D3: EXCLUDED_COLUMNS.providers is exactly created_at/updated_at/lamport_ts", () => { +test("D3: EXCLUDED_COLUMNS.providers is exactly api_key + created_at/updated_at/lamport_ts", () => { + // api_key joined this list in security audit N1b-7 (operator ruling + // 2026-11-05): a providers row syncs, its API key never leaves the instance + // where it was entered. assert.deepEqual( [...EXCLUDED_COLUMNS.providers].sort(), - ["created_at", "lamport_ts", "updated_at"], + ["api_key", "created_at", "lamport_ts", "updated_at"], ); });