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

fix(git-tools): harden Gitea PR metadata wrappers #518

Open
jason.woltje wants to merge 1 commits from fix/t-a292e96f-gitea-pr-metadata into main
pull from: fix/t-a292e96f-gitea-pr-metadata
merge into: mosaicstack:main
Conversation 1 Commits 1 Files Changed 6 +457 -52
jason.woltje commented 2026-05-22 15:39:43 +00:00
Owner
Copy Link

Fix Mosaic Gitea PR metadata and merge preflight wrappers for USC repos. Normalizes Gitea head/base refs, rejects API error payloads, falls back to host-specific credentials, and avoids selecting the git.mosaicstack.dev tea login for git.uscllc.com merge preflight. Verification: bash -n, shellcheck, fixture harness, live sanitized PR #1905/#1908 metadata, and dry-run merge preflight.

Fix Mosaic Gitea PR metadata and merge preflight wrappers for USC repos. Normalizes Gitea head/base refs, rejects API error payloads, falls back to host-specific credentials, and avoids selecting the git.mosaicstack.dev tea login for git.uscllc.com merge preflight. Verification: bash -n, shellcheck, fixture harness, live sanitized PR #1905/#1908 metadata, and dry-run merge preflight.
jason.woltje added 1 commit 2026-05-22 15:39:43 +00:00
fix(git-tools): harden gitea pr metadata wrappers
Some checks failed
ci/woodpecker/push/ci Pipeline failed
Details
ci/woodpecker/pr/ci Pipeline failed
Details
7e1fb898e3
jason.woltje force-pushed fix/t-a292e96f-gitea-pr-metadata from 7e1fb898e3 to 006d3f375e 2026-05-22 16:17:58 +00:00 Compare
jason.woltje force-pushed fix/t-a292e96f-gitea-pr-metadata from 006d3f375e to 88c3f6cd91 2026-05-22 16:21:49 +00:00 Compare
jason.woltje force-pushed fix/t-a292e96f-gitea-pr-metadata from 88c3f6cd91 to a665c0ac4c 2026-05-22 16:23:41 +00:00 Compare
jason.woltje commented 2026-05-22 18:05:08 +00:00
Author
Owner
Copy Link

DO-178B Review: PR #518 — Mosaic pr-metadata.sh Gitea metadata fix

Software Level: B (merge tooling / credentialed Gitea API fallback)
Reviewer: Ultron
Base: 755df9079e
Head: a665c0ac4c
Verdict: REQUEST CHANGES

Scope reviewed:

  • packages/mosaic/framework/tools/git/detect-platform.sh
  • packages/mosaic/framework/tools/git/pr-metadata.sh
  • packages/mosaic/framework/tools/git/pr-merge.sh
  • packages/mosaic/framework/tools/git/test-pr-metadata-gitea.sh
  • docs/TASKS.md and task scratchpad

Verification performed:

  • bash -n changed shell scripts: PASS
  • shellcheck -x -P . -e SC1090 changed shell scripts: PASS
  • MOSAIC_TEST_WORK_DIR=$PWD/.mosaic-test-work/pr-metadata-gitea-review packages/mosaic/framework/tools/git/test-pr-metadata-gitea.sh: PASS
  • Source wrapper live metadata: PR #518 => base=main, head=fix/t-a292e96f-gitea-pr-metadata, state=open
  • Source wrapper live metadata from /src/uconnect: PR #1905 => base=main, head=edith/t_39ce717c-authentik-smoke-gate; PR #1908 => base=main, head=fix/t_23fa9e1d-portal-health-backend
  • Merge policy probe: non-main base was rejected
  • Codex review wrapper attempted but unavailable: codex-code-review.sh failed with OpenAI API 401 before producing output

Blocking findings:

🔴 [BLOCKER] packages/mosaic/framework/tools/git/pr-merge.sh:185 and :208 — host matching is bypassed when GITEA_LOGIN is set.
The fix claims tea login selection only happens when the login URL matches the repo host, but both dry-run and real merge use:

TEA_LOGIN="${GITEA_LOGIN:-$(find_tea_login_for_host "$HOST" || true)}"

That means any inherited GITEA_LOGIN overrides host validation. I verified this with a fixture repo whose origin is git.uscllc.com:

