Repository: Redocly/redocly-cli
Pull request #1143 (2023-06-27 14:31:16)

chore: improve data obfuscation
## What/Why/How?

Improving data obfuscation (better files/folder/organizations/apis handling).
Detecting CI/Docker environments.

## Reference

## Testing

Tested on lab6

## Screenshots (optional)

## Check yourself

- [x] Code is linted
- [x] Tested with redoc/reference-docs/workflows
- [x] All new/updated code is covered with tests

## Security

- [x] Security impact of change has been considered
- [x] Code follows company security practices and guidelines


github-actions[bot]: <!-- jest coverage report action for options with hash 3339ae53a38c44382c0fb4491bdb282d -->

## Coverage report 



| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | Category   | Percentage | Covered / Total |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :--------- | :--------- | :-------------: |
|                                                                           🟡                                                                           | Statements | 74.15%     |    3649/4921    |
|                                                                           🟡                                                                           | Branches   | 65.28%     |    2040/3125    |
|                                                                           🟡                                                                           | Functions  | 66.19%     |     603/911     |
|                                                                           🟡                                                                           | Lines      | 74.19%     |    3401/4584    |


## Test suite run success
598 tests passing in 87 suites.

<p align="right">Report generated by <a href="https://github.com/ArtiomTr/jest-coverage-report-action">🧪jest coverage report action</a> from f21f025b1e4dd3f967fc2498824a6624d48d55e6</p>

lornajane: Suggested additional obfuscation
* remove the full data of the `-o`/`--output` parameter, but keep the file extension (I'm curious about the yaml/json split for bundle, the docs are html but it's still useful)
* change the `apis` data - we're using arrays with `***.yaml` but showing where a config alias is used. It would be better to have `file-yaml`, `file-json`, `api-name`, `api-name@version` and so on - literally these values, masking what the user actually supplied
* 

Patch:
diff --git a/Dockerfile b/Dockerfile
index 7d7dc281..a9889677 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -29,4 +29,6 @@ RUN npm cache clean --force && rm -rf /build
 
 WORKDIR /spec
 
