fix(client): address route review findings
- commit
- 5884320d436c90fffd5ee8e0e1cb35bed5122d6a
- author
- brice · · 2026-08-09 16:35 UTC
- parent
- 15781f2
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
23 files · +270 -205
M AGENTS.md +1 -1
| @@ -64,7 +64,7 @@ Before touching daemon teardown paths, read the lock-ordering notes at the top o | |||
| 64 | 64 | Wire payload types/codecs live in `internal/ports/frame.go` and `internal/ports/wire.go`. Connection framing lives in `internal/adapters/ipc/transport.go`. | |
| 65 | 65 | ||
| 66 | 66 | - IPC frames on a connection are 4-byte big-endian length, 1 type byte, then payload. | |
| 67 | - | - Client message types occupy `1–13` (`MsgOutputResetRequest` is `13`) and `15` (`MsgRemotePreviewRequest`); server types occupy `16–23` (`MsgNavigationAction` is `23`) and `25–26` (`MsgAttachTarget` and `MsgRemotePreviewResponse`). Types `14` and `24` remain reserved. | |
| 67 | + | - Client message types occupy `1–13` (`MsgOutputResetRequest` is `13`) and `15` (`MsgRemotePreviewRequest`); server types occupy `16–23` (`MsgNavigationAction` is `23`) and `25–30` (`MsgAttachTarget`, `MsgRemotePreviewResponse`, `MsgCommittedRouteIdentity`, `MsgRecentRouteSnapshot`, `MsgNavigateRecentRoute`, and `MsgRouteNavigationFailure`). Types `14` and `24` remain reserved. | |
| 68 | 68 | - Version negotiation requires strict equality. | |
| 69 | 69 | - `Hello.Version` and `CommandRequest.Version` must stay first so their version peekers work. | |
| 70 | 70 | - Bump `ProtocolVersion` for any message layout change. | |
| @@ -64,7 +64,7 @@ Before touching daemon teardown paths, read the lock-ordering notes at the top o | |||||
| 64 | Wire payload types/codecs live in `internal/ports/frame.go` and `internal/ports/wire.go`. Connection framing lives in `internal/adapters/ipc/transport.go`. | 64 | Wire payload types/codecs live in `internal/ports/frame.go` and `internal/ports/wire.go`. Connection framing lives in `internal/adapters/ipc/transport.go`. | ||
| 65 | 65 | ||||
| 66 | - IPC frames on a connection are 4-byte big-endian length, 1 type byte, then payload. | 66 | - IPC frames on a connection are 4-byte big-endian length, 1 type byte, then payload. | ||
| 67 | - | - Client message types occupy `1–13` (`MsgOutputResetRequest` is `13`) and `15` (`MsgRemotePreviewRequest`); server types occupy `16–23` (`MsgNavigationAction` is `23`) and `25–26` (`MsgAttachTarget` and `MsgRemotePreviewResponse`). Types `14` and `24` remain reserved. | 67 | + | - Client message types occupy `1–13` (`MsgOutputResetRequest` is `13`) and `15` (`MsgRemotePreviewRequest`); server types occupy `16–23` (`MsgNavigationAction` is `23`) and `25–30` (`MsgAttachTarget`, `MsgRemotePreviewResponse`, `MsgCommittedRouteIdentity`, `MsgRecentRouteSnapshot`, `MsgNavigateRecentRoute`, and `MsgRouteNavigationFailure`). Types `14` and `24` remain reserved. |
| 68 | - Version negotiation requires strict equality. | 68 | - Version negotiation requires strict equality. | ||
| 69 | - `Hello.Version` and `CommandRequest.Version` must stay first so their version peekers work. | 69 | - `Hello.Version` and `CommandRequest.Version` must stay first so their version peekers work. | ||
| 70 | - Bump `ProtocolVersion` for any message layout change. | 70 | - Bump `ProtocolVersion` for any message layout change. | ||
M internal/ports/frame_test.go +0 -35
| @@ -52,38 +52,3 @@ func TestMsgTypeUnique(t *testing.T) { | |||
| 52 | 52 | t.Fatalf("expected %d distinct MsgType values, got %d", len(tests), len(seen)) | |
| 53 | 53 | } | |
| 54 | 54 | } | |
| 55 | - | ||
| 56 | - | func TestNavigationActionFrame(t *testing.T) { | |
| 57 | - | tests := []struct { | |
| 58 | - | name string | |
| 59 | - | action NavigationAction | |
| 60 | - | bytes []byte | |
| 61 | - | }{ | |
| 62 | - | {name: "open home picker", action: NavigationOpenHomePicker, bytes: []byte{1}}, | |
| 63 | - | {name: "back", action: NavigationBack, bytes: []byte{2}}, | |
| 64 | - | } | |
| 65 | - | for _, tt := range tests { | |
| 66 | - | t.Run(tt.name, func(t *testing.T) { | |
| 67 | - | frame := Frame{Type: MsgNavigationAction, Payload: MarshalNavigationAction(tt.action)} | |
| 68 | - | if frame.Type != MsgNavigationAction { | |
| 69 | - | t.Fatalf("frame type = %d, want %d", frame.Type, MsgNavigationAction) | |
| 70 | - | } | |
| 71 | - | if got := frame.Payload; string(got) != string(tt.bytes) { | |
| 72 | - | t.Fatalf("payload = %x, want %x", got, tt.bytes) | |
| 73 | - | } | |
| 74 | - | action, err := UnmarshalNavigationAction(frame.Payload) | |
| 75 | - | if err != nil { | |
| 76 | - | t.Fatalf("UnmarshalNavigationAction() error = %v", err) | |
| 77 | - | } | |
| 78 | - | if action != tt.action { | |
| 79 | - | t.Fatalf("action = %d, want %d", action, tt.action) | |
| 80 | - | } | |
| 81 | - | }) | |
| 82 | - | } | |
| 83 | - | ||
| 84 | - | for _, payload := range [][]byte{nil, []byte{1, 0, 0}, []byte{2, 0, 0}} { | |
| 85 | - | if _, err := UnmarshalNavigationAction(payload); err == nil { | |
| 86 | - | t.Fatalf("UnmarshalNavigationAction(%x) succeeded, want error", payload) | |
| 87 | - | } | |
| 88 | - | } | |
| 89 | - | } | |
| @@ -52,38 +52,3 @@ func TestMsgTypeUnique(t *testing.T) { | |||||
| 52 | t.Fatalf("expected %d distinct MsgType values, got %d", len(tests), len(seen)) | 52 | t.Fatalf("expected %d distinct MsgType values, got %d", len(tests), len(seen)) | ||
| 53 | } | 53 | } | ||
| 54 | } | 54 | } | ||
| 55 | - | ||||
| 56 | - | func TestNavigationActionFrame(t *testing.T) { | |||
| 57 | - | tests := []struct { | |||
| 58 | - | name string | |||
| 59 | - | action NavigationAction | |||
| 60 | - | bytes []byte | |||
| 61 | - | }{ | |||
| 62 | - | {name: "open home picker", action: NavigationOpenHomePicker, bytes: []byte{1}}, | |||
| 63 | - | {name: "back", action: NavigationBack, bytes: []byte{2}}, | |||
| 64 | - | } | |||
| 65 | - | for _, tt := range tests { | |||
| 66 | - | t.Run(tt.name, func(t *testing.T) { | |||
| 67 | - | frame := Frame{Type: MsgNavigationAction, Payload: MarshalNavigationAction(tt.action)} | |||
| 68 | - | if frame.Type != MsgNavigationAction { | |||
| 69 | - | t.Fatalf("frame type = %d, want %d", frame.Type, MsgNavigationAction) | |||
| 70 | - | } | |||
| 71 | - | if got := frame.Payload; string(got) != string(tt.bytes) { | |||
| 72 | - | t.Fatalf("payload = %x, want %x", got, tt.bytes) | |||
| 73 | - | } | |||
| 74 | - | action, err := UnmarshalNavigationAction(frame.Payload) | |||
| 75 | - | if err != nil { | |||
| 76 | - | t.Fatalf("UnmarshalNavigationAction() error = %v", err) | |||
| 77 | - | } | |||
| 78 | - | if action != tt.action { | |||
| 79 | - | t.Fatalf("action = %d, want %d", action, tt.action) | |||
| 80 | - | } | |||
| 81 | - | }) | |||
| 82 | - | } | |||
| 83 | - | ||||
| 84 | - | for _, payload := range [][]byte{nil, []byte{1, 0, 0}, []byte{2, 0, 0}} { | |||
| 85 | - | if _, err := UnmarshalNavigationAction(payload); err == nil { | |||
| 86 | - | t.Fatalf("UnmarshalNavigationAction(%x) succeeded, want error", payload) | |||
| 87 | - | } | |||
| 88 | - | } | |||
| 89 | - | } | |||
M internal/ports/navigation_test.go +1 -1
| @@ -62,7 +62,7 @@ func TestHelloNavigationValidationTable(t *testing.T) { | |||
| 62 | 62 | {name: "client-owned home picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityHomePicker}, valid: false}, | |
| 63 | 63 | {name: "daemon-owned back picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: false}, | |
| 64 | 64 | {name: "remote-target back picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Name: "work", Size: domain.Size{Cols: 80, Rows: 24}, RemoteTarget: remoteTarget, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: false}, | |
| 65 | - | {name: "startup picker requires back", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: true, capability: NavigationCapabilityBack, overlay: StartupOverlaySessionPicker, wantPayload: "001b02" + "00000000000000000000000000000000" + "0000000000000000" + "0000" + "00500018" + "0000" + "0000" + "00" + "00" + "00000000" + "00" + "00" + "00" + "0201"}, | |
| 65 | + | {name: "back with startup picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: true, capability: NavigationCapabilityBack, overlay: StartupOverlaySessionPicker, wantPayload: "001b02" + "00000000000000000000000000000000" + "0000000000000000" + "0000" + "00500018" + "0000" + "0000" + "00" + "00" + "00000000" + "00" + "00" + "00" + "0201"}, | |
| 66 | 66 | {name: "new rejects navigation", hello: Hello{Version: ProtocolVersion, Intent: IntentNew, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: false}, | |
| 67 | 67 | {name: "unknown capability", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: 4}, valid: false}, | |
| 68 | 68 | {name: "back without picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack}, valid: false}, | |
| @@ -62,7 +62,7 @@ func TestHelloNavigationValidationTable(t *testing.T) { | |||||
| 62 | {name: "client-owned home picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityHomePicker}, valid: false}, | 62 | {name: "client-owned home picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityHomePicker}, valid: false}, | ||
| 63 | {name: "daemon-owned back picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: false}, | 63 | {name: "daemon-owned back picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: false}, | ||
| 64 | {name: "remote-target back picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Name: "work", Size: domain.Size{Cols: 80, Rows: 24}, RemoteTarget: remoteTarget, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: false}, | 64 | {name: "remote-target back picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Name: "work", Size: domain.Size{Cols: 80, Rows: 24}, RemoteTarget: remoteTarget, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: false}, | ||
| 65 | - | {name: "startup picker requires back", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: true, capability: NavigationCapabilityBack, overlay: StartupOverlaySessionPicker, wantPayload: "001b02" + "00000000000000000000000000000000" + "0000000000000000" + "0000" + "00500018" + "0000" + "0000" + "00" + "00" + "00000000" + "00" + "00" + "00" + "0201"}, | 65 | + | {name: "back with startup picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: true, capability: NavigationCapabilityBack, overlay: StartupOverlaySessionPicker, wantPayload: "001b02" + "00000000000000000000000000000000" + "0000000000000000" + "0000" + "00500018" + "0000" + "0000" + "00" + "00" + "00000000" + "00" + "00" + "00" + "0201"}, |
| 66 | {name: "new rejects navigation", hello: Hello{Version: ProtocolVersion, Intent: IntentNew, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: false}, | 66 | {name: "new rejects navigation", hello: Hello{Version: ProtocolVersion, Intent: IntentNew, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: false}, | ||
| 67 | {name: "unknown capability", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: 4}, valid: false}, | 67 | {name: "unknown capability", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: 4}, valid: false}, | ||
| 68 | {name: "back without picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack}, valid: false}, | 68 | {name: "back without picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack}, valid: false}, | ||
M internal/ports/routes.go +3 -0
| @@ -102,6 +102,9 @@ func ValidateRouteLabel(value string, allowEmpty bool) error { | |||
| 102 | 102 | if unicode.IsControl(r) { | |
| 103 | 103 | return errors.New("route label contains a control character") | |
| 104 | 104 | } | |
| 105 | + | if unicode.Is(unicode.Bidi_Control, r) || r == '\u2028' || r == '\u2029' { | |
| 106 | + | return errors.New("route label contains a bidirectional or line-separator control character") | |
| 107 | + | } | |
| 105 | 108 | } | |
| 106 | 109 | return nil | |
| 107 | 110 | } | |
| @@ -102,6 +102,9 @@ func ValidateRouteLabel(value string, allowEmpty bool) error { | |||||
| 102 | if unicode.IsControl(r) { | 102 | if unicode.IsControl(r) { | ||
| 103 | return errors.New("route label contains a control character") | 103 | return errors.New("route label contains a control character") | ||
| 104 | } | 104 | } | ||
| 105 | + | if unicode.Is(unicode.Bidi_Control, r) || r == '\u2028' || r == '\u2029' { | |||
| 106 | + | return errors.New("route label contains a bidirectional or line-separator control character") | |||
| 107 | + | } | |||
| 105 | } | 108 | } | ||
| 106 | return nil | 109 | return nil | ||
| 107 | } | 110 | } | ||
M internal/ports/routes_test.go +64 -34
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package ports | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "encoding/hex" | |
| 4 | 5 | "testing" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/bnema/vev/internal/domain" | |
| @@ -15,16 +16,15 @@ func TestExactSessionTargetCodecIsStrict(t *testing.T) { | |||
| 15 | 16 | want := testExactTarget() | |
| 16 | 17 | encoded, err := MarshalExactSessionTarget(want) | |
| 17 | 18 | require.NoError(t, err) | |
| 19 | + | require.Equal(t, "010203000000000000000000000000000004776f726b", hex.EncodeToString(encoded)) | |
| 20 | + | assertAllPrefixesFail(t, encoded, UnmarshalExactSessionTarget) | |
| 21 | + | _, err = UnmarshalExactSessionTarget(append(append([]byte(nil), encoded...), 0)) | |
| 22 | + | require.Error(t, err) | |
| 18 | 23 | ||
| 19 | 24 | got, err := UnmarshalExactSessionTarget(encoded) | |
| 20 | 25 | require.NoError(t, err) | |
| 21 | 26 | require.Equal(t, want, got) | |
| 22 | 27 | ||
| 23 | - | for _, payload := range [][]byte{encoded[:len(encoded)-1], append(append([]byte(nil), encoded...), 0)} { | |
| 24 | - | _, err := UnmarshalExactSessionTarget(payload) | |
| 25 | - | require.Error(t, err) | |
| 26 | - | } | |
| 27 | - | ||
| 28 | 28 | _, err = MarshalExactSessionTarget(ExactSessionTarget{SessionName: "work"}) | |
| 29 | 29 | require.ErrorIs(t, err, ErrInvalidRouteWire) | |
| 30 | 30 | } | |
| @@ -72,6 +72,11 @@ func TestCommittedRouteIdentityCodec(t *testing.T) { | |||
| 72 | 72 | encoded, err := MarshalCommittedRouteIdentity(want) | |
| 73 | 73 | require.NoError(t, err) | |
| 74 | 74 | ||
| 75 | + | require.Equal(t, "010203000000000000000000000000000004776f726b01", hex.EncodeToString(encoded)) | |
| 76 | + | assertAllPrefixesFail(t, encoded, UnmarshalCommittedRouteIdentity) | |
| 77 | + | _, err = UnmarshalCommittedRouteIdentity(append(append([]byte(nil), encoded...), 0)) | |
| 78 | + | require.Error(t, err) | |
| 79 | + | ||
| 75 | 80 | got, err := UnmarshalCommittedRouteIdentity(encoded) | |
| 76 | 81 | require.NoError(t, err) | |
| 77 | 82 | require.Equal(t, want, got) | |
| @@ -103,42 +108,59 @@ func TestRecentRouteSnapshotCodecRoundTripAndBounds(t *testing.T) { | |||
| 103 | 108 | require.NoError(t, err) | |
| 104 | 109 | require.Equal(t, want, got) | |
| 105 | 110 | ||
| 106 | - | for _, payload := range [][]byte{encoded[:len(encoded)-1], append(append([]byte(nil), encoded...), 0)} { | |
| 107 | - | _, err := UnmarshalRecentRouteSnapshot(payload) | |
| 108 | - | require.Error(t, err) | |
| 109 | - | } | |
| 110 | - | ||
| 111 | - | invalid := want | |
| 112 | - | invalid.Previous = RouteRef{Key: 99, Generation: 99} | |
| 113 | - | _, err = MarshalRecentRouteSnapshot(invalid) | |
| 114 | - | require.ErrorIs(t, err, ErrInvalidRouteWire) | |
| 115 | - | ||
| 116 | - | invalid = want | |
| 117 | - | invalid.Entries = append(invalid.Entries, RecentRouteEntry{Key: 11, Generation: 3, Name: "work", Kind: RouteKindLocal}) | |
| 118 | - | _, err = MarshalRecentRouteSnapshot(invalid) | |
| 119 | - | require.ErrorIs(t, err, ErrInvalidRouteWire) | |
| 120 | - | ||
| 121 | - | invalid = want | |
| 122 | - | invalid.Entries = append(invalid.Entries, invalid.Entries[0]) | |
| 123 | - | _, err = MarshalRecentRouteSnapshot(invalid) | |
| 124 | - | require.ErrorIs(t, err, ErrInvalidRouteWire) | |
| 111 | + | require.Equal(t, "0000000000000009000000000000000b0000000000000003000000000000000c0000000000000004000000000000000b000000000000000301000000000000000c000000000000000400046c6f677300046564676502000100", hex.EncodeToString(encoded)) | |
| 112 | + | assertAllPrefixesFail(t, encoded, UnmarshalRecentRouteSnapshot) | |
| 113 | + | _, err = UnmarshalRecentRouteSnapshot(append(append([]byte(nil), encoded...), 0)) | |
| 114 | + | require.Error(t, err) | |
| 125 | 115 | ||
| 126 | - | invalid = want | |
| 127 | - | invalid.Entries = make([]RecentRouteEntry, RouteSnapshotMaxEntries+1) | |
| 128 | - | for i := range invalid.Entries { | |
| 129 | - | invalid.Entries[i] = RecentRouteEntry{Key: uint64(i + 1), Generation: uint64(i + 1), Name: "x", Kind: RouteKindLocal} | |
| 116 | + | invalidCases := []struct { | |
| 117 | + | name string | |
| 118 | + | mutate func(*RecentRouteSnapshot) | |
| 119 | + | }{ | |
| 120 | + | {name: "missing previous reference", mutate: func(snapshot *RecentRouteSnapshot) { snapshot.Previous = RouteRef{Key: 99, Generation: 99} }}, | |
| 121 | + | {name: "entry is active", mutate: func(snapshot *RecentRouteSnapshot) { | |
| 122 | + | snapshot.Entries = append(snapshot.Entries, RecentRouteEntry{Key: 11, Generation: 3, Name: "work", Kind: RouteKindLocal}) | |
| 123 | + | }}, | |
| 124 | + | {name: "duplicate entry", mutate: func(snapshot *RecentRouteSnapshot) { snapshot.Entries = append(snapshot.Entries, snapshot.Entries[0]) }}, | |
| 125 | + | {name: "too many entries", mutate: func(snapshot *RecentRouteSnapshot) { | |
| 126 | + | snapshot.Entries = make([]RecentRouteEntry, RouteSnapshotMaxEntries+1) | |
| 127 | + | for i := range snapshot.Entries { | |
| 128 | + | snapshot.Entries[i] = RecentRouteEntry{Key: uint64(i + 1), Generation: uint64(i + 1), Name: "x", Kind: RouteKindLocal} | |
| 129 | + | } | |
| 130 | + | }}, | |
| 130 | 131 | } | |
| 131 | - | _, err = MarshalRecentRouteSnapshot(invalid) | |
| 132 | + | for _, tc := range invalidCases { | |
| 133 | + | t.Run(tc.name, func(t *testing.T) { | |
| 134 | + | invalid := want | |
| 135 | + | tc.mutate(&invalid) | |
| 136 | + | _, err := MarshalRecentRouteSnapshot(invalid) | |
| 137 | + | require.ErrorIs(t, err, ErrInvalidRouteWire) | |
| 138 | + | }) | |
| 139 | + | } | |
| 140 | + | } | |
| 141 | + | ||
| 142 | + | func TestRecentRouteSnapshotRejectsOversizedFrameBeforeParsing(t *testing.T) { | |
| 143 | + | _, err := UnmarshalRecentRouteSnapshot(make([]byte, MaxFrameLen)) | |
| 132 | 144 | require.ErrorIs(t, err, ErrInvalidRouteWire) | |
| 133 | 145 | } | |
| 134 | 146 | ||
| 135 | 147 | func TestRouteLabelsRejectTerminalUnsafeText(t *testing.T) { | |
| 136 | - | for _, value := range []string{"line\nfeed", "\x1b[31mred", string([]byte{0xff, 0xfe})} { | |
| 137 | - | _, err := MarshalRecentRouteSnapshot(RecentRouteSnapshot{ | |
| 138 | - | Generation: 1, | |
| 139 | - | Entries: []RecentRouteEntry{{Key: 1, Generation: 1, Name: value, Kind: RouteKindLocal}}, | |
| 148 | + | values := []string{"line\nfeed", "\u2028line-separator", "\u2029paragraph-separator", "\x1b[31mred", "\u202eoverride", string([]byte{0xff, 0xfe})} | |
| 149 | + | for _, value := range values { | |
| 150 | + | t.Run("name/"+value, func(t *testing.T) { | |
| 151 | + | _, err := MarshalRecentRouteSnapshot(RecentRouteSnapshot{ | |
| 152 | + | Generation: 1, | |
| 153 | + | Entries: []RecentRouteEntry{{Key: 1, Generation: 1, Name: value, Kind: RouteKindLocal}}, | |
| 154 | + | }) | |
| 155 | + | require.Error(t, err) | |
| 156 | + | }) | |
| 157 | + | t.Run("host/"+value, func(t *testing.T) { | |
| 158 | + | _, err := MarshalRecentRouteSnapshot(RecentRouteSnapshot{ | |
| 159 | + | Generation: 1, | |
| 160 | + | Entries: []RecentRouteEntry{{Key: 1, Generation: 1, Name: "work", HostLabel: value, Kind: RouteKindRemote}}, | |
| 161 | + | }) | |
| 162 | + | require.Error(t, err) | |
| 140 | 163 | }) | |
| 141 | - | require.Error(t, err) | |
| 142 | 164 | } | |
| 143 | 165 | } | |
| 144 | 166 | ||
| @@ -146,6 +168,10 @@ func TestRouteActionAndFailureCodecsAreBounded(t *testing.T) { | |||
| 146 | 168 | action := RouteNavigationAction{SnapshotGeneration: 9, Key: 7, Generation: 8} | |
| 147 | 169 | encoded, err := MarshalRouteNavigationAction(action) | |
| 148 | 170 | require.NoError(t, err) | |
| 171 | + | require.Equal(t, "000000000000000900000000000000070000000000000008", hex.EncodeToString(encoded)) | |
| 172 | + | assertAllPrefixesFail(t, encoded, UnmarshalRouteNavigationAction) | |
| 173 | + | _, err = UnmarshalRouteNavigationAction(append(append([]byte(nil), encoded...), 0)) | |
| 174 | + | require.Error(t, err) | |
| 149 | 175 | got, err := UnmarshalRouteNavigationAction(encoded) | |
| 150 | 176 | require.NoError(t, err) | |
| 151 | 177 | require.Equal(t, action, got) | |
| @@ -153,6 +179,10 @@ func TestRouteActionAndFailureCodecsAreBounded(t *testing.T) { | |||
| 153 | 179 | failure := RouteNavigationFailure{Key: 7, Generation: 8, Code: RouteFailureStaleSelection} | |
| 154 | 180 | encoded, err = MarshalRouteNavigationFailure(failure) | |
| 155 | 181 | require.NoError(t, err) | |
| 182 | + | require.Equal(t, "0000000000000007000000000000000801", hex.EncodeToString(encoded)) | |
| 183 | + | assertAllPrefixesFail(t, encoded, UnmarshalRouteNavigationFailure) | |
| 184 | + | _, err = UnmarshalRouteNavigationFailure(append(append([]byte(nil), encoded...), 0)) | |
| 185 | + | require.Error(t, err) | |
| 156 | 186 | gotFailure, err := UnmarshalRouteNavigationFailure(encoded) | |
| 157 | 187 | require.NoError(t, err) | |
| 158 | 188 | require.Equal(t, failure, gotFailure) | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package ports | 1 | package ports | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "encoding/hex" | |||
| 4 | "testing" | 5 | "testing" | ||
| 5 | 6 | ||||
| 6 | "github.com/bnema/vev/internal/domain" | 7 | "github.com/bnema/vev/internal/domain" | ||
| @@ -15,16 +16,15 @@ func TestExactSessionTargetCodecIsStrict(t *testing.T) { | |||||
| 15 | want := testExactTarget() | 16 | want := testExactTarget() | ||
| 16 | encoded, err := MarshalExactSessionTarget(want) | 17 | encoded, err := MarshalExactSessionTarget(want) | ||
| 17 | require.NoError(t, err) | 18 | require.NoError(t, err) | ||
| 19 | + | require.Equal(t, "010203000000000000000000000000000004776f726b", hex.EncodeToString(encoded)) | |||
| 20 | + | assertAllPrefixesFail(t, encoded, UnmarshalExactSessionTarget) | |||
| 21 | + | _, err = UnmarshalExactSessionTarget(append(append([]byte(nil), encoded...), 0)) | |||
| 22 | + | require.Error(t, err) | |||
| 18 | 23 | ||||
| 19 | got, err := UnmarshalExactSessionTarget(encoded) | 24 | got, err := UnmarshalExactSessionTarget(encoded) | ||
| 20 | require.NoError(t, err) | 25 | require.NoError(t, err) | ||
| 21 | require.Equal(t, want, got) | 26 | require.Equal(t, want, got) | ||
| 22 | 27 | ||||
| 23 | - | for _, payload := range [][]byte{encoded[:len(encoded)-1], append(append([]byte(nil), encoded...), 0)} { | |||
| 24 | - | _, err := UnmarshalExactSessionTarget(payload) | |||
| 25 | - | require.Error(t, err) | |||
| 26 | - | } | |||
| 27 | - | ||||
| 28 | _, err = MarshalExactSessionTarget(ExactSessionTarget{SessionName: "work"}) | 28 | _, err = MarshalExactSessionTarget(ExactSessionTarget{SessionName: "work"}) | ||
| 29 | require.ErrorIs(t, err, ErrInvalidRouteWire) | 29 | require.ErrorIs(t, err, ErrInvalidRouteWire) | ||
| 30 | } | 30 | } | ||
| @@ -72,6 +72,11 @@ func TestCommittedRouteIdentityCodec(t *testing.T) { | |||||
| 72 | encoded, err := MarshalCommittedRouteIdentity(want) | 72 | encoded, err := MarshalCommittedRouteIdentity(want) | ||
| 73 | require.NoError(t, err) | 73 | require.NoError(t, err) | ||
| 74 | 74 | ||||
| 75 | + | require.Equal(t, "010203000000000000000000000000000004776f726b01", hex.EncodeToString(encoded)) | |||
| 76 | + | assertAllPrefixesFail(t, encoded, UnmarshalCommittedRouteIdentity) | |||
| 77 | + | _, err = UnmarshalCommittedRouteIdentity(append(append([]byte(nil), encoded...), 0)) | |||
| 78 | + | require.Error(t, err) | |||
| 79 | + | ||||
| 75 | got, err := UnmarshalCommittedRouteIdentity(encoded) | 80 | got, err := UnmarshalCommittedRouteIdentity(encoded) | ||
| 76 | require.NoError(t, err) | 81 | require.NoError(t, err) | ||
| 77 | require.Equal(t, want, got) | 82 | require.Equal(t, want, got) | ||
| @@ -103,42 +108,59 @@ func TestRecentRouteSnapshotCodecRoundTripAndBounds(t *testing.T) { | |||||
| 103 | require.NoError(t, err) | 108 | require.NoError(t, err) | ||
| 104 | require.Equal(t, want, got) | 109 | require.Equal(t, want, got) | ||
| 105 | 110 | ||||
| 106 | - | for _, payload := range [][]byte{encoded[:len(encoded)-1], append(append([]byte(nil), encoded...), 0)} { | 111 | + | require.Equal(t, "0000000000000009000000000000000b0000000000000003000000000000000c0000000000000004000000000000000b000000000000000301000000000000000c000000000000000400046c6f677300046564676502000100", hex.EncodeToString(encoded)) |
| 107 | - | _, err := UnmarshalRecentRouteSnapshot(payload) | 112 | + | assertAllPrefixesFail(t, encoded, UnmarshalRecentRouteSnapshot) |
| 108 | - | require.Error(t, err) | 113 | + | _, err = UnmarshalRecentRouteSnapshot(append(append([]byte(nil), encoded...), 0)) |
| 109 | - | } | 114 | + | require.Error(t, err) |
| 110 | - | ||||
| 111 | - | invalid := want | |||
| 112 | - | invalid.Previous = RouteRef{Key: 99, Generation: 99} | |||
| 113 | - | _, err = MarshalRecentRouteSnapshot(invalid) | |||
| 114 | - | require.ErrorIs(t, err, ErrInvalidRouteWire) | |||
| 115 | - | ||||
| 116 | - | invalid = want | |||
| 117 | - | invalid.Entries = append(invalid.Entries, RecentRouteEntry{Key: 11, Generation: 3, Name: "work", Kind: RouteKindLocal}) | |||
| 118 | - | _, err = MarshalRecentRouteSnapshot(invalid) | |||
| 119 | - | require.ErrorIs(t, err, ErrInvalidRouteWire) | |||
| 120 | - | ||||
| 121 | - | invalid = want | |||
| 122 | - | invalid.Entries = append(invalid.Entries, invalid.Entries[0]) | |||
| 123 | - | _, err = MarshalRecentRouteSnapshot(invalid) | |||
| 124 | - | require.ErrorIs(t, err, ErrInvalidRouteWire) | |||
| 125 | 115 | ||||
| 126 | - | invalid = want | 116 | + | invalidCases := []struct { |
| 127 | - | invalid.Entries = make([]RecentRouteEntry, RouteSnapshotMaxEntries+1) | 117 | + | name string |
| 128 | - | for i := range invalid.Entries { | 118 | + | mutate func(*RecentRouteSnapshot) |
| 129 | - | invalid.Entries[i] = RecentRouteEntry{Key: uint64(i + 1), Generation: uint64(i + 1), Name: "x", Kind: RouteKindLocal} | 119 | + | }{ |
| 120 | + | {name: "missing previous reference", mutate: func(snapshot *RecentRouteSnapshot) { snapshot.Previous = RouteRef{Key: 99, Generation: 99} }}, | |||
| 121 | + | {name: "entry is active", mutate: func(snapshot *RecentRouteSnapshot) { | |||
| 122 | + | snapshot.Entries = append(snapshot.Entries, RecentRouteEntry{Key: 11, Generation: 3, Name: "work", Kind: RouteKindLocal}) | |||
| 123 | + | }}, | |||
| 124 | + | {name: "duplicate entry", mutate: func(snapshot *RecentRouteSnapshot) { snapshot.Entries = append(snapshot.Entries, snapshot.Entries[0]) }}, | |||
| 125 | + | {name: "too many entries", mutate: func(snapshot *RecentRouteSnapshot) { | |||
| 126 | + | snapshot.Entries = make([]RecentRouteEntry, RouteSnapshotMaxEntries+1) | |||
| 127 | + | for i := range snapshot.Entries { | |||
| 128 | + | snapshot.Entries[i] = RecentRouteEntry{Key: uint64(i + 1), Generation: uint64(i + 1), Name: "x", Kind: RouteKindLocal} | |||
| 129 | + | } | |||
| 130 | + | }}, | |||
| 130 | } | 131 | } | ||
| 131 | - | _, err = MarshalRecentRouteSnapshot(invalid) | 132 | + | for _, tc := range invalidCases { |
| 133 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 134 | + | invalid := want | |||
| 135 | + | tc.mutate(&invalid) | |||
| 136 | + | _, err := MarshalRecentRouteSnapshot(invalid) | |||
| 137 | + | require.ErrorIs(t, err, ErrInvalidRouteWire) | |||
| 138 | + | }) | |||
| 139 | + | } | |||
| 140 | + | } | |||
| 141 | + | ||||
| 142 | + | func TestRecentRouteSnapshotRejectsOversizedFrameBeforeParsing(t *testing.T) { | |||
| 143 | + | _, err := UnmarshalRecentRouteSnapshot(make([]byte, MaxFrameLen)) | |||
| 132 | require.ErrorIs(t, err, ErrInvalidRouteWire) | 144 | require.ErrorIs(t, err, ErrInvalidRouteWire) | ||
| 133 | } | 145 | } | ||
| 134 | 146 | ||||
| 135 | func TestRouteLabelsRejectTerminalUnsafeText(t *testing.T) { | 147 | func TestRouteLabelsRejectTerminalUnsafeText(t *testing.T) { | ||
| 136 | - | for _, value := range []string{"line\nfeed", "\x1b[31mred", string([]byte{0xff, 0xfe})} { | 148 | + | values := []string{"line\nfeed", "\u2028line-separator", "\u2029paragraph-separator", "\x1b[31mred", "\u202eoverride", string([]byte{0xff, 0xfe})} |
| 137 | - | _, err := MarshalRecentRouteSnapshot(RecentRouteSnapshot{ | 149 | + | for _, value := range values { |
| 138 | - | Generation: 1, | 150 | + | t.Run("name/"+value, func(t *testing.T) { |
| 139 | - | Entries: []RecentRouteEntry{{Key: 1, Generation: 1, Name: value, Kind: RouteKindLocal}}, | 151 | + | _, err := MarshalRecentRouteSnapshot(RecentRouteSnapshot{ |
| 152 | + | Generation: 1, | |||
| 153 | + | Entries: []RecentRouteEntry{{Key: 1, Generation: 1, Name: value, Kind: RouteKindLocal}}, | |||
| 154 | + | }) | |||
| 155 | + | require.Error(t, err) | |||
| 156 | + | }) | |||
| 157 | + | t.Run("host/"+value, func(t *testing.T) { | |||
| 158 | + | _, err := MarshalRecentRouteSnapshot(RecentRouteSnapshot{ | |||
| 159 | + | Generation: 1, | |||
| 160 | + | Entries: []RecentRouteEntry{{Key: 1, Generation: 1, Name: "work", HostLabel: value, Kind: RouteKindRemote}}, | |||
| 161 | + | }) | |||
| 162 | + | require.Error(t, err) | |||
| 140 | }) | 163 | }) | ||
| 141 | - | require.Error(t, err) | |||
| 142 | } | 164 | } | ||
| 143 | } | 165 | } | ||
| 144 | 166 | ||||
| @@ -146,6 +168,10 @@ func TestRouteActionAndFailureCodecsAreBounded(t *testing.T) { | |||||
| 146 | action := RouteNavigationAction{SnapshotGeneration: 9, Key: 7, Generation: 8} | 168 | action := RouteNavigationAction{SnapshotGeneration: 9, Key: 7, Generation: 8} | ||
| 147 | encoded, err := MarshalRouteNavigationAction(action) | 169 | encoded, err := MarshalRouteNavigationAction(action) | ||
| 148 | require.NoError(t, err) | 170 | require.NoError(t, err) | ||
| 171 | + | require.Equal(t, "000000000000000900000000000000070000000000000008", hex.EncodeToString(encoded)) | |||
| 172 | + | assertAllPrefixesFail(t, encoded, UnmarshalRouteNavigationAction) | |||
| 173 | + | _, err = UnmarshalRouteNavigationAction(append(append([]byte(nil), encoded...), 0)) | |||
| 174 | + | require.Error(t, err) | |||
| 149 | got, err := UnmarshalRouteNavigationAction(encoded) | 175 | got, err := UnmarshalRouteNavigationAction(encoded) | ||
| 150 | require.NoError(t, err) | 176 | require.NoError(t, err) | ||
| 151 | require.Equal(t, action, got) | 177 | require.Equal(t, action, got) | ||
| @@ -153,6 +179,10 @@ func TestRouteActionAndFailureCodecsAreBounded(t *testing.T) { | |||||
| 153 | failure := RouteNavigationFailure{Key: 7, Generation: 8, Code: RouteFailureStaleSelection} | 179 | failure := RouteNavigationFailure{Key: 7, Generation: 8, Code: RouteFailureStaleSelection} | ||
| 154 | encoded, err = MarshalRouteNavigationFailure(failure) | 180 | encoded, err = MarshalRouteNavigationFailure(failure) | ||
| 155 | require.NoError(t, err) | 181 | require.NoError(t, err) | ||
| 182 | + | require.Equal(t, "0000000000000007000000000000000801", hex.EncodeToString(encoded)) | |||
| 183 | + | assertAllPrefixesFail(t, encoded, UnmarshalRouteNavigationFailure) | |||
| 184 | + | _, err = UnmarshalRouteNavigationFailure(append(append([]byte(nil), encoded...), 0)) | |||
| 185 | + | require.Error(t, err) | |||
| 156 | gotFailure, err := UnmarshalRouteNavigationFailure(encoded) | 186 | gotFailure, err := UnmarshalRouteNavigationFailure(encoded) | ||
| 157 | require.NoError(t, err) | 187 | require.NoError(t, err) | ||
| 158 | require.Equal(t, failure, gotFailure) | 188 | require.Equal(t, failure, gotFailure) | ||
M internal/ports/routes_wire.go +26 -13
| @@ -110,17 +110,25 @@ func MarshalCommittedRouteIdentity(identity CommittedRouteIdentity) ([]byte, err | |||
| 110 | 110 | return nil, err | |
| 111 | 111 | } | |
| 112 | 112 | w := payloadWriter{} | |
| 113 | - | marshalExactSessionTarget(&w, identity.Target) | |
| 114 | - | w.putBool(identity.Ephemeral) | |
| 113 | + | marshalCommittedRouteIdentityFields(&w, identity) | |
| 115 | 114 | return w.b, nil | |
| 116 | 115 | } | |
| 117 | 116 | ||
| 118 | - | func marshalCommittedIdentitySection(w *payloadWriter, identity *CommittedRouteIdentity) { | |
| 117 | + | func marshalCommittedRouteIdentityFields(w *payloadWriter, identity CommittedRouteIdentity) { | |
| 118 | + | marshalExactSessionTarget(w, identity.Target) | |
| 119 | + | w.putBool(identity.Ephemeral) | |
| 120 | + | } | |
| 121 | + | ||
| 122 | + | func marshalCommittedIdentitySection(w *payloadWriter, identity *CommittedRouteIdentity) bool { | |
| 119 | 123 | w.putBool(identity != nil) | |
| 120 | - | if identity != nil { | |
| 121 | - | marshalExactSessionTarget(w, identity.Target) | |
| 122 | - | w.putBool(identity.Ephemeral) | |
| 124 | + | if identity == nil { | |
| 125 | + | return true | |
| 126 | + | } | |
| 127 | + | if err := validateCommittedRouteIdentity(*identity); err != nil { | |
| 128 | + | return false | |
| 123 | 129 | } | |
| 130 | + | marshalCommittedRouteIdentityFields(w, *identity) | |
| 131 | + | return true | |
| 124 | 132 | } | |
| 125 | 133 | ||
| 126 | 134 | func unmarshalCommittedIdentitySection(r *payloadReader) (*CommittedRouteIdentity, error) { | |
| @@ -246,18 +254,20 @@ func validateRecentRouteSnapshot(snapshot RecentRouteSnapshot) error { | |||
| 246 | 254 | } | |
| 247 | 255 | refs[ref] = struct{}{} | |
| 248 | 256 | } | |
| 249 | - | for name, ref := range map[string]RouteRef{ | |
| 250 | - | "active": snapshot.Active, | |
| 251 | - | "previous": snapshot.Previous, | |
| 252 | - | "home": snapshot.Home, | |
| 257 | + | for _, item := range []struct { | |
| 258 | + | name string | |
| 259 | + | ref RouteRef | |
| 260 | + | }{ | |
| 261 | + | {name: "previous", ref: snapshot.Previous}, | |
| 262 | + | {name: "home", ref: snapshot.Home}, | |
| 253 | 263 | } { | |
| 254 | - | if ref.empty() || ref == snapshot.Active { | |
| 264 | + | if item.ref.empty() || item.ref == snapshot.Active { | |
| 255 | 265 | // Active is metadata-only and intentionally excluded from the | |
| 256 | 266 | // recent display entries. Home may point at active as well. | |
| 257 | 267 | continue | |
| 258 | 268 | } | |
| 259 | - | if _, exists := refs[ref]; !exists { | |
| 260 | - | return fmt.Errorf("%w: %s route reference is absent from snapshot", ErrInvalidRouteWire, name) | |
| 269 | + | if _, exists := refs[item.ref]; !exists { | |
| 270 | + | return fmt.Errorf("%w: %s route reference is absent from snapshot", ErrInvalidRouteWire, item.name) | |
| 261 | 271 | } | |
| 262 | 272 | } | |
| 263 | 273 | return nil | |
| @@ -284,6 +294,9 @@ func MarshalRecentRouteSnapshot(snapshot RecentRouteSnapshot) ([]byte, error) { | |||
| 284 | 294 | w.putBool(entry.Attention) | |
| 285 | 295 | w.putUint8(uint8(entry.Reachability)) | |
| 286 | 296 | } | |
| 297 | + | // Keep the frame bound defensive even though current entry/label bounds | |
| 298 | + | // make this unreachable; it protects the wire contract if those bounds | |
| 299 | + | // change independently in a future protocol revision. | |
| 287 | 300 | if len(w.b) > MaxFrameLen-1 { | |
| 288 | 301 | return nil, fmt.Errorf("%w: route snapshot is too large", ErrInvalidRouteWire) | |
| 289 | 302 | } | |
| @@ -110,17 +110,25 @@ func MarshalCommittedRouteIdentity(identity CommittedRouteIdentity) ([]byte, err | |||||
| 110 | return nil, err | 110 | return nil, err | ||
| 111 | } | 111 | } | ||
| 112 | w := payloadWriter{} | 112 | w := payloadWriter{} | ||
| 113 | - | marshalExactSessionTarget(&w, identity.Target) | 113 | + | marshalCommittedRouteIdentityFields(&w, identity) |
| 114 | - | w.putBool(identity.Ephemeral) | |||
| 115 | return w.b, nil | 114 | return w.b, nil | ||
| 116 | } | 115 | } | ||
| 117 | 116 | ||||
| 118 | - | func marshalCommittedIdentitySection(w *payloadWriter, identity *CommittedRouteIdentity) { | 117 | + | func marshalCommittedRouteIdentityFields(w *payloadWriter, identity CommittedRouteIdentity) { |
| 118 | + | marshalExactSessionTarget(w, identity.Target) | |||
| 119 | + | w.putBool(identity.Ephemeral) | |||
| 120 | + | } | |||
| 121 | + | ||||
| 122 | + | func marshalCommittedIdentitySection(w *payloadWriter, identity *CommittedRouteIdentity) bool { | |||
| 119 | w.putBool(identity != nil) | 123 | w.putBool(identity != nil) | ||
| 120 | - | if identity != nil { | 124 | + | if identity == nil { |
| 121 | - | marshalExactSessionTarget(w, identity.Target) | 125 | + | return true |
| 122 | - | w.putBool(identity.Ephemeral) | 126 | + | } |
| 127 | + | if err := validateCommittedRouteIdentity(*identity); err != nil { | |||
| 128 | + | return false | |||
| 123 | } | 129 | } | ||
| 130 | + | marshalCommittedRouteIdentityFields(w, *identity) | |||
| 131 | + | return true | |||
| 124 | } | 132 | } | ||
| 125 | 133 | ||||
| 126 | func unmarshalCommittedIdentitySection(r *payloadReader) (*CommittedRouteIdentity, error) { | 134 | func unmarshalCommittedIdentitySection(r *payloadReader) (*CommittedRouteIdentity, error) { | ||
| @@ -246,18 +254,20 @@ func validateRecentRouteSnapshot(snapshot RecentRouteSnapshot) error { | |||||
| 246 | } | 254 | } | ||
| 247 | refs[ref] = struct{}{} | 255 | refs[ref] = struct{}{} | ||
| 248 | } | 256 | } | ||
| 249 | - | for name, ref := range map[string]RouteRef{ | 257 | + | for _, item := range []struct { |
| 250 | - | "active": snapshot.Active, | 258 | + | name string |
| 251 | - | "previous": snapshot.Previous, | 259 | + | ref RouteRef |
| 252 | - | "home": snapshot.Home, | 260 | + | }{ |
| 261 | + | {name: "previous", ref: snapshot.Previous}, | |||
| 262 | + | {name: "home", ref: snapshot.Home}, | |||
| 253 | } { | 263 | } { | ||
| 254 | - | if ref.empty() || ref == snapshot.Active { | 264 | + | if item.ref.empty() || item.ref == snapshot.Active { |
| 255 | // Active is metadata-only and intentionally excluded from the | 265 | // Active is metadata-only and intentionally excluded from the | ||
| 256 | // recent display entries. Home may point at active as well. | 266 | // recent display entries. Home may point at active as well. | ||
| 257 | continue | 267 | continue | ||
| 258 | } | 268 | } | ||
| 259 | - | if _, exists := refs[ref]; !exists { | 269 | + | if _, exists := refs[item.ref]; !exists { |
| 260 | - | return fmt.Errorf("%w: %s route reference is absent from snapshot", ErrInvalidRouteWire, name) | 270 | + | return fmt.Errorf("%w: %s route reference is absent from snapshot", ErrInvalidRouteWire, item.name) |
| 261 | } | 271 | } | ||
| 262 | } | 272 | } | ||
| 263 | return nil | 273 | return nil | ||
| @@ -284,6 +294,9 @@ func MarshalRecentRouteSnapshot(snapshot RecentRouteSnapshot) ([]byte, error) { | |||||
| 284 | w.putBool(entry.Attention) | 294 | w.putBool(entry.Attention) | ||
| 285 | w.putUint8(uint8(entry.Reachability)) | 295 | w.putUint8(uint8(entry.Reachability)) | ||
| 286 | } | 296 | } | ||
| 297 | + | // Keep the frame bound defensive even though current entry/label bounds | |||
| 298 | + | // make this unreachable; it protects the wire contract if those bounds | |||
| 299 | + | // change independently in a future protocol revision. | |||
| 287 | if len(w.b) > MaxFrameLen-1 { | 300 | if len(w.b) > MaxFrameLen-1 { | ||
| 288 | return nil, fmt.Errorf("%w: route snapshot is too large", ErrInvalidRouteWire) | 301 | return nil, fmt.Errorf("%w: route snapshot is too large", ErrInvalidRouteWire) | ||
| 289 | } | 302 | } | ||
M internal/ports/wire.go +8 -5
| @@ -84,17 +84,18 @@ func validStartupOverlay(overlay StartupOverlay) bool { | |||
| 84 | 84 | } | |
| 85 | 85 | ||
| 86 | 86 | // ValidateNavigation validates route capabilities independently of the rest of | |
| 87 | - | // the Hello payload. | |
| 88 | - | func ValidateNavigation(capabilities NavigationCapabilities, overlay StartupOverlay, remoteTarget bool) error { | |
| 87 | + | // the Hello payload. homePickerRoute indicates that the route may open its | |
| 88 | + | // home/session picker (for example, a remote target or daemon-owned route). | |
| 89 | + | func ValidateNavigation(capabilities NavigationCapabilities, overlay StartupOverlay, homePickerRoute bool) error { | |
| 89 | 90 | if !validNavigationCapabilities(capabilities) || !validStartupOverlay(overlay) { | |
| 90 | 91 | return ErrInvalidNavigation | |
| 91 | 92 | } | |
| 92 | 93 | home := capabilities&NavigationCapabilityHomePicker != 0 | |
| 93 | 94 | back := capabilities&NavigationCapabilityBack != 0 | |
| 94 | - | if (home && !remoteTarget) || back != (overlay == StartupOverlaySessionPicker) { | |
| 95 | + | if (home && !homePickerRoute) || back != (overlay == StartupOverlaySessionPicker) { | |
| 95 | 96 | return ErrInvalidNavigation | |
| 96 | 97 | } | |
| 97 | - | if remoteTarget && back { | |
| 98 | + | if homePickerRoute && back { | |
| 98 | 99 | return ErrInvalidNavigation | |
| 99 | 100 | } | |
| 100 | 101 | return nil | |
| @@ -1302,7 +1303,9 @@ func MarshalWelcome(m Welcome) []byte { | |||
| 1302 | 1303 | } | |
| 1303 | 1304 | w.putUint64(m.ResumeToken) | |
| 1304 | 1305 | w.putUint32(m.Capabilities) | |
| 1305 | - | marshalCommittedIdentitySection(&w, m.CommittedIdentity) | |
| 1306 | + | if !marshalCommittedIdentitySection(&w, m.CommittedIdentity) { | |
| 1307 | + | return nil | |
| 1308 | + | } | |
| 1306 | 1309 | return w.b | |
| 1307 | 1310 | } | |
| 1308 | 1311 | ||
| @@ -84,17 +84,18 @@ func validStartupOverlay(overlay StartupOverlay) bool { | |||||
| 84 | } | 84 | } | ||
| 85 | 85 | ||||
| 86 | // ValidateNavigation validates route capabilities independently of the rest of | 86 | // ValidateNavigation validates route capabilities independently of the rest of | ||
| 87 | - | // the Hello payload. | 87 | + | // the Hello payload. homePickerRoute indicates that the route may open its |
| 88 | - | func ValidateNavigation(capabilities NavigationCapabilities, overlay StartupOverlay, remoteTarget bool) error { | 88 | + | // home/session picker (for example, a remote target or daemon-owned route). |
| 89 | + | func ValidateNavigation(capabilities NavigationCapabilities, overlay StartupOverlay, homePickerRoute bool) error { | |||
| 89 | if !validNavigationCapabilities(capabilities) || !validStartupOverlay(overlay) { | 90 | if !validNavigationCapabilities(capabilities) || !validStartupOverlay(overlay) { | ||
| 90 | return ErrInvalidNavigation | 91 | return ErrInvalidNavigation | ||
| 91 | } | 92 | } | ||
| 92 | home := capabilities&NavigationCapabilityHomePicker != 0 | 93 | home := capabilities&NavigationCapabilityHomePicker != 0 | ||
| 93 | back := capabilities&NavigationCapabilityBack != 0 | 94 | back := capabilities&NavigationCapabilityBack != 0 | ||
| 94 | - | if (home && !remoteTarget) || back != (overlay == StartupOverlaySessionPicker) { | 95 | + | if (home && !homePickerRoute) || back != (overlay == StartupOverlaySessionPicker) { |
| 95 | return ErrInvalidNavigation | 96 | return ErrInvalidNavigation | ||
| 96 | } | 97 | } | ||
| 97 | - | if remoteTarget && back { | 98 | + | if homePickerRoute && back { |
| 98 | return ErrInvalidNavigation | 99 | return ErrInvalidNavigation | ||
| 99 | } | 100 | } | ||
| 100 | return nil | 101 | return nil | ||
| @@ -1302,7 +1303,9 @@ func MarshalWelcome(m Welcome) []byte { | |||||
| 1302 | } | 1303 | } | ||
| 1303 | w.putUint64(m.ResumeToken) | 1304 | w.putUint64(m.ResumeToken) | ||
| 1304 | w.putUint32(m.Capabilities) | 1305 | w.putUint32(m.Capabilities) | ||
| 1305 | - | marshalCommittedIdentitySection(&w, m.CommittedIdentity) | 1306 | + | if !marshalCommittedIdentitySection(&w, m.CommittedIdentity) { |
| 1307 | + | return nil | |||
| 1308 | + | } | |||
| 1306 | return w.b | 1309 | return w.b | ||
| 1307 | } | 1310 | } | ||
| 1308 | 1311 | ||||
M internal/ports/wire_test.go +18 -3
| @@ -53,6 +53,11 @@ func assertTrailingGarbageFails[T any](t *testing.T, full []byte, unmarshal func | |||
| 53 | 53 | }) | |
| 54 | 54 | } | |
| 55 | 55 | ||
| 56 | + | func appendNoNavigationTail(payload []byte) []byte { | |
| 57 | + | // exact-target absent, navigation capabilities none, startup overlay none | |
| 58 | + | return append(payload, 0, 0, 0) | |
| 59 | + | } | |
| 60 | + | ||
| 56 | 61 | func TestHelloGoldenAndRoundTrip(t *testing.T) { | |
| 57 | 62 | tests := []struct { | |
| 58 | 63 | name string | |
| @@ -92,7 +97,7 @@ func TestHelloGoldenAndRoundTrip(t *testing.T) { | |||
| 92 | 97 | ||
| 93 | 98 | for _, tt := range tests { | |
| 94 | 99 | t.Run(tt.name, func(t *testing.T) { | |
| 95 | - | want := append(append([]byte(nil), tt.want...), 0, 0, 0) | |
| 100 | + | want := appendNoNavigationTail(append([]byte(nil), tt.want...)) | |
| 96 | 101 | got := MarshalHello(tt.msg) | |
| 97 | 102 | if !bytes.Equal(got, want) { | |
| 98 | 103 | t.Fatalf("MarshalHello() = %#v, want %#v", got, want) | |
| @@ -139,7 +144,7 @@ func TestHelloEnvironmentCodec(t *testing.T) { | |||
| 139 | 144 | 0x00, 0x00, 0x00, 0x06, 'X', 'Y', '=', '1', '2', '3', | |
| 140 | 145 | 0x00, 0x00, // no remote target, client-owned environment | |
| 141 | 146 | } | |
| 142 | - | want = append(want, 0, 0, 0) | |
| 147 | + | want = appendNoNavigationTail(want) | |
| 143 | 148 | require.Equal(t, want, got) | |
| 144 | 149 | decoded, err := UnmarshalHello(want) | |
| 145 | 150 | require.NoError(t, err) | |
| @@ -415,6 +420,16 @@ func TestPongGoldenAndRoundTrip(t *testing.T) { | |||
| 415 | 420 | assertTrailingGarbageFails(t, got, UnmarshalPong) | |
| 416 | 421 | } | |
| 417 | 422 | ||
| 423 | + | func TestMarshalWelcomeRejectsInvalidCommittedIdentity(t *testing.T) { | |
| 424 | + | got := MarshalWelcome(Welcome{ | |
| 425 | + | SessionName: "work", | |
| 426 | + | CommittedIdentity: &CommittedRouteIdentity{ | |
| 427 | + | Target: ExactSessionTarget{SessionName: "different"}, | |
| 428 | + | }, | |
| 429 | + | }) | |
| 430 | + | require.Nil(t, got) | |
| 431 | + | } | |
| 432 | + | ||
| 418 | 433 | func TestWelcomeGoldenAndRoundTrip(t *testing.T) { | |
| 419 | 434 | tests := []struct { | |
| 420 | 435 | name string | |
| @@ -894,7 +909,7 @@ func TestHelloOutputWindowByteExactValues(t *testing.T) { | |||
| 894 | 909 | hello := Hello{Version: 14, Size: domain.Size{Cols: 1, Rows: 1}, MaxOutputInFlight: window} | |
| 895 | 910 | // The empty Hello has a fixed 1x1 size before the negotiated output window. | |
| 896 | 911 | want := append(make([]byte, 38), window, 0, 0, 0, 0, 0, 0) | |
| 897 | - | want = append(want, 0, 0, 0) | |
| 912 | + | want = appendNoNavigationTail(want) | |
| 898 | 913 | want[1] = 14 | |
| 899 | 914 | want[30], want[32] = 1, 1 | |
| 900 | 915 | got := MarshalHello(hello) | |
| @@ -53,6 +53,11 @@ func assertTrailingGarbageFails[T any](t *testing.T, full []byte, unmarshal func | |||||
| 53 | }) | 53 | }) | ||
| 54 | } | 54 | } | ||
| 55 | 55 | ||||
| 56 | + | func appendNoNavigationTail(payload []byte) []byte { | |||
| 57 | + | // exact-target absent, navigation capabilities none, startup overlay none | |||
| 58 | + | return append(payload, 0, 0, 0) | |||
| 59 | + | } | |||
| 60 | + | ||||
| 56 | func TestHelloGoldenAndRoundTrip(t *testing.T) { | 61 | func TestHelloGoldenAndRoundTrip(t *testing.T) { | ||
| 57 | tests := []struct { | 62 | tests := []struct { | ||
| 58 | name string | 63 | name string | ||
| @@ -92,7 +97,7 @@ func TestHelloGoldenAndRoundTrip(t *testing.T) { | |||||
| 92 | 97 | ||||
| 93 | for _, tt := range tests { | 98 | for _, tt := range tests { | ||
| 94 | t.Run(tt.name, func(t *testing.T) { | 99 | t.Run(tt.name, func(t *testing.T) { | ||
| 95 | - | want := append(append([]byte(nil), tt.want...), 0, 0, 0) | 100 | + | want := appendNoNavigationTail(append([]byte(nil), tt.want...)) |
| 96 | got := MarshalHello(tt.msg) | 101 | got := MarshalHello(tt.msg) | ||
| 97 | if !bytes.Equal(got, want) { | 102 | if !bytes.Equal(got, want) { | ||
| 98 | t.Fatalf("MarshalHello() = %#v, want %#v", got, want) | 103 | t.Fatalf("MarshalHello() = %#v, want %#v", got, want) | ||
| @@ -139,7 +144,7 @@ func TestHelloEnvironmentCodec(t *testing.T) { | |||||
| 139 | 0x00, 0x00, 0x00, 0x06, 'X', 'Y', '=', '1', '2', '3', | 144 | 0x00, 0x00, 0x00, 0x06, 'X', 'Y', '=', '1', '2', '3', | ||
| 140 | 0x00, 0x00, // no remote target, client-owned environment | 145 | 0x00, 0x00, // no remote target, client-owned environment | ||
| 141 | } | 146 | } | ||
| 142 | - | want = append(want, 0, 0, 0) | 147 | + | want = appendNoNavigationTail(want) |
| 143 | require.Equal(t, want, got) | 148 | require.Equal(t, want, got) | ||
| 144 | decoded, err := UnmarshalHello(want) | 149 | decoded, err := UnmarshalHello(want) | ||
| 145 | require.NoError(t, err) | 150 | require.NoError(t, err) | ||
| @@ -415,6 +420,16 @@ func TestPongGoldenAndRoundTrip(t *testing.T) { | |||||
| 415 | assertTrailingGarbageFails(t, got, UnmarshalPong) | 420 | assertTrailingGarbageFails(t, got, UnmarshalPong) | ||
| 416 | } | 421 | } | ||
| 417 | 422 | ||||
| 423 | + | func TestMarshalWelcomeRejectsInvalidCommittedIdentity(t *testing.T) { | |||
| 424 | + | got := MarshalWelcome(Welcome{ | |||
| 425 | + | SessionName: "work", | |||
| 426 | + | CommittedIdentity: &CommittedRouteIdentity{ | |||
| 427 | + | Target: ExactSessionTarget{SessionName: "different"}, | |||
| 428 | + | }, | |||
| 429 | + | }) | |||
| 430 | + | require.Nil(t, got) | |||
| 431 | + | } | |||
| 432 | + | ||||
| 418 | func TestWelcomeGoldenAndRoundTrip(t *testing.T) { | 433 | func TestWelcomeGoldenAndRoundTrip(t *testing.T) { | ||
| 419 | tests := []struct { | 434 | tests := []struct { | ||
| 420 | name string | 435 | name string | ||
| @@ -894,7 +909,7 @@ func TestHelloOutputWindowByteExactValues(t *testing.T) { | |||||
| 894 | hello := Hello{Version: 14, Size: domain.Size{Cols: 1, Rows: 1}, MaxOutputInFlight: window} | 909 | hello := Hello{Version: 14, Size: domain.Size{Cols: 1, Rows: 1}, MaxOutputInFlight: window} | ||
| 895 | // The empty Hello has a fixed 1x1 size before the negotiated output window. | 910 | // The empty Hello has a fixed 1x1 size before the negotiated output window. | ||
| 896 | want := append(make([]byte, 38), window, 0, 0, 0, 0, 0, 0) | 911 | want := append(make([]byte, 38), window, 0, 0, 0, 0, 0, 0) | ||
| 897 | - | want = append(want, 0, 0, 0) | 912 | + | want = appendNoNavigationTail(want) |
| 898 | want[1] = 14 | 913 | want[1] = 14 | ||
| 899 | want[30], want[32] = 1, 1 | 914 | want[30], want[32] = 1, 1 | ||
| 900 | got := MarshalHello(hello) | 915 | got := MarshalHello(hello) | ||
M internal/usecase/client/client.go +20 -17
| @@ -405,6 +405,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 405 | 405 | attemptRequest.Intent = ports.IntentAttach | |
| 406 | 406 | } | |
| 407 | 407 | attemptRequest.StartupOverlay = ports.StartupOverlayNone | |
| 408 | + | attemptRequest.NavigationCapabilities &^= ports.NavigationCapabilityBack | ports.NavigationCapabilityHomePicker | |
| 408 | 409 | resumeToken = route.resumeToken | |
| 409 | 410 | returnResumeFallback = route.resumeToken != 0 | |
| 410 | 411 | remote = syncReconnectRemote(reconnect, attemptRequest.Remote || r.remote) | |
| @@ -623,22 +624,6 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 623 | 624 | backoff = defaultReconnectBackoff.initial | |
| 624 | 625 | continue | |
| 625 | 626 | } | |
| 626 | - | if result.resumeToken != 0 { | |
| 627 | - | resumeToken = result.resumeToken | |
| 628 | - | } | |
| 629 | - | if result.sessionName != "" { | |
| 630 | - | attemptRequest.SessionName = result.sessionName | |
| 631 | - | } | |
| 632 | - | if result.committedIdentity != nil { | |
| 633 | - | identity := *result.committedIdentity | |
| 634 | - | if attemptRequest.ExactTarget != nil && *attemptRequest.ExactTarget != identity.Target { | |
| 635 | - | attemptRequest.RemoteTarget = nil | |
| 636 | - | } | |
| 637 | - | attemptRequest.ExactTarget = &identity.Target | |
| 638 | - | } | |
| 639 | - | if result.welcomed { | |
| 640 | - | homeNavigationPending = false | |
| 641 | - | } | |
| 642 | 627 | if result.err == nil { | |
| 643 | 628 | if result.welcomed && routeNavigationPending { | |
| 644 | 629 | routeNavigationPending = false | |
| @@ -655,6 +640,22 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 655 | 640 | } | |
| 656 | 641 | return nil | |
| 657 | 642 | } | |
| 643 | + | if result.resumeToken != 0 { | |
| 644 | + | resumeToken = result.resumeToken | |
| 645 | + | } | |
| 646 | + | if result.sessionName != "" { | |
| 647 | + | attemptRequest.SessionName = result.sessionName | |
| 648 | + | } | |
| 649 | + | if result.committedIdentity != nil { | |
| 650 | + | identity := *result.committedIdentity | |
| 651 | + | if attemptRequest.ExactTarget != nil && *attemptRequest.ExactTarget != identity.Target { | |
| 652 | + | attemptRequest.RemoteTarget = nil | |
| 653 | + | } | |
| 654 | + | attemptRequest.ExactTarget = &identity.Target | |
| 655 | + | } | |
| 656 | + | if result.welcomed { | |
| 657 | + | homeNavigationPending = false | |
| 658 | + | } | |
| 658 | 659 | if homeNavigationPending && returnRoute != nil { | |
| 659 | 660 | route := *returnRoute | |
| 660 | 661 | returnRoute = nil | |
| @@ -1545,9 +1546,11 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||
| 1545 | 1546 | return welcomedResult(fmt.Errorf("vev: publishing daemon-local route snapshot: %w", derr)) | |
| 1546 | 1547 | } | |
| 1547 | 1548 | case ports.MsgRouteNavigationFailure: | |
| 1548 | - | if _, derr := ports.UnmarshalRouteNavigationFailure(r.frame.Payload); derr != nil { | |
| 1549 | + | failure, derr := ports.UnmarshalRouteNavigationFailure(r.frame.Payload) | |
| 1550 | + | if derr != nil { | |
| 1549 | 1551 | return welcomedResult(fmt.Errorf("vev: decoding route navigation failure: %w", derr)) | |
| 1550 | 1552 | } | |
| 1553 | + | log.Warn("route navigation rejected", "key", failure.Key, "generation", failure.Generation, "code", failure.Code) | |
| 1551 | 1554 | case ports.MsgDetached: | |
| 1552 | 1555 | d, derr := ports.UnmarshalDetached(r.frame.Payload) | |
| 1553 | 1556 | if derr != nil { | |
| @@ -405,6 +405,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 405 | attemptRequest.Intent = ports.IntentAttach | 405 | attemptRequest.Intent = ports.IntentAttach | ||
| 406 | } | 406 | } | ||
| 407 | attemptRequest.StartupOverlay = ports.StartupOverlayNone | 407 | attemptRequest.StartupOverlay = ports.StartupOverlayNone | ||
| 408 | + | attemptRequest.NavigationCapabilities &^= ports.NavigationCapabilityBack | ports.NavigationCapabilityHomePicker | |||
| 408 | resumeToken = route.resumeToken | 409 | resumeToken = route.resumeToken | ||
| 409 | returnResumeFallback = route.resumeToken != 0 | 410 | returnResumeFallback = route.resumeToken != 0 | ||
| 410 | remote = syncReconnectRemote(reconnect, attemptRequest.Remote || r.remote) | 411 | remote = syncReconnectRemote(reconnect, attemptRequest.Remote || r.remote) | ||
| @@ -623,22 +624,6 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 623 | backoff = defaultReconnectBackoff.initial | 624 | backoff = defaultReconnectBackoff.initial | ||
| 624 | continue | 625 | continue | ||
| 625 | } | 626 | } | ||
| 626 | - | if result.resumeToken != 0 { | |||
| 627 | - | resumeToken = result.resumeToken | |||
| 628 | - | } | |||
| 629 | - | if result.sessionName != "" { | |||
| 630 | - | attemptRequest.SessionName = result.sessionName | |||
| 631 | - | } | |||
| 632 | - | if result.committedIdentity != nil { | |||
| 633 | - | identity := *result.committedIdentity | |||
| 634 | - | if attemptRequest.ExactTarget != nil && *attemptRequest.ExactTarget != identity.Target { | |||
| 635 | - | attemptRequest.RemoteTarget = nil | |||
| 636 | - | } | |||
| 637 | - | attemptRequest.ExactTarget = &identity.Target | |||
| 638 | - | } | |||
| 639 | - | if result.welcomed { | |||
| 640 | - | homeNavigationPending = false | |||
| 641 | - | } | |||
| 642 | if result.err == nil { | 627 | if result.err == nil { | ||
| 643 | if result.welcomed && routeNavigationPending { | 628 | if result.welcomed && routeNavigationPending { | ||
| 644 | routeNavigationPending = false | 629 | routeNavigationPending = false | ||
| @@ -655,6 +640,22 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 655 | } | 640 | } | ||
| 656 | return nil | 641 | return nil | ||
| 657 | } | 642 | } | ||
| 643 | + | if result.resumeToken != 0 { | |||
| 644 | + | resumeToken = result.resumeToken | |||
| 645 | + | } | |||
| 646 | + | if result.sessionName != "" { | |||
| 647 | + | attemptRequest.SessionName = result.sessionName | |||
| 648 | + | } | |||
| 649 | + | if result.committedIdentity != nil { | |||
| 650 | + | identity := *result.committedIdentity | |||
| 651 | + | if attemptRequest.ExactTarget != nil && *attemptRequest.ExactTarget != identity.Target { | |||
| 652 | + | attemptRequest.RemoteTarget = nil | |||
| 653 | + | } | |||
| 654 | + | attemptRequest.ExactTarget = &identity.Target | |||
| 655 | + | } | |||
| 656 | + | if result.welcomed { | |||
| 657 | + | homeNavigationPending = false | |||
| 658 | + | } | |||
| 658 | if homeNavigationPending && returnRoute != nil { | 659 | if homeNavigationPending && returnRoute != nil { | ||
| 659 | route := *returnRoute | 660 | route := *returnRoute | ||
| 660 | returnRoute = nil | 661 | returnRoute = nil | ||
| @@ -1545,9 +1546,11 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||||
| 1545 | return welcomedResult(fmt.Errorf("vev: publishing daemon-local route snapshot: %w", derr)) | 1546 | return welcomedResult(fmt.Errorf("vev: publishing daemon-local route snapshot: %w", derr)) | ||
| 1546 | } | 1547 | } | ||
| 1547 | case ports.MsgRouteNavigationFailure: | 1548 | case ports.MsgRouteNavigationFailure: | ||
| 1548 | - | if _, derr := ports.UnmarshalRouteNavigationFailure(r.frame.Payload); derr != nil { | 1549 | + | failure, derr := ports.UnmarshalRouteNavigationFailure(r.frame.Payload) |
| 1550 | + | if derr != nil { | |||
| 1549 | return welcomedResult(fmt.Errorf("vev: decoding route navigation failure: %w", derr)) | 1551 | return welcomedResult(fmt.Errorf("vev: decoding route navigation failure: %w", derr)) | ||
| 1550 | } | 1552 | } | ||
| 1553 | + | log.Warn("route navigation rejected", "key", failure.Key, "generation", failure.Generation, "code", failure.Code) | |||
| 1551 | case ports.MsgDetached: | 1554 | case ports.MsgDetached: | ||
| 1552 | d, derr := ports.UnmarshalDetached(r.frame.Payload) | 1555 | d, derr := ports.UnmarshalDetached(r.frame.Payload) | ||
| 1553 | if derr != nil { | 1556 | if derr != nil { | ||
M internal/usecase/client/routes.go +3 -3
| @@ -101,8 +101,8 @@ func cloneCommittedIdentity(identity *ports.CommittedRouteIdentity) *ports.Commi | |||
| 101 | 101 | if identity == nil { | |
| 102 | 102 | return nil | |
| 103 | 103 | } | |
| 104 | - | copy := *identity | |
| 105 | - | return © | |
| 104 | + | clone := *identity | |
| 105 | + | return &clone | |
| 106 | 106 | } | |
| 107 | 107 | ||
| 108 | 108 | func routeCandidateForAttach(request AttachRequest, identity ports.CommittedRouteIdentity, dialer ports.Dialer, resumeToken uint64) routeCandidate { | |
| @@ -553,7 +553,7 @@ func (l *routeLedger) navigate(ctx context.Context, action ports.RouteNavigation | |||
| 553 | 553 | return errors.New("route transition connector is nil") | |
| 554 | 554 | } | |
| 555 | 555 | if ctx == nil { | |
| 556 | - | ctx = context.Background() | |
| 556 | + | return errors.New("route transition context is nil") | |
| 557 | 557 | } | |
| 558 | 558 | selected := ports.RouteRef{Key: action.Key, Generation: action.Generation} | |
| 559 | 559 | plan := routeTransitionPlan{ | |
| @@ -101,8 +101,8 @@ func cloneCommittedIdentity(identity *ports.CommittedRouteIdentity) *ports.Commi | |||||
| 101 | if identity == nil { | 101 | if identity == nil { | ||
| 102 | return nil | 102 | return nil | ||
| 103 | } | 103 | } | ||
| 104 | - | copy := *identity | 104 | + | clone := *identity |
| 105 | - | return © | 105 | + | return &clone |
| 106 | } | 106 | } | ||
| 107 | 107 | ||||
| 108 | func routeCandidateForAttach(request AttachRequest, identity ports.CommittedRouteIdentity, dialer ports.Dialer, resumeToken uint64) routeCandidate { | 108 | func routeCandidateForAttach(request AttachRequest, identity ports.CommittedRouteIdentity, dialer ports.Dialer, resumeToken uint64) routeCandidate { | ||
| @@ -553,7 +553,7 @@ func (l *routeLedger) navigate(ctx context.Context, action ports.RouteNavigation | |||||
| 553 | return errors.New("route transition connector is nil") | 553 | return errors.New("route transition connector is nil") | ||
| 554 | } | 554 | } | ||
| 555 | if ctx == nil { | 555 | if ctx == nil { | ||
| 556 | - | ctx = context.Background() | 556 | + | return errors.New("route transition context is nil") |
| 557 | } | 557 | } | ||
| 558 | selected := ports.RouteRef{Key: action.Key, Generation: action.Generation} | 558 | selected := ports.RouteRef{Key: action.Key, Generation: action.Generation} | ||
| 559 | plan := routeTransitionPlan{ | 559 | plan := routeTransitionPlan{ | ||
M internal/usecase/client/routes_test.go +1 -3
| @@ -80,7 +80,7 @@ func TestCommittedIdentityDoesNotReassignHomeRoute(t *testing.T) { | |||
| 80 | 80 | Ephemeral: true, | |
| 81 | 81 | }) | |
| 82 | 82 | require.NoError(t, err) | |
| 83 | - | require.Equal(t, uint64(homeIdentity.key), uint64(ledger.homeRef().Key)) | |
| 83 | + | require.Equal(t, uint64(homeIdentity.key), ledger.homeRef().Key) | |
| 84 | 84 | require.NotEqual(t, ledger.homeRef(), committed.wire()) | |
| 85 | 85 | active, ok := ledger.lookup(ledger.activeRef()) | |
| 86 | 86 | require.True(t, ok) | |
| @@ -97,7 +97,6 @@ func TestRouteLedgerConcurrentInitialAttachmentsKeepOneHome(t *testing.T) { | |||
| 97 | 97 | errs := make(chan error, attachments) | |
| 98 | 98 | var wg sync.WaitGroup | |
| 99 | 99 | for i := byte(0); i < attachments; i++ { | |
| 100 | - | i := i | |
| 101 | 100 | wg.Add(1) | |
| 102 | 101 | go func() { | |
| 103 | 102 | defer wg.Done() | |
| @@ -301,7 +300,6 @@ func TestRouteLedgerConcurrentSnapshotsAndCommits(t *testing.T) { | |||
| 301 | 300 | ledger := newRouteLedger() | |
| 302 | 301 | var wg sync.WaitGroup | |
| 303 | 302 | for worker := byte(0); worker < 4; worker++ { | |
| 304 | - | worker := worker | |
| 305 | 303 | wg.Add(1) | |
| 306 | 304 | go func() { | |
| 307 | 305 | defer wg.Done() | |
| @@ -80,7 +80,7 @@ func TestCommittedIdentityDoesNotReassignHomeRoute(t *testing.T) { | |||||
| 80 | Ephemeral: true, | 80 | Ephemeral: true, | ||
| 81 | }) | 81 | }) | ||
| 82 | require.NoError(t, err) | 82 | require.NoError(t, err) | ||
| 83 | - | require.Equal(t, uint64(homeIdentity.key), uint64(ledger.homeRef().Key)) | 83 | + | require.Equal(t, uint64(homeIdentity.key), ledger.homeRef().Key) |
| 84 | require.NotEqual(t, ledger.homeRef(), committed.wire()) | 84 | require.NotEqual(t, ledger.homeRef(), committed.wire()) | ||
| 85 | active, ok := ledger.lookup(ledger.activeRef()) | 85 | active, ok := ledger.lookup(ledger.activeRef()) | ||
| 86 | require.True(t, ok) | 86 | require.True(t, ok) | ||
| @@ -97,7 +97,6 @@ func TestRouteLedgerConcurrentInitialAttachmentsKeepOneHome(t *testing.T) { | |||||
| 97 | errs := make(chan error, attachments) | 97 | errs := make(chan error, attachments) | ||
| 98 | var wg sync.WaitGroup | 98 | var wg sync.WaitGroup | ||
| 99 | for i := byte(0); i < attachments; i++ { | 99 | for i := byte(0); i < attachments; i++ { | ||
| 100 | - | i := i | |||
| 101 | wg.Add(1) | 100 | wg.Add(1) | ||
| 102 | go func() { | 101 | go func() { | ||
| 103 | defer wg.Done() | 102 | defer wg.Done() | ||
| @@ -301,7 +300,6 @@ func TestRouteLedgerConcurrentSnapshotsAndCommits(t *testing.T) { | |||||
| 301 | ledger := newRouteLedger() | 300 | ledger := newRouteLedger() | ||
| 302 | var wg sync.WaitGroup | 301 | var wg sync.WaitGroup | ||
| 303 | for worker := byte(0); worker < 4; worker++ { | 302 | for worker := byte(0); worker < 4; worker++ { | ||
| 304 | - | worker := worker | |||
| 305 | wg.Add(1) | 303 | wg.Add(1) | ||
| 306 | go func() { | 304 | go func() { | ||
| 307 | defer wg.Done() | 305 | defer wg.Done() | ||
M internal/usecase/client/session_connection_test.go +8 -4
| @@ -27,12 +27,13 @@ func TestSessionConnectionUsesOneAttachShapeForLocalAndRemoteTransports(t *testi | |||
| 27 | 27 | ||
| 28 | 28 | func TestSessionTargetValidation(t *testing.T) { | |
| 29 | 29 | tests := []struct { | |
| 30 | - | name string | |
| 31 | - | target client.SessionTarget | |
| 32 | - | wantErr bool | |
| 30 | + | name string | |
| 31 | + | target client.SessionTarget | |
| 32 | + | wantErr bool | |
| 33 | + | wantErrIs error | |
| 33 | 34 | }{ | |
| 34 | 35 | {name: "ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral}}, | |
| 35 | - | {name: "named ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral, SessionName: "work"}, wantErr: true}, | |
| 36 | + | {name: "named ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral, SessionName: "work"}, wantErr: true, wantErrIs: client.ErrEphemeralSessionName}, | |
| 36 | 37 | {name: "new", target: client.SessionTarget{Intent: ports.IntentNew, SessionName: "work"}}, | |
| 37 | 38 | {name: "attach", target: client.SessionTarget{Intent: ports.IntentAttach, SessionName: "work"}}, | |
| 38 | 39 | {name: "resume", target: client.SessionTarget{Intent: ports.IntentResume, SessionName: "work"}}, | |
| @@ -46,6 +47,9 @@ func TestSessionTargetValidation(t *testing.T) { | |||
| 46 | 47 | _, err := client.NewSessionConnection(transport, tt.target) | |
| 47 | 48 | if tt.wantErr { | |
| 48 | 49 | require.Error(t, err) | |
| 50 | + | if tt.wantErrIs != nil { | |
| 51 | + | require.ErrorIs(t, err, tt.wantErrIs) | |
| 52 | + | } | |
| 49 | 53 | return | |
| 50 | 54 | } | |
| 51 | 55 | require.NoError(t, err) | |
| @@ -27,12 +27,13 @@ func TestSessionConnectionUsesOneAttachShapeForLocalAndRemoteTransports(t *testi | |||||
| 27 | 27 | ||||
| 28 | func TestSessionTargetValidation(t *testing.T) { | 28 | func TestSessionTargetValidation(t *testing.T) { | ||
| 29 | tests := []struct { | 29 | tests := []struct { | ||
| 30 | - | name string | 30 | + | name string |
| 31 | - | target client.SessionTarget | 31 | + | target client.SessionTarget |
| 32 | - | wantErr bool | 32 | + | wantErr bool |
| 33 | + | wantErrIs error | |||
| 33 | }{ | 34 | }{ | ||
| 34 | {name: "ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral}}, | 35 | {name: "ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral}}, | ||
| 35 | - | {name: "named ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral, SessionName: "work"}, wantErr: true}, | 36 | + | {name: "named ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral, SessionName: "work"}, wantErr: true, wantErrIs: client.ErrEphemeralSessionName}, |
| 36 | {name: "new", target: client.SessionTarget{Intent: ports.IntentNew, SessionName: "work"}}, | 37 | {name: "new", target: client.SessionTarget{Intent: ports.IntentNew, SessionName: "work"}}, | ||
| 37 | {name: "attach", target: client.SessionTarget{Intent: ports.IntentAttach, SessionName: "work"}}, | 38 | {name: "attach", target: client.SessionTarget{Intent: ports.IntentAttach, SessionName: "work"}}, | ||
| 38 | {name: "resume", target: client.SessionTarget{Intent: ports.IntentResume, SessionName: "work"}}, | 39 | {name: "resume", target: client.SessionTarget{Intent: ports.IntentResume, SessionName: "work"}}, | ||
| @@ -46,6 +47,9 @@ func TestSessionTargetValidation(t *testing.T) { | |||||
| 46 | _, err := client.NewSessionConnection(transport, tt.target) | 47 | _, err := client.NewSessionConnection(transport, tt.target) | ||
| 47 | if tt.wantErr { | 48 | if tt.wantErr { | ||
| 48 | require.Error(t, err) | 49 | require.Error(t, err) | ||
| 50 | + | if tt.wantErrIs != nil { | |||
| 51 | + | require.ErrorIs(t, err, tt.wantErrIs) | |||
| 52 | + | } | |||
| 49 | return | 53 | return | ||
| 50 | } | 54 | } | ||
| 51 | require.NoError(t, err) | 55 | require.NoError(t, err) | ||
M internal/usecase/daemon/client.go +5 -2
| @@ -860,9 +860,12 @@ func (d *Daemon) applyThemeForAttachment(token attachmentConnectionToken, msg po | |||
| 860 | 860 | if !token.attachmentEffectCurrent() { | |
| 861 | 861 | return | |
| 862 | 862 | } | |
| 863 | - | if rc := sess.renderCoordinator(); rc != nil { | |
| 864 | - | rc.invalidateForLease(token.ac, token.lease, renderInvalidation{class: invalidateUrgent, reset: true, producer: "client.go"}) | |
| 863 | + | rc := sess.renderCoordinator() | |
| 864 | + | if token.lease == nil || rc == nil { | |
| 865 | + | d.invalidateRender(sess, token.ac, true, "client.go") | |
| 866 | + | return | |
| 865 | 867 | } | |
| 868 | + | rc.invalidateForLease(token.ac, token.lease, renderInvalidation{class: invalidateUrgent, reset: true, producer: "client.go"}) | |
| 866 | 869 | } | |
| 867 | 870 | ||
| 868 | 871 | func (d *Daemon) resize(sess *session, ac *attachedClient, sz domain.Size) { | |
| @@ -860,9 +860,12 @@ func (d *Daemon) applyThemeForAttachment(token attachmentConnectionToken, msg po | |||||
| 860 | if !token.attachmentEffectCurrent() { | 860 | if !token.attachmentEffectCurrent() { | ||
| 861 | return | 861 | return | ||
| 862 | } | 862 | } | ||
| 863 | - | if rc := sess.renderCoordinator(); rc != nil { | 863 | + | rc := sess.renderCoordinator() |
| 864 | - | rc.invalidateForLease(token.ac, token.lease, renderInvalidation{class: invalidateUrgent, reset: true, producer: "client.go"}) | 864 | + | if token.lease == nil || rc == nil { |
| 865 | + | d.invalidateRender(sess, token.ac, true, "client.go") | |||
| 866 | + | return | |||
| 865 | } | 867 | } | ||
| 868 | + | rc.invalidateForLease(token.ac, token.lease, renderInvalidation{class: invalidateUrgent, reset: true, producer: "client.go"}) | |||
| 866 | } | 869 | } | ||
| 867 | 870 | ||||
| 868 | func (d *Daemon) resize(sess *session, ac *attachedClient, sz domain.Size) { | 871 | func (d *Daemon) resize(sess *session, ac *attachedClient, sz domain.Size) { | ||
M internal/usecase/daemon/daemon.go +24 -10
| @@ -1206,6 +1206,7 @@ func (d *Daemon) handleHelloWithContext(handshakeCtx context.Context, timedOut < | |||
| 1206 | 1206 | paintToken := sess.attachmentToken(ac, tr) | |
| 1207 | 1207 | painted := make(chan bool, 1) | |
| 1208 | 1208 | paintDone, paintErr := boundedHandshakeOperationTracked(handshakeCtx, tr, func() error { | |
| 1209 | + | //nolint:contextcheck // firstPaintForTransition is capability-bounded; the surrounding handshake operation owns cancellation. | |
| 1209 | 1210 | painted <- d.firstPaintForTransition(paintToken) | |
| 1210 | 1211 | return nil | |
| 1211 | 1212 | }) | |
| @@ -1352,16 +1353,7 @@ func (d *Daemon) waitForTargetRestore(ctx context.Context, name string) error { | |||
| 1352 | 1353 | // route resolves a Hello to a session and a freshly attached client, creating | |
| 1353 | 1354 | // the session for ephemeral/new intents. Direct package callers retain the | |
| 1354 | 1355 | // daemon lifetime context; inbound handshakes use routeWithContext. | |
| 1355 | - | func (d *Daemon) validateExactSessionTarget(ctx context.Context, target ports.ExactSessionTarget) error { | |
| 1356 | - | if err := target.Validate(); err != nil { | |
| 1357 | - | return &protoErr{ports.ErrNoSuchSession, "invalid exact session target"} | |
| 1358 | - | } | |
| 1359 | - | if err := d.waitForTargetRestore(ctx, target.SessionName); err != nil { | |
| 1360 | - | return err | |
| 1361 | - | } | |
| 1362 | - | ||
| 1363 | - | d.mu.Lock() | |
| 1364 | - | defer d.mu.Unlock() | |
| 1356 | + | func (d *Daemon) validateExactSessionTargetLocked(target ports.ExactSessionTarget) error { | |
| 1365 | 1357 | if sess := d.findByNameLocked(target.SessionName); sess != nil { | |
| 1366 | 1358 | if sess.incarnation != target.LifecycleID { | |
| 1367 | 1359 | return &protoErr{ports.ErrNoSuchSession, "session lifecycle has changed: " + target.SessionName} | |
| @@ -1375,6 +1367,19 @@ func (d *Daemon) validateExactSessionTarget(ctx context.Context, target ports.Ex | |||
| 1375 | 1367 | return nil | |
| 1376 | 1368 | } | |
| 1377 | 1369 | ||
| 1370 | + | func (d *Daemon) validateExactSessionTarget(ctx context.Context, target ports.ExactSessionTarget) error { | |
| 1371 | + | if err := target.Validate(); err != nil { | |
| 1372 | + | return &protoErr{ports.ErrNoSuchSession, "invalid exact session target"} | |
| 1373 | + | } | |
| 1374 | + | if err := d.waitForTargetRestore(ctx, target.SessionName); err != nil { | |
| 1375 | + | return err | |
| 1376 | + | } | |
| 1377 | + | ||
| 1378 | + | d.mu.Lock() | |
| 1379 | + | defer d.mu.Unlock() | |
| 1380 | + | return d.validateExactSessionTargetLocked(target) | |
| 1381 | + | } | |
| 1382 | + | ||
| 1378 | 1383 | func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedClient, error) { | |
| 1379 | 1384 | ctx := d.serveCtx | |
| 1380 | 1385 | if ctx == nil { | |
| @@ -1462,6 +1467,15 @@ func (d *Daemon) routeWithContext(ctx context.Context, h ports.Hello, tr ports.T | |||
| 1462 | 1467 | d.mu.Unlock() | |
| 1463 | 1468 | return nil, nil, &protoErr{ports.ErrServerShutdown, "daemon is shutting down"} | |
| 1464 | 1469 | } | |
| 1470 | + | // The first exact-target check happens before restore I/O. Recheck while | |
| 1471 | + | // holding d.mu so a same-name lifecycle replacement cannot slip between | |
| 1472 | + | // validation and attachment creation. | |
| 1473 | + | if h.ExactTarget != nil && h.ResumeToken == 0 { | |
| 1474 | + | if err := d.validateExactSessionTargetLocked(*h.ExactTarget); err != nil { | |
| 1475 | + | d.mu.Unlock() | |
| 1476 | + | return nil, nil, err | |
| 1477 | + | } | |
| 1478 | + | } | |
| 1465 | 1479 | switch h.Intent { | |
| 1466 | 1480 | case ports.IntentEphemeral: | |
| 1467 | 1481 | name := d.allocEphemeralNameLocked() | |
| @@ -1206,6 +1206,7 @@ func (d *Daemon) handleHelloWithContext(handshakeCtx context.Context, timedOut < | |||||
| 1206 | paintToken := sess.attachmentToken(ac, tr) | 1206 | paintToken := sess.attachmentToken(ac, tr) | ||
| 1207 | painted := make(chan bool, 1) | 1207 | painted := make(chan bool, 1) | ||
| 1208 | paintDone, paintErr := boundedHandshakeOperationTracked(handshakeCtx, tr, func() error { | 1208 | paintDone, paintErr := boundedHandshakeOperationTracked(handshakeCtx, tr, func() error { | ||
| 1209 | + | //nolint:contextcheck // firstPaintForTransition is capability-bounded; the surrounding handshake operation owns cancellation. | |||
| 1209 | painted <- d.firstPaintForTransition(paintToken) | 1210 | painted <- d.firstPaintForTransition(paintToken) | ||
| 1210 | return nil | 1211 | return nil | ||
| 1211 | }) | 1212 | }) | ||
| @@ -1352,16 +1353,7 @@ func (d *Daemon) waitForTargetRestore(ctx context.Context, name string) error { | |||||
| 1352 | // route resolves a Hello to a session and a freshly attached client, creating | 1353 | // route resolves a Hello to a session and a freshly attached client, creating | ||
| 1353 | // the session for ephemeral/new intents. Direct package callers retain the | 1354 | // the session for ephemeral/new intents. Direct package callers retain the | ||
| 1354 | // daemon lifetime context; inbound handshakes use routeWithContext. | 1355 | // daemon lifetime context; inbound handshakes use routeWithContext. | ||
| 1355 | - | func (d *Daemon) validateExactSessionTarget(ctx context.Context, target ports.ExactSessionTarget) error { | 1356 | + | func (d *Daemon) validateExactSessionTargetLocked(target ports.ExactSessionTarget) error { |
| 1356 | - | if err := target.Validate(); err != nil { | |||
| 1357 | - | return &protoErr{ports.ErrNoSuchSession, "invalid exact session target"} | |||
| 1358 | - | } | |||
| 1359 | - | if err := d.waitForTargetRestore(ctx, target.SessionName); err != nil { | |||
| 1360 | - | return err | |||
| 1361 | - | } | |||
| 1362 | - | ||||
| 1363 | - | d.mu.Lock() | |||
| 1364 | - | defer d.mu.Unlock() | |||
| 1365 | if sess := d.findByNameLocked(target.SessionName); sess != nil { | 1357 | if sess := d.findByNameLocked(target.SessionName); sess != nil { | ||
| 1366 | if sess.incarnation != target.LifecycleID { | 1358 | if sess.incarnation != target.LifecycleID { | ||
| 1367 | return &protoErr{ports.ErrNoSuchSession, "session lifecycle has changed: " + target.SessionName} | 1359 | return &protoErr{ports.ErrNoSuchSession, "session lifecycle has changed: " + target.SessionName} | ||
| @@ -1375,6 +1367,19 @@ func (d *Daemon) validateExactSessionTarget(ctx context.Context, target ports.Ex | |||||
| 1375 | return nil | 1367 | return nil | ||
| 1376 | } | 1368 | } | ||
| 1377 | 1369 | ||||
| 1370 | + | func (d *Daemon) validateExactSessionTarget(ctx context.Context, target ports.ExactSessionTarget) error { | |||
| 1371 | + | if err := target.Validate(); err != nil { | |||
| 1372 | + | return &protoErr{ports.ErrNoSuchSession, "invalid exact session target"} | |||
| 1373 | + | } | |||
| 1374 | + | if err := d.waitForTargetRestore(ctx, target.SessionName); err != nil { | |||
| 1375 | + | return err | |||
| 1376 | + | } | |||
| 1377 | + | ||||
| 1378 | + | d.mu.Lock() | |||
| 1379 | + | defer d.mu.Unlock() | |||
| 1380 | + | return d.validateExactSessionTargetLocked(target) | |||
| 1381 | + | } | |||
| 1382 | + | ||||
| 1378 | func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedClient, error) { | 1383 | func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedClient, error) { | ||
| 1379 | ctx := d.serveCtx | 1384 | ctx := d.serveCtx | ||
| 1380 | if ctx == nil { | 1385 | if ctx == nil { | ||
| @@ -1462,6 +1467,15 @@ func (d *Daemon) routeWithContext(ctx context.Context, h ports.Hello, tr ports.T | |||||
| 1462 | d.mu.Unlock() | 1467 | d.mu.Unlock() | ||
| 1463 | return nil, nil, &protoErr{ports.ErrServerShutdown, "daemon is shutting down"} | 1468 | return nil, nil, &protoErr{ports.ErrServerShutdown, "daemon is shutting down"} | ||
| 1464 | } | 1469 | } | ||
| 1470 | + | // The first exact-target check happens before restore I/O. Recheck while | |||
| 1471 | + | // holding d.mu so a same-name lifecycle replacement cannot slip between | |||
| 1472 | + | // validation and attachment creation. | |||
| 1473 | + | if h.ExactTarget != nil && h.ResumeToken == 0 { | |||
| 1474 | + | if err := d.validateExactSessionTargetLocked(*h.ExactTarget); err != nil { | |||
| 1475 | + | d.mu.Unlock() | |||
| 1476 | + | return nil, nil, err | |||
| 1477 | + | } | |||
| 1478 | + | } | |||
| 1465 | switch h.Intent { | 1479 | switch h.Intent { | ||
| 1466 | case ports.IntentEphemeral: | 1480 | case ports.IntentEphemeral: | ||
| 1467 | name := d.allocEphemeralNameLocked() | 1481 | name := d.allocEphemeralNameLocked() | ||
M internal/usecase/daemon/exact_route_test.go +19 -0
| @@ -31,6 +31,25 @@ func TestRouteExactSessionTargetSelectsLifecycle(t *testing.T) { | |||
| 31 | 31 | d.clientGone(sess, ac, tr, false) | |
| 32 | 32 | } | |
| 33 | 33 | ||
| 34 | + | func TestLockedExactSessionTargetRejectsReplacement(t *testing.T) { | |
| 35 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 36 | + | sess := addControlSession(d, "work", "tab-1", "pane-1") | |
| 37 | + | sess.ephemeral = false | |
| 38 | + | sess.incarnation = remoteLifecycleForTest() | |
| 39 | + | target := ports.ExactSessionTarget{LifecycleID: sess.incarnation, SessionName: sess.name} | |
| 40 | + | replacement := sess.incarnation | |
| 41 | + | replacement[0]++ | |
| 42 | + | sess.incarnation = replacement | |
| 43 | + | ||
| 44 | + | d.mu.Lock() | |
| 45 | + | err := d.validateExactSessionTargetLocked(target) | |
| 46 | + | d.mu.Unlock() | |
| 47 | + | ||
| 48 | + | var protocol *protoErr | |
| 49 | + | require.ErrorAs(t, err, &protocol) | |
| 50 | + | require.Equal(t, ports.ErrNoSuchSession, protocol.code) | |
| 51 | + | } | |
| 52 | + | ||
| 34 | 53 | func TestRouteExactSessionTargetRejectsLifecycleReplacement(t *testing.T) { | |
| 35 | 54 | d := newTestDaemon(t, nil, stubClock{}) | |
| 36 | 55 | sess := addControlSession(d, "work", "tab-1", "pane-1") | |
| @@ -31,6 +31,25 @@ func TestRouteExactSessionTargetSelectsLifecycle(t *testing.T) { | |||||
| 31 | d.clientGone(sess, ac, tr, false) | 31 | d.clientGone(sess, ac, tr, false) | ||
| 32 | } | 32 | } | ||
| 33 | 33 | ||||
| 34 | + | func TestLockedExactSessionTargetRejectsReplacement(t *testing.T) { | |||
| 35 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 36 | + | sess := addControlSession(d, "work", "tab-1", "pane-1") | |||
| 37 | + | sess.ephemeral = false | |||
| 38 | + | sess.incarnation = remoteLifecycleForTest() | |||
| 39 | + | target := ports.ExactSessionTarget{LifecycleID: sess.incarnation, SessionName: sess.name} | |||
| 40 | + | replacement := sess.incarnation | |||
| 41 | + | replacement[0]++ | |||
| 42 | + | sess.incarnation = replacement | |||
| 43 | + | ||||
| 44 | + | d.mu.Lock() | |||
| 45 | + | err := d.validateExactSessionTargetLocked(target) | |||
| 46 | + | d.mu.Unlock() | |||
| 47 | + | ||||
| 48 | + | var protocol *protoErr | |||
| 49 | + | require.ErrorAs(t, err, &protocol) | |||
| 50 | + | require.Equal(t, ports.ErrNoSuchSession, protocol.code) | |||
| 51 | + | } | |||
| 52 | + | ||||
| 34 | func TestRouteExactSessionTargetRejectsLifecycleReplacement(t *testing.T) { | 53 | func TestRouteExactSessionTargetRejectsLifecycleReplacement(t *testing.T) { | ||
| 35 | d := newTestDaemon(t, nil, stubClock{}) | 54 | d := newTestDaemon(t, nil, stubClock{}) | ||
| 36 | sess := addControlSession(d, "work", "tab-1", "pane-1") | 55 | sess := addControlSession(d, "work", "tab-1", "pane-1") | ||
M internal/usecase/daemon/input_test.go +2 -20
| @@ -804,32 +804,14 @@ func TestPaletteBackSessionSendsClientPreviousRouteAction(t *testing.T) { | |||
| 804 | 804 | require.Equal(t, ports.RouteNavigationAction{SnapshotGeneration: 3, Key: 2, Generation: 2}, action) | |
| 805 | 805 | } | |
| 806 | 806 | ||
| 807 | - | func TestPaletteBackSessionWithoutClientHistoryIsNoop(t *testing.T) { | |
| 808 | - | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |
| 809 | - | defer releaseAll(releases) | |
| 810 | - | ||
| 811 | - | d.backSession(current, ac) | |
| 812 | - | ||
| 813 | - | require.Same(t, current, ac.currentSession()) | |
| 814 | - | } | |
| 815 | - | ||
| 816 | - | func TestBackSessionWithoutSnapshotDoesNotReportDaemonHandoffFailure(t *testing.T) { | |
| 817 | - | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |
| 818 | - | defer releaseAll(releases) | |
| 819 | - | ||
| 820 | - | d.backSession(current, ac) | |
| 821 | - | ||
| 822 | - | require.Same(t, current, ac.currentSession()) | |
| 823 | - | require.Empty(t, d.notices.history()) | |
| 824 | - | } | |
| 825 | - | ||
| 826 | - | func TestBackSessionWithoutSnapshotDoesNotPublishFrames(t *testing.T) { | |
| 807 | + | func TestBackSessionWithoutClientHistoryIsNoop(t *testing.T) { | |
| 827 | 808 | d, current, ac, sends, releases := newRecentNavigationTestSessions(t) | |
| 828 | 809 | defer releaseAll(releases) | |
| 829 | 810 | ||
| 830 | 811 | d.backSession(current, ac) | |
| 831 | 812 | ||
| 832 | 813 | require.Same(t, current, ac.currentSession()) | |
| 814 | + | require.Empty(t, d.notices.history()) | |
| 833 | 815 | requireNoOutputFrame(t, sends) | |
| 834 | 816 | } | |
| 835 | 817 | ||
| @@ -804,32 +804,14 @@ func TestPaletteBackSessionSendsClientPreviousRouteAction(t *testing.T) { | |||||
| 804 | require.Equal(t, ports.RouteNavigationAction{SnapshotGeneration: 3, Key: 2, Generation: 2}, action) | 804 | require.Equal(t, ports.RouteNavigationAction{SnapshotGeneration: 3, Key: 2, Generation: 2}, action) | ||
| 805 | } | 805 | } | ||
| 806 | 806 | ||||
| 807 | - | func TestPaletteBackSessionWithoutClientHistoryIsNoop(t *testing.T) { | 807 | + | func TestBackSessionWithoutClientHistoryIsNoop(t *testing.T) { |
| 808 | - | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |||
| 809 | - | defer releaseAll(releases) | |||
| 810 | - | ||||
| 811 | - | d.backSession(current, ac) | |||
| 812 | - | ||||
| 813 | - | require.Same(t, current, ac.currentSession()) | |||
| 814 | - | } | |||
| 815 | - | ||||
| 816 | - | func TestBackSessionWithoutSnapshotDoesNotReportDaemonHandoffFailure(t *testing.T) { | |||
| 817 | - | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |||
| 818 | - | defer releaseAll(releases) | |||
| 819 | - | ||||
| 820 | - | d.backSession(current, ac) | |||
| 821 | - | ||||
| 822 | - | require.Same(t, current, ac.currentSession()) | |||
| 823 | - | require.Empty(t, d.notices.history()) | |||
| 824 | - | } | |||
| 825 | - | ||||
| 826 | - | func TestBackSessionWithoutSnapshotDoesNotPublishFrames(t *testing.T) { | |||
| 827 | d, current, ac, sends, releases := newRecentNavigationTestSessions(t) | 808 | d, current, ac, sends, releases := newRecentNavigationTestSessions(t) | ||
| 828 | defer releaseAll(releases) | 809 | defer releaseAll(releases) | ||
| 829 | 810 | ||||
| 830 | d.backSession(current, ac) | 811 | d.backSession(current, ac) | ||
| 831 | 812 | ||||
| 832 | require.Same(t, current, ac.currentSession()) | 813 | require.Same(t, current, ac.currentSession()) | ||
| 814 | + | require.Empty(t, d.notices.history()) | |||
| 833 | requireNoOutputFrame(t, sends) | 815 | requireNoOutputFrame(t, sends) | ||
| 834 | } | 816 | } | ||
| 835 | 817 | ||||
M internal/usecase/daemon/picker.go +36 -38
| @@ -946,18 +946,26 @@ func (d *Daemon) sendRemoteAttachTargetForAttachment(token attachmentConnectionT | |||
| 946 | 946 | return nil | |
| 947 | 947 | } | |
| 948 | 948 | ||
| 949 | + | func (d *Daemon) remoteCatalogEntryLocked(host string) (ports.RemoteCatalogCacheEntry, bool) { | |
| 950 | + | if d.remoteCatalog.status[host] != remoteHostFresh { | |
| 951 | + | return ports.RemoteCatalogCacheEntry{}, false | |
| 952 | + | } | |
| 953 | + | entry, ok := d.remoteCatalog.cache[host] | |
| 954 | + | if !ok || remoteCatalogExpired(entry.FetchedAt, d.clock.Now()) { | |
| 955 | + | d.remoteCatalog.status[host] = remoteHostStale | |
| 956 | + | return ports.RemoteCatalogCacheEntry{}, false | |
| 957 | + | } | |
| 958 | + | return entry, true | |
| 959 | + | } | |
| 960 | + | ||
| 949 | 961 | func (d *Daemon) remotePickerTargetReady(key domain.RemoteSessionKey) bool { | |
| 950 | 962 | if d == nil { | |
| 951 | 963 | return false | |
| 952 | 964 | } | |
| 953 | 965 | d.remoteCatalog.mu.Lock() | |
| 954 | 966 | defer d.remoteCatalog.mu.Unlock() | |
| 955 | - | if d.remoteCatalog.status[key.Host] != remoteHostFresh { | |
| 956 | - | return false | |
| 957 | - | } | |
| 958 | - | entry, ok := d.remoteCatalog.cache[key.Host] | |
| 959 | - | if !ok || remoteCatalogExpired(entry.FetchedAt, d.clock.Now()) { | |
| 960 | - | d.remoteCatalog.status[key.Host] = remoteHostStale | |
| 967 | + | entry, ok := d.remoteCatalogEntryLocked(key.Host) | |
| 968 | + | if !ok { | |
| 961 | 969 | return false | |
| 962 | 970 | } | |
| 963 | 971 | for _, session := range entry.Sessions { | |
| @@ -974,47 +982,37 @@ func (d *Daemon) remotePickerTargetReadyTarget(target domain.RemoteSessionTarget | |||
| 974 | 982 | } | |
| 975 | 983 | d.remoteCatalog.mu.Lock() | |
| 976 | 984 | defer d.remoteCatalog.mu.Unlock() | |
| 977 | - | now := d.clock.Now() | |
| 978 | - | if d.remoteCatalog.status[target.Endpoint] == remoteHostFresh { | |
| 979 | - | if entry, ok := d.remoteCatalog.cache[target.Endpoint]; ok && remoteCatalogExpired(entry.FetchedAt, now) { | |
| 980 | - | d.remoteCatalog.status[target.Endpoint] = remoteHostStale | |
| 981 | - | } | |
| 982 | - | } | |
| 983 | - | if d.remoteCatalog.status[target.Endpoint] != remoteHostFresh { | |
| 985 | + | entry, ok := d.remoteCatalogEntryLocked(target.Endpoint) | |
| 986 | + | if !ok { | |
| 984 | 987 | return false | |
| 985 | 988 | } | |
| 986 | - | for _, entry := range d.remoteCatalog.cache { | |
| 987 | - | if entry.Host != target.Endpoint { | |
| 989 | + | for _, session := range entry.Sessions { | |
| 990 | + | if session.Name != target.SessionName || session.LifecycleID != target.LifecycleID { | |
| 988 | 991 | continue | |
| 989 | 992 | } | |
| 990 | - | for _, session := range entry.Sessions { | |
| 991 | - | if session.Name != target.SessionName || session.LifecycleID != target.LifecycleID { | |
| 993 | + | if target.Stopped != remoteSessionStateStopped(session.State) || session.State == "broken" { | |
| 994 | + | return false | |
| 995 | + | } | |
| 996 | + | tabs := ports.CatalogTabs(session) | |
| 997 | + | metadata := make([]domain.TabSelectorTab, 0, len(tabs)) | |
| 998 | + | for _, tab := range tabs { | |
| 999 | + | metadata = append(metadata, domain.TabSelectorTab{ID: domain.TabStableID(tab.ID), Name: tab.Name}) | |
| 1000 | + | } | |
| 1001 | + | if target.Stopped { | |
| 1002 | + | _, ok := target.StoppedTab.Resolve(metadata) | |
| 1003 | + | return ok | |
| 1004 | + | } | |
| 1005 | + | found := false | |
| 1006 | + | for _, tab := range tabs { | |
| 1007 | + | if tab.ID != string(target.LiveTabID) { | |
| 992 | 1008 | continue | |
| 993 | 1009 | } | |
| 994 | - | if target.Stopped != remoteSessionStateStopped(session.State) || session.State == "broken" { | |
| 1010 | + | if found { | |
| 995 | 1011 | return false | |
| 996 | 1012 | } | |
| 997 | - | tabs := ports.CatalogTabs(session) | |
| 998 | - | metadata := make([]domain.TabSelectorTab, 0, len(tabs)) | |
| 999 | - | for _, tab := range tabs { | |
| 1000 | - | metadata = append(metadata, domain.TabSelectorTab{ID: domain.TabStableID(tab.ID), Name: tab.Name}) | |
| 1001 | - | } | |
| 1002 | - | if target.Stopped { | |
| 1003 | - | _, ok := target.StoppedTab.Resolve(metadata) | |
| 1004 | - | return ok | |
| 1005 | - | } | |
| 1006 | - | found := false | |
| 1007 | - | for _, tab := range tabs { | |
| 1008 | - | if tab.ID != string(target.LiveTabID) { | |
| 1009 | - | continue | |
| 1010 | - | } | |
| 1011 | - | if found { | |
| 1012 | - | return false | |
| 1013 | - | } | |
| 1014 | - | found = true | |
| 1015 | - | } | |
| 1016 | - | return found | |
| 1013 | + | found = true | |
| 1017 | 1014 | } | |
| 1015 | + | return found | |
| 1018 | 1016 | } | |
| 1019 | 1017 | return false | |
| 1020 | 1018 | } | |
| @@ -946,18 +946,26 @@ func (d *Daemon) sendRemoteAttachTargetForAttachment(token attachmentConnectionT | |||||
| 946 | return nil | 946 | return nil | ||
| 947 | } | 947 | } | ||
| 948 | 948 | ||||
| 949 | + | func (d *Daemon) remoteCatalogEntryLocked(host string) (ports.RemoteCatalogCacheEntry, bool) { | |||
| 950 | + | if d.remoteCatalog.status[host] != remoteHostFresh { | |||
| 951 | + | return ports.RemoteCatalogCacheEntry{}, false | |||
| 952 | + | } | |||
| 953 | + | entry, ok := d.remoteCatalog.cache[host] | |||
| 954 | + | if !ok || remoteCatalogExpired(entry.FetchedAt, d.clock.Now()) { | |||
| 955 | + | d.remoteCatalog.status[host] = remoteHostStale | |||
| 956 | + | return ports.RemoteCatalogCacheEntry{}, false | |||
| 957 | + | } | |||
| 958 | + | return entry, true | |||
| 959 | + | } | |||
| 960 | + | ||||
| 949 | func (d *Daemon) remotePickerTargetReady(key domain.RemoteSessionKey) bool { | 961 | func (d *Daemon) remotePickerTargetReady(key domain.RemoteSessionKey) bool { | ||
| 950 | if d == nil { | 962 | if d == nil { | ||
| 951 | return false | 963 | return false | ||
| 952 | } | 964 | } | ||
| 953 | d.remoteCatalog.mu.Lock() | 965 | d.remoteCatalog.mu.Lock() | ||
| 954 | defer d.remoteCatalog.mu.Unlock() | 966 | defer d.remoteCatalog.mu.Unlock() | ||
| 955 | - | if d.remoteCatalog.status[key.Host] != remoteHostFresh { | 967 | + | entry, ok := d.remoteCatalogEntryLocked(key.Host) |
| 956 | - | return false | 968 | + | if !ok { |
| 957 | - | } | |||
| 958 | - | entry, ok := d.remoteCatalog.cache[key.Host] | |||
| 959 | - | if !ok || remoteCatalogExpired(entry.FetchedAt, d.clock.Now()) { | |||
| 960 | - | d.remoteCatalog.status[key.Host] = remoteHostStale | |||
| 961 | return false | 969 | return false | ||
| 962 | } | 970 | } | ||
| 963 | for _, session := range entry.Sessions { | 971 | for _, session := range entry.Sessions { | ||
| @@ -974,47 +982,37 @@ func (d *Daemon) remotePickerTargetReadyTarget(target domain.RemoteSessionTarget | |||||
| 974 | } | 982 | } | ||
| 975 | d.remoteCatalog.mu.Lock() | 983 | d.remoteCatalog.mu.Lock() | ||
| 976 | defer d.remoteCatalog.mu.Unlock() | 984 | defer d.remoteCatalog.mu.Unlock() | ||
| 977 | - | now := d.clock.Now() | 985 | + | entry, ok := d.remoteCatalogEntryLocked(target.Endpoint) |
| 978 | - | if d.remoteCatalog.status[target.Endpoint] == remoteHostFresh { | 986 | + | if !ok { |
| 979 | - | if entry, ok := d.remoteCatalog.cache[target.Endpoint]; ok && remoteCatalogExpired(entry.FetchedAt, now) { | |||
| 980 | - | d.remoteCatalog.status[target.Endpoint] = remoteHostStale | |||
| 981 | - | } | |||
| 982 | - | } | |||
| 983 | - | if d.remoteCatalog.status[target.Endpoint] != remoteHostFresh { | |||
| 984 | return false | 987 | return false | ||
| 985 | } | 988 | } | ||
| 986 | - | for _, entry := range d.remoteCatalog.cache { | 989 | + | for _, session := range entry.Sessions { |
| 987 | - | if entry.Host != target.Endpoint { | 990 | + | if session.Name != target.SessionName || session.LifecycleID != target.LifecycleID { |
| 988 | continue | 991 | continue | ||
| 989 | } | 992 | } | ||
| 990 | - | for _, session := range entry.Sessions { | 993 | + | if target.Stopped != remoteSessionStateStopped(session.State) || session.State == "broken" { |
| 991 | - | if session.Name != target.SessionName || session.LifecycleID != target.LifecycleID { | 994 | + | return false |
| 995 | + | } | |||
| 996 | + | tabs := ports.CatalogTabs(session) | |||
| 997 | + | metadata := make([]domain.TabSelectorTab, 0, len(tabs)) | |||
| 998 | + | for _, tab := range tabs { | |||
| 999 | + | metadata = append(metadata, domain.TabSelectorTab{ID: domain.TabStableID(tab.ID), Name: tab.Name}) | |||
| 1000 | + | } | |||
| 1001 | + | if target.Stopped { | |||
| 1002 | + | _, ok := target.StoppedTab.Resolve(metadata) | |||
| 1003 | + | return ok | |||
| 1004 | + | } | |||
| 1005 | + | found := false | |||
| 1006 | + | for _, tab := range tabs { | |||
| 1007 | + | if tab.ID != string(target.LiveTabID) { | |||
| 992 | continue | 1008 | continue | ||
| 993 | } | 1009 | } | ||
| 994 | - | if target.Stopped != remoteSessionStateStopped(session.State) || session.State == "broken" { | 1010 | + | if found { |
| 995 | return false | 1011 | return false | ||
| 996 | } | 1012 | } | ||
| 997 | - | tabs := ports.CatalogTabs(session) | 1013 | + | found = true |
| 998 | - | metadata := make([]domain.TabSelectorTab, 0, len(tabs)) | |||
| 999 | - | for _, tab := range tabs { | |||
| 1000 | - | metadata = append(metadata, domain.TabSelectorTab{ID: domain.TabStableID(tab.ID), Name: tab.Name}) | |||
| 1001 | - | } | |||
| 1002 | - | if target.Stopped { | |||
| 1003 | - | _, ok := target.StoppedTab.Resolve(metadata) | |||
| 1004 | - | return ok | |||
| 1005 | - | } | |||
| 1006 | - | found := false | |||
| 1007 | - | for _, tab := range tabs { | |||
| 1008 | - | if tab.ID != string(target.LiveTabID) { | |||
| 1009 | - | continue | |||
| 1010 | - | } | |||
| 1011 | - | if found { | |||
| 1012 | - | return false | |||
| 1013 | - | } | |||
| 1014 | - | found = true | |||
| 1015 | - | } | |||
| 1016 | - | return found | |||
| 1017 | } | 1014 | } | ||
| 1015 | + | return found | |||
| 1018 | } | 1016 | } | ||
| 1019 | return false | 1017 | return false | ||
| 1020 | } | 1018 | } | ||
M internal/usecase/daemon/picker_test.go +9 -0
| @@ -3,6 +3,7 @@ package daemon | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | + | "io" | |
| 6 | 7 | "strings" | |
| 7 | 8 | "testing" | |
| 8 | 9 | "time" | |
| @@ -28,6 +29,14 @@ var ( | |||
| 28 | 29 | ||
| 29 | 30 | // --- test doubles ----------------------------------------------------------- | |
| 30 | 31 | ||
| 32 | + | type remotePickerSendErrorTransport struct { | |
| 33 | + | err error | |
| 34 | + | } | |
| 35 | + | ||
| 36 | + | func (t *remotePickerSendErrorTransport) Send(ports.Frame) error { return t.err } | |
| 37 | + | func (*remotePickerSendErrorTransport) Recv() (ports.Frame, error) { return ports.Frame{}, io.EOF } | |
| 38 | + | func (*remotePickerSendErrorTransport) Close() error { return nil } | |
| 39 | + | ||
| 31 | 40 | type refusingSnapshotDeleteRepository struct { | |
| 32 | 41 | noOpSnapshotRepository | |
| 33 | 42 | err error | |
| @@ -3,6 +3,7 @@ package daemon | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | + | "io" | |||
| 6 | "strings" | 7 | "strings" | ||
| 7 | "testing" | 8 | "testing" | ||
| 8 | "time" | 9 | "time" | ||
| @@ -28,6 +29,14 @@ var ( | |||||
| 28 | 29 | ||||
| 29 | // --- test doubles ----------------------------------------------------------- | 30 | // --- test doubles ----------------------------------------------------------- | ||
| 30 | 31 | ||||
| 32 | + | type remotePickerSendErrorTransport struct { | |||
| 33 | + | err error | |||
| 34 | + | } | |||
| 35 | + | ||||
| 36 | + | func (t *remotePickerSendErrorTransport) Send(ports.Frame) error { return t.err } | |||
| 37 | + | func (*remotePickerSendErrorTransport) Recv() (ports.Frame, error) { return ports.Frame{}, io.EOF } | |||
| 38 | + | func (*remotePickerSendErrorTransport) Close() error { return nil } | |||
| 39 | + | ||||
| 31 | type refusingSnapshotDeleteRepository struct { | 40 | type refusingSnapshotDeleteRepository struct { | ||
| 32 | noOpSnapshotRepository | 41 | noOpSnapshotRepository | ||
| 33 | err error | 42 | err error | ||
M internal/usecase/daemon/recent_route_snapshot_test.go +2 -0
| @@ -20,6 +20,7 @@ func TestAttachmentStatusUsesClientRouteSnapshot(t *testing.T) { | |||
| 20 | 20 | ||
| 21 | 21 | state := d.barStateForAttachmentPaletteHintsFor(sess, ac, "", nil, ports.RecentRouteSnapshot{}) | |
| 22 | 22 | ||
| 23 | + | require.Len(t, state.mru, 2) | |
| 23 | 24 | require.Equal(t, []string{"logs@edge", "work"}, []string{state.mru[0].name, state.mru[1].name}) | |
| 24 | 25 | } | |
| 25 | 26 | ||
| @@ -85,6 +86,7 @@ func TestRecentRouteHintsRetainSnapshotSelectionIdentity(t *testing.T) { | |||
| 85 | 86 | ||
| 86 | 87 | hints := recentRouteHints(snapshot, nil) | |
| 87 | 88 | ||
| 89 | + | require.Len(t, hints.Recent, 1) | |
| 88 | 90 | require.Equal(t, "logs@edge", hints.Recent[0].Name) | |
| 89 | 91 | require.Equal(t, uint64(8), hints.Recent[0].SnapshotGeneration) | |
| 90 | 92 | require.Equal(t, uint64(11), hints.Recent[0].Key) | |
| @@ -20,6 +20,7 @@ func TestAttachmentStatusUsesClientRouteSnapshot(t *testing.T) { | |||||
| 20 | 20 | ||||
| 21 | state := d.barStateForAttachmentPaletteHintsFor(sess, ac, "", nil, ports.RecentRouteSnapshot{}) | 21 | state := d.barStateForAttachmentPaletteHintsFor(sess, ac, "", nil, ports.RecentRouteSnapshot{}) | ||
| 22 | 22 | ||||
| 23 | + | require.Len(t, state.mru, 2) | |||
| 23 | require.Equal(t, []string{"logs@edge", "work"}, []string{state.mru[0].name, state.mru[1].name}) | 24 | require.Equal(t, []string{"logs@edge", "work"}, []string{state.mru[0].name, state.mru[1].name}) | ||
| 24 | } | 25 | } | ||
| 25 | 26 | ||||
| @@ -85,6 +86,7 @@ func TestRecentRouteHintsRetainSnapshotSelectionIdentity(t *testing.T) { | |||||
| 85 | 86 | ||||
| 86 | hints := recentRouteHints(snapshot, nil) | 87 | hints := recentRouteHints(snapshot, nil) | ||
| 87 | 88 | ||||
| 89 | + | require.Len(t, hints.Recent, 1) | |||
| 88 | require.Equal(t, "logs@edge", hints.Recent[0].Name) | 90 | require.Equal(t, "logs@edge", hints.Recent[0].Name) | ||
| 89 | require.Equal(t, uint64(8), hints.Recent[0].SnapshotGeneration) | 91 | require.Equal(t, uint64(8), hints.Recent[0].SnapshotGeneration) | ||
| 90 | require.Equal(t, uint64(11), hints.Recent[0].Key) | 92 | require.Equal(t, uint64(11), hints.Recent[0].Key) | ||
M internal/usecase/daemon/remote_attach_test.go +7 -4
| @@ -69,9 +69,12 @@ func TestRemotePickerRichHandoffRejectsReplacedLifecycle(t *testing.T) { | |||
| 69 | 69 | remoteTarget := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | |
| 70 | 70 | target := picker.Target{Session: key.ID(), RemoteKey: &key, RemoteTarget: &remoteTarget, TabID: "tab-1"} | |
| 71 | 71 | require.ErrorIs(t, d.sendRemoteAttachTargetForAttachment(token, target, key, sessionHandoffGuard{}, "picker-select"), errAttachmentTransition) | |
| 72 | - | select { | |
| 73 | - | case frame := <-sends: | |
| 74 | - | require.NotEqual(t, ports.MsgAttachTarget, frame.Type, "rejected handoff must not send an attach target") | |
| 75 | - | default: | |
| 72 | + | for { | |
| 73 | + | select { | |
| 74 | + | case frame := <-sends: | |
| 75 | + | require.NotEqual(t, ports.MsgAttachTarget, frame.Type, "rejected handoff must not send an attach target") | |
| 76 | + | default: | |
| 77 | + | return | |
| 78 | + | } | |
| 76 | 79 | } | |
| 77 | 80 | } | |
| @@ -69,9 +69,12 @@ func TestRemotePickerRichHandoffRejectsReplacedLifecycle(t *testing.T) { | |||||
| 69 | remoteTarget := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | 69 | remoteTarget := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | ||
| 70 | target := picker.Target{Session: key.ID(), RemoteKey: &key, RemoteTarget: &remoteTarget, TabID: "tab-1"} | 70 | target := picker.Target{Session: key.ID(), RemoteKey: &key, RemoteTarget: &remoteTarget, TabID: "tab-1"} | ||
| 71 | require.ErrorIs(t, d.sendRemoteAttachTargetForAttachment(token, target, key, sessionHandoffGuard{}, "picker-select"), errAttachmentTransition) | 71 | require.ErrorIs(t, d.sendRemoteAttachTargetForAttachment(token, target, key, sessionHandoffGuard{}, "picker-select"), errAttachmentTransition) | ||
| 72 | - | select { | 72 | + | for { |
| 73 | - | case frame := <-sends: | 73 | + | select { |
| 74 | - | require.NotEqual(t, ports.MsgAttachTarget, frame.Type, "rejected handoff must not send an attach target") | 74 | + | case frame := <-sends: |
| 75 | - | default: | 75 | + | require.NotEqual(t, ports.MsgAttachTarget, frame.Type, "rejected handoff must not send an attach target") |
| 76 | + | default: | |||
| 77 | + | return | |||
| 78 | + | } | |||
| 76 | } | 79 | } | ||
| 77 | } | 80 | } | ||
M internal/usecase/daemon/remote_picker_test.go +4 -9
| @@ -10,10 +10,12 @@ import ( | |||
| 10 | 10 | "testing" | |
| 11 | 11 | "time" | |
| 12 | 12 | ||
| 13 | + | "github.com/stretchr/testify/mock" | |
| 13 | 14 | "github.com/stretchr/testify/require" | |
| 14 | 15 | ||
| 15 | 16 | "github.com/bnema/vev/internal/domain" | |
| 16 | 17 | "github.com/bnema/vev/internal/ports" | |
| 18 | + | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |
| 17 | 19 | "github.com/bnema/vev/internal/usecase/picker" | |
| 18 | 20 | ) | |
| 19 | 21 | ||
| @@ -678,7 +680,8 @@ func TestRemotePickerHandoffSendFailureKeepsPickerOpen(t *testing.T) { | |||
| 678 | 680 | d.remoteCatalog.status["arch"] = remoteHostFresh | |
| 679 | 681 | d.remoteCatalog.mu.Unlock() | |
| 680 | 682 | cause := errors.New("remote attach send failed") | |
| 681 | - | tr := &remotePickerSendErrorTransport{err: cause} | |
| 683 | + | tr := portsmocks.NewMockTransport(t) | |
| 684 | + | tr.EXPECT().Send(mock.Anything).Return(cause) | |
| 682 | 685 | sess, ac, _ := addRemoteRefreshPickerOwner(t, d, "local", tr) | |
| 683 | 686 | ac.overlays.pickerMu.Lock() | |
| 684 | 687 | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) | |
| @@ -708,14 +711,6 @@ func TestRemotePickerHandoffSendFailureKeepsPickerOpen(t *testing.T) { | |||
| 708 | 711 | require.Same(t, sess, ac.currentAttachmentSession()) | |
| 709 | 712 | } | |
| 710 | 713 | ||
| 711 | - | type remotePickerSendErrorTransport struct { | |
| 712 | - | err error | |
| 713 | - | } | |
| 714 | - | ||
| 715 | - | func (t *remotePickerSendErrorTransport) Send(ports.Frame) error { return t.err } | |
| 716 | - | func (*remotePickerSendErrorTransport) Recv() (ports.Frame, error) { return ports.Frame{}, io.EOF } | |
| 717 | - | func (*remotePickerSendErrorTransport) Close() error { return nil } | |
| 718 | - | ||
| 719 | 714 | func addRemoteRefreshPickerOwner(t *testing.T, d *Daemon, id domain.SessionID, transports ...ports.Transport) (*session, *attachedClient, chan ports.Frame) { | |
| 720 | 715 | t.Helper() | |
| 721 | 716 | var tr ports.Transport | |
| @@ -10,10 +10,12 @@ import ( | |||||
| 10 | "testing" | 10 | "testing" | ||
| 11 | "time" | 11 | "time" | ||
| 12 | 12 | ||||
| 13 | + | "github.com/stretchr/testify/mock" | |||
| 13 | "github.com/stretchr/testify/require" | 14 | "github.com/stretchr/testify/require" | ||
| 14 | 15 | ||||
| 15 | "github.com/bnema/vev/internal/domain" | 16 | "github.com/bnema/vev/internal/domain" | ||
| 16 | "github.com/bnema/vev/internal/ports" | 17 | "github.com/bnema/vev/internal/ports" | ||
| 18 | + | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |||
| 17 | "github.com/bnema/vev/internal/usecase/picker" | 19 | "github.com/bnema/vev/internal/usecase/picker" | ||
| 18 | ) | 20 | ) | ||
| 19 | 21 | ||||
| @@ -678,7 +680,8 @@ func TestRemotePickerHandoffSendFailureKeepsPickerOpen(t *testing.T) { | |||||
| 678 | d.remoteCatalog.status["arch"] = remoteHostFresh | 680 | d.remoteCatalog.status["arch"] = remoteHostFresh | ||
| 679 | d.remoteCatalog.mu.Unlock() | 681 | d.remoteCatalog.mu.Unlock() | ||
| 680 | cause := errors.New("remote attach send failed") | 682 | cause := errors.New("remote attach send failed") | ||
| 681 | - | tr := &remotePickerSendErrorTransport{err: cause} | 683 | + | tr := portsmocks.NewMockTransport(t) |
| 684 | + | tr.EXPECT().Send(mock.Anything).Return(cause) | |||
| 682 | sess, ac, _ := addRemoteRefreshPickerOwner(t, d, "local", tr) | 685 | sess, ac, _ := addRemoteRefreshPickerOwner(t, d, "local", tr) | ||
| 683 | ac.overlays.pickerMu.Lock() | 686 | ac.overlays.pickerMu.Lock() | ||
| 684 | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) | 687 | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) | ||
| @@ -708,14 +711,6 @@ func TestRemotePickerHandoffSendFailureKeepsPickerOpen(t *testing.T) { | |||||
| 708 | require.Same(t, sess, ac.currentAttachmentSession()) | 711 | require.Same(t, sess, ac.currentAttachmentSession()) | ||
| 709 | } | 712 | } | ||
| 710 | 713 | ||||
| 711 | - | type remotePickerSendErrorTransport struct { | |||
| 712 | - | err error | |||
| 713 | - | } | |||
| 714 | - | ||||
| 715 | - | func (t *remotePickerSendErrorTransport) Send(ports.Frame) error { return t.err } | |||
| 716 | - | func (*remotePickerSendErrorTransport) Recv() (ports.Frame, error) { return ports.Frame{}, io.EOF } | |||
| 717 | - | func (*remotePickerSendErrorTransport) Close() error { return nil } | |||
| 718 | - | ||||
| 719 | func addRemoteRefreshPickerOwner(t *testing.T, d *Daemon, id domain.SessionID, transports ...ports.Transport) (*session, *attachedClient, chan ports.Frame) { | 714 | func addRemoteRefreshPickerOwner(t *testing.T, d *Daemon, id domain.SessionID, transports ...ports.Transport) (*session, *attachedClient, chan ports.Frame) { | ||
| 720 | t.Helper() | 715 | t.Helper() | ||
| 721 | var tr ports.Transport | 716 | var tr ports.Transport | ||
M internal/usecase/daemon/remote_route_test.go +2 -2
| @@ -37,8 +37,8 @@ func TestFinishRouteAttachRollsBackCreatedSession(t *testing.T) { | |||
| 37 | 37 | Version: ports.ProtocolVersion, Intent: ports.IntentAttach, Name: "work", Size: defaultSize, | |
| 38 | 38 | RemoteTarget: &target, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | |
| 39 | 39 | } | |
| 40 | - | // finishRouteAttach follows the locked-caller contract: it releases d.mu | |
| 41 | - | // on both the success and error paths, so retain the lock across the call. | |
| 40 | + | // The caller must hold d.mu on entry; finishRouteAttach releases it on | |
| 41 | + | // both success and error paths before returning. | |
| 42 | 42 | d.mu.Lock() | |
| 43 | 43 | _, err = d.finishRouteAttach(sess, &closeTrackingTransport{}, defaultSize, terminalEnv{}, hello, true, true) | |
| 44 | 44 | var protocol *protoErr | |
| @@ -37,8 +37,8 @@ func TestFinishRouteAttachRollsBackCreatedSession(t *testing.T) { | |||||
| 37 | Version: ports.ProtocolVersion, Intent: ports.IntentAttach, Name: "work", Size: defaultSize, | 37 | Version: ports.ProtocolVersion, Intent: ports.IntentAttach, Name: "work", Size: defaultSize, | ||
| 38 | RemoteTarget: &target, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | 38 | RemoteTarget: &target, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | ||
| 39 | } | 39 | } | ||
| 40 | - | // finishRouteAttach follows the locked-caller contract: it releases d.mu | 40 | + | // The caller must hold d.mu on entry; finishRouteAttach releases it on |
| 41 | - | // on both the success and error paths, so retain the lock across the call. | 41 | + | // both success and error paths before returning. |
| 42 | d.mu.Lock() | 42 | d.mu.Lock() | ||
| 43 | _, err = d.finishRouteAttach(sess, &closeTrackingTransport{}, defaultSize, terminalEnv{}, hello, true, true) | 43 | _, err = d.finishRouteAttach(sess, &closeTrackingTransport{}, defaultSize, terminalEnv{}, hello, true, true) | ||
| 44 | var protocol *protoErr | 44 | var protocol *protoErr | ||
M internal/usecase/daemon/session_back.go +7 -1
| @@ -13,7 +13,13 @@ func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken) error | |||
| 13 | 13 | return nil | |
| 14 | 14 | } | |
| 15 | 15 | snapshot := token.ac.routeSnapshotCopy() | |
| 16 | - | if snapshot.Generation == 0 || snapshot.Previous.Key == 0 || snapshot.Previous.Generation == 0 { | |
| 16 | + | if snapshot.Generation == 0 { | |
| 17 | + | if d.log != nil { | |
| 18 | + | d.log.Debug("back-session skipped: attachment has no published route snapshot") | |
| 19 | + | } | |
| 20 | + | return nil | |
| 21 | + | } | |
| 22 | + | if snapshot.Previous.Key == 0 || snapshot.Previous.Generation == 0 { | |
| 17 | 23 | return nil | |
| 18 | 24 | } | |
| 19 | 25 | return d.sendRecentRouteNavigationActionForAttachment(token, ports.RouteNavigationAction{ | |
| @@ -13,7 +13,13 @@ func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken) error | |||||
| 13 | return nil | 13 | return nil | ||
| 14 | } | 14 | } | ||
| 15 | snapshot := token.ac.routeSnapshotCopy() | 15 | snapshot := token.ac.routeSnapshotCopy() | ||
| 16 | - | if snapshot.Generation == 0 || snapshot.Previous.Key == 0 || snapshot.Previous.Generation == 0 { | 16 | + | if snapshot.Generation == 0 { |
| 17 | + | if d.log != nil { | |||
| 18 | + | d.log.Debug("back-session skipped: attachment has no published route snapshot") | |||
| 19 | + | } | |||
| 20 | + | return nil | |||
| 21 | + | } | |||
| 22 | + | if snapshot.Previous.Key == 0 || snapshot.Previous.Generation == 0 { | |||
| 17 | return nil | 23 | return nil | ||
| 18 | } | 24 | } | ||
| 19 | return d.sendRecentRouteNavigationActionForAttachment(token, ports.RouteNavigationAction{ | 25 | return d.sendRecentRouteNavigationActionForAttachment(token, ports.RouteNavigationAction{ | ||