Skip to content

docker*.sh scripts: add kvm support and missing authentication so that .docker_*.sh make BOARD=qemu* run launches kvm through gtk window as expected #2036

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
tlaurion wants to merge 1 commit into
base: master
Choose a base branch
from
+418 −147
Draft

docker*.sh scripts: add kvm support and missing authentication so that .docker_*.sh make BOARD=qemu* run launches kvm through gtk window as expected #2036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
143 changes: 106 additions & 37 deletions docker_latest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,58 +1,127 @@
#!/bin/bash
#
# Run Heads build in Docker using the latest published image
# This is suitable for development and testing with the most up-to-date environment
#

# Inform the user that the latest published Docker image is being used
echo "Using the latest Docker image: tlaurion/heads-dev-env:latest"
DOCKER_IMAGE="tlaurion/heads-dev-env:latest"
set -euo pipefail

# ============================================================================
# CONSTANTS
# ============================================================================

readonly DOCKER_IMAGE="tlaurion/heads-dev-env:latest"
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

docker_latest.sh hard-codes DOCKER_IMAGE="tlaurion/heads-dev-env:latest", which causes Docker to pull and execute whatever image is currently published under that mutable latest tag. If the Docker Hub repository or tag is compromised, a malicious image could be served to developers, gaining access to host USB/KVM devices and build outputs when this script is used for Heads development or testing. To reduce supply chain risk, avoid latest and instead pin to a vetted, immutable image reference (for example a specific tag plus digest) or a locally built image under project control.

Copilot uses AI. Check for mistakes.

# ============================================================================
# FUNCTIONS
# ============================================================================

# Function to display usage information
usage() {
echo "Usage: $0 [OPTIONS] -- [COMMAND]"
echo "Options:"
echo " CPUS=N Set the number of CPUs"
echo " V=1 Enable verbose mode"
echo "Command:"
echo " The command to run inside the Docker container, e.g., make BOARD=BOARD_NAME"
cat << EOF
Usage: $0 [OPTIONS] -- [COMMAND]
Options:
CPUS=N Set the number of CPUs
V=1 Enable verbose mode
-h, --help Display this help message
Command:
The command to run inside the Docker container (e.g., make BOARD=BOARD_NAME)
Examples:
$0 make BOARD=qemu-coreboot-fbwhiptail-tpm2
$0 make BOARD=t440p V=1
For more advanced QEMU testing options, refer to targets/qemu.md and boards/qemu-*/*.config
EOF
}

# Function to kill GPG toolstack related processes using USB devices
# Kill GPG toolstack related processes using USB devices
kill_usb_processes() {
# check if scdaemon or pcscd processes are using USB devices
if [ -d /dev/bus/usb ]; then
if sudo lsof /dev/bus/usb/00*/0* 2>/dev/null | awk 'NR>1 {print $2}' | xargs -r ps -p | grep -E 'scdaemon|pcscd' >/dev/null; then
echo "Killing GPG toolstack related processes using USB devices..."
sudo lsof /dev/bus/usb/00*/0* 2>/dev/null | awk 'NR>1 {print $2}' | xargs -r ps -p | grep -E 'scdaemon|pcscd' | awk '{print $1}' | xargs -r sudo kill -9
fi
if [ ! -d "/dev/bus/usb" ]; then
return 0
fi

if sudo lsof /dev/bus/usb/00*/0* 2>/dev/null | \
awk 'NR>1 {print $2}' | \
xargs -r ps -p | \
grep -E 'scdaemon|pcscd' >/dev/null 2>&1; then
echo "Killing GPG toolstack related processes using USB devices..."
sudo lsof /dev/bus/usb/00*/0* 2>/dev/null | \
awk 'NR>1 {print $2}' | \
xargs -r ps -p | \
grep -E 'scdaemon|pcscd' | \
awk '{print $1}' | \
xargs -r sudo kill -9
fi
}

# Handle Ctrl-C (SIGINT) to exit gracefully
trap "echo 'Script interrupted. Exiting...'; exit 1" SIGINT
# Build Docker run options based on available host capabilities
build_docker_opts() {
local opts="-e DISPLAY=${DISPLAY} --network host --rm -ti"

Comment on lines +61 to +62
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

The DISPLAY environment variable is used without checking if it's set. If DISPLAY is unset, this will result in "-e DISPLAY=" being passed to Docker. Consider adding validation to check if DISPLAY is set before using it, or provide a default value.

Suggested change
local opts="-e DISPLAY=${DISPLAY} --network host --rm -ti"
local opts="--network host --rm -ti"
# Add DISPLAY if set
if [ -n "${DISPLAY:-}" ]; then
opts="-e DISPLAY=${DISPLAY} ${opts}"
else
echo "Warning: DISPLAY environment variable is not set. X11 applications may not work." >&2
fi

Copilot uses AI. Check for mistakes.
# Add USB device if available
if [ -d "/dev/bus/usb" ]; then
opts="${opts} --device=/dev/bus/usb:/dev/bus/usb"
echo "--->Launching container with access to host's USB buses..." >&2
else
echo "--->Launching container without access to host's USB buses..." >&2
fi

# Add KVM device if available
if [ -e "/dev/kvm" ]; then
opts="${opts} --device=/dev/kvm:/dev/kvm"
fi

# Add X11 display support
opts="${opts} -v /tmp/.X11-unix:/tmp/.X11-unix"

# Check if --help or -h is provided
# Add Xauthority if it exists
if [ -f "${HOME}/.Xauthority" ]; then
opts="${opts} -v ${HOME}/.Xauthority:/root/.Xauthority:ro"
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

The Xauthority file is mounted read-only into the container at /root/.Xauthority. This allows any process in the container to use the host's X11 authentication credentials. While this is necessary for X11 display forwarding, be aware this grants the container access to display on the host's X server. Consider documenting this security implication, especially since the container runs as root.

Copilot uses AI. Check for mistakes.
fi

echo "${opts}"
}

# ============================================================================
# MAIN
# ============================================================================

# Handle help request
for arg in "$@"; do
if [[ "$arg" == "--help" || "$arg" == "-h" ]]; then
usage
exit 0
fi
done

echo "Using the latest Docker image: ${DOCKER_IMAGE}"

# Handle Ctrl-C gracefully
trap "echo 'Script interrupted. Exiting...'; exit 130" SIGINT
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

The SIGINT trap exits with code 130, which is the conventional exit code for a process terminated by SIGINT (128 + 2). However, since this is using 'exec' on line 127, the trap may not be triggered as expected when the Docker process receives SIGINT. The signal would go directly to the Docker process. Consider whether this trap is still useful given the use of 'exec'.

Suggested change
trap "echo 'Script interrupted. Exiting...'; exit 130" SIGINT

Copilot uses AI. Check for mistakes.

# Kill processes using USB devices
kill_usb_processes

# Inform the user about entering the Docker container
echo "----"
echo "Usage reminder: The minimal command is 'make BOARD=XYZ', where additional options, including 'V=1' or 'CPUS=N' are optional."
echo "For more advanced QEMU testing options, refer to targets/qemu.md and boards/qemu-*/*.config."
echo
echo "Type exit within docker image to get back to host if launched interactively!"
echo "----"
echo

# Execute the docker run command with the provided parameters
if [ -d "/dev/bus/usb" ]; then
echo "--->Launching container with access to host's USB buses (some USB devices were connected to host)..."
docker run --device=/dev/bus/usb:/dev/bus/usb -e DISPLAY=$DISPLAY --network host --rm -ti -v $(pwd):$(pwd) -w $(pwd) $DOCKER_IMAGE -- "$@"
else
echo "--->Launching container without access to host's USB buses (no USB devices was connected to host)..."
docker run -e DISPLAY=$DISPLAY --network host --rm -ti -v $(pwd):$(pwd) -w $(pwd) $DOCKER_IMAGE -- "$@"
fi
# Display usage information
cat << EOF
----
Usage reminder: The minimal command is 'make BOARD=XYZ', where additional
options, including 'V=1' or 'CPUS=N' are optional.
For more advanced QEMU testing options, refer to:
- targets/qemu.md
- boards/qemu-*/*.config
Type 'exit' within the Docker container to return to the host.
----
EOF

# Build Docker options and execute
DOCKER_RUN_OPTS=$(build_docker_opts)

Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

The shellcheck disable directive for SC2086 (word splitting) is used without explanation. Consider adding a comment explaining why word splitting is intentionally needed here (to expand the space-separated Docker options).

Suggested change
# Intentionally allow word splitting to expand space-separated Docker options.

Copilot uses AI. Check for mistakes.
# shellcheck disable=SC2086
exec docker run ${DOCKER_RUN_OPTS} -v "$(pwd):$(pwd)" -w "$(pwd)" "${DOCKER_IMAGE}" -- "$@"
Loading