From c68ba768572aa239dd8230983972b47fd6626c8f Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Apr 2026 15:12:45 -0500 Subject: [PATCH 1/3] fix(tunnel): resolve SSH handshake EOF race in container tunnel The Run() method used a select-first-wins pattern that returned on the first result from either the tunnel or container goroutine. When the host tunnel exited first, Run() returned and deferred pipe closures caused the container goroutine's StdioClient to receive EOF mid-handshake, producing the confusing "ssh: handshake failed: EOF" error. The runInContainer() method had a similar issue: the inner tunnel goroutine called defer cancel() which prematurely cancelled the handler's context when the tunnel command exited before the SSH handshake completed. Changes: - Extract awaitGoroutines() to properly coordinate both goroutines: when the tunnel exits first, close pipes to unblock the container goroutine then wait for it to report - Extract classifyTunnelErrors() to surface the tunnel error as root cause when the container gets EOF (instead of the confusing EOF message) - Remove defer cancel() from runInContainer's tunnel goroutine to prevent premature context cancellation of the handler - Extract runContainerTunnel() method and track its error via a channel so StdioClient failures can check if the tunnel was the root cause --- pkg/tunnel/container.go | 149 +++++++++++++++++++++++++++------------- 1 file changed, 100 insertions(+), 49 deletions(-) diff --git a/pkg/tunnel/container.go b/pkg/tunnel/container.go index 6f64f093a..39493e152 100644 --- a/pkg/tunnel/container.go +++ b/pkg/tunnel/container.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "os" + "strings" "time" "github.com/devsy-org/devsy/pkg/agent" @@ -44,6 +45,8 @@ type Handler func(ctx context.Context, containerClient *ssh.Client) error // Run creates an "outer" tunnel to the host to start the SSH server so that the "inner" tunnel can // connect to the container over SSH. +// +//nolint:funlen // goroutine setup with InjectAgent options is inherently verbose func (c *ContainerTunnel) Run( ctx context.Context, handler Handler, @@ -54,11 +57,9 @@ func (c *ContainerTunnel) Run( return nil } - // create context cancelCtx, cancel := context.WithCancel(ctx) defer cancel() - // create readers stdoutReader, stdoutWriter, err := os.Pipe() if err != nil { return err @@ -70,7 +71,6 @@ func (c *ContainerTunnel) Run( defer func() { _ = stdoutWriter.Close() }() defer func() { _ = stdinWriter.Close() }() - // Get the timeout from the context options timeout := config.ParseTimeOption(cfg, config.ContextOptionAgentInjectTimeout) // tunnel to host @@ -108,7 +108,6 @@ func (c *ContainerTunnel) Run( // connect to container containerChan := make(chan error, 1) go func() { - // start ssh client as root / default user sshClient, err := devssh.StdioClient(stdoutReader, stdinWriter, false) if err != nil { containerChan <- fmt.Errorf("create ssh client: %w", err) @@ -120,14 +119,12 @@ func (c *ContainerTunnel) Run( defer log.Debugf("connection to container closed") log.Debugf("connected to host") - // update workspace remotely if c.updateConfigInterval > 0 { go func() { c.updateConfig(cancelCtx, sshClient) }() } - // wait until we are done if err := c.runInContainer(cancelCtx, sshClient, handler, envVars); err != nil { containerChan <- fmt.Errorf("run in container: %w", err) } else { @@ -135,19 +132,55 @@ func (c *ContainerTunnel) Run( } }() - // wait for result + return awaitGoroutines(tunnelChan, containerChan, stdoutWriter, stdinWriter) +} + +// awaitGoroutines waits for both the container and tunnel goroutines to report +// and returns the appropriate error. The container result is the primary result; +// the tunnel result provides root-cause context when the container gets EOF. +func awaitGoroutines( + tunnelChan, containerChan <-chan error, + stdoutWriter, stdinWriter *os.File, +) error { + var tunnelErr, containerErr error + select { - case err := <-containerChan: - if err != nil { - return fmt.Errorf("tunnel to container: %w", err) + case containerErr = <-containerChan: + select { + case tunnelErr = <-tunnelChan: + default: } + case tunnelErr = <-tunnelChan: + // Host tunnel exited before container finished. Close pipes to unblock + // the container goroutine (may be blocked in SSH handshake). + _ = stdoutWriter.Close() + _ = stdinWriter.Close() + containerErr = <-containerChan + } + + return classifyTunnelErrors(tunnelErr, containerErr) +} + +// classifyTunnelErrors determines which error to report when the tunnel and/or +// container goroutines fail. EOF errors from the container are suppressed when +// the tunnel error is the root cause. +func classifyTunnelErrors(tunnelErr, containerErr error) error { + if containerErr == nil { return nil - case err := <-tunnelChan: - if err != nil { - return fmt.Errorf("connect to server: %w", err) + } + if isEOFError(containerErr) { + if tunnelErr != nil { + return fmt.Errorf("connect to server: %w", tunnelErr) } return nil } + return fmt.Errorf("tunnel to container: %w", containerErr) +} + +// isEOFError reports whether an error is caused by an EOF condition, +// including wrapped SSH handshake failures from closed pipes. +func isEOFError(err error) bool { + return errors.Is(err, io.EOF) || strings.Contains(err.Error(), ": EOF") } // updateConfig is called periodically to keep the workspace agent config up to date. @@ -207,13 +240,11 @@ func (c *ContainerTunnel) runInContainer( handler Handler, envVars map[string]string, ) error { - // compress info workspaceInfo, _, err := c.client.AgentInfo(provider.CLIOptions{}) if err != nil { return err } - // create pipes stdoutReader, stdoutWriter, err := os.Pipe() if err != nil { return err @@ -225,53 +256,73 @@ func (c *ContainerTunnel) runInContainer( defer func() { _ = stdoutWriter.Close() }() defer func() { _ = stdinWriter.Close() }() - // create cancel context cancelCtx, cancel := context.WithCancel(ctx) defer cancel() // tunnel to container + tunnelDone := make(chan error, 1) go func() { - writer := log.Writer(log.LevelInfo) - defer func() { _ = writer.Close() }() - defer func() { _ = stdoutWriter.Close() }() - defer cancel() - - log.Debugf("Run container tunnel") - defer log.Debugf("Container tunnel exited") - - command := fmt.Sprintf( - "'%s' agent container-tunnel --workspace-info '%s'", - c.client.AgentPath(), - workspaceInfo, + tunnelDone <- c.runContainerTunnel( + cancelCtx, sshClient, workspaceInfo, stdinReader, stdoutWriter, envVars, ) - if log.DebugEnabled() { - command += " --debug" - } - if err := devssh.Run(cancelCtx, devssh.RunOptions{ - Client: sshClient, - Command: command, - Stdin: stdinReader, - Stdout: stdoutWriter, - Stderr: writer, - EnvVars: envVars, - }); err != nil { - if errors.Is(err, context.Canceled) { - log.Debugf("container tunnel closed: %v", err) - } else { - log.Errorf("error tunneling to container: %v", err) - } - return - } }() - // start ssh client containerClient, err := devssh.StdioClient(stdoutReader, stdinWriter, false) if err != nil { - return err + // StdioClient failed — check if the tunnel goroutine already exited + // with an error. If so, the tunnel error is the root cause. + select { + case tunnelErr := <-tunnelDone: + if tunnelErr != nil { + return tunnelErr + } + default: + } + return fmt.Errorf("ssh client: %w", err) } defer func() { _ = containerClient.Close() }() log.Debugf("connected to container") - // start handler return handler(cancelCtx, containerClient) } + +// runContainerTunnel runs the container tunnel SSH command. It closes +// stdoutWriter on exit so StdioClient gets EOF when the tunnel dies. +// Context-cancelled errors are suppressed (expected during normal shutdown). +// +//nolint:revive // argument-limit: pipe endpoints require separate reader/writer files +func (c *ContainerTunnel) runContainerTunnel( + ctx context.Context, + sshClient *ssh.Client, + workspaceInfo string, + stdinReader, stdoutWriter *os.File, + envVars map[string]string, +) error { + writer := log.Writer(log.LevelInfo) + defer func() { _ = writer.Close() }() + defer func() { _ = stdoutWriter.Close() }() + + log.Debugf("Run container tunnel") + defer log.Debugf("Container tunnel exited") + + command := fmt.Sprintf( + "'%s' agent container-tunnel --workspace-info '%s'", + c.client.AgentPath(), + workspaceInfo, + ) + if log.DebugEnabled() { + command += " --debug" + } + err := devssh.Run(ctx, devssh.RunOptions{ + Client: sshClient, + Command: command, + Stdin: stdinReader, + Stdout: stdoutWriter, + Stderr: writer, + EnvVars: envVars, + }) + if err != nil && ctx.Err() == nil { + return fmt.Errorf("container tunnel: %w", err) + } + return nil +} From 2b05d0361ce7e73b633d035f5bcf08a105aacbe6 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Apr 2026 15:46:34 -0500 Subject: [PATCH 2/3] refactor: extract runHostTunnel to satisfy funlen without nolint Extract the host tunnel goroutine body into runHostTunnel(), bringing Run() under the funlen statement limit and removing the nolint:funlen directive. --- pkg/tunnel/container.go | 66 +++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/pkg/tunnel/container.go b/pkg/tunnel/container.go index 39493e152..1e0cc7296 100644 --- a/pkg/tunnel/container.go +++ b/pkg/tunnel/container.go @@ -45,8 +45,6 @@ type Handler func(ctx context.Context, containerClient *ssh.Client) error // Run creates an "outer" tunnel to the host to start the SSH server so that the "inner" tunnel can // connect to the container over SSH. -// -//nolint:funlen // goroutine setup with InjectAgent options is inherently verbose func (c *ContainerTunnel) Run( ctx context.Context, handler Handler, @@ -76,33 +74,7 @@ func (c *ContainerTunnel) Run( // tunnel to host tunnelChan := make(chan error, 1) go func() { - writer := log.Writer(log.LevelInfo) - defer func() { _ = writer.Close() }() - defer log.Debugf("Tunnel to host closed") - - command := fmt.Sprintf("'%s' helper ssh-server --stdio", c.client.AgentPath()) - if log.DebugEnabled() { - command += " --debug" - } - tunnelChan <- agent.InjectAgent(&agent.InjectOptions{ - Ctx: cancelCtx, - Exec: func(ctx context.Context, command string, stdin io.Reader, stdout io.Writer, stderr io.Writer) error { - return c.client.Command(ctx, client.CommandOptions{ - Command: command, - Stdin: stdin, - Stdout: stdout, - Stderr: stderr, - }) - }, - IsLocal: c.client.AgentLocal(), - RemoteAgentPath: c.client.AgentPath(), - DownloadURL: c.client.AgentURL(), - Command: command, - Stdin: stdinReader, - Stdout: stdoutWriter, - Stderr: writer, - Timeout: timeout, - }) + tunnelChan <- c.runHostTunnel(cancelCtx, stdinReader, stdoutWriter, timeout) }() // connect to container @@ -135,6 +107,42 @@ func (c *ContainerTunnel) Run( return awaitGoroutines(tunnelChan, containerChan, stdoutWriter, stdinWriter) } +// runHostTunnel injects the devsy agent onto the host and starts the SSH server, +// forwarding stdio through the provided pipes. +func (c *ContainerTunnel) runHostTunnel( + ctx context.Context, + stdinReader, stdoutWriter *os.File, + timeout time.Duration, +) error { + writer := log.Writer(log.LevelInfo) + defer func() { _ = writer.Close() }() + defer log.Debugf("Tunnel to host closed") + + command := fmt.Sprintf("'%s' helper ssh-server --stdio", c.client.AgentPath()) + if log.DebugEnabled() { + command += " --debug" + } + return agent.InjectAgent(&agent.InjectOptions{ + Ctx: ctx, + Exec: func(ctx context.Context, command string, stdin io.Reader, stdout io.Writer, stderr io.Writer) error { + return c.client.Command(ctx, client.CommandOptions{ + Command: command, + Stdin: stdin, + Stdout: stdout, + Stderr: stderr, + }) + }, + IsLocal: c.client.AgentLocal(), + RemoteAgentPath: c.client.AgentPath(), + DownloadURL: c.client.AgentURL(), + Command: command, + Stdin: stdinReader, + Stdout: stdoutWriter, + Stderr: writer, + Timeout: timeout, + }) +} + // awaitGoroutines waits for both the container and tunnel goroutines to report // and returns the appropriate error. The container result is the primary result; // the tunnel result provides root-cause context when the container gets EOF. From 3af3fdd1ba681736c96b5914cb39a9462751374d Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Apr 2026 15:56:01 -0500 Subject: [PATCH 3/3] refactor: replace nolint:revive with options struct for runContainerTunnel Bundle the 5 parameters of runContainerTunnel into a containerTunnelOpts struct to satisfy revive's argument-limit (max 4). This avoids suppressing the lint rule while keeping the function signature clean. --- pkg/tunnel/container.go | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/pkg/tunnel/container.go b/pkg/tunnel/container.go index 1e0cc7296..cb11e3440 100644 --- a/pkg/tunnel/container.go +++ b/pkg/tunnel/container.go @@ -270,9 +270,13 @@ func (c *ContainerTunnel) runInContainer( // tunnel to container tunnelDone := make(chan error, 1) go func() { - tunnelDone <- c.runContainerTunnel( - cancelCtx, sshClient, workspaceInfo, stdinReader, stdoutWriter, envVars, - ) + tunnelDone <- c.runContainerTunnel(cancelCtx, containerTunnelOpts{ + sshClient: sshClient, + workspaceInfo: workspaceInfo, + stdinReader: stdinReader, + stdoutWriter: stdoutWriter, + envVars: envVars, + }) }() containerClient, err := devssh.StdioClient(stdoutReader, stdinWriter, false) @@ -294,21 +298,21 @@ func (c *ContainerTunnel) runInContainer( return handler(cancelCtx, containerClient) } +type containerTunnelOpts struct { + sshClient *ssh.Client + workspaceInfo string + stdinReader *os.File + stdoutWriter *os.File + envVars map[string]string +} + // runContainerTunnel runs the container tunnel SSH command. It closes // stdoutWriter on exit so StdioClient gets EOF when the tunnel dies. // Context-cancelled errors are suppressed (expected during normal shutdown). -// -//nolint:revive // argument-limit: pipe endpoints require separate reader/writer files -func (c *ContainerTunnel) runContainerTunnel( - ctx context.Context, - sshClient *ssh.Client, - workspaceInfo string, - stdinReader, stdoutWriter *os.File, - envVars map[string]string, -) error { +func (c *ContainerTunnel) runContainerTunnel(ctx context.Context, opts containerTunnelOpts) error { writer := log.Writer(log.LevelInfo) defer func() { _ = writer.Close() }() - defer func() { _ = stdoutWriter.Close() }() + defer func() { _ = opts.stdoutWriter.Close() }() log.Debugf("Run container tunnel") defer log.Debugf("Container tunnel exited") @@ -316,18 +320,18 @@ func (c *ContainerTunnel) runContainerTunnel( command := fmt.Sprintf( "'%s' agent container-tunnel --workspace-info '%s'", c.client.AgentPath(), - workspaceInfo, + opts.workspaceInfo, ) if log.DebugEnabled() { command += " --debug" } err := devssh.Run(ctx, devssh.RunOptions{ - Client: sshClient, + Client: opts.sshClient, Command: command, - Stdin: stdinReader, - Stdout: stdoutWriter, + Stdin: opts.stdinReader, + Stdout: opts.stdoutWriter, Stderr: writer, - EnvVars: envVars, + EnvVars: opts.envVars, }) if err != nil && ctx.Err() == nil { return fmt.Errorf("container tunnel: %w", err)