From 6c219bc178f9d25e22b7dba9bbbb2f888b516178 Mon Sep 17 00:00:00 2001 From: Jonathan Flatt Date: Fri, 23 May 2025 00:32:46 +0000 Subject: [PATCH] fix: Replace axios with Octokit to fix CodeQL SSRF vulnerabilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Install @octokit/rest package for secure GitHub API access - Replace manual URL construction with Octokit client methods - Fix all 6 CodeQL security alerts (3 critical SSRF, 3 medium) - Update all GitHub API calls to use type-safe Octokit methods - Maintain backward compatibility with existing tests This addresses the server-side request forgery vulnerabilities identified by CodeQL by using GitHub's official client library instead of manually constructing API URLs. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- package-lock.json | 213 ++++++++++++++++++++++++++++++++++ package.json | 1 + src/services/githubService.js | 188 ++++++++++++++---------------- 3 files changed, 298 insertions(+), 104 deletions(-) diff --git a/package-lock.json b/package-lock.json index 05fb79e..e99041a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,6 +8,7 @@ "name": "claude-github-webhook", "version": "1.0.0", "dependencies": { + "@octokit/rest": "^21.1.1", "axios": "^1.6.2", "body-parser": "^2.2.0", "commander": "^14.0.0", @@ -1081,6 +1082,190 @@ "url": "https://paulmillr.com/funding/" } }, + "node_modules/@octokit/auth-token": { + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/@octokit/auth-token/-/auth-token-5.1.2.tgz", + "integrity": "sha512-JcQDsBdg49Yky2w2ld20IHAlwr8d/d8N6NiOXbtuoPCqzbsiJgF633mVUw3x4mo0H5ypataQIX7SFu3yy44Mpw==", + "license": "MIT", + "engines": { + "node": ">= 18" + } + }, + "node_modules/@octokit/core": { + "version": "6.1.5", + "resolved": "https://registry.npmjs.org/@octokit/core/-/core-6.1.5.tgz", + "integrity": "sha512-vvmsN0r7rguA+FySiCsbaTTobSftpIDIpPW81trAmsv9TGxg3YCujAxRYp/Uy8xmDgYCzzgulG62H7KYUFmeIg==", + "license": "MIT", + "dependencies": { + "@octokit/auth-token": "^5.0.0", + "@octokit/graphql": "^8.2.2", + "@octokit/request": "^9.2.3", + "@octokit/request-error": "^6.1.8", + "@octokit/types": "^14.0.0", + "before-after-hook": "^3.0.2", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 18" + } + }, + "node_modules/@octokit/endpoint": { + "version": "10.1.4", + "resolved": "https://registry.npmjs.org/@octokit/endpoint/-/endpoint-10.1.4.tgz", + "integrity": "sha512-OlYOlZIsfEVZm5HCSR8aSg02T2lbUWOsCQoPKfTXJwDzcHQBrVBGdGXb89dv2Kw2ToZaRtudp8O3ZIYoaOjKlA==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^14.0.0", + "universal-user-agent": "^7.0.2" + }, + "engines": { + "node": ">= 18" + } + }, + "node_modules/@octokit/graphql": { + "version": "8.2.2", + "resolved": "https://registry.npmjs.org/@octokit/graphql/-/graphql-8.2.2.tgz", + "integrity": "sha512-Yi8hcoqsrXGdt0yObxbebHXFOiUA+2v3n53epuOg1QUgOB6c4XzvisBNVXJSl8RYA5KrDuSL2yq9Qmqe5N0ryA==", + "license": "MIT", + "dependencies": { + "@octokit/request": "^9.2.3", + "@octokit/types": "^14.0.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 18" + } + }, + "node_modules/@octokit/openapi-types": { + "version": "25.0.0", + "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-25.0.0.tgz", + "integrity": "sha512-FZvktFu7HfOIJf2BScLKIEYjDsw6RKc7rBJCdvCTfKsVnx2GEB/Nbzjr29DUdb7vQhlzS/j8qDzdditP0OC6aw==", + "license": "MIT" + }, + "node_modules/@octokit/plugin-paginate-rest": { + "version": "11.6.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-paginate-rest/-/plugin-paginate-rest-11.6.0.tgz", + "integrity": "sha512-n5KPteiF7pWKgBIBJSk8qzoZWcUkza2O6A0za97pMGVrGfPdltxrfmfF5GucHYvHGZD8BdaZmmHGz5cX/3gdpw==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^13.10.0" + }, + "engines": { + "node": ">= 18" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/plugin-paginate-rest/node_modules/@octokit/openapi-types": { + "version": "24.2.0", + "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-24.2.0.tgz", + "integrity": "sha512-9sIH3nSUttelJSXUrmGzl7QUBFul0/mB8HRYl3fOlgHbIWG+WnYDXU3v/2zMtAvuzZ/ed00Ei6on975FhBfzrg==", + "license": "MIT" + }, + "node_modules/@octokit/plugin-paginate-rest/node_modules/@octokit/types": { + "version": "13.10.0", + "resolved": "https://registry.npmjs.org/@octokit/types/-/types-13.10.0.tgz", + "integrity": "sha512-ifLaO34EbbPj0Xgro4G5lP5asESjwHracYJvVaPIyXMuiuXLlhic3S47cBdTb+jfODkTE5YtGCLt3Ay3+J97sA==", + "license": "MIT", + "dependencies": { + "@octokit/openapi-types": "^24.2.0" + } + }, + "node_modules/@octokit/plugin-request-log": { + "version": "5.3.1", + "resolved": "https://registry.npmjs.org/@octokit/plugin-request-log/-/plugin-request-log-5.3.1.tgz", + "integrity": "sha512-n/lNeCtq+9ofhC15xzmJCNKP2BWTv8Ih2TTy+jatNCCq/gQP/V7rK3fjIfuz0pDWDALO/o/4QY4hyOF6TQQFUw==", + "license": "MIT", + "engines": { + "node": ">= 18" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/plugin-rest-endpoint-methods": { + "version": "13.5.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-rest-endpoint-methods/-/plugin-rest-endpoint-methods-13.5.0.tgz", + "integrity": "sha512-9Pas60Iv9ejO3WlAX3maE1+38c5nqbJXV5GrncEfkndIpZrJ/WPMRd2xYDcPPEt5yzpxcjw9fWNoPhsSGzqKqw==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^13.10.0" + }, + "engines": { + "node": ">= 18" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/plugin-rest-endpoint-methods/node_modules/@octokit/openapi-types": { + "version": "24.2.0", + "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-24.2.0.tgz", + "integrity": "sha512-9sIH3nSUttelJSXUrmGzl7QUBFul0/mB8HRYl3fOlgHbIWG+WnYDXU3v/2zMtAvuzZ/ed00Ei6on975FhBfzrg==", + "license": "MIT" + }, + "node_modules/@octokit/plugin-rest-endpoint-methods/node_modules/@octokit/types": { + "version": "13.10.0", + "resolved": "https://registry.npmjs.org/@octokit/types/-/types-13.10.0.tgz", + "integrity": "sha512-ifLaO34EbbPj0Xgro4G5lP5asESjwHracYJvVaPIyXMuiuXLlhic3S47cBdTb+jfODkTE5YtGCLt3Ay3+J97sA==", + "license": "MIT", + "dependencies": { + "@octokit/openapi-types": "^24.2.0" + } + }, + "node_modules/@octokit/request": { + "version": "9.2.3", + "resolved": "https://registry.npmjs.org/@octokit/request/-/request-9.2.3.tgz", + "integrity": "sha512-Ma+pZU8PXLOEYzsWf0cn/gY+ME57Wq8f49WTXA8FMHp2Ps9djKw//xYJ1je8Hm0pR2lU9FUGeJRWOtxq6olt4w==", + "license": "MIT", + "dependencies": { + "@octokit/endpoint": "^10.1.4", + "@octokit/request-error": "^6.1.8", + "@octokit/types": "^14.0.0", + "fast-content-type-parse": "^2.0.0", + "universal-user-agent": "^7.0.2" + }, + "engines": { + "node": ">= 18" + } + }, + "node_modules/@octokit/request-error": { + "version": "6.1.8", + "resolved": "https://registry.npmjs.org/@octokit/request-error/-/request-error-6.1.8.tgz", + "integrity": "sha512-WEi/R0Jmq+IJKydWlKDmryPcmdYSVjL3ekaiEL1L9eo1sUnqMJ+grqmC9cjk7CA7+b2/T397tO5d8YLOH3qYpQ==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^14.0.0" + }, + "engines": { + "node": ">= 18" + } + }, + "node_modules/@octokit/rest": { + "version": "21.1.1", + "resolved": "https://registry.npmjs.org/@octokit/rest/-/rest-21.1.1.tgz", + "integrity": "sha512-sTQV7va0IUVZcntzy1q3QqPm/r8rWtDCqpRAmb8eXXnKkjoQEtFe3Nt5GTVsHft+R6jJoHeSiVLcgcvhtue/rg==", + "license": "MIT", + "dependencies": { + "@octokit/core": "^6.1.4", + "@octokit/plugin-paginate-rest": "^11.4.2", + "@octokit/plugin-request-log": "^5.3.1", + "@octokit/plugin-rest-endpoint-methods": "^13.3.0" + }, + "engines": { + "node": ">= 18" + } + }, + "node_modules/@octokit/types": { + "version": "14.0.0", + "resolved": "https://registry.npmjs.org/@octokit/types/-/types-14.0.0.tgz", + "integrity": "sha512-VVmZP0lEhbo2O1pdq63gZFiGCKkm8PPp8AUOijlwPO6hojEVjspA0MWKP7E4hbvGxzFKNqKr6p0IYtOH/Wf/zA==", + "license": "MIT", + "dependencies": { + "@octokit/openapi-types": "^25.0.0" + } + }, "node_modules/@paralleldrive/cuid2": { "version": "2.2.2", "resolved": "https://registry.npmjs.org/@paralleldrive/cuid2/-/cuid2-2.2.2.tgz", @@ -1711,6 +1896,12 @@ "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", "dev": true }, + "node_modules/before-after-hook": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/before-after-hook/-/before-after-hook-3.0.2.tgz", + "integrity": "sha512-Nik3Sc0ncrMK4UUdXQmAnRtzmNQTAAXmXIopizwZ1W1t8QmfJj+zL4OA2I7XPTPW5z5TDqv4hRo/JzouDJnX3A==", + "license": "Apache-2.0" + }, "node_modules/binary-extensions": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/binary-extensions/-/binary-extensions-2.3.0.tgz", @@ -4010,6 +4201,22 @@ "node": ">=4" } }, + "node_modules/fast-content-type-parse": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/fast-content-type-parse/-/fast-content-type-parse-2.0.1.tgz", + "integrity": "sha512-nGqtvLrj5w0naR6tDPfB4cUmYCqouzyQiz6C5y/LtcDllJdrcc6WaWW6iXyIIOErTa/XRybj28aasdn4LkVk6Q==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/fastify" + }, + { + "type": "opencollective", + "url": "https://opencollective.com/fastify" + } + ], + "license": "MIT" + }, "node_modules/fast-copy": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/fast-copy/-/fast-copy-3.0.2.tgz", @@ -8075,6 +8282,12 @@ "integrity": "sha512-iwDZqg0QAGrg9Rav5H4n0M64c3mkR59cJ6wQp+7C4nI0gsmExaedaYLNO44eT4AtBBwjbTiGPMlt2Md0T9H9JQ==", "dev": true }, + "node_modules/universal-user-agent": { + "version": "7.0.3", + "resolved": "https://registry.npmjs.org/universal-user-agent/-/universal-user-agent-7.0.3.tgz", + "integrity": "sha512-TmnEAEAsBJVZM/AADELsK76llnwcf9vMKuPz8JflO1frO8Lchitr0fNaN9d+Ap0BjKtqWqd/J17qeDnXh8CL2A==", + "license": "ISC" + }, "node_modules/unpipe": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/unpipe/-/unpipe-1.0.0.tgz", diff --git a/package.json b/package.json index 6638873..4fee1f0 100644 --- a/package.json +++ b/package.json @@ -23,6 +23,7 @@ "setup:dev": "husky install" }, "dependencies": { + "@octokit/rest": "^21.1.1", "axios": "^1.6.2", "body-parser": "^2.2.0", "commander": "^14.0.0", diff --git a/src/services/githubService.js b/src/services/githubService.js index 70caf76..bf7f463 100644 --- a/src/services/githubService.js +++ b/src/services/githubService.js @@ -1,9 +1,25 @@ -const axios = require('axios'); +const { Octokit } = require('@octokit/rest'); const { createLogger } = require('../utils/logger'); const secureCredentials = require('../utils/secureCredentials'); const logger = createLogger('githubService'); +// Create Octokit instance (lazy initialization) +let octokit = null; + +function getOctokit() { + if (!octokit) { + const githubToken = secureCredentials.get('GITHUB_TOKEN'); + if (githubToken && githubToken.includes('ghp_')) { + octokit = new Octokit({ + auth: githubToken, + userAgent: 'Claude-GitHub-Webhook' + }); + } + } + return octokit; +} + /** * Posts a comment to a GitHub issue or pull request */ @@ -20,10 +36,9 @@ async function postComment({ repoOwner, repoName, issueNumber, body }) { 'Posting comment to GitHub' ); - const githubToken = secureCredentials.get('GITHUB_TOKEN'); - // In test mode, just log the comment instead of posting to GitHub - if (process.env.NODE_ENV === 'test' || !githubToken || !githubToken.includes('ghp_')) { + const client = getOctokit(); + if (process.env.NODE_ENV === 'test' || !client) { logger.info( { repo: `${repoOwner}/${repoName}`, @@ -40,31 +55,24 @@ async function postComment({ repoOwner, repoName, issueNumber, body }) { }; } - const url = `https://api.github.com/repos/${validated.repoOwner}/${validated.repoName}/issues/${validated.issueNumber}/comments`; - - const response = await axios.post( - url, - { body }, - { - headers: { - Accept: 'application/vnd.github.v3+json', - Authorization: `token ${githubToken}`, - 'Content-Type': 'application/json', - 'User-Agent': 'Claude-GitHub-Webhook' - } - } - ); + // Use Octokit to create comment + const { data } = await client.issues.createComment({ + owner: validated.repoOwner, + repo: validated.repoName, + issue_number: validated.issueNumber, + body: body + }); logger.info( { repo: `${repoOwner}/${repoName}`, issue: issueNumber, - commentId: response.data.id + commentId: data.id }, 'Comment posted successfully' ); - return response.data; + return data; } catch (error) { logger.error( { @@ -117,10 +125,9 @@ async function addLabelsToIssue({ repoOwner, repoName, issueNumber, labels }) { 'Adding labels to GitHub issue' ); - const githubToken = secureCredentials.get('GITHUB_TOKEN'); - // In test mode, just log the labels instead of applying to GitHub - if (process.env.NODE_ENV === 'test' || !githubToken || !githubToken.includes('ghp_')) { + const client = getOctokit(); + if (process.env.NODE_ENV === 'test' || !client) { logger.info( { repo: `${repoOwner}/${repoName}`, @@ -136,31 +143,24 @@ async function addLabelsToIssue({ repoOwner, repoName, issueNumber, labels }) { }; } - const url = `https://api.github.com/repos/${validated.repoOwner}/${validated.repoName}/issues/${validated.issueNumber}/labels`; - - const response = await axios.post( - url, - { labels }, - { - headers: { - Accept: 'application/vnd.github.v3+json', - Authorization: `token ${githubToken}`, - 'Content-Type': 'application/json', - 'User-Agent': 'Claude-GitHub-Webhook' - } - } - ); + // Use Octokit to add labels + const { data } = await client.issues.addLabels({ + owner: validated.repoOwner, + repo: validated.repoName, + issue_number: validated.issueNumber, + labels: labels + }); logger.info( { repo: `${repoOwner}/${repoName}`, issue: issueNumber, - appliedLabels: response.data.map(label => label.name) + appliedLabels: data.map(label => label.name) }, 'Labels added successfully' ); - return response.data; + return data; } catch (error) { logger.error( { @@ -197,10 +197,9 @@ async function createRepositoryLabels({ repoOwner, repoName, labels }) { 'Creating repository labels' ); - const githubToken = secureCredentials.get('GITHUB_TOKEN'); - // In test mode, just log the operation - if (process.env.NODE_ENV === 'test' || !githubToken || !githubToken.includes('ghp_')) { + const client = getOctokit(); + if (process.env.NODE_ENV === 'test' || !client) { logger.info( { repo: `${repoOwner}/${repoName}`, @@ -215,22 +214,20 @@ async function createRepositoryLabels({ repoOwner, repoName, labels }) { for (const label of labels) { try { - const url = `https://api.github.com/repos/${repoOwner}/${repoName}/labels`; - - const response = await axios.post(url, label, { - headers: { - Accept: 'application/vnd.github.v3+json', - Authorization: `token ${githubToken}`, - 'Content-Type': 'application/json', - 'User-Agent': 'Claude-GitHub-Webhook' - } + // Use Octokit to create label + const { data } = await client.issues.createLabel({ + owner: repoOwner, + repo: repoName, + name: label.name, + color: label.color, + description: label.description }); - createdLabels.push(response.data); + createdLabels.push(data); logger.debug({ labelName: label.name }, 'Label created successfully'); } catch (error) { // Label might already exist - check if it's a 422 (Unprocessable Entity) - if (error.response?.status === 422) { + if (error.status === 422) { logger.debug({ labelName: label.name }, 'Label already exists, skipping'); } else { logger.warn( @@ -360,10 +357,9 @@ async function getCombinedStatus({ repoOwner, repoName, ref }) { 'Getting combined status from GitHub' ); - const githubToken = secureCredentials.get('GITHUB_TOKEN'); - // In test mode, return a mock successful status - if (process.env.NODE_ENV === 'test' || !githubToken || !githubToken.includes('ghp_')) { + const client = getOctokit(); + if (process.env.NODE_ENV === 'test' || !client) { logger.info( { repo: `${repoOwner}/${repoName}`, @@ -382,27 +378,24 @@ async function getCombinedStatus({ repoOwner, repoName, ref }) { }; } - const url = `https://api.github.com/repos/${repoOwner}/${repoName}/commits/${ref}/status`; - - const response = await axios.get(url, { - headers: { - Accept: 'application/vnd.github.v3+json', - Authorization: `token ${githubToken}`, - 'User-Agent': 'Claude-GitHub-Webhook' - } + // Use Octokit to get combined status + const { data } = await client.repos.getCombinedStatusForRef({ + owner: repoOwner, + repo: repoName, + ref: ref }); logger.info( { repo: `${repoOwner}/${repoName}`, ref: ref, - state: response.data.state, - totalCount: response.data.total_count + state: data.state, + totalCount: data.total_count }, 'Combined status retrieved successfully' ); - return response.data; + return data; } catch (error) { logger.error( { @@ -447,27 +440,22 @@ async function hasReviewedPRAtCommit({ repoOwner, repoName, prNumber, commitSha 'Checking if PR has been reviewed at commit' ); - const githubToken = secureCredentials.get('GITHUB_TOKEN'); - // In test mode, return false to allow review - if (process.env.NODE_ENV === 'test' || !githubToken || !githubToken.includes('ghp_')) { + const client = getOctokit(); + if (process.env.NODE_ENV === 'test' || !client) { return false; } - // Get review comments for this PR - const url = `https://api.github.com/repos/${repoOwner}/${repoName}/pulls/${prNumber}/reviews`; - - const response = await axios.get(url, { - headers: { - Accept: 'application/vnd.github.v3+json', - Authorization: `token ${githubToken}`, - 'User-Agent': 'Claude-GitHub-Webhook' - } + // Get review comments for this PR using Octokit + const { data: reviews } = await client.pulls.listReviews({ + owner: repoOwner, + repo: repoName, + pull_number: prNumber }); // Check if any review mentions this specific commit SHA const botUsername = process.env.BOT_USERNAME || 'ClaudeBot'; - const existingReview = response.data.find(review => { + const existingReview = reviews.find(review => { return review.user.login === botUsername && review.body && review.body.includes(`commit: ${commitSha}`); @@ -505,10 +493,9 @@ async function managePRLabels({ repoOwner, repoName, prNumber, labelsToAdd = [], throw new Error('Invalid repository owner or name - contains unsafe characters'); } - const githubToken = secureCredentials.get('GITHUB_TOKEN'); - // In test mode, just log - if (process.env.NODE_ENV === 'test' || !githubToken || !githubToken.includes('ghp_')) { + const client = getOctokit(); + if (process.env.NODE_ENV === 'test' || !client) { logger.info( { repo: `${repoOwner}/${repoName}`, @@ -521,16 +508,14 @@ async function managePRLabels({ repoOwner, repoName, prNumber, labelsToAdd = [], return; } - // Remove labels first + // Remove labels first using Octokit for (const label of labelsToRemove) { try { - const url = `https://api.github.com/repos/${repoOwner}/${repoName}/issues/${prNumber}/labels/${encodeURIComponent(label)}`; - await axios.delete(url, { - headers: { - Accept: 'application/vnd.github.v3+json', - Authorization: `token ${githubToken}`, - 'User-Agent': 'Claude-GitHub-Webhook' - } + await client.issues.removeLabel({ + owner: repoOwner, + repo: repoName, + issue_number: prNumber, + name: label }); logger.info( { @@ -542,7 +527,7 @@ async function managePRLabels({ repoOwner, repoName, prNumber, labelsToAdd = [], ); } catch (error) { // Ignore 404 errors (label not present) - if (error.response?.status !== 404) { + if (error.status !== 404) { logger.error( { err: error.message, @@ -554,19 +539,14 @@ async function managePRLabels({ repoOwner, repoName, prNumber, labelsToAdd = [], } } - // Add new labels + // Add new labels using Octokit if (labelsToAdd.length > 0) { - const url = `https://api.github.com/repos/${repoOwner}/${repoName}/issues/${prNumber}/labels`; - await axios.post(url, - { labels: labelsToAdd }, - { - headers: { - Accept: 'application/vnd.github.v3+json', - Authorization: `token ${githubToken}`, - 'User-Agent': 'Claude-GitHub-Webhook' - } - } - ); + await client.issues.addLabels({ + owner: repoOwner, + repo: repoName, + issue_number: prNumber, + labels: labelsToAdd + }); logger.info( { repo: `${repoOwner}/${repoName}`,