Bug 1910968 - Make sure unverified search engines stay unverified during backup recovery. r=Standard8,backup-reviewers

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

browser/components/backup/resources/PreferencesBackupResource.sys.mjs

@@ -52,10 +52,14 @@ export class PreferencesBackupResource extends BackupResource {
     let prefsDestFile = await IOUtils.getFile(prefsDestPath);
     await Services.prefs.backupPrefFile(prefsDestFile);
 
-    return null;
+    // During recovery, we need to recompute verification hashes for any
+    // custom engines, but only for engines that were originally passing
+    // verification. We'll store the profile path at backup time in our
+    // ManifestEntry so that we can do that verification check at recover-time.
+    return { profilePath };
   }
 
-  async recover(_manifestEntry, recoveryPath, destProfilePath) {
+  async recover(manifestEntry, recoveryPath, destProfilePath) {
     const SEARCH_PREF_FILENAME = "search.json.mozlz4";
     const RECOVERY_SEARCH_PREF_PATH = PathUtils.join(
       recoveryPath,
@@ -69,28 +73,58 @@ export class PreferencesBackupResource extends BackupResource {
         decompress: true,
       });
 
-      searchPrefs.engines = searchPrefs.engines.map(engine => {
-        if (engine._metaData.loadPathHash) {
-          let loadPath = engine._loadPath;
-          engine._metaData.loadPathHash = lazy.SearchUtils.getVerificationHash(
-            loadPath,
-            destProfilePath
-          );
+      // ... but we only want to do this for engines that had valid verification
+      // hashes for the original profile path.
+      const ORIGINAL_PROFILE_PATH = manifestEntry.profilePath;
+
+      if (ORIGINAL_PROFILE_PATH) {
+        searchPrefs.engines = searchPrefs.engines.map(engine => {
+          if (engine._metaData.loadPathHash) {
+            let loadPath = engine._loadPath;
+            if (
+              engine._metaData.loadPathHash ==
+              lazy.SearchUtils.getVerificationHash(
+                loadPath,
+                ORIGINAL_PROFILE_PATH
+              )
+            ) {
+              engine._metaData.loadPathHash =
+                lazy.SearchUtils.getVerificationHash(loadPath, destProfilePath);
+            }
+          }
+          return engine;
+        });
+
+        if (
+          searchPrefs.metaData.defaultEngineIdHash &&
+          searchPrefs.metaData.defaultEngineIdHash ==
+            lazy.SearchUtils.getVerificationHash(
+              searchPrefs.metaData.defaultEngineId,
+              ORIGINAL_PROFILE_PATH
+            )
+        ) {
+          searchPrefs.metaData.defaultEngineIdHash =
+            lazy.SearchUtils.getVerificationHash(
+              searchPrefs.metaData.defaultEngineId,
+              destProfilePath
+            );
         }
-        return engine;
-      });
 
-      searchPrefs.metaData.defaultEngineIdHash =
-        lazy.SearchUtils.getVerificationHash(
-          searchPrefs.metaData.defaultEngineId,
-          destProfilePath
-        );
-
-      searchPrefs.metaData.privateDefaultEngineIdHash =
-        lazy.SearchUtils.getVerificationHash(
-          searchPrefs.metaData.privateDefaultEngineId,
-          destProfilePath
-        );
+        if (
+          searchPrefs.metaData.privateDefaultEngineIdHash &&
+          searchPrefs.metaData.privateDefaultEngineIdHash ==
+            lazy.SearchUtils.getVerificationHash(
+              searchPrefs.metaData.privateDefaultEngineId,
+              ORIGINAL_PROFILE_PATH
+            )
+        ) {
+          searchPrefs.metaData.privateDefaultEngineIdHash =
+            lazy.SearchUtils.getVerificationHash(
+              searchPrefs.metaData.privateDefaultEngineId,
+              destProfilePath
+            );
+        }
+      }
 
       await IOUtils.writeJSON(
         PathUtils.join(destProfilePath, SEARCH_PREF_FILENAME),

browser/components/backup/tests/xpcshell/test_PreferencesBackupResource.js

@@ -111,10 +111,11 @@ add_task(async function test_backup() {
     stagingPath,
     sourcePath
   );
-  Assert.equal(
+  Assert.deepEqual(
     manifestEntry,
-    null,
-    "PreferencesBackupResource.backup should return null as its ManifestEntry"
+    { profilePath: sourcePath },
+    "PreferencesBackupResource.backup should return the original profile path " +
+      "in its ManifestEntry"
   );
 
   await assertFilesExist(stagingPath, simpleCopyFiles);
@@ -184,21 +185,25 @@ add_task(async function test_recover() {
   // ensure that PreferencesBackupResource knows how to update the
   // verification hashes for non-default engines.
   const TEST_SEARCH_ENGINE_LOAD_PATH = "some/path/on/disk";
+  const TEST_SEARCH_ENGINE_LOAD_PATH_HASH = "some pre-existing hash";
   const TEST_DEFAULT_ENGINE_ID = "bugle";
+  const TEST_DEFAULT_ENGINE_ID_HASH = "default engine original hash";
   const TEST_PRIVATE_DEFAULT_ENGINE_ID = "goose";
+  const TEST_PRIVATE_DEFAULT_ENGINE_ID_HASH =
+    "private default engine original hash";
 
   let fakeSearchPrefs = {
     metaData: {
       defaultEngineId: TEST_DEFAULT_ENGINE_ID,
-      defaultEngineIdHash: "default engine original hash",
+      defaultEngineIdHash: TEST_DEFAULT_ENGINE_ID_HASH,
       privateDefaultEngineId: TEST_PRIVATE_DEFAULT_ENGINE_ID,
-      privateDefaultEngineIdHash: "private default engine original hash",
+      privateDefaultEngineIdHash: TEST_PRIVATE_DEFAULT_ENGINE_ID_HASH,
     },
     engines: [
       {
         _loadPath: TEST_SEARCH_ENGINE_LOAD_PATH,
         _metaData: {
-          loadPathHash: "some pre-existing hash",
+          loadPathHash: TEST_SEARCH_ENGINE_LOAD_PATH_HASH,
         },
       },
     ],
@@ -214,11 +219,26 @@ add_task(async function test_recover() {
   );
 
   const EXPECTED_HASH = "this is some newly generated hash";
-  sandbox.stub(SearchUtils, "getVerificationHash").returns(EXPECTED_HASH);
+  sandbox
+    .stub(SearchUtils, "getVerificationHash")
+    .onCall(0)
+    .returns(TEST_SEARCH_ENGINE_LOAD_PATH_HASH)
+    .onCall(1)
+    .returns(EXPECTED_HASH)
+    .onCall(2)
+    .returns(TEST_DEFAULT_ENGINE_ID_HASH)
+    .onCall(3)
+    .returns(EXPECTED_HASH)
+    .onCall(4)
+    .returns(TEST_PRIVATE_DEFAULT_ENGINE_ID_HASH)
+    .onCall(5)
+    .returns(EXPECTED_HASH);
+
+  const PRETEND_ORIGINAL_PATH = "some/original/path";
 
   // The backup method is expected to have returned a null ManifestEntry
   let postRecoveryEntry = await preferencesBackupResource.recover(
-    null /* manifestEntry */,
+    { profilePath: PRETEND_ORIGINAL_PATH },
     recoveryPath,
     destProfilePath
   );
@@ -231,34 +251,55 @@ add_task(async function test_recover() {
   await assertFilesExist(destProfilePath, simpleCopyFiles);
 
   // Now ensure that the verification was properly recomputed. We should
-  // Have called getVerificationHash 3 times - once each for:
+  // Have called getVerificationHash 6 times - twice each for:
   //
   // - The single engine in the engines list
   // - The defaultEngineId
   // - The privateDefaultEngineId
+  //
+  // The first call is to verify the hash against the original profile path,
+  // and the second call is to generate the hash for the new profile path.
   Assert.equal(
     SearchUtils.getVerificationHash.callCount,
-    3,
+    6,
     "SearchUtils.getVerificationHash was called the right number of times."
   );
   Assert.ok(
     SearchUtils.getVerificationHash
       .getCall(0)
-      .calledWith(TEST_SEARCH_ENGINE_LOAD_PATH, destProfilePath),
+      .calledWith(TEST_SEARCH_ENGINE_LOAD_PATH, PRETEND_ORIGINAL_PATH),
     "SearchUtils.getVerificationHash first call called with the right arguments."
   );
   Assert.ok(
     SearchUtils.getVerificationHash
       .getCall(1)
-      .calledWith(TEST_DEFAULT_ENGINE_ID, destProfilePath),
+      .calledWith(TEST_SEARCH_ENGINE_LOAD_PATH, destProfilePath),
     "SearchUtils.getVerificationHash second call called with the right arguments."
   );
   Assert.ok(
     SearchUtils.getVerificationHash
       .getCall(2)
-      .calledWith(TEST_PRIVATE_DEFAULT_ENGINE_ID, destProfilePath),
+      .calledWith(TEST_DEFAULT_ENGINE_ID, PRETEND_ORIGINAL_PATH),
     "SearchUtils.getVerificationHash third call called with the right arguments."
   );
+  Assert.ok(
+    SearchUtils.getVerificationHash
+      .getCall(3)
+      .calledWith(TEST_DEFAULT_ENGINE_ID, destProfilePath),
+    "SearchUtils.getVerificationHash fourth call called with the right arguments."
+  );
+  Assert.ok(
+    SearchUtils.getVerificationHash
+      .getCall(4)
+      .calledWith(TEST_PRIVATE_DEFAULT_ENGINE_ID, PRETEND_ORIGINAL_PATH),
+    "SearchUtils.getVerificationHash fifth call called with the right arguments."
+  );
+  Assert.ok(
+    SearchUtils.getVerificationHash
+      .getCall(5)
+      .calledWith(TEST_PRIVATE_DEFAULT_ENGINE_ID, destProfilePath),
+    "SearchUtils.getVerificationHash sixth call called with the right arguments."
+  );
 
   let recoveredSearchPrefs = await IOUtils.readJSON(
     PathUtils.join(destProfilePath, SEARCH_PREFS_FILENAME),

browser/components/backup/tests/xpcshell/test_PreferencesBackupResource_searchEngines.js

@@ -24,12 +24,21 @@ updateAppInfo({ name: "XPCShell", version: "48", platformVersion: "48" });
 do_get_profile();
 
 const FAKE_SEARCH_EXTENSION_NAME = "Some WebExtension Search Engine";
+const FAKE_PRIVATE_SEARCH_EXTENSION_NAME =
+  "Some Private WebExtension Search Engine";
 
 add_setup(async function () {
+  Services.prefs.setBoolPref("browser.search.separatePrivateDefault", true);
+  Services.prefs.setBoolPref(
+    "browser.search.separatePrivateDefault.ui.enabled",
+    true
+  );
+
   await SearchTestUtils.setRemoteSettingsConfig([
     { identifier: "engine1" },
     { identifier: "engine2" },
   ]);
+
   Services.prefs.setCharPref(SearchUtils.BROWSER_SEARCH_PREF + "region", "US");
   Services.locale.availableLocales = ["en-US"];
   Services.locale.requestedLocales = ["en-US"];
@@ -44,6 +53,15 @@ add_setup(async function () {
     { setAsDefault: true }
   );
 
+  await SearchTestUtils.installSearchExtension(
+    {
+      name: FAKE_PRIVATE_SEARCH_EXTENSION_NAME,
+      search_url: "https://example.com/",
+      search_url_get_params: "private={searchTerms}",
+    },
+    { setAsDefaultPrivate: true }
+  );
+
   await SearchTestUtils.promiseSearchNotification(
     "write-settings-to-disk-complete"
   );
@@ -71,7 +89,7 @@ add_task(async function test_recover_searchEngines_verified() {
   );
 
   let postRecoveryEntry = await preferencesBackupResource.recover(
-    null /* manifestEntry */,
+    { profilePath: PathUtils.profileDir },
     recoveryPath,
     destProfilePath
   );
@@ -160,3 +178,110 @@ add_task(async function test_recover_searchEngines_verified() {
   await maybeRemovePath(recoveryPath);
   await maybeRemovePath(destProfilePath);
 });
+
+/**
+ * Tests that PreferencesBackupResource will not update the verification hashes
+ * for any search engines that fail to verify for the original path.
+ */
+add_task(async function test_recover_searchEngines_unverified() {
+  let preferencesBackupResource = new PreferencesBackupResource();
+  let recoveryPath = await IOUtils.createUniqueDirectory(
+    PathUtils.tempDir,
+    "PreferencesBackupResource-recovery-test"
+  );
+  let destProfilePath = await IOUtils.createUniqueDirectory(
+    PathUtils.tempDir,
+    "PreferencesBackupResource-test-profile"
+  );
+
+  // Now let's read in the original search preferences file and break some
+  // of the verification hashes...
+  let searchEngineSettings = await IOUtils.readJSON(
+    PathUtils.join(PathUtils.profileDir, SEARCH_PREFS_FILENAME),
+    { decompress: true }
+  );
+
+  const BOGUS_HASH = "bogus hash!";
+
+  searchEngineSettings.metaData.defaultEngineIdHash = BOGUS_HASH;
+  searchEngineSettings.metaData.privateDefaultEngineIdHash = BOGUS_HASH;
+
+  for (let engine of searchEngineSettings.engines) {
+    if (engine._metaData.loadPathHash) {
+      engine._metaData.loadPathHash = BOGUS_HASH;
+    }
+  }
+
+  // And now let us write this data out to the recovery path.
+  await IOUtils.writeJSON(
+    PathUtils.join(recoveryPath, SEARCH_PREFS_FILENAME),
+    searchEngineSettings,
+    {
+      compress: true,
+    }
+  );
+
+  let postRecoveryEntry = await preferencesBackupResource.recover(
+    { profilePath: PathUtils.profileDir },
+    recoveryPath,
+    destProfilePath
+  );
+
+  Assert.equal(
+    postRecoveryEntry,
+    null,
+    "PreferencesBackupResource.recover should return null as its post recovery entry"
+  );
+
+  // And now let us write this data out to the recovery path.
+  let recoveredSearchEngineSettings = await IOUtils.readJSON(
+    PathUtils.join(destProfilePath, SEARCH_PREFS_FILENAME),
+    {
+      decompress: true,
+    }
+  );
+
+  Assert.equal(
+    recoveredSearchEngineSettings.metaData.defaultEngineIdHash,
+    BOGUS_HASH,
+    "Bogus defaultEngineIdHash was not changed."
+  );
+
+  Assert.equal(
+    recoveredSearchEngineSettings.metaData.privateDefaultEngineIdHash,
+    BOGUS_HASH,
+    "Bogus privateDefaultEngineIdHash was not changed."
+  );
+
+  Assert.equal(
+    searchEngineSettings.engines.length,
+    recoveredSearchEngineSettings.engines.length,
+    "Got the same number of engines"
+  );
+
+  for (let i = 0; i < searchEngineSettings.engines.length; ++i) {
+    let originalEngine = searchEngineSettings.engines[i];
+    let recoveredEngine = recoveredSearchEngineSettings.engines[i];
+
+    if (originalEngine._metaData.loadPathHash) {
+      Assert.ok(
+        recoveredEngine._metaData.loadPathHash,
+        "Recovered engine also has a loadPathHash"
+      );
+      Assert.equal(
+        recoveredEngine._metaData.loadPathHash,
+        BOGUS_HASH,
+        "Bogus loadPathHash was not changed."
+      );
+    } else {
+      Assert.deepEqual(
+        originalEngine,
+        recoveredEngine,
+        "Engine was not changed."
+      );
+    }
+  }
+
+  await maybeRemovePath(recoveryPath);
+  await maybeRemovePath(destProfilePath);
+});