Files
stack/docs/scratchpads/t_5aab9cc8-pr-merge-eval-injection.md
2026-05-26 14:59:16 -05:00

3.0 KiB

t_5aab9cc8 — pr-merge.sh eval injection remediation

Objective

Remediate PR #521 review blocker: packages/mosaic/framework/tools/git/pr-merge.sh must reject non-numeric PR numbers before metadata lookup/merge and must not use eval for GitHub merge execution.

Scope

  • Shell wrapper only: packages/mosaic/framework/tools/git/pr-merge.sh
  • Focused regression harness: packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh
  • No API/frontend/infra surfaces.

Acceptance Criteria

  • AC1: PR_NUMBER is validated as digits-only immediately after required-argument parsing, before metadata lookup.
  • AC2: GitHub merge path uses a quoted argv array, not command-string construction plus eval.
  • AC3: Focused tests prove PR-number metacharacters are rejected and cannot execute injected shell commands on GitHub path.
  • AC4: Focused tests prove PR-number metacharacters are rejected on Gitea path before tea/curl merge calls.
  • AC5: Existing Gitea empty-uid fallback behavior remains green.
  • AC6: Syntax, shellcheck where available, focused harness, and relevant repo gates are rerun or absence documented.

Plan

  1. Add failing regression tests for GitHub eval injection and Gitea invalid PR rejection.
  2. Implement fail-closed PR number validation before metadata lookup.
  3. Replace GitHub eval command with argv array execution.
  4. Run required validation and update this scratchpad with evidence.
  5. Commit, queue-guard, push branch, update PR #521.

TDD Log

  • RED: AGENT_WORK_ROOT="$HERMES_KANBAN_WORKSPACE/work" bash packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh failed on vulnerable code with Expected GitHub metacharacter PR number to be rejected and showed the injected PR number reached the GitHub merge path.
  • GREEN: Added digits-only validation before metadata lookup and replaced GitHub eval with an argv array. The focused harness now passes and verifies invalid PR numbers are rejected before GitHub gh calls and before Gitea tea/curl calls.

Validation Evidence

  • PASS: AGENT_WORK_ROOT="$HERMES_KANBAN_WORKSPACE/work" bash -n packages/mosaic/framework/tools/git/pr-merge.sh packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh
  • PASS: shellcheck -x packages/mosaic/framework/tools/git/pr-merge.sh packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh
  • PASS: AGENT_WORK_ROOT="$HERMES_KANBAN_WORKSPACE/work" bash packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh
  • PASS: pnpm --filter @mosaicstack/mosaic... build
  • PASS: pnpm --filter @mosaicstack/mosaic lint
  • PASS: pnpm --filter @mosaicstack/mosaic typecheck
  • PASS: pnpm --filter @mosaicstack/mosaic test — 32 files / 291 tests passed.
  • REVIEW: /home/hermes/.config/mosaic/tools/codex/codex-code-review.sh --uncommitted could not run due Codex 401 Unauthorized. Independent delegate review completed read-only with PASS / no blockers; non-blocking suggestion to assert GitHub mock log remains empty was applied.

Risks / Blockers

  • No active blockers.