Restrict use of two app-launching command line flags

chrome/browser/extensions/unpacked_installer.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/unpacked_installer.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/files/file_util.h"
 #include "base/strings/string_util.h"
@@ skipping 22 matching lines @@
 
 using content::BrowserThread;
 using extensions::Extension;
 using extensions::SharedModuleInfo;
 
 namespace {
 
 const char kUnpackedExtensionsBlacklistedError[] =
     "Loading of unpacked extensions is disabled by the administrator.";
 
+const char kUnpackedExtensionInsteadOfAppError[] =
+    "App loading flags cannot be used to load extensions. Please use "
+    "--load-extension instead.";
+
 const char kImportMinVersionNewer[] =
     "'import' version requested is newer than what is installed.";
 const char kImportMissing[] = "'import' extension is not installed.";
 const char kImportNotSharedModule[] = "'import' is not a shared module.";
 
 // Manages an ExtensionInstallPrompt for a particular extension.
 class SimpleExtensionLoadPrompt {
  public:
   SimpleExtensionLoadPrompt(const Extension* extension,
                             Profile* profile,
@@ skipping 70 matching lines @@
 void UnpackedInstaller::Load(const base::FilePath& path_in) {
   DCHECK(extension_path_.empty());
   extension_path_ = path_in;
   BrowserThread::PostTask(
       BrowserThread::FILE,
       FROM_HERE,
       base::Bind(&UnpackedInstaller::GetAbsolutePath, this));
 }
 
 bool UnpackedInstaller::LoadFromCommandLine(const base::FilePath& path_in,
-                                            std::string* extension_id) {
+                                            std::string* extension_id,
+                                            bool only_allow_apps) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(extension_path_.empty());
 
   if (!service_weak_.get())
     return false;
   // Load extensions from the command line synchronously to avoid a race
   // between extension loading and loading an URL from the command line.
   base::ThreadRestrictions::ScopedAllowIO allow_io;
 
   extension_path_ = base::MakeAbsoluteFilePath(path_in);
 
   if (!IsLoadingUnpackedAllowed()) {
     ReportExtensionLoadError(kUnpackedExtensionsBlacklistedError);
     return false;
   }
 
   std::string error;
   install_checker_.set_extension(
       file_util::LoadExtension(
           extension_path_, Manifest::COMMAND_LINE, GetFlags(), &error).get());
 
+  if (only_allow_apps && !extension()->is_app()) {

[Devlin, 2016/06/29 14:59:54]

If we don't opt for the std::set<Type> allowed_types approach, we should
restrict this to is_platform_app(), rather than is_app().  We don't want to
allow loading of legacy packaged apps here.

Otherwise, this can just be allowed_types.count(extension()->type()).

[proberge, 2016/06/29 20:56:49]

Done.

+#if defined(GOOGLE_CHROME_BUILD)
+    // Avoid crashing for users with hijacked shortcuts.

[Devlin, 2016/06/29 14:59:54]

I can understand the motivation to avoid crashing, but if this fails, we just
pop up a dialog saying the extension can't be loaded.  Since that doesn't brick
the user's chrome, it seems like it would also serve as an indicator that
something was wrong, and allow them to fix it.  If a user has one bad
commandline flag, it makes sense they might have more issues, so I'd rather we
let them know so they can take action to get back into a good state, rather than
just try to continue gracefully and give no indication.

[proberge, 2016/06/29 20:56:49]

The comment is to explain the "return true;". If we "return false", Chrome stops
trying to load (and actually crashes during shutdown), effectively bricking the
user's Chrome unless they know how to edit command line flags. 

I'm open to having ReportExtensionLoadError in both cases, but I'm worried that
for most users this will just be an annoying popup on every Chrome launch that
they don't know how to (and will never) fix. 

+    return true;
+#else
+    ReportExtensionLoadError(kUnpackedExtensionInsteadOfAppError);
+    return false;
+#endif
+  }
+
   if (!extension() ||
       !extension_l10n_util::ValidateExtensionLocales(
           extension_path_, extension()->manifest()->value(), &error)) {
     ReportExtensionLoadError(error);
     return false;
   }
 
   extension()->permissions_data()->BindToCurrentThread();
   PermissionsUpdater(
       service_weak_->profile(), PermissionsUpdater::INIT_FLAG_TRANSIENT)
@@ skipping 196 matching lines @@
   service_weak_->OnExtensionInstalled(
       extension(), syncer::StringOrdinal(), kInstallFlagInstallImmediately);
 
   if (!callback_.is_null()) {
     callback_.Run(extension(), extension_path_, std::string());
     callback_.Reset();
   }
 }
 
 }  // namespace extensions