mosaicstack/stack
1
0
Fork 0
Code Issues 22 Pull Requests 7 Actions Packages Projects Releases 13 Wiki Activity

fix(mosaic): harden Gitea pr merge fallback #521

Open
jason.woltje wants to merge 2 commits from fix/t_301e4e3b-pr-merge-gitea-empty-uid into main
pull from: fix/t_301e4e3b-pr-merge-gitea-empty-uid
merge into: mosaicstack:main
Conversation 1 Commits 2 Files Changed 4 +424 -6
jason.woltje commented 2026-05-22 21:04:40 +00:00
Owner
Copy Link

Problem: pr-merge.sh preferred tea pr merge whenever a host tea login existed. On Gitea, tea identity preflight can succeed while non-interactive tea merge fails with the known empty identity error, permanently blocking a valid squash merge.

Change:

  • Preserve policy gates: base main, squash-only, queue guard before real merges.
  • Preserve existing authenticated Gitea API merge fallback for no tea login.
  • When tea login exists, fall back to authenticated Gitea API merge only when tea output matches the known empty uid/name user failure class.
  • Unknown tea failures remain blocking and print the tea error; they do not silently API-merge.

Exact fallback trigger:

  • user does not exist [uid: 0, name: ]
  • equivalent empty uid/name wording from Gitea/tea for the same known failure class

Tests/evidence:

  • bash -n packages/mosaic/framework/tools/git/pr-merge.sh
  • shellcheck -x packages/mosaic/framework/tools/git/pr-merge.sh packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh
  • bash packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh
  • pnpm typecheck
  • pnpm lint
  • pnpm format:check
  • pnpm --filter @mosaicstack/mosaic test
  • Full pnpm test was attempted and hit out-of-scope gateway DB setup failure in apps/gateway/src/tests/cross-user-isolation.test.ts: Postgres relation messages does not exist. No gateway/DB files were modified.
  • /home/hermes/.config/mosaic/tools/git/ci-queue-wait.sh --purpose push completed before push.

Tracking:

Problem: pr-merge.sh preferred tea pr merge whenever a host tea login existed. On Gitea, tea identity preflight can succeed while non-interactive tea merge fails with the known empty identity error, permanently blocking a valid squash merge. Change: - Preserve policy gates: base main, squash-only, queue guard before real merges. - Preserve existing authenticated Gitea API merge fallback for no tea login. - When tea login exists, fall back to authenticated Gitea API merge only when tea output matches the known empty uid/name user failure class. - Unknown tea failures remain blocking and print the tea error; they do not silently API-merge. Exact fallback trigger: - user does not exist [uid: 0, name: ] - equivalent empty uid/name wording from Gitea/tea for the same known failure class Tests/evidence: - bash -n packages/mosaic/framework/tools/git/pr-merge.sh - shellcheck -x packages/mosaic/framework/tools/git/pr-merge.sh packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh - bash packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh - pnpm typecheck - pnpm lint - pnpm format:check - pnpm --filter @mosaicstack/mosaic test - Full pnpm test was attempted and hit out-of-scope gateway DB setup failure in apps/gateway/src/__tests__/cross-user-isolation.test.ts: Postgres relation messages does not exist. No gateway/DB files were modified. - /home/hermes/.config/mosaic/tools/git/ci-queue-wait.sh --purpose push completed before push. Tracking: - Kanban implementation task: t_301e4e3b - Parent Kanban task: t_245ad9d8 - Issue: #520
jason.woltje added 1 commit 2026-05-22 21:04:41 +00:00
fix(mosaic): harden Gitea pr merge fallback (#520)
Some checks failed
ci/woodpecker/push/ci Pipeline failed
Details
ci/woodpecker/pr/ci Pipeline failed
Details
893dd19efb
jason.woltje commented 2026-05-22 21:11:57 +00:00
Author
Owner
Copy Link

DO-178B Review: PR #521 — fix(mosaic): harden Gitea pr merge fallback

Software Level: C (Mosaic delivery wrapper/business-critical tooling)
Reviewer: ultron
Base: 755df9079e
Head reviewed: 893dd19efb
Scope: packages/mosaic/framework/tools/git/pr-merge.sh, packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh, docs/scratchpads/t_301e4e3b-pr-merge-gitea-empty-uid.md

Objective summary:
1 Requirements Traceability: PASS — PR body links t_245ad9d8/t_301e4e3b and issue #520.
2 Derived Requirements: PASS — API fallback safety behavior documented.
3 Configuration Management: PASS — branch targets main; head pinned above; scoped files only.
4 Functional Correctness: PASS for Gitea fallback — known empty uid path falls back; arbitrary tea failure blocks.
5 Structural Coverage: PASS for focused shell harness; broader coverage not applicable to shell wrapper.
6 Security Analysis: FAIL — eval injection remains in GitHub path of touched wrapper.
7 Data Integrity: PASS — merge action remains guarded by squash/main/queue checks before execution.
8 Robustness: PASS — fallback handles no-login and known tea identity failure; unknown failures fail closed.
9 Code Quality: FAIL — eval-based command construction remains in pr-merge.sh.
10 Framework Compliance: PASS — Mosaic squash/main/queue policies preserved for Gitea path.
11 Regression Protection: PASS for focused checks; full pnpm test failure documented as out-of-scope gateway DB setup.
12 Hands-Off Verification: PASS — diff is limited to declared wrapper/test/scratchpad scope.

Findings:

🔴 [BLOCKER] packages/mosaic/framework/tools/git/pr-merge.sh:193-195 — GitHub merge path still builds a command string with unvalidated PR_NUMBER and executes it via eval.

The review scope explicitly required verifying that shell quoting avoids eval-style injection risks. The Gitea path was fixed to quote arguments, but the same touched wrapper still has:

CMD="gh pr merge $PR_NUMBER --squash"
[[ "$DELETE_BRANCH" == true ]] && CMD="$CMD --delete-branch"
eval "$CMD"

A mocked GitHub-platform probe with -n '123; touch <sentinel> #' executed the injected command before reporting success. This is a command injection risk in a privileged delivery wrapper and fails the required shell-quoting check.

Remediation:

  • Validate PR_NUMBER before any metadata lookup/merge, e.g. [[ "$PR_NUMBER" =~ ^[0-9]+$ ]] || exit 1.
  • Replace the GitHub eval path with an argv array, e.g. cmd=(gh pr merge "$PR_NUMBER" --squash); [[ "$DELETE_BRANCH" == true ]] && cmd+=(--delete-branch); "${cmd[@]}".
  • Add a focused regression proving metacharacters in -n/--number are rejected and do not execute on both GitHub and Gitea paths.

Verification evidence run by reviewer:

  • bash -n packages/mosaic/framework/tools/git/pr-merge.sh
  • bash -n packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh
  • shellcheck -x packages/mosaic/framework/tools/git/pr-merge.sh packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh — PASS
  • bash packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh — PASS
  • Focused guard probe: non-main base exits before tea/API merge; non-squash method exits before external calls — PASS
  • Focused injection probe: GitHub eval path executed injected sentinel command — FAIL
  • Codex automated review wrapper attempted but unavailable due 401 Unauthorized (/home/hermes/.config/mosaic/tools/codex/codex-code-review.sh -b origin/main -o ...). Independent subagent review completed and agreed the Gitea-specific behavior is acceptable but GitHub eval is a material residual issue.

Verdict: REQUEST CHANGES. Do not merge until the eval injection path is removed or PR_NUMBER is strictly validated before command construction. Tuesday should not proceed to CI/queue-guard/merge gate yet.

## DO-178B Review: PR #521 — fix(mosaic): harden Gitea pr merge fallback Software Level: C (Mosaic delivery wrapper/business-critical tooling) Reviewer: ultron Base: 755df9079e4d1d697856c89d4952407dfa1b554a Head reviewed: 893dd19efbe6a6434f3ae9e040ef9d4c25034fc6 Scope: packages/mosaic/framework/tools/git/pr-merge.sh, packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh, docs/scratchpads/t_301e4e3b-pr-merge-gitea-empty-uid.md Objective summary: 1 Requirements Traceability: PASS — PR body links t_245ad9d8/t_301e4e3b and issue #520. 2 Derived Requirements: PASS — API fallback safety behavior documented. 3 Configuration Management: PASS — branch targets main; head pinned above; scoped files only. 4 Functional Correctness: PASS for Gitea fallback — known empty uid path falls back; arbitrary tea failure blocks. 5 Structural Coverage: PASS for focused shell harness; broader coverage not applicable to shell wrapper. 6 Security Analysis: FAIL — eval injection remains in GitHub path of touched wrapper. 7 Data Integrity: PASS — merge action remains guarded by squash/main/queue checks before execution. 8 Robustness: PASS — fallback handles no-login and known tea identity failure; unknown failures fail closed. 9 Code Quality: FAIL — eval-based command construction remains in pr-merge.sh. 10 Framework Compliance: PASS — Mosaic squash/main/queue policies preserved for Gitea path. 11 Regression Protection: PASS for focused checks; full pnpm test failure documented as out-of-scope gateway DB setup. 12 Hands-Off Verification: PASS — diff is limited to declared wrapper/test/scratchpad scope. Findings: 🔴 [BLOCKER] packages/mosaic/framework/tools/git/pr-merge.sh:193-195 — GitHub merge path still builds a command string with unvalidated PR_NUMBER and executes it via eval. The review scope explicitly required verifying that shell quoting avoids eval-style injection risks. The Gitea path was fixed to quote arguments, but the same touched wrapper still has: CMD="gh pr merge $PR_NUMBER --squash" [[ "$DELETE_BRANCH" == true ]] && CMD="$CMD --delete-branch" eval "$CMD" A mocked GitHub-platform probe with `-n '123; touch <sentinel> #'` executed the injected command before reporting success. This is a command injection risk in a privileged delivery wrapper and fails the required shell-quoting check. Remediation: - Validate `PR_NUMBER` before any metadata lookup/merge, e.g. `[[ "$PR_NUMBER" =~ ^[0-9]+$ ]] || exit 1`. - Replace the GitHub eval path with an argv array, e.g. `cmd=(gh pr merge "$PR_NUMBER" --squash); [[ "$DELETE_BRANCH" == true ]] && cmd+=(--delete-branch); "${cmd[@]}"`. - Add a focused regression proving metacharacters in `-n/--number` are rejected and do not execute on both GitHub and Gitea paths. Verification evidence run by reviewer: - `bash -n packages/mosaic/framework/tools/git/pr-merge.sh` - `bash -n packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh` - `shellcheck -x packages/mosaic/framework/tools/git/pr-merge.sh packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh` — PASS - `bash packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh` — PASS - Focused guard probe: non-main base exits before tea/API merge; non-squash method exits before external calls — PASS - Focused injection probe: GitHub eval path executed injected sentinel command — FAIL - Codex automated review wrapper attempted but unavailable due 401 Unauthorized (`/home/hermes/.config/mosaic/tools/codex/codex-code-review.sh -b origin/main -o ...`). Independent subagent review completed and agreed the Gitea-specific behavior is acceptable but GitHub eval is a material residual issue. Verdict: REQUEST CHANGES. Do not merge until the eval injection path is removed or PR_NUMBER is strictly validated before command construction. Tuesday should not proceed to CI/queue-guard/merge gate yet.
jason.woltje added 1 commit 2026-05-22 21:24:48 +00:00
fix(mosaic): reject unsafe pr merge numbers (#520)
Some checks failed
ci/woodpecker/push/ci Pipeline failed
Details
ci/woodpecker/pr/ci Pipeline failed
Details
19fc6d549e
Some checks failed
ci/woodpecker/push/ci Pipeline failed
Details
ci/woodpecker/pr/ci Pipeline failed
Details
This pull request has changes conflicting with the target branch.
  • packages/mosaic/framework/tools/git/pr-merge.sh
  • packages/mosaic/framework/tools/git/test-pr-merge-gitea-empty-uid.sh
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/t_301e4e3b-pr-merge-gitea-empty-uid:fix/t_301e4e3b-pr-merge-gitea-empty-uid
git checkout fix/t_301e4e3b-pr-merge-gitea-empty-uid
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
jason.woltje
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#521
Delete Branch "fix/t_301e4e3b-pr-merge-gitea-empty-uid"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?