Skip to content

Commit faef7e1

Browse files
committed
feat(errs): typed envelope contract for auth-domain errors
Every failure on the authentication, authorization, and configuration path now surfaces as a typed structured error instead of an ad-hoc envelope. Users and scripts that consume CLI output get: - a fixed nine-category taxonomy on the wire, each mapped to a stable shell exit code (authentication/authorization/config = 3, network = 4, internal = 5, policy = 6, confirmation = 10) - identity-aware detail fields (missing_scopes, requested_scopes, granted_scopes, console_url, log_id, retryable, hint) carried uniformly on the envelope - a single canonical policy envelope at exit 6; the legacy auth_error carve-out is retired - per-subtype canonical message + hint that preserves Lark's diagnostic phrasing and routes recovery to the right actor: app developer (app_scope_not_applied), user (missing_scope, token_scope_insufficient, user_unauthorized), or tenant admin (app_unavailable, app_disabled) - wrong app credentials classify as config/invalid_client whether surfaced by the Open API endpoint (99991543) or the tenant access-token mint endpoint (10014), instead of collapsing to a transport error - bind workflows (Hermes / OpenClaw / lark-channel) flatten dynamic Type tags to wire 'config' with the original module name kept as a metric label All 10 typed errors are cause-bearing, nil-safe on .Error() and .Unwrap(), and defensively clone slice setter inputs. Four lint rules (CheckNilSafeError / CheckBuilderImmutable / CheckUnwrapSymmetry / CheckBuildAPIErrorArms) lock these invariants on migrated paths.
1 parent ab94ee9 commit faef7e1

92 files changed

Lines changed: 5859 additions & 1624 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.golangci.yml

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,18 +49,26 @@ linters:
4949
- gocritic
5050
- depguard
5151
- forbidigo
52-
- path-except: (shortcuts/|internal/)
52+
# Paths that run forbidigo. Add an entry when a path joins one of
53+
# the rules below.
54+
- path-except: (shortcuts/|internal/|cmd/auth/|cmd/config/|cmd/service/)
5355
linters:
5456
- forbidigo
5557
- path: internal/vfs/
5658
linters:
5759
- forbidigo
58-
# The shortcuts-no-raw-http forbidigo rule below is shortcuts-only;
59-
# internal/ legitimately wraps raw HTTP for the client / credential layer.
60+
# shortcuts-no-raw-http is shortcuts-only; internal/ wraps raw HTTP
61+
# for the client / credential layer.
6062
- path-except: shortcuts/
6163
text: shortcuts-no-raw-http
6264
linters:
6365
- forbidigo
66+
# errs-typed-only enforced on paths already migrated to errs.NewXxxError.
67+
# Add a path when its migration is complete.
68+
- path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|cmd/auth/|cmd/config/|cmd/service/|shortcuts/common/mcp_client\.go|shortcuts/calendar/helpers\.go)
69+
text: errs-typed-only
70+
linters:
71+
- forbidigo
6472

6573
settings:
6674
depguard:
@@ -79,6 +87,13 @@ linters:
7987
Use runtime.FileIO() for file operations or runtime.ValidatePath() for path validation.
8088
forbidigo:
8189
forbid:
90+
# ── legacy output.Err* helpers banned on migrated paths ──
91+
# output.ErrBare is intentionally not listed — it is the predicate-
92+
# command silent-exit signal, outside the typed envelope contract.
93+
- pattern: output\.(ErrValidation|ErrAuth|ErrNetwork|ErrAPI|ErrWithHint|Errorf)\b
94+
msg: >-
95+
[errs-typed-only] use errs.NewXxxError(...) builder
96+
(see errs/types.go).
8297
# ── http: shortcuts must not construct raw HTTP requests ──
8398
# Bans request / client construction; constants (http.MethodPost,
8499
# http.StatusOK) and pure helpers (http.StatusText, http.Header) are

