Skip to content
Merged
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: 10 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,18 @@ generate-wire: $(WIRE)
@echo "Generating wire code..."
cd ./cmd/api && $(WIRE)

# Install proto generators from go.mod versions (pinned via tools.go)
install-proto-tools:
@echo "Installing proto generators from go.mod versions..."
go install google.golang.org/protobuf/cmd/protoc-gen-go
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc

# Generate gRPC code from proto
generate-grpc:
# Run 'make install-proto-tools' first to install generators from go.mod
generate-grpc: install-proto-tools
@echo "Generating gRPC code from proto..."
@echo "Using protoc-gen-go: $$(protoc-gen-go --version)"
@echo "Using protoc-gen-go-grpc: $$(protoc-gen-go-grpc --version)"
protoc --go_out=. --go_opt=paths=source_relative \
--go-grpc_out=. --go-grpc_opt=paths=source_relative \
lib/guest/guest.proto
Expand Down
96 changes: 69 additions & 27 deletions cmd/api/api/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,18 @@ type ExecRequest struct {
TTY bool `json:"tty"`
Env map[string]string `json:"env,omitempty"`
Cwd string `json:"cwd,omitempty"`
Timeout int32 `json:"timeout,omitempty"` // seconds
Timeout int32 `json:"timeout,omitempty"` // seconds
WaitForAgent int32 `json:"wait_for_agent,omitempty"` // seconds to wait for guest agent to be ready
Rows uint32 `json:"rows,omitempty"` // Initial terminal rows (0 = default)
Cols uint32 `json:"cols,omitempty"` // Initial terminal cols (0 = default)
}

// ResizeMessage represents a window resize control message
type ResizeMessage struct {
Resize struct {
Rows uint32 `json:"rows"`
Cols uint32 `json:"cols"`
} `json:"resize"`
}

// ExecHandler handles exec requests via WebSocket for bidirectional streaming
Expand Down Expand Up @@ -108,10 +118,19 @@ func (s *ApiService) ExecHandler(w http.ResponseWriter, r *http.Request) {
"cwd", execReq.Cwd,
"timeout", execReq.Timeout,
"wait_for_agent", execReq.WaitForAgent,
"rows", execReq.Rows,
"cols", execReq.Cols,
)

// Create WebSocket read/writer wrapper
wsConn := &wsReadWriter{ws: ws, ctx: ctx}
// Create resize channel for TTY sessions
var resizeChan chan *guest.WindowSize
if execReq.TTY {
resizeChan = make(chan *guest.WindowSize, 10)
defer close(resizeChan)
}

// Create WebSocket read/writer wrapper that handles resize messages
wsConn := &wsReadWriter{ws: ws, ctx: ctx, resizeChan: resizeChan}

// Create vsock dialer for this hypervisor type
dialer, err := hypervisor.NewVsockDialer(hypervisor.Type(inst.HypervisorType), inst.VsockSocket, inst.VsockCID)
Expand All @@ -133,6 +152,9 @@ func (s *ApiService) ExecHandler(w http.ResponseWriter, r *http.Request) {
Cwd: execReq.Cwd,
Timeout: execReq.Timeout,
WaitForAgent: time.Duration(execReq.WaitForAgent) * time.Second,
Rows: execReq.Rows,
Cols: execReq.Cols,
ResizeChan: resizeChan,
})

duration := time.Since(startTime)
Expand Down Expand Up @@ -167,41 +189,61 @@ func (s *ApiService) ExecHandler(w http.ResponseWriter, r *http.Request) {
}

// wsReadWriter wraps a WebSocket connection to implement io.ReadWriter
// It also handles resize control messages for TTY sessions
type wsReadWriter struct {
ws *websocket.Conn
ctx context.Context
reader io.Reader
mu sync.Mutex
ws *websocket.Conn
ctx context.Context
reader io.Reader
mu sync.Mutex
resizeChan chan<- *guest.WindowSize // Channel to send resize events (nil if not TTY)
}

func (w *wsReadWriter) Read(p []byte) (n int, err error) {
w.mu.Lock()
defer w.mu.Unlock()

// If we have a pending reader, continue reading from it
if w.reader != nil {
n, err = w.reader.Read(p)
if err != io.EOF {
return n, err
for {
// If we have a pending reader, continue reading from it
if w.reader != nil {
n, err = w.reader.Read(p)
if err != io.EOF {
return n, err
}
// EOF means we finished this message, get next one
w.reader = nil
}
// EOF means we finished this message, get next one
w.reader = nil
}

// Read next WebSocket message
messageType, data, err := w.ws.ReadMessage()
if err != nil {
return 0, err
}
// Read next WebSocket message
messageType, data, err := w.ws.ReadMessage()
if err != nil {
return 0, err
}

// Only handle binary and text messages
if messageType != websocket.BinaryMessage && messageType != websocket.TextMessage {
return 0, fmt.Errorf("unexpected message type: %d", messageType)
}
// Handle text messages as potential control messages
if messageType == websocket.TextMessage && w.resizeChan != nil {
// Try to parse as resize message
var resizeMsg ResizeMessage
if err := json.Unmarshal(data, &resizeMsg); err == nil && resizeMsg.Resize.Rows > 0 && resizeMsg.Resize.Cols > 0 {
// Send resize event (non-blocking)
select {
case w.resizeChan <- &guest.WindowSize{Rows: resizeMsg.Resize.Rows, Cols: resizeMsg.Resize.Cols}:
default:
// Channel full, skip
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panic risk when sending to closed resize channel

Medium Severity

The resizeChan is closed via defer (line 129) before the WebSocket is closed (line 75), due to LIFO defer ordering. The wsReadWriter.Read method's select statement at lines 228-232 sends to w.resizeChan, but sending to a closed channel in Go causes a panic—even within a select with a default case. If a resize message arrives after ExecIntoInstance returns but before ExecHandler completes, the orphaned stdin goroutine could panic when processing that message.

Additional Locations (1)

Fix in Cursor Fix in Web

continue // Get next message
}
// Not a resize message, treat as stdin
}

// Create reader for this message
w.reader = bytes.NewReader(data)
return w.reader.Read(p)
// Binary messages and non-resize text messages are stdin data
if messageType != websocket.BinaryMessage && messageType != websocket.TextMessage {
return 0, fmt.Errorf("unexpected message type: %d", messageType)
}

// Create reader for this message
w.reader = bytes.NewReader(data)
return w.reader.Read(p)
}
}

func (w *wsReadWriter) Write(p []byte) (n int, err error) {
Expand Down
1 change: 0 additions & 1 deletion cmd/api/api/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ func TestExecWithDebianMinimal(t *testing.T) {
assert.Contains(t, stdout.String(), "Debian", "Should be running Debian")
assert.Contains(t, stdout.String(), "bookworm", "Should be Debian 12 (bookworm)")
t.Logf("OS: %s", strings.Split(stdout.String(), "\n")[0])

}

// collectTestLogs collects logs from an instance (non-streaming)
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ require (
github.com/ghodss/yaml v1.0.0
github.com/go-chi/chi/v5 v5.2.3
github.com/golang-jwt/jwt/v5 v5.3.0
github.com/golang/protobuf v1.5.4
github.com/google/go-containerregistry v0.20.6
github.com/google/uuid v1.6.0
github.com/google/wire v0.7.0
Expand Down Expand Up @@ -49,6 +48,8 @@ require (
golang.org/x/sync v0.17.0
golang.org/x/sys v0.38.0
google.golang.org/grpc v1.77.0
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.6.0
google.golang.org/protobuf v1.36.10
gvisor.dev/gvisor v0.0.0-20251125014920-fc40e232ff54
)

Expand Down Expand Up @@ -115,7 +116,6 @@ require (
golang.org/x/tools v0.37.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20251022142026-3a174f9686a8 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20251022142026-3a174f9686a8 // indirect
google.golang.org/protobuf v1.36.10 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
gotest.tools/v3 v3.5.2 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,8 @@ google.golang.org/genproto/googleapis/rpc v0.0.0-20251022142026-3a174f9686a8 h1:
google.golang.org/genproto/googleapis/rpc v0.0.0-20251022142026-3a174f9686a8/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk=
google.golang.org/grpc v1.77.0 h1:wVVY6/8cGA6vvffn+wWK5ToddbgdU3d8MNENr4evgXM=
google.golang.org/grpc v1.77.0/go.mod h1:z0BY1iVj0q8E1uSQCjL9cppRj+gnZjzDnzV0dHhrNig=
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.6.0 h1:6Al3kEFFP9VJhRz3DID6quisgPnTeZVr4lep9kkxdPA=
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.6.0/go.mod h1:QLvsjh0OIR0TYBeiu2bkWGTJBUNQ64st52iWj/yA93I=
google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
google.golang.org/protobuf v1.36.10 h1:AYd7cD/uASjIL6Q9LiTjz8JLcrh/88q5UObnmY3aOOE=
Expand Down
40 changes: 35 additions & 5 deletions lib/guest/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,17 +111,23 @@ type ExitStatus struct {
Code int
}

// Note: WindowSize is defined in guest.pb.go (proto-generated)
// Use guest.WindowSize{Rows: N, Cols: M} for resize events

// ExecOptions configures command execution
type ExecOptions struct {
Command []string
Stdin io.Reader
Stdout io.Writer
Stderr io.Writer
TTY bool
Env map[string]string // Environment variables
Cwd string // Working directory (optional)
Timeout int32 // Execution timeout in seconds (0 = no timeout)
WaitForAgent time.Duration // Max time to wait for agent to be ready (0 = no wait, fail immediately)
Env map[string]string // Environment variables
Cwd string // Working directory (optional)
Timeout int32 // Execution timeout in seconds (0 = no timeout)
WaitForAgent time.Duration // Max time to wait for agent to be ready (0 = no wait, fail immediately)
Rows uint32 // Initial terminal rows (0 = default 24)
Cols uint32 // Initial terminal cols (0 = default 80)
ResizeChan <-chan *WindowSize // Optional: channel to receive resize events (pointer to avoid copying mutex)
}

// ExecIntoInstance executes command in instance via vsock using gRPC.
Expand Down Expand Up @@ -203,7 +209,7 @@ func execIntoInstanceOnce(ctx context.Context, dialer hypervisor.VsockDialer, op
// Ensure stream is properly closed when we're done
defer stream.CloseSend()

// Send start request
// Send start request with initial window size
if err := stream.Send(&ExecRequest{
Request: &ExecRequest_Start{
Start: &ExecStart{
Expand All @@ -212,32 +218,56 @@ func execIntoInstanceOnce(ctx context.Context, dialer hypervisor.VsockDialer, op
Env: opts.Env,
Cwd: opts.Cwd,
TimeoutSeconds: opts.Timeout,
Rows: opts.Rows,
Cols: opts.Cols,
},
},
}); err != nil {
return nil, fmt.Errorf("send start request: %w", err)
}

// Mutex to protect concurrent stream.Send/CloseSend calls (gRPC streams are not thread-safe)
var streamMu sync.Mutex

// Handle stdin in background
if opts.Stdin != nil {
go func() {
buf := make([]byte, 32*1024)
for {
n, err := opts.Stdin.Read(buf)
if n > 0 {
streamMu.Lock()
stream.Send(&ExecRequest{
Request: &ExecRequest_Stdin{Stdin: buf[:n]},
})
streamMu.Unlock()
atomic.AddInt64(&bytesSent, int64(n))
}
if err != nil {
streamMu.Lock()
stream.CloseSend()
streamMu.Unlock()
return
}
}
}()
}

// Handle resize events in background (if channel provided)
if opts.ResizeChan != nil {
go func() {
for resize := range opts.ResizeChan {
streamMu.Lock()
stream.Send(&ExecRequest{
Request: &ExecRequest_Resize{
Resize: resize,
},
})
streamMu.Unlock()
}
}()
}

// Receive responses
var totalStdout, totalStderr int
for {
Expand Down
Loading