Filter secrets from stderr and redact them + Add log message for log …

…collection (#1563) * Filter secerts and redact them + Add log message for log collection * Add test improve coverage * redaction + log collection * Refactoring code * removing extra method * Sending build logs to API * Lint fixes * Adding some specs * Fixing specs * Fixing more specs * Adding temp code to know spec issue * Adding timing.js and fixing specs * Updating reference_id pattern * Resolving comments + semgrep ignore * Adding new line * Adding attribution for secret-database --------- Co-authored-by: rishigupta1599 <[email protected]>
percy · Apr 17, 2024 · ae0a1f8 · ae0a1f8
1 parent 5d41045
commit ae0a1f8
Show file tree

Hide file tree

Showing 31 changed files with 7,731 additions and 414 deletions.
diff --git a/.semgrepignore b/.semgrepignore
@@ -0,0 +1 @@
+packages/core/src/secretPatterns.yml
diff --git a/packages/cli-exec/src/exec.js b/packages/cli-exec/src/exec.js
@@ -2,11 +2,8 @@ import command from '@percy/cli-command';
 import start from './start.js';
 import stop from './stop.js';
 import ping from './ping.js';
-import { getPackageJSON } from '@percy/cli-command/utils';
 import { waitForTimeout } from '@percy/client/utils';
 
-const pkg = getPackageJSON(import.meta.url);
-
 export const exec = command('exec', {
   description: 'Start and stop Percy around a supplied command',
   usage: '[options] -- <command>',
@@ -105,7 +102,6 @@ export const exec = command('exec', {
 async function* spawn(cmd, args, percy) {
   let { default: crossSpawn } = await import('cross-spawn');
   let proc, closed, error;
-  let errorMessage = '';
 
   try {
     proc = crossSpawn(cmd, args, { stdio: 'pipe' });
@@ -120,7 +116,9 @@ async function* spawn(cmd, args, percy) {
 
     if (proc.stderr) {
       proc.stderr.on('data', (data) => {
-        errorMessage += data.toString();
+        const message = data.toString();
+        let entry = { message, timestamp: Date.now(), type: 'ci' };
+        percy?.log?.error(entry, null, true);
         process.stderr.write(`${data}`);
       });
     }
@@ -129,22 +127,6 @@ async function* spawn(cmd, args, percy) {
 
     proc.on('close', code => {
       closed = code;
-      if (code !== 0) {
-        // Only send cli error when PERCY_CLIENT_ERROR_LOGS is set to true
-        if (process.env.PERCY_CLIENT_ERROR_LOGS === 'false') {
-          errorMessage = 'Log sharing is disabled';
-        }
-        // Only send event when there is a global error code and
-        // percy token is present
-        if (process.env.PERCY_TOKEN) {
-          const myObject = {
-            errorKind: 'cli',
-            cliVersion: pkg.version,
-            message: errorMessage
-          };
-          percy?.client?.sendBuildEvents(percy?.build?.id, myObject);
-        }
-      }
     });
 
     // run until an event is triggered

diff --git a/packages/cli-exec/src/start.js b/packages/cli-exec/src/start.js
@@ -26,6 +26,7 @@ export const start = command('start', {
     // run until stopped or terminated
     yield* yieldFor(() => percy.readyState >= 3);
   } catch (error) {
+    log.error(error);
     await percy.stop(true);
     throw error;
   }

diff --git a/packages/cli-exec/test/exec.test.js b/packages/cli-exec/test/exec.test.js
@@ -1,7 +1,5 @@
 import { logger, api, setupTest } from '@percy/cli-command/test/helpers';
 import exec from '@percy/cli-exec';
-import { getPackageJSON } from '@percy/cli-command/utils';
-
 describe('percy exec', () => {
   beforeEach(async () => {
     process.env.PERCY_TOKEN = '<<PERCY_TOKEN>>';
@@ -76,11 +74,14 @@ describe('percy exec', () => {
   it('starts and stops the percy process around the command', async () => {
     await exec(['--', 'node', '--eval', '']);
 
-    expect(logger.stderr).toEqual([]);
+    expect(logger.stderr).toEqual([
+      '[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
+    ]);
     expect(logger.stdout).toEqual([
       '[percy] Percy has started!',
       '[percy] Running "node --eval "',
-      '[percy] Finalized build #1: https://percy.io/test/test/123'
+      '[percy] Finalized build #1: https://percy.io/test/test/123',
+      '[percy] Build logs sent successfully. Please share this log ID with Percy team in case of any issues - random_sha'
     ]);
   });
 
@@ -112,69 +113,67 @@ describe('percy exec', () => {
     delete process.env.PERCY_TOKEN;
     await exec(['--', 'node', '--eval', '']);
 
-    expect(logger.stderr).toEqual([
+    expect(logger.stderr).toEqual(jasmine.arrayContaining([
       '[percy] Skipping visual tests',
       '[percy] Error: Missing Percy token'
-    ]);
-    expect(logger.stdout).toEqual([
+    ]));
+    expect(logger.stdout).toEqual(jasmine.arrayContaining([
       '[percy] Running "node --eval "'
-    ]);
+    ]));
   });
 
   it('forwards the command status', async () => {
     await expectAsync(
       exec(['--', 'node', '--eval', 'process.exit(3)'])
     ).toBeRejectedWithError('EEXIT: 3');
 
-    expect(logger.stderr).toEqual([]);
-    expect(logger.stdout).toEqual([
+    expect(logger.stderr).toEqual([
+      '[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
+    ]);
+    expect(logger.stdout).toEqual(jasmine.arrayContaining([
       '[percy] Percy has started!',
       '[percy] Running "node --eval process.exit(3)"',
       '[percy] Finalized build #1: https://percy.io/test/test/123'
-    ]);
+    ]));
   });
 
   it('tests process.stdout', async () => {
     let stdoutSpy = spyOn(process.stdout, 'write').and.resolveTo('some response');
     await exec(['--', 'echo', 'Hi!']);
 
     expect(stdoutSpy).toHaveBeenCalled();
-    expect(logger.stderr).toEqual([]);
-    expect(logger.stdout).toEqual([
+    expect(logger.stderr).toEqual([
+      '[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
+    ]);
+    expect(logger.stdout).toEqual(jasmine.arrayContaining([
       '[percy] Percy has started!',
       '[percy] Running "echo Hi!"',
       '[percy] Finalized build #1: https://percy.io/test/test/123'
-    ]);
+    ]));
   });
 
-  it('tests process.stderr when token is present and PERCY_CLIENT_ERROR_LOGS is not present', async () => {
-    delete process.env.PERCY_CLIENT_ERROR_LOGS;
-    const pkg = getPackageJSON(import.meta.url);
+  it('adds process.stderr logs in CI logs', async () => {
     let stderrSpy = spyOn(process.stderr, 'write').and.resolveTo(jasmine.stringMatching(/Some error/));
     await expectAsync(
       exec(['--', 'node', './test/test-data/test_prog.js', 'error']) // Throws Error
     ).toBeRejectedWithError('EEXIT: 1');
 
-    expect(logger.stderr).toEqual([]);
-    expect(logger.stdout).toEqual([
+    expect(logger.stderr).toEqual([
+      '[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
+    ]);
+    expect(logger.stdout).toEqual(jasmine.arrayContaining([
       '[percy] Percy has started!',
       '[percy] Running "node ./test/test-data/test_prog.js error"',
       '[percy] Finalized build #1: https://percy.io/test/test/123'
-    ]);
+    ]));
 
+    expect(Array.from(logger.instance.ciMessages)[0].message).toContain([
+      'Some error with secret: [REDACTED]'
+    ]);
     expect(stderrSpy).toHaveBeenCalled();
-    expect(api.requests['/builds/123/send-events']).toBeDefined();
-    expect(api.requests['/builds/123/send-events'][0].body).toEqual({
-      data: {
-        errorKind: 'cli',
-        cliVersion: pkg.version,
-        message: jasmine.stringMatching(/Some error/)
-      }
-    });
   });
 
-  it('tests process.stderr percy is disabled', async () => {
-    delete process.env.PERCY_CLIENT_ERROR_LOGS;
+  it('does not adds process.stderr logs if percy is disabled', async () => {
     process.env.PERCY_ENABLE = '0';
     let stderrSpy = spyOn(process.stderr, 'write').and.resolveTo(jasmine.stringMatching(/Some error/));
     await expectAsync(
@@ -184,83 +183,12 @@ describe('percy exec', () => {
     expect(logger.stderr).toEqual([
       '[percy] Percy is disabled'
     ]);
-    expect(logger.stdout).toEqual([
+    expect(logger.stdout).toEqual(jasmine.arrayContaining([
       '[percy] Running "node ./test/test-data/test_prog.js error"'
-    ]);
-
-    expect(stderrSpy).toHaveBeenCalled();
-    expect(api.requests['/builds/123/send-events']).not.toBeDefined();
-  });
-
-  it('tests process.stderr when token is present and PERCY_CLIENT_ERROR_LOGS is present and set to false', async () => {
-    process.env.PERCY_CLIENT_ERROR_LOGS = 'false';
-    const pkg = getPackageJSON(import.meta.url);
-    let stderrSpy = spyOn(process.stderr, 'write').and.resolveTo(jasmine.stringMatching(/Some error/));
-    await expectAsync(
-      exec(['--', 'node', './test/test-data/test_prog.js', 'error']) // Throws Error
-    ).toBeRejectedWithError('EEXIT: 1');
-
-    expect(stderrSpy).toHaveBeenCalled();
-    expect(logger.stderr).toEqual([]);
-    expect(logger.stdout).toEqual([
-      '[percy] Percy has started!',
-      '[percy] Running "node ./test/test-data/test_prog.js error"',
-      '[percy] Finalized build #1: https://percy.io/test/test/123'
-    ]);
-
-    expect(api.requests['/builds/123/send-events']).toBeDefined();
-    expect(api.requests['/builds/123/send-events'][0].body).toEqual({
-      data: {
-        errorKind: 'cli',
-        cliVersion: pkg.version,
-        message: 'Log sharing is disabled'
-      }
-    });
-  });
-
-  it('tests process.stderr when token is present and PERCY_CLIENT_ERROR_LOGS is present and set to true', async () => {
-    process.env.PERCY_CLIENT_ERROR_LOGS = 'true';
-    const pkg = getPackageJSON(import.meta.url);
-    let stderrSpy = spyOn(process.stderr, 'write').and.resolveTo(jasmine.stringMatching(/Some error/));
-    await expectAsync(
-      exec(['--', 'node', './test/test-data/test_prog.js', 'error']) // Throws Error
-    ).toBeRejectedWithError('EEXIT: 1');
-
-    expect(stderrSpy).toHaveBeenCalled();
-    expect(logger.stderr).toEqual([]);
-    expect(logger.stdout).toEqual([
-      '[percy] Percy has started!',
-      '[percy] Running "node ./test/test-data/test_prog.js error"',
-      '[percy] Finalized build #1: https://percy.io/test/test/123'
-    ]);
-
-    expect(api.requests['/builds/123/send-events']).toBeDefined();
-    expect(api.requests['/builds/123/send-events'][0].body).toEqual({
-      data: {
-        errorKind: 'cli',
-        cliVersion: pkg.version,
-        message: jasmine.stringMatching(/Some error/)
-      }
-    });
-  });
-
-  it('tests process.stderr when token is not present', async () => {
-    delete process.env.PERCY_TOKEN;
-    let stderrSpy = spyOn(process.stderr, 'write').and.resolveTo('some response');
-    await expectAsync(
-      exec(['--', 'node', 'random.js']) // invalid command
-    ).toBeRejectedWithError('EEXIT: 1');
+    ]));
 
+    expect(Array.from(logger.instance.ciMessages)).toEqual([]);
     expect(stderrSpy).toHaveBeenCalled();
-    expect(logger.stderr).toEqual([
-      '[percy] Skipping visual tests',
-      '[percy] Error: Missing Percy token'
-    ]);
-    expect(logger.stdout).toEqual([
-      '[percy] Running "node random.js"'
-    ]);
-
-    expect(api.requests['/builds/123/send-events']).not.toBeDefined();
   });
 
   it('does not run the command if canceled beforehand', async () => {
@@ -278,7 +206,9 @@ describe('percy exec', () => {
     // user termination is not considered an error
     await expectAsync(test).toBeResolved();
 
-    expect(logger.stderr).toEqual([]);
+    expect(logger.stderr).toEqual([
+      '[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
+    ]);
     expect(logger.stdout).not.toContain(
       '[percy] Running "node --eval "');
   });
@@ -293,14 +223,15 @@ describe('percy exec', () => {
     await expectAsync(exec(['--', 'foobar'])).toBeRejected();
     expect(stdinSpy).toHaveBeenCalled();
     expect(logger.stderr).toEqual([
+      '[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false',
       '[percy] Error: spawn error'
     ]);
-    expect(logger.stdout).toEqual([
+    expect(logger.stdout).toEqual(jasmine.arrayContaining([
       '[percy] Percy has started!',
       '[percy] Running "foobar"',
       '[percy] Stopping percy...',
       '[percy] Finalized build #1: https://percy.io/test/test/123'
-    ]);
+    ]));
   });
 
   it('handles terminating the child process when interrupted', async () => {
@@ -320,7 +251,9 @@ describe('percy exec', () => {
     // user termination is not considered an error
     await expectAsync(test).toBeResolved();
 
-    expect(logger.stderr).toEqual([]);
+    expect(logger.stderr).toEqual([
+      '[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
+    ]);
     expect(logger.stdout).toContain(
       '[percy] Stopping percy...'
     );
@@ -335,37 +268,43 @@ describe('percy exec', () => {
       'await request(url).catch(e => (console.error(e), process.exit(2)));'
     ].join('')]);
 
-    expect(logger.stderr).toEqual([]);
-    expect(logger.stdout).toEqual([
+    expect(logger.stderr).toEqual([
+      '[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
+    ]);
+    expect(logger.stdout).toEqual(jasmine.arrayContaining([
       '[percy] Percy has started!',
       jasmine.stringMatching('\\[percy] Running "node '),
       '[percy] Finalized build #1: https://percy.io/test/test/123'
-    ]);
+    ]));
   });
 
   it('provides the child process with a percy build id env var', async () => {
     await exec(['--', 'node', '--eval', (
       'process.env.PERCY_BUILD_ID === "123" || process.exit(2)'
     )]);
 
-    expect(logger.stderr).toEqual([]);
-    expect(logger.stdout).toEqual([
+    expect(logger.stderr).toEqual([
+      '[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
+    ]);
+    expect(logger.stdout).toEqual(jasmine.arrayContaining([
       '[percy] Percy has started!',
       jasmine.stringMatching('\\[percy] Running "node '),
       '[percy] Finalized build #1: https://percy.io/test/test/123'
-    ]);
+    ]));
   });
 
   it('provides the child process with a percy build url env var', async () => {
     await exec(['--', 'node', '--eval', (
       'process.env.PERCY_BUILD_URL === "https://percy.io/test/test/123" || process.exit(2)'
     )]);
 
-    expect(logger.stderr).toEqual([]);
-    expect(logger.stdout).toEqual([
+    expect(logger.stderr).toEqual([
+      '[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
+    ]);
+    expect(logger.stdout).toEqual(jasmine.arrayContaining([
       '[percy] Percy has started!',
       jasmine.stringMatching('\\[percy] Running "node '),
       '[percy] Finalized build #1: https://percy.io/test/test/123'
-    ]);
+    ]));
   });
 });
diff --git a/packages/cli-exec/test/start.test.js b/packages/cli-exec/test/start.test.js
@@ -92,9 +92,9 @@ describe('percy exec:start', () => {
     await expectAsync(start()).toBeRejected();
 
     expect(logger.stdout).toEqual([]);
-    expect(logger.stderr).toEqual([
+    expect(logger.stderr).toEqual(jasmine.arrayContaining([
       '[percy] Error: Percy is already running or the port is in use'
-    ]);
+    ]));
   });
 
   it('logs when percy has been disabled', async () => {

diff --git a/packages/cli-exec/test/test-data/test_prog.js b/packages/cli-exec/test/test-data/test_prog.js
@@ -1,6 +1,6 @@
 // Error
 function throwError() {
-  throw new Error('Some error');
+  throw new Error('Some error with secret: AKIAIOSFODNN7EXAMPLE');
 }
 const args = process.argv.slice(2); // Extract command-line arguments, excluding the first two (node and script path)
 

diff --git a/packages/cli-snapshot/src/snapshot.js b/packages/cli-snapshot/src/snapshot.js
@@ -89,6 +89,7 @@ export const snapshot = command('snapshot', {
     yield* percy.yield.snapshot(options);
     yield* percy.yield.stop();
   } catch (error) {
+    log.error(error);
     await percy.stop(true);
     throw error;
   }