-ENTRYPOINT [ "openapi" ]
+ENTRYPOINT [ "redocly" ]
+
+ENV REDOCLY_ENVIRONMENT=docker
diff --git a/jest.config.js b/jest.config.js
index 8212fca6..ca59f74d 100644
--- a/jest.config.js
+++ b/jest.config.js
@@ -14,12 +14,12 @@ module.exports = {
     'packages/core/': {
       statements: 80,
       branches: 71,
-      functions: 69,
+      functions: 70,
       lines: 80,
     },
     'packages/cli/': {
       statements: 55,
-      branches: 46,
+      branches: 47,
       functions: 55,
       lines: 55,
     },
diff --git a/packages/cli/src/utils.ts b/packages/cli/src/utils.ts
index 7963c9d1..3062b3eb 100644
--- a/packages/cli/src/utils.ts
+++ b/packages/cli/src/utils.ts
@@ -496,7 +496,6 @@ export async function sendTelemetry(
     } = argv;
     const event_time = new Date().toISOString();
     const redoclyClient = new RedoclyClient();
-    const node_version = process.version;
     const logged_in = await redoclyClient.isAuthorizedWithRedoclyByRegion();
     const data: Analytics = {
       event: 'cli_command',
@@ -504,10 +503,11 @@ export async function sendTelemetry(
       logged_in,
       command,
       arguments: cleanArgs(args),
-      node_version,
+      node_version: process.version,
       version,
       exit_code,
       environment: process.env.REDOCLY_ENVIRONMENT,
+      environment_ci: process.env.CI,
       raw_input: cleanRawInput(process.argv.slice(2)),
       has_config,
     };
@@ -535,22 +535,34 @@ export type Analytics = {
   version: string;
   exit_code: ExitCode;
   environment?: string;
+  environment_ci?: string;
   raw_input: string;
   has_config?: boolean;
 };
 
+function isFile(value: string) {
+  return fs.existsSync(value) && fs.statSync(value).isFile();
+}
+
+function isDirectory(value: string) {
+  return fs.existsSync(value) && fs.statSync(value).isDirectory();
+}
+
 function cleanString(value?: string): string | undefined {
   if (!value) {
     return value;
   }
   if (isAbsoluteUrl(value)) {
-    return value.split('://')[0] + '://***';
+    return value.split('://')[0] + '://url';
+  }
+  if (isFile(value)) {
+    return value.replace(/.+\.([^.]+)$/, (_, ext) => 'file-' + ext);
   }
-  if (value.endsWith('.json') || value.endsWith('.yaml') || value.endsWith('.yml')) {
-    return value.replace(/^(.*)\.(yaml|yml|json)$/gi, (_, __, ext) => '***.' + ext);
+  if (isDirectory(value)) {
+    return 'folder';
   }
   if (DESTINATION_REGEX.test(value)) {
-    return value.replace(/^@[\w\-\s]+\//, () => '@***/');
+    return value.startsWith('@') ? '@organization/api-name@api-version' : 'api-name@api-version';
   }
   return value;
 }


Test patch:
diff --git a/packages/cli/src/__tests__/fixtures/openapi.json b/packages/cli/src/__tests__/fixtures/openapi.json
new file mode 100644
index 00000000..e69de29b
diff --git a/packages/cli/src/__tests__/fixtures/openapi.yaml b/packages/cli/src/__tests__/fixtures/openapi.yaml
new file mode 100644
index 00000000..e69de29b
diff --git a/packages/cli/src/__tests__/fixtures/redocly.yaml b/packages/cli/src/__tests__/fixtures/redocly.yaml
new file mode 100644
index 00000000..e69de29b
diff --git a/packages/cli/src/__tests__/utils.test.ts b/packages/cli/src/__tests__/utils.test.ts
index c455621a..25061c0f 100644
--- a/packages/cli/src/__tests__/utils.test.ts
+++ b/packages/cli/src/__tests__/utils.test.ts
@@ -21,7 +21,7 @@ import {
   YamlParseError,
 } from '@redocly/openapi-core';
 import { blue, red, yellow } from 'colorette';
-import { existsSync } from 'fs';
+import { existsSync, statSync } from 'fs';
 import * as path from 'path';
 import * as process from 'process';
 
@@ -489,16 +489,23 @@ describe('cleanArgs', () => {
   beforeEach(() => {
     // @ts-ignore
     isAbsoluteUrl = jest.requireActual('@redocly/openapi-core').isAbsoluteUrl;
+    // @ts-ignore
+    existsSync = (value) => jest.requireActual('fs').existsSync(path.resolve(__dirname, value));
+    // @ts-ignore
+    statSync = (value) => jest.requireActual('fs').statSync(path.resolve(__dirname, value));
+  });
+  afterEach(() => {
+    jest.clearAllMocks();
   });
   it('should remove potentially sensitive data from args', () => {
     const testArgs = {
-      config: 'some-folder/redocly.yaml',
-      apis: ['main@v1', 'openapi.yaml', 'http://some.url/openapi.yaml'],
+      config: './fixtures/redocly.yaml',
+      apis: ['main@v1', 'fixtures/openapi.yaml', 'http://some.url/openapi.yaml'],
       format: 'codeframe',
     };
     expect(cleanArgs(testArgs)).toEqual({
-      config: '***.yaml',
-      apis: ['main@v1', '***.yaml', 'http://***'],
+      config: 'file-yaml',
+      apis: ['api-name@api-version', 'file-yaml', 'http://url'],
       format: 'codeframe',
     });
   });
@@ -507,26 +514,51 @@ describe('cleanArgs', () => {
       destination: '@org/name@version',
     };
     expect(cleanArgs(testArgs)).toEqual({
-      destination: '@***/name@version',
+      destination: '@organization/api-name@api-version',
     });
   });
 });
 
 describe('cleanRawInput', () => {
-  it('should remove  potentially sensitive data from raw CLI input', () => {
+  beforeEach(() => {
     // @ts-ignore
     isAbsoluteUrl = jest.requireActual('@redocly/openapi-core').isAbsoluteUrl;
-
+    // @ts-ignore
+    existsSync = (value) => jest.requireActual('fs').existsSync(path.resolve(__dirname, value));
+    // @ts-ignore
+    statSync = (value) => jest.requireActual('fs').statSync(path.resolve(__dirname, value));
+  });
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+  it('should remove  potentially sensitive data from raw CLI input', () => {
     const rawInput = [
       'redocly',
-      'lint',
-      'main@v1',
-      'openapi.yaml',
+      'bundle',
+      'api-name@api-version',
+      './fixtures/openapi.yaml',
       'http://some.url/openapi.yaml',
-      '--config=some-folder/redocly.yaml',
+      '--config=fixtures/redocly.yaml',
+      '--output',
+      'fixtures',
+    ];
+    expect(cleanRawInput(rawInput)).toEqual(
+      'redocly bundle api-name@api-version file-yaml http://url --config=file-yaml --output folder'
+    );
+  });
+  it('should preserve safe data from raw CLI input', () => {
+    const rawInput = [
+      'redocly',
+      'lint',
+      './fixtures/openapi.json',
+      '--format',
+      'stylish',
+      '--extends=minimal',
+      '--skip-rule',
+      'operation-4xx-response',
     ];
     expect(cleanRawInput(rawInput)).toEqual(
-      'redocly lint main@v1 ***.yaml http://*** --config=***.yaml'
+      'redocly lint file-json --format stylish --extends=minimal --skip-rule operation-4xx-response'
     );
   });
 });