cmd/api/api.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,10 @@ func apiRun(opts *APIOptions) error {
238238

239239
resp, err := ac.DoAPI(opts.Ctx, request)
240240
if err != nil {
241-
// MarkRaw tells the dispatcher to skip enrichPermissionError so the
242-
// raw API error detail (log_id, troubleshooter, permission_violations)
243-
// stays on the wire — `lark-cli api` callers explicitly want the raw
244-
// envelope.
241+
// MarkRaw tells the dispatcher to skip the legacy enrichPermissionError
242+
// pass on *output.ExitError values. Typed *errs.* errors that flow
243+
// through here keep their canonical message / hint from BuildAPIError;
244+
// MarkRaw is a no-op on those (it only flips a flag on *ExitError).
245245
return output.MarkRaw(err)
246246
}
247247
err = client.HandleResponse(resp, client.ResponseOptions{
@@ -253,14 +253,14 @@ func apiRun(opts *APIOptions) error {
253253
FileIO: f.ResolveFileIO(opts.Ctx),
254254
CommandPath: opts.Cmd.CommandPath(),
255255
Identity: opts.As,
256-
// Stage 1: CheckResponse emits the legacy *output.ExitError envelope.
257-
// Per-domain migration in stage 2+ will route through
258-
// errclass.BuildAPIError to populate identity-aware fields
259-
// (PermissionError.ConsoleURL needs Brand+AppID from the client).
256+
// CheckResponse routes through errclass.BuildAPIError for known Lark
257+
// codes (typed PermissionError / AuthenticationError / ...). For
258+
// unknown codes it falls back to *errs.APIError. The Brand+AppID on
259+
// the client populate identity-aware fields (ConsoleURL etc.).
260260
CheckError: ac.CheckResponse,
261261
})
262-
// MarkRaw: see comment above on the DoAPI path. Applies equally to
263-
// HandleResponse failures so the raw API error survives to the wire.
262+
// MarkRaw: see comment above on the DoAPI path. Skips legacy
263+
// *ExitError enrichment; typed errors flow through unchanged.
264264
if err != nil {
265265
return output.MarkRaw(err)
266266
}

cmd/api/api_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@
44
package api
55

66
import (
7+
"errors"
78
"os"
89
"sort"
910
"strings"
1011
"testing"
1112

13+
"github.com/larksuite/cli/errs"
1214
"github.com/larksuite/cli/internal/cmdutil"
1315
"github.com/larksuite/cli/internal/core"
1416
"github.com/larksuite/cli/internal/httpmock"
@@ -670,3 +672,49 @@ func TestApiCmd_DryRunWithFile(t *testing.T) {
670672
t.Errorf("expected dry-run header, got: %s", out)
671673
}
672674
}
675+
676+
// TestApiCmd_PermissionError_DerivesFirstClassFields pins that when a Lark
677+
// API returns a missing-scope failure, the typed *errs.PermissionError
678+
// surfaced by `lark-cli api` lifts the diagnostic signals BuildAPIError
679+
// consumed during classification into first-class wire fields
680+
// (MissingScopes, LogID, ConsoleURL). The wire shape is the typed envelope
681+
// — there is no raw-payload passthrough; new Lark diagnostic fields require
682+
// a CLI release.
683+
func TestApiCmd_PermissionError_DerivesFirstClassFields(t *testing.T) {
684+
f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{
685+
AppID: "cli_test_perm", AppSecret: "secret", Brand: core.BrandFeishu,
686+
})
687+
688+
reg.Register(&httpmock.Stub{
689+
URL: "/open-apis/docx/v1/documents/test",
690+
Body: map[string]interface{}{
691+
"code": 99991679,
692+
"msg": "scope missing",
693+
"log_id": "20260527-test-log",
694+
"error": map[string]interface{}{
695+
"permission_violations": []interface{}{
696+
map[string]interface{}{"subject": "docx:document"},
697+
},
698+
},
699+
},
700+
})
701+
702+
cmd := NewCmdApi(f, nil)
703+
cmd.SetArgs([]string{"GET", "/open-apis/docx/v1/documents/test", "--as", "bot"})
704+
err := cmd.Execute()
705+
if err == nil {
706+
t.Fatal("expected error for non-zero code")
707+
}
708+
709+
var pe *errs.PermissionError
710+
if !errors.As(err, &pe) {
711+
t.Fatalf("expected *errs.PermissionError, got %T: %v", err, err)
712+
}
713+
714+
if len(pe.MissingScopes) != 1 || pe.MissingScopes[0] != "docx:document" {
715+
t.Errorf("MissingScopes = %v, want [docx:document]", pe.MissingScopes)
716+
}
717+
if pe.LogID != "20260527-test-log" {
718+
t.Errorf("LogID = %q, want %q", pe.LogID, "20260527-test-log")
719+
}
720+
}

cmd/auth/auth.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
larkauth "github.com/larksuite/cli/internal/auth"
1818
"github.com/larksuite/cli/internal/cmdutil"
1919
"github.com/larksuite/cli/internal/core"
20+
"github.com/larksuite/cli/internal/errclass"
2021
)
2122

