5ee2a06 fix(protocol): bound attach handoffs · vev

M internal/app/run.go +7 -0


@@ -835,6 +835,8 @@ func remoteDiscoveryDaemonOption(stateDir string, observer ports.SerializedRunti
835	835		}, nil
836	836		}
837	837
	838	+	const maxAttachTargetHandoffs = 32
	839	+
838	840		func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, activeSession string, log *slog.Logger, deps runAttachDeps) error {
839	841		if activeSession != "" {
840	842		if remoteTarget == "" && intent == ports.IntentNew {

@@ -911,6 +913,7 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac
911	913		}, nil
912	914		}
913	915
	916	+	handoffAttempts := 0
914	917		for {
915	918		var err error
916	919		if remoteTarget != "" {

@@ -970,6 +973,10 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac
970	973		if err := validateRemoteAttachHandoff(handoffErr.Target); err != nil {
971	974		return fmt.Errorf("vev: invalid remote attach handoff: %w", err)
972	975		}
	976	+	if handoffAttempts >= maxAttachTargetHandoffs {
	977	+	return fmt.Errorf("vev: attach handoff exceeded maximum of %d attempts", maxAttachTargetHandoffs)
	978	+	}
	979	+	handoffAttempts++
973	980		remoteTarget = handoffErr.Target.Endpoint
974	981		remoteDisplayOrigin = remoteTarget
975	982		routeOrigin = ports.RouteOriginDiscovery


@@ -835,6 +835,8 @@ func remoteDiscoveryDaemonOption(stateDir string, observer ports.SerializedRunti
835	}, nil	835		}, nil
836	}	836		}
837		837
		838	+	const maxAttachTargetHandoffs = 32
		839	+
