From da65f777a239376275f9145fff84c50b143e6531 Mon Sep 17 00:00:00 2001 From: liangshuo-1 <266696938+liangshuo-1@users.noreply.github.com> Date: Thu, 14 May 2026 22:37:42 +0800 Subject: [PATCH 1/3] fix(commands): reject unsupported root commands and shortcuts Unsupported root commands and +shortcut tokens could fall through to parent Cobra commands, producing normal help output or generic Cobra text instead of a stable agent-readable error. That made command typos look successful to agents. Add root command preflight validation for two narrow cases: an unmatched top-level command token returns structured unknown_command, and an unmatched service +shortcut token returns structured unknown_shortcut. Normal help, known top-level commands, known shortcut flag errors, and dynamic API resource/method paths stay on the existing Cobra paths. Expose available_commands and available_services for root typos, and available_shortcuts for shortcut typos, with truncated human hints and machine-readable detail. Cover unknown command, unknown command with --help, unknown command followed by +shortcut, unsupported shortcut, unsupported shortcut with --help, empty shortcut token, root/service help, non-service top-level commands, and known-shortcut unknown flag handling in integration tests. Change-Id: I045a4e7fbcb56aa57d5314e812d602a543dc5575 --- CHANGELOG.md | 4 + cmd/command_preflight.go | 205 +++++++++++++++++++++++++++++++++++ cmd/root.go | 3 + cmd/root_integration_test.go | 200 ++++++++++++++++++++++++++++++++++ 4 files changed, 412 insertions(+) create mode 100644 cmd/command_preflight.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 8af91bad8..7ccb0dbe7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ All notable changes to this project will be documented in this file. - **drive**: Add modified-time smart sync mode (#859) - **approval**: Add `tasks.add_sign` and `tasks.rollback` methods (#867) +### Bug Fixes + +- **commands**: Return structured errors for unsupported top-level commands and `+shortcut` invocations instead of silently showing parent help + ## [v1.0.30] - 2026-05-13 ### Features diff --git a/cmd/command_preflight.go b/cmd/command_preflight.go new file mode 100644 index 000000000..4e1b3ae57 --- /dev/null +++ b/cmd/command_preflight.go @@ -0,0 +1,205 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package cmd + +import ( + "fmt" + "strings" + + "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/internal/registry" + "github.com/larksuite/cli/shortcuts" + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +const maxShortcutHintItems = 8 + +func validateCommandInvocation(root *cobra.Command, args []string) error { + if token, ok := firstRootCommandToken(root, args); ok { + if _, found := findTopLevelCommand(root, token); !found { + return unknownCommandError(root, token) + } + } + + // Traverse can still fail for ordinary Cobra errors (for example unknown + // flags). Leave those on the normal Execute path so this preflight only + // replaces silent parent-help fallbacks with structured validation errors. + cmd, remaining, err := root.Traverse(args) + if err != nil || cmd == nil || len(remaining) == 0 { + return nil + } + unknown := remaining[0] + if !strings.HasPrefix(unknown, "+") { + return nil + } + available := availableShortcutCommands(cmd.Name()) + if len(available) == 0 { + return nil + } + + commandPath := cmd.CommandPath() + return &output.ExitError{ + Code: output.ExitValidation, + Detail: &output.ErrDetail{ + Type: "unknown_shortcut", + Message: fmt.Sprintf("shortcut %q is not supported for %q", unknown, commandPath), + Hint: fmt.Sprintf("available shortcuts: %s; run `%s --help` to see all", shortcutHintList(available), commandPath), + Detail: map[string]interface{}{ + "shortcut": unknown, + "service": cmd.Name(), + "available_shortcuts": available, + }, + }, + } +} + +func firstRootCommandToken(root *cobra.Command, args []string) (string, bool) { + flags := root.PersistentFlags() + for i := 0; i < len(args); i++ { + arg := args[i] + if arg == "" { + continue + } + if arg == "--" { + if i+1 < len(args) { + return args[i+1], true + } + return "", false + } + if arg == "--help" || arg == "-h" { + return "", false + } + if strings.HasPrefix(arg, "--") { + flagName := strings.TrimPrefix(arg, "--") + hasInlineValue := false + if idx := strings.Index(flagName, "="); idx >= 0 { + flagName = flagName[:idx] + hasInlineValue = true + } + flag := flags.Lookup(flagName) + if flag == nil { + return "", false + } + if !hasInlineValue && flagConsumesValue(flag) { + i++ + } + continue + } + if strings.HasPrefix(arg, "-") && arg != "-" { + if len(arg) != 2 { + return "", false + } + flag := flags.ShorthandLookup(arg[1:]) + if flag == nil { + return "", false + } + if flagConsumesValue(flag) { + i++ + } + continue + } + return arg, true + } + return "", false +} + +func flagConsumesValue(flag *pflag.Flag) bool { + return flag != nil && flag.NoOptDefVal == "" +} + +func findTopLevelCommand(root *cobra.Command, token string) (*cobra.Command, bool) { + if token == "help" { + return nil, true + } + for _, cmd := range root.Commands() { + if cmd.Name() == token || cmd.HasAlias(token) { + return cmd, true + } + } + return nil, false +} + +func unknownCommandError(root *cobra.Command, token string) error { + available := availableTopLevelCommands(root) + services := availableServiceCommands(root) + commandPath := root.CommandPath() + return &output.ExitError{ + Code: output.ExitValidation, + Detail: &output.ErrDetail{ + Type: "unknown_command", + Message: fmt.Sprintf("command %q is not supported for %q", token, commandPath), + Hint: fmt.Sprintf("available commands: %s; run `%s --help` to see all", shortcutHintList(available), commandPath), + Detail: map[string]interface{}{ + "command": token, + "scope": "root", + "available_commands": available, + "available_services": services, + }, + }, + } +} + +func availableTopLevelCommands(root *cobra.Command) []string { + out := make([]string, 0) + for _, cmd := range root.Commands() { + if cmd.Hidden { + continue + } + out = append(out, cmd.Name()) + } + return out +} + +func availableServiceCommands(root *cobra.Command) []string { + registered := map[string]struct{}{} + for _, cmd := range root.Commands() { + registered[cmd.Name()] = struct{}{} + } + seen := map[string]struct{}{} + out := make([]string, 0) + for _, project := range registry.ListFromMetaProjects() { + spec := registry.LoadFromMeta(project) + if spec == nil { + continue + } + name := registry.GetStrFromMap(spec, "name") + if name == "" { + continue + } + if _, ok := registered[name]; !ok { + continue + } + if _, ok := seen[name]; ok { + continue + } + seen[name] = struct{}{} + out = append(out, name) + } + return out +} + +func availableShortcutCommands(service string) []string { + seen := map[string]struct{}{} + out := make([]string, 0) + for _, shortcut := range shortcuts.AllShortcuts() { + if shortcut.Service != service || shortcut.Hidden { + continue + } + if _, ok := seen[shortcut.Command]; ok { + continue + } + seen[shortcut.Command] = struct{}{} + out = append(out, shortcut.Command) + } + return out +} + +func shortcutHintList(commands []string) string { + if len(commands) <= maxShortcutHintItems { + return strings.Join(commands, ", ") + } + head := strings.Join(commands[:maxShortcutHintItems], ", ") + return fmt.Sprintf("%s, ...and %d more", head, len(commands)-maxShortcutHintItems) +} diff --git a/cmd/root.go b/cmd/root.go index 54fb5ed34..8ee9f8520 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -96,6 +96,9 @@ func Execute() int { // --- Notices (non-blocking) --- if !isCompletionCommand(os.Args) { + if err := validateCommandInvocation(rootCmd, os.Args[1:]); err != nil { + return handleRootError(f, err) + } setupNotices() } diff --git a/cmd/root_integration_test.go b/cmd/root_integration_test.go index 416777a44..5f98ebbba 100644 --- a/cmd/root_integration_test.go +++ b/cmd/root_integration_test.go @@ -49,6 +49,9 @@ func buildIntegrationRootCmd(t *testing.T, f *cmdutil.Factory) *cobra.Command { func executeRootIntegration(t *testing.T, f *cmdutil.Factory, rootCmd *cobra.Command, args []string) int { t.Helper() rootCmd.SetArgs(args) + if err := validateCommandInvocation(rootCmd, args); err != nil { + return handleRootError(f, err) + } if err := rootCmd.Execute(); err != nil { return handleRootError(f, err) } @@ -504,6 +507,203 @@ func TestIntegration_Shortcut_BusinessError_OutputsEnvelope(t *testing.T) { }) } +func TestIntegration_UnknownShortcut_ReturnsStructuredError(t *testing.T) { + f, stdout, stderr, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "e2e-unknown-shortcut", AppSecret: "secret", Brand: core.BrandFeishu, + }) + rootCmd := buildIntegrationRootCmd(t, f) + + code := executeRootIntegration(t, f, rootCmd, []string{"drive", "+nonexistent-thing"}) + + if code != output.ExitValidation { + t.Fatalf("exit code = %d, want %d", code, output.ExitValidation) + } + if stdout.Len() != 0 { + t.Fatalf("stdout = %q, want empty", stdout.String()) + } + env := parseEnvelope(t, stderr) + if env.Error == nil { + t.Fatal("expected error envelope detail") + } + if env.Error.Type != "unknown_shortcut" { + t.Fatalf("error type = %q, want unknown_shortcut", env.Error.Type) + } + if !strings.Contains(env.Error.Message, `shortcut "+nonexistent-thing" is not supported for "lark-cli drive"`) { + t.Fatalf("message missing unsupported shortcut context: %q", env.Error.Message) + } + if !strings.Contains(env.Error.Hint, "+upload") || !strings.Contains(env.Error.Hint, "lark-cli drive --help") { + t.Fatalf("hint should include available shortcuts and help command, got %q", env.Error.Hint) + } +} + +func TestIntegration_UnknownShortcutHelp_ReturnsStructuredError(t *testing.T) { + f, stdout, stderr, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "e2e-unknown-shortcut-help", AppSecret: "secret", Brand: core.BrandFeishu, + }) + rootCmd := buildIntegrationRootCmd(t, f) + + code := executeRootIntegration(t, f, rootCmd, []string{"drive", "+nonexistent-thing", "--help"}) + + if code != output.ExitValidation { + t.Fatalf("exit code = %d, want %d", code, output.ExitValidation) + } + if stdout.Len() != 0 { + t.Fatalf("stdout = %q, want empty", stdout.String()) + } + env := parseEnvelope(t, stderr) + if env.Error == nil || env.Error.Type != "unknown_shortcut" { + t.Fatalf("error = %#v, want type unknown_shortcut", env.Error) + } + if !strings.Contains(env.Error.Message, `shortcut "+nonexistent-thing" is not supported for "lark-cli drive"`) { + t.Fatalf("message missing unsupported shortcut context: %q", env.Error.Message) + } +} + +func TestIntegration_EmptyShortcut_ReturnsStructuredError(t *testing.T) { + f, stdout, stderr, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "e2e-empty-shortcut", AppSecret: "secret", Brand: core.BrandFeishu, + }) + rootCmd := buildIntegrationRootCmd(t, f) + + code := executeRootIntegration(t, f, rootCmd, []string{"drive", "+"}) + + if code != output.ExitValidation { + t.Fatalf("exit code = %d, want %d", code, output.ExitValidation) + } + if stdout.Len() != 0 { + t.Fatalf("stdout = %q, want empty", stdout.String()) + } + env := parseEnvelope(t, stderr) + if env.Error == nil || env.Error.Type != "unknown_shortcut" { + t.Fatalf("error = %#v, want type unknown_shortcut", env.Error) + } + if !strings.Contains(env.Error.Message, `shortcut "+" is not supported for "lark-cli drive"`) { + t.Fatalf("message missing empty shortcut context: %q", env.Error.Message) + } +} + +func TestIntegration_ServiceHelpStillSucceeds(t *testing.T) { + for _, args := range [][]string{{"drive"}, {"drive", "--help"}} { + t.Run(strings.Join(args, " "), func(t *testing.T) { + f, stdout, stderr, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "e2e-service-help", AppSecret: "secret", Brand: core.BrandFeishu, + }) + rootCmd := buildIntegrationRootCmd(t, f) + + code := executeRootIntegration(t, f, rootCmd, args) + + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) + } + if stderr.Len() != 0 { + t.Fatalf("stderr = %q, want empty", stderr.String()) + } + if !strings.Contains(stdout.String(), "Usage:\n lark-cli drive [command]") { + t.Fatalf("stdout missing drive help, got:\n%s", stdout.String()) + } + }) + } +} + +func TestIntegration_KnownShortcutUnknownFlagKeepsExistingBehavior(t *testing.T) { + f, stdout, stderr, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "e2e-shortcut-unknown-flag", AppSecret: "secret", Brand: core.BrandFeishu, + }) + rootCmd := buildIntegrationRootCmd(t, f) + + code := executeRootIntegration(t, f, rootCmd, []string{"drive", "+search", "--bogus"}) + + if code != 1 { + t.Fatalf("exit code = %d, want 1", code) + } + if !strings.Contains(stdout.String(), "Usage:\n lark-cli drive +search [flags]") { + t.Fatalf("stdout missing shortcut usage, got:\n%s", stdout.String()) + } + if !strings.Contains(stderr.String(), "Error: unknown flag: --bogus") { + t.Fatalf("stderr missing unknown flag error, got:\n%s", stderr.String()) + } +} + +func TestIntegration_UnknownTopLevelCommand_ReturnsStructuredError(t *testing.T) { + for _, args := range [][]string{ + {"nonexistent-service"}, + {"nonexistent-service", "--help"}, + {"nonexistent-service", "+search"}, + } { + t.Run(strings.Join(args, " "), func(t *testing.T) { + f, stdout, stderr, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "e2e-unknown-command", AppSecret: "secret", Brand: core.BrandFeishu, + }) + rootCmd := buildIntegrationRootCmd(t, f) + + code := executeRootIntegration(t, f, rootCmd, args) + + if code != output.ExitValidation { + t.Fatalf("exit code = %d, want %d", code, output.ExitValidation) + } + if stdout.Len() != 0 { + t.Fatalf("stdout = %q, want empty", stdout.String()) + } + env := parseEnvelope(t, stderr) + if env.Error == nil || env.Error.Type != "unknown_command" { + t.Fatalf("error = %#v, want type unknown_command", env.Error) + } + if !strings.Contains(env.Error.Message, `command "nonexistent-service" is not supported for "lark-cli"`) { + t.Fatalf("message missing unknown command context: %q", env.Error.Message) + } + if strings.Contains(stderr.String(), "Did you mean") || strings.Contains(stderr.String(), "unknown command") { + t.Fatalf("stderr should not include cobra unknown-command text, got:\n%s", stderr.String()) + } + }) + } +} + +func TestIntegration_RootHelpStillSucceeds(t *testing.T) { + for _, args := range [][]string{{}, {"--help"}} { + name := "no args" + if len(args) > 0 { + name = strings.Join(args, " ") + } + t.Run(name, func(t *testing.T) { + f, stdout, stderr, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "e2e-root-help", AppSecret: "secret", Brand: core.BrandFeishu, + }) + rootCmd := buildIntegrationRootCmd(t, f) + + code := executeRootIntegration(t, f, rootCmd, args) + + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) + } + if stderr.Len() != 0 { + t.Fatalf("stderr = %q, want empty", stderr.String()) + } + if !strings.Contains(stdout.String(), "Usage:") { + t.Fatalf("stdout missing root help, got:\n%s", stdout.String()) + } + }) + } +} + +func TestIntegration_NonServiceTopLevelCommandStillWorks(t *testing.T) { + f, stdout, stderr, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "e2e-core-command", AppSecret: "secret", Brand: core.BrandFeishu, + }) + rootCmd := buildIntegrationRootCmd(t, f) + + code := executeRootIntegration(t, f, rootCmd, []string{"api", "--help"}) + + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) + } + if stderr.Len() != 0 { + t.Fatalf("stderr = %q, want empty", stderr.String()) + } + if !strings.Contains(stdout.String(), "lark-cli api") { + t.Fatalf("stdout missing api help, got:\n%s", stdout.String()) + } +} + // TestSetupNotices_ColdStart_NoNotice verifies that a missing stamp // produces no skills key in the composed notice. Users who installed // skills via `npx skills add` (no stamp) must not see the misleading From fecd7ddc8f7f119227cee44a3a927b3af6c9bb59 Mon Sep 17 00:00:00 2001 From: liangshuo-1 <266696938+liangshuo-1@users.noreply.github.com> Date: Thu, 14 May 2026 23:06:56 +0800 Subject: [PATCH 2/3] fix(lint): satisfy nilerr in validateCommandInvocation Traverse handling Split the err branch and document intentional deferral to Execute with //nolint:nilerr so golangci-lint no longer flags returning nil after Traverse errors. Change-Id: Id671fc7f4fda15e74a45f86d2723c958401860a9 Co-authored-by: Cursor --- cmd/command_preflight.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/command_preflight.go b/cmd/command_preflight.go index 4e1b3ae57..3efc96b6d 100644 --- a/cmd/command_preflight.go +++ b/cmd/command_preflight.go @@ -27,7 +27,10 @@ func validateCommandInvocation(root *cobra.Command, args []string) error { // flags). Leave those on the normal Execute path so this preflight only // replaces silent parent-help fallbacks with structured validation errors. cmd, remaining, err := root.Traverse(args) - if err != nil || cmd == nil || len(remaining) == 0 { + if err != nil { + return nil //nolint:nilerr // defer to RootCmd.Execute* for normal Cobra errors + } + if cmd == nil || len(remaining) == 0 { return nil } unknown := remaining[0] From df71c4d8d91f7374459e633bf4a43fee384d875c Mon Sep 17 00:00:00 2001 From: liangshuo-1 <266696938+liangshuo-1@users.noreply.github.com> Date: Thu, 14 May 2026 23:08:14 +0800 Subject: [PATCH 3/3] fix(lint): discard traverse errors in command preflight The command preflight intentionally leaves ordinary Cobra parse errors on the normal Execute path. Discard the Traverse error directly instead of returning nil from a non-nil error branch, avoiding nilerr without a nolint escape hatch. Change-Id: I482033a5de635cc0a93621ef046ca94680142fed --- cmd/command_preflight.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cmd/command_preflight.go b/cmd/command_preflight.go index 3efc96b6d..37cf81d85 100644 --- a/cmd/command_preflight.go +++ b/cmd/command_preflight.go @@ -24,12 +24,10 @@ func validateCommandInvocation(root *cobra.Command, args []string) error { } // Traverse can still fail for ordinary Cobra errors (for example unknown - // flags). Leave those on the normal Execute path so this preflight only - // replaces silent parent-help fallbacks with structured validation errors. - cmd, remaining, err := root.Traverse(args) - if err != nil { - return nil //nolint:nilerr // defer to RootCmd.Execute* for normal Cobra errors - } + // flags). Discard that error here and leave it on the normal Execute path + // so this preflight only replaces silent parent-help fallbacks with + // structured validation errors. + cmd, remaining, _ := root.Traverse(args) if cmd == nil || len(remaining) == 0 { return nil }