builder: Make add_pax() return an error in GNU mode

Previously, add_pax() accepted calls regardless of ExtensionMode but
finish() only emitted PAX records in Pax mode -- silently discarding
any PAX data added in Gnu mode. The doc comment incorrectly claimed
PAX extensions would be emitted regardless of mode.

Return HeaderError::IncompatibleMode instead, so callers get a clear
error rather than quietly losing xattrs or other PAX metadata.

This is a breaking API change: add_pax() now returns Result<&mut Self>
instead of &mut Self.

Assisted-by: OpenCode (Claude Opus 4)
Signed-off-by: Colin Walters <walters@verbum.org>

Author: cgwalters

fuzz/generate_corpus.rs

@@ -272,7 +272,9 @@ fn main() {
             .unwrap()
             .size(0)
             .unwrap();
-        builder.add_pax("SCHILY.xattr.user.test", b"value123");
+        builder
+            .add_pax("SCHILY.xattr.user.test", b"value123")
+            .unwrap();
         let hdr = builder.finish_bytes();
         let mut archive = hdr;
         archive.extend_from_slice(&EOA);
@@ -292,10 +294,12 @@ fn main() {
             .unwrap()
             .mtime(9999999999)
             .unwrap();
-        builder.add_pax(
-            "SCHILY.xattr.security.selinux",
-            b"system_u:object_r:usr_t:s0",
-        );
+        builder
+            .add_pax(
+                "SCHILY.xattr.security.selinux",
+                b"system_u:object_r:usr_t:s0",
+            )
+            .unwrap();
         let hdr = builder.finish_bytes();
         let mut archive = hdr;
         archive.extend_from_slice(b"payload");
@@ -512,7 +516,7 @@ fn main() {
             .unwrap()
             .mtime(1700000000)
             .unwrap();
-        builder.add_pax("mtime", b"1700000000.123456789");
+        builder.add_pax("mtime", b"1700000000.123456789").unwrap();
         let hdr = builder.finish_bytes();
         let mut archive = hdr;
         archive.extend_from_slice(&EOA);

src/builder.rs

@@ -863,11 +863,23 @@ impl EntryBuilder {
     /// Add a custom PAX extension record.
     ///
     /// This is useful for adding metadata that doesn't fit in standard
-    /// header fields. The PAX extension will be emitted regardless of
-    /// the extension mode setting.
-    pub fn add_pax(&mut self, key: &str, value: impl AsRef<[u8]>) -> &mut Self {
+    /// header fields (e.g. extended attributes, high-precision timestamps).
+    ///
+    /// # Errors
+    ///
+    /// Returns [`HeaderError::IncompatibleMode`] if the extension mode is
+    /// [`ExtensionMode::Gnu`]. PAX records are only emitted in PAX mode;
+    /// call [`Self::set_extension_mode`] first or use [`Self::new_ustar`]
+    /// to create a PAX-mode builder.
+    pub fn add_pax(&mut self, key: &str, value: impl AsRef<[u8]>) -> Result<&mut Self> {
+        if self.mode != ExtensionMode::Pax {
+            return Err(HeaderError::IncompatibleMode {
+                required: ExtensionMode::Pax,
+                current: self.mode,
+            });
+        }
         self.pax_mut().add(key, value);
-        self
+        Ok(self)
     }
 
     /// Get or create the PAX builder for this entry.
@@ -1861,6 +1873,7 @@ mod tests {
             .size(0)
             .unwrap()
             .add_pax("SCHILY.xattr.user.test", b"value")
+            .unwrap()
             .entry_type(EntryType::Regular);
 
         assert!(builder.needs_extension()); // Due to custom PAX
@@ -1876,6 +1889,40 @@ mod tests {
         assert!(pax_str.contains("SCHILY.xattr.user.test=value"));
     }
 
+    #[test]
+    fn test_add_pax_rejects_gnu_mode() {
+        let mut builder = EntryBuilder::new_gnu();
+        let result = builder.add_pax("SCHILY.xattr.user.test", b"value");
+        assert!(result.is_err());
+        let err = result.unwrap_err();
+        assert!(
+            err.to_string().contains("Pax"),
+            "error should mention Pax mode: {err}"
+        );
+    }
+
+    #[test]
+    fn test_add_pax_after_mode_switch() {
+        // Switching from GNU to PAX mode should allow add_pax().
+        let mut builder = EntryBuilder::new_gnu();
+        assert!(builder.add_pax("key", b"val").is_err());
+
+        builder.set_extension_mode(ExtensionMode::Pax);
+        builder
+            .path(b"file.txt")
+            .mode(0o644)
+            .unwrap()
+            .size(0)
+            .unwrap()
+            .add_pax("SCHILY.xattr.user.test", b"value")
+            .unwrap();
+
+        let blocks = builder.finish();
+        // Should have PAX 'x' header + PAX data + main header
+        assert!(blocks.len() >= 3);
+        assert_eq!(blocks[0].entry_type(), EntryType::XHeader);
+    }
+
     #[test]
     fn test_entry_builder_extension_mode_switching() {
         let long_path = "x/".repeat(60);

src/lib.rs

@@ -94,6 +94,15 @@ pub enum HeaderError {
         detail: String,
     },
 
+    /// Operation requires a different extension mode.
+    #[error("operation requires {required:?} extension mode, but builder is in {current:?} mode")]
+    IncompatibleMode {
+        /// The mode the operation requires.
+        required: crate::builder::ExtensionMode,
+        /// The mode the builder is currently in.
+        current: crate::builder::ExtensionMode,
+    },
+
     /// The header checksum does not match the computed value.
     #[error("checksum mismatch: expected {expected}, computed {computed}")]
     ChecksumMismatch {
@@ -4103,7 +4112,8 @@ mod tests {
                         .mtime(params.mtime)
                         .unwrap()
                         .entry_type(EntryType::Regular)
-                        .add_pax(&params.xattr_key, params.xattr_value.as_bytes());
+                        .add_pax(&params.xattr_key, params.xattr_value.as_bytes())
+                        .unwrap();
 
                     builder.finish()
                 }