Merge pull request #932 from snyk/fix/revert-deep-merging-ruleset

chore: revert filters merging logic
snyk · Feb 21, 2025 · 309c97f · 309c97f
2 parents 784f479 + 0548783
commit 309c97f
Show file tree

Hide file tree

Showing 6 changed files with 324 additions and 137 deletions.
diff --git a/defaultFilters/apprisk/azure-repos.json b/defaultFilters/apprisk/azure-repos.json
@@ -219,25 +219,6 @@
       "token": "${BROKER_CLIENT_VALIDATION_BASIC_AUTH}"
     }
   },
-  {
-    "//": "get organization context from file",
-    "method": "GET",
-    "path": "/:org/_apis/git/repositories/:repo/items",
-    "origin": "https://${AZURE_REPOS_HOST}",
-    "auth": {
-      "scheme": "basic",
-      "token": "${BROKER_CLIENT_VALIDATION_BASIC_AUTH}"
-    },
-    "valid": [
-      {
-        "queryParam": "path",
-        "values": [
-          "**/*.json",
-          "**%2F*.json"
-        ]
-      }
-    ]
-  },
   {
     "//": "get organization context from file without org",
     "method": "GET",

diff --git a/defaultFilters/azure-repos.json b/defaultFilters/azure-repos.json
@@ -149,7 +149,11 @@
               "**/go.sum",
               "**%2Fgo.sum",
               "**/*Dockerfile*",
-              "**%2F*Dockerfile*"
+              "**%2F*Dockerfile*",
+              "**/*.json",
+              "**%2F*.json",
+              "**/.azuredevops/snyk_pull_request_template.yaml",
+              "**%2F.azuredevops%2Fsnyk_pull_request_template.yaml"
             ]
           },
           {
@@ -242,7 +246,11 @@
               "**/go.sum",
               "**%2Fgo.sum",
               "**/*Dockerfile*",
-              "**%2F*Dockerfile*"
+              "**%2F*Dockerfile*",
+              "**/*.json",
+              "**%2F*.json",
+              "**/.azuredevops/snyk_pull_request_template.yaml",
+              "**%2F.azuredevops%2Fsnyk_pull_request_template.yaml"
             ]
           },
           {

diff --git a/defaultFilters/customPrTemplates/azure-repos.json b/defaultFilters/customPrTemplates/azure-repos.json
@@ -1,17 +1 @@
-[
-  {
-    "//": "get file existence. restrict by file types",
-    "method": "GET",
-    "path": "/:owner/_apis/git/repositories/:repo/items",
-    "origin": "https://${AZURE_REPOS_HOST}/${AZURE_REPOS_ORG}",
-    "valid": [
-      {
-        "queryParam": "path",
-        "values": [
-          "**/.azuredevops/snyk_pull_request_template.yaml",
-          "**%2F.azuredevops%2Fsnyk_pull_request_template.yaml"
-        ]
-      }
-    ]
-  }
-]
+[]
diff --git a/lib/hybrid-sdk/common/filter/filter-rules-loading.ts b/lib/hybrid-sdk/common/filter/filter-rules-loading.ts
@@ -6,7 +6,7 @@ import { log as logger } from '../../../logs/logger';
 import { findProjectRoot } from '../config/config';
 import camelcase from 'camelcase';
 import { FiltersType, Rule } from '../types/filter';
-import { retrieveFilters, isValidURI, deepMergeRules } from './utils';
+import { retrieveFilters, isValidURI } from './utils';
 import { CONFIGURATION } from '../types/options';
 
 const SUPPORTED_IAC_EXTENSIONS = ['tf', 'yaml', 'yml', 'tpl', 'json'];
@@ -208,6 +208,7 @@ function injectRulesAtRuntime(
           templateGET.path = '*/info/refs*';
           templateGETForSnippets.path =
             '/:owner/_apis/git/repositories/:repo/items';
+          delete templateGETForSnippets.valid;
           templatePOST.path = '*/git-upload-pack';
           break;
         case 'GITHUB':
@@ -280,7 +281,20 @@ function injectRulesAtRuntime(
         `defaultFilters/apprisk/${type}.json`,
       )) as Rule[];
 
-      filters.private = deepMergeRules(filters.private, appRiskRules);
+      const appRiskRulesMethodPattern = appRiskRules.map(
+        (x) => `${x.method}|${x.path}`,
+      );
+
+      const possibleOverlappingRules = filters.private.filter((x) => {
+        return !appRiskRulesMethodPattern.includes(`${x.method}|${x.path}`);
+      });
+      if (possibleOverlappingRules.length > 0) {
+        logger.debug(
+          { possibleOverlappingRules },
+          `Caution, possible overlapping rules with apprisk rules extension.`,
+        );
+      }
+      filters.private.push(...appRiskRules);
     }
   }
 
