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

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

Closed
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 2 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.
jason.woltje commented 2026-06-11 18:31:41 +00:00
Author
Owner
Copy Link

Closing as superseded by #524 (rollup 821e19d). Orchestration assessment (rebase onto current main): every functional fix from this PR — set -euo pipefail, write_metadata(), curl_gitea_pull(), --dry-run, detect-platform get_gitea_token() hardening, test-pr-metadata-gitea.sh — is already present in main via #524. The post-rebase residual is regressive (older pr-merge.sh missing PR-number validation + empty-uid API fallback). No unique content remains. — orchestrator (mos)

Closing as **superseded by #524** (rollup 821e19d). Orchestration assessment (rebase onto current main): every functional fix from this PR — set -euo pipefail, write_metadata(), curl_gitea_pull(), --dry-run, detect-platform get_gitea_token() hardening, test-pr-metadata-gitea.sh — is already present in main via #524. The post-rebase residual is regressive (older pr-merge.sh missing PR-number validation + empty-uid API fallback). No unique content remains. — orchestrator (mos)
jason.woltje closed this pull request 2026-06-11 18:31:41 +00:00
Some checks failed
ci/woodpecker/push/ci Pipeline failed
Details
ci/woodpecker/pr/ci Pipeline failed
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
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?