From a89a5c5b9cb77e18ba6a5e14ea18d1caf80023d4 Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Sat, 13 Jun 2026 16:26:11 +0200 Subject: [PATCH] [client] fix privileged test CI failures and run the harness on macOS The host-safe unit run dropped sudo but two privileged test groups were never tagged, and the Docker privileged job silently never ran the suite: - Gate the ssh/server PrivilegeDropper command-construction tests behind the privileged tag (they require root to target a different UID); split them into executor_unix_privileged_test.go. - Tag sharedsock raw-socket tests privileged (need CAP_NET_RAW). - Fix the Docker job command: nested single quotes around the build tags closed the sh -c wrapper early, dropping the go list package set and the privileged tag, so go test ran on the empty repo root. Use double quotes. Make the self-hosting harness usable from a dev Mac: - Build it on darwin as well as linux; it only drives Docker. - Resolve the active docker context endpoint into DOCKER_HOST when the default /var/run/docker.sock is absent (Docker Desktop, Colima, OrbStack). - Rename the misspelled containerGoModache constant to containerGoModCache. --- .github/workflows/golang-test-linux.yml | 2 +- .../server/executor_unix_privileged_test.go | 66 +++++++++++++++++++ client/ssh/server/executor_unix_test.go | 55 ---------------- client/testutil/privileged/runner_test.go | 38 +++++++++-- sharedsock/sock_linux_test.go | 2 + 5 files changed, 101 insertions(+), 62 deletions(-) create mode 100644 client/ssh/server/executor_unix_privileged_test.go diff --git a/.github/workflows/golang-test-linux.yml b/.github/workflows/golang-test-linux.yml index 6a77cb180..5d7f9c499 100644 --- a/.github/workflows/golang-test-linux.yml +++ b/.github/workflows/golang-test-linux.yml @@ -229,7 +229,7 @@ jobs: sh -c ' \ apk update; apk add --no-cache \ ca-certificates iptables ip6tables dbus dbus-dev libpcap-dev build-base; \ - go test -buildvcs=false -tags 'devcert privileged' -v -timeout 10m -p 1 $(go list -buildvcs=false ./... | grep -v -e /management -e /signal -e /relay -e /proxy -e /combined -e /client/ui -e /upload-server -e /client/testutil/privileged) + go test -buildvcs=false -tags "devcert privileged" -v -timeout 10m -p 1 $(go list -buildvcs=false ./... | grep -v -e /management -e /signal -e /relay -e /proxy -e /combined -e /client/ui -e /upload-server -e /client/testutil/privileged) ' test_relay: diff --git a/client/ssh/server/executor_unix_privileged_test.go b/client/ssh/server/executor_unix_privileged_test.go new file mode 100644 index 000000000..f1b0805d9 --- /dev/null +++ b/client/ssh/server/executor_unix_privileged_test.go @@ -0,0 +1,66 @@ +//go:build unix && privileged + +package server + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPrivilegeDropper_CreateExecutorCommand(t *testing.T) { + pd := NewPrivilegeDropper() + + config := ExecutorConfig{ + UID: 1000, + GID: 1000, + Groups: []uint32{1000, 1001}, + WorkingDir: "/home/testuser", + Shell: "/bin/bash", + Command: "ls -la", + } + + cmd, err := pd.CreateExecutorCommand(context.Background(), config) + require.NoError(t, err) + require.NotNil(t, cmd) + + // Verify the command is calling netbird ssh exec + assert.Contains(t, cmd.Args, "ssh") + assert.Contains(t, cmd.Args, "exec") + assert.Contains(t, cmd.Args, "--uid") + assert.Contains(t, cmd.Args, "1000") + assert.Contains(t, cmd.Args, "--gid") + assert.Contains(t, cmd.Args, "1000") + assert.Contains(t, cmd.Args, "--groups") + assert.Contains(t, cmd.Args, "1000") + assert.Contains(t, cmd.Args, "1001") + assert.Contains(t, cmd.Args, "--working-dir") + assert.Contains(t, cmd.Args, "/home/testuser") + assert.Contains(t, cmd.Args, "--shell") + assert.Contains(t, cmd.Args, "/bin/bash") + assert.Contains(t, cmd.Args, "--cmd") + assert.Contains(t, cmd.Args, "ls -la") +} + +func TestPrivilegeDropper_CreateExecutorCommandInteractive(t *testing.T) { + pd := NewPrivilegeDropper() + + config := ExecutorConfig{ + UID: 1000, + GID: 1000, + Groups: []uint32{1000}, + WorkingDir: "/home/testuser", + Shell: "/bin/bash", + Command: "", + } + + cmd, err := pd.CreateExecutorCommand(context.Background(), config) + require.NoError(t, err) + require.NotNil(t, cmd) + + // Verify no command mode (command is empty so no --cmd flag) + assert.NotContains(t, cmd.Args, "--cmd") + assert.NotContains(t, cmd.Args, "--interactive") +} diff --git a/client/ssh/server/executor_unix_test.go b/client/ssh/server/executor_unix_test.go index 0c5108f57..171e78b83 100644 --- a/client/ssh/server/executor_unix_test.go +++ b/client/ssh/server/executor_unix_test.go @@ -73,61 +73,6 @@ func TestPrivilegeDropper_ValidatePrivileges(t *testing.T) { } } -func TestPrivilegeDropper_CreateExecutorCommand(t *testing.T) { - pd := NewPrivilegeDropper() - - config := ExecutorConfig{ - UID: 1000, - GID: 1000, - Groups: []uint32{1000, 1001}, - WorkingDir: "/home/testuser", - Shell: "/bin/bash", - Command: "ls -la", - } - - cmd, err := pd.CreateExecutorCommand(context.Background(), config) - require.NoError(t, err) - require.NotNil(t, cmd) - - // Verify the command is calling netbird ssh exec - assert.Contains(t, cmd.Args, "ssh") - assert.Contains(t, cmd.Args, "exec") - assert.Contains(t, cmd.Args, "--uid") - assert.Contains(t, cmd.Args, "1000") - assert.Contains(t, cmd.Args, "--gid") - assert.Contains(t, cmd.Args, "1000") - assert.Contains(t, cmd.Args, "--groups") - assert.Contains(t, cmd.Args, "1000") - assert.Contains(t, cmd.Args, "1001") - assert.Contains(t, cmd.Args, "--working-dir") - assert.Contains(t, cmd.Args, "/home/testuser") - assert.Contains(t, cmd.Args, "--shell") - assert.Contains(t, cmd.Args, "/bin/bash") - assert.Contains(t, cmd.Args, "--cmd") - assert.Contains(t, cmd.Args, "ls -la") -} - -func TestPrivilegeDropper_CreateExecutorCommandInteractive(t *testing.T) { - pd := NewPrivilegeDropper() - - config := ExecutorConfig{ - UID: 1000, - GID: 1000, - Groups: []uint32{1000}, - WorkingDir: "/home/testuser", - Shell: "/bin/bash", - Command: "", - } - - cmd, err := pd.CreateExecutorCommand(context.Background(), config) - require.NoError(t, err) - require.NotNil(t, cmd) - - // Verify no command mode (command is empty so no --cmd flag) - assert.NotContains(t, cmd.Args, "--cmd") - assert.NotContains(t, cmd.Args, "--interactive") -} - // TestPrivilegeDropper_ActualPrivilegeDrop tests actual privilege dropping // This test requires root privileges and will be skipped if not running as root func TestPrivilegeDropper_ActualPrivilegeDrop(t *testing.T) { diff --git a/client/testutil/privileged/runner_test.go b/client/testutil/privileged/runner_test.go index d0b449866..d1945894d 100644 --- a/client/testutil/privileged/runner_test.go +++ b/client/testutil/privileged/runner_test.go @@ -1,4 +1,4 @@ -//go:build privileged && linux +//go:build privileged && (linux || darwin) // Package privileged provides a self-hosting harness that runs the repo's // privileged-tagged test suite inside a --privileged --cap-add=NET_ADMIN @@ -29,9 +29,9 @@ const ( ) const ( - containerWorkdir = "/app" - containerGoCache = "/root/.cache/go-build" - containerGoModache = "/go/pkg/mod" + containerWorkdir = "/app" + containerGoCache = "/root/.cache/go-build" + containerGoModCache = "/go/pkg/mod" ) // alpinePackages are the build/runtime deps the privileged tests need, mirroring @@ -67,6 +67,12 @@ func TestRunPrivilegedSuiteInDocker(t *testing.T) { } goCache, goModCache := hostGoCaches(t) + // dockertest reads DOCKER_HOST; point it at the active context's socket when + // the default one is absent (macOS Docker Desktop, Colima, OrbStack). + if host := dockerHost(); host != "" { + t.Setenv("DOCKER_HOST", host) + } + // NewPoolT registers container cleanup via t.Cleanup automatically. pool := dockertest.NewPoolT(t, "", dockertest.WithMaxWait(30*time.Minute)) @@ -78,7 +84,7 @@ func TestRunPrivilegedSuiteInDocker(t *testing.T) { dockertest.WithMounts([]string{ repoRoot + ":" + containerWorkdir, goCache + ":" + containerGoCache, - goModCache + ":" + containerGoModache, + goModCache + ":" + containerGoModCache, }), dockertest.WithEnv([]string{ "CGO_ENABLED=1", @@ -86,7 +92,7 @@ func TestRunPrivilegedSuiteInDocker(t *testing.T) { "DOCKER_CI=true", "CONTAINER=true", "GOCACHE=" + containerGoCache, - "GOMODCACHE=" + containerGoModache, + "GOMODCACHE=" + containerGoModCache, }), dockertest.WithCmd([]string{"sleep", "infinity"}), dockertest.WithHostConfig(func(hc *container.HostConfig) { @@ -131,6 +137,26 @@ func findRepoRoot() (string, error) { } } +// dockerHost returns a DOCKER_HOST override when the default socket is missing. +// An empty result means the caller should leave DOCKER_HOST untouched (it is +// already set, or the default unix socket exists). When neither is present +// (common on macOS Docker Desktop, Colima and OrbStack, which use a per-user +// socket), it resolves the active docker context's endpoint. +func dockerHost() string { + if os.Getenv("DOCKER_HOST") != "" { + return "" + } + if _, err := os.Stat("/var/run/docker.sock"); err == nil { + return "" + } + + out, err := exec.Command("docker", "context", "inspect", "-f", "{{.Endpoints.docker.Host}}").Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + // hostGoCaches resolves the host GOCACHE/GOMODCACHE so the container reuses the // existing build/module cache for speed. func hostGoCaches(t *testing.T) (string, string) { diff --git a/sharedsock/sock_linux_test.go b/sharedsock/sock_linux_test.go index a22af461a..0ed15e282 100644 --- a/sharedsock/sock_linux_test.go +++ b/sharedsock/sock_linux_test.go @@ -1,3 +1,5 @@ +//go:build privileged + package sharedsock import (