diff --git a/attach.go b/attach.go index 131d849..f7c54ec 100644 --- a/attach.go +++ b/attach.go @@ -63,10 +63,18 @@ func (e *Engine) AttachWith(ctx context.Context, id WorkspaceID, opts AttachOpti localEnv = environAsMap(os.Environ()) } - return &Workspace{ + ws := &Workspace{ ID: id, Config: cfg, Container: details, subst: newSubstituter(cfg, details, localEnv), - }, nil + } + + // Re-probe on attach so subsequent Exec calls see PATH additions + // from the user's rc files. The original Up populated probedEnv, + // but a fresh Attach doesn't share that workspace value. + if probed, err := e.probeUserEnv(ctx, ws, cfg.UserEnvProbe); err == nil { + ws.probedEnv = probed + } + return ws, nil } diff --git a/engine_test.go b/engine_test.go index 2a3f1c6..9622da3 100644 --- a/engine_test.go +++ b/engine_test.go @@ -376,6 +376,9 @@ func TestExec_SubstitutesContainerEnv(t *testing.T) { t.Fatalf("Up: %v", err) } + // Up performs a userEnvProbe exec; clear the recorded calls so this + // test only asserts on the user-driven Exec. + rt.execCalls = nil rt.execResult = runtime.ExecResult{ExitCode: 0, Stdout: "ok"} _, err = eng.Exec(context.Background(), wsObj, ExecOptions{ Cmd: []string{"echo", "${containerEnv:HOME}"}, diff --git a/exec.go b/exec.go index 04b1edb..8a0e6a3 100644 --- a/exec.go +++ b/exec.go @@ -20,6 +20,21 @@ type ExecOptions struct { Stdin io.Reader Stdout io.Writer Stderr io.Writer + + // SkipUserEnvProbe, when true, makes Exec bypass the merge of + // probedEnv AND cfg.RemoteEnv into the process environment. + // Only opts.Env (after substitution) plus whatever the runtime + // inherits from the container reaches the exec'd process. + // + // Default false: every Exec inherits both probedEnv (so PATH and + // other vars set by the user's rc files are visible — nvm/asdf/ + // feature-installed tools just work) and RemoteEnv (the + // devcontainer.json author's declared env). + // + // Set this for callers that need a clean, deterministic + // environment — internal probes, low-level fs operations, + // DiscoverPath-style helpers that read raw container env. + SkipUserEnvProbe bool } // ExecResult is the outcome of Engine.Exec. @@ -48,6 +63,11 @@ func (e *Engine) Exec(ctx context.Context, ws *Workspace, opts ExecOptions) (Exe user, _ := ws.subst.String(opts.User) wd, _ := ws.subst.String(opts.WorkingDir) + if !opts.SkipUserEnvProbe { + remoteEnv, _ := ws.subst.Map(ws.Config.RemoteEnv) + env = mergeProbedEnv(ws.probedEnv, remoteEnv, env, effectiveUser(ws.Config)) + } + res, err := e.runtime.ExecContainer(ctx, ws.Container.ID, runtime.ExecOptions{ Cmd: cmd, Env: env, diff --git a/test/integration/userenvprobe_test.go b/test/integration/userenvprobe_test.go new file mode 100644 index 0000000..1708008 --- /dev/null +++ b/test/integration/userenvprobe_test.go @@ -0,0 +1,226 @@ +//go:build integration + +package integration + +import ( + "context" + "strings" + "testing" + "time" + + devcontainer "github.com/crunchloop/devcontainer" +) + +// bashImage is a tiny image with bash available so the userEnvProbe's +// `bash -lic` invocation succeeds. plain alpine:3.20 ships only ash; +// the probe falls back to printenv there, exercised in +// TestUserEnvProbe_NoBashFallback. +const bashImage = "bash:5.2-alpine3.20" + +// TestUserEnvProbe_PathFromBashrc covers the original bug: a +// postCreateCommand-installed tool writes its PATH addition to ~/.bashrc; +// later Exec calls must see that PATH without per-call shell wrapping. +func TestUserEnvProbe_PathFromBashrc(t *testing.T) { + if testing.Short() { + t.Skip() + } + eng, rt := newEngine(t) + defer rt.Close() + + ws := writeWorkspace(t, `{ + "image": "`+bashImage+`", + "postCreateCommand": "echo 'export EXTRA_PATH=/from/bashrc' > /etc/profile.d/dc-go-test.sh" + }`) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + wsObj, err := eng.Up(ctx, devcontainer.UpOptions{ + LocalWorkspaceFolder: ws, + Recreate: true, + }) + if err != nil { + t.Fatalf("Up: %v", err) + } + defer func() { _ = eng.Down(context.Background(), wsObj, devcontainer.DownOptions{Remove: true}) }() + + res, err := eng.Exec(ctx, wsObj, devcontainer.ExecOptions{ + Cmd: []string{"printenv", "EXTRA_PATH"}, + }) + if err != nil { + t.Fatalf("Exec: %v", err) + } + if res.ExitCode != 0 { + t.Fatalf("printenv EXTRA_PATH exit=%d stderr=%q", res.ExitCode, res.Stderr) + } + if got := strings.TrimSpace(res.Stdout); got != "/from/bashrc" { + t.Errorf("EXTRA_PATH = %q, want %q (probedEnv didn't inject from .bashrc)", got, "/from/bashrc") + } +} + +// TestUserEnvProbe_None disables probing via devcontainer.json. The +// .bashrc-exported var should NOT be visible to subsequent execs. +func TestUserEnvProbe_None(t *testing.T) { + if testing.Short() { + t.Skip() + } + eng, rt := newEngine(t) + defer rt.Close() + + ws := writeWorkspace(t, `{ + "image": "`+bashImage+`", + "userEnvProbe": "none", + "postCreateCommand": "echo 'export EXTRA_PATH=/from/bashrc' > /etc/profile.d/dc-go-test.sh" + }`) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + wsObj, err := eng.Up(ctx, devcontainer.UpOptions{ + LocalWorkspaceFolder: ws, + Recreate: true, + }) + if err != nil { + t.Fatalf("Up: %v", err) + } + defer func() { _ = eng.Down(context.Background(), wsObj, devcontainer.DownOptions{Remove: true}) }() + + res, err := eng.Exec(ctx, wsObj, devcontainer.ExecOptions{ + Cmd: []string{"sh", "-c", "printenv EXTRA_PATH; true"}, + }) + if err != nil { + t.Fatalf("Exec: %v", err) + } + if got := strings.TrimSpace(res.Stdout); got != "" { + t.Errorf("EXTRA_PATH = %q, want empty (userEnvProbe=none should skip injection)", got) + } +} + +// TestUserEnvProbe_SkipOption verifies the per-Exec opt-out: even with +// the default probe, a caller passing SkipUserEnvProbe gets a clean env. +func TestUserEnvProbe_SkipOption(t *testing.T) { + if testing.Short() { + t.Skip() + } + eng, rt := newEngine(t) + defer rt.Close() + + ws := writeWorkspace(t, `{ + "image": "`+bashImage+`", + "postCreateCommand": "echo 'export EXTRA_PATH=/from/bashrc' > /etc/profile.d/dc-go-test.sh" + }`) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + wsObj, err := eng.Up(ctx, devcontainer.UpOptions{ + LocalWorkspaceFolder: ws, + Recreate: true, + }) + if err != nil { + t.Fatalf("Up: %v", err) + } + defer func() { _ = eng.Down(context.Background(), wsObj, devcontainer.DownOptions{Remove: true}) }() + + // Default Exec sees it. + res, err := eng.Exec(ctx, wsObj, devcontainer.ExecOptions{ + Cmd: []string{"printenv", "EXTRA_PATH"}, + }) + if err != nil { + t.Fatalf("default Exec: %v", err) + } + if got := strings.TrimSpace(res.Stdout); got != "/from/bashrc" { + t.Fatalf("default exec EXTRA_PATH = %q, want %q (sanity precondition failed)", got, "/from/bashrc") + } + + // SkipUserEnvProbe Exec doesn't. + res, err = eng.Exec(ctx, wsObj, devcontainer.ExecOptions{ + Cmd: []string{"sh", "-c", "printenv EXTRA_PATH; true"}, + SkipUserEnvProbe: true, + }) + if err != nil { + t.Fatalf("skip Exec: %v", err) + } + if got := strings.TrimSpace(res.Stdout); got != "" { + t.Errorf("SkipUserEnvProbe Exec saw EXTRA_PATH = %q, want empty", got) + } +} + +// TestUserEnvProbe_NoBashFallback runs against a bash-less image +// (alpine ships only ash). The probe's first attempt (bash -lic) fails; +// we still want Up to succeed and Exec to work normally. +func TestUserEnvProbe_NoBashFallback(t *testing.T) { + if testing.Short() { + t.Skip() + } + eng, rt := newEngine(t) + defer rt.Close() + + ws := writeWorkspace(t, `{"image":"`+testImage+`"}`) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + wsObj, err := eng.Up(ctx, devcontainer.UpOptions{ + LocalWorkspaceFolder: ws, + Recreate: true, + }) + if err != nil { + t.Fatalf("Up: %v", err) + } + defer func() { _ = eng.Down(context.Background(), wsObj, devcontainer.DownOptions{Remove: true}) }() + + // Exec a basic command — failure mode would be an Up-level error + // (already caught above) or Exec returning nonzero / wrong output. + res, err := eng.Exec(ctx, wsObj, devcontainer.ExecOptions{ + Cmd: []string{"sh", "-c", "echo ok"}, + }) + if err != nil { + t.Fatalf("Exec: %v", err) + } + if !strings.Contains(res.Stdout, "ok") { + t.Errorf("Exec stdout = %q, want contains 'ok'", res.Stdout) + } +} + +// TestUserEnvProbe_RemoteEnvMerged confirms cfg.RemoteEnv reaches Exec +// independently of whether the probe ran. RemoteEnv is the +// devcontainer-author-declared env; it must layer on top of probedEnv +// and be visible to every Exec call (matching @devcontainers/cli and +// devpod). +func TestUserEnvProbe_RemoteEnvMerged(t *testing.T) { + if testing.Short() { + t.Skip() + } + eng, rt := newEngine(t) + defer rt.Close() + + ws := writeWorkspace(t, `{ + "image": "`+bashImage+`", + "remoteEnv": { + "FROM_REMOTE": "remote-value" + } + }`) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + wsObj, err := eng.Up(ctx, devcontainer.UpOptions{ + LocalWorkspaceFolder: ws, + Recreate: true, + }) + if err != nil { + t.Fatalf("Up: %v", err) + } + defer func() { _ = eng.Down(context.Background(), wsObj, devcontainer.DownOptions{Remove: true}) }() + + res, err := eng.Exec(ctx, wsObj, devcontainer.ExecOptions{ + Cmd: []string{"printenv", "FROM_REMOTE"}, + }) + if err != nil { + t.Fatalf("Exec: %v", err) + } + if got := strings.TrimSpace(res.Stdout); got != "remote-value" { + t.Errorf("FROM_REMOTE = %q, want %q", got, "remote-value") + } +} diff --git a/up.go b/up.go index 8322f25..c5842c7 100644 --- a/up.go +++ b/up.go @@ -163,6 +163,16 @@ func (e *Engine) Up(ctx context.Context, opts UpOptions) (*Workspace, error) { return nil, err } } + + // Probe the user's interactive shell environment AFTER lifecycle — + // postCreateCommand and friends typically install rc-modifying tools + // (nvm, asdf, etc.) and we want their effects reflected in subsequent + // Exec calls. Failures are non-fatal: we log via warnings and proceed + // without probed env (matches devpod's behavior). + probed, err := e.probeUserEnv(ctx, ws, ws.Config.UserEnvProbe) + if err == nil { + ws.probedEnv = probed + } return ws, nil } diff --git a/userenvprobe.go b/userenvprobe.go new file mode 100644 index 0000000..75bdc2d --- /dev/null +++ b/userenvprobe.go @@ -0,0 +1,185 @@ +package devcontainer + +import ( + "context" + "regexp" + "slices" + "strings" + "time" + + "github.com/crunchloop/devcontainer/config" + "github.com/crunchloop/devcontainer/runtime" +) + +// userEnvProbeTimeout caps the time we wait for a login/interactive shell +// to print its environment. A misconfigured rc file (infinite loop, +// blocking prompt) shouldn't hang Up indefinitely; we'd rather fall back +// to no probed env than block. +const userEnvProbeTimeout = 10 * time.Second + +// probeUserEnv runs the shell strategy named by probe inside the +// container and returns the parsed environment. Mirrors @devcontainers/cli +// and devpod's behavior: +// +// - "none" → returns nil (no probing). +// - others → spawn bash with the appropriate flags, dump +// /proc/self/environ; fall back to printenv if that fails. +// +// Errors from the probe are non-fatal at the caller (Up); we return +// (nil, err) so the caller can log and proceed without injection. +func (e *Engine) probeUserEnv(ctx context.Context, ws *Workspace, probe config.UserEnvProbe) (map[string]string, error) { + if probe == config.UserEnvProbeNone { + return nil, nil + } + + flags := probeShellFlags(probe) + + probeCtx, cancel := context.WithTimeout(ctx, userEnvProbeTimeout) + defer cancel() + + // First attempt: read /proc/self/environ (NUL-delimited, captures + // values with embedded newlines correctly). + out, err := e.runtime.ExecContainer(probeCtx, ws.Container.ID, runtime.ExecOptions{ + Cmd: []string{"bash", flags, "cat /proc/self/environ"}, + User: effectiveUser(ws.Config), + }) + if err == nil && out.ExitCode == 0 && out.Stdout != "" { + return parseEnvironBytes(out.Stdout, '\x00'), nil + } + + // Fallback: printenv (newline-delimited). Used when /proc isn't + // available (some non-Linux containers) or bash isn't installed + // and we ended up running another shell that lacks /proc/self. + out, err2 := e.runtime.ExecContainer(probeCtx, ws.Container.ID, runtime.ExecOptions{ + Cmd: []string{"sh", flags, "printenv"}, + User: effectiveUser(ws.Config), + }) + if err2 != nil { + // Surface the original /proc/self/environ error if the + // fallback also failed — that's the more diagnostic one. + if err != nil { + return nil, err + } + return nil, err2 + } + if out.ExitCode != 0 { + return nil, nil + } + return parseEnvironBytes(out.Stdout, '\n'), nil +} + +func probeShellFlags(probe config.UserEnvProbe) string { + switch probe { + case config.UserEnvProbeLoginShell: + return "-lc" + case config.UserEnvProbeInteractiveShell: + return "-ic" + case config.UserEnvProbeLoginInteractive: + return "-lic" + default: + return "-lic" + } +} + +func parseEnvironBytes(s string, sep byte) map[string]string { + out := map[string]string{} + start := 0 + for i := 0; i <= len(s); i++ { + if i == len(s) || s[i] == sep { + line := s[start:i] + start = i + 1 + if line == "" { + continue + } + eq := strings.IndexByte(line, '=') + if eq <= 0 { + continue + } + // Don't trim: env values can legitimately contain leading + // or trailing whitespace, and /proc/self/environ entries + // have no separator framing to strip. + out[line[:eq]] = line[eq+1:] + } + } + // PWD reflects the probing shell's cwd, not the caller's intent — + // matches devpod's behavior of dropping it. + delete(out, "PWD") + if len(out) == 0 { + return nil + } + return out +} + +// mergeProbedEnv composes the effective environment for an exec call. +// Layering (lowest precedence first): +// +// 1. probed (from userEnvProbe) +// 2. cfg.RemoteEnv (devcontainer author intent — overrides probed) +// 3. callerEnv (the explicit ExecOptions.Env passed by the user — wins) +// +// PATH gets a special merge that mirrors devpod: probed PATH is the +// base, then RemoteEnv PATH entries are inserted in their declared +// order so author-added prefix entries land in front. Caller's PATH, +// if any, still wins outright (consistent with the rest of the +// precedence rule — caller is most explicit). +// +// /sbin paths from RemoteEnv are stripped for non-root users, again +// matching devpod and the @devcontainers/cli reference, since /sbin +// binaries usually require root. +func mergeProbedEnv(probed, remoteEnv, callerEnv map[string]string, remoteUser string) map[string]string { + if len(probed) == 0 && len(remoteEnv) == 0 { + return callerEnv + } + out := make(map[string]string, len(probed)+len(remoteEnv)+len(callerEnv)) + for k, v := range probed { + out[k] = v + } + for k, v := range remoteEnv { + out[k] = v + } + + // PATH-aware merge between probed and remoteEnv (caller PATH still + // wins below). + probedPath, hasProbedPath := probed["PATH"] + remotePath, hasRemotePath := remoteEnv["PATH"] + if hasProbedPath && hasRemotePath { + out["PATH"] = mergePath(probedPath, remotePath, remoteUser) + } + + for k, v := range callerEnv { + out[k] = v + } + if len(out) == 0 { + return nil + } + return out +} + +var sbinSegment = regexp.MustCompile(`/sbin(/|$)`) + +// isRootUser reports whether u names the root user. Accepts the +// canonical "root", the numeric uid "0", and "" (image default — +// many minimal images, including alpine and bash:*-alpine, run as +// root by default; misclassifying that as non-root would strip +// /sbin entries the user actually has access to). +func isRootUser(u string) bool { + return u == "root" || u == "0" || u == "" +} + +func mergePath(probed, remote, remoteUser string) string { + probedTokens := strings.Split(probed, ":") + insertAt := 0 + root := isRootUser(remoteUser) + for _, e := range strings.Split(remote, ":") { + i := slices.Index(probedTokens, e) + if i == -1 { + if root || !sbinSegment.MatchString(e) { + probedTokens = slices.Insert(probedTokens, insertAt, e) + insertAt++ + } + } else { + insertAt = i + 1 + } + } + return strings.Join(probedTokens, ":") +} diff --git a/workspace.go b/workspace.go index 38b198b..95c5b1d 100644 --- a/workspace.go +++ b/workspace.go @@ -23,6 +23,15 @@ type Workspace struct { Container *runtime.ContainerDetails subst *Substituter + + // probedEnv holds the environment captured by running a login/ + // interactive shell inside the container (per cfg.UserEnvProbe). + // Engine.Exec merges it under opts.Env so callers see PATH, NVM, + // asdf-style additions injected by the user's rc files. Populated + // after Up's lifecycle phase (so probes reflect any rc-modifying + // postCreate scripts) and on Attach. nil means the probe hasn't + // run yet or userEnvProbe was "none". + probedEnv map[string]string } // Substituter resolves devcontainer.json substitution placeholders against