2223
// NewCmdAuth creates the auth command with subcommands.
@@ -110,6 +111,11 @@ type appInfoResponse struct {
110111
} `json:"data"`
111112
}
112113

114+
// getAppInfoFn is the package-level seam used by callers (scopes.go) so tests
115+
// can substitute a fake without standing up a full SDK + httpmock pipeline.
116+
// Mirrors the pollDeviceToken pattern in login.go.
117+
var getAppInfoFn = getAppInfo
118+
113119
// getAppInfo queries app info from the Lark API.
114120
func getAppInfo(ctx context.Context, f *cmdutil.Factory, appId string) (*appInfo, error) {
115121
ac, err := f.NewAPIClient()
@@ -134,7 +140,7 @@ func getAppInfo(ctx context.Context, f *cmdutil.Factory, appId string) (*appInfo
134140
return nil, fmt.Errorf("failed to parse response: %v", err)
135141
}
136142
if resp.Code != 0 {
137-
return nil, fmt.Errorf("API error [%d]: %s", resp.Code, resp.Msg)
143+
return nil, classifyAppInfoErr(apiResp.RawBody, resp.Code, resp.Msg, f, appId)
138144
}
139145

140146
app := resp.Data.App
@@ -153,3 +159,21 @@ func getAppInfo(ctx context.Context, f *cmdutil.Factory, appId string) (*appInfo
153159

154160
return &appInfo{OwnerOpenId: ownerOpenId, UserScopes: userScopes}, nil
155161
}
162+
163+
// classifyAppInfoErr re-decodes the raw body so BuildAPIError sees the
164+
// upstream `error` block — the typed appInfoResponse shape drops it.
165+
func classifyAppInfoErr(rawBody []byte, code int, msg string, f *cmdutil.Factory, appId string) error {
166+
var raw map[string]any
167+
_ = json.Unmarshal(rawBody, &raw)
168+
if raw == nil {
169+
raw = map[string]any{}
170+
}
171+
raw["code"] = code
172+
raw["msg"] = msg
173+
cc := errclass.ClassifyContext{Identity: string(core.AsBot)}
174+
if cfg, _ := f.Config(); cfg != nil {
175+
cc.Brand = string(cfg.Brand)
176+
cc.AppID = appId
177+
}
178+
return errclass.BuildAPIError(raw, cc)
179+
}

cmd/auth/auth_test.go

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"strings"
1313
"testing"
1414

15+
"github.com/larksuite/cli/errs"
1516
extcred "github.com/larksuite/cli/extension/credential"
1617
"github.com/larksuite/cli/internal/cmdutil"
1718
"github.com/larksuite/cli/internal/core"
@@ -318,6 +319,54 @@ func TestAuthScopesRun_UsesTenantAccessTokenFromCredentialProvider(t *testing.T)
318319
}
319320
}
320321

