feat(nix-compat/narinfo/signature): generalize name field r/8540

Requiring `name` to be a `&str` means it'll get annoying to pass around
`Signature`, but being able to pass them around in an owned fashion is
kinda a requirement for a stronger typed `PathInfo` struct, where we
want to have full ownership.

Rework the `Signature` struct to become generic over the type of the
`name` field. This means, it becomes possible to have owned versions
of it.

We don't want to impose `String` or `SmolStr` for example, but want to
leave it up to the nix-compat user to decide.

Provide a type alias for the existing `&str` variant (`SignatureRef`),
and use it where we previously used the non-generic `Signature` one.

Add some tests to ensure it's possible to *use* `Signature` with both
`String` and `SmolStr` (but only pull in `smol_str` as dev dependency
for the tests).

Also, add some more docstrings, these were a bit sparse.

Change-Id: I3f75691498c6bda9cd072d2d9dac83c4f6c57287
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12253
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>

9 files changed, 117 insertions, 45 deletions

diff --git a/tvix/Cargo.lock b/tvix/Cargo.lock
index 3eddfcbe7d33..d61877ded625 100644
--- a/tvix/Cargo.lock
+++ b/tvix/Cargo.lock
@@ -2357,6 +2357,7 @@ dependencies = [
  "serde",
  "serde_json",
  "sha2",
+ "smol_str",
  "thiserror",
  "tokio",
  "tokio-test",
@@ -2917,7 +2918,7 @@ checksum = "5bb182580f71dd070f88d01ce3de9f4da5021db7115d2e1c3605a754153b77c1"
 dependencies = [
  "bytes",
  "heck",
- "itertools 0.10.5",
+ "itertools 0.13.0",
  "log",
  "multimap",
  "once_cell",
@@ -2950,7 +2951,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "18bec9b0adc4eba778b33684b7ba3e7137789434769ee3ce3930463ef904cfca"
 dependencies = [
  "anyhow",
- "itertools 0.10.5",
+ "itertools 0.13.0",
  "proc-macro2",
  "quote",
  "syn 2.0.48",
@@ -3878,9 +3879,9 @@ checksum = "e6ecd384b10a64542d77071bd64bd7b231f4ed5940fba55e98c3de13824cf3d7"
 
 [[package]]
 name = "smol_str"
-version = "0.2.1"
+version = "0.2.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e6845563ada680337a52d43bb0b29f396f2d911616f6573012645b9e3d048a49"
+checksum = "dd538fb6910ac1099850255cf94a94df6551fbdd602454387d0adb2d1ca6dead"
 dependencies = [
  "serde",
 ]
diff --git a/tvix/Cargo.nix b/tvix/Cargo.nix
index 54cbcf20721f..a9990b2e72d9 100644
--- a/tvix/Cargo.nix
+++ b/tvix/Cargo.nix
@@ -7481,6 +7481,10 @@ rec {
             packageId = "serde_json";
           }
           {
+            name = "smol_str";
+            packageId = "smol_str";
+          }
+          {
             name = "tokio-test";
             packageId = "tokio-test";
           }
@@ -9301,7 +9305,7 @@ rec {
           }
           {
             name = "itertools";
-            packageId = "itertools 0.10.5";
+            packageId = "itertools 0.13.0";
             usesDefaultFeatures = false;
             features = [ "use_alloc" ];
           }
@@ -9421,7 +9425,7 @@ rec {
           }
           {
             name = "itertools";
-            packageId = "itertools 0.10.5";
+            packageId = "itertools 0.13.0";
           }
           {
             name = "proc-macro2";
@@ -12491,9 +12495,9 @@ rec {
       };
       "smol_str" = rec {
         crateName = "smol_str";
-        version = "0.2.1";
+        version = "0.2.2";
         edition = "2018";
-        sha256 = "0jca0hyrwnv428q5gxhn2s8jsvrrkyrb0fyla9x37056mmimb176";
+        sha256 = "1bfylqf2vnqaglw58930vpxm2rfzji5gjp15a2c0kh8aj6v8ylyx";
         authors = [
           "Aleksey Kladov <aleksey.kladov@gmail.com>"
         ];
diff --git a/tvix/nix-compat/Cargo.toml b/tvix/nix-compat/Cargo.toml
index dc77c5cf0ad1..e399d5a2254a 100644
--- a/tvix/nix-compat/Cargo.toml
+++ b/tvix/nix-compat/Cargo.toml
@@ -46,6 +46,7 @@ mimalloc = "0.1.43"
 pretty_assertions = "1.4.0"
 rstest = "0.19.0"
 serde_json = "1.0"
+smol_str = "0.2.2"
 tokio-test = "0.4.3"
 zstd = "^0.13.0"
 
diff --git a/tvix/nix-compat/src/narinfo/mod.rs b/tvix/nix-compat/src/narinfo/mod.rs
index a77eba200f8d..21aecf80b5a2 100644
--- a/tvix/nix-compat/src/narinfo/mod.rs
+++ b/tvix/nix-compat/src/narinfo/mod.rs
@@ -32,7 +32,7 @@ mod signing_keys;
 mod verifying_keys;
 
 pub use fingerprint::fingerprint;
-pub use signature::{Error as SignatureError, Signature};
+pub use signature::{Error as SignatureError, Signature, SignatureRef};
 pub use signing_keys::parse_keypair;
 pub use signing_keys::{Error as SigningKeyError, SigningKey};
 pub use verifying_keys::{Error as VerifyingKeyError, VerifyingKey};
@@ -51,7 +51,7 @@ pub struct NarInfo<'a> {
     pub references: Vec<StorePathRef<'a>>,
     // authenticity
     /// Ed25519 signature over the path fingerprint
-    pub signatures: Vec<Signature<'a>>,
+    pub signatures: Vec<SignatureRef<'a>>,
     /// Content address (for content-defined paths)
     pub ca: Option<CAHash>,
     // derivation metadata
@@ -246,7 +246,7 @@ impl<'a> NarInfo<'a> {
                     };
                 }
                 "Sig" => {
-                    let val = Signature::parse(val)
+                    let val = SignatureRef::parse(val)
                         .map_err(|e| Error::UnableToParseSignature(signatures.len(), e))?;
 
                     signatures.push(val);
@@ -578,7 +578,7 @@ CA: fixed:r:sha1:1ak1ymbmsfx7z8kh09jzkr3a4dvkrfjw
 
         // ensure the signature is added
         let new_sig = narinfo.signatures.last().unwrap();
-        assert_eq!(signing_key.name(), new_sig.name());
+        assert_eq!(signing_key.name(), *new_sig.name());
 
         // verify the new signature against the verifying key
         let verifying_key = super::VerifyingKey::parse(super::DUMMY_VERIFYING_KEY)
diff --git a/tvix/nix-compat/src/narinfo/signature.rs b/tvix/nix-compat/src/narinfo/signature.rs
index e5567f44109a..33c49128c2d5 100644
--- a/tvix/nix-compat/src/narinfo/signature.rs
+++ b/tvix/nix-compat/src/narinfo/signature.rs
@@ -1,4 +1,7 @@
-use std::fmt::{self, Display};
+use std::{
+    fmt::{self, Display},
+    ops::Deref,
+};
 
 use data_encoding::BASE64;
 use serde::{Deserialize, Serialize};
@@ -6,17 +9,36 @@ use serde::{Deserialize, Serialize};
 const SIGNATURE_LENGTH: usize = std::mem::size_of::<ed25519::SignatureBytes>();
 
 #[derive(Clone, Debug, Eq, PartialEq)]
-pub struct Signature<'a> {
-    name: &'a str,
+pub struct Signature<S> {
+    name: S,
     bytes: ed25519::SignatureBytes,
 }
 
-impl<'a> Signature<'a> {
-    pub fn new(name: &'a str, bytes: ed25519::SignatureBytes) -> Self {
+/// Type alias of a [Signature] using a `&str` as `name` field.
+pub type SignatureRef<'a> = Signature<&'a str>;
+
+/// Represents the signatures that Nix emits.
+/// It consists of a name (an identifier for a public key), and an ed25519
+/// signature (64 bytes).
+/// It is generic over the string type that's used for the name, and there's
+/// [SignatureRef] as a type alias for one containing &str.
+impl<S> Signature<S>
+where
+    S: Deref<Target = str>,
+{
+    /// Constructs a new [Signature] from a name and public key.
+    pub fn new(name: S, bytes: ed25519::SignatureBytes) -> Self {
         Self { name, bytes }
     }
 
-    pub fn parse(input: &'a str) -> Result<Self, Error> {
+    /// Parses a [Signature] from a string containing the name, a colon, and 64
+    /// base64-encoded bytes (plus padding).
+    /// These strings are commonly seen in the `Signature:` field of a NARInfo
+    /// file.
+    pub fn parse<'a>(input: &'a str) -> Result<Self, Error>
+    where
+        S: From<&'a str>,
+    {
         let (name, bytes64) = input.split_once(':').ok_or(Error::MissingSeparator)?;
 
         if name.is_empty()
@@ -40,13 +62,18 @@ impl<'a> Signature<'a> {
             Err(_) => return Err(Error::DecodeError(input.to_string())),
         }
 
-        Ok(Signature { name, bytes })
+        Ok(Self {
+            name: name.into(),
+            bytes,
+        })
     }
 
-    pub fn name(&self) -> &'a str {
-        self.name
+    /// Returns the name field of the signature.
+    pub fn name(&self) -> &S {
+        &self.name
     }
 
+    /// Returns the 64 bytes of signatures.
     pub fn bytes(&self) -> &ed25519::SignatureBytes {
         &self.bytes
     }
@@ -57,9 +84,21 @@ impl<'a> Signature<'a> {
 
         verifying_key.verify_strict(fingerprint, &signature).is_ok()
     }
+
+    /// Constructs a [SignatureRef] from this signature.
+    pub fn as_ref(&self) -> SignatureRef<'_> {
+        SignatureRef {
+            name: self.name.deref(),
+            bytes: self.bytes,
+        }
+    }
 }
 
-impl<'de: 'a, 'a> Deserialize<'de> for Signature<'a> {
+impl<'a, 'de, S> Deserialize<'de> for Signature<S>
+where
+    S: Deref<Target = str> + From<&'a str>,
+    'de: 'a,
+{
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: serde::Deserializer<'de>,
@@ -71,10 +110,13 @@ impl<'de: 'a, 'a> Deserialize<'de> for Signature<'a> {
     }
 }
 
-impl<'a> Serialize for Signature<'a> {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+impl<S: Display> Serialize for Signature<S>
+where
+    S: Deref<Target = str>,
+{
+    fn serialize<SR>(&self, serializer: SR) -> Result<SR::Ok, SR::Error>
     where
-        S: serde::Serializer,
+        SR: serde::Serializer,
     {
         let string: String = self.to_string();
 
@@ -82,6 +124,15 @@ impl<'a> Serialize for Signature<'a> {
     }
 }
 
+impl<S> Display for Signature<S>
+where
+    S: Display,
+{
+    fn fmt(&self, w: &mut fmt::Formatter) -> fmt::Result {
+        write!(w, "{}:{}", self.name, BASE64.encode(&self.bytes))
+    }
+}
+
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
     #[error("Invalid name: {0}")]
@@ -94,12 +145,6 @@ pub enum Error {
     DecodeError(String),
 }
 
-impl Display for Signature<'_> {
-    fn fmt(&self, w: &mut fmt::Formatter) -> fmt::Result {
-        write!(w, "{}:{}", self.name, BASE64.encode(&self.bytes))
-    }
-}
-
 #[cfg(test)]
 mod test {
     use data_encoding::BASE64;
@@ -144,7 +189,7 @@ mod test {
         #[case] fp: &str,
         #[case] expect_valid: bool,
     ) {
-        let sig = Signature::parse(sig_str).expect("must parse");
+        let sig = Signature::<&str>::parse(sig_str).expect("must parse");
         assert_eq!(expect_valid, sig.verify(fp.as_bytes(), verifying_key));
     }
 
@@ -159,7 +204,7 @@ mod test {
         "u01BybwQhyI5H1bW1EIWXssMDhDDIvXOG5uh8Qzgdyjz6U1qg6DHhMAvXZOUStIj6X5t4/ufFgR8i3fjf0bMAw=="
     )]
     fn parse_fail(#[case] input: &'static str) {
-        Signature::parse(input).expect_err("must fail");
+        Signature::<&str>::parse(input).expect_err("must fail");
     }
 
     #[test]
@@ -178,8 +223,29 @@ mod test {
         let serialized = serde_json::to_string(&signature_actual).expect("must serialize");
         assert_eq!(signature_str_json, &serialized);
 
-        let deserialized: Signature<'_> =
+        let deserialized: Signature<&str> =
             serde_json::from_str(signature_str_json).expect("must deserialize");
         assert_eq!(&signature_actual, &deserialized);
     }
+
+    /// Construct a [Signature], using different String types for the name field.
+    #[test]
+    fn signature_owned() {
+        let signature1 = Signature::<String>::parse("cache.nixos.org-1:TsTTb3WGTZKphvYdBHXwo6weVILmTytUjLB+vcX89fOjjRicCHmKA4RCPMVLkj6TMJ4GMX3HPVWRdD1hkeKZBQ==").expect("must parse");
+        let signature2 = Signature::<smol_str::SmolStr>::parse("cache.nixos.org-1:TsTTb3WGTZKphvYdBHXwo6weVILmTytUjLB+vcX89fOjjRicCHmKA4RCPMVLkj6TMJ4GMX3HPVWRdD1hkeKZBQ==").expect("must parse");
+        let signature3 = Signature::<&str>::parse("cache.nixos.org-1:TsTTb3WGTZKphvYdBHXwo6weVILmTytUjLB+vcX89fOjjRicCHmKA4RCPMVLkj6TMJ4GMX3HPVWRdD1hkeKZBQ==").expect("must parse");
+
+        assert!(
+            signature1.verify(FINGERPRINT.as_bytes(), &PUB_CACHE_NIXOS_ORG_1),
+            "must verify"
+        );
+        assert!(
+            signature2.verify(FINGERPRINT.as_bytes(), &PUB_CACHE_NIXOS_ORG_1),
+            "must verify"
+        );
+        assert!(
+            signature3.verify(FINGERPRINT.as_bytes(), &PUB_CACHE_NIXOS_ORG_1),
+            "must verify"
+        );
+    }
 }
diff --git a/tvix/nix-compat/src/narinfo/signing_keys.rs b/tvix/nix-compat/src/narinfo/signing_keys.rs
index e33687bc88f8..cf513b7ba475 100644
--- a/tvix/nix-compat/src/narinfo/signing_keys.rs
+++ b/tvix/nix-compat/src/narinfo/signing_keys.rs
@@ -7,7 +7,7 @@
 use data_encoding::BASE64;
 use ed25519_dalek::{PUBLIC_KEY_LENGTH, SECRET_KEY_LENGTH};
 
-use super::{Signature, VerifyingKey};
+use super::{SignatureRef, VerifyingKey};
 
 pub struct SigningKey<S> {
     name: String,
@@ -23,9 +23,9 @@ where
         Self { name, signing_key }
     }
 
-    /// Signs a fingerprint using the internal signing key, returns the [Signature]
-    pub(crate) fn sign<'a>(&'a self, fp: &[u8]) -> Signature<'a> {
-        Signature::new(&self.name, self.signing_key.sign(fp).to_bytes())
+    /// Signs a fingerprint using the internal signing key, returns the [SignatureRef]
+    pub(crate) fn sign<'a>(&'a self, fp: &[u8]) -> SignatureRef<'a> {
+        SignatureRef::new(&self.name, self.signing_key.sign(fp).to_bytes())
     }
 
     pub fn name(&self) -> &str {
diff --git a/tvix/nix-compat/src/narinfo/verifying_keys.rs b/tvix/nix-compat/src/narinfo/verifying_keys.rs
index b8ed2b9531c1..67ef2e3a459c 100644
--- a/tvix/nix-compat/src/narinfo/verifying_keys.rs
+++ b/tvix/nix-compat/src/narinfo/verifying_keys.rs
@@ -6,7 +6,7 @@ use std::fmt::Display;
 use data_encoding::BASE64;
 use ed25519_dalek::PUBLIC_KEY_LENGTH;
 
-use super::Signature;
+use super::SignatureRef;
 
 /// This represents a ed25519 public key and "name".
 /// These are normally passed in the `trusted-public-keys` Nix config option,
@@ -69,8 +69,8 @@ impl VerifyingKey {
     /// which means the name in the signature has to match,
     /// and the signature bytes themselves need to be a valid signature made by
     /// the signing key identified by [Self::verifying key].
-    pub fn verify(&self, fingerprint: &str, signature: &Signature) -> bool {
-        if self.name() != signature.name() {
+    pub fn verify(&self, fingerprint: &str, signature: &SignatureRef<'_>) -> bool {
+        if self.name() != *signature.name() {
             return false;
         }
 
@@ -109,7 +109,7 @@ mod test {
     use ed25519_dalek::PUBLIC_KEY_LENGTH;
     use rstest::rstest;
 
-    use crate::narinfo::Signature;
+    use crate::narinfo::SignatureRef;
 
     use super::VerifyingKey;
     const FINGERPRINT: &str = "1;/nix/store/syd87l2rxw8cbsxmxl853h0r6pdwhwjr-curl-7.82.0-bin;sha256:1b4sb93wp679q4zx9k1ignby1yna3z7c4c2ri3wphylbc2dwsys0;196040;/nix/store/0jqd0rlxzra1rs38rdxl43yh6rxchgc6-curl-7.82.0,/nix/store/6w8g7njm4mck5dmjxws0z1xnrxvl81xa-glibc-2.34-115,/nix/store/j5jxw3iy7bbz4a57fh9g2xm2gxmyal8h-zlib-1.2.12,/nix/store/yxvjs9drzsphm9pcf42a4byzj1kb9m7k-openssl-1.1.1n";
@@ -146,7 +146,7 @@ mod test {
         #[case] expected: bool,
     ) {
         let pubkey = VerifyingKey::parse(pubkey_str).expect("must parse");
-        let signature = Signature::parse(signature_str).expect("must parse");
+        let signature = SignatureRef::parse(signature_str).expect("must parse");
 
         assert_eq!(expected, pubkey.verify(fingerprint, &signature));
     }
diff --git a/tvix/store/src/proto/mod.rs b/tvix/store/src/proto/mod.rs
index 13b24f3aaeb4..e6b411664037 100644
--- a/tvix/store/src/proto/mod.rs
+++ b/tvix/store/src/proto/mod.rs
@@ -221,7 +221,7 @@ impl PathInfo {
                 .signatures
                 .iter()
                 .map(|sig| {
-                    nix_compat::narinfo::Signature::new(
+                    nix_compat::narinfo::SignatureRef::new(
                         &sig.name,
                         // This shouldn't pass validation
                         sig.data[..].try_into().expect("invalid signature len"),
diff --git a/tvix/tools/narinfo2parquet/src/main.rs b/tvix/tools/narinfo2parquet/src/main.rs
index b96b134f103a..ea3d39af5503 100644
--- a/tvix/tools/narinfo2parquet/src/main.rs
+++ b/tvix/tools/narinfo2parquet/src/main.rs
@@ -187,7 +187,7 @@ impl FrameBuilder {
         assert!(entry.signatures.len() <= 1);
         self.signature
             .append_option(entry.signatures.get(0).map(|sig| {
-                assert_eq!(sig.name(), "cache.nixos.org-1");
+                assert_eq!(sig.name(), &"cache.nixos.org-1");
                 sig.bytes()
             }));