From f82f3659d56c945b641db5a7d75705cf8c1e94c5 Mon Sep 17 00:00:00 2001
From: Mark Yen <mark.yen@suse.com>
Date: Tue, 9 May 2023 16:54:37 -0700
Subject: [PATCH 1/6] RDX: Add preferences for extension allow list

Nothing is hooked up to it yet.

Signed-off-by: Mark Yen <mark.yen@suse.com>
---
 .../assets/specs/command-api.yaml             | 14 +++++++
 pkg/rancher-desktop/config/settings.ts        | 10 ++++-
 .../main/commandServer/settingsValidator.ts   | 39 +++++++++++++++++--
 scripts/generateCliCode.ts                    |  1 +
 4 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/pkg/rancher-desktop/assets/specs/command-api.yaml b/pkg/rancher-desktop/assets/specs/command-api.yaml
index 6f960290467..9309f08d536 100644
--- a/pkg/rancher-desktop/assets/specs/command-api.yaml
+++ b/pkg/rancher-desktop/assets/specs/command-api.yaml
@@ -345,6 +345,20 @@ components:
             debug:
               type: boolean
               x-rd-usage: generate more verbose logging
+            extensions:
+              type: object
+              properties:
+                allowed:
+                  type: object
+                  properties:
+                    enabled:
+                      type: boolean
+                      x-rd-hidden: true
+                    list:
+                      type: array
+                      items:
+                        type: string
+                        x-rd-hidden: true
             pathManagementStrategy:
               type: string
               enum: [manual, rcfiles]
