Bug 1807010 - Migrate ExtensionPermissions kvstore to a separate file path. r=willdurand,robwu

Differential Revision: https://phabricator.services.mozilla.com/D166046

toolkit/components/extensions/ExtensionPermissions.jsm

@@ -41,10 +41,16 @@ XPCOMUtils.defineLazyGetter(
   () => lazy.ExtensionParent.apiManager
 );
 
-var EXPORTED_SYMBOLS = ["ExtensionPermissions", "OriginControls"];
-
-// This is the old preference file pre-migration to rkv
-const FILE_NAME = "extension-preferences.json";
+var EXPORTED_SYMBOLS = [
+  "ExtensionPermissions",
+  "OriginControls",
+  // Constants exported for testing purpose.
+  "OLD_JSON_FILENAME",
+  "OLD_RKV_DIRNAME",
+  "RKV_DIRNAME",
+  "VERSION_KEY",
+  "VERSION_VALUE",
+];
 
 function emptyPermissions() {
   return { permissions: [], origins: [] };
@@ -52,11 +58,19 @@ function emptyPermissions() {
 
 const DEFAULT_VALUE = JSON.stringify(emptyPermissions());
 
-const VERSION_KEY = "_version";
-const VERSION_VALUE = 1;
-
 const KEY_PREFIX = "id-";
 
+// This is the old preference file pre-migration to rkv.
+const OLD_JSON_FILENAME = "extension-preferences.json";
+// This is the old path to the rkv store dir (which used to be shared with ExtensionScriptingStore).
+const OLD_RKV_DIRNAME = "extension-store";
+// This is the new path to the rkv store dir.
+const RKV_DIRNAME = "extension-store-permissions";
+
+const VERSION_KEY = "_version";
+
+const VERSION_VALUE = 1;
+
 // Bug 1646182: remove once we fully migrate to rkv
 let prefs;
 
@@ -72,7 +86,7 @@ class LegacyPermissionStore {
   async _init() {
     let path = PathUtils.join(
       Services.dirsvc.get("ProfD", Ci.nsIFile).path,
-      FILE_NAME
+      OLD_JSON_FILENAME
     );
 
     prefs = new lazy.JSONFile({ path });
@@ -134,8 +148,10 @@ class LegacyPermissionStore {
 }
 
 class PermissionStore {
+  _shouldMigrateFromOldKVStorePath = AppConstants.NIGHTLY_BUILD;
+
   async _init() {
-    const storePath = lazy.FileUtils.getDir("ProfD", ["extension-store"]).path;
+    const storePath = lazy.FileUtils.getDir("ProfD", [RKV_DIRNAME]).path;
     // Make sure the folder exists
     await IOUtils.makeDirectory(storePath, { ignoreExisting: true });
     this._store = await lazy.KeyValueService.getOrCreate(
@@ -143,7 +159,27 @@ class PermissionStore {
       "permissions"
     );
     if (!(await this._store.has(VERSION_KEY))) {
-      await this.maybeMigrateData();
+      // If _shouldMigrateFromOldKVStorePath is true (default only on Nightly channel
+      // where the rkv store has been enabled by default for a while), we need to check
+      // if we would need to import data from the old kvstore path (ProfD/extensions-store)
+      // first, and fallback to try to import from the JSONFile if there was no data in
+      // the old kvstore path.
+      // NOTE: _shouldMigrateFromOldKVStorePath is also explicitly set to true in unit tests
+      // that are meant to explicitly cover this path also when running on on non-Nightly channels.
+      if (this._shouldMigrateFromOldKVStorePath) {
+        // Try to import data from the old kvstore path (ProfD/extensions-store).
+        await this.maybeImportFromOldKVStorePath();
+        if (!(await this._store.has(VERSION_KEY))) {
+          // There was no data in the old kvstore path, migrate any data
+          // available from the LegacyPermissionStore JSONFile if any.
+          await this.maybeMigrateDataFromOldJSONFile();
+        }
+      } else {
+        // On non-Nightly channels, where LegacyPermissionStore was still the
+        // only backend ever enabled, try to import permissions data from the
+        // legacy JSONFile, if any data is available there.
+        await this.maybeMigrateDataFromOldJSONFile();
+      }
     }
   }
 
@@ -170,11 +206,11 @@ class PermissionStore {
     return data;
   }
 
-  async maybeMigrateData() {
+  async maybeMigrateDataFromOldJSONFile() {
     let migrationWasSuccessful = false;
     let oldStore = PathUtils.join(
       Services.dirsvc.get("ProfD", Ci.nsIFile).path,
-      FILE_NAME
+      OLD_JSON_FILENAME
     );
     try {
       await this.migrateFrom(oldStore);
@@ -192,6 +228,42 @@ class PermissionStore {
     }
   }
 
+  async maybeImportFromOldKVStorePath() {
+    try {
+      const oldStorePath = lazy.FileUtils.getDir("ProfD", [OLD_RKV_DIRNAME])
+        .path;
+      if (!(await IOUtils.exists(oldStorePath))) {
+        return;
+      }
+      const oldStore = await lazy.KeyValueService.getOrCreate(
+        oldStorePath,
+        "permissions"
+      );
+      const enumerator = await oldStore.enumerate();
+      const kvpairs = [];
+      while (enumerator.hasMoreElements()) {
+        const { key, value } = enumerator.getNext();
+        kvpairs.push([key, value]);
+      }
+
+      // NOTE: we don't add a VERSION_KEY entry explicitly here because
+      // if the database was not empty the VERSION_KEY is already set to
+      // 1 and will be copied into the new file as part of the pairs
+      // written below (along with the entries for the actual extensions
+      // permissions).
+      if (kvpairs.length) {
+        await this._store.writeMany(kvpairs);
+      }
+
+      // NOTE: the old rkv store path used to be shared with the
+      // ExtensionScriptingStore, and so we are not removing the old
+      // rkv store dir here (that is going to be left to a separate
+      // migration we will be adding to ExtensionScriptingStore).
+    } catch (err) {
+      Cu.reportError(err);
+    }
+  }
+
   async migrateFrom(oldStore) {
     // Some other migration job might have started and not completed, let's
     // start from scratch
@@ -410,9 +482,17 @@ var ExtensionPermissions = {
   _useLegacyStorageBackend: false,
 
   // This is meant for tests only
-  async _uninit() {
-    await store.uninitForTest();
-    store = createStore(!this._useLegacyStorageBackend);
+  async _uninit({ recreateStore = true } = {}) {
+    await store?.uninitForTest();
+    store = null;
+    if (recreateStore) {
+      store = createStore(!this._useLegacyStorageBackend);
+    }
+  },
+
+  // This is meant for tests only
+  _getStore() {
+    return store;
   },
 
   // Convenience listener members for all permission changes.

toolkit/components/extensions/test/xpcshell/test_extension_permissions_migrate_kvstore_path.js

@@ -0,0 +1,232 @@
+"use strict";
+
+const {
+  ExtensionPermissions,
+  OLD_RKV_DIRNAME,
+  RKV_DIRNAME,
+  VERSION_KEY,
+  VERSION_VALUE,
+} = ChromeUtils.import("resource://gre/modules/ExtensionPermissions.jsm");
+const { FileTestUtils } = ChromeUtils.importESModule(
+  "resource://testing-common/FileTestUtils.sys.mjs"
+);
+const { FileUtils } = ChromeUtils.importESModule(
+  "resource://gre/modules/FileUtils.sys.mjs"
+);
+const { KeyValueService } = ChromeUtils.import(
+  "resource://gre/modules/kvstore.jsm"
+);
+
+add_setup(async () => {
+  // Bug 1646182: Force ExtensionPermissions to run in rkv mode, because this
+  // test does not make sense with the legacy method (which will be removed in
+  // the above bug).
+  ExtensionPermissions._useLegacyStorageBackend = false;
+  await ExtensionPermissions._uninit();
+});
+
+// NOTE: this test lives in its own test file to make sure it is isolated
+// from other tests that would be creating the kvstore instance and
+// would prevent this test to properly simulate the kvstore path migration.
+add_task(async function test_migrate_to_separate_kvstore_store_path() {
+  const ADDON_ID_01 = "test-addon-01@test-extension";
+  const ADDON_ID_02 = "test-addon-02@test-extension";
+  // This third test extension is only used as the one that should
+  // have some content scripts stored in ExtensionScriptingStore.
+  const ADDON_ID_03 = "test-addon-03@test-extension";
+
+  const oldStorePath = FileUtils.getDir("ProfD", [OLD_RKV_DIRNAME]).path;
+  const newStorePath = FileUtils.getDir("ProfD", [RKV_DIRNAME]).path;
+
+  // Verify that we are going to be using the expected backend, and that
+  // the rkv path migration is only enabled by default in Nightly builds.
+  info("Verify test environment match the expected pre-conditions");
+
+  const permsStore = ExtensionPermissions._getStore();
+  equal(
+    permsStore.constructor.name,
+    "PermissionStore",
+    "active ExtensionPermissions store should be an instance of PermissionStore"
+  );
+
+  equal(
+    permsStore._shouldMigrateFromOldKVStorePath,
+    AppConstants.NIGHTLY_BUILD,
+    "ExtensionPermissions rkv migration expected to be enabled by default only in Nightly"
+  );
+
+  info(
+    "Uninitialize ExtensionPermissions and make sure no existing kvstore dir"
+  );
+  await ExtensionPermissions._uninit({ recreateStore: false });
+  equal(
+    ExtensionPermissions._getStore(),
+    null,
+    "PermissionStore has been nullified"
+  );
+  await IOUtils.remove(oldStorePath, { ignoreAbsent: true, recursive: true });
+  await IOUtils.remove(newStorePath, { ignoreAbsent: true, recursive: true });
+
+  info("Create an existing kvstore dir on the old path");
+
+  // Populated the kvstore with some expected permissions.
+  const expectedPermsAddon01 = {
+    permissions: ["tabs"],
+    origins: ["http://*/*"],
+  };
+  const expectedPermsAddon02 = {
+    permissions: ["proxy"],
+    origins: ["https://*/*"],
+  };
+
+  const expectedScriptAddon01 = {
+    id: "script-addon-01",
+    allFrames: false,
+    matches: ["<all_urls>"],
+    js: ["/test-script-addon-01.js"],
+    persistAcrossSessions: true,
+    runAt: "document_end",
+  };
+
+  const expectedScriptAddon02 = {
+    id: "script-addon-02",
+    allFrames: false,
+    matches: ["<all_urls"],
+    css: ["/test-script-addon-02.css"],
+    persistAcrossSessions: true,
+    runAt: "document_start",
+  };
+
+  {
+    // Make sure the folder exists
+    await IOUtils.makeDirectory(oldStorePath, { ignoreExisting: true });
+    // Create a permission kvstore dir on the old file path.
+    const kvstore = await KeyValueService.getOrCreate(
+      oldStorePath,
+      "permissions"
+    );
+    await kvstore.writeMany([
+      ["_version", 1],
+      [`id-${ADDON_ID_01}`, JSON.stringify(expectedPermsAddon01)],
+      [`id-${ADDON_ID_02}`, JSON.stringify(expectedPermsAddon02)],
+    ]);
+  }
+
+  {
+    // Add also scripting kvstore data into the same temp dir path.
+    const kvstore = await KeyValueService.getOrCreate(
+      oldStorePath,
+      "scripting-contentScripts"
+    );
+    await kvstore.writeMany([
+      [
+        `${ADDON_ID_03}/${expectedScriptAddon01.id}`,
+        JSON.stringify(expectedScriptAddon01),
+      ],
+      [
+        `${ADDON_ID_03}/${expectedScriptAddon02.id}`,
+        JSON.stringify(expectedScriptAddon02),
+      ],
+    ]);
+  }
+
+  ok(
+    await IOUtils.exists(oldStorePath),
+    "Found kvstore dir for the old store path"
+  );
+  ok(
+    !(await IOUtils.exists(newStorePath)),
+    "Expect kvstore dir for the new store path to don't exist yet"
+  );
+
+  info("Re-initialize the ExtensionPermission store and assert migrated data");
+  await ExtensionPermissions._uninit({ recreateStore: true });
+
+  // Explicitly enable migration (needed to make sure we hit the migration code
+  // that is only enabled by default on Nightly).
+  if (!AppConstants.NIGHTLY_BUILD) {
+    info("Enable ExtensionPermissions rkv migration on non-nightly channel");
+    const newStoreInstance = ExtensionPermissions._getStore();
+    newStoreInstance._shouldMigrateFromOldKVStorePath = true;
+  }
+
+  const permsAddon01 = await ExtensionPermissions._get(ADDON_ID_01);
+  const permsAddon02 = await ExtensionPermissions._get(ADDON_ID_02);
+
+  Assert.deepEqual(
+    { permsAddon01, permsAddon02 },
+    {
+      permsAddon01: expectedPermsAddon01,
+      permsAddon02: expectedPermsAddon02,
+    },
+    "Got the expected permissions migrated to the new store file path"
+  );
+
+  await ExtensionPermissions._uninit({ recreateStore: false });
+
+  ok(
+    await IOUtils.exists(newStorePath),
+    "Found kvstore dir for the new store path"
+  );
+
+  {
+    const newKVStore = await KeyValueService.getOrCreate(
+      newStorePath,
+      "permissions"
+    );
+    Assert.equal(
+      await newKVStore.get(VERSION_KEY),
+      VERSION_VALUE,
+      "Got the expected value set on the kvstore _version key"
+    );
+  }
+
+  // kvstore internally caching behavior doesn't make it easy to make sure
+  // we would be hitting a failure if the ExtensionPermissions kvstore migration
+  // would be mistakenly removing the old kvstore dir as part of that migration,
+  // and so the test case is explicitly verifying that the directory does still
+  // exist and then it copies it into a new path to confirm that the expected
+  // data have been kept in the old kvstore dir.
+  ok(
+    await IOUtils.exists(oldStorePath),
+    "Found kvstore dir for the old store path"
+  );
+  const oldStoreCopiedPath = FileTestUtils.getTempFile("kvstore-dir").path;
+  await IOUtils.copy(oldStorePath, oldStoreCopiedPath, { recursive: true });
+
+  // Confirm that the content scripts have not been copied into
+  // the new kvstore path.
+  async function assertStoredContentScripts(storePath, expectedKeys) {
+    const kvstore = await KeyValueService.getOrCreate(
+      storePath,
+      "scripting-contentScripts"
+    );
+    const enumerator = await kvstore.enumerate();
+    const keys = [];
+    while (enumerator.hasMoreElements()) {
+      keys.push(enumerator.getNext().key);
+    }
+    Assert.deepEqual(
+      keys,
+      expectedKeys,
+      `Got the expected scripts in the kvstore path ${storePath}`
+    );
+  }
+
+  info(
+    "Verify that no content scripts are stored in the new kvstore dir reserved for permissions"
+  );
+  await assertStoredContentScripts(newStorePath, []);
+  info(
+    "Verify that existing content scripts have been not been removed old kvstore dir"
+  );
+  await assertStoredContentScripts(oldStoreCopiedPath, [
+    `${ADDON_ID_03}/${expectedScriptAddon01.id}`,
+    `${ADDON_ID_03}/${expectedScriptAddon02.id}`,
+  ]);
+
+  await ExtensionPermissions._uninit({ recreateStore: true });
+
+  await IOUtils.remove(newStorePath, { recursive: true });
+  await IOUtils.remove(oldStorePath, { recursive: true });
+});

toolkit/components/extensions/test/xpcshell/test_extension_permissions_migration.js

@@ -1,16 +1,11 @@
 "use strict";
 
-const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
-const { ExtensionPermissions } = ChromeUtils.import(
-  "resource://gre/modules/ExtensionPermissions.jsm"
-);
-
-add_task(async function setup() {
-  // Bug 1646182: Force ExtensionPermissions to run in rkv mode, because this
-  // test does not make sense with the legacy method (which will be removed in
-  // the above bug).
-  await ExtensionPermissions._uninit();
-});
+const {
+  ExtensionPermissions,
+  OLD_JSON_FILENAME,
+  OLD_RKV_DIRNAME,
+  RKV_DIRNAME,
+} = ChromeUtils.import("resource://gre/modules/ExtensionPermissions.jsm");
 
 const GOOD_JSON_FILE = {
   "wikipedia@search.mozilla.org": {
@@ -33,16 +28,15 @@ const BAD_JSON_FILE = {
 
 const BAD_FILE = "what is this { } {";
 
-const gOldSettingsJSON = do_get_profile().clone();
-gOldSettingsJSON.append("extension-preferences.json");
+const gOldJSONPath = FileUtils.getDir("ProfD", [OLD_JSON_FILENAME]).path;
+const gOldRkvPath = FileUtils.getDir("ProfD", [OLD_RKV_DIRNAME]).path;
+const gNewRkvPath = FileUtils.getDir("ProfD", [RKV_DIRNAME]).path;
 
 async function test_file(json, extensionIds, expected, fileDeleted) {
   await ExtensionPermissions._resetVersion();
   await ExtensionPermissions._uninit();
 
-  await OS.File.writeAtomic(gOldSettingsJSON.path, json, {
-    encoding: "utf-8",
-  });
+  await IOUtils.writeUTF8(gOldJSONPath, json);
 
   for (let extensionId of extensionIds) {
     let permissions = await ExtensionPermissions.get(extensionId);
@@ -50,12 +44,29 @@ async function test_file(json, extensionIds, expected, fileDeleted) {
   }
 
   Assert.equal(
-    await OS.File.exists(gOldSettingsJSON.path),
+    await IOUtils.exists(gOldJSONPath),
     !fileDeleted,
     "old file was deleted"
   );
+
+  Assert.ok(
+    await IOUtils.exists(gNewRkvPath),
+    "found the store at the new rkv path"
+  );
+
+  Assert.ok(
+    !(await IOUtils.exists(gOldRkvPath)),
+    "expect old rkv path to not exist"
+  );
 }
 
+add_setup(async () => {
+  // Bug 1646182: Force ExtensionPermissions to run in rkv mode, because this
+  // test does not make sense with the legacy method (which will be removed in
+  // the above bug).
+  await ExtensionPermissions._uninit();
+});
+
 add_task(async function test_migrate_good_json() {
   let expected = {
     permissions: ["internal:privateBrowsingAllowed"],
@@ -83,7 +94,7 @@ add_task(async function test_migrate_bad_json() {
     expected,
     /* fileDeleted */ false
   );
-  await OS.File.remove(gOldSettingsJSON.path);
+  await IOUtils.remove(gOldJSONPath);
 });
 
 add_task(async function test_migrate_bad_file() {
@@ -95,5 +106,5 @@ add_task(async function test_migrate_bad_file() {
     expected,
     /* fileDeleted */ false
   );
-  await OS.File.remove(gOldSettingsJSON.path);
+  await IOUtils.remove(gOldJSONPath);
 });

toolkit/components/extensions/test/xpcshell/xpcshell.ini

@@ -85,6 +85,9 @@ run-sequentially = very high failure rate in parallel
 [test_ext_unknown_permissions.js]
 [test_ext_webRequest_urlclassification.js]
 [test_extension_permissions_migration.js]
+skip-if =
+  condprof  # Bug 1769184 - by design for now
+[test_extension_permissions_migrate_kvstore_path.js]
 skip-if =
   condprof  # Bug 1769184 - by design for now
 [test_load_all_api_modules.js]