Minor fixes

internal/clipboard/clipboard.go

@@ -83,6 +83,10 @@ func copyCommands(from, target uint8, contentType, cookiePath string) (out, in *
 	inArgs = append(inArgs, "-i", "-display", display(target))
 
 	in = execabs.Command(files.XclipBinary, inArgs...) //nolint:gosec
+
+	// The environment may already carry an XAUTHORITY, and appending
+	// leaves two entries for the key. os/exec keeps the last value of a
+	// duplicated key, so the cookie set here is the one xclip reads.
 	in.Env = append(os.Environ(), "XAUTHORITY="+cookiePath)
 
 	return out, in

internal/clipboard/clipboard_test.go

@@ -154,12 +154,28 @@ func TestPipeReportsBothFailures(t *testing.T) {
 	// Both commands fail on their own. Returning only one of them would
 	// hide the other, and when the reading command is the one that failed
 	// the writer's error is just the broken pipe it caused.
-	out := execabs.Command("/usr/bin/env", "sh", "-c", "sleep 0.2; exit 4")
-	in := execabs.Command("/usr/bin/env", "sh", "-c", "exit 3")
+	out := execabs.Command(files.ShBinary, "-c", "sleep 0.2; exit 4") //nolint:gosec // fixed test command.
+	in := execabs.Command(files.ShBinary, "-c", "exit 3")             //nolint:gosec // fixed test command.
 
 	err := pipe(out, in)
 	require.Error(t, err)
 
 	assert.Contains(t, err.Error(), "exit status 3", "the reading command's failure must be reported")
 	assert.Contains(t, err.Error(), "exit status 4", "the writing command's failure must be reported")
 }
+
+func TestCopyCommandsXauthorityTakesEffect(t *testing.T) {
+	t.Setenv("XAUTHORITY", "/from/the/environment")
+
+	const cookiePath = "/run/user/1000/qubesome/work.cookie"
+	_, in := copyCommands(0, 1, "", cookiePath)
+
+	// The value the child actually reads is what matters, not how many
+	// times the key appears in the slice: os/exec keeps the last one.
+	echo := execabs.Command(files.ShBinary, "-c", "printf %s \"$XAUTHORITY\"") //nolint:gosec // fixed test command.
+	echo.Env = in.Env
+
+	out, err := echo.Output()
+	require.NoError(t, err)
+	assert.Equal(t, cookiePath, string(out))
+}

internal/profiles/profiles.go

@@ -528,7 +528,7 @@ func createNewDisplay(bin string, ca, cert, key []byte, profile *types.Profile,
 		}
 
 		if t.Before(time.Now()) {
-			return fmt.Errorf("time out waiting for socket to be created")
+			return fmt.Errorf("timed out waiting for socket to be created")
 		}
 
 		// Without this the loop spins on os.Stat for the whole timeout,

internal/runners/docker/run.go

@@ -61,7 +61,14 @@ func Run(ew types.EffectiveWorkload) error {
 		"-d",
 		"--security-opt=label=disable",
 		"--security-opt=no-new-privileges=true",
-		"--cap-drop=ALL",
+	}
+
+	// Workloads start with no capabilities and ask for the ones they need
+	// through capsAdd. A privileged workload is the deliberate opt out of
+	// all of this, and combining the two leaves what the container ends up
+	// with down to the runtime and its version.
+	if !wl.HostAccess.Privileged {
+		args = append(args, "--cap-drop=ALL")
 	}
 
 	// Workloads run untrusted code, so they keep the runtime's seccomp

internal/runners/firecracker/deps.go

@@ -2,6 +2,7 @@ package firecracker
 
 import (
 	"bytes"
+	"context"
 	"crypto/sha256"
 	"encoding/hex"
 	"errors"
@@ -15,6 +16,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strconv"
+	"time"
 
 	"github.com/qubesome/cli/internal/files"
 	"github.com/qubesome/cli/internal/util/dbus"
@@ -41,6 +43,7 @@ const (
 
 	MB              = 1024 * 1024
 	maxDownloadSize = 100 * MB
+	downloadTimeout = 10 * time.Minute
 
 	networkDevName = "tap1"
 )
@@ -147,12 +150,29 @@ func download(url, target, wantSHA256 string) error {
 	if err != nil {
 		return err
 	}
+
+	// The file is closed explicitly once it has been written, so that a
+	// failure to flush is reported rather than discarded. This only cleans
+	// up after the paths that return early.
 	defer func() {
-		f.Close()
-		_ = os.Remove(part)
+		if f != nil {
+			_ = f.Close()
+			_ = os.Remove(part)
+		}
 	}()
 
-	r, err := http.Get(url) //nolint
+	// Requests carry no deadline of their own, so a connection that stalls
+	// would hang dependency setup for as long as the peer keeps it open.
+	// The deadline covers reading the body, not just the response.
+	ctx, cancel := context.WithTimeout(context.Background(), downloadTimeout)
+	defer cancel()
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+	if err != nil {
+		return err
+	}
+
+	r, err := http.DefaultClient.Do(req)
 	if err != nil {
 		return err
 	}
@@ -174,17 +194,22 @@ func download(url, target, wantSHA256 string) error {
 		return fmt.Errorf("download is larger than the %d byte limit", int64(maxDownloadSize))
 	}
 
-	if !bytes.Equal(h.Sum(nil), want) {
+	got := h.Sum(nil)
+	if !bytes.Equal(got, want) {
 		return fmt.Errorf("checksum mismatch for %s: got %s, want %s",
-			url, hex.EncodeToString(h.Sum(nil)), wantSHA256)
+			url, hex.EncodeToString(got), wantSHA256)
 	}
 
 	if err := f.Chmod(files.FileMode); err != nil {
 		return err
 	}
+
+	// Closing a file that was written to can fail, and the failure means
+	// the contents are not what was verified above.
 	if err := f.Close(); err != nil {
-		return err
+		return fmt.Errorf("failed to close %s: %w", part, err)
 	}
+	f = nil
 
 	return os.Rename(part, target)
 }

internal/runners/podman/run.go

@@ -62,7 +62,14 @@ func Run(ew types.EffectiveWorkload) error {
 		"--security-opt=no-new-privileges=true",
 		"--security-opt=label=disable",
 		"--group-add=keep-groups",
-		"--cap-drop=ALL",
+	}
+
+	// Workloads start with no capabilities and ask for the ones they need
+	// through capsAdd. A privileged workload is the deliberate opt out of
+	// all of this, and combining the two leaves what the container ends up
+	// with down to the runtime and its version.
+	if !wl.HostAccess.Privileged {
+		args = append(args, "--cap-drop=ALL")
 	}
 
 	// Workloads run untrusted code, so they keep the runtime's seccomp

internal/runners/util/container/home.go

@@ -102,12 +102,18 @@ func homeOfUser(user, image string) (string, error) {
 	return "/home/" + name, nil
 }
 
+// envValue returns the value of key in an image's environment.
+//
+// The list may hold a key more than once. Runtimes build the container's
+// environment by walking it in order, so the last entry is the one the
+// process ends up with.
 func envValue(env []string, key string) string {
+	value := ""
 	for _, e := range env {
 		if k, v, ok := strings.Cut(e, "="); ok && k == key {
-			return v
+			value = v
 		}
 	}
 
-	return ""
+	return value
 }

internal/runners/util/container/home_test.go

@@ -21,6 +21,16 @@ func TestHomeDir(t *testing.T) {
 		{name: "relative HOME", cfg: imageConfig{Env: []string{"HOME=home/chrome"}}, wantErr: true},
 		{name: "unclean HOME", cfg: imageConfig{Env: []string{"HOME=/home/../etc"}}, wantErr: true},
 		{name: "HOME with a colon", cfg: imageConfig{Env: []string{"HOME=/home/ch:rome"}}, wantErr: true},
+		{
+			name: "duplicate HOME takes the last",
+			cfg:  imageConfig{User: "chrome", Env: []string{"HOME=/home/first", "PATH=/bin", "HOME=/home/last"}},
+			want: "/home/last",
+		},
+		{
+			name: "duplicate HOME where the last is empty falls back to the user",
+			cfg:  imageConfig{User: "chrome", Env: []string{"HOME=/home/first", "HOME="}},
+			want: "/home/chrome",
+		},
 		{name: "user with a colon in the name", cfg: imageConfig{User: "ch:rome:g"}, wantErr: true},
 		{name: "empty HOME falls back", cfg: imageConfig{User: "chrome", Env: []string{"HOME="}}, want: "/home/chrome"},
 	}

internal/types/device.go

@@ -6,6 +6,10 @@ import (
 	"strings"
 )
 
+// maxDeviceLen bounds a device request, matching the bound on the other
+// path-like fields.
+const maxDeviceLen = 500
+
 // ParseDevice splits a device request into its source, destination and
 // permissions components.
 //
@@ -17,6 +21,14 @@ import (
 // the r, w and m flags. When omitted, dst defaults to src and perms
 // defaults to rwm, mirroring the runner defaults.
 func ParseDevice(device string) (src, dst, perms string, err error) {
+	// Bound the input before splitting it or quoting it back. Device
+	// requests come from a workload config, and an oversized one would
+	// otherwise be allocated per field and echoed into an error and the
+	// log line that reports it.
+	if len(device) > maxDeviceLen {
+		return "", "", "", fmt.Errorf("invalid device: longer than %d characters", maxDeviceLen)
+	}
+
 	parts := strings.Split(device, ":")
 	if len(parts) > 3 {
 		return "", "", "", fmt.Errorf("invalid device %q: too many fields", device)
@@ -51,7 +63,7 @@ func ParseDevice(device string) (src, dst, perms string, err error) {
 // A grant names a source device, so it is held to the same rules as the
 // source of a workload's device request.
 func ValidateDeviceGrant(device string) error {
-	if err := valid(device, "devices", 500, false, nil); err != nil {
+	if err := valid(device, "devices", maxDeviceLen, false, nil); err != nil {
 		return err
 	}
 

internal/types/device_test.go

@@ -1,6 +1,9 @@
 package types
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func TestParseDevice(t *testing.T) {
 	t.Parallel()
@@ -27,6 +30,7 @@ func TestParseDevice(t *testing.T) {
 		{device: "/dev/null:/dev/sda:mwr", wantSrc: "/dev/null", wantDst: "/dev/sda", wantPerms: "mwr"},
 		{device: "/dev/null:/dev/sda:rwm:extra", wantErr: true},
 		{device: "/dev", wantErr: true},
+		{device: "/dev/" + strings.Repeat("a", maxDeviceLen), wantErr: true},
 	}
 
 	for _, tc := range tests {
@@ -123,3 +127,17 @@ func TestApplyProfileDevices(t *testing.T) {
 		})
 	}
 }
+
+func TestParseDeviceDoesNotEchoOversizedValue(t *testing.T) {
+	t.Parallel()
+
+	device := "/dev/" + strings.Repeat("a", 10_000)
+
+	src, _, _, err := ParseDevice(device)
+	if err == nil {
+		t.Fatalf("expected error, got src %q", src)
+	}
+	if strings.Contains(err.Error(), "aaaa") {
+		t.Errorf("error echoes the oversized value: %d chars", len(err.Error()))
+	}
+}

internal/util/xauth/xauth.go

@@ -13,8 +13,14 @@ import (
 // https://gitlab.freedesktop.org/xorg/app/xauth/-/blob/master/process.c?ref_type=heads
 // https://gitlab.freedesktop.org/xorg/app/xauth/-/blob/master/xauth.h?ref_type=heads
 
-// familyLocal marks an entry as matching any local connection.
-const familyLocal = 0xffff
+// familyWild is the wildcard family of X11's Xauth.h, which matches any
+// family rather than naming one. FamilyLocal, the family the host entry
+// carries, is 256.
+//
+// The workload's copy is written as wildcard so that it is accepted from
+// inside a container, where the connection does not look like it came
+// from the host the entry names.
+const familyWild = 0xffff
 
 var cookieFunc = newCookie
 
@@ -40,9 +46,7 @@ func AuthPair(display uint8, parent io.Reader, server, client io.Writer) error {
 		return fmt.Errorf("failed to write server auth file: %w", err)
 	}
 
-	// The workload's copy is family local, which is what lets it connect
-	// from inside a container.
-	rec.family = familyLocal
+	rec.family = familyWild
 
 	if err := rec.writeTo(client); err != nil {
 		return fmt.Errorf("failed to write workload auth file: %w", err)

pkg/inception/server.go

@@ -91,15 +91,15 @@ func (s *grpcServer) XdgOpen(ctx context.Context, in *pb.XdgOpenRequest) (*pb.Xd
 }
 
 func (s *grpcServer) RunWorkload(ctx context.Context, in *pb.RunWorkloadRequest) (*pb.RunWorkloadReply, error) {
-	worload := in.GetWorkload()
+	workload := in.GetWorkload()
 	args := in.GetArgs()
 	profile := s.profile.Name
-	slog.Debug("[server] run-workload received", "workload", worload, "profile", profile, "args", args)
+	slog.Debug("[server] run-workload received", "workload", workload, "profile", profile, "args", args)
 
 	opts := []command.Option[qubesome.Options]{
 		qubesome.WithConfig(s.config),
 		qubesome.WithProfile(profile),
-		qubesome.WithWorkload(worload),
+		qubesome.WithWorkload(workload),
 	}
 
 	if err := checkRPCArgs(args); err != nil {
@@ -115,15 +115,15 @@ func (s *grpcServer) RunWorkload(ctx context.Context, in *pb.RunWorkloadRequest)
 }
 
 func (s *grpcServer) FlatpakRunWorkload(ctx context.Context, in *pb.FlatpakRunWorkloadRequest) (*pb.FlatpakRunWorkloadReply, error) {
-	worload := in.GetWorkload()
+	workload := in.GetWorkload()
 	args := in.GetArgs()
 	profile := s.profile.Name
-	slog.Debug("[server] flatpak-run-workload received", "workload", worload, "profile", profile, "args", args)
+	slog.Debug("[server] flatpak-run-workload received", "workload", workload, "profile", profile, "args", args)
 
 	opts := []command.Option[flatpak.Options]{
 		flatpak.WithConfig(s.config),
 		flatpak.WithProfile(profile),
-		flatpak.WithName(worload),
+		flatpak.WithName(workload),
 	}
 
 	if err := checkRPCArgs(args); err != nil {