diff --git a/openspec/changes/move-scum-feature-ownership-to-plugin/tasks.md b/openspec/changes/move-scum-feature-ownership-to-plugin/tasks.md index 10e7d4d..3809c38 100644 --- a/openspec/changes/move-scum-feature-ownership-to-plugin/tasks.md +++ b/openspec/changes/move-scum-feature-ownership-to-plugin/tasks.md @@ -15,8 +15,8 @@ ## 3. Implement the long-running SCUM Companion runtime - [x] 3.1 Implement authenticated registration, bounded dispatch polling, acknowledgement, idempotent result completion, backoff, and typed diagnostics in the SCUM Companion. -- [ ] 3.2 Add a handler registry that validates declared schema, bound server, approval, server version, capability discovery, expiry, and idempotency before invoking an adapter. -- [ ] 3.3 Implement safe configuration read/patch and diagnostics adapters that use only platform-authorized channels and redact host paths, credentials, and raw command text. +- [x] 3.2 Add a handler registry that validates declared schema, bound server, approval, server version, capability discovery, expiry, and idempotency before invoking an adapter. +- [x] 3.3 Implement safe configuration read/patch and diagnostics adapters that use only platform-authorized channels and redact host paths, credentials, and raw command text. - [ ] 3.4 Add Companion integration tests for command claiming, duplicate delivery, cancellation/expiry, malformed payloads, unsupported versions, and redaction. ## 4. Add verified SCUM data collectors diff --git a/plugins/examples/scum-server-plugin/companion/adapters.go b/plugins/examples/scum-server-plugin/companion/adapters.go new file mode 100644 index 0000000..fc76df5 --- /dev/null +++ b/plugins/examples/scum-server-plugin/companion/adapters.go @@ -0,0 +1,103 @@ +package companion + +import ( + "context" + "fmt" + "sort" + "strings" +) + +// AuthorizedConfigPort is supplied by a version-bound Companion integration. +// It exposes logical configuration values only: never a host path, connection +// string, credential, arbitrary command, or direct database handle. +type AuthorizedConfigPort interface { + ReadConfig(context.Context) (map[string]string, error) + ApplyConfigPatch(ctx context.Context, revision string, fields []ConfigFieldPatch) (map[string]string, error) +} +type ConfigFieldPatch struct { + Key string + Value string +} + +type VersionedAdapter struct { + ServerVersion string + Config AuthorizedConfigPort + DiagnosticsState map[string]string +} + +func (adapter VersionedAdapter) ReadConfiguration(ctx context.Context) (map[string]any, error) { + if !supportedAdapterVersion(adapter.ServerVersion) || adapter.Config == nil { + return nil, fmt.Errorf("configuration adapter is unsupported") + } + fields, err := adapter.Config.ReadConfig(ctx) + if err != nil { + return nil, err + } + return map[string]any{"version": adapter.ServerVersion, "fields": redactConfigValues(fields)}, nil +} +func (adapter VersionedAdapter) PatchConfiguration(ctx context.Context, payload map[string]any) (map[string]any, error) { + if !supportedAdapterVersion(adapter.ServerVersion) || adapter.Config == nil { + return nil, fmt.Errorf("configuration adapter is unsupported") + } + revision, _ := payload["revision"].(string) + raw, _ := payload["fields"].([]any) + fields := make([]ConfigFieldPatch, 0, len(raw)) + for _, value := range raw { + item, ok := value.(map[string]any) + if !ok { + return nil, fmt.Errorf("configuration patch payload is invalid") + } + key, keyOK := item["key"].(string) + fieldValue, valueOK := item["value"].(string) + if !keyOK || !valueOK || !supportedConfigKey(key) { + return nil, fmt.Errorf("configuration patch field is unsupported") + } + fields = append(fields, ConfigFieldPatch{Key: key, Value: fieldValue}) + } + applied, err := adapter.Config.ApplyConfigPatch(ctx, revision, fields) + if err != nil { + return nil, err + } + return map[string]any{"version": adapter.ServerVersion, "appliedFields": redactConfigValues(applied)}, nil +} +func (adapter VersionedAdapter) Diagnostics(context.Context) (map[string]any, error) { + state := map[string]any{"version": adapter.ServerVersion, "adapter": "version-bound", "configuration": supportedAdapterVersion(adapter.ServerVersion)} + for key, value := range adapter.DiagnosticsState { + if safeDiagnosticField(key, value) { + state[key] = value + } + } + return state, nil +} +func (VersionedAdapter) PatchGameState(context.Context, map[string]any) (map[string]any, error) { + return nil, fmt.Errorf("state adapter is unsupported") +} +func (VersionedAdapter) DeliverReward(context.Context, map[string]any) (map[string]any, error) { + return nil, fmt.Errorf("reward adapter is unsupported") +} +func (VersionedAdapter) NotifyPlayer(context.Context, map[string]any) (map[string]any, error) { + return nil, fmt.Errorf("notification adapter is unsupported") +} + +func supportedAdapterVersion(version string) bool { return version == "0.9.700.90357" } +func supportedConfigKey(key string) bool { + return map[string]bool{"ServerName": true, "GamePort": true, "QueryPort": true, "MaxPlayers": true, "WelcomeMessage": true}[key] +} +func redactConfigValues(values map[string]string) map[string]string { + result := map[string]string{} + keys := make([]string, 0, len(values)) + for key := range values { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + if supportedConfigKey(key) && safeDiagnosticField(key, values[key]) { + result[key] = values[key] + } + } + return result +} +func safeDiagnosticField(key, value string) bool { + lowered := strings.ToLower(key + "=" + value) + return !strings.Contains(lowered, "path") && !strings.Contains(lowered, "credential") && !strings.Contains(lowered, "password") && !strings.Contains(lowered, "bearer ") && !strings.Contains(lowered, "rcon") && !strings.Contains(lowered, "sql") && !strings.Contains(lowered, "://") +} diff --git a/plugins/examples/scum-server-plugin/companion/adapters_test.go b/plugins/examples/scum-server-plugin/companion/adapters_test.go new file mode 100644 index 0000000..1731d3f --- /dev/null +++ b/plugins/examples/scum-server-plugin/companion/adapters_test.go @@ -0,0 +1,43 @@ +package companion + +import ( + "context" + "testing" +) + +type configPortFixture struct { + fields map[string]string + patches []ConfigFieldPatch +} + +func (fixture *configPortFixture) ReadConfig(context.Context) (map[string]string, error) { + return fixture.fields, nil +} +func (fixture *configPortFixture) ApplyConfigPatch(_ context.Context, _ string, fields []ConfigFieldPatch) (map[string]string, error) { + fixture.patches = fields + return map[string]string{"ServerName": "Moon", "hostPath": "C:/secret"}, nil +} + +func TestVersionedAdapterUsesOnlyLogicalConfigValuesAndRedactsDiagnostics(t *testing.T) { + port := &configPortFixture{fields: map[string]string{"ServerName": "Moon", "hostPath": "C:/secret", "Password": "nope"}} + adapter := VersionedAdapter{ServerVersion: "0.9.700.90357", Config: port, DiagnosticsState: map[string]string{"status": "healthy", "hostPath": "C:/secret"}} + read, err := adapter.ReadConfiguration(context.Background()) + if err != nil { + t.Fatalf("read config: %v", err) + } + fields := read["fields"].(map[string]string) + if fields["ServerName"] != "Moon" || len(fields) != 1 { + t.Fatalf("unsafe config was exposed: %+v", fields) + } + patched, err := adapter.PatchConfiguration(context.Background(), map[string]any{"revision": "r1", "fields": []any{map[string]any{"key": "ServerName", "value": "Moon"}}}) + if err != nil { + t.Fatalf("patch config: %v", err) + } + if len(port.patches) != 1 || patched["appliedFields"].(map[string]string)["hostPath"] != "" { + t.Fatalf("patch leaked unsafe details: %+v", patched) + } + diagnostics, _ := adapter.Diagnostics(context.Background()) + if diagnostics["hostPath"] != nil || diagnostics["status"] != "healthy" { + t.Fatalf("diagnostics leaked unsafe details: %+v", diagnostics) + } +} diff --git a/plugins/examples/scum-server-plugin/companion/dispatcher.go b/plugins/examples/scum-server-plugin/companion/dispatcher.go index 1f27899..b47318f 100644 --- a/plugins/examples/scum-server-plugin/companion/dispatcher.go +++ b/plugins/examples/scum-server-plugin/companion/dispatcher.go @@ -20,6 +20,7 @@ type SafeAdapter interface { } type HandlerAvailability struct { + BoundServerID string ServerVersion string Capabilities map[string]bool Approved bool @@ -66,7 +67,7 @@ func (registry *HandlerRegistry) Execute(ctx context.Context, command ClaimedCom if err := validateDeclaredCommandAt(command, time.Now); err != nil { return unsupportedResult("validation-failed"), nil } - if !registry.availability.Approved || !registry.availability.Capabilities[command.CommandType] { + if strings.TrimSpace(registry.availability.BoundServerID) == "" || !registry.availability.Approved || !registry.availability.Capabilities[command.CommandType] { return unsupportedResult("unsupported"), nil } handler, exists := registry.handlers[command.CommandType] @@ -93,7 +94,104 @@ func validateDeclaredCommandAt(command ClaimedCommand, now func() time.Time) err return fmt.Errorf("unsafe payload") } } - return nil + return validateCommandPayload(command.CommandType, command.Payload) +} + +func validateCommandPayload(commandType string, payload map[string]any) error { + require := func(keys ...string) error { + for _, key := range keys { + if _, ok := payload[key]; !ok { + return fmt.Errorf("payload is incomplete") + } + } + return nil + } + noUnknown := func(keys ...string) error { + allowed := map[string]bool{} + for _, key := range keys { + allowed[key] = true + } + for key := range payload { + if !allowed[key] { + return fmt.Errorf("payload has unsupported field") + } + } + return nil + } + switch commandType { + case "config.read": + return noUnknown() + case "config.patch": + if err := require("revision", "fields"); err != nil { + return err + } + if err := noUnknown("revision", "fields"); err != nil { + return err + } + _, revisionOK := payload["revision"].(string) + fields, fieldsOK := payload["fields"].([]any) + if !revisionOK || !fieldsOK || len(fields) == 0 || len(fields) > 32 { + return fmt.Errorf("config patch payload is invalid") + } + return nil + case "companion.diagnostics": + if err := noUnknown("includeWindowState", "maxEntries"); err != nil { + return err + } + if value, ok := payload["includeWindowState"]; ok { + if _, valid := value.(bool); !valid { + return fmt.Errorf("diagnostics payload is invalid") + } + } + if value, ok := payload["maxEntries"]; ok { + if !boundedDiagnosticsEntries(value) { + return fmt.Errorf("diagnostics payload is invalid") + } + } + return nil + case "game-state.patch": + if err := require("playerId", "gameVersion", "expectedStateVersion", "safetyWindow", "reason", "changes"); err != nil { + return err + } + if err := noUnknown("playerId", "gameVersion", "expectedStateVersion", "safetyWindow", "reason", "changes"); err != nil { + return err + } + version, ok := payload["gameVersion"].(string) + changes, changesOK := payload["changes"].([]any) + if !ok || version == "" || !changesOK || len(changes) == 0 || len(changes) > 8 { + return fmt.Errorf("state patch payload is invalid") + } + return nil + case "reward.deliver": + if err := require("grantId", "playerId", "items"); err != nil { + return err + } + if err := noUnknown("grantId", "playerId", "items"); err != nil { + return err + } + _, grantOK := payload["grantId"].(string) + _, playerOK := payload["playerId"].(string) + items, itemsOK := payload["items"].([]any) + if !grantOK || !playerOK || !itemsOK || len(items) == 0 || len(items) > 8 { + return fmt.Errorf("reward payload is invalid") + } + return nil + case "player.notify": + if err := require("playerId", "message"); err != nil { + return err + } + if err := noUnknown("playerId", "message"); err != nil { + return err + } + _, playerOK := payload["playerId"].(string) + message, messageOK := payload["message"].(string) + if !playerOK || !messageOK || strings.TrimSpace(message) == "" || len(message) > 200 { + return fmt.Errorf("notification payload is invalid") + } + return nil + default: + return fmt.Errorf("command type is not declared") + } } func safeCommandField(key string, value any) bool { diff --git a/plugins/examples/scum-server-plugin/companion/dispatcher_test.go b/plugins/examples/scum-server-plugin/companion/dispatcher_test.go index 4d7cc29..b9abb1f 100644 --- a/plugins/examples/scum-server-plugin/companion/dispatcher_test.go +++ b/plugins/examples/scum-server-plugin/companion/dispatcher_test.go @@ -49,7 +49,7 @@ func (*adapterFixture) NotifyPlayer(context.Context, map[string]any) (map[string func TestDispatcherAcknowledgesOnlyLiveValidatedTypedCommands(t *testing.T) { stamp := time.Now().UTC() adapter := &adapterFixture{} - registry := NewHandlerRegistry(HandlerAvailability{ServerVersion: "0.9.700.90357", Approved: true, Capabilities: map[string]bool{"config.read": true}}, adapter) + registry := NewHandlerRegistry(HandlerAvailability{BoundServerID: "server-1", ServerVersion: "0.9.700.90357", Approved: true, Capabilities: map[string]bool{"config.read": true}}, adapter) fixture := &dispatchFixture{commands: []ClaimedCommand{{ID: "read-1", ProfileKey: ProfileKey, CommandType: "config.read", Payload: map[string]any{}, FencingToken: 7, LeaseExpiresAt: stamp.Add(time.Minute), ExpiresAt: stamp.Add(time.Minute)}, {ID: "expired-1", ProfileKey: ProfileKey, CommandType: "config.read", Payload: map[string]any{}, FencingToken: 8, LeaseExpiresAt: stamp.Add(-time.Second), ExpiresAt: stamp.Add(-time.Second)}}} dispatcher := Dispatcher{Client: fixture, Registry: registry, Now: func() time.Time { return stamp }} if err := dispatcher.DispatchOnce(context.Background()); err != nil { @@ -66,7 +66,7 @@ func TestDispatcherAcknowledgesOnlyLiveValidatedTypedCommands(t *testing.T) { func TestRegistryReturnsCachedResultForDuplicateDelivery(t *testing.T) { stamp := time.Now().UTC() adapter := &adapterFixture{} - registry := NewHandlerRegistry(HandlerAvailability{ServerVersion: "0.9.700.90357", Approved: true, Capabilities: map[string]bool{"config.read": true}}, adapter) + registry := NewHandlerRegistry(HandlerAvailability{BoundServerID: "server-1", ServerVersion: "0.9.700.90357", Approved: true, Capabilities: map[string]bool{"config.read": true}}, adapter) command := ClaimedCommand{ID: "duplicate-1", ProfileKey: ProfileKey, CommandType: "config.read", Payload: map[string]any{}, FencingToken: 7, LeaseExpiresAt: stamp.Add(time.Minute), ExpiresAt: stamp.Add(time.Minute)} if _, err := registry.Execute(context.Background(), command); err != nil { t.Fatalf("first execute: %v", err) @@ -78,3 +78,14 @@ func TestRegistryReturnsCachedResultForDuplicateDelivery(t *testing.T) { t.Fatalf("duplicate delivery invoked adapter %d times", adapter.reads) } } + +func TestRegistryRejectsUndeclaredAndMalformedPayloads(t *testing.T) { + stamp := time.Now().UTC() + registry := NewHandlerRegistry(HandlerAvailability{BoundServerID: "server-1", ServerVersion: "0.9.700.90357", Approved: true, Capabilities: map[string]bool{"config.patch": true}}, &adapterFixture{}) + for _, command := range []ClaimedCommand{{ID: "bad-type", ProfileKey: ProfileKey, CommandType: "raw.rcon", Payload: map[string]any{}, FencingToken: 1, LeaseExpiresAt: stamp.Add(time.Minute), ExpiresAt: stamp.Add(time.Minute)}, {ID: "bad-payload", ProfileKey: ProfileKey, CommandType: "config.patch", Payload: map[string]any{"revision": "r1"}, FencingToken: 1, LeaseExpiresAt: stamp.Add(time.Minute), ExpiresAt: stamp.Add(time.Minute)}} { + result, err := registry.Execute(context.Background(), command) + if err != nil || result.Payload["result"] != "validation-failed" { + t.Fatalf("unsafe command was not rejected: result=%+v err=%v", result, err) + } + } +}