From 146f13e5e2f39cd80da8dec5282ce3dbc6123792 Mon Sep 17 00:00:00 2001 From: AlbertSun Date: Tue, 23 Jun 2026 20:57:18 +0800 Subject: [PATCH] feat(auth): add --restore to recover the previous app when credentials are corrupted --- cmd/config/init.go | 143 +++++++++++++++++++------ cmd/config/init_interactive.go | 10 +- cmd/config/init_test.go | 15 +++ internal/auth/app_registration.go | 6 ++ internal/auth/app_registration_test.go | 22 ++++ 5 files changed, 161 insertions(+), 35 deletions(-) diff --git a/cmd/config/init.go b/cmd/config/init.go index d17bf9b0f..348bd8b40 100644 --- a/cmd/config/init.go +++ b/cmd/config/init.go @@ -41,6 +41,8 @@ type ConfigInitOptions struct { ProfileName string // when set, create/update a named profile instead of replacing Apps[0] + Restore bool // Restore re-registers the app already in config to recover a lost credential + // 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 @@ -89,6 +91,7 @@ if the user explicitly wants a separate app inside the Agent workspace.`, cmd.Flags().StringVar(&opts.Brand, "brand", "feishu", "feishu or lark (non-interactive, default feishu)") cmd.Flags().StringVar(&opts.Lang, "lang", "", "language preference (e.g. zh or zh_cn)") cmd.Flags().StringVar(&opts.ProfileName, "name", "", "create or update a named profile (append instead of replace)") + cmd.Flags().BoolVar(&opts.Restore, "restore", false, "re-register the app already in config to recover a lost credential (keychain key / app secret); reuses the stored app ID and auth method") 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") cmdutil.SetRisk(cmd, "write") @@ -135,7 +138,7 @@ func guardAgentWorkspace(opts *ConfigInitOptions) error { // hasAnyNonInteractiveFlag returns true if any non-interactive flag is set. func (o *ConfigInitOptions) hasAnyNonInteractiveFlag() bool { - return o.New || o.AppID != "" || o.AppSecretStdin + return o.New || o.Restore || o.AppID != "" || o.AppSecretStdin } // cleanupOldConfig clears keychain entries (AppSecret + UAT) for all apps in existing config except the app whose AppId equals skipAppID. @@ -347,6 +350,94 @@ func updateExistingProfileWithoutSecret(existing *core.MultiAppConfig, profileNa return core.SaveMultiAppConfig(existing) } +// persistAndProbeResult saves a registration/restore result into profileName and +// runs the post-registration probe. profileName == "" replaces the single app +// (legacy); a named profile is updated in place. Shared by --new and --restore. +func persistAndProbeResult(opts *ConfigInitOptions, f *cmdutil.Factory, profileName string, result *configInitResult) error { + existing, _ := core.LoadMultiAppConfig() + + // private_key_jwt apps have no secret: persist auth method + TEE key ref. + // Registration success already validated the key (server bound the public + // key), so the app_secret probe is skipped. + if result.AuthMethod == core.AuthMethodPrivateKeyJWT { + if err := saveInitConfig(profileName, existing, f, result.AppID, core.SecretInput{}, result.Brand, opts.Lang, result.AuthMethod, keyRefFromResult(result)); err != nil { + return wrapSaveConfigError(err) + } + removeStaleSecretForPKJWT(existing, profileName, result.AppID, f.Keychain) + printLangPreferenceConfirmation(opts) + output.PrintJson(f.IOStreams.Out, map[string]interface{}{"appId": result.AppID, "authMethod": result.AuthMethod, "brand": result.Brand}) + return runProbePKJWT(opts.Ctx, f, result.Brand, result.AppID, keysigner.Active(), result.KeyLabel) + } + + secret, err := core.ForStorage(result.AppID, core.PlainSecret(result.AppSecret), f.Keychain) + if err != nil { + return errs.NewInternalError(errs.SubtypeSDKError, "%v", err).WithCause(err) + } + if err := saveInitConfig(profileName, existing, f, result.AppID, secret, result.Brand, opts.Lang, "", nil); err != nil { + return wrapSaveConfigError(err) + } + printLangPreferenceConfirmation(opts) + output.PrintJson(f.IOStreams.Out, map[string]interface{}{"appId": result.AppID, "appSecret": "****", "brand": result.Brand}) + return runProbe(opts.Ctx, f, result.AppID, result.AppSecret, result.Brand) +} + +// runRestoreFlow re-registers the app already in config to recover a lost +// credential (deleted keychain key / lost app secret). It reads the existing +// app id + auth method + brand from config (no secret needed — that's the lost +// part) and re-runs the device-flow registration with the app id sent on begin, +// so the server re-registers that app instead of creating a new one. The +// re-issued credential is written back to the same profile. +func runRestoreFlow(opts *ConfigInitOptions, existing *core.MultiAppConfig, f *cmdutil.Factory, msg *initMsg) error { + if existing == nil { + return errs.NewConfigError(errs.SubtypeNotConfigured, "nothing to restore: no config found"). + WithHint("run: lark-cli config init") + } + app := existing.CurrentAppConfig(opts.ProfileName) + if app == nil || app.AppId == "" { + return errs.NewConfigError(errs.SubtypeNotConfigured, "nothing to restore: no app id in config%s", profileSuffix(opts.ProfileName)). + WithHint("run: lark-cli config init") + } + + restoreAppID := app.AppId + // Reuse the stored auth method authoritatively — never prompt. Empty on disk + // means client_secret (omitempty back-compat); pass it explicitly so + // resolveRegisterAuthMethod doesn't fall through to the interactive picker. + authMethod := app.AuthMethod + if authMethod == "" { + authMethod = core.AuthMethodClientSecret + } + result, err := runCreateAppFlow(opts.Ctx, f, app.Brand, authMethod, msg, restoreAppID) + if err != nil { + return err + } + if result == nil { + return errs.NewInternalError(errs.SubtypeSDKError, "app restore returned no result") + } + + // Safety: if the server did not honor app_id (e.g. not yet supported), it may + // have created a NEW app instead of restoring. Warn so the user is not silently + // switched to a different app id. + if result.AppID != restoreAppID { + fmt.Fprintf(f.IOStreams.ErrOut, "[lark-cli] [WARN] restore: server returned app %s, expected %s — it may have created a new app instead of restoring\n", result.AppID, restoreAppID) + } + + // Write back to the profile we restored: an explicit --name, else the resolved + // app's own name. Empty name => legacy single-app replace. + saveProfile := opts.ProfileName + if saveProfile == "" { + saveProfile = app.Name + } + return persistAndProbeResult(opts, f, saveProfile, result) +} + +// profileSuffix renders " (profile %q)" for error messages, or "" when unnamed. +func profileSuffix(profileName string) string { + if profileName == "" { + return "" + } + return fmt.Sprintf(" (profile %q)", profileName) +} + func configInitRun(opts *ConfigInitOptions) error { f := opts.Factory @@ -377,6 +468,17 @@ func configInitRun(opts *ConfigInitOptions) error { } } + // --restore recovers an existing app; it is incompatible with creating a new + // app (--new) or importing one non-interactively (--app-id / stdin secret). + if opts.Restore { + if opts.New { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--restore cannot be combined with --new").WithParam("--restore") + } + if opts.AppID != "" || opts.AppSecretStdin { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--restore cannot be combined with --app-id / --app-secret-stdin").WithParam("--restore") + } + } + // Mode 1: Non-interactive if opts.AppID != "" && opts.appSecret != "" { brand := parseBrand(opts.Brand) @@ -410,46 +512,21 @@ func configInitRun(opts *ConfigInitOptions) error { msg := getInitMsg(opts.UILang) + // Mode: Restore (--restore) — re-register the app already in config. + if opts.Restore { + return runRestoreFlow(opts, existing, f, msg) + } + // Mode 3: Create new app directly (--new) if opts.New { - result, err := runCreateAppFlow(opts.Ctx, f, parseBrand(opts.Brand), opts.AuthMethod, msg) + result, err := runCreateAppFlow(opts.Ctx, f, parseBrand(opts.Brand), opts.AuthMethod, msg, "") if err != nil { return err } if result == nil { return errs.NewInternalError(errs.SubtypeSDKError, "app creation returned no result") } - existing, _ := core.LoadMultiAppConfig() - - // private_key_jwt apps have no secret: persist auth method + TEE key ref. - // Registration success already validated the key (server bound the public - // key), so the app_secret probe is skipped. - if result.AuthMethod == core.AuthMethodPrivateKeyJWT { - if err := saveInitConfig(opts.ProfileName, existing, f, result.AppID, core.SecretInput{}, result.Brand, opts.Lang, result.AuthMethod, keyRefFromResult(result)); err != nil { - return wrapSaveConfigError(err) - } - removeStaleSecretForPKJWT(existing, opts.ProfileName, result.AppID, f.Keychain) - printLangPreferenceConfirmation(opts) - output.PrintJson(f.IOStreams.Out, map[string]interface{}{"appId": result.AppID, "authMethod": result.AuthMethod, "brand": result.Brand}) - if err := runProbePKJWT(opts.Ctx, f, result.Brand, result.AppID, keysigner.Active(), result.KeyLabel); err != nil { - return err - } - return nil - } - - secret, err := core.ForStorage(result.AppID, core.PlainSecret(result.AppSecret), f.Keychain) - if err != nil { - return errs.NewInternalError(errs.SubtypeSDKError, "%v", err).WithCause(err) - } - if err := saveInitConfig(opts.ProfileName, existing, f, result.AppID, secret, result.Brand, opts.Lang, "", nil); err != nil { - return wrapSaveConfigError(err) - } - printLangPreferenceConfirmation(opts) - output.PrintJson(f.IOStreams.Out, map[string]interface{}{"appId": result.AppID, "appSecret": "****", "brand": result.Brand}) - if err := runProbe(opts.Ctx, f, result.AppID, result.AppSecret, result.Brand); err != nil { - return err - } - return nil + return persistAndProbeResult(opts, f, opts.ProfileName, result) } // Mode 4: Interactive TUI (terminal) diff --git a/cmd/config/init_interactive.go b/cmd/config/init_interactive.go index 88fba21b7..03f088ccf 100644 --- a/cmd/config/init_interactive.go +++ b/cmd/config/init_interactive.go @@ -62,7 +62,7 @@ func runInteractiveConfigInit(ctx context.Context, f *cmdutil.Factory, authMetho return runExistingAppForm(f, msg) } - return runCreateAppFlow(ctx, f, "", authMethodFlag, msg) + return runCreateAppFlow(ctx, f, "", authMethodFlag, msg, "") } // runExistingAppForm shows a huh form for manually entering App ID / App Secret / Brand. @@ -203,7 +203,10 @@ func resolveRegisterAuthMethod(f *cmdutil.Factory, flag string) (string, error) // runCreateAppFlow runs the "create new app" flow via OpenClaw device flow. // If brandOverride is non-empty, skip the interactive brand selection. // authMethodFlag is the raw --auth-method value ("" when unset). -func runCreateAppFlow(ctx context.Context, f *cmdutil.Factory, brandOverride core.LarkBrand, authMethodFlag string, msg *initMsg) (*configInitResult, error) { +// restoreAppID, when non-empty, is sent on the registration begin request so the +// server re-registers that existing app (credential recovery) instead of creating +// a new one. Empty preserves the normal new-app flow. +func runCreateAppFlow(ctx context.Context, f *cmdutil.Factory, brandOverride core.LarkBrand, authMethodFlag string, msg *initMsg, restoreAppID string) (*configInitResult, error) { var larkBrand core.LarkBrand if brandOverride != "" { larkBrand = brandOverride @@ -272,6 +275,9 @@ func runCreateAppFlow(ctx context.Context, f *cmdutil.Factory, brandOverride cor } } + // Restore flow: re-register the existing app instead of creating a new one. + beginOpts.RestoreAppID = restoreAppID + authResp, err := larkauth.RequestAppRegistration(httpClient, larkBrand, beginOpts, f.IOStreams.ErrOut) if err != nil { return nil, errs.NewConfigError(errs.SubtypeInvalidClient, "app registration failed: %v", err).WithCause(err) diff --git a/cmd/config/init_test.go b/cmd/config/init_test.go index 52e2617f5..25584bbd7 100644 --- a/cmd/config/init_test.go +++ b/cmd/config/init_test.go @@ -14,6 +14,21 @@ import ( "github.com/larksuite/cli/internal/output" ) +// TestRunRestoreFlow_NothingToRestore covers the early guards that return before +// any network/registration call: no config at all, and a config whose resolved +// app has no app id (nothing to send on begin). +func TestRunRestoreFlow_NothingToRestore(t *testing.T) { + // No config on disk. + if err := runRestoreFlow(&ConfigInitOptions{}, nil, nil, nil); err == nil { + t.Fatal("expected error when there is no config to restore") + } + // Config present but the resolved app has no app id. + existing := &core.MultiAppConfig{Apps: []core.AppConfig{{AppId: ""}}} + if err := runRestoreFlow(&ConfigInitOptions{}, existing, nil, nil); err == nil { + t.Fatal("expected error when the resolved app has no app id") + } +} + // updateExistingProfileWithoutSecret guards four blank-input scenarios. Each // must surface as *ValidationError(SubtypeInvalidArgument) per RFC 6749 §5.2: // SubtypeInvalidClient is reserved for IAM rejection of malformed credentials, diff --git a/internal/auth/app_registration.go b/internal/auth/app_registration.go index 916cffaea..9d4bf2a47 100644 --- a/internal/auth/app_registration.go +++ b/internal/auth/app_registration.go @@ -55,6 +55,7 @@ type AppRegistrationInit struct { type AppRegistrationBeginOptions struct { AuthMethod string // "" => client_secret; core.AuthMethodPrivateKeyJWT AuthAttestation string // private_key_jwt: the TEE-signed attestation JWT + RestoreAppID string // when set, asks the server to re-register this existing app } // RequestAppRegistrationInit performs the init step of the registration flow, @@ -139,6 +140,11 @@ func RequestAppRegistration(httpClient *http.Client, brand core.LarkBrand, opts if opts.AuthAttestation != "" { form.Set("auth_attestation", opts.AuthAttestation) } + // Restore flow: carry the existing app id so the server re-registers it + // rather than creating a new app. + if opts.RestoreAppID != "" { + form.Set("app_id", opts.RestoreAppID) + } req, err := http.NewRequest("POST", endpoint, strings.NewReader(form.Encode())) if err != nil { diff --git a/internal/auth/app_registration_test.go b/internal/auth/app_registration_test.go index 2a607224b..9c57dabdd 100644 --- a/internal/auth/app_registration_test.go +++ b/internal/auth/app_registration_test.go @@ -132,6 +132,28 @@ func TestRequestAppRegistration_BeginDefaultsToClientSecret(t *testing.T) { if body.Has("auth_attestation") { t.Errorf("auth_attestation should be absent for client_secret, got %q", body.Get("auth_attestation")) } + // Normal (non-restore) begin must NOT carry app_id. + if body.Has("app_id") { + t.Errorf("app_id should be absent when RestoreAppID is empty, got %q", body.Get("app_id")) + } +} + +// TestRequestAppRegistration_BeginRestoreAppID verifies the restore flow sends the +// existing app id on begin so the server re-registers that app. +func TestRequestAppRegistration_BeginRestoreAppID(t *testing.T) { + var body url.Values + hc := captureClient(&body, beginRespJSON) + + opts := AppRegistrationBeginOptions{RestoreAppID: "cli_restore_me"} + if _, err := RequestAppRegistration(hc, core.BrandFeishu, opts, nil); err != nil { + t.Fatal(err) + } + if body.Get("action") != "begin" { + t.Errorf("action = %q, want begin", body.Get("action")) + } + if body.Get("app_id") != "cli_restore_me" { + t.Errorf("app_id = %q, want cli_restore_me", body.Get("app_id")) + } } func TestRequestAppRegistration_VerificationURICompleteFallback(t *testing.T) {