From a9dc522286570ea530924e77ef26c81cf647a8f3 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 22 Nov 2024 14:39:14 -0500 Subject: [PATCH 1/5] cli: Factor out helper to parse "callname" No functional changes; prep for further work. Signed-off-by: Colin Walters --- lib/src/cli.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/lib/src/cli.rs b/lib/src/cli.rs index 9f19a6a67..e5e13a073 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -2,8 +2,7 @@ //! //! Command line tool to manage bootable ostree-based containers. -use std::ffi::CString; -use std::ffi::OsString; +use std::ffi::{CString, OsStr, OsString}; use std::io::Seek; use std::os::unix::process::CommandExt; use std::process::Command; @@ -879,6 +878,18 @@ where run_from_opt(Opt::parse_including_static(args)).await } +/// Find the base binary name from argv0 (without a full path). The empty string +/// is never returned; instead a fallback string is used. If the input is not valid +/// UTF-8, a default is used. +fn callname_from_argv0(argv0: &OsStr) -> &str { + let default = "bootc"; + std::path::Path::new(argv0) + .file_name() + .and_then(|s| s.to_str()) + .filter(|s| !s.is_empty()) + .unwrap_or(default) +} + impl Opt { /// In some cases (e.g. systemd generator) we dispatch specifically on argv0. This /// requires some special handling in clap. @@ -890,9 +901,9 @@ impl Opt { let mut args = args.into_iter(); let first = if let Some(first) = args.next() { let first: OsString = first.into(); - let argv0 = first.to_str().and_then(|s| s.rsplit_once('/')).map(|s| s.1); + let argv0 = callname_from_argv0(&first); tracing::debug!("argv0={argv0:?}"); - if matches!(argv0, Some(InternalsOpts::GENERATOR_BIN)) { + if argv0 == InternalsOpts::GENERATOR_BIN { let base_args = ["bootc", "internals", "systemd-generator"] .into_iter() .map(OsString::from); @@ -1022,6 +1033,41 @@ async fn run_from_opt(opt: Opt) -> Result<()> { } } +#[test] +fn test_callname() { + use std::os::unix::ffi::OsStrExt; + + // Cases that change + let mapped_cases = [ + ("", "bootc"), + ("/foo/bar", "bar"), + ("/foo/bar/", "bar"), + ("foo/bar", "bar"), + ("../foo/bar", "bar"), + ("usr/bin/ostree-container", "ostree-container"), + ]; + for (input, output) in mapped_cases { + assert_eq!( + output, + callname_from_argv0(OsStr::new(input)), + "Handling mapped case {input}" + ); + } + + // Invalid UTF-8 + assert_eq!("bootc", callname_from_argv0(OsStr::from_bytes(b"foo\x80"))); + + // Cases that are identical + let ident_cases = ["foo", "bootc"]; + for case in ident_cases { + assert_eq!( + case, + callname_from_argv0(OsStr::new(case)), + "Handling ident case {case}" + ); + } +} + #[test] fn test_parse_install_args() { // Verify we still process the legacy --target-no-signature-verification From 79e715ad06d45e86b6170ff8b8e3ccd5c9c95fd6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 22 Nov 2024 14:44:38 -0500 Subject: [PATCH 2/5] cli: Prepare for other argv0 dispatching Clean up the interception to prepare for other cases. Signed-off-by: Colin Walters --- lib/src/cli.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/src/cli.rs b/lib/src/cli.rs index e5e13a073..5d1e4f69d 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -903,10 +903,14 @@ impl Opt { let first: OsString = first.into(); let argv0 = callname_from_argv0(&first); tracing::debug!("argv0={argv0:?}"); - if argv0 == InternalsOpts::GENERATOR_BIN { - let base_args = ["bootc", "internals", "systemd-generator"] - .into_iter() - .map(OsString::from); + let mapped = match argv0 { + InternalsOpts::GENERATOR_BIN => { + Some(["bootc", "internals", "systemd-generator"].as_slice()) + } + _ => None, + }; + if let Some(base_args) = mapped { + let base_args = base_args.into_iter().map(OsString::from); return Opt::parse_from(base_args.chain(args.map(|i| i.into()))); } Some(first) From 8dbe27310db7207824cd84e5847d242b7a1b326d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 22 Nov 2024 14:59:32 -0500 Subject: [PATCH 3/5] cli: Add interception for ostree extension verbs This allows us to fully own the symlinks in `/usr/libexec/libostree/ext`. Signed-off-by: Colin Walters --- lib/src/cli.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/lib/src/cli.rs b/lib/src/cli.rs index 5d1e4f69d..b1135c4a5 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -907,6 +907,9 @@ impl Opt { InternalsOpts::GENERATOR_BIN => { Some(["bootc", "internals", "systemd-generator"].as_slice()) } + "ostree-container" | "ostree-ima-sign" | "ostree-provisional-repair" => { + Some(["bootc", "internals", "ostree-ext"].as_slice()) + } _ => None, }; if let Some(base_args) = mapped { @@ -1133,4 +1136,31 @@ fn test_parse_ostree_ext() { Opt::parse_including_static(["bootc", "internals", "ostree-container"]), Opt::Internals(InternalsOpts::OstreeContainer { .. }) )); + + fn peel(o: Opt) -> Vec { + match o { + Opt::Internals(InternalsOpts::OstreeExt { args }) => args, + o => panic!("unexpected {o:?}"), + } + } + let args = peel(Opt::parse_including_static([ + "/usr/libexec/libostree/ext/ostree-ima-sign", + "ima-sign", + "--repo=foo", + "foo", + "bar", + "baz", + ])); + assert_eq!( + args.as_slice(), + ["ima-sign", "--repo=foo", "foo", "bar", "baz"] + ); + + let args = peel(Opt::parse_including_static([ + "/usr/libexec/libostree/ext/ostree-container", + "container", + "image", + "pull", + ])); + assert_eq!(args.as_slice(), ["container", "image", "pull"]); } From 2e8142bca04a4858413eefcd2de27599c93bba5b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 25 Nov 2024 14:17:49 -0500 Subject: [PATCH 4/5] build: Add install-ostree-hooks This an optional bit for those that need `ostree container`. Signed-off-by: Colin Walters --- Makefile | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Makefile b/Makefile index 1a7fd64b3..41a6454c6 100644 --- a/Makefile +++ b/Makefile @@ -26,6 +26,14 @@ install: done install -D -m 0644 -t $(DESTDIR)/$(prefix)/lib/systemd/system systemd/*.service systemd/*.timer +# Run this to also take over the functionality of `ostree container` for example. +# Only needed for OS/distros that have callers invoking `ostree container` and not bootc. +install-ostree-hooks: + install -d $(DESTDIR)$(prefix)/libexec/libostree/ext + for x in ostree-container ostree-ima-sign ostree-provisional-repair; do \ + ln -sf ../../../bin/bootc $(DESTDIR)$(prefix)/libexec/libostree/ext/$$x; \ + done + install-with-tests: install install -D -m 0755 target/release/tests-integration $(DESTDIR)$(prefix)/bin/bootc-integration-tests From 5ae6732b97f1c4b096478bf712784c66169219a2 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 26 Nov 2024 10:59:24 -0500 Subject: [PATCH 5/5] cli: Strengthen generator parsing test Signed-off-by: Colin Walters --- lib/src/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/cli.rs b/lib/src/cli.rs index b1135c4a5..84eb58b6e 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -1126,7 +1126,7 @@ fn test_parse_generator() { "/usr/lib/systemd/system/bootc-systemd-generator", "/run/systemd/system" ]), - Opt::Internals(InternalsOpts::SystemdGenerator { .. }) + Opt::Internals(InternalsOpts::SystemdGenerator { normal_dir, .. }) if normal_dir == "/run/systemd/system" )); }