ostree-ext: Adapt to new containers policy config load order

Closes: #2258

The real fix here will be to migrate to `--require-signed`
from skopeo in the future.

Generated-by: AI (with only a few tweaks)
Signed-off-by: Colin Walters <walters@verbum.org>

Author: cgwalters

crates/lib/src/cli.rs

@@ -165,8 +165,9 @@ pub(crate) struct SwitchOpts {
     /// This is the inverse of the previous `--target-no-signature-verification` (which is now
     /// a no-op).
     ///
-    /// Enabling this option enforces that `/etc/containers/policy.json` includes a
-    /// default policy which requires signatures.
+    /// Enabling this option enforces that `containers-policy.json` (see `man
+    /// containers-policy.json` for the full search path) includes a default
+    /// policy which requires signatures.
     #[clap(long)]
     pub(crate) enforce_container_sigpolicy: bool,
 

crates/lib/src/install.rs

@@ -265,8 +265,9 @@ pub(crate) struct InstallTargetOpts {
     pub(crate) target_no_signature_verification: bool,
 
     /// This is the inverse of the previous `--target-no-signature-verification` (which is now
-    /// a no-op).  Enabling this option enforces that `/etc/containers/policy.json` includes a
-    /// default policy which requires signatures.
+    /// a no-op).  Enabling this option enforces that `containers-policy.json` (see `man
+    /// containers-policy.json` for the full search path) includes a default policy which
+    /// requires signatures.
     #[clap(long)]
     #[serde(default)]
     pub(crate) enforce_container_sigpolicy: bool,

crates/ostree-ext/src/container/skopeo.rs

@@ -2,6 +2,7 @@
 
 use super::ImageReference;
 use anyhow::{Context, Result};
+use cap_std_ext::cap_std;
 use cap_std_ext::cmdext::{CapStdExtCommandExt, CmdFds};
 use containers_image_proxy::oci_spec::image as oci_image;
 use fn_error_context::context;
@@ -17,9 +18,84 @@ use tokio::process::Command;
 // https://github.com/containers/image/blob/main/signature/policy_types.go
 // Ideally we add something like `skopeo pull --disallow-insecure-accept-anything`
 // but for now we parse the policy.
-const POLICY_PATH: &str = "/etc/containers/policy.json";
 const INSECURE_ACCEPT_ANYTHING: &str = "insecureAcceptAnything";
 
+/// The env var that overrides the policy path, matching the upstream Go
+/// containers/image library behavior.
+const POLICY_ENV_VAR: &str = "CONTAINERS_POLICY_JSON";
+
+/// Well-known system paths for `containers-policy.json`, checked in order.
+const SYSTEM_POLICY_PATHS: &[&str] = &[
+    "etc/containers/policy.json",
+    "usr/share/containers/policy.json",
+];
+
+/// Suffix appended under `$XDG_CONFIG_HOME` (or `$HOME/.config`).
+const USER_POLICY_SUFFIX: &str = "containers/policy.json";
+
+/// Resolve the containers policy path using the same load order as the
+/// upstream Go containers/image library, with all lookups relative to `root`:
+///
+/// 1. `CONTAINERS_POLICY_JSON` env var (trusted, no existence check)
+/// 2. `$XDG_CONFIG_HOME/containers/policy.json` (or `$HOME/.config/…`)
+/// 3. `/etc/containers/policy.json`
+/// 4. `/usr/share/containers/policy.json`
+///
+/// For candidates 2–4 we only return a path when the file exists on disk.
+///
+/// Absolute paths (from env vars) have their leading `/` stripped so they
+/// resolve under `root`. Passing `root` opened on `/` gives normal behaviour;
+/// tests can pass a cap-std `Dir` backed by a temporary directory.
+fn resolve_policy_path(
+    root: &cap_std::fs::Dir,
+    env_override: Option<&Path>,
+    xdg_config_home: Option<&Path>,
+    home: Option<&Path>,
+) -> Result<cap_std::fs::File> {
+    // Helper: strip a leading `/` so the path is relative to root.
+    fn strip_abs(p: &Path) -> &Path {
+        p.strip_prefix("/").unwrap_or(p)
+    }
+
+    // 1. Env var override – trust unconditionally (no existence check).
+    if let Some(raw) = env_override.filter(|v| !v.as_os_str().is_empty()) {
+        let relative = strip_abs(raw);
+        tracing::debug!("Using policy path from {POLICY_ENV_VAR}: {}", raw.display());
+        return root.open(relative).with_context(|| {
+            format!(
+                "Opening policy file from {POLICY_ENV_VAR}={}",
+                raw.display()
+            )
+        });
+    }
+
+    // 2. Per-user config dir.
+    let user_candidate = if let Some(xdg) = xdg_config_home {
+        Some(strip_abs(xdg).join(USER_POLICY_SUFFIX))
+    } else {
+        home.map(|h| strip_abs(h).join(".config").join(USER_POLICY_SUFFIX))
+    };
+    if let Some(p) = &user_candidate {
+        if let Ok(f) = root.open(p) {
+            tracing::debug!("Using user policy path: {}", p.display());
+            return Ok(f);
+        }
+    }
+
+    // 3–4. System paths.
+    for candidate in SYSTEM_POLICY_PATHS {
+        if let Ok(f) = root.open(candidate) {
+            tracing::debug!("Using system policy path: {candidate}");
+            return Ok(f);
+        }
+    }
+
+    anyhow::bail!(
+        "No containers policy.json found; \
+         checked ${POLICY_ENV_VAR}, user config dir, and system paths"
+    )
+}
+
 #[derive(Deserialize)]
 struct PolicyEntry {
     #[serde(rename = "type")]
@@ -43,8 +119,17 @@ impl ContainerPolicy {
     }
 }
 
-pub(crate) fn container_policy_is_default_insecure() -> Result<bool> {
-    let r = std::io::BufReader::new(std::fs::File::open(POLICY_PATH)?);
+pub(crate) fn container_policy_is_default_insecure(root: &cap_std::fs::Dir) -> Result<bool> {
+    let f = resolve_policy_path(
+        root,
+        std::env::var_os(POLICY_ENV_VAR).as_deref().map(Path::new),
+        std::env::var_os("XDG_CONFIG_HOME")
+            .as_deref()
+            .map(Path::new),
+        std::env::var_os("HOME").as_deref().map(Path::new),
+    )
+    .context("Resolving containers policy path")?;
+    let r = std::io::BufReader::new(f);
     let policy: ContainerPolicy = serde_json::from_reader(r)?;
     Ok(policy.is_default_insecure())
 }
@@ -106,6 +191,7 @@ pub async fn copy(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use cap_std_ext::cap_tempfile;
 
     // Default value as of the Fedora 34 containers-common-1-21.fc34.noarch package.
     const DEFAULT_POLICY: &str = indoc::indoc! {r#"
@@ -155,4 +241,84 @@ mod tests {
             assert!(!p.is_default_insecure());
         }
     }
+
+    /// Create `<dir>/<path>` with empty JSON content, creating parent dirs.
+    /// Returns the (dev, ino) of the created file for identity checks.
+    fn touch(dir: &cap_std::fs::Dir, path: &str) -> (u64, u64) {
+        use cap_std::fs::MetadataExt;
+        if let Some(parent) = Path::new(path).parent() {
+            dir.create_dir_all(parent).unwrap();
+        }
+        dir.write(path, b"{}").unwrap();
+        let m = dir.metadata(path).unwrap();
+        (m.dev(), m.ino())
+    }
+
+    /// Return (dev, ino) for an open cap-std file.
+    fn file_id(f: &cap_std::fs::File) -> (u64, u64) {
+        use cap_std::fs::MetadataExt;
+        let m = f.metadata().unwrap();
+        (m.dev(), m.ino())
+    }
+
+    #[test]
+    fn resolve_policy_path_cases() -> Result<()> {
+        let td = cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
+
+        let etc_id = touch(&td, "etc/containers/policy.json");
+        let _usr_id = touch(&td, "usr/share/containers/policy.json");
+
+        // Env var override wins (trusted — errors if file missing)
+        let custom = Path::new("/custom/policy.json");
+        assert!(resolve_policy_path(&td, Some(custom), None, None).is_err());
+        let custom_id = touch(&td, "custom/policy.json");
+        let f = resolve_policy_path(&td, Some(custom), None, None)?;
+        assert_eq!(
+            file_id(&f),
+            custom_id,
+            "env var should open the custom file"
+        );
+
+        // Empty env var is ignored, falls through to /etc
+        let f = resolve_policy_path(&td, Some(Path::new("")), None, None)?;
+        assert_eq!(
+            file_id(&f),
+            etc_id,
+            "empty env var should fall through to /etc"
+        );
+
+        // XDG_CONFIG_HOME wins when file exists
+        let xdg_id = touch(&td, "xdg/containers/policy.json");
+        let f = resolve_policy_path(&td, None, Some(Path::new("/xdg")), None)?;
+        assert_eq!(file_id(&f), xdg_id, "XDG_CONFIG_HOME should win");
+
+        // XDG_CONFIG_HOME skipped when file missing, falls through to /etc
+        let f = resolve_policy_path(&td, None, Some(Path::new("/xdg-empty")), None)?;
+        assert_eq!(file_id(&f), etc_id, "missing XDG dir should fall through");
+
+        // HOME/.config fallback when XDG unset
+        let home_id = touch(&td, "home/.config/containers/policy.json");
+        let f = resolve_policy_path(&td, None, None, Some(Path::new("/home")))?;
+        assert_eq!(file_id(&f), home_id, "HOME fallback should work");
+
+        // /etc preferred over /usr/share
+        let f = resolve_policy_path(&td, None, None, None)?;
+        assert_eq!(
+            file_id(&f),
+            etc_id,
+            "/etc should be preferred over /usr/share"
+        );
+
+        // Falls through to /usr/share when /etc missing
+        let td2 = cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
+        let usr2_id = touch(&td2, "usr/share/containers/policy.json");
+        let f = resolve_policy_path(&td2, None, None, None)?;
+        assert_eq!(file_id(&f), usr2_id, "should fall through to /usr/share");
+
+        // Nothing found returns error
+        let td3 = cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
+        assert!(resolve_policy_path(&td3, None, None, None).is_err());
+
+        Ok(())
+    }
 }

crates/ostree-ext/src/container/store.rs

@@ -302,6 +302,8 @@ struct LayerRef {
 #[derive(Debug)]
 pub struct ImageImporter {
     repo: ostree::Repo,
+    /// The root filesystem directory, used for policy lookups.
+    root: Dir,
     pub(crate) proxy: ImageProxy,
     imgref: OstreeImageReference,
     target_imgref: Option<OstreeImageReference>,
@@ -648,8 +650,11 @@ impl ImageImporter {
 
         let diffid_to_digest = Self::build_diffid_to_digest_map(&repo)?;
 
+        let root = Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
+
         Ok(ImageImporter {
             repo,
+            root,
             proxy,
             target_imgref: None,
             no_imgref: false,
@@ -935,7 +940,9 @@ impl ImageImporter {
     #[context("Fetching manifest")]
     pub(crate) async fn prepare_internal(&mut self, verify_layers: bool) -> Result<PrepareResult> {
         match &self.imgref.sigverify {
-            SignatureSource::ContainerPolicy if skopeo::container_policy_is_default_insecure()? => {
+            SignatureSource::ContainerPolicy
+                if skopeo::container_policy_is_default_insecure(&self.root)? =>
+            {
                 return Err(anyhow!(
                     "containers-policy.json specifies a default of `insecureAcceptAnything`; refusing usage"
                 ));
@@ -1039,7 +1046,7 @@ impl ImageImporter {
     ) -> Result<()> {
         tracing::debug!("Fetching base");
         if matches!(self.imgref.sigverify, SignatureSource::ContainerPolicy)
-            && skopeo::container_policy_is_default_insecure()?
+            && skopeo::container_policy_is_default_insecure(&self.root)?
         {
             return Err(anyhow!(
                 "containers-policy.json specifies a default of `insecureAcceptAnything`; refusing usage"

docs/src/man/bootc-install-config.5.md

@@ -40,7 +40,8 @@ The `install` section supports these subfields:
    needs the `bli` module (available in newer builds).  Defaults to `true`
    when using systemd-boot, `false` otherwise.
 - `enforce-container-sigpolicy`: A boolean that controls whether to enforce that
-   `/etc/containers/policy.json` includes a default policy which requires signatures.
+   `containers-policy.json` (see `man containers-policy.json` for the full search
+   path) includes a default policy which requires signatures.
    When `true`, image pulls will be rejected if the policy file specifies
    `insecureAcceptAnything` as the default. Defaults to `false`.
    This is equivalent to the `--enforce-container-sigpolicy` CLI flag.

docs/src/man/bootc-install-to-disk.8.md

@@ -111,7 +111,7 @@ set `discoverable-partitions = true` in their install configuration
 
 **--enforce-container-sigpolicy**
 
-    This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op).  Enabling this option enforces that `/etc/containers/policy.json` includes a default policy which requires signatures
+    This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op).  Enabling this option enforces that `containers-policy.json` (see `man containers-policy.json` for the full search path) includes a default policy which requires signatures
 
 **--run-fetch-check**
 

docs/src/man/bootc-install-to-existing-root.8.md

@@ -157,7 +157,7 @@ of migrating the fstab entries. See the "Injecting kernel arguments" section abo
 
 **--enforce-container-sigpolicy**
 
-    This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op).  Enabling this option enforces that `/etc/containers/policy.json` includes a default policy which requires signatures
+    This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op).  Enabling this option enforces that `containers-policy.json` (see `man containers-policy.json` for the full search path) includes a default policy which requires signatures
 
 **--run-fetch-check**
 

docs/src/man/bootc-install-to-filesystem.8.md

@@ -65,7 +65,7 @@ is currently expected to be empty by default.
 
 **--enforce-container-sigpolicy**
 
-    This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op).  Enabling this option enforces that `/etc/containers/policy.json` includes a default policy which requires signatures
+    This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op).  Enabling this option enforces that `containers-policy.json` (see `man containers-policy.json` for the full search path) includes a default policy which requires signatures
 
 **--run-fetch-check**