From 33ba65fc91e12e7dc19c0c868aea8533e8452b55 Mon Sep 17 00:00:00 2001 From: mingmxren Date: Sun, 1 Mar 2026 15:34:22 +0800 Subject: [PATCH] fix(commands): enforce reject-vs-passthrough command policy --- pkg/commands/builtin.go | 106 ++++++++++++------------ pkg/commands/builtin_test.go | 22 +++++ pkg/commands/show_list_handlers_test.go | 76 +++++++++++++++++ 3 files changed, 153 insertions(+), 51 deletions(-) create mode 100644 pkg/commands/show_list_handlers_test.go diff --git a/pkg/commands/builtin.go b/pkg/commands/builtin.go index 5e6937a68..ecb4c6ea5 100644 --- a/pkg/commands/builtin.go +++ b/pkg/commands/builtin.go @@ -75,29 +75,7 @@ func builtinDefinitions(cfg *config.Config, runtime Runtime) []Definition { Usage: "/show [model|channel]", Channels: []string{"telegram"}, Handler: func(_ context.Context, req Request) error { - if req.Reply == nil { - return nil - } - if cfg == nil { - return req.Reply("Command unavailable in current context.") - } - args := commandArgs(req.Text) - if args == "" { - return req.Reply("Usage: /show [model|channel]") - } - - switch args { - case "model": - return req.Reply(fmt.Sprintf( - "Current Model: %s (Provider: %s)", - cfg.Agents.Defaults.GetModelName(), - cfg.Agents.Defaults.Provider, - )) - case "channel": - return req.Reply(fmt.Sprintf("Current Channel: %s", req.Channel)) - default: - return req.Reply(fmt.Sprintf("Unknown parameter: %s. Try 'model' or 'channel'.", args)) - } + return handleShowCommand(req, cfg) }, }, { @@ -106,34 +84,7 @@ func builtinDefinitions(cfg *config.Config, runtime Runtime) []Definition { Usage: "/list [models|channels]", Channels: []string{"telegram"}, Handler: func(_ context.Context, req Request) error { - if req.Reply == nil { - return nil - } - if cfg == nil { - return req.Reply("Command unavailable in current context.") - } - args := commandArgs(req.Text) - if args == "" { - return req.Reply("Usage: /list [models|channels]") - } - - switch args { - case "models": - provider := cfg.Agents.Defaults.Provider - if provider == "" { - provider = "configured default" - } - return req.Reply(fmt.Sprintf( - "Configured Model: %s\nProvider: %s\n\nTo change models, update config.json", - cfg.Agents.Defaults.GetModelName(), - provider, - )) - case "channels": - enabled := enabledChannels(cfg) - return req.Reply(fmt.Sprintf("Enabled Channels:\n- %s", strings.Join(enabled, "\n- "))) - default: - return req.Reply(fmt.Sprintf("Unknown parameter: %s. Try 'models' or 'channels'.", args)) - } + return handleListCommand(req, cfg) }, }, } @@ -262,6 +213,59 @@ func handleSessionCommand(req Request, runtime Runtime) error { } } +func handleShowCommand(req Request, cfg *config.Config) error { + if cfg == nil { + return reply(req, "Command unavailable in current context.") + } + + args := commandArgs(req.Text) + if args == "" { + return reply(req, "Usage: /show [model|channel]") + } + + switch args { + case "model": + return reply(req, fmt.Sprintf( + "Current Model: %s (Provider: %s)", + cfg.Agents.Defaults.GetModelName(), + cfg.Agents.Defaults.Provider, + )) + case "channel": + return reply(req, fmt.Sprintf("Current Channel: %s", req.Channel)) + default: + return reply(req, fmt.Sprintf("Unknown parameter: %s. Try 'model' or 'channel'.", args)) + } +} + +func handleListCommand(req Request, cfg *config.Config) error { + if cfg == nil { + return reply(req, "Command unavailable in current context.") + } + + args := commandArgs(req.Text) + if args == "" { + return reply(req, "Usage: /list [models|channels]") + } + + switch args { + case "models": + provider := cfg.Agents.Defaults.Provider + if provider == "" { + provider = "configured default" + } + return reply(req, fmt.Sprintf( + "Configured Model: %s\nProvider: %s\n\nTo change models, update config.json", + cfg.Agents.Defaults.GetModelName(), + provider, + )) + case "channels": + enabled := enabledChannels(cfg) + return reply(req, fmt.Sprintf("Enabled Channels:\n- %s", strings.Join(enabled, "\n- "))) + default: + return reply(req, fmt.Sprintf("Unknown parameter: %s. Try 'models' or 'channels'.", args)) + } +} + func reply(req Request, text string) error { if req.Reply == nil { return nil diff --git a/pkg/commands/builtin_test.go b/pkg/commands/builtin_test.go index 0398cc627..c18e281cd 100644 --- a/pkg/commands/builtin_test.go +++ b/pkg/commands/builtin_test.go @@ -101,3 +101,25 @@ func TestBuiltinDefinitionsWithRuntime_EnablesSessionHandlers(t *testing.T) { t.Fatalf("/session should provide runtime-backed handler when runtime is available") } } + +func TestBuiltinDefinitions_ShowAndListAreTelegramOnlyHandlers(t *testing.T) { + defs := BuiltinDefinitions(&config.Config{}) + + defByName := map[string]Definition{} + for _, def := range defs { + defByName[def.Name] = def + } + + for _, name := range []string{"show", "list"} { + def, ok := defByName[name] + if !ok { + t.Fatalf("missing /%s definition", name) + } + if def.Handler == nil { + t.Fatalf("/%s should provide a builtin handler", name) + } + if len(def.Channels) != 1 || def.Channels[0] != "telegram" { + t.Fatalf("/%s channels=%v, want [telegram]", name, def.Channels) + } + } +} diff --git a/pkg/commands/show_list_handlers_test.go b/pkg/commands/show_list_handlers_test.go new file mode 100644 index 000000000..84df50f1f --- /dev/null +++ b/pkg/commands/show_list_handlers_test.go @@ -0,0 +1,76 @@ +package commands + +import ( + "context" + "testing" + + "github.com/sipeed/picoclaw/pkg/config" +) + +func TestShowListHandlers_ChannelPolicy(t *testing.T) { + cfg := &config.Config{} + ex := NewExecutor(NewRegistry(BuiltinDefinitions(cfg))) + + var telegramReply string + handled := ex.Execute(context.Background(), Request{ + Channel: "telegram", + Text: "/show channel", + Reply: func(text string) error { + telegramReply = text + return nil + }, + }) + if handled.Outcome != OutcomeHandled { + t.Fatalf("telegram /show outcome=%v, want=%v", handled.Outcome, OutcomeHandled) + } + if telegramReply != "Current Channel: telegram" { + t.Fatalf("telegram /show reply=%q, want=%q", telegramReply, "Current Channel: telegram") + } + + rejected := ex.Execute(context.Background(), Request{ + Channel: "whatsapp", + Text: "/show channel", + }) + if rejected.Outcome != OutcomeRejected { + t.Fatalf("whatsapp /show outcome=%v, want=%v", rejected.Outcome, OutcomeRejected) + } + if rejected.Command != "show" { + t.Fatalf("whatsapp /show command=%q, want=%q", rejected.Command, "show") + } + if rejected.Reply != "Command /show is not supported on whatsapp." { + t.Fatalf("whatsapp /show reply=%q, want=%q", rejected.Reply, "Command /show is not supported on whatsapp.") + } + + passthrough := ex.Execute(context.Background(), Request{ + Channel: "whatsapp", + Text: "/foo", + }) + if passthrough.Outcome != OutcomePassthrough { + t.Fatalf("whatsapp /foo outcome=%v, want=%v", passthrough.Outcome, OutcomePassthrough) + } + if passthrough.Command != "foo" { + t.Fatalf("whatsapp /foo command=%q, want=%q", passthrough.Command, "foo") + } + if passthrough.Reply != "" { + t.Fatalf("whatsapp /foo reply=%q, want empty", passthrough.Reply) + } +} + +func TestShowListHandlers_ListRejectsUnsupportedChannel(t *testing.T) { + cfg := &config.Config{} + ex := NewExecutor(NewRegistry(BuiltinDefinitions(cfg))) + + res := ex.Execute(context.Background(), Request{ + Channel: "whatsapp", + Text: "/list channels", + }) + if res.Outcome != OutcomeRejected { + t.Fatalf("whatsapp /list outcome=%v, want=%v", res.Outcome, OutcomeRejected) + } + if res.Command != "list" { + t.Fatalf("whatsapp /list command=%q, want=%q", res.Command, "list") + } + if res.Reply != "Command /list is not supported on whatsapp." { + t.Fatalf("whatsapp /list reply=%q, want=%q", res.Reply, "Command /list is not supported on whatsapp.") + } +}