feat(asv3): markdown port-only path + safeHref tighten (WP-4)#407
Open
Luis85 wants to merge 2 commits into
Open
feat(asv3): markdown port-only path + safeHref tighten (WP-4)#407Luis85 wants to merge 2 commits into
Luis85 wants to merge 2 commits into
Conversation
… tighten (WP-4)
Make `MarkdownRenderPort` the only renderer for completed assistant turns,
bypass the port entirely while a turn is still streaming, and tighten
`safeHref` so the rendered link surface cannot smuggle unsafe URI schemes.
New port adapters & pure parser
- `src/ui/components/agent/internal/markdown-parser.ts` — extracts the
hand-rolled paragraph / code-fence / list / blockquote / inline
bold-italic-code-link rendering that used to live inside
`MarkdownBlock.vue` into a pure module exporting `renderMarkdownInto` and
`safeHref`. The adapters write real DOM (no `innerHTML`, ADR-008).
- `src/infrastructure/mock/MockMarkdownRenderPort.ts` — unit-test and
`npm run dev` adapter, delegates to the pure parser.
- `src/infrastructure/localstorage/LocalStorageMarkdownRenderPort.ts` —
GitHub Pages adapter, delegates to the mock so the three-bridge symmetry
is preserved (`ObsidianBridge` keeps its native Obsidian-renderer adapter).
- `src/ui/main.ts` — wires the new port via `app.provide(MARKDOWN_RENDER_PORT, …)`
in both PROD (LocalStorage) and dev (Mock) modes so the port is always
present in the standalone build.
MarkdownBlock simplification
- `src/ui/components/agent/MarkdownBlock.vue` — drops from 458 LOC to 205.
The hand-rolled VNode parser branch is gone; the SFC now delegates to
`MARKDOWN_RENDER_PORT` exclusively. Throws at mount if the port is
missing (port-only invariant closed). Monotonic `latestSeq` guards
against out-of-order async port renders disposing the wrong tree.
`max-lines` ESLint warning on this file is gone.
safeHref tightening with rejection table
- `safeHref` is now an explicit deny → allow → default-reject pipeline:
rejects `javascript:`, `data:`, `file:`, `blob:`, `vbscript:`, `about:`,
`chrome:`, `chrome-extension:`, `obsidian:` (whitespace- and
case-tolerant) plus protocol-relative `//host`. Accepts `https:`,
`http:`, `mailto:`, `/root/relative`, `#fragment`. Default-rejects
everything else. Unit-tested with a full rejection table in
`tests/ui/components/agent/internal/markdown-parser.test.ts`.
Scroll / streaming bypass
- New `streaming?: boolean` prop on `MarkdownBlock`. When `true`, the
template renders `<pre class="sp-markdown--streaming">{{ text }}</pre>`
with `white-space: pre-wrap` and zero markdown parsing — pure text via
Vue interpolation, no per-token reparse / re-mount flicker, no port
round-trip. On stream-complete the parent flips it to `false` and the
port renders the final tree once.
- `MessageList.vue:363` passes `:streaming="true"` for the in-flight
bubble; completed turns keep the port-rendered tree.
Tests
- `tests/ui/components/agent/internal/markdown-parser.test.ts` — parser
+ safeHref rejection table.
- `tests/infrastructure/mock/MockMarkdownRenderPort.test.ts` — adapter
contract test.
- `MarkdownBlock.test.ts` — drops fallback-branch tests, adds
streaming-prop coverage and port-only invariant assertion.
- `MessageList.test.ts` / `MessageList.compactBoundary.test.ts` /
`InlinePlanApprovalCard.test.ts` — updated to provide the new port.
Full AGENTS.md §3 pre-PR gate green: audit / typecheck / lint / 1915 tests
passed / build / build:web / docs:api.
https://claude.ai/code/session_01UWDtzLuFJU3QmQmLrXCxWj
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
MarkdownBlock.vuedrops from 458 → 205 LOC. The hand-rolled VNode parser branch is gone; the SFC now delegates exclusively toMARKDOWN_RENDER_PORTand throws at mount if the port is missing (port-only invariant closed). The extracted pure parser lives atsrc/ui/components/agent/internal/markdown-parser.ts.streaming?: booleanprop.MessageList.vuepasses:streaming="true"for the in-flight bubble; the template renders<pre>{{ text }}</pre>(Vue interpolation,white-space: pre-wrap) with zero markdown parsing — no per-token re-mount flicker, no port round-trip. The parent flips tofalseon stream complete and the port renders the final tree once.safeHreftightened to an explicit deny → allow → default-reject pipeline. Rejectsjavascript:,data:,file:,blob:,vbscript:,about:,chrome:,chrome-extension:,obsidian:(whitespace- and case-tolerant), plus protocol-relative//host. Acceptshttps:,http:,mailto:,/root/relative,#fragment. Tested with a full rejection table.MockMarkdownRenderPort(src/infrastructure/mock/) andLocalStorageMarkdownRenderPort(src/infrastructure/localstorage/, delegates to mock) so the port is always present in test / dev / GitHub Pages builds.src/ui/main.tswires them viaapp.provide(MARKDOWN_RENDER_PORT, …).max-lineswarning onMarkdownBlock.vueis gone (205 LOC, below the 350 threshold) — addresses the WP-4 audit finding.Test plan
DoD checklist from
specs/agent-sidepanel-v3/wp-04-markdown-hardening/brief.md:npm audit --audit-level=high --omit=dev— clean (0 vulnerabilities).npm run typecheck— clean.npm run lint— 0 errors;max-lineswarning onMarkdownBlock.vueis gone.npm run test— 1915 passed (151 files).npm run build— succeeds; regeneratesstyles.css(scoped Vue styles, included in commit).npm run build:web— succeeds.npm run docs:api— succeeds.MessageList.test.tsasserts that whenstreamingText.length > 0the rendered DOM is a<pre>(no markdown parsing). After stream complete, the DOM is the port-rendered tree.safeHreftable closed —tests/ui/components/agent/internal/markdown-parser.test.tsrejection table coversjavascript:,data:,file:,blob:,vbscript:,about:,chrome:,chrome-extension:,obsidian:, and protocol-relative//host; acceptance ofhttps:,http:,mailto:,/root/relative,#fragment.inject(MARKDOWN_RENDER_PORT, undefined)fallback is gone;MarkdownBlockthrows at mount if the port is missing, asserted inMarkdownBlock.test.ts.Audit rationale:
MarkdownBlock.vuecarried two implementations (Obsidian renderer + hand-rolled VNode parser) and re-mounted the native renderer per token delta during streaming — both flagged by the WP-4 audit alongside themax-lineswarning and thesafeHrefallowlist gap (protocol-relative//hostslipped through the/-prefix branch). This PR closes all three.https://claude.ai/code/session_01UWDtzLuFJU3QmQmLrXCxWj
Generated by Claude Code