From 88671cb2367d02140a82b253976024d054a0bf4b Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 5 Feb 2026 08:26:49 -0800 Subject: [PATCH] Force `rctx.{download_and,}extract` to create user-readable files (https://github.com/bazelbuild/bazel/pull/28531) Archives in the wild do sometimes contain non-readable files, but other tools work around this and thus mask their brokenness. Context: https://bazelbuild.slack.com/archives/CDCMRLS23/p1770213515354229 Closes #28531. PiperOrigin-RevId: 865960367 Change-Id: I7273eb983d63d6960d184764cec5040bba77b2c2 --- .../repository/decompressor/ArFunction.java | 4 +- .../decompressor/CompressedTarFunction.java | 4 +- .../decompressor/ZipDecompressor.java | 4 +- .../shell/bazel/starlark_repository_test.sh | 89 +++++++++++++++++++ 4 files changed, 98 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ArFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ArFunction.java index d72df5c70cff74..e76835c4aa6807 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ArFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ArFunction.java @@ -63,7 +63,9 @@ public Path decompress(DecompressorDescriptor descriptor) try (OutputStream out = filePath.getOutputStream()) { arStream.transferTo(out); } - filePath.chmod(entry.getMode()); + // Ensure that all files are at least user-readable. Some archives contain files that + // are not, but many other tools are working around this and thus mask these issues. + filePath.chmod(entry.getMode() | 0400); // entry.getLastModified() appears to be in seconds, so we need to convert // it into milliseconds for setLastModifiedTime filePath.setLastModifiedTime(entry.getLastModified() * 1000L); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/CompressedTarFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/CompressedTarFunction.java index c883f63d0de96c..8d30f03c5d031d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/CompressedTarFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/CompressedTarFunction.java @@ -136,7 +136,9 @@ public Path decompress(DecompressorDescriptor descriptor) try (OutputStream out = filePath.getOutputStream()) { tarStream.transferTo(out); } - filePath.chmod(entry.getMode()); + // Ensure that all files are at least user-readable. Some archives contain files that + // are not, but many other tools are working around this and thus mask these issues. + filePath.chmod(entry.getMode() | 0400); // This can only be done on real files, not links, or it will skip the reader to // the next "real" file to try to find the mod time info. diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ZipDecompressor.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ZipDecompressor.java index 4e503a89a4542c..f1858a0b96c3a9 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ZipDecompressor.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ZipDecompressor.java @@ -167,7 +167,9 @@ private static void extractZipEntry( throw new InterruptedException(); } } - outputPath.chmod(permissions); + // Ensure that all files are at least user-readable. Some archives contain files that + // are not, but many other tools are working around this and thus mask these issues. + outputPath.chmod(permissions | 0400); outputPath.setLastModifiedTime(entry.getTime()); } } diff --git a/src/test/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh index da15c3c2f3afdc..f38943f96057b5 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -3598,6 +3598,95 @@ EOF [[ -f "$output_base/external/+repo+foo/ruff" ]] || fail "Expected ruff binary to be extracted" } +# Verifies that files without user-readable permissions in archives are made +# readable after extraction. +function _run_extract_test() { + local archive_path="$1" + local path_in_archive="$2" + local expected_content="$3" + + cat > $(setup_module_dot_bazel) <test.bzl <& $TEST_log || fail "Failed to build" +} + +function test_extract_non_readable_file_tar() { + local archive_tar="${TEST_TMPDIR}/non_readable.tar.gz" + + python3 -c " +import tarfile +import io +import gzip + +content = b'secret content' +with gzip.open('${archive_tar}', 'wb') as gz: + with tarfile.open(fileobj=gz, mode='w') as tar: + info = tarfile.TarInfo(name='non_readable_dir/non_readable.txt') + info.size = len(content) + info.mode = 0o000 # No permissions + tar.addfile(info, io.BytesIO(content)) +" + + _run_extract_test "${archive_tar}" "non_readable_dir/non_readable.txt" "secret content" +} + +function test_extract_non_readable_file_zip() { + local archive_zip="${TEST_TMPDIR}/non_readable.zip" + + pushd "${TEST_TMPDIR}" + mkdir -p non_readable_zip_dir + echo "secret zip content" > non_readable_zip_dir/non_readable.txt + python3 -c " +import zipfile +with zipfile.ZipFile('non_readable.zip', 'w') as zf: + info = zipfile.ZipInfo('non_readable_zip_dir/non_readable.txt') + # S_IFREG (0o100000) marks it as a regular file, but with no permission bits. + # This ensures getPermissions() returns the actual mode rather than defaulting to 0755. + info.external_attr = 0o100000 << 16 + zf.writestr(info, 'secret zip content') +" + popd + + _run_extract_test "${archive_zip}" "non_readable_zip_dir/non_readable.txt" "secret zip content" +} + +function test_extract_non_readable_file_ar() { + local archive_ar="${TEST_TMPDIR}/non_readable.ar" + + # Create a valid AR file, then patch the permission field to 0000 + pushd "${TEST_TMPDIR}" + echo "secret ar content" > non_readable.txt + ar rc non_readable.ar non_readable.txt + # Patch the mode field (bytes 40-47 after the 8-byte magic) to "0 " + python3 -c " +with open('non_readable.ar', 'r+b') as f: + # AR magic is 8 bytes, then header starts + # Header format: filename(16) + mtime(12) + owner(6) + group(6) + mode(8) + size(10) + magic(2) + # Mode is at offset 8 + 16 + 12 + 6 + 6 = 48 + f.seek(48) + f.write(b'0 ') # 8 bytes for mode 0000 in octal +" + popd + + _run_extract_test "${archive_ar}" "non_readable.txt" "secret ar content" +} + # Regression test for https://github.com/bazelbuild/bazel/issues/27446. function do_test_local_module_file_patch() { cat > $(setup_module_dot_bazel) <<'EOF'