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

fix(tools/git): use tea comment, keep one principal, add a red-first regression

Browse Source 
Addresses both blockers from review 127 (rev-security-02) and corrects the
severity claim in the original report.

Blocker 1 -- mixed principals. Routing the comment through the token-
authenticated gitea_issue_comment_api() attributed the comment to the token
holder while the close still used --login $GITEA_LOGIN_NAME: two principals for
one operation. `tea comment` accepts the same --repo/--login flags, so the tea
branch now uses it and both calls carry the same principal. The no-login branch
keeps the API helper for both, also a single principal.

Blocker 2 -- no regression test. Adds test-issue-close-fail-closed.sh on the
existing mocked-tea/sandboxed-git harness pattern. Asserts: a failed comment
does not close the issue and exits non-zero; a successful comment does close it;
the subcommand is top-level `tea comment`, never `tea issue comment`; and the
comment and close carry the same --login. GREEN on this branch, RED on main.

Severity correction. The original report said the issue closes anyway and the
audit trail is silently lost. It does not: set -e at line 5 aborts the script
when the comment fails, so the close is never reached. The real defect is that
issue-close.sh -c cannot succeed at all where a tea login resolves -- loud, not
silent. The explicit || guard is retained deliberately: a fail-closed property
that depends on set -e disappears the moment anyone adds `|| true` or wraps the
call in a conditional. Posted as a comment on #1081 and #1085.

Refs #1081

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Amf1Neca162odgcbCWMk1y
This commit is contained in:
Hermes Agent
2026-08-07 00:33:57 -05:00
committed by Mos
co-authored by Claude Opus 5
parent 5d342f77af
commit 549b6fbaae
2 changed files with 91 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/mosaic/framework/tools/git/issue-close.sh
+12 -7
View File
@@ -91,13 +91,18 @@ elif [[ "$PLATFORM" == "gitea" ]]; then
GITEA_LOGIN_NAME=$(get_gitea_login || true)
if [[ -n "$GITEA_LOGIN_NAME" ]]; then
if [[ -n "$COMMENT" ]]; then
# `tea issue comment` is NOT a subcommand (tea 0.11.x lists only
# list/create/edit/reopen/close); comments are the top-level `tea comment`.
# The call therefore always failed, was unchecked, and the script proceeded
# to close the issue anyway -- losing the record of WHY it was closed.
# Route through the authenticated API helper: it is login-independent and is
# already the mechanism used by the no-login branch below.
gitea_issue_comment_api || {
# `tea issue comment` is NOT a subcommand -- tea 0.11.x lists only
# list/create/edit/reopen/close under `tea issue`. Comments are the
# TOP-LEVEL `tea comment`, which takes the same --repo/--login flags.
# The old call therefore always failed, was unchecked, and the script
# closed the issue anyway, losing the record of WHY.
#
# Use `tea comment` rather than the API helper so the comment and the
# close are made by the SAME principal ($GITEA_LOGIN_NAME). Routing the
# comment through the token-authenticated helper here would attribute the
# comment to the token holder and the close to the tea login -- two
# principals for one operation.
tea comment "$ISSUE_NUMBER" "$COMMENT" --repo "$OWNER/$REPO" --login "$GITEA_LOGIN_NAME" || {
echo "Error: failed to post comment on #$ISSUE_NUMBER -- NOT closing (fail closed)." >&2
exit 1
}