@@ -312,7 +326,21 @@ function injectRulesAtRuntime(
         `defaultFilters/customPrTemplates/${type}.json`,
       )) as Rule[];
 
-      filters.private = deepMergeRules(filters.private, customPRTemplatesRules);
+      const customPRTemplatesRulesMethodPattern = customPRTemplatesRules.map(
+        (x) => `${x.method}|${x.path}`,
+      );
+      const possibleOverlappingRules = filters.private.filter((x) => {
+        return !customPRTemplatesRulesMethodPattern.includes(
+          `${x.method}|${x.path}`,
+        );
+      });
+      if (possibleOverlappingRules.length > 0) {
+        logger.debug(
+          { possibleOverlappingRules },
+          `Caution, possible overlapping rules with custom PR templates.`,
+        );
+      }
+      filters.private.push(...customPRTemplatesRules);
     }
   }
 

diff --git a/lib/hybrid-sdk/common/filter/utils.ts b/lib/hybrid-sdk/common/filter/utils.ts
@@ -2,7 +2,6 @@ import path from 'node:path';
 import fs from 'fs';
 import { findProjectRoot } from '../config/config';
 import { log as logger } from '../../../logs/logger';
-import { Rule } from '../types/filter';
 import { PostFilterPreparedRequest } from '../../../broker-workload/prepareRequest';
 import { makeSingleRawRequestToDownstream } from '../../http/request';
 
@@ -70,65 +69,3 @@ export const retrieveFilters = async (locations: Map<string, string>) => {
 
   return retrievedFiltersMap;
 };
-
-export function deepMergeRules(arr1: Rule[], arr2: Rule[]): Rule[] {
-  const isObject = (item: any): item is Record<string, any> =>
-    item && typeof item === 'object' && !Array.isArray(item);
-
-  const mergeArrays = (target: any[], source: any[]): any[] => {
-    const result = [...target];
-
-    for (const item of source) {
-      if (isObject(item) && item.queryParam) {
-        const existing = result.find(
-          (i) => isObject(i) && i.queryParam === item.queryParam,
-        );
-        if (existing) {
-          existing.values = [
-            ...new Set([...(existing.values || []), ...(item.values || [])]),
-          ];
-        } else {
-          result.push(item);
-        }
-      } else if (!result.includes(item)) {
-        result.push(item);
-      }
-    }
-
-    return result;
-  };
-
-  const mergeObjects = (
-    target: Record<string, any>,
-    source: Record<string, any>,
-  ): Record<string, any> => {
-    for (const key of Object.keys(source)) {
-      if (isObject(target[key]) && isObject(source[key])) {
-        target[key] = mergeObjects(target[key], source[key]);
-      } else if (Array.isArray(target[key]) && Array.isArray(source[key])) {
-        target[key] = mergeArrays(target[key], source[key]);
-      } else {
-        target[key] = source[key];
-      }
-    }
-    return target;
-  };
-
-  const grouped: Record<string, Rule> = {};
-
-  for (const rule of [...arr1, ...arr2]) {
-    if (!rule || !isObject(rule)) continue;
-
-    if ('method' in rule && 'path' in rule && 'origin' in rule) {
-      const key = `${rule.method}:${rule.path}:${rule.origin}`;
-
-      if (grouped[key]) {
-        grouped[key] = mergeObjects(grouped[key], rule) as Rule;
-      } else {
-        grouped[key] = rule;
-      }
-    }
-  }
-
-  return Object.values(grouped);
-}