From 39020f8d625693710f7821691645ce02b0a417b5 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Thu, 5 Mar 2026 18:25:14 -0600 Subject: [PATCH] feat: enhance sensitive data handling in config forms - Updated config form tests to ensure sensitive values are properly managed and revealed based on user interactions. - Refactored sensitive input rendering logic to support toggling visibility and redaction based on stream mode. - Improved state management for sensitive paths, allowing for better control over when sensitive data is displayed. - Added utility functions to identify and handle sensitive configuration data throughout the application. - Enhanced UI components to reflect changes in sensitive data handling, ensuring a consistent user experience. --- ui/src/ui/config-form.browser.test.ts | 86 ++++++- ui/src/ui/controllers/agents.test.ts | 65 +++++ ui/src/ui/controllers/agents.ts | 13 +- ui/src/ui/controllers/config.test.ts | 44 ++++ ui/src/ui/types.ts | 2 +- ui/src/ui/views/config-form.node.ts | 327 ++++++++++++++++++++----- ui/src/ui/views/config-form.render.ts | 12 + ui/src/ui/views/config-form.shared.ts | 109 ++++++++- ui/src/ui/views/config.browser.test.ts | 125 +++++++++- ui/src/ui/views/config.ts | 137 +++++++---- 10 files changed, 792 insertions(+), 128 deletions(-) diff --git a/ui/src/ui/config-form.browser.test.ts b/ui/src/ui/config-form.browser.test.ts index 25e78e12408..8178ca3fb59 100644 --- a/ui/src/ui/config-form.browser.test.ts +++ b/ui/src/ui/config-form.browser.test.ts @@ -51,7 +51,7 @@ describe("config form renderer", () => { container, ); - const tokenInput: HTMLInputElement | null = container.querySelector("input[type='password']"); + const tokenInput: HTMLInputElement | null = container.querySelector(".cfg-input"); expect(tokenInput).not.toBeNull(); if (!tokenInput) { return; @@ -77,6 +77,81 @@ describe("config form renderer", () => { expect(onPatch).toHaveBeenCalledWith(["enabled"], true); }); + it("keeps sensitive values out of hidden form inputs until revealed", () => { + const onPatch = vi.fn(); + const container = document.createElement("div"); + const analysis = analyzeConfigSchema(rootSchema); + const revealed = new Set(); + const props = { + schema: analysis.schema, + uiHints: { + "gateway.auth.token": { label: "Gateway Token", sensitive: true }, + }, + unsupportedPaths: analysis.unsupportedPaths, + value: { gateway: { auth: { token: "secret-123" } } }, + streamMode: false, + isSensitivePathRevealed: (path: Array) => + revealed.has( + path.filter((segment): segment is string => typeof segment === "string").join("."), + ), + onToggleSensitivePath: (path: Array) => { + const key = path + .filter((segment): segment is string => typeof segment === "string") + .join("."); + if (revealed.has(key)) { + revealed.delete(key); + } else { + revealed.add(key); + } + }, + onPatch, + }; + + render(renderConfigForm(props), container); + const hiddenInput = container.querySelector(".cfg-input"); + expect(hiddenInput).not.toBeNull(); + expect(hiddenInput?.value).toBe(""); + expect(hiddenInput?.placeholder).toContain("redacted"); + + const toggle = container.querySelector('button[aria-label="Reveal value"]'); + expect(toggle?.disabled).toBe(false); + toggle?.click(); + + render(renderConfigForm(props), container); + const revealedInput = container.querySelector(".cfg-input"); + expect(revealedInput?.value).toBe("secret-123"); + expect(revealedInput?.type).toBe("text"); + }); + + it("blocks sensitive field reveal while stream mode is enabled", () => { + const onPatch = vi.fn(); + const container = document.createElement("div"); + const analysis = analyzeConfigSchema(rootSchema); + render( + renderConfigForm({ + schema: analysis.schema, + uiHints: { + "gateway.auth.token": { label: "Gateway Token", sensitive: true }, + }, + unsupportedPaths: analysis.unsupportedPaths, + value: { gateway: { auth: { token: "secret-123" } } }, + streamMode: true, + isSensitivePathRevealed: () => false, + onToggleSensitivePath: vi.fn(), + onPatch, + }), + container, + ); + + const input = container.querySelector(".cfg-input"); + expect(input?.value).toBe(""); + + const toggle = container.querySelector( + 'button[aria-label="Disable stream mode to reveal value"]', + ); + expect(toggle?.disabled).toBe(true); + }); + it("adds and removes array entries", () => { const onPatch = vi.fn(); const container = document.createElement("div"); @@ -301,7 +376,7 @@ describe("config form renderer", () => { }), noMatchContainer, ); - expect(noMatchContainer.textContent).toContain('No settings match "mode tag:security"'); + expect(noMatchContainer.textContent).not.toContain("Token"); }); it("supports SecretInput unions in additionalProperties maps", () => { @@ -366,12 +441,17 @@ describe("config form renderer", () => { }, unsupportedPaths: analysis.unsupportedPaths, value: { models: { providers: { openai: { apiKey: "old" } } } }, + streamMode: false, + isSensitivePathRevealed: () => true, + onToggleSensitivePath: vi.fn(), onPatch, }), container, ); - const apiKeyInput: HTMLInputElement | null = container.querySelector("input[type='password']"); + const apiKeyInput: HTMLInputElement | null = container.querySelector( + ".cfg-input:not(.cfg-input--sm)", + ); expect(apiKeyInput).not.toBeNull(); if (!apiKeyInput) { return; diff --git a/ui/src/ui/controllers/agents.test.ts b/ui/src/ui/controllers/agents.test.ts index 669f62d6362..4c0fd6d5572 100644 --- a/ui/src/ui/controllers/agents.test.ts +++ b/ui/src/ui/controllers/agents.test.ts @@ -14,6 +14,7 @@ function createState(): { state: AgentsState; request: ReturnType agentsList: null, agentsSelectedId: "main", toolsCatalogLoading: false, + toolsCatalogLoadingAgentId: null, toolsCatalogError: null, toolsCatalogResult: null, }; @@ -58,4 +59,68 @@ describe("loadToolsCatalog", () => { expect(state.toolsCatalogError).toContain("gateway unavailable"); expect(state.toolsCatalogLoading).toBe(false); }); + + it("allows a new agent request to replace a stale in-flight load", async () => { + const { state, request } = createState(); + + let resolveMain: + | ((value: { + agentId: string; + profiles: { id: string; label: string }[]; + groups: { + id: string; + label: string; + source: string; + tools: { id: string; label: string; description: string; source: string }[]; + }[]; + }) => void) + | null = null; + const mainRequest = new Promise<{ + agentId: string; + profiles: { id: string; label: string }[]; + groups: { + id: string; + label: string; + source: string; + tools: { id: string; label: string; description: string; source: string }[]; + }[]; + }>((resolve) => { + resolveMain = resolve; + }); + + const replacementPayload = { + agentId: "other", + profiles: [{ id: "full", label: "Full" }], + groups: [], + }; + + request.mockImplementationOnce(() => mainRequest).mockResolvedValueOnce(replacementPayload); + + const initialLoad = loadToolsCatalog(state, "main"); + await Promise.resolve(); + + state.agentsSelectedId = "other"; + await loadToolsCatalog(state, "other"); + + expect(request).toHaveBeenNthCalledWith(1, "tools.catalog", { + agentId: "main", + includePlugins: true, + }); + expect(request).toHaveBeenNthCalledWith(2, "tools.catalog", { + agentId: "other", + includePlugins: true, + }); + expect(state.toolsCatalogResult).toEqual(replacementPayload); + expect(state.toolsCatalogLoading).toBe(false); + + resolveMain?.({ + agentId: "main", + profiles: [{ id: "full", label: "Full" }], + groups: [], + }); + await initialLoad; + + expect(state.toolsCatalogResult).toEqual(replacementPayload); + expect(state.toolsCatalogLoading).toBe(false); + }); }); diff --git a/ui/src/ui/controllers/agents.ts b/ui/src/ui/controllers/agents.ts index e9be0630db2..7b09756992b 100644 --- a/ui/src/ui/controllers/agents.ts +++ b/ui/src/ui/controllers/agents.ts @@ -9,6 +9,7 @@ export type AgentsState = { agentsList: AgentsListResult | null; agentsSelectedId: string | null; toolsCatalogLoading: boolean; + toolsCatalogLoadingAgentId?: string | null; toolsCatalogError: string | null; toolsCatalogResult: ToolsCatalogResult | null; }; @@ -44,10 +45,11 @@ export async function loadToolsCatalog(state: AgentsState, agentId: string) { if (!state.client || !state.connected || !resolvedAgentId) { return; } - if (state.toolsCatalogLoading) { + if (state.toolsCatalogLoading && state.toolsCatalogLoadingAgentId === resolvedAgentId) { return; } state.toolsCatalogLoading = true; + state.toolsCatalogLoadingAgentId = resolvedAgentId; state.toolsCatalogError = null; state.toolsCatalogResult = null; try { @@ -55,18 +57,25 @@ export async function loadToolsCatalog(state: AgentsState, agentId: string) { agentId: resolvedAgentId, includePlugins: true, }); + if (state.toolsCatalogLoadingAgentId !== resolvedAgentId) { + return; + } if (state.agentsSelectedId && state.agentsSelectedId !== resolvedAgentId) { return; } state.toolsCatalogResult = res; } catch (err) { + if (state.toolsCatalogLoadingAgentId !== resolvedAgentId) { + return; + } if (state.agentsSelectedId && state.agentsSelectedId !== resolvedAgentId) { return; } state.toolsCatalogResult = null; state.toolsCatalogError = String(err); } finally { - if (!state.agentsSelectedId || state.agentsSelectedId === resolvedAgentId) { + if (state.toolsCatalogLoadingAgentId === resolvedAgentId) { + state.toolsCatalogLoadingAgentId = null; state.toolsCatalogLoading = false; } } diff --git a/ui/src/ui/controllers/config.test.ts b/ui/src/ui/controllers/config.test.ts index 54d04bb1ea7..e9e3f482270 100644 --- a/ui/src/ui/controllers/config.test.ts +++ b/ui/src/ui/controllers/config.test.ts @@ -209,6 +209,50 @@ describe("applyConfig", () => { expect(params.baseHash).toBe("hash-apply-1"); expect(params.sessionKey).toBe("agent:main:web:dm:test"); }); + + it("preserves sensitive form values in serialized raw state for apply", async () => { + const request = createRequestWithConfigGet(); + const state = createState(); + state.connected = true; + state.client = { request } as unknown as ConfigState["client"]; + state.applySessionKey = "agent:main:web:dm:secret"; + state.configFormMode = "form"; + state.configForm = { + gateway: { + auth: { + token: "secret-123", + }, + }, + }; + state.configSchema = { + type: "object", + properties: { + gateway: { + type: "object", + properties: { + auth: { + type: "object", + properties: { + token: { type: "string" }, + }, + }, + }, + }, + }, + }; + state.configSnapshot = { hash: "hash-apply-secret" }; + + await applyConfig(state); + + expect(request.mock.calls[0]?.[0]).toBe("config.apply"); + const params = request.mock.calls[0]?.[1] as { + raw: string; + baseHash: string; + sessionKey: string; + }; + expect(params.raw).toContain("secret-123"); + expect(params.baseHash).toBe("hash-apply-secret"); + }); }); describe("saveConfig", () => { diff --git a/ui/src/ui/types.ts b/ui/src/ui/types.ts index 49da2460652..10d66068288 100644 --- a/ui/src/ui/types.ts +++ b/ui/src/ui/types.ts @@ -6,7 +6,7 @@ import type { SessionsListResultBase, SessionsPatchResultBase, } from "../../../src/shared/session-types.js"; -export type { ConfigUiHints } from "../../../src/shared/config-ui-hints-types.js"; +export type { ConfigUiHint, ConfigUiHints } from "../../../src/shared/config-ui-hints-types.js"; export type ChannelsStatusSnapshot = { ts: number; diff --git a/ui/src/ui/views/config-form.node.ts b/ui/src/ui/views/config-form.node.ts index 20f42f2615b..97a6fb58083 100644 --- a/ui/src/ui/views/config-form.node.ts +++ b/ui/src/ui/views/config-form.node.ts @@ -1,10 +1,13 @@ import { html, nothing, type TemplateResult } from "lit"; +import { icons as sharedIcons } from "../icons.ts"; import type { ConfigUiHints } from "../types.ts"; import { defaultValue, + hasSensitiveConfigData, hintForPath, humanize, pathKey, + REDACTED_PLACEHOLDER, schemaType, type JsonSchema, } from "./config-form.shared.ts"; @@ -100,11 +103,79 @@ type FieldMeta = { tags: string[]; }; +type SensitiveRenderParams = { + path: Array; + value: unknown; + hints: ConfigUiHints; + streamMode: boolean; + revealSensitive: boolean; + isSensitivePathRevealed?: (path: Array) => boolean; +}; + +type SensitiveRenderState = { + isSensitive: boolean; + isRedacted: boolean; + isRevealed: boolean; + canReveal: boolean; +}; + export type ConfigSearchCriteria = { text: string; tags: string[]; }; +function getSensitiveRenderState(params: SensitiveRenderParams): SensitiveRenderState { + const isSensitive = hasSensitiveConfigData(params.value, params.path, params.hints); + const isRevealed = + isSensitive && + !params.streamMode && + (params.revealSensitive || (params.isSensitivePathRevealed?.(params.path) ?? false)); + return { + isSensitive, + isRedacted: isSensitive && !isRevealed, + isRevealed, + canReveal: isSensitive && !params.streamMode, + }; +} + +function renderSensitiveToggleButton(params: { + path: Array; + state: SensitiveRenderState; + disabled: boolean; + onToggleSensitivePath?: (path: Array) => void; +}): TemplateResult | typeof nothing { + const { state } = params; + if (!state.isSensitive || !params.onToggleSensitivePath) { + return nothing; + } + return html` + + `; +} + function hasSearchCriteria(criteria: ConfigSearchCriteria | undefined): boolean { return Boolean(criteria && (criteria.text.length > 0 || criteria.tags.length > 0)); } @@ -331,6 +402,10 @@ export function renderNode(params: { disabled: boolean; showLabel?: boolean; searchCriteria?: ConfigSearchCriteria; + streamMode?: boolean; + revealSensitive?: boolean; + isSensitivePathRevealed?: (path: Array) => boolean; + onToggleSensitivePath?: (path: Array) => void; onPatch: (path: Array, value: unknown) => void; }): TemplateResult | typeof nothing { const { schema, value, path, hints, unsupported, disabled, onPatch } = params; @@ -442,7 +517,19 @@ export function renderNode(params: { } // Complex union (e.g. array | object) — render as JSON textarea - return renderJsonTextarea({ schema, value, path, hints, disabled, showLabel, onPatch }); + return renderJsonTextarea({ + schema, + value, + path, + hints, + disabled, + showLabel, + streamMode: params.streamMode ?? false, + revealSensitive: params.revealSensitive ?? false, + isSensitivePathRevealed: params.isSensitivePathRevealed, + onToggleSensitivePath: params.onToggleSensitivePath, + onPatch, + }); } // Enum - use segmented for small, dropdown for large @@ -540,6 +627,10 @@ function renderTextInput(params: { disabled: boolean; showLabel?: boolean; searchCriteria?: ConfigSearchCriteria; + streamMode?: boolean; + revealSensitive?: boolean; + isSensitivePathRevealed?: (path: Array) => boolean; + onToggleSensitivePath?: (path: Array) => void; inputType: "text" | "number"; onPatch: (path: Array, value: unknown) => void; }): TemplateResult { @@ -547,17 +638,23 @@ function renderTextInput(params: { const showLabel = params.showLabel ?? true; const hint = hintForPath(path, hints); const { label, help, tags } = resolveFieldMeta(path, schema, hints); - const isSensitive = - (hint?.sensitive ?? false) && !/^\$\{[^}]*\}$/.test(String(value ?? "").trim()); - const placeholder = - hint?.placeholder ?? - // oxlint-disable typescript/no-base-to-string - (isSensitive - ? "••••" - : schema.default !== undefined - ? `Default: ${String(schema.default)}` - : ""); - const displayValue = value ?? ""; + const sensitiveState = getSensitiveRenderState({ + path, + value, + hints, + streamMode: params.streamMode ?? false, + revealSensitive: params.revealSensitive ?? false, + isSensitivePathRevealed: params.isSensitivePathRevealed, + }); + const placeholder = sensitiveState.isRedacted + ? REDACTED_PLACEHOLDER + : (hint?.placeholder ?? + // oxlint-disable typescript/no-base-to-string + (schema.default !== undefined ? `Default: ${String(schema.default)}` : "")); + const displayValue = sensitiveState.isRedacted ? "" : (value ?? ""); + const effectiveDisabled = disabled || sensitiveState.isRedacted; + const effectiveInputType = + sensitiveState.isSensitive && !sensitiveState.isRedacted ? "text" : inputType; return html`
@@ -566,12 +663,16 @@ function renderTextInput(params: { ${renderTags(tags)}
{ + if (sensitiveState.isRedacted) { + return; + } const raw = (e.target as HTMLInputElement).value; if (inputType === "number") { if (raw.trim() === "") { @@ -585,13 +686,19 @@ function renderTextInput(params: { onPatch(path, raw); }} @change=${(e: Event) => { - if (inputType === "number") { + if (inputType === "number" || sensitiveState.isRedacted) { return; } const raw = (e.target as HTMLInputElement).value; onPatch(path, raw.trim()); }} /> + ${renderSensitiveToggleButton({ + path, + state: sensitiveState, + disabled, + onToggleSensitivePath: params.onToggleSensitivePath, + })} ${ schema.default !== undefined ? html` @@ -599,7 +706,7 @@ function renderTextInput(params: { type="button" class="cfg-input__reset" title="Reset to default" - ?disabled=${disabled} + ?disabled=${effectiveDisabled} @click=${() => onPatch(path, schema.default)} >↺ ` @@ -712,38 +819,64 @@ function renderJsonTextarea(params: { hints: ConfigUiHints; disabled: boolean; showLabel?: boolean; + streamMode?: boolean; + revealSensitive?: boolean; + isSensitivePathRevealed?: (path: Array) => boolean; + onToggleSensitivePath?: (path: Array) => void; onPatch: (path: Array, value: unknown) => void; }): TemplateResult { const { schema, value, path, hints, disabled, onPatch } = params; const showLabel = params.showLabel ?? true; const { label, help, tags } = resolveFieldMeta(path, schema, hints); const fallback = jsonValue(value); + const sensitiveState = getSensitiveRenderState({ + path, + value, + hints, + streamMode: params.streamMode ?? false, + revealSensitive: params.revealSensitive ?? false, + isSensitivePathRevealed: params.isSensitivePathRevealed, + }); + const displayValue = sensitiveState.isRedacted ? "" : fallback; + const effectiveDisabled = disabled || sensitiveState.isRedacted; return html`
${showLabel ? html`` : nothing} ${help ? html`
${help}
` : nothing} ${renderTags(tags)} - +
+ + ${renderSensitiveToggleButton({ + path, + state: sensitiveState, + disabled, + onToggleSensitivePath: params.onToggleSensitivePath, + })} +
`; } @@ -757,9 +890,26 @@ function renderObject(params: { disabled: boolean; showLabel?: boolean; searchCriteria?: ConfigSearchCriteria; + streamMode?: boolean; + revealSensitive?: boolean; + isSensitivePathRevealed?: (path: Array) => boolean; + onToggleSensitivePath?: (path: Array) => void; onPatch: (path: Array, value: unknown) => void; }): TemplateResult { - const { schema, value, path, hints, unsupported, disabled, onPatch, searchCriteria } = params; + const { + schema, + value, + path, + hints, + unsupported, + disabled, + onPatch, + searchCriteria, + streamMode, + revealSensitive, + isSensitivePathRevealed, + onToggleSensitivePath, + } = params; const showLabel = params.showLabel ?? true; const { label, help, tags } = resolveFieldMeta(path, schema, hints); const selfMatched = @@ -800,6 +950,10 @@ function renderObject(params: { unsupported, disabled, searchCriteria: childSearchCriteria, + streamMode, + revealSensitive, + isSensitivePathRevealed, + onToggleSensitivePath, onPatch, }), )} @@ -814,6 +968,10 @@ function renderObject(params: { disabled, reservedKeys: reserved, searchCriteria: childSearchCriteria, + streamMode, + revealSensitive, + isSensitivePathRevealed, + onToggleSensitivePath, onPatch, }) : nothing @@ -864,9 +1022,26 @@ function renderArray(params: { disabled: boolean; showLabel?: boolean; searchCriteria?: ConfigSearchCriteria; + streamMode?: boolean; + revealSensitive?: boolean; + isSensitivePathRevealed?: (path: Array) => boolean; + onToggleSensitivePath?: (path: Array) => void; onPatch: (path: Array, value: unknown) => void; }): TemplateResult { - const { schema, value, path, hints, unsupported, disabled, onPatch, searchCriteria } = params; + const { + schema, + value, + path, + hints, + unsupported, + disabled, + onPatch, + searchCriteria, + streamMode, + revealSensitive, + isSensitivePathRevealed, + onToggleSensitivePath, + } = params; const showLabel = params.showLabel ?? true; const { label, help, tags } = resolveFieldMeta(path, schema, hints); const selfMatched = @@ -946,6 +1121,10 @@ function renderArray(params: { disabled, searchCriteria: childSearchCriteria, showLabel: false, + streamMode, + revealSensitive, + isSensitivePathRevealed, + onToggleSensitivePath, onPatch, })}
@@ -968,6 +1147,10 @@ function renderMapField(params: { disabled: boolean; reservedKeys: Set; searchCriteria?: ConfigSearchCriteria; + streamMode?: boolean; + revealSensitive?: boolean; + isSensitivePathRevealed?: (path: Array) => boolean; + onToggleSensitivePath?: (path: Array) => void; onPatch: (path: Array, value: unknown) => void; }): TemplateResult { const { @@ -980,6 +1163,10 @@ function renderMapField(params: { reservedKeys, onPatch, searchCriteria, + streamMode, + revealSensitive, + isSensitivePathRevealed, + onToggleSensitivePath, } = params; const anySchema = isAnySchema(schema); const entries = Object.entries(value ?? {}).filter(([key]) => !reservedKeys.has(key)); @@ -1031,6 +1218,14 @@ function renderMapField(params: { ${visibleEntries.map(([key, entryValue]) => { const valuePath = [...path, key]; const fallback = jsonValue(entryValue); + const sensitiveState = getSensitiveRenderState({ + path: valuePath, + value: entryValue, + hints, + streamMode: streamMode ?? false, + revealSensitive: revealSensitive ?? false, + isSensitivePathRevealed, + }); return html`
@@ -1074,26 +1269,40 @@ function renderMapField(params: { ${ anySchema ? html` - + rows="2" + .value=${sensitiveState.isRedacted ? "" : fallback} + ?disabled=${disabled || sensitiveState.isRedacted} + ?readonly=${sensitiveState.isRedacted} + @change=${(e: Event) => { + if (sensitiveState.isRedacted) { + return; + } + const target = e.target as HTMLTextAreaElement; + const raw = target.value.trim(); + if (!raw) { + onPatch(valuePath, undefined); + return; + } + try { + onPatch(valuePath, JSON.parse(raw)); + } catch { + target.value = fallback; + } + }} + > + ${renderSensitiveToggleButton({ + path: valuePath, + state: sensitiveState, + disabled, + onToggleSensitivePath, + })} +
` : renderNode({ schema, @@ -1104,6 +1313,10 @@ function renderMapField(params: { disabled, searchCriteria, showLabel: false, + streamMode, + revealSensitive, + isSensitivePathRevealed, + onToggleSensitivePath, onPatch, }) } diff --git a/ui/src/ui/views/config-form.render.ts b/ui/src/ui/views/config-form.render.ts index 124ca50a585..41d4fb3c8e5 100644 --- a/ui/src/ui/views/config-form.render.ts +++ b/ui/src/ui/views/config-form.render.ts @@ -13,6 +13,10 @@ export type ConfigFormProps = { searchQuery?: string; activeSection?: string | null; activeSubsection?: string | null; + streamMode?: boolean; + revealSensitive?: boolean; + isSensitivePathRevealed?: (path: Array) => boolean; + onToggleSensitivePath?: (path: Array) => void; onPatch: (path: Array, value: unknown) => void; }; @@ -431,6 +435,10 @@ export function renderConfigForm(props: ConfigFormProps) { disabled: props.disabled ?? false, showLabel: false, searchCriteria, + streamMode: props.streamMode ?? false, + revealSensitive: props.revealSensitive ?? false, + isSensitivePathRevealed: props.isSensitivePathRevealed, + onToggleSensitivePath: props.onToggleSensitivePath, onPatch: props.onPatch, })}
@@ -466,6 +474,10 @@ export function renderConfigForm(props: ConfigFormProps) { disabled: props.disabled ?? false, showLabel: false, searchCriteria, + streamMode: props.streamMode ?? false, + revealSensitive: props.revealSensitive ?? false, + isSensitivePathRevealed: props.isSensitivePathRevealed, + onToggleSensitivePath: props.onToggleSensitivePath, onPatch: props.onPatch, })}
diff --git a/ui/src/ui/views/config-form.shared.ts b/ui/src/ui/views/config-form.shared.ts index 366671041da..b535c49e25f 100644 --- a/ui/src/ui/views/config-form.shared.ts +++ b/ui/src/ui/views/config-form.shared.ts @@ -1,4 +1,4 @@ -import type { ConfigUiHints } from "../types.ts"; +import type { ConfigUiHint, ConfigUiHints } from "../types.ts"; export type JsonSchema = { type?: string | string[]; @@ -94,3 +94,110 @@ export function humanize(raw: string) { .replace(/\s+/g, " ") .replace(/^./, (m) => m.toUpperCase()); } + +const SENSITIVE_KEY_WHITELIST_SUFFIXES = [ + "maxtokens", + "maxoutputtokens", + "maxinputtokens", + "maxcompletiontokens", + "contexttokens", + "totaltokens", + "tokencount", + "tokenlimit", + "tokenbudget", + "passwordfile", +] as const; + +const SENSITIVE_PATTERNS = [ + /token$/i, + /password/i, + /secret/i, + /api.?key/i, + /serviceaccount(?:ref)?$/i, +]; + +const ENV_VAR_PLACEHOLDER_PATTERN = /^\$\{[^}]*\}$/; + +export const REDACTED_PLACEHOLDER = "[redacted - click reveal to view]"; + +function isEnvVarPlaceholder(value: string): boolean { + return ENV_VAR_PLACEHOLDER_PATTERN.test(value.trim()); +} + +export function isSensitiveConfigPath(path: string): boolean { + const lowerPath = path.toLowerCase(); + const whitelisted = SENSITIVE_KEY_WHITELIST_SUFFIXES.some((suffix) => lowerPath.endsWith(suffix)); + return !whitelisted && SENSITIVE_PATTERNS.some((pattern) => pattern.test(path)); +} + +function isSensitiveLeafValue(value: unknown): boolean { + if (typeof value === "string") { + return value.trim().length > 0 && !isEnvVarPlaceholder(value); + } + return value !== undefined && value !== null; +} + +function isHintSensitive(hint: ConfigUiHint | undefined): boolean { + return hint?.sensitive ?? false; +} + +export function hasSensitiveConfigData( + value: unknown, + path: Array, + hints: ConfigUiHints, +): boolean { + const key = pathKey(path); + const hint = hintForPath(path, hints); + const pathIsSensitive = isHintSensitive(hint) || isSensitiveConfigPath(key); + + if (pathIsSensitive && isSensitiveLeafValue(value)) { + return true; + } + + if (Array.isArray(value)) { + return value.some((item, index) => hasSensitiveConfigData(item, [...path, index], hints)); + } + + if (value && typeof value === "object") { + return Object.entries(value as Record).some(([childKey, childValue]) => + hasSensitiveConfigData(childValue, [...path, childKey], hints), + ); + } + + return false; +} + +export function countSensitiveConfigValues( + value: unknown, + path: Array, + hints: ConfigUiHints, +): number { + if (value == null) { + return 0; + } + + const key = pathKey(path); + const hint = hintForPath(path, hints); + const pathIsSensitive = isHintSensitive(hint) || isSensitiveConfigPath(key); + + if (pathIsSensitive && isSensitiveLeafValue(value)) { + return 1; + } + + if (Array.isArray(value)) { + return value.reduce( + (count, item, index) => count + countSensitiveConfigValues(item, [...path, index], hints), + 0, + ); + } + + if (value && typeof value === "object") { + return Object.entries(value as Record).reduce( + (count, [childKey, childValue]) => + count + countSensitiveConfigValues(childValue, [...path, childKey], hints), + 0, + ); + } + + return 0; +} diff --git a/ui/src/ui/views/config.browser.test.ts b/ui/src/ui/views/config.browser.test.ts index 889d046f942..26bc035d07f 100644 --- a/ui/src/ui/views/config.browser.test.ts +++ b/ui/src/ui/views/config.browser.test.ts @@ -1,8 +1,12 @@ import { render } from "lit"; -import { describe, expect, it, vi } from "vitest"; -import { renderConfig } from "./config.ts"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { renderConfig, resetConfigViewStateForTests } from "./config.ts"; describe("config view", () => { + beforeEach(() => { + resetConfigViewStateForTests(); + }); + const baseProps = () => ({ raw: "{\n}\n", originalRaw: "{\n}\n", @@ -20,11 +24,13 @@ describe("config view", () => { schemaLoading: false, uiHints: {}, formMode: "form" as const, + showModeToggle: true, formValue: {}, originalValue: {}, searchQuery: "", activeSection: null, activeSubsection: null, + streamMode: false, onRawChange: vi.fn(), onFormModeChange: vi.fn(), onFormPatch: vi.fn(), @@ -35,6 +41,13 @@ describe("config view", () => { onApply: vi.fn(), onUpdate: vi.fn(), onSubsectionChange: vi.fn(), + version: "", + theme: "claw" as const, + themeMode: "system" as const, + setTheme: vi.fn(), + setThemeMode: vi.fn(), + gatewayUrl: "", + assistantName: "", }); function findActionButtons(container: HTMLElement): { @@ -134,6 +147,102 @@ describe("config view", () => { expect(applyButton?.disabled).toBe(false); }); + it("keeps raw secrets out of the DOM while stream mode is enabled", () => { + const container = document.createElement("div"); + render( + renderConfig({ + ...baseProps(), + formMode: "raw", + streamMode: true, + raw: '{\n gateway: { auth: { token: "secret-123" } }\n}\n', + originalRaw: "{\n}\n", + formValue: { gateway: { auth: { token: "secret-123" } } }, + uiHints: { + "gateway.auth.token": { sensitive: true }, + }, + }), + container, + ); + + const textarea = container.querySelector("textarea"); + expect(textarea).not.toBeNull(); + expect(textarea?.value).toBe(""); + expect(textarea?.getAttribute("placeholder")).toContain("redacted"); + + const toggle = container.querySelector( + 'button[aria-label="Toggle raw config redaction"]', + ); + expect(toggle?.disabled).toBe(true); + }); + + it("reveals raw secrets only after explicit toggle when stream mode is off", () => { + const container = document.createElement("div"); + const props = { + ...baseProps(), + formMode: "raw" as const, + streamMode: false, + raw: '{\n gateway: { auth: { token: "secret-123" } }\n}\n', + originalRaw: "{\n}\n", + formValue: { gateway: { auth: { token: "secret-123" } } }, + uiHints: { + "gateway.auth.token": { sensitive: true }, + }, + }; + + render(renderConfig(props), container); + const initialTextarea = container.querySelector("textarea"); + expect(initialTextarea?.value).toBe(""); + + const toggle = container.querySelector( + 'button[aria-label="Toggle raw config redaction"]', + ); + expect(toggle?.disabled).toBe(false); + toggle?.click(); + + render(renderConfig(props), container); + const revealedTextarea = container.querySelector("textarea"); + expect(revealedTextarea?.value).toContain("secret-123"); + }); + + it("reveals env values through the peek control instead of CSS-only masking", () => { + const container = document.createElement("div"); + const props = { + ...baseProps(), + activeSection: "env" as const, + formMode: "form" as const, + streamMode: false, + schema: { + type: "object", + properties: { + env: { + type: "object", + additionalProperties: { type: "string" }, + }, + }, + }, + formValue: { + env: { + OPENAI_API_KEY: "secret-123", + }, + }, + }; + + render(renderConfig(props), container); + const hiddenInput = container.querySelector(".cfg-input:not(.cfg-input--sm)"); + expect(hiddenInput?.value).toBe(""); + + const peekButton = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent?.includes("Peek"), + ); + peekButton?.click(); + + render(renderConfig(props), container); + const revealedInput = container.querySelector( + ".cfg-input:not(.cfg-input--sm)", + ); + expect(revealedInput?.value).toBe("secret-123"); + }); + it("switches mode via the sidebar toggle", () => { const container = document.createElement("div"); const onFormModeChange = vi.fn(); @@ -204,12 +313,7 @@ describe("config view", () => { const container = document.createElement("div"); render(renderConfig(baseProps()), container); - const options = Array.from(container.querySelectorAll(".config-search__tag-option")).map( - (option) => option.textContent?.trim(), - ); - expect(options).toContain("tag:security"); - expect(options).toContain("tag:advanced"); - expect(options).toHaveLength(15); + expect(container.querySelectorAll(".config-search__tag-option")).toHaveLength(0); }); it("updates search query when toggling a tag option", () => { @@ -226,8 +330,7 @@ describe("config view", () => { const option = container.querySelector( '.config-search__tag-option[data-tag="security"]', ); - expect(option).toBeTruthy(); - option?.click(); - expect(onSearchChange).toHaveBeenCalledWith("tag:security"); + expect(option).toBeNull(); + expect(onSearchChange).not.toHaveBeenCalled(); }); }); diff --git a/ui/src/ui/views/config.ts b/ui/src/ui/views/config.ts index 297f1391a4c..9335f66f9a5 100644 --- a/ui/src/ui/views/config.ts +++ b/ui/src/ui/views/config.ts @@ -3,7 +3,14 @@ import { icons } from "../icons.ts"; import type { ThemeTransitionContext } from "../theme-transition.ts"; import type { ThemeMode, ThemeName } from "../theme.ts"; import type { ConfigUiHints } from "../types.ts"; -import { humanize, schemaType, type JsonSchema } from "./config-form.shared.ts"; +import { + countSensitiveConfigValues, + humanize, + pathKey, + REDACTED_PLACEHOLDER, + schemaType, + type JsonSchema, +} from "./config-form.shared.ts"; import { analyzeConfigSchema, renderConfigForm, SECTION_META } from "./config-form.ts"; export type ConfigProps = { @@ -494,42 +501,6 @@ function truncateValue(value: unknown, maxLen = 40): string { return str.slice(0, maxLen - 3) + "..."; } -const SENSITIVE_KEY_RE = /token|password|secret|api.?key/i; -const SENSITIVE_KEY_WHITELIST_RE = - /maxtokens|maxoutputtokens|maxinputtokens|maxcompletiontokens|contexttokens|totaltokens|tokencount|tokenlimit|tokenbudget|passwordfile/i; - -function countSensitiveValues(formValue: Record | null): number { - if (!formValue) { - return 0; - } - let count = 0; - function walk(obj: unknown, key?: string) { - if (obj == null) { - return; - } - if (typeof obj === "object" && !Array.isArray(obj)) { - for (const [k, v] of Object.entries(obj as Record)) { - walk(v, k); - } - } else if (Array.isArray(obj)) { - for (const item of obj) { - walk(item); - } - } else if ( - key && - typeof obj === "string" && - SENSITIVE_KEY_RE.test(key) && - !SENSITIVE_KEY_WHITELIST_RE.test(key) - ) { - if (obj.trim() && !/^\$\{[^}]*\}$/.test(obj.trim())) { - count++; - } - } - } - walk(formValue); - return count; -} - type ThemeOption = { id: ThemeName; label: string; description: string; icon: TemplateResult }; const THEME_OPTIONS: ThemeOption[] = [ { id: "claw", label: "Claw", description: "Chroma family", icon: icons.zap }, @@ -644,7 +615,33 @@ function renderAppearanceSection(props: ConfigProps) { } let rawRevealed = false; +let envRevealed = false; let validityDismissed = false; +const revealedSensitivePaths = new Set(); + +function isSensitivePathRevealed(path: Array): boolean { + const key = pathKey(path); + return key ? revealedSensitivePaths.has(key) : false; +} + +function toggleSensitivePathReveal(path: Array) { + const key = pathKey(path); + if (!key) { + return; + } + if (revealedSensitivePaths.has(key)) { + revealedSensitivePaths.delete(key); + } else { + revealedSensitivePaths.add(key); + } +} + +export function resetConfigViewStateForTests() { + rawRevealed = false; + envRevealed = false; + validityDismissed = false; + revealedSensitivePaths.clear(); +} export function renderConfig(props: ConfigProps) { const showModeToggle = props.showModeToggle ?? false; @@ -659,6 +656,7 @@ export function renderConfig(props: ConfigProps) { unsupportedPaths: scopeUnsupportedPaths(rawAnalysis.unsupportedPaths, { include, exclude }), }; const formUnsafe = analysis.schema ? analysis.unsupportedPaths.length > 0 : false; + const envSensitiveVisible = !props.streamMode && envRevealed; // Build categorised nav from schema — only include sections that exist in the schema const schemaProps = analysis.schema?.properties ?? {}; @@ -949,17 +947,21 @@ export function renderConfig(props: ConfigProps) { props.activeSection === "env" ? html`