Skip to content

Commit 6f0d774

Browse files
chmouelclaude
chmouel
and
claude
committed
fix: Refactor GH Actions script permission checks
Refactored the GitHub script used in the E2E workflow for checking pull request submitter permissions. The logic was expanded to check for trusted bots, organization team membership, organization membership, and finally repository collaborator permissions, allowing for a more robust set of conditions to permit workflow execution. Additionally, removed legacy steps related to logging test failures and Slack reporting that are now handled elsewhere or implicitly by the job status. The step was also simplified to rely on the standard test runner output. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
1 parent 2106ee6 commit 6f0d774

File tree

1 file changed

+156
-54
lines changed

1 file changed

+156
-54
lines changed

‎.github/workflows/e2e.yaml‎

Lines changed: 156 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ jobs:
4343
provider: [providers, gitea_others]
4444

4545
env:
46+
TARGET_TEAM_SLUGS: "pipeline-as-code,pipeline-as-code-contributors"
4647
KO_DOCKER_REPO: localhost:5000
4748
CONTROLLER_DOMAIN_URL: controller.paac-127-0-0-1.nip.io
4849
TEST_GITHUB_REPO_OWNER_GITHUBAPP: openshift-pipelines/pipelines-as-code-e2e-tests
@@ -79,35 +80,164 @@ jobs:
7980
uses: actions/github-script@v8
8081
with:
8182
script: |
82-
const actor = context.payload.pull_request.user.login;
83-
const org = context.repo.owner;
84-
85-
// Allow a specific list of trusted bots to bypass the permission check.
86-
const trustedBots = ['dependabot[bot]']; // Add any other trusted bot accounts here
87-
if (trustedBots.includes(actor)) {
88-
core.info(`User @${actor} is a trusted bot, allowing.`);
83+
if (!github || !github.context || !github.context.payload || !github.context.payload.pull_request) {
84+
core.setFailed('Invalid GitHub context: missing required pull_request information');
8985
return;
9086
}
87+
const context = github.context;
88+
89+
async function run() {
90+
const actor = context.payload.pull_request.user.login;
91+
const repoOwner = context.repo.owner;
92+
const repoName = context.repo.repo;
93+
const targetOrg = context.repo.owner;
94+
95+
core.info(`🔍 Starting permission check for user: @${actor}`);
96+
core.info(`📋 Repository: ${repoOwner}/${repoName}`);
97+
core.info(`🏢 Target organization: ${targetOrg}`);
98+
99+
// Condition 1: Check if the user is a trusted bot.
100+
const trustedBots = ["dependabot[bot]", "renovate[bot]"];
101+
core.info(`🤖 Checking if @${actor} is a trusted bot...`);
102+
core.info(` Trusted bots list: ${trustedBots.join(', ')}`);
103+
104+
if (trustedBots.includes(actor)) {
105+
core.info(`✅ Condition met: User @${actor} is a trusted bot. Proceeding.`);
106+
return; // Success
107+
}
108+
core.info(` ❌ User @${actor} is not a trusted bot.`);
109+
110+
// Condition 2: Check for public membership in the target organization.
111+
core.info(`\n👥 Condition 2: Checking organization and team membership...`);
112+
core.info(
113+
`User @${actor} is not a trusted bot. Checking for membership in '${targetOrg}'...`,
114+
);
115+
try {
116+
// Optional: check membership in one or more org teams (set TARGET_TEAM_SLUGS as comma-separated slugs in workflow env)
117+
const teamSlugsEnv = process.env.TARGET_TEAM_SLUGS || "";
118+
const teamSlugs = teamSlugsEnv
119+
.split(",")
120+
.map((s) => s.trim())
121+
.filter(Boolean);
122+
123+
core.info(`🔧 TARGET_TEAM_SLUGS environment variable: "${teamSlugsEnv}"`);
124+
core.info(`📝 Parsed team slugs: [${teamSlugs.join(', ')}]`);
125+
126+
if (teamSlugs.length > 0) {
127+
core.info(`🔍 Checking team membership for ${teamSlugs.length} team(s)...`);
128+
for (const team_slug of teamSlugs) {
129+
core.info(` Checking team: ${team_slug}...`);
130+
try {
131+
const membership = await github.rest.teams.getMembershipForUserInOrg({
132+
org: targetOrg,
133+
team_slug,
134+
username: actor,
135+
});
136+
core.info(` API response for team '${team_slug}': ${JSON.stringify(membership.data)}`);
137+
if (
138+
membership &&
139+
membership.data &&
140+
membership.data.state === "active"
141+
) {
142+
core.info(
143+
`✅ Condition met: User @${actor} is a member of team '${team_slug}' in '${targetOrg}'. Proceeding.`,
144+
);
145+
return; // Success
146+
} else {
147+
core.info(` ⚠️ Team membership found but state is not 'active': ${membership.data.state}`);
148+
}
149+
} catch (err) {
150+
// Not a member of this team or team doesn't exist — continue to next
151+
core.info(
152+
` ❌ User @${actor} is not a member of team '${team_slug}' (or team not found). Error: ${err.message}`,
153+
);
154+
}
155+
}
156+
// If we tried team checks and none matched, continue to next org membership checks
157+
core.info(
158+
`ⓘ User @${actor} is not a member of any configured teams in '${targetOrg}'. Falling back to org membership checks.`,
159+
);
160+
} else {
161+
core.info(`ℹ️ No teams configured in TARGET_TEAM_SLUGS. Skipping team membership checks.`);
162+
}
163+
core.info(`🏢 Checking organization membership for @${actor} in '${targetOrg}'...`);
164+
try {
165+
core.info(` Attempting checkMembershipForUser API call...`);
166+
await github.rest.orgs.checkMembershipForUser({
167+
org: targetOrg,
168+
username: actor,
169+
});
170+
core.info(
171+
`✅ Condition met: User @${actor} is a member of '${targetOrg}'. Proceeding.`,
172+
);
173+
return; // Success
174+
} catch (err) {
175+
// Try public membership as fallback
176+
core.info(` ❌ Private membership check failed: ${err.message}`);
177+
core.info(` Attempting checkPublicMembershipForUser API call...`);
178+
try {
179+
await github.rest.orgs.checkPublicMembershipForUser({
180+
org: targetOrg,
181+
username: actor,
182+
});
183+
core.info(
184+
`✅ Condition met: User @${actor} is a public member of '${targetOrg}'. Proceeding.`,
185+
);
186+
return; // Success
187+
} catch (publicErr) {
188+
// Neither private nor public member - will be caught by outer catch
189+
core.info(` ❌ Public membership check failed: ${publicErr.message}`);
190+
throw publicErr;
191+
}
192+
}
193+
} catch (error) {
194+
// This is not a failure, just one unmet condition. Log and continue.
195+
core.info(
196+
`ⓘ User @${actor} is not a public member of '${targetOrg}'. Checking repository permissions as a fallback.`,
197+
);
198+
}
199+
200+
// Condition 3: Check for write/admin permission on the repository.
201+
core.info(`\n🔐 Condition 3: Checking repository collaborator permissions...`);
202+
try {
203+
core.info(` Attempting getCollaboratorPermissionLevel API call...`);
204+
const response = await github.rest.repos.getCollaboratorPermissionLevel({
205+
owner: repoOwner,
206+
repo: repoName,
207+
username: actor,
208+
});
209+
210+
core.info(` API response: ${JSON.stringify(response.data)}`);
211+
const permission = response.data.permission;
212+
core.info(` User @${actor} has '${permission}' permission on ${repoOwner}/${repoName}`);
91213
92-
try {
93-
// Directly check the user's permission level on the repository.
94-
// This covers both org members and external collaborators with sufficient access.
95-
const response = await github.rest.repos.getCollaboratorPermissionLevel({
96-
owner: org,
97-
repo: context.repo.repo,
98-
username: actor,
99-
});
100-
101-
const permission = response.data.permission;
102-
if (permission !== 'admin' && permission !== 'write') {
103-
core.setFailed(`❌ User @${actor} has only '${permission}' repository permission. 'write' or 'admin' is required.`);
104-
} else {
105-
core.info(`✅ User @${actor} has '${permission}' repository permission. Proceeding.`);
214+
if (permission === "admin" || permission === "write") {
215+
core.info(
216+
`✅ Condition met: User @${actor} has '${permission}' repository permission. Proceeding.`,
217+
);
218+
return; // Success
219+
} else {
220+
// If we reach here, no conditions were met. This is the final failure.
221+
core.info(` ❌ Permission '${permission}' is insufficient (requires 'write' or 'admin')`);
222+
core.setFailed(
223+
`❌ Permission check failed. User @${actor} did not meet any required conditions (trusted bot, org member, or repo write access).`,
224+
);
225+
}
226+
} catch (error) {
227+
// This error means they are not even a collaborator.
228+
core.info(` ❌ Collaborator permission check failed: ${error.message}`);
229+
core.setFailed(
230+
`❌ Permission check failed. User @${actor} is not a collaborator on this repository and did not meet other conditions.`,
231+
);
106232
}
107-
} catch (error) {
108-
core.setFailed(`Permission check failed for @${actor}. They are likely not a collaborator on the repository. Error: ${error.message}`);
109233
}
110234
235+
run().catch(err => {
236+
core.error(`💥 Unexpected error during permission check: ${err.message}`);
237+
core.error(` Stack trace: ${err.stack}`);
238+
core.setFailed(`Unexpected error during permission check: ${err.message}`);
239+
});
240+
111241
- uses: actions/setup-go@v6
112242
with:
113243
go-version-file: "go.mod"
@@ -175,9 +305,7 @@ jobs:
175305
TEST_BITBUCKET_SERVER_API_URL: ${{ secrets.BITBUCKET_SERVER_API_URL }}
176306
TEST_BITBUCKET_SERVER_WEBHOOK_SECRET: ${{ secrets.BITBUCKET_SERVER_WEBHOOK_SECRET }}
177307
run: |
178-
set -o pipefail
179-
LOG_FILE="/tmp/e2e-tests-${{ matrix.provider }}.log"
180-
./hack/gh-workflow-ci.sh run_e2e_tests 2>&1 | tee "${LOG_FILE}"
308+
./hack/gh-workflow-ci.sh run_e2e_tests
181309
182310
- name: Run E2E Tests on nightly
183311
# This step still runs specifically for schedule or workflow_dispatch
@@ -196,30 +324,7 @@ jobs:
196324
TEST_BITBUCKET_SERVER_API_URL: ${{ secrets.BITBUCKET_SERVER_API_URL }}
197325
TEST_BITBUCKET_SERVER_WEBHOOK_SECRET: ${{ secrets.BITBUCKET_SERVER_WEBHOOK_SECRET }}
198326
run: |
199-
set -o pipefail
200-
LOG_FILE="/tmp/e2e-tests-${{ matrix.provider }}.log"
201-
./hack/gh-workflow-ci.sh run_e2e_tests 2>&1 | tee "${LOG_FILE}"
202-
203-
- name: Summarize failing tests
204-
if: ${{ failure() }}
205-
id: failing-tests
206-
run: |
207-
LOG_FILE="/tmp/e2e-tests-${{ matrix.provider }}.log"
208-
if [[ ! -f "${LOG_FILE}" || ! -s "${LOG_FILE}" ]]; then
209-
echo "slack_message=E2E job failed but no test log was captured for provider '${{ matrix.provider }}'." >> "${GITHUB_OUTPUT}"
210-
exit 0
211-
fi
212-
213-
mapfile -t FAILURES < <(grep -E '^--- FAIL: ' "${LOG_FILE}" | sed -E 's/^--- FAIL: ([^[:space:]]+).*/\1/' | sort -u)
214-
215-
if [[ ${#FAILURES[@]} -eq 0 ]]; then
216-
echo "slack_message=E2E job failed without explicit go test failures; check workflow logs for provider '${{ matrix.provider }}'." >> "${GITHUB_OUTPUT}"
217-
exit 0
218-
fi
219-
220-
joined_failures=$(printf '%s, ' "${FAILURES[@]}")
221-
joined_failures=${joined_failures%, }
222-
echo "slack_message=Failing tests for provider ${{ matrix.provider }}: ${joined_failures}" >> "${GITHUB_OUTPUT}"
327+
./hack/gh-workflow-ci.sh run_e2e_tests
223328
224329
- name: Collect logs
225330
if: ${{ always() }}
@@ -241,14 +346,11 @@ jobs:
241346
name: logs-e2e-tests-${{ matrix.provider }}
242347
path: /tmp/logs
243348

244-
- name: Report E2E test results to Slack
349+
- name: Report Status
245350
if: ${{ always() && github.ref_name == 'main' && github.event_name == 'schedule' }}
246351
uses: ravsamhq/notify-slack-action@v2
247352
with:
248353
status: ${{ job.status }}
249354
notify_when: "failure"
250-
message_format: |
251-
{emoji} *{workflow}* {status_message} in <{repo_url}|{repo}@{branch}> on <{commit_url}|{commit_sha}>
252-
${{ steps.failing-tests.outputs.slack_message }}
253355
env:
254356
SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}

0 commit comments

Comments
 (0)