fix(tvix/store/fuse): fix executable bit

We were blindly returning 0o444 for all regular files, but regular files
with executable bit need to be 0o555.

This wasn't spotted because stat'ing executable files was not part of
the test suite, it's now added.

Change-Id: I04c69784053e7e43d838c01bb288f2df48f40b4e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9345
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
This commit is contained in:
Florian Klink 2023-09-17 14:36:15 +03:00 committed by flokli
parent da6cbb4a45
commit bf2fe88a5c
2 changed files with 38 additions and 1 deletions

View file

@ -43,7 +43,8 @@ pub fn gen_file_attr(inode_data: &InodeData, inode: u64) -> FileAttr {
crtime: SystemTime::UNIX_EPOCH,
kind: inode_data.into(),
perm: match inode_data {
InodeData::Regular(..) => 0o444,
InodeData::Regular(_, _, false) => 0o444, // no-executable files
InodeData::Regular(_, _, true) => 0o555, // executable files
InodeData::Symlink(_) => 0o444,
InodeData::Directory(..) => 0o555,
},

View file

@ -16,6 +16,7 @@ use crate::{proto, FUSE};
const BLOB_A_NAME: &str = "00000000000000000000000000000000-test";
const BLOB_B_NAME: &str = "55555555555555555555555555555555-test";
const HELLOWORLD_BLOB_NAME: &str = "66666666666666666666666666666666-test";
const SYMLINK_NAME: &str = "11111111111111111111111111111111-test";
const SYMLINK_NAME2: &str = "44444444444444444444444444444444-test";
const DIRECTORY_WITH_KEEP_NAME: &str = "22222222222222222222222222222222-test";
@ -103,6 +104,37 @@ async fn populate_blob_b(
path_info_service.put(path_info).expect("must succeed");
}
/// adds a blob containing helloworld and marks it as executable
async fn populate_helloworld_blob(
blob_service: &Arc<dyn BlobService>,
_directory_service: &Arc<dyn DirectoryService>,
path_info_service: &Arc<dyn PathInfoService>,
) {
// Upload BLOB_B
let mut bw = blob_service.open_write().await;
tokio::io::copy(
&mut Cursor::new(fixtures::HELLOWORLD_BLOB_CONTENTS.to_vec()),
&mut bw,
)
.await
.expect("must succeed uploading");
bw.close().await.expect("must succeed closing");
// Create a PathInfo for it
let path_info = PathInfo {
node: Some(proto::Node {
node: Some(proto::node::Node::File(FileNode {
name: HELLOWORLD_BLOB_NAME.into(),
digest: fixtures::HELLOWORLD_BLOB_DIGEST.clone().into(),
size: fixtures::HELLOWORLD_BLOB_CONTENTS.len() as u32,
executable: true,
})),
}),
..Default::default()
};
path_info_service.put(path_info).expect("must succeed");
}
fn populate_symlink(
_blob_service: &Arc<dyn BlobService>,
_directory_service: &Arc<dyn DirectoryService>,
@ -760,6 +792,7 @@ async fn check_attributes() {
populate_blob_a(&blob_service, &directory_service, &path_info_service).await;
populate_directory_with_keep(&blob_service, &directory_service, &path_info_service).await;
populate_symlink(&blob_service, &directory_service, &path_info_service);
populate_helloworld_blob(&blob_service, &directory_service, &path_info_service).await;
let fuser_session = do_mount(
blob_service,
@ -773,14 +806,17 @@ async fn check_attributes() {
let p_file = tmpdir.path().join(BLOB_A_NAME);
let p_directory = tmpdir.path().join(DIRECTORY_WITH_KEEP_NAME);
let p_symlink = tmpdir.path().join(SYMLINK_NAME);
let p_executable_file = tmpdir.path().join(HELLOWORLD_BLOB_NAME);
// peek at metadata. We use symlink_metadata to ensure we don't traverse a symlink by accident.
let metadata_file = fs::symlink_metadata(&p_file).expect("must succeed");
let metadata_executable_file = fs::symlink_metadata(&p_executable_file).expect("must succeed");
let metadata_directory = fs::symlink_metadata(&p_directory).expect("must succeed");
let metadata_symlink = fs::symlink_metadata(&p_symlink).expect("must succeed");
// modes should match. We & with 0o777 to remove any higher bits.
assert_eq!(0o444, metadata_file.mode() & 0o777);
assert_eq!(0o555, metadata_executable_file.mode() & 0o777);
assert_eq!(0o555, metadata_directory.mode() & 0o777);
assert_eq!(0o444, metadata_symlink.mode() & 0o777);