diff --git a/cmd/root.go b/cmd/root.go index 00d9a24bc..edacf51c4 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -10,7 +10,6 @@ import ( "errors" "fmt" "io" - "net/url" "os" "sort" "strconv" @@ -389,8 +388,8 @@ func enrichPermissionError(f *cmdutil.Factory, exitErr *output.ExitError) { if exitErr.Detail == nil || exitErr.Detail.Type != "permission" { return } - // Extract required scopes from API error detail - scopes := extractRequiredScopes(exitErr.Detail.Detail) + // Extract required scopes from API error detail (shared helper) + scopes := registry.ExtractRequiredScopes(exitErr.Detail.Detail) if len(scopes) == 0 { return } @@ -401,21 +400,10 @@ func enrichPermissionError(f *cmdutil.Factory, exitErr *output.ExitError) { } // Select the recommended (least-privilege) scope - scopeIfaces := make([]interface{}, len(scopes)) - for i, s := range scopes { - scopeIfaces[i] = s - } - recommended := registry.SelectRecommendedScope(scopeIfaces, "tenant") - if recommended == "" { - recommended = scopes[0] - } + recommended := registry.SelectRecommendedScopeFromStrings(scopes, "tenant") // Build admin console URL with the recommended scope - host := "open.feishu.cn" - if cfg.Brand == "lark" { - host = "open.larksuite.com" - } - consoleURL := fmt.Sprintf("https://%s/page/scope-apply?clientID=%s&scopes=%s", host, url.QueryEscape(cfg.AppID), url.QueryEscape(recommended)) + consoleURL := registry.BuildConsoleScopeURL(cfg.Brand, cfg.AppID, recommended) // Clear raw API detail — useful info is now in message/hint/console_url exitErr.Detail.Detail = nil @@ -452,26 +440,3 @@ func enrichPermissionError(f *cmdutil.Factory, exitErr *output.ExitError) { exitErr.Detail.ConsoleURL = consoleURL } } - -// extractRequiredScopes extracts scope names from the API error's permission_violations field. -func extractRequiredScopes(detail interface{}) []string { - m, ok := detail.(map[string]interface{}) - if !ok { - return nil - } - violations, ok := m["permission_violations"].([]interface{}) - if !ok { - return nil - } - var scopes []string - for _, v := range violations { - vm, ok := v.(map[string]interface{}) - if !ok { - continue - } - if subject, ok := vm["subject"].(string); ok { - scopes = append(scopes, subject) - } - } - return scopes -} diff --git a/internal/registry/scope_hint.go b/internal/registry/scope_hint.go new file mode 100644 index 000000000..1431ac210 --- /dev/null +++ b/internal/registry/scope_hint.go @@ -0,0 +1,82 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package registry + +import ( + "fmt" + "net/url" + + "github.com/larksuite/cli/internal/core" +) + +// ExtractRequiredScopes pulls scope names out of the API error's +// permission_violations field. The detail argument is the raw `error` block +// that the platform returns alongside lark code 99991672 / 99991679 — typically +// shaped as: +// +// { "permission_violations": [ {"subject": ""}, ... ] } +// +// Returns nil when the structure does not match or no non-empty subjects are +// present, so callers can branch on a simple len() == 0 check. +func ExtractRequiredScopes(detail interface{}) []string { + m, ok := detail.(map[string]interface{}) + if !ok { + return nil + } + violations, ok := m["permission_violations"].([]interface{}) + if !ok { + return nil + } + scopes := make([]string, 0, len(violations)) + for _, v := range violations { + vm, ok := v.(map[string]interface{}) + if !ok { + continue + } + if subject, ok := vm["subject"].(string); ok && subject != "" { + scopes = append(scopes, subject) + } + } + if len(scopes) == 0 { + return nil + } + return scopes +} + +// SelectRecommendedScopeFromStrings is a string-typed convenience wrapper +// around SelectRecommendedScope. When no scope is recognized by the priority +// table, it falls back to the first input scope so callers always have +// something to surface to users. +func SelectRecommendedScopeFromStrings(scopes []string, identity string) string { + if len(scopes) == 0 { + return "" + } + ifaces := make([]interface{}, len(scopes)) + for i, s := range scopes { + ifaces[i] = s + } + if recommended := SelectRecommendedScope(ifaces, identity); recommended != "" { + return recommended + } + return scopes[0] +} + +// BuildConsoleScopeURL returns the developer-console "apply scope" URL for the +// given app and scope, branded for feishu / lark. Returns "" when appID or +// scope is empty so callers can omit the field cleanly. +func BuildConsoleScopeURL(brand core.LarkBrand, appID, scope string) string { + if appID == "" || scope == "" { + return "" + } + host := "open.feishu.cn" + if brand == core.BrandLark { + host = "open.larksuite.com" + } + return fmt.Sprintf( + "https://%s/page/scope-apply?clientID=%s&scopes=%s", + host, + url.QueryEscape(appID), + url.QueryEscape(scope), + ) +} diff --git a/internal/registry/scope_hint_test.go b/internal/registry/scope_hint_test.go new file mode 100644 index 000000000..e19628d6f --- /dev/null +++ b/internal/registry/scope_hint_test.go @@ -0,0 +1,104 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package registry + +import ( + "strings" + "testing" + + "github.com/larksuite/cli/internal/core" +) + +func TestExtractRequiredScopes_HappyPath(t *testing.T) { + detail := map[string]interface{}{ + "permission_violations": []interface{}{ + map[string]interface{}{"subject": "docs:permission.member:create"}, + map[string]interface{}{"subject": "docs:doc"}, + map[string]interface{}{"subject": ""}, // empty subject filtered + "not-a-map", // ignored + }, + } + got := ExtractRequiredScopes(detail) + want := []string{"docs:permission.member:create", "docs:doc"} + if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] { + t.Fatalf("ExtractRequiredScopes mismatch: got %v, want %v", got, want) + } +} + +func TestExtractRequiredScopes_NilOrMalformed(t *testing.T) { + cases := []interface{}{ + nil, + "plain string", + map[string]interface{}{}, + map[string]interface{}{"permission_violations": "not-a-list"}, + map[string]interface{}{"permission_violations": []interface{}{}}, + map[string]interface{}{"permission_violations": []interface{}{ + map[string]interface{}{"subject": ""}, + }}, + } + for i, in := range cases { + if got := ExtractRequiredScopes(in); got != nil { + t.Errorf("case %d: expected nil, got %v", i, got) + } + } +} + +func TestBuildConsoleScopeURL_BrandSpecificHost(t *testing.T) { + got := BuildConsoleScopeURL(core.BrandFeishu, "cli_xxx", "docs:permission.member:create") + if !strings.Contains(got, "open.feishu.cn") { + t.Errorf("feishu brand should use open.feishu.cn host, got %s", got) + } + if !strings.Contains(got, "clientID=cli_xxx") { + t.Errorf("missing app id in url: %s", got) + } + if !strings.Contains(got, "scopes=docs%3Apermission.member%3Acreate") { + t.Errorf("scope not URL-escaped: %s", got) + } + + got = BuildConsoleScopeURL(core.BrandLark, "cli_yyy", "drive:drive") + if !strings.Contains(got, "open.larksuite.com") { + t.Errorf("lark brand should use open.larksuite.com host, got %s", got) + } +} + +func TestBuildConsoleScopeURL_EmptyInput(t *testing.T) { + if got := BuildConsoleScopeURL(core.BrandFeishu, "", "docs:doc"); got != "" { + t.Errorf("empty appID should yield empty url, got %s", got) + } + if got := BuildConsoleScopeURL(core.BrandFeishu, "cli_xxx", ""); got != "" { + t.Errorf("empty scope should yield empty url, got %s", got) + } +} + +func TestSelectRecommendedScopeFromStrings_FallsBackToFirst(t *testing.T) { + ensureFreshRegistry(t) + // Unknown scopes (not in priority table) → fallback to first + got := SelectRecommendedScopeFromStrings([]string{"unknown:foo", "unknown:bar"}, "tenant") + if got != "unknown:foo" { + t.Errorf("expected fallback to first, got %s", got) + } +} + +// When at least one scope is recognized by the priority table, the +// recommended scope wins over the fallback (first input). +func TestSelectRecommendedScopeFromStrings_PicksKnownScopeOverFallback(t *testing.T) { + ensureFreshRegistry(t) + // docs:permission.member:create is recommended (recommend=true) in + // scope_priorities.json. Putting an unknown scope first would otherwise + // win via the fallback path; this ensures the priority table is consulted + // before falling back. + got := SelectRecommendedScopeFromStrings([]string{"unknown:foo", "docs:permission.member:create"}, "tenant") + if got != "docs:permission.member:create" { + t.Errorf("expected priority-table winner, got %s", got) + } +} + +func TestSelectRecommendedScopeFromStrings_Empty(t *testing.T) { + if got := SelectRecommendedScopeFromStrings(nil, "tenant"); got != "" { + t.Errorf("nil slice should return empty, got %s", got) + } + if got := SelectRecommendedScopeFromStrings([]string{}, "tenant"); got != "" { + t.Errorf("empty slice should return empty, got %s", got) + } +} diff --git a/shortcuts/common/permission_grant.go b/shortcuts/common/permission_grant.go index 1a67488d0..3556b86c6 100644 --- a/shortcuts/common/permission_grant.go +++ b/shortcuts/common/permission_grant.go @@ -4,9 +4,12 @@ package common import ( + "errors" "fmt" "strings" + "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/internal/registry" "github.com/larksuite/cli/internal/validate" ) @@ -81,6 +84,12 @@ func autoGrantCurrentUserDrivePermission(runtime *RuntimeContext, token, resourc fmt.Sprintf("Resource was created, but granting current user %s failed: %s. You can retry later or continue using bot identity.", permissionGrantPermMessage(), errMsg), fmt.Sprintf("Auto-grant failed: %s. The app may lack the required scope or the resource restricts permission changes.", errMsg), ) + // Best-effort: when the underlying error is a structured permission + // ExitError (lark code 99991672/99991679), surface lark_code, + // required_scope and console_url so agents can guide users straight + // to the dev console. Overrides the generic hint with a more + // actionable one when console_url is available. + annotateGrantPermissionError(runtime, result, err) fmt.Fprintf(runtime.IO().ErrOut, "Warning: resource was created, but auto-grant failed: %s. Retry later or grant permission manually.\n", errMsg) return result } @@ -151,3 +160,54 @@ func compactPermissionGrantError(err error) string { } return strings.Join(strings.Fields(err.Error()), " ") } + +// annotateGrantPermissionError enriches a failed permission_grant result with +// structured fields (lark_code / required_scope / console_url) when the +// underlying error is a permission-class *output.ExitError. The CLI's main +// permission-error path (cmd/root.go::enrichPermissionError) handles the same +// case for top-level failures; this helper covers best-effort sub-calls whose +// error is folded into a result map instead of propagated as ExitError. +// +// When console_url is available, the existing generic hint is overridden with +// a more actionable one pointing at the developer console — that's the +// concrete next step a user can take. +func annotateGrantPermissionError(runtime *RuntimeContext, result map[string]interface{}, err error) { + if runtime == nil || result == nil || err == nil { + return + } + var exitErr *output.ExitError + if !errors.As(err, &exitErr) || exitErr.Detail == nil { + return + } + if exitErr.Detail.Type != "permission" { + return + } + if exitErr.Detail.Code != 0 { + result["lark_code"] = exitErr.Detail.Code + } + + scopes := registry.ExtractRequiredScopes(exitErr.Detail.Detail) + if len(scopes) == 0 { + return + } + recommended := registry.SelectRecommendedScopeFromStrings(scopes, "tenant") + if recommended == "" { + return + } + result["required_scope"] = recommended + + if runtime.Config == nil || runtime.Config.AppID == "" { + return + } + consoleURL := registry.BuildConsoleScopeURL(runtime.Config.Brand, runtime.Config.AppID, recommended) + if consoleURL == "" { + return + } + result["console_url"] = consoleURL + // Override the generic hint: pointing at the dev console is more actionable + // than the generic "retry later" fallback set by buildPermissionGrantResult. + result["hint"] = fmt.Sprintf( + "App is missing the %q scope; enable it in the developer console (see console_url), then retry.", + recommended, + ) +} diff --git a/shortcuts/common/permission_grant_test.go b/shortcuts/common/permission_grant_test.go index e029ef099..8e87e8fd8 100644 --- a/shortcuts/common/permission_grant_test.go +++ b/shortcuts/common/permission_grant_test.go @@ -5,12 +5,14 @@ package common import ( "context" + "errors" "strings" "testing" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" ) func TestAutoGrantStderrWarning_SkippedNoUser(t *testing.T) { @@ -94,3 +96,216 @@ func TestAutoGrantStderrWarning_GrantFailed(t *testing.T) { t.Fatalf("hint = %#v, want string containing 'permission changes'", result["hint"]) } } + +// ── annotateGrantPermissionError unit tests ──────────────────────────────── + +func newAnnotateRuntime(brand core.LarkBrand, appID string) *RuntimeContext { + return &RuntimeContext{ + Config: &core.CliConfig{ + AppID: appID, + Brand: brand, + }, + } +} + +// permission_violations subjects must surface as required_scope, and the +// console_url must be brand-specific. The hint should be overridden to point +// at the developer console. +func TestAnnotateGrantPermissionError_AppScopeNotEnabled(t *testing.T) { + rt := newAnnotateRuntime(core.BrandFeishu, "cli_demo") + result := map[string]interface{}{ + "hint": "generic fallback hint", + } + + err := output.ErrAPI(99991672, "Permission denied [99991672]", map[string]interface{}{ + "permission_violations": []interface{}{ + map[string]interface{}{"subject": "docs:permission.member:create"}, + }, + }) + + annotateGrantPermissionError(rt, result, err) + + if got := result["lark_code"]; got != 99991672 { + t.Errorf("expected lark_code=99991672, got %v", got) + } + if got, _ := result["required_scope"].(string); got != "docs:permission.member:create" { + t.Errorf("required_scope mismatch: got %v", got) + } + consoleURL, _ := result["console_url"].(string) + if !strings.HasPrefix(consoleURL, "https://open.feishu.cn/page/scope-apply") { + t.Errorf("console_url should target open.feishu.cn, got %s", consoleURL) + } + if !strings.Contains(consoleURL, "clientID=cli_demo") { + t.Errorf("console_url missing clientID, got %s", consoleURL) + } + hint, _ := result["hint"].(string) + if !strings.Contains(hint, "console_url") { + t.Errorf("hint should reference console_url, got %s", hint) + } + if !strings.Contains(hint, "docs:permission.member:create") { + t.Errorf("hint should mention required scope, got %s", hint) + } +} + +func TestAnnotateGrantPermissionError_LarkBrand(t *testing.T) { + rt := newAnnotateRuntime(core.BrandLark, "cli_demo") + result := map[string]interface{}{} + err := output.ErrAPI(99991679, "Permission denied [99991679]", map[string]interface{}{ + "permission_violations": []interface{}{ + map[string]interface{}{"subject": "docs:permission.member:create"}, + }, + }) + + annotateGrantPermissionError(rt, result, err) + + if u, _ := result["console_url"].(string); !strings.Contains(u, "open.larksuite.com") { + t.Errorf("lark brand should yield larksuite host, got %s", u) + } +} + +// Non-permission errors (network, validation, plain errors) must not be +// annotated — keep the existing generic hint untouched. +func TestAnnotateGrantPermissionError_NonPermissionErrorNoOp(t *testing.T) { + rt := newAnnotateRuntime(core.BrandFeishu, "cli_demo") + + cases := []error{ + errors.New("plain error"), + output.ErrNetwork("connection reset"), + output.ErrValidation("bad request"), + // Non-permission API errors (e.g. 230001) — type is "api_error" not "permission" + output.ErrAPI(230001, "no permission", map[string]interface{}{ + "permission_violations": []interface{}{ + map[string]interface{}{"subject": "docs:doc"}, + }, + }), + } + for i, e := range cases { + result := map[string]interface{}{ + "hint": "untouched hint", + } + annotateGrantPermissionError(rt, result, e) + if _, ok := result["lark_code"]; ok { + t.Errorf("case %d: expected no lark_code, got %v", i, result["lark_code"]) + } + if _, ok := result["console_url"]; ok { + t.Errorf("case %d: expected no console_url, got %v", i, result["console_url"]) + } + if got, _ := result["hint"].(string); got != "untouched hint" { + t.Errorf("case %d: hint should be unchanged, got %s", i, got) + } + } +} + +// permission_violations missing → only lark_code is annotated; no console_url +// and the existing hint stays as-is (caller's generic fallback wins). +func TestAnnotateGrantPermissionError_NoViolations(t *testing.T) { + rt := newAnnotateRuntime(core.BrandFeishu, "cli_demo") + result := map[string]interface{}{ + "hint": "untouched fallback", + } + err := output.ErrAPI(99991672, "Permission denied [99991672]", nil) + + annotateGrantPermissionError(rt, result, err) + + if got := result["lark_code"]; got != 99991672 { + t.Errorf("expected lark_code captured, got %v", got) + } + if _, ok := result["console_url"]; ok { + t.Errorf("console_url must not be set when violations are absent") + } + if got, _ := result["hint"].(string); got != "untouched fallback" { + t.Errorf("hint should remain fallback when no console_url, got %s", got) + } +} + +// AppID empty → no console_url even when violations exist. +func TestAnnotateGrantPermissionError_EmptyAppID(t *testing.T) { + rt := newAnnotateRuntime(core.BrandFeishu, "") + result := map[string]interface{}{} + err := output.ErrAPI(99991672, "Permission denied", map[string]interface{}{ + "permission_violations": []interface{}{ + map[string]interface{}{"subject": "docs:doc"}, + }, + }) + + annotateGrantPermissionError(rt, result, err) + if _, ok := result["console_url"]; ok { + t.Errorf("console_url must not be set when appID is empty") + } + if got, _ := result["required_scope"].(string); got != "docs:doc" { + t.Errorf("required_scope should still be set when appID is empty, got %s", got) + } +} + +// Defensive: nil/empty arguments must be safe no-ops. +func TestAnnotateGrantPermissionError_NilArgsSafe(t *testing.T) { + rt := newAnnotateRuntime(core.BrandFeishu, "cli_demo") + + annotateGrantPermissionError(nil, map[string]interface{}{}, nil) + annotateGrantPermissionError(rt, nil, nil) + annotateGrantPermissionError(rt, map[string]interface{}{}, nil) + annotateGrantPermissionError(rt, map[string]interface{}{}, errors.New("")) +} + +// Integration-style: end-to-end through AutoGrantCurrentUserDrivePermission +// with a mocked 99991672 response — verifies the annotated fields show up +// in the JSON result that callers downstream consume. +func TestAutoGrantStderrWarning_GrantFailed_AppScopeNotEnabled_Annotated(t *testing.T) { + config := &core.CliConfig{ + AppID: "cli_app_demo", + AppSecret: "secret", + Brand: core.BrandFeishu, + UserOpenId: "ou_test_user", + } + f, _, _, reg := cmdutil.TestFactory(t, config) + + // Stub the permission member create endpoint with a 99991672 response that + // includes permission_violations — what the platform returns when the app + // has not enabled the API scope. + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/drive/v1/permissions/tkn_doc/members", + Body: map[string]interface{}{ + "code": 99991672, + "msg": "App scope not enabled", + "error": map[string]interface{}{ + "permission_violations": []interface{}{ + map[string]interface{}{"subject": "docs:permission.member:create"}, + }, + }, + }, + }) + + ctx := cmdutil.ContextWithShortcut(context.Background(), "test:shortcut", "exec-3") + runtime := &RuntimeContext{ + ctx: ctx, + Config: config, + Factory: f, + resolvedAs: core.AsBot, + } + + result := AutoGrantCurrentUserDrivePermission(runtime, "tkn_doc", "docx") + if result == nil { + t.Fatal("expected non-nil result") + } + if result["status"] != PermissionGrantFailed { + t.Fatalf("status = %v, want failed", result["status"]) + } + if result["lark_code"] != 99991672 { + t.Errorf("lark_code = %v, want 99991672", result["lark_code"]) + } + if got, _ := result["required_scope"].(string); got != "docs:permission.member:create" { + t.Errorf("required_scope = %v, want docs:permission.member:create", got) + } + consoleURL, _ := result["console_url"].(string) + if !strings.Contains(consoleURL, "open.feishu.cn/page/scope-apply") { + t.Errorf("console_url missing or wrong host: %s", consoleURL) + } + if !strings.Contains(consoleURL, "scopes=docs%3Apermission.member%3Acreate") { + t.Errorf("console_url missing escaped scope: %s", consoleURL) + } + hint, _ := result["hint"].(string) + if !strings.Contains(hint, "console_url") { + t.Errorf("hint should be overridden to mention console_url, got %s", hint) + } +}