Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions scripts/check-e2e-authorization-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,20 @@ run_case "trusted collaborator author" "true" "trusted_author" "false"
write_pr "CONTRIBUTOR" '[]'
run_case "contributor author denied" "false" "unauthorized" "false"

# --- Trusted bot tests ---

export PR_AUTHOR_ASSOCIATION="CONTRIBUTOR"
export PR_AUTHOR_LOGIN="renovate-fullsend[bot]"
echo "" >"${COLLAB_ROLE}"
write_pr "NONE" '[]'
run_case "renovate bot authorized as trusted bot" "true" "trusted_bot" "false"

export PR_AUTHOR_LOGIN="some-other-bot[bot]"
write_pr "NONE" '[]'
run_case "unknown bot not authorized" "false" "unauthorized" "false"

unset PR_AUTHOR_ASSOCIATION PR_AUTHOR_LOGIN

write_pr "MEMBER" '[{"name":"ok-to-test"}]'
run_case "trusted member ignores stale ok-to-test label" "true" "trusted_author" "false"

Expand Down
19 changes: 16 additions & 3 deletions scripts/check-e2e-authorization.sh
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
#!/usr/bin/env bash
# check-e2e-authorization.sh — Decide whether a PR may run e2e tests in CI.
#
# Authorized when the PR author is OWNER/MEMBER/COLLABORATOR, or when the
# collaborator permission API confirms write+ access, or when a fresh
# ok-to-test label was applied after the latest push.
# Authorized when the PR author is OWNER/MEMBER/COLLABORATOR, when the author
# is a trusted bot (e.g. renovate-fullsend[bot]), when the collaborator
# permission API confirms write+ access, or when a fresh ok-to-test label was
# applied after the latest push.
#
# The author_association field from the event payload can misreport org members
# whose membership visibility is private (returns CONTRIBUTOR/NONE instead of
Expand Down Expand Up @@ -31,6 +32,7 @@ PR_NUMBER="${1:?PR number required}"
REPOSITORY="${2:?repository (owner/repo) required}"

TRUSTED_ASSOCIATIONS="OWNER MEMBER COLLABORATOR"
TRUSTED_BOT_LOGINS="renovate-fullsend[bot]"
OK_TO_TEST_LABEL="ok-to-test"

write_error_output() {
Expand All @@ -55,6 +57,14 @@ is_trusted_author() {
esac
}

is_trusted_bot() {
local login="${1:-}"
case " ${TRUSTED_BOT_LOGINS} " in
*" ${login} "*) return 0 ;;
*) return 1 ;;
esac
}

# Fallback: check actor has write+ permission via the collaborator permission
# API, which correctly resolves org membership regardless of visibility
# (private vs public). Same approach as the dispatch workflow.
Expand Down Expand Up @@ -89,6 +99,9 @@ fi
if is_trusted_author "${author_association}"; then
authorized=true
reason="trusted_author"
elif is_trusted_bot "${PR_AUTHOR_LOGIN:-}"; then
authorized=true
reason="trusted_bot"
Comment on lines +102 to +104

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Informational

1. Bot trust lacks login fallback 🐞 Bug ≡ Correctness

The new trusted-bot authorization path only checks PR_AUTHOR_LOGIN, so callers that don’t set this
env var cannot recognize trusted bots even when PR data is fetched via the GitHub API. This can
incorrectly deny trusted bot PRs in ad-hoc/local invocations or future workflows that omit
PR_AUTHOR_LOGIN.
Agent Prompt
### Issue description
`is_trusted_bot` is only evaluated against `PR_AUTHOR_LOGIN`. When `PR_AUTHOR_LOGIN` is unset, the script never derives the author login from the PR API response (even when it already fetches `pr_json`), so trusted bots can be incorrectly treated as unauthorized depending on how the script is invoked.

### Issue Context
- The script documents `PR_AUTHOR_LOGIN` as optional.
- The composite action/workflows currently pass `PR_AUTHOR_LOGIN`, so CI isn’t broken today; this is a robustness/contract mismatch for other invocation paths.

### Fix Focus Areas
- scripts/check-e2e-authorization.sh[60-66]
- scripts/check-e2e-authorization.sh[89-113]

### Suggested implementation
- Introduce a local `author_login` variable.
- Populate it as:
  - `author_login="${PR_AUTHOR_LOGIN:-}"`
  - If empty and `pr_json` is available (or needs to be fetched for other reasons), set `author_login="$(jq -r '.user.login // empty' <<<"${pr_json}")"`.
- Use `author_login` for both `is_trusted_bot` and `has_write_permission`.
- Optional hardening: have `is_trusted_bot` return false when `login` is empty to avoid any future edge cases if the trusted list ever becomes empty/malformed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

elif has_write_permission "${PR_AUTHOR_LOGIN:-}" 2>/dev/null; then
# author_association was wrong (e.g. private org membership); collaborator
# permission API confirms write+ access.
Expand Down
Loading