diff --git a/e2e/tests/up/podman_rootful.go b/e2e/tests/up/podman_rootful.go index 3732ba1b94..bc7a4632e1 100644 --- a/e2e/tests/up/podman_rootful.go +++ b/e2e/tests/up/podman_rootful.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net/url" "os" "os/exec" "strings" @@ -12,6 +13,7 @@ import ( "github.com/devsy-org/devsy/e2e/framework" "github.com/devsy-org/devsy/pkg/docker" + "github.com/devsy-org/devsy/pkg/secrets" "github.com/onsi/ginkgo/v2" ) @@ -88,6 +90,26 @@ func shouldAttemptPodmanRecovery(class podmanHealthClass) bool { return class == podmanHealthTimeout || class == podmanHealthUnavailable } +func rootfulPodmanUsesManagedDaemon(endpoint string) bool { + if endpoint == "" { + return true + } + return endpoint == "unix:///run/podman/podman.sock" || + endpoint == "unix:///var/run/podman/podman.sock" +} + +func rootfulPodmanWrapperScript() string { + return `#!/bin/sh +set -eu + +if [ -n "${DOCKER_HOST:-}" ]; then + exec sudo podman --remote --url "$DOCKER_HOST" "$@" +fi + +exec sudo podman "$@" +` +} + // podmanDaemonGate records the first unrecoverable daemon failure so later // specs in the shard skip instead of cascading into identical infrastructure // failures that would bury the actionable one. Scoped to the test process, @@ -139,9 +161,9 @@ func checkPodmanHealth(ctx context.Context, wrapperPath string) (podmanHealthCla return podmanHealthOK, nil } class := classifyPodmanHealthFailure(healthCtx, string(out)) - return class, fmt.Errorf( + return class, redactPodmanError(err, fmt.Sprintf( "rootful Podman readiness check failed (class: %s) or exceeded %s\n"+ - "command: %s ps\nDOCKER_HOST: %s\ncontext err: %v\noutput:\n%s\nerror: %w", + "command: %s ps\nDOCKER_HOST: %s\ncontext err: %v\noutput:\n%s\nerror: %v", class, podmanHealthCheckTimeout, wrapperPath, @@ -149,7 +171,7 @@ func checkPodmanHealth(ctx context.Context, wrapperPath string) (podmanHealthCla healthCtx.Err(), string(out), err, - ) + )) } // runDiagCommand never fails the caller: diagnostics are best-effort so a @@ -168,6 +190,7 @@ func runDiagCommand(ctx context.Context, name string, args ...string) string { if err != nil { text += fmt.Sprintf("\n(command failed: %v; context err: %v)", err, diagCtx.Err()) } + text = redactPodmanDiagnostics(text) limit := podmanDiagMaxOutput if name == "ps" || name == "lslocks" || name == "sh" || strings.Contains(strings.Join(args, " "), "lslocks") { @@ -274,12 +297,12 @@ func attemptPodmanRecovery(ctx context.Context, wrapperPath string) (podmanHealt docker.PrepareForGroupCancellation(cmd) started := time.Now() if out, err := cmd.CombinedOutput(); err != nil { - return podmanHealthPoisoned, fmt.Errorf( - "PODMAN_ROOTFUL_RECOVERY_FAILED: systemctl restart failed after %s: %w\noutput:\n%s", + return podmanHealthPoisoned, redactPodmanError(err, fmt.Sprintf( + "PODMAN_ROOTFUL_RECOVERY_FAILED: systemctl restart failed after %s: %v\noutput:\n%s", time.Since(started), err, string(out), - ) + )) } ginkgo.GinkgoWriter.Printf("[podman-recovery] restart elapsed=%s\n", time.Since(started)) class, err := checkPodmanHealth(ctx, wrapperPath) @@ -300,7 +323,8 @@ func attemptPodmanRecovery(ctx context.Context, wrapperPath string) (podmanHealt func setupRootfulPodman(ctx context.Context, initialDir string) *framework.Framework { wrapperPath := initialDir + "/bin/" + podmanRootfulWrapperName - if since := rootfulDaemonGate.unhealthy(); since != "" { + managedDaemon := rootfulPodmanUsesManagedDaemon(os.Getenv("DOCKER_HOST")) + if since := rootfulDaemonGate.unhealthy(); managedDaemon && since != "" { ginkgo.Skip(fmt.Sprintf( "rootful Podman daemon unhealthy since first failure in %q; "+ "skipping to avoid cascading infrastructure failures", @@ -311,7 +335,7 @@ func setupRootfulPodman(ctx context.Context, initialDir string) *framework.Frame wrapper, err := os.Create(wrapperPath) //nolint:gosec // G304: test-controlled path framework.ExpectNoError(err) - _, err = wrapper.WriteString("#!/bin/sh\nsudo podman \"$@\"\n") + _, err = wrapper.WriteString(rootfulPodmanWrapperScript()) if err != nil { _ = wrapper.Close() } @@ -335,7 +359,7 @@ func setupRootfulPodman(ctx context.Context, initialDir string) *framework.Frame healthErr, ) collectPodmanDiagnostics(wrapperPath) - if shouldAttemptPodmanRecovery(class) { + if managedDaemon && shouldAttemptPodmanRecovery(class) { if !rootfulDaemonGate.claimRecovery() { rootfulDaemonGate.markUnhealthy(ginkgo.CurrentSpecReport().FullText()) framework.ExpectNoError( @@ -360,8 +384,8 @@ func setupRootfulPodman(ctx context.Context, initialDir string) *framework.Frame "[podman-recovery] daemon healthy again after single restart", ) } else { - // The daemon answers but errors: a restart would only hide a - // product or configuration problem, so fail without gating. + // The managed service cannot repair another endpoint or a + // responsive daemon error, so fail without gating. framework.ExpectNoError(healthErr) } } @@ -404,7 +428,8 @@ func recoverPodmanCleanup( // mirroring CleanupWorkspace. recoveryCtx := context.WithoutCancel(ctx) class, healthErr := checkPodmanHealth(recoveryCtx, wrapperPath) - if healthErr == nil || !shouldAttemptPodmanRecovery(class) { + if healthErr == nil || !rootfulPodmanUsesManagedDaemon(os.Getenv("DOCKER_HOST")) || + !shouldAttemptPodmanRecovery(class) { return cleanupErr } if !rootfulDaemonGate.claimRecovery() { @@ -446,3 +471,49 @@ func recoverPodmanCleanup( ) return nil } + +// Redact before truncation so a clipped URL cannot expose partial credentials. +func redactPodmanDiagnostics(text string) string { + return secrets.Combine(secrets.NewRedactor(podmanEndpointSecrets(os.Getenv("DOCKER_HOST"))), + secrets.NewEnvironmentRedactor(os.Environ())).Redact(text) +} + +func podmanEndpointSecrets(value string) []string { + endpoint, err := url.Parse(value) + if err != nil { + return []string{"endpoint=" + value} + } + if endpoint.User == nil { + return nil + } + entries := podmanUserinfoSecrets(endpoint.User.String()) + entries = append(entries, "username="+endpoint.User.Username()) + if password, ok := endpoint.User.Password(); ok { + entries = append(entries, "password="+password) + } + start, end := strings.Index(value, "://"), strings.LastIndex(value, "@") + if start >= 0 && end > start+3 { + entries = append(entries, podmanUserinfoSecrets(value[start+3:end])...) + } + return entries +} + +func podmanUserinfoSecrets(userinfo string) []string { + entries := []string{"userinfo=" + userinfo} + if _, encoded, ok := strings.Cut(userinfo, ":"); ok { + entries = append(entries, "encoded="+encoded) + } + return entries +} + +type podmanDiagnosticError struct { + cause error + message string +} + +func (e podmanDiagnosticError) Error() string { return e.message } +func (e podmanDiagnosticError) Unwrap() error { return e.cause } + +func redactPodmanError(cause error, message string) error { + return podmanDiagnosticError{cause: cause, message: redactPodmanDiagnostics(message)} +} diff --git a/e2e/tests/up/podman_rootful_test.go b/e2e/tests/up/podman_rootful_test.go index 6b45c46de1..1a740335ee 100644 --- a/e2e/tests/up/podman_rootful_test.go +++ b/e2e/tests/up/podman_rootful_test.go @@ -1,11 +1,17 @@ package up import ( + "bytes" "context" + "errors" + "os" + "os/exec" + "path/filepath" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func expiredContext(t *testing.T) context.Context { @@ -106,3 +112,210 @@ func TestRunPodmanDiagnosticsStopsAtBudget(t *testing.T) { }, func(_, _ string) {}) assert.Equal(t, []string{"ps"}, commands) } + +//nolint:gosec // Synthetic endpoint credentials verify unchanged routing. +func TestRootfulPodmanWrapper(t *testing.T) { + const ( + remoteFlag = "--remote" + urlFlag = "--url" + ) + endpoint := "unix:///run/podman/podman.sock" + emptyEndpoint := "" + credentialEndpoint := "ssh://fixture-user:fixture%40password@podman.example/run/podman.sock" + cases := []struct { + name string + endpoint *string + wantPrefix []string + args []string + }{ + { + name: "configured endpoint", + endpoint: &endpoint, + wantPrefix: []string{"podman", remoteFlag, urlFlag, endpoint}, + args: []string{"ps", "-a"}, + }, + { + name: "credential endpoint remains unchanged", + endpoint: &credentialEndpoint, + wantPrefix: []string{"podman", remoteFlag, urlFlag, credentialEndpoint}, + args: []string{"ps"}, + }, + { + name: "preserves arguments", + endpoint: &endpoint, + wantPrefix: []string{"podman", remoteFlag, urlFlag, endpoint}, + args: []string{"run", "value with spaces", "$(printf unsafe)"}, + }, + { + name: "unset endpoint uses local mode", + wantPrefix: []string{"podman"}, + args: []string{"ps", "-a"}, + }, + { + name: "empty endpoint uses local mode", + endpoint: &emptyEndpoint, + wantPrefix: []string{"podman"}, + args: []string{"ps", "-a"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := runRootfulPodmanWrapper(t, tc.endpoint, tc.args...) + require.Equal(t, append(tc.wantPrefix, tc.args...), got) + require.NotContains(t, got, "-E") + }) + } +} + +func runRootfulPodmanWrapper(t *testing.T, endpoint *string, args ...string) []string { + t.Helper() + + dir := t.TempDir() + capturePath := filepath.Join(dir, "argv") + sudoPath := filepath.Join(dir, "sudo") + sudoScript := `#!/bin/sh +printf '%s\0' "$@" > "$CAPTURE_FILE" + ` + require.NoError(t, os.WriteFile(sudoPath, []byte(sudoScript), 0o600)) + //nolint:gosec // G302: test executable needs owner execute permission. + require.NoError(t, os.Chmod(sudoPath, 0o700)) + + wrapperPath := filepath.Join(dir, "podman-rootful") + require.NoError(t, os.WriteFile(wrapperPath, []byte(rootfulPodmanWrapperScript()), 0o600)) + //nolint:gosec // G302: generated wrapper needs owner execute permission. + require.NoError(t, os.Chmod(wrapperPath, 0o700)) + + //nolint:gosec // G204: generated wrapper path is test-controlled. + cmd := exec.Command(wrapperPath, args...) + cmd.Env = []string{"PATH=" + dir, "CAPTURE_FILE=" + capturePath} + if endpoint != nil { + cmd.Env = append(cmd.Env, "DOCKER_HOST="+*endpoint) + } + output, err := cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + argv, err := os.ReadFile(capturePath) //nolint:gosec // G304: test-controlled capture path. + require.NoError(t, err) + encodedArgs := bytes.Split(argv, []byte{0}) + encodedArgs = encodedArgs[:len(encodedArgs)-1] + got := make([]string, len(encodedArgs)) + for i, arg := range encodedArgs { + got[i] = string(arg) + } + return got +} + +func TestRootfulPodmanUsesManagedDaemon(t *testing.T) { + cases := []struct { + endpoint string + local bool + }{ + {"", true}, + {"unix:///run/podman/podman.sock", true}, + {"unix:///var/run/podman/podman.sock", true}, + {"unix:///run/user/1000/podman/podman.sock", false}, + {"unix:///custom/podman.sock", false}, + {"tcp://127.0.0.1:2375", false}, + {"ssh://root@remote/run/podman/podman.sock", false}, + {"unix://remote/run/podman/podman.sock", false}, + {"unix://", false}, + {":invalid", false}, + } + for _, tc := range cases { + t.Run(tc.endpoint, func(t *testing.T) { + assert.Equal(t, tc.local, rootfulPodmanUsesManagedDaemon(tc.endpoint)) + }) + } +} + +func TestUnmanagedPodmanCleanupPreservesLocalRecovery(t *testing.T) { + for _, endpoint := range []string{ + "ssh://root@remote/run/podman/podman.sock", + "unix:///custom/podman.sock", + "unix:///run/user/1000/podman/podman.sock", + } { + t.Run(endpoint, func(t *testing.T) { + checkUnmanagedPodmanCleanup(t, endpoint) + }) + } +} + +func checkUnmanagedPodmanCleanup(t *testing.T, endpoint string) { + t.Helper() + dir := t.TempDir() + binDir := filepath.Join(dir, "bin") + require.NoError(t, os.Mkdir(binDir, 0o700)) + wrapper := "#!/bin/sh\necho 'cannot connect to remote endpoint' >&2\nexit 1\n" + require.NoError( + t, + os.WriteFile(filepath.Join(binDir, podmanRootfulWrapperName), []byte(wrapper), 0o600), + ) + //nolint:gosec // G302: test wrapper needs owner execute permission. + require.NoError(t, os.Chmod(filepath.Join(binDir, podmanRootfulWrapperName), 0o700)) + capture := filepath.Join(dir, "sudo-commands") + sudo := "#!/bin/sh\nprintf '%s\\n' \"$*\" >> \"$CAPTURE_FILE\"\n" + require.NoError(t, os.WriteFile(filepath.Join(binDir, podmanSudoCommand), []byte(sudo), 0o600)) + //nolint:gosec // G302: test sudo stub needs owner execute permission. + require.NoError(t, os.Chmod(filepath.Join(binDir, podmanSudoCommand), 0o700)) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("CAPTURE_FILE", capture) + t.Setenv("DOCKER_HOST", endpoint) + previousGate := rootfulDaemonGate + rootfulDaemonGate = &podmanDaemonGate{} + t.Cleanup(func() { rootfulDaemonGate = previousGate }) + cleanupErr := errors.New("remote cleanup failed") + require.ErrorIs( + t, + recoverPodmanCleanup( + context.Background(), + nil, + podmanCleanupDirs{initialDir: dir}, + cleanupErr, + ), + cleanupErr, + ) + assert.Empty(t, rootfulDaemonGate.unhealthy()) + assert.True(t, rootfulDaemonGate.claimRecovery()) + commands, err := os.ReadFile(capture) //nolint:gosec // G304: test-controlled capture path. + require.NoError(t, err) + assert.NotContains(t, string(commands), "systemctl restart") +} + +//nolint:gosec // Synthetic credentials exercise diagnostic redaction. +func TestPodmanDiagnosticsRedactEndpointCredentials(t *testing.T) { + const endpoint = "ssh://fixture-user:fixture%40password@podman.example/run/podman.sock" + t.Setenv("DOCKER_HOST", endpoint) + dir := t.TempDir() + wrapper := filepath.Join(dir, "wrapper") + script := `#!/bin/sh +printf 'cannot connect to %s; password=fixture@password; encoded=fixture%%40password\n' "$DOCKER_HOST" +exit 1 +` + require.NoError(t, os.WriteFile(wrapper, []byte(script), 0o700)) + class, err := checkPodmanHealth(context.Background(), wrapper) + require.Error(t, err) + assert.Equal(t, podmanHealthUnavailable, class) + var exitError *exec.ExitError + assert.ErrorAs(t, err, &exitError) + assert.Contains(t, err.Error(), "ssh://***@podman.example/run/podman.sock") + assert.Contains(t, err.Error(), "cannot connect") + diagnostics := runDiagCommand(context.Background(), wrapper) + // The error must remain safe even after the originating environment changes. + t.Setenv("DOCKER_HOST", "") + for _, text := range []string{err.Error(), errors.Unwrap(err).Error(), diagnostics} { + assert.NotContains(t, text, "fixture-user") + assert.NotContains(t, text, "fixture@password") + assert.NotContains(t, text, "fixture%40password") + } + assert.Contains(t, diagnostics, "podman.example") +} + +//nolint:gosec // Synthetic credentials cover noncanonical URL escaping. +func TestPodmanDiagnosticsRedactOriginalURLEscaping(t *testing.T) { + t.Setenv("DOCKER_HOST", "ssh://fixture-user:fixture%4apassword@podman.example/run/podman.sock") + text := redactPodmanDiagnostics( + os.Getenv("DOCKER_HOST") + " fixture%4apassword fixtureJpassword", + ) + assert.Equal(t, "ssh://***@podman.example/run/podman.sock *** ***", text) +} diff --git a/e2e/tests/up/provider_microsandbox.go b/e2e/tests/up/provider_microsandbox.go index af6d31feed..3a363864ac 100644 --- a/e2e/tests/up/provider_microsandbox.go +++ b/e2e/tests/up/provider_microsandbox.go @@ -2,13 +2,16 @@ package up import ( "context" + "encoding/json" "fmt" "os" "os/exec" "path/filepath" "runtime" + "strings" "github.com/devsy-org/devsy/e2e/framework" + "github.com/devsy-org/devsy/pkg/devcontainer" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" ) @@ -64,7 +67,7 @@ var _ = ginkgo.Describe( }) // full up: boots the microVM, streams in the agent, opens the tunnel - err = f.DevsyUp(ctx, tempDir) + err = f.DevsyUp(ctx, tempDir, "--devcontainer", ".devcontainer.json") framework.ExpectNoError(err) ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, tempDir) @@ -73,42 +76,157 @@ var _ = ginkgo.Describe( framework.ExpectNoError(err) workspacePath := filepath.Join("/workspaces", filepath.Base(tempDir)) - guestFile := filepath.Join(tempDir, "guest-created.txt") _, err = f.DevsySSHOnce(ctx, tempDir, fmt.Sprintf( - "touch %s/guest-created.txt && chmod 0644 %s/guest-created.txt", + "umask 022; : > %s/guest-created.txt && mkdir %s/guest-created-dir", workspacePath, workspacePath, )) framework.ExpectNoError(err) - guestInfo, err := os.Stat(guestFile) + for name, mode := range map[string]os.FileMode{"guest-created.txt": 0o644, "guest-created-dir": 0o755} { + info, err := os.Stat(filepath.Join(tempDir, name)) + framework.ExpectNoError(err) + gomega.Expect(info.Mode().Perm()).To(gomega.Equal(mode)) + } + + workspace, err := f.FindWorkspace(ctx, tempDir) + framework.ExpectNoError(err) + sandbox := "devsy-" + devcontainer.GetRunnerIDFromWorkspace(workspace) + createdAt := microsandboxCreationTime(ctx, sandbox) + // No --user override: verify the runtime's default execution identity. + // #nosec G204 -- fixed command and Devsy-generated sandbox name + workloadUser, err := exec.CommandContext( + ctx, + "msb", + "exec", + "--stream", + sandbox, + "--", + "id", + "-u", + ).Output() framework.ExpectNoError(err) - gomega.Expect(guestInfo.Mode().Perm()).To(gomega.Equal(os.FileMode(0o644))) + gomega.Expect(strings.TrimSpace(string(workloadUser))).To(gomega.Equal("0")) - hostFile := filepath.Join(tempDir, "host-created.txt") - //nolint:gosec // permission mirroring is the behavior under test - err = os.WriteFile( - hostFile, - []byte("host"), - 0o644, - ) + developer, err := f.DevsySSHOnce(ctx, tempDir, "id -un") framework.ExpectNoError(err) - //nolint:gosec // permission mirroring is the behavior under test - err = os.Chmod(hostFile, 0o644) + gomega.Expect(strings.TrimSpace(developer)).To(gomega.Equal("vscode")) + assertMicrosandboxHostEntries(ctx, f, tempDir, "fresh") + err = f.DevsyWorkspaceStop(ctx, tempDir) framework.ExpectNoError(err) - owner, err := f.DevsySSHOnce(ctx, tempDir, fmt.Sprintf( - "stat -c '%%U %%a' %s/host-created.txt", workspacePath, - )) + err = f.DevsyUp(ctx, tempDir) framework.ExpectNoError(err) - gomega.Expect(owner).To(gomega.ContainSubstring("vscode 644")) + gomega.Expect(microsandboxCreationTime(ctx, sandbox)).To(gomega.Equal(createdAt)) + assertMicrosandboxHostEntries(ctx, f, tempDir, "restarted") + err = f.DevsyUpRecreate(ctx, tempDir) + framework.ExpectNoError(err) + gomega.Expect(microsandboxCreationTime(ctx, sandbox)).NotTo(gomega.Equal(createdAt)) + assertMicrosandboxHostEntries(ctx, f, tempDir, "recreated") - // stop, then bring it back up and confirm it is reachable again - err = f.DevsyWorkspaceStop(ctx, tempDir) + previousCreation := microsandboxCreationTime(ctx, sandbox) + // #nosec G204 -- fixed command and Devsy-generated sandbox name + _, err = exec.CommandContext( + ctx, "msb", "exec", "--stream", sandbox, "--", "sh", "-c", + "echo preserved > /root/recreation-preserved.txt", + ). + Output() framework.ExpectNoError(err) - err = f.DevsyUp(ctx, tempDir) + configPath := filepath.Join(tempDir, ".devcontainer.json") + // #nosec G304 -- configuration copied into the test-owned temporary directory + data, err := os.ReadFile(configPath) + framework.ExpectNoError(err) + var devConfig map[string]any + framework.ExpectNoError(json.Unmarshal(data, &devConfig)) + devConfig["remoteUser"] = "root" + data, err = json.Marshal(devConfig) framework.ExpectNoError(err) + framework.ExpectNoError(os.WriteFile(configPath, data, 0o600)) - err = f.DevsySSHEchoTestString(ctx, tempDir) + stdout, stderr, err := f.DevsyUpStreams( + ctx, tempDir, "--devcontainer", ".devcontainer.json", + ) + gomega.Expect(err).To(gomega.HaveOccurred()) + gomega.Expect(stdout + stderr).To(gomega.ContainSubstring("--recreate")) + gomega.Expect(microsandboxCreationTime(ctx, sandbox)).To(gomega.Equal(previousCreation)) + // #nosec G204 -- fixed command and Devsy-generated sandbox name + preserved, err := exec.CommandContext( + ctx, + "msb", + "exec", + "--stream", + sandbox, + "--", + "cat", + "/root/recreation-preserved.txt", + ). + Output() + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(string(preserved))).To(gomega.Equal("preserved")) + err = f.DevsyUp(ctx, tempDir, "--devcontainer", ".devcontainer.json", "--recreate") framework.ExpectNoError(err) + gomega.Expect(microsandboxCreationTime(ctx, sandbox)). + NotTo(gomega.Equal(previousCreation)) + developer, err = f.DevsySSHOnce(ctx, tempDir, "id -u") + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(developer)).To(gomega.Equal("0")) + assertMicrosandboxHostEntries(ctx, f, tempDir, "identity-changed") }, ginkgo.SpecTimeout(framework.TimeoutModerate())) }, ) + +// Entries are created after setup so recursive workspace chown cannot mask fallback ownership. +func assertMicrosandboxHostEntries( + ctx context.Context, + f *framework.Framework, + hostPath, prefix string, +) { + guestPath := filepath.Join("/workspaces", filepath.Base(hostPath)) + identity, err := f.DevsySSHOnce(ctx, hostPath, `printf '%s:%s' "$(id -u)" "$(id -g)"`) + framework.ExpectNoError(err) + identity = strings.TrimSpace(identity) + + for name, mode := range map[string]os.FileMode{prefix + "-file": 0o644, prefix + "-dir": 0o755} { + hostEntry := filepath.Join(hostPath, name) + if mode == 0o644 { + //nolint:gosec // permission mirroring is the behavior under test + err = os.WriteFile(hostEntry, []byte("host"), mode) + } else { + err = os.Mkdir(hostEntry, mode) + } + framework.ExpectNoError(err) + //nolint:gosec // normalize host umask before checking guest-visible modes + framework.ExpectNoError(os.Chmod(hostEntry, mode)) + ownerBefore := microsandboxHostOwner(ctx, hostEntry) + result, err := f.DevsySSHOnce( + ctx, + hostPath, + fmt.Sprintf("stat -c '%%u:%%g %%a' %s/%s", guestPath, name), + ) + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(result)). + To(gomega.Equal(fmt.Sprintf("%s %o", identity, mode))) + gomega.Expect(microsandboxHostOwner(ctx, hostEntry)).To(gomega.Equal(ownerBefore)) + } +} + +func microsandboxHostOwner(ctx context.Context, entry string) string { + args := []string{"-c", "%u:%g", entry} + if runtime.GOOS == "darwin" { + args = []string{"-f", "%u:%g", entry} + } + // #nosec G204 -- fixed stat executable and test-created path + output, err := exec.CommandContext(ctx, "stat", args...).Output() + framework.ExpectNoError(err) + return strings.TrimSpace(string(output)) +} + +func microsandboxCreationTime(ctx context.Context, sandbox string) string { + // #nosec G204 -- fixed inspect command and Devsy-generated sandbox name + output, err := exec.CommandContext(ctx, "msb", "inspect", sandbox, "--format", "json").Output() + framework.ExpectNoError(err) + var info struct { + CreatedAt string `json:"created_at"` + } + framework.ExpectNoError(json.Unmarshal(output, &info)) + gomega.Expect(info.CreatedAt).NotTo(gomega.BeEmpty()) + return info.CreatedAt +} diff --git a/e2e/tests/up/testdata/microsandbox/.devcontainer.json b/e2e/tests/up/testdata/microsandbox/.devcontainer.json index c8d7c4041c..3517343eed 100644 --- a/e2e/tests/up/testdata/microsandbox/.devcontainer.json +++ b/e2e/tests/up/testdata/microsandbox/.devcontainer.json @@ -1,6 +1,6 @@ { "name": "microsandbox", "image": "mcr.microsoft.com/devcontainers/base:ubuntu", - "containerUser": "vscode", + "containerUser": "root", "remoteUser": "vscode" } diff --git a/pkg/devcontainer/capabilities_test.go b/pkg/devcontainer/capabilities_test.go index 7821abe297..0a21dbcf2d 100644 --- a/pkg/devcontainer/capabilities_test.go +++ b/pkg/devcontainer/capabilities_test.go @@ -135,7 +135,9 @@ type recreateDriver struct { func (d *recreateDriver) RecreateMode() driver.RecreateMode { return d.mode } func TestRecreatePolicy(t *testing.T) { - for _, mode := range []driver.RecreateMode{driver.RecreateDelete, driver.RecreateStop, "invalid"} { + for _, mode := range []driver.RecreateMode{ + driver.RecreateDelete, driver.RecreateStop, driver.RecreateOnRun, "invalid", + } { t.Run(string(mode), func(t *testing.T) { d := &recreateDriver{ mode: mode, diff --git a/pkg/devcontainer/metadata/metadata.go b/pkg/devcontainer/metadata/metadata.go index 57878b5379..91b492f294 100644 --- a/pkg/devcontainer/metadata/metadata.go +++ b/pkg/devcontainer/metadata/metadata.go @@ -10,6 +10,9 @@ import ( const ImageMetadataLabel = pkgconfig.DevcontainerMetadataLabel +// CreationConfigLabel identifies a metadata chain ending with the creation-time workspace config. +const CreationConfigLabel = "devsy.sh/metadata-creation-config" + const metadataLabelSizeWarningThreshold = 100 * 1024 func MarshalImageMetadata(raw []*config.ImageMetadata) ([]byte, error) { diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index 9455d24109..8087abd84a 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -138,52 +138,10 @@ func (r *runner) resolveContainer( params *resolveParams, containerDetails *config.ContainerDetails, ) (*resolvedContainer, error) { - options := params.options - if containerDetails != nil && !options.Recreate && - r.needsTerminalSecretEnvironmentMigration(containerDetails) { - if err := r.validateTerminalSecretEnvironmentSupport(); err != nil { - return nil, err - } - if params.parsedConfig.Config.ContainerID != "" { - return nil, fmt.Errorf( - "cannot inject attached terminal secrets into externally managed container: "+ - "the container does not have the required %s tmpfs mount", - config.SecretsEnvDir, - ) - } - log.Info( - "recreating workspace because attached terminal secrets require the secure runtime mount", - ) - options.Recreate = true - params.options.Recreate = true - } - if containerDetails != nil && !options.Recreate && - needsSecretFileMountMigration(containerDetails, options.SecretsMount) { - if !driver.DriverSupportsMountType(r.driver, driver.MountTypeTmpfs) { - return nil, fmt.Errorf( - "the current provider does not support securely mounting workspace file secrets", - ) - } - if params.parsedConfig.Config.ContainerID != "" { - return nil, fmt.Errorf( - "cannot inject file secrets into externally managed container: "+ - "the container does not have the required %s tmpfs mount", - config.SecretsMountDir, - ) - } - log.Info("recreating workspace because file secrets require the secure runtime mount") - options.Recreate = true - params.options.Recreate = true - } - - if options.Recreate && params.parsedConfig.Config.ContainerID != "" { - return nil, fmt.Errorf("cannot recreate container not created by Devsy") - } - if err := r.provisioningPreflightForRecreate(ctx, options); err != nil { + if err := r.prepareContainerRecreation(ctx, containerDetails, params); err != nil { return nil, err } - - if options.Recreate || containerDetails == nil { + if params.options.Recreate || containerDetails == nil { return r.resolveNewContainer(ctx, params) } @@ -199,6 +157,73 @@ func (r *runner) resolveContainer( return r.resolveExistingContainer(ctx, containerDetails, params) } +func (r *runner) prepareContainerRecreation( + ctx context.Context, + details *config.ContainerDetails, + params *resolveParams, +) error { + if err := r.applyDriverRecreateRequirement(ctx, details, params); err != nil { + return err + } + if err := r.applyTerminalSecretRecreateRequirement(details, params); err != nil { + return err + } + if err := r.applyFileSecretRecreateRequirement(details, params); err != nil { + return err + } + if params.options.Recreate && params.parsedConfig.Config.ContainerID != "" { + return fmt.Errorf("cannot recreate container not created by Devsy") + } + return r.provisioningPreflightForRecreate(ctx, params.options) +} + +func (r *runner) applyTerminalSecretRecreateRequirement( + details *config.ContainerDetails, + params *resolveParams, +) error { + if details == nil || params.options.Recreate || + !r.needsTerminalSecretEnvironmentMigration(details) { + return nil + } + if err := r.validateTerminalSecretEnvironmentSupport(); err != nil { + return err + } + if params.parsedConfig.Config.ContainerID != "" { + return fmt.Errorf( + "cannot inject attached terminal secrets into externally managed container: "+ + "the container does not have the required %s tmpfs mount", + config.SecretsEnvDir, + ) + } + return r.scheduleContainerRecreation( + params, + "attached terminal secrets require the secure runtime mount", + ) +} + +func (r *runner) applyFileSecretRecreateRequirement( + details *config.ContainerDetails, + params *resolveParams, +) error { + if details == nil || params.options.Recreate || + !needsSecretFileMountMigration(details, params.options.SecretsMount) { + return nil + } + if !driver.DriverSupportsMountType(r.driver, driver.MountTypeTmpfs) { + return fmt.Errorf( + "the current provider does not support securely mounting workspace file secrets", + ) + } + if params.parsedConfig.Config.ContainerID != "" { + return fmt.Errorf( + "cannot inject file secrets into externally managed container: "+ + "the container does not have the required %s tmpfs mount", + config.SecretsMountDir, + ) + } + return r.scheduleContainerRecreation(params, "file secrets require the secure runtime mount") +} + func (r *runner) provisioningPreflightForRecreate(ctx context.Context, options UpOptions) error { if !options.Recreate { return nil @@ -288,6 +313,9 @@ func (r *runner) mergeExistingContainerConfig( containerDetails *config.ContainerDetails, p *resolveParams, ) (*config.MergedDevContainerConfig, error) { + if _, ok := r.driver.(driver.RecreateRequiredDriver); ok { + return r.currentContainerIdentity(ctx, containerDetails, p) + } imageMetadataConfig, err := metadata.GetImageMetadataFromContainer( containerDetails, p.substitutionContext, @@ -296,6 +324,14 @@ func (r *runner) mergeExistingContainerConfig( return nil, err } + return r.mergeContainerMetadata(ctx, imageMetadataConfig, p) +} + +func (r *runner) mergeContainerMetadata( + ctx context.Context, + imageMetadataConfig *config.ImageMetadataConfig, + p *resolveParams, +) (*config.MergedDevContainerConfig, error) { if p.options.ExtraDevContainerPath != "" { if imageMetadataConfig == nil { imageMetadataConfig = &config.ImageMetadataConfig{} @@ -425,9 +461,8 @@ func (r *runner) lingerWarning(ctx context.Context) string { return helper.LingerWarning(ctx) } -// buildNewContainerConfig builds the image (deleting the existing container -// first when recreating) and produces the merged devcontainer config from the -// build's image metadata. +// buildNewContainerConfig builds the image and merges its metadata. Drivers +// with runner-managed recreation are torn down after the build succeeds. func (r *runner) buildNewContainerConfig( ctx context.Context, p *resolveParams, @@ -561,6 +596,8 @@ func (r *runner) deleteForRecreate(ctx context.Context) error { } return nil + case driver.RecreateOnRun: + return nil case driver.RecreateStop: if err := r.driver.StopDevContainer(ctx, r.id); err != nil { return fmt.Errorf("stop devcontainer: %w", err) @@ -733,7 +770,11 @@ func (r *runner) runContainer( } } + runOptions.AllowRecreate = p.options.Recreate runOptions.Env = r.addExtraEnvVars(runOptions.Env) + if _, ok := r.driver.(driver.RecreateRequiredDriver); ok { + runOptions.Labels = append(runOptions.Labels, metadata.CreationConfigLabel+"="+stringTrue) + } // Image drivers (Docker, Apple) build and run a local OCI image. if imageDriver, ok := r.driver.(driver.ImageRunner); ok { @@ -856,6 +897,8 @@ func (r *runner) getDockerlessRunOptions( UID: r.workspaceUID(), Image: image, User: containerRootUser, + RemoteUser: effectiveRemoteUser(mergedConfig, buildInfo.Dockerless.User), + Dockerless: true, Entrypoint: "/.dockerless/dockerless", Cmd: []string{ "start", @@ -932,6 +975,7 @@ func (r *runner) getRunOptions( Image: buildInfo.ImageName, ImageBuilt: buildInfo.BuiltLocally, User: user, + RemoteUser: effectiveRemoteUser(mergedConfig, user), Entrypoint: entrypoint, Cmd: cmd, Env: mergedConfig.ContainerEnv, @@ -1097,3 +1141,79 @@ func GetContainerEntrypointAndArgs( } return shShellPath, cmd } + +func effectiveRemoteUser(merged *config.MergedDevContainerConfig, containerUser string) string { + if merged.RemoteUser != "" { + return merged.RemoteUser + } + if containerUser != "" { + return containerUser + } + return containerRootUser +} + +func (r *runner) applyDriverRecreateRequirement( + ctx context.Context, + details *config.ContainerDetails, + p *resolveParams, +) error { + if details == nil || p.options.Recreate { + return nil + } + if _, ok := r.driver.(driver.RecreateRequiredDriver); !ok { + return nil + } + merged, err := r.currentContainerIdentity(ctx, details, p) + if err != nil { + return err + } + containerUser := merged.ContainerUser + if containerUser == "" { + containerUser = details.Config.Labels[config.UserLabel] + } + required, reason := driver.DriverRequiresRecreate( + r.driver, + details, + effectiveRemoteUser(merged, containerUser), + ) + if !required { + return nil + } + if p.parsedConfig.Config.ContainerID != "" { + return fmt.Errorf("cannot migrate externally managed container: %s", reason) + } + return r.scheduleContainerRecreation(p, reason) +} + +func (r *runner) currentContainerIdentity( + ctx context.Context, + details *config.ContainerDetails, + p *resolveParams, +) (*config.MergedDevContainerConfig, error) { + imageMetadata, err := metadata.GetImageMetadataFromContainer(details, p.substitutionContext) + if err != nil { + return nil, err + } + if details.Config.Labels[metadata.CreationConfigLabel] == stringTrue && + len(imageMetadata.Config) > 0 { + imageMetadata.Config = imageMetadata.Config[:len(imageMetadata.Config)-1] + } + imageMetadata.Config = append( + imageMetadata.Config, + metadata.DevContainerConfigToImageMetadata(p.parsedConfig.Config), + ) + return r.mergeContainerMetadata(ctx, imageMetadata, p) +} + +func (r *runner) scheduleContainerRecreation(params *resolveParams, reason string) error { + if driver.DriverRecreateMode(r.driver) == driver.RecreateOnRun { + return fmt.Errorf( + "workspace requires recreation because %s; back up VM-local data and rerun with --recreate: "+ + "replacement discards the VM root disk and cannot roll back after removal", + reason, + ) + } + log.Infof("recreating workspace because %s", reason) + params.options.Recreate = true + return nil +} diff --git a/pkg/devcontainer/single_recreate_test.go b/pkg/devcontainer/single_recreate_test.go new file mode 100644 index 0000000000..0e9b99d202 --- /dev/null +++ b/pkg/devcontainer/single_recreate_test.go @@ -0,0 +1,172 @@ +package devcontainer + +import ( + "errors" + "fmt" + "testing" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/driver" + "github.com/stretchr/testify/require" +) + +const ( + migrationTerminal = "terminal" + migrationFile = "file" +) + +type secretMigrationDriver struct { + *migrationMockDriver + tmpfsSupported bool +} + +func (d *secretMigrationDriver) SupportsMountType(mountType string) bool { + return mountType == driver.MountTypeTmpfs && d.tmpfsSupported +} + +func newSecretMigrationRunner() (*runner, *resolveParams, *secretMigrationDriver) { + d := &secretMigrationDriver{ + migrationMockDriver: &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + }, + } + params := recreateResolveParams() + params.options.Recreate = false + return newTestRunner(d), params, d +} + +func TestSecretMigrationPreflightFailurePreservesExistingContainer(t *testing.T) { + for _, tc := range []struct { + name string + terminal, file, driver bool + explicit, tmpfsSupported bool + }{ + {name: migrationTerminal, terminal: true, tmpfsSupported: true}, + {name: migrationFile, file: true, tmpfsSupported: true}, + {name: "both", terminal: true, file: true, tmpfsSupported: true}, + {name: "driver migration precedes secrets", terminal: true, file: true, driver: true}, + {name: "explicit recreation skips migration", terminal: true, file: true, explicit: true}, + } { + t.Run(tc.name, func(t *testing.T) { + r, params, d := newSecretMigrationRunner() + d.required, d.tmpfsSupported = tc.driver, tc.tmpfsSupported + d.provisioningErr = errors.New("provisioning unavailable") + params.options.Recreate = tc.explicit + if tc.terminal { + r.workspaceConfig.CLIOptions.TerminalSecretEnvNames = []string{ + terminalSecretSentinelName, + } + } + if tc.file { + params.options.SecretsMount = []string{secretFileMountRequestSentinel} + } + _, err := r.resolveContainer(t.Context(), params, runningContainerDetails()) + require.ErrorIs(t, err, d.provisioningErr) + require.True(t, params.options.Recreate) + require.True(t, d.provisioningCalled) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) + }) + } +} + +func TestSecretMigrationRejectsBeforeProvisioning(t *testing.T) { + for _, tc := range []struct { + name string + terminal, file, external bool + tmpfsSupported bool + want string + }{ + {name: "terminal unsupported", terminal: true, want: "securely injecting workspace secrets"}, + {name: "file unsupported", file: true, want: "securely mounting workspace file secrets"}, + { + name: "terminal external", terminal: true, external: true, tmpfsSupported: true, + want: "cannot inject attached terminal secrets into externally managed container", + }, + { + name: "file external", file: true, external: true, tmpfsSupported: true, + want: "cannot inject file secrets into externally managed container", + }, + { + name: "terminal rejection precedes file", terminal: true, file: true, + external: true, tmpfsSupported: true, want: "cannot inject attached terminal secrets", + }, + } { + t.Run(tc.name, func(t *testing.T) { + r, params, d := newSecretMigrationRunner() + d.tmpfsSupported = tc.tmpfsSupported + if tc.terminal { + r.workspaceConfig.CLIOptions.TerminalSecretEnvNames = []string{ + terminalSecretSentinelName, + } + } + if tc.file { + params.options.SecretsMount = []string{secretFileMountRequestSentinel} + } + if tc.external { + params.parsedConfig.Config.ContainerID = testContainerID + } + _, err := r.resolveContainer(t.Context(), params, runningContainerDetails()) + require.ErrorContains(t, err, tc.want) + require.False(t, params.options.Recreate) + require.False(t, d.provisioningCalled) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) + }) + } +} + +func TestPrepareContainerRecreationKeepsCompatibleContainer(t *testing.T) { + for _, details := range []*config.ContainerDetails{ + nil, + {Mounts: []config.ContainerMount{ + {Type: driver.MountTypeTmpfs, Destination: config.SecretsEnvDir}, + {Type: driver.MountTypeTmpfs, Destination: config.SecretsMountDir}, + }}, + } { + r, params, d := newSecretMigrationRunner() + r.workspaceConfig.CLIOptions.TerminalSecretEnvNames = []string{terminalSecretSentinelName} + params.options.SecretsMount = []string{secretFileMountRequestSentinel} + require.NoError(t, r.prepareContainerRecreation(t.Context(), details, params)) + require.False(t, params.options.Recreate) + require.False(t, d.provisioningCalled) + } +} + +type onRunMigrationDriver struct{ *secretMigrationDriver } + +func (*onRunMigrationDriver) RecreateMode() driver.RecreateMode { return driver.RecreateOnRun } + +func TestOnRunMigrationRequiresExplicitRecreation(t *testing.T) { + for _, reason := range []string{"driver", migrationTerminal, migrationFile} { + for _, explicit := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/explicit=%t", reason, explicit), func(t *testing.T) { + _, params, base := newSecretMigrationRunner() + base.tmpfsSupported = true + base.required = reason == "driver" + r := newTestRunner(&onRunMigrationDriver{base}) + params.options.Recreate = explicit + if reason == migrationTerminal { + r.workspaceConfig.CLIOptions.TerminalSecretEnvNames = []string{ + terminalSecretSentinelName, + } + } + if reason == migrationFile { + params.options.SecretsMount = []string{secretFileMountRequestSentinel} + } + err := r.prepareContainerRecreation(t.Context(), runningContainerDetails(), params) + if explicit { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, "--recreate") + } + require.Equal(t, explicit, params.options.Recreate) + require.False(t, base.stopCalled) + require.False(t, base.deleteCalled) + require.Equal(t, explicit, base.provisioningCalled) + }) + } + } +} diff --git a/pkg/devcontainer/single_test.go b/pkg/devcontainer/single_test.go index 9672023c58..869064faf5 100644 --- a/pkg/devcontainer/single_test.go +++ b/pkg/devcontainer/single_test.go @@ -8,6 +8,7 @@ import ( pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/devcontainer/metadata" "github.com/devsy-org/devsy/pkg/provider" "github.com/devsy-org/devsy/pkg/types" "github.com/stretchr/testify/require" @@ -425,3 +426,203 @@ func TestIsRecoveryContainer(t *testing.T) { t.Error("container with the recovery label must be flagged") } } + +type migrationMockDriver struct { + *provisioningPreflightMockDriver + required bool + migrationCalls int + remoteUser string +} + +func (d *migrationMockDriver) RequiresRecreate( + _ *config.ContainerDetails, + user string, +) (bool, string) { + d.remoteUser = user + d.migrationCalls++ + return d.required, "workspace mount contract changed" +} + +func TestDriverMountContractMigration(t *testing.T) { + for _, tt := range []struct { + name string + required, explicit, external, wantRecreate, wantError bool + calls int + }{ + {"incompatible", true, false, false, true, false, 1}, + {"compatible", false, false, false, false, false, 1}, + {"explicit", true, true, false, true, false, 0}, + {"external", true, false, true, false, true, 1}, + } { + t.Run(tt.name, func(t *testing.T) { + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + required: tt.required, + } + r := newTestRunner(d) + params := recreateResolveParams() + params.options.Recreate = tt.explicit + if tt.external { + params.parsedConfig.Config.ContainerID = testContainerID + } + err := r.applyDriverRecreateRequirement( + context.Background(), + runningContainerDetails(), + params, + ) + if tt.wantError { + require.ErrorContains(t, err, "cannot migrate externally managed container") + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantRecreate, params.options.Recreate) + require.Equal(t, tt.calls, d.migrationCalls) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) + }) + } +} + +func TestAutomaticMigrationPreflightFailurePreservesExisting(t *testing.T) { + sentinel := errors.New("unsupported provisioning runtime") + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + provisioningErr: sentinel, + }, + required: true, + } + params := recreateResolveParams() + params.options.Recreate = false + _, err := newTestRunner( + d, + ).resolveContainer(context.Background(), params, runningContainerDetails()) + require.ErrorIs(t, err, sentinel) + require.True(t, d.provisioningCalled) + require.True(t, params.options.Recreate) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) +} + +func TestEffectiveRemoteUser(t *testing.T) { + for _, tt := range []struct{ remote, container, want string }{ + {"vscode", containerRootUser, "vscode"}, {"", "node", "node"}, {"", "", containerRootUser}, + } { + cfg := &config.MergedDevContainerConfig{} + cfg.RemoteUser = tt.remote + require.Equal(t, tt.want, effectiveRemoteUser(cfg, tt.container)) + } +} + +func TestRunOptionsSeparateDeveloperIdentity(t *testing.T) { + r := newTestRunner(&mockDriver{}) + cfg := &config.MergedDevContainerConfig{} + cfg.ContainerUser = containerRootUser + cfg.RemoteUser = "vscode" + info := &config.BuildInfo{ + ImageName: "final-image", + ImageMetadata: &config.ImageMetadataConfig{}, + ImageDetails: &config.ImageDetails{}, + } + info.ImageDetails.Config.User = "node" + options, err := r.getRunOptions(cfg, &config.SubstitutionContext{}, info, false) + require.NoError(t, err) + require.Equal(t, containerRootUser, options.User) + require.Equal(t, "vscode", options.RemoteUser) + info.Dockerless = &config.BuildInfoDockerless{User: "node"} + options, err = r.getDockerlessRunOptions(cfg, &config.SubstitutionContext{}, info, false) + require.NoError(t, err) + require.Equal(t, containerRootUser, options.User) + require.Equal(t, "vscode", options.RemoteUser) + require.True(t, options.Dockerless) +} + +func TestMigrationUsesCurrentDeveloperIdentity(t *testing.T) { + for _, tt := range []struct{ remote, container, image, want string }{ + {"new-user", containerRootUser, "node", "new-user"}, + {"", "node", "vscode", "node"}, + {"", "", "node", "node"}, + {"", "", "", containerRootUser}, + } { + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + } + p := recreateResolveParams() + p.options.Recreate = false + p.parsedConfig.Config.RemoteUser = tt.remote + p.parsedConfig.Config.ContainerUser = tt.container + details := runningContainerDetails() + details.Config.Labels[config.UserLabel] = tt.image + require.NoError( + t, + newTestRunner(d).applyDriverRecreateRequirement(context.Background(), details, p), + ) + require.Equal(t, tt.want, d.remoteUser) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) + } +} + +func TestMigrationUsesImageMetadataDeveloperIdentity(t *testing.T) { + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + } + p := recreateResolveParams() + p.options.Recreate = false + p.substitutionContext = &config.SubstitutionContext{} + details := runningContainerDetails() + details.Config.Labels["devcontainer.metadata"] = `[{"remoteUser":"feature-user"}]` + require.NoError( + t, + newTestRunner(d).applyDriverRecreateRequirement(context.Background(), details, p), + ) + require.Equal(t, "feature-user", d.remoteUser) +} + +func TestMigrationReplacesSavedUserSettingsWithCurrentConfiguration(t *testing.T) { + for _, tt := range []struct{ current, want string }{{"root", "root"}, {"", "feature-user"}} { + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + } + p := recreateResolveParams() + p.options.Recreate = false + p.substitutionContext = &config.SubstitutionContext{} + p.parsedConfig.Config.RemoteUser = tt.current + details := runningContainerDetails() + details.Config.Labels[metadata.CreationConfigLabel] = stringTrue + details.Config.Labels[metadata.ImageMetadataLabel] = `[{"remoteUser":"feature-user"},{"remoteUser":"old-user"}]` + require.NoError( + t, + newTestRunner(d).applyDriverRecreateRequirement(context.Background(), details, p), + ) + require.Equal(t, tt.want, d.remoteUser) + } +} + +func TestOwnerIndependentReuseRefreshesDeveloperIdentity(t *testing.T) { + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + } + p := recreateResolveParams() + p.options.Recreate = false + p.parsedConfig.Config.RemoteUser = "current-user" + p.substitutionContext = &config.SubstitutionContext{} + details := runningContainerDetails() + details.Config.Labels[metadata.CreationConfigLabel] = stringTrue + details.Config.Labels[metadata.ImageMetadataLabel] = `[{"remoteUser":"previous-user"}]` + merged, err := newTestRunner(d).mergeExistingContainerConfig(context.Background(), details, p) + require.NoError(t, err) + require.Equal(t, "current-user", merged.RemoteUser) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) +} diff --git a/pkg/driver/drivercreate/capabilities_test.go b/pkg/driver/drivercreate/capabilities_test.go index 298f3d7b6d..d606dcc3b0 100644 --- a/pkg/driver/drivercreate/capabilities_test.go +++ b/pkg/driver/drivercreate/capabilities_test.go @@ -19,6 +19,7 @@ func TestBuiltInCapabilities(t *testing.T) { name string runtime string images, imageRunner, streaming, chown, argv bool + recreateRequired bool recreate driver.RecreateMode }{ {name: provider.DockerDriver, images: true, imageRunner: true, recreate: driver.RecreateDelete}, @@ -27,7 +28,7 @@ func TestBuiltInCapabilities(t *testing.T) { {name: provider.CustomDriver, streaming: true, chown: true, recreate: driver.RecreateStop}, { name: provider.MicrosandboxDriver, images: true, imageRunner: true, - chown: true, argv: true, recreate: driver.RecreateDelete, + chown: true, argv: true, recreateRequired: true, recreate: driver.RecreateOnRun, }, } { t.Run(tc.name+tc.runtime, func(t *testing.T) { @@ -55,6 +56,8 @@ func TestBuiltInCapabilities(t *testing.T) { assert.Equal(t, tc.streaming, driver.DriverRequiresMountStreaming(bundle.Runtime)) assert.Equal(t, tc.chown, driver.DriverRequiresWorkspaceChown(bundle.Runtime)) assert.Equal(t, tc.recreate, driver.DriverRecreateMode(bundle.Runtime)) + _, recreateRequired := bundle.Runtime.(driver.RecreateRequiredDriver) + assert.Equal(t, tc.recreateRequired, recreateRequired) if tc.name == provider.MicrosandboxDriver { assert.NotSame(t, bundle.Runtime, bundle.Images) _, runtimeImages := bundle.Runtime.(driver.ImageBackend) diff --git a/pkg/driver/microsandbox/cliclient.go b/pkg/driver/microsandbox/cliclient.go index 25f9d7f24c..9624bcff30 100644 --- a/pkg/driver/microsandbox/cliclient.go +++ b/pkg/driver/microsandbox/cliclient.go @@ -9,18 +9,19 @@ import ( "os" "os/exec" "path/filepath" - "runtime" "strconv" "strings" "time" + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/driver" "github.com/devsy-org/devsy/pkg/flags/names" - "github.com/devsy-org/devsy/pkg/image" "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/tarball" ) -type cliClient struct{} +type cliClient struct{ dockerPath string } var _ sandboxClient = cliClient{} @@ -50,25 +51,36 @@ func (cliClient) Version(ctx context.Context) (string, error) { return strings.TrimSpace(string(out)), nil } -func (cliClient) EnsureImage(ctx context.Context, imageRef string) error { - if dockerImageExists(ctx, imageRef) { - return loadFromDocker(ctx, imageRef) - } - // #nosec G204 -- args are a resolved binary path and a validated image ref - out, err := exec.CommandContext(ctx, msbBinary(), "pull", imageRef).CombinedOutput() - if err == nil { - return nil +func (c cliClient) PrepareImage( + ctx context.Context, + ref string, + builtLocally bool, +) (*preparedImage, error) { + img, cleanup, err := filesystemUserResolver(c).openImage(ctx, ref, builtLocally) + if err != nil { + return nil, err } - if loadErr := loadViaRegistry(ctx, imageRef); loadErr != nil { - return fmt.Errorf("msb pull %s: %s: %w; registry fallback: %v", imageRef, out, err, loadErr) + digest, err := img.Digest() + if err != nil { + cleanup() + return nil, fmt.Errorf("identify final image %q: %w", ref, err) } - return nil + return &preparedImage{ + reference: "devsy-msb-image:" + digest.Hex, + image: img, + cleanup: cleanup, + }, nil +} + +func (cliClient) EnsureImage(ctx context.Context, prepared *preparedImage) error { + return loadImageSnapshot(ctx, prepared.reference, prepared.image) +} + +func (c cliClient) PrepareVolumes(ctx context.Context, mounts []volumeMount) error { + return c.ensureVolumes(ctx, mounts) } func (c cliClient) Create(ctx context.Context, sandbox string, spec sandboxSpec) error { - if err := c.ensureVolumes(ctx, spec.Mounts); err != nil { - return err - } return msbRun(ctx, runArgs(sandbox, spec)...) } @@ -89,24 +101,41 @@ func (cliClient) Find(ctx context.Context, sandbox string) (*sandboxInfo, error) } return nil, fmt.Errorf("inspect microsandbox VM %q: %w", sandbox, err) } + return parseSandboxInfo(out) +} + +func parseSandboxInfo(out []byte) (*sandboxInfo, error) { type activeConfig struct { Labels map[string]string `json:"labels"` + Mounts []inspectedMount `json:"mounts"` } var raw struct { - Name string `json:"name"` - Status string `json:"status"` - CreatedAt string `json:"created_at"` - ActiveConfig activeConfig `json:"active_config"` + Name string `json:"name"` + Status string `json:"status"` + CreatedAt string `json:"created_at"` + ActiveConfig *activeConfig `json:"active_config"` + Config *activeConfig `json:"config"` } if err := json.Unmarshal(out, &raw); err != nil { return nil, fmt.Errorf("parse microsandbox inspect output: %w", err) } + current := raw.ActiveConfig + if current == nil { + current = raw.Config + } + var labels map[string]string + var mounts []config.ContainerMount + if current != nil { + labels = current.Labels + mounts = inspectedMountDetails(current.Mounts) + } created, _ := time.Parse(time.RFC3339Nano, raw.CreatedAt) return &sandboxInfo{ Name: raw.Name, Running: strings.EqualFold(raw.Status, "running"), CreatedAt: created, - Labels: raw.ActiveConfig.Labels, + Labels: labels, + Mounts: mounts, }, nil } @@ -290,72 +319,38 @@ func redactArgs(args []string) string { return strings.Join(out, " ") } -func dockerImageExists(ctx context.Context, image string) bool { - docker, err := exec.LookPath("docker") +func loadImageSnapshot(ctx context.Context, imageRef string, img v1.Image) error { + ref, err := name.ParseReference(imageRef) if err != nil { - return false + return fmt.Errorf("parse image reference %q: %w", imageRef, err) } - // #nosec G204 -- docker path is resolved and the image ref is validated - return exec.CommandContext(ctx, docker, "image", "inspect", image).Run() == nil -} -func loadFromDocker(ctx context.Context, image string) error { - docker, err := exec.LookPath("docker") + // #nosec G204 -- resolved runtime binary and content-derived image reference + load := exec.CommandContext(ctx, msbBinary(), "load", "-t", imageRef) + var output strings.Builder + load.Stdout, load.Stderr = &output, &output + writer, err := load.StdinPipe() if err != nil { - return fmt.Errorf("docker not found to load built image %q: %w", image, err) + return fmt.Errorf("pipe image import: %w", err) } - // #nosec G204 -- docker/msb paths are resolved and the image ref is validated - save := exec.CommandContext(ctx, docker, "save", image) - // #nosec G204 -- docker/msb paths are resolved and the image ref is validated - load := exec.CommandContext(ctx, msbBinary(), "load", "-t", image) - pipe, err := save.StdoutPipe() - if err != nil { - return fmt.Errorf("pipe docker save: %w", err) - } - load.Stdin = pipe - var loadErr strings.Builder - load.Stderr = &loadErr + defer func() { _ = writer.Close() }() + stopClose := context.AfterFunc(ctx, func() { _ = writer.Close() }) + defer stopClose() if err := load.Start(); err != nil { - return fmt.Errorf("start msb load: %w", err) + return fmt.Errorf("start msb image import: %w", err) } - if err := save.Run(); err != nil { - // load is still running on the broken pipe; kill and reap it. + writeErr := tarball.Write(ref, img, writer) + closeErr := writer.Close() + if err := errors.Join(writeErr, closeErr, ctx.Err()); err != nil { _ = load.Process.Kill() _ = load.Wait() - return fmt.Errorf("docker save %q: %w", image, err) + return fmt.Errorf("stream image %q: %s: %w", imageRef, output.String(), err) } if err := load.Wait(); err != nil { - return fmt.Errorf("msb load %q: %s: %w", image, loadErr.String(), err) - } - return nil -} - -func loadViaRegistry(ctx context.Context, imageRef string) error { - ref, err := name.ParseReference(imageRef) - if err != nil { - return fmt.Errorf("parse image reference %q: %w", imageRef, err) - } - img, err := image.GetImageForArch(ctx, imageRef, runtime.GOARCH) - if err != nil { - return fmt.Errorf("pull image %q: %w", imageRef, err) - } - - tmp, err := os.CreateTemp("", "devsy-msb-*.tar") - if err != nil { - return fmt.Errorf("create image tarball: %w", err) - } - tmpPath := tmp.Name() - _ = tmp.Close() - defer func() { _ = os.Remove(tmpPath) }() - - if err := tarball.WriteToFile(tmpPath, ref, img); err != nil { - return fmt.Errorf("write image tarball: %w", err) - } - - // #nosec G204 -- args are a resolved binary path and an internally-created file - load := exec.CommandContext(ctx, msbBinary(), "load", "-i", tmpPath, "-t", imageRef) - if out, err := load.CombinedOutput(); err != nil { - return fmt.Errorf("msb load %q: %s: %w", imageRef, out, err) + return fmt.Errorf( + "msb load %q: %s: %w", + imageRef, output.String(), errors.Join(err, ctx.Err()), + ) } return nil } @@ -376,3 +371,29 @@ func msbBinary() string { } return "msb" } + +type inspectedMount struct { + Type string `json:"type"` + Host string `json:"host"` + Guest string `json:"guest"` +} + +func inspectedMountDetails(mounts []inspectedMount) []config.ContainerMount { + var details []config.ContainerMount + for _, mount := range mounts { + var mountType string + switch mount.Type { + case "Bind": + mountType = driver.MountTypeBind + case "Tmpfs": + mountType = driver.MountTypeTmpfs + default: + continue + } + details = append( + details, + config.ContainerMount{Type: mountType, Source: mount.Host, Destination: mount.Guest}, + ) + } + return details +} diff --git a/pkg/driver/microsandbox/cliclient_test.go b/pkg/driver/microsandbox/cliclient_test.go index 0ff80ff49e..14f77b8e85 100644 --- a/pkg/driver/microsandbox/cliclient_test.go +++ b/pkg/driver/microsandbox/cliclient_test.go @@ -7,7 +7,10 @@ import ( "testing" "time" + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/driver" "github.com/devsy-org/devsy/pkg/flags/names" + "github.com/stretchr/testify/require" ) func runPrefix() []string { @@ -228,3 +231,72 @@ func hasFlagValue(args []string, flag, value string) bool { } return false } + +func TestInspectStoppedSandboxRetainsMountContract(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + output := fmt.Sprintf( + `{"name":"%s","status":"Stopped","active_config":null,"config":{"labels":{`+ + `"%s":"%s","devsy.sh/microsandbox-workspace-user":"root","%s":"root"}}}`, + wsName, + workspaceMountContractLabel, + d.workspaceMountContract(), + userLabel, + ) + info, err := parseSandboxInfo([]byte(output)) + if err != nil { + t.Fatal(err) + } + details := toContainerDetails(info) + required, _ := d.RequiresRecreate(details, rootUser) + if required { + t.Fatal("stopped sandbox lost persisted contract") + } + if details.Config.User != "root" { + t.Fatal("stopped sandbox lost container identity") + } +} + +func TestInspectUsesActiveContractWhenConfigHasPendingChanges(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + output := fmt.Sprintf( + `{"name":"%s","status":"Running","active_config":{"labels":{`+ + `"%s":"%s","devsy.sh/microsandbox-workspace-user":"root"}},"config":{"labels":{"%s":"pending"}}}`, + wsName, + workspaceMountContractLabel, + d.workspaceMountContract(), + workspaceMountContractLabel, + ) + info, err := parseSandboxInfo([]byte(output)) + if err != nil { + t.Fatal(err) + } + required, _ := d.RequiresRecreate(toContainerDetails(info), rootUser) + if required { + t.Fatal("pending configuration replaced active mount contract") + } +} + +func TestInspectReportsPersistedMounts(t *testing.T) { + for _, field := range []string{"config", "active_config"} { + t.Run(field, func(t *testing.T) { + output := fmt.Sprintf( + `{"name":"%s","%s":{"mounts":[`+ + `{"type":"Bind","host":"/host/project","guest":"/workspaces/project"},`+ + `{"type":"Tmpfs","guest":"%s"}]}}`, + wsName, + field, + config.SecretsEnvDir, + ) + info, err := parseSandboxInfo([]byte(output)) + require.NoError(t, err) + require.Equal(t, []config.ContainerMount{ + { + Type: driver.MountTypeBind, + Source: "/host/project", + Destination: "/workspaces/project", + }, + {Type: driver.MountTypeTmpfs, Destination: config.SecretsEnvDir}, + }, toContainerDetails(info).Mounts) + }) + } +} diff --git a/pkg/driver/microsandbox/client.go b/pkg/driver/microsandbox/client.go index ac0a325a61..6fa3be4f9b 100644 --- a/pkg/driver/microsandbox/client.go +++ b/pkg/driver/microsandbox/client.go @@ -4,8 +4,17 @@ import ( "context" "io" "time" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" + v1 "github.com/google/go-containerregistry/pkg/v1" ) +type preparedImage struct { + reference string + image v1.Image + cleanup func() +} + type sandboxSpec struct { Image string User string @@ -38,6 +47,7 @@ type sandboxInfo struct { Running bool CreatedAt time.Time Labels map[string]string + Mounts []config.ContainerMount } type execRequest struct { @@ -55,7 +65,9 @@ type execRequest struct { type sandboxClient interface { EnsureInstalled(ctx context.Context) error Version(ctx context.Context) (string, error) - EnsureImage(ctx context.Context, image string) error + PrepareImage(ctx context.Context, image string, builtLocally bool) (*preparedImage, error) + EnsureImage(ctx context.Context, image *preparedImage) error + PrepareVolumes(ctx context.Context, mounts []volumeMount) error Create(ctx context.Context, name string, spec sandboxSpec) error Find(ctx context.Context, name string) (*sandboxInfo, error) Start(ctx context.Context, name string) error diff --git a/pkg/driver/microsandbox/image_user.go b/pkg/driver/microsandbox/image_user.go new file mode 100644 index 0000000000..a6b0916770 --- /dev/null +++ b/pkg/driver/microsandbox/image_user.go @@ -0,0 +1,334 @@ +package microsandbox + +import ( + "archive/tar" + "context" + "encoding/json" + "fmt" + "io" + "os" + "os/exec" + "path" + "runtime" + "strconv" + "strings" + "time" + + "github.com/devsy-org/devsy/pkg/image" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/tarball" +) + +const ( + maxAccountFileSize = 4 << 20 + rootUser = "root" + dockerExecutable = "docker" + passwdPath = "etc/passwd" + groupPath = "etc/group" +) + +type imageUserIdentity struct { + name, group string + uid, gid uint32 + numericUID, numericGID bool +} + +type imageUserResolver interface { + ResolveImage(ctx context.Context, img v1.Image, user string) (*mountOwner, error) +} + +type filesystemUserResolver struct{ dockerPath string } + +func (filesystemUserResolver) ResolveImage( + ctx context.Context, + img v1.Image, + user string, +) (*mountOwner, error) { + if owner, complete, err := explicitOwner(user); complete || err != nil { + return owner, err + } + return ownerFromImage(ctx, img, user) +} + +func (r filesystemUserResolver) openImage( + ctx context.Context, + ref string, + builtLocally bool, +) (v1.Image, func(), error) { + if err := ctx.Err(); err != nil { + return nil, nil, err + } + if builtLocally || localImageAvailable(ctx, r.dockerPath, ref) { + return r.localImage(ctx, ref) + } + img, err := image.GetImageForArch(ctx, ref, runtime.GOARCH) + if err != nil { + return nil, nil, err + } + return img, func() {}, nil +} + +// The archive must remain available until lazy layer reads finish. +func (r filesystemUserResolver) localImage( + ctx context.Context, + ref string, +) (v1.Image, func(), error) { + archive, err := os.CreateTemp("", "devsy-msb-owner-*.tar") + if err != nil { + return nil, nil, err + } + cleanup := func() { _ = os.Remove(archive.Name()) } + docker := r.dockerPath + if docker == "" { + docker = dockerExecutable + } + // #nosec G204 -- configured Docker-compatible executable, fixed subcommand, image argument + cmd := exec.CommandContext(ctx, docker, "save", ref) + cmd.Stdout = archive + var stderr strings.Builder + cmd.Stderr = &stderr + runErr := cmd.Run() + closeErr := archive.Close() + if runErr != nil { + cleanup() + return nil, nil, fmt.Errorf( + "save final local image %q: %s: %w", + ref, + strings.TrimSpace(stderr.String()), + runErr, + ) + } + if closeErr != nil { + cleanup() + return nil, nil, closeErr + } + img, err := tarball.ImageFromPath(archive.Name(), nil) + if err != nil { + cleanup() + return nil, nil, fmt.Errorf("read final local image %q: %w", ref, err) + } + return img, cleanup, nil +} + +func numericID(value string) (uint32, bool, error) { + if value == "" { + return 0, false, nil + } + for _, c := range value { + if c < '0' || c > '9' { + return 0, false, nil + } + } + n, err := strconv.ParseUint(value, 10, 32) + if err != nil { + return 0, true, fmt.Errorf("invalid numeric account ID %q: %w", value, err) + } + return uint32(n), true, nil +} + +func parseImageUserIdentity(user string) (imageUserIdentity, error) { + name, group, hasGroup := strings.Cut(user, ":") + if name == "" || (hasGroup && (group == "" || strings.Contains(group, ":"))) { + return imageUserIdentity{}, fmt.Errorf("invalid user identity %q", user) + } + uid, numericUID, err := numericID(name) + if err != nil { + return imageUserIdentity{}, err + } + gid, numericGID, err := numericID(group) + if err != nil { + return imageUserIdentity{}, err + } + return imageUserIdentity{ + name: name, + group: group, + uid: uid, + gid: gid, + numericUID: numericUID, + numericGID: numericGID, + }, nil +} + +func explicitOwner(user string) (*mountOwner, bool, error) { + identity, err := parseImageUserIdentity(user) + if err != nil { + return nil, false, err + } + if identity.group == "" { + if identity.name == rootUser || (identity.numericUID && identity.uid == 0) { + return &mountOwner{}, true, nil + } + } + if identity.numericUID && identity.numericGID { + return &mountOwner{UID: identity.uid, GID: identity.gid}, true, nil + } + return nil, false, nil +} + +func ownerFromImage(ctx context.Context, img v1.Image, user string) (*mountOwner, error) { + if owner, complete, err := explicitOwner(user); complete || err != nil { + return owner, err + } + accounts, err := imageAccountFiles(ctx, img) + if err != nil { + return nil, err + } + return ownerFromAccounts(user, accounts[passwdPath], accounts[groupPath]) +} + +func imageAccountFiles(ctx context.Context, img v1.Image) (map[string]string, error) { + stream := mutate.Extract(img) + defer func() { _ = stream.Close() }() + accounts := map[string]string{} + reader := tar.NewReader(stream) + for len(accounts) < 2 { + if err := ctx.Err(); err != nil { + return nil, err + } + header, err := reader.Next() + if err == io.EOF { + break + } + if err != nil { + return nil, fmt.Errorf("read image accounts: %w", err) + } + file := path.Clean(strings.TrimPrefix(header.Name, "/")) + if file != passwdPath && file != groupPath { + continue + } + accounts[file], err = readAccountFile(reader, header) + if err != nil { + return nil, err + } + } + return accounts, nil +} + +func readAccountFile(reader io.Reader, header *tar.Header) (string, error) { + if header.Typeflag != tar.TypeReg { + return "", fmt.Errorf("final image /%s is not a regular account file", header.Name) + } + data, err := io.ReadAll(io.LimitReader(reader, maxAccountFileSize+1)) + if err != nil { + return "", fmt.Errorf("read image /%s: %w", header.Name, err) + } + if len(data) > maxAccountFileSize { + return "", fmt.Errorf("image /%s exceeds account file size limit", header.Name) + } + return string(data), nil +} + +func ownerFromAccounts(user, passwd, groups string) (*mountOwner, error) { + name, group, hasGroup := strings.Cut(user, ":") + uid, numeric, err := numericID(name) + if err != nil { + return nil, err + } + owner := &mountOwner{UID: uid} + // Explicit numeric IDs need no passwd entry when a group is supplied. + if !numeric || !hasGroup { + owner, err = passwdOwner(passwd, name, uid, numeric) + if err != nil { + return nil, err + } + } + if hasGroup { + owner.GID, err = groupID(groups, group) + if err != nil { + return nil, err + } + } + return owner, nil +} + +func passwdOwner(passwd, name string, uid uint32, numeric bool) (*mountOwner, error) { + for line := range strings.SplitSeq(passwd, "\n") { + fields := strings.Split(line, ":") + if len(fields) != 7 { + continue + } + if !matchesPasswdRow(fields, name, uid, numeric) { + continue + } + return passwdRowOwner(fields) + } + return nil, fmt.Errorf("user %q not found in final image /etc/passwd", name) +} + +func matchesPasswdRow(fields []string, name string, uid uint32, numeric bool) bool { + if !numeric { + return fields[0] == name + } + entryUID, valid, err := numericID(fields[2]) + return valid && err == nil && entryUID == uid +} + +func passwdRowOwner(fields []string) (*mountOwner, error) { + uid, validUID, uidErr := numericID(fields[2]) + gid, validGID, gidErr := numericID(fields[3]) + if uidErr != nil || gidErr != nil || !validUID || !validGID { + return nil, fmt.Errorf("invalid account IDs for %q in final image /etc/passwd", fields[0]) + } + return &mountOwner{UID: uid, GID: gid}, nil +} + +func groupID(groups, group string) (uint32, error) { + gid, numeric, err := numericID(group) + if err != nil || numeric { + return gid, err + } + for line := range strings.SplitSeq(groups, "\n") { + fields := strings.Split(line, ":") + if len(fields) != 4 || fields[0] != group { + continue + } + gid, valid, err := numericID(fields[2]) + if err != nil || !valid { + return 0, fmt.Errorf("invalid group ID for %q in final image /etc/group", group) + } + return gid, nil + } + return 0, fmt.Errorf("group %q not found in final image /etc/group", group) +} + +func localImageAvailable(ctx context.Context, dockerPath, ref string) bool { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + if dockerPath == "" { + dockerPath = dockerExecutable + } + // #nosec G204 -- configured Docker-compatible executable and fixed inspection arguments + return exec.CommandContext(ctx, dockerPath, "image", "inspect", ref).Run() == nil +} + +func inspectImageConfig(ctx context.Context, dockerPath, ref string) (*v1.Config, error) { + if dockerPath == "" { + dockerPath = dockerExecutable + } + inspectCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + // #nosec G204 -- configured Docker-compatible executable and fixed inspection arguments + out, err := exec.CommandContext(inspectCtx, dockerPath, + "image", "inspect", "--format", "{{json .Config}}", ref).Output() + if err == nil { + var cfg v1.Config + if err := json.Unmarshal(out, &cfg); err != nil { + return nil, fmt.Errorf("parse cached image config for %s: %w", ref, err) + } + return &cfg, nil + } + if err := ctx.Err(); err != nil { + return nil, err + } + // A failed cache probe must not make Docker a requirement for registry images. + img, err := image.GetImageForArch(ctx, ref, runtime.GOARCH) + if err != nil { + return nil, err + } + cfg, err := img.ConfigFile() + if err != nil { + return nil, err + } + return &cfg.Config, nil +} diff --git a/pkg/driver/microsandbox/image_user_test.go b/pkg/driver/microsandbox/image_user_test.go new file mode 100644 index 0000000000..3862dbf387 --- /dev/null +++ b/pkg/driver/microsandbox/image_user_test.go @@ -0,0 +1,376 @@ +package microsandbox + +import ( + "archive/tar" + "bytes" + "context" + "crypto/rand" + "encoding/json" + "io" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/tarball" + "github.com/stretchr/testify/suite" +) + +//nolint:gosec // Synthetic account fixture contains no credentials. +const snapshotPasswd = "vscode:x:2000:2001:dev:/home/vscode:/bin/sh\n" + +type imageUserSuite struct{ suite.Suite } + +func TestImageUser(t *testing.T) { suite.Run(t, new(imageUserSuite)) } + +func (s *imageUserSuite) TestResolution() { + img, err := mutate.AppendLayers(empty.Image, s.layer(map[string]string{ + passwdPath: "invalid-row\nroot:x:0:0:root:/root:/bin/sh\nvscode:x:1000:1002:dev:/home/vscode:/bin/sh\n", + groupPath: "developers:x:1001:vscode\n", + })) + s.Require().NoError(err) + for _, tt := range []struct { + user string + want mountOwner + }{ + {rootUser, mountOwner{}}, + {"0", mountOwner{}}, + {testUser, mountOwner{1000, 1002}}, + {"1000", mountOwner{1000, 1002}}, + {testNumericIdentity, mountOwner{1000, 1001}}, + {"vscode:developers", mountOwner{1000, 1001}}, + {"vscode:1003", mountOwner{1000, 1003}}, + {"1000:developers", mountOwner{1000, 1001}}, + } { + s.Run(tt.user, func() { + owner, err := ownerFromImage(context.Background(), img, tt.user) + s.Require().NoError(err) + s.Equal(tt.want, *owner) + }) + } + for _, user := range []string{ + "missing", "1009", "vscode:missing", "4294967296:1", "vscode:4294967296", "", "vscode:", "1:2:3", + } { + s.Run( + user, + func() { _, err := ownerFromImage(context.Background(), img, user); s.Error(err) }, + ) + } +} + +func (s *imageUserSuite) TestMissingAccounts() { + owner, err := ownerFromImage(context.Background(), empty.Image, testNumericIdentity) + s.Require().NoError(err) + s.Equal(mountOwner{1000, 1001}, *owner) + _, err = ownerFromImage(context.Background(), empty.Image, testUser) + s.ErrorContains(err, "not found in final image /etc/passwd") +} + +//nolint:gosec // Synthetic account rows contain no credentials. +func (s *imageUserSuite) TestFinalLayerOverridesAndWhiteouts() { + base := s.layer( + map[string]string{ + passwdPath: "vscode:x:10:11:dev:/home/vscode:/bin/sh\n", + groupPath: "old:x:99:\n", + }, + ) + final := s.layer( + map[string]string{ + passwdPath: snapshotPasswd, + "etc/.wh.group": "", + }, + ) + img, err := mutate.AppendLayers(empty.Image, base, final) + s.Require().NoError(err) + owner, err := ownerFromImage(context.Background(), img, testUser) + s.Require().NoError(err) + s.Equal(mountOwner{2000, 2001}, *owner) + _, err = ownerFromImage(context.Background(), img, "vscode:old") + s.ErrorContains(err, "not found") +} + +func (s *imageUserSuite) TestMalformedAccount() { + for _, passwd := range []string{ + "vscode:x:no:1000:dev:/home/vscode:/bin/sh\n", + "vscode:x:4294967296:1000:dev:/home/vscode:/bin/sh\n", + } { + _, err := ownerFromAccounts(testUser, passwd, "") + s.ErrorContains(err, "invalid account IDs") + } +} + +func (s *imageUserSuite) TestBoundedAccountFile() { + img, err := mutate.AppendLayers( + empty.Image, + s.layer(map[string]string{passwdPath: strings.Repeat("x", maxAccountFileSize+1)}), + ) + s.Require().NoError(err) + _, err = ownerFromImage(context.Background(), img, testUser) + s.ErrorContains(err, "size limit") +} + +func (s *imageUserSuite) TestCanceledExtraction() { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := ownerFromImage(ctx, empty.Image, testUser) + s.ErrorIs(err, context.Canceled) +} + +//nolint:gosec // Synthetic account archive and executable fixture under TempDir. +func (s *imageUserSuite) TestLocalFinalImageUsesConfiguredCLI() { + s.requirePOSIXShell() + img, err := mutate.AppendLayers( + empty.Image, + s.layer(map[string]string{passwdPath: snapshotPasswd}), + ) + s.Require().NoError(err) + cfg, err := img.ConfigFile() + s.Require().NoError(err) + cfg.Config.User = testUser + img, err = mutate.ConfigFile(img, cfg) + s.Require().NoError(err) + dir := s.T().TempDir() + archive := filepath.Join(dir, "image.tar") + tag, err := name.NewTag("final-image:latest") + s.Require().NoError(err) + s.Require().NoError(tarball.WriteToFile(archive, tag, img)) + argsFile := filepath.Join(dir, "args") + s.T().Setenv("DEVSY_TEST_ARCHIVE", archive) + s.T().Setenv("DEVSY_TEST_ARGS", argsFile) + executable := filepath.Join(dir, "configured-cli") + script := `#!/bin/sh +if [ "$1" = image ]; then exit 0; fi +printf '%s\n' "$@" > "$DEVSY_TEST_ARGS" +cat "$DEVSY_TEST_ARCHIVE" +` + s.Require().NoError(os.WriteFile(executable, []byte(script), 0o700)) + for _, built := range []bool{true, false} { + resolver := filesystemUserResolver{dockerPath: executable} + snapshot, cleanup, err := resolver.openImage( + context.Background(), + "final-image:latest", + built, + ) + s.Require().NoError(err) + defer cleanup() + owner, err := resolver.ResolveImage(context.Background(), snapshot, testUser) + s.Require().NoError(err) + s.Equal(mountOwner{2000, 2001}, *owner) + } + args, err := os.ReadFile(argsFile) + s.Require().NoError(err) + s.Equal("save\nfinal-image:latest\n", string(args)) +} + +//nolint:gosec // Synthetic archives and executable fixtures under TempDir. +func (s *imageUserSuite) TestSnapshotSurvivesRetagBeforeImport() { + s.requirePOSIXShell() + for _, source := range []string{"local", "registry"} { + s.Run(source, func() { + ctx := context.Background() + original := s.snapshotImage(snapshotPasswd) + replacement := s.snapshotImage("vscode:x:3000:3001:dev:/home/vscode:/bin/sh\n") + dir := s.T().TempDir() + archive := filepath.Join(dir, "source.tar") + imported := filepath.Join(dir, "imported.tar") + aliasFile := filepath.Join(dir, "alias") + s.T().Setenv("DEVSY_TEST_ARCHIVE", archive) + s.T().Setenv("DEVSY_TEST_IMPORTED", imported) + s.T().Setenv("DEVSY_TEST_ALIAS", aliasFile) + s.T().Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + cliPath := filepath.Join(dir, "configured-cli") + script := "#!/bin/sh\nif [ \"$1\" = image ]; then exit 0; fi\ncat \"$DEVSY_TEST_ARCHIVE\"\n" + ref := "final-image:latest" + if source == "registry" { + server := httptest.NewServer(registry.New()) + defer server.Close() + ref = strings.TrimPrefix(server.URL, "http://") + "/final-image:latest" + script = "#!/bin/sh\nexit 1\n" + } + s.Require().NoError(os.WriteFile(cliPath, []byte(script), 0o700)) + tag, err := name.NewTag(ref) + s.Require().NoError(err) + writeImage := func(img v1.Image) { + if source == "local" { + s.Require().NoError(tarball.WriteToFile(archive, tag, img)) + } else { + s.Require().NoError(remote.Write(tag, img)) + } + } + writeImage(original) + client := cliClient{dockerPath: cliPath} + prepared, err := client.PrepareImage(ctx, ref, false) + s.Require().NoError(err) + defer prepared.cleanup() + writeImage(replacement) + newer, err := client.PrepareImage(ctx, ref, false) + s.Require().NoError(err) + defer newer.cleanup() + s.NotEqual(prepared.reference, newer.reference) + owner, err := (filesystemUserResolver{}).ResolveImage(ctx, prepared.image, testUser) + s.Require().NoError(err) + s.Equal(mountOwner{2000, 2001}, *owner) + s.writeSnapshotImportCLI(dir) + s.Require().NoError(client.EnsureImage(ctx, prepared)) + importedImage, err := tarball.ImageFromPath(imported, nil) + s.Require().NoError(err) + importedOwner, err := ownerFromImage(ctx, importedImage, testUser) + s.Require().NoError(err) + s.Equal(*owner, *importedOwner) + alias, err := os.ReadFile(aliasFile) + s.Require().NoError(err) + s.Equal(prepared.reference, string(alias)) + }) + } +} + +//nolint:gosec // Test-created executable under TempDir. +func (s *imageUserSuite) TestStreamImportReapsEarlyConsumerExit() { + s.requirePOSIXShell() + dir := s.T().TempDir() + s.T().Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + script := "#!/bin/sh\necho rejected >&2\nexit 1\n" + s.Require().NoError(os.WriteFile(filepath.Join(dir, "msb"), []byte(script), 0o700)) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + err := loadImageSnapshot(ctx, "devsy-msb-image:test", s.snapshotImage(snapshotPasswd)) + s.ErrorContains(err, "rejected") + s.NoError(ctx.Err(), "an early consumer exit must not leave a blocked writer") +} + +//nolint:gosec // Test-created executable under TempDir. +func (s *imageUserSuite) TestStreamImportHonorsCancellation() { + s.requirePOSIXShell() + dir := s.T().TempDir() + s.T().Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + script := "#!/bin/sh\nexec sleep 60\n" + s.Require().NoError(os.WriteFile(filepath.Join(dir, "msb"), []byte(script), 0o700)) + img := s.snapshotImage(snapshotPasswd) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + s.ErrorIs(loadImageSnapshot(ctx, "devsy-msb-image:test", img), context.DeadlineExceeded) +} + +func (s *imageUserSuite) TestRegistryMetadataWithoutCLI() { + s.requirePOSIXShell() + server := httptest.NewServer(registry.New()) + defer server.Close() + ref, err := name.NewTag(strings.TrimPrefix(server.URL, "http://") + "/metadata:latest") + s.Require().NoError(err) + img := s.snapshotImage(snapshotPasswd) + img, err = mutate.Config(img, v1.Config{User: testUser}) + s.Require().NoError(err) + s.Require().NoError(remote.Write(ref, img)) + d := newDriver(newFakeClient(), nil, specDefaults{}) + dir := s.T().TempDir() + failingCLI := filepath.Join(dir, "failing-cli") + // #nosec G306 -- executable fixture under TempDir. + s.Require().NoError(os.WriteFile(failingCLI, []byte("#!/bin/sh\nexit 1\n"), 0o700)) + for _, cli := range []string{filepath.Join(dir, "missing-cli"), failingCLI} { + d.dockerPath = cli + details, err := d.InspectImage(context.Background(), ref.Name()) + s.Require().NoError(err) + s.Equal(testUser, details.Config.User) + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err = d.InspectImage(ctx, ref.Name()) + s.ErrorIs(err, context.Canceled) +} + +//nolint:gosec // Test-created executable under TempDir. +func (s *imageUserSuite) TestInspectCachedImageConfig() { + s.requirePOSIXShell() + cfg := v1.Config{ + User: testUser, + Env: []string{"EXAMPLE=value"}, + Labels: map[string]string{ + "example": "value", + }, + Entrypoint: []string{"/bin/sh"}, + Cmd: []string{"-l"}, + } + dir := s.T().TempDir() + configFile, argsFile := filepath.Join(dir, "config.json"), filepath.Join(dir, "args") + configJSON, err := json.Marshal(cfg) + s.Require().NoError(err) + s.Require().NoError(os.WriteFile(configFile, configJSON, 0o600)) + s.T().Setenv("DEVSY_TEST_CONFIG", configFile) + s.T().Setenv("DEVSY_TEST_ARGS", argsFile) + executable := filepath.Join(dir, "configured-cli") + script := `#!/bin/sh +[ "$1" = image ] && [ "$2" = inspect ] || exit 1 +printf '%s\n' "$@" > "$DEVSY_TEST_ARGS" +cat "$DEVSY_TEST_CONFIG" +` + s.Require().NoError(os.WriteFile(executable, []byte(script), 0o700)) + d := newDriver(newFakeClient(), nil, specDefaults{}) + d.dockerPath = executable + details, err := d.InspectImage(context.Background(), "final-image:latest") + s.Require().NoError(err) + s.Equal(cfg.User, details.Config.User) + s.Equal(cfg.Env, details.Config.Env) + s.Equal(cfg.Labels, details.Config.Labels) + s.Equal(cfg.Entrypoint, details.Config.Entrypoint) + s.Equal(cfg.Cmd, details.Config.Cmd) + args, err := os.ReadFile(argsFile) + s.Require().NoError(err) + s.Equal("image\ninspect\n--format\n{{json .Config}}\nfinal-image:latest\n", string(args)) + s.Require().NoError(os.WriteFile(configFile, []byte("invalid JSON"), 0o600)) + _, err = d.InspectImage(context.Background(), "final-image:latest") + s.ErrorContains(err, "parse cached image config") +} + +func (s *imageUserSuite) layer(files map[string]string) v1.Layer { + var buf bytes.Buffer + writer := tar.NewWriter(&buf) + for name, data := range files { + s.Require(). + NoError(writer.WriteHeader(&tar.Header{Name: name, Mode: 0o644, Size: int64(len(data))})) + _, err := writer.Write([]byte(data)) + s.Require().NoError(err) + } + s.Require().NoError(writer.Close()) + layer, err := tarball.LayerFromOpener( + func() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(buf.Bytes())), nil }, + ) + s.Require().NoError(err) + return layer +} + +func (s *imageUserSuite) snapshotImage(accounts string) v1.Image { + padding := make([]byte, 1024*1024) + _, err := rand.Read(padding) + s.Require().NoError(err) + img, err := mutate.AppendLayers(empty.Image, s.layer(map[string]string{ + passwdPath: accounts, "padding.bin": string(padding), + })) + s.Require().NoError(err) + return img +} + +//nolint:gosec // Test-created executable under TempDir. +func (s *imageUserSuite) writeSnapshotImportCLI(dir string) { + msb := `#!/bin/sh +[ "$1" = load ] || exit 1 +cat > "$DEVSY_TEST_IMPORTED" +printf '%s' "$3" > "$DEVSY_TEST_ALIAS" +` + s.T().Setenv("TMPDIR", filepath.Join(dir, "unavailable-temp")) + s.Require().NoError(os.WriteFile(filepath.Join(dir, "msb"), []byte(msb), 0o700)) +} + +func (s *imageUserSuite) requirePOSIXShell() { + if runtime.GOOS == "windows" { + s.T().Skip("fixture executable requires a POSIX shell") + } +} diff --git a/pkg/driver/microsandbox/microsandbox.go b/pkg/driver/microsandbox/microsandbox.go index c925ca2aba..15201519cf 100644 --- a/pkg/driver/microsandbox/microsandbox.go +++ b/pkg/driver/microsandbox/microsandbox.go @@ -7,6 +7,7 @@ import ( "encoding/hex" "fmt" "io" + "maps" "math" "regexp" "runtime" @@ -18,9 +19,9 @@ import ( pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/driver" - "github.com/devsy-org/devsy/pkg/image" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/provider" + v1 "github.com/google/go-containerregistry/pkg/v1" ) const userLabel = "devsy.sh/user" @@ -40,16 +41,19 @@ type specDefaults struct { type microsandboxDriver struct { client sandboxClient + dockerPath string idLabels []string defaults specDefaults workspaceMountPolicy workspaceMountPolicy + userResolver imageUserResolver } var ( - _ driver.MountDeliveryDriver = (*microsandboxDriver)(nil) - _ driver.RecreatePolicyDriver = (*microsandboxDriver)(nil) - _ driver.WorkspaceChowner = (*microsandboxDriver)(nil) - _ driver.ArgvExecDriver = (*microsandboxDriver)(nil) + _ driver.MountDeliveryDriver = (*microsandboxDriver)(nil) + _ driver.RecreatePolicyDriver = (*microsandboxDriver)(nil) + _ driver.RecreateRequiredDriver = (*microsandboxDriver)(nil) + _ driver.WorkspaceChowner = (*microsandboxDriver)(nil) + _ driver.ArgvExecDriver = (*microsandboxDriver)(nil) _ driver.RunOptionsDriver = (*microsandboxDriver)(nil) _ driver.ReprovisioningDriver = (*microsandboxDriver)(nil) @@ -98,7 +102,7 @@ func NewMicrosandboxDriver( _ context.Context, workspaceInfo *provider.AgentWorkspaceInfo, ) (driver.Driver, error) { - client := cliClient{} + client := cliClient{dockerPath: workspaceInfo.Agent.Docker.Path} cfg := workspaceInfo.Agent.Microsandbox defaults := specDefaults{ @@ -122,12 +126,15 @@ func NewMicrosandboxDriver( ) d := newDriver(client, workspaceInfo.CLIOptions.IDLabels, defaults) d.workspaceMountPolicy = workspacePolicy + d.dockerPath = workspaceInfo.Agent.Docker.Path + d.userResolver = filesystemUserResolver{dockerPath: d.dockerPath} return d, nil } func newDriver(client sandboxClient, idLabels []string, defaults specDefaults) *microsandboxDriver { return µsandboxDriver{ client: client, idLabels: idLabels, defaults: defaults, + userResolver: filesystemUserResolver{}, workspaceMountPolicy: workspaceMountPolicy{ StatVirtualization: statVirtStrict, HostPermissions: hostPermissionsMirror, @@ -234,22 +241,21 @@ func (d *microsandboxDriver) InspectImage( ctx context.Context, imageName string, ) (*config.ImageDetails, error) { - arch, err := d.TargetArchitecture(ctx, "") - if err != nil { + if _, err := d.TargetArchitecture(ctx, ""); err != nil { return nil, err } - cfg, _, err := image.GetImageConfigForArch(ctx, imageName, arch) + cfg, err := inspectImageConfig(ctx, d.dockerPath, imageName) if err != nil { return nil, fmt.Errorf("get image config for %s: %w", imageName, err) } return &config.ImageDetails{ ID: imageName, Config: config.ImageDetailsConfig{ - User: cfg.Config.User, - Env: cfg.Config.Env, - Labels: cfg.Config.Labels, - Entrypoint: cfg.Config.Entrypoint, - Cmd: cfg.Config.Cmd, + User: cfg.User, + Env: cfg.Env, + Labels: cfg.Labels, + Entrypoint: cfg.Entrypoint, + Cmd: cfg.Cmd, }, }, nil } @@ -270,8 +276,8 @@ func (d *microsandboxDriver) UpdateContainerUserUID( return nil } -// RequiresWorkspaceChown reports that the virtiofs workspace share is -// root-owned in the guest, so the agent must chown it to the remote user. +// RequiresWorkspaceChown repairs existing per-file guest ownership overrides, +// which take precedence over the workspace mount fallback owner. func (d *microsandboxDriver) RequiresWorkspaceChown() bool { return true } @@ -298,18 +304,7 @@ func (d *microsandboxDriver) DeleteDevContainer(ctx context.Context, workspaceID if err != nil { return err } - if info == nil { - return nil - } - if info.Running { - if err := d.client.Stop(ctx, name); err != nil { - return fmt.Errorf("stop microsandbox VM before delete: %w", err) - } - } - if err := d.client.Remove(ctx, name); err != nil { - return fmt.Errorf("remove microsandbox VM: %w", err) - } - return nil + return d.deleteSandbox(ctx, name, info) } func (d *microsandboxDriver) GetDevContainerLogs( @@ -364,17 +359,41 @@ func (d *microsandboxDriver) runFromOptions( if err := d.ProvisioningPreflight(ctx); err != nil { return err } + prepared, err := d.client.PrepareImage(ctx, options.Image, options.ImageBuilt) + if err != nil { + return fmt.Errorf("prepare microsandbox final image: %w", err) + } + defer prepared.cleanup() + owner, err := d.resolveWorkspaceOwner(ctx, options, prepared.image) + if err != nil { + return err + } warnUnsupportedOptions(options) - if err := d.DeleteDevContainer(ctx, workspaceID); err != nil { - return fmt.Errorf("clean stale microsandbox VM before create: %w", err) + if err := d.client.EnsureImage(ctx, prepared); err != nil { + return fmt.Errorf("ensure microsandbox final image: %w", err) } - if err := d.client.EnsureImage(ctx, options.Image); err != nil { - log.Debugf("microsandbox ensure image %q failed (continuing): %v", options.Image, err) + info, err := d.client.Find(ctx, sandboxName(workspaceID)) + if err != nil { + return err + } + if info != nil && !options.AllowRecreate { + return fmt.Errorf( + "microsandbox VM already exists; back up VM-local data and rerun with --recreate: " + + "replacement discards the VM root disk and cannot roll back after removal", + ) + } + spec := d.buildSpec(workspaceID, options, hostReqs, owner) + spec.Image = prepared.reference + if err := d.client.PrepareVolumes(ctx, spec.Mounts); err != nil { + return fmt.Errorf("prepare microsandbox volumes: %w", err) + } + if err := d.deleteSandbox(ctx, sandboxName(workspaceID), info); err != nil { + return fmt.Errorf("clean stale microsandbox VM before create: %w", err) } if err := d.client.Create( ctx, sandboxName(workspaceID), - d.buildSpec(workspaceID, options, hostReqs), + spec, ); err != nil { return fmt.Errorf("create microsandbox VM: %w", err) } @@ -385,12 +404,20 @@ func (d *microsandboxDriver) runFromOptions( // MICROSANDBOX_* defaults, then the devcontainer's hostRequirements, falling // back to the microsandbox runtime default (zero) when neither is set. func (d *microsandboxDriver) buildSpec( - workspaceID string, options *driver.RunOptions, hostReqs *config.HostRequirements, + workspaceID string, + options *driver.RunOptions, + hostReqs *config.HostRequirements, + owner *mountOwner, ) sandboxSpec { - labels := config.ListToObject(config.GetIDLabels(workspaceID, d.idLabels)) + labels := config.ListToObject(options.Labels) if labels == nil { labels = map[string]string{} } + maps.Copy(labels, config.ListToObject(config.GetIDLabels(workspaceID, d.idLabels))) + labels[workspaceMountContractLabel] = d.workspaceMountContract() + if owner != nil { + labels[workspaceRemoteUserLabel] = workspaceRemoteUser(options) + } if options.User != "" { labels[userLabel] = options.User } @@ -417,7 +444,7 @@ func (d *microsandboxDriver) buildSpec( Labels: labels, Ephemeral: d.defaults.ephemeral, IdleTimeout: d.defaults.idleTimeout, - Mounts: d.volumeMounts(options), + Mounts: d.volumeMounts(options, owner), MaxMemory: d.defaults.maxMemory, MaxCPUs: d.defaults.maxCPUs, BlockEgress: d.defaults.blockEgress, @@ -463,6 +490,7 @@ func toContainerDetails(info *sandboxInfo) *config.ContainerDetails { return &config.ContainerDetails{ ID: info.Name, Created: info.CreatedAt.Format(createdAtLayout), + Mounts: info.Mounts, State: config.ContainerDetailsState{Status: status}, Config: config.ContainerDetailsConfig{ Labels: info.Labels, @@ -590,4 +618,100 @@ func ceilBytesToUint32(bytes, unit uint64) uint32 { func (d *microsandboxDriver) RequiresMountStreaming() bool { return false } -func (d *microsandboxDriver) RecreateMode() driver.RecreateMode { return driver.RecreateDelete } +func (d *microsandboxDriver) RecreateMode() driver.RecreateMode { return driver.RecreateOnRun } + +const ( + workspaceMountContractLabel = "devsy.sh/microsandbox-workspace-mount" + workspaceRemoteUserLabel = "devsy.sh/microsandbox-workspace-user" +) + +func (d *microsandboxDriver) RequiresRecreate( + details *config.ContainerDetails, + remoteUser string, +) (bool, string) { + if details == nil { + return false, "" + } + if details.Config.Labels[workspaceMountContractLabel] != d.workspaceMountContract() { + return true, "microsandbox workspace mount contract changed" + } + if d.workspaceMountPolicy.StatVirtualization != statVirtOff && + details.Config.Labels[workspaceRemoteUserLabel] != "" && + details.Config.Labels[workspaceRemoteUserLabel] != remoteUser { + return true, "microsandbox workspace developer identity changed" + } + return false, "" +} + +func (d *microsandboxDriver) workspaceMountContract() string { + owner := "none" + if d.workspaceMountPolicy.StatVirtualization != statVirtOff { + owner = "remote-user" + } + return fmt.Sprintf( + "v3;stat=%s;host=%s;owner=%s", + d.workspaceMountPolicy.StatVirtualization, + d.workspaceMountPolicy.HostPermissions, + owner, + ) +} + +func (d *microsandboxDriver) resolveWorkspaceOwner( + ctx context.Context, + options *driver.RunOptions, + img v1.Image, +) (*mountOwner, error) { + if d.workspaceMountPolicy.StatVirtualization == statVirtOff || + bindMount(options.WorkspaceMount) == nil { + return nil, nil + } + user := workspaceRemoteUser(options) + if options.Dockerless { + if user != rootUser { + return nil, fmt.Errorf( + "microsandbox cannot resolve a non-root Dockerless workspace owner before VM creation; " + + "use a prebuilt developer image, remoteUser=root, or " + + "workspaceStatVirtualization=off with private host permissions", + ) + } + return &mountOwner{}, nil + } + owner, err := d.userResolver.ResolveImage(ctx, img, user) + if err != nil { + return nil, fmt.Errorf( + "resolve microsandbox workspace owner for remoteUser %q: %w", + user, + err, + ) + } + return owner, nil +} + +func workspaceRemoteUser(options *driver.RunOptions) string { + if options.RemoteUser != "" { + return options.RemoteUser + } + if options.User != "" { + return options.User + } + return rootUser +} + +func (d *microsandboxDriver) deleteSandbox( + ctx context.Context, + name string, + info *sandboxInfo, +) error { + if info == nil { + return nil + } + if info.Running { + if err := d.client.Stop(ctx, name); err != nil { + return fmt.Errorf("stop microsandbox VM before delete: %w", err) + } + } + if err := d.client.Remove(ctx, name); err != nil { + return fmt.Errorf("remove microsandbox VM: %w", err) + } + return nil +} diff --git a/pkg/driver/microsandbox/microsandbox_test.go b/pkg/driver/microsandbox/microsandbox_test.go index 8c205dce6f..6e9823b5bc 100644 --- a/pkg/driver/microsandbox/microsandbox_test.go +++ b/pkg/driver/microsandbox/microsandbox_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "io" "slices" "strings" @@ -12,6 +13,8 @@ import ( "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/driver" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/stretchr/testify/require" ) const ( @@ -24,6 +27,7 @@ const ( testVersion = "0.7.2" oldVersion = "microsandbox 0.7.1" shPath = "/bin/sh" + callEnsure = "ensure:x:1" callFind = "find:" + wsName callRemove = "remove:" + wsName testBindSrc = "/host/proj" @@ -39,6 +43,8 @@ type fakeClient struct { calls []string execReq execRequest execName string + failPrepare error + failVolumes error failFind error failStop error failCreat error @@ -66,11 +72,22 @@ func (f *fakeClient) Version(context.Context) (string, error) { return testVersion, nil } -func (f *fakeClient) EnsureImage(_ context.Context, image string) error { - f.calls = append(f.calls, "ensure:"+image) +func (f *fakeClient) PrepareImage(_ context.Context, image string, _ bool) (*preparedImage, error) { + if f.failPrepare != nil { + return nil, f.failPrepare + } + return &preparedImage{reference: image, image: empty.Image, cleanup: func() {}}, nil +} + +func (f *fakeClient) EnsureImage(_ context.Context, image *preparedImage) error { + f.calls = append(f.calls, "ensure:"+image.reference) return f.failEnsure } +func (f *fakeClient) PrepareVolumes(_ context.Context, _ []volumeMount) error { + return f.failVolumes +} + func (f *fakeClient) Create(_ context.Context, name string, spec sandboxSpec) error { f.calls = append(f.calls, "create:"+name) if f.failCreat != nil { @@ -175,15 +192,19 @@ func TestRunDevContainerReplacesStaleSandbox(t *testing.T) { f.info[wsName] = &sandboxInfo{Name: wsName, Running: true} d := newDriver(f, nil, specDefaults{}) - err := d.RunDevContainer(context.Background(), wsID, &driver.RunOptions{Image: imgX}) + err := d.RunDevContainer( + context.Background(), + wsID, + &driver.RunOptions{Image: imgX, AllowRecreate: true}, + ) if err != nil { t.Fatalf("RunDevContainer: %v", err) } want := []string{ + callEnsure, callFind, "stop:devsy-ws1", callRemove, - "ensure:x:1", "create:devsy-ws1", } if !slices.Equal(f.calls, want) { @@ -218,7 +239,7 @@ func TestRunDevContainerAcceptsProvisioningRuntimeVersions(t *testing.T) { if err != nil { t.Fatalf("RunDevContainer: %v", err) } - want := []string{callFind, "ensure:x:1", "create:" + wsName} + want := []string{callEnsure, callFind, "create:" + wsName} if !slices.Equal(f.calls, want) { t.Fatalf("calls = %v, want %v", f.calls, want) } @@ -293,21 +314,17 @@ func assertProvisioningUnchanged(t *testing.T, f *fakeClient) { } } -func TestRunDevContainerContinuesWhenPrePullFails(t *testing.T) { +func TestRunDevContainerImageFailurePreservesExisting(t *testing.T) { f := newFakeClient() - f.failEnsure = errors.New("registry hiccup") + f.failEnsure = errors.New("image unavailable") + f.info[wsName] = &sandboxInfo{Name: wsName, Running: true} d := newDriver(f, nil, specDefaults{}) - - // Pre-pull failure must not abort the run; create should still be attempted. - if err := d.RunDevContainer( - context.Background(), - wsID, - &driver.RunOptions{Image: imgX}, - ); err != nil { - t.Fatalf("RunDevContainer should proceed despite pull failure: %v", err) + err := d.RunDevContainer(context.Background(), wsID, &driver.RunOptions{Image: imgX}) + if err == nil || !strings.Contains(err.Error(), "image unavailable") { + t.Fatalf("unexpected error: %v", err) } - if _, ok := f.created[wsName]; !ok { - t.Error("expected create to run even though pre-pull failed") + if !slices.Equal(f.calls, []string{callEnsure}) || !f.info[wsName].Running { + t.Fatalf("image failure changed sandbox: %v", f.calls) } } @@ -322,7 +339,7 @@ func TestRunImageDevContainerRunsFromParams(t *testing.T) { if err != nil { t.Fatalf("RunImageDevContainer: %v", err) } - want := []string{callFind, "ensure:built-local:latest", "create:devsy-ws1"} + want := []string{"ensure:built-local:latest", callFind, "create:devsy-ws1"} if !slices.Equal(f.calls, want) { t.Errorf("call order = %v, want %v", f.calls, want) } @@ -600,7 +617,7 @@ func TestBuildSpecMapsAllMountTypes(t *testing.T) { {Type: driver.MountTypeBind, Source: testBindSrc, Target: "/mnt"}, nil, }, - }, nil) + }, nil, nil) want := []volumeMount{ {Target: "/data", Volume: "vol1"}, {Target: "/scratch", Tmpfs: true}, @@ -620,7 +637,7 @@ func TestBuildSpecMapsWorkspaceMount(t *testing.T) { Source: testBindSrc, Target: testBindDst, }, - }, nil) + }, nil, nil) want := []volumeMount{ { Target: testBindDst, @@ -638,7 +655,7 @@ func TestBuildSpecMapsWorkspaceMount(t *testing.T) { func TestBuildSpecCarriesIdleTimeout(t *testing.T) { d := newDriver(newFakeClient(), nil, specDefaults{idleTimeout: 90 * time.Second}) - spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil) + spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil, nil) if spec.IdleTimeout != 90*time.Second { t.Errorf("idle timeout = %s, want 90s", spec.IdleTimeout) } @@ -650,7 +667,7 @@ func TestBuildSpecCarriesCeilingsAndEgress(t *testing.T) { nil, specDefaults{maxMemory: 4096, maxCPUs: 4, blockEgress: true}, ) - spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil) + spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil, nil) if spec.MaxMemory != 4096 || spec.MaxCPUs != 4 || !spec.BlockEgress { t.Errorf("unexpected spec ceilings/egress: %+v", spec) } @@ -659,7 +676,7 @@ func TestBuildSpecCarriesCeilingsAndEgress(t *testing.T) { func TestBuildSpecUsesHostRequirementsWhenDefaultsUnset(t *testing.T) { d := newDriver(newFakeClient(), nil, specDefaults{}) hostReqs := &config.HostRequirements{CPUs: 4, Memory: "8gb", Storage: size32GB} - spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, hostReqs) + spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, hostReqs, nil) if spec.CPUs != 4 { t.Errorf("CPUs = %d, want 4", spec.CPUs) } @@ -678,7 +695,7 @@ func TestBuildSpecPrefersConfiguredDefaultsOverHostRequirements(t *testing.T) { specDefaults{memory: 2048, cpus: 2, rootDiskGB: 16}, ) hostReqs := &config.HostRequirements{CPUs: 8, Memory: size32GB, Storage: "64gb"} - spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, hostReqs) + spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, hostReqs, nil) if spec.CPUs != 2 || spec.Memory != 2048 || spec.RootDiskGB != 16 { t.Errorf("configured defaults should win, got %+v", spec) } @@ -686,7 +703,7 @@ func TestBuildSpecPrefersConfiguredDefaultsOverHostRequirements(t *testing.T) { func TestBuildSpecIgnoresNilHostRequirements(t *testing.T) { d := newDriver(newFakeClient(), nil, specDefaults{}) - spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil) + spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil, nil) if spec.CPUs != 0 || spec.Memory != 0 || spec.RootDiskGB != 0 { t.Errorf("nil hostRequirements with no defaults should leave sizing zero, got %+v", spec) } @@ -741,3 +758,44 @@ func TestParseUint8(t *testing.T) { } } } + +func TestReplacementRequiresConsentAndSuccessfulPreparation(t *testing.T) { + sentinel := errors.New("preparation failed") + for _, running := range []bool{false, true} { + for _, tc := range []struct { + name string + allow bool + imageErr, volumesErr error + }{ + {name: "consent"}, + {name: "image", allow: true, imageErr: sentinel}, + {name: "volumes", allow: true, volumesErr: sentinel}, + } { + t.Run(fmt.Sprintf("running=%t/%s", running, tc.name), func(t *testing.T) { + f := newFakeClient() + original := &sandboxInfo{Name: wsName, Running: running, CreatedAt: time.Now()} + f.info[wsName] = original + f.failPrepare, f.failVolumes = tc.imageErr, tc.volumesErr + opts := &driver.RunOptions{Image: imgX, AllowRecreate: tc.allow} + err := newDriver(f, nil, specDefaults{}).RunDevContainer(t.Context(), wsID, opts) + require.Error(t, err) + if !tc.allow { + require.ErrorContains(t, err, "--recreate") + } else { + require.ErrorIs(t, err, sentinel) + } + require.Same(t, original, f.info[wsName]) + require.Equal(t, running, original.Running) + require.Empty(t, f.created) + for _, call := range f.calls { + require.False( + t, + strings.HasPrefix(call, "stop:") || strings.HasPrefix(call, "remove:") || + strings.HasPrefix(call, "create:"), + call, + ) + } + }) + } + } +} diff --git a/pkg/driver/microsandbox/mounts.go b/pkg/driver/microsandbox/mounts.go index 49742eb285..b3710a21be 100644 --- a/pkg/driver/microsandbox/mounts.go +++ b/pkg/driver/microsandbox/mounts.go @@ -85,9 +85,12 @@ func parseHostPermissions(value string) (hostPermissions, error) { } } -func (d *microsandboxDriver) volumeMounts(options *driver.RunOptions) []volumeMount { +func (d *microsandboxDriver) volumeMounts( + options *driver.RunOptions, + owner *mountOwner, +) []volumeMount { var out []volumeMount - if m := d.workspaceMount(options.WorkspaceMount); m != nil { + if m := d.workspaceMount(options.WorkspaceMount, owner); m != nil { out = append(out, *m) } for _, m := range options.Mounts { @@ -98,14 +101,21 @@ func (d *microsandboxDriver) volumeMounts(options *driver.RunOptions) []volumeMo return out } -func (d *microsandboxDriver) workspaceMount(m *devcontainerconfig.Mount) *volumeMount { +func (d *microsandboxDriver) workspaceMount( + m *devcontainerconfig.Mount, + owner *mountOwner, +) *volumeMount { b := bindMount(m) if b == nil { return nil } + if d.workspaceMountPolicy.StatVirtualization == statVirtOff { + owner = nil + } b.Policy = mountPolicy{ StatVirtualization: d.workspaceMountPolicy.StatVirtualization, HostPermissions: d.workspaceMountPolicy.HostPermissions, + Owner: owner, } return b } diff --git a/pkg/driver/microsandbox/workspace_owner_test.go b/pkg/driver/microsandbox/workspace_owner_test.go new file mode 100644 index 0000000000..d4f13609fc --- /dev/null +++ b/pkg/driver/microsandbox/workspace_owner_test.go @@ -0,0 +1,220 @@ +package microsandbox + +import ( + "context" + "errors" + "testing" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/driver" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/stretchr/testify/require" +) + +const testNumericIdentity = "1000:1001" + +type recordingUserResolver struct { + user string + owner *mountOwner + err error + calls int +} + +func (r *recordingUserResolver) ResolveImage( + _ context.Context, + _ v1.Image, + user string, +) (*mountOwner, error) { + r.user = user + r.calls++ + return r.owner, r.err +} + +func TestWorkspaceOwnerKeepsContainerIdentity(t *testing.T) { + f := newFakeClient() + d := newDriver(f, nil, specDefaults{}) + resolver := &recordingUserResolver{owner: &mountOwner{UID: 1000, GID: 1001}} + d.userResolver = resolver + options := &driver.RunOptions{ + Image: testImage, + ImageBuilt: true, + User: rootUser, + RemoteUser: testUser, + WorkspaceMount: &config.Mount{ + Type: driver.MountTypeBind, + Source: testBindSrc, + Target: testBindDst, + }, + Mounts: []*config.Mount{ + {Type: driver.MountTypeBind, Source: "/extra", Target: "/extra"}, + }, + } + require.NoError( + t, + d.RunImageDevContainer( + context.Background(), + &driver.RunImageDevContainerParams{WorkspaceID: wsID, Options: options}, + ), + ) + require.Equal(t, testUser, resolver.user) + spec := f.created[wsName] + require.Equal(t, rootUser, spec.User) + require.Equal( + t, + testBindSrc+":"+testBindDst+":stat-virt=strict,host-perms=mirror,uid=1000,gid=1001", + bindMountSpec(spec.Mounts[0]), + ) + require.Nil(t, spec.Mounts[1].Policy.Owner) + require.Equal(t, d.workspaceMountContract(), spec.Labels[workspaceMountContractLabel]) +} + +func TestWorkspaceOwnerFailurePreservesExistingSandbox(t *testing.T) { + f := newFakeClient() + f.info[wsName] = &sandboxInfo{Name: wsName, Running: true} + d := newDriver(f, nil, specDefaults{}) + d.userResolver = &recordingUserResolver{err: errors.New("user not found")} + err := d.RunDevContainer( + context.Background(), + wsID, + &driver.RunOptions{ + Image: imgX, + RemoteUser: "missing", + WorkspaceMount: &config.Mount{Source: testBindSrc, Target: testBindDst}, + }, + ) + require.ErrorContains(t, err, `remoteUser "missing"`) + require.Empty(t, f.calls) + require.True(t, f.info[wsName].Running) +} + +func TestWorkspaceOwnerPolicyOff(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + d.workspaceMountPolicy = workspaceMountPolicy{ + StatVirtualization: statVirtOff, + HostPermissions: hostPermissionsPrivate, + } + resolver := &recordingUserResolver{err: errors.New("must not resolve")} + d.userResolver = resolver + options := &driver.RunOptions{ + WorkspaceMount: &config.Mount{Source: testBindSrc, Target: testBindDst}, + RemoteUser: testUser, + Dockerless: true, + } + owner, err := d.resolveWorkspaceOwner(context.Background(), options, nil) + require.NoError(t, err) + require.Nil(t, owner) + require.Zero(t, resolver.calls) + mount := d.workspaceMount(options.WorkspaceMount, &mountOwner{UID: 1000, GID: 1000}) + require.Equal( + t, + testBindSrc+":"+testBindDst+":stat-virt=off,host-perms=private", + bindMountSpec(*mount), + ) +} + +func TestWorkspaceMountContractRecreation(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + for _, label := range []string{"", "v1", d.workspaceMountContract(), "v2;stat=relaxed;host=mirror;owner=remote-user"} { + details := &config.ContainerDetails{ + Config: config.ContainerDetailsConfig{ + Labels: map[string]string{ + workspaceMountContractLabel: label, + workspaceRemoteUserLabel: rootUser, + }, + }, + } + required, _ := driver.DriverRequiresRecreate(d, details, rootUser) + require.Equal(t, label != d.workspaceMountContract(), required) + } + required, _ := d.RequiresRecreate(nil, rootUser) + require.False(t, required) +} + +func TestWorkspaceOwnerFallbackIdentity(t *testing.T) { + for _, tt := range []struct{ user, remote, want string }{ + {"node", "", "node"}, {"", "", rootUser}, {rootUser, testUser, testUser}, + } { + d := newDriver(newFakeClient(), nil, specDefaults{}) + resolver := &recordingUserResolver{owner: &mountOwner{}} + d.userResolver = resolver + _, err := d.resolveWorkspaceOwner( + context.Background(), + &driver.RunOptions{ + User: tt.user, + RemoteUser: tt.remote, + WorkspaceMount: &config.Mount{Source: testBindSrc, Target: testBindDst}, + }, nil, + ) + require.NoError(t, err) + require.Equal(t, tt.want, resolver.user) + } +} + +func TestWorkspaceIdentityChangeRequiresRecreation(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + spec := d.buildSpec( + wsID, + &driver.RunOptions{User: rootUser, RemoteUser: testUser}, + nil, + &mountOwner{UID: 1000, GID: 1000}, + ) + details := toContainerDetails(&sandboxInfo{Labels: spec.Labels}) + for _, user := range []string{testUser, "2000:2001", rootUser} { + required, _ := d.RequiresRecreate(details, user) + require.Equal(t, user != testUser, required) + } + d.workspaceMountPolicy.StatVirtualization = statVirtOff + details.Config.Labels[workspaceMountContractLabel] = d.workspaceMountContract() + required, _ := d.RequiresRecreate(details, rootUser) + require.False(t, required) +} + +func TestDockerlessOwnerNeverLooksUpRunnerAccounts(t *testing.T) { + for _, user := range []string{testUser, "1000", testNumericIdentity, rootUser} { + t.Run(user, func(t *testing.T) { + f := newFakeClient() + f.info[wsName] = &sandboxInfo{Name: wsName, Running: true} + d := newDriver(f, nil, specDefaults{}) + resolver := &recordingUserResolver{err: errors.New("runner accounts must not be read")} + d.userResolver = resolver + err := d.RunDevContainer(context.Background(), wsID, &driver.RunOptions{ + Image: imgX, + User: rootUser, + RemoteUser: user, + Dockerless: true, + AllowRecreate: true, + WorkspaceMount: &config.Mount{Source: testBindSrc, Target: testBindDst}, + }) + if user != rootUser { + require.ErrorContains(t, err, "non-root Dockerless workspace owner") + require.Empty(t, f.calls) + require.True(t, f.info[wsName].Running) + } else { + require.NoError(t, err) + require.Equal(t, &mountOwner{}, f.created[wsName].Mounts[0].Policy.Owner) + } + require.Zero(t, resolver.calls) + }) + } +} + +func TestBuildSpecPreservesImageMetadataAndImageUser(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + spec := d.buildSpec(wsID, &driver.RunOptions{ + Labels: []string{"devcontainer.metadata=[]", config.UserLabel + "=node"}, + User: rootUser, RemoteUser: testUser, + }, nil, nil) + require.Equal(t, "[]", spec.Labels["devcontainer.metadata"]) + require.Equal(t, "node", spec.Labels[config.UserLabel]) + require.Equal(t, rootUser, spec.Labels[userLabel]) +} + +func TestWorkspaceWithoutOwnerDoesNotMigrateOnIdentityChange(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + spec := d.buildSpec(wsID, &driver.RunOptions{RemoteUser: testUser}, nil, nil) + required, _ := d.RequiresRecreate( + toContainerDetails(&sandboxInfo{Labels: spec.Labels}), + rootUser, + ) + require.False(t, required) +} diff --git a/pkg/driver/types.go b/pkg/driver/types.go index b70465a469..0ab7c13e9a 100644 --- a/pkg/driver/types.go +++ b/pkg/driver/types.go @@ -92,6 +92,8 @@ type RecreateMode string const ( RecreateDelete RecreateMode = "delete" RecreateStop RecreateMode = "stop" + // RecreateOnRun lets the driver validate final run options before replacing the container. + RecreateOnRun RecreateMode = "on-run" ) type RecreatePolicyDriver interface { @@ -170,6 +172,15 @@ type RunOptions struct { // User is the user to run the container as User string `json:"user,omitempty"` + // RemoteUser is the developer identity for remote tooling and workspace ownership. + RemoteUser string `json:"remoteUser,omitempty"` + + // Dockerless builds the developer filesystem inside the runner after creation. + Dockerless bool `json:"dockerless,omitempty"` + + // AllowRecreate authorizes replacing an existing VM and discarding its root disk. + AllowRecreate bool `json:"allowRecreate,omitempty"` + // Entrypoint is the entrypoint of the container Entrypoint string `json:"entrypoint,omitempty"` @@ -219,3 +230,19 @@ type RunOptions struct { // pullable from a registry. False for a devcontainer.json "image:" reference. ImageBuilt bool `json:"imageBuilt,omitempty"` } + +// RecreateRequiredDriver detects incompatible creation-time container contracts. +type RecreateRequiredDriver interface { + RequiresRecreate(details *config.ContainerDetails, remoteUser string) (bool, string) +} + +func DriverRequiresRecreate( + d Driver, + details *config.ContainerDetails, + remoteUser string, +) (bool, string) { + if migration, ok := d.(RecreateRequiredDriver); ok { + return migration.RequiresRecreate(details, remoteUser) + } + return false, "" +} diff --git a/pkg/image/image.go b/pkg/image/image.go index edc32cd337..86caae7094 100644 --- a/pkg/image/image.go +++ b/pkg/image/image.go @@ -54,6 +54,7 @@ func GetImageForArch(ctx context.Context, image, arch string) (v1.Image, error) } remoteOptions := []remote.Option{ + remote.WithContext(ctx), remote.WithAuthFromKeychain(keychain), remote.WithPlatform(v1.Platform{Architecture: arch, OS: osLinux}), } diff --git a/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx b/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx index 778ded7797..52416f7f9e 100644 --- a/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx +++ b/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx @@ -204,13 +204,47 @@ Available options: support; `relaxed` tolerates filesystems without it; `off` exposes literal host ownership and modes. `off` cannot be combined with `mirror`. -The workspace mount uses `stat-virt=strict` and `host-perms=mirror` by default, -while additional bind mounts retain MicroSandbox's safer defaults. Devsy passes -the resolved devcontainer `containerUser` to the MicroSandbox runtime so files -created on the host appear as the effective sandbox user in the guest. Devsy -requires MicroSandbox v0.7.2 or newer when creating or recreating workspaces -that use workspace ownership synchronization. This provisioning version check -does not block inspection, logs, stop, or deletion of an existing sandbox. +The primary workspace bind mount uses `stat-virt=strict` and `host-perms=mirror` +by default, while additional bind mounts retain MicroSandbox's defaults. +`containerUser` controls the workload execution user; `remoteUser` controls the +workspace developer identity, falling back to `containerUser`, the image user, +and then root. Devsy resolves the developer's numeric UID and primary GID from +the final image's account files and sets them as the mount's guest-visible +fallback owner. These IDs apply to host-created entries without per-file guest +metadata; they do not change host inode ownership. Account resolution does not +execute image code. An unresolvable user fails provisioning before replacement +of the existing VM. With `workspaceStatVirtualization: "off"`, Devsy omits the +owner override. + +In Dockerless mode, the developer filesystem is built inside the VM after its +workspace mount is created. Ownership synchronization therefore supports only +`remoteUser: "root"` in this mode. Other identities fail before replacing an +existing VM. For a non-root developer, use a prebuilt image or explicitly select +`workspaceStatVirtualization: "off"` with private host permissions. Devsy never +looks up the developer account in the unrelated Dockerless runner image. + +Image inspection and preparation prefer an image cached in the configured +Docker-compatible CLI, permitting offline/private-registry startup. Preparation +freezes the final image before account resolution and import. Both use that same +snapshot, imported under a content-derived tag, so retagging the original image +cannot change the imported filesystem or its fallback owner. Registry-only +images do not require Docker initialization. + +Devsy-managed workspaces with an incompatible persisted workspace mount contract +require explicit `devsy up --recreate`, including workspaces created before the +contract label existed. Changing workspace permission policy or the effective +developer identity also requires explicit recreation. Ordinary `devsy up` reports +an actionable error and preserves the existing VM. Back up VM-local data before +recreating: the host workspace and named volumes persist, but the VM root disk +is discarded. Replacement cannot roll back failures after removal. +Externally managed containers cannot be replaced through this flow. + +Devsy requires MicroSandbox v0.7.2 or newer when creating or recreating workspaces +that use workspace ownership synchronization. Runtime compatibility, account +resolution, final image import, and volume preparation complete before removing +an existing VM. +The provisioning version check does not block inspection, logs, stop, or deletion +of an existing sandbox. Apple Silicon macOS is the canonical supported integration-test host; use `relaxed` when a host filesystem cannot provide strict metadata virtualization.