diff --git a/dato-agent/src/components/AgentSurface.module.css b/dato-agent/src/components/AgentSurface.module.css index 860a527a..f98dd120 100644 --- a/dato-agent/src/components/AgentSurface.module.css +++ b/dato-agent/src/components/AgentSurface.module.css @@ -644,6 +644,51 @@ line-height: 1.4; } +.diagnosticsReview { + display: flex; + flex-direction: column; + gap: var(--spacing-xs); + margin: var(--spacing-s) 0 0; + padding: var(--spacing-s); + border: 1px solid var(--color--border); + border-radius: var(--border-radius-m, 6px); + font-size: var(--font-size-xs); +} + +.diagnosticsReview legend { + padding: 0 var(--spacing-xs); + color: var(--color--ink); + font-weight: 600; +} + +.diagnosticsReviewItem { + display: flex; + align-items: baseline; + gap: var(--spacing-xs); + color: var(--color--ink-subtle); +} + +.diagnosticsReviewHint { + margin: 0; + color: var(--color--ink-subtle); +} + +.diagnosticsReviewItem .diagnosticsReviewAction { + flex: none; + color: var(--color--ink); + font-weight: 600; +} + +.diagnosticsReviewItem code { + flex: none; + white-space: nowrap; +} + +.diagnosticsReviewItem span { + min-width: 0; + overflow-wrap: anywhere; +} + .activityDisclosure { color: var(--color--ink-subtle); font-size: var(--font-size-s); diff --git a/dato-agent/src/components/AgentSurface.test.tsx b/dato-agent/src/components/AgentSurface.test.tsx index 1f3c3fc2..fd0c5fc5 100644 --- a/dato-agent/src/components/AgentSurface.test.tsx +++ b/dato-agent/src/components/AgentSurface.test.tsx @@ -1384,6 +1384,78 @@ describe('AgentSurface', () => { ).not.toBeInTheDocument(); }); + it('asks which secret-looking values to redact before copying diagnostics', async () => { + const onCopyFailureDiagnostics = vi + .fn() + .mockResolvedValueOnce({ + status: 'review', + candidates: [ + { + value: 'first-secret-value-0001', + preview: '…0001 · 23 chars', + path: 'conversation[0].text', + occurrences: 1, + }, + { + value: 'second-secret-value-002', + preview: '…0002 · 23 chars', + path: 'turn.events[1].output', + occurrences: 2, + }, + ], + }) + .mockResolvedValueOnce({ status: 'copied' }); + const user = userEvent.setup(); + + render( + , + ); + + await user.click(screen.getByRole('button', { name: 'Copy diagnostics' })); + expect( + screen.getByText(/2 values in the diagnostics look like secrets/), + ).toBeInTheDocument(); + expect(screen.getByText('…0002 · 23 chars')).toBeInTheDocument(); + expect( + screen.getByText( + 'Ticked values are replaced with [redacted] in the copy.', + ), + ).toBeInTheDocument(); + expect(screen.getAllByText('Redact')).toHaveLength(2); + expect( + screen.queryByText('second-secret-value-002'), + ).not.toBeInTheDocument(); + + // Everything starts selected; untick one, then copy. + const checkboxes = screen.getAllByRole('checkbox'); + expect( + checkboxes.every((checkbox) => (checkbox as HTMLInputElement).checked), + ).toBe(true); + await user.click(checkboxes[1] as HTMLElement); + await user.click( + screen.getByRole('button', { name: 'Copy with 1 redacted' }), + ); + + expect(onCopyFailureDiagnostics).toHaveBeenLastCalledWith('failure-1', [ + 'first-secret-value-0001', + ]); + expect(screen.queryByText(/look like secrets/)).not.toBeInTheDocument(); + }); + it('offers compact recovery actions only for a terminal assistant failure', async () => { const onRetryFailedTurn = vi.fn(); const onCopyFailureDiagnostics = vi.fn().mockResolvedValue(undefined); @@ -1442,7 +1514,10 @@ describe('AgentSurface', () => { await user.click(copyButton); expect(onCopyFailureDiagnostics).toHaveBeenCalledOnce(); - expect(onCopyFailureDiagnostics).toHaveBeenCalledWith('failure-1'); + expect(onCopyFailureDiagnostics).toHaveBeenCalledWith( + 'failure-1', + undefined, + ); }); it('shows only diagnostics for a non-retryable terminal failure', () => { diff --git a/dato-agent/src/components/AgentSurface.tsx b/dato-agent/src/components/AgentSurface.tsx index 416398b4..0ab660d9 100644 --- a/dato-agent/src/components/AgentSurface.tsx +++ b/dato-agent/src/components/AgentSurface.tsx @@ -31,6 +31,7 @@ import type { Mention, } from '../lib/mentions'; import { fallbackAssetMention, fallbackRecordMention } from '../lib/mentions'; +import type { SecretCandidate } from '../lib/secretCandidates'; import { MentionDisplay } from '../recordComments/entrypoints/components/shared/MentionDisplay'; import commentStyles from '../recordComments/entrypoints/styles/comment.module.css'; import { @@ -313,7 +314,14 @@ export type AgentSurfaceProps = { onRejectUnsafeAction?: (approval: UnsafeApprovalViewModel) => void; onRepairUnsafeAction?: (approval: UnsafeApprovalViewModel) => void; onRetryFailedTurn?: (failureId: string) => void | Promise; - onCopyFailureDiagnostics?: (failureId: string) => Promise; + /** + * Copies a failure's diagnostics. Without `redact`, it may return `review` with values that look + * like secrets instead of copying; calling again with the values to redact (possibly none) copies. + */ + onCopyFailureDiagnostics?: ( + failureId: string, + redact?: readonly string[], + ) => Promise; autoApproveEnabled?: boolean; autoApproveChanging?: boolean; autoApproveError?: string; @@ -695,6 +703,10 @@ function EmptyState() { ); } +export type CopyDiagnosticsResult = + | { status: 'copied' } + | { status: 'review'; candidates: readonly SecretCandidate[] }; + function FailureActions({ failure, onRetry, @@ -708,6 +720,9 @@ function FailureActions({ const [copyState, setCopyState] = useState< 'idle' | 'copying' | 'copied' | 'error' >('idle'); + // Values that look like secrets, awaiting the user's choice; all start selected. + const [review, setReview] = useState(); + const [selected, setSelected] = useState>(new Set()); const copiedTimerRef = useRef>(undefined); useEffect( @@ -735,7 +750,7 @@ function FailureActions({ } }; - const copyDiagnostics = async () => { + const copyDiagnostics = async (redact?: readonly string[]) => { if (retryPending || copyState === 'copying' || !onCopyDiagnostics) { return; } @@ -748,7 +763,16 @@ function FailureActions({ setCopyState('copying'); try { - await onCopyDiagnostics(failure.id); + const result = await onCopyDiagnostics(failure.id, redact); + if (result?.status === 'review') { + setReview(result.candidates); + setSelected( + new Set(result.candidates.map((candidate) => candidate.value)), + ); + setCopyState('idle'); + return; + } + setReview(undefined); setCopyState('copied'); copiedTimerRef.current = setTimeout(() => { copiedTimerRef.current = undefined; @@ -797,6 +821,66 @@ function FailureActions({ Couldn’t copy diagnostics

)} + {review && ( +
+ + {review.length === 1 + ? 'This value in the diagnostics looks like a secret.' + : `${review.length} values in the diagnostics look like secrets.`} + +

+ Ticked values are replaced with [redacted] in the copy. +

+ {review.map((candidate) => ( + + ))} +
+ + +
+
+ )} ); } diff --git a/dato-agent/src/entrypoints/AgentFrame.test.tsx b/dato-agent/src/entrypoints/AgentFrame.test.tsx index 03f8cf89..95601e24 100644 --- a/dato-agent/src/entrypoints/AgentFrame.test.tsx +++ b/dato-agent/src/entrypoints/AgentFrame.test.tsx @@ -6708,11 +6708,18 @@ describe('AgentFrame', () => { expect(writeText).toHaveBeenCalledOnce(); const diagnostics = JSON.parse(String(writeText.mock.calls[0]?.[0])); - expect(diagnostics.configuration.openAiApiKey).toBe('sk-diagnostic-secret'); - expect(diagnostics.configuration.activeApiKey).toBe('sk-diagnostic-secret'); - expect(diagnostics.oauthCredentials.token.accessToken).toBe( + // Allowlisted payload: keys and tokens are absent, not just masked. + expect(diagnostics.configuration.openAiApiKey).toBeUndefined(); + expect(diagnostics.configuration.activeApiKey).toBeUndefined(); + expect(diagnostics.configuration.hasOpenAiApiKey).toBe(true); + expect(diagnostics.oauthCredentials.token.accessToken).toBeUndefined(); + expect(diagnostics.oauthCredentials.token.tokenType).toBe('Bearer'); + expect(String(writeText.mock.calls[0]?.[0])).not.toContain( 'diagnostic-oauth-secret', ); + expect(String(writeText.mock.calls[0]?.[0])).not.toContain( + 'sk-diagnostic-secret', + ); expect(diagnostics.turn.message).toBe( 'Find the private record with all of its content', ); @@ -6752,6 +6759,100 @@ describe('AgentFrame', () => { ); }); + it('masks the OAuth token a turn actually used, not only the one its render closed over', async () => { + const frameProps = props({ + currentUserId: `diagnostics-rotated-user-${testUser}`, + }); + const credentialStore = createCredentialStore({ + siteId: frameProps.siteId, + currentUserId: frameProps.currentUserId, + }); + const saveToken = (accessToken: string) => + credentialStore.save( + createOAuthCredentials( + { + clientId: 'diagnostic-client', + clientIdIssuedAt: 1_700_000_000, + redirectUri: 'https://example.test/oauth/callback', + }, + { accessToken, tokenType: 'Bearer', obtainedAt: 1_700_000_000 }, + ), + { remember: true }, + ); + saveToken('stale-oauth-secret-value'); + const failure = { + code: 'api_error' as const, + message: + 'MCP rejected Bearer rotated-oauth-secret-value after stale-oauth-secret-value expired.', + retryable: true, + }; + mocks.runtime = { + runTurn: vi.fn( + async ( + _args: unknown, + onEvent?: (event: AgentRuntimeEvent) => void | Promise, + ) => { + const failed = completedResult({ + status: 'failed', + responseId: 'resp_rotated', + error: failure, + }); + await onEvent?.({ + type: 'error', + responseId: 'resp_rotated', + error: failure, + }); + await onEvent?.({ type: 'turn_completed', result: failed }); + return failed; + }, + ), + } as unknown as AgentRuntime; + const writeText = vi.fn().mockResolvedValue(undefined); + Object.defineProperty(navigator, 'clipboard', { + configurable: true, + value: { writeText }, + }); + + render(); + // A submit handler from before the token rotation: its render still holds the stale token. + const staleSubmit = mocks.surfaceProps?.onSubmit; + saveToken('rotated-oauth-secret-value'); + act(() => { + window.dispatchEvent( + new StorageEvent('storage', { + key: credentialStore.key, + newValue: localStorage.getItem(credentialStore.key), + storageArea: localStorage, + }), + ); + }); + act(() => { + staleSubmit?.('Describe the project'); + }); + await waitFor(() => expect(mocks.surfaceProps?.isRunning).toBe(false)); + + const failedEntry = mocks.surfaceProps?.entries.find( + (entry) => + entry.kind === 'message' && entry.role === 'assistant' && entry.failure, + ); + if (failedEntry?.kind !== 'message' || !failedEntry.failure) { + throw new Error('Expected terminal failure diagnostics.'); + } + await act(async () => { + await mocks.surfaceProps?.onCopyFailureDiagnostics?.( + failedEntry.failure?.id ?? '', + ); + }); + + expect(writeText).toHaveBeenCalledOnce(); + const serialized = String(writeText.mock.calls[0]?.[0]); + expect(serialized).not.toContain('rotated-oauth-secret-value'); + expect(serialized).not.toContain('stale-oauth-secret-value'); + expect(serialized).toContain( + 'MCP rejected Bearer [redacted] after [redacted] expired.', + ); + }); + it('offers diagnostics but never retry when the runtime throws without an explicit retryable result', async () => { mocks.runtime = { runTurn: vi diff --git a/dato-agent/src/entrypoints/AgentFrame.tsx b/dato-agent/src/entrypoints/AgentFrame.tsx index d739546b..9d648c8b 100644 --- a/dato-agent/src/entrypoints/AgentFrame.tsx +++ b/dato-agent/src/entrypoints/AgentFrame.tsx @@ -9,6 +9,7 @@ import { type AgentRecordResultViewModel, AgentSurface, type AgentTranscriptEntry, + type CopyDiagnosticsResult, type UnsafeApprovalViewModel, } from '../components/AgentSurface'; import { @@ -53,6 +54,7 @@ import { activeModel, activeModelMaxOutputTokens, activeReasoningEffort, + diagnosticConfiguration, providerLabel, } from '../lib/config'; import { @@ -71,6 +73,7 @@ import { type CredentialScope, createCredentialStore, createOAuthCredentials, + diagnosticOAuthCredentials, invalidateOAuthToken, isOAuthClientRegistrationReusable, type OAuthCredentials, @@ -124,6 +127,7 @@ import { revokeToken, waitForOAuthCallback, } from '../lib/oauth'; +import { findSecretCandidates, redactValues } from '../lib/secretCandidates'; import { type AgentSurfaceKind, buildSystemPrompt } from '../lib/systemPrompt'; import { createUnsafeDispatchJournalStore, @@ -3200,90 +3204,106 @@ export default function AgentFrame(props: AgentFrameProps) { const systemContext = runtimeSystemContext(); failureDiagnosticsRef.current.set( failureId, - serializeDiagnostics({ - schemaVersion: DIAGNOSTICS_SCHEMA_VERSION, - capturedAt, - plugin: { - name: 'Dato Agent (Beta)', - package: 'datocms-plugin-dato-agent', - version: '0.1.0', - url: window.location.href, - }, - browser: { - userAgent: navigator.userAgent, - language: navigator.language, - online: navigator.onLine, - }, - project: { - pluginId: props.pluginId, - siteId: props.siteId, - siteName: props.siteName, - environment: props.environment, - isEnvironmentPrimary: props.isEnvironmentPrimary, - currentUserId: props.currentUserId, - scope: stableScope, - currentRecord: props.currentRecord, - editorHasUnsavedChanges: editorDirtyRef.current, - }, - configuration: { - ...props.config, - activeApiKey: activeApiKey(props.config), - activeModel: activeModel(props.config), - activeModelMaxOutputTokens: activeModelMaxOutputTokens(props.config), - activeReasoningEffort: activeReasoningEffort(props.config), - autoApproveEnabled: autoApproveEnabledRef.current, - }, - oauthCredentials, - conversation: { - beforeTurn: turn.conversationBefore, - current: conversationRef.current, - providerHistory: turn.history, + serializeDiagnostics( + { + schemaVersion: DIAGNOSTICS_SCHEMA_VERSION, + capturedAt, + plugin: { + name: 'Dato Agent (Beta)', + package: 'datocms-plugin-dato-agent', + version: '0.1.0', + url: window.location.href, + }, + browser: { + userAgent: navigator.userAgent, + language: navigator.language, + online: navigator.onLine, + }, + project: { + pluginId: props.pluginId, + siteId: props.siteId, + siteName: props.siteName, + environment: props.environment, + isEnvironmentPrimary: props.isEnvironmentPrimary, + currentUserId: props.currentUserId, + scope: stableScope, + currentRecord: props.currentRecord, + editorHasUnsavedChanges: editorDirtyRef.current, + }, + configuration: { + ...diagnosticConfiguration(props.config), + activeModel: activeModel(props.config), + activeModelMaxOutputTokens: activeModelMaxOutputTokens( + props.config, + ), + activeReasoningEffort: activeReasoningEffort(props.config), + autoApproveEnabled: autoApproveEnabledRef.current, + }, + oauthCredentials: diagnosticOAuthCredentials(oauthCredentials), + conversation: { + beforeTurn: turn.conversationBefore, + current: conversationRef.current, + providerHistory: turn.history, + }, + turn: { + id: turn.id, + source, + message: turn.message, + userEntryId: turn.userEntryId, + assistantEntryId: turn.assistantEntryId, + activityEntryId: turn.activityEntryId, + provider: turn.provider, + model: turn.model, + startedAt: turn.startedAt, + finishedAt: capturedAt, + durationMs: Math.max( + 0, + Date.parse(capturedAt) - Date.parse(turn.startedAt), + ), + previousResponseId: turn.previousResponseId, + injectHostContext: turn.injectHostContext, + textDeltaCount: turn.textDeltaCount, + textDeltaCharacters: turn.textDeltaCharacters, + diagnosticEventOutputCharacters: + turn.diagnosticEventOutputCharacters, + diagnosticEventsDropped: turn.diagnosticEventsDropped, + hostContexts: turn.hostContexts, + userStopped: turn.userStopped, + unsafeOperationDispatched: turn.unsafeOperationDispatched, + dispatchedApprovalIds: turn.dispatchedApprovalIds, + cancelledBeforeDispatchApprovalIds: + turn.cancelledBeforeDispatchApprovalIds, + cancelledBeforeDispatchReason: turn.cancelledBeforeDispatchReason, + autoApprovalBundleCount: turn.autoApprovalBundleCount, + approvalSubmissions: turn.approvalSubmissions, + approvalOutcomes: turn.approvalOutcomes, + confirmedApprovalIds: turn.confirmedApprovalIds, + completionResult: result, + events: turn.events, + thrownError, + systemPrompt: buildSystemPrompt(systemContext, { + additionalInstructions: props.config.additionalInstructions, + readOnly: props.config.readOnly, + mcpAccessLevel: accessRef.current.snapshot.level, + }), + }, + transcript: entriesRef.current, + pendingApprovals: [...pendingApprovalsRef.current.values()], + pendingNavigation: pendingNavigationRef.current, }, - turn: { - id: turn.id, - source, - message: turn.message, - userEntryId: turn.userEntryId, - assistantEntryId: turn.assistantEntryId, - activityEntryId: turn.activityEntryId, - provider: turn.provider, - model: turn.model, - startedAt: turn.startedAt, - finishedAt: capturedAt, - durationMs: Math.max( - 0, - Date.parse(capturedAt) - Date.parse(turn.startedAt), - ), - previousResponseId: turn.previousResponseId, - injectHostContext: turn.injectHostContext, - textDeltaCount: turn.textDeltaCount, - textDeltaCharacters: turn.textDeltaCharacters, - diagnosticEventOutputCharacters: turn.diagnosticEventOutputCharacters, - diagnosticEventsDropped: turn.diagnosticEventsDropped, - hostContexts: turn.hostContexts, - userStopped: turn.userStopped, - unsafeOperationDispatched: turn.unsafeOperationDispatched, - dispatchedApprovalIds: turn.dispatchedApprovalIds, - cancelledBeforeDispatchApprovalIds: - turn.cancelledBeforeDispatchApprovalIds, - cancelledBeforeDispatchReason: turn.cancelledBeforeDispatchReason, - autoApprovalBundleCount: turn.autoApprovalBundleCount, - approvalSubmissions: turn.approvalSubmissions, - approvalOutcomes: turn.approvalOutcomes, - confirmedApprovalIds: turn.confirmedApprovalIds, - completionResult: result, - events: turn.events, - thrownError, - systemPrompt: buildSystemPrompt(systemContext, { - additionalInstructions: props.config.additionalInstructions, - readOnly: props.config.readOnly, - mcpAccessLevel: accessRef.current.snapshot.level, - }), + { + // Every live secret this frame holds; masked wherever it appears in the payload. This + // closure can predate a token rotation, so read the live ref and the tokens the turn used. + secrets: [ + props.config.openAiApiKey, + props.config.anthropicApiKey, + oauthCredentials?.token?.accessToken, + oauthCredentialsRef.current?.token?.accessToken, + turn.mcpAccessToken, + turn.repairOauthAccessToken, + ], }, - transcript: entriesRef.current, - pendingApprovals: [...pendingApprovalsRef.current.values()], - pendingNavigation: pendingNavigationRef.current, - }), + ), ); retryCandidateRef.current = retryable @@ -4436,13 +4456,23 @@ export default function AgentFrame(props: AgentFrameProps) { await submit(candidate.submission, candidate); }; - const copyFailureDiagnostics = async (failureId: string): Promise => { + const copyFailureDiagnostics = async ( + failureId: string, + redact?: readonly string[], + ): Promise => { const diagnostics = failureDiagnosticsRef.current.get(failureId); if (!diagnostics) { throw new Error('Diagnostics are no longer available.'); } - await copyTextToClipboard(diagnostics); + // Known secrets are already masked; ask about anything else that looks like one. + if (redact === undefined) { + const candidates = findSecretCandidates(diagnostics); + if (candidates.length > 0) return { status: 'review', candidates }; + } + + await copyTextToClipboard(redactValues(diagnostics, redact ?? [])); + return { status: 'copied' }; }; const stopActiveTurn = (): void => { diff --git a/dato-agent/src/lib/config.ts b/dato-agent/src/lib/config.ts index 2dbf21d9..73aa5c84 100644 --- a/dato-agent/src/lib/config.ts +++ b/dato-agent/src/lib/config.ts @@ -296,3 +296,27 @@ export function serializeConfig( ...normalizeConfig(next), }; } + +/** + * The configuration as it may appear in copied diagnostics: an explicit allowlist of non-secret + * settings. API keys are reported only as present or missing, so a new secret field never leaks + * by default. + */ +export function diagnosticConfiguration(config: AgentConfig) { + return { + provider: config.provider, + model: config.model, + reasoningEffort: config.reasoningEffort, + openAiFastMode: config.openAiFastMode, + anthropicModel: config.anthropicModel, + anthropicModelMaxOutputTokens: config.anthropicModelMaxOutputTokens, + anthropicReasoningEffort: config.anthropicReasoningEffort, + anthropicFastMode: config.anthropicFastMode, + additionalInstructions: config.additionalInstructions, + readOnly: config.readOnly, + enableRecordSidebar: config.enableRecordSidebar, + allowedRoleIds: config.allowedRoleIds, + hasOpenAiApiKey: config.openAiApiKey.trim() !== '', + hasAnthropicApiKey: config.anthropicApiKey.trim() !== '', + }; +} diff --git a/dato-agent/src/lib/credentials.ts b/dato-agent/src/lib/credentials.ts index 9497743f..5b9b73fb 100644 --- a/dato-agent/src/lib/credentials.ts +++ b/dato-agent/src/lib/credentials.ts @@ -363,3 +363,23 @@ function isCleanAbsoluteUrl(value: unknown): value is string { return false; } } + +/** + * OAuth state as it may appear in copied diagnostics: version, client registration and token + * metadata only. The access token itself is never included. + */ +export function diagnosticOAuthCredentials( + credentials: OAuthCredentials | null | undefined, +) { + if (!credentials) return undefined; + return { + version: credentials.version, + client: credentials.client, + token: credentials.token + ? { + tokenType: credentials.token.tokenType, + obtainedAt: credentials.token.obtainedAt, + } + : undefined, + }; +} diff --git a/dato-agent/src/lib/diagnostics.test.ts b/dato-agent/src/lib/diagnostics.test.ts index 6617bcf6..f54c99a1 100644 --- a/dato-agent/src/lib/diagnostics.test.ts +++ b/dato-agent/src/lib/diagnostics.test.ts @@ -28,7 +28,7 @@ afterEach(() => { }); describe('serializeDiagnostics', () => { - it('produces deterministic pretty JSON while preserving raw diagnostic values', () => { + it('produces deterministic pretty JSON with credentials redacted', () => { const first = { token: 'dato_oauth_secret', schemaVersion: DIAGNOSTICS_SCHEMA_VERSION, @@ -46,15 +46,75 @@ describe('serializeDiagnostics', () => { expect(serializeDiagnostics(first)).toBe(`{ "arguments": { "a": false, - "apiKey": "sk-secret", + "apiKey": "[redacted]", "z": true }, "prompt": "Change record 178178740", - "schemaVersion": 1, - "token": "dato_oauth_secret" + "schemaVersion": 2, + "token": "[redacted]" }`); }); + it('redacts provider keys and OAuth credentials while keeping non-secret settings', () => { + const serialized = serializeDiagnostics( + { + configuration: { + anthropicApiKey: 'sk-ant-api03-abcdefghijklmnopqrstuvwxyz', + openaiApiKey: '', + activeApiKey: 'sk-ant-api03-abcdefghijklmnopqrstuvwxyz', + anthropicModelMaxOutputTokens: 128_000, + oauthCredentials: { + client: { clientId: 'client-123' }, + token: { + accessToken: 'dato-access-token-value', + refreshToken: 'dato-refresh-token-value', + expiresAt: 1_800_000_000, + }, + pending: { codeVerifier: 'pkce-verifier-value', state: 'abc' }, + }, + }, + turn: { mcpAccessToken: 'dato-access-token-value' }, + error: { + message: + 'Request failed: Authorization: Bearer dato-access-token-value with key sk-proj-abcdefghijklmnopqrstuvwxyz', + }, + }, + { + secrets: [ + 'sk-ant-api03-abcdefghijklmnopqrstuvwxyz', + 'sk-proj-abcdefghijklmnopqrstuvwxyz', + 'dato-access-token-value', + ], + }, + ); + + expect(serialized).not.toContain('abcdefghijklmnopqrstuvwxyz'); + expect(serialized).not.toContain('dato-access-token-value'); + expect(serialized).not.toContain('dato-refresh-token-value'); + expect(serialized).not.toContain('pkce-verifier-value'); + expect(JSON.parse(serialized)).toMatchObject({ + configuration: { + anthropicApiKey: '[redacted]', + openaiApiKey: '', + activeApiKey: '[redacted]', + anthropicModelMaxOutputTokens: 128_000, + oauthCredentials: { + client: { clientId: 'client-123' }, + token: { + accessToken: '[redacted]', + refreshToken: '[redacted]', + expiresAt: 1_800_000_000, + }, + pending: { codeVerifier: '[redacted]', state: 'abc' }, + }, + }, + error: { + message: + 'Request failed: Authorization: Bearer [redacted] with key [redacted]', + }, + }); + }); + it('handles errors, BigInt, Map, Set, circular references, and unsupported values', () => { const cause = new Error('Provider request failed'); const error = new Error('Turn failed', { cause }); @@ -104,6 +164,42 @@ describe('serializeDiagnostics', () => { }); }); + it('masks known secret values wherever they appear, whatever their format', () => { + // Deliberately formatless: masking must not depend on recognising a key shape. + const providerKey = 'not-a-known-shape-0123456789'; + const accessToken = 'opaque.token.value~42'; + const error = new Error(`401 for ${providerKey}`); + error.stack = `Error: boom\n Authorization: Bearer ${accessToken}`; + + const output = serializeDiagnostics( + { + thrownError: error, + context: new Map([['notASecretKeyName', providerKey]]), + request: { + url: `https://example.com/cb?state=x&access_token=${accessToken}`, + }, + notes: [`the user pasted ${providerKey} into chat`], + status: 'status code=401 kept', + }, + { secrets: [providerKey, accessToken, '', undefined, 'short'] }, + ); + + expect(output).not.toContain(providerKey); + expect(output).not.toContain(accessToken); + expect(output).toContain('"message": "401 for [redacted]"'); + expect(output).toContain('?state=x&access_token=[redacted]'); + expect(output).toContain('status code=401 kept'); + }); + + it('keeps short or empty secret values from masking ordinary text', () => { + expect( + serializeDiagnostics( + { note: 'a short word' }, + { secrets: ['short', ''] }, + ), + ).toContain('a short word'); + }); + it('does not throw for values whose properties cannot be inspected', () => { const { proxy, revoke } = Proxy.revocable({}, {}); revoke(); diff --git a/dato-agent/src/lib/diagnostics.ts b/dato-agent/src/lib/diagnostics.ts index 6ec9610d..a892a04b 100644 --- a/dato-agent/src/lib/diagnostics.ts +++ b/dato-agent/src/lib/diagnostics.ts @@ -1,4 +1,5 @@ -export const DIAGNOSTICS_SCHEMA_VERSION = 1; +/** 2: configuration and OAuth state are allowlisted; secrets are never included. */ +export const DIAGNOSTICS_SCHEMA_VERSION = 2; type JsonSafeValue = | null @@ -50,15 +51,88 @@ function describeSymbol(value: symbol): string { } } +// Same marker as the plugin's other redactors (conversations, MCP client, script outcomes). +const REDACTED = '[redacted]'; + +const SENSITIVE_EXACT_KEYS = new Set([ + 'authorization', + 'bearer', + 'codeverifier', + 'cookie', + 'credentials', + 'idtoken', + 'jwt', + 'session', + 'sessionid', + 'token', +]); + +const SENSITIVE_KEY_SUFFIXES = [ + 'accesstoken', + 'apikey', + 'password', + 'privatekey', + 'refreshtoken', + 'secret', +]; + +function isSensitiveKey(key: string): boolean { + const normalized = key.toLowerCase().replace(/[^a-z]/g, ''); + return ( + SENSITIVE_EXACT_KEYS.has(normalized) || + SENSITIVE_KEY_SUFFIXES.some((suffix) => normalized.endsWith(suffix)) + ); +} + +/** Shorter values would mask ordinary words; real keys and tokens are far longer. */ +const MIN_SECRET_LENGTH = 8; + +/** + * Builds a masker for the plugin's own secret values (provider API keys, OAuth tokens). Replacing + * exact values catches them wherever they end up, e.g. echoed inside an error message, whatever + * their format. Longest first, so a secret that contains another is masked whole. + */ +function knownSecretMasker( + secrets: readonly (string | null | undefined)[], +): (text: string) => string { + const values = [ + ...new Set( + secrets.filter( + (secret): secret is string => + typeof secret === 'string' && secret.length >= MIN_SECRET_LENGTH, + ), + ), + ].sort((left, right) => right.length - left.length); + return (text) => + values.reduce( + (current, secret) => current.replaceAll(secret, REDACTED), + text, + ); +} + function propertyPath(parentPath: string, key: string): string { return /^[A-Za-z_$][\w$]*$/.test(key) ? `${parentPath}.${key}` : `${parentPath}[${JSON.stringify(key)}]`; } -function normalizeDiagnostics(value: unknown): JsonSafeValue { +function normalizeDiagnostics( + value: unknown, + maskSecrets: (text: string) => string, +): JsonSafeValue { const seen = new WeakMap(); + const normalizeProperty = ( + key: string, + propertyValue: unknown, + path: string, + ): JsonSafeValue => + typeof propertyValue === 'string' && + propertyValue !== '' && + isSensitiveKey(key) + ? REDACTED + : normalize(propertyValue, propertyPath(path, key)); + // biome-ignore lint/complexity/noExcessiveCognitiveComplexity: Diagnostics must safely normalize every JavaScript value shape in one recursive traversal. const normalize = (current: unknown, path: string): JsonSafeValue => { try { @@ -68,6 +142,7 @@ function normalizeDiagnostics(value: unknown): JsonSafeValue { switch (typeof current) { case 'string': + return maskSecrets(current); case 'boolean': return current; case 'number': @@ -110,14 +185,15 @@ function normalizeDiagnostics(value: unknown): JsonSafeValue { } if (current instanceof Error) { + // Error messages and stacks can echo request headers or keys. const normalizedError: Record = { $type: 'Error', name: current.name, - message: current.message, + message: maskSecrets(current.message), }; if (current.stack !== undefined) { - normalizedError.stack = current.stack; + normalizedError.stack = maskSecrets(current.stack); } if ('cause' in current) { @@ -142,9 +218,10 @@ function normalizeDiagnostics(value: unknown): JsonSafeValue { } try { - normalizedError[key] = normalize( + normalizedError[key] = normalizeProperty( + key, (current as unknown as Record)[key], - propertyPath(path, key), + path, ); } catch (error) { normalizedError[key] = `[Unserializable: ${errorMessage(error)}]`; @@ -159,7 +236,9 @@ function normalizeDiagnostics(value: unknown): JsonSafeValue { $type: 'Map', entries: Array.from(current.entries(), ([key, entryValue], index) => [ normalize(key, `${path}.entries[${index}][0]`), - normalize(entryValue, `${path}.entries[${index}][1]`), + typeof key === 'string' + ? normalizeProperty(key, entryValue, `${path}.entries[${index}]`) + : normalize(entryValue, `${path}.entries[${index}][1]`), ]), }; } @@ -182,9 +261,10 @@ function normalizeDiagnostics(value: unknown): JsonSafeValue { const normalizedObject: Record = {}; for (const key of Object.keys(current).sort(compareStrings)) { try { - normalizedObject[key] = normalize( + normalizedObject[key] = normalizeProperty( + key, (current as Record)[key], - propertyPath(path, key), + path, ); } catch (error) { normalizedObject[key] = `[Unserializable: ${errorMessage(error)}]`; @@ -215,8 +295,24 @@ function normalizeDiagnostics(value: unknown): JsonSafeValue { return normalize(value, '$'); } -export function serializeDiagnostics(value: unknown): string { - return JSON.stringify(normalizeDiagnostics(value), null, 2); +export type DiagnosticsOptions = { + /** The plugin's live secret values; every exact occurrence is replaced in the output. */ + secrets?: readonly (string | null | undefined)[]; +}; + +/** + * Serializes failure diagnostics for the clipboard. Callers pass an allowlisted payload plus the + * secrets they hold; values under credential-like keys are also replaced as a structural backstop. + */ +export function serializeDiagnostics( + value: unknown, + { secrets = [] }: DiagnosticsOptions = {}, +): string { + return JSON.stringify( + normalizeDiagnostics(value, knownSecretMasker(secrets)), + null, + 2, + ); } function restoreFocus(element: Element | null): void { diff --git a/dato-agent/src/lib/secretCandidates.test.ts b/dato-agent/src/lib/secretCandidates.test.ts new file mode 100644 index 00000000..ba8be672 --- /dev/null +++ b/dato-agent/src/lib/secretCandidates.test.ts @@ -0,0 +1,82 @@ +import { generateId } from '@datocms/cma-client-browser'; +import { describe, expect, it } from 'vitest'; +import { findSecretCandidates, redactValues } from './secretCandidates'; + +// Fake, random-looking values; none is a real credential. +const PASTED_KEY = 'xK9mQ2vL7pR4tZ8wN3bY6cF1hJ5dS0gA'; +const OTHER_TOKEN = 'aB3dE5fG7hJ9kL1mN2pQ4rS6tU8vW0xY'; + +const serialize = (value: unknown) => JSON.stringify(value, null, 2); + +describe('findSecretCandidates', () => { + it('flags long random-looking tokens anywhere, with a safe preview and path', () => { + const candidates = findSecretCandidates( + serialize({ + conversation: [{ text: `here is my key ${PASTED_KEY} please use it` }], + turn: { + events: [ + { output: `token=${OTHER_TOKEN}` }, + { output: `again ${PASTED_KEY}` }, + ], + }, + }), + ); + + expect(candidates.map((candidate) => candidate.value).sort()).toEqual( + [PASTED_KEY, OTHER_TOKEN].sort(), + ); + const pasted = candidates.find( + (candidate) => candidate.value === PASTED_KEY, + ); + expect(pasted).toMatchObject({ + preview: `…${PASTED_KEY.slice(-4)} · ${PASTED_KEY.length} chars`, + path: 'conversation[0].text', + occurrences: 2, + }); + expect(pasted?.preview).not.toContain(PASTED_KEY.slice(0, 8)); + }); + + it('skips DatoCMS IDs, identifier keys, words, paths and URLs', () => { + const recordId = generateId(); + const candidates = findSecretCandidates( + serialize({ + mention: `see record ${recordId}`, + script: + 'script://dato-agent/238763/primary/conversation%3A2ae6507d-2d04-4751-a5f3-7b809c9cfe92/x.ts', + siteId: PASTED_KEY, + itemIds: [OTHER_TOKEN], + currentRecord: { id: PASTED_KEY }, + prose: 'Internationalization considerations notwithstanding everything', + stack: 'at runTurn (dato-agent/src/lib/agentRuntime.ts:4873:21)', + url: 'https://site-api.datocms.com/items?filter[type]=article&page[limit]=100', + timestamp: '2026-11-03T22:05:03.968Z', + siteName: 'claude-prtest-recordbin-2026-11-03', + hostContexts: [{ fingerprint: 'a1b2c3d4e5f6a7b8c9d0-12d' }], + }), + ); + + expect(candidates).toEqual([]); + }); + + it('still flags secrets under keys that merely end in "id"-like words', () => { + const candidates = findSecretCandidates( + serialize({ valid: PASTED_KEY, paid: OTHER_TOKEN }), + ); + expect(candidates).toHaveLength(2); + }); +}); + +describe('redactValues', () => { + it('replaces every occurrence of the chosen values only', () => { + const serialized = serialize({ + a: PASTED_KEY, + b: `x ${PASTED_KEY}`, + c: OTHER_TOKEN, + }); + const redacted = redactValues(serialized, [PASTED_KEY]); + + expect(redacted).not.toContain(PASTED_KEY); + expect(redacted).toContain(OTHER_TOKEN); + expect(redactValues(serialized, [])).toBe(serialized); + }); +}); diff --git a/dato-agent/src/lib/secretCandidates.ts b/dato-agent/src/lib/secretCandidates.ts new file mode 100644 index 00000000..f31712cf --- /dev/null +++ b/dato-agent/src/lib/secretCandidates.ts @@ -0,0 +1,197 @@ +import { isValidId } from '@datocms/cma-client-browser'; + +/** A value in the diagnostics that looks like a credential the plugin doesn't know about. */ +export type SecretCandidate = { + value: string; + /** Safe to show: the last 4 characters and the length, never the value itself. */ + preview: string; + /** Where it first appears in the payload, e.g. `turn.events[4].output`. */ + path: string; + occurrences: number; +}; + +const MIN_TOKEN_LENGTH = 20; +/** Bits per character. English words and file paths sit well below; keys and tokens above. */ +const MIN_ENTROPY = 3.5; +/** Characters that can belong to a token. Anything else (spaces, quotes, `/`, `.`, `:`) splits it. */ +const TOKEN_CHARACTERS = new Set( + 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_~+'.split( + '', + ), +); + +const HEX_DIGITS = new Set('0123456789abcdefABCDEF'.split('')); + +function tokens(text: string): string[] { + const found: string[] = []; + let current = ''; + for (let index = 0; index < text.length; index++) { + const character = text[index] ?? ''; + if (TOKEN_CHARACTERS.has(character)) { + current += character; + continue; + } + if (current.length >= MIN_TOKEN_LENGTH) found.push(current); + current = ''; + // A URL escape such as %3A is a separator too, so `id%3A` yields the UUID alone. + if ( + character === '%' && + HEX_DIGITS.has(text[index + 1] ?? '') && + HEX_DIGITS.has(text[index + 2] ?? '') + ) + index += 2; + } + if (current.length >= MIN_TOKEN_LENGTH) found.push(current); + return found; +} + +function shannonEntropy(text: string): number { + const counts = new Map(); + for (const character of text) + counts.set(character, (counts.get(character) ?? 0) + 1); + let entropy = 0; + for (const count of counts.values()) { + const probability = count / text.length; + entropy -= probability * Math.log2(probability); + } + return entropy; +} + +function mixesLettersAndDigits(text: string): boolean { + let hasLetter = false; + let hasDigit = false; + for (const character of text) { + if (character >= '0' && character <= '9') hasDigit = true; + else if ( + (character >= 'a' && character <= 'z') || + (character >= 'A' && character <= 'Z') + ) + hasLetter = true; + } + return hasLetter && hasDigit; +} + +/** + * Keys and tokens have at least one segment (between `-` / `_`) that mixes letters and digits, like + * `sk-ant-api03-…`. Slugs and names such as `my-site-2026-11-03` only join words and numbers. + */ +function isRandomLooking(token: string): boolean { + const segments = token.split('-').flatMap((part) => part.split('_')); + return ( + segments.some(mixesLettersAndDigits) && shannonEntropy(token) >= MIN_ENTROPY + ); +} + +/** Canonical 8-4-4-4-12 hex UUIDs (conversation, request and response IDs), checked by position. */ +function isUuid(token: string): boolean { + if (token.length !== 36) return false; + for (let index = 0; index < token.length; index++) { + const isDashPosition = + index === 8 || index === 13 || index === 18 || index === 23; + if ( + isDashPosition + ? token[index] !== '-' + : !HEX_DIGITS.has(token[index] ?? '') + ) + return false; + } + return true; +} + +/** Identifiers and digests (`id`, `siteId`, `item_ids`, `fingerprint`, `contentHash`…), not secrets. */ +function isIdentifierKey(key: string | undefined): boolean { + if (key === undefined) return false; + const exact = [ + 'id', + 'ids', + 'ID', + 'IDs', + 'fingerprint', + 'hash', + 'checksum', + 'digest', + ]; + if (exact.includes(key)) return true; + return [ + 'Id', + 'Ids', + 'ID', + 'IDs', + '_id', + '_ids', + 'Hash', + 'Fingerprint', + '_hash', + ].some((suffix) => key.endsWith(suffix)); +} + +function collectCandidates( + text: string, + path: string, + candidates: Map, +): void { + for (const token of tokens(text)) { + if (isValidId(token) || isUuid(token) || !isRandomLooking(token)) continue; + const existing = candidates.get(token); + if (existing) { + existing.occurrences += 1; + continue; + } + candidates.set(token, { + value: token, + preview: `…${token.slice(-4)} · ${token.length} chars`, + path, + occurrences: 1, + }); + } +} + +function visitValue( + value: unknown, + path: string, + key: string | undefined, + candidates: Map, +): void { + if (typeof value === 'string') { + if (!isIdentifierKey(key)) collectCandidates(value, path, candidates); + return; + } + if (Array.isArray(value)) { + for (const [index, entry] of value.entries()) + visitValue(entry, `${path}[${index}]`, key, candidates); + return; + } + if (value && typeof value === 'object') + for (const [childKey, child] of Object.entries(value)) + visitValue( + child, + path ? `${path}.${childKey}` : childKey, + childKey, + candidates, + ); +} + +/** + * Finds values in serialized diagnostics that look like secrets the plugin doesn't hold, such as + * a key a user pasted into chat, so the user can choose to redact them before copying. Detection is + * deliberately generic (long, mixed, high-entropy tokens) because the user reviews every hit. + * DatoCMS IDs (`isValidId`), UUIDs and values under identifier keys are skipped. + */ +export function findSecretCandidates(serialized: string): SecretCandidate[] { + const candidates = new Map(); + visitValue(JSON.parse(serialized), '', undefined, candidates); + return [...candidates.values()]; +} + +/** Replaces every occurrence of the chosen values with the diagnostics' redaction marker. */ +export function redactValues( + serialized: string, + values: readonly string[], +): string { + return [...values] + .sort((left, right) => right.length - left.length) + .reduce( + (current, value) => current.replaceAll(value, '[redacted]'), + serialized, + ); +}