[P1 security] Provider keys persist in localStorage and can be redirected by imported provider configs #128

New issue

Open

opened 2026-07-28 22:55:38 +00:00 by lost-rob0t · 0 comments

lost-rob0t commented

2026-07-28 22:55:38 +00:00

(Migrated from github.com)

Finding

src/lib/agent-secrets.js stores provider, Brave Search, and MCP secrets in localStorage. The UI claims keys stay “in this browser session,” but localStorage survives browser restarts.

Separately, agent-pack imports can replace provider records, including id, baseUrl, requiresKey, and headers. A pack that replaces the existing openai, openrouter, or another keyed provider can point its base URL at an attacker-controlled endpoint. The next model test/run retrieves the existing secret by provider ID and sends it as an authorization header to the imported URL.

This creates a practical credential-exfiltration path from a config pack that contains no secret itself.

Required fix

1. Stop silently persisting raw keys

Use session-only storage by default and make persistent storage an explicit opt-in with accurate UI copy.

function storageFor(mode = "session") {
  if (mode === "persistent") return localStorage;
  return sessionStorage;
}

export function setProviderSecret(providerId, secret, { persistence = "session" } = {}) {
  const value = String(secret || "");
  if (!value) throw new TypeError("Provider key is required");
  sessionStorage.removeItem(key(providerId));
  localStorage.removeItem(key(providerId));
  storageFor(persistence).setItem(key(providerId), value);
}

Persistent mode should require an explicit warning that any same-origin script can read the key. Prefer a local companion/proxy or OS-backed secret service for durable credentials.

2. Bind secrets to the reviewed provider endpoint

Store non-secret metadata with the secret and invalidate it whenever the provider’s security identity changes.

function providerCredentialScope(provider) {
  const url = new URL(provider.baseUrl);
  return JSON.stringify({
    providerId: provider.id,
    origin: url.origin,
    pathPrefix: url.pathname.replace(/\/+$/, ""),
    type: provider.type
  });
}

export function getScopedProviderSecret(provider) {
  const stored = readSecretRecord(provider.id);
  return stored?.scope === providerCredentialScope(provider) ? stored.value : "";
}

3. Make imported endpoint changes fail closed

Imported providers must be normalized with the same provider-specific validator used by the editor. Replacing a provider whose baseUrl, type, or credential-bearing headers differ must:

clear the existing key;
disable the provider;
require the user to review the endpoint and re-enter the key;
show the exact origin that will receive credentials.

if (existingProvider && credentialScopeChanged(existingProvider, importedProvider)) {
  deleteProviderSecret(existingProvider.id);
  importedProvider.enabled = false;
  importedProvider.requiresCredentialReview = true;
}

4. Restrict credential destinations

Require HTTPS except explicit localhost/loopback development endpoints.
Reject embedded credentials in URLs.
Reject non-HTTP(S) schemes.
Do not allow imported custom headers to override Authorization, x-api-key, Host, or proxy-related headers.

Acceptance criteria

Default key storage ends with the browser session.
UI copy matches actual persistence.
Changing/importing a provider endpoint invalidates the old credential.
A config pack cannot cause an existing key to be sent to a new origin without explicit endpoint review and key re-entry.
Tests cover provider-ID collision, base-URL replacement, malicious schemes, and session restart behavior.

## Finding `src/lib/agent-secrets.js` stores provider, Brave Search, and MCP secrets in `localStorage`. The UI claims keys stay “in this browser session,” but `localStorage` survives browser restarts. Separately, agent-pack imports can replace provider records, including `id`, `baseUrl`, `requiresKey`, and `headers`. A pack that replaces the existing `openai`, `openrouter`, or another keyed provider can point its base URL at an attacker-controlled endpoint. The next model test/run retrieves the existing secret by provider ID and sends it as an authorization header to the imported URL. This creates a practical credential-exfiltration path from a config pack that contains no secret itself. ## Required fix ### 1. Stop silently persisting raw keys Use session-only storage by default and make persistent storage an explicit opt-in with accurate UI copy. ```js function storageFor(mode = "session") { if (mode === "persistent") return localStorage; return sessionStorage; } export function setProviderSecret(providerId, secret, { persistence = "session" } = {}) { const value = String(secret || ""); if (!value) throw new TypeError("Provider key is required"); sessionStorage.removeItem(key(providerId)); localStorage.removeItem(key(providerId)); storageFor(persistence).setItem(key(providerId), value); } ``` Persistent mode should require an explicit warning that any same-origin script can read the key. Prefer a local companion/proxy or OS-backed secret service for durable credentials. ### 2. Bind secrets to the reviewed provider endpoint Store non-secret metadata with the secret and invalidate it whenever the provider’s security identity changes. ```js function providerCredentialScope(provider) { const url = new URL(provider.baseUrl); return JSON.stringify({ providerId: provider.id, origin: url.origin, pathPrefix: url.pathname.replace(/\/+$/, ""), type: provider.type }); } export function getScopedProviderSecret(provider) { const stored = readSecretRecord(provider.id); return stored?.scope === providerCredentialScope(provider) ? stored.value : ""; } ``` ### 3. Make imported endpoint changes fail closed Imported providers must be normalized with the same provider-specific validator used by the editor. Replacing a provider whose `baseUrl`, `type`, or credential-bearing headers differ must: - clear the existing key; - disable the provider; - require the user to review the endpoint and re-enter the key; - show the exact origin that will receive credentials. ```js if (existingProvider && credentialScopeChanged(existingProvider, importedProvider)) { deleteProviderSecret(existingProvider.id); importedProvider.enabled = false; importedProvider.requiresCredentialReview = true; } ``` ### 4. Restrict credential destinations - Require HTTPS except explicit localhost/loopback development endpoints. - Reject embedded credentials in URLs. - Reject non-HTTP(S) schemes. - Do not allow imported custom headers to override `Authorization`, `x-api-key`, `Host`, or proxy-related headers. ## Acceptance criteria - Default key storage ends with the browser session. - UI copy matches actual persistence. - Changing/importing a provider endpoint invalidates the old credential. - A config pack cannot cause an existing key to be sent to a new origin without explicit endpoint review and key re-entry. - Tests cover provider-ID collision, base-URL replacement, malicious schemes, and session restart behavior.