Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions packages/core/src/__tests__/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
56 changes: 56 additions & 0 deletions packages/core/src/__tests__/observer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});
12 changes: 8 additions & 4 deletions packages/core/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] {
Expand Down
18 changes: 13 additions & 5 deletions packages/core/src/observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ function isBrowser(): boolean {

function parseMeta(raw: string): Record<string, unknown> | string {
try {
return JSON.parse(raw) as Record<string, unknown>;
const parsed: unknown = JSON.parse(raw);
if (parsed !== null && typeof parsed === 'object' && !Array.isArray(parsed)) {
return parsed as Record<string, unknown>;
}
return raw;
} catch {
return raw;
}
Expand All @@ -70,10 +74,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 = {
Expand Down
4 changes: 4 additions & 0 deletions packages/create-askable-app/src/scaffold.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
Expand Down
9 changes: 8 additions & 1 deletion packages/create-askable-app/test/scaffold.test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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/,
);
});
6 changes: 3 additions & 3 deletions packages/mcp/src/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
6 changes: 4 additions & 2 deletions packages/mcp/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.' }],
};
}
},
Expand Down Expand Up @@ -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.' }],
};
}
},
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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 (
<button type="button" onClick={() => capture.start()}>Start</button>
);
}

const { rerender } = render(<Consumer cb={first} />);
act(() => { fireEvent.click(screen.getByText('Start')); });

rerender(<Consumer cb={second} />);

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({
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/useAskableTextSelectionCapture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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?.();
},
});

Expand Down
Loading