fix(ui): resolve remaining important and minor issues
4. fix(theme): prevent stale theme listener after component remount - Replace module-level systemThemeCleanup with WeakMap keyed by host - Prevents stale closure responding to theme changes after remount - Addresses Greptile test/HMR issue 5. fix(security): validate usage/cost metadata from chat history - Add sanitizeUsage() and sanitizeCost() helpers - Validate numeric fields are finite numbers - Only allow usage/cost on assistant messages - Prevents UI crash from malformed transcript JSON (cost.toFixed on non-number) - Addresses Aisle Security Low severity (but UI-breaking) issue 6. refactor(chat): deduplicate export functions - Extract exportChatMarkdown to shared chat-export.ts module - Remove duplicate from app.ts and chat.ts - Prevents silent divergence during maintenance - Addresses Greptile technical debt concern 7. fix(security): add noopener to external links - Use buildExternalLinkRel() helper in overview-attention.ts - Prevents reverse tabnabbing on attention item doc links - Addresses Aisle Security Low severity CWE-1022 8. fix(security): scan raw config for sensitive keywords in stream mode - Add containsSensitiveKeywords() helper - Check props.raw for token/password/secret/apiKey patterns - Redact raw textarea when keywords detected in stream mode - Prevents newly-entered secrets from staying visible before parse - Addresses Aisle Security Low severity issue
This commit is contained in:
Val Alexander
parent
03aa0f969c
commit
43430d4900
@ -168,6 +168,60 @@ function sanitizeChatHistoryContentBlock(block: unknown): { block: unknown; chan
|
||||
return { block: changed ? entry : block, changed };
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate that a value is a finite number, returning undefined otherwise.
|
||||
*/
|
||||
function toFiniteNumber(x: unknown): number | undefined {
|
||||
return typeof x === "number" && Number.isFinite(x) ? x : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize usage metadata to ensure only finite numeric fields are included.
|
||||
* Prevents UI crashes from malformed transcript JSON.
|
||||
*/
|
||||
function sanitizeUsage(raw: unknown): Record<string, number> | undefined {
|
||||
if (!raw || typeof raw !== "object") {
|
||||
return undefined;
|
||||
}
|
||||
const u = raw as Record<string, unknown>;
|
||||
const out: Record<string, number> = {};
|
||||
|
||||
// Whitelist known usage fields and validate they're finite numbers
|
||||
const knownFields = [
|
||||
"input",
|
||||
"output",
|
||||
"totalTokens",
|
||||
"inputTokens",
|
||||
"outputTokens",
|
||||
"cacheRead",
|
||||
"cacheWrite",
|
||||
"cache_read_input_tokens",
|
||||
"cache_creation_input_tokens",
|
||||
];
|
||||
|
||||
for (const k of knownFields) {
|
||||
const n = toFiniteNumber(u[k]);
|
||||
if (n !== undefined) {
|
||||
out[k] = n;
|
||||
}
|
||||
}
|
||||
|
||||
return Object.keys(out).length > 0 ? out : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize cost metadata to ensure only finite numeric fields are included.
|
||||
* Prevents UI crashes from calling .toFixed() on non-numbers.
|
||||
*/
|
||||
function sanitizeCost(raw: unknown): { total?: number } | undefined {
|
||||
if (!raw || typeof raw !== "object") {
|
||||
return undefined;
|
||||
}
|
||||
const c = raw as Record<string, unknown>;
|
||||
const total = toFiniteNumber(c.total);
|
||||
return total !== undefined ? { total } : undefined;
|
||||
}
|
||||
|
||||
function sanitizeChatHistoryMessage(message: unknown): { message: unknown; changed: boolean } {
|
||||
if (!message || typeof message !== "object") {
|
||||
return { message, changed: false };
|
||||
@ -181,6 +235,37 @@ function sanitizeChatHistoryMessage(message: unknown): { message: unknown; chang
|
||||
}
|
||||
|
||||
// Keep usage/cost so the chat UI can render per-message token and cost badges.
|
||||
// Only retain usage/cost on assistant messages and validate numeric fields to prevent UI crashes.
|
||||
if (entry.role !== "assistant") {
|
||||
if ("usage" in entry) {
|
||||
delete entry.usage;
|
||||
changed = true;
|
||||
}
|
||||
if ("cost" in entry) {
|
||||
delete entry.cost;
|
||||
changed = true;
|
||||
}
|
||||
} else {
|
||||
// Validate and sanitize usage/cost for assistant messages
|
||||
if ("usage" in entry) {
|
||||
const sanitized = sanitizeUsage(entry.usage);
|
||||
if (sanitized) {
|
||||
entry.usage = sanitized;
|
||||
} else {
|
||||
delete entry.usage;
|
||||
}
|
||||
changed = true;
|
||||
}
|
||||
if ("cost" in entry) {
|
||||
const sanitized = sanitizeCost(entry.cost);
|
||||
if (sanitized) {
|
||||
entry.cost = sanitized;
|
||||
} else {
|
||||
delete entry.cost;
|
||||
}
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (typeof entry.content === "string") {
|
||||
const stripped = stripInlineDirectiveTagsForDisplay(entry.content);
|
||||
|
||||
@ -36,7 +36,11 @@ import { startThemeTransition, type ThemeTransitionContext } from "./theme-trans
|
||||
import { resolveTheme, type ResolvedTheme, type ThemeMode, type ThemeName } from "./theme.ts";
|
||||
import { cleanupChatModuleState } from "./views/chat.ts";
|
||||
|
||||
let systemThemeCleanup: (() => void) | null = null;
|
||||
/**
|
||||
* Per-host theme listener cleanup functions.
|
||||
* Prevents stale closures after component remount by keying cleanup by host instance.
|
||||
*/
|
||||
const systemThemeCleanupMap = new WeakMap<SettingsHost, () => void>();
|
||||
import type { AgentsListResult, AttentionItem } from "./types.ts";
|
||||
|
||||
type SettingsHost = {
|
||||
@ -282,10 +286,11 @@ export function attachThemeListener(host: SettingsHost) {
|
||||
syncSystemThemeListener(host);
|
||||
}
|
||||
|
||||
export function detachThemeListener(_host: SettingsHost) {
|
||||
if (systemThemeCleanup) {
|
||||
systemThemeCleanup();
|
||||
systemThemeCleanup = null;
|
||||
export function detachThemeListener(host: SettingsHost) {
|
||||
const cleanup = systemThemeCleanupMap.get(host);
|
||||
if (cleanup) {
|
||||
cleanup();
|
||||
systemThemeCleanupMap.delete(host);
|
||||
}
|
||||
}
|
||||
|
||||
@ -300,16 +305,21 @@ export function applyResolvedTheme(host: SettingsHost, resolved: ResolvedTheme)
|
||||
}
|
||||
|
||||
function syncSystemThemeListener(host: SettingsHost) {
|
||||
// Clean up existing listener if mode is not "system"
|
||||
if (host.themeMode !== "system") {
|
||||
if (systemThemeCleanup) {
|
||||
systemThemeCleanup();
|
||||
systemThemeCleanup = null;
|
||||
const cleanup = systemThemeCleanupMap.get(host);
|
||||
if (cleanup) {
|
||||
cleanup();
|
||||
systemThemeCleanupMap.delete(host);
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (systemThemeCleanup) {
|
||||
|
||||
// Skip if listener already attached for this host
|
||||
if (systemThemeCleanupMap.has(host)) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (typeof globalThis.matchMedia !== "function") {
|
||||
return;
|
||||
}
|
||||
@ -322,7 +332,7 @@ function syncSystemThemeListener(host: SettingsHost) {
|
||||
applyResolvedTheme(host, resolveTheme(host.theme, "system"));
|
||||
};
|
||||
mql.addEventListener("change", onChange);
|
||||
systemThemeCleanup = () => mql.removeEventListener("change", onChange);
|
||||
systemThemeCleanupMap.set(host, () => mql.removeEventListener("change", onChange));
|
||||
}
|
||||
|
||||
export function syncTabWithLocation(host: SettingsHost, replace: boolean) {
|
||||
|
||||
@ -53,6 +53,7 @@ import {
|
||||
} from "./app-tool-stream.ts";
|
||||
import type { AppViewState } from "./app-view-state.ts";
|
||||
import { normalizeAssistantIdentity } from "./assistant-identity.ts";
|
||||
import { exportChatMarkdown } from "./chat-export.ts";
|
||||
import { loadAssistantIdentity as loadAssistantIdentityInternal } from "./controllers/assistant-identity.ts";
|
||||
import type { DevicePairingList } from "./controllers/devices.ts";
|
||||
import type { ExecApprovalRequest } from "./controllers/exec-approval.ts";
|
||||
@ -94,28 +95,6 @@ declare global {
|
||||
|
||||
const bootAssistantIdentity = normalizeAssistantIdentity({});
|
||||
|
||||
function exportChatMarkdown(messages: unknown[], assistantName: string): void {
|
||||
const history = Array.isArray(messages) ? messages : [];
|
||||
if (history.length === 0) {
|
||||
return;
|
||||
}
|
||||
const lines: string[] = [`# Chat with ${assistantName}`, ""];
|
||||
for (const msg of history) {
|
||||
const m = msg as Record<string, unknown>;
|
||||
const role = m.role === "user" ? "You" : m.role === "assistant" ? assistantName : "Tool";
|
||||
const content = typeof m.content === "string" ? m.content : "";
|
||||
const ts = typeof m.timestamp === "number" ? new Date(m.timestamp).toISOString() : "";
|
||||
lines.push(`## ${role}${ts ? ` (${ts})` : ""}`, "", content, "");
|
||||
}
|
||||
const blob = new Blob([lines.join("\n")], { type: "text/markdown" });
|
||||
const url = URL.createObjectURL(blob);
|
||||
const a = document.createElement("a");
|
||||
a.href = url;
|
||||
a.download = `chat-${assistantName}-${Date.now()}.md`;
|
||||
a.click();
|
||||
URL.revokeObjectURL(url);
|
||||
}
|
||||
|
||||
function resolveOnboardingMode(): boolean {
|
||||
if (!window.location.search) {
|
||||
return false;
|
||||
|
||||
25
ui/src/ui/chat-export.ts
Normal file
25
ui/src/ui/chat-export.ts
Normal file
@ -0,0 +1,25 @@
|
||||
/**
|
||||
* Export chat history as markdown file.
|
||||
* Shared utility to prevent code duplication between chat.ts and app.ts.
|
||||
*/
|
||||
export function exportChatMarkdown(messages: unknown[], assistantName: string): void {
|
||||
const history = Array.isArray(messages) ? messages : [];
|
||||
if (history.length === 0) {
|
||||
return;
|
||||
}
|
||||
const lines: string[] = [`# Chat with ${assistantName}`, ""];
|
||||
for (const msg of history) {
|
||||
const m = msg as Record<string, unknown>;
|
||||
const role = m.role === "user" ? "You" : m.role === "assistant" ? assistantName : "Tool";
|
||||
const content = typeof m.content === "string" ? m.content : "";
|
||||
const ts = typeof m.timestamp === "number" ? new Date(m.timestamp).toISOString() : "";
|
||||
lines.push(`## ${role}${ts ? ` (${ts})` : ""}`, "", content, "");
|
||||
}
|
||||
const blob = new Blob([lines.join("\n")], { type: "text/markdown" });
|
||||
const url = URL.createObjectURL(blob);
|
||||
const a = document.createElement("a");
|
||||
a.href = url;
|
||||
a.download = `chat-${assistantName}-${Date.now()}.md`;
|
||||
a.click();
|
||||
URL.revokeObjectURL(url);
|
||||
}
|
||||
@ -1,6 +1,7 @@
|
||||
import { html, nothing, type TemplateResult } from "lit";
|
||||
import { ref } from "lit/directives/ref.js";
|
||||
import { repeat } from "lit/directives/repeat.js";
|
||||
import { exportChatMarkdown } from "../chat-export.ts";
|
||||
import { DeletedMessages } from "../chat/deleted-messages.ts";
|
||||
import {
|
||||
renderMessageGroup,
|
||||
@ -533,26 +534,11 @@ function tokenEstimate(draft: string): string | null {
|
||||
return `~${Math.ceil(draft.length / 4)} tokens`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Export chat markdown - delegates to shared utility.
|
||||
*/
|
||||
function exportMarkdown(props: ChatProps): void {
|
||||
const history = Array.isArray(props.messages) ? props.messages : [];
|
||||
if (history.length === 0) {
|
||||
return;
|
||||
}
|
||||
const lines: string[] = [`# Chat with ${props.assistantName}`, ""];
|
||||
for (const msg of history) {
|
||||
const m = msg as Record<string, unknown>;
|
||||
const role = m.role === "user" ? "You" : m.role === "assistant" ? props.assistantName : "Tool";
|
||||
const content = typeof m.content === "string" ? m.content : "";
|
||||
const ts = typeof m.timestamp === "number" ? new Date(m.timestamp).toISOString() : "";
|
||||
lines.push(`## ${role}${ts ? ` (${ts})` : ""}`, "", content, "");
|
||||
}
|
||||
const blob = new Blob([lines.join("\n")], { type: "text/markdown" });
|
||||
const url = URL.createObjectURL(blob);
|
||||
const a = document.createElement("a");
|
||||
a.href = url;
|
||||
a.download = `chat-${props.assistantName}-${Date.now()}.md`;
|
||||
a.click();
|
||||
URL.revokeObjectURL(url);
|
||||
exportChatMarkdown(props.messages, props.assistantName);
|
||||
}
|
||||
|
||||
const WELCOME_SUGGESTIONS = [
|
||||
|
||||
@ -522,6 +522,20 @@ function renderDiffValue(
|
||||
return truncateValue(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Lightweight scan for sensitive keywords in raw config text.
|
||||
* Used when stream mode is on and formValue hasn't been parsed yet.
|
||||
*/
|
||||
function containsSensitiveKeywords(raw: string): boolean {
|
||||
// Match key patterns from SENSITIVE_PATTERNS in config-form.shared.ts
|
||||
return (
|
||||
/["']?\w*token["']?\s*:/i.test(raw) ||
|
||||
/["']?\w*password["']?\s*:/i.test(raw) ||
|
||||
/["']?\w*secret["']?\s*:/i.test(raw) ||
|
||||
/["']?\w*api.?key["']?\s*:/i.test(raw)
|
||||
);
|
||||
}
|
||||
|
||||
type ThemeOption = { id: ThemeName; label: string; description: string; icon: TemplateResult };
|
||||
const THEME_OPTIONS: ThemeOption[] = [
|
||||
{ id: "claw", label: "Claw", description: "Chroma family", icon: icons.zap },
|
||||
@ -610,7 +624,7 @@ function renderAppearanceSection(props: ConfigProps) {
|
||||
<div class="settings-info-grid">
|
||||
<div class="settings-info-row">
|
||||
<span class="settings-info-row__label">Gateway</span>
|
||||
<span class="settings-info-row__value mono">${props.gatewayUrl || "—"}</span>
|
||||
<span class="settings-info-row__value mono">${props.gatewayUrl || "-"}</span>
|
||||
</div>
|
||||
<div class="settings-info-row">
|
||||
<span class="settings-info-row__label">Status</span>
|
||||
@ -679,7 +693,7 @@ export function renderConfig(props: ConfigProps) {
|
||||
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
|
||||
// Build categorised nav from schema - only include sections that exist in the schema
|
||||
const schemaProps = analysis.schema?.properties ?? {};
|
||||
|
||||
const VIRTUAL_SECTIONS = new Set(["__appearance__"]);
|
||||
@ -1052,8 +1066,15 @@ export function renderConfig(props: ConfigProps) {
|
||||
[],
|
||||
props.uiHints,
|
||||
);
|
||||
const blurred = sensitiveCount > 0 && (props.streamMode || !rawRevealed);
|
||||
const canReveal = sensitiveCount > 0 && !props.streamMode;
|
||||
// In stream mode, also check raw content for sensitive keywords
|
||||
// to prevent newly-entered secrets from being visible before parse
|
||||
const rawHasSensitiveKeywords =
|
||||
props.streamMode && containsSensitiveKeywords(props.raw);
|
||||
const blurred =
|
||||
(sensitiveCount > 0 || rawHasSensitiveKeywords) &&
|
||||
(props.streamMode || !rawRevealed);
|
||||
const canReveal =
|
||||
(sensitiveCount > 0 || rawHasSensitiveKeywords) && !props.streamMode;
|
||||
return html`
|
||||
<label class="field config-raw-field">
|
||||
<span style="display:flex;align-items:center;gap:8px;">
|
||||
|
||||
@ -1,5 +1,6 @@
|
||||
import { html, nothing } from "lit";
|
||||
import { t } from "../../i18n/index.ts";
|
||||
import { buildExternalLinkRel, EXTERNAL_LINK_TARGET } from "../external-link.ts";
|
||||
import { icons, type IconName } from "../icons.ts";
|
||||
import type { AttentionItem } from "../types.ts";
|
||||
|
||||
@ -46,8 +47,8 @@ export function renderOverviewAttention(props: OverviewAttentionProps) {
|
||||
? html`<a
|
||||
class="ov-attention-link"
|
||||
href=${item.href}
|
||||
target=${item.external ? "_blank" : ""}
|
||||
rel=${item.external ? "noreferrer" : ""}
|
||||
target=${item.external ? EXTERNAL_LINK_TARGET : nothing}
|
||||
rel=${item.external ? buildExternalLinkRel() : nothing}
|
||||
>${t("common.docs")}</a>`
|
||||
: nothing
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user