Skip to content

Commit 4e58823

Browse files
fmeumbazel-io
authored andcommitted
Force rctx.{download_and,}extract to create user-readable files (bazelbuild#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 bazelbuild#28531. PiperOrigin-RevId: 865960367 Change-Id: I7273eb983d63d6960d184764cec5040bba77b2c2
1 parent 51c8fc7 commit 4e58823

4 files changed

Lines changed: 98 additions & 3 deletions

File tree

src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ArFunction.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ public Path decompress(DecompressorDescriptor descriptor)
6363
try (OutputStream out = filePath.getOutputStream()) {
6464
arStream.transferTo(out);
6565
}
66-
filePath.chmod(entry.getMode());
66+
// Ensure that all files are at least user-readable. Some archives contain files that
67+
// are not, but many other tools are working around this and thus mask these issues.
68+
filePath.chmod(entry.getMode() | 0400);
6769
// entry.getLastModified() appears to be in seconds, so we need to convert
6870
// it into milliseconds for setLastModifiedTime
6971
filePath.setLastModifiedTime(entry.getLastModified() * 1000L);

src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/CompressedTarFunction.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,9 @@ public Path decompress(DecompressorDescriptor descriptor)
136136
try (OutputStream out = filePath.getOutputStream()) {
137137
tarStream.transferTo(out);
138138
}
139-
filePath.chmod(entry.getMode());
139+
// Ensure that all files are at least user-readable. Some archives contain files that
140+
// are not, but many other tools are working around this and thus mask these issues.
141+
filePath.chmod(entry.getMode() | 0400);
140142

141143
// This can only be done on real files, not links, or it will skip the reader to
142144
// the next "real" file to try to find the mod time info.

src/main/java/com/google/devtools/build/lib/bazel/repository/decompressor/ZipDecompressor.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ private static void extractZipEntry(
167167
throw new InterruptedException();
168168
}
169169
}
170-
outputPath.chmod(permissions);
170+
// Ensure that all files are at least user-readable. Some archives contain files that
171+
// are not, but many other tools are working around this and thus mask these issues.
172+
outputPath.chmod(permissions | 0400);
171173
outputPath.setLastModifiedTime(entry.getTime());
172174
}
173175
}

src/test/shell/bazel/starlark_repository_test.sh

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3598,6 +3598,95 @@ EOF
35983598
[[ -f "$output_base/external/+repo+foo/ruff" ]] || fail "Expected ruff binary to be extracted"
35993599
}
36003600

3601+
# Verifies that files without user-readable permissions in archives are made
3602+
# readable after extraction.
3603+
function _run_extract_test() {
3604+
local archive_path="$1"
3605+
local path_in_archive="$2"
3606+
local expected_content="$3"
3607+
3608+
cat > $(setup_module_dot_bazel) <<EOF
3609+
repo = use_repo_rule('//:test.bzl', 'repo')
3610+
repo(name = 'foo')
3611+
EOF
3612+
touch BUILD
3613+
3614+
cat >test.bzl <<EOF
3615+
def _impl(repository_ctx):
3616+
repository_ctx.extract('${archive_path}', 'out_dir')
3617+
# Verify the file is readable by reading it
3618+
content = repository_ctx.read('out_dir/${path_in_archive}')
3619+
if '${expected_content}' not in content:
3620+
fail('Expected to read file content, got: ' + content)
3621+
repository_ctx.file("BUILD", "filegroup(name='bar', srcs=[])")
3622+
3623+
repo = repository_rule(implementation=_impl)
3624+
EOF
3625+
3626+
bazel build @foo//:bar >& $TEST_log || fail "Failed to build"
3627+
}
3628+
3629+
function test_extract_non_readable_file_tar() {
3630+
local archive_tar="${TEST_TMPDIR}/non_readable.tar.gz"
3631+
3632+
python3 -c "
3633+
import tarfile
3634+
import io
3635+
import gzip
3636+
3637+
content = b'secret content'
3638+
with gzip.open('${archive_tar}', 'wb') as gz:
3639+
with tarfile.open(fileobj=gz, mode='w') as tar:
3640+
info = tarfile.TarInfo(name='non_readable_dir/non_readable.txt')
3641+
info.size = len(content)
3642+
info.mode = 0o000 # No permissions
3643+
tar.addfile(info, io.BytesIO(content))
3644+
"
3645+
3646+
_run_extract_test "${archive_tar}" "non_readable_dir/non_readable.txt" "secret content"
3647+
}
3648+
3649+
function test_extract_non_readable_file_zip() {
3650+
local archive_zip="${TEST_TMPDIR}/non_readable.zip"
3651+
3652+
pushd "${TEST_TMPDIR}"
3653+
mkdir -p non_readable_zip_dir
3654+
echo "secret zip content" > non_readable_zip_dir/non_readable.txt
3655+
python3 -c "
3656+
import zipfile
3657+
with zipfile.ZipFile('non_readable.zip', 'w') as zf:
3658+
info = zipfile.ZipInfo('non_readable_zip_dir/non_readable.txt')
3659+
# S_IFREG (0o100000) marks it as a regular file, but with no permission bits.
3660+
# This ensures getPermissions() returns the actual mode rather than defaulting to 0755.
3661+
info.external_attr = 0o100000 << 16
3662+
zf.writestr(info, 'secret zip content')
3663+
"
3664+
popd
3665+
3666+
_run_extract_test "${archive_zip}" "non_readable_zip_dir/non_readable.txt" "secret zip content"
3667+
}
3668+
3669+
function test_extract_non_readable_file_ar() {
3670+
local archive_ar="${TEST_TMPDIR}/non_readable.ar"
3671+
3672+
# Create a valid AR file, then patch the permission field to 0000
3673+
pushd "${TEST_TMPDIR}"
3674+
echo "secret ar content" > non_readable.txt
3675+
ar rc non_readable.ar non_readable.txt
3676+
# Patch the mode field (bytes 40-47 after the 8-byte magic) to "0 "
3677+
python3 -c "
3678+
with open('non_readable.ar', 'r+b') as f:
3679+
# AR magic is 8 bytes, then header starts
3680+
# Header format: filename(16) + mtime(12) + owner(6) + group(6) + mode(8) + size(10) + magic(2)
3681+
# Mode is at offset 8 + 16 + 12 + 6 + 6 = 48
3682+
f.seek(48)
3683+
f.write(b'0 ') # 8 bytes for mode 0000 in octal
3684+
"
3685+
popd
3686+
3687+
_run_extract_test "${archive_ar}" "non_readable.txt" "secret ar content"
3688+
}
3689+
36013690
# Regression test for https://github.com/bazelbuild/bazel/issues/27446.
36023691
function do_test_local_module_file_patch() {
36033692
cat > $(setup_module_dot_bazel) <<'EOF'

0 commit comments

Comments
 (0)