Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To improve readability and maintainability, it's good practice to avoid magic numbers. Since defining a shared constant might be outside the scope of this change, consider adding a comment to clarify the meaning of 0400.

Suggested change
filePath.chmod(entry.getMode() | 0400);
filePath.chmod(entry.getMode() | 0400 /* S_IRUSR */);

// entry.getLastModified() appears to be in seconds, so we need to convert
// it into milliseconds for setLastModifiedTime
filePath.setLastModifiedTime(entry.getLastModified() * 1000L);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To improve readability and maintainability, it's good practice to avoid magic numbers. Since defining a shared constant might be outside the scope of this change, consider adding a comment to clarify the meaning of 0400.

Suggested change
filePath.chmod(entry.getMode() | 0400);
filePath.chmod(entry.getMode() | 0400 /* S_IRUSR */);


// 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To improve readability and maintainability, it's good practice to avoid magic numbers. Since defining a shared constant might be outside the scope of this change, consider adding a comment to clarify the meaning of 0400.

Suggested change
outputPath.chmod(permissions | 0400);
outputPath.chmod(permissions | 0400 /* S_IRUSR */);

outputPath.setLastModifiedTime(entry.getTime());
}
}
Expand Down
89 changes: 89 additions & 0 deletions src/test/shell/bazel/starlark_repository_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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) <<EOF
repo = use_repo_rule('//:test.bzl', 'repo')
repo(name = 'foo')
EOF
touch BUILD

cat >test.bzl <<EOF
def _impl(repository_ctx):
repository_ctx.extract('${archive_path}', 'out_dir')
# Verify the file is readable by reading it
content = repository_ctx.read('out_dir/${path_in_archive}')
if '${expected_content}' not in content:
fail('Expected to read file content, got: ' + content)
repository_ctx.file("BUILD", "filegroup(name='bar', srcs=[])")

repo = repository_rule(implementation=_impl)
EOF

bazel build @foo//:bar >& $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'
Expand Down
Loading