fix: annotate auto-grant permission failures with required_scope and console_url (#1045)

When AutoGrantCurrentUserDrivePermission encounters lark code 99991672/99991679,
extract permission_violations from the underlying ExitError and surface
lark_code, required_scope, and console_url on the result map. Override the
generic fallback hint with one pointing at the developer console — the
concrete next step a user can take.

Refactor extractRequiredScopes / SelectRecommendedScope wrapping / console URL
construction out of cmd/root.go into internal/registry/scope_hint.go so both
the top-level enrichPermissionError path and the best-effort sub-call path in
shortcuts/common share one implementation.

Change-Id: Ida63ed160d1167b7961b6faac5c2cf9b7f971c65
This commit is contained in:
ZEden0
2026-05-25 11:01:01 +08:00
committed by GitHub
parent 6e3e120ec8
commit 6d1f9980fa
5 changed files with 465 additions and 39 deletions

View File

@@ -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
}

View File

@@ -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": "<scope>"}, ... ] }
//
// 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),
)
}

View File

@@ -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)
}
}

View File

@@ -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,
)
}

View File

@@ -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)
}
}