From 9a89fc1dc35a5236a5e4703122de135ff45f122b Mon Sep 17 00:00:00 2001 From: liangshuo-1 <266696938+liangshuo-1@users.noreply.github.com> Date: Thu, 30 Apr 2026 15:29:02 +0800 Subject: [PATCH] fix(config): make agent-binding hints workspace-aware and surface user-identity risks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AI agents running inside OpenClaw / Hermes were routinely creating a parallel app via `config init --new` instead of binding to the agent's existing app, because every "not configured" hint and several deny errors hard-coded `config init` regardless of workspace. Once bound, the same agents could silently grant themselves user identity (impersonation) without the user ever seeing a risk message in chat. Changes: - Introduce `core.NotConfiguredError` / `NoActiveProfileError` / `reconfigureHint` helpers that branch on `CurrentWorkspace()`. In agent workspaces they point at `lark-cli config bind --help` (a help page, not a ready-to-run command) so AI must read the binding workflow and confirm identity preset with the user before acting. In local terminals they preserve the previous `config init --new` guidance. - Migrate every `config init` hint that should be workspace-aware: RequireConfigForProfile, default credential provider, credential provider fallback, secret-resolve mismatch, config show, strict-mode entry-point errors, default-as, profile use/rename/remove, auth list, doctor's config_file check (which now also wraps the OS-level "no such file" noise into the user-shaped "not configured" message). - Refuse `config init` when run inside an OpenClaw / Hermes workspace by default; add `--force-init` for the rare case the user genuinely wants a parallel app. Without this guard, hint fixes were undone the moment AI ignored them. - Rewrite the strict-mode deny errors in cmd/auth/login.go, cmd/prune.go, and internal/cmdutil/factory.go. The previous "AI agents are strictly prohibited from modifying this setting" terminated AI reasoning while providing no real gate. New errors point at `config strict-mode --help` with the legitimate confirmation flow and explicitly note that switching does NOT require re-bind. Integration test envelopes updated. - Tighten `config bind --help` and `config strict-mode --help` to encode the user-confirmation discipline directly: identity preset semantics (bot-only vs user-default), "DO NOT switch without explicit user confirmation", and a cross-reference clarifying that `config bind` is for changing the underlying app while `config strict-mode` is the policy-only switch (resolves an ambiguity an audit run found). - Surface user-identity (impersonation) risk at every config write that newly grants it, by reusing the canonical IdentityEscalationMessage string from bind_messages.go: - `noticeUserDefaultRisk` fires on flag-mode bind landing on user-default, including the first-time case `warnIdentityEscalation` misses (it requires a previous bot lock). - `setStrictMode` warns when transitioning bot → user or bot → off (newly permits user identity); stays quiet on narrowing changes and on off → user (off already permitted user). - Add tests: notconfigured_test.go (workspace branches), init_guard_test.go (refuse + --force-init bypass), bind_warning_test.go (user-default warning fires; bot-only does not), strict_mode_warning_test.go (5 transitions covering both warn and no-warn paths). Two follow-ups intentionally deferred: the keychain master-key hint at internal/keychain/keychain.go:42 still suggests `config init` because the keychain package can't import core (would be circular); fixing requires either parameterizing the hint via callback or extracting workspace into its own package. The lark-shared skill doc still tells AI to run `config init` for first-time setup; updating the skill is in scope for a follow-up PR. Change-Id: I02273e044d9e061d211ceaa4f3ed5a3fb28325b3 --- cmd/auth/list.go | 14 +- cmd/auth/list_test.go | 59 +++++++ cmd/auth/login.go | 21 ++- cmd/auth/login_messages.go | 3 + cmd/auth/login_messages_test.go | 19 +++ cmd/config/bind.go | 49 +++++- cmd/config/bind_test.go | 30 +++- cmd/config/bind_warning_test.go | 62 +++++++ cmd/config/config_test.go | 14 +- cmd/config/default_as.go | 6 +- cmd/config/init.go | 41 ++++- cmd/config/init_guard_test.go | 69 ++++++++ cmd/config/show.go | 19 +-- cmd/config/strict_mode.go | 67 ++++++-- cmd/config/strict_mode_warning_test.go | 140 ++++++++++++++++ cmd/doctor/doctor.go | 16 +- cmd/profile/remove.go | 4 +- cmd/profile/rename.go | 4 +- cmd/profile/use.go | 4 +- cmd/prune.go | 8 +- cmd/root_integration_test.go | 21 ++- internal/cmdutil/factory.go | 7 +- internal/core/config.go | 2 +- internal/core/notconfigured.go | 120 ++++++++++++++ internal/core/notconfigured_test.go | 181 +++++++++++++++++++++ internal/core/secret_resolve.go | 5 +- internal/credential/credential_provider.go | 2 +- internal/credential/default_provider.go | 2 +- tests/cli_e2e/config/bind_test.go | 4 +- 29 files changed, 899 insertions(+), 94 deletions(-) create mode 100644 cmd/auth/list_test.go create mode 100644 cmd/config/bind_warning_test.go create mode 100644 cmd/config/init_guard_test.go create mode 100644 cmd/config/strict_mode_warning_test.go create mode 100644 internal/core/notconfigured.go create mode 100644 internal/core/notconfigured_test.go diff --git a/cmd/auth/list.go b/cmd/auth/list.go index f20c97fe8..2cb1b778e 100644 --- a/cmd/auth/list.go +++ b/cmd/auth/list.go @@ -4,6 +4,7 @@ package auth import ( + "errors" "fmt" "github.com/spf13/cobra" @@ -42,7 +43,18 @@ func authListRun(opts *ListOptions) error { multi, _ := core.LoadMultiAppConfig() if multi == nil || len(multi.Apps) == 0 { - fmt.Fprintln(f.IOStreams.ErrOut, "Not configured yet. Run `lark-cli config init` to initialize.") + // auth list is a read-only probe; the "configured but no users" + // branch below already returns exit 0 with a stderr hint, so we + // keep the same contract here. We still want the hint to be + // workspace-aware, so we pull the message+hint out of + // NotConfiguredError() instead of hard-coding it. + var cfgErr *core.ConfigError + if errors.As(core.NotConfiguredError(), &cfgErr) { + fmt.Fprintln(f.IOStreams.ErrOut, cfgErr.Message) + if cfgErr.Hint != "" { + fmt.Fprintln(f.IOStreams.ErrOut, " hint: "+cfgErr.Hint) + } + } return nil } diff --git a/cmd/auth/list_test.go b/cmd/auth/list_test.go new file mode 100644 index 000000000..e26266d64 --- /dev/null +++ b/cmd/auth/list_test.go @@ -0,0 +1,59 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package auth + +import ( + "strings" + "testing" + + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" +) + +// TestAuthListRun_NotConfigured_ReturnsExitZero pins the contract that +// `lark-cli auth list` is a read-only probe and must not fail-hard when no +// config exists yet — scripts and AI agents use it as an idempotent "do I +// have any users?" check, so the exit code carries semantic weight. Pair +// that with the existing "configured but no logged-in users" branch (also +// exit 0) and both empty states are consistent. +func TestAuthListRun_NotConfigured_ReturnsExitZero(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + + f, _, stderr, _ := cmdutil.TestFactory(t, nil) + if err := authListRun(&ListOptions{Factory: f}); err != nil { + t.Fatalf("auth list should succeed when not configured (exit 0); got: %v", err) + } + // Local workspace → hint must mention init, not bind. + out := stderr.String() + if !strings.Contains(out, "config init") { + t.Errorf("local hint missing config init: %s", out) + } + if strings.Contains(out, "config bind") { + t.Errorf("local hint must not mention config bind: %s", out) + } +} + +// TestAuthListRun_NotConfigured_AgentWorkspace_RoutesToBindHelp covers the +// reason this hint exists workspace-aware in the first place: an AI agent +// in OpenClaw / Hermes that probes auth list before binding gets routed to +// `config bind --help` instead of the local-only `config init`. +func TestAuthListRun_NotConfigured_AgentWorkspace_RoutesToBindHelp(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + + prev := core.CurrentWorkspace() + t.Cleanup(func() { core.SetCurrentWorkspace(prev) }) + core.SetCurrentWorkspace(core.WorkspaceOpenClaw) + + f, _, stderr, _ := cmdutil.TestFactory(t, nil) + if err := authListRun(&ListOptions{Factory: f}); err != nil { + t.Fatalf("auth list should still succeed under agent workspace; got: %v", err) + } + out := stderr.String() + if !strings.Contains(out, "config bind --help") { + t.Errorf("agent hint must point at config bind --help: %s", out) + } + if strings.Contains(out, "config init") { + t.Errorf("agent hint must not mention config init: %s", out) + } +} diff --git a/cmd/auth/login.go b/cmd/auth/login.go index add8762bb..aac1b4bc5 100644 --- a/cmd/auth/login.go +++ b/cmd/auth/login.go @@ -49,10 +49,9 @@ For AI agents: this command blocks until the user completes authorization in the browser. Run it in the background and retrieve the verification URL from its output.`, RunE: func(cmd *cobra.Command, args []string) error { if mode := f.ResolveStrictMode(cmd.Context()); mode == core.StrictModeBot { - return output.Errorf(output.ExitValidation, "strict_mode", - "strict mode is %q, user login is not allowed. "+ - "This setting is managed by the administrator and must not be modified by AI agents.", - mode) + return output.ErrWithHint(output.ExitValidation, "strict_mode", + fmt.Sprintf("strict mode is %q, user login is disabled in this profile", mode), + "if the user explicitly wants to switch to user identity, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)") } opts.Ctx = cmd.Context() if runF != nil { @@ -243,7 +242,11 @@ func authLoginRun(opts *LoginOptions) error { return nil } - // Step 2: Show user code and verification URL + // Step 2: Show user code and verification URL. + // Both branches surface AgentTimeoutHint, but on different channels: + // JSON mode embeds it as a structured field (so an agent that captures + // stdout into a JSON parser sees it without stream-mixing surprises), + // text mode prints to stderr (alongside the URL prompt). if opts.JSON { data := map[string]interface{}{ "event": "device_authorization", @@ -251,6 +254,7 @@ func authLoginRun(opts *LoginOptions) error { "verification_uri_complete": authResp.VerificationUriComplete, "user_code": authResp.UserCode, "expires_in": authResp.ExpiresIn, + "agent_hint": msg.AgentTimeoutHint, } encoder := json.NewEncoder(f.IOStreams.Out) encoder.SetEscapeHTML(false) @@ -260,6 +264,7 @@ func authLoginRun(opts *LoginOptions) error { } else { fmt.Fprintf(f.IOStreams.ErrOut, msg.OpenURL) fmt.Fprintf(f.IOStreams.ErrOut, " %s\n\n", authResp.VerificationUriComplete) + fmt.Fprintln(f.IOStreams.ErrOut, msg.AgentTimeoutHint) } // Step 3: Poll for token @@ -346,6 +351,12 @@ func authLoginPollDeviceCode(opts *LoginOptions, config *core.CliConfig, msg *lo fmt.Fprintf(f.IOStreams.ErrOut, "[lark-cli] [WARN] auth login: failed to remove cached requested scopes: %v\n", err) } } + // Skip the stderr hint in JSON mode — the --no-wait call that issued the + // device_code already returned the hint as a JSON field, and writing + // text to stderr would pollute consumers that combine streams via 2>&1. + if !opts.JSON { + fmt.Fprintln(f.IOStreams.ErrOut, msg.AgentTimeoutHint) + } log(msg.WaitingAuth) result := pollDeviceToken(opts.Ctx, httpClient, config.AppID, config.AppSecret, config.Brand, opts.DeviceCode, 5, 180, f.IOStreams.ErrOut) diff --git a/cmd/auth/login_messages.go b/cmd/auth/login_messages.go index 48a9e9f91..dd0f0402c 100644 --- a/cmd/auth/login_messages.go +++ b/cmd/auth/login_messages.go @@ -22,6 +22,7 @@ type loginMsg struct { // Non-interactive prompts (login.go) OpenURL string WaitingAuth string + AgentTimeoutHint string AuthSuccess string LoginSuccess string AuthorizedUser string @@ -58,6 +59,7 @@ var loginMsgZh = &loginMsg{ OpenURL: "在浏览器中打开以下链接进行认证:\n\n", WaitingAuth: "等待用户授权...", + AgentTimeoutHint: "[AI agent] 此命令最长阻塞约 10 分钟，等待用户在浏览器内完成授权。请确保 runner 的 timeout ≥ 600s；如不支持长 timeout，请改用 `lark-cli auth login --no-wait --json` 拿到 device_code 后再用 `lark-cli auth login --device-code ` 续上轮询，**不要短 timeout 反复重试**——每次重启会作废上一轮的 device code，导致用户授权的链接失效。", AuthSuccess: "已收到授权确认，正在获取用户信息并校验授权结果...", LoginSuccess: "授权成功! 用户: %s (%s)", AuthorizedUser: "当前授权账号: %s (%s)", @@ -93,6 +95,7 @@ var loginMsgEn = &loginMsg{ OpenURL: "Open this URL in your browser to authenticate:\n\n", WaitingAuth: "Waiting for user authorization...", + AgentTimeoutHint: "[AI agent] This command blocks for up to ~10 minutes while waiting for the user to authorize in their browser. Make sure your runner's timeout is ≥ 600s. If long timeouts are not supported, use `lark-cli auth login --no-wait --json` to get a device_code, then `lark-cli auth login --device-code ` to resume polling. **Do NOT retry with a short timeout** — each restart invalidates the previous device code, so any URL the user already authorized becomes useless.", AuthSuccess: "Authorization confirmed, fetching user info and validating granted scopes...", LoginSuccess: "Authorization successful! User: %s (%s)", AuthorizedUser: "Authorized account: %s (%s)", diff --git a/cmd/auth/login_messages_test.go b/cmd/auth/login_messages_test.go index 558876edb..9471f344b 100644 --- a/cmd/auth/login_messages_test.go +++ b/cmd/auth/login_messages_test.go @@ -6,6 +6,7 @@ package auth import ( "fmt" "reflect" + "strings" "testing" ) @@ -94,3 +95,21 @@ func TestLoginMsg_FormatStrings(t *testing.T) { } } } + +// TestAgentTimeoutHint_CarriesKeyInfo guards the contract that the synchronous +// auth-login output tells AI agents two things: (a) this command blocks for +// minutes — set a long runner timeout, and (b) the alternative is the +// --no-wait + --device-code split-flow. Without (a) AI sets a 10s timeout and +// kills the process before the user can authorize; without (b) the AI has no +// recovery path and just retries with the same short timeout, invalidating +// each new device code in turn. +func TestAgentTimeoutHint_CarriesKeyInfo(t *testing.T) { + for _, lang := range []string{"zh", "en"} { + hint := getLoginMsg(lang).AgentTimeoutHint + for _, want := range []string{"--no-wait", "--device-code"} { + if !strings.Contains(hint, want) { + t.Errorf("%s AgentTimeoutHint missing %q: %s", lang, want, hint) + } + } + } +} diff --git a/cmd/config/bind.go b/cmd/config/bind.go index b37d6d2a3..2494971f6 100644 --- a/cmd/config/bind.go +++ b/cmd/config/bind.go @@ -62,11 +62,32 @@ func NewCmdConfigBind(f *cmdutil.Factory, runF func(*BindOptions) error) *cobra. Short: "Bind Agent config to a workspace (source / app-id / force)", Long: `Bind an AI Agent's (OpenClaw / Hermes) Feishu credentials to a lark-cli workspace. -For AI agents: pass --source and --app-id to bind non-interactively. -Credentials are synced once; subsequent calls in the Agent's process -context automatically use the bound workspace.`, - Example: ` lark-cli config bind --source openclaw --app-id - lark-cli config bind --source hermes`, +--source is auto-detected from env (OPENCLAW_HOME / HERMES_HOME); pass it only to override. + +For AI agents — DO NOT bind without user confirmation. Binding may +overwrite an existing one and locks in an identity policy. Ask the user: + + --identity bot-only bot only (safer default; no impersonation; + cannot access user resources like personal + calendar / mail / drive) + --identity user-default user identity allowed (impersonates the user; + needed for personal-resource access) + +Default to bot-only if the user is unsure. Only run the command after +the user confirms both intent and identity preset. + +If lark-cli is already bound and the user only wants to change identity +policy on the SAME app, use 'config strict-mode' — that's the policy +switch and does not require re-bind. Use 'config bind' only when the +underlying app itself changes. + +Interactive terminal use: run with no flags to enter the TUI form.`, + Example: ` # AI flow: confirm intent + identity with user FIRST, then run: + lark-cli config bind --source openclaw --app-id --identity bot-only + lark-cli config bind --source hermes --identity user-default + + # Interactive (terminal user) — TUI prompts for everything: + lark-cli config bind`, RunE: func(cmd *cobra.Command, args []string) error { opts.langExplicit = cmd.Flags().Changed("lang") if runF != nil { @@ -125,6 +146,7 @@ func configBindRun(opts *BindOptions) error { return err } applyPreferences(appConfig, opts) + noticeUserDefaultRisk(opts) return commitBinding(opts, appConfig, existing.ConfigBytes, source, targetConfigPath) } @@ -308,6 +330,23 @@ func warnIdentityEscalation(opts *BindOptions, previousConfigBytes []byte) error msg.IdentityEscalationMessage, msg.IdentityEscalationHint) } +// noticeUserDefaultRisk surfaces the user-identity impersonation risk on every +// flag-mode bind that lands on user-default. The bot-only → user-default +// escalation is already covered by warnIdentityEscalation (errors out before +// applyPreferences runs), and the TUI flow shows IdentityUserDefaultDesc +// during identity selection — so this fires specifically for the case those +// two miss: a fresh flag-mode bind that goes directly to user-default with +// no previous bot lock to escalate from. Without this, AI agents finish such +// a bind with only a "配置成功" message and never relay to the user that the +// AI can now act under their identity. +func noticeUserDefaultRisk(opts *BindOptions) { + if opts.IsTUI || opts.Identity != "user-default" { + return + } + msg := getBindMsg(opts.Lang) + fmt.Fprintln(opts.Factory.IOStreams.ErrOut, "⚠️ "+msg.IdentityEscalationMessage) +} + // applyPreferences expands the chosen identity preset into the underlying // StrictMode + DefaultAs on the AppConfig. Always writes both fields so the // profile's intent survives later changes to global strict-mode settings. diff --git a/cmd/config/bind_test.go b/cmd/config/bind_test.go index bed11e408..800f8241f 100644 --- a/cmd/config/bind_test.go +++ b/cmd/config/bind_test.go @@ -377,16 +377,28 @@ func TestConfigShowRun_AgentWorkspaceNotBound(t *testing.T) { if err == nil { t.Fatal("expected error for unbound workspace") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("error type = %T, want *output.ExitError", err) + // Should be a structured ConfigError suggesting config bind, not config init. + var cfgErr *core.ConfigError + if !errors.As(err, &cfgErr) { + t.Fatalf("error type = %T, want *core.ConfigError", err) + } + if cfgErr.Code != output.ExitValidation { + t.Errorf("exit code = %d, want %d", cfgErr.Code, output.ExitValidation) + } + if cfgErr.Type != "openclaw" { + t.Errorf("type = %q, want %q", cfgErr.Type, "openclaw") + } + if !strings.Contains(cfgErr.Message, "openclaw context detected") { + t.Errorf("message missing 'openclaw context detected': %q", cfgErr.Message) + } + // Hint must point at config bind --help (NOT a ready-to-run bind command): + // AI must read the help and confirm identity preset with the user first. + if !strings.Contains(cfgErr.Hint, "config bind --help") { + t.Errorf("hint must point at `config bind --help`; got %q", cfgErr.Hint) + } + if strings.Contains(cfgErr.Hint, "config init") { + t.Errorf("agent hint must not mention config init; got %q", cfgErr.Hint) } - // Should suggest config bind, not config init - assertExitError(t, err, output.ExitValidation, output.ErrDetail{ - Type: "openclaw", - Message: "openclaw context detected but lark-cli not bound to openclaw workspace", - Hint: "run: lark-cli config bind --source openclaw", - }) } // ── Helper function tests (dotenv, brand, path resolution) ── diff --git a/cmd/config/bind_warning_test.go b/cmd/config/bind_warning_test.go new file mode 100644 index 000000000..fe5799619 --- /dev/null +++ b/cmd/config/bind_warning_test.go @@ -0,0 +1,62 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/larksuite/cli/internal/cmdutil" +) + +// runHermesBindWithIdentity boots a Hermes-shaped fake env, runs `config bind` +// with the given identity preset in flag (non-TUI) mode, and returns captured +// stderr. Hermes is the simplest source to fake (single .env file). +func runHermesBindWithIdentity(t *testing.T, identity string) string { + t.Helper() + saveWorkspace(t) + configDir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", configDir) + + hermesHome := t.TempDir() + t.Setenv("HERMES_HOME", hermesHome) + envContent := "FEISHU_APP_ID=cli_hermes_abc\nFEISHU_APP_SECRET=hermes_secret_123\nFEISHU_DOMAIN=lark\n" + if err := os.WriteFile(filepath.Join(hermesHome, ".env"), []byte(envContent), 0600); err != nil { + t.Fatalf("write .env: %v", err) + } + + f, _, stderr, _ := cmdutil.TestFactory(t, nil) + err := configBindRun(&BindOptions{ + Factory: f, + Source: "hermes", + Identity: identity, + Lang: "zh", + }) + if err != nil { + t.Fatalf("bind failed: %v", err) + } + return stderr.String() +} + +// TestConfigBindRun_UserDefaultIdentity_WarnsAboutImpersonation covers the +// gap that previously slipped through: a fresh flag-mode bind landing on +// user-default. warnIdentityEscalation requires a previous bot lock to fire, +// and IdentityUserDefaultDesc only renders in TUI selection — so without +// noticeUserDefaultRisk the user/AI never see the impersonation risk on a +// first-time user-default bind. +func TestConfigBindRun_UserDefaultIdentity_WarnsAboutImpersonation(t *testing.T) { + out := runHermesBindWithIdentity(t, "user-default") + if !strings.Contains(out, bindMsgZh.IdentityEscalationMessage) { + t.Errorf("user-default bind must surface IdentityEscalationMessage; got: %s", out) + } +} + +func TestConfigBindRun_BotOnlyIdentity_NoImpersonationWarning(t *testing.T) { + out := runHermesBindWithIdentity(t, "bot-only") + if strings.Contains(out, bindMsgZh.IdentityEscalationMessage) { + t.Errorf("bot-only bind must NOT warn about impersonation; got: %s", out) + } +} diff --git a/cmd/config/config_test.go b/cmd/config/config_test.go index 300865548..da734eb2c 100644 --- a/cmd/config/config_test.go +++ b/cmd/config/config_test.go @@ -90,15 +90,15 @@ func TestConfigShowRun_NotConfiguredReturnsStructuredError(t *testing.T) { t.Fatal("expected error") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("error type = %T, want *output.ExitError", err) + var cfgErr *core.ConfigError + if !errors.As(err, &cfgErr) { + t.Fatalf("error type = %T, want *core.ConfigError", err) } - if exitErr.Code != output.ExitValidation { - t.Fatalf("exit code = %d, want %d", exitErr.Code, output.ExitValidation) + if cfgErr.Code != output.ExitValidation { + t.Fatalf("exit code = %d, want %d", cfgErr.Code, output.ExitValidation) } - if exitErr.Detail == nil || exitErr.Detail.Type != "config" || exitErr.Detail.Message != "not configured" { - t.Fatalf("detail = %#v, want config/not configured", exitErr.Detail) + if cfgErr.Type != "config" || cfgErr.Message != "not configured" { + t.Fatalf("detail = %+v, want config/not configured", cfgErr) } } diff --git a/cmd/config/default_as.go b/cmd/config/default_as.go index 25bf824f1..1b590f5ad 100644 --- a/cmd/config/default_as.go +++ b/cmd/config/default_as.go @@ -20,14 +20,14 @@ func NewCmdConfigDefaultAs(f *cmdutil.Factory) *cobra.Command { Long: "Without arguments, shows the current default identity. Pass user, bot, or auto to set a new default.", Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - multi, err := core.LoadMultiAppConfig() + multi, err := core.LoadOrNotConfigured() if err != nil { - return output.ErrWithHint(output.ExitValidation, "config", "not configured", "run: lark-cli config init") + return err } app := multi.CurrentAppConfig(f.Invocation.Profile) if app == nil { - return output.ErrWithHint(output.ExitValidation, "config", "no active profile", "run: lark-cli config init") + return core.NoActiveProfileError() } if len(args) == 0 { diff --git a/cmd/config/init.go b/cmd/config/init.go index fa98861c9..ed77d1a76 100644 --- a/cmd/config/init.go +++ b/cmd/config/init.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "os" "strings" "github.com/charmbracelet/huh" @@ -33,6 +34,13 @@ type ConfigInitOptions struct { Lang string langExplicit bool // true when --lang was explicitly passed ProfileName string // when set, create/update a named profile instead of replacing Apps[0] + + // ForceInit overrides the agent-workspace guard. Without it, running + // init under OPENCLAW_HOME / HERMES_HOME refuses and points the caller + // at config bind — which is what AI agents almost always want. Manual + // users with a legitimate need for a separate app can pass --force-init + // to bypass. + ForceInit bool } // NewCmdConfigInit creates the config init subcommand. @@ -46,10 +54,18 @@ func NewCmdConfigInit(f *cmdutil.Factory, runF func(*ConfigInitOptions) error) * For AI agents: use --new to create a new app. The command blocks until the user completes setup in the browser. Run it in the background and retrieve the -verification URL from its output.`, +verification URL from its output. + +Inside an Agent context (OPENCLAW_HOME / HERMES_HOME set) this command +refuses by default — use 'lark-cli config bind' to bind to the Agent's +existing app instead of creating a parallel one. Pass --force-init only +if the user explicitly wants a separate app inside the Agent workspace.`, RunE: func(cmd *cobra.Command, args []string) error { opts.Ctx = cmd.Context() opts.langExplicit = cmd.Flags().Changed("lang") + if err := guardAgentWorkspace(opts); err != nil { + return err + } if runF != nil { return runF(opts) } @@ -63,10 +79,33 @@ verification URL from its output.`, cmd.Flags().StringVar(&opts.Brand, "brand", "feishu", "feishu or lark (non-interactive, default feishu)") cmd.Flags().StringVar(&opts.Lang, "lang", "zh", "language for interactive prompts (zh or en)") cmd.Flags().StringVar(&opts.ProfileName, "name", "", "create or update a named profile (append instead of replace)") + cmd.Flags().BoolVar(&opts.ForceInit, "force-init", false, "allow init inside an Agent workspace (OPENCLAW_HOME / HERMES_HOME); use config bind instead unless you really want a separate app") return cmd } +// guardAgentWorkspace refuses 'config init' when run inside an OpenClaw or +// Hermes Agent context, because the Agent has already provisioned an app +// and 'config bind' is the right tool for hooking lark-cli into it. +// Running init here would create a parallel app under the agent's workspace +// dir, breaking the binding the user actually wants. --force-init lets a +// human user override when they really do want a separate app. +func guardAgentWorkspace(opts *ConfigInitOptions) error { + if opts.ForceInit { + return nil + } + ws := core.DetectWorkspaceFromEnv(os.Getenv) + if ws.IsLocal() { + return nil + } + return &core.ConfigError{ + Code: 2, + Type: ws.Display(), + Message: fmt.Sprintf("config init is refused inside %s context (would create a parallel app and shadow the existing %s binding)", ws.Display(), ws.Display()), + Hint: "see `lark-cli config bind --help` to bind lark-cli to the Agent's existing app instead. Pass --force-init only if the user explicitly wants a separate app in this workspace.", + } +} + // hasAnyNonInteractiveFlag returns true if any non-interactive flag is set. func (o *ConfigInitOptions) hasAnyNonInteractiveFlag() bool { return o.New || o.AppID != "" || o.AppSecretStdin diff --git a/cmd/config/init_guard_test.go b/cmd/config/init_guard_test.go new file mode 100644 index 000000000..6e2ba1885 --- /dev/null +++ b/cmd/config/init_guard_test.go @@ -0,0 +1,69 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package config + +import ( + "errors" + "strings" + "testing" + + "github.com/larksuite/cli/internal/core" +) + +func TestGuardAgentWorkspace_LocalAllows(t *testing.T) { + t.Setenv("OPENCLAW_HOME", "") + t.Setenv("OPENCLAW_CLI", "") + t.Setenv("HERMES_HOME", "") + + if err := guardAgentWorkspace(&ConfigInitOptions{}); err != nil { + t.Errorf("local workspace should allow init, got: %v", err) + } +} + +func TestGuardAgentWorkspace_OpenClawRefuses(t *testing.T) { + t.Setenv("OPENCLAW_HOME", t.TempDir()) + + err := guardAgentWorkspace(&ConfigInitOptions{}) + if err == nil { + t.Fatal("expected refusal in OpenClaw context, got nil") + } + var cfgErr *core.ConfigError + if !errors.As(err, &cfgErr) { + t.Fatalf("error type = %T, want *core.ConfigError", err) + } + if cfgErr.Type != "openclaw" { + t.Errorf("type = %q, want %q", cfgErr.Type, "openclaw") + } + if !strings.Contains(cfgErr.Hint, "config bind --help") { + t.Errorf("hint must point to config bind --help; got %q", cfgErr.Hint) + } + if !strings.Contains(cfgErr.Hint, "--force-init") { + t.Errorf("hint must mention --force-init escape hatch; got %q", cfgErr.Hint) + } +} + +func TestGuardAgentWorkspace_HermesRefuses(t *testing.T) { + t.Setenv("HERMES_HOME", t.TempDir()) + + err := guardAgentWorkspace(&ConfigInitOptions{}) + if err == nil { + t.Fatal("expected refusal in Hermes context, got nil") + } + var cfgErr *core.ConfigError + if !errors.As(err, &cfgErr) { + t.Fatalf("error type = %T, want *core.ConfigError", err) + } + if cfgErr.Type != "hermes" { + t.Errorf("type = %q, want %q", cfgErr.Type, "hermes") + } +} + +func TestGuardAgentWorkspace_ForceInitOverride(t *testing.T) { + t.Setenv("OPENCLAW_HOME", t.TempDir()) + + // --force-init must let the user proceed even inside an Agent context. + if err := guardAgentWorkspace(&ConfigInitOptions{ForceInit: true}); err != nil { + t.Errorf("--force-init should bypass the guard, got: %v", err) + } +} diff --git a/cmd/config/show.go b/cmd/config/show.go index 7e3a19b7f..96018008f 100644 --- a/cmd/config/show.go +++ b/cmd/config/show.go @@ -44,12 +44,12 @@ func configShowRun(opts *ConfigShowOptions) error { config, err := core.LoadMultiAppConfig() if err != nil { if errors.Is(err, os.ErrNotExist) { - return notConfiguredError() + return core.NotConfiguredError() } return output.Errorf(output.ExitValidation, "config", "failed to load config: %v", err) } if config == nil || len(config.Apps) == 0 { - return output.ErrWithHint(output.ExitValidation, "config", "not configured", "run: lark-cli config init") + return core.NotConfiguredError() } app := config.CurrentAppConfig(f.Invocation.Profile) if app == nil { @@ -75,18 +75,3 @@ func configShowRun(opts *ConfigShowOptions) error { fmt.Fprintf(f.IOStreams.ErrOut, "\nConfig file path: %s\n", core.GetConfigPath()) return nil } - -// notConfiguredError returns the "not configured" error with a hint that -// points the user to the right next step: config init for the default local -// workspace, config bind for an Agent workspace that has not been bound yet. -func notConfiguredError() error { - ws := core.CurrentWorkspace() - if ws.IsLocal() { - return output.ErrWithHint(output.ExitValidation, "config", - "not configured", - "run: lark-cli config init") - } - return output.ErrWithHint(output.ExitValidation, ws.Display(), - fmt.Sprintf("%s context detected but lark-cli not bound to %s workspace", ws.Display(), ws.Display()), - fmt.Sprintf("run: lark-cli config bind --source %s", ws.Display())) -} diff --git a/cmd/config/strict_mode.go b/cmd/config/strict_mode.go index 09e81cde7..709010914 100644 --- a/cmd/config/strict_mode.go +++ b/cmd/config/strict_mode.go @@ -21,44 +21,44 @@ func NewCmdConfigStrictMode(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "strict-mode [bot|user|off]", Short: "View or set strict mode (identity restriction policy)", - Long: `View or set strict mode (identity restriction policy). + Long: `View or set strict mode — the identity restriction policy. -Without arguments, shows the current strict mode status and its source. -Pass "bot", "user", or "off" to set strict mode. -Use --global to set at the global level. -Use --reset to clear the profile-level setting (inherit global). + bot only bot identity allowed (user commands hidden) + user only user identity allowed (bot commands hidden) + off no restriction (default) -Modes: - bot — only bot identity is allowed, user commands are hidden - user — only user identity is allowed, bot commands are hidden - off — no restriction (default) +No args: show current mode. Switching does NOT require re-bind. -WARNING: Strict mode is a security policy set by the administrator. -AI agents are strictly prohibited from modifying this setting.`, +For AI agents: this is a security policy. DO NOT switch without +explicit user confirmation — never run on your own initiative.`, + Example: ` lark-cli config strict-mode # show current + lark-cli config strict-mode user # switch (after user confirms) + lark-cli config strict-mode bot --global # set globally + lark-cli config strict-mode --reset # clear profile override`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - multi, err := core.LoadMultiAppConfig() + multi, err := core.LoadOrNotConfigured() if err != nil { - return output.ErrWithHint(output.ExitValidation, "config", "not configured", "run: lark-cli config init") + return err } if reset { app := multi.CurrentAppConfig(f.Invocation.Profile) if app == nil { - return output.ErrWithHint(output.ExitValidation, "config", "no active profile", "run: lark-cli config init") + return core.NoActiveProfileError() } return resetStrictMode(f, multi, app, global, args) } if len(args) == 0 { app := multi.CurrentAppConfig(f.Invocation.Profile) if app == nil { - return output.ErrWithHint(output.ExitValidation, "config", "no active profile", "run: lark-cli config init") + return core.NoActiveProfileError() } return showStrictMode(cmd.Context(), f, multi, app) } app := multi.CurrentAppConfig(f.Invocation.Profile) if !global && app == nil { - return output.ErrWithHint(output.ExitValidation, "config", "no active profile", "run: lark-cli config init") + return core.NoActiveProfileError() } return setStrictMode(f, multi, app, args[0], global) }, @@ -106,6 +106,24 @@ func setStrictMode(f *cmdutil.Factory, multi *core.MultiAppConfig, app *core.App return output.ErrValidation("invalid value %q, valid values: bot | user | off", value) } + // Capture the old mode at the SAME scope being changed, so we can warn + // only when the policy actually expands user-identity at that scope. + // --global → compare raw multi.StrictMode (profiles with explicit + // overrides are unaffected; their warning comes from the existing + // "profile %q has strict-mode explicitly set" notice below). + // profile → compare effective mode (override > global > default), so + // a profile flipping from inherited bot to explicit off still warns. + // The previous version always used the profile's effective mode, which + // false-positived (--global change while current profile has an explicit + // override) and false-negatived (--global broadening that doesn't affect + // the current profile but does affect other inheriting profiles). + var oldMode core.StrictMode + if global { + oldMode = multi.StrictMode + } else { + oldMode, _ = resolveStrictModeStatus(multi, app) + } + if global { multi.StrictMode = mode for _, a := range multi.Apps { @@ -119,7 +137,7 @@ func setStrictMode(f *cmdutil.Factory, multi *core.MultiAppConfig, app *core.App } } else { if app == nil { - return output.ErrWithHint(output.ExitValidation, "config", "no active profile", "run: lark-cli config init") + return core.NoActiveProfileError() } app.StrictMode = &mode } @@ -127,6 +145,11 @@ func setStrictMode(f *cmdutil.Factory, multi *core.MultiAppConfig, app *core.App if err := core.SaveMultiAppConfig(multi); err != nil { return output.Errorf(output.ExitInternal, "internal", "failed to save config: %v", err) } + + if oldMode == core.StrictModeBot && (mode == core.StrictModeUser || mode == core.StrictModeOff) { + fmt.Fprintln(f.IOStreams.ErrOut, "⚠️ "+strictModeRelaxLang(app).IdentityEscalationMessage) + } + scope := "profile" if global { scope = "global" @@ -135,6 +158,16 @@ func setStrictMode(f *cmdutil.Factory, multi *core.MultiAppConfig, app *core.App return nil } +// strictModeRelaxLang picks the bind-message bundle whose language matches the +// active profile's Lang setting. Falls back to bindMsgZh when no profile is +// available (global mutation with no current app). +func strictModeRelaxLang(app *core.AppConfig) *bindMsg { + if app != nil { + return getBindMsg(app.Lang) + } + return getBindMsg("") +} + func resolveStrictModeStatus(multi *core.MultiAppConfig, app *core.AppConfig) (core.StrictMode, string) { if app != nil && app.StrictMode != nil { return *app.StrictMode, fmt.Sprintf("profile %q", app.ProfileName()) diff --git a/cmd/config/strict_mode_warning_test.go b/cmd/config/strict_mode_warning_test.go new file mode 100644 index 000000000..62e7324ae --- /dev/null +++ b/cmd/config/strict_mode_warning_test.go @@ -0,0 +1,140 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package config + +import ( + "strings" + "testing" + + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" +) + +// runStrictMode is a small helper that runs `config strict-mode ` and +// returns the captured stderr — that's where success-path messages and the +// new user-identity warning land. +func runStrictMode(t *testing.T, args ...string) string { + t.Helper() + f, _, stderr, _ := cmdutil.TestFactory(t, &core.CliConfig{AppID: "test-app", AppSecret: "secret"}) + cmd := NewCmdConfigStrictMode(f) + cmd.SetArgs(args) + if err := cmd.Execute(); err != nil { + t.Fatalf("strict-mode %v failed: %v", args, err) + } + return stderr.String() +} + +// expandsUserIdentity covers the only two transitions where AI gains the +// ability to act under the user's identity, and asserts the warning fires. +// Reuses bind_messages.go's IdentityEscalationMessage as the canonical text +// so all three call sites (bind upgrade, fresh user-default bind, strict-mode +// relax) stay phrased identically. +func TestStrictMode_BotToUser_WarnsAboutIdentityRisk(t *testing.T) { + setupStrictModeTestConfig(t) + runStrictMode(t, "bot") + + out := runStrictMode(t, "user") + if !strings.Contains(out, bindMsgZh.IdentityEscalationMessage) { + t.Errorf("bot→user transition must surface IdentityEscalationMessage; got: %s", out) + } +} + +func TestStrictMode_BotToOff_WarnsAboutIdentityRisk(t *testing.T) { + setupStrictModeTestConfig(t) + runStrictMode(t, "bot") + + out := runStrictMode(t, "off") + if !strings.Contains(out, bindMsgZh.IdentityEscalationMessage) { + t.Errorf("bot→off transition must surface IdentityEscalationMessage; got: %s", out) + } +} + +// narrowingDoesNotWarn covers the cases that revoke or keep user-identity +// scope — those should stay quiet, otherwise AI will spam users with risk +// text on every restrictive change. +func TestStrictMode_UserToBot_NoWarning(t *testing.T) { + setupStrictModeTestConfig(t) + runStrictMode(t, "user") + + out := runStrictMode(t, "bot") + if strings.Contains(out, bindMsgZh.IdentityEscalationMessage) { + t.Errorf("user→bot is a narrowing change; must not warn. got: %s", out) + } +} + +func TestStrictMode_OffToBot_NoWarning(t *testing.T) { + setupStrictModeTestConfig(t) + // Default starts at off; explicitly set bot — narrowing. + out := runStrictMode(t, "bot") + if strings.Contains(out, bindMsgZh.IdentityEscalationMessage) { + t.Errorf("off→bot is a narrowing change; must not warn. got: %s", out) + } +} + +func TestStrictMode_OffToUser_NoWarning(t *testing.T) { + // Off already permits user-identity, so off→user is not a NEW grant + // even though it forces user identity. Don't warn. + setupStrictModeTestConfig(t) + out := runStrictMode(t, "user") + if strings.Contains(out, bindMsgZh.IdentityEscalationMessage) { + t.Errorf("off→user does not newly permit user identity; must not warn. got: %s", out) + } +} + +// --- --global path: comparison must use multi.StrictMode, not profile's +// effective mode. The previous (buggy) version used resolveStrictModeStatus +// here too, leading to both false positives (current profile has explicit +// override unaffected by --global → still warned) and false negatives +// (current profile has explicit override that masks an actual bot → off +// global broadening for OTHER inheriting profiles → didn't warn). + +func TestStrictMode_GlobalBotToUser_Warns(t *testing.T) { + setupStrictModeTestConfig(t) + runStrictMode(t, "bot", "--global") + + out := runStrictMode(t, "user", "--global") + if !strings.Contains(out, bindMsgZh.IdentityEscalationMessage) { + t.Errorf("global bot→user must warn (broadens user-identity for inheriting profiles); got: %s", out) + } +} + +func TestStrictMode_GlobalBotToOff_Warns(t *testing.T) { + setupStrictModeTestConfig(t) + runStrictMode(t, "bot", "--global") + + out := runStrictMode(t, "off", "--global") + if !strings.Contains(out, bindMsgZh.IdentityEscalationMessage) { + t.Errorf("global bot→off must warn (newly permits user identity in inheriting profiles); got: %s", out) + } +} + +// FalsePositive: current profile has explicit "bot" override, global goes +// off → user. The current profile is unaffected (still bot via override), +// and off→user at the global level is not a new grant either. Must not warn. +func TestStrictMode_GlobalOffToUser_WithProfileBotOverride_NoWarning(t *testing.T) { + setupStrictModeTestConfig(t) + runStrictMode(t, "bot") // profile-level explicit bot + runStrictMode(t, "off", "--global") // global = off + + out := runStrictMode(t, "user", "--global") + if strings.Contains(out, bindMsgZh.IdentityEscalationMessage) { + t.Errorf("global off→user with profile-bot-override must not warn (profile unaffected, global wasn't bot); got: %s", out) + } +} + +// FalseNegative: global = bot, current profile has explicit "off" override. +// Running --global off broadens OTHER inheriting profiles (bot → off). The +// current profile doesn't change effective mode, but the policy still expanded +// user-identity, so warning must fire. The pre-fix logic compared via the +// current profile's effective mode and missed this case. +func TestStrictMode_GlobalBotToOff_WithProfileOffOverride_Warns(t *testing.T) { + setupStrictModeTestConfig(t) + runStrictMode(t, "bot", "--global") // global = bot + runStrictMode(t, "off") // profile-level explicit off (already shows the warning at profile scope) + + out := runStrictMode(t, "off", "--global") + if !strings.Contains(out, bindMsgZh.IdentityEscalationMessage) { + t.Errorf("global bot→off must warn even when current profile has explicit off (other profiles inherit and newly permit user identity); got: %s", out) + } +} diff --git a/cmd/doctor/doctor.go b/cmd/doctor/doctor.go index 4b53aceb9..0f569bab0 100644 --- a/cmd/doctor/doctor.go +++ b/cmd/doctor/doctor.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "net/http" + "os" "sync" "time" @@ -83,7 +84,20 @@ func doctorRun(opts *DoctorOptions) error { // ── 1. Config file ── _, err := core.LoadMultiAppConfig() if err != nil { - checks = append(checks, fail("config_file", err.Error(), "run: lark-cli config init")) + // For "config not present" cases, prefer the workspace-aware + // NotConfiguredError message + hint (e.g. "openclaw context + // detected but lark-cli is not bound to it" → bind --help) over + // the OS-level "open ... no such file or directory". + // For other errors (parse, perms), keep the raw error so the + // underlying problem is still visible. + msg, hint := err.Error(), "" + if errors.Is(err, os.ErrNotExist) { + var cfgErr *core.ConfigError + if errors.As(core.NotConfiguredError(), &cfgErr) { + msg, hint = cfgErr.Message, cfgErr.Hint + } + } + checks = append(checks, fail("config_file", msg, hint)) return finishDoctor(f, checks) } checks = append(checks, pass("config_file", "config.json found")) diff --git a/cmd/profile/remove.go b/cmd/profile/remove.go index 00599c0da..124c32e58 100644 --- a/cmd/profile/remove.go +++ b/cmd/profile/remove.go @@ -32,9 +32,9 @@ func NewCmdProfileRemove(f *cmdutil.Factory) *cobra.Command { } func profileRemoveRun(f *cmdutil.Factory, name string) error { - multi, err := core.LoadMultiAppConfig() + multi, err := core.LoadOrNotConfigured() if err != nil { - return output.ErrWithHint(output.ExitValidation, "config", "not configured", "run: lark-cli config init") + return err } idx := multi.FindAppIndex(name) diff --git a/cmd/profile/rename.go b/cmd/profile/rename.go index e86b569c5..37fbc787a 100644 --- a/cmd/profile/rename.go +++ b/cmd/profile/rename.go @@ -32,9 +32,9 @@ func profileRenameRun(f *cmdutil.Factory, oldName, newName string) error { return output.ErrValidation("%v", err) } - multi, err := core.LoadMultiAppConfig() + multi, err := core.LoadOrNotConfigured() if err != nil { - return output.ErrWithHint(output.ExitValidation, "config", "not configured", "run: lark-cli config init") + return err } idx := multi.FindAppIndex(oldName) diff --git a/cmd/profile/use.go b/cmd/profile/use.go index f73a47be4..de0964d7e 100644 --- a/cmd/profile/use.go +++ b/cmd/profile/use.go @@ -31,9 +31,9 @@ func NewCmdProfileUse(f *cmdutil.Factory) *cobra.Command { } func profileUseRun(f *cmdutil.Factory, name string) error { - multi, err := core.LoadMultiAppConfig() + multi, err := core.LoadOrNotConfigured() if err != nil { - return output.ErrWithHint(output.ExitValidation, "config", "not configured", "run: lark-cli config init") + return err } // Handle "-" for toggle-back diff --git a/cmd/prune.go b/cmd/prune.go index 6ae18a709..1a3f05f52 100644 --- a/cmd/prune.go +++ b/cmd/prune.go @@ -4,6 +4,7 @@ package cmd import ( + "fmt" "slices" "github.com/larksuite/cli/internal/cmdutil" @@ -48,10 +49,9 @@ func strictModeStubFrom(child *cobra.Command, mode core.StrictMode) *cobra.Comma Hidden: true, DisableFlagParsing: true, RunE: func(cmd *cobra.Command, args []string) error { - return output.Errorf(output.ExitValidation, "strict_mode", - "strict mode is %q, only %s identity is allowed. "+ - "This setting is managed by the administrator and must not be modified by AI agents.", - mode, mode.ForcedIdentity()) + return output.ErrWithHint(output.ExitValidation, "strict_mode", + fmt.Sprintf("strict mode is %q, only %s-identity commands are available", mode, mode.ForcedIdentity()), + "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)") }, } } diff --git a/cmd/root_integration_test.go b/cmd/root_integration_test.go index c5c6a1d75..affadafe4 100644 --- a/cmd/root_integration_test.go +++ b/cmd/root_integration_test.go @@ -343,11 +343,15 @@ func TestIntegration_StrictModeBot_ProfileOverride_DirectAuthLoginReturnsEnvelop "auth", "login", "--json", "--scope", "im:message.send_as_user", }) + // auth login is user-only, so it gets pruned in strict-mode-bot and the + // stub error fires (not login.go's inline check, which is shadowed by + // pruning). assertEnvelope(t, code, output.ExitValidation, stdout, stderr, output.ErrorEnvelope{ OK: false, Error: &output.ErrDetail{ Type: "strict_mode", - Message: `strict mode is "bot", only bot identity is allowed. This setting is managed by the administrator and must not be modified by AI agents.`, + Message: `strict mode is "bot", only bot-identity commands are available`, + Hint: "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)", }, }) } @@ -364,7 +368,8 @@ func TestIntegration_StrictModeBot_ProfileOverride_DirectUserShortcutReturnsEnve OK: false, Error: &output.ErrDetail{ Type: "strict_mode", - Message: `strict mode is "bot", only bot identity is allowed. This setting is managed by the administrator and must not be modified by AI agents.`, + Message: `strict mode is "bot", only bot-identity commands are available`, + Hint: "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)", }, }) } @@ -401,7 +406,8 @@ func TestIntegration_StrictModeUser_ProfileOverride_ShortcutExplicitBotReturnsEn Identity: "bot", Error: &output.ErrDetail{ Type: "strict_mode", - Message: `strict mode is "user", only user identity is allowed. This setting is managed by the administrator and must not be modified by AI agents.`, + Message: `strict mode is "user", only user-identity commands are available`, + Hint: "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)", }, }) } @@ -419,7 +425,8 @@ func TestIntegration_StrictModeBot_ProfileOverride_ServiceExplicitUserReturnsEnv Identity: "user", Error: &output.ErrDetail{ Type: "strict_mode", - Message: `strict mode is "bot", only bot identity is allowed. This setting is managed by the administrator and must not be modified by AI agents.`, + Message: `strict mode is "bot", only bot-identity commands are available`, + Hint: "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)", }, }) } @@ -436,7 +443,8 @@ func TestIntegration_StrictModeUser_ProfileOverride_ServiceBotOnlyMethodReturnsE OK: false, Error: &output.ErrDetail{ Type: "strict_mode", - Message: `strict mode is "user", only user identity is allowed. This setting is managed by the administrator and must not be modified by AI agents.`, + Message: `strict mode is "user", only user-identity commands are available`, + Hint: "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)", }, }) } @@ -454,7 +462,8 @@ func TestIntegration_StrictModeBot_ProfileOverride_APIExplicitUserReturnsEnvelop Identity: "user", Error: &output.ErrDetail{ Type: "strict_mode", - Message: `strict mode is "bot", only bot identity is allowed. This setting is managed by the administrator and must not be modified by AI agents.`, + Message: `strict mode is "bot", only bot-identity commands are available`, + Hint: "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)", }, }) } diff --git a/internal/cmdutil/factory.go b/internal/cmdutil/factory.go index 3f8ced799..e0669fc7a 100644 --- a/internal/cmdutil/factory.go +++ b/internal/cmdutil/factory.go @@ -160,10 +160,9 @@ func (f *Factory) ResolveStrictMode(ctx context.Context) core.StrictMode { func (f *Factory) CheckStrictMode(ctx context.Context, as core.Identity) error { mode := f.ResolveStrictMode(ctx) if mode.IsActive() && !mode.AllowsIdentity(as) { - return output.Errorf(output.ExitValidation, "strict_mode", - "strict mode is %q, only %s identity is allowed. "+ - "This setting is managed by the administrator and must not be modified by AI agents.", - mode, mode.ForcedIdentity()) + return output.ErrWithHint(output.ExitValidation, "strict_mode", + fmt.Sprintf("strict mode is %q, only %s-identity commands are available", mode, mode.ForcedIdentity()), + "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)") } return nil } diff --git a/internal/core/config.go b/internal/core/config.go index ca27a3b39..eff57c762 100644 --- a/internal/core/config.go +++ b/internal/core/config.go @@ -225,7 +225,7 @@ func RequireConfig(kc keychain.KeychainAccess) (*CliConfig, error) { func RequireConfigForProfile(kc keychain.KeychainAccess, profileOverride string) (*CliConfig, error) { raw, err := LoadMultiAppConfig() if err != nil || raw == nil || len(raw.Apps) == 0 { - return nil, &ConfigError{Code: 2, Type: "config", Message: "not configured", Hint: "run `lark-cli config init --new` in the background. It blocks and outputs a verification URL — retrieve the URL and open it in a browser to complete setup."} + return nil, NotConfiguredError() } return ResolveConfigFromMulti(raw, kc, profileOverride) } diff --git a/internal/core/notconfigured.go b/internal/core/notconfigured.go new file mode 100644 index 000000000..cf76e60a9 --- /dev/null +++ b/internal/core/notconfigured.go @@ -0,0 +1,120 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package core + +import ( + "errors" + "fmt" + "os" +) + +// LoadOrNotConfigured wraps LoadMultiAppConfig with the standard "not yet +// configured vs. couldn't read" disambiguation that every config-required +// command should use: +// +// - file missing → workspace-aware NotConfiguredError (init / bind hint) +// - parse error / permission error → real load failure with the original +// cause preserved, so the user can actually fix the broken file +// +// Without this, every call site that did `if err != nil { return +// NotConfiguredError() }` silently coerced corrupt-config into "run init", +// which sent users in circles when their config.json was just malformed. +func LoadOrNotConfigured() (*MultiAppConfig, error) { + multi, err := LoadMultiAppConfig() + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, NotConfiguredError() + } + // Surface the real cause (parse error, permission denied, etc.) + // so the user can fix the broken file. Wrapping as ConfigError + // keeps it on the standard structured-envelope path at the root + // command's error sink. + return nil, &ConfigError{ + Code: 2, + Type: "config", + Message: fmt.Sprintf("failed to load config: %v", err), + } + } + if multi == nil || len(multi.Apps) == 0 { + return nil, NotConfiguredError() + } + return multi, nil +} + +const ( + // localInitHint is the canonical "you're in a regular terminal, run + // init" guidance — shared by NotConfiguredError and NoActiveProfileError + // so the same session can't show two different recommended commands. + localInitHint = "run `lark-cli config init --new` in the background. It blocks and outputs a verification URL — retrieve the URL and open it in a browser to complete setup." + + // agentBindHint is the canonical "you're in an Agent workspace, see + // the binding workflow" guidance. Always points at --help (never a + // ready-to-run bind command) so the AI reads the confirmation + // discipline (identity preset, user opt-in) before acting. + agentBindHint = "read `lark-cli config bind --help`, then ask the user to confirm intent and identity preset (bot-only or user-default); only after both are confirmed, run `lark-cli config bind`" +) + +// NotConfiguredError returns the canonical "not configured" error, with a +// hint that depends on the active workspace: +// +// - WorkspaceLocal → suggest `config init --new` (creates a new app). +// - WorkspaceOpenClaw / WorkspaceHermes → point at `config bind --help` +// rather than a ready-to-run command, because binding is policy-laden: +// the user must pick an identity preset (bot-only vs user-default), +// and re-binding may overwrite an existing one. The help text walks +// the AI through the confirmation flow. +// +// All "config not loaded yet" call sites should use this helper rather than +// hand-rolling a hint, so AI agents always get a workspace-correct next step. +func NotConfiguredError() error { + ws := CurrentWorkspace() + if ws.IsLocal() { + return &ConfigError{ + Code: 2, + Type: "config", + Message: "not configured", + Hint: localInitHint, + } + } + return &ConfigError{ + Code: 2, + Type: ws.Display(), + Message: fmt.Sprintf("%s context detected but lark-cli is not bound to it", ws.Display()), + Hint: agentBindHint, + } +} + +// reconfigureHint returns the workspace-aware "fix it from scratch" hint +// used by error paths that aren't full ConfigErrors (e.g. plain fmt.Errorf +// strings from keychain / secret validation). Local → `config init`; +// Agent → `config bind --help` so the AI reads the binding workflow and +// confirms identity preset with the user before running the actual command. +func reconfigureHint() string { + if CurrentWorkspace().IsLocal() { + return "please run `lark-cli config init` to reconfigure" + } + return agentBindHint +} + +// NoActiveProfileError mirrors NotConfiguredError for the related +// "config exists but the requested profile cannot be resolved" case. In agent +// workspaces a missing profile typically means the binding was wiped while +// the workspace marker remained — re-binding is the correct fix, not init. +func NoActiveProfileError() error { + ws := CurrentWorkspace() + if ws.IsLocal() { + return &ConfigError{ + Code: 2, + Type: "config", + Message: "no active profile", + Hint: localInitHint, + } + } + return &ConfigError{ + Code: 2, + Type: ws.Display(), + Message: fmt.Sprintf("no active profile in %s workspace", ws.Display()), + Hint: agentBindHint, + } +} diff --git a/internal/core/notconfigured_test.go b/internal/core/notconfigured_test.go new file mode 100644 index 000000000..a65e3a270 --- /dev/null +++ b/internal/core/notconfigured_test.go @@ -0,0 +1,181 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package core + +import ( + "errors" + "os" + "strings" + "testing" +) + +// saveAndRestoreWorkspace ensures package-level currentWorkspace is reset +// between subtests so cross-test pollution can't make assertions pass by +// accident. +func saveAndRestoreWorkspace(t *testing.T) { + t.Helper() + prev := CurrentWorkspace() + t.Cleanup(func() { SetCurrentWorkspace(prev) }) +} + +func TestNotConfiguredError_Local(t *testing.T) { + saveAndRestoreWorkspace(t) + SetCurrentWorkspace(WorkspaceLocal) + + err := NotConfiguredError() + var cfgErr *ConfigError + if !errors.As(err, &cfgErr) { + t.Fatalf("error type = %T, want *ConfigError", err) + } + if cfgErr.Type != "config" || cfgErr.Message != "not configured" { + t.Errorf("unexpected detail: %+v", cfgErr) + } + if !strings.Contains(cfgErr.Hint, "config init --new") { + t.Errorf("local hint should suggest config init --new; got %q", cfgErr.Hint) + } + if strings.Contains(cfgErr.Hint, "config bind") { + t.Errorf("local hint must not mention config bind; got %q", cfgErr.Hint) + } +} + +func TestNotConfiguredError_OpenClaw(t *testing.T) { + saveAndRestoreWorkspace(t) + SetCurrentWorkspace(WorkspaceOpenClaw) + + err := NotConfiguredError() + var cfgErr *ConfigError + if !errors.As(err, &cfgErr) { + t.Fatalf("error type = %T, want *ConfigError", err) + } + if cfgErr.Type != "openclaw" { + t.Errorf("type = %q, want %q", cfgErr.Type, "openclaw") + } + // Hint must point at --help (read first, confirm with user, then bind), + // NOT a directly-executable bind command — binding is policy-laden + // (identity preset, may overwrite existing binding). + if !strings.Contains(cfgErr.Hint, "config bind --help") { + t.Errorf("agent hint must point to `config bind --help`; got %q", cfgErr.Hint) + } + if strings.Contains(cfgErr.Hint, "config init") { + t.Errorf("agent hint must NOT mention config init (would cause AI to create a new app); got %q", cfgErr.Hint) + } +} + +func TestNotConfiguredError_Hermes(t *testing.T) { + saveAndRestoreWorkspace(t) + SetCurrentWorkspace(WorkspaceHermes) + + err := NotConfiguredError() + var cfgErr *ConfigError + if !errors.As(err, &cfgErr) { + t.Fatalf("error type = %T, want *ConfigError", err) + } + if cfgErr.Type != "hermes" { + t.Errorf("type = %q, want %q", cfgErr.Type, "hermes") + } + if !strings.Contains(cfgErr.Hint, "config bind --help") { + t.Errorf("hermes hint must point to `config bind --help`; got %q", cfgErr.Hint) + } +} + +func TestNoActiveProfileError_Local(t *testing.T) { + saveAndRestoreWorkspace(t) + SetCurrentWorkspace(WorkspaceLocal) + + err := NoActiveProfileError() + var cfgErr *ConfigError + if !errors.As(err, &cfgErr) { + t.Fatalf("error type = %T, want *ConfigError", err) + } + if cfgErr.Message != "no active profile" { + t.Errorf("message = %q, want %q", cfgErr.Message, "no active profile") + } +} + +func TestNoActiveProfileError_AgentSuggestsBind(t *testing.T) { + saveAndRestoreWorkspace(t) + SetCurrentWorkspace(WorkspaceOpenClaw) + + err := NoActiveProfileError() + var cfgErr *ConfigError + if !errors.As(err, &cfgErr) { + t.Fatalf("error type = %T, want *ConfigError", err) + } + if !strings.Contains(cfgErr.Hint, "config bind --help") { + t.Errorf("agent hint must point to `config bind --help`; got %q", cfgErr.Hint) + } +} + +func TestReconfigureHint_Local(t *testing.T) { + saveAndRestoreWorkspace(t) + SetCurrentWorkspace(WorkspaceLocal) + + got := reconfigureHint() + if !strings.Contains(got, "config init") { + t.Errorf("local reconfigure hint must mention config init; got %q", got) + } +} + +func TestReconfigureHint_Agent(t *testing.T) { + saveAndRestoreWorkspace(t) + SetCurrentWorkspace(WorkspaceHermes) + + got := reconfigureHint() + if !strings.Contains(got, "config bind --help") { + t.Errorf("agent reconfigure hint must point to `config bind --help`; got %q", got) + } +} + +func TestLoadOrNotConfigured_FileMissing_ReturnsNotConfigured(t *testing.T) { + saveAndRestoreWorkspace(t) + SetCurrentWorkspace(WorkspaceLocal) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + + _, err := LoadOrNotConfigured() + if err == nil { + t.Fatal("expected error") + } + var cfgErr *ConfigError + if !errors.As(err, &cfgErr) { + t.Fatalf("error type = %T, want *ConfigError", err) + } + if cfgErr.Message != "not configured" { + t.Errorf("message = %q, want \"not configured\"", cfgErr.Message) + } + if !strings.Contains(cfgErr.Hint, "config init --new") { + t.Errorf("missing-file in local must hint `config init --new`; got %q", cfgErr.Hint) + } +} + +// TestLoadOrNotConfigured_CorruptFile_PreservesCause is the regression guard +// for the previous "every load error → not configured" coercion: a malformed +// config.json must surface its real failure cause so the user can fix it, +// not get sent in circles by an init/bind hint that wouldn't help here. +func TestLoadOrNotConfigured_CorruptFile_PreservesCause(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + // Write garbage that will fail JSON parsing. + if err := os.WriteFile(dir+"/config.json", []byte("{not valid json"), 0600); err != nil { + t.Fatal(err) + } + + _, err := LoadOrNotConfigured() + if err == nil { + t.Fatal("expected error for corrupt config") + } + var cfgErr *ConfigError + if !errors.As(err, &cfgErr) { + t.Fatalf("error type = %T, want *ConfigError", err) + } + if !strings.Contains(cfgErr.Message, "failed to load config") { + t.Errorf("corrupt-file message must say 'failed to load config'; got %q", cfgErr.Message) + } + // And it must NOT pretend the user just hasn't initialised yet. + if cfgErr.Message == "not configured" { + t.Errorf("corrupt-file must not be coerced to 'not configured'") + } + if strings.Contains(cfgErr.Hint, "config init") || strings.Contains(cfgErr.Hint, "config bind") { + t.Errorf("corrupt-file hint must not redirect to init/bind; got %q", cfgErr.Hint) + } +} diff --git a/internal/core/secret_resolve.go b/internal/core/secret_resolve.go index 337360bde..072aa2791 100644 --- a/internal/core/secret_resolve.go +++ b/internal/core/secret_resolve.go @@ -63,9 +63,8 @@ func ValidateSecretKeyMatch(appId string, secret SecretInput) error { expected := secretAccountKey(appId) if secret.Ref.ID != expected { return fmt.Errorf( - "appSecret keychain key %q does not match appId %q (expected %q); "+ - "please run `lark-cli config init` to reconfigure", - secret.Ref.ID, appId, expected, + "appSecret keychain key %q does not match appId %q (expected %q); %s", + secret.Ref.ID, appId, expected, reconfigureHint(), ) } return nil diff --git a/internal/credential/credential_provider.go b/internal/credential/credential_provider.go index 442468d01..8f20be11d 100644 --- a/internal/credential/credential_provider.go +++ b/internal/credential/credential_provider.go @@ -203,7 +203,7 @@ func (p *CredentialProvider) doResolveAccount(ctx context.Context) (*Account, er p.selectedSource = defaultTokenSource{resolver: p.defaultToken} return acct, nil } - return nil, fmt.Errorf("no credential provider returned an account; run 'lark-cli config' to set up") + return nil, core.NotConfiguredError() } // enrichUserInfo resolves user identity when extension provides a UAT. diff --git a/internal/credential/default_provider.go b/internal/credential/default_provider.go index a3ebb90c9..45ec3ef00 100644 --- a/internal/credential/default_provider.go +++ b/internal/credential/default_provider.go @@ -36,7 +36,7 @@ func (p *DefaultAccountProvider) ResolveAccount(ctx context.Context) (*Account, // Load config once — used for both credentials and strict mode. multi, err := core.LoadMultiAppConfig() if err != nil { - return nil, &core.ConfigError{Code: 2, Type: "config", Message: "not configured", Hint: "run `lark-cli config init --new` in the background. It blocks and outputs a verification URL — retrieve the URL and open it in a browser to complete setup."} + return nil, core.NotConfiguredError() } cfg, err := core.ResolveConfigFromMulti(multi, p.keychain(), p.profile) diff --git a/tests/cli_e2e/config/bind_test.go b/tests/cli_e2e/config/bind_test.go index 56182f1fb..088750800 100644 --- a/tests/cli_e2e/config/bind_test.go +++ b/tests/cli_e2e/config/bind_test.go @@ -254,8 +254,8 @@ func TestBind_ConfigShow_UnboundWorkspace(t *testing.T) { }) require.NoError(t, err) assertStderrError(t, result, 2, "openclaw", - "openclaw context detected but lark-cli not bound to openclaw workspace", - "run: lark-cli config bind --source openclaw") + "openclaw context detected but lark-cli is not bound to it", + "read `lark-cli config bind --help`, then ask the user to confirm intent and identity preset (bot-only or user-default); only after both are confirmed, run `lark-cli config bind`") } func TestBind_OpenClaw_MissingFile(t *testing.T) {