mosaicstack/stack
1
0
Fork 1
Code Issues 170 Pull Requests 9 Actions Packages Projects Releases 15 Wiki Activity

git wrappers: issue-create.sh / issue-comment.sh / pr-review.sh have no -F/--body-file, so every long body is routed through shell interpolation that silently rewrites it #988

New Issue
Open
opened 2026-07-31 08:06:43 +00:00 by mos-dt-0 · 0 comments
mos-dt-0 commented 2026-07-31 08:06:43 +00:00
Collaborator
Copy Link
Copy Source

Summary

issue-create.sh, issue-comment.sh, and pr-review.sh accept a body only as a shell argument.
There is no -F/--body-file. Every long body — a review verdict, a root-cause report, an issue with a
code block — therefore has to be written as -b "$(cat file)", which routes durable, durable-forever
content through one extra layer of shell interpolation, and that layer rewrites content silently.

Measured arg parsers (2026-07-31):

issue-create.sh:84-104    -t|--title  -b|--body  -l|--labels  -m|--milestone  -i|--interactive  -h|--help
issue-comment.sh:37-49    -i|--issue  -c|--comment  -l|--login  -h|--help
pr-review.sh:48-72        -n|--number -a|--action -c|--comment -l|--login -r|--repo -H|--host -h|--help

No file-input option on any of the three.

Why this is a correctness problem, not an ergonomics one

Measured on this host through a double-quoted body argument. All four rows are the same message going
through the same wrapper; only the quoting differs:

Form Written Received Does the sender see a problem?
$N $160k 60k no — silent deletion
backtick the shape `A && B || C` binds the shape binds partly — stderr A: command not found
$( ) cost was $(echo 99)k cost was 99k no — fully silent substitution
single-quoted any of the above delivered intact n/a

$( ) is the worst of the three: it neither errors nor deletes — it substitutes real command output
into your prose, so the record stays fluent and plausible while saying something the author did not
write. The author's copy and the posted copy differ, and only the reader can tell.

This is not hypothetical on this fleet. A peer agent's technical explanation lost its subject in transit
on 2026-07-31 — the sharper one: binds the guard to init's FUTURE return behaviour — because the
sending shell command-substituted a backticked code span. That was a message. The same class landing in
an issue body or a review verdict is durable and is read later as authoritative.

Bodies that discuss shell tooling are exactly the bodies most likely to contain $, a backtick, or
$( — so the hazard concentrates precisely where these wrappers are used most.

Note on the existing mitigation

issue-create.sh already builds its payload through Python with the body passed via the environment
(BODY="$BODY" python3 - <<'PY'), and there is a test-issue-create-body-safety.sh. That protects
the wrapper's own internals and does not reach this defect — by the time $BODY is set, the calling
shell has already expanded the argument. No change inside the wrapper can recover the lost bytes. The
only fix is an input path that never passes through shell quoting.

Proposed fix

Add -F/--body-file <path> (and -F - for stdin) to issue-create.sh, issue-comment.sh, and
pr-review.sh, reading the file verbatim. gh has had --body-file for years, for this reason.

Mutually exclusive with -b/-c; supplying both should be an error rather than a precedence rule.

Acceptance criterion — one needle per wrapper, since a shared helper still needs to be wired at each
call site. For each of the three, post a body containing all three hazard forms from a file and read
the artifact back from the API (not from the wrapper's own echo, which shows the author's intent
rather than what was stored), asserting the three forms survive byte-for-byte.

Not a duplicate

Checked against the full corpus (985 issues, #1–#986, paginated — the API caps at 50 per page regardless
of limit). The 21 matches for body-file/-F are all incidental. Same family as #867
(pr-metadata/pr-merge lack -r) and #954 (issue-view.sh lacks -r) — wrapper input-surface gaps —
but a different tool and a different flag, and unlike those two this one can corrupt content rather than
target the wrong repo.

Reported by mos-dt (sb-it-1-dt). Filed through issue-create.sh, so the account on this record is
mos-dt-0 (id 13) — a shared per-host identity, not an author identity. The in-body signature is a
labelled claim, never provenance.

## Summary `issue-create.sh`, `issue-comment.sh`, and `pr-review.sh` accept a body **only as a shell argument**. There is no `-F/--body-file`. Every long body — a review verdict, a root-cause report, an issue with a code block — therefore has to be written as `-b "$(cat file)"`, which routes durable, durable-forever content through **one extra layer of shell interpolation**, and that layer rewrites content silently. Measured arg parsers (2026-07-31): ``` issue-create.sh:84-104 -t|--title -b|--body -l|--labels -m|--milestone -i|--interactive -h|--help issue-comment.sh:37-49 -i|--issue -c|--comment -l|--login -h|--help pr-review.sh:48-72 -n|--number -a|--action -c|--comment -l|--login -r|--repo -H|--host -h|--help ``` No file-input option on any of the three. ## Why this is a correctness problem, not an ergonomics one Measured on this host through a double-quoted body argument. All four rows are the same message going through the same wrapper; only the quoting differs: | Form | Written | Received | Does the sender see a problem? | |---|---|---|---| | `$N` | `$160k` | `60k` | **no** — silent deletion | | backtick | ``the shape `A && B \|\| C` binds`` | `the shape binds` | partly — stderr `A: command not found` | | `$( )` | `cost was $(echo 99)k` | `cost was 99k` | **no** — fully silent substitution | | single-quoted | any of the above | delivered intact | n/a | `$( )` is the worst of the three: it neither errors nor deletes — it **substitutes real command output into your prose**, so the record stays fluent and plausible while saying something the author did not write. The author's copy and the posted copy differ, and **only the reader can tell**. This is not hypothetical on this fleet. A peer agent's technical explanation lost its subject in transit on 2026-07-31 — `the sharper one: binds the guard to init's FUTURE return behaviour` — because the sending shell command-substituted a backticked code span. That was a message. The same class landing in an **issue body or a review verdict** is durable and is read later as authoritative. Bodies that discuss shell tooling are exactly the bodies most likely to contain `$`, a backtick, or `$(` — so the hazard concentrates precisely where these wrappers are used most. ## Note on the existing mitigation `issue-create.sh` already builds its payload through Python with the body passed via the environment (`BODY="$BODY" python3 - <<'PY'`), and there is a `test-issue-create-body-safety.sh`. **That protects the wrapper's own internals and does not reach this defect** — by the time `$BODY` is set, the calling shell has already expanded the argument. No change inside the wrapper can recover the lost bytes. The only fix is an input path that never passes through shell quoting. ## Proposed fix Add `-F/--body-file <path>` (and `-F -` for stdin) to `issue-create.sh`, `issue-comment.sh`, and `pr-review.sh`, reading the file verbatim. `gh` has had `--body-file` for years, for this reason. Mutually exclusive with `-b`/`-c`; supplying both should be an error rather than a precedence rule. **Acceptance criterion — one needle per wrapper, since a shared helper still needs to be wired at each call site.** For each of the three, post a body containing all three hazard forms from a file and read the artifact back **from the API** (not from the wrapper's own echo, which shows the author's intent rather than what was stored), asserting the three forms survive byte-for-byte. ## Not a duplicate Checked against the full corpus (985 issues, #1–#986, paginated — the API caps at 50 per page regardless of `limit`). The 21 matches for `body-file`/`-F` are all incidental. Same *family* as `#867` (pr-metadata/pr-merge lack `-r`) and `#954` (issue-view.sh lacks `-r`) — wrapper input-surface gaps — but a different tool and a different flag, and unlike those two this one can corrupt content rather than target the wrong repo. --- Reported by **mos-dt** (sb-it-1-dt). Filed through `issue-create.sh`, so the account on this record is `mos-dt-0` (id 13) — a shared per-host identity, not an author identity. The in-body signature is a labelled claim, never provenance.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
Defect in framework tooling or delivered code
 fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
be-coder-05 be-coder-06 be-coder-07 be-coder-08 coder-mos1 coder-mos2 f10-coder jason.woltje (Jason Woltje) merge-gate mos-dt-0 pepper rev-974 (Rev-974 (Mosaic reviewer seat, web1)) rev-security-02
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#988