diff options
5 files changed, 184 insertions, 7 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java index bf2650486e..bbac83ac69 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.bazel.repository; import com.google.common.base.Ascii; import com.google.common.base.Optional; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.net.UrlEscapers; import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; @@ -30,7 +31,6 @@ import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; -import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; import java.net.URL; import java.util.Map; @@ -103,7 +103,7 @@ public class GitCloner { return false; } - public static SkyValue clone( + public static HttpDownloadValue clone( Rule rule, Path outputDirectory, ExtendedEventHandler eventHandler, @@ -160,15 +160,24 @@ public class GitCloner { } } + String uncheckedSha256 = getUncheckedSha256(mapper); if (repositoryLooksTgzable(descriptor.remote)) { Optional<Exception> maybeException = downloadRepositoryAsHttpArchive( - descriptor, eventHandler, clientEnvironment, downloader); + descriptor, eventHandler, clientEnvironment, downloader, uncheckedSha256); if (maybeException.isPresent()) { suppressedException = maybeException.get(); } else { return new HttpDownloadValue(descriptor.directory); } - // Fallthrough to cloning from git. + } + if (!Strings.isNullOrEmpty(uncheckedSha256)) { + // Specifying a sha256 forces this to use a tarball download. + IOException e = new IOException( + "Could not download tarball, but sha256 specified (" + uncheckedSha256 + ")"); + if (suppressedException != null) { + e.addSuppressed(suppressedException); + } + throw new RepositoryFunctionException(e, Transience.TRANSIENT); } git = @@ -246,6 +255,18 @@ public class GitCloner { return new HttpDownloadValue(descriptor.directory); } + private static String getUncheckedSha256(WorkspaceAttributeMapper mapper) + throws RepositoryFunctionException { + if (mapper.isAttributeValueExplicitlySpecified("sha256")) { + try { + return mapper.get("sha256", Type.STRING); + } catch (EvalException e) { + throw new RepositoryFunctionException(e, Transience.PERSISTENT); + } + } + return ""; + } + private static boolean repositoryLooksTgzable(String remote) { // Only handles GitHub right now. return GITHUB_URL.matcher(remote).matches(); @@ -253,7 +274,7 @@ public class GitCloner { private static Optional<Exception> downloadRepositoryAsHttpArchive( GitRepositoryDescriptor descriptor, ExtendedEventHandler eventHandler, - Map<String, String> clientEnvironment, HttpDownloader downloader) + Map<String, String> clientEnvironment, HttpDownloader downloader, String uncheckedSha256) throws RepositoryFunctionException { Matcher matcher = GITHUB_URL.matcher(descriptor.remote); Preconditions.checkState( @@ -266,7 +287,7 @@ public class GitCloner { user + "/" + repositoryName + "/archive/" + descriptor.ref + ".tar.gz"); try { FileSystemUtils.createDirectoryAndParents(descriptor.directory); - Path tgz = downloader.download(ImmutableList.of(new URL(downloadUrl)), "", + Path tgz = downloader.download(ImmutableList.of(new URL(downloadUrl)), uncheckedSha256, Optional.of("tar.gz"), descriptor.directory, eventHandler, clientEnvironment); DecompressorValue.decompress(DecompressorDescriptor.builder() .setArchivePath(tgz) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/GitRepositoryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/GitRepositoryRule.java index dd15fe5e5c..483cfd7e9d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/GitRepositoryRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/GitRepositoryRule.java @@ -57,6 +57,17 @@ public class GitRepositoryRule implements RuleDefinition { <p>Currently, only cloning the top-level submodules is supported</p> <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ .add(attr("init_submodules", BOOLEAN).value(false)) + /* <!-- #BLAZE_RULE(git_repository).ATTRIBUTE(sha256) --> + The expected SHA-256 hash of the file downloaded. Specifying this forces the repository to + be downloaded as a tarball. Currently, this is only supported for public GitHub + repositories. + + <p>This must match the SHA-256 hash of the file downloaded. <em>It is a security risk to + omit the SHA-256 as remote files can change.</em> At best omitting this field will make + your build non-hermetic. It is optional to make development easier but should be set + before shipping.</p> + <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ + .add(attr("sha256", STRING)) .setWorkspaceOnly() .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewGitRepositoryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewGitRepositoryRule.java index ce1a48e1ba..37246c5515 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewGitRepositoryRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewGitRepositoryRule.java @@ -89,6 +89,17 @@ public class NewGitRepositoryRule implements RuleDefinition { <p>Currently, only cloning the top-level submodules is supported</p> <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ .add(attr("init_submodules", BOOLEAN).value(false)) + /* <!-- #BLAZE_RULE(new_git_repository).ATTRIBUTE(sha256) --> + The expected SHA-256 hash of the file downloaded. Specifying this forces the repository to + be downloaded as a tarball. Currently, this is only supported for public GitHub + repositories. + + <p>This must match the SHA-256 hash of the file downloaded. <em>It is a security risk to + omit the SHA-256 as remote files can change.</em> At best omitting this field will make + your build non-hermetic. It is optional to make development easier but should be set + before shipping.</p> + <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ + .add(attr("sha256", STRING)) .setWorkspaceOnly() .build(); } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD index da28b50f64..a9476021eb 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD @@ -7,12 +7,26 @@ filegroup( visibility = ["//src/test/java/com/google/devtools/build/lib:__pkg__"], ) +genrule( + name = "empty-tarball", + outs = ["empty.tar.gz"], + cmd = """ +mkdir -p bar-1.2.3 +touch bar-1.2.3/whatever +tar czf $@ bar-1.2.3 +""", +) + java_test( name = "RepositoryTests", srcs = glob([ "**/*.java", ]), - tags = ["rules"], + data = [":empty-tarball"], + tags = [ + "no_windows", # Runfiles aren't supported. + "rules", + ], test_class = "com.google.devtools.build.lib.AllTests", deps = [ "//src/main/java/com/google/devtools/build/lib:bazel-main", diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/GitClonerTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/GitClonerTest.java new file mode 100644 index 0000000000..10b668b171 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/GitClonerTest.java @@ -0,0 +1,120 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// 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. + +package com.google.devtools.build.lib.bazel.repository; + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyListOf; +import static org.mockito.Matchers.anyMapOf; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.when; + +import com.google.common.base.Optional; +import com.google.common.collect.Maps; +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache; +import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; +import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.events.StoredEventHandler; +import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.util.FileSystems; +import java.net.URL; +import java.util.Map; +import org.junit.Before; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Mockito; + +/** + * Tests for {@link GitCloner}. + */ +@RunWith(JUnit4.class) +public class GitClonerTest extends BuildViewTestCase { + private FileSystem diskFs = FileSystems.getNativeFileSystem(); + private Path diskTarball; + private Path outputDirectory; + private StoredEventHandler eventHandler = new StoredEventHandler(); + private Map<String, String> clientEnvironment = Maps.newHashMap(); + + @org.junit.Rule + public final ExpectedException expected = ExpectedException.none(); + + @Before + public void initialize() throws Exception { + outputDirectory = diskFs.getPath(System.getenv("TEST_TMPDIR")) + .getRelative("output-dir"); + diskTarball = diskFs.getPath(System.getenv("TEST_SRCDIR")) + .getRelative( + "io_bazel/src/test/java/com/google/devtools/build/lib/bazel/repository/empty.tar.gz"); + } + + @Test + public void testSha256TarballOkay() throws Exception { + Rule rule = scratchRule("external", "foo", + "git_repository(", + " name = 'foo',", + " remote = 'https://github.com/foo/bar.git',", + " tag = '1.2.3',", + " sha256 = '2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9826',", + ")"); + HttpDownloader downloader = Mockito.mock(HttpDownloader.class); + when(downloader.download( + anyListOf(URL.class), any(String.class), eq(Optional.of("tar.gz")), eq(outputDirectory), + any(ExtendedEventHandler.class), anyMapOf(String.class, String.class))) + .thenReturn(diskTarball); + + HttpDownloadValue value = GitCloner.clone( + rule, outputDirectory, eventHandler, clientEnvironment, downloader); + assertThat(value).isNotNull(); + assertThat(value.getPath()).isEqualTo(outputDirectory); + } + + @Test + public void testNonGitHubSha256Throws() throws Exception { + Rule nonGitHubRule = scratchRule("external", "foo", + "git_repository(", + " name = 'foo',", + " remote = 'https://example.com/foo/bar.git',", + " tag = '1.2.3',", + " sha256 = '2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9826',", + ")"); + HttpDownloader downloader = new HttpDownloader(Mockito.mock(RepositoryCache.class)); + expected.expect(RepositoryFunctionException.class); + expected.expectMessage("Could not download tarball, but sha256 specified"); + GitCloner.clone( + nonGitHubRule, outputDirectory, eventHandler, clientEnvironment, downloader); + } + + @Test + public void testSha256TarballErrorThrows() throws Exception { + Rule rule = scratchRule("external", "foo", + "git_repository(", + " name = 'foo',", + " remote = 'https://github.com/foo/bar.git',", + " tag = '1.2.3',", + " sha256 = '2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9826',", + ")"); + HttpDownloader downloader = new HttpDownloader(Mockito.mock(RepositoryCache.class)); + expected.expect(RepositoryFunctionException.class); + expected.expectMessage("Could not download tarball, but sha256 specified"); + GitCloner.clone( + rule, outputDirectory, eventHandler, clientEnvironment, downloader); + } +} |