diff --git a/servers/gateway/dashboard/shared/i18n.js b/servers/gateway/dashboard/shared/i18n.js
index 0e384f4f0..380ad5a4b 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,10 @@ 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." },
"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." },
diff --git a/servers/shared/provider-key-need.js b/servers/shared/provider-key-need.js
new file mode 100644
index 000000000..eaa872556
--- /dev/null
+++ b/servers/shared/provider-key-need.js
@@ -0,0 +1,191 @@
+/**
+ * "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).
+ *
+ * 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)}`;
+}
+
+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: an
+ * open (unread, undismissed) notice for that id means it was already raised, so
+ * a re-delivered sync entry never rings again — and a dismissed one is the
+ * operator's answer, so it is not re-raised either.
+ *
+ * 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);
+ const { rows: open } = await db.execute({
+ sql: "SELECT id FROM notifications WHERE source = ? AND is_read = 0 AND is_dismissed = 0 LIMIT 1",
+ args: [source],
+ });
+ if (open.length > 0) 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.
+ * @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')
+ WHERE source = ? AND is_read = 0`,
+ args: [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..449db47cf 100644
--- a/servers/sharing/instance-sync.js
+++ b/servers/sharing/instance-sync.js
@@ -31,6 +31,7 @@ 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";
/** 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 +103,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 +197,53 @@ 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;
+}
+
// 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 +312,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 +441,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 +469,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 +485,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;
@@ -3052,11 +3107,32 @@ 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);
}
}
+ /**
+ * 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.
diff --git a/servers/sharing/sync-conflict-resolve.js b/servers/sharing/sync-conflict-resolve.js
index 1519e8fa2..66de9f329 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) ─────────────────────────────
@@ -291,6 +291,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/tests/providers-key-locality.test.js b/tests/providers-key-locality.test.js
new file mode 100644
index 000000000..14fa9fae5
--- /dev/null
+++ b/tests/providers-key-locality.test.js
@@ -0,0 +1,546 @@
+/**
+ * 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.
+ *
+ * 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 } 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 * 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://10.0.0.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://10.0.0.9:8080/v1", apiKey: null, instance_id: "peer-instance-id" }, { ownInstanceId: ownId }), "",
+ "a keyless provider (llama.cpp on the LAN) 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"));
+});
+
+
+/* ═══════════════════════════ 5. "needs a key on this device" ═════════════════ */
+
+const noticeRows = async (db, id) => {
+ 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.
+ assert.equal(providerNeedsKey({ base_url: "https://llm.example.com/v1", provider_type: "openai" }), true);
+ // Keyless: a llama.cpp on the LAN, a tailnet box, a loopback sidecar, a bare name.
+ assert.equal(providerNeedsKey({ base_url: "http://10.0.0.9:8080/v1", provider_type: "openai-compat" }), false);
+ 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://10.0.0.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();
+ }
+});
\ 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"],
);
});
From 8aae20b71346ec8bd7ffa4f8d1ed9f465c9567cd Mon Sep 17 00:00:00 2001
From: kh0pper <93635715+kh0pper@users.noreply.github.com>
Date: Fri, 6 Nov 2026 08:54:11 -0600
Subject: [PATCH 2/4] fix(sync): a peer's address change to a provider that
holds a key here waits for approval
An inbound change that moves a provider's endpoint, on a device that holds
a key for it, is no longer applied. It is recorded as a sync conflict with
this device's row kept, named once in the bell, and the conflicts page
says that accepting sends this device's key to the new address. A newer
address from the same peer replaces the held one, a re-delivered entry
raises nothing, and keyless providers update as before. The stored
conflict never contains the local key.
Claude-Session: https://claude.ai/code/session_015Vve3soBJ9qTcyeCCLyYyh
---
.../settings/sections/sync-conflicts.js | 65 +++-
servers/gateway/dashboard/shared/i18n.js | 11 +
servers/shared/provider-endpoint.js | 171 +++++++++
servers/sharing/instance-sync.js | 116 ++++++
tests/providers-key-locality.test.js | 330 ++++++++++++++++++
5 files changed, 691 insertions(+), 2 deletions(-)
create mode 100644 servers/shared/provider-endpoint.js
diff --git a/servers/gateway/dashboard/settings/sections/sync-conflicts.js b/servers/gateway/dashboard/settings/sections/sync-conflicts.js
index dcd489a3a..40dbd8978 100644
--- a/servers/gateway/dashboard/settings/sections/sync-conflicts.js
+++ b/servers/gateway/dashboard/settings/sections/sync-conflicts.js
@@ -24,7 +24,14 @@
*/
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,
@@ -68,6 +75,54 @@ function opBadge(op) {
return `${escapeHtml(op || "update")}`;
}
+/**
+ * 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(r.winning_data);
+ lose = JSON.parse(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.
+ }
+}
+
function prettyJson(raw) {
if (!raw) return "(none)";
try {
@@ -197,6 +252,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 +515,7 @@ export default {
}
await resolveConflict(db, String(conflictId));
await markResolvedVia(db, conflictId, "operator");
+ await clearHoldNotice(db, conflictId);
res.redirectAfterPost(conflictPage);
return true;
}
@@ -469,7 +526,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 +545,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 380ad5a4b..4564298cc 100644
--- a/servers/gateway/dashboard/shared/i18n.js
+++ b/servers/gateway/dashboard/shared/i18n.js
@@ -2745,6 +2745,11 @@ export const translations = {
// 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." },
@@ -3462,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..89b777e2a
--- /dev/null
+++ b/servers/shared/provider-endpoint.js
@@ -0,0 +1,171 @@
+/**
+ * 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) — one bell item, 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)}`;
+}
+
+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 provider id: "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 it must not ring
+ * again either — an open notice for this id means it was already raised.
+ *
+ * @param {object} db
+ * @param {object} row — this device's providers row
+ * @returns {Promise<"raised"|"already"|"filtered">}
+ */
+export async function noticeProviderEndpointHeld(db, row) {
+ const id = row?.id;
+ if (typeof id !== "string" || id === "") return "already";
+ const source = endpointHoldNoticeSource(id);
+ const { rows: open } = await db.execute({
+ sql: "SELECT id FROM notifications WHERE source = ? AND is_read = 0 AND is_dismissed = 0 LIMIT 1",
+ args: [source],
+ });
+ if (open.length > 0) 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,
+ });
+ 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/sharing/instance-sync.js b/servers/sharing/instance-sync.js
index 449db47cf..dfa8beade 100644
--- a/servers/sharing/instance-sync.js
+++ b/servers/sharing/instance-sync.js
@@ -32,6 +32,7 @@ import { SYNC_PB_KEY_PREFIX, SYNC_PB_CLEARED_PREFIX } from "../shared/self-dial-
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;
@@ -3088,6 +3089,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
@@ -3119,6 +3131,110 @@ export class InstanceSyncManager {
}
}
+ /**
+ * 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 the local key must never be stored in it.
+ const winningData = JSON.stringify(stripLocalOnlyColumns("providers", localRow));
+ const losingData = JSON.stringify(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) {
+ try {
+ await noticeProviderEndpointHeld(this.db, localRow);
+ } 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
diff --git a/tests/providers-key-locality.test.js b/tests/providers-key-locality.test.js
index 14fa9fae5..a036fed7f 100644
--- a/tests/providers-key-locality.test.js
+++ b/tests/providers-key-locality.test.js
@@ -14,6 +14,10 @@
* 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.
*
* Harness: real instance DBs (scripts/init-db.js per dir, like paired Crow
* boxes), real InstanceSyncManagers over stub Hypercore feeds that carry the
@@ -35,6 +39,7 @@ 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";
@@ -450,6 +455,331 @@ test("providers tab: the key line by the key field, and 'needs a key' on a peer
});
+/* ═════════ 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 LAN llama.cpp: no key on either side (the ruling's keyless case).
+ setProviderSyncManager(a.mgr);
+ await upsertProvider(a.db, {
+ id: "prov-lan", baseUrl: "http://10.0.0.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://10.0.0.10:8080/v1" });
+ await b.mgr._processNewEntries(A_ID, feedAtoB);
+
+ assert.equal((await b.row("prov-lan")).base_url, "http://10.0.0.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 {
From bbf6691859fc97702c2240ae8fdf71b17db17ee7 Mon Sep 17 00:00:00 2001
From: kh0pper <93635715+kh0pper@users.noreply.github.com>
Date: Fri, 6 Nov 2026 09:23:54 -0600
Subject: [PATCH 3/4] fix(sync): no stored sync conflict ever holds a provider
key, and old ones are cleaned at boot
Review follow-ups: every conflict snapshot is stripped of local-only
columns where it is written (one choke point in _insertConflictRow, plus
the held-change replace and the restore's stale check); a boot step
removes keys from conflict rows saved before this change, never deleting
a row; the needs-a-key notice rings once per provider, even after it is
read or dismissed; the address hold is tested in both directions.
Claude-Session: https://claude.ai/code/session_015Vve3soBJ9qTcyeCCLyYyh
---
servers/gateway/boot/post-listen.js | 21 ++
servers/shared/provider-endpoint.js | 56 +++-
servers/shared/provider-key-need.js | 52 ++-
servers/sharing/instance-sync.js | 64 +++-
servers/sharing/sync-conflict-redact.js | 79 +++++
servers/sharing/sync-conflict-resolve.js | 8 +-
tests/providers-key-locality.test.js | 396 ++++++++++++++++++++++-
7 files changed, 636 insertions(+), 40 deletions(-)
create mode 100644 servers/sharing/sync-conflict-redact.js
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/shared/provider-endpoint.js b/servers/shared/provider-endpoint.js
index 89b777e2a..7332fd80a 100644
--- a/servers/shared/provider-endpoint.js
+++ b/servers/shared/provider-endpoint.js
@@ -14,9 +14,10 @@
* 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) — one bell item, by name, linking to
- * the conflict review; and clearProviderEndpointHeldNotice(db, id) — quiet
- * it once the operator has decided.
+ * 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
@@ -91,6 +92,25 @@ 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({
@@ -104,24 +124,37 @@ async function readLang(db) {
}
/**
- * Ring the bell ONCE per provider id: "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 it must not ring
- * again either — an open notice for this id means it was already raised.
+ * 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) {
+export async function noticeProviderEndpointHeld(db, row, heldConflictId = null) {
const id = row?.id;
if (typeof id !== "string" || id === "") return "already";
const source = endpointHoldNoticeSource(id);
- const { rows: open } = await db.execute({
- sql: "SELECT id FROM notifications WHERE source = ? AND is_read = 0 AND is_dismissed = 0 LIMIT 1",
+ 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],
});
- if (open.length > 0) return "already";
+ 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).
@@ -137,6 +170,7 @@ export async function noticeProviderEndpointHeld(db, row) {
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";
}
diff --git a/servers/shared/provider-key-need.js b/servers/shared/provider-key-need.js
index eaa872556..f79fd27c8 100644
--- a/servers/shared/provider-key-need.js
+++ b/servers/shared/provider-key-need.js
@@ -14,7 +14,8 @@
* 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).
+ * 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
@@ -113,6 +114,26 @@ 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({
@@ -126,10 +147,12 @@ async function readLang(db) {
}
/**
- * Raise the notice for provider `id`, if it is owed. ONCE per provider id: an
- * open (unread, undismissed) notice for that id means it was already raised, so
- * a re-delivered sync entry never rings again — and a dismissed one is the
- * operator's answer, so it is not re-raised either.
+ * 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.
@@ -150,11 +173,14 @@ export async function noticeProviderKeyNeeded(db, id) {
if (key !== "") return "has-key";
const source = providerKeyNoticeSource(id);
- const { rows: open } = await db.execute({
- sql: "SELECT id FROM notifications WHERE source = ? AND is_read = 0 AND is_dismissed = 0 LIMIT 1",
+ // 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 (open.length > 0) return "already";
+ 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).
@@ -177,15 +203,17 @@ export async function noticeProviderKeyNeeded(db, id) {
/**
* 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.
+ * 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')
- WHERE source = ? AND is_read = 0`,
- args: [providerKeyNoticeSource(id)],
+ 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/sharing/instance-sync.js b/servers/sharing/instance-sync.js
index dfa8beade..e31966d51 100644
--- a/servers/sharing/instance-sync.js
+++ b/servers/sharing/instance-sync.js
@@ -233,6 +233,34 @@ export function stripLocalOnlyColumns(table, row) {
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 comes back
+ * byte-identical, a JSON null (`'null'`, a row that was gone) and an
+ * unparseable blob come back as they went in — a snapshot we cannot read cannot
+ * be redacted here either, and losing the conflict trace would be worse.
+ */
+export function stripConflictSnapshot(table, raw) {
+ if (!LOCAL_ONLY_COLUMNS[table]) return raw;
+ try {
+ const parsed = typeof raw === "string" ? JSON.parse(raw) : raw;
+ if (!parsed || typeof parsed !== "object") return raw;
+ 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"],
@@ -3194,9 +3222,10 @@ export class InstanceSyncManager {
}
// The conflict row is shown on the sync-conflicts page and returned by
- // crow_list_sync_conflicts, so the local key must never be stored in it.
+ // 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(row);
+ 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) {
@@ -3222,8 +3251,23 @@ export class InstanceSyncManager {
"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);
+ await noticeProviderEndpointHeld(this.db, localRow, heldConflictId);
} catch (err) {
console.warn(`[instance-sync] provider endpoint notice failed for ${rowId.slice(0, 64)}:`, err.message);
}
@@ -4202,12 +4246,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();
@@ -4238,6 +4288,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..033cd48e2
--- /dev/null
+++ b/servers/sharing/sync-conflict-redact.js
@@ -0,0 +1,79 @@
+/**
+ * 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.
+ *
+ * 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({
+ sql: `SELECT id, table_name, winning_data, losing_data FROM sync_conflicts
+ WHERE table_name IN (${placeholders})`,
+ 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
+ await db.execute({
+ sql: "UPDATE sync_conflicts SET winning_data = ?, losing_data = ? WHERE id = ?",
+ args: [win, lose, row.id],
+ });
+ changed += 1;
+ }
+ return changed;
+}
+
+/**
+ * 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 66de9f329..598de6c7e 100644
--- a/servers/sharing/sync-conflict-resolve.js
+++ b/servers/sharing/sync-conflict-resolve.js
@@ -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],
diff --git a/tests/providers-key-locality.test.js b/tests/providers-key-locality.test.js
index a036fed7f..9a3f471fb 100644
--- a/tests/providers-key-locality.test.js
+++ b/tests/providers-key-locality.test.js
@@ -17,7 +17,14 @@
* 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.
+ * 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.
*
* Harness: real instance DBs (scripts/init-db.js per dir, like paired Crow
* boxes), real InstanceSyncManagers over stub Hypercore feeds that carry the
@@ -418,7 +425,7 @@ test("providers tab: the key line by the key field, and 'needs a key' on a peer
{ 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://10.0.0.9:8080/v1", api_key: null, host: "cloud", bundle_id: null,
+ { 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,
@@ -444,8 +451,8 @@ test("providers tab: the key line by the key field, and 'needs a key' on a peer
/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://10.0.0.9:8080/v1", apiKey: null, instance_id: "peer-instance-id" }, { ownInstanceId: ownId }), "",
- "a keyless provider (llama.cpp on the LAN) is not 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).
@@ -625,19 +632,20 @@ test("endpoint hold: the same address change to a row with NO local key applies
const feedAtoB = a.peerFeed(B_ID);
b.peerFeed(A_ID);
try {
- // A LAN llama.cpp: no key on either side (the ruling's keyless case).
+ // 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://10.0.0.9:8080/v1", host: "cloud",
+ 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://10.0.0.10:8080/v1" });
+ 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://10.0.0.10:8080/v1",
+ 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 {
@@ -795,10 +803,13 @@ test("providerNeedsKey: a keyed cloud type/preset needs one; a LAN llama.cpp and
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.
+ // 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 llama.cpp on the LAN, a tailnet box, a loopback sidecar, a bare name.
- assert.equal(providerNeedsKey({ base_url: "http://10.0.0.9:8080/v1", provider_type: "openai-compat" }), false);
+ // 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);
@@ -819,7 +830,7 @@ test("'needs a key' notice: raised ONCE by name for a keyed row, not for a keyle
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://10.0.0.9:8080/v1", host: "cloud",
+ 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");
@@ -873,4 +884,365 @@ test("'needs a key' notice: raised ONCE by name for a keyed row, not for a keyle
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();
+ }
});
\ No newline at end of file
From f624ca38f71cc40e9452a051028df5715372abc0 Mon Sep 17 00:00:00 2001
From: kh0pper <93635715+kh0pper@users.noreply.github.com>
Date: Fri, 6 Nov 2026 09:39:29 -0600
Subject: [PATCH 4/4] fix(sync): conflict snapshots are stripped of provider
keys when read, too
Review follow-ups: crow_list_sync_conflicts and the conflicts page strip
local-only columns before showing a snapshot, so an old row on a host that
never ran the gateway's boot redaction shows no key; a 'null' snapshot no
longer throws; the boot redaction reads only rows that can hold a key and
writes back only if the row is unchanged since it was read.
Claude-Session: https://claude.ai/code/session_015Vve3soBJ9qTcyeCCLyYyh
---
.../settings/sections/sync-conflicts.js | 26 ++-
servers/sharing/instance-sync.js | 15 +-
servers/sharing/sync-conflict-redact.js | 47 ++++-
servers/sharing/tools/instances.js | 38 +++-
tests/providers-key-locality.test.js | 193 +++++++++++++++++-
5 files changed, 295 insertions(+), 24 deletions(-)
diff --git a/servers/gateway/dashboard/settings/sections/sync-conflicts.js b/servers/gateway/dashboard/settings/sections/sync-conflicts.js
index 40dbd8978..d757dd784 100644
--- a/servers/gateway/dashboard/settings/sections/sync-conflicts.js
+++ b/servers/gateway/dashboard/settings/sections/sync-conflicts.js
@@ -37,6 +37,7 @@ import {
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,
@@ -87,8 +88,8 @@ function addressChangeLine(r, lang) {
let win;
let lose;
try {
- win = JSON.parse(r.winning_data);
- lose = JSON.parse(r.losing_data);
+ win = JSON.parse(stripConflictSnapshot(r.table_name, r.winning_data));
+ lose = JSON.parse(stripConflictSnapshot(r.table_name, r.losing_data));
} catch {
return "";
}
@@ -123,12 +124,23 @@ async function clearHoldNotice(db, conflictId) {
}
}
-function prettyJson(raw) {
+/**
+ * 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)";
}
}
@@ -188,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
diff --git a/servers/sharing/instance-sync.js b/servers/sharing/instance-sync.js
index e31966d51..3290083d8 100644
--- a/servers/sharing/instance-sync.js
+++ b/servers/sharing/instance-sync.js
@@ -245,16 +245,21 @@ export function stripLocalOnlyColumns(table, row) {
* 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 comes back
- * byte-identical, a JSON null (`'null'`, a row that was gone) and an
- * unparseable blob come back as they went in — a snapshot we cannot read cannot
- * be redacted here either, and losing the conflict trace would be worse.
+ * 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) {
- if (!LOCAL_ONLY_COLUMNS[table]) return 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;
diff --git a/servers/sharing/sync-conflict-redact.js b/servers/sharing/sync-conflict-redact.js
index 033cd48e2..d0cc24953 100644
--- a/servers/sharing/sync-conflict-redact.js
+++ b/servers/sharing/sync-conflict-redact.js
@@ -16,6 +16,14 @@
* 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
@@ -40,8 +48,11 @@ export async function redactConflictSnapshots(db, { tables = TABLES } = {}) {
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})`,
+ WHERE table_name IN (${placeholders})
+ AND (winning_data LIKE '%"api_key"%' OR losing_data LIKE '%"api_key"%')`,
args: wanted,
});
@@ -50,15 +61,41 @@ export async function redactConflictSnapshots(db, { tables = TABLES } = {}) {
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
- await db.execute({
- sql: "UPDATE sync_conflicts SET winning_data = ?, losing_data = ? WHERE id = ?",
- args: [win, lose, row.id],
+ const written = await writeBackRedacted(db, row.id, {
+ winning: win,
+ losing: lose,
+ expectedWinning: row.winning_data,
+ expectedLosing: row.losing_data,
});
- changed += 1;
+ 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).
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
index 9a3f471fb..605f90166 100644
--- a/tests/providers-key-locality.test.js
+++ b/tests/providers-key-locality.test.js
@@ -25,6 +25,12 @@
* 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
@@ -38,7 +44,7 @@ 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 } from "../servers/sharing/instance-sync.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";
@@ -1245,4 +1251,189 @@ test("endpoint hold notice: one per OPEN held conflict — read or dismissed is
} 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
|