Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile-tests.am
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ AM_TESTS_ENVIRONMENT += BUILDOPT_ASAN=yes ASAN_OPTIONS=detect_leaks=false
endif

uninstalled_test_scripts = \
tests/unit/bwrap-script.sh \
$(NULL)

uninstalled_test_extra_programs = \
Expand Down
4 changes: 3 additions & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,10 @@ else
CARGO_RELEASE_ARGS=--release
endif

# Note omitting --workspace as this triggers a build system bug
# .../rpm-ostree/rust/libdnf-sys/cxx/libdnf.cxx:56:(.text._ZN6dnfcxx14hy_split_nevraEN4rust10cxxbridge13StrE+0x66): undefined reference to `g_strndup'
check-local-cargo:
cargo test --workspace
cargo test
CHECK_LOCAL_HOOKS += check-local-cargo

clean-local-cargo:
Expand Down
2 changes: 1 addition & 1 deletion rust/src/bwrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ impl Bubblewrap {
}

/// Execute the container. This method uses the normal gtk-rs `Option<T>` for the cancellable.
fn run_inner(&mut self, cancellable: Option<&gio::Cancellable>) -> Result<()> {
pub(crate) fn run_inner(&mut self, cancellable: Option<&gio::Cancellable>) -> Result<()> {
// Merge STDERR/STDOUT so we don't swallow STDERR during execution
self.launcher.set_flags(gio::SubprocessFlags::STDERR_MERGE);
let (child, argv0) = self.spawn()?;
Expand Down
108 changes: 108 additions & 0 deletions rust/src/cli_internals.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

use std::{
io::{BufRead as _, Seek},
os::fd::IntoRawFd,
};

use anyhow::{Context as _, Result};
use cap_std::fs::Dir;
use cap_std_ext::cap_tempfile;
use clap::Parser;
use ostree_ext::gio;

use crate::{bwrap, ffi::BubblewrapMutability, impl_sealed_memfd};

#[derive(Debug, Parser)]
#[clap(rename_all = "kebab-case")]
/// Main options struct
struct Internals {
#[clap(subcommand)]
cmd: Cmd,
}

#[derive(Debug, Parser)]
#[clap(rename_all = "kebab-case")]
/// Options for invoking bubblewrap
struct BwrapOpts {
/// Path to rootfs
root: String,

/// Arguments
args: Vec<String>,
}

#[derive(Debug, Parser)]
#[clap(rename_all = "kebab-case")]
/// Options for invoking bubblewrap
struct BwrapScriptOpts {
/// Path to rootfs
root: String,

/// Path to interpeter
interp: String,

/// Path to script
script: String,
}

#[derive(Debug, clap::Subcommand)]
#[clap(rename_all = "kebab-case")]
/// Subcommands
enum Cmd {
/// Invoke bubblewrap
Bwrap(BwrapOpts),
/// Invoke bubblewrap the same way rpm-ostree does for scripts.
BwrapScript(BwrapScriptOpts),
}

impl BwrapOpts {
fn run(self) -> Result<()> {
let root = &Dir::open_ambient_dir(&self.root, cap_std::ambient_authority())?;
let mut bwrap =
bwrap::Bubblewrap::new_with_mutability(root, BubblewrapMutability::MutateFreely)?;
bwrap.append_child_argv(self.args.iter().map(|s| s.as_str()));
bwrap.run_inner(gio::Cancellable::NONE)?;
Ok(())
}
}

impl BwrapScriptOpts {
fn run(self) -> Result<()> {
let authority = cap_std::ambient_authority();
let root = &Dir::open_ambient_dir(&self.root, authority)?;
let mut bwrap =
bwrap::Bubblewrap::new_with_mutability(root, BubblewrapMutability::MutateFreely)?;
let td = Dir::open_ambient_dir("/var/tmp", authority)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding /var/tmp might reduce portability. This will fail if the tool is run in an environment where /var/tmp does not exist or is not writable. Using std::env::temp_dir() is more robust as it returns a platform-specific temporary directory.

Suggested change
let td = Dir::open_ambient_dir("/var/tmp", authority)?;
let td = Dir::open_ambient_dir(std::env::temp_dir(), authority)?;

let mut output = cap_tempfile::TempFile::new_anonymous(&td)?.into_std();
bwrap.append_child_arg(&self.interp);
bwrap.take_stdout_and_stderr_fd(output.try_clone()?.into_raw_fd());
let script = std::fs::read_to_string(self.script)?;
let mfd = impl_sealed_memfd("script", script.as_bytes())?;
bwrap.take_fd(mfd.into_raw_fd(), 5);
bwrap.append_child_arg("/proc/self/fd/5");
Comment on lines +82 to +83

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The file descriptor 5 is used as a magic number here and on the next line. It's better to define it as a constant to improve readability and maintainability.

Suggested change
bwrap.take_fd(mfd.into_raw_fd(), 5);
bwrap.append_child_arg("/proc/self/fd/5");
const SCRIPT_FD: i32 = 5;
bwrap.take_fd(mfd.into_raw_fd(), SCRIPT_FD);
bwrap.append_child_arg(&format!("/proc/self/fd/{}", SCRIPT_FD));

bwrap.run_inner(gio::Cancellable::NONE)?;
output.seek(std::io::SeekFrom::Start(0))?;
let output = std::io::BufReader::new(output);
for line in output.lines() {
let line = line.context("Reading line")?;
println!("script: {line}");
}
Ok(())
}
}

impl Cmd {
fn run(self) -> Result<()> {
match self {
Cmd::Bwrap(args) => args.run(),
Cmd::BwrapScript(args) => args.run(),
}
}
}

pub fn main(argv: &[&str]) -> Result<i32> {
let opt = Internals::parse_from(argv.into_iter().skip(1));
opt.cmd.run()?;
Ok(0)
}
1 change: 1 addition & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,7 @@ mod core;
use crate::core::*;
mod capstdext;
pub mod cli_experimental;
pub mod cli_internals;
mod daemon;
pub(crate) use daemon::*;
mod deployment_utils;
Expand Down
3 changes: 3 additions & 0 deletions rust/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ async fn inner_async_main(args: Vec<String>) -> Result<i32> {
rpmostree_rust::container::container_encapsulate(args_orig).map(|_| 0)
.map_err(anyhow::Error::msg)
},
"internals" => {
rpmostree_rust::cli_internals::main(args)
}
"experimental" => {
rpmostree_rust::cli_experimental::main(args)
}
Expand Down
1 change: 0 additions & 1 deletion src/app/rpmostree-builtins.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ BUILTINPROTO (initramfs);
BUILTINPROTO (status);
BUILTINPROTO (refresh_md);
BUILTINPROTO (db);
BUILTINPROTO (internals);
BUILTINPROTO (container);
BUILTINPROTO (install);
BUILTINPROTO (uninstall);
Expand Down
26 changes: 26 additions & 0 deletions tests/unit/bwrap-script.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#!/bin/bash
set -euo pipefail

dn=$(cd $(dirname $0) && pwd)

Check warning

Code scanning / shellcheck

Quote this to prevent word splitting. Warning test

Quote this to prevent word splitting.
commondir=${dn}/../common
. "$commondir/libtest.sh"

set -x

# We use TAP
echo 1..1

# Run a script in the host environment
td=$(mktemp -d)
cd $td
cat >script <<EOF
echo hello
echo someerr 1>&2
echo world
EOF
rpm-ostree internals bwrap-script / /bin/bash $(pwd)/script >out.txt

Check warning

Code scanning / shellcheck

Quote this to prevent word splitting. Warning test

Quote this to prevent word splitting.
assert_file_has_content_literal out.txt 'script: hello
script: someerr
script: world'

echo "ok bwrap script"
Loading