Skip to content

Commit b042778

Browse files
committed
fix: use REST API for draft conversion to avoid contents:write permission
The GraphQL convertPullRequestToDraft mutation requires contents: write, which raises security concerns for workflows triggered by fork PR reviews. Switch to the REST API pulls.update endpoint with draft: true, which works with the existing pull-requests: write permission. - Replace graphql mutation with github.rest.pulls.update - Remove node_id validation since we no longer need GraphQL node IDs - Update mocks and assertions in tests accordingly Addresses review feedback from exploreriii and darshit2308. Signed-off-by: Gourav NSS <gourav341111@gmail.com>
1 parent 8e29a51 commit b042778

3 files changed

Lines changed: 27 additions & 23 deletions

File tree

.github/scripts/revision-guard/helpers/draft.js

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,13 @@ function isDraft(pr) {
77
return pr?.draft === true;
88
}
99

10-
async function convertToDraft(github, pullRequestId) {
11-
await github.graphql(
12-
`
13-
mutation($pullRequestId: ID!) {
14-
convertPullRequestToDraft(input: { pullRequestId: $pullRequestId }) {
15-
pullRequest {
16-
isDraft
17-
}
18-
}
19-
}
20-
`,
21-
{ pullRequestId }
22-
);
10+
async function convertToDraft(github, { owner, repo, pullNumber }) {
11+
await github.rest.pulls.update({
12+
owner,
13+
repo,
14+
pull_number: pullNumber,
15+
draft: true,
16+
});
2317
}
2418

2519
module.exports = {

.github/scripts/revision-guard/index.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ module.exports = async function revisionGuard({ github, context, core }) {
1818
!repo?.repo ||
1919
!pr ||
2020
typeof pr.number !== 'number' ||
21-
!pr.node_id ||
2221
reviewState !== 'changes_requested'
2322
) {
2423
core?.info?.('Skipping revision guard due to missing or non-matching payload data.');
@@ -36,7 +35,11 @@ module.exports = async function revisionGuard({ github, context, core }) {
3635
}
3736

3837
try {
39-
await convertToDraft(github, pr.node_id);
38+
await convertToDraft(github, {
39+
owner: repo.owner,
40+
repo: repo.repo,
41+
pullNumber: pr.number,
42+
});
4043
core?.info?.(`Converted PR #${pr.number} to draft.`);
4144
} catch (error) {
4245
core?.error?.(`Failed to convert PR #${pr.number} to draft: ${error.message}`);

.github/scripts/revision-guard/index.test.js

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,22 @@ function freshRequire() {
1717

1818
function createGithubMock() {
1919
const removedLabels = [];
20+
const pullUpdates = [];
2021

2122
return {
2223
removedLabels,
23-
graphqlCalls: [],
24-
graphql: async function (_query, variables) {
25-
this.graphqlCalls.push(variables);
26-
},
24+
pullUpdates,
2725
rest: {
2826
issues: {
2927
removeLabel: async ({ name }) => {
3028
removedLabels.push(name);
3129
},
3230
},
31+
pulls: {
32+
update: async (params) => {
33+
pullUpdates.push(params);
34+
},
35+
},
3336
},
3437
};
3538
}
@@ -71,7 +74,9 @@ describe('revision-guard index', () => {
7174

7275
await handler({ github, context, core: { info() {} } });
7376

74-
assert.deepEqual(github.graphqlCalls, [{ pullRequestId: 'PR_node_42' }]);
77+
assert.deepEqual(github.pullUpdates, [
78+
{ owner: 'hiero-ledger', repo: 'hiero-sdk-python', pull_number: 42, draft: true },
79+
]);
7580
assert.deepEqual(github.removedLabels, [
7681
'queue:committers',
7782
'status: ready-to-merge',
@@ -93,7 +98,7 @@ describe('revision-guard index', () => {
9398

9499
await handler({ github, context, core: { info() {} } });
95100

96-
assert.equal(github.graphqlCalls.length, 0);
101+
assert.equal(github.pullUpdates.length, 0);
97102
assert.equal(github.removedLabels.length, 0);
98103
});
99104

@@ -112,7 +117,7 @@ describe('revision-guard index', () => {
112117

113118
await handler({ github, context, core: { info() {} } });
114119

115-
assert.equal(github.graphqlCalls.length, 0);
120+
assert.equal(github.pullUpdates.length, 0);
116121
assert.equal(github.removedLabels.length, 0);
117122
});
118123

@@ -137,7 +142,9 @@ describe('revision-guard index', () => {
137142
await handler({ github, context, core: { info() {} } });
138143

139144
// Draft conversion must also fire for configurable-label scenarios.
140-
assert.deepEqual(github.graphqlCalls, [{ pullRequestId: 'PR_node_45' }]);
145+
assert.deepEqual(github.pullUpdates, [
146+
{ owner: 'hiero-ledger', repo: 'hiero-sdk-python', pull_number: 45, draft: true },
147+
]);
141148
// Custom labels AND the matching default (queue:committers) must both be removed.
142149
assert.deepEqual(github.removedLabels, ['queue:committers', 'custom: one', 'custom: two']);
143150
});

0 commit comments

Comments
 (0)