From 25b6aaa26f13273c48c4b3239d6f00edadb22fdd Mon Sep 17 00:00:00 2001 From: saibhotla Date: Thu, 17 Oct 2024 10:10:25 -0500 Subject: [PATCH] feat: add new package.json for improved secrets reporting feat: format violations on block feat: improve test --- package.json | 2 + src/proxy/processors/push-action/getDiff.js | 19 +- src/proxy/processors/push-action/scanDiff.js | 170 +++++++---- test/scanDiff.test.js | 280 +++++++++++++++++++ 4 files changed, 410 insertions(+), 61 deletions(-) create mode 100644 test/scanDiff.test.js diff --git a/package.json b/package.json index 54398b71..8928e731 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "moment": "^2.29.4", "mongodb": "^5.0.0", "nodemailer": "^6.6.1", + "parse-diff": "^0.11.1", "passport": "^0.7.0", "passport-activedirectory": "^1.0.4", "passport-local": "^1.0.0", @@ -65,6 +66,7 @@ "react-dom": "^16.13.1", "react-html-parser": "^2.0.2", "react-router-dom": "6.26.2", + "simple-git": "^3.25.0", "uuid": "^10.0.0", "yargs": "^17.7.2" }, diff --git a/src/proxy/processors/push-action/getDiff.js b/src/proxy/processors/push-action/getDiff.js index 4344810b..0a91161f 100644 --- a/src/proxy/processors/push-action/getDiff.js +++ b/src/proxy/processors/push-action/getDiff.js @@ -1,12 +1,13 @@ -const child = require('child_process'); const Step = require('../../actions').Step; +const simpleGit = require('simple-git') + const exec = async (req, action) => { const step = new Step('diff'); try { const path = `${action.proxyGitPath}/${action.repoName}`; - + const git = simpleGit(path); // https://stackoverflow.com/questions/40883798/how-to-get-git-diff-of-the-first-commit let commitFrom = `4b825dc642cb6eb9a060e54bf8d69288fbee4904`; @@ -19,16 +20,10 @@ const exec = async (req, action) => { } step.log(`Executing "git diff ${commitFrom} ${action.commitTo}" in ${path}`); - - // Get the diff - const content = child.spawnSync('git', ['diff', commitFrom, action.commitTo], { - cwd: path, - encoding: 'utf-8', - maxBuffer: 50 * 1024 * 1024, - }).stdout; - - step.log(content); - step.setContent(content); + const revisionRange = `${commitFrom}..${action.commitTo}`; + const diff = await git.diff([revisionRange]); + step.log(diff); + step.setContent(diff); } catch (e) { step.setError(e.toString('utf-8')); } finally { diff --git a/src/proxy/processors/push-action/scanDiff.js b/src/proxy/processors/push-action/scanDiff.js index 3fcce9d5..61fa529c 100644 --- a/src/proxy/processors/push-action/scanDiff.js +++ b/src/proxy/processors/push-action/scanDiff.js @@ -1,73 +1,136 @@ const Step = require('../../actions').Step; const config = require('../../../config'); +const parseDiff = require('parse-diff') const commitConfig = config.getCommitConfig(); const privateOrganizations = config.getPrivateOrganizations(); -const isDiffLegal = (diff, organization) => { +const BLOCK_TYPE = { + LITERAL: 'Offending Literal', + PATTERN: 'Offending Pattern', + PROVIDER: 'PROVIDER' +} + + +const getDiffViolations = (diff, organization) => { // Commit diff is empty, i.e. '', null or undefined if (!diff) { console.log('No commit diff...'); - return false; + return 'No commit diff...'; } // Validation for configured block pattern(s) check... if (typeof diff !== 'string') { console.log('A non-string value has been captured for the commit diff...'); - return false; + return 'A non-string value has been captured for the commit diff...'; } - // Configured blocked literals - const blockedLiterals = commitConfig.diff.block.literals; - - // Configured blocked patterns - const blockedPatterns = commitConfig.diff.block.patterns; - - // Configured blocked providers - const blockedProviders = Object.values(commitConfig.diff.block.providers); - - // Find all instances of blocked literals in diff... - const positiveLiterals = blockedLiterals.map((literal) => - diff.toLowerCase().includes(literal.toLowerCase()), - ); - - // Find all instances of blocked patterns in diff... - const positivePatterns = blockedPatterns.map((pattern) => diff.match(new RegExp(pattern, 'gi'))); - - // Find all instances of blocked providers in diff... - const positiveProviders = blockedProviders.map((pattern) => - diff.match(new RegExp(pattern, 'gi')), - ); - - console.log({ positiveLiterals }); - console.log({ positivePatterns }); - console.log({ positiveProviders }); - - // Flatten any positive literal results into a 1D array... - const literalMatches = positiveLiterals.flat().filter((result) => !!result); + const parsedDiff = parseDiff(diff); + const combinedMatches = combineMatches(organization); - // Flatten any positive pattern results into a 1D array... - const patternMatches = positivePatterns.flat().filter((result) => !!result); - - // Flatten any positive pattern results into a 1D array... - const providerMatches = - organization && privateOrganizations.includes(organization) // Return empty results for private organizations - ? [] - : positiveProviders.flat().filter((result) => !!result); - - console.log({ literalMatches }); - console.log({ patternMatches }); - console.log({ providerMatches }); + const res = collectMatches(parsedDiff, combinedMatches); // Diff matches configured block pattern(s) - if (literalMatches.length || patternMatches.length || providerMatches.length) { + if (res.length > 0) { console.log('Diff is blocked via configured literals/patterns/providers...'); - return false; + // combining matches with file and line number + return res } - return true; + return null; }; +const combineMatches = (organization) => { + + // Configured blocked literals + const blockedLiterals = commitConfig.diff.block.literals; + + // Configured blocked patterns + const blockedPatterns = commitConfig.diff.block.patterns; + + // Configured blocked providers + const blockedProviders = organization && privateOrganizations.includes(organization) ? [] : + Object.entries(commitConfig.diff.block.providers); + + // Combine all matches (literals, paterns) + const combinedMatches = [ + ...blockedLiterals.map(literal => ({ + type: BLOCK_TYPE.LITERAL, + match: new RegExp(literal, 'gi') + })), + ...blockedPatterns.map(pattern => ({ + type: BLOCK_TYPE.PATTERN, + match: new RegExp(pattern, 'gi') + })), + ...blockedProviders.map(([key, value]) => ({ + type: key, + match: new RegExp(value, 'gi') + })), + ]; + return combinedMatches; +} + +const collectMatches = (parsedDiff, combinedMatches) => { + const allMatches = {}; + parsedDiff.forEach(file => { + const fileName = file.to || file.from; + console.log("CHANGE", file.chunks) + + file.chunks.forEach(chunk => { + chunk.changes.forEach(change => { + if (change.add) { + // store line number + const lineNumber = change.ln; + // Iterate through each match types - literal, patterns, providers + combinedMatches.forEach(({ type, match }) => { + // using Match all to find all occurences of the pattern in the line + const matches = [...change.content.matchAll(match)] + + matches.forEach(matchInstance => { + const matchLiteral = matchInstance[0]; + const matchKey = `${type}_${matchLiteral}_${fileName}`; // unique key + + + if (!allMatches[matchKey]) { + // match entry + allMatches[matchKey] = { + type, + literal: matchLiteral, + file: fileName, + lines: [], + content: change.content.trim() + }; + } + + // apend line numbers to the list of lines + allMatches[matchKey].lines.push(lineNumber) + }) + }); + } + }); + }); + }); + + // convert matches into a final result array, joining line numbers + const result = Object.values(allMatches).map(match => ({ + ...match, + lines: match.lines.join(',') // join the line numbers into a comma-separated string + })) + + console.log("RESULT", result) + return result; +} + +const formatMatches = (matches) => { + return matches.map((match, index) => { + return `---------------------------------- #${index + 1} ${match.type} ------------------------------ + Policy Exception Type: ${match.type} + DETECTED: ${match.literal} + FILE(S) LOCATED: ${match.file} + Line(s) of code: ${match.lines}` + }); +} + const exec = async (req, action) => { const step = new Step('scanDiff'); @@ -76,17 +139,26 @@ const exec = async (req, action) => { const diff = steps.find((s) => s.stepName === 'diff')?.content; - const legalDiff = isDiffLegal(diff, action.project); + console.log(diff) + const diffViolations = getDiffViolations(diff, action.project); + + if (diffViolations) { + const formattedMatches = Array.isArray(diffViolations) ? formatMatches(diffViolations).join('\n\n') : diffViolations; + const errorMsg = []; + errorMsg.push(`\n\n\n\nYour push has been blocked.\n`); + errorMsg.push(`Please ensure your code does not contain sensitive information or URLs.\n\n`); + errorMsg.push(formattedMatches) + errorMsg.push('\n') - if (!legalDiff) { console.log(`The following diff is illegal: ${commitFrom}:${commitTo}`); step.error = true; step.log(`The following diff is illegal: ${commitFrom}:${commitTo}`); step.setError( - '\n\n\n\nYour push has been blocked.\nPlease ensure your code does not contain sensitive information or URLs.\n\n\n', + errorMsg.join('\n') ); + action.addStep(step); return action; } diff --git a/test/scanDiff.test.js b/test/scanDiff.test.js new file mode 100644 index 00000000..a8f64819 --- /dev/null +++ b/test/scanDiff.test.js @@ -0,0 +1,280 @@ +const chai = require('chai'); +const crypto = require('crypto'); +const processor = require('../src/proxy/processors/push-action/scanDiff.js'); +const { Action } = require('../src/proxy/actions/Action'); +const { expect } = chai; +const config = require('../src/config'); +chai.should(); + +// Load blocked literals and patterns from configuration... +const commitConfig = require('../src/config/index').getCommitConfig(); +const privateOrganizations = config.getPrivateOrganizations(); + +const blockedLiterals = commitConfig.diff.block.literals; +const generateDiff = (value) => { + return `diff --git a/package.json b/package.json +index 38cdc3e..8a9c321 100644 +--- a/package.json ++++ b/package.json +@@ -36,7 +36,7 @@ + "express-session": "^1.17.1", + "generate-password": "^1.5.1", + "history": "5.3.0", +- "lodash": "^4.17.21", ++ "lodash": "^4.1${value}7.21", + "moment": "^2.29.4", + "mongodb": "^5.0", + "nodemailer": "^6.6.1", + `; +}; + +const generateMultiLineDiff = () => { + return `diff --git a/README.md b/README.md +index 8b97e49..de18d43 100644 +--- a/README.md ++++ b/README.md +@@ -1,2 +1,5 @@ + # gitproxy-test-delete-me + Project to test gitproxy ++AKIAIOSFODNN7EXAMPLE ++AKIAIOSFODNN7EXAMPLE ++AKIAIOSFODNN8EXAMPLE +`; +}; + +const generateMultiLineDiffWithLiteral = () => { + return `diff --git a/README.md b/README.md +index 8b97e49..de18d43 100644 +--- a/README.md ++++ b/README.md +@@ -1,2 +1,5 @@ + # gitproxy-test-delete-me + Project to test gitproxy ++AKIAIOSFODNN7EXAMPLE ++AKIAIOSFODNN8EXAMPLE ++blockedTestLiteral +`; +}; +describe('Scan commit diff...', async () => { + privateOrganizations[0] = "private-org-test" + commitConfig.diff = { + + "block": { + "literals": ["blockedTestLiteral"], + "patterns": [], + "providers": { + "AWS (Amazon Web Services) Access Key ID": "A(AG|CC|GP|ID|IP|KI|NP|NV|PK|RO|SC|SI)A[A-Z0-9]{16}", + "Google Cloud Platform API Key": "AIza[0-9A-Za-z-_]{35}", + "GitHub Personal Access Token": "ghp_[a-zA-Z0-9]{36}", + "GitHub Fine Grained Personal Access Token": "github_pat_[a-zA-Z0-9]{22}_[a-zA-Z0-9]{59}", + "GitHub Actions Token": "ghs_[a-zA-Z0-9]{36}", + "JSON Web Token (JWT)": "ey[A-Za-z0-9-_=]{18,}.ey[A-Za-z0-9-_=]{18,}.[A-Za-z0-9-_.]{18,}" + } + } + +} + it('A diff including an AWS (Amazon Web Services) Access Key ID blocks the proxy...', async () => { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: generateDiff('AKIAIOSFODNN7EXAMPLE'), + }, + ]; + + const { error, errorMessage } = await processor.exec(null, action); + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + }); + + // Formatting test + it('A diff including multiple AWS (Amazon Web Services) Access Keys ID blocks the proxy...', async () => { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: generateMultiLineDiff(), + }, + ]; + + const { error, errorMessage } = await processor.exec(null, action); + + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + expect(errorMessage).to.contains('Line(s) of code: 3,4'); // blocked lines + expect(errorMessage).to.contains('#1 AWS (Amazon Web Services) Access Key ID'); // type of error + expect(errorMessage).to.contains('#2 AWS (Amazon Web Services) Access Key ID'); // type of error + }); + + // Formatting test + it('A diff including multiple AWS Access Keys ID and Literal blocks the proxy with appropriate message...', async () => { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: generateMultiLineDiffWithLiteral(), + }, + ]; + + const { error, errorMessage } = await processor.exec(null, action); + + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + expect(errorMessage).to.contains('Line(s) of code: 3'); // blocked lines + expect(errorMessage).to.contains('Line(s) of code: 4'); // blocked lines + expect(errorMessage).to.contains('Line(s) of code: 5'); // blocked lines + expect(errorMessage).to.contains('#1 AWS (Amazon Web Services) Access Key ID'); // type of error + expect(errorMessage).to.contains('#2 AWS (Amazon Web Services) Access Key ID'); // type of error + expect(errorMessage).to.contains('#3 Offending Literal'); + + }); + + it('A diff including a Google Cloud Platform API Key blocks the proxy...', async () => { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: generateDiff('AIza0aB7Z4Rfs23MnPqars81yzu19KbH72zaFda'), + }, + ]; + + const { error, errorMessage } = await processor.exec(null, action); + + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + }); + + it('A diff including a GitHub Personal Access Token blocks the proxy...', async () => { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: generateDiff(`ghp_${crypto.randomBytes(36).toString('hex')}`), + }, + ]; + + const { error, errorMessage } = await processor.exec(null, action); + + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + }); + + it('A diff including a GitHub Fine Grained Personal Access Token blocks the proxy...', async () => { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: generateDiff(`github_pat_1SMAGDFOYZZK3P9ndFemen_${crypto.randomBytes(59).toString('hex')}`), + }, + ]; + + const { error, errorMessage } = await processor.exec(null, action); + + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + }); + + it('A diff including a GitHub Actions Token blocks the proxy...', async () => { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: generateDiff(`ghs_${crypto.randomBytes(20).toString('hex')}`), + }, + ]; + + const { error, errorMessage } = await processor.exec(null, action); + + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + }); + + it('A diff including a JSON Web Token (JWT) blocks the proxy...', async () => { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: generateDiff(`eyJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJ1cm46Z21haWwuY29tOmNsaWVudElkOjEyMyIsInN1YiI6IkphbmUgRG9lIiwiaWF0IjoxNTIzOTAxMjM0LCJleHAiOjE1MjM5ODc2MzR9.s5_hA8hyIT5jXfU9PlXJ-R74m5F_aPcVEFJSV-g-_kX`), + }, + ]; + + const { error, errorMessage } = await processor.exec(null, action); + + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + }); + + it('A diff including a blocked literal blocks the proxy...', async () => { + for (const [literal] of blockedLiterals.entries()) { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: generateDiff(literal), + }, + ]; + + const { error, errorMessage } = await processor.exec(null, action); + + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + } + }); + it('When no diff is present, the proxy is blocked...', async () => { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: null, + }, + ]; + + const { error, errorMessage } = await processor.exec(null, action); + + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + }); + + it('When diff is not a string, the proxy is blocked...', async () => { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: 1337, + }, + ]; + + const { error, errorMessage } = await processor.exec(null, action); + + expect(error).to.be.true; + expect(errorMessage).to.contains('Your push has been blocked'); + }); + + it('A diff with no secrets or sensitive information does not block the proxy...', async () => { + const action = new Action('1', 'type', 'method', 1, 'project/name'); + action.steps = [ + { + stepName: 'diff', + content: generateDiff(''), + }, + ]; + + const { error } = await processor.exec(null, action); + expect(error).to.be.false; + }); + + it('A diff including a provider token in a private organization does not block the proxy...', async () => { + const action = new Action('1', 'type', 'method', 1, 'private-org-test'); + action.steps = [ + { + stepName: 'diff', + content: generateDiff('AKIAIOSFODNN7EXAMPLE'), + }, + ]; + + const { error } = await processor.exec(null, action); + expect(error).to.be.false; + }); +}); + +