diff --git a/claude_code_log/factories/tool_factory.py b/claude_code_log/factories/tool_factory.py index a0356201..a844acf6 100644 --- a/claude_code_log/factories/tool_factory.py +++ b/claude_code_log/factories/tool_factory.py @@ -1646,6 +1646,18 @@ def create_tool_result_message( tool_use_result, ) + # The Artifact result line shows the favicon and version label, but the + # wire result carries only {url, path, title} — carry both over from the + # paired tool_use input (issue #262). Missing pairing degrades to the + # title/url fallbacks in the formatters. + if isinstance(parsed_output, ArtifactOutput): + artifact_use = tool_use_context.get(tool_result.tool_use_id) + if artifact_use is not None: + favicon = artifact_use.input.get("favicon") + label = artifact_use.input.get("label") + parsed_output.favicon = favicon if isinstance(favicon, str) else None + parsed_output.label = label if isinstance(label, str) else None + # An AskUserQuestion "clarify" rejection arrives as an ``is_error`` result, # but we re-present it as a normal answered card (questions + the user's # free-form reply), so drop the error framing (#180). diff --git a/claude_code_log/html/templates/components/message_styles.css b/claude_code_log/html/templates/components/message_styles.css index 8983c961..4dc45b03 100644 --- a/claude_code_log/html/templates/components/message_styles.css +++ b/claude_code_log/html/templates/components/message_styles.css @@ -1794,3 +1794,12 @@ a.workflow-phase-pill:hover { color: var(--text-muted); font-family: var(--font-monospace); } + +/* Artifact tool result (issue #262): "Published artifact (favicon) (label)". + The favicon is usually an emoji span; when it is a real image URL it + renders as an inline sized to the text line. */ +img.artifact-favicon { + height: 1em; + width: 1em; + vertical-align: -0.125em; +} diff --git a/claude_code_log/html/tool_formatters.py b/claude_code_log/html/tool_formatters.py index 21bb6fab..2872ee07 100644 --- a/claude_code_log/html/tool_formatters.py +++ b/claude_code_log/html/tool_formatters.py @@ -34,7 +34,7 @@ render_user_markdown_collapsible, resolve_memory_body_links, ) -from ..utils import is_safe_web_url, strip_error_tags +from ..utils import ARTIFACT_FAVICON_TEXT_MAX, is_safe_web_url, strip_error_tags from ..workflow import resolve_workflow_header, resolve_workflow_script from ..models import ( AskUserQuestionInput, @@ -862,27 +862,37 @@ def format_webfetch_output(output: WebFetchOutput) -> str: # -- Artifact Tool ------------------------------------------------------------ -def _artifact_link(url: str) -> str: - """Render an artifact URL as a link, or as escaped text for odd schemes. +def _artifact_labeled_link(url: str, text: str) -> str: + """Link ``text`` to ``url``, keeping the URL in href + hover only. The URL is transcript-derived and thus attacker-controllable; the shared ``is_safe_web_url`` scheme gate decides link-worthiness - (escaping alone doesn't neutralise a hostile ``javascript:`` scheme). + (escaping alone doesn't neutralise a hostile ``javascript:`` + scheme). A rejected URL degrades to the escaped URL itself, not the + label — hiding it would mask what the transcript contains (#262). """ - escaped = escape_html(url) if is_safe_web_url(url): - return f'{escaped}' - return escaped + escaped_url = escape_html(url) + return ( + f'{escape_html(text)}' + ) + return escape_html(url) def format_artifact_input(artifact_input: ArtifactInput) -> str: - """Format Artifact tool use content. - - The source file path is shown in the title; the body carries the - optional metadata (description, favicon, version label, redeploy - target) when present. The page content itself never appears in the - transcript — the tool takes a file path — so there is nothing to - render live; every field is escaped. + """Format Artifact tool use content — description + redeploy meta. + + The source file path is shown in the title. The favicon and version + label belong to the published page, so the tool result line carries + them (issue #262); the description stays primary, with a meta row + only for the redeploy addendum: the target URL and the ``force`` + flag. ``force`` means "overwrite without a conflict check" — a + normal redeploy sends the artifact's base version so a concurrent + write from another session fails instead of being silently + clobbered; ``force`` skips that check. The page content itself never + appears in the transcript — the tool takes a file path — so there is + nothing to render live; every field is escaped. """ rows: list[str] = [] if artifact_input.description: @@ -891,41 +901,67 @@ def format_artifact_input(artifact_input: ArtifactInput) -> str: f"{escape_html(artifact_input.description)}" ) meta_parts: list[str] = [] - if artifact_input.favicon: - meta_parts.append( - f'{escape_html(artifact_input.favicon)}' - ) - if artifact_input.label: - meta_parts.append( - f'{escape_html(artifact_input.label)}' - ) if artifact_input.url: meta_parts.append( - f'redeploys {_artifact_link(artifact_input.url)}' + f'redeploys ' + f"{_artifact_labeled_link(artifact_input.url, 'existing artifact')}" ) if artifact_input.force: - meta_parts.append('force') + meta_parts.append( + 'force: overwrite without conflict check' + ) if meta_parts: rows.append(f'
{" · ".join(meta_parts)}
') return "".join(rows) +def _artifact_favicon(favicon: str) -> str: + """Render the artifact favicon — a real image when one exists. + + The value is transcript-derived and thus attacker-controllable: only + an ``is_safe_web_url`` http(s) URL may become an ```` (a + ``data:``/``javascript:`` src or an attribute breakout is an XSS + vector — the src is escaped and the scheme gated). Anything else is + treated as the usual emoji favicon and rendered as escaped text; + over-long non-URL values are dropped rather than echoed. + ``referrerpolicy="no-referrer"`` keeps the local transcript page out + of the remote host's logs. + """ + if is_safe_web_url(favicon): + return ( + f'' + ) + if len(favicon) <= ARTIFACT_FAVICON_TEXT_MAX: + return f'{escape_html(favicon)}' + return "" + + def format_artifact_output(output: ArtifactOutput) -> str: - """Format Artifact tool result — published page title + link. + """Format Artifact tool result — 'Published artifact (favicon) (label)'. - Falls back to the escaped raw text for error/denial results that - carry no structured data. + The label (falling back to the page title, then the source file + basename) is the clickable link; the full URL stays in ``href`` and + the hover ``title`` instead of the visible text (issue #262). Falls + back to the escaped raw text for error/denial results that carry no + structured data. """ if output.url: - title_html = ( - f'{escape_html(output.title)} → ' - if output.title - else "" - ) - return ( - f'
Published ' - f"{title_html}{_artifact_link(output.url)}
" + parts: list[str] = ["Published artifact"] + if output.favicon: + favicon_html = _artifact_favicon(output.favicon) + if favicon_html: + parts.append(favicon_html) + link_text = ( + output.label + or output.title + or (output.path.rsplit("/", 1)[-1] if output.path else "") + or output.url ) + parts.append(_artifact_labeled_link(output.url, link_text)) + return f'
{" ".join(parts)}
' if output.raw_text: return f'
{escape_html(output.raw_text)}
' return "" diff --git a/claude_code_log/markdown/renderer.py b/claude_code_log/markdown/renderer.py index 2a18a417..94cb6886 100644 --- a/claude_code_log/markdown/renderer.py +++ b/claude_code_log/markdown/renderer.py @@ -20,6 +20,7 @@ render_user_markdown, ) from ..utils import ( + ARTIFACT_FAVICON_TEXT_MAX, format_timestamp, generate_unified_diff, is_safe_web_url, @@ -311,19 +312,55 @@ def safe_markdown_inline(text: str) -> str: return _protect_html_tags(text) if "<" in text else text -def _markdown_safe_url(url: str) -> str: +_MARKDOWN_TARGET_ENCODINGS = ( + ("(", "%28"), # destination-syntax breakout + (")", "%29"), + ('"', "%22"), # attribute breakout after Markdown→HTML conversion + ("'", "%27"), + ("<", "%3C"), + (">", "%3E"), + (" ", "%20"), # terminates the destination early +) + + +def safe_markdown_link_target(url: str) -> str: + """Percent-encode characters that could escape a ``(target)`` slot. + + Parentheses break the destination syntax itself; quotes/angle + brackets could smuggle attributes or tags through a downstream + Markdown→HTML converter; a space — or, per CommonMark, any ASCII + control character (U+0000-U+001F, U+007F) — terminates an + un-bracketed destination early, letting trailing text escape the + link. + + Plugin-facing (re-exported from ``claude_code_log.plugins``): any + plugin emitting a transcript-derived URL as a Markdown link/image + destination needs this in ADDITION to the ``is_safe_web_url`` scheme + gate — the gate rejects hostile schemes, this neutralises breakout + characters inside an accepted URL. + """ + for char, encoded in _MARKDOWN_TARGET_ENCODINGS: + url = url.replace(char, encoded) + return "".join( + f"%{ord(char):02X}" if ord(char) <= 0x1F or ord(char) == 0x7F else char + for char in url + ) + + +def _markdown_safe_url(url: str, text: Optional[str] = None) -> str: """Render a transcript-derived URL as a Markdown link, or inline code. Only URLs passing the shared ``is_safe_web_url`` scheme gate become clickable — escaping alone doesn't neutralise a hostile scheme (``javascript:`` would survive into a live link once a downstream - viewer converts the Markdown to HTML). The link target gets its - parentheses percent-encoded so it can't break out of the ``(url)`` - destination syntax. + viewer converts the Markdown to HTML). The link target is + percent-encoded against destination breakout. ``text`` labels the + link instead of the URL itself; a hostile-scheme URL falls back to + inline code regardless, so the hostile string stays visible rather + than hidden behind a label. """ if is_safe_web_url(url): - target = url.replace("(", "%28").replace(")", "%29") - return f"[{safe_markdown_inline(url)}]({target})" + return f"[{safe_markdown_inline(text if text else url)}]({safe_markdown_link_target(url)})" return _inline_code(url) @@ -1099,24 +1136,24 @@ def format_WebFetchInput(self, input: WebFetchInput, _: TemplateMessage) -> str: return "" def format_ArtifactInput(self, input: ArtifactInput, _: TemplateMessage) -> str: - """Format → description + favicon/label/redeploy meta lines. + """Format → description + redeploy meta line. - The page content never reaches the transcript (the tool takes a - file path), so the body only carries the optional metadata; the - strings are inert as Markdown via ``safe_markdown_inline``. + The favicon and version label belong to the published page, so + the tool result line carries them (issue #262); the meta line + keeps only the redeploy addendum — target URL and the ``force`` + flag (= overwrite without the concurrent-edit conflict check). + The strings are inert as Markdown via ``safe_markdown_inline``. """ parts: list[str] = [] if input.description: parts.append(f"*{safe_markdown_inline(input.description)}*") meta_parts: list[str] = [] - if input.favicon: - meta_parts.append(safe_markdown_inline(input.favicon)) - if input.label: - meta_parts.append(safe_markdown_inline(input.label)) if input.url: - meta_parts.append(f"redeploys {_markdown_safe_url(input.url)}") + meta_parts.append( + f"redeploys {_markdown_safe_url(input.url, text='existing artifact')}" + ) if input.force: - meta_parts.append("force") + meta_parts.append("force: overwrite without conflict check") if meta_parts: parts.append(" · ".join(meta_parts)) return "\n\n".join(parts) @@ -1676,20 +1713,50 @@ def format_WebFetchOutput(self, output: WebFetchOutput, _: TemplateMessage) -> s return meta_line + self._quote(output.result) def format_ArtifactOutput(self, output: ArtifactOutput, _: TemplateMessage) -> str: - """Format → 'Published → <link>', or quoted raw text. - - The raw-text branch carries error/denial messages (no structured + """Format → 'Published artifact (favicon) (label link)', or quoted + raw text. + + Mirrors the HTML renderer (issue #262): the label (falling back + to title, then source basename) is the link text, keeping the + full URL out of the visible line; an http(s) favicon value + becomes a Markdown image, an emoji one stays inline text. The + raw-text branch carries error/denial messages (no structured toolUseResult); quote it like other verbatim harness output. """ if output.url: - title_part = ( - f"**{safe_markdown_inline(output.title)}** → " if output.title else "" + parts: list[str] = ["Published artifact"] + if output.favicon: + favicon_md = self._artifact_favicon_md(output.favicon) + if favicon_md: + parts.append(favicon_md) + link_text = ( + output.label + or output.title + or (output.path.rsplit("/", 1)[-1] if output.path else "") + or None ) - return f"Published {title_part}{_markdown_safe_url(output.url)}" + parts.append(_markdown_safe_url(output.url, text=link_text)) + return " ".join(parts) if output.raw_text: return self._quote(output.raw_text) return "" + @staticmethod + def _artifact_favicon_md(favicon: str) -> str: + """Favicon for the published-artifact line — image or inline text. + + Same gate as the HTML renderer: only an ``is_safe_web_url`` + http(s) value may become an image source (a hostile scheme would + go live once a downstream viewer converts the Markdown); emoji + values render as inert inline text, over-long non-URL values are + dropped. + """ + if is_safe_web_url(favicon): + return f"![]({safe_markdown_link_target(favicon)})" + if len(favicon) <= ARTIFACT_FAVICON_TEXT_MAX: + return safe_markdown_inline(favicon) + return "" + # --- Teammate-feature tool outputs ------------------------------------- def format_TeamCreateOutput( diff --git a/claude_code_log/models.py b/claude_code_log/models.py index 8fca1011..80bd5be3 100644 --- a/claude_code_log/models.py +++ b/claude_code_log/models.py @@ -2073,6 +2073,10 @@ class ArtifactOutput: path: Optional[str] = None # Source file that was published title: Optional[str] = None # Page title extracted by the harness raw_text: Optional[str] = None # Fallback text when structured data absent + # Carried over from the paired tool_use input (the wire result has no + # favicon/label) so the result line can show them (issue #262). + favicon: Optional[str] = None + label: Optional[str] = None # ============================================================================= diff --git a/claude_code_log/plugins.py b/claude_code_log/plugins.py index 38ba005c..17f31fc0 100644 --- a/claude_code_log/plugins.py +++ b/claude_code_log/plugins.py @@ -44,7 +44,7 @@ render_markdown, render_markdown_collapsible, ) - from .markdown.renderer import safe_markdown_inline + from .markdown.renderer import safe_markdown_inline, safe_markdown_link_target from .utils import is_safe_web_url from .models import MessageContent, MessageMeta @@ -363,19 +363,25 @@ def apply_transformers( # plugin building an ``href`` / Markdown link target needs the same # http(s) whitelist the core renderers use. "is_safe_web_url", + # Companion to the scheme gate for Markdown destinations (#262): an + # ACCEPTED http(s) URL can still carry quotes/angle-brackets/spaces + # that break out of the ``(target)`` slot or smuggle attributes + # through a downstream Markdown->HTML conversion. + "safe_markdown_link_target", } ) def __getattr__(name: str) -> Any: # PEP 562 if name in _PUBLIC_HELPERS: - # ``safe_markdown_inline`` (the markdown renderer's inline HTML- - # neutralising gate — entity-escapes raw HTML tags in an inline - # markdown fragment, preserving markdown) lives in - # ``markdown/renderer.py``; the others in ``html/utils.py``. Resolved - # lazily to keep package init acyclic. + # The ``safe_markdown_*`` pair (inline HTML-neutralising gate + + # link-target percent-encoder) lives in ``markdown/renderer.py``; + # ``is_safe_web_url`` in top-level ``utils.py``; the others in + # ``html/utils.py``. Resolved lazily to keep package init acyclic. if name == "safe_markdown_inline": from .markdown.renderer import safe_markdown_inline as resolved + elif name == "safe_markdown_link_target": + from .markdown.renderer import safe_markdown_link_target as resolved elif name == "is_safe_web_url": # Format-neutral (shared by both renderers) → top-level utils. from .utils import is_safe_web_url as resolved @@ -399,4 +405,5 @@ def __getattr__(name: str) -> Any: # PEP 562 "render_markdown_collapsible", "reset_cache", "safe_markdown_inline", + "safe_markdown_link_target", ] diff --git a/claude_code_log/utils.py b/claude_code_log/utils.py index 639aa825..91d60450 100644 --- a/claude_code_log/utils.py +++ b/claude_code_log/utils.py @@ -692,6 +692,12 @@ def get_warmup_session_ids(messages: list[TranscriptEntry]) -> set[str]: return warmup_sessions +# Artifact favicons are 1-2 emoji (ZWJ sequences can span ~a dozen code +# points); anything longer is malformed input not worth echoing inline. +# Shared by the HTML and Markdown renderers so the cap can't drift. +ARTIFACT_FAVICON_TEXT_MAX = 16 + + def is_safe_web_url(url: str) -> bool: """True if ``url`` is a plain-web (http/https) URL safe to emit as a link. diff --git a/dev-docs/plugins.md b/dev-docs/plugins.md index e0f47b44..9e6bb446 100644 --- a/dev-docs/plugins.md +++ b/dev-docs/plugins.md @@ -296,6 +296,7 @@ from claude_code_log.plugins import ( escape_html, safe_markdown_inline, is_safe_web_url, + safe_markdown_link_target, ) ``` @@ -306,6 +307,7 @@ from claude_code_log.plugins import ( | `escape_html(text)` | `(str) -> str` | Interpolating transcript-derived text into a `format_html` raw-HTML string, OR into a `title` return (which is emitted via `\| safe`). Entity-escapes `<`, `>`, `&`, quotes. | | `safe_markdown_inline(text)` | `(str) -> str` | Interpolating transcript-derived text into a Markdown **inline** fragment in `format_markdown` (a link label, a list item, inline prose) — the Markdown-output path emits `format_markdown` verbatim. Entity-escapes raw HTML tags while preserving Markdown formatting. | | `is_safe_web_url(url)` | `(str) -> bool` | Gating a transcript-derived URL before making it clickable — an `href` in `format_html` or a `[text](target)` destination in `format_markdown`. Escaping cannot neutralise a hostile scheme (`javascript:alert(1)` survives `escape_html` intact), so emit the link only when this returns True and degrade to inert escaped text otherwise. http(s) whitelist, fails closed. | +| `safe_markdown_link_target(url)` | `(str) -> str` | Encoding a URL you are about to place in a Markdown `(target)` slot — a `[text](target)` link or `![](target)` image in `format_markdown`. An `is_safe_web_url`-accepted URL can still carry `"`/`'`/`<`/`>`/space/parens that break out of the destination or smuggle attributes once a downstream viewer converts the Markdown to HTML; this percent-encodes them. Use **after** the scheme gate, on the target only (label the link with `safe_markdown_inline` text). | The reference plugin's [`tool_communicate_result.py`](../test/_plugins/clmail/src/claude_code_log_clmail_test/transformers/tool_communicate_result.py) @@ -337,7 +339,8 @@ output paths: | `format_html` embedding a Markdown body | `render_markdown(value)` / `render_markdown_collapsible(value, …)` | | `format_markdown` inline fragment (link label, list item, inline prose) | `safe_markdown_inline(value)` | | `title()` return | `escape_html(value)` — the HTML header emits it via `\| safe` (no core escaping); the Markdown heading is auto-gated by the core | -| URL made clickable (`href` attribute, Markdown link target) | gate on `is_safe_web_url(url)` first, **then** `escape_html(url)` for the attribute text — the scheme check and the escaping guard different attacks | +| URL made clickable (`href` attribute) | gate on `is_safe_web_url(url)` first, **then** `escape_html(url)` for the attribute text — the scheme check and the escaping guard different attacks | +| URL made clickable (Markdown link/image target) | gate on `is_safe_web_url(url)` first, **then** `safe_markdown_link_target(url)` for the `(target)` slot — guards against attribute-smuggling through the downstream Markdown→HTML pass | Notes: diff --git a/test/__snapshots__/test_snapshot_html.ambr b/test/__snapshots__/test_snapshot_html.ambr index 4002beb8..317ebea5 100644 --- a/test/__snapshots__/test_snapshot_html.ambr +++ b/test/__snapshots__/test_snapshot_html.ambr @@ -2163,6 +2163,15 @@ color: var(--text-muted); font-family: var(--font-monospace); } + + /* Artifact tool result (issue #262): "Published artifact (favicon) (label)". + The favicon is usually an emoji span; when it is a real image URL it + renders as an inline <img> sized to the text line. */ + img.artifact-favicon { + height: 1em; + width: 1em; + vertical-align: -0.125em; + } /* Session navigation styles */ .navigation { background-color: var(--bg-neutral); @@ -8645,6 +8654,15 @@ color: var(--text-muted); font-family: var(--font-monospace); } + + /* Artifact tool result (issue #262): "Published artifact (favicon) (label)". + The favicon is usually an emoji span; when it is a real image URL it + renders as an inline <img> sized to the text line. */ + img.artifact-favicon { + height: 1em; + width: 1em; + vertical-align: -0.125em; + } /* Session navigation styles */ .navigation { background-color: var(--bg-neutral); @@ -17229,6 +17247,15 @@ color: var(--text-muted); font-family: var(--font-monospace); } + + /* Artifact tool result (issue #262): "Published artifact (favicon) (label)". + The favicon is usually an emoji span; when it is a real image URL it + renders as an inline <img> sized to the text line. */ + img.artifact-favicon { + height: 1em; + width: 1em; + vertical-align: -0.125em; + } /* Session navigation styles */ .navigation { background-color: var(--bg-neutral); @@ -23826,6 +23853,15 @@ color: var(--text-muted); font-family: var(--font-monospace); } + + /* Artifact tool result (issue #262): "Published artifact (favicon) (label)". + The favicon is usually an emoji span; when it is a real image URL it + renders as an inline <img> sized to the text line. */ + img.artifact-favicon { + height: 1em; + width: 1em; + vertical-align: -0.125em; + } /* Session navigation styles */ .navigation { background-color: var(--bg-neutral); @@ -30690,6 +30726,15 @@ color: var(--text-muted); font-family: var(--font-monospace); } + + /* Artifact tool result (issue #262): "Published artifact (favicon) (label)". + The favicon is usually an emoji span; when it is a real image URL it + renders as an inline <img> sized to the text line. */ + img.artifact-favicon { + height: 1em; + width: 1em; + vertical-align: -0.125em; + } /* Session navigation styles */ .navigation { background-color: var(--bg-neutral); @@ -37517,6 +37562,15 @@ color: var(--text-muted); font-family: var(--font-monospace); } + + /* Artifact tool result (issue #262): "Published artifact (favicon) (label)". + The favicon is usually an emoji span; when it is a real image URL it + renders as an inline <img> sized to the text line. */ + img.artifact-favicon { + height: 1em; + width: 1em; + vertical-align: -0.125em; + } /* Session navigation styles */ .navigation { background-color: var(--bg-neutral); @@ -44287,6 +44341,15 @@ color: var(--text-muted); font-family: var(--font-monospace); } + + /* Artifact tool result (issue #262): "Published artifact (favicon) (label)". + The favicon is usually an emoji span; when it is a real image URL it + renders as an inline <img> sized to the text line. */ + img.artifact-favicon { + height: 1em; + width: 1em; + vertical-align: -0.125em; + } /* Session navigation styles */ .navigation { background-color: var(--bg-neutral); diff --git a/test/test_artifact_rendering.py b/test/test_artifact_rendering.py index 3d49ba1e..8a99e20a 100644 --- a/test/test_artifact_rendering.py +++ b/test/test_artifact_rendering.py @@ -83,6 +83,8 @@ def test_input_tolerates_unknown_fields(self): assert isinstance(parsed, ArtifactInput) def test_format_input_metadata(self): + """The use side keeps only the description — favicon and label + belong to the published page and move to the result line (#262).""" html = format_artifact_input( ArtifactInput( file_path="/workspace/page.html", @@ -92,10 +94,13 @@ def test_format_input_metadata(self): ) ) assert "A dashboard" in html - assert "📊" in html - assert "v2" in html + assert "📊" not in html + assert "v2" not in html - def test_format_input_redeploy_link(self): + def test_format_input_redeploy_meta(self): + """The redeploy addendum keeps a meta row: labeled target link + (URL in href/hover only) + the force flag spelled out as + 'overwrite without conflict check' (#262).""" html = format_artifact_input( ArtifactInput( file_path="/workspace/page.html", @@ -104,8 +109,10 @@ def test_format_input_redeploy_link(self): force=True, ) ) - assert '<a href="https://claude.ai/code/artifact/abc">' in html - assert "force" in html + assert 'href="https://claude.ai/code/artifact/abc"' in html + assert ">existing artifact</a>" in html + assert ">https://claude.ai/code/artifact/abc</a>" not in html + assert "force: overwrite without conflict check" in html def test_format_input_escapes_html(self): """XSS guard: hostile description/label render escaped, never live.""" @@ -122,7 +129,8 @@ def test_format_input_escapes_html(self): assert "<b>" not in html def test_format_input_javascript_url_not_linked(self): - """A hostile scheme must not become a clickable redeploy link.""" + """A hostile scheme must not become a clickable redeploy link — + it degrades to visible escaped text.""" html = format_artifact_input( ArtifactInput( file_path="/workspace/page.html", @@ -202,26 +210,104 @@ def test_parse_empty_returns_none(self): class TestArtifactOutputFormatting: - """Test Artifact output HTML formatting.""" + """Test Artifact output HTML formatting. - def test_format_output_full(self): + Issue #262 shape: 'Published artifact (favicon) (label)' — the label + (→ title → source basename) is the link text; the full URL lives only + in href and the hover title. + """ + + def test_format_output_label_is_link_text(self): html = format_artifact_output( ArtifactOutput( url="https://claude.ai/code/artifact/abc", path="/workspace/page.html", title="My page", + favicon="📊", + label="first-sweep", ) ) - assert "Published" in html - assert "My page" in html - assert '<a href="https://claude.ai/code/artifact/abc">' in html + assert "Published artifact" in html + assert '<span class="artifact-favicon">📊</span>' in html + assert 'href="https://claude.ai/code/artifact/abc"' in html + assert ">first-sweep</a>" in html + # The URL is not visible text — only href + hover title carry it. + assert 'title="https://claude.ai/code/artifact/abc"' in html + assert ">https://claude.ai/code/artifact/abc</a>" not in html + + def test_format_output_falls_back_to_title(self): + html = format_artifact_output( + ArtifactOutput( + url="https://claude.ai/code/artifact/abc", + path="/workspace/page.html", + title="My page", + ) + ) + assert ">My page</a>" in html - def test_format_output_no_title(self): + def test_format_output_falls_back_to_basename_then_url(self): + html = format_artifact_output( + ArtifactOutput( + url="https://claude.ai/code/artifact/abc", + path="/workspace/page.html", + ) + ) + assert ">page.html</a>" in html html = format_artifact_output( ArtifactOutput(url="https://claude.ai/code/artifact/abc") ) - assert "Published" in html - assert '<a href="https://claude.ai/code/artifact/abc">' in html + assert ">https://claude.ai/code/artifact/abc</a>" in html + + def test_format_output_favicon_image_url(self): + """A real http(s) favicon URL renders as an actual <img>.""" + html = format_artifact_output( + ArtifactOutput( + url="https://claude.ai/code/artifact/abc", + favicon="https://claude.ai/fav.png", + label="v2", + ) + ) + assert '<img class="artifact-favicon" src="https://claude.ai/fav.png"' in html + assert 'referrerpolicy="no-referrer"' in html + + def test_format_output_favicon_hostile_scheme_not_img(self): + """A javascript:/data: favicon must never become an <img src>.""" + for hostile in ("javascript:alert(1)", "data:text/html,<script>x</script>"): + html = format_artifact_output( + ArtifactOutput( + url="https://claude.ai/code/artifact/abc", + favicon=hostile, + label="v2", + ) + ) + assert "<img" not in html, hostile + assert "src=" not in html, hostile + + def test_format_output_favicon_attribute_breakout_escaped(self): + """An https favicon passes the scheme gate — quote injection in the + src attribute must be neutralised by escaping.""" + html = format_artifact_output( + ArtifactOutput( + url="https://claude.ai/code/artifact/abc", + favicon='https://evil.example/f.png" onerror="alert(2)', + label="v2", + ) + ) + assert 'onerror="alert(2)"' not in html + assert "" onerror="" in html + + def test_format_output_overlong_favicon_dropped(self): + """A non-URL favicon longer than any emoji sequence is dropped, not + echoed.""" + html = format_artifact_output( + ArtifactOutput( + url="https://claude.ai/code/artifact/abc", + favicon="x" * 40, + label="v2", + ) + ) + assert "x" * 17 not in html + assert "artifact-favicon" not in html def test_format_output_raw_text(self): html = format_artifact_output(ArtifactOutput(raw_text="Denied by policy")) @@ -300,6 +386,8 @@ def test_title(self): assert title == "🖼️ Artifact `/workspace/dashboard.html`" def test_format_input(self): + """Use side keeps only the description (#262) — favicon/label move + to the result line.""" renderer = MarkdownRenderer() artifact_input = ArtifactInput( file_path="/workspace/dashboard.html", @@ -311,16 +399,67 @@ def test_format_input(self): artifact_input, _input_message(artifact_input) ) assert "*A dashboard*" in md - assert "📊 · v2" in md + assert "📊" not in md + assert "v2" not in md + + def test_format_input_redeploy_meta(self): + renderer = MarkdownRenderer() + artifact_input = ArtifactInput( + file_path="/workspace/dashboard.html", + favicon="📊", + url="https://claude.ai/code/artifact/abc", + force=True, + ) + md = renderer.format_ArtifactInput( + artifact_input, _input_message(artifact_input) + ) + assert ( + "redeploys [existing artifact](https://claude.ai/code/artifact/abc)" in md + ) + assert "force: overwrite without conflict check" in md def test_format_output_link(self): + """Label (→ title) is the link text; the URL is only the target.""" + renderer = MarkdownRenderer() + output = ArtifactOutput( + url="https://claude.ai/code/artifact/abc", + title="My page", + favicon="📊", + label="first-sweep", + ) + md = renderer.format_ArtifactOutput(output, _output_message(output)) + assert "Published artifact 📊 " in md + assert "[first-sweep](https://claude.ai/code/artifact/abc)" in md + + def test_format_output_title_fallback(self): renderer = MarkdownRenderer() output = ArtifactOutput( url="https://claude.ai/code/artifact/abc", title="My page" ) md = renderer.format_ArtifactOutput(output, _output_message(output)) - assert "**My page**" in md - assert "(https://claude.ai/code/artifact/abc)" in md + assert "[My page](https://claude.ai/code/artifact/abc)" in md + + def test_format_output_favicon_image_url(self): + renderer = MarkdownRenderer() + output = ArtifactOutput( + url="https://claude.ai/code/artifact/abc", + favicon="https://claude.ai/fav.png", + label="v2", + ) + md = renderer.format_ArtifactOutput(output, _output_message(output)) + assert "![](https://claude.ai/fav.png)" in md + + def test_format_output_hostile_favicon_not_image(self): + """A hostile-scheme favicon must not become a Markdown image + target (it would go live once converted to HTML).""" + renderer = MarkdownRenderer() + output = ArtifactOutput( + url="https://claude.ai/code/artifact/abc", + favicon="javascript:alert(1)", + label="v2", + ) + md = renderer.format_ArtifactOutput(output, _output_message(output)) + assert "![](javascript:" not in md def test_format_output_javascript_url_not_linked(self): """A hostile scheme must render as inline code, not a Markdown link.""" @@ -353,13 +492,30 @@ def test_html_end_to_end(self): messages = load_transcript(self.FIXTURE) html = generate_html(messages, "Artifact Test") - # Success + redeploy + markdown variants all render with links - assert "Flaky tests report" in html + # Result lines: 'Published artifact (favicon) (label link)' — the + # label comes from the paired tool_use input, the URL stays in + # href/hover only (#262). + assert "Published artifact" in html assert ( - '<a href="https://claude.ai/code/artifact/11111111-2222-3333-4444-555555555555">' + 'href="https://claude.ai/code/artifact/11111111-2222-3333-4444-555555555555"' in html ) - assert "notes.md" in html + assert ">first-sweep</a>" in html + assert ">after-fixes</a>" in html + assert '<span class="artifact-favicon">📊</span>' in html + # No label on the markdown variant → page title is the link text + assert ">notes.md</a>" in html + # The URL never appears as visible link text + assert ( + ">https://claude.ai/code/artifact/11111111-2222-3333-4444-555555555555</a>" + not in html + ) + # Use side: favicon/label dropped, description kept, redeploy + # addendum as a labeled link + spelled-out force flag + assert "Dashboard of flaky tests found in the CI sweep." in html + assert '<span class="artifact-label">' not in html + assert ">existing artifact</a>" in html + assert "force: overwrite without conflict check" in html # Denied variant renders its text assert "denied by the Claude Code auto mode classifier" in html # Title icon suppresses the default wrench (no double icon) @@ -384,6 +540,10 @@ def test_html_end_to_end_xss(self): assert "<img src=x onerror=alert(1)>" not in html assert 'href="javascript:' not in html assert "href='javascript:" not in html + # The hostile favicon passes the https scheme gate but its quote + # injection must be neutralised in the img src attribute (#262). + assert 'onerror="alert(2)"' not in html + assert "" onerror="alert(2)" in html def test_markdown_end_to_end_xss(self): messages = load_transcript(self.FIXTURE) @@ -398,5 +558,12 @@ def test_markdown_end_to_end_xss(self): # carrier; a Markdown converter escapes their content itself. assert "<img" not in md.lower() assert "<img" in md.lower() - assert "<script>alert(1)</script> hostile title" not in md - assert "<script>alert(1)</script> hostile title" in md + # The hostile title no longer renders at all (#262: the result + # line shows the label link, and this entry's URL is unsafe so + # even that degrades to inline code). + assert "hostile title" not in md + # The hostile favicon passes the https scheme gate but its quote + # injection is percent-encoded inside the image target, so no + # attribute can survive a Markdown→HTML conversion. + assert "![](https://evil.example/f.png%22%20onerror=%22alert%282%29)" in md + assert 'f.png" onerror=' not in md diff --git a/test/test_data/artifact_tool.jsonl b/test/test_data/artifact_tool.jsonl index 43472601..c0b9a6dc 100644 --- a/test/test_data/artifact_tool.jsonl +++ b/test/test_data/artifact_tool.jsonl @@ -8,5 +8,5 @@ {"type":"user","uuid":"u-artifact-redeploy-result","parentUuid":"a-artifact-redeploy","timestamp":"2026-07-02T10:00:06.000Z","sessionId":"s1","version":"2.1.198","cwd":"/tmp","userType":"external","isSidechain":false,"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_test_artifact_redeploy","content":"Published /workspace/demo/flaky-tests-report.html at https://claude.ai/code/artifact/11111111-2222-3333-4444-555555555555"}]},"toolUseResult":{"url":"https://claude.ai/code/artifact/11111111-2222-3333-4444-555555555555","path":"/workspace/demo/flaky-tests-report.html","title":"Flaky tests report"}} {"type":"assistant","uuid":"a-artifact-denied","parentUuid":"u-artifact-redeploy-result","timestamp":"2026-07-02T10:00:07.000Z","sessionId":"s1","version":"2.1.198","cwd":"/tmp","userType":"external","isSidechain":false,"message":{"id":"msg-4","model":"claude-fable-5","type":"message","role":"assistant","content":[{"type":"tool_use","id":"toolu_test_artifact_denied","name":"Artifact","input":{"file_path":"/workspace/demo/private-notes.md","favicon":"🔒"}}]}} {"type":"user","uuid":"u-artifact-denied-result","parentUuid":"a-artifact-denied","timestamp":"2026-07-02T10:00:08.000Z","sessionId":"s1","version":"2.1.198","cwd":"/tmp","userType":"external","isSidechain":false,"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_test_artifact_denied","content":"Permission for this action was denied by the Claude Code auto mode classifier. Reason: [Create Public Surface] This Artifact action publishes a page to claude.ai (an external service).","is_error":true}]}} -{"type":"assistant","uuid":"a-artifact-xss","parentUuid":"u-artifact-denied-result","timestamp":"2026-07-02T10:00:09.000Z","sessionId":"s1","version":"2.1.198","cwd":"/tmp","userType":"external","isSidechain":false,"message":{"id":"msg-5","model":"claude-fable-5","type":"message","role":"assistant","content":[{"type":"tool_use","id":"toolu_test_artifact_xss","name":"Artifact","input":{"file_path":"/workspace/demo/<script>alert(1)</script>.html","favicon":"🧪","description":"<img src=x onerror=alert(1)> hostile description","label":"<b>bold</b>","url":"javascript:alert(1)"}}]}} +{"type":"assistant","uuid":"a-artifact-xss","parentUuid":"u-artifact-denied-result","timestamp":"2026-07-02T10:00:09.000Z","sessionId":"s1","version":"2.1.198","cwd":"/tmp","userType":"external","isSidechain":false,"message":{"id":"msg-5","model":"claude-fable-5","type":"message","role":"assistant","content":[{"type":"tool_use","id":"toolu_test_artifact_xss","name":"Artifact","input":{"file_path":"/workspace/demo/<script>alert(1)</script>.html","favicon":"https://evil.example/f.png\" onerror=\"alert(2)","description":"<img src=x onerror=alert(1)> hostile description","label":"<b>bold</b>","url":"javascript:alert(1)"}}]}} {"type":"user","uuid":"u-artifact-xss-result","parentUuid":"a-artifact-xss","timestamp":"2026-07-02T10:00:10.000Z","sessionId":"s1","version":"2.1.198","cwd":"/tmp","userType":"external","isSidechain":false,"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_test_artifact_xss","content":"Published /workspace/demo/<script>alert(1)</script>.html at https://claude.ai/code/artifact/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"}]},"toolUseResult":{"url":"javascript:alert(1)","path":"/workspace/demo/<script>alert(1)</script>.html","title":"<script>alert(1)</script> hostile title"}} diff --git a/test/test_plugin_xss_api.py b/test/test_plugin_xss_api.py index b45ce0db..bb826be4 100644 --- a/test/test_plugin_xss_api.py +++ b/test/test_plugin_xss_api.py @@ -35,6 +35,7 @@ def test_helpers_importable_and_in_all(self): "escape_html", "safe_markdown_inline", "is_safe_web_url", + "safe_markdown_link_target", ): assert hasattr(plugins, name), name assert name in plugins.__all__, name @@ -60,6 +61,31 @@ def test_is_safe_web_url_behaviour(self): assert not is_safe_web_url("HTTP://example.com") # fails closed, by design assert not is_safe_web_url("") + def test_safe_markdown_link_target_behaviour(self): + from claude_code_log.plugins import safe_markdown_link_target + + # Breakout characters are percent-encoded so an accepted URL cannot + # escape a Markdown (target) slot or smuggle attributes through a + # downstream Markdown→HTML conversion (#262). + assert ( + safe_markdown_link_target('https://x/f.png" onerror="alert(1)') + == "https://x/f.png%22%20onerror=%22alert%281%29" + ) + assert safe_markdown_link_target("https://x/a(b)'c<d>") == ( + "https://x/a%28b%29%27c%3Cd%3E" + ) + # ASCII control characters also terminate an un-bracketed CommonMark + # destination — the full range (U+0000-U+001F, U+007F) is encoded. + assert ( + safe_markdown_link_target("https://x/a\nb\tc\rd\x00e\x7ff") + == "https://x/a%0Ab%09c%0Dd%00e%7Ff" + ) + # A clean URL passes through unchanged. + assert ( + safe_markdown_link_target("https://example.com/page?q=1&r=2") + == "https://example.com/page?q=1&r=2" + ) + # ----------------------------- plugin-author contract ------------------------