ssh/tailssh: keep acceptEnv values and names out of the incubator cmdline (#20552)

Previously the acceptEnv variables forwarded to the incubator child were
JSON-encoded onto its command line (--encoded-env), so their values were
visible in /proc/<pid>/cmdline to any other local user and were logged in
the session-start argv (locally and to log.tailscale.com except where
--no-logs-no-support was specified).

This change now carries those variables through an os.Pipe file
descriptor as a json encoded payload. Added end-to-end testing
helps validate secrets reach the session but are not in flags or logged.

Fixes tailscale/corp#44903

Change-Id: I5b137b20e9c06feec6b70aaf4e6925e6db74017e

Signed-off-by: Mike Jensen <mikej@tailscale.com>
Co-authored-by: Mike Jensen <mikej@tailscale.com>

ssh/tailssh/accept_env.go

@@ -13,11 +13,26 @@
 // is unconditionally prohibited from being forwarded, regardless of
 // acceptEnv policy. This prevents privilege escalation via dynamic
 // linker environment variables (e.g. LD_PRELOAD, LD_LIBRARY_PATH,
-// DYLD_INSERT_LIBRARIES) even when a wildcard acceptEnv pattern like
-// "*" is configured.
+// DYLD_INSERT_LIBRARIES) or leaking of secrets (e.g. GOTRACEBACK)
+// even when a wildcard acceptEnv pattern like "*" is configured.
 func isDangerousEnvVar(name string) bool {
 	upper := strings.ToUpper(name)
-	return strings.HasPrefix(upper, "LD_") || strings.HasPrefix(upper, "DYLD_")
+	return strings.HasPrefix(upper, "LD_") || strings.HasPrefix(upper, "DYLD_") ||
+		upper == "GOTRACEBACK"
+}
+
+// forbiddenEnvKey reports whether name must never be accepted from the client as a
+// forwarded environment variable, independent of the acceptEnv policy. Names are
+// restricted to a known-safe charset so they cannot corrupt the "su -w" allowlist
+// built from them, truncate on exec, or confuse downstream consumers of the user's
+// environment.
+func forbiddenEnvKey(name string) bool {
+	for _, r := range name {
+		if r != '_' && r != '-' && (r < 'a' || r > 'z') && (r < 'A' || r > 'Z') && (r < '0' || r > '9') {
+			return true
+		}
+	}
+	return name == ""
 }
 
 // filterEnv filters a passed in environ string slice (a slice with strings
@@ -47,12 +62,23 @@ func filterEnv(acceptEnv []string, environ []string) ([]string, error) {
 			return nil, fmt.Errorf(`invalid environment variable: %q. Variables must be in "KEY=VALUE" format`, envPair)
 		}
 
+		// Reject NUL bytes: envp entries are NUL-terminated, so a NUL would silently truncate on exec
+		if strings.Contains(envPair, "\x00") {
+			continue
+		}
+
 		// Always reject dangerous environment variables that could
 		// enable privilege escalation, regardless of acceptEnv policy.
 		if isDangerousEnvVar(variableName) {
 			continue
 		}
 
+		// Always reject names that would corrupt the incubator's "su -w"
+		// allowlist (see reservedEnvKey), regardless of acceptEnv policy.
+		if forbiddenEnvKey(variableName) {
+			continue
+		}
+
 		// Short circuit if we have a direct match between the environment
 		// variable and an AcceptEnv value.
 		if slices.Contains(acceptEnv, variableName) {

ssh/tailssh/accept_env_test.go

@@ -204,6 +204,48 @@ func TestFilterEnv(t *testing.T) {
 			environ:          []string{"DYLD_INSERT_LIBRARIES=/tmp/evil.dylib", "DYLD_LIBRARY_PATH=/tmp", "TERM=xterm"},
 			expectedFiltered: []string{"TERM=xterm"},
 		},
+		{
+			// A forwarded key containing the "," allowlist separator must be
+			// rejected, since it would otherwise inject extra "su -w" entries.
+			name:             "comma-in-key-rejected",
+			acceptEnv:        []string{"*"},
+			environ:          []string{"A,B=x", "GOOD=1"},
+			expectedFiltered: []string{"GOOD=1"},
+		},
+		{
+			name:             "empty-key-rejected",
+			acceptEnv:        []string{"*"},
+			environ:          []string{"=x", "GOOD=1"},
+			expectedFiltered: []string{"GOOD=1"},
+		},
+		{
+			// GOTRACEBACK controls crash tracebacks/core dumps of the
+			// privileged incubator child, which could leak secrets
+			name:             "gotraceback-rejected",
+			acceptEnv:        []string{"*"},
+			environ:          []string{"GOTRACEBACK=crash", "TERM=xterm"},
+			expectedFiltered: []string{"TERM=xterm"},
+		},
+		{
+			name:             "nul-in-name-rejected",
+			acceptEnv:        []string{"*"},
+			environ:          []string{"A\x00B=x", "GOOD=1"},
+			expectedFiltered: []string{"GOOD=1"},
+		},
+		{
+			name:             "nul-in-value-rejected",
+			acceptEnv:        []string{"*"},
+			environ:          []string{"GOOD=a\x00b"},
+			expectedFiltered: nil,
+		},
+		{
+			// Key names are closed to a known charset: spaces, control
+			// chars, punctuation other than '-' and non-ASCII are rejected.
+			name:             "key-charset-restricted",
+			acceptEnv:        []string{"*"},
+			environ:          []string{"MY VAR=1", "WEIRD\tNAME=2", "MY.VAR=3", "MY-VAR=ok", "B\xc3\xa4R=5", "MY_VAR=ok2"},
+			expectedFiltered: []string{"MY-VAR=ok", "MY_VAR=ok2"},
+		},
 	}
 
 	for _, tc := range testCases {

ssh/tailssh/incubator.go

@@ -114,8 +114,14 @@ func tryExecInDir(ctx context.Context, dir string) error {
 // behavior of SSHD when by falling back to the root directory if it cannot run
 // a command in the user’s home directory.
 //
-// The returned Cmd.Env is guaranteed to be nil; the caller populates it.
-func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, err error) {
+// It also returns forwardedEnv, the set of client-forwarded "KEY=VALUE"
+// environment variables accepted by the acceptEnv policy. These may contain
+// secrets, so the caller passes them to the child via an inherited file rather
+// than the command line or environment, where they could leak, or influence
+// the privileged child.
+//
+// The returned Cmd.Env is guaranteed to be nil; the caller must populate it.
+func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, forwardedEnv []string, err error) {
 	defer func() {
 		if cmd != nil && cmd.Env != nil {
 			panic("internal error")
@@ -136,7 +142,7 @@ func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, err
 		if isSFTP {
 			// SFTP relies on the embedded Go-based SFTP server in tailscaled,
 			// so without tailscaled, we can't serve SFTP.
-			return nil, errors.New("no tailscaled found on path, can't serve SFTP")
+			return nil, nil, errors.New("no tailscaled found on path, can't serve SFTP")
 		}
 
 		loginShell := ss.conn.localUser.LoginShell()
@@ -163,13 +169,13 @@ func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, err
 				// we will try to run this command in the root directory.
 				cmd.Dir = "/"
 			} else {
-				return nil, err
+				return nil, nil, err
 			}
 		case err != nil:
-			return nil, err
+			return nil, nil, err
 		}
 
-		return cmd, nil
+		return cmd, nil, nil
 	}
 
 	lu := ss.conn.localUser
@@ -231,26 +237,27 @@ func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, err
 	if allowSendEnv {
 		env, err := filterEnv(ss.conn.acceptEnv, ss.Session.Environ())
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 
 		if len(env) > 0 {
-			encoded, err := json.Marshal(env)
-			if err != nil {
-				return nil, fmt.Errorf("failed to encode environment: %w", err)
-			}
-			incubatorArgs = append(incubatorArgs, fmt.Sprintf("--encoded-env=%q", encoded))
+			// The accepted environment may contain secrets, so it is communicated through a fd
+			// created by the caller, never the argv or the child's environment.
+			incubatorArgs = append(incubatorArgs, fmt.Sprintf("--env-fd=%d", forwardedEnvChildFD))
+			forwardedEnv = env
 		}
 	}
 
 	cmd = exec.CommandContext(ss.ctx, ss.conn.srv.tailscaledPath, incubatorArgs...)
 	// The incubator will chdir into the home directory after it drops privileges.
 	cmd.Dir = "/"
-	return cmd, nil
+	return cmd, forwardedEnv, nil
 }
 
-var debugIncubator bool
-var debugTest atomic.Bool
+var (
+	debugIncubator bool
+	debugTest      atomic.Bool
+)
 
 type stdRWC struct{}
 
@@ -284,7 +291,15 @@ type incubatorArgs struct {
 	forceV1Behavior    bool
 	debugTest          bool
 	isSELinuxEnforcing bool
-	encodedEnv         string
+	// Deprecated: encodedEnv is deprecated and must not be used by new code.
+	// It is parsed only so this child keeps working when exec'd by an
+	// outdated parent tailscaled that still passes it.
+	encodedEnv string
+	// envFD is the file descriptor to read the forwarded environment from
+	// (a JSON array of KEY=VALUE pairs), or -1 if none.
+	envFD int
+	// forwardedEnv holds the pairs loaded by loadForwardedEnv.
+	forwardedEnv []string
 }
 
 func parseIncubatorArgs(args []string) (incubatorArgs, error) {
@@ -308,7 +323,9 @@ func parseIncubatorArgs(args []string) (incubatorArgs, error) {
 	flags.BoolVar(&ia.forceV1Behavior, "force-v1-behavior", false, "allow falling back to the su command if login is unavailable")
 	flags.BoolVar(&ia.debugTest, "debug-test", false, "should debug in test mode")
 	flags.BoolVar(&ia.isSELinuxEnforcing, "is-selinux-enforcing", false, "whether SELinux is in enforcing mode")
-	flags.StringVar(&ia.encodedEnv, "encoded-env", "", "JSON encoded array of environment variables in '['key=value']' format")
+	// DEPRECATED: retained for version-skew compatibility only. DO NOT USE.
+	flags.StringVar(&ia.encodedEnv, "encoded-env", "", "deprecated; do not use")
+	flags.IntVar(&ia.envFD, "env-fd", -1, "file descriptor to read the forwarded environment from (JSON array of KEY=VALUE pairs)")
 	flags.Parse(args)
 
 	for g := range strings.SplitSeq(groups, ",") {
@@ -319,47 +336,62 @@ func parseIncubatorArgs(args []string) (incubatorArgs, error) {
 		ia.gids = append(ia.gids, gid)
 	}
 
+	// envFD comes from an ExtraFiles entry, so it must never name stdin/out/err
+	if ia.envFD >= 0 && ia.envFD < 3 {
+		return ia, fmt.Errorf("invalid --env-fd %d: must be >= 3", ia.envFD)
+	}
+
 	return ia, nil
 }
 
-// forwardedEnviron returns the concatenation of the current environment with
-// any environment variables specified in ia.encodedEnv.
-//
-// It also returns allowedExtraKeys, containing the env keys that were passed in
-// to ia.encodedEnv.
-func (ia incubatorArgs) forwardedEnviron() (env, allowedExtraKeys []string, err error) {
-	environ := os.Environ()
-
-	// pass through SSH_AUTH_SOCK environment variable to support ssh agent forwarding
-	// TODO(bradfitz,percy): why is this listed specially? If the parent wanted to included
-	// it, couldn't it have just passed it to the incubator in encodedEnv?
-	// If it didn't, no reason for us to pass it to "su -w ..." if it's not in our env
-	// anyway? (Surely we don't want to inherit the tailscaled parent SSH_AUTH_SOCK, if any)
-	allowedExtraKeys = []string{"SSH_AUTH_SOCK"}
-
-	if ia.encodedEnv != "" {
-		unquoted, err := strconv.Unquote(ia.encodedEnv)
-		if err != nil {
-			return nil, nil, fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
+// loadForwardedEnv reads the client-forwarded environment pairs into ia.forwardedEnv, from the
+// inherited file named by --env-fd. The pairs only enter the su/login/shell environment,
+// never this process's own environment.
+func (ia *incubatorArgs) loadForwardedEnv() error {
+	var pairs []string
+	switch {
+	case ia.envFD >= 0:
+		if ia.envFD < 3 {
+			return fmt.Errorf("invalid --env-fd=%d: must be >= 3", ia.envFD)
 		}
-
-		var extraEnviron []string
-
-		err = json.Unmarshal([]byte(unquoted), &extraEnviron)
-		if err != nil {
-			return nil, nil, fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
+		f := os.NewFile(uintptr(ia.envFD), "forwarded-env")
+		defer f.Close()
+		if err := json.NewDecoder(f).Decode(&pairs); err != nil {
+			return fmt.Errorf("unable to read forwarded environment: %w", err)
 		}
-
-		environ = append(environ, extraEnviron...)
-
-		for _, kv := range extraEnviron {
-			if k, _, ok := strings.Cut(kv, "="); ok {
-				allowedExtraKeys = append(allowedExtraKeys, k)
-			}
+	case ia.encodedEnv != "": // Legacy path to support an outdated parent tailscaled
+		if unquoted, err := strconv.Unquote(ia.encodedEnv); err != nil {
+			return fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
+		} else if err := json.Unmarshal([]byte(unquoted), &pairs); err != nil {
+			return fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
 		}
 	}
+	// Enforce "su -w" integrity child-side: old parents may not have filtered these
+	pairs = slices.DeleteFunc(pairs, func(kv string) bool {
+		k, _, ok := strings.Cut(kv, "=")
+		return !ok || forbiddenEnvKey(k) || strings.Contains(kv, "\x00")
+	})
+	ia.forwardedEnv = pairs
+	return nil
+}
 
-	return environ, allowedExtraKeys, nil
+// forwardedEnviron returns the environment to hand to the user's process. This includes the
+// current environment plus the client-forwarded pairs. It also returns allowedExtraKeys for
+// the "su -w" allowlist, the forwarded key names, plus SSH_AUTH_SOCK.
+func (ia *incubatorArgs) forwardedEnviron() (env, allowedExtraKeys []string) {
+	// SSH_AUTH_SOCK is allowlisted here rather than forwarded because old
+	// parents set it without forwarding any keys. It can only be present if
+	// the parent enabled agent forwarding: the child's environment is built
+	// by incubatorEnv, never from the parent's os.Environ.
+	allowedExtraKeys = []string{"SSH_AUTH_SOCK"}
+
+	env = append(os.Environ(), ia.forwardedEnv...)
+	for _, kv := range ia.forwardedEnv {
+		if k, _, ok := strings.Cut(kv, "="); ok {
+			allowedExtraKeys = append(allowedExtraKeys, k)
+		}
+	}
+	return env, allowedExtraKeys
 }
 
 // beIncubator is the entrypoint to the `tailscaled be-child ssh` subcommand.
@@ -383,6 +415,13 @@ func beIncubator(args []string) error {
 	if err != nil {
 		return err
 	}
+	if ia.encodedEnv != "" {
+		log.Printf("WARNING: tailscaled be-child: accepted SSH environment variables were passed via the deprecated --encoded-env flag; " +
+			"the running tailscaled is outdated. Update tailscaled to the latest version and restart for the latest security fixes.")
+	}
+	if err := ia.loadForwardedEnv(); err != nil {
+		return err
+	}
 	if ia.isSFTP && ia.isShell {
 		return fmt.Errorf("--sftp and --shell are mutually exclusive")
 	}
@@ -395,7 +434,7 @@ func beIncubator(args []string) error {
 		}
 	} else if ia.debugTest {
 		// In testing, we don't always have syslog, so log to a temp file.
-		if logFile, err := os.OpenFile("/tmp/tailscalessh.log", os.O_APPEND|os.O_WRONLY, 0666); err == nil {
+		if logFile, err := os.OpenFile("/tmp/tailscalessh.log", os.O_APPEND|os.O_WRONLY, 0o666); err == nil {
 			lf := log.New(logFile, "", 0)
 			dlogf = func(msg string, args ...any) {
 				lf.Printf(msg, args...)
@@ -539,10 +578,7 @@ func tryExecLogin(dlogf logger.Logf, ia incubatorArgs) error {
 	loginArgs := ia.loginArgs(loginCmdPath)
 	dlogf("logging in with %+v", loginArgs)
 
-	environ, _, err := ia.forwardedEnviron()
-	if err != nil {
-		return err
-	}
+	environ, _ := ia.forwardedEnviron()
 
 	// If Exec works, the Go code will not proceed past this:
 	err = unix.Exec(loginCmdPath, loginArgs, environ)
@@ -578,10 +614,7 @@ func trySU(dlogf logger.Logf, ia incubatorArgs) (handled bool, err error) {
 		defer sessionCloser()
 	}
 
-	environ, allowListEnvKeys, err := ia.forwardedEnviron()
-	if err != nil {
-		return false, err
-	}
+	environ, allowListEnvKeys := ia.forwardedEnviron()
 
 	loginArgs := []string{
 		su,
@@ -626,10 +659,7 @@ func findSU(dlogf logger.Logf, ia incubatorArgs) string {
 		return ""
 	}
 
-	_, allowListEnvKeys, err := ia.forwardedEnviron()
-	if err != nil {
-		return ""
-	}
+	_, allowListEnvKeys := ia.forwardedEnviron()
 
 	// First try to execute su -w <allow listed env> -l <user> -c true
 	// to make sure su supports the necessary arguments.
@@ -662,15 +692,12 @@ func handleSSHInProcess(dlogf logger.Logf, ia incubatorArgs) error {
 		return err
 	}
 
-	environ, _, err := ia.forwardedEnviron()
-	if err != nil {
-		return err
-	}
+	environ, _ := ia.forwardedEnviron()
 
 	args := shellArgs(ia.isShell, ia.cmd)
 	dlogf("running %s %q", ia.loginShell, args)
 	cmd := newCommand(ia.hasTTY, ia.loginShell, environ, args)
-	err = cmd.Run()
+	err := cmd.Run()
 	if ee, ok := err.(*exec.ExitError); ok {
 		ps := ee.ProcessState
 		code := ps.ExitCode()
@@ -832,6 +859,32 @@ func doDropPrivileges(dlogf logger.Logf, wantUid, wantGid int, supplementaryGrou
 	return nil
 }
 
+// incubatorEnv builds the environment (cmd.Env) for the incubator child:
+// the user's login environment, the client's TERM/LANG/LC_* (matching
+// OpenSSH's default AcceptEnv), the connection metadata, and the optional
+// agent socket. acceptEnv-forwarded variables are not included; they travel
+// via an inherited file instead (see forwardedEnvFile).
+func (ss *sshSession) incubatorEnv() []string {
+	env := envForUser(ss.conn.localUser)
+	for _, kv := range ss.Environ() {
+		if acceptEnvPair(kv) {
+			env = append(env, kv)
+		}
+	}
+
+	ci := ss.conn.info
+	env = append(env,
+		fmt.Sprintf("SSH_CLIENT=%s %d %d", ci.src.Addr(), ci.src.Port(), ci.dst.Port()),
+		fmt.Sprintf("SSH_CONNECTION=%s %d %s %d", ci.src.Addr(), ci.src.Port(), ci.dst.Addr(), ci.dst.Port()),
+	)
+
+	if ss.agentListener != nil {
+		env = append(env, fmt.Sprintf("SSH_AUTH_SOCK=%s", ss.agentListener.Addr()))
+	}
+
+	return env
+}
+
 // launchProcess launches an incubator process for the provided session.
 // It is responsible for configuring the process execution environment.
 // The caller can wait for the process to exit by calling cmd.Wait().
@@ -839,27 +892,25 @@ func doDropPrivileges(dlogf logger.Logf, wantUid, wantGid int, supplementaryGrou
 // It sets ss.cmd, stdin, stdout, and stderr.
 func (ss *sshSession) launchProcess() error {
 	var err error
-	ss.cmd, err = ss.newIncubatorCommand(ss.logf)
+	var forwardedEnv []string
+	ss.cmd, forwardedEnv, err = ss.newIncubatorCommand(ss.logf)
 	if err != nil {
 		return err
 	}
 
 	cmd := ss.cmd
-	cmd.Env = envForUser(ss.conn.localUser)
-	for _, kv := range ss.Environ() {
-		if acceptEnvPair(kv) {
-			cmd.Env = append(cmd.Env, kv)
+	cmd.Env = ss.incubatorEnv()
+
+	if len(forwardedEnv) > 0 {
+		// The accepted environment may contain secrets, so it is passed to the child via an
+		// inherited file, never the argv or the environment. Closing after Start is safe: the
+		// child has its own copy by then.
+		envFile, err := forwardedEnvFile(forwardedEnv)
+		if err != nil {
+			return err
 		}
-	}
-
-	ci := ss.conn.info
-	cmd.Env = append(cmd.Env,
-		fmt.Sprintf("SSH_CLIENT=%s %d %d", ci.src.Addr(), ci.src.Port(), ci.dst.Port()),
-		fmt.Sprintf("SSH_CONNECTION=%s %d %s %d", ci.src.Addr(), ci.src.Port(), ci.dst.Addr(), ci.dst.Port()),
-	)
-
-	if ss.agentListener != nil {
-		cmd.Env = append(cmd.Env, fmt.Sprintf("SSH_AUTH_SOCK=%s", ss.agentListener.Addr()))
+		defer envFile.Close()
+		cmd.ExtraFiles = []*os.File{envFile}
 	}
 
 	ptyReq, winCh, isPty := ss.Pty()
@@ -1123,7 +1174,7 @@ func updateStringInSlice(ss []string, a, b string) {
 // AcceptEnv.
 func acceptEnvPair(kv string) bool {
 	k, _, ok := strings.Cut(kv, "=")
-	if !ok {
+	if !ok || isDangerousEnvVar(k) || forbiddenEnvKey(k) {
 		return false
 	}
 	return k == "TERM" || k == "LANG" || strings.HasPrefix(k, "LC_")

ssh/tailssh/incubator_plan9.go

@@ -19,6 +19,7 @@
 	"os"
 	"os/exec"
 	"runtime"
+	"slices"
 	"strconv"
 	"strings"
 	"sync/atomic"
@@ -42,10 +43,15 @@ func init() {
 // If ss.srv.tailscaledPath is empty, this method is equivalent to
 // exec.CommandContext.
 //
-// The returned Cmd.Env is guaranteed to be nil; the caller populates it.
-func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, err error) {
+// It also returns forwardedEnv, the client-forwarded environment variables
+// accepted by the acceptEnv policy. These may contain secrets, so the caller
+// passes them to the child via an inherited file rather than the command line
+// or environment.
+//
+// The returned Cmd.Env is guaranteed to be nil; the caller must populate it.
+func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, forwardedEnv []string, err error) {
 	defer func() {
-		if cmd.Env != nil {
+		if cmd != nil && cmd.Env != nil {
 			panic("internal error")
 		}
 	}()
@@ -64,12 +70,12 @@ func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, err
 		if isSFTP {
 			// SFTP relies on the embedded Go-based SFTP server in tailscaled,
 			// so without tailscaled, we can't serve SFTP.
-			return nil, errors.New("no tailscaled found on path, can't serve SFTP")
+			return nil, nil, errors.New("no tailscaled found on path, can't serve SFTP")
 		}
 
 		loginShell := ss.conn.localUser.LoginShell()
 		logf("directly running /bin/rc -c %q", ss.RawCommand())
-		return exec.CommandContext(ss.ctx, loginShell, "-c", ss.RawCommand()), nil
+		return exec.CommandContext(ss.ctx, loginShell, "-c", ss.RawCommand()), nil, nil
 	}
 
 	lu := ss.conn.localUser
@@ -121,19 +127,18 @@ func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, err
 	if allowSendEnv {
 		env, err := filterEnv(ss.conn.acceptEnv, ss.Session.Environ())
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 
 		if len(env) > 0 {
-			encoded, err := json.Marshal(env)
-			if err != nil {
-				return nil, fmt.Errorf("failed to encode environment: %w", err)
-			}
-			incubatorArgs = append(incubatorArgs, fmt.Sprintf("--encoded-env=%q", encoded))
+			// The accepted environment may contain secrets, so it travels via an
+			// inherited file (created by the caller). The fd number is not sensitive.
+			incubatorArgs = append(incubatorArgs, fmt.Sprintf("--env-fd=%d", forwardedEnvChildFD))
+			forwardedEnv = env
 		}
 	}
 
-	return exec.CommandContext(ss.ctx, ss.conn.srv.tailscaledPath, incubatorArgs...), nil
+	return exec.CommandContext(ss.ctx, ss.conn.srv.tailscaledPath, incubatorArgs...), forwardedEnv, nil
 }
 
 var debugTest atomic.Bool
@@ -166,7 +171,15 @@ type incubatorArgs struct {
 	forceV1Behavior    bool
 	debugTest          bool
 	isSELinuxEnforcing bool
-	encodedEnv         string
+	// Deprecated: encodedEnv is deprecated and must not be used by new code.
+	// It is parsed only so this child keeps working when exec'd by an
+	// outdated parent tailscaled that still passes it.
+	encodedEnv string
+	// envFD is the file descriptor to read the forwarded environment from
+	// (a JSON array of KEY=VALUE pairs), or -1 if none.
+	envFD int
+	// forwardedEnv holds the pairs loaded by loadForwardedEnv.
+	forwardedEnv []string
 }
 
 func parseIncubatorArgs(args []string) (incubatorArgs, error) {
@@ -185,37 +198,62 @@ func parseIncubatorArgs(args []string) (incubatorArgs, error) {
 	flags.BoolVar(&ia.forceV1Behavior, "force-v1-behavior", false, "allow falling back to the su command if login is unavailable")
 	flags.BoolVar(&ia.debugTest, "debug-test", false, "should debug in test mode")
 	flags.BoolVar(&ia.isSELinuxEnforcing, "is-selinux-enforcing", false, "whether SELinux is in enforcing mode")
-	flags.StringVar(&ia.encodedEnv, "encoded-env", "", "JSON encoded array of environment variables in '['key=value']' format")
+	// DEPRECATED: retained for version-skew compatibility only. DO NOT USE.
+	flags.StringVar(&ia.encodedEnv, "encoded-env", "", "deprecated; do not use")
+	flags.IntVar(&ia.envFD, "env-fd", -1, "file descriptor to read the forwarded environment from (JSON array of KEY=VALUE pairs)")
 	flags.Parse(args)
+	// envFD comes from an ExtraFiles entry, so it must never name stdin/out/err
+	if ia.envFD >= 0 && ia.envFD < 3 {
+		return ia, fmt.Errorf("invalid --env-fd %d: must be >= 3", ia.envFD)
+	}
 	return ia, nil
 }
 
-func (ia incubatorArgs) forwardedEnviron() ([]string, string, error) {
-	environ := os.Environ()
-	// pass through SSH_AUTH_SOCK environment variable to support ssh agent forwarding
-	allowListKeys := "SSH_AUTH_SOCK"
-
-	if ia.encodedEnv != "" {
-		unquoted, err := strconv.Unquote(ia.encodedEnv)
-		if err != nil {
-			return nil, "", fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
+// loadForwardedEnv reads the client-forwarded environment pairs into ia.forwardedEnv, from the
+// inherited file named by --env-fd. The pairs only enter the su/login/shell environment,
+// never this process's own environment.
+func (ia *incubatorArgs) loadForwardedEnv() error {
+	var pairs []string
+	switch {
+	case ia.envFD >= 0:
+		if ia.envFD < 3 {
+			return fmt.Errorf("invalid --env-fd=%d: must be >= 3", ia.envFD)
 		}
-
-		var extraEnviron []string
-
-		err = json.Unmarshal([]byte(unquoted), &extraEnviron)
-		if err != nil {
-			return nil, "", fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
+		f := os.NewFile(uintptr(ia.envFD), "forwarded-env")
+		defer f.Close()
+		if err := json.NewDecoder(f).Decode(&pairs); err != nil {
+			return fmt.Errorf("unable to read forwarded environment: %w", err)
 		}
+	case ia.encodedEnv != "": // Legacy path to support an outdated parent tailscaled
+		if unquoted, err := strconv.Unquote(ia.encodedEnv); err != nil {
+			return fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
+		} else if err := json.Unmarshal([]byte(unquoted), &pairs); err != nil {
+			return fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
+		}
+	}
+	// Enforce "su -w" integrity child-side: old parents may not have filtered these
+	pairs = slices.DeleteFunc(pairs, func(kv string) bool {
+		k, _, ok := strings.Cut(kv, "=")
+		return !ok || forbiddenEnvKey(k) || strings.Contains(kv, "\x00")
+	})
+	ia.forwardedEnv = pairs
+	return nil
+}
 
-		environ = append(environ, extraEnviron...)
+// forwardedEnviron returns the environment to hand to the user's process (the
+// current environment plus the client-forwarded pairs) and the comma-separated
+// allowlist of forwarded key names, plus SSH_AUTH_SOCK.
+func (ia *incubatorArgs) forwardedEnviron() ([]string, string) {
+	allowListKeys := []string{"SSH_AUTH_SOCK"}
 
-		for _, v := range extraEnviron {
-			allowListKeys = fmt.Sprintf("%s,%s", allowListKeys, strings.Split(v, "=")[0])
+	environ := append(os.Environ(), ia.forwardedEnv...)
+	for _, kv := range ia.forwardedEnv {
+		if k, _, ok := strings.Cut(kv, "="); ok {
+			allowListKeys = append(allowListKeys, k)
 		}
 	}
 
-	return environ, allowListKeys, nil
+	return environ, strings.Join(allowListKeys, ",")
 }
 
 func beNetshell(args []string) error {
@@ -244,6 +282,13 @@ func beIncubator(args []string) error {
 	if err != nil {
 		return err
 	}
+	if ia.encodedEnv != "" {
+		log.Printf("WARNING: tailscaled be-child: accepted SSH environment variables were passed via the deprecated --encoded-env flag; " +
+			"the running tailscaled is outdated. Update tailscaled to the latest version and restart for the latest security fixes.")
+	}
+	if err := ia.loadForwardedEnv(); err != nil {
+		return err
+	}
 	if ia.isSFTP && ia.isShell {
 		return fmt.Errorf("--sftp and --shell are mutually exclusive")
 	}
@@ -306,14 +351,11 @@ func serveSFTP() error {
 // login shell.
 func handleSSHInProcess(dlogf logger.Logf, ia incubatorArgs) error {
 
-	environ, _, err := ia.forwardedEnviron()
-	if err != nil {
-		return err
-	}
+	environ, _ := ia.forwardedEnviron()
 
 	dlogf("running /bin/rc -c %q", ia.cmd)
 	cmd := newCommand("/bin/rc", environ, []string{"-c", ia.cmd})
-	err = cmd.Run()
+	err := cmd.Run()
 	if ee, ok := err.(*exec.ExitError); ok {
 		ps := ee.ProcessState
 		code := ps.ExitCode()
@@ -346,7 +388,8 @@ func newCommand(cmdPath string, cmdEnviron []string, cmdArgs []string) *exec.Cmd
 // It sets ss.cmd, stdin, stdout, and stderr.
 func (ss *sshSession) launchProcess() error {
 	var err error
-	ss.cmd, err = ss.newIncubatorCommand(ss.logf)
+	var forwardedEnv []string
+	ss.cmd, forwardedEnv, err = ss.newIncubatorCommand(ss.logf)
 	if err != nil {
 		return err
 	}
@@ -370,6 +413,17 @@ func (ss *sshSession) launchProcess() error {
 		cmd.Env = append(cmd.Env, fmt.Sprintf("SSH_AUTH_SOCK=%s", ss.agentListener.Addr()))
 	}
 
+	// Client-forwarded environment variables may contain secrets, so they
+	// are passed to the child via an inherited pipe.
+	if len(forwardedEnv) > 0 {
+		envFile, err := forwardedEnvFile(forwardedEnv)
+		if err != nil {
+			return err
+		}
+		defer envFile.Close()
+		cmd.ExtraFiles = []*os.File{envFile}
+	}
+
 	return ss.startWithStdPipes()
 }
 
@@ -416,6 +470,10 @@ func acceptEnvPair(kv string) bool {
 	if !ok {
 		return false
 	}
-	_ = k
-	return true // permit anything on plan9 during bringup, for debugging at least
+	// Never forward names reserved for our own parent->child bookkeeping or unsafe for the child,
+	// even during bringup, so a client cannot spoof the incubator's env.
+	if forbiddenEnvKey(k) || isDangerousEnvVar(k) {
+		return false
+	}
+	return true // permit anything else on plan9 during bringup, for debugging at least
 }

ssh/tailssh/incubator_test.go

@@ -0,0 +1,286 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build (linux && !android) || (darwin && !ios) || freebsd || openbsd
+
+package tailssh
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"net/netip"
+	"os/user"
+	"slices"
+	"strings"
+	"testing"
+
+	gliderssh "github.com/tailscale/gliderssh"
+	"golang.org/x/sys/unix"
+	"tailscale.com/tailcfg"
+	"tailscale.com/types/logger"
+)
+
+// fakeSession is a minimal gliderssh.Session for exercising the incubator
+// command/env construction. Only the methods used by newIncubatorCommand and
+// incubatorEnv are implemented; the embedded interface is nil, so any other
+// method call panics and catches unexpected use.
+type fakeSession struct {
+	gliderssh.Session
+	rawCommand string
+	subsystem  string
+	environ    []string
+}
+
+func (s fakeSession) RawCommand() string { return s.rawCommand }
+func (s fakeSession) Subsystem() string  { return s.subsystem }
+func (s fakeSession) Environ() []string  { return s.environ }
+func (s fakeSession) Pty() (gliderssh.Pty, <-chan gliderssh.Window, bool) {
+	return gliderssh.Pty{}, nil, false
+}
+
+// newTestSession builds an sshSession wired to the localState fake, with the
+// NodeAttrSSHEnvironmentVariables capability enabled and the given acceptEnv
+// policy, so newIncubatorCommand exercises the env-forwarding path.
+func newTestSession(t *testing.T, acceptEnv []string, clientEnviron []string) *sshSession {
+	t.Helper()
+	srv := &server{
+		logf: logger.Discard,
+		lb: &localState{
+			sshEnabled: true,
+			caps:       []tailcfg.NodeCapability{tailcfg.NodeAttrSSHEnvironmentVariables},
+		},
+		// tailscaledPath must be non-empty to take the incubator (be-child)
+		// path rather than the direct-exec fallback.
+		tailscaledPath: "/usr/sbin/tailscaled",
+	}
+	c := &conn{
+		srv:       srv,
+		acceptEnv: acceptEnv,
+		info: &sshConnInfo{
+			sshUser: "alice",
+			src:     netip.MustParseAddrPort("100.100.100.101:2222"),
+			dst:     netip.MustParseAddrPort("100.100.100.102:22"),
+			node:    (&tailcfg.Node{}).View(),
+		},
+		localUser:    &userMeta{User: user.User{Username: "alice", Uid: "1000", Gid: "1000", HomeDir: "/home/alice"}},
+		userGroupIDs: []string{"1000"},
+	}
+	ctx, cancel := context.WithCancelCause(context.Background())
+	t.Cleanup(func() { cancel(nil) })
+	return &sshSession{
+		Session: fakeSession{environ: clientEnviron},
+		conn:    c,
+		ctx:     ctx,
+	}
+}
+
+// TestForwardedEnvSecretNotOnArgv is the core security regression test for the
+// acceptEnv secret leak: a forwarded value must be returned for delivery via
+// an inherited file, and must appear neither on the command line (cmd.Args,
+// logged at session start and visible in /proc/<pid>/cmdline) nor in the
+// privileged child's environment (see incubatorEnv).
+func TestForwardedEnvSecretNotOnArgv(t *testing.T) {
+	const secret = "s3cr3t-token-value"
+	ss := newTestSession(t,
+		[]string{"GITLAB_API_TOKEN"},
+		[]string{"GITLAB_API_TOKEN=" + secret, "IGNORED=nope"},
+	)
+
+	cmd, forwardedEnv, err := ss.newIncubatorCommand(logger.Discard)
+	if err != nil {
+		t.Fatalf("newIncubatorCommand: %v", err)
+	}
+
+	// The pair must be returned for delivery via the inherited file, and the
+	// argv must name the fd to read it from.
+	if !slices.Contains(forwardedEnv, "GITLAB_API_TOKEN="+secret) {
+		t.Errorf("forwardedEnv = %q, want it to contain the forwarded secret", forwardedEnv)
+	}
+	if !slices.Contains(cmd.Args, "--env-fd=3") {
+		t.Errorf("cmd.Args = %q, want --env-fd=3", cmd.Args)
+	}
+
+	// Neither the secret value nor the key name may appear anywhere on the argv.
+	argv := strings.Join(cmd.Args, "\x00")
+	if strings.Contains(argv, secret) {
+		t.Errorf("secret value leaked onto cmd.Args: %q", cmd.Args)
+	}
+	if strings.Contains(argv, "GITLAB_API_TOKEN") {
+		t.Errorf("forwarded key name leaked onto cmd.Args: %q", cmd.Args)
+	}
+
+	// The privileged child's environment must not contain the forwarded pair.
+	for _, kv := range ss.incubatorEnv() {
+		if strings.Contains(kv, secret) {
+			t.Errorf("forwarded pair present in child environment: %q", kv)
+		}
+	}
+}
+
+// TestIncubatorEnvServerOnly verifies that no client-forwarded variable enters
+// the privileged child's environment, even under a wildcard acceptEnv policy:
+// cmd.Env carries only server-chosen values and the client's TERM/LANG/LC_*
+// (matching OpenSSH's default AcceptEnv). Forwarded pairs travel via an
+// inherited file instead (see forwardedEnvFile), so even names like PATH and
+// GODEBUG that are unsafe in the privileged child can still be delivered to
+// the user's session.
+func TestIncubatorEnvServerOnly(t *testing.T) {
+	ss := newTestSession(t,
+		[]string{"*"},
+		[]string{
+			"PATH=/client/evil",
+			"HOME=/tmp/evil",
+			"GODEBUG=asyncpreemptoff=1",
+			"GIT_TOKEN=fromclient",
+			"TERM=xterm-256color", // accepted via acceptEnvPair, not filterEnv
+		},
+	)
+	_, forwardedEnv, err := ss.newIncubatorCommand(logger.Discard)
+	if err != nil {
+		t.Fatalf("newIncubatorCommand: %v", err)
+	}
+	env := ss.incubatorEnv()
+
+	for _, kv := range env {
+		for _, evil := range []string{"/client/evil", "/tmp/evil", "asyncpreemptoff", "fromclient"} {
+			if strings.Contains(kv, evil) {
+				t.Errorf("client-controlled value %q present in child environment: %q", evil, env)
+			}
+		}
+	}
+	// USER/HOME come from the server; TERM is the one client-controlled value
+	// allowed in (OpenSSH parity).
+	for _, want := range []string{"USER=alice", "HOME=/home/alice", "TERM=xterm-256color"} {
+		if !slices.Contains(env, want) {
+			t.Errorf("%q missing from child environment: %q", want, env)
+		}
+	}
+	// filterEnv accepted the pairs for fd delivery to the user's session.
+	for _, want := range []string{"GIT_TOKEN=fromclient", "GODEBUG=asyncpreemptoff=1", "PATH=/client/evil"} {
+		if !slices.Contains(forwardedEnv, want) {
+			t.Errorf("%q missing from forwardedEnv: %q", want, forwardedEnv)
+		}
+	}
+}
+
+// TestForwardedEnvFileRoundTrip verifies the parent-side payload pipe: the
+// child decodes the JSON-encoded pairs from the read end, and calling it with
+// nothing to forward is an error.
+func TestForwardedEnvFileRoundTrip(t *testing.T) {
+	if f, err := forwardedEnvFile(nil); err == nil {
+		t.Fatalf("forwardedEnvFile(nil) = %v, nil; want error", f)
+	}
+
+	pairs := []string{"GITLAB_API_TOKEN=s3cr3t", "PATH=/client/bin"}
+	f, err := forwardedEnvFile(pairs)
+	if err != nil {
+		t.Fatalf("forwardedEnvFile: %v", err)
+	}
+
+	// Dup the read end (like the child's ExtraFiles fd) and close the parent's copy
+	dup, err := unix.Dup(int(f.Fd()))
+	if err != nil {
+		t.Fatalf("dup: %v", err)
+	}
+	f.Close()
+	ia := incubatorArgs{envFD: dup}
+	if err := ia.loadForwardedEnv(); err != nil {
+		t.Fatalf("loadForwardedEnv: %v", err)
+	}
+	if !slices.Equal(ia.forwardedEnv, pairs) {
+		t.Errorf("forwardedEnv = %q, want %q", ia.forwardedEnv, pairs)
+	}
+
+	// forwardedEnviron applies the pairs to the user's environment and names
+	// them in the "su -w" allowlist, alongside SSH_AUTH_SOCK.
+	env, keys := ia.forwardedEnviron()
+	for _, p := range pairs {
+		if !slices.Contains(env, p) {
+			t.Errorf("pair %q missing from forwardedEnviron env", p)
+		}
+	}
+	for _, k := range []string{"SSH_AUTH_SOCK", "GITLAB_API_TOKEN", "PATH"} {
+		if !slices.Contains(keys, k) {
+			t.Errorf("allowlist missing %q: %q", k, keys)
+		}
+	}
+}
+
+// TestParseIncubatorArgsEnvFD verifies that --env-fd values naming
+// stdin/stdout/stderr are rejected: a legitimate payload fd always comes from
+// an ExtraFiles entry, so it is >= 3.
+func TestParseIncubatorArgsEnvFD(t *testing.T) {
+	if _, err := parseIncubatorArgs([]string{"--groups=1000", "--env-fd=2"}); err == nil {
+		t.Errorf("--env-fd=2: got nil error, want rejection")
+	}
+	for _, args := range [][]string{
+		{"--groups=1000", "--env-fd=3"},
+		{"--groups=1000"}, // unset defaults to -1
+	} {
+		if _, err := parseIncubatorArgs(args); err != nil {
+			t.Errorf("%v: got error %v, want nil", args, err)
+		}
+	}
+}
+
+// TestLoadForwardedEnvLegacyEncodedEnv covers the deprecated --encoded-env
+// compatibility path: an outdated parent tailscaled passes the accepted
+// environment as a quoted JSON argv flag, and the child must still decode it
+// into the user's environment and the "su -w" allowlist.
+func TestLoadForwardedEnvLegacyEncodedEnv(t *testing.T) {
+	pairs := []string{"GITLAB_API_TOKEN=s3cr3t", "OTHER=1"}
+	raw, _ := json.Marshal(pairs)
+	// Exactly what an old parent puts on the argv.
+	ia, err := parseIncubatorArgs([]string{"--groups=1000", "--encoded-env=" + fmt.Sprintf("%q", raw)})
+	if err != nil {
+		t.Fatalf("parseIncubatorArgs: %v", err)
+	}
+	if err := ia.loadForwardedEnv(); err != nil {
+		t.Fatalf("loadForwardedEnv: %v", err)
+	}
+	if !slices.Equal(ia.forwardedEnv, pairs) {
+		t.Errorf("forwardedEnv = %q, want %q", ia.forwardedEnv, pairs)
+	}
+
+	env, keys := ia.forwardedEnviron()
+	for _, p := range pairs {
+		if !slices.Contains(env, p) {
+			t.Errorf("legacy pair %q missing from env", p)
+		}
+	}
+	for _, k := range []string{"SSH_AUTH_SOCK", "GITLAB_API_TOKEN", "OTHER"} {
+		if !slices.Contains(keys, k) {
+			t.Errorf("allowlist missing %q: %q", k, keys)
+		}
+	}
+
+	// Malformed values are rejected.
+	bad := incubatorArgs{encodedEnv: "%q-not-json"}
+	if err := bad.loadForwardedEnv(); err == nil {
+		t.Errorf("malformed encodedEnv: got nil error")
+	}
+}
+
+// TestLoadForwardedEnvSanitizesPairs verifies the child-side integrity check:
+// pairs that would corrupt the "su -w" allowlist (empty or comma-carrying
+// names), truncate on exec (NUL bytes), or lack "=" are dropped when the
+// payload is loaded, even though a new parent would never send them.
+func TestLoadForwardedEnvSanitizesPairs(t *testing.T) {
+	pairs := []string{"GOOD=1", "A,B=x", "C\x00D=2", "E=3\x004", "=x", "malformed", "MY VAR=6", "MY.VAR=7"}
+	raw, _ := json.Marshal(pairs)
+	ia, err := parseIncubatorArgs([]string{"--groups=1000", "--encoded-env=" + fmt.Sprintf("%q", raw)})
+	if err != nil {
+		t.Fatalf("parseIncubatorArgs: %v", err)
+	}
+	if err := ia.loadForwardedEnv(); err != nil {
+		t.Fatalf("loadForwardedEnv: %v", err)
+	}
+	if !slices.Equal(ia.forwardedEnv, []string{"GOOD=1"}) {
+		t.Errorf("forwardedEnv = %q, want [GOOD=1]", ia.forwardedEnv)
+	}
+	_, keys := ia.forwardedEnviron()
+	if !slices.Equal(keys, []string{"SSH_AUTH_SOCK", "GOOD"}) {
+		t.Errorf("allowlist = %q, want [SSH_AUTH_SOCK GOOD]", keys)
+	}
+}

ssh/tailssh/localstate_test.go

@@ -0,0 +1,105 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build (linux && !android) || (darwin && !ios) || freebsd || openbsd
+
+package tailssh
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"net/netip"
+	"strings"
+
+	"tailscale.com/net/tsdial"
+	"tailscale.com/tailcfg"
+	"tailscale.com/types/key"
+	"tailscale.com/types/netmap"
+	"tailscale.com/util/set"
+)
+
+// localState implements ipnLocalBackend for testing.
+type localState struct {
+	sshEnabled   bool
+	matchingRule *tailcfg.SSHRule
+	varRoot      string // if empty, TailscaleVarRoot returns ""
+
+	// caps, if non-empty, are advertised via NetMap().AllCaps. Used to gate
+	// features like NodeAttrSSHEnvironmentVariables in tests.
+	caps []tailcfg.NodeCapability
+
+	// serverActions is a map of the action name to the action.
+	// It is served for paths like https://unused/ssh-action/<action-name>.
+	// The action name is the last part of the action URL.
+	serverActions map[string]*tailcfg.SSHAction
+}
+
+func (ts *localState) Dialer() *tsdial.Dialer {
+	return &tsdial.Dialer{}
+}
+
+func (ts *localState) ShouldRunSSH() bool {
+	return ts.sshEnabled
+}
+
+func (ts *localState) NetMap() *netmap.NetworkMap {
+	var policy *tailcfg.SSHPolicy
+	if ts.matchingRule != nil {
+		policy = &tailcfg.SSHPolicy{
+			Rules: []*tailcfg.SSHRule{
+				ts.matchingRule,
+			},
+		}
+	}
+
+	return &netmap.NetworkMap{
+		SelfNode: (&tailcfg.Node{
+			ID: 1,
+		}).View(),
+		SSHPolicy: policy,
+		AllCaps:   set.SetOf(ts.caps),
+	}
+}
+
+func (ts *localState) NetMapNoPeers() *netmap.NetworkMap { return ts.NetMap() }
+
+func (ts *localState) WhoIs(proto string, ipp netip.AddrPort) (n tailcfg.NodeView, u tailcfg.UserProfile, ok bool) {
+	if proto != "tcp" {
+		return tailcfg.NodeView{}, tailcfg.UserProfile{}, false
+	}
+
+	return (&tailcfg.Node{
+			ID:       2,
+			StableID: "peer-id",
+		}).View(), tailcfg.UserProfile{
+			LoginName: "peer",
+		}, true
+
+}
+
+func (ts *localState) DoNoiseRequest(req *http.Request) (*http.Response, error) {
+	rec := httptest.NewRecorder()
+	k, ok := strings.CutPrefix(req.URL.Path, "/ssh-action/")
+	if !ok {
+		rec.WriteHeader(http.StatusNotFound)
+	}
+	a, ok := ts.serverActions[k]
+	if !ok {
+		rec.WriteHeader(http.StatusNotFound)
+		return rec.Result(), nil
+	}
+	rec.WriteHeader(http.StatusOK)
+	if err := json.NewEncoder(rec).Encode(a); err != nil {
+		return nil, err
+	}
+	return rec.Result(), nil
+}
+
+func (ts *localState) TailscaleVarRoot() string {
+	return ts.varRoot
+}
+
+func (ts *localState) NodeKey() key.NodePublic {
+	return key.NewNode().Public()
+}

ssh/tailssh/tailssh.go

@@ -771,6 +771,35 @@ type sshSession struct {
 	exitHandled chan struct{}
 }
 
+// forwardedEnvChildFD is the fd the incubator child reads the forwarded environment from, sent via
+// --env-fd. It must match the payload file's index in launchProcess's ExtraFiles (fd = 3 + index).
+const forwardedEnvChildFD = 3
+
+// forwardedEnvFile returns the read end of a pipe holding the JSON-encoded forwarded pairs.
+// The read end is passed to the incubator child via exec.Cmd.ExtraFiles to communicate
+// secrets and config; the payload only ever exists in memory, never on any filesystem. A
+// goroutine writes the payload and closes the write end. Caller must close the read end
+// after the child starts.
+func forwardedEnvFile(forwardedEnv []string) (*os.File, error) {
+	if len(forwardedEnv) == 0 {
+		return nil, errors.New("no forwarded environment")
+	}
+	b, err := json.Marshal(forwardedEnv)
+	if err != nil {
+		return nil, fmt.Errorf("marshaling forwarded environment: %w", err)
+	}
+	r, w, err := os.Pipe()
+	if err != nil {
+		return nil, fmt.Errorf("creating forwarded environment pipe: %w", err)
+	}
+	go func() {
+		defer w.Close()
+		// A short read fails the session child-side
+		_, _ = w.Write(b)
+	}()
+	return r, nil
+}
+
 func (ss *sshSession) vlogf(format string, args ...any) {
 	if sshVerboseLogging() {
 		ss.logf(format, args...)

ssh/tailssh/tailssh_integration_test.go

@@ -24,6 +24,7 @@
 	"path/filepath"
 	"runtime"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -36,6 +37,7 @@
 	"tailscale.com/net/tsdial"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
+	"tailscale.com/types/logger"
 	"tailscale.com/types/netmap"
 	"tailscale.com/util/set"
 )
@@ -190,6 +192,65 @@ func TestIntegrationSSH(t *testing.T) {
 	}
 }
 
+// TestIntegrationAcceptEnvSecretNotLogged is the end-to-end regression test
+// for the acceptEnv secret leak: a forwarded secret must reach the session
+// environment, but its value must NOT appear on any process command line
+// nor in the server or incubator logs.
+func TestIntegrationAcceptEnvSecretNotLogged(t *testing.T) {
+	for _, forceV1Behavior := range []bool{false, true} {
+		name := "v2"
+		if forceV1Behavior {
+			name = "v1"
+		}
+		t.Run(name, func(t *testing.T) {
+			canary := fmt.Sprintf("e2e-canary-%d", time.Now().UnixNano())
+
+			var logBuf lockedBuffer
+			addr := testServerWithOpts(t, testServerOpts{
+				username:        "testuser",
+				forceV1Behavior: forceV1Behavior,
+				allowSendEnv:    true,
+				logf:            log.New(&logBuf, "", 0).Printf,
+			})
+			cl, err := ssh.Dial("tcp", addr, &ssh.ClientConfig{
+				HostKeyCallback: ssh.InsecureIgnoreHostKey(),
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
+			t.Cleanup(func() { cl.Close() })
+
+			s := testSessionFor(t, cl, map[string]string{"GIT_E2E_CANARY": canary})
+
+			// Prove delivery, and keep the session alive while we scan /proc
+			if err := s.Start("env; sleep 5"); err != nil {
+				t.Fatalf("unable to start command: %s", err)
+			}
+			if got := s.read(); !strings.Contains(got, "GIT_E2E_CANARY="+canary) {
+				t.Fatalf("forwarded secret not delivered to session env; got %q", got)
+			}
+			if runtime.GOOS == "linux" {
+				if path := findInProcCmdlines(canary); path != "" {
+					t.Errorf("secret value visible on command line at %s", path)
+				}
+			}
+			s.Close()
+
+			// The parent's session logs include the session-start argv log
+			if got := logBuf.String(); strings.Contains(got, canary) {
+				t.Errorf("secret value present in server logs: %q", got)
+			} else if strings.Contains(got, "GIT_E2E_CANARY") {
+				t.Errorf("forwarded key name present in server logs: %q", got)
+			}
+
+			// The incubator child writes its debug log in debugTest mode
+			if b, err := os.ReadFile("/tmp/tailscalessh.log"); err == nil && bytes.Contains(b, []byte(canary)) {
+				t.Errorf("secret value present in incubator debug log")
+			}
+		})
+	}
+}
+
 func TestIntegrationSFTP(t *testing.T) {
 	for _, forceV1Behavior := range []bool{false, true} {
 		name := "v2"
@@ -663,6 +724,36 @@ func fallbackToSUAvailable() bool {
 	return err == nil
 }
 
+// lockedBuffer is a goroutine-safe bytes.Buffer for capturing server logs.
+type lockedBuffer struct {
+	mu  sync.Mutex
+	buf bytes.Buffer
+}
+
+func (b *lockedBuffer) Write(p []byte) (int, error) {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	return b.buf.Write(p)
+}
+
+func (b *lockedBuffer) String() string {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	return b.buf.String()
+}
+
+// findInProcCmdlines returns the path of the first /proc/<pid>/cmdline
+// containing s, or "" if none. Linux only.
+func findInProcCmdlines(s string) string {
+	paths, _ := filepath.Glob("/proc/[0-9]*/cmdline")
+	for _, p := range paths {
+		if b, err := os.ReadFile(p); err == nil && strings.Contains(string(b), s) {
+			return p
+		}
+	}
+	return ""
+}
+
 type session struct {
 	*ssh.Session
 
@@ -779,10 +870,15 @@ type testServerOpts struct {
 	allowSendEnv              bool
 	allowLocalPortForwarding  bool
 	allowRemotePortForwarding bool
+	logf                      logger.Logf // defaults to log.Printf
 }
 
 func testServerWithOpts(t *testing.T, opts testServerOpts) string {
 	t.Helper()
+	logf := opts.logf
+	if logf == nil {
+		logf = log.Printf
+	}
 	srv := &server{
 		lb: &testBackend{
 			localUser:                 opts.username,
@@ -791,7 +887,7 @@ func testServerWithOpts(t *testing.T, opts testServerOpts) string {
 			allowLocalPortForwarding:  opts.allowLocalPortForwarding,
 			allowRemotePortForwarding: opts.allowRemotePortForwarding,
 		},
-		logf:           log.Printf,
+		logf:           logf,
 		tailscaledPath: os.Getenv("TAILSCALED_PATH"),
 		timeNow:        time.Now,
 	}

ssh/tailssh/tailssh_test.go

@@ -37,13 +37,10 @@
 	"golang.org/x/net/http2/h2c"
 	"tailscale.com/cmd/testwrapper/flakytest"
 	"tailscale.com/net/memnet"
-	"tailscale.com/net/tsdial"
 	"tailscale.com/sessionrecording"
 	"tailscale.com/tailcfg"
 	testssh "tailscale.com/tempfork/sshtest/ssh"
 	"tailscale.com/tstest"
-	"tailscale.com/types/key"
-	"tailscale.com/types/netmap"
 	"tailscale.com/util/cibuild"
 	"tailscale.com/util/lineiter"
 	"tailscale.com/util/must"
@@ -373,18 +370,6 @@ func TestEvalSSHPolicy(t *testing.T) {
 	}
 }
 
-// localState implements ipnLocalBackend for testing.
-type localState struct {
-	sshEnabled   bool
-	matchingRule *tailcfg.SSHRule
-	varRoot      string // if empty, TailscaleVarRoot returns ""
-
-	// serverActions is a map of the action name to the action.
-	// It is served for paths like https://unused/ssh-action/<action-name>.
-	// The action name is the last part of the action URL.
-	serverActions map[string]*tailcfg.SSHAction
-}
-
 var currentUser = func() string {
 	// Prefer user.Current because the USER env var is not set in
 	// some environments (e.g. the golang:latest container used by CI).
@@ -427,74 +412,6 @@ func aNonRootUser(t *testing.T) string {
 	return ""
 }
 
-func (ts *localState) Dialer() *tsdial.Dialer {
-	return &tsdial.Dialer{}
-}
-
-func (ts *localState) ShouldRunSSH() bool {
-	return ts.sshEnabled
-}
-
-func (ts *localState) NetMap() *netmap.NetworkMap {
-	var policy *tailcfg.SSHPolicy
-	if ts.matchingRule != nil {
-		policy = &tailcfg.SSHPolicy{
-			Rules: []*tailcfg.SSHRule{
-				ts.matchingRule,
-			},
-		}
-	}
-
-	return &netmap.NetworkMap{
-		SelfNode: (&tailcfg.Node{
-			ID: 1,
-		}).View(),
-		SSHPolicy: policy,
-	}
-}
-
-func (ts *localState) NetMapNoPeers() *netmap.NetworkMap { return ts.NetMap() }
-
-func (ts *localState) WhoIs(proto string, ipp netip.AddrPort) (n tailcfg.NodeView, u tailcfg.UserProfile, ok bool) {
-	if proto != "tcp" {
-		return tailcfg.NodeView{}, tailcfg.UserProfile{}, false
-	}
-
-	return (&tailcfg.Node{
-			ID:       2,
-			StableID: "peer-id",
-		}).View(), tailcfg.UserProfile{
-			LoginName: "peer",
-		}, true
-
-}
-
-func (ts *localState) DoNoiseRequest(req *http.Request) (*http.Response, error) {
-	rec := httptest.NewRecorder()
-	k, ok := strings.CutPrefix(req.URL.Path, "/ssh-action/")
-	if !ok {
-		rec.WriteHeader(http.StatusNotFound)
-	}
-	a, ok := ts.serverActions[k]
-	if !ok {
-		rec.WriteHeader(http.StatusNotFound)
-		return rec.Result(), nil
-	}
-	rec.WriteHeader(http.StatusOK)
-	if err := json.NewEncoder(rec).Encode(a); err != nil {
-		return nil, err
-	}
-	return rec.Result(), nil
-}
-
-func (ts *localState) TailscaleVarRoot() string {
-	return ts.varRoot
-}
-
-func (ts *localState) NodeKey() key.NodePublic {
-	return key.NewNode().Public()
-}
-
 func newSSHRule(action *tailcfg.SSHAction) *tailcfg.SSHRule {
 	return &tailcfg.SSHRule{
 		SSHUsers: map[string]string{