mosaicstack/stack
16
0
Fork 1
Code Issues 335 Pull Requests 15 Actions Packages Projects Releases 15 Wiki Activity

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

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

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 commented 2026-05-22 21:11:57 +00:00
Author
Owner
Copy Link
Copy Source

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: 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.

## 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 force-pushed fix/t_301e4e3b-pr-merge-gitea-empty-uid from 19fc6d549e to 59b611ba8a 2026-06-11 18:23:34 +00:00 Compare
jason.woltje commented 2026-06-11 18:24:31 +00:00
Author
Owner
Copy Link
Copy Source

Closing as superseded by #524 (rollup 821e19d, merged 2026-05-26). Verified during orchestration rebase: both commits (893dd19 harden Gitea fallback, 19fc6d5 reject unsafe PR numbers) are already on main, which is a strict superset (adds basic_auth fallback, HTTP-code tracking, --dry-run). The api_url-unbound bug is FIXED on main (pr-merge.sh declares+assigns api_url before use). NOTE: the bug still reproduces from the installed framework copy on hosts (v0.0.30, 2026-05-06) which predates #524 — the real remaining action is upgrading the deployed framework, not merging this PR. — orchestrator (mos)

Closing as **superseded by #524** (rollup 821e19d, merged 2026-05-26). Verified during orchestration rebase: both commits (893dd19 harden Gitea fallback, 19fc6d5 reject unsafe PR numbers) are already on main, which is a strict superset (adds basic_auth fallback, HTTP-code tracking, --dry-run). The api_url-unbound bug is FIXED on main (pr-merge.sh declares+assigns api_url before use). NOTE: the bug still reproduces from the *installed* framework copy on hosts (v0.0.30, 2026-05-06) which predates #524 — the real remaining action is upgrading the deployed framework, not merging this PR. — orchestrator (mos)
jason.woltje closed this pull request 2026-06-11 18:24:31 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Defect in framework tooling or delivered code
 fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
code-be-01 (Mosaic fleet seat code-be-01) code-be-02 (Mosaic fleet seat code-be-02) code-infra-01 (Mosaic fleet seat code-infra-01) fargo fred gate-merge-01 (Mosaic fleet seat gate-merge-01) happy jason.woltje (Jason Woltje) merge-gate ops-01 (Mosaic fleet seat ops-01) ops-02 (Mosaic fleet seat ops-02) ops-03 (Mosaic fleet seat ops-03) ops-ci-01 (Mosaic fleet seat ops-ci-01) ops-deploy-01 (Mosaic fleet seat ops-deploy-01) pepper rev-code-01 rev-code-02 rev-security-01 rev-security-02 rev-security-03 (Mosaic fleet seat rev-security-03) sanity scooby (Scooby) scrappy shaggy tiny velma veronica (Mosaic fleet seat veronica) vision woodpecker
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#521