Skip to content

Escape control characters in the tong approval prompt - #93

Merged
CrypticSwarm merged 2 commits into
masterfrom
approval-prompt-escape
Oct 9, 2026
Merged

CrypticSwarm merged 2 commits into
masterfrom
approval-prompt-escape

Conversation

@CrypticSwarm

Copy link
Copy Markdown
Owner

Stacked on #92 (GPU support for tongs): this PR also escapes the approval prompt's new gpus: line. Merge #92 first; GitHub will then retarget this PR to master.

Problem

The first-run approval gate shows what a workspace-sourced tong asks for (image, secret references, mounts, networks, docker socket, GPUs) before asking y/N. Those values come straight from a definition in a repo you cloned. The YAML reader passes raw control bytes through, and the gate runs before schema validation, so even a value validation would later reject reached the terminal unescaped. A crafted value could move the cursor, erase lines, or switch on concealed text, and so hide a grant such as the docker socket or a secret reference from the person approving it. A secret reference placed under an unknown key is enough: validation tolerates unknown keys and the summary scans the whole definition. This was already the case on master; a review of the GPU work found it.

What changes

  • tongs.printable(text) (swarmforge/tongs/model.py): replaces every character str.isprintable() rejects with its Python escape (\x1b, \n, ‮). That covers C0 and C1 controls, DEL, bidi overrides, zero-width/format characters, and non-ASCII spaces. Ordinary text, including non-ASCII letters, prints unchanged, so a normal prompt looks exactly as before.
  • Approval summary: render_privilege_summary escapes each finished line before joining them, so the summary keeps its own line structure and any line added later is covered without anyone remembering to wrap it. The y/N question and the denial messages already render the tong name with %r, which escapes the same set.
  • Skipped-tong warning: discovery prints a bad file's path before the gate runs, and a workspace file name could switch on a terminal mode that hides what follows. That warning now goes through printable too.
  • Docs: docs/tongs/approval.md says values are shown with control and formatting characters escaped.

What is guaranteed: nothing the prompt prints from a definition can move the cursor, restyle the text, break a line, or reverse the text direction. Lookalike characters Python counts as printable (blank Hangul fillers, combining overlays) pass through unchanged. They can't hide a grant: each grant has its own fixed label line, and approval is keyed to the hash of the exact definition. Backslashes are left alone, so ordinary values look the same.

Testing

  • make test: 959 tests pass. ruff check is clean.
  • Tests check that an escape sequence in every rendered field (image, secret provider/ref including one under an unknown key, mounts, networks, gpus) comes out escaped and that a newline cannot forge a line. They also check that the socket line still appears, that plain and non-ASCII values render unchanged, and that the discovery warning is escaped. Removing the escaping fails 5 approval tests and the discovery test.
  • During review, every non-printable code point was pushed through each rendered field, and hostile file names were tried end to end through bin/run-anvil. Nothing reached the terminal unescaped before the y/N answer.

Not in this PR

  • Output printed before the launcher starts (the Makefile's set -x echo, git-guard warnings) is still raw.
  • A few warnings after the gate print the tong name with '%s'; that name comes from a trusted layer or a file name.

The privilege summary printed before the y/N question interpolated
values straight from a workspace tong definition, which is untrusted,
and the YAML reader passes raw control bytes through. A value carrying
terminal escape sequences could therefore hide what the reviewer is
being asked to approve: cursor movement can rewrite or erase earlier
lines, and a mode switch such as concealed text hides every line after
it. Either way a grant (a secret, a mount, the docker socket, the host's
GPUs) could be approved unseen. The gate runs before schema validation,
and secret references are collected from anywhere in the definition,
unknown keys included, so a value that is never validated or resolved
still reached the terminal.

Every rendered line now passes through a new `tongs.printable`, which
shows each non-printable character as its Python escape (`\x1b`, `\n`,
`\u202e`). Escaping whole lines rather than each interpolated value
means a field added to the summary later is covered without anyone
having to remember to wrap it, while the summary's own line breaks are
added after escaping and stay intact.

The cut is `str.isprintable`, so besides C0, DEL, and C1 controls it
also escapes bidi overrides, zero-width and other format characters, and
non-ASCII spaces: none belongs in an image, mount, network, or secret
reference, and each can make a value look like something it is not.
Ordinary text, non-ASCII letters included, renders unchanged. A few
printable characters are still blank or confusable, so lookalikes are
not caught; the guarantee is that no value can move the cursor, restyle,
break a line, or override the text direction. The tong name was already
shown with %r, which escapes the same set.
When a tong file fails to parse, discovery warns with its path before
the approval gate runs, on the same terminal. A workspace file name is
untrusted and can carry escape sequences, and unlike the summary's own
values these do not need to move the cursor: modes they switch on,
such as concealed text or a shrunken scroll region, persist into the
privilege summary and question printed right after, hiding them just as
effectively.

Run the warning through `printable`, the same escaping the summary now
uses. The parse error text it includes is already repr-quoted, so only
the path changes in practice.
Base automatically changed from gpu-support to master October 9, 2026 01:50
@CrypticSwarm
CrypticSwarm merged commit c325387 into master Oct 9, 2026
4 checks passed
@CrypticSwarm
CrypticSwarm deleted the approval-prompt-escape branch October 9, 2026 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant