fix(protocol): harden boundary validation
- commit
- 7295b7ba9fcf9a25b8d5093461ac14193c47f80f
- author
- brice · · 2026-08-28 16:08 UTC
- parent
- 7052600
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
14 files · +64 -33
M internal/adapters/sessionwire/server.go +1 -1
| @@ -50,7 +50,7 @@ func (c *serverConnection) ReceiveClient() (protocol.ClientMessage, error) { | |||
| 50 | 50 | case wire.MsgCommand: | |
| 51 | 51 | failure.Kind = protocol.DecodeMessageCommand | |
| 52 | 52 | failure.Version, _ = wire.PeekCommandVersion(frame.Payload) | |
| 53 | - | failure.RequestID, _ = wire.PeekCommandRequestID(frame.Payload) | |
| 53 | + | failure.RequestID, failure.HasRequestID = wire.PeekCommandRequestID(frame.Payload) | |
| 54 | 54 | case wire.MsgKill: | |
| 55 | 55 | failure.Kind = protocol.DecodeMessageKill | |
| 56 | 56 | case wire.MsgRemotePreviewRequest: | |
| @@ -50,7 +50,7 @@ func (c *serverConnection) ReceiveClient() (protocol.ClientMessage, error) { | |||||
| 50 | case wire.MsgCommand: | 50 | case wire.MsgCommand: | ||
| 51 | failure.Kind = protocol.DecodeMessageCommand | 51 | failure.Kind = protocol.DecodeMessageCommand | ||
| 52 | failure.Version, _ = wire.PeekCommandVersion(frame.Payload) | 52 | failure.Version, _ = wire.PeekCommandVersion(frame.Payload) | ||
| 53 | - | failure.RequestID, _ = wire.PeekCommandRequestID(frame.Payload) | 53 | + | failure.RequestID, failure.HasRequestID = wire.PeekCommandRequestID(frame.Payload) |
| 54 | case wire.MsgKill: | 54 | case wire.MsgKill: | ||
| 55 | failure.Kind = protocol.DecodeMessageKill | 55 | failure.Kind = protocol.DecodeMessageKill | ||
| 56 | case wire.MsgRemotePreviewRequest: | 56 | case wire.MsgRemotePreviewRequest: | ||
M internal/adapters/sessionwire/server_test.go +9 -6
| @@ -191,14 +191,16 @@ func TestServerConnectionPreservesPayloadAgnosticClientControls(t *testing.T) { | |||
| 191 | 191 | ||
| 192 | 192 | func TestServerConnectionClassifiesDecodeFailures(t *testing.T) { | |
| 193 | 193 | tests := []struct { | |
| 194 | - | name string | |
| 195 | - | frame wire.Frame | |
| 196 | - | category protocol.DecodeCategory | |
| 197 | - | version uint16 | |
| 198 | - | request uint64 | |
| 194 | + | name string | |
| 195 | + | frame wire.Frame | |
| 196 | + | category protocol.DecodeCategory | |
| 197 | + | version uint16 | |
| 198 | + | request uint64 | |
| 199 | + | hasRequest bool | |
| 199 | 200 | }{ | |
| 200 | 201 | {name: "malformed hello", frame: wire.Frame{Type: wire.MsgHello, Payload: []byte{0, 38}}, category: protocol.DecodeMalformed, version: 38}, | |
| 201 | - | {name: "malformed command", frame: wire.Frame{Type: wire.MsgCommand, Payload: []byte{0, 37, 0, 0, 0, 0, 0, 0, 0, 9}}, category: protocol.DecodeMalformed, version: 37, request: 9}, | |
| 202 | + | {name: "truncated command", frame: wire.Frame{Type: wire.MsgCommand, Payload: []byte{0}}, category: protocol.DecodeMalformed}, | |
| 203 | + | {name: "malformed command", frame: wire.Frame{Type: wire.MsgCommand, Payload: []byte{0, 37, 0, 0, 0, 0, 0, 0, 0, 9}}, category: protocol.DecodeMalformed, version: 37, request: 9, hasRequest: true}, | |
| 202 | 204 | {name: "wrong direction", frame: wire.Frame{Type: wire.MsgOutput}, category: protocol.DecodeWrongDirection}, | |
| 203 | 205 | {name: "unknown", frame: wire.Frame{Type: 255}, category: protocol.DecodeUnknownType}, | |
| 204 | 206 | } | |
| @@ -210,6 +212,7 @@ func TestServerConnectionClassifiesDecodeFailures(t *testing.T) { | |||
| 210 | 212 | require.Equal(t, tt.category, failure.Category) | |
| 211 | 213 | require.Equal(t, tt.version, failure.Version) | |
| 212 | 214 | require.Equal(t, tt.request, failure.RequestID) | |
| 215 | + | require.Equal(t, tt.hasRequest, failure.HasRequestID) | |
| 213 | 216 | }) | |
| 214 | 217 | } | |
| 215 | 218 | } | |
| @@ -191,14 +191,16 @@ func TestServerConnectionPreservesPayloadAgnosticClientControls(t *testing.T) { | |||||
| 191 | 191 | ||||
| 192 | func TestServerConnectionClassifiesDecodeFailures(t *testing.T) { | 192 | func TestServerConnectionClassifiesDecodeFailures(t *testing.T) { | ||
| 193 | tests := []struct { | 193 | tests := []struct { | ||
| 194 | - | name string | 194 | + | name string |
| 195 | - | frame wire.Frame | 195 | + | frame wire.Frame |
| 196 | - | category protocol.DecodeCategory | 196 | + | category protocol.DecodeCategory |
| 197 | - | version uint16 | 197 | + | version uint16 |
| 198 | - | request uint64 | 198 | + | request uint64 |
| 199 | + | hasRequest bool | |||
| 199 | }{ | 200 | }{ | ||
| 200 | {name: "malformed hello", frame: wire.Frame{Type: wire.MsgHello, Payload: []byte{0, 38}}, category: protocol.DecodeMalformed, version: 38}, | 201 | {name: "malformed hello", frame: wire.Frame{Type: wire.MsgHello, Payload: []byte{0, 38}}, category: protocol.DecodeMalformed, version: 38}, | ||
| 201 | - | {name: "malformed command", frame: wire.Frame{Type: wire.MsgCommand, Payload: []byte{0, 37, 0, 0, 0, 0, 0, 0, 0, 9}}, category: protocol.DecodeMalformed, version: 37, request: 9}, | 202 | + | {name: "truncated command", frame: wire.Frame{Type: wire.MsgCommand, Payload: []byte{0}}, category: protocol.DecodeMalformed}, |
| 203 | + | {name: "malformed command", frame: wire.Frame{Type: wire.MsgCommand, Payload: []byte{0, 37, 0, 0, 0, 0, 0, 0, 0, 9}}, category: protocol.DecodeMalformed, version: 37, request: 9, hasRequest: true}, | |||
| 202 | {name: "wrong direction", frame: wire.Frame{Type: wire.MsgOutput}, category: protocol.DecodeWrongDirection}, | 204 | {name: "wrong direction", frame: wire.Frame{Type: wire.MsgOutput}, category: protocol.DecodeWrongDirection}, | ||
| 203 | {name: "unknown", frame: wire.Frame{Type: 255}, category: protocol.DecodeUnknownType}, | 205 | {name: "unknown", frame: wire.Frame{Type: 255}, category: protocol.DecodeUnknownType}, | ||
| 204 | } | 206 | } | ||
| @@ -210,6 +212,7 @@ func TestServerConnectionClassifiesDecodeFailures(t *testing.T) { | |||||
| 210 | require.Equal(t, tt.category, failure.Category) | 212 | require.Equal(t, tt.category, failure.Category) | ||
| 211 | require.Equal(t, tt.version, failure.Version) | 213 | require.Equal(t, tt.version, failure.Version) | ||
| 212 | require.Equal(t, tt.request, failure.RequestID) | 214 | require.Equal(t, tt.request, failure.RequestID) | ||
| 215 | + | require.Equal(t, tt.hasRequest, failure.HasRequestID) | |||
| 213 | }) | 216 | }) | ||
| 214 | } | 217 | } | ||
| 215 | } | 218 | } | ||
M internal/app/daemon_integration_test.go +9 -6
| @@ -149,22 +149,21 @@ func TestIntegration_MalformedCommandPreservesVersionAndRequestID(t *testing.T) | |||
| 149 | 149 | tests := []struct { | |
| 150 | 150 | name string | |
| 151 | 151 | payload []byte | |
| 152 | - | want protocol.CommandResult | |
| 152 | + | want *protocol.CommandResult | |
| 153 | 153 | }{ | |
| 154 | 154 | { | |
| 155 | 155 | name: "incompatible version", | |
| 156 | 156 | payload: incompatibleVersion, | |
| 157 | - | want: protocol.CommandResult{RequestID: 42, Code: protocol.ErrVersionMismatch, Text: "protocol version mismatch"}, | |
| 157 | + | want: &protocol.CommandResult{RequestID: 42, Code: protocol.ErrVersionMismatch, Text: "protocol version mismatch"}, | |
| 158 | 158 | }, | |
| 159 | 159 | { | |
| 160 | 160 | name: "truncated version prefix", | |
| 161 | 161 | payload: []byte{0}, | |
| 162 | - | want: protocol.CommandResult{Code: protocol.ErrInternal, Text: "malformed command request"}, | |
| 163 | 162 | }, | |
| 164 | 163 | { | |
| 165 | 164 | name: "trailing garbage", | |
| 166 | 165 | payload: append(append([]byte(nil), valid...), 0xff), | |
| 167 | - | want: protocol.CommandResult{RequestID: 43, Code: protocol.ErrInternal, Text: "malformed command request"}, | |
| 166 | + | want: &protocol.CommandResult{RequestID: 43, Code: protocol.ErrInternal, Text: "malformed command request"}, | |
| 168 | 167 | }, | |
| 169 | 168 | } | |
| 170 | 169 | for _, tt := range tests { | |
| @@ -176,12 +175,16 @@ func TestIntegration_MalformedCommandPreservesVersionAndRequestID(t *testing.T) | |||
| 176 | 175 | ||
| 177 | 176 | require.NoError(t, tr.Send(wire.Frame{Type: wire.MsgCommand, Payload: tt.payload})) | |
| 178 | 177 | frame, err := tr.Recv() | |
| 178 | + | if tt.want == nil { | |
| 179 | + | require.ErrorIs(t, err, io.EOF) | |
| 180 | + | return | |
| 181 | + | } | |
| 179 | 182 | require.NoError(t, err) | |
| 180 | 183 | require.Equal(t, wire.MsgCommandResult, frame.Type) | |
| 181 | - | require.Equal(t, wire.MarshalCommandResult(tt.want), frame.Payload) | |
| 184 | + | require.Equal(t, wire.MarshalCommandResult(*tt.want), frame.Payload) | |
| 182 | 185 | result, err := wire.UnmarshalCommandResult(frame.Payload) | |
| 183 | 186 | require.NoError(t, err) | |
| 184 | - | require.Equal(t, tt.want, result) | |
| 187 | + | require.Equal(t, *tt.want, result) | |
| 185 | 188 | }) | |
| 186 | 189 | } | |
| 187 | 190 | } | |
| @@ -149,22 +149,21 @@ func TestIntegration_MalformedCommandPreservesVersionAndRequestID(t *testing.T) | |||||
| 149 | tests := []struct { | 149 | tests := []struct { | ||
| 150 | name string | 150 | name string | ||
| 151 | payload []byte | 151 | payload []byte | ||
| 152 | - | want protocol.CommandResult | 152 | + | want *protocol.CommandResult |
| 153 | }{ | 153 | }{ | ||
| 154 | { | 154 | { | ||
| 155 | name: "incompatible version", | 155 | name: "incompatible version", | ||
| 156 | payload: incompatibleVersion, | 156 | payload: incompatibleVersion, | ||
| 157 | - | want: protocol.CommandResult{RequestID: 42, Code: protocol.ErrVersionMismatch, Text: "protocol version mismatch"}, | 157 | + | want: &protocol.CommandResult{RequestID: 42, Code: protocol.ErrVersionMismatch, Text: "protocol version mismatch"}, |
| 158 | }, | 158 | }, | ||
| 159 | { | 159 | { | ||
| 160 | name: "truncated version prefix", | 160 | name: "truncated version prefix", | ||
| 161 | payload: []byte{0}, | 161 | payload: []byte{0}, | ||
| 162 | - | want: protocol.CommandResult{Code: protocol.ErrInternal, Text: "malformed command request"}, | |||
| 163 | }, | 162 | }, | ||
| 164 | { | 163 | { | ||
| 165 | name: "trailing garbage", | 164 | name: "trailing garbage", | ||
| 166 | payload: append(append([]byte(nil), valid...), 0xff), | 165 | payload: append(append([]byte(nil), valid...), 0xff), | ||
| 167 | - | want: protocol.CommandResult{RequestID: 43, Code: protocol.ErrInternal, Text: "malformed command request"}, | 166 | + | want: &protocol.CommandResult{RequestID: 43, Code: protocol.ErrInternal, Text: "malformed command request"}, |
| 168 | }, | 167 | }, | ||
| 169 | } | 168 | } | ||
| 170 | for _, tt := range tests { | 169 | for _, tt := range tests { | ||
| @@ -176,12 +175,16 @@ func TestIntegration_MalformedCommandPreservesVersionAndRequestID(t *testing.T) | |||||
| 176 | 175 | ||||
| 177 | require.NoError(t, tr.Send(wire.Frame{Type: wire.MsgCommand, Payload: tt.payload})) | 176 | require.NoError(t, tr.Send(wire.Frame{Type: wire.MsgCommand, Payload: tt.payload})) | ||
| 178 | frame, err := tr.Recv() | 177 | frame, err := tr.Recv() | ||
| 178 | + | if tt.want == nil { | |||
| 179 | + | require.ErrorIs(t, err, io.EOF) | |||
| 180 | + | return | |||
| 181 | + | } | |||
| 179 | require.NoError(t, err) | 182 | require.NoError(t, err) | ||
| 180 | require.Equal(t, wire.MsgCommandResult, frame.Type) | 183 | require.Equal(t, wire.MsgCommandResult, frame.Type) | ||
| 181 | - | require.Equal(t, wire.MarshalCommandResult(tt.want), frame.Payload) | 184 | + | require.Equal(t, wire.MarshalCommandResult(*tt.want), frame.Payload) |
| 182 | result, err := wire.UnmarshalCommandResult(frame.Payload) | 185 | result, err := wire.UnmarshalCommandResult(frame.Payload) | ||
| 183 | require.NoError(t, err) | 186 | require.NoError(t, err) | ||
| 184 | - | require.Equal(t, tt.want, result) | 187 | + | require.Equal(t, *tt.want, result) |
| 185 | }) | 188 | }) | ||
| 186 | } | 189 | } | ||
| 187 | } | 190 | } | ||
M internal/app/run.go +1 -1
| @@ -1062,7 +1062,7 @@ func detachedLocalHello(name, cwd string) protocol.Hello { | |||
| 1062 | 1062 | Size: domain.Size{Cols: 80, Rows: 24}, | |
| 1063 | 1063 | TermEnv: termEnv, | |
| 1064 | 1064 | Cwd: cwd, | |
| 1065 | - | TrueColor: client.DetectTrueColor(termEnv, os.Getenv("COLORTERM")), | |
| 1065 | + | TrueColor: client.DetectTrueColor(termEnv, os.Getenv("COLORTERM"), os.Environ()), | |
| 1066 | 1066 | Env: os.Environ(), | |
| 1067 | 1067 | } | |
| 1068 | 1068 | } | |
| @@ -1062,7 +1062,7 @@ func detachedLocalHello(name, cwd string) protocol.Hello { | |||||
| 1062 | Size: domain.Size{Cols: 80, Rows: 24}, | 1062 | Size: domain.Size{Cols: 80, Rows: 24}, | ||
| 1063 | TermEnv: termEnv, | 1063 | TermEnv: termEnv, | ||
| 1064 | Cwd: cwd, | 1064 | Cwd: cwd, | ||
| 1065 | - | TrueColor: client.DetectTrueColor(termEnv, os.Getenv("COLORTERM")), | 1065 | + | TrueColor: client.DetectTrueColor(termEnv, os.Getenv("COLORTERM"), os.Environ()), |
| 1066 | Env: os.Environ(), | 1066 | Env: os.Environ(), | ||
| 1067 | } | 1067 | } | ||
| 1068 | } | 1068 | } | ||
M internal/domain/terminalcap/capabilities.go +7 -3
| @@ -94,7 +94,11 @@ func environmentValues(env []string) map[string]string { | |||
| 94 | 94 | return values | |
| 95 | 95 | } | |
| 96 | 96 | ||
| 97 | - | // DetectTrueColor reports whether TERM/COLORTERM advertise direct color support. | |
| 98 | - | func DetectTrueColor(termEnv, colorTerm string) bool { | |
| 99 | - | return Detect([]string{"TERM=" + termEnv, "COLORTERM=" + colorTerm}).TrueColor() | |
| 97 | + | // DetectTrueColor reports whether the supplied terminal environment advertises | |
| 98 | + | // direct color support. Explicit TERM/COLORTERM values override env entries. | |
| 99 | + | func DetectTrueColor(termEnv, colorTerm string, env []string) bool { | |
| 100 | + | detectionEnv := make([]string, 0, len(env)+2) | |
| 101 | + | detectionEnv = append(detectionEnv, env...) | |
| 102 | + | detectionEnv = append(detectionEnv, "TERM="+termEnv, "COLORTERM="+colorTerm) | |
| 103 | + | return Detect(detectionEnv).TrueColor() | |
| 100 | 104 | } | |
| @@ -94,7 +94,11 @@ func environmentValues(env []string) map[string]string { | |||||
| 94 | return values | 94 | return values | ||
| 95 | } | 95 | } | ||
| 96 | 96 | ||||
| 97 | - | // DetectTrueColor reports whether TERM/COLORTERM advertise direct color support. | 97 | + | // DetectTrueColor reports whether the supplied terminal environment advertises |
| 98 | - | func DetectTrueColor(termEnv, colorTerm string) bool { | 98 | + | // direct color support. Explicit TERM/COLORTERM values override env entries. |
| 99 | - | return Detect([]string{"TERM=" + termEnv, "COLORTERM=" + colorTerm}).TrueColor() | 99 | + | func DetectTrueColor(termEnv, colorTerm string, env []string) bool { |
| 100 | + | detectionEnv := make([]string, 0, len(env)+2) | |||
| 101 | + | detectionEnv = append(detectionEnv, env...) | |||
| 102 | + | detectionEnv = append(detectionEnv, "TERM="+termEnv, "COLORTERM="+colorTerm) | |||
| 103 | + | return Detect(detectionEnv).TrueColor() | |||
| 100 | } | 104 | } | ||
M internal/domain/terminalcap/capabilities_test.go +4 -0
| @@ -6,6 +6,10 @@ import ( | |||
| 6 | 6 | "github.com/stretchr/testify/require" | |
| 7 | 7 | ) | |
| 8 | 8 | ||
| 9 | + | func TestDetectTrueColorPreservesKittyEnvironment(t *testing.T) { | |
| 10 | + | require.True(t, DetectTrueColor("xterm-kitty", "", []string{"KITTY_WINDOW_ID=1"})) | |
| 11 | + | } | |
| 12 | + | ||
| 9 | 13 | func TestDetect(t *testing.T) { | |
| 10 | 14 | tests := []struct { | |
| 11 | 15 | name string | |
| @@ -6,6 +6,10 @@ import ( | |||||
| 6 | "github.com/stretchr/testify/require" | 6 | "github.com/stretchr/testify/require" | ||
| 7 | ) | 7 | ) | ||
| 8 | 8 | ||||
| 9 | + | func TestDetectTrueColorPreservesKittyEnvironment(t *testing.T) { | |||
| 10 | + | require.True(t, DetectTrueColor("xterm-kitty", "", []string{"KITTY_WINDOW_ID=1"})) | |||
| 11 | + | } | |||
| 12 | + | ||||
| 9 | func TestDetect(t *testing.T) { | 13 | func TestDetect(t *testing.T) { | ||
| 10 | tests := []struct { | 14 | tests := []struct { | ||
| 11 | name string | 15 | name string | ||
M internal/protocol/catalogue/catalogue.go +5 -5
| @@ -110,10 +110,10 @@ func (e *RemoteCatalogVersionMismatchError) Error() string { | |||
| 110 | 110 | } | |
| 111 | 111 | ||
| 112 | 112 | var ( | |
| 113 | - | ErrInvalidRemoteCatalog = errors.New("ports: invalid remote catalog") | |
| 114 | - | ErrRemoteCatalogTooLarge = errors.New("ports: remote catalog exceeds size limit") | |
| 115 | - | ErrRemoteCatalogUnknownState = errors.New("ports: remote catalog has unknown state") | |
| 116 | - | ErrRemoteCatalogInvalidReason = errors.New("ports: remote catalog has unknown reason") | |
| 113 | + | ErrInvalidRemoteCatalog = errors.New("catalogue: invalid remote catalog") | |
| 114 | + | ErrRemoteCatalogTooLarge = errors.New("catalogue: remote catalog exceeds size limit") | |
| 115 | + | ErrRemoteCatalogUnknownState = errors.New("catalogue: remote catalog has unknown state") | |
| 116 | + | ErrRemoteCatalogInvalidReason = errors.New("catalogue: remote catalog has unknown reason") | |
| 117 | 117 | ) | |
| 118 | 118 | ||
| 119 | 119 | // ValidateRemoteCatalog applies the exact current schema and all bounds before use. | |
| @@ -223,7 +223,7 @@ func ValidateRemoteCatalogCacheEntries(entries []RemoteCatalogCacheEntry) error | |||
| 223 | 223 | } | |
| 224 | 224 | seenHosts := make(map[string]struct{}, len(entries)) | |
| 225 | 225 | for _, entry := range entries { | |
| 226 | - | if err := domain.ValidateRemoteHostTarget(entry.Host); err != nil || len(entry.Host) > RemoteCatalogMaxNameBytes { | |
| 226 | + | if !validText(entry.Host) || len(entry.Host) > RemoteCatalogMaxNameBytes || domain.ValidateRemoteHostTarget(entry.Host) != nil { | |
| 227 | 227 | return fmt.Errorf("%w: invalid host", ErrInvalidRemoteCatalog) | |
| 228 | 228 | } | |
| 229 | 229 | if entry.FetchedAt.IsZero() || entry.FetchedAt.UnixNano() <= 0 { | |
| @@ -110,10 +110,10 @@ func (e *RemoteCatalogVersionMismatchError) Error() string { | |||||
| 110 | } | 110 | } | ||
| 111 | 111 | ||||
| 112 | var ( | 112 | var ( | ||
| 113 | - | ErrInvalidRemoteCatalog = errors.New("ports: invalid remote catalog") | 113 | + | ErrInvalidRemoteCatalog = errors.New("catalogue: invalid remote catalog") |
| 114 | - | ErrRemoteCatalogTooLarge = errors.New("ports: remote catalog exceeds size limit") | 114 | + | ErrRemoteCatalogTooLarge = errors.New("catalogue: remote catalog exceeds size limit") |
| 115 | - | ErrRemoteCatalogUnknownState = errors.New("ports: remote catalog has unknown state") | 115 | + | ErrRemoteCatalogUnknownState = errors.New("catalogue: remote catalog has unknown state") |
| 116 | - | ErrRemoteCatalogInvalidReason = errors.New("ports: remote catalog has unknown reason") | 116 | + | ErrRemoteCatalogInvalidReason = errors.New("catalogue: remote catalog has unknown reason") |
| 117 | ) | 117 | ) | ||
| 118 | 118 | ||||
| 119 | // ValidateRemoteCatalog applies the exact current schema and all bounds before use. | 119 | // ValidateRemoteCatalog applies the exact current schema and all bounds before use. | ||
| @@ -223,7 +223,7 @@ func ValidateRemoteCatalogCacheEntries(entries []RemoteCatalogCacheEntry) error | |||||
| 223 | } | 223 | } | ||
| 224 | seenHosts := make(map[string]struct{}, len(entries)) | 224 | seenHosts := make(map[string]struct{}, len(entries)) | ||
| 225 | for _, entry := range entries { | 225 | for _, entry := range entries { | ||
| 226 | - | if err := domain.ValidateRemoteHostTarget(entry.Host); err != nil || len(entry.Host) > RemoteCatalogMaxNameBytes { | 226 | + | if !validText(entry.Host) || len(entry.Host) > RemoteCatalogMaxNameBytes || domain.ValidateRemoteHostTarget(entry.Host) != nil { |
| 227 | return fmt.Errorf("%w: invalid host", ErrInvalidRemoteCatalog) | 227 | return fmt.Errorf("%w: invalid host", ErrInvalidRemoteCatalog) | ||
| 228 | } | 228 | } | ||
| 229 | if entry.FetchedAt.IsZero() || entry.FetchedAt.UnixNano() <= 0 { | 229 | if entry.FetchedAt.IsZero() || entry.FetchedAt.UnixNano() <= 0 { | ||
M internal/protocol/catalogue/catalogue_test.go +8 -0
| @@ -2,6 +2,7 @@ package catalogue | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "testing" | |
| 5 | + | "time" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/bnema/vev/internal/protocol" | |
| 7 | 8 | "github.com/stretchr/testify/require" | |
| @@ -61,6 +62,13 @@ func TestValidateRemoteCatalogRejectsTerminalUnsafeText(t *testing.T) { | |||
| 61 | 62 | } | |
| 62 | 63 | } | |
| 63 | 64 | ||
| 65 | + | func TestValidateRemoteCatalogCacheEntriesRejectsTerminalUnsafeHost(t *testing.T) { | |
| 66 | + | err := ValidateRemoteCatalogCacheEntries([]RemoteCatalogCacheEntry{{ | |
| 67 | + | Host: "host\u202eoverride", FetchedAt: time.Unix(1, 0), Sessions: []RemoteCatalogSession{}, | |
| 68 | + | }}) | |
| 69 | + | require.ErrorIs(t, err, ErrInvalidRemoteCatalog) | |
| 70 | + | } | |
| 71 | + | ||
| 64 | 72 | func TestValidateRemoteCatalogRejectsDuplicateSessionNames(t *testing.T) { | |
| 65 | 73 | err := ValidateRemoteCatalog(RemoteCatalog{ | |
| 66 | 74 | ProtocolVersion: protocol.Version, | |
| @@ -2,6 +2,7 @@ package catalogue | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "testing" | 4 | "testing" | ||
| 5 | + | "time" | |||
| 5 | 6 | ||||
| 6 | "github.com/bnema/vev/internal/protocol" | 7 | "github.com/bnema/vev/internal/protocol" | ||
| 7 | "github.com/stretchr/testify/require" | 8 | "github.com/stretchr/testify/require" | ||
| @@ -61,6 +62,13 @@ func TestValidateRemoteCatalogRejectsTerminalUnsafeText(t *testing.T) { | |||||
| 61 | } | 62 | } | ||
| 62 | } | 63 | } | ||
| 63 | 64 | ||||
| 65 | + | func TestValidateRemoteCatalogCacheEntriesRejectsTerminalUnsafeHost(t *testing.T) { | |||
| 66 | + | err := ValidateRemoteCatalogCacheEntries([]RemoteCatalogCacheEntry{{ | |||
| 67 | + | Host: "host\u202eoverride", FetchedAt: time.Unix(1, 0), Sessions: []RemoteCatalogSession{}, | |||
| 68 | + | }}) | |||
| 69 | + | require.ErrorIs(t, err, ErrInvalidRemoteCatalog) | |||
| 70 | + | } | |||
| 71 | + | ||||
| 64 | func TestValidateRemoteCatalogRejectsDuplicateSessionNames(t *testing.T) { | 72 | func TestValidateRemoteCatalogRejectsDuplicateSessionNames(t *testing.T) { | ||
| 65 | err := ValidateRemoteCatalog(RemoteCatalog{ | 73 | err := ValidateRemoteCatalog(RemoteCatalog{ | ||
| 66 | ProtocolVersion: protocol.Version, | 74 | ProtocolVersion: protocol.Version, | ||
M internal/protocol/decode.go +7 -6
| @@ -25,12 +25,13 @@ const ( | |||
| 25 | 25 | ||
| 26 | 26 | // DecodeFailure describes rejected encoded input without retaining payload bytes. | |
| 27 | 27 | type DecodeFailure struct { | |
| 28 | - | Category DecodeCategory | |
| 29 | - | Kind DecodeMessageKind | |
| 30 | - | Type uint8 | |
| 31 | - | Version uint16 | |
| 32 | - | RequestID uint64 | |
| 33 | - | Err error | |
| 28 | + | Category DecodeCategory | |
| 29 | + | Kind DecodeMessageKind | |
| 30 | + | Type uint8 | |
| 31 | + | Version uint16 | |
| 32 | + | RequestID uint64 | |
| 33 | + | HasRequestID bool | |
| 34 | + | Err error | |
| 34 | 35 | } | |
| 35 | 36 | ||
| 36 | 37 | func (e *DecodeFailure) Error() string { | |
| @@ -25,12 +25,13 @@ const ( | |||||
| 25 | 25 | ||||
| 26 | // DecodeFailure describes rejected encoded input without retaining payload bytes. | 26 | // DecodeFailure describes rejected encoded input without retaining payload bytes. | ||
| 27 | type DecodeFailure struct { | 27 | type DecodeFailure struct { | ||
| 28 | - | Category DecodeCategory | 28 | + | Category DecodeCategory |
| 29 | - | Kind DecodeMessageKind | 29 | + | Kind DecodeMessageKind |
| 30 | - | Type uint8 | 30 | + | Type uint8 |
| 31 | - | Version uint16 | 31 | + | Version uint16 |
| 32 | - | RequestID uint64 | 32 | + | RequestID uint64 |
| 33 | - | Err error | 33 | + | HasRequestID bool |
| 34 | + | Err error | |||
| 34 | } | 35 | } | ||
| 35 | 36 | ||||
| 36 | func (e *DecodeFailure) Error() string { | 37 | func (e *DecodeFailure) Error() string { | ||
M internal/usecase/client/client.go +5 -4
| @@ -1120,9 +1120,9 @@ func paletteSlot(slot int) (uint8, bool) { | |||
| 1120 | 1120 | return uint8(slot), true | |
| 1121 | 1121 | } | |
| 1122 | 1122 | ||
| 1123 | - | // DetectTrueColor reports whether TERM/COLORTERM advertise direct color support. | |
| 1124 | - | func DetectTrueColor(termEnv, colorTerm string) bool { | |
| 1125 | - | return terminalcap.DetectTrueColor(termEnv, colorTerm) | |
| 1123 | + | // DetectTrueColor reports whether the client environment advertises direct color support. | |
| 1124 | + | func DetectTrueColor(termEnv, colorTerm string, env []string) bool { | |
| 1125 | + | return terminalcap.DetectTrueColor(termEnv, colorTerm, env) | |
| 1126 | 1126 | } | |
| 1127 | 1127 | ||
| 1128 | 1128 | func requestedOutputWindow(connection ports.ClientConnection) uint8 { | |
| @@ -1212,7 +1212,7 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||
| 1212 | 1212 | } | |
| 1213 | 1213 | termEnv := os.Getenv("TERM") | |
| 1214 | 1214 | colorTerm := os.Getenv("COLORTERM") | |
| 1215 | - | trueColor := DetectTrueColor(termEnv, colorTerm) | |
| 1215 | + | trueColor := DetectTrueColor(termEnv, colorTerm, os.Environ()) | |
| 1216 | 1216 | themeState.setTrueColor(trueColor) | |
| 1217 | 1217 | exactTarget := request.ExactTarget | |
| 1218 | 1218 | if exactTarget == nil && request.RemoteTarget != nil { | |
| @@ -2122,6 +2122,7 @@ func runRecv(ctx context.Context, transport ports.ClientConnection, out chan<- r | |||
| 2122 | 2122 | "type", failure.Type, | |
| 2123 | 2123 | "version", failure.Version, | |
| 2124 | 2124 | "request", failure.RequestID, | |
| 2125 | + | "has_request", failure.HasRequestID, | |
| 2125 | 2126 | ) | |
| 2126 | 2127 | continue | |
| 2127 | 2128 | } | |
| @@ -1120,9 +1120,9 @@ func paletteSlot(slot int) (uint8, bool) { | |||||
| 1120 | return uint8(slot), true | 1120 | return uint8(slot), true | ||
| 1121 | } | 1121 | } | ||
| 1122 | 1122 | ||||
| 1123 | - | // DetectTrueColor reports whether TERM/COLORTERM advertise direct color support. | 1123 | + | // DetectTrueColor reports whether the client environment advertises direct color support. |
| 1124 | - | func DetectTrueColor(termEnv, colorTerm string) bool { | 1124 | + | func DetectTrueColor(termEnv, colorTerm string, env []string) bool { |
| 1125 | - | return terminalcap.DetectTrueColor(termEnv, colorTerm) | 1125 | + | return terminalcap.DetectTrueColor(termEnv, colorTerm, env) |
| 1126 | } | 1126 | } | ||
| 1127 | 1127 | ||||
| 1128 | func requestedOutputWindow(connection ports.ClientConnection) uint8 { | 1128 | func requestedOutputWindow(connection ports.ClientConnection) uint8 { | ||
| @@ -1212,7 +1212,7 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||||
| 1212 | } | 1212 | } | ||
| 1213 | termEnv := os.Getenv("TERM") | 1213 | termEnv := os.Getenv("TERM") | ||
| 1214 | colorTerm := os.Getenv("COLORTERM") | 1214 | colorTerm := os.Getenv("COLORTERM") | ||
| 1215 | - | trueColor := DetectTrueColor(termEnv, colorTerm) | 1215 | + | trueColor := DetectTrueColor(termEnv, colorTerm, os.Environ()) |
| 1216 | themeState.setTrueColor(trueColor) | 1216 | themeState.setTrueColor(trueColor) | ||
| 1217 | exactTarget := request.ExactTarget | 1217 | exactTarget := request.ExactTarget | ||
| 1218 | if exactTarget == nil && request.RemoteTarget != nil { | 1218 | if exactTarget == nil && request.RemoteTarget != nil { | ||
| @@ -2122,6 +2122,7 @@ func runRecv(ctx context.Context, transport ports.ClientConnection, out chan<- r | |||||
| 2122 | "type", failure.Type, | 2122 | "type", failure.Type, | ||
| 2123 | "version", failure.Version, | 2123 | "version", failure.Version, | ||
| 2124 | "request", failure.RequestID, | 2124 | "request", failure.RequestID, | ||
| 2125 | + | "has_request", failure.HasRequestID, | |||
| 2125 | ) | 2126 | ) | ||
| 2126 | continue | 2127 | continue | ||
| 2127 | } | 2128 | } | ||
M internal/usecase/client/truecolor_test.go +3 -1
| @@ -11,6 +11,7 @@ func TestDetectTrueColor(t *testing.T) { | |||
| 11 | 11 | name string | |
| 12 | 12 | termEnv string | |
| 13 | 13 | colorTerm string | |
| 14 | + | env []string | |
| 14 | 15 | want bool | |
| 15 | 16 | }{ | |
| 16 | 17 | {name: "COLORTERM truecolor", termEnv: "screen", colorTerm: "truecolor", want: true}, | |
| @@ -19,13 +20,14 @@ func TestDetectTrueColor(t *testing.T) { | |||
| 19 | 20 | {name: "xterm-direct", termEnv: "xterm-direct", colorTerm: "", want: true}, | |
| 20 | 21 | {name: "direct suffix", termEnv: "foot-direct", colorTerm: "", want: true}, | |
| 21 | 22 | {name: "direct suffix ignores case and space", termEnv: " FOOT-DIRECT ", colorTerm: "", want: true}, | |
| 23 | + | {name: "kitty without COLORTERM", termEnv: "xterm-kitty", env: []string{"KITTY_WINDOW_ID=1"}, want: true}, | |
| 22 | 24 | {name: "xterm 256 only", termEnv: "xterm-256color", colorTerm: "", want: false}, | |
| 23 | 25 | {name: "empty", termEnv: "", colorTerm: "", want: false}, | |
| 24 | 26 | } | |
| 25 | 27 | ||
| 26 | 28 | for _, tt := range tests { | |
| 27 | 29 | t.Run(tt.name, func(t *testing.T) { | |
| 28 | - | require.Equal(t, tt.want, DetectTrueColor(tt.termEnv, tt.colorTerm)) | |
| 30 | + | require.Equal(t, tt.want, DetectTrueColor(tt.termEnv, tt.colorTerm, tt.env)) | |
| 29 | 31 | }) | |
| 30 | 32 | } | |
| 31 | 33 | } | |
| @@ -11,6 +11,7 @@ func TestDetectTrueColor(t *testing.T) { | |||||
| 11 | name string | 11 | name string | ||
| 12 | termEnv string | 12 | termEnv string | ||
| 13 | colorTerm string | 13 | colorTerm string | ||
| 14 | + | env []string | |||
| 14 | want bool | 15 | want bool | ||
| 15 | }{ | 16 | }{ | ||
| 16 | {name: "COLORTERM truecolor", termEnv: "screen", colorTerm: "truecolor", want: true}, | 17 | {name: "COLORTERM truecolor", termEnv: "screen", colorTerm: "truecolor", want: true}, | ||
| @@ -19,13 +20,14 @@ func TestDetectTrueColor(t *testing.T) { | |||||
| 19 | {name: "xterm-direct", termEnv: "xterm-direct", colorTerm: "", want: true}, | 20 | {name: "xterm-direct", termEnv: "xterm-direct", colorTerm: "", want: true}, | ||
| 20 | {name: "direct suffix", termEnv: "foot-direct", colorTerm: "", want: true}, | 21 | {name: "direct suffix", termEnv: "foot-direct", colorTerm: "", want: true}, | ||
| 21 | {name: "direct suffix ignores case and space", termEnv: " FOOT-DIRECT ", colorTerm: "", want: true}, | 22 | {name: "direct suffix ignores case and space", termEnv: " FOOT-DIRECT ", colorTerm: "", want: true}, | ||
| 23 | + | {name: "kitty without COLORTERM", termEnv: "xterm-kitty", env: []string{"KITTY_WINDOW_ID=1"}, want: true}, | |||
| 22 | {name: "xterm 256 only", termEnv: "xterm-256color", colorTerm: "", want: false}, | 24 | {name: "xterm 256 only", termEnv: "xterm-256color", colorTerm: "", want: false}, | ||
| 23 | {name: "empty", termEnv: "", colorTerm: "", want: false}, | 25 | {name: "empty", termEnv: "", colorTerm: "", want: false}, | ||
| 24 | } | 26 | } | ||
| 25 | 27 | ||||
| 26 | for _, tt := range tests { | 28 | for _, tt := range tests { | ||
| 27 | t.Run(tt.name, func(t *testing.T) { | 29 | t.Run(tt.name, func(t *testing.T) { | ||
| 28 | - | require.Equal(t, tt.want, DetectTrueColor(tt.termEnv, tt.colorTerm)) | 30 | + | require.Equal(t, tt.want, DetectTrueColor(tt.termEnv, tt.colorTerm, tt.env)) |
| 29 | }) | 31 | }) | ||
| 30 | } | 32 | } | ||
| 31 | } | 33 | } | ||
M internal/usecase/daemon/client_frame_routing.go +1 -0
| @@ -59,6 +59,7 @@ func (d *Daemon) runConnLoop(ac *attachedClient) { | |||
| 59 | 59 | "type", failure.Type, | |
| 60 | 60 | "version", failure.Version, | |
| 61 | 61 | "request", failure.RequestID, | |
| 62 | + | "has_request", failure.HasRequestID, | |
| 62 | 63 | ) | |
| 63 | 64 | continue | |
| 64 | 65 | } | |
| @@ -59,6 +59,7 @@ func (d *Daemon) runConnLoop(ac *attachedClient) { | |||||
| 59 | "type", failure.Type, | 59 | "type", failure.Type, | ||
| 60 | "version", failure.Version, | 60 | "version", failure.Version, | ||
| 61 | "request", failure.RequestID, | 61 | "request", failure.RequestID, | ||
| 62 | + | "has_request", failure.HasRequestID, | |||
| 62 | ) | 63 | ) | ||
| 63 | continue | 64 | continue | ||
| 64 | } | 65 | } | ||
M internal/usecase/daemon/daemon.go +3 -0
| @@ -1078,6 +1078,9 @@ func (d *Daemon) handleInitialDecodeFailure(ctx context.Context, tr ports.Server | |||
| 1078 | 1078 | } | |
| 1079 | 1079 | send(serverError(protocol.ErrInternal, "malformed hello")) | |
| 1080 | 1080 | case protocol.DecodeMessageCommand: | |
| 1081 | + | if !failure.HasRequestID { | |
| 1082 | + | return | |
| 1083 | + | } | |
| 1081 | 1084 | code, text := protocol.ErrInternal, "malformed command request" | |
| 1082 | 1085 | if failure.Version != 0 && failure.Version != protocol.Version { | |
| 1083 | 1086 | code, text = protocol.ErrVersionMismatch, "protocol version mismatch" | |
| @@ -1078,6 +1078,9 @@ func (d *Daemon) handleInitialDecodeFailure(ctx context.Context, tr ports.Server | |||||
| 1078 | } | 1078 | } | ||
| 1079 | send(serverError(protocol.ErrInternal, "malformed hello")) | 1079 | send(serverError(protocol.ErrInternal, "malformed hello")) | ||
| 1080 | case protocol.DecodeMessageCommand: | 1080 | case protocol.DecodeMessageCommand: | ||
| 1081 | + | if !failure.HasRequestID { | |||
| 1082 | + | return | |||
| 1083 | + | } | |||
| 1081 | code, text := protocol.ErrInternal, "malformed command request" | 1084 | code, text := protocol.ErrInternal, "malformed command request" | ||
| 1082 | if failure.Version != 0 && failure.Version != protocol.Version { | 1085 | if failure.Version != 0 && failure.Version != protocol.Version { | ||
| 1083 | code, text = protocol.ErrVersionMismatch, "protocol version mismatch" | 1086 | code, text = protocol.ErrVersionMismatch, "protocol version mismatch" | ||
M internal/usecase/daemon/typed_connection_test.go +1 -0
| @@ -113,6 +113,7 @@ func testReceiveClient(raw wire.Transport) (protocol.ClientMessage, error) { | |||
| 113 | 113 | case wire.MsgCommand: | |
| 114 | 114 | failure.Kind = protocol.DecodeMessageCommand | |
| 115 | 115 | failure.Version, _ = wire.PeekCommandVersion(frame.Payload) | |
| 116 | + | failure.RequestID, failure.HasRequestID = wire.PeekCommandRequestID(frame.Payload) | |
| 116 | 117 | case wire.MsgKill: | |
| 117 | 118 | failure.Kind = protocol.DecodeMessageKill | |
| 118 | 119 | case wire.MsgRemotePreviewRequest: | |
| @@ -113,6 +113,7 @@ func testReceiveClient(raw wire.Transport) (protocol.ClientMessage, error) { | |||||
| 113 | case wire.MsgCommand: | 113 | case wire.MsgCommand: | ||
| 114 | failure.Kind = protocol.DecodeMessageCommand | 114 | failure.Kind = protocol.DecodeMessageCommand | ||
| 115 | failure.Version, _ = wire.PeekCommandVersion(frame.Payload) | 115 | failure.Version, _ = wire.PeekCommandVersion(frame.Payload) | ||
| 116 | + | failure.RequestID, failure.HasRequestID = wire.PeekCommandRequestID(frame.Payload) | |||
| 116 | case wire.MsgKill: | 117 | case wire.MsgKill: | ||
| 117 | failure.Kind = protocol.DecodeMessageKill | 118 | failure.Kind = protocol.DecodeMessageKill | ||
| 118 | case wire.MsgRemotePreviewRequest: | 119 | case wire.MsgRemotePreviewRequest: | ||