GITEA_LOGIN=mosaicstack ... pr-merge.sh -n 1905 --dry-run
-> Dry run: would merge PR #1905 on git.uscllc.com with tea login 'mosaicstack'

This preserves the exact wrong-login failure mode when the environment contains GITEA_LOGIN=mosaicstack, and it contradicts the acceptance criterion that the wrapper must not select the unrelated MosaicStack login for USC repos. Fix approach: validate any explicit GITEA_LOGIN against tea login list URL before use, or remove the env override and require host-derived selection only; fallback to the authenticated API path if the named login does not match HOST.

🟡 [IMPORTANT] packages/mosaic/framework/tools/git/pr-metadata.sh:62/:73 and pr-merge.sh:138-143/:152-157 — credential material is still passed to curl via command-line arguments.
The new basic-auth fallback passes username:password through curl -u "$basic_auth", and token auth is passed through -H "Authorization: token $token". This avoids logging in normal script output, but command-line arguments may be visible to same-host process inspection while curl is running. Because the review request explicitly includes “no token leakage,” this should be hardened before merge if feasible. Fix approach: use curl config via a temporary chmod 600 file/fd, netrc with restricted permissions, or another mechanism that avoids putting bearer/basic credentials in argv; ensure cleanup uses trap.

Non-blocking observations:

  • Gitea metadata normalization now correctly fails API error payloads instead of emitting empty branch fields.
  • baseRefName/headRefName are non-empty for the live PRs tested.
  • GitHub behavior is structurally preserved by keeping the gh pr view path; I did not run a live GitHub PR because this worktree is Gitea-backed.
  • Docs/TASKS.md and the scratchpad include task tracking and verification notes. No root CHANGELOG.md exists per scratchpad.
  • CI remains externally blocked by Woodpecker/Kubernetes PVC API timeout before repo steps, so local/source verification is the available evidence.

DO-178B objective summary:
1 Requirements Traceability: PASS — task and scratchpad map the regression and acceptance criteria.
2 Derived Requirements: PASS — API fallback and login matching are documented.
3 Configuration Management: PARTIAL — PR/head verified; CI blocked by infra.
4 Functional Correctness: FAIL — GITEA_LOGIN env override can reselect the wrong login.
5 Structural Coverage: PASS for added targeted shell harness; no full repo coverage expected for wrapper script.
6 Security Analysis: FAIL — credential argv exposure remains; wrong-login override can route a merge through unintended auth config.
7 Data Integrity: PASS — merge still requires base main and squash; non-main fixture rejected.
8 Robustness: PASS for API error/non-JSON handling in metadata path.
9 Code Quality: PASS — no shellcheck findings.
10 Framework Compliance: PARTIAL — docs/scratchpad present; CI blocked externally.
11 Regression Protection: PASS for local harness and live metadata checks; CI not green due infra.
12 Hands-Off Verification: PASS — review worktree clean after inspection.

Verdict: REQUEST CHANGES until the GITEA_LOGIN host-validation bypass is fixed. The credential argv exposure should also be addressed or formally accepted as existing tooling risk before merge.

