Have the Debugger extension api check that it has access to the tab

chrome/browser/extensions/api/debugger/debugger_api.cc

 // Copyright (c) 2012 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.
 
 // Implements the Chrome Extensions Debugger API.
 
 #include "chrome/browser/extensions/api/debugger/debugger_api.h"
 
 #include <map>
 #include <set>
 
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
 #include "base/scoped_observer.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/devtools/devtools_target_impl.h"
 #include "chrome/browser/extensions/api/debugger/debugger_api_constants.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/webui/chrome_web_ui_controller_factory.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/infobars/core/confirm_infobar_delegate.h"
 #include "components/infobars/core/infobar.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/devtools_client_host.h"
 #include "content/public/browser/devtools_http_handler.h"
 #include "content/public/browser/devtools_manager.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/url_utils.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_host.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_registry_observer.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/permissions/permissions_data.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 using content::DevToolsAgentHost;
 using content::DevToolsClientHost;
 using content::DevToolsHttpHandler;
 using content::DevToolsManager;
 using content::RenderProcessHost;
 using content::RenderViewHost;
 using content::RenderWidgetHost;
@@ skipping 519 matching lines @@
         keys::kProtocolVersionNotSupportedError,
         params->required_version);
     return false;
   }
 
   if (agent_host_->IsAttached()) {
     FormatErrorMessage(keys::kAlreadyAttachedError);
     return false;
   }
 
+  const Extension* extension = GetExtension();
+
   infobars::InfoBar* infobar = NULL;
   if (!CommandLine::ForCurrentProcess()->
        HasSwitch(switches::kSilentDebuggerExtensionAPI)) {
     // Do not attach to the target if for any reason the infobar cannot be shown
     // for this WebContents instance.
     infobar = ExtensionDevToolsInfoBarDelegate::Create(
-        agent_host_->GetRenderViewHost(), GetExtension()->name());
+        agent_host_->GetRenderViewHost(), extension->name());

[not at google - send to devlin, 2014/06/23 19:34:19]

what is the lifetime/behaviour of constructing the infobar here and then later
(with your permission check) not using it?

for safety's sake we might as well do the permission check you're adding before
attempting this.

[Devlin, 2014/06/24 17:49:29]

Done.

     if (!infobar) {
       error_ = ErrorUtils::FormatErrorMessage(
           keys::kSilentDebuggingRequired,
           switches::kSilentDebuggerExtensionAPI);
       return false;
     }
   }
 
+  content::WebContents* web_contents =
+      content::WebContents::FromRenderViewHost(
+          agent_host_->GetRenderViewHost());
+  if (!web_contents) {
+    FormatErrorMessage(keys::kNoTargetError);
+    return false;
+  }
+
+  if (!extension->permissions_data()->CanAccessPage(
+          extension,

[not at google - send to devlin, 2014/06/23 19:34:19]

tracing this down, it looks like this is backwards incompatible; extensions
didn't previously need to declare host permissions, now they do.

could you add a special-case for the debugger permission to
PermissionData::CanRunOnPage? we *should* be able to back that change out in
manifest v3 since I can't see anything that allows the API to break site
isolation (e.g. webRequest can break site isolation) but cross that bridge.

[Devlin, 2014/06/24 17:49:29]

I don't really like the idea of giving any extension with the debugger
permission access to all pages, even if we already warned the user about it. 
With the new patch, though, we don't need to.

+          web_contents->GetLastCommittedURL(),
+          web_contents->GetLastCommittedURL(),
+          SessionID::IdForTab(web_contents),
+          -1,  // igore process id.

[meacer, 2014/06/23 19:29:07]

igore -> ignore

[Devlin, 2014/06/24 17:49:29]

Done.

+          &error_)) {
+    return false;

[meacer, 2014/06/23 19:29:07]

You can print an error message here too.

[Devlin, 2014/06/24 17:49:29]

Moot now (but |error_| would have been populated via CanAccessPage()).

+  }
+

[Devlin, 2014/06/23 19:17:47]

Ben, I think you were mentioning that we should also do a check here for where
access to chrome urls could count for chrome-extension urls.  Do you still want
that?

[not at google - send to devlin, 2014/06/23 19:34:19]

I think we should just do that as part of the low-level code which checks the
effect of that flag, but it's a mostly unrelated change (just happens to be
relevant after this change).

   new ExtensionDevToolsClientHost(GetProfile(),
                                   agent_host_.get(),
-                                  GetExtension()->id(),
-                                  GetExtension()->name(),
+                                  extension->id(),
+                                  extension->name(),
                                   debuggee_,
                                   infobar);
   SendResponse(true);
   return true;
 }
 
 
 // DebuggerDetachFunction -----------------------------------------------------
 
 DebuggerDetachFunction::DebuggerDetachFunction() {
@@ skipping 117 matching lines @@
     const std::vector<DevToolsTargetImpl*>& target_list) {
   scoped_ptr<base::ListValue> result(new base::ListValue());
   for (size_t i = 0; i < target_list.size(); ++i)
     result->Append(SerializeTarget(*target_list[i]));
   STLDeleteContainerPointers(target_list.begin(), target_list.end());
   SetResult(result.release());
   SendResponse(true);
 }
 
 }  // namespace extensions