4 Commits

Author	SHA1	Message	Date
Michael Ramos	356b628b6f	ci(security): add Semgrep CE and Trivy monitoring (#1294) ... * ci(security): add Semgrep CE and Trivy monitoring * fix(ci): diagnose Trivy coverage assertions * fix(ci): accept Trivy repository scan metadata * fix(ci): harden scanner failure diagnostics	2026-08-12 20:40:08 -07:00
Raúl	9389a8c543	feat(ui): render markdown reference links (#1168) ... * feat(ui): render markdown reference links The simplified markdown parser only understood inline links `[text](url)`, so CommonMark reference links rendered as raw text: `[text][id]` and the `[id]: url` definition both showed literally (#923). Add `resolveReferenceLinks`, a pure pass run at the top of `parseMarkdownToBlocks` that rewrites full (`[text][id]`), collapsed (`[text][]`), and shortcut (`[text]`) references, plus their image forms, into inline `[text](url)` links, so the existing inline renderer draws them. Link reference definitions are collected first (first definition wins, labels matched case-insensitively with collapsed whitespace, `<url>` and quoted-title forms supported) and then blanked in place, so a definition never renders and every block keeps its original source line number. Resolution is code-aware: references and definitions inside fenced code blocks and inline code spans are left verbatim, a shortcut is skipped when an inline `(...)` destination follows it or when it is a task-list checkbox marker at the start of a list item, and an unknown reference stays literal so bracketed prose like `[TODO]` or `[0]` never becomes a false link. A definition-shaped line is only collected when it can start a block (after a blank line, a code fence, another definition, or the document start), so a `[word]: token` line that continues a paragraph is left as text rather than deleted (CommonMark: a definition cannot interrupt a paragraph). A document with no definitions is returned unchanged. * fix(ui): protect code/HTML/footnotes and quadratic risk in reference-link resolution Owner review round for reference-style link resolution (#923): - Fence detection now mirrors the block parser's own naive rule exactly (full .trim() + startsWith('```'), any indentation, backtick-only — no ~~~ support) instead of a looser 0-3-space approximation, so indented and list-nested fences the block parser treats as code can never be rewritten. Aligns tilde-fence behavior the same way: since the block parser has no ~~~ support, the resolver no longer protects ~~~ blocks either. - Raw HTML blocks (<details>, <pre>, etc.) are now protected using the same HTML_BLOCK_TAGS/HTML_BLOCK_OPEN_RE/VOID_HTML_TAGS the block parser itself uses, with the same three termination rules (blank-line, void single-line, balanced-depth). - GFM footnote definitions ([^label]: ...) are excluded from collection entirely, so they and their [^label] references are never rewritten into inline links. - A definition-shaped line is now only blanked when its label was actually consumed by a resolved reference outside a protected region. Unused definitions, and definitions referenced only from inside code/HTML, stay visible. This also fixes a plan-diff bug: a URL-only edit to a definition line used to blank to nothing on both sides of the diff (a real change rendering as empty); now the isolated diff chunk keeps the definition visible and diffs normally. - CRLF lines are now recognized (definition regex tolerates a trailing \r) and preserved (a blanked line keeps its own \r). - Bound the label/text capture groups (999 chars, CommonMark's own label limit) and the code-span alternative (5000 chars) so a long run of unmatched brackets/backticks can no longer cause quadratic backtracking within the 2MB annotate cap; added a defense-in-depth cap on the number of definitions tracked per document. - Added coverage for nested brackets, backslash-escaped brackets, parenthesized destinations, idempotence, and confirmed dangerous destinations still flow through the existing sanitizeLinkUrl path unchanged. Added a migration-caveat note to the existing annotation-anchor section of packages/ui/HANDOFF.md: documents using reference-style links render differently now, which can shift position-based anchors captured before a host upgrades past this change. * fix(ui): bound HTML-block extent scan to kill quadratic unclosed-opener case markProtectedLines and parseMarkdownToBlocks each independently scanned line-by-line from a multi-line HTML opener until its balanced open/close depth returned to zero, giving up only at end-of-document. That scan never advanced the outer index on failure, so a document with many consecutive unclosed openers (e.g. thousands of bare <div> lines with no </div> anywhere) made every one of them re-run the same O(N) tail scan — O(N^2) total, a real hazard well within the 2MB annotate cap. Extract the scan into one shared helper, findHtmlBlockEnd, used by both call sites so they can't drift apart: - closeExistsFromLine lazily builds (once per tag name, cached per document) a suffix array answering whether a closing tag exists at or after a given line, so an opener that can never close is rejected in O(1) instead of scanning to EOF. - MAX_HTML_BLOCK_SCAN_LINES bounds the residual case (a closing tag exists far away but depth never actually reaches zero before it) to a constant amount of work per start position — a documented, safe degradation: a block whose true close sits beyond the cap is treated as unclosed, identically to today's 'no close ever found' case. Added a failing-before-fix perf test (many unclosed <div> lines took ~2.3-3.4s and blew a 800ms bound; now ~12-15ms) for both parseMarkdownToBlocks and resolveReferenceLinks, plus a parity test proving a real <details>...</details> block stays intact and identically protected/parsed among thousands of decoy unclosed <div> lines. * fix(ui): remove HTML-block scan cap that truncated valid long blocks MAX_HTML_BLOCK_SCAN_LINES (2000) fixed the O(N^2) unclosed-opener case but as a side effect also truncated genuinely valid, longer HTML blocks: a <details> or raw <table> block whose closing tag sits beyond 2000 lines got cut off mid-block, with its remaining content and the real closing tag falling through as separate, incorrect blocks. closeExistsFromLine already rejects an opener that can never close in O(1) (no closing tag anywhere in the document) without scanning a single line — that already eliminates the pathological 'many failing scans' case on its own. A cap on top of that only ever hurt the opposite case: a scan that DOES succeed, which happens once per document and costs O(L) for an L-line block exactly like reading any other block's content once. So the cap bought nothing further and could silently corrupt valid parsing for any block longer than it, however generous its value. Removed it; the scan now runs unbounded to its real end once closeExistsFromLine confirms a close exists at all. findHtmlBlockEnd is still the single shared implementation used by both markProtectedLines and parseMarkdownToBlocks, so both stay in parity. Added regression tests: a >2000-line <details> and a >2000-line raw <table> block each stay one whole html block (previously truncated); a link definition inside a >2000-line <details> block stays protected and never wins over a real definition outside it; a valid long <details> block survives even preceded by thousands of unclosed <div> decoys. Re-verified the 40k-unclosed-opener perf/parity case (already handled by closeExistsFromLine alone) stays fast and unaffected. * fix(ui): replace HTML-block close scan with a linear prefix-sum index Removing the fixed line-count cap fixed truncation of valid long HTML blocks, but reopened a closely related O(N^2) case: N unclosed <div> openers followed by a single trailing </div> all still pass the 'does a close exist anywhere' pre-check, so every one of them independently scanned forward (mostly to end-of-document) before giving up. Measured before this fix: 5000 openers ~1.0s, 10000 ~4.1s, 40000 timed out past 69s. Replaced the scan entirely with a per-tag-name prefix-sum index (buildTagCloseIndex): the running open-minus-close count for a tag name, plus a classic 'next element at or below this one' index over that prefix sum (an O(N) monotonic-stack construction, each position pushed/popped at most once). Finding where (if anywhere) a block starting at a given line closes is exactly that classic query, so it is now an O(1) lookup with zero scanning per opener, whether the block never closes, closes after 3 lines, or closes 3000 lines away. Both markProtectedLines and parseMarkdownToBlocks still share the single findHtmlBlockEnd implementation, so they stay in parity. 40000 unclosed <div> openers + one trailing </div> now resolve in ~30-45ms (parser and resolver both), with block-boundary parity between them. Re-verified: no truncating cap reintroduced (>2000-line <details>/<table> blocks still stay whole), nested same-tag blocks still balance on true depth (not just tag presence), and independent tag types (e.g. a <table> nested inside a <details>) don't cross-talk between their separate per-tag indices.	2026-08-03 13:25:38 -07:00
Peter Bowyer	4d8d3a2ca8	feat(ui): quieter plan diffs on prose edits (#603) ... * feat(ui): atomize balanced emphasis pairs in plan diff Before word-diffing, replace each balanced `**…**`, `__…__`, `~~…~~`, `*…*`, `_…_` (and triples `***…***` / `___…___`) with a unique word-char sentinel — same pattern as the existing code-span / link atomization passes. Identical phrases pair as unchanged; different phrases produce a single remove+add. Fixes the "preliminary analysis" → "final analysis" demo case (⑯), which previously orphaned the closing `**` into the unchanged tail and rendered as literal asterisks. Now renders as one clean bold-struck → bold-green swap. Pair matching uses CommonMark-ish flanking rules so stray `2**3` or intraword `my__var` / `snake_case` stay literal. Longest-first ordering prevents single delimiters from eating the inside of a double-delim pair. * feat(ui): coalesce adjacent diff sites separated by thin tokens After `diffWordsWithSpace` and sentinel restoration, merge dirty runs of ≥2 change sites separated only by thin unchanged tokens (whitespace, commas, periods, semicolons, colons, dashes, quotes) into a single phrase-level swap. Parens and brackets are excluded so inline links and bracketed content stay as hard boundaries. Turns alternating red/green word-noise (e.g. paragraph reworks with multiple adjacent word swaps) into a readable before/after. Also rescues the atomization edge case where wrapping a previously-plain phrase in emphasis (`foo bar baz` → `foo **bar baz**`) would otherwise surface as fragmented literal delimiters inside colored tags. Single-site dirty runs pass through unchanged so isolated word swaps keep word-level highlighting. * feat(ui): atomize hyphenated compounds in plan diff Hyphens between word chars (`ninety-five`, `64-byte`, `state-of-the-art`) are semantic compound words, not two tokens. `diffWordsWithSpace` splits on word boundaries, so without this pass `ninety-five` → `ninety-nine` fragments into an unchanged `ninety-` prefix and a swapped `five`/`nine` suffix — a visually noisy partial-word diff. Added a sentinel pass that replaces infix hyphens with a word-char marker before diffing and restores them afterwards. Runs after the code/link/emphasis passes so hyphens inside those constructs stay hidden. Unlike the other sentinels this one uses a fixed marker — all hyphens restore to the same character, so uniqueness isn't needed. Leading/trailing dashes and em-dash-like separators (dash with space on one side) are not substituted; only true compound infixes.	2026-04-23 20:32:30 -07:00
Michael Ramos	4139999526	feat(plan-diff): word-level inline diff rendering (#565) ... * feat(plan-diff): word-level inline diff rendering Two-pass hierarchical diff (diffLines outer + diffWordsWithSpace inner) so modified plan blocks render with inline insertions/deletions in context instead of showing the whole old block struck-through above the whole new block. Resolves #560. Engine (packages/ui/utils/planDiffEngine.ts): - computeInlineDiff runs a second-pass word diff on modified blocks that pass a whitelist gate (paragraph/heading/list-item with matching structural fields). - Sentinel substitution atomizes inline-code spans, markdown links, and fenced code blocks before diffWordsWithSpace runs, so diff markers never land inside backticks, link hrefs, or across fence boundaries. Fence regex uses a backreference so variable-length (e.g., 4-backtick wrapping 3-backtick) fences are matched atomically. - Annotation context for an inline-diffed modified block now captures both old and new content so comments on struck-through words preserve that text in the exported feedback. Renderer (packages/ui/components/plan-diff/PlanCleanDiffView.tsx): - New InlineModifiedBlock component renders a modified block as one structural wrapper with <ins>/<del> wrappers inside, parsed through the local InlineMarkdown in a single pass so markdown delimiter pairs survive across token boundaries. - InlineMarkdown extended to recognize <ins>/<del> tag passthrough (with recursive parsing of the wrapped content) and to recursively parse link anchor text so diff markers inside links render correctly. - Plain-text stop-char scanner includes '<' so <ins>/<del> dispatch re-enters the loop instead of swallowing tag text. - Click-to-annotate works in every editor mode (not just comment), with the block-level onClick opening the popover directly. Mode switcher (packages/ui/components/plan-diff/PlanDiffModeSwitcher.tsx): - Adds a third "Classic" tab between Rendered and Raw. Rendered is the new word-level default (labeled "exp"); Classic forces the legacy block-level stacked fallback for every modified block. Styling (packages/ui/theme.css, packages/editor/index.css): - plan-diff-word-added / plan-diff-word-removed utility classes for inline highlights with box-decoration-break: clone across line wraps. - Inline <code> inside the diff wrappers picks up a tinted background so code-pill changes read unambiguously green/red. - New plan-diff-modified class (amber border) for inline-diff modified blocks, matching the GitHub/VSCode convention of green=add, red=remove, yellow=both. Tests (packages/ui/utils/planDiffEngine.test.ts): - 18 tests covering the engine's qualification gate, structural-field matching, sentinel round-trip (inline code / links / fences), token content for common edit patterns. For provenance purposes, this commit was AI assisted. * chore(demo): restructure default demo, add VITE_DIFF_DEMO stress test Demo content changes that support the word-level diff work but do not alter shipped app behavior — only what other devs see running dev:hook. packages/editor/demoPlan.ts (default V3 editor content): - Added a "Context" section at the top of the plan with prose that showcases the word-level engine in V2→V3 diff: bold phrase swap, inline-code pill swaps, a link URL change, and a single-line code edit inside a config block. - Moved the mermaid architecture diagram and graphviz service map to an "Appendix: Diagrams" section at the end of the plan; they were rendering ugly mid-document. apps/hook/dev-mock-api.ts (Vite mock for the diff API): - PLAN_V1 / PLAN_V2 split into *_DEFAULT (original Real-time Collaboration plan — preserved identically from pre-branch state) and *_DIFF_TEST (the 20-case Auth Service Refactor diff-engine stress test, kept as an opt-in tool). - Resolves which pair to serve based on VITE_DIFF_DEMO env var. Matches the V2 Context section to the new V3 Context, with differences that produce rich word-level inline diffs on first load. - Diagrams moved to Appendix in V2_DEFAULT to match V3. packages/editor/App.tsx: - Both demo imports are active. VITE_DIFF_DEMO=1 swaps DIFF_DEMO_PLAN_CONTENT into the editor's default; unset renders the original Real-time Collaboration plan as before. packages/editor/demoPlanDiffDemo.ts (new): - 20-case stress test (paragraphs, headings, lists, tables, fences, blockquotes, known limitations). Each case has an identical "What to watch for" blockquote label in both V2 and V3 so the diff view cleanly isolates each case. Opt-in only. .gitignore: - Ignore .claude/ runtime lock/state files. Machine-specific content that should not be tracked. For provenance purposes, this commit was AI assisted. * style(plan-diff): refine modified-block visual — amber gutter, no fill Drop the yellow background fill from .plan-diff-modified and keep only a softened amber left border. Added/removed blocks remain loud (full fill + strong border) because add/remove are block-scope events — the whole block matters. Modify is a word-scope event — the individual changed words carry loud inline red/green highlights, and a block-level fill would compete with that inline work. The amber gutter at 75% opacity now reads as a quiet "look inside, the change is in the text" marker that sits coherently with the rest of the palette. For provenance purposes, this commit was AI assisted. * fix(plan-diff): sanitize link hrefs against javascript: / data: schemes PlanCleanDiffView has its own local copy of InlineMarkdown (separate from the one in Viewer.tsx). The link-rendering branch was passing the captured URL directly to href with no validation, so a plan containing [click me](javascript:alert(document.cookie)) would render as a live clickable anchor in the diff view. Plan content is attacker-influenced — Claude pulls from source comments, READMEs, fetched URLs — so this is a real exploit path in the diff flow. Port the same guard Viewer.tsx already has: sanitizeLinkUrl() rejects javascript:, data:, vbscript:, and file: schemes (case-insensitive, with optional leading whitespace). Rejected links render their anchor text as plain text instead of a clickable <a>, so the content is still visible to the reader but no longer dangerous. For provenance purposes, this commit was AI assisted.	2026-04-14 18:43:38 -07:00