838	func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, activeSession string, log *slog.Logger, deps runAttachDeps) error {	840		func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, activeSession string, log *slog.Logger, deps runAttachDeps) error {
839	if activeSession != "" {	841		if activeSession != "" {
840	if remoteTarget == "" && intent == ports.IntentNew {	842		if remoteTarget == "" && intent == ports.IntentNew {

@@ -911,6 +913,7 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac
911	}, nil	913		}, nil
912	}	914		}
913		915
		916	+	handoffAttempts := 0
914	for {	917		for {
915	var err error	918		var err error
916	if remoteTarget != "" {	919		if remoteTarget != "" {

@@ -970,6 +973,10 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac
970	if err := validateRemoteAttachHandoff(handoffErr.Target); err != nil {	973		if err := validateRemoteAttachHandoff(handoffErr.Target); err != nil {
971	return fmt.Errorf("vev: invalid remote attach handoff: %w", err)	974		return fmt.Errorf("vev: invalid remote attach handoff: %w", err)
972	}	975		}
		976	+	if handoffAttempts >= maxAttachTargetHandoffs {
		977	+	return fmt.Errorf("vev: attach handoff exceeded maximum of %d attempts", maxAttachTargetHandoffs)
		978	+	}
		979	+	handoffAttempts++
973	remoteTarget = handoffErr.Target.Endpoint	980		remoteTarget = handoffErr.Target.Endpoint
974	remoteDisplayOrigin = remoteTarget	981		remoteDisplayOrigin = remoteTarget
975	routeOrigin = ports.RouteOriginDiscovery	982		routeOrigin = ports.RouteOriginDiscovery

M internal/app/run_test.go +16 -0


@@ -935,6 +935,22 @@ func TestRunAttachWithDepsAllowsRevisitingRemoteHandoff(t *testing.T) {
935	935		require.NoError(t, err)
936	936		}
937	937
	938	+	func TestRunAttachWithDepsBoundsRepeatedAttachTargetHandoffs(t *testing.T) {
	939	+	factory := portsmocks.NewMockRemoteDialerFactory(t)
	940	+	factory.EXPECT().DialerForRemote("remote.example", "work", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Times(maxAttachTargetHandoffs + 1)
	941	+
	942	+	calls := 0
	943	+	err := runAttachWithDeps(context.Background(), ports.IntentAttach, "work", "remote.example", "", nil, runAttachDeps{
	944	+	remoteDialerFactory: factory,
	945	+	runClient: func(_ context.Context, _ client.Dependencies, _ client.AttachRequest) error {
	946	+	calls++
	947	+	return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: "remote.example", Session: "work", Intent: ports.IntentAttach}}
	948	+	},
	949	+	})
	950	+	require.ErrorContains(t, err, "attach handoff exceeded maximum")
	951	+	require.Equal(t, maxAttachTargetHandoffs+1, calls)
	952	+	}
	953	+
938	954		func TestRunAttachWithDepsLocalPickerHandoffAttachesSelectedRemote(t *testing.T) {
939	955		factory := portsmocks.NewMockRemoteDialerFactory(t)
940	956		factory.EXPECT().DialerForRemote("selected.example", "picked", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once()


@@ -935,6 +935,22 @@ func TestRunAttachWithDepsAllowsRevisitingRemoteHandoff(t *testing.T) {
935	require.NoError(t, err)	935		require.NoError(t, err)
936	}	936		}
937		937
		938	+	func TestRunAttachWithDepsBoundsRepeatedAttachTargetHandoffs(t *testing.T) {
		939	+	factory := portsmocks.NewMockRemoteDialerFactory(t)
		940	+	factory.EXPECT().DialerForRemote("remote.example", "work", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Times(maxAttachTargetHandoffs + 1)
		941	+
		942	+	calls := 0
		943	+	err := runAttachWithDeps(context.Background(), ports.IntentAttach, "work", "remote.example", "", nil, runAttachDeps{
		944	+	remoteDialerFactory: factory,
		945	+	runClient: func(_ context.Context, _ client.Dependencies, _ client.AttachRequest) error {
		946	+	calls++
		947	+	return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: "remote.example", Session: "work", Intent: ports.IntentAttach}}
		948	+	},
		949	+	})
		950	+	require.ErrorContains(t, err, "attach handoff exceeded maximum")
		951	+	require.Equal(t, maxAttachTargetHandoffs+1, calls)
		952	+	}
		953	+
938	func TestRunAttachWithDepsLocalPickerHandoffAttachesSelectedRemote(t *testing.T) {	954		func TestRunAttachWithDepsLocalPickerHandoffAttachesSelectedRemote(t *testing.T) {
939	factory := portsmocks.NewMockRemoteDialerFactory(t)	955		factory := portsmocks.NewMockRemoteDialerFactory(t)
940	factory.EXPECT().DialerForRemote("selected.example", "picked", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once()	956		factory.EXPECT().DialerForRemote("selected.example", "picked", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once()

M internal/ports/wire_test.go +14 -3


@@ -173,16 +173,24 @@ func TestHelloEnvironmentCodec(t *testing.T) {
173	173		base := MarshalHello(Hello{Version: ProtocolVersion, Size: domain.Size{Cols: 1, Rows: 1}})
174	174		withCount := func(count byte) []byte {
175	175		payload := append([]byte(nil), base...)
176		-	payload[len(payload)-6] = count
	176	+	// The uint32 environment count occupies bytes len-13 through len-10.
	177	+	payload[len(payload)-10] = count
177	178		return payload
178	179		}
	180	+	withEntries := func(count byte, entries ...byte) []byte {
	181	+	payload := withCount(count)
	182	+	// The nine-byte tail follows the environment-count field.
	183	+	tail := append([]byte(nil), payload[len(payload)-9:]...)
	184	+	payload = append(payload[:len(payload)-9], entries...)
	185	+	return append(payload, tail...)
	186	+	}
179	187		tests := []struct {
180	188		name string
181	189		payload []byte
182	190		}{
183	191		{name: "truncated count", payload: base[:len(base)-1]},
184		-	{name: "truncated entry length", payload: append(withCount(1), 0, 0)},
185		-	{name: "truncated entry body", payload: append(withCount(1), 0, 0, 0, 3, 'x')},
	192	+	{name: "truncated entry length", payload: withEntries(1, 0, 0)},
	193	+	{name: "truncated entry body", payload: withEntries(1, 0, 0, 0, 3, 'x')},
186	194		{name: "impossible count", payload: withCount(2)},
187	195		{name: "trailing garbage", payload: append(append([]byte(nil), base...), 0xff)},
188	196		}

@@ -190,6 +198,9 @@ func TestHelloEnvironmentCodec(t *testing.T) {
190	198		t.Run(tt.name, func(t *testing.T) {
191	199		_, err := UnmarshalHello(tt.payload)
192	200		require.Error(t, err)
	201	+	if tt.name != "trailing garbage" {
	202	+	require.ErrorIs(t, err, errShortPayload)
	203	+	}
193	204		})
194	205		}
195	206		}

 	base := MarshalHello(Hello{Version: ProtocolVersion, Size: domain.Size{Cols: 1, Rows: 1}})
 	withCount := func(count byte) []byte {
 		payload := append([]byte(nil), base...)
-		payload[len(payload)-6] = count
+		// The uint32 environment count occupies bytes len-13 through len-10.
+		payload[len(payload)-10] = count
 		return payload
 	}
+	withEntries := func(count byte, entries ...byte) []byte {
+		payload := withCount(count)
+		// The nine-byte tail follows the environment-count field.
+		tail := append([]byte(nil), payload[len(payload)-9:]...)
+		payload = append(payload[:len(payload)-9], entries...)
+		return append(payload, tail...)
+	}
 	tests := []struct {
 		name    string
 		payload []byte
 	}{
 		{name: "truncated count", payload: base[:len(base)-1]},
-		{name: "truncated entry length", payload: append(withCount(1), 0, 0)},
+		{name: "truncated entry length", payload: withEntries(1, 0, 0)},
-		{name: "truncated entry body", payload: append(withCount(1), 0, 0, 0, 3, 'x')},
+		{name: "truncated entry body", payload: withEntries(1, 0, 0, 0, 3, 'x')},
 		{name: "impossible count", payload: withCount(2)},
 		{name: "trailing garbage", payload: append(append([]byte(nil), base...), 0xff)},
 	}
 		t.Run(tt.name, func(t *testing.T) {
 			_, err := UnmarshalHello(tt.payload)
 			require.Error(t, err)
+			if tt.name != "trailing garbage" {
+				require.ErrorIs(t, err, errShortPayload)
+			}
 		})
 	}
 }