Merge pull request #184 from bnema/fix/remote-protocol-hardening
fix(remote): validate rich attach targets
- commit
- c1cc94ce9ddd02747fff4b035464580f8d2c2b7c
- author
- bnema · · 2026-08-06 12:25 UTC
- committer
- GitHub · · 2026-08-06 12:25 UTC
- parent
- 1724a11
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
10 files · +235 -48
M cmd/vev-perf-harness/evidence.go +8 -10
| @@ -200,23 +200,22 @@ func mergeProcessTraces(mappings []processMapping) ([]span, error) { | |||
| 200 | 200 | // and exact correlation domain may pair, and each pair's ticks must retain | |
| 201 | 201 | // its own start-before-end ordering. | |
| 202 | 202 | starts := map[string]traceRecord{} | |
| 203 | - | discarded := map[string]struct{}{} | |
| 203 | + | traceIncomplete := false | |
| 204 | 204 | for _, r := range records { | |
| 205 | 205 | if r.Component == "observer" && r.Kind == "transport_diagnostic" && !r.Valid { | |
| 206 | - | // A bounded producer queue reports dropped marks explicitly. Any | |
| 207 | - | // in-flight span may have lost either endpoint, so discard those | |
| 208 | - | // partial samples; only their exact keys may be missing an end. | |
| 209 | - | for key := range starts { | |
| 210 | - | discarded[key] = struct{}{} | |
| 211 | - | } | |
| 206 | + | // A bounded producer queue reports that one or more marks were lost, | |
| 207 | + | // but not their correlation keys. A later unmatched end can therefore | |
| 208 | + | // be the end of a span whose start was dropped. Treat all subsequent | |
| 209 | + | // unpaired ends as incomplete evidence rather than rejecting the run. | |
| 210 | + | // Starts already observed before the gap are likewise incomplete. | |
| 212 | 211 | starts = map[string]traceRecord{} | |
| 212 | + | traceIncomplete = true | |
| 213 | 213 | continue | |
| 214 | 214 | } | |
| 215 | 215 | for _, pair := range spanPairs { | |
| 216 | 216 | key := fmt.Sprintf("%s/%s/%s/%d/%d/%d/%d", r.ProcessID, r.Component, m.Scenario, m.Run, r.Sequence, r.RequestID, r.Epoch) | |
| 217 | 217 | if r.Kind == pair.start { | |
| 218 | 218 | k := pair.name + "/" + key | |
| 219 | - | delete(discarded, k) | |
| 220 | 219 | if _, exists := starts[k]; exists { | |
| 221 | 220 | return nil, errors.New("duplicate process-local span start") | |
| 222 | 221 | } | |
| @@ -226,8 +225,7 @@ func mergeProcessTraces(mappings []processMapping) ([]span, error) { | |||
| 226 | 225 | k := pair.name + "/" + key | |
| 227 | 226 | start, ok := starts[k] | |
| 228 | 227 | if !ok { | |
| 229 | - | if _, ok := discarded[k]; ok { | |
| 230 | - | delete(discarded, k) | |
| 228 | + | if traceIncomplete { | |
| 231 | 229 | continue | |
| 232 | 230 | } | |
| 233 | 231 | return nil, errors.New("span end without same-process start") | |
| @@ -200,23 +200,22 @@ func mergeProcessTraces(mappings []processMapping) ([]span, error) { | |||||
| 200 | // and exact correlation domain may pair, and each pair's ticks must retain | 200 | // and exact correlation domain may pair, and each pair's ticks must retain | ||
| 201 | // its own start-before-end ordering. | 201 | // its own start-before-end ordering. | ||
| 202 | starts := map[string]traceRecord{} | 202 | starts := map[string]traceRecord{} | ||
| 203 | - | discarded := map[string]struct{}{} | 203 | + | traceIncomplete := false |
| 204 | for _, r := range records { | 204 | for _, r := range records { | ||
| 205 | if r.Component == "observer" && r.Kind == "transport_diagnostic" && !r.Valid { | 205 | if r.Component == "observer" && r.Kind == "transport_diagnostic" && !r.Valid { | ||
| 206 | - | // A bounded producer queue reports dropped marks explicitly. Any | 206 | + | // A bounded producer queue reports that one or more marks were lost, |
| 207 | - | // in-flight span may have lost either endpoint, so discard those | 207 | + | // but not their correlation keys. A later unmatched end can therefore |
| 208 | - | // partial samples; only their exact keys may be missing an end. | 208 | + | // be the end of a span whose start was dropped. Treat all subsequent |
| 209 | - | for key := range starts { | 209 | + | // unpaired ends as incomplete evidence rather than rejecting the run. |
| 210 | - | discarded[key] = struct{}{} | 210 | + | // Starts already observed before the gap are likewise incomplete. |
| 211 | - | } | |||
| 212 | starts = map[string]traceRecord{} | 211 | starts = map[string]traceRecord{} | ||
| 212 | + | traceIncomplete = true | |||
| 213 | continue | 213 | continue | ||
| 214 | } | 214 | } | ||
| 215 | for _, pair := range spanPairs { | 215 | for _, pair := range spanPairs { | ||
| 216 | key := fmt.Sprintf("%s/%s/%s/%d/%d/%d/%d", r.ProcessID, r.Component, m.Scenario, m.Run, r.Sequence, r.RequestID, r.Epoch) | 216 | key := fmt.Sprintf("%s/%s/%s/%d/%d/%d/%d", r.ProcessID, r.Component, m.Scenario, m.Run, r.Sequence, r.RequestID, r.Epoch) | ||
| 217 | if r.Kind == pair.start { | 217 | if r.Kind == pair.start { | ||
| 218 | k := pair.name + "/" + key | 218 | k := pair.name + "/" + key | ||
| 219 | - | delete(discarded, k) | |||
| 220 | if _, exists := starts[k]; exists { | 219 | if _, exists := starts[k]; exists { | ||
| 221 | return nil, errors.New("duplicate process-local span start") | 220 | return nil, errors.New("duplicate process-local span start") | ||
| 222 | } | 221 | } | ||
| @@ -226,8 +225,7 @@ func mergeProcessTraces(mappings []processMapping) ([]span, error) { | |||||
| 226 | k := pair.name + "/" + key | 225 | k := pair.name + "/" + key | ||
| 227 | start, ok := starts[k] | 226 | start, ok := starts[k] | ||
| 228 | if !ok { | 227 | if !ok { | ||
| 229 | - | if _, ok := discarded[k]; ok { | 228 | + | if traceIncomplete { |
| 230 | - | delete(discarded, k) | |||
| 231 | continue | 229 | continue | ||
| 232 | } | 230 | } | ||
| 233 | return nil, errors.New("span end without same-process start") | 231 | return nil, errors.New("span end without same-process start") | ||
M cmd/vev-perf-harness/evidence_test.go +4 -14
| @@ -364,7 +364,7 @@ func TestHarnessRejectsCrossProcessAndBadTraceSpans(t *testing.T) { | |||
| 364 | 364 | } | |
| 365 | 365 | } | |
| 366 | 366 | ||
| 367 | - | func TestHarnessObserverGapSuppressesOnlyDiscardedSpanKeys(t *testing.T) { | |
| 367 | + | func TestHarnessObserverGapSuppressesPotentiallyDroppedSpanEnds(t *testing.T) { | |
| 368 | 368 | base := func(kind string, sequence, requestID, epoch uint64, tick int64) traceRecord { | |
| 369 | 369 | return traceRecord{ProcessID: "one", Component: "ipc", Scenario: "s", Run: 1, Sequence: sequence, RequestID: requestID, Epoch: epoch, Kind: kind, Tick: tick, Valid: true} | |
| 370 | 370 | } | |
| @@ -372,10 +372,9 @@ func TestHarnessObserverGapSuppressesOnlyDiscardedSpanKeys(t *testing.T) { | |||
| 372 | 372 | for _, tc := range []struct { | |
| 373 | 373 | name string | |
| 374 | 374 | records []traceRecord | |
| 375 | - | wantErr bool | |
| 376 | 375 | }{ | |
| 377 | 376 | { | |
| 378 | - | name: "discarded span end is suppressed", | |
| 377 | + | name: "span began before observed gap", | |
| 379 | 378 | records: []traceRecord{ | |
| 380 | 379 | base("adapter_receive_start", 2, 2, 2, 10), | |
| 381 | 380 | gap, | |
| @@ -383,14 +382,11 @@ func TestHarnessObserverGapSuppressesOnlyDiscardedSpanKeys(t *testing.T) { | |||
| 383 | 382 | }, | |
| 384 | 383 | }, | |
| 385 | 384 | { | |
| 386 | - | name: "unrelated unmatched end remains an error", | |
| 385 | + | name: "span start was dropped before observed gap", | |
| 387 | 386 | records: []traceRecord{ | |
| 388 | - | base("adapter_receive_start", 2, 2, 2, 10), | |
| 389 | 387 | gap, | |
| 390 | - | base("adapter_receive_end", 2, 2, 2, 20), | |
| 391 | 388 | base("adapter_receive_end", 3, 3, 3, 30), | |
| 392 | 389 | }, | |
| 393 | - | wantErr: true, | |
| 394 | 390 | }, | |
| 395 | 391 | } { | |
| 396 | 392 | t.Run(tc.name, func(t *testing.T) { | |
| @@ -403,14 +399,8 @@ func TestHarnessObserverGapSuppressesOnlyDiscardedSpanKeys(t *testing.T) { | |||
| 403 | 399 | t.Fatal(err) | |
| 404 | 400 | } | |
| 405 | 401 | spans, err := mergeProcessTraces([]processMapping{{ProcessID: "one", ClockDomain: "one", TracePath: path, Scenario: "s", Run: 1}}) | |
| 406 | - | if tc.wantErr { | |
| 407 | - | if err == nil { | |
| 408 | - | t.Fatalf("unrelated unmatched end accepted: spans=%+v", spans) | |
| 409 | - | } | |
| 410 | - | return | |
| 411 | - | } | |
| 412 | 402 | if err != nil { | |
| 413 | - | t.Fatalf("discarded span end was rejected: %v", err) | |
| 403 | + | t.Fatalf("potentially dropped span end was rejected: %v", err) | |
| 414 | 404 | } | |
| 415 | 405 | if len(spans) != 0 { | |
| 416 | 406 | t.Fatalf("spans=%+v, want no sample after observer gap", spans) | |
| @@ -364,7 +364,7 @@ func TestHarnessRejectsCrossProcessAndBadTraceSpans(t *testing.T) { | |||||
| 364 | } | 364 | } | ||
| 365 | } | 365 | } | ||
| 366 | 366 | ||||
| 367 | - | func TestHarnessObserverGapSuppressesOnlyDiscardedSpanKeys(t *testing.T) { | 367 | + | func TestHarnessObserverGapSuppressesPotentiallyDroppedSpanEnds(t *testing.T) { |
| 368 | base := func(kind string, sequence, requestID, epoch uint64, tick int64) traceRecord { | 368 | base := func(kind string, sequence, requestID, epoch uint64, tick int64) traceRecord { | ||
| 369 | return traceRecord{ProcessID: "one", Component: "ipc", Scenario: "s", Run: 1, Sequence: sequence, RequestID: requestID, Epoch: epoch, Kind: kind, Tick: tick, Valid: true} | 369 | return traceRecord{ProcessID: "one", Component: "ipc", Scenario: "s", Run: 1, Sequence: sequence, RequestID: requestID, Epoch: epoch, Kind: kind, Tick: tick, Valid: true} | ||
| 370 | } | 370 | } | ||
| @@ -372,10 +372,9 @@ func TestHarnessObserverGapSuppressesOnlyDiscardedSpanKeys(t *testing.T) { | |||||
| 372 | for _, tc := range []struct { | 372 | for _, tc := range []struct { | ||
| 373 | name string | 373 | name string | ||
| 374 | records []traceRecord | 374 | records []traceRecord | ||
| 375 | - | wantErr bool | |||
| 376 | }{ | 375 | }{ | ||
| 377 | { | 376 | { | ||
| 378 | - | name: "discarded span end is suppressed", | 377 | + | name: "span began before observed gap", |
| 379 | records: []traceRecord{ | 378 | records: []traceRecord{ | ||
| 380 | base("adapter_receive_start", 2, 2, 2, 10), | 379 | base("adapter_receive_start", 2, 2, 2, 10), | ||
| 381 | gap, | 380 | gap, | ||
| @@ -383,14 +382,11 @@ func TestHarnessObserverGapSuppressesOnlyDiscardedSpanKeys(t *testing.T) { | |||||
| 383 | }, | 382 | }, | ||
| 384 | }, | 383 | }, | ||
| 385 | { | 384 | { | ||
| 386 | - | name: "unrelated unmatched end remains an error", | 385 | + | name: "span start was dropped before observed gap", |
| 387 | records: []traceRecord{ | 386 | records: []traceRecord{ | ||
| 388 | - | base("adapter_receive_start", 2, 2, 2, 10), | |||
| 389 | gap, | 387 | gap, | ||
| 390 | - | base("adapter_receive_end", 2, 2, 2, 20), | |||
| 391 | base("adapter_receive_end", 3, 3, 3, 30), | 388 | base("adapter_receive_end", 3, 3, 3, 30), | ||
| 392 | }, | 389 | }, | ||
| 393 | - | wantErr: true, | |||
| 394 | }, | 390 | }, | ||
| 395 | } { | 391 | } { | ||
| 396 | t.Run(tc.name, func(t *testing.T) { | 392 | t.Run(tc.name, func(t *testing.T) { | ||
| @@ -403,14 +399,8 @@ func TestHarnessObserverGapSuppressesOnlyDiscardedSpanKeys(t *testing.T) { | |||||
| 403 | t.Fatal(err) | 399 | t.Fatal(err) | ||
| 404 | } | 400 | } | ||
| 405 | spans, err := mergeProcessTraces([]processMapping{{ProcessID: "one", ClockDomain: "one", TracePath: path, Scenario: "s", Run: 1}}) | 401 | spans, err := mergeProcessTraces([]processMapping{{ProcessID: "one", ClockDomain: "one", TracePath: path, Scenario: "s", Run: 1}}) | ||
| 406 | - | if tc.wantErr { | |||
| 407 | - | if err == nil { | |||
| 408 | - | t.Fatalf("unrelated unmatched end accepted: spans=%+v", spans) | |||
| 409 | - | } | |||
| 410 | - | return | |||
| 411 | - | } | |||
| 412 | if err != nil { | 402 | if err != nil { | ||
| 413 | - | t.Fatalf("discarded span end was rejected: %v", err) | 403 | + | t.Fatalf("potentially dropped span end was rejected: %v", err) |
| 414 | } | 404 | } | ||
| 415 | if len(spans) != 0 { | 405 | if len(spans) != 0 { | ||
| 416 | t.Fatalf("spans=%+v, want no sample after observer gap", spans) | 406 | t.Fatalf("spans=%+v, want no sample after observer gap", spans) | ||
M internal/adapters/remote/preview.go +4 -0
| @@ -22,6 +22,7 @@ const ( | |||
| 22 | 22 | var ( | |
| 23 | 23 | errRemotePreviewSSH = errors.New("remote preview: ssh command failed") | |
| 24 | 24 | errRemotePreviewTooLarge = errors.New("remote preview: output exceeds size limit") | |
| 25 | + | errRemotePreviewIdentity = errors.New("remote preview: response identity mismatch") | |
| 25 | 26 | ) | |
| 26 | 27 | ||
| 27 | 28 | // PreviewClient invokes the hidden remote preview command over SSH. The | |
| @@ -83,6 +84,9 @@ func (c *PreviewClient) Preview(ctx context.Context, target domain.RemoteSession | |||
| 83 | 84 | if preview.Status != ports.RemotePreviewOK { | |
| 84 | 85 | return ports.RemotePreview{}, fmt.Errorf("remote preview unavailable: status %d", preview.Status) | |
| 85 | 86 | } | |
| 87 | + | if preview.LifecycleID != target.LifecycleID || preview.TabID != target.LiveTabID { | |
| 88 | + | return ports.RemotePreview{}, errRemotePreviewIdentity | |
| 89 | + | } | |
| 86 | 90 | return preview, nil | |
| 87 | 91 | } | |
| 88 | 92 | ||
| @@ -22,6 +22,7 @@ const ( | |||||
| 22 | var ( | 22 | var ( | ||
| 23 | errRemotePreviewSSH = errors.New("remote preview: ssh command failed") | 23 | errRemotePreviewSSH = errors.New("remote preview: ssh command failed") | ||
| 24 | errRemotePreviewTooLarge = errors.New("remote preview: output exceeds size limit") | 24 | errRemotePreviewTooLarge = errors.New("remote preview: output exceeds size limit") | ||
| 25 | + | errRemotePreviewIdentity = errors.New("remote preview: response identity mismatch") | |||
| 25 | ) | 26 | ) | ||
| 26 | 27 | ||||
| 27 | // PreviewClient invokes the hidden remote preview command over SSH. The | 28 | // PreviewClient invokes the hidden remote preview command over SSH. The | ||
| @@ -83,6 +84,9 @@ func (c *PreviewClient) Preview(ctx context.Context, target domain.RemoteSession | |||||
| 83 | if preview.Status != ports.RemotePreviewOK { | 84 | if preview.Status != ports.RemotePreviewOK { | ||
| 84 | return ports.RemotePreview{}, fmt.Errorf("remote preview unavailable: status %d", preview.Status) | 85 | return ports.RemotePreview{}, fmt.Errorf("remote preview unavailable: status %d", preview.Status) | ||
| 85 | } | 86 | } | ||
| 87 | + | if preview.LifecycleID != target.LifecycleID || preview.TabID != target.LiveTabID { | |||
| 88 | + | return ports.RemotePreview{}, errRemotePreviewIdentity | |||
| 89 | + | } | |||
| 86 | return preview, nil | 90 | return preview, nil | ||
| 87 | } | 91 | } | ||
| 88 | 92 | ||||
M internal/adapters/remote/preview_test.go +32 -0
| @@ -75,6 +75,38 @@ func TestRemotePreviewClientBuildsExactSSHCommandAndDecodesResponse(t *testing.T | |||
| 75 | 75 | require.Contains(t, gotArgs[2], "'_remote-preview'") | |
| 76 | 76 | } | |
| 77 | 77 | ||
| 78 | + | func TestRemotePreviewClientRejectsMismatchedResponseIdentity(t *testing.T) { | |
| 79 | + | target := previewClientTargetForTest() | |
| 80 | + | var otherLifecycle domain.SessionLifecycleID | |
| 81 | + | otherLifecycle[0] = 2 | |
| 82 | + | valid := ports.RemotePreview{ | |
| 83 | + | Version: ports.RemotePreviewSchemaVersion, Status: ports.RemotePreviewOK, | |
| 84 | + | LifecycleID: target.LifecycleID, TabID: target.LiveTabID, Revision: 1, Width: 1, Height: 1, | |
| 85 | + | Cells: []renderer.Cell{{Rune: 'x', Style: renderer.DefaultStyle()}}, | |
| 86 | + | } | |
| 87 | + | tests := []struct { | |
| 88 | + | name string | |
| 89 | + | mutate func(*ports.RemotePreview) | |
| 90 | + | }{ | |
| 91 | + | {name: "lifecycle ID", mutate: func(preview *ports.RemotePreview) { preview.LifecycleID = otherLifecycle }}, | |
| 92 | + | {name: "tab ID", mutate: func(preview *ports.RemotePreview) { preview.TabID = "tab-2" }}, | |
| 93 | + | } | |
| 94 | + | for _, tt := range tests { | |
| 95 | + | t.Run(tt.name, func(t *testing.T) { | |
| 96 | + | preview := valid | |
| 97 | + | tt.mutate(&preview) | |
| 98 | + | payload := ports.MarshalRemotePreview(preview) | |
| 99 | + | require.NotNil(t, payload) | |
| 100 | + | ||
| 101 | + | client := &PreviewClient{command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | |
| 102 | + | return stdoutCmd(ctx, string(payload)) | |
| 103 | + | }} | |
| 104 | + | _, err := client.Preview(context.Background(), target, 1, 1) | |
| 105 | + | require.ErrorIs(t, err, errRemotePreviewIdentity) | |
| 106 | + | }) | |
| 107 | + | } | |
| 108 | + | } | |
| 109 | + | ||
| 78 | 110 | func TestRemotePreviewClientRejectsMalformedResponse(t *testing.T) { | |
| 79 | 111 | client := &PreviewClient{command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | |
| 80 | 112 | return stdoutCmd(ctx, "not-a-preview") | |
| @@ -75,6 +75,38 @@ func TestRemotePreviewClientBuildsExactSSHCommandAndDecodesResponse(t *testing.T | |||||
| 75 | require.Contains(t, gotArgs[2], "'_remote-preview'") | 75 | require.Contains(t, gotArgs[2], "'_remote-preview'") | ||
| 76 | } | 76 | } | ||
| 77 | 77 | ||||
| 78 | + | func TestRemotePreviewClientRejectsMismatchedResponseIdentity(t *testing.T) { | |||
| 79 | + | target := previewClientTargetForTest() | |||
| 80 | + | var otherLifecycle domain.SessionLifecycleID | |||
| 81 | + | otherLifecycle[0] = 2 | |||
| 82 | + | valid := ports.RemotePreview{ | |||
| 83 | + | Version: ports.RemotePreviewSchemaVersion, Status: ports.RemotePreviewOK, | |||
| 84 | + | LifecycleID: target.LifecycleID, TabID: target.LiveTabID, Revision: 1, Width: 1, Height: 1, | |||
| 85 | + | Cells: []renderer.Cell{{Rune: 'x', Style: renderer.DefaultStyle()}}, | |||
| 86 | + | } | |||
| 87 | + | tests := []struct { | |||
| 88 | + | name string | |||
| 89 | + | mutate func(*ports.RemotePreview) | |||
| 90 | + | }{ | |||
| 91 | + | {name: "lifecycle ID", mutate: func(preview *ports.RemotePreview) { preview.LifecycleID = otherLifecycle }}, | |||
| 92 | + | {name: "tab ID", mutate: func(preview *ports.RemotePreview) { preview.TabID = "tab-2" }}, | |||
| 93 | + | } | |||
| 94 | + | for _, tt := range tests { | |||
| 95 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 96 | + | preview := valid | |||
| 97 | + | tt.mutate(&preview) | |||
| 98 | + | payload := ports.MarshalRemotePreview(preview) | |||
| 99 | + | require.NotNil(t, payload) | |||
| 100 | + | ||||
| 101 | + | client := &PreviewClient{command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | |||
| 102 | + | return stdoutCmd(ctx, string(payload)) | |||
| 103 | + | }} | |||
| 104 | + | _, err := client.Preview(context.Background(), target, 1, 1) | |||
| 105 | + | require.ErrorIs(t, err, errRemotePreviewIdentity) | |||
| 106 | + | }) | |||
| 107 | + | } | |||
| 108 | + | } | |||
| 109 | + | ||||
| 78 | func TestRemotePreviewClientRejectsMalformedResponse(t *testing.T) { | 110 | func TestRemotePreviewClientRejectsMalformedResponse(t *testing.T) { | ||
| 79 | client := &PreviewClient{command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | 111 | client := &PreviewClient{command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | ||
| 80 | return stdoutCmd(ctx, "not-a-preview") | 112 | return stdoutCmd(ctx, "not-a-preview") | ||
M internal/ports/remote_target_wire_test.go +104 -22
| @@ -79,29 +79,111 @@ func TestRemoteTargetWireRoundTripPreservesExactSelector(t *testing.T) { | |||
| 79 | 79 | } | |
| 80 | 80 | } | |
| 81 | 81 | ||
| 82 | - | func TestRemoteTargetWireRejectsTruncationAndTrailingData(t *testing.T) { | |
| 83 | - | target := testRemoteTarget(true) | |
| 84 | - | payload := MarshalAttachTarget(AttachTarget{ | |
| 85 | - | Endpoint: target.Endpoint, | |
| 86 | - | Session: target.SessionName, | |
| 87 | - | Intent: IntentAttach, | |
| 88 | - | RemoteTarget: target, | |
| 89 | - | EnvironmentPolicy: EnvironmentPolicyDaemonOwned, | |
| 90 | - | }) | |
| 91 | - | legacyLen := len(MarshalAttachTarget(AttachTarget{Endpoint: target.Endpoint, Session: target.SessionName, Intent: IntentAttach})) | |
| 92 | - | for i := 0; i < len(payload); i++ { | |
| 93 | - | if i == legacyLen { | |
| 94 | - | // The extension is optional for old peers; the complete legacy | |
| 95 | - | // prefix is therefore a valid count-only target by design. | |
| 96 | - | continue | |
| 97 | - | } | |
| 98 | - | if _, err := UnmarshalAttachTarget(payload[:i]); err == nil { | |
| 99 | - | t.Fatalf("prefix %d unexpectedly decoded", i) | |
| 100 | - | } | |
| 82 | + | func TestRemoteTargetWireRejectsClientOwnedEnvironmentPolicy(t *testing.T) { | |
| 83 | + | target := testRemoteTarget(false) | |
| 84 | + | tests := []struct { | |
| 85 | + | name string | |
| 86 | + | validate func() error | |
| 87 | + | marshalBad func() []byte | |
| 88 | + | marshalGood func() []byte | |
| 89 | + | marshalLegacy func() []byte | |
| 90 | + | unmarshal func([]byte) error | |
| 91 | + | }{ | |
| 92 | + | { | |
| 93 | + | name: "hello", | |
| 94 | + | validate: func() error { | |
| 95 | + | return ValidateHello(Hello{ | |
| 96 | + | Version: ProtocolVersion, Intent: IntentAttach, | |
| 97 | + | Name: target.SessionName, Size: domain.Size{Cols: 80, Rows: 24}, | |
| 98 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyClientOwned, | |
| 99 | + | }) | |
| 100 | + | }, | |
| 101 | + | marshalBad: func() []byte { | |
| 102 | + | return MarshalHello(Hello{ | |
| 103 | + | Version: ProtocolVersion, Intent: IntentAttach, | |
| 104 | + | Name: target.SessionName, Size: domain.Size{Cols: 80, Rows: 24}, | |
| 105 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyClientOwned, | |
| 106 | + | }) | |
| 107 | + | }, | |
| 108 | + | marshalGood: func() []byte { | |
| 109 | + | return MarshalHello(Hello{ | |
| 110 | + | Version: ProtocolVersion, Intent: IntentAttach, | |
| 111 | + | Name: target.SessionName, Size: domain.Size{Cols: 80, Rows: 24}, | |
| 112 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyDaemonOwned, | |
| 113 | + | }) | |
| 114 | + | }, | |
| 115 | + | marshalLegacy: func() []byte { | |
| 116 | + | return MarshalHello(Hello{ | |
| 117 | + | Version: ProtocolVersion, Intent: IntentAttach, | |
| 118 | + | Name: target.SessionName, Size: domain.Size{Cols: 80, Rows: 24}, | |
| 119 | + | }) | |
| 120 | + | }, | |
| 121 | + | unmarshal: func(payload []byte) error { _, err := UnmarshalHello(payload); return err }, | |
| 122 | + | }, | |
| 123 | + | { | |
| 124 | + | name: "attach target", | |
| 125 | + | validate: func() error { | |
| 126 | + | return ValidateAttachTarget(AttachTarget{ | |
| 127 | + | Endpoint: target.Endpoint, Session: target.SessionName, Intent: IntentAttach, | |
| 128 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyClientOwned, | |
| 129 | + | }) | |
| 130 | + | }, | |
| 131 | + | marshalBad: func() []byte { | |
| 132 | + | return MarshalAttachTarget(AttachTarget{ | |
| 133 | + | Endpoint: target.Endpoint, Session: target.SessionName, Intent: IntentAttach, | |
| 134 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyClientOwned, | |
| 135 | + | }) | |
| 136 | + | }, | |
| 137 | + | marshalGood: func() []byte { | |
| 138 | + | return MarshalAttachTarget(AttachTarget{ | |
| 139 | + | Endpoint: target.Endpoint, Session: target.SessionName, Intent: IntentAttach, | |
| 140 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyDaemonOwned, | |
| 141 | + | }) | |
| 142 | + | }, | |
| 143 | + | marshalLegacy: func() []byte { | |
| 144 | + | return MarshalAttachTarget(AttachTarget{ | |
| 145 | + | Endpoint: target.Endpoint, Session: target.SessionName, Intent: IntentAttach, | |
| 146 | + | }) | |
| 147 | + | }, | |
| 148 | + | unmarshal: func(payload []byte) error { _, err := UnmarshalAttachTarget(payload); return err }, | |
| 149 | + | }, | |
| 101 | 150 | } | |
| 102 | - | trailing := append(append([]byte(nil), payload...), 0xff) | |
| 103 | - | if _, err := UnmarshalAttachTarget(trailing); err == nil { | |
| 104 | - | t.Fatal("trailing garbage unexpectedly decoded") | |
| 151 | + | ||
| 152 | + | for _, tt := range tests { | |
| 153 | + | t.Run(tt.name, func(t *testing.T) { | |
| 154 | + | if err := tt.validate(); err == nil { | |
| 155 | + | t.Fatal("direct validation accepted client-owned policy for remote target") | |
| 156 | + | } | |
| 157 | + | if payload := tt.marshalBad(); payload != nil { | |
| 158 | + | t.Fatalf("marshal accepted client-owned policy: %x", payload) | |
| 159 | + | } | |
| 160 | + | payload := tt.marshalGood() | |
| 161 | + | if payload == nil { | |
| 162 | + | t.Fatal("failed to marshal daemon-owned control payload") | |
| 163 | + | } | |
| 164 | + | legacyLen := len(tt.marshalLegacy()) | |
| 165 | + | if err := tt.unmarshal(payload[:legacyLen]); err != nil { | |
| 166 | + | t.Fatalf("complete legacy prefix failed to decode: %v", err) | |
| 167 | + | } | |
| 168 | + | for i := 0; i < len(payload); i++ { | |
| 169 | + | if i == legacyLen { | |
| 170 | + | // The complete legacy prefix is a valid count-only payload by | |
| 171 | + | // design; every other rich-target prefix is malformed. | |
| 172 | + | continue | |
| 173 | + | } | |
| 174 | + | if err := tt.unmarshal(payload[:i]); err == nil { | |
| 175 | + | t.Fatalf("prefix %d unexpectedly decoded", i) | |
| 176 | + | } | |
| 177 | + | } | |
| 178 | + | trailing := append(append([]byte(nil), payload...), 0xff) | |
| 179 | + | if err := tt.unmarshal(trailing); err == nil { | |
| 180 | + | t.Fatal("trailing garbage unexpectedly decoded") | |
| 181 | + | } | |
| 182 | + | payload[len(payload)-1] = byte(EnvironmentPolicyClientOwned) | |
| 183 | + | if err := tt.unmarshal(payload); err == nil { | |
| 184 | + | t.Fatal("unmarshal accepted client-owned policy for remote target") | |
| 185 | + | } | |
| 186 | + | }) | |
| 105 | 187 | } | |
| 106 | 188 | } | |
| 107 | 189 | ||
| @@ -79,29 +79,111 @@ func TestRemoteTargetWireRoundTripPreservesExactSelector(t *testing.T) { | |||||
| 79 | } | 79 | } | ||
| 80 | } | 80 | } | ||
| 81 | 81 | ||||
| 82 | - | func TestRemoteTargetWireRejectsTruncationAndTrailingData(t *testing.T) { | 82 | + | func TestRemoteTargetWireRejectsClientOwnedEnvironmentPolicy(t *testing.T) { |
| 83 | - | target := testRemoteTarget(true) | 83 | + | target := testRemoteTarget(false) |
| 84 | - | payload := MarshalAttachTarget(AttachTarget{ | 84 | + | tests := []struct { |
| 85 | - | Endpoint: target.Endpoint, | 85 | + | name string |
| 86 | - | Session: target.SessionName, | 86 | + | validate func() error |
| 87 | - | Intent: IntentAttach, | 87 | + | marshalBad func() []byte |
| 88 | - | RemoteTarget: target, | 88 | + | marshalGood func() []byte |
| 89 | - | EnvironmentPolicy: EnvironmentPolicyDaemonOwned, | 89 | + | marshalLegacy func() []byte |
| 90 | - | }) | 90 | + | unmarshal func([]byte) error |
| 91 | - | legacyLen := len(MarshalAttachTarget(AttachTarget{Endpoint: target.Endpoint, Session: target.SessionName, Intent: IntentAttach})) | 91 | + | }{ |
| 92 | - | for i := 0; i < len(payload); i++ { | 92 | + | { |
| 93 | - | if i == legacyLen { | 93 | + | name: "hello", |
| 94 | - | // The extension is optional for old peers; the complete legacy | 94 | + | validate: func() error { |
| 95 | - | // prefix is therefore a valid count-only target by design. | 95 | + | return ValidateHello(Hello{ |
| 96 | - | continue | 96 | + | Version: ProtocolVersion, Intent: IntentAttach, |
| 97 | - | } | 97 | + | Name: target.SessionName, Size: domain.Size{Cols: 80, Rows: 24}, |
| 98 | - | if _, err := UnmarshalAttachTarget(payload[:i]); err == nil { | 98 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyClientOwned, |
| 99 | - | t.Fatalf("prefix %d unexpectedly decoded", i) | 99 | + | }) |
| 100 | - | } | 100 | + | }, |
| 101 | + | marshalBad: func() []byte { | |||
| 102 | + | return MarshalHello(Hello{ | |||
| 103 | + | Version: ProtocolVersion, Intent: IntentAttach, | |||
| 104 | + | Name: target.SessionName, Size: domain.Size{Cols: 80, Rows: 24}, | |||
| 105 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyClientOwned, | |||
| 106 | + | }) | |||
| 107 | + | }, | |||
| 108 | + | marshalGood: func() []byte { | |||
| 109 | + | return MarshalHello(Hello{ | |||
| 110 | + | Version: ProtocolVersion, Intent: IntentAttach, | |||
| 111 | + | Name: target.SessionName, Size: domain.Size{Cols: 80, Rows: 24}, | |||
| 112 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyDaemonOwned, | |||
| 113 | + | }) | |||
| 114 | + | }, | |||
| 115 | + | marshalLegacy: func() []byte { | |||
| 116 | + | return MarshalHello(Hello{ | |||
| 117 | + | Version: ProtocolVersion, Intent: IntentAttach, | |||
| 118 | + | Name: target.SessionName, Size: domain.Size{Cols: 80, Rows: 24}, | |||
| 119 | + | }) | |||
| 120 | + | }, | |||
| 121 | + | unmarshal: func(payload []byte) error { _, err := UnmarshalHello(payload); return err }, | |||
| 122 | + | }, | |||
| 123 | + | { | |||
| 124 | + | name: "attach target", | |||
| 125 | + | validate: func() error { | |||
| 126 | + | return ValidateAttachTarget(AttachTarget{ | |||
| 127 | + | Endpoint: target.Endpoint, Session: target.SessionName, Intent: IntentAttach, | |||
| 128 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyClientOwned, | |||
| 129 | + | }) | |||
| 130 | + | }, | |||
| 131 | + | marshalBad: func() []byte { | |||
| 132 | + | return MarshalAttachTarget(AttachTarget{ | |||
| 133 | + | Endpoint: target.Endpoint, Session: target.SessionName, Intent: IntentAttach, | |||
| 134 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyClientOwned, | |||
| 135 | + | }) | |||
| 136 | + | }, | |||
| 137 | + | marshalGood: func() []byte { | |||
| 138 | + | return MarshalAttachTarget(AttachTarget{ | |||
| 139 | + | Endpoint: target.Endpoint, Session: target.SessionName, Intent: IntentAttach, | |||
| 140 | + | RemoteTarget: target, EnvironmentPolicy: EnvironmentPolicyDaemonOwned, | |||
| 141 | + | }) | |||
| 142 | + | }, | |||
| 143 | + | marshalLegacy: func() []byte { | |||
| 144 | + | return MarshalAttachTarget(AttachTarget{ | |||
| 145 | + | Endpoint: target.Endpoint, Session: target.SessionName, Intent: IntentAttach, | |||
| 146 | + | }) | |||
| 147 | + | }, | |||
| 148 | + | unmarshal: func(payload []byte) error { _, err := UnmarshalAttachTarget(payload); return err }, | |||
| 149 | + | }, | |||
| 101 | } | 150 | } | ||
| 102 | - | trailing := append(append([]byte(nil), payload...), 0xff) | 151 | + | |
| 103 | - | if _, err := UnmarshalAttachTarget(trailing); err == nil { | 152 | + | for _, tt := range tests { |
| 104 | - | t.Fatal("trailing garbage unexpectedly decoded") | 153 | + | t.Run(tt.name, func(t *testing.T) { |
| 154 | + | if err := tt.validate(); err == nil { | |||
| 155 | + | t.Fatal("direct validation accepted client-owned policy for remote target") | |||
| 156 | + | } | |||
| 157 | + | if payload := tt.marshalBad(); payload != nil { | |||
| 158 | + | t.Fatalf("marshal accepted client-owned policy: %x", payload) | |||
| 159 | + | } | |||
| 160 | + | payload := tt.marshalGood() | |||
| 161 | + | if payload == nil { | |||
| 162 | + | t.Fatal("failed to marshal daemon-owned control payload") | |||
| 163 | + | } | |||
| 164 | + | legacyLen := len(tt.marshalLegacy()) | |||
| 165 | + | if err := tt.unmarshal(payload[:legacyLen]); err != nil { | |||
| 166 | + | t.Fatalf("complete legacy prefix failed to decode: %v", err) | |||
| 167 | + | } | |||
| 168 | + | for i := 0; i < len(payload); i++ { | |||
| 169 | + | if i == legacyLen { | |||
| 170 | + | // The complete legacy prefix is a valid count-only payload by | |||
| 171 | + | // design; every other rich-target prefix is malformed. | |||
| 172 | + | continue | |||
| 173 | + | } | |||
| 174 | + | if err := tt.unmarshal(payload[:i]); err == nil { | |||
| 175 | + | t.Fatalf("prefix %d unexpectedly decoded", i) | |||
| 176 | + | } | |||
| 177 | + | } | |||
| 178 | + | trailing := append(append([]byte(nil), payload...), 0xff) | |||
| 179 | + | if err := tt.unmarshal(trailing); err == nil { | |||
| 180 | + | t.Fatal("trailing garbage unexpectedly decoded") | |||
| 181 | + | } | |||
| 182 | + | payload[len(payload)-1] = byte(EnvironmentPolicyClientOwned) | |||
| 183 | + | if err := tt.unmarshal(payload); err == nil { | |||
| 184 | + | t.Fatal("unmarshal accepted client-owned policy for remote target") | |||
| 185 | + | } | |||
| 186 | + | }) | |||
| 105 | } | 187 | } | ||
| 106 | } | 188 | } | ||
| 107 | 189 | ||||
M internal/ports/wire.go +6 -0
| @@ -712,6 +712,9 @@ func ValidateHello(h Hello) error { | |||
| 712 | 712 | } | |
| 713 | 713 | return nil | |
| 714 | 714 | } | |
| 715 | + | if h.EnvironmentPolicy != EnvironmentPolicyDaemonOwned { | |
| 716 | + | return ErrInvalidHello | |
| 717 | + | } | |
| 715 | 718 | if h.Intent != IntentAttach && h.Intent != IntentResume { | |
| 716 | 719 | return ErrInvalidHello | |
| 717 | 720 | } | |
| @@ -771,6 +774,9 @@ func ValidateAttachTarget(m AttachTarget) error { | |||
| 771 | 774 | } | |
| 772 | 775 | return nil | |
| 773 | 776 | } | |
| 777 | + | if m.EnvironmentPolicy != EnvironmentPolicyDaemonOwned { | |
| 778 | + | return ErrInvalidAttachTarget | |
| 779 | + | } | |
| 774 | 780 | if m.Intent != IntentAttach && m.Intent != IntentResume { | |
| 775 | 781 | return ErrInvalidAttachTarget | |
| 776 | 782 | } | |
| @@ -712,6 +712,9 @@ func ValidateHello(h Hello) error { | |||||
| 712 | } | 712 | } | ||
| 713 | return nil | 713 | return nil | ||
| 714 | } | 714 | } | ||
| 715 | + | if h.EnvironmentPolicy != EnvironmentPolicyDaemonOwned { | |||
| 716 | + | return ErrInvalidHello | |||
| 717 | + | } | |||
| 715 | if h.Intent != IntentAttach && h.Intent != IntentResume { | 718 | if h.Intent != IntentAttach && h.Intent != IntentResume { | ||
| 716 | return ErrInvalidHello | 719 | return ErrInvalidHello | ||
| 717 | } | 720 | } | ||
| @@ -771,6 +774,9 @@ func ValidateAttachTarget(m AttachTarget) error { | |||||
| 771 | } | 774 | } | ||
| 772 | return nil | 775 | return nil | ||
| 773 | } | 776 | } | ||
| 777 | + | if m.EnvironmentPolicy != EnvironmentPolicyDaemonOwned { | |||
| 778 | + | return ErrInvalidAttachTarget | |||
| 779 | + | } | |||
| 774 | if m.Intent != IntentAttach && m.Intent != IntentResume { | 780 | if m.Intent != IntentAttach && m.Intent != IntentResume { | ||
| 775 | return ErrInvalidAttachTarget | 781 | return ErrInvalidAttachTarget | ||
| 776 | } | 782 | } | ||
M internal/usecase/client/client.go +2 -2
| @@ -236,8 +236,8 @@ func validateAttachRequest(request AttachRequest) error { | |||
| 236 | 236 | if request.SessionName != request.RemoteTarget.SessionName { | |
| 237 | 237 | return errors.New("vev: remote attach target session mismatch") | |
| 238 | 238 | } | |
| 239 | - | if request.EnvironmentPolicy != ports.EnvironmentPolicyClientOwned && request.EnvironmentPolicy != ports.EnvironmentPolicyDaemonOwned { | |
| 240 | - | return errors.New("vev: invalid remote environment policy") | |
| 239 | + | if request.EnvironmentPolicy != ports.EnvironmentPolicyDaemonOwned { | |
| 240 | + | return errors.New("vev: remote attach target requires daemon-owned environment") | |
| 241 | 241 | } | |
| 242 | 242 | return nil | |
| 243 | 243 | } | |
| @@ -236,8 +236,8 @@ func validateAttachRequest(request AttachRequest) error { | |||||
| 236 | if request.SessionName != request.RemoteTarget.SessionName { | 236 | if request.SessionName != request.RemoteTarget.SessionName { | ||
| 237 | return errors.New("vev: remote attach target session mismatch") | 237 | return errors.New("vev: remote attach target session mismatch") | ||
| 238 | } | 238 | } | ||
| 239 | - | if request.EnvironmentPolicy != ports.EnvironmentPolicyClientOwned && request.EnvironmentPolicy != ports.EnvironmentPolicyDaemonOwned { | 239 | + | if request.EnvironmentPolicy != ports.EnvironmentPolicyDaemonOwned { |
| 240 | - | return errors.New("vev: invalid remote environment policy") | 240 | + | return errors.New("vev: remote attach target requires daemon-owned environment") |
| 241 | } | 241 | } | ||
| 242 | return nil | 242 | return nil | ||
| 243 | } | 243 | } | ||
A internal/usecase/client/remote_attach_validation_test.go +42 -0
| @@ -0,0 +1,42 @@ | |||
| 1 | + | package client | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/stretchr/testify/require" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/vev/internal/domain" | |
| 9 | + | "github.com/bnema/vev/internal/ports" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | func TestValidateAttachRequestRequiresDaemonOwnedEnvironmentForRemoteTarget(t *testing.T) { | |
| 13 | + | var lifecycle domain.SessionLifecycleID | |
| 14 | + | lifecycle[0] = 1 | |
| 15 | + | target := &domain.RemoteSessionTarget{ | |
| 16 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, | |
| 17 | + | SessionName: "work", LiveTabID: "tab-1", | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | tests := []struct { | |
| 21 | + | name string | |
| 22 | + | policy ports.EnvironmentPolicy | |
| 23 | + | wantOK bool | |
| 24 | + | }{ | |
| 25 | + | {name: "daemon owned", policy: ports.EnvironmentPolicyDaemonOwned, wantOK: true}, | |
| 26 | + | {name: "client owned", policy: ports.EnvironmentPolicyClientOwned}, | |
| 27 | + | {name: "unknown", policy: ports.EnvironmentPolicy(99)}, | |
| 28 | + | } | |
| 29 | + | for _, test := range tests { | |
| 30 | + | t.Run(test.name, func(t *testing.T) { | |
| 31 | + | err := validateAttachRequest(AttachRequest{ | |
| 32 | + | Intent: ports.IntentAttach, SessionName: target.SessionName, | |
| 33 | + | RemoteTarget: target, EnvironmentPolicy: test.policy, | |
| 34 | + | }) | |
| 35 | + | if test.wantOK { | |
| 36 | + | require.NoError(t, err) | |
| 37 | + | return | |
| 38 | + | } | |
| 39 | + | require.EqualError(t, err, "vev: remote attach target requires daemon-owned environment") | |
| 40 | + | }) | |
| 41 | + | } | |
| 42 | + | } | |
| @@ -0,0 +1,42 @@ | |||||
| 1 | + | package client | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/stretchr/testify/require" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/vev/internal/domain" | |||
| 9 | + | "github.com/bnema/vev/internal/ports" | |||
| 10 | + | ) | |||
| 11 | + | ||||
| 12 | + | func TestValidateAttachRequestRequiresDaemonOwnedEnvironmentForRemoteTarget(t *testing.T) { | |||
| 13 | + | var lifecycle domain.SessionLifecycleID | |||
| 14 | + | lifecycle[0] = 1 | |||
| 15 | + | target := &domain.RemoteSessionTarget{ | |||
| 16 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, | |||
| 17 | + | SessionName: "work", LiveTabID: "tab-1", | |||
| 18 | + | } | |||
| 19 | + | ||||
| 20 | + | tests := []struct { | |||
| 21 | + | name string | |||
| 22 | + | policy ports.EnvironmentPolicy | |||
| 23 | + | wantOK bool | |||
| 24 | + | }{ | |||
| 25 | + | {name: "daemon owned", policy: ports.EnvironmentPolicyDaemonOwned, wantOK: true}, | |||
| 26 | + | {name: "client owned", policy: ports.EnvironmentPolicyClientOwned}, | |||
| 27 | + | {name: "unknown", policy: ports.EnvironmentPolicy(99)}, | |||
| 28 | + | } | |||
| 29 | + | for _, test := range tests { | |||
| 30 | + | t.Run(test.name, func(t *testing.T) { | |||
| 31 | + | err := validateAttachRequest(AttachRequest{ | |||
| 32 | + | Intent: ports.IntentAttach, SessionName: target.SessionName, | |||
| 33 | + | RemoteTarget: target, EnvironmentPolicy: test.policy, | |||
| 34 | + | }) | |||
| 35 | + | if test.wantOK { | |||
| 36 | + | require.NoError(t, err) | |||
| 37 | + | return | |||
| 38 | + | } | |||
| 39 | + | require.EqualError(t, err, "vev: remote attach target requires daemon-owned environment") | |||
| 40 | + | }) | |||
| 41 | + | } | |||
| 42 | + | } | |||
M internal/usecase/daemon/picker.go +6 -0
| @@ -941,6 +941,12 @@ func (d *Daemon) remotePickerTargetReadyTarget(target domain.RemoteSessionTarget | |||
| 941 | 941 | } | |
| 942 | 942 | d.remoteCatalog.mu.Lock() | |
| 943 | 943 | defer d.remoteCatalog.mu.Unlock() | |
| 944 | + | now := d.clock.Now() | |
| 945 | + | if d.remoteCatalog.status[target.Endpoint] == remoteHostFresh { | |
| 946 | + | if entry, ok := d.remoteCatalog.cache[target.Endpoint]; ok && remoteCatalogExpired(entry.FetchedAt, now) { | |
| 947 | + | d.remoteCatalog.status[target.Endpoint] = remoteHostStale | |
| 948 | + | } | |
| 949 | + | } | |
| 944 | 950 | if d.remoteCatalog.status[target.Endpoint] != remoteHostFresh { | |
| 945 | 951 | return false | |
| 946 | 952 | } | |
| @@ -941,6 +941,12 @@ func (d *Daemon) remotePickerTargetReadyTarget(target domain.RemoteSessionTarget | |||||
| 941 | } | 941 | } | ||
| 942 | d.remoteCatalog.mu.Lock() | 942 | d.remoteCatalog.mu.Lock() | ||
| 943 | defer d.remoteCatalog.mu.Unlock() | 943 | defer d.remoteCatalog.mu.Unlock() | ||
| 944 | + | now := d.clock.Now() | |||
| 945 | + | if d.remoteCatalog.status[target.Endpoint] == remoteHostFresh { | |||
| 946 | + | if entry, ok := d.remoteCatalog.cache[target.Endpoint]; ok && remoteCatalogExpired(entry.FetchedAt, now) { | |||
| 947 | + | d.remoteCatalog.status[target.Endpoint] = remoteHostStale | |||
| 948 | + | } | |||
| 949 | + | } | |||
| 944 | if d.remoteCatalog.status[target.Endpoint] != remoteHostFresh { | 950 | if d.remoteCatalog.status[target.Endpoint] != remoteHostFresh { | ||
| 945 | return false | 951 | return false | ||
| 946 | } | 952 | } | ||
M internal/usecase/daemon/remote_picker_test.go +27 -0
| @@ -76,6 +76,33 @@ func TestRemotePickerCatalogExpiresAtAttachTTL(t *testing.T) { | |||
| 76 | 76 | require.False(t, d.remotePickerTargetReadyTarget(target), "attachment must reject a catalogue exactly at the TTL boundary") | |
| 77 | 77 | } | |
| 78 | 78 | ||
| 79 | + | func TestRemotePickerTargetReadyExpiresAfterPickerBuildWithoutSnapshot(t *testing.T) { | |
| 80 | + | now := time.Unix(100, 0) | |
| 81 | + | clock := &remotePreviewTestClock{now: now} | |
| 82 | + | d := newTestDaemon(t, nil, clock) | |
| 83 | + | lifecycle := remoteLifecycleForTest() | |
| 84 | + | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | |
| 85 | + | Host: "arch", FetchedAt: now, Sessions: []ports.RemoteCatalogSession{{ | |
| 86 | + | LifecycleID: lifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | |
| 87 | + | }}, | |
| 88 | + | }}) | |
| 89 | + | d.remoteCatalog.mu.Lock() | |
| 90 | + | d.remoteCatalog.status["arch"] = remoteHostFresh | |
| 91 | + | d.remoteCatalog.mu.Unlock() | |
| 92 | + | ||
| 93 | + | model := d.newPickerModel(nil, nil, pickerNavigate, moveSourceLocator{}, picker.SourceFilter{}) | |
| 94 | + | target, ok := model.Selected() | |
| 95 | + | require.True(t, ok) | |
| 96 | + | require.NotNil(t, target.RemoteTarget) | |
| 97 | + | require.True(t, d.remotePickerTargetReadyTarget(*target.RemoteTarget), "fresh picker target must be ready before the attach TTL") | |
| 98 | + | ||
| 99 | + | clock.Advance(remotePickerAttachTTL) | |
| 100 | + | require.False(t, d.remotePickerTargetReadyTarget(*target.RemoteTarget), "handoff must reject a picker target whose catalog reached the attach TTL") | |
| 101 | + | d.remoteCatalog.mu.Lock() | |
| 102 | + | require.Equal(t, remoteHostStale, d.remoteCatalog.status["arch"]) | |
| 103 | + | d.remoteCatalog.mu.Unlock() | |
| 104 | + | } | |
| 105 | + | ||
| 79 | 106 | func TestRemotePickerSelectsCatalogActiveTab(t *testing.T) { | |
| 80 | 107 | tests := []struct { | |
| 81 | 108 | name string | |
| @@ -76,6 +76,33 @@ func TestRemotePickerCatalogExpiresAtAttachTTL(t *testing.T) { | |||||
| 76 | require.False(t, d.remotePickerTargetReadyTarget(target), "attachment must reject a catalogue exactly at the TTL boundary") | 76 | require.False(t, d.remotePickerTargetReadyTarget(target), "attachment must reject a catalogue exactly at the TTL boundary") | ||
| 77 | } | 77 | } | ||
| 78 | 78 | ||||
| 79 | + | func TestRemotePickerTargetReadyExpiresAfterPickerBuildWithoutSnapshot(t *testing.T) { | |||
| 80 | + | now := time.Unix(100, 0) | |||
| 81 | + | clock := &remotePreviewTestClock{now: now} | |||
| 82 | + | d := newTestDaemon(t, nil, clock) | |||
| 83 | + | lifecycle := remoteLifecycleForTest() | |||
| 84 | + | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | |||
| 85 | + | Host: "arch", FetchedAt: now, Sessions: []ports.RemoteCatalogSession{{ | |||
| 86 | + | LifecycleID: lifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | |||
| 87 | + | }}, | |||
| 88 | + | }}) | |||
| 89 | + | d.remoteCatalog.mu.Lock() | |||
| 90 | + | d.remoteCatalog.status["arch"] = remoteHostFresh | |||
| 91 | + | d.remoteCatalog.mu.Unlock() | |||
| 92 | + | ||||
| 93 | + | model := d.newPickerModel(nil, nil, pickerNavigate, moveSourceLocator{}, picker.SourceFilter{}) | |||
| 94 | + | target, ok := model.Selected() | |||
| 95 | + | require.True(t, ok) | |||
| 96 | + | require.NotNil(t, target.RemoteTarget) | |||
| 97 | + | require.True(t, d.remotePickerTargetReadyTarget(*target.RemoteTarget), "fresh picker target must be ready before the attach TTL") | |||
| 98 | + | ||||
| 99 | + | clock.Advance(remotePickerAttachTTL) | |||
| 100 | + | require.False(t, d.remotePickerTargetReadyTarget(*target.RemoteTarget), "handoff must reject a picker target whose catalog reached the attach TTL") | |||
| 101 | + | d.remoteCatalog.mu.Lock() | |||
| 102 | + | require.Equal(t, remoteHostStale, d.remoteCatalog.status["arch"]) | |||
| 103 | + | d.remoteCatalog.mu.Unlock() | |||
| 104 | + | } | |||
| 105 | + | ||||
| 79 | func TestRemotePickerSelectsCatalogActiveTab(t *testing.T) { | 106 | func TestRemotePickerSelectsCatalogActiveTab(t *testing.T) { | ||
| 80 | tests := []struct { | 107 | tests := []struct { | ||
| 81 | name string | 108 | name string | ||