diff --git a/pkg/rancher-desktop/config/settings.ts b/pkg/rancher-desktop/config/settings.ts
index b148e3d37bc..7cea538e697 100644
--- a/pkg/rancher-desktop/config/settings.ts
+++ b/pkg/rancher-desktop/config/settings.ts
@@ -69,8 +69,14 @@ export enum CacheMode {
 export const defaultSettings = {
   version:     CURRENT_SETTINGS_VERSION,
   application: {
-    adminAccess:            true,
-    debug:                  false,
+    adminAccess: true,
+    debug:       false,
+    extensions:  {
+      allowed: {
+        enabled: false,
+        list:    [],
+      },
+    },
     pathManagementStrategy: PathManagementStrategy.NotSet,
     telemetry:              { enabled: true },
     /** Whether we should check for updates and apply them. */
diff --git a/pkg/rancher-desktop/main/commandServer/settingsValidator.ts b/pkg/rancher-desktop/main/commandServer/settingsValidator.ts
index 2ad65a0a7ab..620544e605c 100644
--- a/pkg/rancher-desktop/main/commandServer/settingsValidator.ts
+++ b/pkg/rancher-desktop/main/commandServer/settingsValidator.ts
@@ -15,7 +15,7 @@ import {
 } from '@pkg/config/settings';
 import { NavItemName, navItemNames, TransientSettings } from '@pkg/config/transientSettings';
 import { PathManagementStrategy } from '@pkg/integrations/pathManager';
-import { validateImageName, validateImageTag } from '@pkg/utils/dockerUtils';
+import { parseImageReference, validateImageName, validateImageTag } from '@pkg/utils/dockerUtils';
 import { RecursivePartial } from '@pkg/utils/typeUtils';
 import { preferencesNavItems } from '@pkg/window/preferences';
 
@@ -73,8 +73,14 @@ export default class SettingsValidator {
     this.allowedSettings ||= {
       version:     this.checkUnchanged,
       application: {
-        adminAccess:            this.checkLima(this.checkBoolean),
-        debug:                  this.checkBoolean,
+        adminAccess: this.checkLima(this.checkBoolean),
+        debug:       this.checkBoolean,
+        extensions:  {
+          allowed: {
+            enabled: this.checkBoolean,
+            list:    this.checkExtensionAllowList,
+          },
+        },
         pathManagementStrategy: this.checkLima(this.checkPathManagementStrategy),
         telemetry:              { enabled: this.checkBoolean },
         /** Whether we should check for updates and apply them. */
@@ -548,6 +554,33 @@ export default class SettingsValidator {
     return !_.isEqual(desiredValue, currentValue);
   }
 
+  protected checkExtensionAllowList(
+    mergedSettings: Settings,
+    currentValue: string[],
+    desiredValue: any,
+    errors: string[],
+    fqname: string,
+  ): boolean {
+    if (_.isEqual(desiredValue, currentValue)) {
+      // Accept no-op changes
+      return false;
+    }
+
+    const changed = this.checkUniqueStringArray(mergedSettings, currentValue, desiredValue, errors, fqname);
+
+    if (errors.length) {
+      return changed;
+    }
+
+    for (const pattern of desiredValue as string[]) {
+      if (!parseImageReference(pattern)) {
+        errors.push(`${ fqname }: "${ pattern }" does not describe an image reference`);
+      }
+    }
+
+    return errors.length === 0 && changed;
+  }
+
   protected checkPreferencesNavItemCurrent(
     mergedSettings: TransientSettings,
     currentValue: NavItemName,
diff --git a/scripts/generateCliCode.ts b/scripts/generateCliCode.ts
index fb253cea364..29c11244cef 100644
--- a/scripts/generateCliCode.ts
+++ b/scripts/generateCliCode.ts
@@ -275,6 +275,7 @@ class Generator {
     settingsTree: settingsTreeType): void {
     const platforms = preference['x-rd-platforms'] ?? [];
 
+    notAvailable ||= preference['x-rd-hidden'];
     notAvailable ||= platforms.length > 0 && !platforms.includes(process.platform);
     switch (preference.type) {
     case 'object':

From 1a92bbfe69b2b5c459829fa9556d484dbce47176 Mon Sep 17 00:00:00 2001
From: Mark Yen <mark.yen@suse.com>
Date: Wed, 10 May 2023 11:22:53 -0700
Subject: [PATCH 2/6] RDX: Implement image allow list

This lets administrators supply a list of image references that describe
which extensions are allowed to be installed (optionally with a tag).  It's
possible to set this to an empty list (disallowing any extensions).

Signed-off-by: Mark Yen <mark.yen@suse.com>
---
 background.ts                                 |   6 +-
 bats/tests/extensions/allow-list.bats         | 107 ++++++++++++++++++
 pkg/rancher-desktop/config/settings.ts        |   2 +-
 .../main/commandServer/settingsValidator.ts   |   2 +-
 .../extensions/__tests__/extensions.spec.ts   |  49 ++++++++
 .../main/extensions/extensions.ts             |  53 ++++++++-
 .../main/extensions/manager.ts                |   4 +-
 pkg/rancher-desktop/main/extensions/types.ts  |   6 +-
 .../utils/__tests__/dockerUtils.spec.ts       |  17 +++
 pkg/rancher-desktop/utils/dockerUtils.ts      |  47 ++++++--
 10 files changed, 279 insertions(+), 14 deletions(-)
 create mode 100644 bats/tests/extensions/allow-list.bats
 create mode 100644 pkg/rancher-desktop/main/extensions/__tests__/extensions.spec.ts

diff --git a/background.ts b/background.ts
index d7a67e08ddd..802271b3444 100644
--- a/background.ts
+++ b/background.ts
@@ -1097,7 +1097,9 @@ class BackgroundCommandWorker implements CommandWorkerInterface {
     if (state === 'install') {
       console.debug(`Installing extension ${ image }...`);
       try {
-        if (await extension.install()) {
+        const { enabled, list } = cfg.application.extensions.allowed;
+
+        if (await extension.install(enabled ? list : undefined)) {
           return { status: 201 };
         } else {
           return { status: 204 };
@@ -1109,6 +1111,8 @@ class BackgroundCommandWorker implements CommandWorkerInterface {
             return { status: 422, data: `The image ${ image } has invalid extension metadata` };
           case ExtensionErrorCode.FILE_NOT_FOUND:
             return { status: 422, data: `The image ${ image } failed to install: ${ ex.message }` };
+          case ExtensionErrorCode.INSTALL_DENIED:
+            return { status: 403, data: `The image ${ image } is not an allowed extension` };
           }
         }
         throw ex;
diff --git a/bats/tests/extensions/allow-list.bats b/bats/tests/extensions/allow-list.bats
new file mode 100644
index 00000000000..c2a91537c0c
--- /dev/null
+++ b/bats/tests/extensions/allow-list.bats
@@ -0,0 +1,107 @@
+load '../helpers/load'
+
+setup() {
+    TESTDATA_DIR="${PATH_BATS_ROOT}/tests/extensions/testdata/"
+
+    if using_windows_exe; then
+        TESTDATA_DIR_CLI="$(wslpath -m "${TESTDATA_DIR}")"
+    else
+        TESTDATA_DIR_CLI="${TESTDATA_DIR}"
+    fi
+
+    if using_containerd; then
+        namespace_arg=('--namespace=rancher-desktop-extensions')
+    else
+        namespace_arg=()
+    fi
+}
+
+write_allow_list() { # list
+    local list=${1:-}
+    local allowed=true
+
+    if [ -z "$list" ]; then
+        allowed=false
+    fi
+
+    # Note that the list preference is not writable using `rdctl set`, and we
+    # need to do a direct API call instead.
+
+    rdctl api /v1/settings --input - <<< '{
+        "version": 7,
+        "application": {
+            "extensions": {
+                "allowed": {
+                    "enabled": '"${allowed}"',
+                    "list": '"${list:-[]}"'
+                }
+            }
+        }
+    }'
+}
+
+@test 'factory reset' {
+    factory_reset
+}
+
+@test 'start container engine' {
+    RD_ENV_EXTENSIONS=1 start_container_engine
+    wait_for_container_engine
+}
+
+@test 'build extension testing image' {
+    ctrctl "${namespace_arg[@]}" build \
+        --tag "rd/extension/basic" \
+        --build-arg "variant=basic" \
+        "$TESTDATA_DIR_CLI"
+
+    run ctrctl "${namespace_arg[@]}" image list --format '{{ .Repository }}'
+    assert_success
+    assert_line "rd/extension/basic"
+}
+
+@test 'when no extension allow list is set up, all extensions can install' {
+    write_allow_list ''
+    rdctl extension install rd/extension/basic
+    rdctl extension uninstall rd/extension/basic
+}
+
+@test 'when an extension is explicitly allowed, it can be installed' {
+    write_allow_list '["rd/extension/basic:latest"]'
+    rdctl extension install rd/extension/basic:latest
+    rdctl extension uninstall rd/extension/basic
+}
+
+@test 'when an extension is not in the allowe list, it cannot be installed' {
+    write_allow_list '["rd/extension/other"]'
+    run rdctl extension install rd/extension/basic
+    assert_failure
+}
+
+@test 'when no tags given, any tag is allowed' {
+    write_allow_list '["rd/extension/basic"]'
+    ctrctl "${namespace_arg[@]}" tag rd/extension/basic rd/extension/basic:0.0.3
+    rdctl extension install rd/extension/basic:0.0.3
+    rdctl extension uninstall rd/extension/basic
+}
+
+@test 'when tags are given, only the specified tag is allowed' {
+    sleep 20
+    write_allow_list '["rd/extension/basic:0.0.2"]'
+    ctrctl "${namespace_arg[@]}" tag rd/extension/basic rd/extension/basic:0.0.3
+    run rdctl extension install rd/extension/basic:0.0.3
+    assert_failure
+}
+
+@test 'extensions can be allowed by organization' {
+    write_allow_list '["rd/extension/"]'
+    rdctl extension install rd/extension/basic
+    rdctl extension uninstall rd/extension/basic
+}
+
+@test 'extensions can be allowed by repository host' {
+    write_allow_list '["registry.test/"]'
+    ctrctl "${namespace_arg[@]}" tag rd/extension/basic registry.test/basic:0.0.3
+    rdctl extension install registry.test/basic:0.0.3
+    rdctl extension uninstall registry.test/basic
+}
diff --git a/pkg/rancher-desktop/config/settings.ts b/pkg/rancher-desktop/config/settings.ts
index 7cea538e697..4589f4e428b 100644
--- a/pkg/rancher-desktop/config/settings.ts
+++ b/pkg/rancher-desktop/config/settings.ts
@@ -74,7 +74,7 @@ export const defaultSettings = {
     extensions:  {
       allowed: {
         enabled: false,
-        list:    [],
+        list:    [] as Array<string>,
       },
     },
     pathManagementStrategy: PathManagementStrategy.NotSet,
diff --git a/pkg/rancher-desktop/main/commandServer/settingsValidator.ts b/pkg/rancher-desktop/main/commandServer/settingsValidator.ts
index 620544e605c..2f40a0b7525 100644
--- a/pkg/rancher-desktop/main/commandServer/settingsValidator.ts
+++ b/pkg/rancher-desktop/main/commandServer/settingsValidator.ts
@@ -573,7 +573,7 @@ export default class SettingsValidator {
     }
 
     for (const pattern of desiredValue as string[]) {
-      if (!parseImageReference(pattern)) {
+      if (!parseImageReference(pattern, true)) {
         errors.push(`${ fqname }: "${ pattern }" does not describe an image reference`);
       }
     }
diff --git a/pkg/rancher-desktop/main/extensions/__tests__/extensions.spec.ts b/pkg/rancher-desktop/main/extensions/__tests__/extensions.spec.ts
new file mode 100644
index 00000000000..45e14d79f8f
--- /dev/null
+++ b/pkg/rancher-desktop/main/extensions/__tests__/extensions.spec.ts
@@ -0,0 +1,49 @@
+import { ExtensionImpl } from '@pkg/main/extensions/extensions';
+
+describe('ExtensionImpl', () => {
+  describe('checkInstallAllowed', () => {
+    const subject = ExtensionImpl['checkInstallAllowed'];
+
+    it('should reject invalid image references', () => {
+      expect(() => subject(undefined, '/')).toThrow();
+    });
+
+    it('should allow images if the allow list is not enabled', () => {
+      expect(() => subject(undefined, 'image')).not.toThrow();
+    });
+
+    it('should disallow any images given an empty list', () => {
+      expect(() => subject([], 'image')).toThrow();
+    });
+
+    it('should allow specified image', () => {
+      expect(() => subject(['other', 'image'], 'image')).not.toThrow();
+    });
+
+    it('should reject unknown image', () => {
+      expect(() => subject(['allowed'], 'image')).toThrow();
+    });
+
+    it('should support missing tags', () => {
+      expect(() => subject(['image'], 'image:1.0.0')).not.toThrow();
+    });
+
+    it('should reject images with the wrong tag', () => {
+      expect(() => subject(['image:0.1'], 'image:0.2')).toThrow();
+    });
+
+    it('should support image references with registries', () => {
+      const ref = 'r.example.test:1234/org/name:tag';
+
+      expect(() => subject([ref], ref)).not.toThrow();
+    });
+
+    it('should support org-level references', () => {
+      expect(() => subject(['test.invalid/org/'], 'test.invalid/org/image:tag')).not.toThrow();
+    });
+
+    it('should support registry-level references', () => {
+      expect(() => subject(['registry.test/'], 'registry.test/image:tag')).not.toThrow();
+    });
+  });
+});
diff --git a/pkg/rancher-desktop/main/extensions/extensions.ts b/pkg/rancher-desktop/main/extensions/extensions.ts
index 95171817214..cdee3b9f49c 100644
--- a/pkg/rancher-desktop/main/extensions/extensions.ts
+++ b/pkg/rancher-desktop/main/extensions/extensions.ts
@@ -12,6 +12,7 @@ import {
 
 import type { ContainerEngineClient } from '@pkg/backend/containerClient';
 import mainEvents from '@pkg/main/mainEvents';
+import { parseImageReference } from '@pkg/utils/dockerUtils';
 import Logging from '@pkg/utils/logging';
 import paths from '@pkg/utils/paths';
 import { defined } from '@pkg/utils/typeUtils';
@@ -142,9 +143,59 @@ export class ExtensionImpl implements Extension {
     return this._iconName as Promise<string>;
   }
 
-  async install(): Promise<boolean> {
+  /**
+   * Check if the given image is allowed to be installed according to the
+   * extension allow list.
+   * @throws If the image is not allowed to be installed.
+   */
+  protected static checkInstallAllowed(allowedImages: readonly string[] | undefined, image: string) {
+    const desired = parseImageReference(image);
+    const code = ExtensionErrorCode.INSTALL_DENIED;
+    const prefix = `Disallowing install of ${ image }:`;
+
+    if (!desired) {
+      throw new ExtensionErrorImpl(code, `${ prefix } Invalid image reference`);
+    }
+    if (!allowedImages) {
+      return;
+    }
+    for (const pattern of allowedImages) {
+      const allowed = parseImageReference(pattern, true);
+
+      if (allowed?.tag && allowed.tag !== desired.tag) {
+        // This pattern doesn't match the tag, look for something else.
+        continue;
+      }
+
+      if (allowed?.registry.href !== desired.registry.href) {
+        // This pattern has a different registry
+        continue;
+      }
+
+      if (!allowed.name) {
+        // If there's no name given, the whole registry is allowed.
+        return '';
+      }
+
+      if (allowed.name.endsWith('/')) {
+        if (desired.name.startsWith(allowed.name)) {
+          // The allowed pattern ends with a slash, anything in the org is fine.
+          return '';
+        }
+      } else if (allowed.name === desired.name) {
+        return '';
+      }
+    }
+
+    throw new ExtensionErrorImpl(code, `${ prefix } Image is not allowed`);
+  }
+
+  async install(allowedImages: readonly string[] | undefined): Promise<boolean> {
     const metadata = await this.metadata;
 
+    ExtensionImpl.checkInstallAllowed(allowedImages, this.image);
+    console.debug(`Image ${ this.image } is allowed to install: ${ allowedImages }`);
+
     await fs.promises.mkdir(this.dir, { recursive: true });
     try {
       await this.installMetadata(this.dir, metadata);
diff --git a/pkg/rancher-desktop/main/extensions/manager.ts b/pkg/rancher-desktop/main/extensions/manager.ts
index 9aa49a87e7a..45aa328816e 100644
--- a/pkg/rancher-desktop/main/extensions/manager.ts
+++ b/pkg/rancher-desktop/main/extensions/manager.ts
@@ -211,6 +211,8 @@ class ExtensionManagerImpl implements ExtensionManager {
 
     // Install / uninstall extensions as needed.
     const tasks: Promise<any>[] = [];
+    const { enabled: allowEnabled, list: allowListRaw } = config.application.extensions.allowed;
+    const allowList = allowEnabled ? allowListRaw : undefined;
 
     for (const [repo, tag] of Object.entries(config.extensions)) {
       if (!tag) {
@@ -221,7 +223,7 @@ class ExtensionManagerImpl implements ExtensionManager {
 
       tasks.push((async(id: string) => {
         try {
-          return (await this.getExtension(id)).install();
+          return (await this.getExtension(id)).install(allowList);
         } catch (ex) {
           console.error(`Failed to install extension ${ id }`, ex);
         }
diff --git a/pkg/rancher-desktop/main/extensions/types.ts b/pkg/rancher-desktop/main/extensions/types.ts
index 4cd0b5d49dc..3ed03ec0914 100644
--- a/pkg/rancher-desktop/main/extensions/types.ts
+++ b/pkg/rancher-desktop/main/extensions/types.ts
@@ -71,10 +71,13 @@ export interface Extension {
 
   /**
    * Install this extension.
+   * @param allowedImages The list of extension images that are allowed to be
+   *        used; if all images are allowed, pass in undefined.
    * @note If the extension is already installed, this is a no-op.
+   * @throws If the settings specify an allow list and this is not in it.
    * @return Whether the extension was installed.
    */
-  install(): Promise<boolean>;
+  install(allowedImages: readonly string[] | undefined): Promise<boolean>;
   /**
    * Uninstall this extension.
    * @note If the extension was not installed, this is a no-op.
@@ -169,6 +172,7 @@ export const ExtensionErrorMarker = Symbol('extension-error');
 export enum ExtensionErrorCode {
   INVALID_METADATA,
   FILE_NOT_FOUND,
+  INSTALL_DENIED,
 }
 
 export interface ExtensionError extends Error {
diff --git a/pkg/rancher-desktop/utils/__tests__/dockerUtils.spec.ts b/pkg/rancher-desktop/utils/__tests__/dockerUtils.spec.ts
index 21cc916e811..d98ef983076 100644
--- a/pkg/rancher-desktop/utils/__tests__/dockerUtils.spec.ts
+++ b/pkg/rancher-desktop/utils/__tests__/dockerUtils.spec.ts
@@ -12,9 +12,26 @@ describe('parseImageReference', () => {
     ':10/tag':                          null,
     [`xxx:${ Array(130).join('x') }`]:  null,
     'name:':                            null,
+    'dir/':                             null,
+    '':                                 null,
   };
 
   test.each(Object.entries(testCases))('%s', (input, expected) => {
     expect(parseImageReference(input)).toEqual(expected);
   });
+
+  describe('when parsing for prefix', () => {
+    const testCases: Record<string, ReturnType<typeof parseImageReference>> = {
+      component:            new imageInfo(dockerHub, 'library/component'),
+      'name:tag':           new imageInfo(dockerHub, 'library/name', 'tag'),
+      'dir/':               new imageInfo(dockerHub, 'dir/'),
+      'registry.test/':     new imageInfo(new URL('https://registry.test/'), ''),
+      'registry.test/dir/': new imageInfo(new URL('https://registry.test/'), 'dir/'),
+      '':                   null,
+    };
+
+    test.each(Object.entries(testCases))('%s', (input, expected) => {
+      expect(parseImageReference(input, true)).toEqual(expected);
+    });
+  });
 });
diff --git a/pkg/rancher-desktop/utils/dockerUtils.ts b/pkg/rancher-desktop/utils/dockerUtils.ts
index f6455a53303..30b53844bf2 100644
--- a/pkg/rancher-desktop/utils/dockerUtils.ts
+++ b/pkg/rancher-desktop/utils/dockerUtils.ts
@@ -46,11 +46,7 @@ function makeRE(strings: TemplateStringsArray, ...substitutions: any[]) {
   return new RegExp(uncommentedLines.join('').replace(/\s+/g, ''));
 }
 
-/**
- * ImageNameRegExp is a regular expression that matches a docker image name
- * (including optional registry and one or more name components).
- */
-const ImageNameRegExp = (function() {
+const { ImageNameRegExp, ImageNamePrefixRegExp } = (function() {
   // a domain component is alpha-numeric-or-dash, but the start and end
   // characters may not be a dash.
   const domainComponent = /[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?/;
@@ -64,13 +60,34 @@ const ImageNameRegExp = (function() {
   // a set of separators.
   const nameComponent = /[a-z0-9]+(?:(?:\.|_|__|-*)[a-z0-9]+)*/;
 
-  return makeRE`
+  /**
+   * ImageNameRegExp is a regular expression that matches a docker image name
+   * (including optional registry and one or more name components).
+   */
+  const ImageNameRegExp = makeRE`
     (?:(?<domain>${ domain })/)?
     (?<name>
       ${ nameComponent }
       (?:/${ nameComponent })*
     )
     `;
+
+  /**
+   * ImageRefPrefixRegExp is a regular expression similar to ImageNameRegExp but
+   * supports looking for prefixes (i.e. a name that ends in a slash).
+   * Note that we may end up with just the domain (no name).
+   */
+  const ImageNamePrefixRegExp = makeRE`
+    (?:
+      (?:(?<domain>${ domain })/)?
+      (?<name>
+        (?:${ nameComponent }/)*
+        (?:${ nameComponent })?
+      )
+    )
+  `;
+
+  return { ImageNameRegExp, ImageNamePrefixRegExp };
 })();
 
 /**
@@ -86,17 +103,31 @@ const ImageRefRegExp = makeRE`
   $
   `;
 
+const ImageRefPrefixRegExp = makeRE`
+  ^
+  ${ ImageNamePrefixRegExp }
+  (?::(?<tag>${ ImageTagRegExp }))?
+  $
+  `;
+
 /**
  * Given an image reference, parse it into (possibly) registry, name, and
  * (possibly) tag components.
+ * @param prefix If set, accept prefixes (names that end with a slash).
  */
-export function parseImageReference(reference: string): imageInfo | null {
-  const result = ImageRefRegExp.exec(reference);
+export function parseImageReference(reference: string, prefix = false): imageInfo | null {
+  const result = (prefix ? ImageRefPrefixRegExp : ImageRefRegExp).exec(reference);
 
   if (!result?.groups) {
     return null;
   }
 
+  if (!result.groups['domain'] && !result.groups['name']) {
+    // When checking for a prefix, parsing an empty string can succeed; in that
+    // case, reject it rather than accepting anything from Docker Hub.
+    return null;
+  }
+
   let registry = result.groups['domain'] ?? 'index.docker.io';
   let name = result.groups['name'];
 

From 3cccdb1c7b85ec00551b2eee128f93539cb0084f Mon Sep 17 00:00:00 2001
From: Mark Yen <mark.yen@suse.com>
Date: Thu, 11 May 2023 13:35:55 -0700
Subject: [PATCH 3/6] BATS: fix lint issues, determine pref version dynamically

Signed-off-by: Mark Yen <mark.yen@suse.com>
---
 bats/tests/extensions/allow-list.bats | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bats/tests/extensions/allow-list.bats b/bats/tests/extensions/allow-list.bats
index c2a91537c0c..5b5396301bb 100644
--- a/bats/tests/extensions/allow-list.bats
+++ b/bats/tests/extensions/allow-list.bats
@@ -17,7 +17,7 @@ setup() {
 }
 
 write_allow_list() { # list
-    local list=${1:-}
+    local list=${1-}
     local allowed=true
 
     if [ -z "$list" ]; then
@@ -27,8 +27,8 @@ write_allow_list() { # list
     # Note that the list preference is not writable using `rdctl set`, and we
     # need to do a direct API call instead.
 
-    rdctl api /v1/settings --input - <<< '{
-        "version": 7,
+    rdctl api /v1/settings --input - <<<'{
+        "version": '"$(get_setting .version)"',
         "application": {
             "extensions": {
                 "allowed": {

From 2c1896c9fcef638cf981c13bed5f966c9db7dcfc Mon Sep 17 00:00:00 2001
From: Mark Yen <mark.yen@suse.com>
Date: Mon, 15 May 2023 16:58:34 -0700
Subject: [PATCH 4/6] RDX: Address review comments

- Typos
- When checking allow list, explicitly check that the extensions are
  installed / not installed, as desired.
- Make sure to add test cases with no, and multiple, items in the allow
  list.

Signed-off-by: Mark Yen <mark.yen@suse.com>
---
 bats/tests/extensions/allow-list.bats         | 30 +++++++++++++++++--
 .../main/extensions/extensions.ts             |  6 ++--
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/bats/tests/extensions/allow-list.bats b/bats/tests/extensions/allow-list.bats
index 5b5396301bb..276fccca490 100644
--- a/bats/tests/extensions/allow-list.bats
+++ b/bats/tests/extensions/allow-list.bats
@@ -40,6 +40,12 @@ write_allow_list() { # list
     }'
 }
 
+check_extension_installed() { # refute, name
+    run rdctl extension ls
+    assert_success
+    ${1:-assert}_output --partial ${2:-rd/extension/basic}
+}
+
 @test 'factory reset' {
     factory_reset
 }
@@ -63,26 +69,39 @@ write_allow_list() { # list
 @test 'when no extension allow list is set up, all extensions can install' {
     write_allow_list ''
     rdctl extension install rd/extension/basic
+    check_extension_installed
     rdctl extension uninstall rd/extension/basic
 }
 
+@test 'empty allow list disables extension installs' {
+    write_allow_list '[]'
+    run rdctl extension install rd/extension/basic
+    assert_failure
+    check_extension_installed refute
+}
+
 @test 'when an extension is explicitly allowed, it can be installed' {
-    write_allow_list '["rd/extension/basic:latest"]'
+    write_allow_list '["irrelevant/image","rd/extension/basic:latest"]'
     rdctl extension install rd/extension/basic:latest
+    check_extension_installed
     rdctl extension uninstall rd/extension/basic
+    check_extension_installed refute
 }
 
-@test 'when an extension is not in the allowe list, it cannot be installed' {
-    write_allow_list '["rd/extension/other"]'
+@test 'when an extension is not in the allowed list, it cannot be installed' {
+    write_allow_list '["rd/extension/other","registry.test/image"]'
     run rdctl extension install rd/extension/basic
     assert_failure
+    check_extension_installed refute
 }
 
 @test 'when no tags given, any tag is allowed' {
     write_allow_list '["rd/extension/basic"]'
     ctrctl "${namespace_arg[@]}" tag rd/extension/basic rd/extension/basic:0.0.3
     rdctl extension install rd/extension/basic:0.0.3
+    check_extension_installed
     rdctl extension uninstall rd/extension/basic
+    check_extension_installed refute
 }
 
 @test 'when tags are given, only the specified tag is allowed' {
@@ -91,17 +110,22 @@ write_allow_list() { # list
     ctrctl "${namespace_arg[@]}" tag rd/extension/basic rd/extension/basic:0.0.3
     run rdctl extension install rd/extension/basic:0.0.3
     assert_failure
+    check_extension_installed refute
 }
 
 @test 'extensions can be allowed by organization' {
     write_allow_list '["rd/extension/"]'
     rdctl extension install rd/extension/basic
+    check_extension_installed
     rdctl extension uninstall rd/extension/basic
+    check_extension_installed refute
 }
 
 @test 'extensions can be allowed by repository host' {
     write_allow_list '["registry.test/"]'
     ctrctl "${namespace_arg[@]}" tag rd/extension/basic registry.test/basic:0.0.3
     rdctl extension install registry.test/basic:0.0.3
+    check_extension_installed '' registry.test/basic
     rdctl extension uninstall registry.test/basic
+    check_extension_installed refute registry.test/basic
 }
diff --git a/pkg/rancher-desktop/main/extensions/extensions.ts b/pkg/rancher-desktop/main/extensions/extensions.ts
index cdee3b9f49c..21e2c8707dc 100644
--- a/pkg/rancher-desktop/main/extensions/extensions.ts
+++ b/pkg/rancher-desktop/main/extensions/extensions.ts
@@ -25,9 +25,9 @@ class ExtensionErrorImpl extends Error implements ExtensionError {
 
   constructor(code: ExtensionErrorCode, message: string, cause?: Error) {
     // XXX We're currently using a version of TypeScript that doesn't have the
-    // ES2022.Error lib, so it does't understand the "cause" option to the Error
-    // constructor.  Work around this by explicitly calling setting the cause.
-    // It appears to still be printed in that case.
+    // ES2022.Error lib, so it doesn't understand the "cause" option to the
+    // Error constructor.  Work around this by explicitly calling setting the
+    // cause.  It appears to still be printed in that case.
     super(message);
     if (cause) {
       (this as any).cause = cause;

From 8cdcbd945f19f8091049d8e92b7b2a928978c252 Mon Sep 17 00:00:00 2001
From: Mark Yen <mark.yen@suse.com>
Date: Mon, 15 May 2023 17:34:29 -0700
Subject: [PATCH 5/6] BATS: fix lint error

Signed-off-by: Mark Yen <mark.yen@suse.com>
---
 bats/tests/extensions/allow-list.bats | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bats/tests/extensions/allow-list.bats b/bats/tests/extensions/allow-list.bats
index 276fccca490..b3a27677e3a 100644
--- a/bats/tests/extensions/allow-list.bats
+++ b/bats/tests/extensions/allow-list.bats
@@ -43,7 +43,7 @@ write_allow_list() { # list
 check_extension_installed() { # refute, name
     run rdctl extension ls
     assert_success
-    ${1:-assert}_output --partial ${2:-rd/extension/basic}
+    "${1:-assert}_output" --partial "${2:-rd/extension/basic}"
 }
 
 @test 'factory reset' {

From 2a8f22e4a608b173a89998859a5364085428fd02 Mon Sep 17 00:00:00 2001
From: Mark Yen <mark.yen@suse.com>
Date: Wed, 17 May 2023 14:15:54 -0700
Subject: [PATCH 6/6] Address review comments

- extensions/types: Fix parameter name
- extensions/manager: Wait for tasks _after_ starting all of them
- extensions/manager: remove unnecessary assignment to `tag`

Signed-off-by: Mark Yen <mark.yen@suse.com>
---
 pkg/rancher-desktop/main/extensions/manager.ts | 4 ++--
 pkg/rancher-desktop/main/extensions/types.ts   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/pkg/rancher-desktop/main/extensions/manager.ts b/pkg/rancher-desktop/main/extensions/manager.ts
index 45aa328816e..0893f1bc260 100644
--- a/pkg/rancher-desktop/main/extensions/manager.ts
+++ b/pkg/rancher-desktop/main/extensions/manager.ts
@@ -228,17 +228,17 @@ class ExtensionManagerImpl implements ExtensionManager {
           console.error(`Failed to install extension ${ id }`, ex);
         }
       })(`${ repo }:${ tag }`));
-      await Promise.all(tasks);
     }
+    await Promise.all(tasks);
   }
 
   async getExtension(image: string): Promise<Extension> {
+    // eslint-disable-next-line prefer-const
     let [, repo, tag] = /^(.*):(.*?)$/.exec(image) ?? ['', image, undefined];
     const extGroup = this.extensions[image] ?? {};
 
     // The build process uses an older TypeScript that can't infer repo correctly.
     repo ??= image;
-    tag ??= undefined;
 
     this.extensions[repo] = extGroup;
 
diff --git a/pkg/rancher-desktop/main/extensions/types.ts b/pkg/rancher-desktop/main/extensions/types.ts
index 3ed03ec0914..ff4e46ec37a 100644
--- a/pkg/rancher-desktop/main/extensions/types.ts
+++ b/pkg/rancher-desktop/main/extensions/types.ts
@@ -109,8 +109,8 @@ export interface ExtensionManager {
 
   /**
    * Get the given extension.
-   * @param id The image ID of the extension, possibly including the tag.
-   *        If the tag is not supplied, the currently-installed version is
+   * @param image The image reference of the extension, possibly including the
+   *        tag.  If the tag is not supplied, the currently-installed version is
    *        used; if no version is installed, "latest" is assumed.
    * @note This may cause the given image to be downloaded.
    * @note The extension will not be automatically installed.