Skip to content

Commit c6c7d7e

Browse files
wip: harden worker permission reuse (#25096)
1 parent f5f4b85 commit c6c7d7e

2 files changed

Lines changed: 40 additions & 10 deletions

File tree

.agents/scripts/pulse-merge-process.sh

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,7 +1056,7 @@ _issue_has_verified_crypto_approval() {
10561056
#
10571057
# Args: $1=pr_number, $2=repo_slug, $3=labels_str (comma-separated),
10581058
# $4=is_draft, $5=linked_issue,
1059-
# $6=precomputed_issue_author_permission(optional),
1059+
# $6=precomputed_pr_author_permission(optional),
10601060
# $7=precomputed_permission_login(optional)
10611061
# Returns: 0=all gates pass (eligible for auto-merge), 1=blocked
10621062
#######################################
@@ -1066,7 +1066,7 @@ _attempt_worker_briefed_auto_merge() {
10661066
local labels_str="$3"
10671067
local is_draft="$4"
10681068
local linked_issue="$5"
1069-
local precomputed_issue_author_permission="${6:-}"
1069+
local precomputed_pr_author_permission="${6:-}"
10701070
local precomputed_permission_login="${7:-}"
10711071

10721072
# Feature flag — when OFF, all origin:worker PRs fall back to manual merge
@@ -1104,8 +1104,9 @@ _attempt_worker_briefed_auto_merge() {
11041104
local issue_author_login=""
11051105
read -r issue_author_assoc issue_author_login <<< "$_issue_meta"
11061106
local issue_author_permission=""
1107-
if [[ -n "$precomputed_issue_author_permission" && -n "$precomputed_permission_login" && "$precomputed_permission_login" == "$issue_author_login" ]]; then
1108-
issue_author_permission="$precomputed_issue_author_permission"
1107+
#aidevops:trust-boundary — reuse PR-author permission only for the same non-empty issue-author login.
1108+
if [[ -n "$precomputed_pr_author_permission" && -n "$precomputed_permission_login" && "$precomputed_permission_login" == "$issue_author_login" ]]; then
1109+
issue_author_permission="$precomputed_pr_author_permission"
11091110
fi
11101111

11111112
# Fetch auto-approval signal once for the NMR crypto-vs-auto check (t2449).

.agents/scripts/tests/test-pulse-merge-worker-briefed.sh

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -806,10 +806,38 @@ test_case_t_worker_gate_ignores_mismatched_precomputed_permission() {
806806
}
807807

808808
# =============================================================================
809-
# Case (u): issue-author=NONE + spoofed approval marker but failed verification
809+
# Case (u): worker gate ignores precomputed permission with empty login (GH#25090)
810+
# =============================================================================
811+
test_case_u_worker_gate_ignores_empty_precomputed_permission_login() {
812+
setup_test_env
813+
define_helpers_under_test || { teardown_test_env; return 0; }
814+
815+
printf '{"author_association":"COLLABORATOR","user":{"login":"issue-author"}}' >"${TEST_ROOT}/issue.json"
816+
printf '[]' >"${TEST_ROOT}/comments.json"
817+
printf 'read' >"${TEST_ROOT}/permission.txt"
818+
export AIDEVOPS_WORKER_BRIEFED_AUTO_MERGE=1
819+
820+
local result=0
821+
_attempt_worker_briefed_auto_merge "120" "owner/repo" "origin:worker" "false" "62" "write" "" && result=0 || result=$?
822+
823+
if [[ "$result" -eq 0 ]]; then
824+
print_result "Case (u): worker gate ignores empty precomputed permission login (GH#25090)" 1 \
825+
"Expected non-zero exit, got 0 (empty cached-permission login should not trust issue author)"
826+
elif grep -q "/collaborators/issue-author/permission" "$GH_LOG" 2>/dev/null; then
827+
print_result "Case (u): worker gate ignores empty precomputed permission login (GH#25090)" 0
828+
else
829+
print_result "Case (u): worker gate ignores empty precomputed permission login (GH#25090)" 1 \
830+
"Expected fallback collaborator permission API call for issue-author"
831+
fi
832+
teardown_test_env
833+
return 0
834+
}
835+
836+
# =============================================================================
837+
# Case (v): issue-author=NONE + spoofed approval marker but failed verification
810838
# → blocked. Comment marker presence alone is not a trust signal (GH#21936).
811839
# =============================================================================
812-
test_case_u_spoofed_crypto_marker_blocked() {
840+
test_case_v_spoofed_crypto_marker_blocked() {
813841
setup_test_env
814842
define_helpers_under_test || { teardown_test_env; return 0; }
815843

@@ -822,13 +850,13 @@ test_case_u_spoofed_crypto_marker_blocked() {
822850
_attempt_worker_briefed_auto_merge "117" "owner/repo" "origin:worker" "false" "59" && result=0 || result=$?
823851

824852
if [[ "$result" -eq 0 ]]; then
825-
print_result "Case (u): spoofed crypto marker without verification → blocked" 1 \
853+
print_result "Case (v): spoofed crypto marker without verification → blocked" 1 \
826854
"Expected non-zero exit, got 0 (unverified marker should block)"
827855
else
828856
if grep -q "no cryptographic approval signature found (t2449/t3052)" "$LOGFILE" 2>/dev/null; then
829-
print_result "Case (u): spoofed crypto marker without verification → blocked" 0
857+
print_result "Case (v): spoofed crypto marker without verification → blocked" 0
830858
else
831-
print_result "Case (u): spoofed crypto marker without verification → blocked" 1 \
859+
print_result "Case (v): spoofed crypto marker without verification → blocked" 1 \
832860
"Exit was non-zero but expected block log message not found"
833861
fi
834862
fi
@@ -865,7 +893,8 @@ main() {
865893
test_case_r_precomputed_permission_skips_api
866894
test_case_s_worker_gate_uses_matching_precomputed_permission
867895
test_case_t_worker_gate_ignores_mismatched_precomputed_permission
868-
test_case_u_spoofed_crypto_marker_blocked
896+
test_case_u_worker_gate_ignores_empty_precomputed_permission_login
897+
test_case_v_spoofed_crypto_marker_blocked
869898

870899
echo ""
871900
printf 'Results: %d/%d passed\n' "$((TESTS_RUN - TESTS_FAILED))" "$TESTS_RUN"

0 commit comments

Comments
 (0)