fix(remote): enforce exact catalogue authority
- commit
- 81f1e151338c5fcfef6ca78db64d8f6bff46c838
- author
- brice · · 2026-08-24 12:03 UTC
- parent
- 2be2cd7
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +224 -34
M internal/adapters/remote/catalog.go +77 -22
| @@ -111,55 +111,99 @@ func (c *CatalogClient) List(ctx context.Context, target string) (ports.RemoteCa | |||
| 111 | 111 | return catalog, nil | |
| 112 | 112 | } | |
| 113 | 113 | ||
| 114 | + | type catalogVersionEnvelope struct { | |
| 115 | + | ProtocolVersion *uint16 `json:"protocol_version"` | |
| 116 | + | SchemaVersion *uint16 `json:"schema_version"` | |
| 117 | + | Sessions json.RawMessage `json:"sessions"` | |
| 118 | + | } | |
| 119 | + | ||
| 114 | 120 | type catalogEnvelope struct { | |
| 115 | - | ProtocolVersion *uint16 `json:"protocol_version"` | |
| 116 | - | SchemaVersion *uint16 `json:"schema_version"` | |
| 117 | - | Sessions *[]ports.RemoteCatalogSession `json:"sessions"` | |
| 121 | + | ProtocolVersion *uint16 `json:"protocol_version"` | |
| 122 | + | SchemaVersion *uint16 `json:"schema_version"` | |
| 123 | + | Sessions *[]catalogSessionEnvelope `json:"sessions"` | |
| 124 | + | } | |
| 125 | + | ||
| 126 | + | type catalogSessionEnvelope struct { | |
| 127 | + | LifecycleID *domain.SessionLifecycleID `json:"lifecycle_id"` | |
| 128 | + | Name *string `json:"name"` | |
| 129 | + | State *ports.RemoteCatalogSessionState `json:"state"` | |
| 130 | + | Ephemeral *bool `json:"ephemeral"` | |
| 131 | + | Tabs *[]catalogTabEnvelope `json:"tabs"` | |
| 132 | + | Attached *bool `json:"attached"` | |
| 133 | + | LastUsedSeq uint64 `json:"last_used_seq,omitempty"` | |
| 134 | + | ActiveTabID string `json:"active_tab_id,omitempty"` | |
| 135 | + | Reason string `json:"reason,omitempty"` | |
| 136 | + | } | |
| 137 | + | ||
| 138 | + | type catalogTabEnvelope struct { | |
| 139 | + | ID *string `json:"id"` | |
| 140 | + | Index *uint16 `json:"index"` | |
| 141 | + | Name *string `json:"name"` | |
| 142 | + | Detail string `json:"detail,omitempty"` | |
| 143 | + | Attention bool `json:"attention,omitempty"` | |
| 118 | 144 | } | |
| 119 | 145 | ||
| 120 | 146 | func decodeRemoteCatalog(raw []byte) (ports.RemoteCatalog, error) { | |
| 121 | - | dec := json.NewDecoder(bytes.NewReader(raw)) | |
| 122 | - | var envelope catalogEnvelope | |
| 123 | - | if err := dec.Decode(&envelope); err != nil { | |
| 124 | - | return ports.RemoteCatalog{}, fmt.Errorf("%w: %w", errCatalogDecode, err) | |
| 147 | + | var version catalogVersionEnvelope | |
| 148 | + | if err := decodeCatalogEnvelope(raw, false, &version); err != nil { | |
| 149 | + | return ports.RemoteCatalog{}, err | |
| 125 | 150 | } | |
| 126 | - | if envelope.ProtocolVersion == nil || envelope.Sessions == nil { | |
| 151 | + | if version.ProtocolVersion == nil { | |
| 127 | 152 | return ports.RemoteCatalog{}, errCatalogRequired | |
| 128 | 153 | } | |
| 129 | - | if envelope.SchemaVersion == nil { | |
| 154 | + | if version.SchemaVersion == nil { | |
| 130 | 155 | return ports.RemoteCatalog{}, &ports.RemoteCatalogVersionMismatchError{ | |
| 131 | 156 | Got: 0, | |
| 132 | 157 | Want: ports.RemoteCatalogSchemaVersion, | |
| 133 | 158 | Kind: "catalog", | |
| 134 | 159 | } | |
| 135 | 160 | } | |
| 136 | - | if err := rejectTrailingJSON(dec); err != nil { | |
| 137 | - | return ports.RemoteCatalog{}, err | |
| 138 | - | } | |
| 139 | - | ||
| 140 | - | schemaVersion := *envelope.SchemaVersion | |
| 141 | - | if *envelope.ProtocolVersion != ports.ProtocolVersion { | |
| 161 | + | if *version.ProtocolVersion != ports.ProtocolVersion { | |
| 142 | 162 | return ports.RemoteCatalog{}, &ports.RemoteCatalogVersionMismatchError{ | |
| 143 | - | Got: *envelope.ProtocolVersion, | |
| 163 | + | Got: *version.ProtocolVersion, | |
| 144 | 164 | Want: ports.ProtocolVersion, | |
| 145 | 165 | Kind: "protocol", | |
| 146 | 166 | } | |
| 147 | 167 | } | |
| 148 | - | if schemaVersion != ports.RemoteCatalogSchemaVersion { | |
| 168 | + | if *version.SchemaVersion != ports.RemoteCatalogSchemaVersion { | |
| 149 | 169 | return ports.RemoteCatalog{}, &ports.RemoteCatalogVersionMismatchError{ | |
| 150 | - | Got: schemaVersion, | |
| 170 | + | Got: *version.SchemaVersion, | |
| 151 | 171 | Want: ports.RemoteCatalogSchemaVersion, | |
| 152 | 172 | Kind: "catalog", | |
| 153 | 173 | } | |
| 154 | 174 | } | |
| 155 | 175 | ||
| 156 | - | sessions := *envelope.Sessions | |
| 157 | - | if sessions == nil { | |
| 158 | - | sessions = []ports.RemoteCatalogSession{} | |
| 176 | + | var envelope catalogEnvelope | |
| 177 | + | if err := decodeCatalogEnvelope(raw, true, &envelope); err != nil { | |
| 178 | + | return ports.RemoteCatalog{}, err | |
| 179 | + | } | |
| 180 | + | if envelope.ProtocolVersion == nil || envelope.SchemaVersion == nil || envelope.Sessions == nil { | |
| 181 | + | return ports.RemoteCatalog{}, errCatalogRequired | |
| 182 | + | } | |
| 183 | + | sessions := make([]ports.RemoteCatalogSession, 0, len(*envelope.Sessions)) | |
| 184 | + | for _, session := range *envelope.Sessions { | |
| 185 | + | if session.LifecycleID == nil || session.Name == nil || session.State == nil || session.Ephemeral == nil || session.Tabs == nil || session.Attached == nil { | |
| 186 | + | return ports.RemoteCatalog{}, errCatalogRequired | |
| 187 | + | } | |
| 188 | + | tabs := make([]ports.RemoteCatalogTab, 0, len(*session.Tabs)) | |
| 189 | + | for _, tab := range *session.Tabs { | |
| 190 | + | if tab.ID == nil || tab.Index == nil || tab.Name == nil { | |
| 191 | + | return ports.RemoteCatalog{}, errCatalogRequired | |
| 192 | + | } | |
| 193 | + | tabs = append(tabs, ports.RemoteCatalogTab{ | |
| 194 | + | ID: *tab.ID, Index: *tab.Index, Name: *tab.Name, | |
| 195 | + | Detail: tab.Detail, Attention: tab.Attention, | |
| 196 | + | }) | |
| 197 | + | } | |
| 198 | + | sessions = append(sessions, ports.RemoteCatalogSession{ | |
| 199 | + | LifecycleID: *session.LifecycleID, Name: *session.Name, State: *session.State, | |
| 200 | + | Ephemeral: *session.Ephemeral, Tabs: tabs, Attached: *session.Attached, | |
| 201 | + | LastUsedSeq: session.LastUsedSeq, ActiveTabID: session.ActiveTabID, Reason: session.Reason, | |
| 202 | + | }) | |
| 159 | 203 | } | |
| 160 | 204 | catalog := ports.RemoteCatalog{ | |
| 161 | 205 | ProtocolVersion: *envelope.ProtocolVersion, | |
| 162 | - | SchemaVersion: schemaVersion, | |
| 206 | + | SchemaVersion: *envelope.SchemaVersion, | |
| 163 | 207 | Sessions: sessions, | |
| 164 | 208 | } | |
| 165 | 209 | if err := ports.ValidateRemoteCatalog(catalog); err != nil { | |
| @@ -168,6 +212,17 @@ func decodeRemoteCatalog(raw []byte) (ports.RemoteCatalog, error) { | |||
| 168 | 212 | return catalog, nil | |
| 169 | 213 | } | |
| 170 | 214 | ||
| 215 | + | func decodeCatalogEnvelope(raw []byte, strict bool, dst any) error { | |
| 216 | + | dec := json.NewDecoder(bytes.NewReader(raw)) | |
| 217 | + | if strict { | |
| 218 | + | dec.DisallowUnknownFields() | |
| 219 | + | } | |
| 220 | + | if err := dec.Decode(dst); err != nil { | |
| 221 | + | return fmt.Errorf("%w: %w", errCatalogDecode, err) | |
| 222 | + | } | |
| 223 | + | return rejectTrailingJSON(dec) | |
| 224 | + | } | |
| 225 | + | ||
| 171 | 226 | func rejectTrailingJSON(dec *json.Decoder) error { | |
| 172 | 227 | var extra json.RawMessage | |
| 173 | 228 | err := dec.Decode(&extra) | |
| @@ -111,55 +111,99 @@ func (c *CatalogClient) List(ctx context.Context, target string) (ports.RemoteCa | |||||
| 111 | return catalog, nil | 111 | return catalog, nil | ||
| 112 | } | 112 | } | ||
| 113 | 113 | ||||
| 114 | + | type catalogVersionEnvelope struct { | |||
| 115 | + | ProtocolVersion *uint16 `json:"protocol_version"` | |||
| 116 | + | SchemaVersion *uint16 `json:"schema_version"` | |||
| 117 | + | Sessions json.RawMessage `json:"sessions"` | |||
| 118 | + | } | |||
| 119 | + | ||||
| 114 | type catalogEnvelope struct { | 120 | type catalogEnvelope struct { | ||
| 115 | - | ProtocolVersion *uint16 `json:"protocol_version"` | 121 | + | ProtocolVersion *uint16 `json:"protocol_version"` |
| 116 | - | SchemaVersion *uint16 `json:"schema_version"` | 122 | + | SchemaVersion *uint16 `json:"schema_version"` |
| 117 | - | Sessions *[]ports.RemoteCatalogSession `json:"sessions"` | 123 | + | Sessions *[]catalogSessionEnvelope `json:"sessions"` |
| 124 | + | } | |||
| 125 | + | ||||
| 126 | + | type catalogSessionEnvelope struct { | |||
| 127 | + | LifecycleID *domain.SessionLifecycleID `json:"lifecycle_id"` | |||
| 128 | + | Name *string `json:"name"` | |||
| 129 | + | State *ports.RemoteCatalogSessionState `json:"state"` | |||
| 130 | + | Ephemeral *bool `json:"ephemeral"` | |||
| 131 | + | Tabs *[]catalogTabEnvelope `json:"tabs"` | |||
| 132 | + | Attached *bool `json:"attached"` | |||
| 133 | + | LastUsedSeq uint64 `json:"last_used_seq,omitempty"` | |||
| 134 | + | ActiveTabID string `json:"active_tab_id,omitempty"` | |||
| 135 | + | Reason string `json:"reason,omitempty"` | |||
| 136 | + | } | |||
| 137 | + | ||||
| 138 | + | type catalogTabEnvelope struct { | |||
| 139 | + | ID *string `json:"id"` | |||
| 140 | + | Index *uint16 `json:"index"` | |||
| 141 | + | Name *string `json:"name"` | |||
| 142 | + | Detail string `json:"detail,omitempty"` | |||
| 143 | + | Attention bool `json:"attention,omitempty"` | |||
| 118 | } | 144 | } | ||
| 119 | 145 | ||||
| 120 | func decodeRemoteCatalog(raw []byte) (ports.RemoteCatalog, error) { | 146 | func decodeRemoteCatalog(raw []byte) (ports.RemoteCatalog, error) { | ||
| 121 | - | dec := json.NewDecoder(bytes.NewReader(raw)) | 147 | + | var version catalogVersionEnvelope |
| 122 | - | var envelope catalogEnvelope | 148 | + | if err := decodeCatalogEnvelope(raw, false, &version); err != nil { |
| 123 | - | if err := dec.Decode(&envelope); err != nil { | 149 | + | return ports.RemoteCatalog{}, err |
| 124 | - | return ports.RemoteCatalog{}, fmt.Errorf("%w: %w", errCatalogDecode, err) | |||
| 125 | } | 150 | } | ||
| 126 | - | if envelope.ProtocolVersion == nil || envelope.Sessions == nil { | 151 | + | if version.ProtocolVersion == nil { |
| 127 | return ports.RemoteCatalog{}, errCatalogRequired | 152 | return ports.RemoteCatalog{}, errCatalogRequired | ||
| 128 | } | 153 | } | ||
| 129 | - | if envelope.SchemaVersion == nil { | 154 | + | if version.SchemaVersion == nil { |
| 130 | return ports.RemoteCatalog{}, &ports.RemoteCatalogVersionMismatchError{ | 155 | return ports.RemoteCatalog{}, &ports.RemoteCatalogVersionMismatchError{ | ||
| 131 | Got: 0, | 156 | Got: 0, | ||
| 132 | Want: ports.RemoteCatalogSchemaVersion, | 157 | Want: ports.RemoteCatalogSchemaVersion, | ||
| 133 | Kind: "catalog", | 158 | Kind: "catalog", | ||
| 134 | } | 159 | } | ||
| 135 | } | 160 | } | ||
| 136 | - | if err := rejectTrailingJSON(dec); err != nil { | 161 | + | if *version.ProtocolVersion != ports.ProtocolVersion { |
| 137 | - | return ports.RemoteCatalog{}, err | |||
| 138 | - | } | |||
| 139 | - | ||||
| 140 | - | schemaVersion := *envelope.SchemaVersion | |||
| 141 | - | if *envelope.ProtocolVersion != ports.ProtocolVersion { | |||
| 142 | return ports.RemoteCatalog{}, &ports.RemoteCatalogVersionMismatchError{ | 162 | return ports.RemoteCatalog{}, &ports.RemoteCatalogVersionMismatchError{ | ||
| 143 | - | Got: *envelope.ProtocolVersion, | 163 | + | Got: *version.ProtocolVersion, |
| 144 | Want: ports.ProtocolVersion, | 164 | Want: ports.ProtocolVersion, | ||
| 145 | Kind: "protocol", | 165 | Kind: "protocol", | ||
| 146 | } | 166 | } | ||
| 147 | } | 167 | } | ||
| 148 | - | if schemaVersion != ports.RemoteCatalogSchemaVersion { | 168 | + | if *version.SchemaVersion != ports.RemoteCatalogSchemaVersion { |
| 149 | return ports.RemoteCatalog{}, &ports.RemoteCatalogVersionMismatchError{ | 169 | return ports.RemoteCatalog{}, &ports.RemoteCatalogVersionMismatchError{ | ||
| 150 | - | Got: schemaVersion, | 170 | + | Got: *version.SchemaVersion, |
| 151 | Want: ports.RemoteCatalogSchemaVersion, | 171 | Want: ports.RemoteCatalogSchemaVersion, | ||
| 152 | Kind: "catalog", | 172 | Kind: "catalog", | ||
| 153 | } | 173 | } | ||
| 154 | } | 174 | } | ||
| 155 | 175 | ||||
| 156 | - | sessions := *envelope.Sessions | 176 | + | var envelope catalogEnvelope |
| 157 | - | if sessions == nil { | 177 | + | if err := decodeCatalogEnvelope(raw, true, &envelope); err != nil { |
| 158 | - | sessions = []ports.RemoteCatalogSession{} | 178 | + | return ports.RemoteCatalog{}, err |
| 179 | + | } | |||
| 180 | + | if envelope.ProtocolVersion == nil || envelope.SchemaVersion == nil || envelope.Sessions == nil { | |||
| 181 | + | return ports.RemoteCatalog{}, errCatalogRequired | |||
| 182 | + | } | |||
| 183 | + | sessions := make([]ports.RemoteCatalogSession, 0, len(*envelope.Sessions)) | |||
| 184 | + | for _, session := range *envelope.Sessions { | |||
| 185 | + | if session.LifecycleID == nil || session.Name == nil || session.State == nil || session.Ephemeral == nil || session.Tabs == nil || session.Attached == nil { | |||
| 186 | + | return ports.RemoteCatalog{}, errCatalogRequired | |||
| 187 | + | } | |||
| 188 | + | tabs := make([]ports.RemoteCatalogTab, 0, len(*session.Tabs)) | |||
| 189 | + | for _, tab := range *session.Tabs { | |||
| 190 | + | if tab.ID == nil || tab.Index == nil || tab.Name == nil { | |||
| 191 | + | return ports.RemoteCatalog{}, errCatalogRequired | |||
| 192 | + | } | |||
| 193 | + | tabs = append(tabs, ports.RemoteCatalogTab{ | |||
| 194 | + | ID: *tab.ID, Index: *tab.Index, Name: *tab.Name, | |||
| 195 | + | Detail: tab.Detail, Attention: tab.Attention, | |||
| 196 | + | }) | |||
| 197 | + | } | |||
| 198 | + | sessions = append(sessions, ports.RemoteCatalogSession{ | |||
| 199 | + | LifecycleID: *session.LifecycleID, Name: *session.Name, State: *session.State, | |||
| 200 | + | Ephemeral: *session.Ephemeral, Tabs: tabs, Attached: *session.Attached, | |||
| 201 | + | LastUsedSeq: session.LastUsedSeq, ActiveTabID: session.ActiveTabID, Reason: session.Reason, | |||
| 202 | + | }) | |||
| 159 | } | 203 | } | ||
| 160 | catalog := ports.RemoteCatalog{ | 204 | catalog := ports.RemoteCatalog{ | ||
| 161 | ProtocolVersion: *envelope.ProtocolVersion, | 205 | ProtocolVersion: *envelope.ProtocolVersion, | ||
| 162 | - | SchemaVersion: schemaVersion, | 206 | + | SchemaVersion: *envelope.SchemaVersion, |
| 163 | Sessions: sessions, | 207 | Sessions: sessions, | ||
| 164 | } | 208 | } | ||
| 165 | if err := ports.ValidateRemoteCatalog(catalog); err != nil { | 209 | if err := ports.ValidateRemoteCatalog(catalog); err != nil { | ||
| @@ -168,6 +212,17 @@ func decodeRemoteCatalog(raw []byte) (ports.RemoteCatalog, error) { | |||||
| 168 | return catalog, nil | 212 | return catalog, nil | ||
| 169 | } | 213 | } | ||
| 170 | 214 | ||||
| 215 | + | func decodeCatalogEnvelope(raw []byte, strict bool, dst any) error { | |||
| 216 | + | dec := json.NewDecoder(bytes.NewReader(raw)) | |||
| 217 | + | if strict { | |||
| 218 | + | dec.DisallowUnknownFields() | |||
| 219 | + | } | |||
| 220 | + | if err := dec.Decode(dst); err != nil { | |||
| 221 | + | return fmt.Errorf("%w: %w", errCatalogDecode, err) | |||
| 222 | + | } | |||
| 223 | + | return rejectTrailingJSON(dec) | |||
| 224 | + | } | |||
| 225 | + | ||||
| 171 | func rejectTrailingJSON(dec *json.Decoder) error { | 226 | func rejectTrailingJSON(dec *json.Decoder) error { | ||
| 172 | var extra json.RawMessage | 227 | var extra json.RawMessage | ||
| 173 | err := dec.Decode(&extra) | 228 | err := dec.Decode(&extra) | ||
M internal/adapters/remote/catalog_cache_test.go +13 -2
| @@ -31,7 +31,10 @@ func TestCatalogCacheStoreLoad(t *testing.T) { | |||
| 31 | 31 | Host: "zebra", | |
| 32 | 32 | FetchedAt: fetchedAt, | |
| 33 | 33 | Sessions: []ports.RemoteCatalogSession{ | |
| 34 | - | {LifecycleID: [16]byte{1}, Name: "work", State: "up", Tabs: []ports.RemoteCatalogTab{{ID: "work-1"}, {ID: "work-2", Index: 1}}}, | |
| 34 | + | { | |
| 35 | + | LifecycleID: [16]byte{1}, Name: "work", State: "up", LastUsedSeq: 42, ActiveTabID: "work-2", | |
| 36 | + | Tabs: []ports.RemoteCatalogTab{{ID: "work-1", Detail: "dynamic", Attention: true}, {ID: "work-2", Index: 1}}, | |
| 37 | + | }, | |
| 35 | 38 | {LifecycleID: [16]byte{2}, Name: "alpha", State: "down", Ephemeral: true, Tabs: []ports.RemoteCatalogTab{{ID: "alpha-1"}}, Attached: true}, | |
| 36 | 39 | }, | |
| 37 | 40 | }, | |
| @@ -59,6 +62,12 @@ func TestCatalogCacheStoreLoad(t *testing.T) { | |||
| 59 | 62 | if alpha < 0 || work < 0 || alpha >= work { | |
| 60 | 63 | t.Fatalf("stored sessions are not ordered: %s", raw) | |
| 61 | 64 | } | |
| 65 | + | if !strings.Contains(string(raw), `"version":3`) || !strings.Contains(string(raw), `"tabs":[{"id":"work-1"`) { | |
| 66 | + | t.Fatalf("stored cache does not pin the v3 typed-tab contract: %s", raw) | |
| 67 | + | } | |
| 68 | + | if strings.Contains(string(raw), `"detail"`) || strings.Contains(string(raw), `"attention"`) { | |
| 69 | + | t.Fatalf("stored cache contains dynamic tab fields: %s", raw) | |
| 70 | + | } | |
| 62 | 71 | got, err := cache.Load() | |
| 63 | 72 | if err != nil { | |
| 64 | 73 | t.Fatalf("Load() error = %v", err) | |
| @@ -74,7 +83,7 @@ func TestCatalogCacheStoreLoad(t *testing.T) { | |||
| 74 | 83 | FetchedAt: fetchedAt, | |
| 75 | 84 | Sessions: []ports.RemoteCatalogSession{ | |
| 76 | 85 | {LifecycleID: [16]byte{2}, Name: "alpha", State: "down", Ephemeral: true, Tabs: []ports.RemoteCatalogTab{{ID: "alpha-1"}}, Attached: true}, | |
| 77 | - | {LifecycleID: [16]byte{1}, Name: "work", State: "up", Tabs: []ports.RemoteCatalogTab{{ID: "work-1"}, {ID: "work-2", Index: 1}}}, | |
| 86 | + | {LifecycleID: [16]byte{1}, Name: "work", State: "up", LastUsedSeq: 42, ActiveTabID: "work-2", Tabs: []ports.RemoteCatalogTab{{ID: "work-1"}, {ID: "work-2", Index: 1}}}, | |
| 78 | 87 | }, | |
| 79 | 88 | }, | |
| 80 | 89 | } | |
| @@ -125,6 +134,8 @@ func TestCatalogCacheLoadRejectsInvalidFilesWithoutReplacingThem(t *testing.T) { | |||
| 125 | 134 | {name: "negative fetched at", raw: []byte(`{"version":3,"hosts":[{"target":"arch","fetched_at_unix_nano":-1,"sessions":[]}]}`)}, | |
| 126 | 135 | {name: "duplicate hosts", raw: []byte(`{"version":3,"hosts":[{"target":"arch","fetched_at_unix_nano":1,"sessions":[]},{"target":"arch","fetched_at_unix_nano":2,"sessions":[]}]}`)}, | |
| 127 | 136 | {name: "missing exact session fields", raw: []byte(`{"version":3,"hosts":[{"target":"arch","fetched_at_unix_nano":1,"sessions":[{"name":"work","state":"up","ephemeral":false,"tabs":[],"attached":false}]}]}`)}, | |
| 137 | + | {name: "count-only tabs", raw: []byte(`{"version":3,"hosts":[{"target":"arch","fetched_at_unix_nano":1,"sessions":[{"lifecycle_id":"01000000000000000000000000000000","name":"work","state":"up","ephemeral":false,"tabs":1,"attached":false}]}]}`)}, | |
| 138 | + | {name: "unknown field", raw: []byte(`{"version":3,"hosts":[],"future":true}`)}, | |
| 128 | 139 | {name: "invalid utf8", raw: []byte("{\"version\":3,\"hosts\":[{\"target\":\"\xff\",\"fetched_at_unix_nano\":1,\"sessions\":[]}]}")}, | |
| 129 | 140 | } | |
| 130 | 141 | for _, tc := range cases { | |
| @@ -31,7 +31,10 @@ func TestCatalogCacheStoreLoad(t *testing.T) { | |||||
| 31 | Host: "zebra", | 31 | Host: "zebra", | ||
| 32 | FetchedAt: fetchedAt, | 32 | FetchedAt: fetchedAt, | ||
| 33 | Sessions: []ports.RemoteCatalogSession{ | 33 | Sessions: []ports.RemoteCatalogSession{ | ||
| 34 | - | {LifecycleID: [16]byte{1}, Name: "work", State: "up", Tabs: []ports.RemoteCatalogTab{{ID: "work-1"}, {ID: "work-2", Index: 1}}}, | 34 | + | { |
| 35 | + | LifecycleID: [16]byte{1}, Name: "work", State: "up", LastUsedSeq: 42, ActiveTabID: "work-2", | |||
| 36 | + | Tabs: []ports.RemoteCatalogTab{{ID: "work-1", Detail: "dynamic", Attention: true}, {ID: "work-2", Index: 1}}, | |||
| 37 | + | }, | |||
| 35 | {LifecycleID: [16]byte{2}, Name: "alpha", State: "down", Ephemeral: true, Tabs: []ports.RemoteCatalogTab{{ID: "alpha-1"}}, Attached: true}, | 38 | {LifecycleID: [16]byte{2}, Name: "alpha", State: "down", Ephemeral: true, Tabs: []ports.RemoteCatalogTab{{ID: "alpha-1"}}, Attached: true}, | ||
| 36 | }, | 39 | }, | ||
| 37 | }, | 40 | }, | ||
| @@ -59,6 +62,12 @@ func TestCatalogCacheStoreLoad(t *testing.T) { | |||||
| 59 | if alpha < 0 || work < 0 || alpha >= work { | 62 | if alpha < 0 || work < 0 || alpha >= work { | ||
| 60 | t.Fatalf("stored sessions are not ordered: %s", raw) | 63 | t.Fatalf("stored sessions are not ordered: %s", raw) | ||
| 61 | } | 64 | } | ||
| 65 | + | if !strings.Contains(string(raw), `"version":3`) || !strings.Contains(string(raw), `"tabs":[{"id":"work-1"`) { | |||
| 66 | + | t.Fatalf("stored cache does not pin the v3 typed-tab contract: %s", raw) | |||
| 67 | + | } | |||
| 68 | + | if strings.Contains(string(raw), `"detail"`) || strings.Contains(string(raw), `"attention"`) { | |||
| 69 | + | t.Fatalf("stored cache contains dynamic tab fields: %s", raw) | |||
| 70 | + | } | |||
| 62 | got, err := cache.Load() | 71 | got, err := cache.Load() | ||
| 63 | if err != nil { | 72 | if err != nil { | ||
| 64 | t.Fatalf("Load() error = %v", err) | 73 | t.Fatalf("Load() error = %v", err) | ||
| @@ -74,7 +83,7 @@ func TestCatalogCacheStoreLoad(t *testing.T) { | |||||
| 74 | FetchedAt: fetchedAt, | 83 | FetchedAt: fetchedAt, | ||
| 75 | Sessions: []ports.RemoteCatalogSession{ | 84 | Sessions: []ports.RemoteCatalogSession{ | ||
| 76 | {LifecycleID: [16]byte{2}, Name: "alpha", State: "down", Ephemeral: true, Tabs: []ports.RemoteCatalogTab{{ID: "alpha-1"}}, Attached: true}, | 85 | {LifecycleID: [16]byte{2}, Name: "alpha", State: "down", Ephemeral: true, Tabs: []ports.RemoteCatalogTab{{ID: "alpha-1"}}, Attached: true}, | ||
| 77 | - | {LifecycleID: [16]byte{1}, Name: "work", State: "up", Tabs: []ports.RemoteCatalogTab{{ID: "work-1"}, {ID: "work-2", Index: 1}}}, | 86 | + | {LifecycleID: [16]byte{1}, Name: "work", State: "up", LastUsedSeq: 42, ActiveTabID: "work-2", Tabs: []ports.RemoteCatalogTab{{ID: "work-1"}, {ID: "work-2", Index: 1}}}, |
| 78 | }, | 87 | }, | ||
| 79 | }, | 88 | }, | ||
| 80 | } | 89 | } | ||
| @@ -125,6 +134,8 @@ func TestCatalogCacheLoadRejectsInvalidFilesWithoutReplacingThem(t *testing.T) { | |||||
| 125 | {name: "negative fetched at", raw: []byte(`{"version":3,"hosts":[{"target":"arch","fetched_at_unix_nano":-1,"sessions":[]}]}`)}, | 134 | {name: "negative fetched at", raw: []byte(`{"version":3,"hosts":[{"target":"arch","fetched_at_unix_nano":-1,"sessions":[]}]}`)}, | ||
| 126 | {name: "duplicate hosts", raw: []byte(`{"version":3,"hosts":[{"target":"arch","fetched_at_unix_nano":1,"sessions":[]},{"target":"arch","fetched_at_unix_nano":2,"sessions":[]}]}`)}, | 135 | {name: "duplicate hosts", raw: []byte(`{"version":3,"hosts":[{"target":"arch","fetched_at_unix_nano":1,"sessions":[]},{"target":"arch","fetched_at_unix_nano":2,"sessions":[]}]}`)}, | ||
| 127 | {name: "missing exact session fields", raw: []byte(`{"version":3,"hosts":[{"target":"arch","fetched_at_unix_nano":1,"sessions":[{"name":"work","state":"up","ephemeral":false,"tabs":[],"attached":false}]}]}`)}, | 136 | {name: "missing exact session fields", raw: []byte(`{"version":3,"hosts":[{"target":"arch","fetched_at_unix_nano":1,"sessions":[{"name":"work","state":"up","ephemeral":false,"tabs":[],"attached":false}]}]}`)}, | ||
| 137 | + | {name: "count-only tabs", raw: []byte(`{"version":3,"hosts":[{"target":"arch","fetched_at_unix_nano":1,"sessions":[{"lifecycle_id":"01000000000000000000000000000000","name":"work","state":"up","ephemeral":false,"tabs":1,"attached":false}]}]}`)}, | |||
| 138 | + | {name: "unknown field", raw: []byte(`{"version":3,"hosts":[],"future":true}`)}, | |||
| 128 | {name: "invalid utf8", raw: []byte("{\"version\":3,\"hosts\":[{\"target\":\"\xff\",\"fetched_at_unix_nano\":1,\"sessions\":[]}]}")}, | 139 | {name: "invalid utf8", raw: []byte("{\"version\":3,\"hosts\":[{\"target\":\"\xff\",\"fetched_at_unix_nano\":1,\"sessions\":[]}]}")}, | ||
| 129 | } | 140 | } | ||
| 130 | for _, tc := range cases { | 141 | for _, tc := range cases { | ||
M internal/adapters/remote/catalog_test.go +39 -1
| @@ -119,11 +119,34 @@ func TestRemoteCatalogClientDecode(t *testing.T) { | |||
| 119 | 119 | if !errors.As(err, &mismatch) { | |
| 120 | 120 | t.Fatalf("error = %v, want RemoteCatalogVersionMismatchError", err) | |
| 121 | 121 | } | |
| 122 | - | if mismatch.Got != ports.ProtocolVersion+1 || mismatch.Want != ports.ProtocolVersion { | |
| 122 | + | if mismatch.Kind != "protocol" || mismatch.Got != ports.ProtocolVersion+1 || mismatch.Want != ports.ProtocolVersion { | |
| 123 | 123 | t.Fatalf("mismatch = %#v", mismatch) | |
| 124 | 124 | } | |
| 125 | 125 | }, | |
| 126 | 126 | }, | |
| 127 | + | { | |
| 128 | + | name: "obsolete count-only schema is classified before session decoding", | |
| 129 | + | stdout: fmt.Sprintf(`{"protocol_version":%d,"schema_version":2,"sessions":[{"tabs":1}]}`, | |
| 130 | + | ports.ProtocolVersion), | |
| 131 | + | check: func(t *testing.T, err error) { | |
| 132 | + | t.Helper() | |
| 133 | + | var mismatch *ports.RemoteCatalogVersionMismatchError | |
| 134 | + | if !errors.As(err, &mismatch) || mismatch.Kind != "catalog" || mismatch.Got != 2 || mismatch.Want != ports.RemoteCatalogSchemaVersion { | |
| 135 | + | t.Fatalf("error = %v, want obsolete catalogue version mismatch", err) | |
| 136 | + | } | |
| 137 | + | }, | |
| 138 | + | }, | |
| 139 | + | { | |
| 140 | + | name: "unknown catalogue schema", | |
| 141 | + | stdout: fmt.Sprintf(`{"protocol_version":%d,"schema_version":4,"sessions":[]}`, ports.ProtocolVersion), | |
| 142 | + | check: func(t *testing.T, err error) { | |
| 143 | + | t.Helper() | |
| 144 | + | var mismatch *ports.RemoteCatalogVersionMismatchError | |
| 145 | + | if !errors.As(err, &mismatch) || mismatch.Kind != "catalog" || mismatch.Got != 4 || mismatch.Want != ports.RemoteCatalogSchemaVersion { | |
| 146 | + | t.Fatalf("error = %v, want unknown catalogue version mismatch", err) | |
| 147 | + | } | |
| 148 | + | }, | |
| 149 | + | }, | |
| 127 | 150 | { | |
| 128 | 151 | name: "malformed json", | |
| 129 | 152 | stdout: `{"protocol_version":`, | |
| @@ -140,6 +163,18 @@ func TestRemoteCatalogClientDecode(t *testing.T) { | |||
| 140 | 163 | ports.ProtocolVersion, ports.RemoteCatalogSchemaVersion), | |
| 141 | 164 | wantErr: errCatalogDecode, | |
| 142 | 165 | }, | |
| 166 | + | { | |
| 167 | + | name: "unknown fields rejected", | |
| 168 | + | stdout: fmt.Sprintf(`{"protocol_version":%d,"schema_version":%d,"sessions":[],"future":true}`, | |
| 169 | + | ports.ProtocolVersion, ports.RemoteCatalogSchemaVersion), | |
| 170 | + | wantErr: errCatalogDecode, | |
| 171 | + | }, | |
| 172 | + | { | |
| 173 | + | name: "missing required session field", | |
| 174 | + | stdout: fmt.Sprintf(`{"protocol_version":%d,"schema_version":%d,"sessions":[{"lifecycle_id":"01000000000000000000000000000000","name":"work","state":"up","tabs":[],"attached":false}]}`, | |
| 175 | + | ports.ProtocolVersion, ports.RemoteCatalogSchemaVersion), | |
| 176 | + | wantErr: errCatalogRequired, | |
| 177 | + | }, | |
| 143 | 178 | { | |
| 144 | 179 | name: "trailing non-whitespace", | |
| 145 | 180 | stdout: strings.TrimSpace(validJSON) + `{"extra":true}`, | |
| @@ -222,6 +257,9 @@ func TestRemoteCatalogClientDecode(t *testing.T) { | |||
| 222 | 257 | if got.ProtocolVersion != tt.want.ProtocolVersion { | |
| 223 | 258 | t.Fatalf("ProtocolVersion = %d, want %d", got.ProtocolVersion, tt.want.ProtocolVersion) | |
| 224 | 259 | } | |
| 260 | + | if got.SchemaVersion != tt.want.SchemaVersion { | |
| 261 | + | t.Fatalf("SchemaVersion = %d, want %d", got.SchemaVersion, tt.want.SchemaVersion) | |
| 262 | + | } | |
| 225 | 263 | if len(got.Sessions) != len(tt.want.Sessions) { | |
| 226 | 264 | t.Fatalf("Sessions len = %d, want %d", len(got.Sessions), len(tt.want.Sessions)) | |
| 227 | 265 | } | |
| @@ -119,11 +119,34 @@ func TestRemoteCatalogClientDecode(t *testing.T) { | |||||
| 119 | if !errors.As(err, &mismatch) { | 119 | if !errors.As(err, &mismatch) { | ||
| 120 | t.Fatalf("error = %v, want RemoteCatalogVersionMismatchError", err) | 120 | t.Fatalf("error = %v, want RemoteCatalogVersionMismatchError", err) | ||
| 121 | } | 121 | } | ||
| 122 | - | if mismatch.Got != ports.ProtocolVersion+1 || mismatch.Want != ports.ProtocolVersion { | 122 | + | if mismatch.Kind != "protocol" || mismatch.Got != ports.ProtocolVersion+1 || mismatch.Want != ports.ProtocolVersion { |
| 123 | t.Fatalf("mismatch = %#v", mismatch) | 123 | t.Fatalf("mismatch = %#v", mismatch) | ||
| 124 | } | 124 | } | ||
| 125 | }, | 125 | }, | ||
| 126 | }, | 126 | }, | ||
| 127 | + | { | |||
| 128 | + | name: "obsolete count-only schema is classified before session decoding", | |||
| 129 | + | stdout: fmt.Sprintf(`{"protocol_version":%d,"schema_version":2,"sessions":[{"tabs":1}]}`, | |||
| 130 | + | ports.ProtocolVersion), | |||
| 131 | + | check: func(t *testing.T, err error) { | |||
| 132 | + | t.Helper() | |||
| 133 | + | var mismatch *ports.RemoteCatalogVersionMismatchError | |||
| 134 | + | if !errors.As(err, &mismatch) || mismatch.Kind != "catalog" || mismatch.Got != 2 || mismatch.Want != ports.RemoteCatalogSchemaVersion { | |||
| 135 | + | t.Fatalf("error = %v, want obsolete catalogue version mismatch", err) | |||
| 136 | + | } | |||
| 137 | + | }, | |||
| 138 | + | }, | |||
| 139 | + | { | |||
| 140 | + | name: "unknown catalogue schema", | |||
| 141 | + | stdout: fmt.Sprintf(`{"protocol_version":%d,"schema_version":4,"sessions":[]}`, ports.ProtocolVersion), | |||
| 142 | + | check: func(t *testing.T, err error) { | |||
| 143 | + | t.Helper() | |||
| 144 | + | var mismatch *ports.RemoteCatalogVersionMismatchError | |||
| 145 | + | if !errors.As(err, &mismatch) || mismatch.Kind != "catalog" || mismatch.Got != 4 || mismatch.Want != ports.RemoteCatalogSchemaVersion { | |||
| 146 | + | t.Fatalf("error = %v, want unknown catalogue version mismatch", err) | |||
| 147 | + | } | |||
| 148 | + | }, | |||
| 149 | + | }, | |||
| 127 | { | 150 | { | ||
| 128 | name: "malformed json", | 151 | name: "malformed json", | ||
| 129 | stdout: `{"protocol_version":`, | 152 | stdout: `{"protocol_version":`, | ||
| @@ -140,6 +163,18 @@ func TestRemoteCatalogClientDecode(t *testing.T) { | |||||
| 140 | ports.ProtocolVersion, ports.RemoteCatalogSchemaVersion), | 163 | ports.ProtocolVersion, ports.RemoteCatalogSchemaVersion), | ||
| 141 | wantErr: errCatalogDecode, | 164 | wantErr: errCatalogDecode, | ||
| 142 | }, | 165 | }, | ||
| 166 | + | { | |||
| 167 | + | name: "unknown fields rejected", | |||
| 168 | + | stdout: fmt.Sprintf(`{"protocol_version":%d,"schema_version":%d,"sessions":[],"future":true}`, | |||
| 169 | + | ports.ProtocolVersion, ports.RemoteCatalogSchemaVersion), | |||
| 170 | + | wantErr: errCatalogDecode, | |||
| 171 | + | }, | |||
| 172 | + | { | |||
| 173 | + | name: "missing required session field", | |||
| 174 | + | stdout: fmt.Sprintf(`{"protocol_version":%d,"schema_version":%d,"sessions":[{"lifecycle_id":"01000000000000000000000000000000","name":"work","state":"up","tabs":[],"attached":false}]}`, | |||
| 175 | + | ports.ProtocolVersion, ports.RemoteCatalogSchemaVersion), | |||
| 176 | + | wantErr: errCatalogRequired, | |||
| 177 | + | }, | |||
| 143 | { | 178 | { | ||
| 144 | name: "trailing non-whitespace", | 179 | name: "trailing non-whitespace", | ||
| 145 | stdout: strings.TrimSpace(validJSON) + `{"extra":true}`, | 180 | stdout: strings.TrimSpace(validJSON) + `{"extra":true}`, | ||
| @@ -222,6 +257,9 @@ func TestRemoteCatalogClientDecode(t *testing.T) { | |||||
| 222 | if got.ProtocolVersion != tt.want.ProtocolVersion { | 257 | if got.ProtocolVersion != tt.want.ProtocolVersion { | ||
| 223 | t.Fatalf("ProtocolVersion = %d, want %d", got.ProtocolVersion, tt.want.ProtocolVersion) | 258 | t.Fatalf("ProtocolVersion = %d, want %d", got.ProtocolVersion, tt.want.ProtocolVersion) | ||
| 224 | } | 259 | } | ||
| 260 | + | if got.SchemaVersion != tt.want.SchemaVersion { | |||
| 261 | + | t.Fatalf("SchemaVersion = %d, want %d", got.SchemaVersion, tt.want.SchemaVersion) | |||
| 262 | + | } | |||
| 225 | if len(got.Sessions) != len(tt.want.Sessions) { | 263 | if len(got.Sessions) != len(tt.want.Sessions) { | ||
| 226 | t.Fatalf("Sessions len = %d, want %d", len(got.Sessions), len(tt.want.Sessions)) | 264 | t.Fatalf("Sessions len = %d, want %d", len(got.Sessions), len(tt.want.Sessions)) | ||
| 227 | } | 265 | } | ||
M internal/ports/remote.go +8 -3
| @@ -201,7 +201,8 @@ func ValidateRemoteCatalog(c RemoteCatalog) error { | |||
| 201 | 201 | if len(c.Sessions) > RemoteCatalogMaxSessions { | |
| 202 | 202 | return fmt.Errorf("%w: too many sessions", ErrRemoteCatalogTooLarge) | |
| 203 | 203 | } | |
| 204 | - | seen := make(map[domain.SessionLifecycleID]string, len(c.Sessions)) | |
| 204 | + | seenLifecycles := make(map[domain.SessionLifecycleID]string, len(c.Sessions)) | |
| 205 | + | seenNames := make(map[string]struct{}, len(c.Sessions)) | |
| 205 | 206 | bytes := 0 | |
| 206 | 207 | for _, session := range c.Sessions { | |
| 207 | 208 | if !validRemoteCatalogText(session.Name) || !validRemoteCatalogText(string(session.State)) || !validRemoteCatalogText(session.Reason) { | |
| @@ -227,10 +228,14 @@ func ValidateRemoteCatalog(c RemoteCatalog) error { | |||
| 227 | 228 | if session.LifecycleID == (domain.SessionLifecycleID{}) { | |
| 228 | 229 | return fmt.Errorf("%w: zero lifecycle ID", ErrInvalidRemoteCatalog) | |
| 229 | 230 | } | |
| 230 | - | if prior, exists := seen[session.LifecycleID]; exists { | |
| 231 | + | if _, exists := seenNames[session.Name]; exists { | |
| 232 | + | return fmt.Errorf("%w: duplicate session name %q", ErrInvalidRemoteCatalog, session.Name) | |
| 233 | + | } | |
| 234 | + | seenNames[session.Name] = struct{}{} | |
| 235 | + | if prior, exists := seenLifecycles[session.LifecycleID]; exists { | |
| 231 | 236 | return fmt.Errorf("%w: lifecycle ID reused by %q and %q", ErrInvalidRemoteCatalog, prior, session.Name) | |
| 232 | 237 | } | |
| 233 | - | seen[session.LifecycleID] = session.Name | |
| 238 | + | seenLifecycles[session.LifecycleID] = session.Name | |
| 234 | 239 | if session.Tabs == nil { | |
| 235 | 240 | return fmt.Errorf("%w: missing tab list", ErrInvalidRemoteCatalog) | |
| 236 | 241 | } | |
| @@ -201,7 +201,8 @@ func ValidateRemoteCatalog(c RemoteCatalog) error { | |||||
| 201 | if len(c.Sessions) > RemoteCatalogMaxSessions { | 201 | if len(c.Sessions) > RemoteCatalogMaxSessions { | ||
| 202 | return fmt.Errorf("%w: too many sessions", ErrRemoteCatalogTooLarge) | 202 | return fmt.Errorf("%w: too many sessions", ErrRemoteCatalogTooLarge) | ||
| 203 | } | 203 | } | ||
| 204 | - | seen := make(map[domain.SessionLifecycleID]string, len(c.Sessions)) | 204 | + | seenLifecycles := make(map[domain.SessionLifecycleID]string, len(c.Sessions)) |
| 205 | + | seenNames := make(map[string]struct{}, len(c.Sessions)) | |||
| 205 | bytes := 0 | 206 | bytes := 0 | ||
| 206 | for _, session := range c.Sessions { | 207 | for _, session := range c.Sessions { | ||
| 207 | if !validRemoteCatalogText(session.Name) || !validRemoteCatalogText(string(session.State)) || !validRemoteCatalogText(session.Reason) { | 208 | if !validRemoteCatalogText(session.Name) || !validRemoteCatalogText(string(session.State)) || !validRemoteCatalogText(session.Reason) { | ||
| @@ -227,10 +228,14 @@ func ValidateRemoteCatalog(c RemoteCatalog) error { | |||||
| 227 | if session.LifecycleID == (domain.SessionLifecycleID{}) { | 228 | if session.LifecycleID == (domain.SessionLifecycleID{}) { | ||
| 228 | return fmt.Errorf("%w: zero lifecycle ID", ErrInvalidRemoteCatalog) | 229 | return fmt.Errorf("%w: zero lifecycle ID", ErrInvalidRemoteCatalog) | ||
| 229 | } | 230 | } | ||
| 230 | - | if prior, exists := seen[session.LifecycleID]; exists { | 231 | + | if _, exists := seenNames[session.Name]; exists { |
| 232 | + | return fmt.Errorf("%w: duplicate session name %q", ErrInvalidRemoteCatalog, session.Name) | |||
| 233 | + | } | |||
| 234 | + | seenNames[session.Name] = struct{}{} | |||
| 235 | + | if prior, exists := seenLifecycles[session.LifecycleID]; exists { | |||
| 231 | return fmt.Errorf("%w: lifecycle ID reused by %q and %q", ErrInvalidRemoteCatalog, prior, session.Name) | 236 | return fmt.Errorf("%w: lifecycle ID reused by %q and %q", ErrInvalidRemoteCatalog, prior, session.Name) | ||
| 232 | } | 237 | } | ||
| 233 | - | seen[session.LifecycleID] = session.Name | 238 | + | seenLifecycles[session.LifecycleID] = session.Name |
| 234 | if session.Tabs == nil { | 239 | if session.Tabs == nil { | ||
| 235 | return fmt.Errorf("%w: missing tab list", ErrInvalidRemoteCatalog) | 240 | return fmt.Errorf("%w: missing tab list", ErrInvalidRemoteCatalog) | ||
| 236 | } | 241 | } | ||
M internal/ports/remote_state_test.go +13 -0
| @@ -39,3 +39,16 @@ func TestRemoteCatalogStateContract(t *testing.T) { | |||
| 39 | 39 | require.Equal(t, "catalog", mismatch.Kind) | |
| 40 | 40 | require.Equal(t, RemoteCatalogSchemaVersion, mismatch.Want) | |
| 41 | 41 | } | |
| 42 | + | ||
| 43 | + | func TestValidateRemoteCatalogRejectsDuplicateSessionNames(t *testing.T) { | |
| 44 | + | err := ValidateRemoteCatalog(RemoteCatalog{ | |
| 45 | + | ProtocolVersion: ProtocolVersion, | |
| 46 | + | SchemaVersion: RemoteCatalogSchemaVersion, | |
| 47 | + | Sessions: []RemoteCatalogSession{ | |
| 48 | + | {LifecycleID: [16]byte{1}, Name: "work", State: RemoteCatalogSessionUp, Tabs: []RemoteCatalogTab{}}, | |
| 49 | + | {LifecycleID: [16]byte{2}, Name: "work", State: RemoteCatalogSessionDown, Tabs: []RemoteCatalogTab{}}, | |
| 50 | + | }, | |
| 51 | + | }) | |
| 52 | + | require.ErrorIs(t, err, ErrInvalidRemoteCatalog) | |
| 53 | + | require.ErrorContains(t, err, "duplicate session name") | |
| 54 | + | } | |
| @@ -39,3 +39,16 @@ func TestRemoteCatalogStateContract(t *testing.T) { | |||||
| 39 | require.Equal(t, "catalog", mismatch.Kind) | 39 | require.Equal(t, "catalog", mismatch.Kind) | ||
| 40 | require.Equal(t, RemoteCatalogSchemaVersion, mismatch.Want) | 40 | require.Equal(t, RemoteCatalogSchemaVersion, mismatch.Want) | ||
| 41 | } | 41 | } | ||
| 42 | + | ||||
| 43 | + | func TestValidateRemoteCatalogRejectsDuplicateSessionNames(t *testing.T) { | |||
| 44 | + | err := ValidateRemoteCatalog(RemoteCatalog{ | |||
| 45 | + | ProtocolVersion: ProtocolVersion, | |||
| 46 | + | SchemaVersion: RemoteCatalogSchemaVersion, | |||
| 47 | + | Sessions: []RemoteCatalogSession{ | |||
| 48 | + | {LifecycleID: [16]byte{1}, Name: "work", State: RemoteCatalogSessionUp, Tabs: []RemoteCatalogTab{}}, | |||
| 49 | + | {LifecycleID: [16]byte{2}, Name: "work", State: RemoteCatalogSessionDown, Tabs: []RemoteCatalogTab{}}, | |||
| 50 | + | }, | |||
| 51 | + | }) | |||
| 52 | + | require.ErrorIs(t, err, ErrInvalidRemoteCatalog) | |||
| 53 | + | require.ErrorContains(t, err, "duplicate session name") | |||
| 54 | + | } | |||
M internal/usecase/daemon/inactive_session_test.go +18 -5
| @@ -42,9 +42,22 @@ func TestInactiveSessionPredicates(t *testing.T) { | |||
| 42 | 42 | ||
| 43 | 43 | func TestInactiveSessionLifecycleMatch(t *testing.T) { | |
| 44 | 44 | entry := inactiveSession{name: "work", createdAt: 7, incarnation: domain.IncarnationID{1}} | |
| 45 | - | require.True(t, entry.sameLifecycle(entry)) | |
| 46 | - | ||
| 47 | - | replacement := entry | |
| 48 | - | replacement.incarnation[0]++ | |
| 49 | - | require.False(t, entry.sameLifecycle(replacement)) | |
| 45 | + | for _, test := range []struct { | |
| 46 | + | name string | |
| 47 | + | mutate func(*inactiveSession) | |
| 48 | + | want bool | |
| 49 | + | }{ | |
| 50 | + | {name: "same lifecycle", want: true}, | |
| 51 | + | {name: "different name", mutate: func(candidate *inactiveSession) { candidate.name = "other" }}, | |
| 52 | + | {name: "different creation time", mutate: func(candidate *inactiveSession) { candidate.createdAt++ }}, | |
| 53 | + | {name: "different incarnation", mutate: func(candidate *inactiveSession) { candidate.incarnation[0]++ }}, | |
| 54 | + | } { | |
| 55 | + | t.Run(test.name, func(t *testing.T) { | |
| 56 | + | candidate := entry | |
| 57 | + | if test.mutate != nil { | |
| 58 | + | test.mutate(&candidate) | |
| 59 | + | } | |
| 60 | + | require.Equal(t, test.want, entry.sameLifecycle(candidate)) | |
| 61 | + | }) | |
| 62 | + | } | |
| 50 | 63 | } | |
| @@ -42,9 +42,22 @@ func TestInactiveSessionPredicates(t *testing.T) { | |||||
| 42 | 42 | ||||
| 43 | func TestInactiveSessionLifecycleMatch(t *testing.T) { | 43 | func TestInactiveSessionLifecycleMatch(t *testing.T) { | ||
| 44 | entry := inactiveSession{name: "work", createdAt: 7, incarnation: domain.IncarnationID{1}} | 44 | entry := inactiveSession{name: "work", createdAt: 7, incarnation: domain.IncarnationID{1}} | ||
| 45 | - | require.True(t, entry.sameLifecycle(entry)) | 45 | + | for _, test := range []struct { |
| 46 | - | 46 | + | name string | |
| 47 | - | replacement := entry | 47 | + | mutate func(*inactiveSession) |
| 48 | - | replacement.incarnation[0]++ | 48 | + | want bool |
| 49 | - | require.False(t, entry.sameLifecycle(replacement)) | 49 | + | }{ |
| 50 | + | {name: "same lifecycle", want: true}, | |||
| 51 | + | {name: "different name", mutate: func(candidate *inactiveSession) { candidate.name = "other" }}, | |||
| 52 | + | {name: "different creation time", mutate: func(candidate *inactiveSession) { candidate.createdAt++ }}, | |||
| 53 | + | {name: "different incarnation", mutate: func(candidate *inactiveSession) { candidate.incarnation[0]++ }}, | |||
| 54 | + | } { | |||
| 55 | + | t.Run(test.name, func(t *testing.T) { | |||
| 56 | + | candidate := entry | |||
| 57 | + | if test.mutate != nil { | |||
| 58 | + | test.mutate(&candidate) | |||
| 59 | + | } | |||
| 60 | + | require.Equal(t, test.want, entry.sameLifecycle(candidate)) | |||
| 61 | + | }) | |||
| 62 | + | } | |||
| 50 | } | 63 | } | ||
M internal/usecase/daemon/remote_attach.go +5 -1
| @@ -27,6 +27,10 @@ func (d *Daemon) routeRemoteTargetWithContext(ctx context.Context, h ports.Hello | |||
| 27 | 27 | return nil, nil, &protoErr{ports.ErrNoSuchTarget, "remote target requires attach or resume"} | |
| 28 | 28 | } | |
| 29 | 29 | if err := d.waitForTargetRestore(ctx, target.SessionName); err != nil { | |
| 30 | + | var protocol *protoErr | |
| 31 | + | if errors.As(err, &protocol) && protocol.code == ports.ErrInternal { | |
| 32 | + | return nil, nil, &protoErr{ports.ErrNoSuchTarget, "remote session is unavailable"} | |
| 33 | + | } | |
| 30 | 34 | return nil, nil, remoteTargetError(err) | |
| 31 | 35 | } | |
| 32 | 36 | ||
| @@ -78,7 +82,7 @@ func (d *Daemon) routeRemoteTargetWithContext(ctx context.Context, h ports.Hello | |||
| 78 | 82 | sess, err := d.resumeRemoteInactiveSessionLocked(target, cwd, h.Size, env, inactive) | |
| 79 | 83 | if err != nil { | |
| 80 | 84 | d.mu.Unlock() | |
| 81 | - | if errors.Is(err, errAttachmentTransition) { | |
| 85 | + | if errors.Is(err, errAttachmentTransition) || errors.Is(err, errSessionNameInUse) { | |
| 82 | 86 | return nil, nil, remoteTargetError(err) | |
| 83 | 87 | } | |
| 84 | 88 | return nil, nil, err | |
| @@ -27,6 +27,10 @@ func (d *Daemon) routeRemoteTargetWithContext(ctx context.Context, h ports.Hello | |||||
| 27 | return nil, nil, &protoErr{ports.ErrNoSuchTarget, "remote target requires attach or resume"} | 27 | return nil, nil, &protoErr{ports.ErrNoSuchTarget, "remote target requires attach or resume"} | ||
| 28 | } | 28 | } | ||
| 29 | if err := d.waitForTargetRestore(ctx, target.SessionName); err != nil { | 29 | if err := d.waitForTargetRestore(ctx, target.SessionName); err != nil { | ||
| 30 | + | var protocol *protoErr | |||
| 31 | + | if errors.As(err, &protocol) && protocol.code == ports.ErrInternal { | |||
| 32 | + | return nil, nil, &protoErr{ports.ErrNoSuchTarget, "remote session is unavailable"} | |||
| 33 | + | } | |||
| 30 | return nil, nil, remoteTargetError(err) | 34 | return nil, nil, remoteTargetError(err) | ||
| 31 | } | 35 | } | ||
| 32 | 36 | ||||
| @@ -78,7 +82,7 @@ func (d *Daemon) routeRemoteTargetWithContext(ctx context.Context, h ports.Hello | |||||
| 78 | sess, err := d.resumeRemoteInactiveSessionLocked(target, cwd, h.Size, env, inactive) | 82 | sess, err := d.resumeRemoteInactiveSessionLocked(target, cwd, h.Size, env, inactive) | ||
| 79 | if err != nil { | 83 | if err != nil { | ||
| 80 | d.mu.Unlock() | 84 | d.mu.Unlock() | ||
| 81 | - | if errors.Is(err, errAttachmentTransition) { | 85 | + | if errors.Is(err, errAttachmentTransition) || errors.Is(err, errSessionNameInUse) { |
| 82 | return nil, nil, remoteTargetError(err) | 86 | return nil, nil, remoteTargetError(err) | ||
| 83 | } | 87 | } | ||
| 84 | return nil, nil, err | 88 | return nil, nil, err | ||
M internal/usecase/daemon/remote_stopped_route_test.go +51 -0
| @@ -92,6 +92,57 @@ func TestRouteRemoteTargetRejectsLiveIntentForInactiveSession(t *testing.T) { | |||
| 92 | 92 | require.Contains(t, d.inactive, "work") | |
| 93 | 93 | } | |
| 94 | 94 | ||
| 95 | + | func TestRouteRemoteTargetMapsUnavailableInactiveStatesToNoSuchTarget(t *testing.T) { | |
| 96 | + | lifecycle := remoteLifecycleForTest() | |
| 97 | + | for _, test := range []struct { | |
| 98 | + | name string | |
| 99 | + | entry inactiveSession | |
| 100 | + | }{ | |
| 101 | + | {name: "broken", entry: inactiveSession{name: "work", incarnation: lifecycle, state: ports.SessionBroken}}, | |
| 102 | + | {name: "degraded", entry: inactiveSession{name: "work", incarnation: lifecycle, state: ports.SessionDown, record: domain.CatalogueRecord{Name: "work", DegradedReason: "checkpoint unavailable"}}}, | |
| 103 | + | {name: "purging", entry: inactiveSession{name: "work", incarnation: lifecycle, state: ports.SessionDown, purging: true}}, | |
| 104 | + | } { | |
| 105 | + | t.Run(test.name, func(t *testing.T) { | |
| 106 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 107 | + | d.inactive["work"] = test.entry | |
| 108 | + | target := domain.RemoteSessionTarget{ | |
| 109 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, | |
| 110 | + | SessionName: "work", Stopped: true, | |
| 111 | + | } | |
| 112 | + | _, _, err := d.routeWithContext(context.Background(), ports.Hello{ | |
| 113 | + | Version: ports.ProtocolVersion, Intent: ports.IntentAttach, Name: "work", | |
| 114 | + | Size: domain.Size{Cols: 80, Rows: 24}, RemoteTarget: &target, | |
| 115 | + | EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | |
| 116 | + | }, &closeTrackingTransport{}) | |
| 117 | + | ||
| 118 | + | var protocol *protoErr | |
| 119 | + | require.ErrorAs(t, err, &protocol) | |
| 120 | + | require.Equal(t, uint16(ports.ErrNoSuchTarget), protocol.code) | |
| 121 | + | }) | |
| 122 | + | } | |
| 123 | + | } | |
| 124 | + | ||
| 125 | + | func TestRouteRemoteTargetMapsConcurrentInactiveResumeToNoSuchTarget(t *testing.T) { | |
| 126 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 127 | + | lifecycle := remoteLifecycleForTest() | |
| 128 | + | d.inactive["work"] = inactiveSession{name: "work", incarnation: lifecycle, state: ports.SessionDown} | |
| 129 | + | d.creating["work"] = struct{}{} | |
| 130 | + | target := domain.RemoteSessionTarget{ | |
| 131 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, | |
| 132 | + | SessionName: "work", Stopped: true, | |
| 133 | + | } | |
| 134 | + | ||
| 135 | + | _, _, err := d.routeWithContext(context.Background(), ports.Hello{ | |
| 136 | + | Version: ports.ProtocolVersion, Intent: ports.IntentAttach, Name: "work", | |
| 137 | + | Size: domain.Size{Cols: 80, Rows: 24}, RemoteTarget: &target, | |
| 138 | + | EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | |
| 139 | + | }, &closeTrackingTransport{}) | |
| 140 | + | ||
| 141 | + | var protocol *protoErr | |
| 142 | + | require.ErrorAs(t, err, &protocol) | |
| 143 | + | require.Equal(t, uint16(ports.ErrNoSuchTarget), protocol.code) | |
| 144 | + | } | |
| 145 | + | ||
| 95 | 146 | func TestRouteRemoteTargetPreservesInactiveResumeFailures(t *testing.T) { | |
| 96 | 147 | newTarget := func(record domain.CatalogueRecord) domain.RemoteSessionTarget { | |
| 97 | 148 | return domain.RemoteSessionTarget{ | |
| @@ -92,6 +92,57 @@ func TestRouteRemoteTargetRejectsLiveIntentForInactiveSession(t *testing.T) { | |||||
| 92 | require.Contains(t, d.inactive, "work") | 92 | require.Contains(t, d.inactive, "work") | ||
| 93 | } | 93 | } | ||
| 94 | 94 | ||||
| 95 | + | func TestRouteRemoteTargetMapsUnavailableInactiveStatesToNoSuchTarget(t *testing.T) { | |||
| 96 | + | lifecycle := remoteLifecycleForTest() | |||
| 97 | + | for _, test := range []struct { | |||
| 98 | + | name string | |||
| 99 | + | entry inactiveSession | |||
| 100 | + | }{ | |||
| 101 | + | {name: "broken", entry: inactiveSession{name: "work", incarnation: lifecycle, state: ports.SessionBroken}}, | |||
| 102 | + | {name: "degraded", entry: inactiveSession{name: "work", incarnation: lifecycle, state: ports.SessionDown, record: domain.CatalogueRecord{Name: "work", DegradedReason: "checkpoint unavailable"}}}, | |||
| 103 | + | {name: "purging", entry: inactiveSession{name: "work", incarnation: lifecycle, state: ports.SessionDown, purging: true}}, | |||
| 104 | + | } { | |||
| 105 | + | t.Run(test.name, func(t *testing.T) { | |||
| 106 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 107 | + | d.inactive["work"] = test.entry | |||
| 108 | + | target := domain.RemoteSessionTarget{ | |||
| 109 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, | |||
| 110 | + | SessionName: "work", Stopped: true, | |||
| 111 | + | } | |||
| 112 | + | _, _, err := d.routeWithContext(context.Background(), ports.Hello{ | |||
| 113 | + | Version: ports.ProtocolVersion, Intent: ports.IntentAttach, Name: "work", | |||
| 114 | + | Size: domain.Size{Cols: 80, Rows: 24}, RemoteTarget: &target, | |||
| 115 | + | EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | |||
| 116 | + | }, &closeTrackingTransport{}) | |||
| 117 | + | ||||
| 118 | + | var protocol *protoErr | |||
| 119 | + | require.ErrorAs(t, err, &protocol) | |||
| 120 | + | require.Equal(t, uint16(ports.ErrNoSuchTarget), protocol.code) | |||
| 121 | + | }) | |||
| 122 | + | } | |||
| 123 | + | } | |||
| 124 | + | ||||
| 125 | + | func TestRouteRemoteTargetMapsConcurrentInactiveResumeToNoSuchTarget(t *testing.T) { | |||
| 126 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 127 | + | lifecycle := remoteLifecycleForTest() | |||
| 128 | + | d.inactive["work"] = inactiveSession{name: "work", incarnation: lifecycle, state: ports.SessionDown} | |||
| 129 | + | d.creating["work"] = struct{}{} | |||
| 130 | + | target := domain.RemoteSessionTarget{ | |||
| 131 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, | |||
| 132 | + | SessionName: "work", Stopped: true, | |||
| 133 | + | } | |||
| 134 | + | ||||
| 135 | + | _, _, err := d.routeWithContext(context.Background(), ports.Hello{ | |||
| 136 | + | Version: ports.ProtocolVersion, Intent: ports.IntentAttach, Name: "work", | |||
| 137 | + | Size: domain.Size{Cols: 80, Rows: 24}, RemoteTarget: &target, | |||
| 138 | + | EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | |||
| 139 | + | }, &closeTrackingTransport{}) | |||
| 140 | + | ||||
| 141 | + | var protocol *protoErr | |||
| 142 | + | require.ErrorAs(t, err, &protocol) | |||
| 143 | + | require.Equal(t, uint16(ports.ErrNoSuchTarget), protocol.code) | |||
| 144 | + | } | |||
| 145 | + | ||||
| 95 | func TestRouteRemoteTargetPreservesInactiveResumeFailures(t *testing.T) { | 146 | func TestRouteRemoteTargetPreservesInactiveResumeFailures(t *testing.T) { | ||
| 96 | newTarget := func(record domain.CatalogueRecord) domain.RemoteSessionTarget { | 147 | newTarget := func(record domain.CatalogueRecord) domain.RemoteSessionTarget { | ||
| 97 | return domain.RemoteSessionTarget{ | 148 | return domain.RemoteSessionTarget{ | ||