## DO-178B Review: PR #518 — Mosaic pr-metadata.sh Gitea metadata fix Software Level: B (merge tooling / credentialed Gitea API fallback) Reviewer: Ultron Base: 755df9079e4d1d697856c89d4952407dfa1b554a Head: a665c0ac4c7573f3b4c37e0160482121690ce304 Verdict: REQUEST CHANGES Scope reviewed: - packages/mosaic/framework/tools/git/detect-platform.sh - packages/mosaic/framework/tools/git/pr-metadata.sh - packages/mosaic/framework/tools/git/pr-merge.sh - packages/mosaic/framework/tools/git/test-pr-metadata-gitea.sh - docs/TASKS.md and task scratchpad Verification performed: - bash -n changed shell scripts: PASS - shellcheck -x -P . -e SC1090 changed shell scripts: PASS - MOSAIC_TEST_WORK_DIR=$PWD/.mosaic-test-work/pr-metadata-gitea-review packages/mosaic/framework/tools/git/test-pr-metadata-gitea.sh: PASS - Source wrapper live metadata: PR #518 => base=main, head=fix/t-a292e96f-gitea-pr-metadata, state=open - Source wrapper live metadata from /src/uconnect: PR #1905 => base=main, head=edith/t_39ce717c-authentik-smoke-gate; PR #1908 => base=main, head=fix/t_23fa9e1d-portal-health-backend - Merge policy probe: non-main base was rejected - Codex review wrapper attempted but unavailable: codex-code-review.sh failed with OpenAI API 401 before producing output Blocking findings: 🔴 [BLOCKER] packages/mosaic/framework/tools/git/pr-merge.sh:185 and :208 — host matching is bypassed when GITEA_LOGIN is set. The fix claims tea login selection only happens when the login URL matches the repo host, but both dry-run and real merge use: TEA_LOGIN="${GITEA_LOGIN:-$(find_tea_login_for_host "$HOST" || true)}" That means any inherited GITEA_LOGIN overrides host validation. I verified this with a fixture repo whose origin is git.uscllc.com: GITEA_LOGIN=mosaicstack ... pr-merge.sh -n 1905 --dry-run -> Dry run: would merge PR #1905 on git.uscllc.com with tea login 'mosaicstack' This preserves the exact wrong-login failure mode when the environment contains GITEA_LOGIN=mosaicstack, and it contradicts the acceptance criterion that the wrapper must not select the unrelated MosaicStack login for USC repos. Fix approach: validate any explicit GITEA_LOGIN against tea login list URL before use, or remove the env override and require host-derived selection only; fallback to the authenticated API path if the named login does not match HOST. 🟡 [IMPORTANT] packages/mosaic/framework/tools/git/pr-metadata.sh:62/:73 and pr-merge.sh:138-143/:152-157 — credential material is still passed to curl via command-line arguments. The new basic-auth fallback passes username:password through `curl -u "$basic_auth"`, and token auth is passed through `-H "Authorization: token $token"`. This avoids logging in normal script output, but command-line arguments may be visible to same-host process inspection while curl is running. Because the review request explicitly includes “no token leakage,” this should be hardened before merge if feasible. Fix approach: use curl config via a temporary chmod 600 file/fd, netrc with restricted permissions, or another mechanism that avoids putting bearer/basic credentials in argv; ensure cleanup uses trap. Non-blocking observations: - Gitea metadata normalization now correctly fails API error payloads instead of emitting empty branch fields. - baseRefName/headRefName are non-empty for the live PRs tested. - GitHub behavior is structurally preserved by keeping the gh pr view path; I did not run a live GitHub PR because this worktree is Gitea-backed. - Docs/TASKS.md and the scratchpad include task tracking and verification notes. No root CHANGELOG.md exists per scratchpad. - CI remains externally blocked by Woodpecker/Kubernetes PVC API timeout before repo steps, so local/source verification is the available evidence. DO-178B objective summary: 1 Requirements Traceability: PASS — task and scratchpad map the regression and acceptance criteria. 2 Derived Requirements: PASS — API fallback and login matching are documented. 3 Configuration Management: PARTIAL — PR/head verified; CI blocked by infra. 4 Functional Correctness: FAIL — GITEA_LOGIN env override can reselect the wrong login. 5 Structural Coverage: PASS for added targeted shell harness; no full repo coverage expected for wrapper script. 6 Security Analysis: FAIL — credential argv exposure remains; wrong-login override can route a merge through unintended auth config. 7 Data Integrity: PASS — merge still requires base main and squash; non-main fixture rejected. 8 Robustness: PASS for API error/non-JSON handling in metadata path. 9 Code Quality: PASS — no shellcheck findings. 10 Framework Compliance: PARTIAL — docs/scratchpad present; CI blocked externally. 11 Regression Protection: PASS for local harness and live metadata checks; CI not green due infra. 12 Hands-Off Verification: PASS — review worktree clean after inspection. Verdict: REQUEST CHANGES until the GITEA_LOGIN host-validation bypass is fixed. The credential argv exposure should also be addressed or formally accepted as existing tooling risk before merge.
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.
  • docs/TASKS.md
  • packages/mosaic/framework/tools/git/pr-merge.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-a292e96f-gitea-pr-metadata:fix/t-a292e96f-gitea-pr-metadata
git checkout fix/t-a292e96f-gitea-pr-metadata
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#518
Delete Branch "fix/t-a292e96f-gitea-pr-metadata"

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?