Merge pull request #2175 from snyk/fix/snyk-protect-multiple

fix(protect): skip previously patched files
snyk · Aug 20, 2021 · dd46c19 · dd46c19
2 parents e7c314f + 5e824c0
commit dd46c19
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 26 deletions.
diff --git a/packages/snyk-protect/bin/snyk-protect b/packages/snyk-protect/bin/snyk-protect
@@ -1,2 +1,7 @@
 #!/usr/bin/env node
-require('../dist/index.js').protect();
+require('../dist/index.js')
+  .protect()
+  .catch((error) => {
+    // don't block pipelines on unexpected errors
+    console.error(error);
+  });
diff --git a/packages/snyk-protect/src/lib/index.ts b/packages/snyk-protect/src/lib/index.ts
@@ -26,7 +26,7 @@ async function protect(projectFolderPath: string) {
   const snykFilePath = path.resolve(projectFolderPath, '.snyk');
 
   if (!fs.existsSync(snykFilePath)) {
-    console.log('No .snyk file found');
+    console.log('No .snyk file found.');
     sendAnalytics({
       type: ProtectResultType.NO_SNYK_FILE,
     });
@@ -67,7 +67,7 @@ async function protect(projectFolderPath: string) {
   > = await getAllPatches(vulnIdAndPackageNames, packageNameToVersionsMap);
 
   if (packageAtVersionsToPatches.size === 0) {
-    console.log('Nothing to patch');
+    console.log('Nothing to patch.');
     sendAnalytics({
       type: ProtectResultType.NOTHING_TO_PATCH,
     });
@@ -83,7 +83,8 @@ async function protect(projectFolderPath: string) {
     vuldIdAndPatches?.forEach((vp) => {
       vp.patches.forEach((patchDiffs) => {
         patchDiffs.patchDiffs.forEach((diff) => {
-          applyPatchToFile(diff, fpp.path);
+          const patchedPath = applyPatchToFile(diff, fpp.path);
+          console.log(`Patched: ${patchedPath}`);
         });
       });
       patchedModules.push({
@@ -94,7 +95,7 @@ async function protect(projectFolderPath: string) {
     });
   });
 
-  console.log('Successfully applied Snyk patches');
+  console.log('Applied Snyk patches.');
 
   sendAnalytics({
     type: ProtectResultType.APPLIED_PATCHES,

diff --git a/packages/snyk-protect/src/lib/patch.ts b/packages/snyk-protect/src/lib/patch.ts
@@ -6,10 +6,17 @@ export function applyPatchToFile(patchContents: string, baseFolder: string) {
     baseFolder,
     extractTargetFilePathFromPatch(patchContents),
   );
+
+  const flagPath = `${targetFilePath}.snyk-protect.flag`;
+  if (fs.existsSync(flagPath)) {
+    return targetFilePath;
+  }
+
   const contentsToPatch = fs.readFileSync(targetFilePath, 'utf-8');
   const patchedContents = patchString(patchContents, contentsToPatch);
   fs.writeFileSync(targetFilePath, patchedContents);
-  console.log(`patched ${targetFilePath}`);
+  fs.writeFileSync(flagPath, '');
+  return targetFilePath;
 }
 
 export function extractTargetFilePathFromPatch(patchContents: string): string {
@@ -39,13 +46,11 @@ export function patchString(
   const contentsToPatchLines = contentsToPatch.split('\n');
 
   if (!patchContentLines[2]) {
-    // return;
-    throw new Error('Invalid patch');
+    throw new Error('Invalid patch.');
   }
   const unparsedLineToPatch = /^@@ -(\d*),.*@@/.exec(patchContentLines[2]);
   if (!unparsedLineToPatch || !unparsedLineToPatch[1]) {
-    // return;
-    throw new Error('Invalid patch');
+    throw new Error('Invalid patch.');
   }
   let lineToPatch = parseInt(unparsedLineToPatch[1], 10) - 2;
 
@@ -66,16 +71,13 @@ export function patchString(
       }
       case ' ': {
         if (currentLine !== nextLine) {
-          console.log(
-            'Expected\n  line from local file\n',
-            JSON.stringify(currentLine),
-            '\n  to match patch line\n',
-            JSON.stringify(nextLine),
-            '\n',
-          );
           throw new Error(
-            // `File ${filename} to be patched does not match, not patching`,
-            `File to be patched does not match, not patching`,
+            'File does not match patch contents.' +
+              '  Expected\n' +
+              '    line from local file\n' +
+              `      ${JSON.stringify(currentLine)}\n` +
+              '    to match patch line\n' +
+              `      ${JSON.stringify(nextLine)}\n`,
           );
         }
         break;

diff --git a/packages/snyk-protect/src/lib/snyk-api.ts b/packages/snyk-protect/src/lib/snyk-api.ts
@@ -7,7 +7,7 @@ export function getApiBaseUrl(): string {
       // snyk CI environment - we use `.../api/v1` though the norm is just `.../api`
       apiBaseUrl = process.env.SNYK_API.replace('/v1', '');
     } else {
-      console.log(
+      console.warn(
         'Malformed SNYK_API value. Using default: https://snyk.io/api',
       );
     }

diff --git a/packages/snyk-protect/test/acceptance/fix-pr.smoke.spec.ts b/packages/snyk-protect/test/acceptance/fix-pr.smoke.spec.ts
@@ -16,8 +16,24 @@ describe('Fix PR', () => {
     ).toEqual(
       expect.objectContaining<RunCLIResult>({
         code: 0,
-        stdout: expect.stringContaining('Successfully applied Snyk patches'),
-        stderr: expect.any(String),
+        stdout: expect.stringContaining('Applied Snyk patches'),
+        stderr: expect.not.stringMatching(/snyk/gi),
+      }),
+    );
+
+    await expect(
+      project.read('node_modules/lodash/lodash.js'),
+    ).resolves.toEqual(patchedLodash);
+
+    expect(
+      await runCommand('npm', ['install'], {
+        cwd: project.path(),
+      }),
+    ).toEqual(
+      expect.objectContaining<RunCLIResult>({
+        code: 0,
+        stdout: expect.stringContaining('Applied Snyk patches.'),
+        stderr: expect.not.stringMatching(/snyk/gi),
       }),
     );
 

diff --git a/packages/snyk-protect/test/acceptance/protect.spec.ts b/packages/snyk-protect/test/acceptance/protect.spec.ts
@@ -29,7 +29,7 @@ describe('@snyk/protect', () => {
         project.read('node_modules/nyc/node_modules/lodash/lodash.js'),
       ).resolves.toEqual(patchedLodash);
 
-      expect(log).toHaveBeenCalledWith('Successfully applied Snyk patches');
+      expect(log).toHaveBeenCalledWith('Applied Snyk patches.');
       expect(postJsonSpy).toHaveBeenCalledTimes(1);
       expect(postJsonSpy.mock.calls[0][1]).toEqual({
         data: {
@@ -68,7 +68,7 @@ describe('@snyk/protect', () => {
         project.read('node_modules/lodash/lodash.js'),
       ).resolves.toEqual(patchedLodash);
 
-      expect(log).toHaveBeenCalledWith('Successfully applied Snyk patches');
+      expect(log).toHaveBeenCalledWith('Applied Snyk patches.');
       expect(postJsonSpy).toHaveBeenCalledTimes(1);
       expect(postJsonSpy.mock.calls[0][1]).toEqual({
         data: {
@@ -107,7 +107,7 @@ describe('@snyk/protect', () => {
 
       await protect(project.path());
 
-      expect(log).toHaveBeenCalledWith('Nothing to patch');
+      expect(log).toHaveBeenCalledWith('Nothing to patch.');
 
       expect(postJsonSpy).toHaveBeenCalledTimes(1);
       expect(postJsonSpy.mock.calls[0][1]).toEqual({
@@ -142,7 +142,7 @@ describe('@snyk/protect', () => {
 
       await protect(project.path());
 
-      expect(log).toHaveBeenCalledWith('No .snyk file found');
+      expect(log).toHaveBeenCalledWith('No .snyk file found.');
 
       expect(postJsonSpy).toHaveBeenCalledTimes(1);
       expect(postJsonSpy.mock.calls[0][1]).toEqual({