ssh/tailssh: record stderr on non-PTY sessions (#21369)

On non-PTY exec sessions stdout was wrapped in the recording writer but
stderr was copied to the client raw, so any command output written to
stderr never appeared in the session recording. PTY sessions were
unaffected because they fold stdout and stderr into a single stream that
already flows through the recording writer.

Wrap stderr in rec.writer too, recording it under the "o" (output)
direction, matching how PTY sessions record combined output. The
asciinema cast format has no separate stderr channel, so "o" is the
correct code for players and tsrecorder to render it.

Strengthen TestSSHRecordingNonInteractive to run a command that writes to
both stdout and stderr and assert both appear in the recorded cast lines.

Updates tailscale/corp#48187

Change-Id: I1dc36073722677a85f2b23b331fd55b8df4d342e
Reported-by: Ben Carman <benthecarman@live.com>
Signed-off-by: Patrick O'Doherty <patrick@tailscale.com>

ssh/tailssh/tailssh.go

@@ -1074,7 +1074,13 @@ func (ss *sshSession) run() {
 	if ss.rdStderr != nil {
 		wg.Go(func() {
 			defer ss.rdStderr.Close()
-			if _, err := io.Copy(ss.Stderr(), ss.rdStderr); err != nil {
+			// Record stderr under the "o" (output) direction: the
+			// asciinema cast format has no separate stderr channel,
+			// and pty sessions already fold stdout+stderr into the
+			// single recorded "o" stream. Wrapping it in rec.writer
+			// (rather than copying raw) is what keeps stderr in the
+			// recording for non-pty sessions.
+			if _, err := io.Copy(rec.writer("o", ss.Stderr()), ss.rdStderr); err != nil {
 				logf("stderr copy: %v", err)
 			}
 		})

ssh/tailssh/tailssh_test.go

@@ -719,7 +719,7 @@ func TestSSHRecordingNonInteractive(t *testing.T) {
 		}
 		defer session.Close()
 		t.Logf("client established session")
-		_, err = session.CombinedOutput("echo Ran echo!")
+		_, err = session.CombinedOutput("sh -c 'echo Ran echo!; echo to-stderr >&2'")
 		if err != nil {
 			t.Errorf("client: %v", err)
 		}
@@ -734,16 +734,54 @@ func TestSSHRecordingNonInteractive(t *testing.T) {
 	case <-time.After(30 * time.Second):
 		t.Fatal("timed out waiting for recording")
 	}
-	var ch sessionrecording.CastHeader
-	if err := json.NewDecoder(bytes.NewReader(recording)).Decode(&ch); err != nil {
-		t.Fatal(err)
-	}
+	ch, recordedOutput := parseRecording(t, recording)
 	if ch.SSHUser != sshUser {
 		t.Errorf("SSHUser = %q; want %q", ch.SSHUser, sshUser)
 	}
-	if ch.Command != "echo Ran echo!" {
-		t.Errorf("Command = %q; want %q", ch.Command, "echo Ran echo!")
+	const wantCommand = "sh -c echo Ran echo!; echo to-stderr >&2"
+	if ch.Command != wantCommand {
+		t.Errorf("Command = %q; want %q", ch.Command, wantCommand)
 	}
+	// Both stdout and stderr must be captured in the recording. On non-PTY
+	// sessions stderr is a separate stream; it must still flow through the
+	// recording writer (recorded under the "o" output direction).
+	if !strings.Contains(recordedOutput, "Ran echo!") {
+		t.Errorf("recording missing stdout output %q; got %q", "Ran echo!", recordedOutput)
+	}
+	if !strings.Contains(recordedOutput, "to-stderr") {
+		t.Errorf("recording missing stderr output %q; got %q", "to-stderr", recordedOutput)
+	}
+}
+
+// parseRecording parses a session recording, returning its CastHeader and the
+// concatenation of all recorded output.
+//
+// The recording is a stream of newline-separated JSON: the CastHeader,
+// followed by asciinema cast lines of the form [time, "o", data].
+func parseRecording(t *testing.T, recording []byte) (sessionrecording.CastHeader, string) {
+	t.Helper()
+	dec := json.NewDecoder(bytes.NewReader(recording))
+	var ch sessionrecording.CastHeader
+	if err := dec.Decode(&ch); err != nil {
+		t.Fatalf("decoding cast header: %v", err)
+	}
+	var output strings.Builder
+	for dec.More() {
+		var line []any
+		if err := dec.Decode(&line); err != nil {
+			t.Fatalf("decoding cast line: %v", err)
+		}
+		if len(line) != 3 {
+			continue
+		}
+		if dir, _ := line[1].(string); dir != "o" {
+			continue
+		}
+		if data, ok := line[2].(string); ok {
+			output.WriteString(data)
+		}
+	}
+	return ch, output.String()
 }
 
 func TestSSHAuthFlow(t *testing.T) {