Skip to content

security: sanitize terminal control bytes in scaffold/CLI output #66

Description

@Lillevang

Problem

agent-init prints several values to the terminal that are influenced by the invocation environment — the target path, the project name (filepath.Base(target)), and symlink targets — without stripping terminal control bytes. If any such value contains raw ANSI/control sequences (e.g. a directory named with \x1b]0;...\x07 for a title-bar spoof, or \x1b[2J to clear the screen, or OSC-8 hyperlink sequences), those bytes are emitted verbatim to the user's terminal and interpreted.

This is a latent terminal escape-sequence injection exposure. It is mostly pre-existing — the code already printed target and file paths unsanitized before any color work — and was surfaced during the security review of #64 (scaffold-output cleanup). #64 itself does not meaningfully worsen it (its ANSI styling uses only constant escape sequences, never interpolated input, and it actually reduces attacker-influenced content in the symlink lines), so this is filed as a separate repo-wide hardening rather than a gate on that PR.

Why it matters

agent-init is a tool people run to scaffold repositories. The primary threat model — a developer scaffolding their own repo, controlling the directory name — makes this low severity: reaching the dangerous case requires scaffolding into an attacker-supplied path or project name (e.g. a name pasted from an untrusted source, or agent-init init <flavor> <attacker-controlled-dir>). It is nonetheless a real terminal-injection vector (title spoofing, cursor/screen manipulation, hidden-text and hyperlink abuse) and cheap to close defensively. Output sanitization is the right layer because the styling is now TTY-aware, i.e. emitted precisely where escape sequences are interpreted.

Affected output sites (current, in internal/scaffold/scaffold.go)

  • The "Scaffolding ... in: <target>" header (interpolates target).
  • The templateData.ProjectName (= filepath.Base(target)) wherever printed.
  • link / dry-run link lines that interpolate the symlink dest (and, historically, target-derived display paths).
  • Any other fmt.Fprintf(out, "... %s ...", <path-or-name>) of a path/name value — audit the package (and internal/cli) for the full set rather than fixing only the sites above.

Note: the symlink display left-hand side now comes from the hardcoded Symlink.Path (trusted), and symlink targets come from flavor.Symlinks in registry.go (also hardcoded), so those specific values are not attacker-influenced. The interpolation of target / ProjectName is the live surface.

Suggested approach

  • Add a small, dependency-free sanitizer (e.g. a strings.Map or equivalent) that strips or escapes C0/C1 control bytes and ESC (0x1b) from any user/environment-influenced string before it is written to output. Preserve ordinary printable text (and tab/newline where intended).
  • Apply it at the print sites that interpolate paths/names in internal/scaffold and internal/cli, not just the ones fix: clean up scaffold output #64 touched.
  • Keep the constant ANSI styling escapes intact — the sanitizer applies to interpolated input, not to the tool's own formatting.

Acceptance criteria

  • A control-byte sanitizer exists (dependency-free, stdlib only) and is unit-tested: input containing \x1b, OSC (\x1b]...\x07 / ...ST), CSI (\x1b[...), and other C0/C1 control bytes is neutralized; ordinary UTF-8 text (including non-ASCII letters) passes through unchanged.
  • All scaffold/CLI output sites that interpolate a path, project name, or other environment-influenced string route that value through the sanitizer before writing. (Audit internal/scaffold and internal/cli; the tool's own constant ANSI/format strings are exempt.)
  • A regression test scaffolds (or exercises the output path) with a target/project name containing an escape sequence and asserts no raw \x1b/control bytes reach the writer.
  • The tool's own coloring/formatting still renders correctly for legitimate input (existing output tests stay green; TestRunCapturedOutputContainsNoANSI and the color-gating tests remain valid).
  • ./.agent/scripts/check.sh passes.

Context

Surfaced by the security review of #64. Severity: Low (defensive hardening; mostly pre-existing exposure; bounded by the dev-scaffolds-own-repo threat model). No external dependency should be introduced — the repo is stdlib-first.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:cliCLI surface, flags, help outputarea:securitySecrets, credentials, sandbox posture

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions