fix(framework/tools): eval injection, broken JSON, tmpfile leak #549

Merged
jason.woltje merged 2 commits from fix/tooling-eval-injection-jq-json into main 2026-06-18 21:35:33 +00:00
Owner

Summary

Fixes three confirmed defects in packages/mosaic/framework/tools/git/.

F-01 (HIGH) — Shell injection in issue-edit.sh and issue-assign.sh

Both scripts built CLI commands via string interpolation then executed them with eval. A title/body/label containing $(id) would execute arbitrary code. Fixed by converting all eval sites to Bash arrays with proper quoting. For the Gitea path, replaced get_gitea_repo_args() (which emits %q-escaped strings designed for eval) with get_repo_slug() + get_gitea_login() passed as array elements.

F-07 (MED) — Broken JSON in milestone-create.sh

The GitHub API JSON payload was built by string interpolation. A title containing " or $ would produce malformed JSON and fail the API call. Fixed with jq -n --arg. Optional description key is omitted when empty, preserving existing behaviour.

F-13 (LOW) — Tmpfile leak in pr-metadata.sh

curl_gitea_pull() created a mktemp tmpfile but only cleaned it up in success paths. An early exit (via set -e or SIGINT) leaked the file. Fixed by adding trap 'rm -f "$body_file"' EXIT immediately after the mktemp call.

Test evidence

  • bash -n syntax check: all 4 files pass
  • F-01 injection test: TITLE='$(touch /home/hermes/agent-work/PWNED)' passed as literal string to stub CLI; PWNED file NOT created
  • F-07 JSON test: jq -n --arg produces valid JSON with titles containing ", $, and \; description key omitted when empty

No merge — awaiting orchestrator go-ahead per AGENTS.md rule 38.

Fixes #548

## Summary Fixes three confirmed defects in `packages/mosaic/framework/tools/git/`. ### F-01 (HIGH) — Shell injection in issue-edit.sh and issue-assign.sh Both scripts built CLI commands via string interpolation then executed them with `eval`. A title/body/label containing `$(id)` would execute arbitrary code. Fixed by converting all eval sites to Bash arrays with proper quoting. For the Gitea path, replaced `get_gitea_repo_args()` (which emits `%q`-escaped strings designed for eval) with `get_repo_slug()` + `get_gitea_login()` passed as array elements. ### F-07 (MED) — Broken JSON in milestone-create.sh The GitHub API JSON payload was built by string interpolation. A title containing `"` or `$` would produce malformed JSON and fail the API call. Fixed with `jq -n --arg`. Optional description key is omitted when empty, preserving existing behaviour. ### F-13 (LOW) — Tmpfile leak in pr-metadata.sh `curl_gitea_pull()` created a mktemp tmpfile but only cleaned it up in success paths. An early exit (via `set -e` or SIGINT) leaked the file. Fixed by adding `trap 'rm -f "$body_file"' EXIT` immediately after the mktemp call. ## Test evidence - `bash -n` syntax check: all 4 files pass - F-01 injection test: `TITLE='$(touch /home/hermes/agent-work/PWNED)'` passed as literal string to stub CLI; PWNED file NOT created - F-07 JSON test: `jq -n --arg` produces valid JSON with titles containing `"`, `$`, and `\`; description key omitted when empty No merge — awaiting orchestrator go-ahead per AGENTS.md rule 38. Fixes #548
jason.woltje added 1 commit 2026-06-18 18:51:39 +00:00
fix(framework/tools): eval injection, broken JSON, tmpfile leak (#548)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/push/ci Pipeline was successful
b0b2c20da0
F-01 (HIGH): issue-edit.sh and issue-assign.sh used string interpolation
+ eval to build CLI commands. Replace all eval sites with Bash arrays so
user-supplied values (title, body, labels) are never shell-expanded.
For the Gitea path, replace get_gitea_repo_args() (which emits %q-escaped
strings designed for eval) with get_repo_slug() + get_gitea_login() so
repo/login are passed as properly-quoted array elements.

F-07 (MED): milestone-create.sh built the GitHub API JSON payload by
string interpolation — a title containing " or $ broke the JSON. Rebuild
with jq -n --arg so all values are safely serialised. Optional description
key is omitted when empty, preserving existing behaviour.

F-13 (LOW): pr-metadata.sh created a mktemp tmpfile inside
curl_gitea_pull() but only removed it in success paths. Add
trap 'rm -f "$body_file"' EXIT immediately after mktemp so early-exit
paths (set -e, SIGINT) also clean up.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kt2D8TsnDwhtzEAPijsNmR
jason.woltje added 1 commit 2026-06-18 21:16:42 +00:00
fix git pr metadata tmpfile cleanup
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
5cd6c83b9b
jason.woltje merged commit 719c6ac3db into main 2026-06-18 21:35:33 +00:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#549