feat(llm): mask API keys in UI responses and prevent masked-key leakage in fetch/test/save payloads
- Add mask_api_key() and is_masked_or_placeholder() to llm_provider service - Return masked keys in all provider CRUD endpoints - Reject masked/placeholder keys in fetch_models and test_provider_config - Show masked key with Change button in ProviderConfig.svelte edit form - Exclude masked keys from fetch-models, test, and submit payloads on frontend - Update semantics-core skill with clarified complexity tier rules - Switch agent modes from subagent to all
This commit is contained in:
@@ -64,6 +64,11 @@
|
||||
);
|
||||
}
|
||||
|
||||
function isMaskedKey(key) {
|
||||
if (!key) return false;
|
||||
return key === "********" || key.includes("...");
|
||||
}
|
||||
|
||||
function updateBaseUrlForType(providerType) {
|
||||
// Only auto-update base_url if user hasn't changed it from the default
|
||||
// or the base_url matches a previous default for a different type
|
||||
@@ -93,11 +98,14 @@
|
||||
console.log("[ProviderConfig][Action] Editing provider", provider?.id);
|
||||
editingProvider = provider;
|
||||
// Normalize provider fields to editable form shape.
|
||||
// The API key from the backend is a masked placeholder (e.g. "sk-...f8k3");
|
||||
// we preserve it in the form so the user can see which key is configured.
|
||||
// If the user modifies the field, the new value replaces the stored key.
|
||||
formData = {
|
||||
name: provider?.name ?? "",
|
||||
provider_type: provider?.provider_type ?? "openai",
|
||||
base_url: provider?.base_url ?? "https://api.openai.com/v1",
|
||||
api_key: "",
|
||||
api_key: provider?.api_key ?? "",
|
||||
default_model: provider?.default_model ?? "gpt-4o",
|
||||
is_active: Boolean(provider?.is_active),
|
||||
};
|
||||
@@ -130,8 +138,8 @@
|
||||
provider_type: formData.provider_type,
|
||||
};
|
||||
|
||||
// Use direct api_key if filled, otherwise fall back to stored key via provider_id
|
||||
if (formData.api_key) {
|
||||
// Use direct api_key only if it's a real key (not a masked placeholder like "sk-...f8k3")
|
||||
if (formData.api_key && !isMaskedKey(formData.api_key)) {
|
||||
payload.api_key = formData.api_key;
|
||||
} else if (editingProvider) {
|
||||
payload.provider_id = editingProvider.id;
|
||||
@@ -178,7 +186,14 @@
|
||||
const endpoint = editingProvider
|
||||
? `/llm/providers/${editingProvider.id}/test`
|
||||
: "/llm/providers/test";
|
||||
const result = await requestApi(endpoint, "POST", formData);
|
||||
|
||||
// For saved providers, the backend uses the stored key (ignore body).
|
||||
// For new providers, only send the real key (reject masked/empty).
|
||||
const testData = { ...formData };
|
||||
if (isMaskedKey(testData.api_key)) {
|
||||
delete testData.api_key;
|
||||
}
|
||||
const result = await requestApi(endpoint, "POST", testData);
|
||||
|
||||
if (result.success) {
|
||||
testStatus = { type: "success", message: $t.llm.connection_success };
|
||||
@@ -208,10 +223,10 @@
|
||||
? `/llm/providers/${editingProvider.id}`
|
||||
: "/llm/providers";
|
||||
|
||||
// When editing, only include api_key if user entered a new one
|
||||
// When editing, only include api_key if user entered a new one (not empty and not masked placeholder)
|
||||
const submitData = { ...formData };
|
||||
if (editingProvider && !submitData.api_key) {
|
||||
// If editing and api_key is empty, don't send it (backend will keep existing)
|
||||
if (editingProvider && (!submitData.api_key || isMaskedKey(submitData.api_key))) {
|
||||
// If editing and api_key is empty or masked, don't send it (backend will keep existing)
|
||||
delete submitData.api_key;
|
||||
}
|
||||
|
||||
@@ -381,14 +396,32 @@
|
||||
class="block text-sm font-medium text-gray-700"
|
||||
>{$t.llm.api_key}</label
|
||||
>
|
||||
<input
|
||||
id="provider-api-key"
|
||||
type="password"
|
||||
bind:value={formData.api_key}
|
||||
class="mt-1 block w-full border rounded-md p-2"
|
||||
placeholder={editingProvider ? "••••••••" : "sk-..."}
|
||||
oninput={debouncedFetchModels}
|
||||
/>
|
||||
{#if editingProvider && formData.api_key && isMaskedKey(formData.api_key)}
|
||||
<div class="mt-1 flex items-center gap-2">
|
||||
<code class="block w-full rounded-md border border-gray-300 bg-gray-50 p-2 font-mono text-sm text-gray-700 select-all">
|
||||
{formData.api_key}
|
||||
</code>
|
||||
<button
|
||||
type="button"
|
||||
class="shrink-0 rounded bg-blue-600 px-3 py-2 text-xs font-medium text-white hover:bg-blue-700"
|
||||
onclick={() => { formData.api_key = ""; }}
|
||||
>
|
||||
{$t.llm?.change_key || "Change"}
|
||||
</button>
|
||||
</div>
|
||||
<p class="mt-1 text-xs text-gray-400">
|
||||
{$t.llm?.key_masked_hint || "Masked for security. Click \"Change\" to enter a new key."}
|
||||
</p>
|
||||
{:else}
|
||||
<input
|
||||
id="provider-api-key"
|
||||
type="password"
|
||||
bind:value={formData.api_key}
|
||||
class="mt-1 block w-full border rounded-md p-2"
|
||||
placeholder={$t.llm?.api_key_placeholder || "sk-..."}
|
||||
oninput={debouncedFetchModels}
|
||||
/>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
<div>
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
// #region ProviderConfigIntegrationTest:Module [TYPE Function]
|
||||
// @SEMANTICS: llm, provider-config, integration-test, edit-flow, delete-flow
|
||||
// @PURPOSE: Protect edit and delete interaction contracts in LLM provider settings UI.
|
||||
// #region ProviderConfigIntegrationTest [C:3] [TYPE Module] [SEMANTICS llm, provider-config, integration-test, edit-flow, delete-flow]
|
||||
// @PURPOSE: Protect edit, submit, test, and delete interaction contracts in LLM provider settings UI.
|
||||
// @LAYER: UI Tests
|
||||
// @RELATION: DEPENDS_ON -> [ProviderConfig]
|
||||
// @RELATION DEPENDS_ON -> [ProviderConfig]
|
||||
// @INVARIANT: Edit action keeps explicit click handler and opens normalized edit form.
|
||||
// @INVARIANT: Masked API keys are excluded from save/test/fetch-models payloads.
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import fs from 'node:fs';
|
||||
@@ -37,6 +37,26 @@ describe('ProviderConfig edit interaction contract', () => {
|
||||
expect(source).toContain('showForm = true;');
|
||||
});
|
||||
|
||||
it('preserves masked api_key from provider in edit form', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
expect(source).toContain('api_key: provider?.api_key ?? ""');
|
||||
});
|
||||
|
||||
it('includes isMaskedKey helper to detect masked placeholders', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
expect(source).toContain('function isMaskedKey(key)');
|
||||
expect(source).toContain('key === "********" || key.includes("...")');
|
||||
});
|
||||
|
||||
it('excludes masked api_key from fetchModels payload, uses provider_id fallback', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
expect(source).toContain("if (formData.api_key && !isMaskedKey(formData.api_key))");
|
||||
expect(source).toContain('payload.provider_id = editingProvider.id;');
|
||||
});
|
||||
|
||||
it('keeps explicit delete flow with confirmation and delete request', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
@@ -46,7 +66,21 @@ describe('ProviderConfig edit interaction contract', () => {
|
||||
expect(source).toContain('onclick={() => handleDelete(provider)}');
|
||||
});
|
||||
|
||||
it('does not forward masked api_key when toggling provider activation', () => {
|
||||
it('excludes masked or empty api_key from submit data when editing', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
expect(source).toContain("if (editingProvider && (!submitData.api_key || isMaskedKey(submitData.api_key)))");
|
||||
expect(source).toContain("delete submitData.api_key;");
|
||||
});
|
||||
|
||||
it('excludes masked api_key from test payload when testing connection', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
expect(source).toContain("if (isMaskedKey(testData.api_key))");
|
||||
expect(source).toContain("delete testData.api_key;");
|
||||
});
|
||||
|
||||
it('does not forward api_key (masked or otherwise) when toggling provider activation', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
expect(source).toContain('const updatePayload = {');
|
||||
|
||||
Reference in New Issue
Block a user