fix(@angular/cli): improve global schema validation

t-bowersox · clydin · commit 8b65abe1b037 · 2022-06-24T09:51:21.000-04:00
- Prevent additional properties being set in cli subproperties (i.e. cli.warnings.zzzz).
- Create cliGlobalOptions definition and reference in the global.cli schema.
- Use global.cli schema to validate changes made with --global flag.
- Add test coverage for validating global/local-only changes.
- Add test coverage for setting invalid properties and sub-properties.
diff --git a/packages/angular/cli/lib/config/workspace-schema.json b/packages/angular/cli/lib/config/workspace-schema.json
@@ -67,7 +67,8 @@
               "description": "Show a warning when the global version is newer than the local one.",
               "type": "boolean"
             }
-          }
+          },
+          "additionalProperties": false
         },
         "analytics": {
           "type": ["boolean", "string"],
@@ -86,7 +87,8 @@
               "type": "string",
               "format": "uuid"
             }
-          }
+          },
+          "additionalProperties": false
         },
         "cache": {
           "description": "Control disk cache.",
@@ -105,7 +107,74 @@
               "description": "Cache base path.",
               "type": "string"
             }
+          },
+          "additionalProperties": false
+        }
+      },
+      "additionalProperties": false
+    },
+    "cliGlobalOptions": {
+      "type": "object",
+      "properties": {
+        "defaultCollection": {
+          "description": "The default schematics collection to use.",
+          "type": "string",
+          "x-deprecated": "Use 'schematicCollections' instead."
+        },
+        "schematicCollections": {
+          "type": "array",
+          "description": "The list of schematic collections to use.",
+          "items": {
+            "type": "string",
+            "uniqueItems": true
           }
+        },
+        "packageManager": {
+          "description": "Specify which package manager tool to use.",
+          "type": "string",
+          "enum": ["npm", "cnpm", "yarn", "pnpm"]
+        },
+        "warnings": {
+          "description": "Control CLI specific console warnings",
+          "type": "object",
+          "properties": {
+            "versionMismatch": {
+              "description": "Show a warning when the global version is newer than the local one.",
+              "type": "boolean"
+            }
+          },
+          "additionalProperties": false
+        },
+        "analytics": {
+          "type": ["boolean", "string"],
+          "description": "Share anonymous usage data with the Angular Team at Google."
+        },
+        "analyticsSharing": {
+          "type": "object",
+          "properties": {
+            "tracking": {
+              "description": "Analytics sharing info tracking ID.",
+              "type": "string",
+              "pattern": "^(GA|UA)?-\\d+-\\d+$"
+            },
+            "uuid": {
+              "description": "Analytics sharing info universally unique identifier.",
+              "type": "string",
+              "format": "uuid"
+            }
+          },
+          "additionalProperties": false
+        },
+        "completion": {
+          "type": "object",
+          "description": "Angular CLI completion settings.",
+          "properties": {
+            "prompted": {
+              "type": "boolean",
+              "description": "Whether the user has been prompted to add completion command prompt."
+            }
+          },
+          "additionalProperties": false
         }
       },
       "additionalProperties": false
@@ -575,14 +644,13 @@
       "type": "object",
       "properties": {
         "$schema": {
-          "type": "string",
-          "format": "uri"
+          "type": "string"
         },
         "version": {
           "$ref": "#/definitions/fileVersion"
         },
         "cli": {
-          "$ref": "#/definitions/cliOptions"
+          "$ref": "#/definitions/cliGlobalOptions"
         },
         "schematics": {
           "$ref": "#/definitions/schematicOptions"
diff --git a/packages/angular/cli/src/commands/config/cli.ts b/packages/angular/cli/src/commands/config/cli.ts
@@ -97,27 +97,6 @@ export class ConfigCommandModule
       throw new CommandModuleError('Invalid Path.');
     }
 
-    const validGlobalCliPaths = new Set<string>([
-      'cli.warnings.versionMismatch',
-      'cli.defaultCollection',
-      'cli.schematicCollections',
-      'cli.packageManager',
-
-      'cli.analytics',
-      'cli.analyticsSharing.tracking',
-      'cli.analyticsSharing.uuid',
-
-      'cli.completion.prompted',
-    ]);
-
-    if (
-      options.global &&
-      !options.jsonPath.startsWith('schematics.') &&
-      !validGlobalCliPaths.has(options.jsonPath)
-    ) {
-      throw new CommandModuleError('Invalid Path.');
-    }
-
     const [config, configPath] = await getWorkspaceRaw(options.global ? 'global' : 'local');
     const { logger } = this.context;
 
@@ -140,7 +119,7 @@ export class ConfigCommandModule
       return 1;
     }
 
-    await validateWorkspace(parseJson(config.content));
+    await validateWorkspace(parseJson(config.content), options.global ?? false);
 
     config.save();
 
diff --git a/packages/angular/cli/src/utilities/config.ts b/packages/angular/cli/src/utilities/config.ts
@@ -240,11 +240,20 @@ export async function getWorkspaceRaw(
   return [new JSONFile(configPath), configPath];
 }
 
-export async function validateWorkspace(data: json.JsonObject): Promise<void> {
-  const schema = readAndParseJson(workspaceSchemaPath) as json.schema.JsonSchema;
+export async function validateWorkspace(data: json.JsonObject, isGlobal: boolean): Promise<void> {
+  const schema = readAndParseJson(workspaceSchemaPath);
+
+  // We should eventually have a dedicated global config schema and use that to validate.
+  const schemaToValidate: json.schema.JsonSchema = isGlobal
+    ? {
+        '$ref': '#/definitions/global',
+        definitions: schema['definitions'],
+      }
+    : schema;
+
   const { formats } = await import('@angular-devkit/schematics');
   const registry = new json.schema.CoreSchemaRegistry(formats.standardFormats);
-  const validator = await registry.compile(schema).toPromise();
+  const validator = await registry.compile(schemaToValidate).toPromise();
 
   const { success, errors } = await validator(data).toPromise();
   if (!success) {
diff --git a/tests/legacy-cli/e2e/tests/commands/config/config-global-validation.ts b/tests/legacy-cli/e2e/tests/commands/config/config-global-validation.ts
@@ -0,0 +1,51 @@
+import { homedir } from 'os';
+import * as path from 'path';
+import { deleteFile, expectFileToExist } from '../../../utils/fs';
+import { ng, silentNg } from '../../../utils/process';
+import { expectToFail } from '../../../utils/utils';
+
+export default async function () {
+  let ngError: Error;
+
+  ngError = await expectToFail(() => silentNg('config', 'cli.completion.prompted', 'true'));
+
+  if (
+    !ngError.message.includes('Data path "/cli" must NOT have additional properties(completion).')
+  ) {
+    throw new Error('Should have failed with must NOT have additional properties(completion).');
+  }
+
+  ngError = await expectToFail(() =>
+    silentNg('config', '--global', 'cli.completion.invalid', 'true'),
+  );
+
+  if (
+    !ngError.message.includes(
+      'Data path "/cli/completion" must NOT have additional properties(invalid).',
+    )
+  ) {
+    throw new Error('Should have failed with must NOT have additional properties(invalid).');
+  }
+
+  ngError = await expectToFail(() => silentNg('config', '--global', 'cli.cache.enabled', 'true'));
+
+  if (!ngError.message.includes('Data path "/cli" must NOT have additional properties(cache).')) {
+    throw new Error('Should have failed with must NOT have additional properties(cache).');
+  }
+
+  ngError = await expectToFail(() => silentNg('config', 'cli.completion.prompted'));
+
+  if (!ngError.message.includes('Value cannot be found.')) {
+    throw new Error('Should have failed with Value cannot be found.');
+  }
+
+  await ng('config', '--global', 'cli.completion.prompted', 'true');
+  const { stdout } = await silentNg('config', '--global', 'cli.completion.prompted');
+
+  if (!stdout.includes('true')) {
+    throw new Error(`Expected "true", received "${JSON.stringify(stdout)}".`);
+  }
+
+  await expectFileToExist(path.join(homedir(), '.angular-config.json'));
+  await deleteFile(path.join(homedir(), '.angular-config.json'));
+}
diff --git a/tests/legacy-cli/e2e/tests/commands/config/config-set.ts b/tests/legacy-cli/e2e/tests/commands/config/config-set.ts
@@ -1,8 +1,23 @@
-import { ng } from '../../../utils/process';
+import { ng, silentNg } from '../../../utils/process';
 import { expectToFail } from '../../../utils/utils';
 
 export default async function () {
-  await expectToFail(() => ng('config', 'cli.warnings.zzzz'));
+  let ngError: Error;
+
+  ngError = await expectToFail(() => silentNg('config', 'cli.warnings.zzzz', 'true'));
+  if (
+    !ngError.message.includes(
+      'Data path "/cli/warnings" must NOT have additional properties(zzzz).',
+    )
+  ) {
+    throw new Error('Should have failed with must NOT have additional properties(zzzz).');
+  }
+
+  ngError = await expectToFail(() => silentNg('config', 'cli.warnings.zzzz'));
+  if (!ngError.message.includes('Value cannot be found.')) {
+    throw new Error('Should have failed with Value cannot be found.');
+  }
+
   await ng('config', 'cli.warnings.versionMismatch', 'false');
   const { stdout } = await ng('config', 'cli.warnings.versionMismatch');
   if (!stdout.includes('false')) {
