49 lines
3.0 KiB
Markdown
49 lines
3.0 KiB
Markdown
# 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.
|