feat: add new package.json for improved secrets reporting

feat: format violations on block feat: improve test
finos · JamieSlome · Nov 12, 2024 · Oct 17, 2024 · Nov 12, 2024 · Nov 12, 2024
commit 25b6aaa26f13273c48c4b3239d6f00edadb22fdd
diff --git 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
@@ -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
@@ -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;
   }