Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions tty.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
10 changes: 10 additions & 0 deletions vhs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
33 changes: 33 additions & 0 deletions vhs_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
}