Skip to content

Commit e61bf66

Browse files
zakiskchmouel
authored andcommitted
fix(ci): remove ok-to-test label immediately after check
Move label removal from the pre-check phase to right after the ok-to-test label condition passes. This ensures the label is consumed on use, eliminating the need to strip it on subsequent events like synchronize. Signed-off-by: Zaki Shaikh <zashaikh@redhat.com> Assisted-by: Claude Opus 4.6 (via Claude Code)
1 parent 33c6fae commit e61bf66

File tree

1 file changed

+17
-31
lines changed

1 file changed

+17
-31
lines changed

.github/scripts/check-pr-permissions.js

Lines changed: 17 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -26,36 +26,6 @@ module.exports = async ({ github, context, core }) => {
2626
core.info(`📋 Repository: ${repoOwner}/${repoName}`);
2727
core.info(`🏢 Target organization: ${targetOrg}`);
2828

29-
// Security: On non-labeled events (opened, reopened, synchronize),
30-
// remove the ok-to-test label if present. This prevents an external
31-
// contributor from pushing malicious code after a maintainer approved via label.
32-
if (context.payload.action !== "labeled") {
33-
const currentLabels = context.payload.pull_request.labels.map(
34-
(l) => l.name,
35-
);
36-
if (currentLabels.includes("ok-to-test")) {
37-
core.info(
38-
`🔒 Removing ok-to-test label due to '${context.payload.action}' event — re-approval required.`,
39-
);
40-
try {
41-
await github.rest.issues.removeLabel({
42-
owner: repoOwner,
43-
repo: repoName,
44-
issue_number: context.payload.pull_request.number,
45-
name: "ok-to-test",
46-
});
47-
core.info(` Label removed successfully.`);
48-
} catch (err) {
49-
// 404 is expected when multiple matrix jobs race to remove the same label
50-
if (err.status === 404) {
51-
core.info(` Label already removed (likely by another matrix job).`);
52-
} else {
53-
core.warning(` Failed to remove ok-to-test label: ${err.message}`);
54-
}
55-
}
56-
}
57-
}
58-
5929
// Condition 1: Check if the user is a trusted bot.
6030
const trustedBots = ["dependabot[bot]", "renovate[bot]"];
6131
core.info(`🤖 Checking if @${actor} is a trusted bot...`);
@@ -207,8 +177,24 @@ module.exports = async ({ github, context, core }) => {
207177
context.payload.label.name === "ok-to-test"
208178
) {
209179
core.info(
210-
`✅ Condition met: ok-to-test label applied by @${context.actor}. Proceeding with tests.`,
180+
`✅ Condition met: ok-to-test label applied by @${context.actor}. Removing label and proceeding with tests.`,
211181
);
182+
try {
183+
await github.rest.issues.removeLabel({
184+
owner: repoOwner,
185+
repo: repoName,
186+
issue_number: context.payload.pull_request.number,
187+
name: "ok-to-test",
188+
});
189+
core.info(` ok-to-test label removed successfully.`);
190+
} catch (err) {
191+
// 404 is expected when multiple matrix jobs race to remove the same label
192+
if (err.status !== 404) {
193+
core.setFailed(` Failed to remove ok-to-test label: ${err.message}`);
194+
return;
195+
}
196+
core.info(` Label already removed (likely by another matrix job).`);
197+
}
212198
return;
213199
}
214200

0 commit comments

Comments
 (0)