Skip to content

Commit 726e911

Browse files
cheese-cakeecheese-cakee
committed
fix: address exploreriii and CodeRabbit review feedback
- Update github-script to v8.0.0 (latest SHA) - Remove setup-node step (pre-installed on runners) - Fix message to say GPG only (not DCO) - Add unverified commit SHAs to bot comment - Add sanitizeUrl function for URL validation - Sanitize all CONFIG env vars - Add try/catch to pagination functions - Implement fail-closed on truncation Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
1 parent b332c0e commit 726e911

File tree

2 files changed

+112
-55
lines changed

2 files changed

+112
-55
lines changed

‎.github/scripts/bot-verified-commits.js‎

Lines changed: 110 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,42 @@
22
// Verifies that all commits in a pull request are GPG-signed.
33
// Posts a one-time VerificationBot comment if unverified commits are found.
44

5-
// Configuration via environment variables
5+
// Sanitizes string input to prevent injection (uses Unicode property escape per Biome lint)
6+
function sanitizeString(input) {
7+
if (typeof input !== 'string') return '';
8+
return input.replace(/\p{Cc}/gu, '').trim();
9+
}
10+
11+
// Validates URL format and returns fallback if invalid
12+
function sanitizeUrl(input, fallback) {
13+
const cleaned = sanitizeString(input);
14+
return /^https?:\/\/[^\s]+$/i.test(cleaned) ? cleaned : fallback;
15+
}
16+
17+
// Configuration via environment variables (sanitized)
618
const CONFIG = {
7-
BOT_NAME: process.env.BOT_NAME || 'VerificationBot',
8-
BOT_LOGIN: process.env.BOT_LOGIN || 'github-actions',
9-
COMMENT_MARKER: process.env.COMMENT_MARKER || '[commit-verification-bot]',
10-
SIGNING_GUIDE_URL: process.env.SIGNING_GUIDE_URL ||
11-
'https://github.com/hiero-ledger/hiero-sdk-python/blob/main/docs/sdk_developers/signing.md',
12-
README_URL: process.env.README_URL ||
13-
'https://github.com/hiero-ledger/hiero-sdk-python/blob/main/README.md',
14-
DISCORD_URL: process.env.DISCORD_URL ||
15-
'https://github.com/hiero-ledger/hiero-sdk-python/blob/main/docs/discord.md',
16-
TEAM_NAME: process.env.TEAM_NAME || 'Hiero Python SDK Team',
19+
BOT_NAME: sanitizeString(process.env.BOT_NAME) || 'VerificationBot',
20+
BOT_LOGIN: sanitizeString(process.env.BOT_LOGIN) || 'github-actions',
21+
COMMENT_MARKER: sanitizeString(process.env.COMMENT_MARKER) || '[commit-verification-bot]',
22+
SIGNING_GUIDE_URL: sanitizeUrl(
23+
process.env.SIGNING_GUIDE_URL,
24+
'https://github.com/hiero-ledger/hiero-sdk-python/blob/main/docs/sdk_developers/signing.md'
25+
),
26+
README_URL: sanitizeUrl(
27+
process.env.README_URL,
28+
'https://github.com/hiero-ledger/hiero-sdk-python/blob/main/README.md'
29+
),
30+
DISCORD_URL: sanitizeUrl(
31+
process.env.DISCORD_URL,
32+
'https://github.com/hiero-ledger/hiero-sdk-python/blob/main/docs/discord.md'
33+
),
34+
TEAM_NAME: sanitizeString(process.env.TEAM_NAME) || 'Hiero Python SDK Team',
1735
MAX_PAGES: (() => {
1836
const parsed = Number.parseInt(process.env.MAX_PAGES ?? '5', 10);
1937
return Number.isInteger(parsed) && parsed > 0 ? parsed : 5;
2038
})(),
2139
};
2240

