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
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ go 1.23.0
require (
github.com/bradfitz/gomemcache v0.0.0-20190913173617-a41fca850d0b
github.com/creack/pty v1.1.18
github.com/gorilla/websocket v1.5.3
github.com/hashicorp/go-retryablehttp v0.7.1
github.com/howeyc/fsnotify v0.9.0
github.com/lthibault/jitterbug/v2 v2.2.2
github.com/prometheus/client_golang v1.11.0
github.com/yookoala/gofast v0.6.0
golang.org/x/net v0.38.0
golang.org/x/sys v0.31.0
golang.org/x/term v0.30.0
gopkg.in/tomb.v2 v2.0.0-20161208151619-d5d1b5820637
Expand All @@ -25,6 +25,7 @@ require (
github.com/prometheus/client_model v0.2.0 // indirect
github.com/prometheus/common v0.26.0 // indirect
github.com/prometheus/procfs v0.6.0 // indirect
golang.org/x/net v0.38.0 // indirect
golang.org/x/tools v0.0.0-20200908211811-12e1bf57a112 // indirect
google.golang.org/protobuf v1.26.0-rc.1 // indirect
)
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
github.com/gopherjs/gopherjs v0.0.0-20200217142428-fce0ec30dd00/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg=
github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM=
github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
github.com/hashicorp/go-hclog v0.9.2 h1:CG6TE5H9/JXsFWJCfoIVpKFIkFe6ysEuHirp4DxCsHI=
Expand Down
75 changes: 69 additions & 6 deletions remote/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ import (
"time"
"unicode"

gorilla "github.com/gorilla/websocket"
"github.com/creack/pty"
"github.com/hashicorp/go-retryablehttp"
"github.com/howeyc/fsnotify"
"golang.org/x/net/websocket"
"golang.org/x/sys/unix"
"golang.org/x/term"
)
Expand All @@ -39,6 +39,59 @@ const (
shutdownErrorCode = 4001 // WebSocket close code when a shutdown signal is detected
)

// wsNetConn wraps a *gorilla.Conn and implements the net.Conn interface so that
// the WebSocket connection can be used wherever a plain net.Conn is expected.
// gorilla/websocket is message-oriented, so Read buffers an entire message and
// returns chunks of it on successive calls.
type wsNetConn struct {
conn *gorilla.Conn
mu sync.Mutex // serialises concurrent writes (gorilla requires one writer at a time)
readBuf []byte
}

func newWSNetConn(conn *gorilla.Conn) *wsNetConn {
return &wsNetConn{conn: conn}
}

func (c *wsNetConn) Read(b []byte) (int, error) {
for len(c.readBuf) == 0 {
_, msg, err := c.conn.ReadMessage()
if err != nil {
return 0, err
}
c.readBuf = msg
}
Comment on lines +56 to +63
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

wsNetConn.Read uses ReadMessage, which buffers an entire WebSocket message into memory before returning any bytes. Without a read limit, a peer can send a very large frame/message and force large allocations (potential DoS). Consider setting a maximum message size (e.g., via conn.SetReadLimit) when creating/accepting the WebSocket connection.

Copilot uses AI. Check for mistakes.
n := copy(b, c.readBuf)
c.readBuf = c.readBuf[n:]
return n, nil
Comment on lines +56 to +66
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

wsNetConn.Read currently returns gorilla's close/EOF errors directly. Downstream code in this package checks for io.EOF specifically when the client disconnects; a *websocket.CloseError will be treated as an unexpected error and logged noisily. Consider translating normal close conditions (CloseError / unexpected EOF) to io.EOF (or net.ErrClosed) to preserve the previous net.Conn-like semantics.

Copilot uses AI. Check for mistakes.
}
Comment on lines +42 to +67
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

wsNetConn is presented as a net.Conn implementation, but Read is not concurrency-protected. gorilla/websocket explicitly allows only one concurrent reader; if two goroutines call Read concurrently (which net.Conn permits), this can race/panic. Consider adding a read-side mutex (or otherwise enforcing single-reader usage) to make the adapter safe for concurrent net.Conn use.

Copilot uses AI. Check for mistakes.

func (c *wsNetConn) Write(b []byte) (int, error) {
c.mu.Lock()
defer c.mu.Unlock()
if err := c.conn.WriteMessage(gorilla.BinaryMessage, b); err != nil {
return 0, err
}
return len(b), nil
}

func (c *wsNetConn) writeClose(code int) error {
c.mu.Lock()
defer c.mu.Unlock()
return c.conn.WriteControl(
gorilla.CloseMessage,
gorilla.FormatCloseMessage(code, ""),
time.Now().Add(time.Second),
)
}

func (c *wsNetConn) Close() error { return c.conn.Close() }
func (c *wsNetConn) LocalAddr() net.Addr { return c.conn.LocalAddr() }
func (c *wsNetConn) RemoteAddr() net.Addr { return c.conn.RemoteAddr() }
func (c *wsNetConn) SetDeadline(t time.Time) error { return c.conn.UnderlyingConn().SetDeadline(t) }
func (c *wsNetConn) SetReadDeadline(t time.Time) error { return c.conn.SetReadDeadline(t) }
func (c *wsNetConn) SetWriteDeadline(t time.Time) error { return c.conn.SetWriteDeadline(t) }

var nonUTF8Replacement = []byte(string(unicode.ReplacementChar))

// Holds info related to a specific remote CLI that is running.
Expand Down Expand Up @@ -115,6 +168,10 @@ func ListenForConnections() {
listenAddr := "0.0.0.0:22122"

if remoteConfig.useWebsockets {
upgrader := &gorilla.Upgrader{
// Allow all origins since this is an internal service, not a browser-facing endpoint.
CheckOrigin: func(r *http.Request) bool { return true },
}
s := &http.Server{
Addr: listenAddr,
ConnContext: func(ctx context.Context, c net.Conn) context.Context {
Expand All @@ -125,9 +182,15 @@ func ListenForConnections() {
}
return ctx
},
Handler: websocket.Handler(func(wsConn *websocket.Conn) {
log.Printf("websocket connection from %s\n", wsConn.RemoteAddr().String())
authConn(wsConn)
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
wsConn, err := upgrader.Upgrade(w, r, nil)
if err != nil {
log.Printf("websocket upgrade error: %v\n", err)
return
}
netConn := newWSNetConn(wsConn)
log.Printf("websocket connection from %s\n", netConn.RemoteAddr().String())
authConn(netConn)
}),
}
log.Printf("Listening for websocket protocol on %q...", listenAddr)
Expand Down Expand Up @@ -408,9 +471,9 @@ func processShutdown(conn net.Conn, wpcli *wpCLIProcess) {

wpcli.padlock.Lock()

wsConn, ok := conn.(*websocket.Conn)
wsConn, ok := conn.(*wsNetConn)
if ok {
wsConn.WriteClose(shutdownErrorCode)
wsConn.writeClose(shutdownErrorCode)
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

processShutdown ignores the error returned by writeClose. If the close control frame fails to send (e.g., broken connection), it may be useful to log the error for diagnosis during shutdown.

Suggested change
wsConn.writeClose(shutdownErrorCode)
if err := wsConn.writeClose(shutdownErrorCode); err != nil {
log.Printf("remote: failed to send websocket close frame during shutdown. GUID: %s, err: %v\n", wpcli.GUID, err)
}

Copilot uses AI. Check for mistakes.
}

conn.Close()
Expand Down
Loading
Loading