ffi: take sizes and timestamps as strings; both SDKs stop parsing

README.md

@@ -246,7 +246,7 @@ sandlock run --no-supervisor -r /proc -r /usr -r /lib -r /lib64 -r /bin -r /etc
 ### Python API
 
 ```python
-from sandlock import Sandbox, confine
+from sandlock import Mount, Sandbox, confine
 
 sandbox = Sandbox(
     fs_writable=["/tmp/sandbox"],
@@ -272,7 +272,7 @@ result = agent.run(["python3", "agent.py"])
 # Chroot with per-sandbox mount (Docker-style -v, no root needed)
 chrooted = Sandbox(
     chroot="/opt/rootfs",
-    fs_mount={"/work": "/tmp/sandbox-1/work"},  # maps /work inside chroot
+    fs_mount=[Mount("/work", "/tmp/sandbox-1/work")],  # maps /work inside chroot
     fs_readable=["/usr", "/bin", "/lib", "/etc"],
     cwd="/work",
 )

crates/sandlock-cli/Cargo.toml

@@ -20,7 +20,6 @@ anyhow = "1"
 toml = "0.8"
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
-jiff = "0.2"
 libc = "0.2"
 
 [dev-dependencies]

crates/sandlock-cli/src/learn.rs

@@ -10,7 +10,7 @@ use std::sync::{Arc, Mutex};
 use anyhow::{anyhow, Result};
 use sandlock_core::policy_fn::{SyscallEvent, Verdict};
 use sandlock_core::profile::{FilesystemSection, ProfileInput};
-use sandlock_core::sandbox::BranchAction;
+use sandlock_core::sandbox::{BranchAction, ByteSize};
 use sandlock_core::Sandbox;
 
 
@@ -686,7 +686,10 @@ pub async fn run(args: LearnArgs) -> Result<()> {
         profile_out.network.allow_bind = merged_bind;
 
         // Limits: take the max of old vs observed.
-        profile_out.limits.memory = max_bytesize(existing.limits.memory.as_deref(), observed.limits.memory.as_deref());
+        profile_out.limits.memory = max_bytesize(
+            existing.limits.memory.as_deref(),
+            observed.limits.memory.as_deref(),
+        )?;
         profile_out.limits.processes = max_opt(existing.limits.processes, observed.limits.processes);
         profile_out.limits.open_files = max_opt(existing.limits.open_files, observed.limits.open_files);
     }
@@ -727,32 +730,29 @@ pub async fn run(args: LearnArgs) -> Result<()> {
 }
 
 
-/// Parse a bytesize string like "128M", "1G", "512K" into bytes.
-fn parse_bytesize_bytes(s: &str) -> Option<u64> {
-    let s = s.trim();
-    let (num, mult) = if let Some(n) = s.strip_suffix('G') {
-        (n, 1024 * 1024 * 1024u64)
-    } else if let Some(n) = s.strip_suffix('M') {
-        (n, 1024 * 1024u64)
-    } else if let Some(n) = s.strip_suffix('K') {
-        (n, 1024u64)
-    } else {
-        (s, 1u64)
-    };
-    num.trim().parse::<u64>().ok().map(|n| n * mult)
-}
-
 /// Return the larger of two optional bytesize strings.
-fn max_bytesize(a: Option<&str>, b: Option<&str>) -> Option<String> {
-    match (a, b) {
+///
+/// Both sides are read with the core's own grammar. A merge file is written by
+/// hand, so a size it carries is whatever the flag and the profile accept, and
+/// a second grammar here would disagree with them: this one was case sensitive
+/// where `ByteSize::parse` is not, so `512m` resolved to nothing, and the
+/// caller's `unwrap_or(0)` then made it the smaller of the two. A profile
+/// merged against `1M` came back with a ceiling five hundred times lower than
+/// the one it went in with, and nothing said so.
+fn max_bytesize(a: Option<&str>, b: Option<&str>) -> Result<Option<String>> {
+    let parse = |s: &str| {
+        ByteSize::parse(s)
+            .map(|b| b.0)
+            .map_err(|e| anyhow!("[limits].memory in the merge file: {e}"))
+    };
+    Ok(match (a, b) {
         (None, None) => None,
         (Some(s), None) | (None, Some(s)) => Some(s.to_string()),
         (Some(sa), Some(sb)) => {
-            let va = parse_bytesize_bytes(sa).unwrap_or(0);
-            let vb = parse_bytesize_bytes(sb).unwrap_or(0);
+            let (va, vb) = (parse(sa)?, parse(sb)?);
             Some(if va >= vb { sa.to_string() } else { sb.to_string() })
         }
-    }
+    })
 }
 
 /// Return the larger of two optional u32 values.
@@ -763,3 +763,53 @@ fn max_opt(a: Option<u32>, b: Option<u32>) -> Option<u32> {
         (Some(va), Some(vb)) => Some(va.max(vb)),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn merging_a_size_reads_it_with_the_cores_grammar() {
+        // A lowercase suffix is what the flag and the profile both accept, so
+        // it has to mean the same here. The private parser this replaced was
+        // case sensitive, resolved "512m" to nothing, and the caller turned
+        // that into 0, so merging a 512MiB profile against a 1MiB observation
+        // silently rewrote the ceiling down to 1MiB.
+        let merged = max_bytesize(Some("512m"), Some("1M")).unwrap();
+        assert_eq!(merged.as_deref(), Some("512m"));
+
+        // The same pair spelled the way the old parser could read, to show the
+        // comparison itself is unchanged.
+        assert_eq!(
+            max_bytesize(Some("512M"), Some("1M")).unwrap().as_deref(),
+            Some("512M")
+        );
+        assert_eq!(
+            max_bytesize(Some("1M"), Some("512M")).unwrap().as_deref(),
+            Some("512M")
+        );
+    }
+
+    #[test]
+    fn a_size_the_core_refuses_stops_the_merge_instead_of_becoming_zero() {
+        // Out of range and plain nonsense both used to read as 0 and lose the
+        // comparison. Naming the section matters: the value comes from a file
+        // the caller wrote, not from a flag they just typed.
+        for spec in ["17179869184G", "not-a-size", "1.5G"] {
+            let err = max_bytesize(Some(spec), Some("1M"))
+                .expect_err(&format!("{spec} must stop the merge"))
+                .to_string();
+            assert!(
+                err.contains("[limits].memory"),
+                "the error must name the field, got {err:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn one_sided_and_absent_limits_are_carried_through() {
+        assert_eq!(max_bytesize(None, None).unwrap(), None);
+        assert_eq!(max_bytesize(Some("8M"), None).unwrap().as_deref(), Some("8M"));
+        assert_eq!(max_bytesize(None, Some("8M")).unwrap().as_deref(), Some("8M"));
+    }
+}

crates/sandlock-cli/src/main.rs

@@ -4,7 +4,6 @@ use sandlock_core::sandbox::{BranchAction, ByteSize, SandboxBuilder};
 use sandlock_core::profile;
 use anyhow::{Result, anyhow};
 use std::path::PathBuf;
-use std::time::SystemTime;
 
 mod learn;
 #[derive(Parser)]
@@ -610,7 +609,7 @@ async fn run_command(args: RunArgs) -> Result<i32> {
     // CLI overrides — non-clap-friendly fields (still parsed here)
     if let Some(ref m) = args.max_memory { builder = builder.max_memory(ByteSize::parse(m)?); }
     if let Some(ref ts) = args.time_start {
-        let t = parse_time_start(ts)?;
+        let t = profile::parse_time_start(ts, "--time-start")?;
         builder = builder.time_start(t);
     }
     if let Some(ref s) = args.max_disk { builder = builder.max_disk(ByteSize::parse(s)?); }
@@ -919,19 +918,6 @@ fn validate_no_supervisor_profile(profile: &Sandbox, source: &str) -> Result<()>
     Ok(())
 }
 
-/// Render a parsed `NetRule` back into a `--net-allow` / `--net-deny` spec
-/// string, so a profile loaded via `--profile-file` round-trips through the
-/// builder. Allow and deny share one grammar. The scheme is always
-/// rendered: a scheme-less spec parses as a TCP + UDP pair, so a
-/// single-protocol rule must carry its scheme to round-trip exactly.
-/// IPv6 is bracketed only when a port follows, and the all-ports case
-/// drops the redundant `:*`.
-fn parse_time_start(s: &str) -> Result<SystemTime> {
-    let ts: jiff::Timestamp = s.parse()
-        .map_err(|e| anyhow!("invalid --time-start '{}': {}", s, e))?;
-    Ok(ts.into())
-}
-
 fn parse_branch_action(flag: &str, s: &str) -> Result<BranchAction> {
     match s {
         "commit" => Ok(BranchAction::Commit),

crates/sandlock-core/src/context.rs

@@ -346,24 +346,25 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! {
         }
     }
 
-    // 4b. Optional: CPU core binding
+    // 4b. Optional: CPU core binding. A set that reached here is non-empty:
+    // the builder refuses an empty one by name. This used to skip the call for
+    // an empty set instead, which turned "pin me to no core" into "pinning did
+    // not happen" without anyone being told.
     if let Some(ref cores) = sandbox.cpu_cores {
-        if !cores.is_empty() {
-            let mut set = unsafe { std::mem::zeroed::<libc::cpu_set_t>() };
-            unsafe { libc::CPU_ZERO(&mut set) };
-            for &core in cores {
-                unsafe { libc::CPU_SET(core as usize, &mut set) };
-            }
-            if unsafe {
-                libc::sched_setaffinity(
-                    0,
-                    std::mem::size_of::<libc::cpu_set_t>(),
-                    &set,
-                )
-            } != 0
-            {
-                fail!("sched_setaffinity");
-            }
+        let mut set = unsafe { std::mem::zeroed::<libc::cpu_set_t>() };
+        unsafe { libc::CPU_ZERO(&mut set) };
+        for &core in cores {
+            unsafe { libc::CPU_SET(core as usize, &mut set) };
+        }
+        if unsafe {
+            libc::sched_setaffinity(
+                0,
+                std::mem::size_of::<libc::cpu_set_t>(),
+                &set,
+            )
+        } != 0
+        {
+            fail!("sched_setaffinity");
         }
     }
 

crates/sandlock-core/src/http.rs

@@ -179,6 +179,12 @@ pub fn http_acl_check(
 /// allowed to reach the original destination on the intercepted ports. Concrete
 /// HTTP rule hosts tighten the IP allowlist to those hosts; wildcard hosts or
 /// explicit HTTP ports with no rules allow any IP on the HTTP ports.
+///
+/// Derived entries a caller already carries are not added twice. A policy can
+/// be taken apart and rebuilt (`sandlock run --profile-file` rebuilds a builder
+/// from the parsed profile, then applies flag overrides on top), and the
+/// rebuilt net allowlist arrives here already holding the entries this
+/// function added on the first build.
 pub(crate) fn extend_net_allow_for_http(
     net_allow: &mut Vec<NetAllow>,
     http_allow: &[HttpRule],
@@ -189,6 +195,12 @@ pub(crate) fn extend_net_allow_for_http(
         return;
     }
 
+    fn push_unique(net_allow: &mut Vec<NetAllow>, rule: NetAllow) {
+        if !net_allow.contains(&rule) {
+            net_allow.push(rule);
+        }
+    }
+
     let mut wildcard_seen = false;
     let mut concrete_hosts: Vec<String> = Vec::new();
     for rule in http_allow.iter().chain(http_deny.iter()) {
@@ -203,7 +215,7 @@ pub(crate) fn extend_net_allow_for_http(
     }
 
     if wildcard_seen || (http_allow.is_empty() && http_deny.is_empty()) {
-        net_allow.push(NetAllow {
+        push_unique(net_allow, NetAllow {
             protocol: Protocol::Tcp,
             target: NetTarget::AnyIp,
             ports: http_ports.to_vec(),
@@ -212,7 +224,7 @@ pub(crate) fn extend_net_allow_for_http(
     }
 
     for host in concrete_hosts {
-        net_allow.push(NetAllow {
+        push_unique(net_allow, NetAllow {
             protocol: Protocol::Tcp,
             target: NetTarget::Host(host),
             ports: http_ports.to_vec(),
@@ -481,6 +493,29 @@ mod tests {
         assert_eq!(net_allow[1].ports, vec![80, 443]);
     }
 
+    #[test]
+    fn extend_net_allow_for_http_is_idempotent() {
+        // A policy that is taken apart and rebuilt feeds the already derived
+        // entries back in as plain net-allow specs (that is what
+        // `sandlock run --profile-file` does before applying flag overrides),
+        // so a second pass must not grow the allowlist.
+        let allow = vec![HttpRule::parse("GET api.example.com/v1/*").unwrap()];
+        let mut net_allow = Vec::new();
+
+        extend_net_allow_for_http(&mut net_allow, &allow, &[], &[80]);
+        let first = net_allow.clone();
+        extend_net_allow_for_http(&mut net_allow, &allow, &[], &[80]);
+
+        assert_eq!(net_allow, first);
+
+        // Same for the any-IP entry, which comes from a different branch.
+        let mut wide = Vec::new();
+        extend_net_allow_for_http(&mut wide, &[], &[], &[8080]);
+        let first_wide = wide.clone();
+        extend_net_allow_for_http(&mut wide, &[], &[], &[8080]);
+        assert_eq!(wide, first_wide);
+    }
+
     #[test]
     fn extend_net_allow_for_http_adds_any_ip_for_wildcard_or_bare_port() {
         let mut net_allow = Vec::new();