23-
// Sanitizes string input to prevent injection (uses Unicode property escape per Biome lint)
24-
function sanitizeString(input) {
25-
if (typeof input !== 'string') return '';
26-
return input.replace(/\p{Cc}/gu, '').trim();
27-
}
28-
2941
// Validates PR number is a positive integer
3042
function validatePRNumber(prNumber) {
3143
const num = parseInt(prNumber, 10);
@@ -38,15 +50,29 @@ async function getCommitVerificationStatus(github, owner, repo, prNumber) {
3850

3951
const commits = [];
4052
let page = 0;
41-
for await (const response of github.paginate.iterator(
42-
github.rest.pulls.listCommits,
43-
{ owner, repo, pull_number: prNumber, per_page: 100 }
44-
)) {
45-
commits.push(...response.data);
46-
if (++page >= CONFIG.MAX_PAGES) {
47-
console.log(`[${CONFIG.BOT_NAME}] Reached MAX_PAGES (${CONFIG.MAX_PAGES}) limit`);
48-
break;
53+
let truncated = false;
54+
55+
try {
56+
for await (const response of github.paginate.iterator(
57+
github.rest.pulls.listCommits,
58+
{ owner, repo, pull_number: prNumber, per_page: 100 }
59+
)) {
60+
commits.push(...response.data);
61+
if (++page >= CONFIG.MAX_PAGES) {
62+
truncated = true;
63+
console.warn(`[${CONFIG.BOT_NAME}] Reached MAX_PAGES (${CONFIG.MAX_PAGES}) limit`);
64+
break;
65+
}
4966
}
67+
} catch (error) {
68+
console.error(`[${CONFIG.BOT_NAME}] Failed to list commits`, {
69+
owner,
70+
repo,
71+
prNumber,
72+
status: error?.status,
73+
message: error?.message,
74+
});
75+
throw error;
5076
}
5177

5278
const unverifiedCommits = commits.filter(
@@ -55,9 +81,16 @@ async function getCommitVerificationStatus(github, owner, repo, prNumber) {
5581

5682
console.log(`[${CONFIG.BOT_NAME}] Found ${commits.length} total, ${unverifiedCommits.length} unverified`);
5783

84+
// Fail-closed: if truncated and no unverified found, treat as potentially unverified
85+
const unverifiedCount = truncated && unverifiedCommits.length === 0
86+
? 1
87+
: unverifiedCommits.length;
88+
5889
return {
5990
total: commits.length,
60-
unverified: unverifiedCommits.length,
91+
unverified: unverifiedCount,
92+
unverifiedCommits,
93+
truncated,
6194
};
6295
}
6396

@@ -74,34 +107,63 @@ async function hasExistingBotComment(github, owner, repo, prNumber) {
74107
]);
75108

76109
let page = 0;
77-
for await (const response of github.paginate.iterator(
78-
github.rest.issues.listComments,
79-
{ owner, repo, issue_number: prNumber, per_page: 100 }
80-
)) {
81-
// Early return if marker found
82-
if (response.data.some(comment =>
83-
botLogins.has(comment.user?.login) &&
84-
typeof comment.body === 'string' &&
85-
comment.body.includes(CONFIG.COMMENT_MARKER)
110+
try {
111+
for await (const response of github.paginate.iterator(
112+
github.rest.issues.listComments,
113+
{ owner, repo, issue_number: prNumber, per_page: 100 }
86114
)) {
87-
console.log(`[${CONFIG.BOT_NAME}] Existing bot comment: true`);
88-
return true;
89-
}
90-
if (++page >= CONFIG.MAX_PAGES) {
91-
console.log(`[${CONFIG.BOT_NAME}] Reached MAX_PAGES (${CONFIG.MAX_PAGES}) limit`);
92-
break;
115+
// Early return if marker found
116+
if (response.data.some(comment =>
117+
botLogins.has(comment.user?.login) &&
118+
typeof comment.body === 'string' &&
119+
comment.body.includes(CONFIG.COMMENT_MARKER)
120+
)) {
121+
console.log(`[${CONFIG.BOT_NAME}] Existing bot comment: true`);
122+
return true;
123+
}
124+
if (++page >= CONFIG.MAX_PAGES) {
125+
// Fail-safe: assume comment exists to prevent duplicates
126+
console.warn(
127+
`[${CONFIG.BOT_NAME}] Reached MAX_PAGES (${CONFIG.MAX_PAGES}) limit; assuming existing comment to avoid duplicates`
128+
);
129+
return true;
130+
}
93131
}
132+
} catch (error) {
133+
console.error(`[${CONFIG.BOT_NAME}] Failed to list comments`, {
134+
owner,
135+
repo,
136+
prNumber,
137+
status: error?.status,
138+
message: error?.message,
139+
});
140+
throw error;
94141
}
95142

96143
console.log(`[${CONFIG.BOT_NAME}] Existing bot comment: false`);
97144
return false;
98145
}
99146

100-
// Builds the verification failure comment
101-
function buildVerificationComment(commitsUrl) {
147+
// Builds the verification failure comment with unverified commit details
148+
function buildVerificationComment(commitsUrl, unverifiedCommits = []) {
149+
// Build list of unverified commits (show first 10 max)
150+
const maxDisplay = 10;
151+
const commitList = unverifiedCommits.slice(0, maxDisplay).map(c => {
152+
const sha = c.sha?.substring(0, 7) || 'unknown';
153+
const msg = sanitizeString(c.commit?.message?.split('\n')[0] || 'No message').substring(0, 50);
154+
return `- \`${sha}\` ${msg}`;
155+
}).join('\n');
156+
157+
const moreCommits = unverifiedCommits.length > maxDisplay
158+
? `\n- ...and ${unverifiedCommits.length - maxDisplay} more`
159+
: '';
160+
102161
return `${CONFIG.COMMENT_MARKER}
103162
Hi, this is ${CONFIG.BOT_NAME}.
104-
Your pull request cannot be merged as it has **unverified commits**.
163+
Your pull request cannot be merged as it has **${unverifiedCommits.length} unverified commit(s)**:
164+
165+
${commitList}${moreCommits}
166+
105167
View your commit verification status: [Commits Tab](${sanitizeString(commitsUrl)}).
106168
107169
To achieve verified status, please read:
@@ -118,15 +180,15 @@ From the ${CONFIG.TEAM_NAME}`;
118180
}
119181

120182
// Posts verification failure comment on the PR with error handling
121-
async function postVerificationComment(github, owner, repo, prNumber, commitsUrl) {
183+
async function postVerificationComment(github, owner, repo, prNumber, commitsUrl, unverifiedCommits) {
122184
console.log(`[${CONFIG.BOT_NAME}] Posting verification failure comment...`);
123185

124186
try {
125187
await github.rest.issues.createComment({
126188
owner,
127189
repo,
128190
issue_number: prNumber,
129-
body: buildVerificationComment(commitsUrl),
191+
body: buildVerificationComment(commitsUrl, unverifiedCommits),
130192
});
131193
console.log(`[${CONFIG.BOT_NAME}] Comment posted on PR #${prNumber}`);
132194
return true;
@@ -164,7 +226,7 @@ async function main({ github, context }) {
164226

165227
try {
166228
// Get commit verification status
167-
const { total, unverified } = await getCommitVerificationStatus(github, owner, repo, prNumber);
229+
const { total, unverified, unverifiedCommits } = await getCommitVerificationStatus(github, owner, repo, prNumber);
168230

169231
// All commits verified - success
170232
if (unverified === 0) {
@@ -182,7 +244,7 @@ async function main({ github, context }) {
182244
console.log(`[${CONFIG.BOT_NAME}] Bot already commented. Skipping duplicate.`);
183245
} else {
184246
const commitsUrl = `https://github.com/${owner}/${repo}/pull/${prNumber}/commits`;
185-
await postVerificationComment(github, owner, repo, prNumber, commitsUrl);
247+
await postVerificationComment(github, owner, repo, prNumber, commitsUrl, unverifiedCommits);
186248
}
187249

188250
return { success: false, unverifiedCount: unverified };

‎.github/workflows/bot-verified-commits.yml‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,8 @@ jobs:
5858
sparse-checkout: .github/scripts
5959
persist-credentials: false
6060

61-
- name: Setup Node.js
62-
uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
63-
with:
64-
node-version: "20"
65-
6661
- name: Verify PR commits
67-
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
62+
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0
6863
id: verify
6964
with:
7065
github-token: ${{ secrets.GITHUB_TOKEN }}
@@ -83,6 +78,6 @@ jobs:
8378
if: steps.verify.outputs.success != 'true'
8479
run: |
8580
echo "❌ Pull request has unverified commits."
86-
echo "Please sign your commits with GPG and DCO."
81+
echo "Please sign your commits with GPG."
8782
echo "See: ${{ env.SIGNING_GUIDE_URL }}"
8883
exit 1

0 commit comments

Comments
 (0)