From 621d5920c17065eb52f3f1e83f7e968908fe0e67 Mon Sep 17 00:00:00 2001 From: Lars Ershammar Date: Sun, 16 Aug 2026 13:03:21 +0000 Subject: [PATCH 1/7] Fetch android_efi_loader builds requested through `cvd load` --- .../host/commands/cvd/cli/parser/BUILD.bazel | 14 +++ .../cvd/cli/parser/fetch_config_parser.cpp | 4 +- .../cli/parser/fetch_config_parser_test.cc | 113 ++++++++++++++++++ 3 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 base/cvd/cuttlefish/host/commands/cvd/cli/parser/fetch_config_parser_test.cc diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/BUILD.bazel b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/BUILD.bazel index e1726b8a972..bfd427a02b2 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/BUILD.bazel +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/BUILD.bazel @@ -80,6 +80,20 @@ cf_cc_library( ], ) +cf_cc_test( + name = "fetch_config_parser_test", + srcs = ["fetch_config_parser_test.cc"], + deps = [ + "//cuttlefish/host/commands/cvd/cli/parser:cf_flags_validator", + "//cuttlefish/host/commands/cvd/cli/parser:fetch_config_parser", + "//cuttlefish/host/commands/cvd/cli/parser:load_config_cc_proto", + "//cuttlefish/host/commands/cvd/cli/parser:test_common", + "//cuttlefish/result", + "//cuttlefish/result:result_matchers", + "@jsoncpp", + ], +) + cf_cc_test( name = "flags_parser_test", srcs = ["flags_parser_test.cc"], diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/fetch_config_parser.cpp b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/fetch_config_parser.cpp index d844380f370..5ad0bec4967 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/fetch_config_parser.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/fetch_config_parser.cpp @@ -44,8 +44,8 @@ bool ShouldFetch(const Instance& instance) { for (const auto& value : {disk.default_build(), disk.super_partition().system(), - boot.kernel().build(), boot.kernel().build(), boot.build(), - boot.bootloader().build(), disk.otatools()}) { + boot.kernel().build(), boot.build(), boot.bootloader().build(), + boot.android_efi_loader().build(), disk.otatools()}) { // expects non-prefixed build strings already converted to empty strings if (!value.empty()) { return true; diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/fetch_config_parser_test.cc b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/fetch_config_parser_test.cc new file mode 100644 index 00000000000..84db0afbbac --- /dev/null +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/fetch_config_parser_test.cc @@ -0,0 +1,113 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "cuttlefish/host/commands/cvd/cli/parser/fetch_config_parser.h" + +#include +#include + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "json/value.h" + +#include "cuttlefish/host/commands/cvd/cli/parser/cf_flags_validator.h" +#include "cuttlefish/host/commands/cvd/cli/parser/load_config.pb.h" +#include "cuttlefish/host/commands/cvd/cli/parser/test_common.h" +#include "cuttlefish/result/result.h" +#include "cuttlefish/result/result_matchers.h" + +namespace cuttlefish { +namespace { + +Result> FetchCvdParserTester(const Json::Value& root) { + const cvd::config::EnvironmentSpecification config = + CF_EXPECT(ValidateCfConfigs(root), "Json validation failed"); + return ParseFetchCvdConfigs(config, "/tmp/fetch_test", {"0"}); +} + +} // namespace + +TEST(FetchConfigParserTests, AndroidEfiLoaderBuildOnlySuccess) { + const char* test_string = R""""( +{ + "instances": [ + { + "boot": { + "android_efi_loader": { + "build": "@ab/branch/target" + } + } + } + ] +} + )""""; + + Json::Value json_configs; + std::string json_text(test_string); + ASSERT_TRUE(ParseJsonString(json_text, json_configs)); + + Result> flags = FetchCvdParserTester(json_configs); + ASSERT_THAT(flags, IsOk()); + EXPECT_TRUE(FindConfig(*flags, "--android_efi_loader_build=branch/target")); +} + +TEST(FetchConfigParserTests, KernelBuildOnlySuccess) { + const char* test_string = R""""( +{ + "instances": [ + { + "boot": { + "kernel": { + "build": "@ab/branch/target" + } + } + } + ] +} + )""""; + + Json::Value json_configs; + std::string json_text(test_string); + ASSERT_TRUE(ParseJsonString(json_text, json_configs)); + + Result> flags = FetchCvdParserTester(json_configs); + ASSERT_THAT(flags, IsOk()); + EXPECT_TRUE(FindConfig(*flags, "--kernel_build=branch/target")); +} + +TEST(FetchConfigParserTests, NoBuildStringsProducesNoFlagsSuccess) { + const char* test_string = R""""( +{ + "instances": [ + { + "vm": { + "memory_mb": 4096 + } + } + ] +} + )""""; + + Json::Value json_configs; + std::string json_text(test_string); + ASSERT_TRUE(ParseJsonString(json_text, json_configs)); + + Result> flags = FetchCvdParserTester(json_configs); + ASSERT_THAT(flags, IsOk()); + EXPECT_TRUE(flags->empty()); +} + +} // namespace cuttlefish From b8b5914d7f61649d51c787a681d8b68fbafb7545 Mon Sep 17 00:00:00 2001 From: Lars Ershammar Date: Sun, 16 Aug 2026 13:05:24 +0000 Subject: [PATCH 2/7] Honor the requested filepath for kernel and bootloader builds --- .../host/commands/cvd/fetch/fetch_cvd.cc | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc index dffeabb6645..f6a1744392f 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc +++ b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc @@ -305,9 +305,11 @@ Result FetchSystemTarget(FetchBuildContext& context, } Result FetchKernelTarget(FetchBuildContext context) { - // If the kernel is from an arm/aarch64 build, the artifact will be called - // Image. - if (!context.Artifact("bzImage").DownloadTo("kernel").has_value()) { + if (std::optional filepath = context.GetFilepath()) { + CF_EXPECT(context.Artifact(*filepath).DownloadTo("kernel")); + } else if (!context.Artifact("bzImage").DownloadTo("kernel").has_value()) { + // If the kernel is from an arm/aarch64 build, the artifact will be called + // Image. CF_EXPECT(context.Artifact("Image").DownloadTo("kernel")); } @@ -337,9 +339,13 @@ Result FetchBootTarget(FetchBuildContext& context, } Result FetchBootloaderTarget(FetchBuildContext& context) { - // If the bootloader is from an arm/aarch64 build, the artifact will be of - // filetype bin. - if (!context.Artifact("u-boot.rom").DownloadTo("bootloader").has_value()) { + if (std::optional filepath = context.GetFilepath()) { + CF_EXPECT(context.Artifact(*filepath).DownloadTo("bootloader")); + } else if (!context.Artifact("u-boot.rom") + .DownloadTo("bootloader") + .has_value()) { + // If the bootloader is from an arm/aarch64 build, the artifact will be of + // filetype bin. CF_EXPECT(context.Artifact("u-boot.bin").DownloadTo("bootloader")); } return {}; From 154b887578d8849dd0f8a0e7fe485521c94140f8 Mon Sep 17 00:00:00 2001 From: Lars Ershammar Date: Sun, 16 Aug 2026 13:06:36 +0000 Subject: [PATCH 3/7] Fetch otatools only when a build explicitly requests it --- .../cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc index f6a1744392f..3d10e15b59a 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc +++ b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc @@ -126,7 +126,7 @@ Result> GetBuildHelper( Result GetBuilds(BuildApi& build_api, const BuildStrings& build_sources) { - Builds result = Builds{ + return Builds{ .default_build = CF_EXPECT(GetBuildHelper( build_api, build_sources.default_build, kDefaultBuildTarget)), .system = CF_EXPECT(GetBuildHelper(build_api, build_sources.system_build, @@ -146,14 +146,6 @@ Result GetBuilds(BuildApi& build_api, build_api, build_sources.test_suites_build, kDefaultBuildTarget)), .chrome_os = build_sources.chrome_os_build, }; - if (!result.otatools) { - if (result.system) { - result.otatools = result.system; - } else if (result.kernel) { - result.otatools = result.default_build; - } - } - return {result}; } Result UpdateTargetsWithBuilds(BuildApi& build_api, From 54dd61e62a4ad7db945e5621ef38f6a179ec1fce Mon Sep 17 00:00:00 2001 From: Lars Ershammar Date: Sun, 16 Aug 2026 13:07:22 +0000 Subject: [PATCH 4/7] Select the CA bundle from CURL_CA_BUNDLE or a known system path --- .../host/libs/web/http_client/BUILD.bazel | 2 + .../libs/web/http_client/curl_http_client.cc | 37 ++++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/base/cvd/cuttlefish/host/libs/web/http_client/BUILD.bazel b/base/cvd/cuttlefish/host/libs/web/http_client/BUILD.bazel index fe8cf878edb..ac5bd5ef0d8 100644 --- a/base/cvd/cuttlefish/host/libs/web/http_client/BUILD.bazel +++ b/base/cvd/cuttlefish/host/libs/web/http_client/BUILD.bazel @@ -18,6 +18,8 @@ cf_cc_library( srcs = ["curl_http_client.cc"], hdrs = ["curl_http_client.h"], deps = [ + "//cuttlefish/common/libs/utils:environment", + "//cuttlefish/files:file_exists", "//cuttlefish/host/libs/web/http_client", "//cuttlefish/host/libs/web/http_client:scrub_secrets", "//cuttlefish/result", diff --git a/base/cvd/cuttlefish/host/libs/web/http_client/curl_http_client.cc b/base/cvd/cuttlefish/host/libs/web/http_client/curl_http_client.cc index 39bb86b1b4f..e8e809af745 100644 --- a/base/cvd/cuttlefish/host/libs/web/http_client/curl_http_client.cc +++ b/base/cvd/cuttlefish/host/libs/web/http_client/curl_http_client.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -31,6 +32,8 @@ #include "curl/easy.h" #include "curl/header.h" +#include "cuttlefish/common/libs/utils/environment.h" +#include "cuttlefish/files/file_exists.h" #include "cuttlefish/host/libs/web/http_client/http_client.h" #include "cuttlefish/host/libs/web/http_client/scrub_secrets.h" #include "cuttlefish/result/result.h" @@ -38,6 +41,34 @@ namespace cuttlefish { namespace { +// The bundled libcurl compiles in the Debian bundle path as its default, so +// hardcoding `CURLOPT_CAINFO` to that same path leaves the trust root tied to +// Debian at both layers. Probing these is what makes other distributions work. +constexpr const char* kCaBundleCandidates[] = { + "/etc/ssl/certs/ca-certificates.crt", // Debian, Ubuntu, Arch, Alpine + "/etc/pki/tls/certs/ca-bundle.crt", // Fedora, RHEL + "/etc/ssl/ca-bundle.pem", // openSUSE + "/etc/ssl/cert.pem", // FreeBSD, macOS +}; + +// The bundle to hand to `CURLOPT_CAINFO`, or nothing to leave libcurl with +// its compiled-in default. `CURL_CA_BUNDLE` is read here because libcurl +// reads no CA environment variable of its own (it is a curl command line +// tool convention), and it comes first so that the trust root stays +// overridable on hosts where a candidate path exists. +std::optional CaBundlePath() { + std::optional from_environment = StringFromEnv("CURL_CA_BUNDLE"); + if (from_environment.has_value() && !from_environment->empty()) { + return from_environment; + } + for (const char* candidate : kCaBundleCandidates) { + if (FileExists(candidate)) { + return std::string(candidate); + } + } + return std::nullopt; +} + std::string TrimWhitespace(const char* data, const size_t size) { std::string_view converted(data, size); return std::string(absl::StripAsciiWhitespace(converted)); @@ -142,8 +173,10 @@ class CurlClient : public HttpClient { default: break; } - curl_easy_setopt(curl_, CURLOPT_CAINFO, - "/etc/ssl/certs/ca-certificates.crt"); + static const std::optional ca_bundle_path = CaBundlePath(); + if (ca_bundle_path.has_value()) { + curl_easy_setopt(curl_, CURLOPT_CAINFO, ca_bundle_path->c_str()); + } curl_easy_setopt(curl_, CURLOPT_HTTPHEADER, curl_headers.get()); curl_easy_setopt(curl_, CURLOPT_URL, request.url.c_str()); curl_easy_setopt(curl_, CURLOPT_WRITEFUNCTION, curl_to_function_cb); From 2435b13ca9f28e36c05db065ce08247d134984b8 Mon Sep 17 00:00:00 2001 From: Lars Ershammar Date: Sun, 16 Aug 2026 13:09:34 +0000 Subject: [PATCH 5/7] Keep URL query strings out of the logs --- base/cvd/cuttlefish/host/libs/web/BUILD.bazel | 1 + .../host/libs/web/android_build_api.cpp | 6 +- .../host/libs/web/http_client/BUILD.bazel | 1 + .../libs/web/http_client/curl_http_client.cc | 4 +- .../host/libs/web/http_client/http_file.cc | 7 +- .../libs/web/http_client/scrub_secrets.cc | 13 ++++ .../host/libs/web/http_client/scrub_secrets.h | 5 ++ .../web/http_client/scrub_secrets_test.cc | 73 +++++++++++++++++++ 8 files changed, 104 insertions(+), 6 deletions(-) diff --git a/base/cvd/cuttlefish/host/libs/web/BUILD.bazel b/base/cvd/cuttlefish/host/libs/web/BUILD.bazel index 998a8a1a1ab..66febc1669f 100644 --- a/base/cvd/cuttlefish/host/libs/web/BUILD.bazel +++ b/base/cvd/cuttlefish/host/libs/web/BUILD.bazel @@ -37,6 +37,7 @@ cf_cc_library( "//cuttlefish/host/libs/web/http_client", "//cuttlefish/host/libs/web/http_client:http_file", "//cuttlefish/host/libs/web/http_client:http_json", + "//cuttlefish/host/libs/web/http_client:scrub_secrets", "//cuttlefish/host/libs/zip:remote_zip", "//cuttlefish/host/libs/zip/libzip_cc:seekable_source", "//cuttlefish/host/libs/zip/libzip_cc:writable_source", diff --git a/base/cvd/cuttlefish/host/libs/web/android_build_api.cpp b/base/cvd/cuttlefish/host/libs/web/android_build_api.cpp index 57b3c16739a..0e00f243411 100644 --- a/base/cvd/cuttlefish/host/libs/web/android_build_api.cpp +++ b/base/cvd/cuttlefish/host/libs/web/android_build_api.cpp @@ -52,6 +52,7 @@ #include "cuttlefish/host/libs/web/http_client/http_client.h" #include "cuttlefish/host/libs/web/http_client/http_file.h" #include "cuttlefish/host/libs/web/http_client/http_json.h" +#include "cuttlefish/host/libs/web/http_client/scrub_secrets.h" #include "cuttlefish/host/libs/web/parse_time.h" #include "cuttlefish/host/libs/zip/libzip_cc/seekable_source.h" #include "cuttlefish/host/libs/zip/libzip_cc/writable_source.h" @@ -77,7 +78,10 @@ Result GetResponseJson(const HttpResponse& response, const bool allow_redirect = false) { // debug information in error responses floods stderr with too much text // logged at a level that still ends up in the log file - VLOG(0) << "API response data:\n" << response.data; + // the artifact response carries a `signedUrl` whose query string is the + // credential, so the body is scrubbed before it reaches the log file + VLOG(0) << "API response data:\n" + << ScrubSecrets(response.data.toStyledString()); const bool response_code_allowed = response.HttpSuccess() || (allow_redirect && response.HttpRedirect()); CF_EXPECTF(std::move(response_code_allowed), diff --git a/base/cvd/cuttlefish/host/libs/web/http_client/BUILD.bazel b/base/cvd/cuttlefish/host/libs/web/http_client/BUILD.bazel index ac5bd5ef0d8..3dae675a1f9 100644 --- a/base/cvd/cuttlefish/host/libs/web/http_client/BUILD.bazel +++ b/base/cvd/cuttlefish/host/libs/web/http_client/BUILD.bazel @@ -71,6 +71,7 @@ cf_cc_library( "//cuttlefish/common/libs/fs:shared_fd_stream", "//cuttlefish/common/libs/utils:files", "//cuttlefish/host/libs/web/http_client", + "//cuttlefish/host/libs/web/http_client:scrub_secrets", "//cuttlefish/posix:remove", "//cuttlefish/result", "@abseil-cpp//absl/log", diff --git a/base/cvd/cuttlefish/host/libs/web/http_client/curl_http_client.cc b/base/cvd/cuttlefish/host/libs/web/http_client/curl_http_client.cc index e8e809af745..5dffbcfe868 100644 --- a/base/cvd/cuttlefish/host/libs/web/http_client/curl_http_client.cc +++ b/base/cvd/cuttlefish/host/libs/web/http_client/curl_http_client.cc @@ -83,7 +83,7 @@ int LoggingCurlDebugFunction(CURL*, curl_infotype type, char* data, size_t size, break; case CURLINFO_HEADER_IN: VLOG(1) << "CURLINFO_HEADER_IN "; - VLOG(0) << TrimWhitespace(data, size); + VLOG(0) << ScrubSecrets(TrimWhitespace(data, size)); break; case CURLINFO_HEADER_OUT: VLOG(1) << "CURLINFO_HEADER_OUT "; @@ -147,7 +147,7 @@ class CurlClient : public HttpClient { Result> DownloadToCallback( HttpRequest request, DataCallback callback) override { std::lock_guard lock(mutex_); - VLOG(0) << "Downloading '" << request.url << "'"; + VLOG(0) << "Downloading '" << ScrubUrl(request.url) << "'"; CF_EXPECT( request.data_to_write.empty() || request.method == HttpMethod::kPost, "data must be empty for non POST requests"); diff --git a/base/cvd/cuttlefish/host/libs/web/http_client/http_file.cc b/base/cvd/cuttlefish/host/libs/web/http_client/http_file.cc index 5f2a0b4f086..1d81c164dc6 100644 --- a/base/cvd/cuttlefish/host/libs/web/http_client/http_file.cc +++ b/base/cvd/cuttlefish/host/libs/web/http_client/http_file.cc @@ -29,6 +29,7 @@ #include "cuttlefish/common/libs/fs/shared_fd_stream.h" #include "cuttlefish/common/libs/utils/files.h" #include "cuttlefish/host/libs/web/http_client/http_client.h" +#include "cuttlefish/host/libs/web/http_client/scrub_secrets.h" #include "cuttlefish/posix/remove.h" #include "cuttlefish/result/result.h" @@ -37,7 +38,7 @@ namespace cuttlefish { Result> HttpGetToFile( HttpClient& http_client, const std::string& url, const std::string& path, const std::vector& headers) { - VLOG(0) << "Saving '" << url << "' to '" << path << "'"; + VLOG(0) << "Saving '" << ScrubUrl(url) << "' to '" << path << "'"; std::string temp_path; std::unique_ptr stream; @@ -83,8 +84,8 @@ Result> HttpGetToFile( HttpResponse http_response = CF_EXPECT(http_client.DownloadToCallback(request, callback)); - VLOG(0) << "Downloaded '" << total_dl << "' total bytes from '" << url - << "' to '" << path << "'."; + VLOG(0) << "Downloaded '" << total_dl << "' total bytes from '" + << ScrubUrl(url) << "' to '" << path << "'."; if (http_response.HttpSuccess()) { CF_EXPECT(RenameFile(temp_path, path)); diff --git a/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets.cc b/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets.cc index ef8a03aabdc..29b77101524 100644 --- a/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets.cc +++ b/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets.cc @@ -17,6 +17,7 @@ #include #include +#include namespace cuttlefish { @@ -31,7 +32,19 @@ std::string ScrubSecrets(const std::string& data) { // []client_secret=token_...[] result = std::regex_replace( result, std::regex("(client_secret=)(\\S{6})[^\\&\\s]*"), "$1$2..."); + // eg []GET /path?signature=token_text HTTP/1.1[] -> + // []GET /path?... HTTP/1.1[] + // Any query string is redacted, so this also covers an absolute URL in a + // header value such as the Location of a redirect, or in a JSON response + // body, and does not depend on a request line ending in " HTTP/". A '"' + // ends the match so a redacted JSON string stays closed. + result = + std::regex_replace(result, std::regex("\\?[^ \\t\\r\\n\"]*"), "?..."); return result; } +std::string ScrubUrl(std::string_view url) { + return std::string(url.substr(0, url.find_first_of("?#"))); +} + } // namespace cuttlefish diff --git a/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets.h b/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets.h index 89881a75fc8..dfb24ed6130 100644 --- a/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets.h +++ b/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets.h @@ -16,9 +16,14 @@ #pragma once #include +#include namespace cuttlefish { std::string ScrubSecrets(const std::string& data); +// Returns `url` without its query string or fragment, both of which can +// carry credentials such as the signature of a pre-signed URL. +std::string ScrubUrl(std::string_view url); + } // namespace cuttlefish diff --git a/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets_test.cc b/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets_test.cc index 2da163bb00b..bc6b19017c8 100644 --- a/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets_test.cc +++ b/base/cvd/cuttlefish/host/libs/web/http_client/scrub_secrets_test.cc @@ -73,5 +73,78 @@ TEST(HttpClientUtilTest, ScrubSecretsClientSecretNoMatch) { EXPECT_EQ(ScrubSecrets("client_id=1234567890"), "client_id=1234567890"); } +TEST(HttpClientUtilTest, ScrubSecretsRequestLineQueryMatch) { + EXPECT_EQ(ScrubSecrets( + "GET /bucket/image.zip?X-Goog-Signature=1234567890 HTTP/1.1"), + "GET /bucket/image.zip?... HTTP/1.1"); + EXPECT_EQ(ScrubSecrets("GET /bucket/image.zip?X-Goog-Signature=1234567890 " + "HTTP/1.1\r\nHost: storage.googleapis.com\r\n"), + "GET /bucket/image.zip?... HTTP/1.1\r\nHost: " + "storage.googleapis.com\r\n"); + EXPECT_EQ(ScrubSecrets("HEAD https://example.com/image.zip?token=1234567890 " + "HTTP/1.1"), + "HEAD https://example.com/image.zip?... HTTP/1.1"); +} + +TEST(HttpClientUtilTest, ScrubSecretsSignedRequestHeadersMatch) { + EXPECT_EQ( + ScrubSecrets("GET /bucket/image.zip?X-Goog-Signature=1234567890 " + "HTTP/1.1\r\nHost: storage.googleapis.com\r\n" + "Authorization: Bearer 1234567890\r\nAccept: */*\r\n"), + "GET /bucket/image.zip?... HTTP/1.1\r\nHost: storage.googleapis.com\r\n" + "Authorization: Bearer 123456...\r\nAccept: */*\r\n"); +} + +TEST(HttpClientUtilTest, ScrubSecretsRedirectLocationMatch) { + EXPECT_EQ(ScrubSecrets("Location: https://storage.googleapis.com/bucket/" + "image.zip?X-Goog-Signature=1234567890"), + "Location: https://storage.googleapis.com/bucket/image.zip?..."); + EXPECT_EQ(ScrubSecrets("HTTP/1.1 302 Found\r\nLocation: " + "https://example.com/a.zip?token=1234567890\r\n"), + "HTTP/1.1 302 Found\r\nLocation: " + "https://example.com/a.zip?...\r\n"); +} + +TEST(HttpClientUtilTest, ScrubSecretsQueryWithoutRequestLineSuffix) { + EXPECT_EQ(ScrubSecrets("GET /bucket/image.zip?X-Goog-Signature=1234567890"), + "GET /bucket/image.zip?..."); + EXPECT_EQ( + ScrubSecrets("GET /bucket/image.zip?X-Goog-Signature=1234567890\r\n"), + "GET /bucket/image.zip?...\r\n"); +} + +TEST(HttpClientUtilTest, ScrubSecretsJsonSignedUrlMatch) { + EXPECT_EQ( + ScrubSecrets( + "{\n \"signedUrl\" : " + "\"https://storage.googleapis.com/a.zip?X-Goog-Sig=123\"\n}"), + "{\n \"signedUrl\" : \"https://storage.googleapis.com/a.zip?...\"\n}"); + EXPECT_EQ( + ScrubSecrets("{\n \"a\" : \"x?tok=123\",\n \"b\" : \"plain\"\n}"), + "{\n \"a\" : \"x?...\",\n \"b\" : \"plain\"\n}"); +} + +TEST(HttpClientUtilTest, ScrubSecretsRequestLineQueryNoMatch) { + EXPECT_EQ(ScrubSecrets("GET /bucket/image.zip HTTP/1.1"), + "GET /bucket/image.zip HTTP/1.1"); + EXPECT_EQ(ScrubSecrets("Host: example.com"), "Host: example.com"); +} + +TEST(HttpClientUtilTest, ScrubUrlRemovesQueryAndFragment) { + EXPECT_EQ( + ScrubUrl("https://example.com/image.zip?X-Goog-Signature=1234567890"), + "https://example.com/image.zip"); + EXPECT_EQ(ScrubUrl("https://example.com/image.zip#sha256=1234567890"), + "https://example.com/image.zip"); + EXPECT_EQ(ScrubUrl("https://example.com/image.zip?a=1#b"), + "https://example.com/image.zip"); +} + +TEST(HttpClientUtilTest, ScrubUrlKeepsPlainUrl) { + EXPECT_EQ(ScrubUrl("https://example.com/image.zip"), + "https://example.com/image.zip"); + EXPECT_EQ(ScrubUrl(""), ""); +} + } // namespace http_client } // namespace cuttlefish From e0c39fe17bace5f136382d9d08dfbceda84e057e Mon Sep 17 00:00:00 2001 From: Lars Ershammar Date: Sun, 16 Aug 2026 13:10:40 +0000 Subject: [PATCH 6/7] Pass the OAuth scope through to the credential sources --- .../cvd/fetch/build_api_credentials.cc | 29 ++++++++++--------- .../cvd/fetch/build_api_credentials.h | 6 +++- .../host/commands/cvd/fetch/downloaders.cc | 6 ++-- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/build_api_credentials.cc b/base/cvd/cuttlefish/host/commands/cvd/fetch/build_api_credentials.cc index ee7c3054827..24949d4140f 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/build_api_credentials.cc +++ b/base/cvd/cuttlefish/host/commands/cvd/fetch/build_api_credentials.cc @@ -36,7 +36,8 @@ namespace cuttlefish { namespace { std::unique_ptr TryParseServiceAccount( - HttpClient& http_client, const std::string& file_content) { + HttpClient& http_client, const std::string& file_content, + const std::string& scope) { Json::Reader reader; Json::Value content; if (!reader.parse(file_content, content)) { @@ -45,8 +46,8 @@ std::unique_ptr TryParseServiceAccount( VLOG(0) << "Could not parse credential file as Service Account"; return {}; } - auto result = ServiceAccountOauthCredentialSource::FromJson( - http_client, content, kAndroidBuildApiScope); + auto result = ServiceAccountOauthCredentialSource::FromJson(http_client, + content, scope); if (!result.has_value()) { VLOG(0) << "Failed to load service account json file: \n" << result.error(); return {}; @@ -56,7 +57,7 @@ std::unique_ptr TryParseServiceAccount( Result> GetCredentialSourceLegacy( HttpClient& http_client, const std::string& credential_source, - const std::string& oauth_filepath) { + const std::string& oauth_filepath, const std::string& scope) { std::unique_ptr result; if (credential_source == "gce") { result = GceMetadataCredentialSource::Make(http_client); @@ -88,7 +89,7 @@ Result> GetCredentialSourceLegacy( CF_EXPECTF(ReadFileContents(credential_source), "Failure getting credential file contents from file \"{}\"", credential_source); - if (auto crds = TryParseServiceAccount(http_client, file_content)) { + if (auto crds = TryParseServiceAccount(http_client, file_content, scope)) { result = std::move(crds); } else { result = FixedCredentialSource::Make(file_content); @@ -101,7 +102,7 @@ Result> GetCredentialSource( HttpClient& http_client, const std::string& credential_source, const std::string& oauth_filepath, const bool use_gce_metadata, const std::string& credential_filepath, - const std::string& service_account_filepath) { + const std::string& service_account_filepath, const std::string& scope) { const int number_of_set_credentials = !credential_source.empty() + use_gce_metadata + !credential_filepath.empty() + !service_account_filepath.empty(); @@ -125,7 +126,7 @@ Result> GetCredentialSource( "from file \"{}\".", service_account_filepath); auto service_account_credentials = - TryParseServiceAccount(http_client, contents); + TryParseServiceAccount(http_client, contents, scope); CF_EXPECTF(service_account_credentials != nullptr, "Unable to parse service account credentials in file \"{}\". " "File contents: {}", @@ -136,19 +137,19 @@ Result> GetCredentialSource( // when this helper is removed its `.acloud_oauth2.dat` processing should be // moved here return GetCredentialSourceLegacy(http_client, credential_source, - oauth_filepath); + oauth_filepath, scope); } } // namespace Result> GetCredentialSourceFromFlags( HttpClient& http_client, const BuildApiFlags& flags, - const std::string& oauth_filepath) { - return CF_EXPECT( - GetCredentialSource(http_client, flags.credential_source, oauth_filepath, - flags.credential_flags.use_gce_metadata, - flags.credential_flags.credential_filepath, - flags.credential_flags.service_account_filepath)); + const std::string& oauth_filepath, const std::string& scope) { + return CF_EXPECT(GetCredentialSource( + http_client, flags.credential_source, oauth_filepath, + flags.credential_flags.use_gce_metadata, + flags.credential_flags.credential_filepath, + flags.credential_flags.service_account_filepath, scope)); } std::string GetAcloudOauthFilepath() { diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/build_api_credentials.h b/base/cvd/cuttlefish/host/commands/cvd/fetch/build_api_credentials.h index f8772873115..8541d66ba6d 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/build_api_credentials.h +++ b/base/cvd/cuttlefish/host/commands/cvd/fetch/build_api_credentials.h @@ -28,9 +28,13 @@ namespace cuttlefish { inline constexpr char kAndroidBuildApiScope[] = "https://www.googleapis.com/auth/androidbuild.internal"; +inline constexpr char kCloudStorageReadScope[] = + "https://www.googleapis.com/auth/devstorage.read_only"; + Result> GetCredentialSourceFromFlags( HttpClient& http_client, const BuildApiFlags& flags, - const std::string& oauth_filepath); + const std::string& oauth_filepath, + const std::string& scope = kAndroidBuildApiScope); std::string GetAcloudOauthFilepath(); diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/downloaders.cc b/base/cvd/cuttlefish/host/commands/cvd/fetch/downloaders.cc index 674e8c7974c..fff493f50e2 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/downloaders.cc +++ b/base/cvd/cuttlefish/host/commands/cvd/fetch/downloaders.cc @@ -100,9 +100,9 @@ Result Downloaders::Create(const BuildApiFlags& flags, impl->luci_credential_source_ = CF_EXPECT(GetCredentialSourceFromFlags( *impl->retrying_http_client_, flags, StringFromEnv("HOME", ".") + "/.config/chrome_infra/auth/tokens.json")); - impl->gsutil_credential_source_ = CF_EXPECT( - GetCredentialSourceFromFlags(*impl->retrying_http_client_, flags, - StringFromEnv("HOME", ".") + "/.boto")); + impl->gsutil_credential_source_ = CF_EXPECT(GetCredentialSourceFromFlags( + *impl->retrying_http_client_, flags, + StringFromEnv("HOME", ".") + "/.boto", kCloudStorageReadScope)); impl->luci_build_api_ = std::make_unique( *impl->retrying_http_client_, impl->luci_credential_source_.get(), From b13db7e1ceab8dbcb7d0a05aeca34f9d9595d70d Mon Sep 17 00:00:00 2001 From: Lars Ershammar Date: Sun, 16 Aug 2026 13:04:54 +0000 Subject: [PATCH 7/7] Only resolve build zip names where they are used --- .../host/commands/cvd/fetch/fetch_cvd.cc | 67 ++++++++++--------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc index 3d10e15b59a..151067731f5 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc +++ b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc @@ -218,35 +218,39 @@ Result FetchDefaultTarget(FetchBuildContext& context, CF_EXPECT(img_zip.DeleteLocalFile()); } } - std::string target_files_name = context.GetBuildZipName("target_files"); - FetchArtifact target_files = context.Artifact(target_files_name); - if (has_system_build || flags.download_target_files_zip) { - LOG(INFO) << "Downloading target files zip for " << context; - std::string download_location = - fmt::format("default/{}", target_files_name); - CF_EXPECT(target_files.DownloadTo(download_location)); - } - if (flags.dynamic_super_image) { - ReadableZip* target_files_zip = CF_EXPECT(target_files.AsZip()); - std::unique_ptr ab_partitions_source = - CF_EXPECT(target_files_zip->OpenReadOnly("META/ab_partitions.txt")); - CF_EXPECT(ab_partitions_source.get()); - std::string ab_partitions_contents = - CF_EXPECT(ReadToString(*ab_partitions_source)); - - CF_EXPECT(target_files.ExtractOneTo("META/ab_partitions.txt", - "default/ab_partitions.txt")); - - std::vector ab_files = - absl::StrSplit(ab_partitions_contents, '\n'); - ab_files.emplace_back("super_empty"); - for (std::string_view ab_file : ab_files) { - if (ab_file.empty()) { - continue; + const bool download_target_files = + has_system_build || flags.download_target_files_zip; + if (download_target_files || flags.dynamic_super_image) { + std::string target_files_name = context.GetBuildZipName("target_files"); + FetchArtifact target_files = context.Artifact(target_files_name); + if (download_target_files) { + LOG(INFO) << "Downloading target files zip for " << context; + const std::string download_location = + fmt::format("default/{}", target_files_name); + CF_EXPECT(target_files.DownloadTo(download_location)); + } + if (flags.dynamic_super_image) { + ReadableZip* target_files_zip = CF_EXPECT(target_files.AsZip()); + std::unique_ptr ab_partitions_source = + CF_EXPECT(target_files_zip->OpenReadOnly("META/ab_partitions.txt")); + CF_EXPECT(ab_partitions_source.get()); + const std::string ab_partitions_contents = + CF_EXPECT(ReadToString(*ab_partitions_source)); + + CF_EXPECT(target_files.ExtractOneTo("META/ab_partitions.txt", + "default/ab_partitions.txt")); + + std::vector ab_files = + absl::StrSplit(ab_partitions_contents, '\n'); + ab_files.emplace_back("super_empty"); + for (std::string_view ab_file : ab_files) { + if (ab_file.empty()) { + continue; + } + const std::string member = fmt::format("IMAGES/{}.img", ab_file); + const std::string output = fmt::format("default/{}.img", ab_file); + CF_EXPECT(target_files.ExtractOneTo(member, output)); } - std::string member = fmt::format("IMAGES/{}.img", ab_file); - std::string output = fmt::format("default/{}.img", ab_file); - CF_EXPECT(target_files.ExtractOneTo(member, output)); } } return {}; @@ -314,12 +318,13 @@ Result FetchKernelTarget(FetchBuildContext context) { Result FetchBootTarget(FetchBuildContext& context, bool keep_downloaded_archives) { - std::string img_zip = context.GetBuildZipName("img"); - std::string to_download = context.GetFilepath().value_or(img_zip); + const std::optional filepath = context.GetFilepath(); + const std::string to_download = + filepath.has_value() ? *filepath : context.GetBuildZipName("img"); FetchArtifact artifact = context.Artifact(to_download); CF_EXPECT(artifact.Download()); - if (to_download == img_zip) { + if (!filepath.has_value()) { CF_EXPECT(artifact.ExtractOne("boot.img")); CF_EXPECT(artifact.ExtractOne("vendor_boot.img")); if (!keep_downloaded_archives) {