plan 8: escape terminal controls in everything krino prints

9 files changed, 252 insertions, 26 deletions

diff --git a/cmd/krino/display.go b/cmd/krino/display.go
new file mode 100644
index 0000000..52e8385
--- /dev/null
+++ b/cmd/krino/display.go
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import (
+	"fmt"
+	"strings"
+	"unicode/utf8"
+)
+
+// display makes s safe to print to a terminal (spec §15.1). File names,
+// paths, reasons and tool messages come from outside krino: a control
+// character in one could move the cursor, recolour or erase what krino
+// prints, and a bidirectional control could reorder it, so a name could
+// hide a step or fake one. Every C0 control, DEL, C1 control, Unicode
+// bidirectional embedding, override or isolate, and every byte that is not
+// valid UTF-8 is shown as an escape instead - \x1b for a byte, \u202e for
+// a rune. Everything else, backslash included, is printed as it is. Text
+// that is plain printable ASCII is returned without copying.
+func display(s string) string {
+	plain := true
+	for i := 0; i < len(s); i++ {
+		if c := s[i]; c < 0x20 || c >= 0x7f {
+			plain = false
+			break
+		}
+	}
+	if plain {
+		return s
+	}
+	var b strings.Builder
+	for i := 0; i < len(s); {
+		r, size := utf8.DecodeRuneInString(s[i:])
+		switch {
+		case r == utf8.RuneError && size == 1:
+			fmt.Fprintf(&b, `\x%02x`, s[i])
+		case r < 0x20 || r == 0x7f:
+			fmt.Fprintf(&b, `\x%02x`, r)
+		case controlRune(r):
+			fmt.Fprintf(&b, `\u%04x`, r)
+		default:
+			b.WriteString(s[i : i+size])
+		}
+		i += size
+	}
+	return b.String()
+}
+
+// controlRune reports whether r is a C1 control or a Unicode bidirectional
+// embedding, override or isolate: printable-looking code points a terminal
+// acts on.
+func controlRune(r rune) bool {
+	return (r >= 0x80 && r <= 0x9f) || (r >= 0x202a && r <= 0x202e) || (r >= 0x2066 && r <= 0x2069)
+}
diff --git a/cmd/krino/display_test.go b/cmd/krino/display_test.go
new file mode 100644
index 0000000..4c1cefe
--- /dev/null
+++ b/cmd/krino/display_test.go
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+	"unicode/utf8"
+)
+
+// firstUnsafe names the first thing in s a terminal could act on - a C0
+// control (a newline too, unless allowNewline), DEL, a C1 control, a
+// bidirectional control, or invalid UTF-8 - or returns "" when there is
+// none.
+func firstUnsafe(s string, allowNewline bool) string {
+	if !utf8.ValidString(s) {
+		return "invalid UTF-8"
+	}
+	for _, r := range s {
+		if r == '\n' && allowNewline {
+			continue
+		}
+		if r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f) || (r >= 0x202a && r <= 0x202e) || (r >= 0x2066 && r <= 0x2069) {
+			return fmt.Sprintf("%U", r)
+		}
+	}
+	return ""
+}
+
+// TestDisplayEscapesTerminalControls: display shows every character a
+// terminal would act on as an escape, and leaves ordinary text - Polish
+// letters and a backslash included - exactly as it is.
+func TestDisplayEscapesTerminalControls(t *testing.T) {
+	for in, want := range map[string]string{
+		"plain name.pdf":           "plain name.pdf",
+		"zażółć gęślą jaźń.pdf":    "zażółć gęślą jaźń.pdf",
+		"esc\x1b[2Kx.pdf":          `esc\x1b[2Kx.pdf`,
+		"bell\a.pdf":               `bell\x07.pdf`,
+		"cr\rline.pdf":             `cr\x0dline.pdf`,
+		"new\nline.pdf":            `new\x0aline.pdf`,
+		"tab\t.pdf":                `tab\x09.pdf`,
+		"del\x7f.pdf":              `del\x7f.pdf`,
+		"c1\u009bcsi.pdf":          `c1\u009bcsi.pdf`,
+		"bidi\u202egnp.pdf":        `bidi\u202egnp.pdf`,
+		"isolate\u2066x\u2069.pdf": `isolate\u2066x\u2069.pdf`,
+		"bad\xffbyte.pdf":          `bad\xffbyte.pdf`,
+		`back\slash.pdf`:           `back\slash.pdf`,
+		"replacement\ufffd.pdf":    "replacement\ufffd.pdf",
+	} {
+		if got := display(in); got != want {
+			t.Errorf("display(%q) = %q, want %q", in, got, want)
+		}
+	}
+}
+
+// FuzzDisplay: whatever a name holds, display's result holds nothing a
+// terminal could act on.
+func FuzzDisplay(f *testing.F) {
+	f.Add("esc\x1b[2K\u202e\xff\x00")
+	f.Add("plain")
+	f.Fuzz(func(t *testing.T, s string) {
+		if bad := firstUnsafe(display(s), false); bad != "" {
+			t.Fatalf("display(%q) = %q still holds %s", s, display(s), bad)
+		}
+	})
+}
+
+// TestReviewEscapesHostileNames: the per-file header and the delete
+// confirmation show a hostile name escaped, in review and in undo's review.
+func TestReviewEscapesHostileNames(t *testing.T) {
+	out := new(strings.Builder)
+	if _, _, _, err := reviewChains(strings.NewReader("cdnn"), out, chains("esc\x1b[2Kx.pdf"), "", palette{}); err != nil {
+		t.Fatal(err)
+	}
+	if bad := firstUnsafe(out.String(), true); bad != "" {
+		t.Errorf("review printed %s:\n%q", bad, out)
+	}
+	undoOut := new(strings.Builder)
+	if _, _, err := reviewUndoFiles(strings.NewReader("cn"), undoOut, undoFiles("esc\x1b[2Kx.pdf"), palette{}); err != nil {
+		t.Fatal(err)
+	}
+	if bad := firstUnsafe(undoOut.String(), true); bad != "" {
+		t.Errorf("undo review printed %s:\n%q", bad, undoOut)
+	}
+}
diff --git a/cmd/krino/explain.go b/cmd/krino/explain.go
index 8fade24..b69f50f 100644
--- a/cmd/krino/explain.go
+++ b/cmd/krino/explain.go
@@ -47,7 +47,7 @@ func cmdExplain(g *globals, args []string, stdout, stderr io.Writer) int {
 		return 2
 	}
 
-	fmt.Fprintf(stdout, "%s  (directory %s)\n", xdg.Abbrev(x.File.Path), x.Dir.Name)
+	fmt.Fprintf(stdout, "%s  (directory %s)\n", display(xdg.Abbrev(x.File.Path)), x.Dir.Name)
 	if x.Skip != "" {
 		fmt.Fprintf(stdout, "krino would not look at this file: %s\n", x.Skip)
 	}
@@ -84,6 +84,6 @@ func printTrace(w io.Writer, t *cond.Trace) {
 	var buf bytes.Buffer
 	t.Format(&buf)
 	for _, line := range strings.Split(strings.TrimRight(buf.String(), "\n"), "\n") {
-		fmt.Fprintf(w, "  %s\n", line)
+		fmt.Fprintf(w, "  %s\n", display(line))
 	}
 }
diff --git a/cmd/krino/hostile_test.go b/cmd/krino/hostile_test.go
new file mode 100644
index 0000000..a2b5d39
--- /dev/null
+++ b/cmd/krino/hostile_test.go
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+// hostileNames are file names a download can carry that try to take over
+// the terminal krino prints them to, or to fake what it shows.
+var hostileNames = []string{
+	"esc\x1b[2K\x1b[1Ahidden.pdf",
+	"bell\a.pdf",
+	"cr\rspoof.pdf",
+	"c1\u009b31mred.pdf",
+	"bidi\u202egnp.pdf",
+	"new\nline.pdf",
+	"-rf.pdf",
+}
+
+// TestHostileNamesNeverReachTheTerminal: with files named to attack the
+// terminal, and an extraction tool whose error message carries an escape
+// sequence, nothing krino prints - plan, verbose lists, warnings, explain,
+// apply, log, undo plan - holds a raw control character.
+func TestHostileNamesNeverReachTheTerminal(t *testing.T) {
+	h := home(t)
+	dl := filepath.Join(h, "dl")
+	if err := os.MkdirAll(dl, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
+	for _, n := range hostileNames {
+		p := filepath.Join(dl, n)
+		if err := os.WriteFile(p, []byte("content of a hostile file"), 0o644); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Chtimes(p, old, old); err != nil {
+			t.Fatal(err)
+		}
+	}
+	bin := filepath.Join(h, "bin")
+	if err := os.Mkdir(bin, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	tool := "#!/bin/sh\nprintf 'bad \\033[2Jpdf\\n' >&2\nexit 1\n"
+	if err := os.WriteFile(filepath.Join(bin, "pdftotext"), []byte(tool), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("PATH", bin)
+	if code, _, errOut := runCLI(t, "init"); code != 0 {
+		t.Fatal(errOut)
+	}
+	if code, _, errOut := runCLI(t, "new", "dl", dl); code != 0 {
+		t.Fatal(errOut)
+	}
+	rules := "(path \"~/dl\")\n(rule \"read\" (when (content \"acme\")) (stop))\n(rule \"all\" (move \"Out\"))\n"
+	if err := os.WriteFile(filepath.Join(h, ".config", "krino", "dirs", "dl.conf"), []byte(rules), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	check := func(args ...string) string {
+		t.Helper()
+		code, out, errOut := runCLI(t, args...)
+		if code != 0 {
+			t.Fatalf("krino %q: exit %d\n%s\n%s", args, code, out, errOut)
+		}
+		for stream, text := range map[string]string{"stdout": out, "stderr": errOut} {
+			if bad := firstUnsafe(text, true); bad != "" {
+				t.Errorf("krino %q: %s holds %s:\n%q", args, stream, bad, text)
+			}
+		}
+		return out
+	}
+	out := check("-n", "-v")
+	if !strings.Contains(out, `esc\x1b[2K\x1b[1Ahidden.pdf`) || !strings.Contains(out, `bad \x1b[2Jpdf`) {
+		t.Errorf("names and the tool's message should be shown escaped:\n%s", out)
+	}
+	check("explain", filepath.Join(dl, hostileNames[0]))
+	check("-y")
+	check("log")
+	check("undo", "-n")
+}
diff --git a/cmd/krino/log.go b/cmd/krino/log.go
index bcbe1d0..6904a93 100644
--- a/cmd/krino/log.go
+++ b/cmd/krino/log.go
@@ -107,7 +107,7 @@ func styleUndone(line string, p palette) string {
 // the directories it touched, and its counts (see countsText), with
 // "(undone)" appended when a later run has reversed it.
 func formatRun(r journal.Run) string {
-	line := fmt.Sprintf("%s  %s  %s  %s", r.ID, r.Start.Format("2006-01-02 15:04"), strings.Join(r.Dirs, ", "), countsText(r.Counts))
+	line := fmt.Sprintf("%s  %s  %s  %s", r.ID, r.Start.Format("2006-01-02 15:04"), display(strings.Join(r.Dirs, ", ")), countsText(r.Counts))
 	if r.Undone {
 		line += "  (undone)"
 	}
diff --git a/cmd/krino/render.go b/cmd/krino/render.go
index f94b15a..50e52b0 100644
--- a/cmd/krino/render.go
+++ b/cmd/krino/render.go
@@ -78,7 +78,7 @@ func printPlan(w io.Writer, dp *engine.DirPlan, verbose bool, p palette, width i
 			fmt.Fprintln(w)
 			fmt.Fprintln(w, "not matched")
 			for _, fm := range r.Unmatched {
-				fmt.Fprintf(w, "  %s\n", fm.File.Rel)
+				fmt.Fprintf(w, "  %s\n", display(fm.File.Rel))
 			}
 		}
 		if len(r.Skipped) > 0 {
@@ -108,8 +108,8 @@ func excludedLines(r *engine.Result, chains []plan.Chain) []string {
 		if reason == "" && len(fm.Rules) > 0 {
 			reason = "rule " + fm.Rules[len(fm.Rules)-1].Rule.Name
 		}
-		rels = append(rels, c.File.Rel)
-		why = append(why, reason)
+		rels = append(rels, display(c.File.Rel))
+		why = append(why, display(reason))
 	}
 	width := relWidth(rels)
 	out := make([]string, len(rels))
@@ -175,7 +175,7 @@ func printBlocks(w io.Writer, chains []plan.Chain, root string, p palette, width
 		i++
 		fmt.Fprintln(w)
 		head := "  " + padLeft(strconv.Itoa(i), numW) + "  "
-		for _, l := range wrapped(head, c.File.Rel, indent, width, plainText) {
+		for _, l := range wrapped(head, display(c.File.Rel), indent, width, plainText) {
 			fmt.Fprintln(w, l)
 		}
 		for _, l := range stepLines(c, indent, root, p, width) {
@@ -201,9 +201,9 @@ func stepLines(c plan.Chain, indent int, root string, p palette, width int) []st
 		if i+1 < len(c.Steps) && c.Steps[i+1].Rule == s.Rule {
 			continue
 		}
-		out = append(out, field(indent, "rule", s.Rule, width, plainText, p.rule)...)
+		out = append(out, field(indent, "rule", display(s.Rule), width, plainText, p.rule)...)
 		if s.Reason != "" && s.Reason != "no condition" {
-			out = append(out, field(indent, "because", s.Reason, width, plainText, p.faint)...)
+			out = append(out, field(indent, "because", display(s.Reason), width, plainText, p.faint)...)
 		}
 	}
 	return out
@@ -215,13 +215,13 @@ func stepLines(c plan.Chain, indent int, root string, p palette, width int) []st
 // note when the step replaces an existing file.
 func stepValue(s plan.Step, root string) string {
 	if s.Skip != "" {
-		return "skipped: " + s.Skip
+		return "skipped: " + display(s.Skip)
 	}
 	switch s.Kind {
 	case plan.Trash, plan.DeletePermanent:
 		return ""
 	}
-	v := "→ " + destText(s, root)
+	v := "→ " + display(destText(s, root))
 	if s.Displaces != "" {
 		v += " (replaces the existing file)"
 	}
diff --git a/cmd/krino/review.go b/cmd/krino/review.go
index 36231be..dcf7663 100644
--- a/cmd/krino/review.go
+++ b/cmd/krino/review.go
@@ -122,7 +122,7 @@ func reviewPerFile(in io.Reader, out io.Writer, chains []plan.Chain, root string
 			continue
 		}
 
-		fmt.Fprintf(out, "\n[%d/%d] %s\n", i+1, len(chains), c.File.Rel)
+		fmt.Fprintf(out, "\n[%d/%d] %s\n", i+1, len(chains), display(c.File.Rel))
 		for _, l := range stepLines(c, 7, root, p, widthPolicy(out)) {
 			fmt.Fprintln(out, l)
 		}
@@ -157,7 +157,7 @@ func reviewPerFile(in io.Reader, out io.Writer, chains []plan.Chain, root string
 				replaced[c.File.Rel] = plan.Trash
 				choice = "trash"
 			case 'd':
-				fmt.Fprintf(out, "  delete %s permanently? [y/N] ", c.File.Rel)
+				fmt.Fprintf(out, "  delete %s permanently? [y/N] ", display(c.File.Rel))
 				confirm, kerr := readKey(in)
 				if kerr != nil {
 					return nil, nil, 0, kerr
diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go
index 04fd3e8..35c242c 100644
--- a/cmd/krino/sort.go
+++ b/cmd/krino/sort.go
@@ -176,7 +176,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 			// line above, not before it, so on a terminal they read as
 			// describing the directory just named instead of floating above it.
 			for _, w := range dp.Result.Warnings {
-				fmt.Fprintf(stderr, "krino: %s: %s\n", d.Name, w)
+				fmt.Fprintf(stderr, "krino: %s: %s\n", d.Name, display(w))
 			}
 			if g.json {
 				// --json is only ever reached with -n (checked above), and
@@ -457,11 +457,11 @@ func warnedCount(lines []warnLine) int {
 func printWarnings(w io.Writer, lines []warnLine, p palette, width int) {
 	rels := make([]string, len(lines))
 	for i, l := range lines {
-		rels[i] = l.rel
+		rels[i] = display(l.rel)
 	}
 	relW := relWidth(rels)
 	for _, l := range lines {
-		for _, piece := range wrapped("  ", padCell(l.rel, relW)+"  "+l.text, 4, width, p.warn) {
+		for _, piece := range wrapped("  ", padCell(display(l.rel), relW)+"  "+display(l.text), 4, width, p.warn) {
 			fmt.Fprintln(w, piece)
 		}
 	}
@@ -472,11 +472,11 @@ func printWarnings(w io.Writer, lines []warnLine, p palette, width int) {
 func printSkipped(w io.Writer, skipped []scan.Skipped) {
 	rels := make([]string, len(skipped))
 	for i, s := range skipped {
-		rels[i] = s.Rel
+		rels[i] = display(s.Rel)
 	}
 	width := relWidth(rels)
 	for _, s := range skipped {
-		fmt.Fprintf(w, "  %s  %s\n", padCell(s.Rel, width), s.Reason.String())
+		fmt.Fprintf(w, "  %s  %s\n", padCell(display(s.Rel), width), s.Reason.String())
 	}
 }
 
diff --git a/cmd/krino/undo.go b/cmd/krino/undo.go
index 5e18a7e..b7a139f 100644
--- a/cmd/krino/undo.go
+++ b/cmd/krino/undo.go
@@ -375,12 +375,12 @@ func reviewUndoPerFile(in io.Reader, out io.Writer, files []engine.UndoFile, p p
 	approved = map[int]bool{}
 	yesRest := false
 	for i, f := range files {
-		fmt.Fprintf(out, "\n[%d/%d] %s/%s\n", i+1, len(files), f.Dir, f.File)
+		fmt.Fprintf(out, "\n[%d/%d] %s/%s\n", i+1, len(files), display(f.Dir), display(f.File))
 		for _, s := range f.Steps {
 			fmt.Fprintf(out, "       %s\n", undoActionCell(s))
 		}
 		if f.Refused != "" {
-			fmt.Fprintf(out, "  refused: %s\n", f.Refused)
+			fmt.Fprintf(out, "  refused: %s\n", display(f.Refused))
 			continue
 		}
 		if yesRest {
@@ -459,15 +459,15 @@ const undoStepWidth = len("undo-displace")
 // no root-relative form to render here the way actionCell has.
 func undoActionCell(s engine.UndoStep) string {
 	if s.Refused != "" {
-		return padCell(s.Action, undoStepWidth) + "  refused: " + s.Refused
+		return padCell(s.Action, undoStepWidth) + "  refused: " + display(s.Refused)
 	}
 	switch s.Action {
 	case "undo-mkdir":
-		return padCell(s.Action, undoStepWidth) + "  " + xdg.Abbrev(s.Src)
+		return padCell(s.Action, undoStepWidth) + "  " + display(xdg.Abbrev(s.Src))
 	case "undo-copy":
-		return padCell(s.Action, undoStepWidth) + "  " + xdg.Abbrev(s.Src) + " → trash"
+		return padCell(s.Action, undoStepWidth) + "  " + display(xdg.Abbrev(s.Src)) + " → trash"
 	}
-	return padCell(s.Action, undoStepWidth) + "  → " + xdg.Abbrev(s.Dst)
+	return padCell(s.Action, undoStepWidth) + "  → " + display(xdg.Abbrev(s.Dst))
 }
 
 // printUndoPlan renders an undo plan the way krino undo shows it, below the
@@ -497,9 +497,9 @@ type undoRow struct {
 func undoRows(files []engine.UndoFile) []undoRow {
 	var rows []undoRow
 	for i, f := range files {
-		label := f.Dir + "/" + f.File
+		label := display(f.Dir + "/" + f.File)
 		if f.Refused != "" {
-			rows = append(rows, undoRow{num: strconv.Itoa(i + 1), file: label, action: "refused: " + f.Refused})
+			rows = append(rows, undoRow{num: strconv.Itoa(i + 1), file: label, action: "refused: " + display(f.Refused)})
 			continue
 		}
 		for j, s := range f.Steps {