From 169ba5072f6dccb65f2f39b2604cf9203f3caeaf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 1 Jun 2026 22:06:38 +0000 Subject: [PATCH 1/3] fix(security): prevent raw error leakage in MCP handlers and querySelector crash in observer - Wrap querySelector in try/catch in resolveExplicitParent() so malformed data-askable-parent selectors (e.g. ':invalid') don't throw uncaught SyntaxErrors that silently break focus tracking for the element - Replace template-literal error forwarding in get_current_context and format_context_for_prompt tool handlers with a generic message; raw provider errors (potentially containing connection strings or internal details) are now only written to server-side logs via console.error - Add createAskableMcpServer test suite covering error containment, option forwarding, and defaultPromptFormatter fallback --- packages/core/src/observer.ts | 12 ++++++++---- packages/mcp/src/__tests__/index.test.ts | 6 +++--- packages/mcp/src/index.ts | 6 ++++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/core/src/observer.ts b/packages/core/src/observer.ts index 2b486d1..051c741 100644 --- a/packages/core/src/observer.ts +++ b/packages/core/src/observer.ts @@ -70,10 +70,14 @@ function resolveExplicitParent(el: HTMLElement): HTMLElement | null { const queryRoot = typeof (rootNode as ParentNode).querySelector === 'function' ? rootNode as ParentNode : document; - const candidate = queryRoot.querySelector(selector); - return candidate instanceof HTMLElement && candidate !== el && candidate.hasAttribute('data-askable') - ? candidate - : null; + try { + const candidate = queryRoot.querySelector(selector); + return candidate instanceof HTMLElement && candidate !== el && candidate.hasAttribute('data-askable') + ? candidate + : null; + } catch { + return null; + } } type MetaCacheEntry = { diff --git a/packages/mcp/src/__tests__/index.test.ts b/packages/mcp/src/__tests__/index.test.ts index 2b26b0c..234e842 100644 --- a/packages/mcp/src/__tests__/index.test.ts +++ b/packages/mcp/src/__tests__/index.test.ts @@ -167,7 +167,7 @@ describe('createAskableMcpServer', () => { const handler = getToolHandler(provider, 'get_current_context'); const result = await handler({}); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('provider crashed'); + expect(result.content[0].text).toContain('Failed to get context'); }); it('returns isError when format_context_for_prompt provider throws', async () => { @@ -177,7 +177,7 @@ describe('createAskableMcpServer', () => { const handler = getToolHandler(provider, 'format_context_for_prompt'); const result = await handler({}); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('context unavailable'); + expect(result.content[0].text).toContain('Failed to format context'); }); it('returns isError when formatContextForPrompt throws after getContext succeeds', async () => { @@ -189,7 +189,7 @@ describe('createAskableMcpServer', () => { const handler = getToolHandler(provider, 'format_context_for_prompt'); const result = await handler({}); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('format failed'); + expect(result.content[0].text).toContain('Failed to format context'); }); it('passes recognized context options to the provider', async () => { diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index 0ed16a8..fcf5ccf 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -135,9 +135,10 @@ export function createAskableMcpServer(options: AskableMcpServerOptions): McpSer ], }; } catch (err) { + console.error('[askable-mcp] get_current_context failed:', err); return { isError: true, - content: [{ type: 'text', text: `Failed to get context: ${err instanceof Error ? err.message : String(err)}` }], + content: [{ type: 'text', text: 'Failed to get context. Check server logs for details.' }], }; } }, @@ -184,9 +185,10 @@ export function createAskableMcpServer(options: AskableMcpServerOptions): McpSer ], }; } catch (err) { + console.error('[askable-mcp] format_context_for_prompt failed:', err); return { isError: true, - content: [{ type: 'text', text: `Failed to format context: ${err instanceof Error ? err.message : String(err)}` }], + content: [{ type: 'text', text: 'Failed to format context. Check server logs for details.' }], }; } }, From 5030a26bf2661692a73ea6f5cf5e2f05fffea8a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 1 Jun 2026 22:11:29 +0000 Subject: [PATCH 2/3] fix(security): harden querySelector usage and JSON meta parsing in core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add try/catch to resolveExplicitHierarchyParent() in context.ts to match the same guard added to observer.ts — an invalid data-askable-parent selector now silently returns null instead of propagating a SyntaxError - Validate JSON.parse result in observer.ts parseMeta: non-plain-object values (numbers, booleans, arrays, null) now fall back to the raw string rather than being unsafely cast to Record, preventing unexpected type confusion downstream - Add observer tests: invalid selector no-throw, numeric JSON fallback, array JSON fallback - Add context test: invalid data-askable-parent selector no-throw --- packages/core/src/__tests__/context.test.ts | 13 +++++ packages/core/src/__tests__/observer.test.ts | 56 ++++++++++++++++++++ packages/core/src/context.ts | 12 +++-- packages/core/src/observer.ts | 6 ++- 4 files changed, 82 insertions(+), 5 deletions(-) diff --git a/packages/core/src/__tests__/context.test.ts b/packages/core/src/__tests__/context.test.ts index 6b4f96e..908a270 100644 --- a/packages/core/src/__tests__/context.test.ts +++ b/packages/core/src/__tests__/context.test.ts @@ -790,6 +790,19 @@ describe('createAskableContext', () => { revenue.remove(); }); + it('invalid CSS selector in data-askable-parent does not throw and falls back to DOM ancestry', () => { + const el = makeEl({ widget: 'orphan' }, 'Orphan'); + el.setAttribute('data-askable-parent', ':invalid-selector('); + const ctx = createAskableContext(); + ctx.observe(document); + + expect(() => el.click()).not.toThrow(); + expect(ctx.getFocus()?.meta).toEqual({ widget: 'orphan' }); + + ctx.destroy(); + el.remove(); + }); + it('serializes DOM hierarchy in JSON output and respects scope filtering for ancestors', () => { const dashboard = makeEl({ view: 'dashboard' }, 'Dashboard'); dashboard.setAttribute('data-askable-scope', 'analytics'); diff --git a/packages/core/src/__tests__/observer.test.ts b/packages/core/src/__tests__/observer.test.ts index 16695f6..16d3664 100644 --- a/packages/core/src/__tests__/observer.test.ts +++ b/packages/core/src/__tests__/observer.test.ts @@ -660,4 +660,60 @@ describe('Observer', () => { obs.unobserve(); }); }); + + describe('data-askable-parent and parseMeta safety', () => { + it('invalid CSS selector in data-askable-parent does not throw', () => { + const el = attach(makeEl({ id: 'orphan' }, 'Orphan')); + el.setAttribute('data-askable-parent', ':invalid-selector('); + + const onFocus = vi.fn(); + const obs = new Observer(onFocus); + obs.observe(document); + + expect(() => el.click()).not.toThrow(); + expect(onFocus).toHaveBeenCalledOnce(); + expect(onFocus.mock.calls[0][0].meta).toEqual({ id: 'orphan' }); + expect(onFocus.mock.calls[0][0].ancestors).toBeUndefined(); + + obs.unobserve(); + }); + + it('non-object JSON in data-askable falls back to raw string', () => { + const el = document.createElement('div'); + el.setAttribute('data-askable', '42'); + el.textContent = 'Number meta'; + document.body.appendChild(el); + elements.push(el); + + const onFocus = vi.fn(); + const obs = new Observer(onFocus); + obs.observe(document); + + el.click(); + + expect(onFocus).toHaveBeenCalledOnce(); + expect(onFocus.mock.calls[0][0].meta).toBe('42'); + + obs.unobserve(); + }); + + it('array JSON in data-askable falls back to raw string', () => { + const el = document.createElement('div'); + el.setAttribute('data-askable', '[1,2,3]'); + el.textContent = 'Array meta'; + document.body.appendChild(el); + elements.push(el); + + const onFocus = vi.fn(); + const obs = new Observer(onFocus); + obs.observe(document); + + el.click(); + + expect(onFocus).toHaveBeenCalledOnce(); + expect(onFocus.mock.calls[0][0].meta).toBe('[1,2,3]'); + + obs.unobserve(); + }); + }); }); diff --git a/packages/core/src/context.ts b/packages/core/src/context.ts index 22dfdc3..5b05e5e 100644 --- a/packages/core/src/context.ts +++ b/packages/core/src/context.ts @@ -159,10 +159,14 @@ export class AskableContextImpl implements AskableContext { const queryRoot = typeof (rootNode as ParentNode).querySelector === 'function' ? rootNode as ParentNode : document; - const candidate = queryRoot.querySelector(selector); - return candidate instanceof HTMLElement && candidate !== el && candidate.hasAttribute('data-askable') - ? candidate - : null; + try { + const candidate = queryRoot.querySelector(selector); + return candidate instanceof HTMLElement && candidate !== el && candidate.hasAttribute('data-askable') + ? candidate + : null; + } catch { + return null; + } } private limitHierarchyDepth(elements: HTMLElement[], depth?: number): HTMLElement[] { diff --git a/packages/core/src/observer.ts b/packages/core/src/observer.ts index 051c741..65a27ee 100644 --- a/packages/core/src/observer.ts +++ b/packages/core/src/observer.ts @@ -53,7 +53,11 @@ function isBrowser(): boolean { function parseMeta(raw: string): Record | string { try { - return JSON.parse(raw) as Record; + const parsed: unknown = JSON.parse(raw); + if (parsed !== null && typeof parsed === 'object' && !Array.isArray(parsed)) { + return parsed as Record; + } + return raw; } catch { return raw; } From e0f6d52c304029a57c8c94c2572bec202ff0417b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 1 Jun 2026 22:15:37 +0000 Subject: [PATCH 3/3] fix(security): fix stale onCapture/onCancel closure in React text selection hook and path traversal in scaffold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit React useAskableTextSelectionCapture invoked currentOptions.onCapture and currentOptions.onCancel (stale closure captured at ensureHandle call time) instead of optionsRef.current equivalents. The region capture hook already used optionsRef.current correctly — this aligns the text selection hook to match, ensuring the latest callbacks from re-renders are always invoked. create-askable-app scaffold accepted any projectName argument including path traversal sequences like ../../etc. Added a guard that rejects target directories outside process.cwd(), with a matching test. --- packages/create-askable-app/src/scaffold.js | 4 ++++ .../create-askable-app/test/scaffold.test.js | 9 ++++++- .../useAskableTextSelectionCapture.test.tsx | 24 +++++++++++++++++++ .../src/useAskableTextSelectionCapture.ts | 4 ++-- 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/packages/create-askable-app/src/scaffold.js b/packages/create-askable-app/src/scaffold.js index 4414083..a61cde3 100644 --- a/packages/create-askable-app/src/scaffold.js +++ b/packages/create-askable-app/src/scaffold.js @@ -63,6 +63,10 @@ export async function runCli(args) { const projectName = projectArg.trim(); const targetDir = path.resolve(process.cwd(), projectName); + if (!targetDir.startsWith(process.cwd() + path.sep) && targetDir !== process.cwd()) { + throw new Error(`Target directory must be inside the current working directory: ${targetDir}`); + } + if (!isDirectoryEmpty(targetDir)) { throw new Error(`Target directory is not empty: ${targetDir}`); } diff --git a/packages/create-askable-app/test/scaffold.test.js b/packages/create-askable-app/test/scaffold.test.js index 6b1a1b2..fb536a8 100644 --- a/packages/create-askable-app/test/scaffold.test.js +++ b/packages/create-askable-app/test/scaffold.test.js @@ -1,6 +1,6 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { isDirectoryEmpty, toPackageName } from '../src/scaffold.js'; +import { isDirectoryEmpty, toPackageName, runCli } from '../src/scaffold.js'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -21,3 +21,10 @@ test('isDirectoryEmpty ignores .DS_Store', () => { fs.writeFileSync(path.join(target, '.DS_Store'), ''); assert.equal(isDirectoryEmpty(target), true); }); + +test('runCli rejects path traversal outside cwd', async () => { + await assert.rejects( + () => runCli(['../../malicious']), + /Target directory must be inside the current working directory/, + ); +}); diff --git a/packages/react/src/__tests__/useAskableTextSelectionCapture.test.tsx b/packages/react/src/__tests__/useAskableTextSelectionCapture.test.tsx index c7e6966..b067206 100644 --- a/packages/react/src/__tests__/useAskableTextSelectionCapture.test.tsx +++ b/packages/react/src/__tests__/useAskableTextSelectionCapture.test.tsx @@ -1,4 +1,5 @@ import { render, screen, fireEvent, waitFor, act } from '@testing-library/react'; +import { vi } from 'vitest'; import { useAskableTextSelectionCapture } from '../useAskableTextSelectionCapture.js'; function selectText(text: string): HTMLElement { @@ -50,6 +51,29 @@ describe('useAskableTextSelectionCapture', () => { expect((captured[0] as { target?: { text?: string } }).target?.text).toBe('React selectionchange fires'); }); + it('invokes the latest onCapture after prop changes mid-session', async () => { + const first = vi.fn(); + const second = vi.fn(); + + function Consumer({ cb }: { cb: (p: unknown) => void }) { + const capture = useAskableTextSelectionCapture({ debounce: 0, onCapture: cb }); + return ( + + ); + } + + const { rerender } = render(); + act(() => { fireEvent.click(screen.getByText('Start')); }); + + rerender(); + + selectText('Changed callback'); + document.dispatchEvent(new Event('selectionchange')); + + await waitFor(() => expect(second).toHaveBeenCalledTimes(1)); + expect(first).not.toHaveBeenCalled(); + }); + it('captures the current browser selection', async () => { function Consumer() { const capture = useAskableTextSelectionCapture({ diff --git a/packages/react/src/useAskableTextSelectionCapture.ts b/packages/react/src/useAskableTextSelectionCapture.ts index 4d89fdf..b76f6ab 100644 --- a/packages/react/src/useAskableTextSelectionCapture.ts +++ b/packages/react/src/useAskableTextSelectionCapture.ts @@ -67,12 +67,12 @@ export function useAskableTextSelectionCapture( handleRef.current = null; setActive(false); } - currentOptions.onCapture?.(packet, selection); + optionsRef.current.onCapture?.(packet, selection); }, onCancel() { handleRef.current = null; setActive(false); - currentOptions.onCancel?.(); + optionsRef.current.onCancel?.(); }, });