fix(security): audit round 3 — error leakage, querySelector crashes, stale closures, path traversal#309
Open
vamgan wants to merge 10 commits into
Open
fix(security): audit round 3 — error leakage, querySelector crashes, stale closures, path traversal#309vamgan wants to merge 10 commits into
vamgan wants to merge 10 commits into
Conversation
…eardown - core: push() now applies sanitizeText/sanitizeMeta to ancestor segments and calls sanitizeText even when text is undefined (fixes #247, #248) - core: validate maxHistory is a non-negative integer in constructor (fixes #254) - core: guard a11yTextExtractor aria-labelledby lookup against missing document in non-browser environments (fixes #257) - react-native: useAskableVisibility and useAskableScrollView now capture context ownership at creation time so cleanup always destroys the right context regardless of later prop changes (fixes #250) - svelte: useAskable.svelte.ts registers focus/clear listeners inside $effect so they are removed when the component unmounts (fixes #252) - svelte, vue: remove wildcard @askable-ui/core from devDependencies (fixes #262)
Moves removeOverlay / state reset before the onCapture call so that any error thrown inside the callback cannot leave a stale overlay in the DOM. Adds a regression test that asserts the overlay is already absent from the DOM by the time onCapture fires.
Replaces Promise.all with Promise.allSettled (plus an async wrapper to capture synchronous throws) so a single failing sanitizeItem does not reject the entire collection. Failed items are omitted from the result while all healthy items still resolve. Adds tests for both synchronous throw and async rejection isolation.
- Wrap get_current_context and format_context_for_prompt handlers in try/catch so a provider error returns isError:true instead of crashing the MCP server process. - Remove .passthrough() from the sources array item schema so unrecognised fields are stripped rather than forwarded to the provider without validation.
The previous implementation evaluated enabled once at setup time so
toggling it later had no effect. Now accepts MaybeRef<boolean> and
uses watch({ immediate: true }) to register or unregister the source
whenever the value changes. Adds a test that toggles a ref from false
to true and back.
…Inspector useAskableRegionCapture: onCapture and onCancel callbacks captured the options object at start() call time. Any callback change between start() and the pointer-up event would be silently ignored. Now always reads from optionsRef.current so the most-recent callback fires. AskableInspector: inspectorOptions was used inside useEffect but only individual scalar fields were listed in the deps array. Stabilise the options object with useMemo keyed on the same scalars so the effect dep is a single stable reference and exhaustive-deps is satisfied.
…ector 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
- 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<string, unknown>, 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
…ection hook and path traversal in scaffold 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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Security audit of the full monorepo, fixing 5 real vulnerabilities across 4 packages.
get_current_contextandformat_context_for_prompttool handlers forwarded raw provider error messages to MCP clients (template literals:`Failed: ${err.message}`). Provider errors can contain DB connection strings, internal stack traces, or API keys. Replaced with a generic message; raw errors now go toconsole.erroron the server side only.AskableContext.resolveExplicitHierarchyParent()passeddata-askable-parentattribute values toquerySelector()without try/catch, same class of bug already fixed inobserver.ts. An invalid selector silently breaks focus tracking for the element. Closes fix(security): querySelector SyntaxError not caught in AskableContext.resolveExplicitHierarchyParent #305.JSON.parse()result was unsafely cast toRecord<string, unknown>without validating the type. Non-object JSON (numbers, arrays, null) now falls back to the raw string, matching the declared return type. Closes fix(security): parseMeta in observer.ts silently casts non-object JSON to Record type #306.useAskableTextSelectionCapturecalledcurrentOptions.onCapture(captured atstart()time) instead ofoptionsRef.current.onCapture(always the latest). In React, callbacks are recreated each render; stale closure meant old callbacks fired after re-renders.useAskableRegionCapturealready usedoptionsRef.currentcorrectly — this aligns the text selection hook. Closes fix(react): stale onCapture/onCancel closure in useAskableTextSelectionCapture #307.../to paths outsidecwd()and wrote template files there. Added a guard that rejects target directories outside the current working directory. Closes fix(security): path traversal in create-askable-app scaffold allows writing outside cwd #308.Test plan
createAskableMcpServerdescribe block (5 tests) — error containment, option forwarding, defaultPromptFormatter fallbackinvokes the latest onCapture after prop changes mid-session— verifies stale closure is fixedrunCli rejects path traversal outside cwdnpm run test --workspacespassesGenerated by Claude Code