Skip to content

Commit ca87414

Browse files
derekmislerderekmisler
authored
Replace author_association auth with org membership checks (docker#110)
1 parent ab2c594 commit ca87414

File tree

6 files changed

+190
-28
lines changed

6 files changed

+190
-28
lines changed

‎.github/workflows/reply-to-feedback.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ jobs:
325325
nebius-api-key: ${{ secrets.NEBIUS_API_KEY }}
326326
mistral-api-key: ${{ secrets.MISTRAL_API_KEY }}
327327
github-token: ${{ steps.app-token.outputs.token || github.token }}
328+
skip-auth: "true" # Org membership already verified above
328329

329330
# ----------------------------------------------------------------
330331
# Failure handling

‎.github/workflows/review-pr.yml‎

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ jobs:
190190
xai-api-key: ${{ secrets.XAI_API_KEY }}
191191
nebius-api-key: ${{ secrets.NEBIUS_API_KEY }}
192192
mistral-api-key: ${{ secrets.MISTRAL_API_KEY }}
193+
skip-auth: "true" # Org membership already verified above
193194

194195
# ==========================================================================
195196
# MANUAL REVIEW PIPELINE
@@ -207,10 +208,50 @@ jobs:
207208
exit-code: ${{ steps.run-review.outputs.exit-code }}
208209

209210
steps:
211+
- name: Check if commenter is org member
212+
id: membership
213+
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
214+
with:
215+
github-token: ${{ secrets.CAGENT_ORG_MEMBERSHIP_TOKEN }}
216+
script: |
217+
const org = '${{ inputs.auto-review-org }}';
218+
const username = context.payload.comment.user.login;
219+
const userType = context.payload.comment.user.type;
220+
221+
// Allow trusted bot to bypass org membership check
222+
if (userType === 'Bot') {
223+
core.setOutput('is_member', 'true');
224+
console.log(`✅ ${username} is a Bot — allowing /review command`);
225+
return;
226+
}
227+
228+
try {
229+
await github.rest.orgs.checkMembershipForUser({ org, username });
230+
core.setOutput('is_member', 'true');
231+
console.log(`✅ ${username} is a ${org} org member — proceeding with manual review`);
232+
} catch (error) {
233+
if (error.status === 404 || error.status === 302) {
234+
core.setOutput('is_member', 'false');
235+
console.log(`⏭️ ${username} is not a ${org} org member — skipping manual review`);
236+
} else if (error.status === 401) {
237+
core.warning(
238+
'❌ CAGENT_ORG_MEMBERSHIP_TOKEN secret is missing or invalid.\n\n' +
239+
`This secret is required to check ${org} org membership.\n\n` +
240+
'To fix this:\n' +
241+
'1. Create a classic PAT with read:org scope at https://github.com/settings/tokens/new\n' +
242+
'2. Add it as an org secret named CAGENT_ORG_MEMBERSHIP_TOKEN'
243+
);
244+
core.setOutput('is_member', 'false');
245+
} else {
246+
core.warning(`Failed to check org membership: ${error.message}`);
247+
core.setOutput('is_member', 'false');
248+
}
249+
}
250+
210251
# Generate GitHub App token first so the check run is created under the app's identity
211252
# (prevents GitHub from nesting it under unrelated pull_request-triggered workflows)
212253
- name: Generate GitHub App token
213-
if: env.HAS_APP_SECRETS == 'true'
254+
if: steps.membership.outputs.is_member == 'true' && env.HAS_APP_SECRETS == 'true'
214255
id: app-token
215256
continue-on-error: true # Don't fail workflow if token generation fails
216257
uses: tibdex/github-app-token@3beb63f4bd073e61482598c45c71c1019b59b73a # v2
@@ -219,6 +260,7 @@ jobs:
219260
private_key: ${{ secrets.CAGENT_REVIEWER_APP_PRIVATE_KEY }}
220261

221262
- name: Create check run
263+
if: steps.membership.outputs.is_member == 'true'
222264
id: create-check
223265
continue-on-error: true # Don't fail if caller didn't grant checks: write
224266
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
@@ -245,15 +287,15 @@ jobs:
245287
});
246288
core.setOutput('check-id', check.id);
247289
248-
# Checkout PR head (not default branch)
249-
# Note: Authorization is handled by the composite action's built-in check
250290
- name: Checkout PR head
291+
if: steps.membership.outputs.is_member == 'true'
251292
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
252293
with:
253294
fetch-depth: 0
254295
ref: refs/pull/${{ github.event.issue.number }}/head
255296

256297
- name: Run PR Review
298+
if: steps.membership.outputs.is_member == 'true'
257299
id: run-review
258300
continue-on-error: true # Don't fail the calling workflow if the review errors
259301
uses: docker/cagent-action/review-pr@latest
@@ -272,6 +314,7 @@ jobs:
272314
xai-api-key: ${{ secrets.XAI_API_KEY }}
273315
nebius-api-key: ${{ secrets.NEBIUS_API_KEY }}
274316
mistral-api-key: ${{ secrets.MISTRAL_API_KEY }}
317+
skip-auth: "true" # Org membership already verified above
275318

276319
- name: Update check run
277320
if: always() && steps.create-check.outputs.check-id != ''
@@ -420,22 +463,10 @@ jobs:
420463
GH_TOKEN: ${{ secrets.CAGENT_ORG_MEMBERSHIP_TOKEN }}
421464
ORG: ${{ inputs.auto-review-org }}
422465
USERNAME: ${{ github.event.comment.user.login }}
423-
# Use the event context expression — $GITHUB_EVENT_PATH is empty/minimal
424-
# in workflow_call context, so jq parsing it fails silently.
425-
AUTHOR_ASSOCIATION: ${{ github.event.comment.author_association }}
426466
run: |
427467
if [ -z "$GH_TOKEN" ]; then
428-
echo "::warning::CAGENT_ORG_MEMBERSHIP_TOKEN not configured — falling back to author_association"
429-
case "$AUTHOR_ASSOCIATION" in
430-
OWNER|MEMBER|COLLABORATOR)
431-
echo "authorized=true" >> $GITHUB_OUTPUT
432-
echo "✅ Authorized via author_association (fallback)"
433-
;;
434-
*)
435-
echo "authorized=false" >> $GITHUB_OUTPUT
436-
echo "⏭️ Not authorized via author_association (fallback)"
437-
;;
438-
esac
468+
echo "::error::CAGENT_ORG_MEMBERSHIP_TOKEN not configured — cannot authorize reply"
469+
echo "authorized=false" >> $GITHUB_OUTPUT
439470
exit 0
440471
fi
441472
# Check org membership with explicit error handling
@@ -618,6 +649,7 @@ jobs:
618649
nebius-api-key: ${{ secrets.NEBIUS_API_KEY }}
619650
mistral-api-key: ${{ secrets.MISTRAL_API_KEY }}
620651
github-token: ${{ steps.app-token.outputs.token || github.token }}
652+
skip-auth: "true" # Org membership already verified above
621653

622654
- name: React on thread-build failure
623655
if: >-

‎.github/workflows/self-review-pr.yml‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ jobs:
100100
xai-api-key: ${{ secrets.XAI_API_KEY }}
101101
nebius-api-key: ${{ secrets.NEBIUS_API_KEY }}
102102
mistral-api-key: ${{ secrets.MISTRAL_API_KEY }}
103+
skip-auth: "true" # Org membership already verified above
103104

104105
# ==========================================================================
105106
# MANUAL REVIEW PIPELINE
@@ -115,10 +116,50 @@ jobs:
115116
HAS_APP_SECRETS: ${{ secrets.CAGENT_REVIEWER_APP_ID != '' }}
116117

117118
steps:
119+
- name: Check if commenter is org member
120+
id: membership
121+
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
122+
with:
123+
github-token: ${{ secrets.CAGENT_ORG_MEMBERSHIP_TOKEN }}
124+
script: |
125+
const org = 'docker';
126+
const username = context.payload.comment.user.login;
127+
const userType = context.payload.comment.user.type;
128+
129+
// Allow trusted bot to bypass org membership check
130+
if (userType === 'Bot') {
131+
core.setOutput('is_member', 'true');
132+
console.log(`✅ ${username} is a Bot — allowing /review command`);
133+
return;
134+
}
135+
136+
try {
137+
await github.rest.orgs.checkMembershipForUser({ org, username });
138+
core.setOutput('is_member', 'true');
139+
console.log(`✅ ${username} is a ${org} org member — proceeding with manual review`);
140+
} catch (error) {
141+
if (error.status === 404 || error.status === 302) {
142+
core.setOutput('is_member', 'false');
143+
console.log(`⏭️ ${username} is not a ${org} org member — skipping manual review`);
144+
} else if (error.status === 401) {
145+
core.warning(
146+
'❌ CAGENT_ORG_MEMBERSHIP_TOKEN secret is missing or invalid.\n\n' +
147+
`This secret is required to check ${org} org membership.\n\n` +
148+
'To fix this:\n' +
149+
'1. Create a classic PAT with read:org scope at https://github.com/settings/tokens/new\n' +
150+
'2. Add it as an org secret named CAGENT_ORG_MEMBERSHIP_TOKEN'
151+
);
152+
core.setOutput('is_member', 'false');
153+
} else {
154+
core.warning(`Failed to check org membership: ${error.message}`);
155+
core.setOutput('is_member', 'false');
156+
}
157+
}
158+
118159
# Generate GitHub App token first so the check run is created under the app's identity
119160
# (prevents GitHub from nesting it under unrelated pull_request-triggered workflows)
120161
- name: Generate GitHub App token
121-
if: env.HAS_APP_SECRETS == 'true'
162+
if: steps.membership.outputs.is_member == 'true' && env.HAS_APP_SECRETS == 'true'
122163
id: app-token
123164
continue-on-error: true # Don't fail workflow if token generation fails
124165
uses: tibdex/github-app-token@3beb63f4bd073e61482598c45c71c1019b59b73a # v2
@@ -127,6 +168,7 @@ jobs:
127168
private_key: ${{ secrets.CAGENT_REVIEWER_APP_PRIVATE_KEY }}
128169

129170
- name: Create check run
171+
if: steps.membership.outputs.is_member == 'true'
130172
id: create-check
131173
continue-on-error: true # Don't fail if checks: write permission is missing
132174
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
@@ -153,15 +195,15 @@ jobs:
153195
});
154196
core.setOutput('check-id', check.id);
155197
156-
# Checkout PR head (not default branch)
157-
# Note: Authorization is handled by the composite action's built-in check
158198
- name: Checkout PR head
199+
if: steps.membership.outputs.is_member == 'true'
159200
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
160201
with:
161202
fetch-depth: 0
162203
ref: refs/pull/${{ github.event.issue.number }}/head
163204

164205
- name: Run PR Review
206+
if: steps.membership.outputs.is_member == 'true'
165207
id: run-review
166208
continue-on-error: true # Don't fail the calling workflow if the review errors
167209
uses: ./review-pr
@@ -177,6 +219,7 @@ jobs:
177219
xai-api-key: ${{ secrets.XAI_API_KEY }}
178220
nebius-api-key: ${{ secrets.NEBIUS_API_KEY }}
179221
mistral-api-key: ${{ secrets.MISTRAL_API_KEY }}
222+
skip-auth: "true" # Org membership already verified above
180223

181224
- name: Update check run
182225
if: always() && steps.create-check.outputs.check-id != ''

‎action.yml‎

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,18 @@ inputs:
9090
description: "Skip writing agent output to the job summary (useful when callers write their own summary)"
9191
required: false
9292
default: "false"
93+
org-membership-token:
94+
description: "PAT with read:org scope for org membership authorization checks (preferred over author_association)"
95+
required: false
96+
default: ""
97+
auth-org:
98+
description: "GitHub organization to check membership against (used with org-membership-token)"
99+
required: false
100+
default: ""
101+
skip-auth:
102+
description: "Skip the built-in authorization check (use when the calling workflow already performed its own auth)"
103+
required: false
104+
default: "false"
93105

94106
outputs:
95107
exit-code:
@@ -192,6 +204,11 @@ runs:
192204
# SECURITY: Authorization Check
193205
# Only enforced for comment-triggered events (the main abuse vector)
194206
# PR-triggered workflows are controlled by the workflow author
207+
#
208+
# Auth strategies (in priority order):
209+
# 1. skip-auth=true — caller already verified authorization
210+
# 2. org-membership-token — check org membership via API (preferred)
211+
# 3. author_association — legacy fallback from event payload
195212
# ========================================
196213
- name: Check authorization
197214
id: check-auth
@@ -200,13 +217,27 @@ runs:
200217
ACTION_PATH: ${{ github.action_path }}
201218
TRUSTED_BOT_APP_ID: ${{ inputs.trusted-bot-app-id }}
202219
DEBUG: ${{ inputs.debug }}
220+
SKIP_AUTH: ${{ inputs.skip-auth }}
221+
ORG_MEMBERSHIP_TOKEN: ${{ inputs.org-membership-token }}
222+
AUTH_ORG: ${{ inputs.auth-org }}
203223
run: |
224+
# Mask the org membership token to prevent accidental exposure in logs
225+
[ -n "$ORG_MEMBERSHIP_TOKEN" ] && echo "::add-mask::$ORG_MEMBERSHIP_TOKEN"
226+
227+
# Strategy 0: Skip auth if caller already verified
228+
if [ "$SKIP_AUTH" = "true" ]; then
229+
echo "ℹ️ Skipping auth check (caller already verified authorization)"
230+
echo "authorized=skipped-by-caller" >> $GITHUB_OUTPUT
231+
exit 0
232+
fi
233+
204234
# Read comment fields directly from the event payload (cannot be overridden by workflow env vars)
205235
COMMENT_ASSOCIATION=$(jq -r '.comment.author_association // empty' "$GITHUB_EVENT_PATH")
236+
COMMENT_USER_LOGIN=$(jq -r '.comment.user.login // empty' "$GITHUB_EVENT_PATH")
206237
207238
# Only enforce auth for comment-triggered events
208239
# This prevents abuse via /commands while allowing PR-triggered workflows to run
209-
if [ -z "$COMMENT_ASSOCIATION" ]; then
240+
if [ -z "$COMMENT_ASSOCIATION" ] && [ -z "$COMMENT_USER_LOGIN" ]; then
210241
echo "ℹ️ Skipping auth check (not a comment-triggered event)"
211242
echo "authorized=skipped" >> $GITHUB_OUTPUT
212243
exit 0
@@ -219,20 +250,42 @@ runs:
219250
COMMENT_APP_ID=$(jq -r '.comment.performed_via_github_app.id // empty' "$GITHUB_EVENT_PATH")
220251
221252
if [ "$COMMENT_USER_TYPE" = "Bot" ] && [ -n "$COMMENT_APP_ID" ] && [ "$COMMENT_APP_ID" = "$TRUSTED_BOT_APP_ID" ]; then
222-
COMMENT_USER_LOGIN=$(jq -r '.comment.user.login // empty' "$GITHUB_EVENT_PATH")
223253
echo "ℹ️ Skipping auth check (trusted bot: $COMMENT_USER_LOGIN, app_id: $COMMENT_APP_ID)"
224254
echo "authorized=true" >> $GITHUB_OUTPUT
225255
exit 0
226256
fi
227257
fi
228258
229-
echo "Using comment author_association: $COMMENT_ASSOCIATION"
230-
231-
# Allowed roles (hardcoded for security - cannot be overridden)
232-
ALLOWED_ROLES='["OWNER", "MEMBER", "COLLABORATOR"]'
259+
# Strategy 1: Org membership check (preferred — reliable for all event types)
260+
if [ -n "$ORG_MEMBERSHIP_TOKEN" ] && [ -n "$AUTH_ORG" ] && [ -n "$COMMENT_USER_LOGIN" ]; then
261+
echo "Checking org membership for @$COMMENT_USER_LOGIN in $AUTH_ORG..."
262+
if ! RESPONSE=$(GH_TOKEN="$ORG_MEMBERSHIP_TOKEN" gh api "orgs/$AUTH_ORG/members/$COMMENT_USER_LOGIN" --silent -i 2>/dev/null); then
263+
echo "::error::❌ Authorization failed: @$COMMENT_USER_LOGIN is not a $AUTH_ORG org member"
264+
echo "authorized=false" >> $GITHUB_OUTPUT
265+
exit 1
266+
fi
267+
STATUS=$(echo "$RESPONSE" | head -1 | grep -oE '[0-9]{3}' || echo "000")
268+
if [ "$STATUS" = "204" ]; then
269+
echo "✅ Authorization successful: @$COMMENT_USER_LOGIN is a $AUTH_ORG org member"
270+
echo "authorized=true" >> $GITHUB_OUTPUT
271+
exit 0
272+
else
273+
echo "::error::❌ Authorization failed: @$COMMENT_USER_LOGIN is not a $AUTH_ORG org member (HTTP $STATUS)"
274+
echo "authorized=false" >> $GITHUB_OUTPUT
275+
exit 1
276+
fi
277+
fi
233278
234-
# Run the authorization check
235-
$ACTION_PATH/security/check-auth.sh "$COMMENT_ASSOCIATION" "$ALLOWED_ROLES"
279+
# Strategy 2: author_association fallback (legacy — unreliable for pull_request_review_comment events)
280+
if [ -n "$COMMENT_ASSOCIATION" ]; then
281+
echo "::warning::Using author_association fallback ($COMMENT_ASSOCIATION). Configure org-membership-token and auth-org for more reliable authorization."
282+
ALLOWED_ROLES='["OWNER", "MEMBER", "COLLABORATOR"]'
283+
$ACTION_PATH/security/check-auth.sh "$COMMENT_ASSOCIATION" "$ALLOWED_ROLES"
284+
else
285+
echo "::error::No authorization method available (no org token, no author_association)"
286+
echo "authorized=false" >> $GITHUB_OUTPUT
287+
exit 1
288+
fi
236289
237290
# ========================================
238291
# GitHub App Token (Optional)

‎review-pr/action.yml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,18 @@ inputs:
5757
description: "GitHub App ID of a trusted bot that can bypass comment-based auth checks"
5858
required: false
5959
default: ""
60+
org-membership-token:
61+
description: "PAT with read:org scope for org membership authorization checks"
62+
required: false
63+
default: ""
64+
auth-org:
65+
description: "GitHub organization to check membership against"
66+
required: false
67+
default: ""
68+
skip-auth:
69+
description: "Skip the built-in authorization check (caller already verified auth)"
70+
required: false
71+
default: "false"
6072

6173
outputs:
6274
exit-code:
@@ -689,6 +701,9 @@ runs:
689701
trusted-bot-app-id: ${{ inputs.trusted-bot-app-id }}
690702
extra-args: ${{ inputs.model && format('--model={0}', inputs.model) || '' }}
691703
skip-summary: "true"
704+
org-membership-token: ${{ inputs.org-membership-token }}
705+
auth-org: ${{ inputs.auth-org }}
706+
skip-auth: ${{ inputs.skip-auth }}
692707

693708
# ========================================
694709
# BUILD REVIEW CONTEXT
@@ -782,6 +797,9 @@ runs:
782797
add-prompt-files: ${{ inputs.add-prompt-files }}
783798
max-retries: "1" # One retry handles transient API failures (e.g. Anthropic 400s) without risking duplicate reviews from full pipeline restarts
784799
skip-summary: "true"
800+
org-membership-token: ${{ inputs.org-membership-token }}
801+
auth-org: ${{ inputs.auth-org }}
802+
skip-auth: ${{ inputs.skip-auth }}
785803

786804
- name: Release review lock
787805
if: always() && steps.lock-check.outputs.skip != 'true'

0 commit comments

Comments
 (0)