From 6e4e5bb92e59a15dc6caeb4a61c2888935967ce7 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Thu, 27 Aug 2026 06:07:42 +0100 Subject: [PATCH] fix: escape inline Markdown in the emitted request's manifest fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EmitRequest rendered App, Participant, and Tasks through session.SafeText only, which deliberately passes the inline-Markdown triggers through, and the three fields sit as list items outside any code fence — so an attacker-authored manifest value survived as a live link or image beacon in a saved request.md, the exact construct report.md already neutralises for the identical fields. Hoist the escape into session.SafeInline as the one shared home (one-canonical-primitive), delegate report's mdInline to it, and route the emit fields through it. The false 'matching report' parity claim in EmitRequest's comment is corrected. Fixes #69. Assisted-by: Claude:claude-fable-5 --- CHANGELOG.md | 7 ++++++ internal/analyze/analyze_test.go | 38 ++++++++++++++++++++++++++++++++ internal/analyze/emit.go | 26 +++++++++++++++------- internal/report/report.go | 14 +++--------- internal/session/session.go | 26 ++++++++++++++++++++++ 5 files changed, 92 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 500d096..287b749 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,13 @@ break an existing invocation is called out in the entry that records it. ### Fixed +- `analyze`'s emitted request escapes inline Markdown in the manifest's App, + Participant, and task fields, as `report.md` already does for the identical + fields: they render as list items outside any code fence, and an + attacker-authored manifest value such as `[x](http://…/beacon.png)` used to + survive verbatim — an active link, or a tracking/exfil image beacon fired + the moment a saved `request.md` is previewed in a Markdown viewer. The + escape set now has one shared home so the two artefacts cannot drift. - `record`'s avfoundation device-listing parser no longer mistakes a device row for a section header when the device's name — an OS-supplied string a crafted USB or virtual-audio device can set — contains the phrase diff --git a/internal/analyze/analyze_test.go b/internal/analyze/analyze_test.go index 477ba62..239a176 100644 --- a/internal/analyze/analyze_test.go +++ b/internal/analyze/analyze_test.go @@ -664,6 +664,44 @@ func TestEmitRequestSanitisesManifestText(t *testing.T) { } } +// TestEmitRequestEscapesInlineMarkdown is the inline-Markdown sibling of +// TestEmitRequestSanitisesManifestText: SafeText strips control/format bytes +// and the newlines that forge BLOCK structure, but it passes the inline +// triggers (backtick, brackets, parens, and the rest) through, and App, +// Participant, and Tasks render as plain Markdown list items outside any code +// fence. Pre-fix an attacker-authored manifest value survived as live inline +// Markdown — a tracking/exfil image beacon or an active link the moment +// request.md is previewed — the exact construct report.md already neutralises +// for the identical fields via its inline escaping. +func TestEmitRequestEscapesInlineMarkdown(t *testing.T) { + dir := t.TempDir() + if err := session.SaveManifest(dir, session.Manifest{ + Session: "fixture", + App: "[x](http://attacker.example/beacon.png)", + Participant: "P1", + Tasks: []string{"Explore [y](http://attacker.example/t.png)"}, + }); err != nil { + t.Fatalf("SaveManifest: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, session.TimelineFile), []byte(timelineFixture), 0o644); err != nil { + t.Fatalf("write timeline: %v", err) + } + + got, err := EmitRequest(dir) + if err != nil { + t.Fatalf("EmitRequest: %v", err) + } + if strings.Contains(got, "- App: [x](http://attacker.example/beacon.png)") { + t.Fatalf("emitted request carries a live inline-Markdown link from the manifest App") + } + if !strings.Contains(got, `- App: \[x\]\(http://attacker.example/beacon.png\)`) { + t.Fatalf("escaped App missing from the emitted request:\n%s", got) + } + if !strings.Contains(got, ` 1. Explore \[y\]\(http://attacker.example/t.png\)`) { + t.Fatalf("escaped task missing from the emitted request:\n%s", got) + } +} + // TestEmitRequestPlaceholdersInvisibleOnlyManifestFields is the emit-side // sibling of the report header's rendered-form fix: orNone(man.App) decided // presence on the raw string, then session.SafeText was applied on the way diff --git a/internal/analyze/emit.go b/internal/analyze/emit.go index 83eba03..199109a 100644 --- a/internal/analyze/emit.go +++ b/internal/analyze/emit.go @@ -82,11 +82,15 @@ func EmitRequest(dir string) (string, error) { // The manifest is attacker-authorable — a session directory is an exchange // unit, so it may have been shared or downloaded — and the request is printed // to the operator's terminal before it is handed to an agent. Every - // manifest-derived string therefore goes through session.SafeText, matching - // report and review: without it an App, Participant, or task carrying ESC - // drives ANSI sequences in the terminal, and one carrying a newline forges - // Markdown structure (a fake "## " heading or extra rubric instructions) inside - // the request the agent is asked to obey. The timeline block below needs the + // manifest-derived string therefore goes through session.SafeInline, matching + // report.md's rendering of the identical fields: SafeText's layer stops an + // App, Participant, or task carrying ESC from driving ANSI sequences in the + // terminal and one carrying a newline from forging BLOCK structure (a fake + // "## " heading or extra rubric instructions) inside the request the agent is + // asked to obey; the inline-escape layer stops the constructs that need no + // newline — these fields render as list items outside any code fence, so an + // unescaped `[x](http://…)` or image form survives as an active link or a + // tracking/exfil beacon the moment a saved request.md is previewed. The timeline block below needs the // same treatment for a narrower reason: json.Marshal escapes the C0 controls // and ESC, but it passes the Unicode Bidi_Control set through as raw bytes, so // an exchanged session's transcript or event text could still smuggle a @@ -106,7 +110,7 @@ func EmitRequest(dir string) (string, error) { // task" instruction, and a task list that disagrees with report.md's. var tasks []string for _, t := range man.Tasks { - if rendered := session.SafeText(t); strings.TrimSpace(rendered) != "" { + if rendered := session.SafeInline(t); strings.TrimSpace(rendered) != "" { tasks = append(tasks, rendered) } } @@ -145,13 +149,19 @@ func EmitRequest(dir string) (string, error) { return b.String(), nil } -// safeOrNone applies session.SafeText and falls back to "(none)" when the +// safeOrNone applies session.SafeInline and falls back to "(none)" when the // result renders as nothing (empty or whitespace-only): a manifest field is // operator-supplied and unvalidated by session.SaveManifest, so a // whitespace-only or invisible-only value must not survive SafeText's Cf // stripping and print as a blank field with the "(none)" placeholder skipped. +// SafeInline rather than bare SafeText: these fields render as Markdown list +// items outside any code fence, so the inline triggers SafeText passes through +// must be escaped — the same neutralisation report.md applies to the identical +// fields — or an attacker-authored manifest value survives into the request as +// a live image beacon or link. The escapes only ever precede a visible +// trigger rune, so they cannot turn a renders-as-nothing value non-blank. func safeOrNone(s string) string { - t := session.SafeText(s) + t := session.SafeInline(s) if strings.TrimSpace(t) == "" { return "(none)" } diff --git a/internal/report/report.go b/internal/report/report.go index ae670bc..33bd03f 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -321,18 +321,10 @@ func clock(sec float64) string { // Backslash-escaping each trigger renders it as literal text in a viewer and keeps it // readable in source. Ordinary transcript, selector, and route text carries none of // these bytes, so the report of a normal session is byte-for-byte unchanged. +// The escape set lives in session.SafeInline, shared with the emitted analysis +// request, so the two Markdown artefacts built from untrusted text cannot drift. func mdInline(s string) string { - s = session.SafeText(s) - var b strings.Builder - b.Grow(len(s)) - for _, r := range s { - switch r { - case '\\', '`', '*', '_', '[', ']', '(', ')', '!', '<', '>', '~': - b.WriteByte('\\') - } - b.WriteRune(r) - } - return b.String() + return session.SafeInline(s) } // mdCode renders untrusted text inside a Markdown code span, where backslash escapes diff --git a/internal/session/session.go b/internal/session/session.go index aa33623..79b21ee 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -393,6 +393,32 @@ func SafeText(s string) string { }, s) } +// SafeInline renders untrusted text inert for a Markdown INLINE context: +// SafeText first (control/format/bidi bytes, and the newlines that could forge +// block structure), then a backslash before each inline-Markdown trigger +// SafeText deliberately passes through. Without the second step a manifest or +// finding value of `![x](http://host/beacon.png)` renders a live remote image — +// a tracking/exfil beacon fired the instant the artefact is opened in a +// Markdown viewer — and `[label](http://host)` an active link disguised as +// evidence. Backslash-escaping renders the triggers as literal text in a viewer +// and keeps them readable in source; ordinary text carries none of these bytes +// and is byte-for-byte unchanged. This is the one home for the escape set, so +// every Markdown artefact built from untrusted text (report.md, the emitted +// analysis request) neutralises the same constructs the same way. +func SafeInline(s string) string { + s = SafeText(s) + var b strings.Builder + b.Grow(len(s)) + for _, r := range s { + switch r { + case '\\', '`', '*', '_', '[', ']', '(', ')', '!', '<', '>', '~': + b.WriteByte('\\') + } + b.WriteRune(r) + } + return b.String() +} + // SafeTextLines applies SafeText to s one line at a time, preserving the // newlines SafeText itself would strip (they fall under r < 0x20). A // subprocess's captured output — ffmpeg's multi-line metadata dump, a device