Skip to content

describe-change.sh corrupts titles containing & on bash 5.2+ #658

Description

@williamthorsen

Problem

describe-change.sh silently corrupts any input containing & when running on bash 5.2 or newer. A title like Add horizontal scroll, edge fade & design polish to cycle list renders as Add horizontal scroll, edge fade {title} design polish to cycle list. The & is replaced with the literal string {title}.

The script uses bash parameter expansion (${s//pattern/replacement}), not sed, so the failure mode is easy to miss when reviewing the code.

Context

Bash 5.2 introduced the patsub_replacement shell option, which makes & in the replacement of pattern substitutions expand to the matched text (the behaviour sed has always had). The option is on by default, so any script that previously assumed & was literal in ${s//pat/repl} now silently corrupts data on bash 5.2+.

The vulnerable substitutions are in describe-change.sh's substitute_tokens function, which places caller-supplied values (scope, type, title, ticket_ref, pr_number) on the replacement side of pattern substitution. The title field is the one most likely to contain & in real use.

An audit of every ${var//pat/repl} site across packages/agents/content/scripts/ found describe-change.sh to be the only script that puts caller-supplied data on the replacement side. The other scripts (resolve-merge-options.sh, resolve-frontmatter.sh, get-ticket-id.sh, resolve-reviewer-context.sh) use only static replacements (JSON escape sequences, slug normalization, YAML quoting), so none are vulnerable today.

Reproduction

bash -c 's="prefix {title} suffix"; title="Add A & B"; s="${s//\{title\}/$title}"; echo "$s"'
# Bash 5.2+: prefix Add A {title} B suffix
# Bash <=5.1: prefix Add A & B suffix

Proposed solution

Disable patsub_replacement within describe-change.sh's own scope so & is treated as literal in the replacement side of every parameter substitution. The fix must remain a no-op on bash 5.1 and earlier, where the option does not exist.

Codify the gotcha in the shell-conventions skill so future scripts that grow caller-supplied replacements catch the pitfall before reintroducing the same class of bug.

Add a regression test that exercises a &-containing title and asserts the character is preserved verbatim through rendering.

Acceptance criteria

  • describe-change.sh preserves & characters in caller-supplied token values when run on bash 5.2+.
  • The fix is compatible with bash 5.1 and earlier (the option-toggling code does not error on shells that lack the option).
  • A regression test in __tests__/describe_change_test.sh exercises a &-containing title and asserts the output preserves the character.
  • The shell-conventions skill's "Common mistakes" table documents the ${var//pat/repl} + dynamic-replacement pitfall.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions