-
Notifications
You must be signed in to change notification settings - Fork 5
Allow docker-agent to request reviews on its own PRs
#72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,6 +78,10 @@ inputs: | |
| description: "Additional arguments to pass to cagent run" | ||
| required: false | ||
| default: "" | ||
| trusted-bot-app-id: | ||
| description: "GitHub App ID of a trusted bot that can bypass comment-based auth checks (e.g., for self-review triggers)" | ||
| required: false | ||
| default: "" | ||
| add-prompt-files: | ||
| description: "Comma-separated list of files to append to the prompt (e.g., 'AGENTS.md,CLAUDE.md')" | ||
| required: false | ||
|
|
@@ -190,10 +194,12 @@ runs: | |
| shell: bash | ||
| env: | ||
| ACTION_PATH: ${{ github.action_path }} | ||
| # Get author_association from comment events (the main risk) | ||
| COMMENT_ASSOCIATION: ${{ github.event.comment.author_association }} | ||
| TRUSTED_BOT_APP_ID: ${{ inputs.trusted-bot-app-id }} | ||
| DEBUG: ${{ inputs.debug }} | ||
| run: | | ||
| # Read comment fields directly from the event payload (cannot be overridden by workflow env vars) | ||
| COMMENT_ASSOCIATION=$(jq -r '.comment.author_association // empty' "$GITHUB_EVENT_PATH") | ||
|
|
||
| # Only enforce auth for comment-triggered events | ||
| # This prevents abuse via /commands while allowing PR-triggered workflows to run | ||
| if [ -z "$COMMENT_ASSOCIATION" ]; then | ||
|
|
@@ -202,6 +208,20 @@ runs: | |
| exit 0 | ||
| fi | ||
|
|
||
| # Allow a trusted GitHub App bot to bypass auth (e.g., auto-triage posts /review). | ||
| # Verified via user type + app ID from the event payload to prevent spoofing. | ||
| if [ -n "$TRUSTED_BOT_APP_ID" ]; then | ||
| COMMENT_USER_TYPE=$(jq -r '.comment.user.type // empty' "$GITHUB_EVENT_PATH") | ||
| COMMENT_APP_ID=$(jq -r '.comment.performed_via_github_app.id // empty' "$GITHUB_EVENT_PATH") | ||
|
|
||
| if [ "$COMMENT_USER_TYPE" = "Bot" ] && [ -n "$COMMENT_APP_ID" ] && [ "$COMMENT_APP_ID" = "$TRUSTED_BOT_APP_ID" ]; then | ||
derekmisler marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| COMMENT_USER_LOGIN=$(jq -r '.comment.user.login // empty' "$GITHUB_EVENT_PATH") | ||
| echo "ℹ️ Skipping auth check (trusted bot: $COMMENT_USER_LOGIN, app_id: $COMMENT_APP_ID)" | ||
| echo "authorized=bot" >> $GITHUB_OUTPUT | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the bot bypass succeeds, the script immediately exits with While this is by design, the lack of fallback verification creates risk if the app ID check is weak (e.g., Finding #1 shows it can succeed with empty IDs). If the app is compromised or misconfigured, this removes all authorization safeguards. Recommendation: Add defense-in-depth:
derekmisler marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| exit 0 | ||
| fi | ||
| fi | ||
|
|
||
| echo "Using comment author_association: $COMMENT_ASSOCIATION" | ||
|
|
||
| # Allowed roles (hardcoded for security - cannot be overridden) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workflow passes
secrets.CAGENT_REVIEWER_APP_IDas thetrusted-bot-app-idinput. This creates a circular trust model where:/reviewWhile this may be intentional for legitimate automation (e.g., a trusted triage bot triggering reviews), if the app's credentials are compromised or if someone exploits the app's webhook, this bypass would allow unauthorized reviews.
Recommendation: Consider using separate apps for triggering (automation) and posting (identity), or add additional verification that the app is being used in the expected context.