3.0 KiB
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_NUMBERis 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
- Add failing regression tests for GitHub eval injection and Gitea invalid PR rejection.
- Implement fail-closed PR number validation before metadata lookup.
- Replace GitHub
evalcommand with argv array execution. - Run required validation and update this scratchpad with evidence.
- 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.shfailed on vulnerable code withExpected GitHub metacharacter PR number to be rejectedand showed the injected PR number reached the GitHub merge path. - GREEN: Added digits-only validation before metadata lookup and replaced GitHub
evalwith an argv array. The focused harness now passes and verifies invalid PR numbers are rejected before GitHubghcalls and before Giteatea/curlcalls.
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 --uncommittedcould 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.