From d0fe2ff7887766e04184abdcdf3be3113aa6c774 Mon Sep 17 00:00:00 2001 From: Phillip Johnsen Date: Sat, 6 May 2017 22:02:18 +0200 Subject: [PATCH 1/2] jenkins: whitelist IPs allowed to push status changes This is needed to ensure not everyone on the internet can push an inline status to any PR if they know the bot URL. --- README.md | 2 ++ scripts/jenkins-status.js | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/README.md b/README.md index a9a7631c..3ae50d0f 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,8 @@ See [CONTRIBUTING.md](CONTRIBUTING.md). The webhook secret that GitHub signs the POSTed payloads with. This is created when the webhook is defined. The default is `hush-hush`. - **`TRAVIS_CI_TOKEN`**
For scripts that communicate with Travis CI. Your Travis token is visible on [yourprofile](https://travis-ci.org/profile) page, by clicking the "show token" link. Also See: https://blog.travis-ci.com/2013-01-28-token-token-token +- **`JENKINS_WORKER_IPS`**
+ List of valid Jenkins worker IPs allowed to push PR status updates, split by comma: `192.168.1.100,192.168.1.101`. - **`JENKINS_API_CREDENTIALS`** (optional)
For scripts that communicate with Jenkins on http://ci.nodejs.org. The Jenkins API token is visible on your own profile page `https://ci.nodejs.org/user//configure`, by clicking the diff --git a/scripts/jenkins-status.js b/scripts/jenkins-status.js index c815533b..6472778c 100644 --- a/scripts/jenkins-status.js +++ b/scripts/jenkins-status.js @@ -3,6 +3,21 @@ const pushJenkinsUpdate = require('../lib/push-jenkins-update') const enabledRepos = ['citgm', 'node'] +const jenkinsIpWhitelist = process.env.JENKINS_WORKER_IPS ? process.env.JENKINS_WORKER_IPS.split(',') : [] + +function isJenkinsIpWhitelisted (req) { + const ip = req.headers['x-forwarded-for'] || req.connection.remoteAddress + + if (jenkinsIpWhitelist.length) { + if (!jenkinsIpWhitelist.includes(ip)) { + req.log.warn({ ip }, 'Ignoring, not allowed to push Jenkins updates') + return false + } + } + + return true +} + module.exports = function (app) { app.post('/:repo/jenkins/start', (req, res) => { const isValid = pushJenkinsUpdate.validate(req.body) @@ -16,6 +31,10 @@ module.exports = function (app) { return res.status(400).end('Invalid repository') } + if (!isJenkinsIpWhitelisted(req)) { + return res.status(401).end('Invalid Jenkins IP') + } + pushJenkinsUpdate.pushStarted({ owner: 'nodejs', repo, @@ -37,6 +56,10 @@ module.exports = function (app) { return res.status(400).end('Invalid repository') } + if (!isJenkinsIpWhitelisted(req)) { + return res.status(401).end('Invalid Jenkins IP') + } + pushJenkinsUpdate.pushEnded({ owner: 'nodejs', repo, From fc72f9ad7a800c5e4c19884e992f7de5e972d57d Mon Sep 17 00:00:00 2001 From: Phillip Johnsen Date: Fri, 14 Jul 2017 22:15:42 +0200 Subject: [PATCH 2/2] removed use of x-forwarded-for header --- scripts/jenkins-status.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/scripts/jenkins-status.js b/scripts/jenkins-status.js index 6472778c..06084f95 100644 --- a/scripts/jenkins-status.js +++ b/scripts/jenkins-status.js @@ -6,13 +6,11 @@ const enabledRepos = ['citgm', 'node'] const jenkinsIpWhitelist = process.env.JENKINS_WORKER_IPS ? process.env.JENKINS_WORKER_IPS.split(',') : [] function isJenkinsIpWhitelisted (req) { - const ip = req.headers['x-forwarded-for'] || req.connection.remoteAddress + const ip = req.connection.remoteAddress - if (jenkinsIpWhitelist.length) { - if (!jenkinsIpWhitelist.includes(ip)) { - req.log.warn({ ip }, 'Ignoring, not allowed to push Jenkins updates') - return false - } + if (jenkinsIpWhitelist.length && !jenkinsIpWhitelist.includes(ip)) { + req.log.warn({ ip }, 'Ignoring, not allowed to push Jenkins updates') + return false } return true