322+
// TestAuthScopesRun_LarkPermissionError_TypedAsPermissionError pins that when
323+
// the Lark API returns a permission code (99991679 with permission_violations),
324+
// getAppInfo classifies it as *errs.PermissionError carrying the server-
325+
// supplied MissingScopes — not a bare error wrapped as InternalError.
326+
func TestAuthScopesRun_LarkPermissionError_TypedAsPermissionError(t *testing.T) {
327+
f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{
328+
AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu,
329+
})
330+
tokenResolver := &authScopesTokenResolver{}
331+
f.Credential = credential.NewCredentialProvider(nil, nil, tokenResolver, nil)
332+
333+
reg.Register(&httpmock.Stub{
334+
Method: http.MethodGet,
335+
URL: "/open-apis/application/v6/applications/test-app",
336+
Body: map[string]interface{}{
337+
"code": 99991679,
338+
"msg": "scope missing",
339+
"error": map[string]interface{}{
340+
"permission_violations": []interface{}{
341+
map[string]interface{}{"subject": "application:application:self_manage"},
342+
},
343+
},
344+
},
345+
})
346+
347+
err := authScopesRun(&ScopesOptions{
348+
Factory: f,
349+
Ctx: context.Background(),
350+
Format: "json",
351+
})
352+
if err == nil {
353+
t.Fatal("expected error, got nil")
354+
}
355+
356+
var pe *errs.PermissionError
357+
if !errors.As(err, &pe) {
358+
t.Fatalf("expected *errs.PermissionError, got %T: %v", err, err)
359+
}
360+
if len(pe.MissingScopes) != 1 || pe.MissingScopes[0] != "application:application:self_manage" {
361+
t.Errorf("MissingScopes = %v, want server-supplied [application:application:self_manage]", pe.MissingScopes)
362+
}
363+
364+
var intErr *errs.InternalError
365+
if errors.As(err, &intErr) {
366+
t.Error("Lark business error must not be wrapped as InternalError; permission semantics lost")
367+
}
368+
}
369+
321370
type authScopesTokenResolver struct {
322371
requests []credential.TokenSpec
323372
}
@@ -389,15 +438,8 @@ func TestAuthBlockedByExternalProvider(t *testing.T) {
389438
if matched != nil && matched != cmd && !matched.SilenceUsage {
390439
t.Error("expected PersistentPreRunE to set SilenceUsage on matched subcommand")
391440
}
392-
var exitErr *output.ExitError
393-
if !errors.As(err, &exitErr) {
394-
t.Fatalf("expected *output.ExitError, got %T: %v", err, err)
395-
}
396-
if exitErr.Code != output.ExitValidation {
397-
t.Errorf("exit code = %d, want %d", exitErr.Code, output.ExitValidation)
398-
}
399-
if exitErr.Detail == nil || exitErr.Detail.Type != "external_provider" {
400-
t.Errorf("error type = %v, want %q", exitErr.Detail, "external_provider")
441+
if gotCode := output.ExitCodeOf(err); gotCode != output.ExitValidation {
442+
t.Errorf("exit code = %d, want %d", gotCode, output.ExitValidation)
401443
}
402444
})
403445
}

cmd/auth/check.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/spf13/cobra"
1111

12+
"github.com/larksuite/cli/errs"
1213
larkauth "github.com/larksuite/cli/internal/auth"
1314
"github.com/larksuite/cli/internal/cmdutil"
1415
"github.com/larksuite/cli/internal/output"
@@ -47,7 +48,7 @@ func authCheckRun(opts *CheckOptions) error {
4748

4849
required := strings.Fields(opts.Scope)
4950
if len(required) == 0 {
50-
return output.ErrValidation("--scope cannot be empty")
51+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--scope cannot be empty").WithParam("--scope")
5152
}
5253

5354
config, err := f.Config()

0 commit comments

Comments
 (0)