Commit 70cda43b authored by Jeroen Dhollander's avatar Jeroen Dhollander Committed by Commit Bot

Remove possible PII information from the logs

Log with |AssistantDebugging| enabled:
    Assistant: OnTextResponse: [PII](What do cats eat for breakfast?)

Log without that flag:
    Assistant: OnTextResponse: [Redacted PII]

This flag also controls the LibAssistant back-end, as using the
LibAssistant beta back-end might also log some PII.

Bug: b/162894119
Change-Id: Ie888a9b4bf636f1cd0ad6f54df58012b782136c4
Tests: Ran with and without this flag.
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366073
Commit-Queue: Jeroen Dhollander <jeroendh@chromium.org>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800270}
parent 8cf7fd8b
...@@ -6,6 +6,8 @@ ...@@ -6,6 +6,8 @@
#include <utility> #include <utility>
#include "chromeos/services/assistant/public/cpp/features.h"
namespace chromeos { namespace chromeos {
namespace assistant { namespace assistant {
...@@ -17,6 +19,17 @@ std::string ResolutionToString(AssistantInteractionResolution resolution) { ...@@ -17,6 +19,17 @@ std::string ResolutionToString(AssistantInteractionResolution resolution) {
return result.str(); return result.str();
} }
bool IsPIILoggingAllowed() {
return features::IsAssistantDebuggingEnabled();
}
std::string HidePiiMaybe(const std::string& value) {
if (IsPIILoggingAllowed())
return "[PII](" + value + ")";
else
return "[Redacted PII]";
}
#define LOG_INTERACTION() \ #define LOG_INTERACTION() \
LOG_INTERACTION_AT_LEVEL(AssistantInteractionLogger::kVLogLevel) LOG_INTERACTION_AT_LEVEL(AssistantInteractionLogger::kVLogLevel)
...@@ -37,8 +50,8 @@ void AssistantInteractionLogger::OnInteractionStarted( ...@@ -37,8 +50,8 @@ void AssistantInteractionLogger::OnInteractionStarted(
const AssistantInteractionMetadata& metadata) { const AssistantInteractionMetadata& metadata) {
switch (metadata.type) { switch (metadata.type) {
case AssistantInteractionType::kText: case AssistantInteractionType::kText:
LOG_INTERACTION() << "Text interaction with query '" << metadata.query LOG_INTERACTION() << "Text interaction with query "
<< "'"; << HidePiiMaybe(metadata.query);
break; break;
case AssistantInteractionType::kVoice: case AssistantInteractionType::kVoice:
LOG_INTERACTION() << "Voice interaction"; LOG_INTERACTION() << "Voice interaction";
...@@ -57,7 +70,7 @@ void AssistantInteractionLogger::OnHtmlResponse(const std::string& response, ...@@ -57,7 +70,7 @@ void AssistantInteractionLogger::OnHtmlResponse(const std::string& response,
// HTML tags and rather large. // HTML tags and rather large.
LOG_INTERACTION() << "with fallback '" << fallback << "'"; LOG_INTERACTION() << "with fallback '" << fallback << "'";
// Display HTML at highest verbosity. // Display HTML at highest verbosity.
LOG_INTERACTION_AT_LEVEL(3) << "with HTML: " << response; LOG_INTERACTION_AT_LEVEL(3) << "with HTML: " << HidePiiMaybe(response);
} }
void AssistantInteractionLogger::OnSuggestionsResponse( void AssistantInteractionLogger::OnSuggestionsResponse(
...@@ -69,7 +82,7 @@ void AssistantInteractionLogger::OnSuggestionsResponse( ...@@ -69,7 +82,7 @@ void AssistantInteractionLogger::OnSuggestionsResponse(
} }
void AssistantInteractionLogger::OnTextResponse(const std::string& response) { void AssistantInteractionLogger::OnTextResponse(const std::string& response) {
LOG_INTERACTION() << "'" << response << "'"; LOG_INTERACTION() << HidePiiMaybe(response);
} }
void AssistantInteractionLogger::OnOpenUrlResponse(const GURL& url, void AssistantInteractionLogger::OnOpenUrlResponse(const GURL& url,
......
...@@ -23,6 +23,9 @@ const base::Feature kAssistantBetterOnboarding{ ...@@ -23,6 +23,9 @@ const base::Feature kAssistantBetterOnboarding{
const base::Feature kAssistantConversationStartersV2{ const base::Feature kAssistantConversationStartersV2{
"AssistantConversationStartersV2", base::FEATURE_DISABLED_BY_DEFAULT}; "AssistantConversationStartersV2", base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kAssistantDebugging{"AssistantDebugging",
base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kAssistantLauncherChipIntegration{ const base::Feature kAssistantLauncherChipIntegration{
"AssistantLauncherChipIntegration", base::FEATURE_DISABLED_BY_DEFAULT}; "AssistantLauncherChipIntegration", base::FEATURE_DISABLED_BY_DEFAULT};
...@@ -93,6 +96,10 @@ bool IsConversationStartersV2Enabled() { ...@@ -93,6 +96,10 @@ bool IsConversationStartersV2Enabled() {
return base::FeatureList::IsEnabled(kAssistantConversationStartersV2); return base::FeatureList::IsEnabled(kAssistantConversationStartersV2);
} }
bool IsAssistantDebuggingEnabled() {
return base::FeatureList::IsEnabled(kAssistantDebugging);
}
bool IsDspHotwordEnabled() { bool IsDspHotwordEnabled() {
return base::FeatureList::IsEnabled(kEnableDspHotword); return base::FeatureList::IsEnabled(kEnableDspHotword);
} }
......
...@@ -100,6 +100,9 @@ COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsBloomEnabled(); ...@@ -100,6 +100,9 @@ COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsBloomEnabled();
COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
bool IsConversationStartersV2Enabled(); bool IsConversationStartersV2Enabled();
COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
bool IsAssistantDebuggingEnabled();
COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsDspHotwordEnabled(); COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsDspHotwordEnabled();
COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
......
...@@ -147,8 +147,10 @@ std::string CreateLibAssistantConfig( ...@@ -147,8 +147,10 @@ std::string CreateLibAssistantConfig(
GetBaseAssistantDir().AsUTF8Unsafe()); GetBaseAssistantDir().AsUTF8Unsafe());
} }
if (features::IsLibAssistantBetaBackendEnabled()) if (features::IsLibAssistantBetaBackendEnabled() ||
features::IsAssistantDebuggingEnabled()) {
config.SetStringPath("internal.backend_type", "BETA_DOGFOOD"); config.SetStringPath("internal.backend_type", "BETA_DOGFOOD");
}
// Use http unless we're using the fake s3 server, which requires grpc. // Use http unless we're using the fake s3 server, which requires grpc.
if (s3_server_uri_override) if (s3_server_uri_override)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment