From 63530a99dc0bf3865a02d4b28633f8a5f6884340 Mon Sep 17 00:00:00 2001 From: Danny Cosson Date: Thu, 21 May 2026 21:03:24 -0700 Subject: [PATCH 1/4] Emit OSC 8 hyperlinks through the render path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The renderer reconstructs its output from midterm's cell grid, so OSC 8 sequences from the inner program were being dropped even though the fork now tracks them per-region. Walk Region.URLID transitions in RenderLineFrom and emit \033]8;;\033\\ open/close pairs around runs sharing a URL. Each row stands alone — opening at the start of a linked run and closing at row end — so the cursor reposition between rows doesn't carry a stale link forward. Scrollback entries store the resolved URI per FormatRun (not just the URL ID) so they're self-contained against later midterm state changes, captured at OnScrollback time. renderHistoryEntry emits OSC 8 the same way as the live path. go.mod uses a local replace for the midterm fork during development; needs to be repointed at a pushed dcosson/midterm commit before merge. Co-Authored-By: Claude Opus 4.7 (1M context) --- go.mod | 2 +- internal/session/client/render.go | 51 +++++++- internal/session/client/render_osc8_test.go | 131 ++++++++++++++++++++ internal/session/virtualterminal/vt.go | 67 ++++++++-- internal/session/virtualterminal/vt_test.go | 2 +- 5 files changed, 239 insertions(+), 14 deletions(-) create mode 100644 internal/session/client/render_osc8_test.go diff --git a/go.mod b/go.mod index 923ad1c..fbcf6f6 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( gopkg.in/yaml.v3 v3.0.1 ) -replace github.com/vito/midterm v0.2.3 => github.com/dcosson/midterm v0.2.4-0.20260511235854-99f47ec830a4 +replace github.com/vito/midterm v0.2.3 => /Users/dcosson/h2home/projects/midterm require ( github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect diff --git a/internal/session/client/render.go b/internal/session/client/render.go index b103194..e6c95a2 100644 --- a/internal/session/client/render.go +++ b/internal/session/client/render.go @@ -147,6 +147,7 @@ func (c *Client) renderHistoryEntry(buf *bytes.Buffer, entry virtualterminal.Scr } var pos int var lastFormat midterm.Format + var lastURL string first := true for _, run := range entry.Runs { if pos >= cols { @@ -163,6 +164,10 @@ func (c *Client) renderHistoryEntry(buf *bytes.Buffer, entry virtualterminal.Scr lastFormat = f first = false } + if run.URL != lastURL { + writeOSC8BoundaryStr(buf, lastURL, run.URL) + lastURL = run.URL + } contentEnd := end if contentEnd > len(entry.Content) { contentEnd = len(entry.Content) @@ -172,9 +177,26 @@ func (c *Client) renderHistoryEntry(buf *bytes.Buffer, entry virtualterminal.Scr } pos = end } + if lastURL != "" { + buf.WriteString("\033]8;;\033\\") + } buf.WriteString("\033[0m") } +// writeOSC8BoundaryStr mirrors writeOSC8Boundary but takes URL strings directly +// (scrollback entries store resolved URLs so they don't need to consult the +// live midterm Terminal's URL table at render time). +func writeOSC8BoundaryStr(buf *bytes.Buffer, prev, next string) { + if prev != "" { + buf.WriteString("\033]8;;\033\\") + } + if next != "" { + buf.WriteString("\033]8;;") + buf.WriteString(next) + buf.WriteString("\033\\") + } +} + // renderScrollIndicator draws the "(scrolling)" indicator at row 1, right-aligned. func (c *Client) renderScrollIndicator(buf *bytes.Buffer) { indicator := "(scrolling)" @@ -211,7 +233,10 @@ func (c *Client) renderScrollIndicator(buf *bytes.Buffer) { // RenderLineFrom writes one row of the given terminal to buf. // This uses explicit SGR resets between format regions to prevent // background colors from bleeding across regions (midterm's RenderLine -// does not reset between regions). +// does not reset between regions). OSC 8 hyperlinks are emitted around +// runs of cells sharing a URL ID — opened on entry, closed on exit, so +// each row stands alone (the next row will reopen if its first run is +// still linked). func (c *Client) RenderLineFrom(buf *bytes.Buffer, vt *midterm.Terminal, row int) { if row >= len(vt.Content) { return @@ -219,6 +244,7 @@ func (c *Client) RenderLineFrom(buf *bytes.Buffer, vt *midterm.Terminal, row int line := vt.Content[row] var pos int var lastFormat midterm.Format + var lastURLID uint32 for region := range vt.Format.Regions(row) { f := region.F if f != lastFormat { @@ -226,6 +252,10 @@ func (c *Client) RenderLineFrom(buf *bytes.Buffer, vt *midterm.Terminal, row int buf.WriteString(f.Render()) lastFormat = f } + if region.URLID != lastURLID { + writeOSC8Boundary(buf, vt, lastURLID, region.URLID) + lastURLID = region.URLID + } end := pos + region.Size if pos < len(line) { contentEnd := end @@ -236,9 +266,28 @@ func (c *Client) RenderLineFrom(buf *bytes.Buffer, vt *midterm.Terminal, row int } pos = end } + if lastURLID != 0 { + buf.WriteString("\033]8;;\033\\") + } buf.WriteString("\033[0m") } +// writeOSC8Boundary emits the OSC 8 transitions between prev and next URL IDs. +// A close (\033]8;;\033\\) is sent before opening a new link, even when prev +// was non-zero, so terminals don't get a nested-open sequence — OSC 8 has no +// stack, and the explicit close lets the outer terminal start the new URL +// cleanly. +func writeOSC8Boundary(buf *bytes.Buffer, vt *midterm.Terminal, prev, next uint32) { + if prev != 0 { + buf.WriteString("\033]8;;\033\\") + } + if next != 0 { + buf.WriteString("\033]8;;") + buf.WriteString(vt.URL(next)) + buf.WriteString("\033\\") + } +} + // RenderLine writes one row of the primary virtual terminal to buf. func (c *Client) RenderLine(buf *bytes.Buffer, row int) { c.RenderLineFrom(buf, c.VT.Vt, row) diff --git a/internal/session/client/render_osc8_test.go b/internal/session/client/render_osc8_test.go new file mode 100644 index 0000000..d39e283 --- /dev/null +++ b/internal/session/client/render_osc8_test.go @@ -0,0 +1,131 @@ +package client + +import ( + "bytes" + "fmt" + "strings" + "testing" + + "github.com/vito/midterm" + + "h2/internal/session/virtualterminal" +) + +// osc8 builds the OSC 8 open sequence for uri (empty closes the link). +func osc8(uri string) string { + return fmt.Sprintf("\x1b]8;;%s\x1b\\", uri) +} + +const osc8Close = "\x1b]8;;\x1b\\" + +func TestRenderLineFrom_EmitsOSC8AroundHyperlink(t *testing.T) { + c := newTestClient(2, 20) + fmt.Fprintf(c.VT.Vt, "%shello%s world", osc8("https://example.com"), osc8("")) + + var buf bytes.Buffer + c.RenderLineFrom(&buf, c.VT.Vt, 0) + got := buf.String() + + openSeq := osc8("https://example.com") + if !strings.Contains(got, openSeq) { + t.Fatalf("missing OSC 8 open sequence: %q", got) + } + if !strings.Contains(got, osc8Close) { + t.Fatalf("missing OSC 8 close sequence: %q", got) + } + if strings.Index(got, openSeq) > strings.Index(got, "hello") { + t.Fatalf("OSC 8 open should precede 'hello': %q", got) + } + // The close must come after "hello" but before the trailing " world", so + // that " world" is unlinked. + closeIdx := strings.Index(got, osc8Close) + helloIdx := strings.Index(got, "hello") + worldIdx := strings.Index(got, " world") + if !(helloIdx < closeIdx && closeIdx < worldIdx) { + t.Fatalf("OSC 8 close should sit between hello and world: %q", got) + } +} + +func TestRenderLineFrom_NoOSC8ForUnlinkedRow(t *testing.T) { + c := newTestClient(2, 20) + fmt.Fprint(c.VT.Vt, "just text") + + var buf bytes.Buffer + c.RenderLineFrom(&buf, c.VT.Vt, 0) + got := buf.String() + + if strings.Contains(got, "\x1b]8;") { + t.Fatalf("unexpected OSC 8 sequence in unlinked row: %q", got) + } +} + +func TestRenderLineFrom_ClosesBetweenAdjacentLinks(t *testing.T) { + // Two back-to-back links with no gap: a close must precede the second + // open so the outer terminal sees clean URL transitions. + c := newTestClient(2, 20) + fmt.Fprintf(c.VT.Vt, "%sa%s%sb%s", + osc8("https://x"), osc8(""), + osc8("https://y"), osc8("")) + + var buf bytes.Buffer + c.RenderLineFrom(&buf, c.VT.Vt, 0) + got := buf.String() + + openX := osc8("https://x") + openY := osc8("https://y") + idxOpenX := strings.Index(got, openX) + idxOpenY := strings.Index(got, openY) + if idxOpenX < 0 || idxOpenY < 0 { + t.Fatalf("both opens should appear: %q", got) + } + // Between the two opens there must be at least one close. + between := got[idxOpenX:idxOpenY] + if !strings.Contains(between, osc8Close) { + t.Fatalf("expected a close between adjacent links: %q", got) + } +} + +func TestRenderHistoryEntry_EmitsOSC8(t *testing.T) { + c := newTestClient(2, 20) + entry := virtualterminal.ScrollHistoryEntry{ + Content: []rune("foobar"), + Runs: []virtualterminal.FormatRun{ + {Size: 3, Format: midterm.Format{}, URL: "https://example.com"}, + {Size: 3, Format: midterm.Format{}, URL: ""}, + }, + } + var buf bytes.Buffer + c.renderHistoryEntry(&buf, entry) + got := buf.String() + + openSeq := osc8("https://example.com") + if !strings.Contains(got, openSeq) { + t.Fatalf("missing OSC 8 open: %q", got) + } + if !strings.Contains(got, osc8Close) { + t.Fatalf("missing OSC 8 close: %q", got) + } + // Close must appear between "foo" (linked) and "bar" (unlinked). + idxFoo := strings.Index(got, "foo") + idxClose := strings.Index(got, osc8Close) + idxBar := strings.Index(got, "bar") + if !(idxFoo < idxClose && idxClose < idxBar) { + t.Fatalf("close should sit between foo and bar: %q", got) + } +} + +func TestRenderHistoryEntry_NoOSC8WhenAllRunsUnlinked(t *testing.T) { + c := newTestClient(2, 20) + entry := virtualterminal.ScrollHistoryEntry{ + Content: []rune("plain"), + Runs: []virtualterminal.FormatRun{ + {Size: 5, Format: midterm.Format{}, URL: ""}, + }, + } + var buf bytes.Buffer + c.renderHistoryEntry(&buf, entry) + got := buf.String() + if strings.Contains(got, "\x1b]8;") { + t.Fatalf("unexpected OSC 8 in unlinked entry: %q", got) + } +} diff --git a/internal/session/virtualterminal/vt.go b/internal/session/virtualterminal/vt.go index fdee0dc..adcf393 100644 --- a/internal/session/virtualterminal/vt.go +++ b/internal/session/virtualterminal/vt.go @@ -84,11 +84,15 @@ type ScrollHistoryEntry struct { Runs []FormatRun } -// FormatRun is a contiguous span of cells sharing one Format. The sum of all -// Sizes across an entry's Runs equals the column count at capture time. +// FormatRun is a contiguous span of cells sharing one Format and (optionally) +// one OSC 8 hyperlink. The sum of all Sizes across an entry's Runs equals the +// column count at capture time. URL is the URI string for the link, resolved +// from the midterm Terminal's URL table at capture time so the entry is +// self-contained even after the table changes. type FormatRun struct { Size int Format midterm.Format + URL string } // SetupScrollCapture installs the OnScrollback callback on VT.Vt so that @@ -109,9 +113,14 @@ func (vt *VT) SetupScrollCapture() { } else if len(line.Format) > len(line.Content) { line.Format = line.Format[:len(line.Content)] } + // Resolve per-cell URL IDs to URI strings at capture time so the + // history entry is self-contained — the midterm URL table is owned + // by the live Terminal and we don't want scrollback to depend on it + // (avoids late lookups during render and decouples lifetime). + urls := resolveLineURLs(line.URLIDs, len(line.Content), vt.Vt.URL) entry := ScrollHistoryEntry{ Content: append([]rune(nil), line.Content...), - Runs: coalesceFormatRuns(line.Format), + Runs: coalesceFormatRuns(line.Format, urls), } vt.ScrollHistory = append(vt.ScrollHistory, entry) if len(vt.ScrollHistory) > vt.scrollHistoryMax { @@ -121,28 +130,64 @@ func (vt *VT) SetupScrollCapture() { }) } -// coalesceFormatRuns RLE-encodes a per-cell []Format into spans of adjacent -// cells sharing a Format. Most TUI rows have a small handful of runs (often -// one — all default), so this typically shrinks Format storage by 50-100× -// compared to the dense per-cell representation midterm hands us. -func coalesceFormatRuns(formats []midterm.Format) []FormatRun { +// coalesceFormatRuns RLE-encodes per-cell []Format (and parallel per-cell +// URLs, if non-nil) into spans where adjacent cells share both Format and +// URL. Most TUI rows have a small handful of runs (often one — all default), +// so this typically shrinks storage by 50-100× compared to the dense per-cell +// representation. urls may be nil, in which case every run gets URL="". +func coalesceFormatRuns(formats []midterm.Format, urls []string) []FormatRun { if len(formats) == 0 { return nil } + urlAt := func(i int) string { + if i < len(urls) { + return urls[i] + } + return "" + } runs := make([]FormatRun, 0, 4) - cur := FormatRun{Size: 1, Format: formats[0]} + cur := FormatRun{Size: 1, Format: formats[0], URL: urlAt(0)} for i := 1; i < len(formats); i++ { - if formats[i] == cur.Format { + u := urlAt(i) + if formats[i] == cur.Format && u == cur.URL { cur.Size++ continue } runs = append(runs, cur) - cur = FormatRun{Size: 1, Format: formats[i]} + cur = FormatRun{Size: 1, Format: formats[i], URL: u} } runs = append(runs, cur) return runs } +// resolveLineURLs converts a per-cell []uint32 of URL IDs (as captured from +// midterm.Line.URLIDs) into a parallel per-cell []string of URI strings. nil +// IDs (no link on the line) returns nil. lookup is typically (*Terminal).URL. +func resolveLineURLs(ids []uint32, n int, lookup func(uint32) string) []string { + if len(ids) == 0 { + return nil + } + if n > len(ids) { + n = len(ids) + } + out := make([]string, n) + cache := make(map[uint32]string, 2) + for i := 0; i < n; i++ { + id := ids[i] + if id == 0 { + continue + } + if s, ok := cache[id]; ok { + out[i] = s + continue + } + s := lookup(id) + cache[id] = s + out[i] = s + } + return out +} + // ResetScrollHistory clears the captured scroll history. func (vt *VT) ResetScrollHistory() { vt.ScrollHistory = nil diff --git a/internal/session/virtualterminal/vt_test.go b/internal/session/virtualterminal/vt_test.go index 0c90545..3370674 100644 --- a/internal/session/virtualterminal/vt_test.go +++ b/internal/session/virtualterminal/vt_test.go @@ -474,7 +474,7 @@ func TestCoalesceFormatRuns(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - runs := coalesceFormatRuns(tc.input) + runs := coalesceFormatRuns(tc.input, nil) if len(runs) != tc.wantRuns { t.Fatalf("got %d runs, want %d", len(runs), tc.wantRuns) } From 9bfe099568448c7cb160090e11ef2f94055ecbfa Mon Sep 17 00:00:00 2001 From: Danny Cosson Date: Thu, 21 May 2026 21:09:05 -0700 Subject: [PATCH 2/4] Sanitize OSC 8 URLs before emission; pin midterm dep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A URI containing ESC/BEL/other C0 controls could break out of the OSC 8 envelope and inject arbitrary terminal sequences into the outer terminal. midterm's parser already terminates OSC at ESC and BEL so the live path is naturally safe in practice, but the history path stores resolved URI strings as plain data — defense in depth at the rendering boundary is cheap and protects against future midterm changes. writeOSC8BoundaryStr is now the single emission gate; it rejects any URL containing bytes < 0x20 or 0x7F (DEL) and drops the link entirely rather than try partial recovery. RenderLineFrom pre-resolves and caches the sanitized URL per region ID so state-tracking stays consistent. Repoint go.mod replace from a local checkout to the pushed dcosson/midterm osc8-hyperlinks branch so other agents and CI can build. Co-Authored-By: Claude Opus 4.7 (1M context) --- go.mod | 2 +- go.sum | 4 +- internal/session/client/render.go | 80 ++++++++++++++------- internal/session/client/render_osc8_test.go | 51 +++++++++++++ 4 files changed, 107 insertions(+), 30 deletions(-) diff --git a/go.mod b/go.mod index fbcf6f6..ca3c2c4 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( gopkg.in/yaml.v3 v3.0.1 ) -replace github.com/vito/midterm v0.2.3 => /Users/dcosson/h2home/projects/midterm +replace github.com/vito/midterm v0.2.3 => github.com/dcosson/midterm v0.2.4-0.20260522040034-c5eaa9b4a2cc require ( github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect diff --git a/go.sum b/go.sum index d9a2008..2ffc8a1 100644 --- a/go.sum +++ b/go.sum @@ -16,8 +16,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dcosson/destructive-command-guard-go v0.2.3 h1:JKgtpXtleRjzRZFTGBM83pSxT3mkKfujrPb5AC+7oeo= github.com/dcosson/destructive-command-guard-go v0.2.3/go.mod h1:5HJPETCjVmyQ5IiF1EKnJzlngxXUDlJVbMYCHsAmoDo= -github.com/dcosson/midterm v0.2.4-0.20260511235854-99f47ec830a4 h1:d0m8RWqLI3lQF3Rv2vinCpno6/bbyh55nImT6qF2X+c= -github.com/dcosson/midterm v0.2.4-0.20260511235854-99f47ec830a4/go.mod h1:WkbqZBIhH4jfxXkE2bjhosM1BdF/dCp7sR4x9wQB6fA= +github.com/dcosson/midterm v0.2.4-0.20260522040034-c5eaa9b4a2cc h1:6kZSzTEiE0Z56/fE5YMkMCPGCkKhKi9xzZF0JI/qFUE= +github.com/dcosson/midterm v0.2.4-0.20260522040034-c5eaa9b4a2cc/go.mod h1:WkbqZBIhH4jfxXkE2bjhosM1BdF/dCp7sR4x9wQB6fA= github.com/dcosson/treesitter-go v0.1.0 h1:E+tXGxZTJT7wPlAAANLMbejaNhreemIKiZgxj4oXV5I= github.com/dcosson/treesitter-go v0.1.0/go.mod h1:ROuyUNRSakznobALBQA2dhJXIEn9JvfG9CGSg6utrpE= github.com/gofrs/flock v0.13.0 h1:95JolYOvGMqeH31+FC7D2+uULf6mG61mEZ/A8dRYMzw= diff --git a/internal/session/client/render.go b/internal/session/client/render.go index e6c95a2..b61afa6 100644 --- a/internal/session/client/render.go +++ b/internal/session/client/render.go @@ -183,18 +183,48 @@ func (c *Client) renderHistoryEntry(buf *bytes.Buffer, entry virtualterminal.Scr buf.WriteString("\033[0m") } -// writeOSC8BoundaryStr mirrors writeOSC8Boundary but takes URL strings directly -// (scrollback entries store resolved URLs so they don't need to consult the -// live midterm Terminal's URL table at render time). +// writeOSC8BoundaryStr emits the OSC 8 close/open transitions between two +// URLs. An explicit close precedes any open — OSC 8 has no stack and a +// "new open inside an old open" can confuse terminals, so we always reset. +// next is sanitized against terminator-equivalent control bytes (ESC, BEL, +// C0/DEL) before emission; an unsafe input drops the link entirely rather +// than risk re-injecting attacker bytes into the outer terminal. func writeOSC8BoundaryStr(buf *bytes.Buffer, prev, next string) { if prev != "" { buf.WriteString("\033]8;;\033\\") } - if next != "" { - buf.WriteString("\033]8;;") - buf.WriteString(next) - buf.WriteString("\033\\") + if next == "" { + return + } + safe := sanitizeOSC8URL(next) + if safe == "" { + return + } + buf.WriteString("\033]8;;") + buf.WriteString(safe) + buf.WriteString("\033\\") +} + +// sanitizeOSC8URL rejects URLs containing any byte that could break out of an +// OSC 8 sequence on the outer terminal: ESC (0x1B) is the ST half, BEL (0x07) +// is the alternate terminator, and other C0 controls / DEL can lead to +// terminal state corruption. The xterm OSC 8 spec restricts URIs to printable +// ASCII anyway. Returns "" to signal "drop this link" — callers treat that +// as no-link rather than attempting partial recovery. +func sanitizeOSC8URL(s string) string { + if s == "" { + return "" } + for i := 0; i < len(s); i++ { + b := s[i] + // Reject anything below 0x20 (C0 controls including ESC, BEL, NUL, + // CR, LF, etc.) and 0x7F (DEL). Bytes >= 0x80 are UTF-8 continuation + // bytes; most terminals accept UTF-8 in OSC parameters. + if b < 0x20 || b == 0x7F { + return "" + } + } + return s } // renderScrollIndicator draws the "(scrolling)" indicator at row 1, right-aligned. @@ -244,6 +274,7 @@ func (c *Client) RenderLineFrom(buf *bytes.Buffer, vt *midterm.Terminal, row int line := vt.Content[row] var pos int var lastFormat midterm.Format + var lastURL string var lastURLID uint32 for region := range vt.Format.Regions(row) { f := region.F @@ -252,10 +283,21 @@ func (c *Client) RenderLineFrom(buf *bytes.Buffer, vt *midterm.Terminal, row int buf.WriteString(f.Render()) lastFormat = f } - if region.URLID != lastURLID { - writeOSC8Boundary(buf, vt, lastURLID, region.URLID) - lastURLID = region.URLID + // Resolve+sanitize URLs once per ID; cache via lastURLID so multiple + // adjacent regions with the same ID skip the lookup. + var curURL string + if region.URLID != 0 { + if region.URLID == lastURLID { + curURL = lastURL + } else { + curURL = sanitizeOSC8URL(vt.URL(region.URLID)) + } + } + if curURL != lastURL { + writeOSC8BoundaryStr(buf, lastURL, curURL) + lastURL = curURL } + lastURLID = region.URLID end := pos + region.Size if pos < len(line) { contentEnd := end @@ -266,28 +308,12 @@ func (c *Client) RenderLineFrom(buf *bytes.Buffer, vt *midterm.Terminal, row int } pos = end } - if lastURLID != 0 { + if lastURL != "" { buf.WriteString("\033]8;;\033\\") } buf.WriteString("\033[0m") } -// writeOSC8Boundary emits the OSC 8 transitions between prev and next URL IDs. -// A close (\033]8;;\033\\) is sent before opening a new link, even when prev -// was non-zero, so terminals don't get a nested-open sequence — OSC 8 has no -// stack, and the explicit close lets the outer terminal start the new URL -// cleanly. -func writeOSC8Boundary(buf *bytes.Buffer, vt *midterm.Terminal, prev, next uint32) { - if prev != 0 { - buf.WriteString("\033]8;;\033\\") - } - if next != 0 { - buf.WriteString("\033]8;;") - buf.WriteString(vt.URL(next)) - buf.WriteString("\033\\") - } -} - // RenderLine writes one row of the primary virtual terminal to buf. func (c *Client) RenderLine(buf *bytes.Buffer, row int) { c.RenderLineFrom(buf, c.VT.Vt, row) diff --git a/internal/session/client/render_osc8_test.go b/internal/session/client/render_osc8_test.go index d39e283..c191d74 100644 --- a/internal/session/client/render_osc8_test.go +++ b/internal/session/client/render_osc8_test.go @@ -114,6 +114,57 @@ func TestRenderHistoryEntry_EmitsOSC8(t *testing.T) { } } +// Note: midterm's OSC parser uses ESC and BEL as OSC terminators, so it +// truncates URIs at those bytes before they ever reach our URL table — making +// the live render path naturally safe against the most obvious injection. The +// renderer's own sanitizer is defense in depth against future parser changes, +// and the history path (which can carry arbitrary URL strings via FormatRun) +// is where we exercise it end-to-end below. + +func TestRenderHistoryEntry_RejectsMaliciousURI(t *testing.T) { + c := newTestClient(2, 20) + bad := "https://evil\x07injected" + entry := virtualterminal.ScrollHistoryEntry{ + Content: []rune("hi"), + Runs: []virtualterminal.FormatRun{ + {Size: 2, Format: midterm.Format{}, URL: bad}, + }, + } + var buf bytes.Buffer + c.renderHistoryEntry(&buf, entry) + got := buf.String() + if strings.Contains(got, bad) || strings.Contains(got, "injected") { + t.Fatalf("malicious URI leaked through history render: %q", got) + } + if strings.Contains(got, "\x1b]8;;https") { + t.Fatalf("rejected URI must not produce an OSC 8 open: %q", got) + } +} + +func TestSanitizeOSC8URL(t *testing.T) { + cases := []struct { + in, want string + }{ + {"", ""}, + {"https://example.com", "https://example.com"}, + {"https://example.com/a?b=c&d=e", "https://example.com/a?b=c&d=e"}, + {"https://example.com/\x1bevil", ""}, // ESC + {"https://example.com/\x07bell", ""}, // BEL + {"https://example.com/\nnewline", ""}, // LF + {"https://example.com/\x00null", ""}, // NUL + {"https://example.com/\x7fdel", ""}, // DEL + {"https://example.com/π", "https://example.com/π"}, // UTF-8 OK + } + for _, tc := range cases { + t.Run(tc.in, func(t *testing.T) { + got := sanitizeOSC8URL(tc.in) + if got != tc.want { + t.Fatalf("sanitizeOSC8URL(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + func TestRenderHistoryEntry_NoOSC8WhenAllRunsUnlinked(t *testing.T) { c := newTestClient(2, 20) entry := virtualterminal.ScrollHistoryEntry{ From 3eca36c98f89abe1500effe702ae1555be47d837 Mon Sep 17 00:00:00 2001 From: Danny Cosson Date: Thu, 21 May 2026 21:10:37 -0700 Subject: [PATCH 3/4] renderHistoryEntry: sanitize URL before tracking lastURL Symmetry fix from review: previously lastURL held the raw run.URL even when writeOSC8BoundaryStr had dropped the URL as unsafe, so an unsafe-only linked run would emit a harmless final close at row end without a corresponding open. Pre-sanitize and track the result, just like RenderLineFrom does, so the state machine reflects what was actually emitted. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/session/client/render.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/session/client/render.go b/internal/session/client/render.go index b61afa6..96b7d0b 100644 --- a/internal/session/client/render.go +++ b/internal/session/client/render.go @@ -164,9 +164,13 @@ func (c *Client) renderHistoryEntry(buf *bytes.Buffer, entry virtualterminal.Scr lastFormat = f first = false } - if run.URL != lastURL { - writeOSC8BoundaryStr(buf, lastURL, run.URL) - lastURL = run.URL + // Sanitize before tracking so an unsafe URL is treated as no-link in + // the state machine — matches RenderLineFrom and avoids a stray + // end-of-row close when no open was emitted. + curURL := sanitizeOSC8URL(run.URL) + if curURL != lastURL { + writeOSC8BoundaryStr(buf, lastURL, curURL) + lastURL = curURL } contentEnd := end if contentEnd > len(entry.Content) { From abfaf2ae7728153b8ed3a6aa243e283dead81476 Mon Sep 17 00:00:00 2001 From: Danny Cosson Date: Sun, 24 May 2026 13:00:47 -0700 Subject: [PATCH 4/4] Pin midterm fork to v0.2.4-dcosson.1 Replace the pseudo-version pin with the v0.2.4-dcosson.1 tag on the dcosson/midterm fork. The pre-release suffix keeps us sorted below any future upstream v0.2.4 release. Co-Authored-By: Claude Opus 4.7 (1M context) --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index ca3c2c4..1464346 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( gopkg.in/yaml.v3 v3.0.1 ) -replace github.com/vito/midterm v0.2.3 => github.com/dcosson/midterm v0.2.4-0.20260522040034-c5eaa9b4a2cc +replace github.com/vito/midterm v0.2.3 => github.com/dcosson/midterm v0.2.4-dcosson.1 require ( github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect diff --git a/go.sum b/go.sum index 2ffc8a1..8a3ff03 100644 --- a/go.sum +++ b/go.sum @@ -16,8 +16,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dcosson/destructive-command-guard-go v0.2.3 h1:JKgtpXtleRjzRZFTGBM83pSxT3mkKfujrPb5AC+7oeo= github.com/dcosson/destructive-command-guard-go v0.2.3/go.mod h1:5HJPETCjVmyQ5IiF1EKnJzlngxXUDlJVbMYCHsAmoDo= -github.com/dcosson/midterm v0.2.4-0.20260522040034-c5eaa9b4a2cc h1:6kZSzTEiE0Z56/fE5YMkMCPGCkKhKi9xzZF0JI/qFUE= -github.com/dcosson/midterm v0.2.4-0.20260522040034-c5eaa9b4a2cc/go.mod h1:WkbqZBIhH4jfxXkE2bjhosM1BdF/dCp7sR4x9wQB6fA= +github.com/dcosson/midterm v0.2.4-dcosson.1 h1:ORRLUKms74cAjTa8qz84qRLPSHoEWCHAa4yFvG22iQU= +github.com/dcosson/midterm v0.2.4-dcosson.1/go.mod h1:WkbqZBIhH4jfxXkE2bjhosM1BdF/dCp7sR4x9wQB6fA= github.com/dcosson/treesitter-go v0.1.0 h1:E+tXGxZTJT7wPlAAANLMbejaNhreemIKiZgxj4oXV5I= github.com/dcosson/treesitter-go v0.1.0/go.mod h1:ROuyUNRSakznobALBQA2dhJXIEn9JvfG9CGSg6utrpE= github.com/gofrs/flock v0.13.0 h1:95JolYOvGMqeH31+FC7D2+uULf6mG61mEZ/A8dRYMzw=