💥 Set missing chmod and chown metadata

ChanTsune · ChanTsune · commit 5eee4530649a · 2026-05-24T09:03:12.000+09:00
diff --git a/cli/src/command/chmod.rs b/cli/src/command/chmod.rs
@@ -18,7 +18,7 @@ use nom::{
     combinator::{map, opt},
     multi::{many0, many1, separated_list1},
 };
-use pna::NormalEntry;
+use pna::{DataKind, NormalEntry};
 use std::{ops::BitOr, path::PathBuf, str::FromStr};
 
 #[derive(Parser, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
@@ -100,9 +100,9 @@ fn archive_chmod(args: ChmodCommand) -> anyhow::Result<()> {
 fn transform_entry<T>(entry: NormalEntry<T>, mode: &Mode) -> NormalEntry<T> {
     let metadata = entry.metadata().clone();
     let own = crate::ext::ResolvedOwnership::from_metadata(&metadata);
-    let Some(cur_mode) = own.mode else {
-        return entry.with_metadata(metadata);
-    };
+    let cur_mode = own
+        .mode
+        .unwrap_or_else(|| default_permission_mode(entry.header().data_kind()));
     let new_mode = mode.apply_to(cur_mode);
     let metadata = metadata
         .with_permission(None)
@@ -132,6 +132,14 @@ fn transform_entry<T>(entry: NormalEntry<T>, mode: &Mode) -> NormalEntry<T> {
     entry.with_metadata(metadata)
 }
 
+#[inline]
+const fn default_permission_mode(kind: DataKind) -> u16 {
+    match kind {
+        DataKind::Directory => 0o755,
+        DataKind::File | DataKind::SymbolicLink | DataKind::HardLink => 0o644,
+    }
+}
+
 bitflags! {
     #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
     pub(crate) struct Who: u8 {
diff --git a/cli/src/command/chown.rs b/cli/src/command/chown.rs
@@ -109,9 +109,6 @@ fn archive_chown(args: ChownCommand) -> anyhow::Result<()> {
 fn transform_entry<T>(entry: NormalEntry<T>, owner: &Ownership) -> NormalEntry<T> {
     let metadata = entry.metadata().clone();
     let own = crate::ext::ResolvedOwnership::from_metadata(&metadata);
-    if own.is_empty() {
-        return entry.with_metadata(metadata);
-    }
     let (uid, uname): (Option<u64>, String) = match owner.user.as_ref() {
         Some(OwnerSpecifier::Name(uname)) => (Some(u64::MAX), uname.clone()),
         Some(OwnerSpecifier::ID(uid)) => (Some(*uid), String::new()),
diff --git a/cli/tests/cli/chmod.rs b/cli/tests/cli/chmod.rs
@@ -120,3 +120,118 @@ fn archive_chmod_drops_legacy_fprm() {
     })
     .unwrap();
 }
+
+/// Precondition: An archive entry has no permission metadata.
+/// Action: Run `pna experimental chmod 600` on that entry.
+/// Expectation: The requested mode is emitted as fMOd without generating legacy fPRM.
+#[test]
+#[allow(deprecated)]
+fn archive_chmod_numeric_sets_missing_permission_mode() {
+    setup();
+    let path = "chmod_missing_mode_numeric.pna";
+    {
+        let mut archive = Archive::write_header(File::create(path).unwrap()).unwrap();
+        let mut builder = EntryBuilder::new_file(
+            EntryName::from_utf8_preserve_root(ENTRY_PATH),
+            WriteOptions::store(),
+        )
+        .unwrap();
+        builder.write_all(ENTRY_CONTENT).unwrap();
+        archive.add_entry(builder.build().unwrap()).unwrap();
+        archive.finalize().unwrap();
+    }
+
+    cli::Cli::try_parse_from([
+        "pna",
+        "--quiet",
+        "experimental",
+        "chmod",
+        "-f",
+        path,
+        "600",
+        ENTRY_PATH,
+    ])
+    .unwrap()
+    .execute()
+    .unwrap();
+
+    archive::for_each_entry(path, |entry| {
+        assert!(
+            entry.metadata().permission().is_none(),
+            "legacy fPRM must not be generated by chmod"
+        );
+        assert_eq!(entry.metadata().permission_mode().unwrap().get(), 0o600);
+    })
+    .unwrap();
+}
+
+/// Precondition: Archive entries of each supported kind have no permission metadata.
+/// Action: Run symbolic `pna experimental chmod a-r` on all entries.
+/// Expectation: Missing modes use default bases: directory 0755, all other
+/// entry kinds 0644.
+#[test]
+fn archive_chmod_symbolic_uses_default_mode_for_missing_permission() {
+    setup();
+    let path = "chmod_missing_mode_symbolic.pna";
+    {
+        let mut archive = Archive::write_header(File::create(path).unwrap()).unwrap();
+
+        let mut file = EntryBuilder::new_file("file.txt".into(), WriteOptions::store()).unwrap();
+        file.write_all(b"file").unwrap();
+        archive.add_entry(file.build().unwrap()).unwrap();
+
+        archive
+            .add_entry(EntryBuilder::new_dir("dir".into()).build().unwrap())
+            .unwrap();
+
+        archive
+            .add_entry(
+                EntryBuilder::new_symlink("link.txt".into(), "file.txt".into())
+                    .unwrap()
+                    .build()
+                    .unwrap(),
+            )
+            .unwrap();
+
+        archive
+            .add_entry(
+                EntryBuilder::new_hard_link("hard.txt".into(), "file.txt".into())
+                    .unwrap()
+                    .build()
+                    .unwrap(),
+            )
+            .unwrap();
+
+        archive.finalize().unwrap();
+    }
+
+    cli::Cli::try_parse_from([
+        "pna",
+        "--quiet",
+        "experimental",
+        "chmod",
+        "-f",
+        path,
+        "a-r",
+        "file.txt",
+        "dir",
+        "link.txt",
+        "hard.txt",
+    ])
+    .unwrap()
+    .execute()
+    .unwrap();
+
+    archive::for_each_entry(path, |entry| {
+        let expected = match entry.header().path().as_str() {
+            "dir" => 0o311,
+            "file.txt" | "link.txt" | "hard.txt" => 0o200,
+            other => panic!("unexpected entry: {other}"),
+        };
+        assert_eq!(
+            entry.metadata().permission_mode().unwrap().get() & 0o777,
+            expected
+        );
+    })
+    .unwrap();
+}
diff --git a/cli/tests/cli/chown/preserve_owner_absence.rs b/cli/tests/cli/chown/preserve_owner_absence.rs
@@ -9,8 +9,8 @@ use std::io::Write;
 /// uid/gid/name), plus an entry with no ownership metadata at all.
 /// Action: `pna experimental chown` changing only the user.
 /// Expectation: The un-overridden group side stays absent (no gid facet, never
-/// a synthesized 0); the overridden user side is set; mode is preserved; the
-/// no-ownership entry is left untouched.
+/// a synthesized 0); the overridden user side is set even for a metadata-empty
+/// entry; existing mode is preserved; absent mode stays absent.
 #[test]
 fn chown_user_only_preserves_missing_gid() {
     setup();
@@ -44,6 +44,7 @@ fn chown_user_only_preserves_missing_gid() {
         path,
         "new_user",
         "mode_only.txt",
+        "bare.txt",
         "--no-owner-lookup",
     ])
     .unwrap()
@@ -69,9 +70,9 @@ fn chown_user_only_preserves_missing_gid() {
                 assert_eq!(m.permission_mode().unwrap().get(), 0o644);
             }
             "bare.txt" => {
-                assert!(m.owner_uid().is_none());
+                assert_eq!(m.owner_user_name().unwrap().as_str(), "new_user");
+                assert_eq!(m.owner_uid().unwrap().get(), u64::MAX);
                 assert!(m.owner_gid().is_none());
-                assert!(m.owner_user_name().is_none());
                 assert!(m.owner_group_name().is_none());
                 assert!(m.permission_mode().is_none());
             }
@@ -82,7 +83,8 @@ fn chown_user_only_preserves_missing_gid() {
     assert_eq!(count, 2, "archive should contain exactly 2 entries");
 }
 
-/// Precondition: An archive entry carries only a permission mode (no owner uid/gid).
+/// Precondition: Archive entries carry either only a permission mode or no
+/// ownership metadata at all.
 /// Action: `pna experimental chown` changing only the group (`:new_grp`).
 /// Expectation: The un-overridden user side stays absent (no uid facet, never a synthesized 0).
 #[test]
@@ -99,6 +101,13 @@ fn chown_group_only_preserves_missing_uid() {
         mo.permission_mode(pna::PermissionMode::from(0o600));
         mo.write_all(b"m").unwrap();
         a.add_entry(mo.build().unwrap()).unwrap();
+        let mut bare = EntryBuilder::new_file(
+            EntryName::from_utf8_preserve_root("bare.txt"),
+            WriteOptions::store(),
+        )
+        .unwrap();
+        bare.write_all(b"x").unwrap();
+        a.add_entry(bare.build().unwrap()).unwrap();
         a.finalize().unwrap();
     }
 
@@ -111,6 +120,7 @@ fn chown_group_only_preserves_missing_uid() {
         path,
         ":new_grp",
         "mode_only.txt",
+        "bare.txt",
         "--no-owner-lookup",
     ])
     .unwrap()
@@ -119,14 +129,26 @@ fn chown_group_only_preserves_missing_uid() {
 
     archive::for_each_entry(path, |entry| {
         let m = entry.metadata();
-        assert_eq!(m.owner_group_name().unwrap().as_str(), "new_grp");
-        assert_eq!(m.owner_gid().unwrap().get(), u64::MAX);
-        assert!(
-            m.owner_uid().is_none(),
-            "un-overridden uid must stay absent, not synthesized to 0"
-        );
-        assert!(m.owner_user_name().is_none());
-        assert_eq!(m.permission_mode().unwrap().get(), 0o600);
+        match entry.header().path().as_str() {
+            "mode_only.txt" => {
+                assert_eq!(m.owner_group_name().unwrap().as_str(), "new_grp");
+                assert_eq!(m.owner_gid().unwrap().get(), u64::MAX);
+                assert!(
+                    m.owner_uid().is_none(),
+                    "un-overridden uid must stay absent, not synthesized to 0"
+                );
+                assert!(m.owner_user_name().is_none());
+                assert_eq!(m.permission_mode().unwrap().get(), 0o600);
+            }
+            "bare.txt" => {
+                assert_eq!(m.owner_group_name().unwrap().as_str(), "new_grp");
+                assert_eq!(m.owner_gid().unwrap().get(), u64::MAX);
+                assert!(m.owner_uid().is_none());
+                assert!(m.owner_user_name().is_none());
+                assert!(m.permission_mode().is_none());
+            }
+            other => panic!("unexpected entry: {other}"),
+        }
     })
     .unwrap();
 }
