diff --git a/tty.go b/tty.go index c8af5dea..dd3dd62c 100644 --- a/tty.go +++ b/tty.go @@ -16,6 +16,12 @@ import ( "os/exec" ) +// recordingEnvVar is set in the environment of the shell that VHS drives so +// that any `vhs` invocation typed into that shell (e.g. a tape that itself +// runs `vhs`) can detect that it is nested inside an existing recording +// session. See VHS.Start for where this is checked. +const recordingEnvVar = "VHS_RECORDING" + // randomPort returns a random port number that is not in use. func randomPort() int { addr, _ := net.Listen("tcp", ":0") //nolint:gosec,noctx @@ -39,8 +45,7 @@ func buildTtyCmd(port int, shell Shell) *exec.Cmd { args = append(args, shell.Command...) cmd := exec.Command("ttyd", args...) - if shell.Env != nil { - cmd.Env = append(shell.Env, os.Environ()...) - } + env := append(shell.Env, os.Environ()...) + cmd.Env = append(env, recordingEnvVar+"=1") return cmd } diff --git a/vhs.go b/vhs.go index 235e7b42..2924bbef 100644 --- a/vhs.go +++ b/vhs.go @@ -130,6 +130,16 @@ func (vhs *VHS) Start() error { return fmt.Errorf("vhs is already started") } + // Refuse to start a nested recording session. Running `vhs` inside a + // tape that is itself being recorded (e.g. `Type "vhs other.tape"`) + // causes the inner instance to spin up its own ttyd and browser while + // sharing the outer process's environment, which leads to port and + // connection conflicts and, previously, a panic. Nested rendering is + // not supported, so fail fast with a clear error instead. + if os.Getenv(recordingEnvVar) != "" { + return errors.New("vhs is already recording; nested rendering is not supported") + } + port := randomPort() vhs.tty = buildTtyCmd(port, vhs.Options.Shell) if err := vhs.tty.Start(); err != nil { diff --git a/vhs_test.go b/vhs_test.go new file mode 100644 index 00000000..31aebf63 --- /dev/null +++ b/vhs_test.go @@ -0,0 +1,33 @@ +package main + +import ( + "strings" + "sync" + "testing" +) + +// TestStart_NestedRecordingRejected ensures that starting a VHS recording +// while already inside a recording session (as signaled by recordingEnvVar, +// which VHS exports into the shell it drives) fails fast with a clear error +// instead of going on to spin up a second ttyd/browser pair, which is what +// used to cause a panic. See charmbracelet/vhs#761. +func TestStart_NestedRecordingRejected(t *testing.T) { + t.Setenv(recordingEnvVar, "1") + + opts := DefaultVHSOptions() + vhs := VHS{ + Options: &opts, + mutex: &sync.Mutex{}, + } + + err := vhs.Start() + if err == nil { + t.Fatal("expected an error when starting a nested recording, got nil") + } + if !strings.Contains(err.Error(), "already recording") { + t.Fatalf("expected error to mention that vhs is already recording, got: %v", err) + } + if vhs.started { + t.Fatal("vhs.started should remain false when a nested recording is rejected") + } +}