detached bf0286e

fix(ports): reject command argument count overflow

commit
bf0286e3c98f5be2d04fbee64ffc07cfdd293ec6
author
brice · · 2026-07-24 06:01 UTC
parent
cac78eb
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
5 files · +73 -12
M internal/app/cmd.go +9 -1
@@ -157,13 +157,21 @@ func runCmdWithDeps(ctx context.Context, invocation cmdInvocation, deps cmdDeps)
157157 return usagef("--self requires running inside a vev pane")
158158 }
159159
160+ payload, err := ports.MarshalCommandRequest(request)
161+ if errors.Is(err, ports.ErrTooManyCommandArgs) {
162+ return usagef("too many command arguments")
163+ }
164+ if err != nil {
165+ return fmt.Errorf("encoding command: %w", err)
166+ }
167+
160168 transport, err := deps.dial(ctx, ipc.SocketDir())
161169 if err != nil {
162170 return &exitCoded{code: 3, err: fmt.Errorf("%w: %w", errDaemonUnreachable, err)}
163171 }
164172 defer func() { _ = transport.Close() }()
165173
166- frame := ports.Frame{Type: ports.MsgCommand, Payload: ports.MarshalCommandRequest(request)}
174+ frame := ports.Frame{Type: ports.MsgCommand, Payload: payload}
167175 if err := transport.Send(frame); err != nil {
168176 return &exitCoded{code: 3, err: fmt.Errorf("sending command: %w", err)}
169177 }
@@ -157,13 +157,21 @@ func runCmdWithDeps(ctx context.Context, invocation cmdInvocation, deps cmdDeps)
157 return usagef("--self requires running inside a vev pane")157 return usagef("--self requires running inside a vev pane")
158 }158 }
159 159
160+ payload, err := ports.MarshalCommandRequest(request)
161+ if errors.Is(err, ports.ErrTooManyCommandArgs) {
162+ return usagef("too many command arguments")
163+ }
164+ if err != nil {
165+ return fmt.Errorf("encoding command: %w", err)
166+ }
167+
160 transport, err := deps.dial(ctx, ipc.SocketDir())168 transport, err := deps.dial(ctx, ipc.SocketDir())
161 if err != nil {169 if err != nil {
162 return &exitCoded{code: 3, err: fmt.Errorf("%w: %w", errDaemonUnreachable, err)}170 return &exitCoded{code: 3, err: fmt.Errorf("%w: %w", errDaemonUnreachable, err)}
163 }171 }
164 defer func() { _ = transport.Close() }()172 defer func() { _ = transport.Close() }()
165 173
166- frame := ports.Frame{Type: ports.MsgCommand, Payload: ports.MarshalCommandRequest(request)}174+ frame := ports.Frame{Type: ports.MsgCommand, Payload: payload}
167 if err := transport.Send(frame); err != nil {175 if err := transport.Send(frame); err != nil {
168 return &exitCoded{code: 3, err: fmt.Errorf("sending command: %w", err)}176 return &exitCoded{code: 3, err: fmt.Errorf("sending command: %w", err)}
169 }177 }
M internal/app/cmd_test.go +23 -0
@@ -4,6 +4,7 @@ import (
44 "context"
55 "errors"
66 "io"
7+ "math"
78 "reflect"
89 "strings"
910 "testing"
@@ -163,6 +164,28 @@ func TestRunCmdExplicitSessionDoesNotUseEnvIDsWithoutSelf(t *testing.T) {
163164 }
164165 }
165166
167+func TestRunCmdRejectsTooManyArgumentsBeforeDialing(t *testing.T) {
168+ dialed := false
169+ err := runCmdWithDeps(context.Background(), cmdInvocation{
170+ slug: "toast",
171+ args: make([]string, math.MaxUint16+1),
172+ }, cmdDeps{
173+ stdout: io.Discard,
174+ getenv: func(string) string { return "" },
175+ dial: func(context.Context, string) (ports.Transport, error) {
176+ dialed = true
177+ return nil, errors.New("must not dial")
178+ },
179+ })
180+
181+ if ExitCode(err) != 2 {
182+ t.Fatalf("error=%v code=%d, want usage error/2", err, ExitCode(err))
183+ }
184+ if dialed {
185+ t.Fatal("oversized command dialed daemon")
186+ }
187+}
188+
166189 func TestRunCmdDoesNotAutostartAndClassifiesDialFailure(t *testing.T) {
167190 dials := 0
168191 dialErr := errors.New("unreachable")
@@ -4,6 +4,7 @@ import (
4 "context"4 "context"
5 "errors"5 "errors"
6 "io"6 "io"
7+ "math"
7 "reflect"8 "reflect"
8 "strings"9 "strings"
9 "testing"10 "testing"
@@ -163,6 +164,28 @@ func TestRunCmdExplicitSessionDoesNotUseEnvIDsWithoutSelf(t *testing.T) {
163 }164 }
164 }165 }
165 166
167+func TestRunCmdRejectsTooManyArgumentsBeforeDialing(t *testing.T) {
168+ dialed := false
169+ err := runCmdWithDeps(context.Background(), cmdInvocation{
170+ slug: "toast",
171+ args: make([]string, math.MaxUint16+1),
172+ }, cmdDeps{
173+ stdout: io.Discard,
174+ getenv: func(string) string { return "" },
175+ dial: func(context.Context, string) (ports.Transport, error) {
176+ dialed = true
177+ return nil, errors.New("must not dial")
178+ },
179+ })
180+
181+ if ExitCode(err) != 2 {
182+ t.Fatalf("error=%v code=%d, want usage error/2", err, ExitCode(err))
183+ }
184+ if dialed {
185+ t.Fatal("oversized command dialed daemon")
186+ }
187+}
188+
166 func TestRunCmdDoesNotAutostartAndClassifiesDialFailure(t *testing.T) {189 func TestRunCmdDoesNotAutostartAndClassifiesDialFailure(t *testing.T) {
167 dials := 0190 dials := 0
168 dialErr := errors.New("unreachable")191 dialErr := errors.New("unreachable")
M internal/ports/wire_command.go +14 -3
@@ -1,6 +1,13 @@
11 package ports
22
3-import "encoding/binary"
3+import (
4+ "encoding/binary"
5+ "errors"
6+ "math"
7+)
8+
9+// ErrTooManyCommandArgs reports that a request exceeds the wire count limit.
10+var ErrTooManyCommandArgs = errors.New("ports: too many command arguments")
411
512 // CommandRequest asks the daemon to run one control command. Version must
613 // stay first so a future payload layout can still be rejected cleanly.
@@ -33,7 +40,11 @@ func PeekCommandVersion(b []byte) (uint16, bool) {
3340 }
3441
3542 // MarshalCommandRequest encodes m into a CommandRequest payload.
36-func MarshalCommandRequest(m CommandRequest) []byte {
43+func MarshalCommandRequest(m CommandRequest) ([]byte, error) {
44+ if len(m.Args) > math.MaxUint16 {
45+ return nil, ErrTooManyCommandArgs
46+ }
47+
3748 w := payloadWriter{}
3849 w.putUint16(m.Version)
3950 if m.Self {
@@ -54,7 +65,7 @@ func MarshalCommandRequest(m CommandRequest) []byte {
5465 } else {
5566 w.putUint8(0)
5667 }
57- return w.b
68+ return w.b, nil
5869 }
5970
6071 // UnmarshalCommandRequest decodes a strict CommandRequest payload.
@@ -1,6 +1,13 @@
1 package ports1 package ports
2 2
3-import "encoding/binary"3+import (
4+ "encoding/binary"
5+ "errors"
6+ "math"
7+)
8+
9+// ErrTooManyCommandArgs reports that a request exceeds the wire count limit.
10+var ErrTooManyCommandArgs = errors.New("ports: too many command arguments")
4 11
5 // CommandRequest asks the daemon to run one control command. Version must12 // CommandRequest asks the daemon to run one control command. Version must
6 // stay first so a future payload layout can still be rejected cleanly.13 // stay first so a future payload layout can still be rejected cleanly.
@@ -33,7 +40,11 @@ func PeekCommandVersion(b []byte) (uint16, bool) {
33 }40 }
34 41
35 // MarshalCommandRequest encodes m into a CommandRequest payload.42 // MarshalCommandRequest encodes m into a CommandRequest payload.
36-func MarshalCommandRequest(m CommandRequest) []byte {43+func MarshalCommandRequest(m CommandRequest) ([]byte, error) {
44+ if len(m.Args) > math.MaxUint16 {
45+ return nil, ErrTooManyCommandArgs
46+ }
47+
37 w := payloadWriter{}48 w := payloadWriter{}
38 w.putUint16(m.Version)49 w.putUint16(m.Version)
39 if m.Self {50 if m.Self {
@@ -54,7 +65,7 @@ func MarshalCommandRequest(m CommandRequest) []byte {
54 } else {65 } else {
55 w.putUint8(0)66 w.putUint8(0)
56 }67 }
57- return w.b68+ return w.b, nil
58 }69 }
59 70
60 // UnmarshalCommandRequest decodes a strict CommandRequest payload.71 // UnmarshalCommandRequest decodes a strict CommandRequest payload.
M internal/ports/wire_test.go +16 -2
@@ -3,6 +3,7 @@ package ports
33 import (
44 "bytes"
55 "fmt"
6+ "math"
67 "reflect"
78 "testing"
89
@@ -498,7 +499,8 @@ func TestCommandRequestGoldenAndRoundTrip(t *testing.T) {
498499
499500 for _, tt := range tests {
500501 t.Run(tt.name, func(t *testing.T) {
501- got := MarshalCommandRequest(tt.msg)
502+ got, err := MarshalCommandRequest(tt.msg)
503+ require.NoError(t, err)
502504 require.Equal(t, tt.want, got)
503505 back, err := UnmarshalCommandRequest(got)
504506 require.NoError(t, err)
@@ -516,8 +518,20 @@ func TestCommandRequestGoldenAndRoundTrip(t *testing.T) {
516518 }
517519 }
518520
521+func TestMarshalCommandRequestRejectsTooManyArguments(t *testing.T) {
522+ payload, err := MarshalCommandRequest(CommandRequest{
523+ Version: ProtocolVersion,
524+ Slug: "toast",
525+ Args: make([]string, math.MaxUint16+1),
526+ })
527+
528+ require.Nil(t, payload)
529+ require.ErrorIs(t, err, ErrTooManyCommandArgs)
530+}
531+
519532 func TestCommandRequestRejectsImpossibleArgumentCount(t *testing.T) {
520- payload := MarshalCommandRequest(CommandRequest{Version: ProtocolVersion, Slug: "toast"})
533+ payload, err := MarshalCommandRequest(CommandRequest{Version: ProtocolVersion, Slug: "toast"})
534+ require.NoError(t, err)
521535 argCountOffset := 2 + 1 + 2 + len("toast")
522536 payload[argCountOffset] = 0xff
523537 payload[argCountOffset+1] = 0xff
@@ -3,6 +3,7 @@ package ports
3 import (3 import (
4 "bytes"4 "bytes"
5 "fmt"5 "fmt"
6+ "math"
6 "reflect"7 "reflect"
7 "testing"8 "testing"
8 9
@@ -498,7 +499,8 @@ func TestCommandRequestGoldenAndRoundTrip(t *testing.T) {
498 499
499 for _, tt := range tests {500 for _, tt := range tests {
500 t.Run(tt.name, func(t *testing.T) {501 t.Run(tt.name, func(t *testing.T) {
501- got := MarshalCommandRequest(tt.msg)502+ got, err := MarshalCommandRequest(tt.msg)
503+ require.NoError(t, err)
502 require.Equal(t, tt.want, got)504 require.Equal(t, tt.want, got)
503 back, err := UnmarshalCommandRequest(got)505 back, err := UnmarshalCommandRequest(got)
504 require.NoError(t, err)506 require.NoError(t, err)
@@ -516,8 +518,20 @@ func TestCommandRequestGoldenAndRoundTrip(t *testing.T) {
516 }518 }
517 }519 }
518 520
521+func TestMarshalCommandRequestRejectsTooManyArguments(t *testing.T) {
522+ payload, err := MarshalCommandRequest(CommandRequest{
523+ Version: ProtocolVersion,
524+ Slug: "toast",
525+ Args: make([]string, math.MaxUint16+1),
526+ })
527+
528+ require.Nil(t, payload)
529+ require.ErrorIs(t, err, ErrTooManyCommandArgs)
530+}
531+
519 func TestCommandRequestRejectsImpossibleArgumentCount(t *testing.T) {532 func TestCommandRequestRejectsImpossibleArgumentCount(t *testing.T) {
520- payload := MarshalCommandRequest(CommandRequest{Version: ProtocolVersion, Slug: "toast"})533+ payload, err := MarshalCommandRequest(CommandRequest{Version: ProtocolVersion, Slug: "toast"})
534+ require.NoError(t, err)
521 argCountOffset := 2 + 1 + 2 + len("toast")535 argCountOffset := 2 + 1 + 2 + len("toast")
522 payload[argCountOffset] = 0xff536 payload[argCountOffset] = 0xff
523 payload[argCountOffset+1] = 0xff537 payload[argCountOffset+1] = 0xff
M internal/usecase/daemon/control_test.go +11 -6
@@ -66,7 +66,9 @@ func TestHandleCommandRejectsVersionBeforeDecodeOrDispatch(t *testing.T) {
6666 func TestHandleConnRoutesCommand(t *testing.T) {
6767 d := newTestDaemon(t, nil, stubClock{})
6868 request := ports.CommandRequest{Version: ports.ProtocolVersion, Slug: "list-sessions"}
69- frame := ports.Frame{Type: ports.MsgCommand, Payload: ports.MarshalCommandRequest(request)}
69+ payload, err := ports.MarshalCommandRequest(request)
70+ require.NoError(t, err)
71+ frame := ports.Frame{Type: ports.MsgCommand, Payload: payload}
7072 tr, sends, _ := newConn(t, frame)
7173
7274 d.handleConn(tr)
@@ -463,7 +465,7 @@ func TestHandleCommandSerializesSelfTargetOnNonActiveTab(t *testing.T) {
463465 sess.mu.Unlock()
464466 original := target.tree.Focus
465467
466- firstFrame := commandFrame(ports.CommandRequest{
468+ firstFrame := commandFrame(t, ports.CommandRequest{
467469 Slug: "split-right", Self: true, TargetSession: "work",
468470 TargetTab: "t_invoking", TargetPane: "p_invoking",
469471 })
@@ -479,7 +481,7 @@ func TestHandleCommandSerializesSelfTargetOnNonActiveTab(t *testing.T) {
479481 t.Fatal("first command did not enter its blocked action")
480482 }
481483
482- secondFrame := commandFrame(ports.CommandRequest{
484+ secondFrame := commandFrame(t, ports.CommandRequest{
483485 Slug: "focus-pane-right", Self: true, TargetSession: "work",
484486 TargetTab: "t_invoking", TargetPane: "p_invoking",
485487 })
@@ -509,16 +511,19 @@ func TestHandleCommandSerializesSelfTargetOnNonActiveTab(t *testing.T) {
509511 require.NotEqual(t, original, target.tree.Focus, "second command must execute after the split creates its right neighbor")
510512 }
511513
512-func commandFrame(request ports.CommandRequest) ports.Frame {
514+func commandFrame(t *testing.T, request ports.CommandRequest) ports.Frame {
515+ t.Helper()
513516 if request.Version == 0 {
514517 request.Version = ports.ProtocolVersion
515518 }
516- return ports.Frame{Type: ports.MsgCommand, Payload: ports.MarshalCommandRequest(request)}
519+ payload, err := ports.MarshalCommandRequest(request)
520+ require.NoError(t, err)
521+ return ports.Frame{Type: ports.MsgCommand, Payload: payload}
517522 }
518523
519524 func sendCommand(t *testing.T, d *Daemon, request ports.CommandRequest) ports.CommandResult {
520525 t.Helper()
521- frame := commandFrame(request)
526+ frame := commandFrame(t, request)
522527 tr, sends, _ := newConn(t, frame)
523528 d.handleCommand(tr, frame)
524529 return awaitCommandResult(t, sends)
@@ -66,7 +66,9 @@ func TestHandleCommandRejectsVersionBeforeDecodeOrDispatch(t *testing.T) {
66 func TestHandleConnRoutesCommand(t *testing.T) {66 func TestHandleConnRoutesCommand(t *testing.T) {
67 d := newTestDaemon(t, nil, stubClock{})67 d := newTestDaemon(t, nil, stubClock{})
68 request := ports.CommandRequest{Version: ports.ProtocolVersion, Slug: "list-sessions"}68 request := ports.CommandRequest{Version: ports.ProtocolVersion, Slug: "list-sessions"}
69- frame := ports.Frame{Type: ports.MsgCommand, Payload: ports.MarshalCommandRequest(request)}69+ payload, err := ports.MarshalCommandRequest(request)
70+ require.NoError(t, err)
71+ frame := ports.Frame{Type: ports.MsgCommand, Payload: payload}
70 tr, sends, _ := newConn(t, frame)72 tr, sends, _ := newConn(t, frame)
71 73
72 d.handleConn(tr)74 d.handleConn(tr)
@@ -463,7 +465,7 @@ func TestHandleCommandSerializesSelfTargetOnNonActiveTab(t *testing.T) {
463 sess.mu.Unlock()465 sess.mu.Unlock()
464 original := target.tree.Focus466 original := target.tree.Focus
465 467
466- firstFrame := commandFrame(ports.CommandRequest{468+ firstFrame := commandFrame(t, ports.CommandRequest{
467 Slug: "split-right", Self: true, TargetSession: "work",469 Slug: "split-right", Self: true, TargetSession: "work",
468 TargetTab: "t_invoking", TargetPane: "p_invoking",470 TargetTab: "t_invoking", TargetPane: "p_invoking",
469 })471 })
@@ -479,7 +481,7 @@ func TestHandleCommandSerializesSelfTargetOnNonActiveTab(t *testing.T) {
479 t.Fatal("first command did not enter its blocked action")481 t.Fatal("first command did not enter its blocked action")
480 }482 }
481 483
482- secondFrame := commandFrame(ports.CommandRequest{484+ secondFrame := commandFrame(t, ports.CommandRequest{
483 Slug: "focus-pane-right", Self: true, TargetSession: "work",485 Slug: "focus-pane-right", Self: true, TargetSession: "work",
484 TargetTab: "t_invoking", TargetPane: "p_invoking",486 TargetTab: "t_invoking", TargetPane: "p_invoking",
485 })487 })
@@ -509,16 +511,19 @@ func TestHandleCommandSerializesSelfTargetOnNonActiveTab(t *testing.T) {
509 require.NotEqual(t, original, target.tree.Focus, "second command must execute after the split creates its right neighbor")511 require.NotEqual(t, original, target.tree.Focus, "second command must execute after the split creates its right neighbor")
510 }512 }
511 513
512-func commandFrame(request ports.CommandRequest) ports.Frame {514+func commandFrame(t *testing.T, request ports.CommandRequest) ports.Frame {
515+ t.Helper()
513 if request.Version == 0 {516 if request.Version == 0 {
514 request.Version = ports.ProtocolVersion517 request.Version = ports.ProtocolVersion
515 }518 }
516- return ports.Frame{Type: ports.MsgCommand, Payload: ports.MarshalCommandRequest(request)}519+ payload, err := ports.MarshalCommandRequest(request)
520+ require.NoError(t, err)
521+ return ports.Frame{Type: ports.MsgCommand, Payload: payload}
517 }522 }
518 523
519 func sendCommand(t *testing.T, d *Daemon, request ports.CommandRequest) ports.CommandResult {524 func sendCommand(t *testing.T, d *Daemon, request ports.CommandRequest) ports.CommandResult {
520 t.Helper()525 t.Helper()
521- frame := commandFrame(request)526+ frame := commandFrame(t, request)
522 tr, sends, _ := newConn(t, frame)527 tr, sends, _ := newConn(t, frame)
523 d.handleCommand(tr, frame)528 d.handleCommand(tr, frame)
524 return awaitCommandResult(t, sends)529 return awaitCommandResult(t, sends)

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 2

Tags 13

↑↓ moveenter openesc close