diff --git a/cmd/build.go b/cmd/build.go index cf8c7c1f1d..c0829c4c51 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -29,6 +29,7 @@ import ( "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/commandhost" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/distribution" "github.com/larksuite/cli/internal/hook" "github.com/larksuite/cli/internal/keychain" internalplatform "github.com/larksuite/cli/internal/platform" @@ -225,6 +226,7 @@ func buildInternalWithConfig(ctx context.Context, inv cmdutil.InvocationContext, if cfg == nil { cfg = &buildConfig{} } + ctx = distribution.CaptureSource(ctx) registeredShortcuts, commandSetErr := resolveShortcutSnapshot(cfg.commandSets) // Default streams when WithIO is not supplied so the root command's // SetIn/Out/Err calls below don't deref nil. NewDefault also normalizes diff --git a/cmd/doctor/doctor.go b/cmd/doctor/doctor.go index 6e4a621933..8dca99fc99 100644 --- a/cmd/doctor/doctor.go +++ b/cmd/doctor/doctor.go @@ -94,7 +94,7 @@ func doctorRun(opts *DoctorOptions, projector *recovery.Projector) error { // ── 0. CLI version & update check ── checks = append(checks, pass("cli_version", build.Version)) if !opts.Offline && projector.CanReference(recovery.TargetUpdate) { - checks = append(checks, checkCLIUpdate()...) + checks = append(checks, checkCLIUpdate(opts.Ctx)...) } // ── 1. Config file ── @@ -241,24 +241,24 @@ func probeEndpoint(ctx context.Context, client *http.Client, url string) error { return nil } -// checkCLIUpdate actively queries the npm registry for the latest version. +// checkCLIUpdate actively queries the configured source for its target version. // Unlike the root-level async check, this does a synchronous fetch with timeout // and works regardless of build version (dev builds included). -func checkCLIUpdate() []checkResult { - latest, err := fetchLatestForDoctor() +func checkCLIUpdate(ctx context.Context) []checkResult { + target, err := fetchLatestForDoctor(ctx) if err != nil { return []checkResult{warn("cli_update", "check failed: "+err.Error(), "")} } current := build.Version - if update.IsNewer(latest, current) { + if target.Available(current) { return []checkResult{warn("cli_update", - fmt.Sprintf("%s → %s available", current, latest), + fmt.Sprintf("%s → %s available", current, target.Version), "run: lark-cli update")} } - return []checkResult{pass("cli_update", latest+" (up to date)")} + return []checkResult{pass("cli_update", target.Version+" (up to date)")} } -var fetchLatestForDoctor = update.FetchLatest +var fetchLatestForDoctor = update.FetchTarget func finishDoctor(f *cmdutil.Factory, checks []checkResult) error { allOK := true diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go index 6da687089f..2c3a7ffbfb 100644 --- a/cmd/doctor/doctor_test.go +++ b/cmd/doctor/doctor_test.go @@ -7,20 +7,61 @@ import ( "bytes" "context" "encoding/json" + "fmt" "net/http" + "net/http/httptest" "strings" "testing" "github.com/spf13/cobra" extcred "github.com/larksuite/cli/extension/credential" + exttransport "github.com/larksuite/cli/extension/transport" + "github.com/larksuite/cli/internal/build" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" + "github.com/larksuite/cli/internal/distribution" "github.com/larksuite/cli/internal/recovery" "github.com/larksuite/cli/internal/surface" + "github.com/larksuite/cli/internal/update" ) +type doctorManifestProvider struct{ manifestURL string } + +func (doctorManifestProvider) Name() string { return "doctor-manifest-test" } +func (doctorManifestProvider) ResolveInterceptor(context.Context) exttransport.Interceptor { + return nil +} +func (p doctorManifestProvider) ResolveManifestURL(context.Context) string { + return p.manifestURL +} + +func TestCheckCLIUpdateReportsDifferentOpaqueManifestTarget(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprintf(w, `{"schema":1,"version":"older-channel","artifacts":{"skills":{"url":"https://distribution.example/skills.zip","checksum":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},%q:{"url":"https://distribution.example/cli.zip","checksum":"sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"}}}`, distribution.CurrentPlatformKey()) + })) + defer server.Close() + previousProvider := exttransport.GetProvider() + previousFetch := fetchLatestForDoctor + previousClient := distribution.DefaultClient + previousVersion := build.Version + exttransport.Register(doctorManifestProvider{manifestURL: server.URL}) + distribution.DefaultClient = server.Client() + fetchLatestForDoctor = update.FetchTarget + build.Version = "newer-channel" + t.Cleanup(func() { + exttransport.Register(previousProvider) + fetchLatestForDoctor = previousFetch + distribution.DefaultClient = previousClient + build.Version = previousVersion + }) + checks := checkCLIUpdate(context.Background()) + if len(checks) != 1 || checks[0].Status != "warn" || !strings.Contains(checks[0].Message, "older-channel") { + t.Fatalf("checks = %#v", checks) + } +} + func TestNewCmdDoctor_FlagParsing(t *testing.T) { f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, @@ -109,9 +150,9 @@ func TestDoctorRunDoesNotFetchUpdateWhenCommandIsConcealed(t *testing.T) { t.Cleanup(func() { fetchLatestForDoctor = oldFetch }) fetches := 0 - fetchLatestForDoctor = func() (string, error) { + fetchLatestForDoctor = func(context.Context) (update.Target, error) { fetches++ - return "9.9.9", nil + return update.Target{Version: "9.9.9"}, nil } plan := surface.NewPlan(map[surface.CommandID]surface.CommandState{ surface.CommandUpdate: surface.CommandConcealed, diff --git a/cmd/notice_test.go b/cmd/notice_test.go index 13f8a51946..1609df5b4d 100644 --- a/cmd/notice_test.go +++ b/cmd/notice_test.go @@ -9,8 +9,22 @@ import ( "github.com/larksuite/cli/internal/deprecation" "github.com/larksuite/cli/internal/skillscheck" + "github.com/larksuite/cli/internal/update" ) +func TestComposePendingNoticeUsesManifestTarget(t *testing.T) { + update.SetPending(&update.UpdateInfo{Current: "newer", Latest: "older", Source: "manifest"}) + t.Cleanup(func() { update.SetPending(nil) }) + + entry := composePendingNotice(nil)["update"].(map[string]interface{}) + if entry["target"] != "older" || entry["source"] != "manifest" { + t.Fatalf("manifest update notice = %#v", entry) + } + if _, exists := entry["latest"]; exists { + t.Fatalf("manifest target was labeled latest: %#v", entry) + } +} + // composePendingNotice must surface a deprecated-command alias under the // "deprecated_command" key, with the migration target and a skill-update hint, // so the JSON "_notice" envelope reaches users who run pre-refactor commands diff --git a/cmd/presentation_test.go b/cmd/presentation_test.go index 6f3a10648d..fd2bd0ac9c 100644 --- a/cmd/presentation_test.go +++ b/cmd/presentation_test.go @@ -595,14 +595,14 @@ func TestSetupNoticesDoesNoProviderWorkWhenUpdateIsConcealed(t *testing.T) { }) var checks, refreshes, skillChecks int - checkCachedUpdate = func(string) *update.UpdateInfo { + checkCachedUpdate = func(context.Context, string) *update.UpdateInfo { checks++ return nil } - refreshUpdateCache = func(string) { refreshes++ } - initializeSkillsCheck = func(string) { skillChecks++ } + refreshUpdateCache = func(context.Context, string) { refreshes++ } + initializeSkillsCheck = func(context.Context, string) { skillChecks++ } - setupNotices(surface.NewPlan(map[surface.CommandID]surface.CommandState{ + setupNotices(context.Background(), surface.NewPlan(map[surface.CommandID]surface.CommandState{ surface.CommandUpdate: surface.CommandConcealed, })) if checks != 0 || refreshes != 0 || skillChecks != 0 { diff --git a/cmd/root.go b/cmd/root.go index a9509e6473..08a6189919 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -21,6 +21,7 @@ import ( "github.com/larksuite/cli/internal/cmdpolicy" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/deprecation" + "github.com/larksuite/cli/internal/distribution" "github.com/larksuite/cli/internal/flagalias" "github.com/larksuite/cli/internal/hook" "github.com/larksuite/cli/internal/output" @@ -106,7 +107,7 @@ func executeWithOptions(opts []BuildOption) int { // --- Notices (non-blocking) --- if !isCompletionCommand(os.Args) { - setupNotices(runtime.surface) + setupNotices(rootCmd.Context(), runtime.surface) } runErr := rootCmd.Execute() @@ -142,17 +143,23 @@ func isDeferredBootstrapProfileError(err error) bool { var ( checkCachedUpdate = update.CheckCached refreshUpdateCache = update.RefreshCache - initializeSkillsCheck = skillscheck.Init + initializeSkillsCheck = func(ctx context.Context, version string) { + if src, err := distribution.ResolveSource(ctx); err == nil && src.ManifestMode() { + skillscheck.InitForSource(version, src.Identity(), true) + return + } + skillscheck.Init(version) + } ) // setupNotices wires both the binary update notice and the skills // staleness notice into output.PendingNotice as a composed function. // Each provider populates an independent key under _notice; either // or both may be present in any given envelope. -func setupNotices(plan *surface.Plan) { +func setupNotices(ctx context.Context, plan *surface.Plan) { if plan.CanReference(surface.CommandUpdate) { // Binary update — synchronous cache check + async refresh. - if info := checkCachedUpdate(build.Version); info != nil { + if info := checkCachedUpdate(ctx, build.Version); info != nil { update.SetPending(info) } ver := build.Version @@ -162,9 +169,9 @@ func setupNotices(plan *surface.Plan) { fmt.Fprintf(os.Stderr, "update check panic: %v\n", r) } }() - refreshUpdateCache(ver) + refreshUpdateCache(ctx, ver) if update.GetPending() == nil { - if info := checkCachedUpdate(ver); info != nil { + if info := checkCachedUpdate(ctx, ver); info != nil { update.SetPending(info) } } @@ -172,7 +179,7 @@ func setupNotices(plan *surface.Plan) { // Skills drift has only one recovery action: lark-cli update. Do not // even inspect local drift state when that action is absent. - initializeSkillsCheck(build.Version) + initializeSkillsCheck(ctx, build.Version) } // Capture this build's immutable plan; never consult another Build's state. @@ -192,12 +199,18 @@ func composePendingNotice(plan *surface.Plan) map[string]interface{} { // both exist solely to steer the caller to `lark-cli update`. if canUpdate { if info := update.GetPending(); info != nil { - notice["update"] = map[string]interface{}{ + entry := map[string]interface{}{ "current": info.Current, - "latest": info.Latest, "message": info.Message(), "command": "lark-cli update", } + if info.Source == "manifest" { + entry["source"] = "manifest" + entry["target"] = info.Latest + } else { + entry["latest"] = info.Latest + } + notice["update"] = entry } if stale := skillscheck.GetPending(); stale != nil { entry := map[string]interface{}{ diff --git a/cmd/root_integration_test.go b/cmd/root_integration_test.go index 1a96094a28..2f979c24e3 100644 --- a/cmd/root_integration_test.go +++ b/cmd/root_integration_test.go @@ -510,7 +510,7 @@ func TestSetupNotices_ColdStart_NoNotice(t *testing.T) { output.PendingNotice = nil }) - setupNotices(nil) + setupNotices(context.Background(), nil) notice := output.GetNotice() if notice == nil { @@ -544,7 +544,7 @@ func TestSetupNotices_InSync(t *testing.T) { output.PendingNotice = nil }) - setupNotices(nil) + setupNotices(context.Background(), nil) notice := output.GetNotice() if notice != nil { @@ -577,7 +577,7 @@ func TestSetupNotices_Drift(t *testing.T) { output.PendingNotice = nil }) - setupNotices(nil) + setupNotices(context.Background(), nil) notice := output.GetNotice() if notice == nil { @@ -626,7 +626,7 @@ func TestSetupNotices_BothUpdateAndSkills(t *testing.T) { output.PendingNotice = nil }) - setupNotices(nil) + setupNotices(context.Background(), nil) // After setupNotices, skills pending is set (drift). Manually populate // the update side so the composed envelope has both keys — the update diff --git a/cmd/root_upgrade.go b/cmd/root_upgrade.go index f1ac4f7ed5..ed3bffb7f3 100644 --- a/cmd/root_upgrade.go +++ b/cmd/root_upgrade.go @@ -67,7 +67,7 @@ func offerRootUpgrade(f *cmdutil.Factory, cmd *cobra.Command, projector *recover // Gate 4: cached newer version. CheckCached applies opt-out (shouldSkip) // and the IsNewer/semver validation chain; it reads the on-disk cache that // the 24h-throttled RefreshCache maintains (CheckCached itself has no TTL). - info := checkRootCachedUpdate(build.Version) + info := checkRootCachedUpdate(cmd.Context(), build.Version) if info == nil { return } diff --git a/cmd/root_upgrade_test.go b/cmd/root_upgrade_test.go index 4b823b4eec..6c115f39e2 100644 --- a/cmd/root_upgrade_test.go +++ b/cmd/root_upgrade_test.go @@ -5,6 +5,7 @@ package cmd import ( "bytes" + "context" "fmt" "os" "path/filepath" @@ -154,7 +155,7 @@ func TestOfferRootUpgradeDoesNotReadCacheWhenUpdateIsConcealed(t *testing.T) { t.Cleanup(func() { checkRootCachedUpdate = oldCheck }) cacheReads := 0 - checkRootCachedUpdate = func(string) *update.UpdateInfo { + checkRootCachedUpdate = func(context.Context, string) *update.UpdateInfo { cacheReads++ return &update.UpdateInfo{Current: "1.0.0", Latest: "2.0.0"} } diff --git a/cmd/update/manifest.go b/cmd/update/manifest.go new file mode 100644 index 0000000000..8a4e72ad3a --- /dev/null +++ b/cmd/update/manifest.go @@ -0,0 +1,94 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package cmdupdate + +import ( + "context" + "fmt" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/distribution" + "github.com/larksuite/cli/internal/output" +) + +func runManifestUpdate(ctx context.Context, opts *UpdateOptions, src distribution.Source) error { + streams := opts.Factory.IOStreams + current := currentVersion() + manifest, err := src.FetchManifest(ctx) + if err != nil { + return reportDistributionError(opts, err) + } + target := manifest.Version + if opts.Check { + return reportManifestStatus(opts, current, target, false) + } + if !opts.Force && target == current { + if err := distribution.SyncSkills(ctx, manifest, distribution.InstallOptions{}); err != nil { + return reportDistributionError(opts, err) + } + return reportManifestStatus(opts, current, target, true) + } + if !opts.JSON { + fmt.Fprintf(streams.ErrOut, "Updating lark-cli %s %s %s from the configured distribution ...\n", current, symArrow(), target) + } + if err := distribution.Install(ctx, manifest, distribution.InstallOptions{}); err != nil { + return reportDistributionError(opts, err) + } + if opts.JSON { + output.PrintJson(streams.Out, map[string]interface{}{ + "ok": true, "source": "manifest", + "previous_version": current, "current_version": target, "target_version": target, + "action": "updated", "skills_action": "synced", + "message": fmt.Sprintf("lark-cli updated from %s to %s", current, target), + }) + return nil + } + fmt.Fprintf(streams.ErrOut, "\n%s Successfully updated lark-cli and Skills from %s to %s\n", symOK(), current, target) + return nil +} + +// reportManifestStatus reports the configured target. The target is an opaque +// string chosen by the distribution, so the JSON field is target_version — +// it must not be labeled latest_version like an npm registry result. +func reportManifestStatus(opts *UpdateOptions, current, target string, skillsSynced bool) error { + streams := opts.Factory.IOStreams + action := "already_up_to_date" + message := fmt.Sprintf("lark-cli %s matches the configured target", current) + if current != target { + action = "update_available" + message = fmt.Sprintf("lark-cli %s %s configured target %s", current, symArrow(), target) + } + if opts.JSON { + result := map[string]interface{}{ + "ok": true, "source": "manifest", + "previous_version": current, "current_version": current, "target_version": target, + "action": action, "message": message, + } + if opts.Check { + result["auto_update"] = true + } + if skillsSynced { + result["skills_action"] = "synced" + } + output.PrintJson(streams.Out, result) + return nil + } + if current == target { + fmt.Fprintf(streams.ErrOut, "%s %s\n", symOK(), message) + if skillsSynced { + fmt.Fprintln(streams.ErrOut, "Skills synchronized from the configured distribution.") + } + } else { + fmt.Fprintf(streams.ErrOut, "Configured target: %s %s %s\n\nRun `lark-cli update` to install.\n", current, symArrow(), target) + } + return nil +} + +func reportDistributionError(opts *UpdateOptions, typed errs.TypedError) error { + errType := "update_error" + if problem, ok := errs.ProblemOf(typed); ok && problem.Category == errs.CategoryNetwork { + errType = "network" + } + return reportError(opts, opts.Factory.IOStreams, errType, typed) +} diff --git a/cmd/update/update.go b/cmd/update/update.go index 3dff4f260a..091da2fb40 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -4,6 +4,7 @@ package cmdupdate import ( + "context" "fmt" stdio "io" "runtime" @@ -15,11 +16,13 @@ import ( "github.com/larksuite/cli/internal/build" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/distribution" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/selfupdate" "github.com/larksuite/cli/internal/skillscheck" "github.com/larksuite/cli/internal/update" "github.com/larksuite/cli/internal/urlrewrite" + "github.com/larksuite/cli/internal/versioncheck" ) const ( @@ -31,7 +34,10 @@ const ( // Overridable for testing. var ( - fetchLatest = func() (string, error) { return update.FetchLatest() } + fetchLatest = func() (string, error) { + target, err := update.FetchTargetForSource(context.Background(), distribution.Source{}) + return target.Version, err + } currentVersion = func() string { return build.Version } currentOS = runtime.GOOS newUpdater = func() *selfupdate.Updater { return selfupdate.New() } @@ -40,15 +46,6 @@ var ( func isWindows() bool { return currentOS == osWindows } -// normalizeVersion canonicalizes a version string for state comparison. -// Strips a leading "v" so versions written from Makefile (git describe → -// "v1.0.0") and npm (no prefix → "1.0.0") compare equal. -func normalizeVersion(s string) string { - s = strings.TrimSpace(s) - s = strings.TrimPrefix(s, "v") - return strings.TrimPrefix(s, "V") -} - func releaseURL(version string) string { return repoURL + "/releases/tag/v" + strings.TrimPrefix(version, "v") } @@ -102,10 +99,11 @@ func NewCmdUpdate(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "update", - Short: "Update lark-cli to the latest version", - Long: `Update lark-cli to the latest version. + Short: "Update lark-cli and its managed Skills", + Long: `Update lark-cli using the active update source. Detects the installation method automatically: + - configured distribution: installs checksum-verified CLI and Skills artifacts - npm install: runs npm install -g @larksuite/cli@ - pnpm install: runs pnpm add -g @larksuite/cli@ - manual/other: shows GitHub Releases download URL @@ -115,7 +113,7 @@ Use --check to only check for updates without installing. The skill name "lark-suite" is reserved for CLI-managed suite layout.`, RunE: func(cmd *cobra.Command, args []string) error { - return updateRun(opts) + return updateRunWithContext(cmd.Context(), opts) }, } cmdutil.DisableAuthCheck(cmd) @@ -129,6 +127,13 @@ The skill name "lark-suite" is reserved for CLI-managed suite layout.`, } func updateRun(opts *UpdateOptions) error { + return updateRunWithContext(nil, opts) +} + +func updateRunWithContext(ctx context.Context, opts *UpdateOptions) error { + if ctx == nil { + ctx = context.Background() + } io := opts.Factory.IOStreams if _, err := skillscheck.ParseLayout(opts.SkillsLayout); err != nil { return reportError(opts, io, "validation", @@ -140,6 +145,19 @@ func updateRun(opts *UpdateOptions) error { WithParam("--skills-layout"). WithHint("Remove --skills-layout when using --check.")) } + src, configErr := distribution.ResolveSource(ctx) + if configErr != nil { + return reportError(opts, io, "configuration", configErr) + } + if src.ManifestMode() { + if strings.TrimSpace(opts.SkillsLayout) != "" { + return reportError(opts, io, "validation", + errs.NewValidationError(errs.SubtypeInvalidArgument, "--skills-layout is not supported by the configured distribution"). + WithParam("--skills-layout")) + } + output.PendingNotice = nil + return runManifestUpdate(ctx, opts, src) + } cur := currentVersion() updater := newUpdater() // Brand only steers skills sync. updateRun skips that resolution in --check, @@ -158,13 +176,13 @@ func updateRun(opts *UpdateOptions) error { } // 2. Validate version format - if update.ParseVersion(latest) == nil { + if versioncheck.Parse(latest) == nil { return reportError(opts, io, "update_error", errs.NewInternalError(errs.SubtypeInvalidResponse, "invalid version from registry: %s", latest)) } // 3. Compare versions - if !opts.Force && !update.IsNewer(latest, cur) { + if !opts.Force && !versioncheck.IsNewer(latest, cur) { var skillsResult *skillscheck.SyncResult if !opts.Check { skillsResult = runSkillsAndState(updater, io, cur, opts.Force, opts.SkillsLayout) @@ -431,8 +449,9 @@ func verificationFailureHint(updater *selfupdate.Updater, latest, pm string) str func runSkillsAndState(updater *selfupdate.Updater, io *cmdutil.IOStreams, stateVersion string, force bool, requestedLayout string) *skillscheck.SyncResult { layout, _ := skillscheck.ParseLayout(requestedLayout) if !force { - if state, ok, err := skillscheck.ReadState(); err == nil && ok && normalizeVersion(state.Version) == normalizeVersion(stateVersion) { - if !state.OfficialSkillsUnknown && (layout == "" || skillscheck.EffectiveLayout(state) == layout) { + if state, ok, err := skillscheck.ReadState(); err == nil && ok && versioncheck.Equal(state.Version, stateVersion) { + if !state.OfficialSkillsUnknown && skillscheck.MatchesSource(state, skillscheck.OfficialSourceIdentity) && + (layout == "" || skillscheck.EffectiveLayout(state) == layout) { return nil } } @@ -498,7 +517,8 @@ func applySkillsStatus(env map[string]interface{}, target string) { status := map[string]interface{}{ "current": state.Version, "target": target, - "in_sync": normalizeVersion(state.Version) == normalizeVersion(target) && !state.OfficialSkillsUnknown, + "in_sync": versioncheck.Equal(state.Version, target) && + !state.OfficialSkillsUnknown && skillscheck.MatchesSource(state, skillscheck.OfficialSourceIdentity), } if state.OfficialSkillsUnknown { status["official_unknown"] = true diff --git a/cmd/update/update_test.go b/cmd/update/update_test.go index 0fab1c2145..050bed7cb3 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -4,21 +4,28 @@ package cmdupdate import ( + "archive/zip" "bytes" "context" + "crypto/sha256" "encoding/json" "errors" "fmt" + "net/http" + "net/http/httptest" "os" "os/exec" "path/filepath" + "runtime" "strings" "testing" "time" "github.com/larksuite/cli/errs" + exttransport "github.com/larksuite/cli/extension/transport" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/distribution" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/selfupdate" "github.com/larksuite/cli/internal/skillscheck" @@ -27,6 +34,182 @@ import ( const runLiveSkillsTestsEnv = "LARKSUITE_CLI_RUN_LIVE_SKILLS_TESTS" +type updateManifestProvider struct { + manifestURL string + onResolve func(context.Context) +} + +func (p updateManifestProvider) Name() string { return "test-manifest" } +func (p updateManifestProvider) ResolveInterceptor(context.Context) exttransport.Interceptor { + return nil +} +func (p updateManifestProvider) ResolveManifestURL(ctx context.Context) string { + if p.onResolve != nil { + p.onResolve(ctx) + } + return p.manifestURL +} + +func TestUpdateCommandPreservesCancellationContext(t *testing.T) { + type contextKey struct{} + ctx := context.WithValue(context.Background(), contextKey{}, "command") + ctx, cancel := context.WithCancel(ctx) + cancel() + previousProvider := exttransport.GetProvider() + exttransport.Register(updateManifestProvider{ + manifestURL: "://invalid", + onResolve: func(resolved context.Context) { + if resolved.Value(contextKey{}) != "command" || !errors.Is(resolved.Err(), context.Canceled) { + t.Error("distribution provider did not receive the command context") + } + }, + }) + t.Cleanup(func() { exttransport.Register(previousProvider) }) + + factory, _, _ := newTestFactory(t) + cmd := NewCmdUpdate(factory) + cmd.SetContext(ctx) + if err := cmd.Execute(); err == nil { + t.Fatal("update succeeded with an invalid manifest URL") + } +} + +func TestManifestCheckAcceptsHTTPAndReportsOpaqueDowngradeTarget(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, `{"schema":1,"version":"older-channel","artifacts":{"skills":{"url":"https://dist.example/skills","checksum":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},%q:{"url":"https://dist.example/binary","checksum":"sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"}}}`, runtime.GOOS+"-"+runtime.GOARCH) + })) + defer server.Close() + previousProvider := exttransport.GetProvider() + previousClient := distribution.DefaultClient + previousVersion := currentVersion + exttransport.Register(updateManifestProvider{manifestURL: server.URL}) + distribution.DefaultClient = server.Client() + currentVersion = func() string { return "newer-channel" } + t.Cleanup(func() { + exttransport.Register(previousProvider) + distribution.DefaultClient = previousClient + currentVersion = previousVersion + }) + + factory, stdout, _ := newTestFactory(t) + err := updateRunWithContext(context.Background(), &UpdateOptions{Factory: factory, JSON: true, Check: true}) + if err != nil { + t.Fatal(err) + } + var got map[string]interface{} + if err := json.Unmarshal(stdout.Bytes(), &got); err != nil { + t.Fatal(err) + } + if got["action"] != "update_available" || got["target_version"] != "older-channel" || got["source"] != "manifest" { + t.Fatalf("output = %#v", got) + } + if _, exists := got["latest_version"]; exists { + t.Fatalf("manifest output must not label an arbitrary target as latest: %#v", got) + } +} + +func TestManifestArtifactProtocolFailureUsesNetworkTaxonomy(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + payload := []byte("not an archive") + digest := fmt.Sprintf("sha256:%x", sha256.Sum256(payload)) + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/manifest.json" { + _, _ = w.Write(payload) + return + } + artifactURL := server.URL + "/artifact" + fmt.Fprintf(w, `{"schema":1,"version":"target","artifacts":{"skills":{"url":%q,"checksum":%q},%q:{"url":%q,"checksum":%q}}}`, + artifactURL, digest, distribution.CurrentPlatformKey(), artifactURL, digest) + })) + defer server.Close() + + previousProvider := exttransport.GetProvider() + previousClient := distribution.DefaultClient + previousVersion := currentVersion + exttransport.Register(updateManifestProvider{manifestURL: server.URL + "/manifest.json"}) + distribution.DefaultClient = server.Client() + currentVersion = func() string { return "current" } + t.Cleanup(func() { + exttransport.Register(previousProvider) + distribution.DefaultClient = previousClient + currentVersion = previousVersion + }) + + factory, stdout, _ := newTestFactory(t) + if err := updateRunWithContext(context.Background(), &UpdateOptions{Factory: factory, JSON: true}); err == nil { + t.Fatal("update succeeded") + } + var got map[string]interface{} + if err := json.Unmarshal(stdout.Bytes(), &got); err != nil { + t.Fatal(err) + } + problem, _ := got["error"].(map[string]interface{}) + if problem["type"] != "network" { + t.Fatalf("output = %#v", got) + } +} + +func TestManifestUpdateRepairsSkillsWhenBinaryMatches(t *testing.T) { + root := t.TempDir() + t.Setenv("HOME", root) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", filepath.Join(root, "config")) + var archive bytes.Buffer + writer := zip.NewWriter(&archive) + entry, err := writer.Create("lark-approval/SKILL.md") + if err != nil { + t.Fatal(err) + } + if _, err := entry.Write([]byte("repaired")); err != nil { + t.Fatal(err) + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + digest := fmt.Sprintf("sha256:%x", sha256.Sum256(archive.Bytes())) + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/skills.zip" { + _, _ = w.Write(archive.Bytes()) + return + } + fmt.Fprintf(w, `{"schema":1,"version":"same","artifacts":{"skills":{"url":%q,"checksum":%q},%q:{"url":%q,"checksum":%q}}}`, + server.URL+"/skills.zip", digest, distribution.CurrentPlatformKey(), server.URL+"/unused.zip", digest) + })) + defer server.Close() + previousProvider := exttransport.GetProvider() + previousClient := distribution.DefaultClient + previousVersion := currentVersion + exttransport.Register(updateManifestProvider{manifestURL: server.URL + "/manifest.json"}) + distribution.DefaultClient = server.Client() + currentVersion = func() string { return "same" } + t.Cleanup(func() { + exttransport.Register(previousProvider) + distribution.DefaultClient = previousClient + currentVersion = previousVersion + }) + + factory, _, _ := newTestFactory(t) + if err := updateRunWithContext(context.Background(), &UpdateOptions{Factory: factory, JSON: true}); err != nil { + t.Fatal(err) + } + skillDir := filepath.Join(root, ".agents", "skills", "lark-approval") + if err := os.RemoveAll(skillDir); err != nil { + t.Fatal(err) + } + + factory, stdout, _ := newTestFactory(t) + if err := updateRunWithContext(context.Background(), &UpdateOptions{Factory: factory, JSON: true}); err != nil { + t.Fatal(err) + } + if got, err := os.ReadFile(filepath.Join(skillDir, "SKILL.md")); err != nil || string(got) != "repaired" { + t.Fatalf("repaired Skill = %q, %v", got, err) + } + if !strings.Contains(stdout.String(), `"skills_action": "synced"`) { + t.Fatalf("output = %s", stdout.String()) + } +} + // newTestFactory creates a test factory with minimal config. func newTestFactory(t *testing.T) (*cmdutil.Factory, *bytes.Buffer, *bytes.Buffer) { t.Helper() @@ -243,25 +426,6 @@ func TestUpdatePnpm_Unavailable_ManualFallback(t *testing.T) { } } -func TestNormalizeVersion(t *testing.T) { - tests := []struct { - input string - want string - }{ - {input: "1.2.3", want: "1.2.3"}, - {input: "v1.2.3", want: "1.2.3"}, - {input: "V1.2.3", want: "1.2.3"}, - {input: " v1.2.3 ", want: "1.2.3"}, - } - for _, tt := range tests { - t.Run(tt.input, func(t *testing.T) { - if got := normalizeVersion(tt.input); got != tt.want { - t.Fatalf("normalizeVersion(%q) = %q, want %q", tt.input, got, tt.want) - } - }) - } -} - func TestUpdateAlreadyUpToDate_JSON(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) mockSkillsSync(t) @@ -1267,6 +1431,26 @@ func TestRunSkillsAndState_UnknownOfficialSkillsBypassesVersionDedup(t *testing. } } +func TestRunSkillsAndState_ManifestSourceBypassesVersionDedup(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := skillscheck.WriteState(skillscheck.SkillsState{ + Version: "1.0.21", + SourceIdentity: "manifest:test", + }); err != nil { + t.Fatal(err) + } + originalSync := syncSkills + t.Cleanup(func() { syncSkills = originalSync }) + called := false + syncSkills = func(skillscheck.SyncOptions) *skillscheck.SyncResult { + called = true + return &skillscheck.SyncResult{Action: "synced"} + } + if got := runSkillsAndState(&selfupdate.Updater{}, newTestIO(), "1.0.21", false, ""); !called || got == nil { + t.Fatalf("runSkillsAndState() = %+v, called = %v", got, called) + } +} + func TestSkillsSummaryMarksUnknownOfficialSkills(t *testing.T) { summary := skillsSummary(&skillscheck.SyncResult{ Layout: skillscheck.LayoutSeparate, diff --git a/extension/README.md b/extension/README.md index 30ea3ade06..fd9e13fc8b 100644 --- a/extension/README.md +++ b/extension/README.md @@ -7,7 +7,7 @@ Main extension points: | Package | Extension point | What it does | | ------- | --------------- | ------------ | | [`credential/`](./credential/) | **Credential** | Bring your own credential source: database, Vault, config center… | -| [`transport/`](./transport/) | **Transport** | Intercept HTTP requests and rewrite CLI-owned network, presentation, and child-process URLs | +| [`transport/`](./transport/) | **Transport** | Register one aggregate provider for request interception, URL rewriting, and an optional distribution manifest | | [`platform/`](./platform/) | **Restrict · Observer · Wrap · On** | Command allow/deny rules, audit hooks, onion-style middleware (approval gates, rate limiting), process lifecycle — see the [Plugin SDK README](./platform/README.md) | 📖 Full guide: [Embed lark-cli in your Agent](https://open.larksuite.com/document/mcp_open_tools/feishu-cli/embed-feishu-cli-in-agent) ([中文](https://open.larkoffice.com/document/mcp_open_tools/feishu-cli/embed-feishu-cli-in-agent)) @@ -17,3 +17,34 @@ provider during `init`, before constructing or executing the CLI. URL rewriting runs before the request interceptor and also covers CLI-owned presentation URLs and URLs passed to child processes. `ScopedProvider` limits only the request interceptor. + +When `DistributionProvider` returns a manifest URL, that URL and the artifact +URLs inside the manifest are final download addresses. Distribution downloads +retain lark-cli's built-in proxy and custom-CA policy, but deliberately bypass +the registered URL rewriter and request interceptor. + +## Distribution manifest protocol + +The manifest is JSON with this fixed schema: + +```json +{ + "schema": 1, + "version": "1.2.3", + "artifacts": { + "darwin-arm64": { "url": "https://dist.example/lark-cli-darwin-arm64.tar.gz", "checksum": "sha256:<64 lowercase hex characters>" }, + "skills": { "url": "https://dist.example/skills.tar.gz", "checksum": "sha256:<64 lowercase hex characters>" } + } +} +``` + +`schema`, `version`, and `artifacts` are required; unknown fields are ignored. +`version` is an exact opaque target; the staged binary is verified by running +` --version`, whose output must be exactly `lark-cli version `. +`artifacts` must contain `skills` and a key named +`-` for every published platform. URLs must be absolute HTTP or +HTTPS URLs. Checksums cover the downloaded archive bytes. + +Archives may be zip or gzip-compressed tar files. A binary archive contains +`lark-cli` at its root (`lark-cli.exe` on Windows). A Skills archive contains +one directory per Skill at its root, for example `lark-doc/SKILL.md`. diff --git a/extension/transport/registry.go b/extension/transport/registry.go index d7ccf126e5..fc45c0836c 100644 --- a/extension/transport/registry.go +++ b/extension/transport/registry.go @@ -12,11 +12,13 @@ var ( // Register sets the process-wide transport Provider. // -// Integrations that need multiple capabilities compose them in one Provider -// and register it during init, before command construction or execution. Later -// registrations replace the earlier Provider for backward compatibility; -// changing the Provider while the CLI is running is unsupported because -// clients may already hold a resolved interceptor or URL rewriter. +// lark-cli supports one aggregate Provider for request interception, URL +// rewriting, and distribution configuration. Integrations that need multiple +// capabilities compose them in that Provider and register it during init, +// before command construction or execution. Later registrations replace the +// earlier Provider for backward compatibility; changing the Provider while the +// CLI is running is unsupported because clients may already hold a resolved +// interceptor or URL rewriter. func Register(p Provider) { mu.Lock() defer mu.Unlock() diff --git a/extension/transport/registry_test.go b/extension/transport/registry_test.go index 836cbca14f..2b07cde9d6 100644 --- a/extension/transport/registry_test.go +++ b/extension/transport/registry_test.go @@ -22,6 +22,15 @@ type stubProvider struct { func (s *stubProvider) Name() string { return s.name } func (s *stubProvider) ResolveInterceptor(context.Context) Interceptor { return &stubInterceptor{} } +type stubDistributionProvider struct { + stubProvider + manifestURL string +} + +func (s *stubDistributionProvider) ResolveManifestURL(context.Context) string { + return s.manifestURL +} + func TestGetProvider_NilByDefault(t *testing.T) { mu.Lock() provider = nil @@ -75,3 +84,20 @@ func TestResolveInterceptor_ReturnsNonNil(t *testing.T) { t.Fatal("expected non-nil Interceptor") } } + +func TestDistributionProviderIsOptional(t *testing.T) { + previous := GetProvider() + t.Cleanup(func() { Register(previous) }) + p := &stubDistributionProvider{ + stubProvider: stubProvider{name: "distribution"}, + manifestURL: "https://dist.example/manifest.json", + } + Register(p) + configured, ok := GetProvider().(DistributionProvider) + if !ok { + t.Fatal("registered provider does not implement DistributionProvider") + } + if got := configured.ResolveManifestURL(context.Background()); got != p.manifestURL { + t.Fatalf("ManifestURL = %q", got) + } +} diff --git a/extension/transport/types.go b/extension/transport/types.go index 4969205c9e..a69d55660f 100644 --- a/extension/transport/types.go +++ b/extension/transport/types.go @@ -31,6 +31,22 @@ type URLRewriterProvider interface { ResolveURLRewriter(ctx context.Context) URLRewriter } +// DistributionProvider optionally supplies a distribution manifest in +// addition to the existing request interceptor. Providers that do not +// implement this interface, or return an empty URL, retain the package-manager +// update flow. +// Manifest and artifact URLs are final download addresses; the CLI does not +// pass them through URL rewriting or the request interceptor. HTTP is supported +// for trusted distribution networks; the provider is responsible for transport +// integrity when it does not use HTTPS. +// ResolveManifestURL must be a fast, local lookup. Manifest fetching, parsing, +// and artifact installation are owned by the CLI. The complete public wire +// contract is documented in extension/README.md. +type DistributionProvider interface { + Provider + ResolveManifestURL(ctx context.Context) string +} + // RequestClass describes the trust boundary of an outbound HTTP request. // Platform requests target endpoints owned by the CLI's endpoint resolver; // external requests target user-provided, pre-signed, CDN, registry, or other diff --git a/go.mod b/go.mod index 8839b7fc71..1c100618bc 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/tidwall/gjson v1.18.0 github.com/zalando/go-keyring v0.2.8 golang.org/x/image v0.30.0 + golang.org/x/mod v0.27.0 golang.org/x/net v0.33.0 golang.org/x/sync v0.16.0 golang.org/x/sys v0.33.0 diff --git a/go.sum b/go.sum index 574157832b..d2c198188a 100644 --- a/go.sum +++ b/go.sum @@ -143,6 +143,8 @@ golang.org/x/image v0.30.0 h1:jD5RhkmVAnjqaCUXfbGBrn3lpxbknfN9w2UhHHU+5B4= golang.org/x/image v0.30.0/go.mod h1:SAEUTxCCMWSrJcCy/4HwavEsfZZJlYxeHLc6tTiAe/c= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.27.0 h1:kb+q2PyFnEADO2IEF935ehFUXlWiNjJWtRNgBLSfbxQ= +golang.org/x/mod v0.27.0/go.mod h1:rWI627Fq0DEoudcK+MBkNkCe0EetEaDSwJJkCcjpazc= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= diff --git a/internal/distribution/archive.go b/internal/distribution/archive.go new file mode 100644 index 0000000000..42cb0db933 --- /dev/null +++ b/internal/distribution/archive.go @@ -0,0 +1,181 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package distribution + +import ( + "archive/tar" + "archive/zip" + "compress/gzip" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/larksuite/cli/internal/vfs" +) + +// artifactExtractedMaxBytes allows expected expansion while bounding the +// temporary disk consumed by one bundle. +const artifactExtractedMaxBytes int64 = 8 << 30 + +// archiveExtractor accumulates extracted size and owns the shared per-entry +// policy: entries must stay under the root, extracted bytes are bounded, and +// file permissions are normalized. +type archiveExtractor struct { + destination string + maxBytes int64 + total int64 +} + +// extractArchive extracts a .tar.gz or .zip bundle into destination. +func extractArchive(archivePath, destination string) error { + return extractArchiveWithLimit(archivePath, destination, artifactExtractedMaxBytes) +} + +func extractArchiveWithLimit(archivePath, destination string, maxBytes int64) error { + file, err := vfs.Open(archivePath) + if err != nil { + return err + } + defer file.Close() + + header := make([]byte, 4) + n, readErr := io.ReadFull(file, header) + if readErr != nil && readErr != io.ErrUnexpectedEOF { + return readErr + } + if _, err := file.Seek(0, io.SeekStart); err != nil { + return err + } + + extractor := &archiveExtractor{destination: destination, maxBytes: maxBytes} + switch { + case n >= 2 && header[0] == 0x1f && header[1] == 0x8b: + return extractor.extractTarGzip(file) + case n >= 4 && string(header[:4]) == "PK\x03\x04": + return extractor.extractZip(file) + default: + return fmt.Errorf("unsupported distribution archive format") + } +} + +func (e *archiveExtractor) extractTarGzip(source io.Reader) error { + gzipReader, err := gzip.NewReader(source) + if err != nil { + return err + } + defer gzipReader.Close() + + reader := tar.NewReader(gzipReader) + for { + header, err := reader.Next() + if err == io.EOF { + return nil + } + if err != nil { + return err + } + switch header.Typeflag { + case tar.TypeDir: + if err := e.mkdir(header.Name); err != nil { + return err + } + case tar.TypeReg, tar.TypeRegA: + if err := e.writeFile(header.Name, header.FileInfo().Mode(), header.Size, reader); err != nil { + return err + } + } + } +} + +func (e *archiveExtractor) extractZip(file *os.File) error { + info, err := file.Stat() + if err != nil { + return err + } + reader, err := zip.NewReader(file, info.Size()) + if err != nil { + return err + } + for _, entry := range reader.File { + switch { + case entry.FileInfo().IsDir(): + if err := e.mkdir(entry.Name); err != nil { + return err + } + case entry.Mode().IsRegular(): + source, err := entry.Open() + if err != nil { + return err + } + writeErr := e.writeFile(entry.Name, entry.Mode(), int64(entry.UncompressedSize64), source) + closeErr := source.Close() + if writeErr != nil { + return writeErr + } + if closeErr != nil { + return closeErr + } + } + } + return nil +} + +func (e *archiveExtractor) mkdir(name string) error { + target, err := archiveEntryPath(e.destination, name) + if err != nil { + return err + } + return vfs.MkdirAll(target, 0o755) +} + +func (e *archiveExtractor) writeFile(name string, mode os.FileMode, size int64, source io.Reader) error { + if size < 0 || size > e.maxBytes-e.total { + return fmt.Errorf("extracted artifact exceeds %d bytes", e.maxBytes) + } + target, err := archiveEntryPath(e.destination, name) + if err != nil { + return err + } + if err := vfs.MkdirAll(filepath.Dir(target), 0o755); err != nil { + return err + } + perm := mode.Perm() + if perm&0o111 != 0 { + perm = 0o755 + } else { + perm = 0o644 + } + file, err := vfs.OpenFile(target, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, perm) + if err != nil { + return err + } + _, copyErr := io.Copy(file, source) + closeErr := file.Close() + if copyErr != nil { + return copyErr + } + if closeErr != nil { + return closeErr + } + e.total += size + return nil +} + +// archiveEntryPath maps an archive entry name to a path under root, rejecting +// absolute paths and ".." traversal. +func archiveEntryPath(root, name string) (string, error) { + localName := filepath.FromSlash(name) + if filepath.IsAbs(localName) || filepath.VolumeName(localName) != "" { + return "", fmt.Errorf("archive entry %q escapes the extraction root", name) + } + root = filepath.Clean(root) + target := filepath.Join(root, localName) + relative, err := filepath.Rel(root, target) + if err != nil || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("archive entry %q escapes the extraction root", name) + } + return target, nil +} diff --git a/internal/distribution/archive_test.go b/internal/distribution/archive_test.go new file mode 100644 index 0000000000..7891d1da14 --- /dev/null +++ b/internal/distribution/archive_test.go @@ -0,0 +1,138 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package distribution + +import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/gzip" + "errors" + "io/fs" + "path/filepath" + "strings" + "testing" + + "github.com/larksuite/cli/internal/vfs" +) + +func TestExtractArchiveFormats(t *testing.T) { + tests := []struct { + name string + build func(*testing.T, string) + }{ + {"tar.gz", writeTestTarGzip}, + {"zip", writeTestZip}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := t.TempDir() + archive := filepath.Join(root, "artifact") + tt.build(t, archive) + destination := filepath.Join(root, "out") + if err := extractArchive(archive, destination); err != nil { + t.Fatal(err) + } + got, err := vfs.ReadFile(filepath.Join(destination, "skill", "SKILL.md")) + if err != nil { + t.Fatal(err) + } + if string(got) != "content" { + t.Fatalf("content = %q", got) + } + }) + } +} + +func TestExtractArchiveRejectsEntriesOutsideDestination(t *testing.T) { + for _, tt := range []struct { + name string + build func(*testing.T, string, string) + }{ + {"tar.gz", writeTestTarGzipEntry}, + {"zip", writeTestZipEntry}, + } { + t.Run(tt.name, func(t *testing.T) { + root := t.TempDir() + archive := filepath.Join(root, "artifact") + tt.build(t, archive, "../escape") + if err := extractArchive(archive, filepath.Join(root, "out")); err == nil { + t.Fatal("extractArchive succeeded") + } + if _, err := vfs.Stat(filepath.Join(root, "escape")); !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("archive wrote outside destination: %v", err) + } + }) + } +} + +func TestExtractArchiveRejectsExcessiveExpandedSize(t *testing.T) { + for _, tt := range []struct { + name string + build func(*testing.T, string) + }{ + {"tar.gz", writeTestTarGzip}, + {"zip", writeTestZip}, + } { + t.Run(tt.name, func(t *testing.T) { + root := t.TempDir() + archive := filepath.Join(root, "artifact") + tt.build(t, archive) + err := extractArchiveWithLimit(archive, filepath.Join(root, "out"), 6) + if err == nil || !strings.Contains(err.Error(), "exceeds 6 bytes") { + t.Fatalf("err = %v", err) + } + }) + } +} + +func writeTestTarGzip(t *testing.T, path string) { + writeTestTarGzipEntry(t, path, "skill/SKILL.md") +} + +func writeTestTarGzipEntry(t *testing.T, path, name string) { + t.Helper() + var data bytes.Buffer + gz := gzip.NewWriter(&data) + tw := tar.NewWriter(gz) + content := []byte("content") + if err := tw.WriteHeader(&tar.Header{Name: name, Mode: 0o644, Size: int64(len(content))}); err != nil { + t.Fatal(err) + } + if _, err := tw.Write(content); err != nil { + t.Fatal(err) + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + if err := gz.Close(); err != nil { + t.Fatal(err) + } + if err := vfs.WriteFile(path, data.Bytes(), 0o600); err != nil { + t.Fatal(err) + } +} + +func writeTestZip(t *testing.T, path string) { + writeTestZipEntry(t, path, "skill/SKILL.md") +} + +func writeTestZipEntry(t *testing.T, path, name string) { + t.Helper() + var data bytes.Buffer + zw := zip.NewWriter(&data) + entry, err := zw.Create(name) + if err != nil { + t.Fatal(err) + } + if _, err := entry.Write([]byte("content")); err != nil { + t.Fatal(err) + } + if err := zw.Close(); err != nil { + t.Fatal(err) + } + if err := vfs.WriteFile(path, data.Bytes(), 0o600); err != nil { + t.Fatal(err) + } +} diff --git a/internal/distribution/download.go b/internal/distribution/download.go new file mode 100644 index 0000000000..e935129ac4 --- /dev/null +++ b/internal/distribution/download.go @@ -0,0 +1,78 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package distribution + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "strings" + "time" + + "github.com/larksuite/cli/extension/download" + "github.com/larksuite/cli/internal/downloadtransport" + "github.com/larksuite/cli/internal/vfs" +) + +// These ceilings bound temporary disk use while leaving ample room for the +// CLI and Skills bundles. Raise them only when a supported bundle outgrows +// the current distribution contract. +const ( + artifactDownloadMaxBytes int64 = 4 << 30 + artifactDownloadTimeout = 10 * time.Minute +) + +func downloadArtifact(ctx context.Context, artifact Artifact, directory, pattern string) (string, error) { + ctx, cancel := context.WithTimeout(ctx, artifactDownloadTimeout) + defer cancel() + return downloadArtifactWithLimit(ctx, artifact, directory, pattern, artifactDownloadMaxBytes) +} + +func downloadArtifactWithLimit(ctx context.Context, artifact Artifact, directory, pattern string, maxBytes int64) (string, error) { + stream, err := download.Open( + ctx, + download.ImmutableSource(downloadtransport.URL(httpClient(), artifact.URL)), + download.Options{}, + ) + if err != nil { + return "", err + } + defer stream.Body.Close() + if stream.ContentLength > maxBytes { + return "", fmt.Errorf("artifact download exceeds %d bytes", maxBytes) + } + temporary, err := vfs.CreateTemp(directory, pattern) + if err != nil { + return "", err + } + path := temporary.Name() + keep := false + defer func() { + _ = temporary.Close() + if !keep { + _ = vfs.Remove(path) + } + }() + + hash := sha256.New() + written, err := io.Copy(io.MultiWriter(temporary, hash), io.LimitReader(stream.Body, maxBytes+1)) + if err != nil { + return "", err + } + if written > maxBytes { + return "", fmt.Errorf("artifact download exceeds %d bytes", maxBytes) + } + if err := temporary.Close(); err != nil { + return "", err + } + want := strings.TrimPrefix(artifact.Checksum, "sha256:") + got := hex.EncodeToString(hash.Sum(nil)) + if got != want { + return "", fmt.Errorf("artifact checksum mismatch") + } + keep = true + return path, nil +} diff --git a/internal/distribution/download_test.go b/internal/distribution/download_test.go new file mode 100644 index 0000000000..dc0e183936 --- /dev/null +++ b/internal/distribution/download_test.go @@ -0,0 +1,60 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package distribution + +import ( + "context" + "crypto/sha256" + "fmt" + "io" + "net/http" + "strings" + "testing" + "time" +) + +func TestDownloadArtifactRejectsExcessiveBody(t *testing.T) { + previousClient := DefaultClient + DefaultClient = &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader("123456789")), + ContentLength: -1, + Header: make(http.Header), + }, nil + })} + t.Cleanup(func() { DefaultClient = previousClient }) + + _, err := downloadArtifactWithLimit(context.Background(), Artifact{ + URL: "https://dist.example/artifact", Checksum: testChecksum, + }, t.TempDir(), "artifact-*", 8) + if err == nil || !strings.Contains(err.Error(), "exceeds 8 bytes") { + t.Fatalf("err = %v", err) + } +} + +func TestDownloadArtifactAppliesTenMinuteDeadline(t *testing.T) { + payload := []byte("artifact") + previousClient := DefaultClient + DefaultClient = &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + deadline, ok := req.Context().Deadline() + if !ok { + t.Fatal("artifact request has no deadline") + } + remaining := time.Until(deadline) + if remaining < 9*time.Minute || remaining > artifactDownloadTimeout { + t.Fatalf("artifact deadline remaining = %s, want about %s", remaining, artifactDownloadTimeout) + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(string(payload))), + Header: make(http.Header), + }, nil + })} + t.Cleanup(func() { DefaultClient = previousClient }) + checksum := fmt.Sprintf("sha256:%x", sha256.Sum256(payload)) + if _, err := downloadArtifact(context.Background(), Artifact{URL: "https://dist.example/artifact", Checksum: checksum}, t.TempDir(), "artifact-*"); err != nil { + t.Fatal(err) + } +} diff --git a/internal/distribution/errors_test.go b/internal/distribution/errors_test.go new file mode 100644 index 0000000000..59265c9613 --- /dev/null +++ b/internal/distribution/errors_test.go @@ -0,0 +1,35 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package distribution + +import ( + "errors" + "os" + "testing" + + "github.com/larksuite/cli/errs" +) + +func TestClassifyArtifactError(t *testing.T) { + for _, tt := range []struct { + name string + err error + category errs.Category + subtype errs.Subtype + }{ + {name: "file IO", err: &os.PathError{Op: "mkdir", Path: "/tmp/config", Err: os.ErrPermission}, category: errs.CategoryInternal, subtype: errs.SubtypeFileIO}, + {name: "bad archive", err: errors.New("unsupported archive format"), category: errs.CategoryNetwork, subtype: errs.SubtypeNetworkProtocol}, + } { + t.Run(tt.name, func(t *testing.T) { + got := classifyArtifactError("extract", "skills", tt.err) + problem, ok := errs.ProblemOf(got) + if !ok || problem.Category != tt.category || problem.Subtype != tt.subtype { + t.Fatalf("problem = %#v, want category=%q subtype=%q", problem, tt.category, tt.subtype) + } + if !errors.Is(got, tt.err) { + t.Fatalf("cause %v was not preserved", tt.err) + } + }) + } +} diff --git a/internal/distribution/install.go b/internal/distribution/install.go new file mode 100644 index 0000000000..1c6afc9ff1 --- /dev/null +++ b/internal/distribution/install.go @@ -0,0 +1,241 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package distribution + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "runtime" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/lockfile" + "github.com/larksuite/cli/internal/selfupdate" + "github.com/larksuite/cli/internal/skillscheck" + "github.com/larksuite/cli/internal/vfs" +) + +// InstallOptions supplies destinations and test seams for a distribution update. +type InstallOptions struct { + ExecutablePath string + // SkillsDir overrides automatic Agent directory discovery when non-empty. + SkillsDir string + VerifyBinary func(path, version string) error +} + +// Install downloads, verifies, and commits the configured Skills and binary +// resources as one rollback-capable local transaction. The executable is +// committed last. +func Install(ctx context.Context, manifest *Manifest, opts InstallOptions) errs.TypedError { + prepared, typedErr := prepareUpdate(ctx, manifest) + if typedErr != nil { + return typedErr + } + defer prepared.cleanup() + if err := installPrepared(prepared, opts); err != nil { + return installError("failed to install distribution update", err) + } + return nil +} + +// SyncSkills repairs the managed Skills from a manifest without replacing an +// already-matching binary. +func SyncSkills(ctx context.Context, manifest *Manifest, opts InstallOptions) errs.TypedError { + if manifest == nil { + return errs.NewInternalError(errs.SubtypeUnknown, "distribution manifest is nil") + } + if err := vfs.MkdirAll(core.GetBaseConfigDir(), 0o700); err != nil { + return prepareFileError(err) + } + root, err := vfs.MkdirTemp(core.GetBaseConfigDir(), ".distribution-skills-*") + if err != nil { + return prepareFileError(err) + } + defer func() { _ = vfs.RemoveAll(root) }() + + skillsRoot, typedErr := prepareArtifact(ctx, manifest, SkillsKey, root, "skills") + if typedErr != nil { + return typedErr + } + if err := withInstallLock(func() error { + _, finalize, err := syncPreparedSkills(skillsRoot, manifest, opts.SkillsDir) + if err == nil { + finalize() + } + return err + }); err != nil { + return installError("failed to synchronize distribution Skills", err) + } + return nil +} + +// installError classifies commit-stage failures. Lock contention means a +// concurrent update owns the transaction; everything else gets the generic +// retry hint. +func installError(message string, err error) errs.TypedError { + if errors.Is(err, lockfile.ErrHeld) { + return errs.NewValidationError(errs.SubtypeFailedPrecondition, + "another lark-cli update is already running"). + WithHint("Wait for it to finish, then retry."). + WithCause(err) + } + return errs.NewInternalError(errs.SubtypeUnknown, "%s: %s", message, err). + WithHint("Retry with `lark-cli update --force`."). + WithCause(err) +} + +func installPrepared(prepared *preparedUpdate, opts InstallOptions) error { + if prepared == nil || prepared.Manifest == nil { + return fmt.Errorf("prepared distribution update is required") + } + return withInstallLock(func() error { return installPreparedLocked(prepared, opts) }) +} + +func installPreparedLocked(prepared *preparedUpdate, opts InstallOptions) error { + candidate, err := selfupdate.PrepareCandidate( + prepared.BinaryPath, + opts.ExecutablePath, + prepared.Manifest.Version, + opts.VerifyBinary, + ) + if err != nil { + return fmt.Errorf("prepare binary: %w", err) + } + defer candidate.Cleanup() + + rollbackSkills, finalizeSkills, err := syncPreparedSkills( + prepared.SkillsRoot, + prepared.Manifest, + opts.SkillsDir, + ) + if err != nil { + return err + } + finalizeBinary, err := candidate.Install() + if err != nil { + cause := fmt.Errorf("replace binary: %w", err) + if rollbackErr := rollbackSkills(); rollbackErr != nil { + return fmt.Errorf("%w (Skills rollback failed: %w)", cause, rollbackErr) + } + return cause + } + finalizeSkills() + finalizeBinary() + return nil +} + +func syncPreparedSkills(root string, manifest *Manifest, targetDir string) (func() error, func(), error) { + return skillscheck.SyncPreparedTree(skillscheck.PreparedTreeOptions{ + Root: root, + Version: manifest.Version, + SourceIdentity: manifest.sourceIdentity, + TargetDir: targetDir, + }) +} + +func withInstallLock(fn func() error) error { + if err := vfs.MkdirAll(core.GetBaseConfigDir(), 0o700); err != nil { + return err + } + lock := lockfile.New(filepath.Join(core.GetBaseConfigDir(), "distribution-update.lock")) + if err := lock.TryLock(); err != nil { + return fmt.Errorf("acquire distribution update lock: %w", err) + } + defer func() { _ = lock.Unlock() }() + return fn() +} + +// preparedUpdate contains fully downloaded, checksum-verified, extracted +// resources owned by one Install call. +type preparedUpdate struct { + Manifest *Manifest + BinaryPath string + SkillsRoot string + root string +} + +// cleanup removes downloaded and extracted temporary resources. +func (p *preparedUpdate) cleanup() { + if p != nil && p.root != "" { + _ = vfs.RemoveAll(p.root) + } +} + +// prepareUpdate downloads and validates every resource before installed state +// is mutated. +func prepareUpdate(ctx context.Context, manifest *Manifest) (*preparedUpdate, errs.TypedError) { + if manifest == nil { + return nil, errs.NewInternalError(errs.SubtypeUnknown, "distribution manifest is nil") + } + if err := vfs.MkdirAll(core.GetBaseConfigDir(), 0o700); err != nil { + return nil, prepareFileError(err) + } + root, err := vfs.MkdirTemp(core.GetBaseConfigDir(), ".distribution-update-*") + if err != nil { + return nil, prepareFileError(err) + } + prepared := &preparedUpdate{Manifest: manifest, root: root} + keep := false + defer func() { + if !keep { + prepared.cleanup() + } + }() + + binaryRoot, typedErr := prepareArtifact(ctx, manifest, CurrentPlatformKey(), root, "binary") + if typedErr != nil { + return nil, typedErr + } + executableName := "lark-cli" + if runtime.GOOS == "windows" { + executableName += ".exe" + } + prepared.BinaryPath = filepath.Join(binaryRoot, executableName) + info, err := vfs.Stat(prepared.BinaryPath) + if err != nil || !info.Mode().IsRegular() { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "binary artifact must contain %s at its root", executableName) + } + prepared.SkillsRoot, typedErr = prepareArtifact(ctx, manifest, SkillsKey, root, "skills") + if typedErr != nil { + return nil, typedErr + } + keep = true + return prepared, nil +} + +func prepareArtifact(ctx context.Context, manifest *Manifest, key, root, directory string) (string, errs.TypedError) { + archive, err := downloadArtifact(ctx, manifest.Artifacts[key], root, directory+"-*.archive") + if err != nil { + return "", classifyArtifactError("download", key, err) + } + destination := filepath.Join(root, directory) + if err := vfs.MkdirAll(destination, 0o700); err != nil { + return "", prepareFileError(err) + } + if err := extractArchive(archive, destination); err != nil { + return "", classifyArtifactError("extract", key, err) + } + return destination, nil +} + +// classifyArtifactError attributes an artifact-stage failure: local file I/O +// is FileIO; fetch, size-limit, checksum, and archive-format failures mean the +// delivered artifact is missing or broken and are reported as network/protocol. +func classifyArtifactError(stage, key string, err error) errs.TypedError { + var pathErr *os.PathError + if errors.As(err, &pathErr) { + return prepareFileError(err) + } + return errs.NewNetworkError(errs.SubtypeNetworkProtocol, "failed to %s %s artifact: %s", stage, key, err). + WithCause(err) +} + +func prepareFileError(err error) errs.TypedError { + return errs.NewInternalError(errs.SubtypeFileIO, "failed to prepare distribution update: %s", err). + WithCause(err) +} diff --git a/internal/distribution/install_test.go b/internal/distribution/install_test.go new file mode 100644 index 0000000000..81f3717338 --- /dev/null +++ b/internal/distribution/install_test.go @@ -0,0 +1,294 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package distribution + +import ( + "archive/zip" + "bytes" + "context" + "crypto/sha256" + "errors" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/lockfile" + "github.com/larksuite/cli/internal/skillscheck" + "github.com/larksuite/cli/internal/vfs" +) + +func TestInstallPreparedRejectsConcurrentUpdate(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := vfs.MkdirAll(core.GetBaseConfigDir(), 0o700); err != nil { + t.Fatal(err) + } + lock := lockfile.New(filepath.Join(core.GetBaseConfigDir(), "distribution-update.lock")) + if err := lock.TryLock(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = lock.Unlock() }) + + err := installPrepared(&preparedUpdate{Manifest: &Manifest{}}, InstallOptions{}) + if !errors.Is(err, lockfile.ErrHeld) { + t.Fatalf("installPrepared() error = %v, want lock held", err) + } + + typed := installError("failed to install distribution update", err) + problem, ok := errs.ProblemOf(typed) + if !ok || problem.Category != errs.CategoryValidation || problem.Subtype != errs.SubtypeFailedPrecondition { + t.Fatalf("lock contention problem = %#v, want validation/failed_precondition", problem) + } + if strings.Contains(problem.Hint, "--force") { + t.Fatalf("lock contention hint = %q, want a retry-later hint", problem.Hint) + } +} + +func TestInstallDownloadsAndCommitsManifestArtifacts(t *testing.T) { + root := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", filepath.Join(root, "config")) + executable := filepath.Join(root, "bin", "lark-cli") + mustWrite(t, executable, "old binary") + + executableName := "lark-cli" + if runtime.GOOS == "windows" { + executableName += ".exe" + } + binaryArchive := buildTestZip(t, map[string]testZipFile{executableName: {content: "new binary", mode: 0o755}}) + skillsArchive := buildTestZip(t, map[string]testZipFile{ + "README.md": {content: "bundle metadata"}, + "lark-alpha/SKILL.md": {content: "alpha"}, + "lark-alpha/references/guide.md": {content: "guide"}, + "lark-alpha/scripts/check-install": {content: "#!/bin/sh\n", mode: 0o755}, + "lark-beta/SKILL.md": {content: "beta"}, + }) + payloads := map[string][]byte{ + "/cli.zip": binaryArchive, + "/skills.zip": skillsArchive, + } + previousClient := DefaultClient + DefaultClient = &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + payload, ok := payloads[req.URL.Path] + if !ok { + return &http.Response{StatusCode: http.StatusNotFound, Body: http.NoBody, Header: make(http.Header)}, nil + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader(payload)), + ContentLength: int64(len(payload)), + Header: make(http.Header), + }, nil + })} + t.Cleanup(func() { DefaultClient = previousClient }) + + manifest := &Manifest{ + Version: "release-channel-7", + sourceIdentity: "test-manifest", + Artifacts: map[string]Artifact{ + CurrentPlatformKey(): {URL: "https://dist.example/cli.zip", Checksum: checksumFor(binaryArchive)}, + SkillsKey: {URL: "https://dist.example/skills.zip", Checksum: checksumFor(skillsArchive)}, + }, + } + skillsDir := filepath.Join(root, "skills") + err := Install(context.Background(), manifest, InstallOptions{ + ExecutablePath: executable, + SkillsDir: skillsDir, + VerifyBinary: func(path, version string) error { + if version != manifest.Version { + return fmt.Errorf("version = %q", version) + } + content, err := vfs.ReadFile(path) + if err != nil { + return err + } + if string(content) != "new binary" { + return fmt.Errorf("binary = %q", content) + } + return nil + }, + }) + if err != nil { + t.Fatal(err) + } + assertFile(t, executable, "new binary") + assertFile(t, filepath.Join(skillsDir, "lark-alpha", "SKILL.md"), "alpha") + assertFile(t, filepath.Join(skillsDir, "lark-alpha", "references", "guide.md"), "guide") + assertFile(t, filepath.Join(skillsDir, "lark-beta", "SKILL.md"), "beta") + script := filepath.Join(skillsDir, "lark-alpha", "scripts", "check-install") + info, statErr := os.Stat(script) + if statErr != nil { + t.Fatal(statErr) + } + if runtime.GOOS != "windows" && info.Mode().Perm()&0o111 == 0 { + t.Fatalf("script mode = %v, want executable", info.Mode().Perm()) + } + state, ok, readErr := skillscheck.ReadState() + if readErr != nil || !ok || state.SourceIdentity != "test-manifest" { + t.Fatalf("Skills state = %#v, %v, %v", state, ok, readErr) + } +} + +func TestInstallRejectsChecksumMismatchBeforeBinaryVerification(t *testing.T) { + root := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", filepath.Join(root, "config")) + executableName := "lark-cli" + if runtime.GOOS == "windows" { + executableName += ".exe" + } + archive := buildTestZip(t, map[string]testZipFile{executableName: {content: "new binary"}}) + previousClient := DefaultClient + DefaultClient = &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader(archive)), + Header: make(http.Header), + }, nil + })} + t.Cleanup(func() { DefaultClient = previousClient }) + manifest := &Manifest{Version: "target", Artifacts: map[string]Artifact{ + CurrentPlatformKey(): {URL: "https://distribution.example/cli.zip", Checksum: "sha256:" + strings.Repeat("0", 64)}, + SkillsKey: {URL: "https://distribution.example/skills.zip", Checksum: checksumFor(archive)}, + }} + verified := false + err := Install(context.Background(), manifest, InstallOptions{ + ExecutablePath: filepath.Join(root, executableName), + VerifyBinary: func(string, string) error { + verified = true + return nil + }, + }) + if err == nil || errors.Unwrap(err) == nil || !strings.Contains(errors.Unwrap(err).Error(), "checksum mismatch") { + t.Fatalf("Install() error = %v, want checksum mismatch", err) + } + if verified { + t.Fatal("binary verification ran before checksum validation") + } +} + +func TestInstallPreparedVerificationFailureDoesNotMutate(t *testing.T) { + root := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", filepath.Join(root, "config")) + executable := filepath.Join(root, "bin", "lark-cli") + mustWrite(t, executable, "old") + binary := filepath.Join(root, "prepared", "lark-cli") + mustWrite(t, binary, "new") + mustWrite(t, filepath.Join(root, "prepared", "skills", "managed", "SKILL.md"), "new") + prepared := &preparedUpdate{ + Manifest: &Manifest{Version: "target"}, + BinaryPath: binary, + SkillsRoot: filepath.Join(root, "prepared", "skills"), + } + err := installPrepared(prepared, InstallOptions{ + ExecutablePath: executable, + SkillsDir: filepath.Join(root, "skills"), + VerifyBinary: func(string, string) error { return errors.New("bad binary") }, + }) + if err == nil { + t.Fatal("installPrepared succeeded") + } + assertFile(t, executable, "old") +} + +func TestInstallPreparedBinaryCommitFailureRollsBackSkillsAndState(t *testing.T) { + root := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", filepath.Join(root, "config")) + executable := filepath.Join(root, "bin", "lark-cli") + // Force the binary commit to fail on either platform contract: on Unix the + // single atomic rename rejects a file-over-directory target; on Windows the + // two-phase replace first removes the stale .old backup, which rejects a + // non-empty directory. + keepPath := executable + if runtime.GOOS == "windows" { + mustWrite(t, filepath.Join(executable+".old", "block-removal"), "blocked") + mustWrite(t, executable, "old") + } else { + keepPath = filepath.Join(executable, "keep") + mustWrite(t, keepPath, "old") + } + skillsDir := filepath.Join(root, "skills") + mustWrite(t, filepath.Join(skillsDir, "managed", "SKILL.md"), "old") + if err := skillscheck.WriteState(skillscheck.SkillsState{Version: "old", OfficialSkills: []string{"managed"}}); err != nil { + t.Fatal(err) + } + binary := filepath.Join(root, "prepared", "lark-cli") + mustWrite(t, binary, "new") + mustWrite(t, filepath.Join(root, "prepared", "skills", "managed", "SKILL.md"), "new") + prepared := &preparedUpdate{ + Manifest: &Manifest{Version: "target"}, + BinaryPath: binary, + SkillsRoot: filepath.Join(root, "prepared", "skills"), + } + if err := installPrepared(prepared, InstallOptions{ + ExecutablePath: executable, + SkillsDir: skillsDir, + VerifyBinary: func(string, string) error { return nil }, + }); err == nil { + t.Fatal("installPrepared succeeded") + } + assertFile(t, filepath.Join(skillsDir, "managed", "SKILL.md"), "old") + state, ok, err := skillscheck.ReadState() + if err != nil || !ok || state.Version != "old" { + t.Fatalf("state after rollback = %#v, %v, %v", state, ok, err) + } + assertFile(t, keepPath, "old") +} + +type testZipFile struct { + content string + mode os.FileMode +} + +func buildTestZip(t *testing.T, files map[string]testZipFile) []byte { + t.Helper() + var data bytes.Buffer + writer := zip.NewWriter(&data) + for name, file := range files { + header := &zip.FileHeader{Name: name, Method: zip.Deflate} + if file.mode != 0 { + header.SetMode(file.mode) + } + entry, err := writer.CreateHeader(header) + if err != nil { + t.Fatal(err) + } + if _, err := entry.Write([]byte(file.content)); err != nil { + t.Fatal(err) + } + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + return data.Bytes() +} + +func checksumFor(data []byte) string { + return fmt.Sprintf("sha256:%x", sha256.Sum256(data)) +} + +func mustWrite(t *testing.T, path, value string) { + t.Helper() + if err := vfs.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := vfs.WriteFile(path, []byte(value), 0o755); err != nil { + t.Fatal(err) + } +} + +func assertFile(t *testing.T, path, want string) { + t.Helper() + got, err := vfs.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != want { + t.Fatalf("%s = %q, want %q", path, got, want) + } +} diff --git a/internal/distribution/manifest.go b/internal/distribution/manifest.go new file mode 100644 index 0000000000..320ebf60a5 --- /dev/null +++ b/internal/distribution/manifest.go @@ -0,0 +1,161 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +// Package distribution owns fixed-schema manifest loading and verified +// artifact installation for wrapper distributions. +package distribution + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "regexp" + "runtime" + "sync" + "time" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/download" + "github.com/larksuite/cli/internal/downloadtransport" + internaltransport "github.com/larksuite/cli/internal/transport" +) + +const ( + manifestSchema = 1 + manifestMaxBody = 256 << 10 + fetchTimeout = 15 * time.Second + SkillsKey = "skills" +) + +var checksumPattern = regexp.MustCompile(`^sha256:[0-9a-f]{64}$`) + +// Artifact identifies one downloadable resource. +type Artifact struct { + URL string `json:"url"` + Checksum string `json:"checksum"` +} + +// Manifest is schema 1 of the distribution protocol. Unknown JSON fields are +// ignored so producers can attach metadata without breaking older CLIs; the +// required fields and artifacts are still validated before use. +type Manifest struct { + Schema int `json:"schema"` + Version string `json:"version"` + Artifacts map[string]Artifact `json:"artifacts"` + + sourceIdentity string +} + +// DefaultClient overrides the manifest/artifact client in tests. Production +// uses a standalone net/http client so distribution URLs bypass extensions. +var DefaultClient *http.Client + +var defaultClientOnce = sync.OnceValue(func() *http.Client { + return &http.Client{ + // Distribution URLs bypass extension hooks, but they still use the CLI's + // built-in proxy, custom CA, and fail-closed transport policy. + Transport: internaltransport.Shared(), + CheckRedirect: distributionRedirectPolicy, + } +}) + +func distributionRedirectPolicy(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return fmt.Errorf("stopped after 10 redirects") + } + if req == nil || (req.URL.Scheme != "http" && req.URL.Scheme != "https") { + return fmt.Errorf("distribution URL redirected to an unsupported scheme") + } + if len(via) > 0 && via[len(via)-1].URL.Scheme == "https" && req.URL.Scheme == "http" { + return fmt.Errorf("distribution URL redirected from HTTPS to HTTP") + } + return nil +} + +func httpClient() *http.Client { + if DefaultClient != nil { + return DefaultClient + } + return defaultClientOnce() +} + +// PlatformKey returns the manifest artifact key for a platform. +func PlatformKey(goos, goarch string) string { return goos + "-" + goarch } + +// CurrentPlatformKey returns the artifact key for this binary. +func CurrentPlatformKey() string { return PlatformKey(runtime.GOOS, runtime.GOARCH) } + +// FetchManifest synchronously loads and validates the source's manifest. +// Fetch failures are network errors; a fetched body that fails validation is +// an invalid response. +func (s Source) FetchManifest(ctx context.Context) (*Manifest, errs.TypedError) { + body, err := fetchManifestBody(ctx, s.manifestURL) + if err != nil { + return nil, errs.NewNetworkError(errs.SubtypeNetworkTransport, "failed to fetch distribution manifest: %s", err). + WithCause(err) + } + manifest, err := parseManifest(body, CurrentPlatformKey()) + if err != nil { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "invalid distribution manifest: %s", err). + WithCause(err) + } + manifest.sourceIdentity = s.Identity() + return manifest, nil +} + +func fetchManifestBody(ctx context.Context, manifestURL string) ([]byte, error) { + ctx, cancel := context.WithTimeout(ctx, fetchTimeout) + defer cancel() + resp, err := downloadtransport.URL(httpClient(), manifestURL)(ctx, download.Request{}) + if err != nil { + return nil, err + } + defer resp.Body.Close() + body, err := io.ReadAll(io.LimitReader(resp.Body, manifestMaxBody+1)) + if err != nil { + return nil, err + } + if len(body) > manifestMaxBody { + return nil, fmt.Errorf("distribution manifest exceeds %d bytes", manifestMaxBody) + } + return body, nil +} + +func parseManifest(data []byte, platformKey string) (*Manifest, error) { + decoder := json.NewDecoder(bytes.NewReader(data)) + var manifest Manifest + if err := decoder.Decode(&manifest); err != nil { + return nil, err + } + if _, err := decoder.Token(); err != io.EOF { + if err == nil { + err = fmt.Errorf("multiple JSON values") + } + return nil, err + } + if manifest.Schema != manifestSchema { + return nil, fmt.Errorf("unsupported schema %d", manifest.Schema) + } + if manifest.Version == "" { + return nil, fmt.Errorf("version must be a non-empty opaque string") + } + if manifest.Artifacts == nil { + return nil, fmt.Errorf("artifacts are required") + } + for _, required := range []string{SkillsKey, platformKey} { + artifact, ok := manifest.Artifacts[required] + if !ok { + return nil, fmt.Errorf("missing required artifact %q", required) + } + if err := validateDistributionURL(artifact.URL); err != nil { + return nil, fmt.Errorf("artifact %q has invalid URL: %w", required, err) + } + if !checksumPattern.MatchString(artifact.Checksum) { + return nil, fmt.Errorf("artifact %q has invalid checksum", required) + } + } + return &manifest, nil +} diff --git a/internal/distribution/manifest_test.go b/internal/distribution/manifest_test.go new file mode 100644 index 0000000000..3947997261 --- /dev/null +++ b/internal/distribution/manifest_test.go @@ -0,0 +1,169 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package distribution + +import ( + "context" + "fmt" + "io" + "net/http" + "strings" + "testing" + + exttransport "github.com/larksuite/cli/extension/transport" + internaltransport "github.com/larksuite/cli/internal/transport" +) + +const testChecksum = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (fn roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { return fn(req) } + +type snapshotProvider struct { + manifestURL string + calls int +} + +func (*snapshotProvider) Name() string { return "snapshot-test" } +func (*snapshotProvider) ResolveInterceptor(context.Context) exttransport.Interceptor { + return nil +} +func (p *snapshotProvider) ResolveManifestURL(context.Context) string { + p.calls++ + return p.manifestURL +} + +func validManifestJSON(version string) string { + return fmt.Sprintf(`{"schema":1,"version":%q,"artifacts":{"skills":{"url":"https://dist.example/skills.tar.gz","checksum":%q},"test-os":{"url":"https://dist.example/cli.tar.gz","checksum":%q}}}`, version, testChecksum, testChecksum) +} + +func TestCaptureSourceKeepsOneProviderResult(t *testing.T) { + previous := exttransport.GetProvider() + first := &snapshotProvider{manifestURL: "https://first.example/manifest.json"} + second := &snapshotProvider{manifestURL: "https://second.example/manifest.json"} + exttransport.Register(first) + t.Cleanup(func() { exttransport.Register(previous) }) + + ctx := CaptureSource(context.Background()) + exttransport.Register(second) + got, err := ResolveSource(ctx) + if err != nil || got.manifestURL != first.manifestURL || first.calls != 1 || second.calls != 0 { + t.Fatalf("captured source = %#v, err = %v, calls = %d/%d", got, err, first.calls, second.calls) + } +} + +func TestDistributionClientUsesSharedBuiltInTransport(t *testing.T) { + previousClient := DefaultClient + DefaultClient = nil + t.Cleanup(func() { DefaultClient = previousClient }) + + if got, want := httpClient().Transport, internaltransport.Shared(); got != want { + t.Fatalf("distribution transport = %T, want shared transport %T", got, want) + } +} + +func TestDistributionRedirectPolicyRejectsDowngradeAndLimitsHops(t *testing.T) { + httpsRequest, _ := http.NewRequest(http.MethodGet, "https://dist.example/manifest.json", nil) + httpRequest, _ := http.NewRequest(http.MethodGet, "http://dist.example/manifest.json", nil) + if err := distributionRedirectPolicy(httpRequest, []*http.Request{httpsRequest}); err == nil { + t.Fatal("HTTPS to HTTP redirect was allowed") + } + if err := distributionRedirectPolicy(httpsRequest, make([]*http.Request, 10)); err == nil { + t.Fatal("eleventh redirect was allowed") + } +} + +func TestValidateDistributionURLAcceptsHTTPAndHTTPS(t *testing.T) { + for _, raw := range []string{"http://dist.example/manifest.json", "https://dist.example/manifest.json"} { + if err := validateDistributionURL(raw); err != nil { + t.Fatalf("validateDistributionURL(%q) = %v", raw, err) + } + } + for _, raw := range []string{"file:///tmp/manifest.json", "dist.example/manifest.json"} { + if err := validateDistributionURL(raw); err == nil { + t.Fatalf("validateDistributionURL(%q) succeeded", raw) + } + } +} + +func TestParseManifestAcceptsOpaqueTarget(t *testing.T) { + manifest, err := parseManifest([]byte(validManifestJSON("release-channel-7")), "test-os") + if err != nil { + t.Fatal(err) + } + if manifest.Version != "release-channel-7" { + t.Fatalf("version = %q", manifest.Version) + } +} + +func TestFetchManifestAppliesManifestDeadline(t *testing.T) { + previousClient := DefaultClient + DefaultClient = &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + if _, ok := req.Context().Deadline(); !ok { + t.Fatal("manifest request has no deadline") + } + body := strings.Replace(validManifestJSON("target"), `"test-os":`, fmt.Sprintf("%q:", CurrentPlatformKey()), 1) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(body)), + Header: make(http.Header), + }, nil + })} + t.Cleanup(func() { DefaultClient = previousClient }) + if _, err := (Source{manifestURL: "https://dist.example/manifest.json"}).FetchManifest(context.Background()); err != nil { + t.Fatal(err) + } +} + +func TestParseManifestAcceptsHTTPArtifacts(t *testing.T) { + input := strings.ReplaceAll(validManifestJSON("1"), "https://", "http://") + if _, err := parseManifest([]byte(input), "test-os"); err != nil { + t.Fatal(err) + } +} + +func TestParseManifestIgnoresArtifactsForOtherPlatforms(t *testing.T) { + input := strings.Replace(validManifestJSON("1"), `"test-os":`, `"other-os":{"url":"not a URL","checksum":"bad"},"test-os":`, 1) + if _, err := parseManifest([]byte(input), "test-os"); err != nil { + t.Fatal(err) + } +} + +func TestParseManifestAllowsExtensionFields(t *testing.T) { + input := strings.Replace( + validManifestJSON("1"), + `"schema":1`, + `"schema":1,"environment":"customer-a"`, + 1, + ) + input = strings.Replace( + input, + `"url":"https://dist.example/skills.tar.gz"`, + `"url":"https://dist.example/skills.tar.gz","channel":"stable"`, + 1, + ) + if _, err := parseManifest([]byte(input), "test-os"); err != nil { + t.Fatal(err) + } +} + +func TestParseManifestRejectsInvalidContracts(t *testing.T) { + tests := []struct{ name, input, contains string }{ + {"schema", strings.Replace(validManifestJSON("1"), `"schema":1`, `"schema":2`, 1), "unsupported schema"}, + {"missing version", strings.Replace(validManifestJSON("1"), `"version":"1",`, "", 1), "version must be"}, + {"unsupported scheme", strings.Replace(validManifestJSON("1"), "https://dist.example/skills", "file:///tmp/skills", 1), "HTTP or HTTPS"}, + {"checksum", strings.Replace(validManifestJSON("1"), testChecksum, "sha256:ABC", 1), "checksum"}, + {"missing skills", strings.Replace(validManifestJSON("1"), `"skills"`, `"other"`, 1), "missing required"}, + {"missing platform", strings.Replace(validManifestJSON("1"), `"test-os"`, `"other-os"`, 1), "missing required"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := parseManifest([]byte(tt.input), "test-os") + if err == nil || !strings.Contains(err.Error(), tt.contains) { + t.Fatalf("err = %v, want containing %q", err, tt.contains) + } + }) + } +} diff --git a/internal/distribution/source.go b/internal/distribution/source.go new file mode 100644 index 0000000000..76c4cc43cd --- /dev/null +++ b/internal/distribution/source.go @@ -0,0 +1,109 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package distribution + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "net/url" + "strings" + + "github.com/larksuite/cli/errs" + exttransport "github.com/larksuite/cli/extension/transport" +) + +// Source describes the active update source. The zero value is the npm +// registry flow; a non-zero Source selects manifest-based distribution. +type Source struct{ manifestURL string } + +// SourceSnapshot freezes source resolution for one command invocation. +type SourceSnapshot struct { + source Source + err errs.TypedError +} + +type sourceSnapshotKey struct{} + +// CaptureSource resolves the provider once and stores the result in ctx. +func CaptureSource(ctx context.Context) context.Context { + if ctx == nil { + ctx = context.Background() + } + if _, ok := ctx.Value(sourceSnapshotKey{}).(SourceSnapshot); ok { + return ctx + } + source, err := resolveSource(ctx) + return context.WithValue(ctx, sourceSnapshotKey{}, SourceSnapshot{source: source, err: err}) +} + +// ResolveSource reads the optional distribution manifest URL from the +// registered transport provider. Providers that do not implement +// exttransport.DistributionProvider, or return an empty URL, yield the zero +// (npm) Source. +func ResolveSource(ctx context.Context) (Source, errs.TypedError) { + if ctx != nil { + if snapshot, ok := ctx.Value(sourceSnapshotKey{}).(SourceSnapshot); ok { + return snapshot.source, snapshot.err + } + } + return resolveSource(ctx) +} + +func resolveSource(ctx context.Context) (Source, errs.TypedError) { + if ctx == nil { + ctx = context.Background() + } + configured, ok := exttransport.GetProvider().(exttransport.DistributionProvider) + if !ok { + return Source{}, nil + } + raw := strings.TrimSpace(configured.ResolveManifestURL(ctx)) + if raw == "" { + return Source{}, nil + } + if err := validateDistributionURL(raw); err != nil { + return Source{}, errs.NewConfigError( + errs.SubtypeInvalidConfig, + "invalid distribution manifest URL: %v", err, + ).WithCause(err) + } + return Source{manifestURL: raw}, nil +} + +// ManifestMode reports whether this source is a manifest distribution. +func (s Source) ManifestMode() bool { return s.manifestURL != "" } + +// Identity is a stable fingerprint of the source used to attribute persisted +// state without storing the URL. The npm source has the empty identity, which +// also matches state files written before source tracking existed. +func (s Source) Identity() string { + if s.manifestURL == "" { + return "" + } + sum := sha256.Sum256([]byte(s.manifestURL)) + return "manifest:" + hex.EncodeToString(sum[:]) +} + +// validateDistributionURL accepts absolute HTTP/HTTPS URLs. Plain HTTP is +// intended for trusted distribution networks only: a manifest served over HTTP +// can be replaced together with its checksums, so the provider owns transport +// integrity when it does not use HTTPS. +func validateDistributionURL(raw string) error { + parsed, err := url.Parse(raw) + if err != nil { + return fmt.Errorf("must be a valid URL") + } + if (parsed.Scheme != "http" && parsed.Scheme != "https") || parsed.Host == "" { + return fmt.Errorf("must be an absolute HTTP or HTTPS URL") + } + if parsed.User != nil { + return fmt.Errorf("must not contain user information") + } + if parsed.Fragment != "" { + return fmt.Errorf("must not contain a fragment") + } + return nil +} diff --git a/internal/registry/loader.go b/internal/registry/loader.go index ab2bbaba16..76319b1c50 100644 --- a/internal/registry/loader.go +++ b/internal/registry/loader.go @@ -15,7 +15,7 @@ import ( "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/meta" - "github.com/larksuite/cli/internal/update" + "github.com/larksuite/cli/internal/versioncheck" ) //go:embed scope_priorities.json scope_overrides.json @@ -94,7 +94,7 @@ func InitWithBrand(brand core.LarkBrand) { if !brandChanged { // After a CLI upgrade the embedded data can be fresher than an old // cache; an equal/older cache must not shadow it. - if cached, err := loadCachedMerged(); err == nil && update.IsNewer(cached.Version, embeddedVersion) { + if cached, err := loadCachedMerged(); err == nil && versioncheck.IsNewer(cached.Version, embeddedVersion) { overlayMergedServices(cached) } } diff --git a/internal/selfupdate/candidate.go b/internal/selfupdate/candidate.go new file mode 100644 index 0000000000..f4f5123e24 --- /dev/null +++ b/internal/selfupdate/candidate.go @@ -0,0 +1,104 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package selfupdate + +import ( + "context" + "fmt" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/internal/vfs" +) + +const candidateVerifyTimeout = 10 * time.Second + +// CandidateVerifier validates a staged executable before installation. +type CandidateVerifier func(path, version string) error + +// Candidate is a verified executable staged beside its target. +type Candidate struct { + path string + target string +} + +// PrepareCandidate stages and verifies source without changing the installed +// executable. An empty target selects the current executable. +func PrepareCandidate(source, target, version string, verify CandidateVerifier) (*Candidate, error) { + resolved, err := resolveCandidateTarget(target) + if err != nil { + return nil, err + } + if err := vfs.MkdirAll(filepath.Dir(resolved), 0o755); err != nil { + return nil, err + } + in, err := vfs.Open(source) + if err != nil { + return nil, err + } + defer in.Close() + path := resolved + ".new" + keep := false + defer func() { + if !keep { + _ = vfs.Remove(path) + } + }() + if _, err := validate.AtomicWriteFromReader(path, in, 0o755); err != nil { + return nil, err + } + if verify == nil { + verify = VerifyCandidateVersion + } + if err := verify(path, version); err != nil { + return nil, fmt.Errorf("verify staged binary: %w", err) + } + keep = true + return &Candidate{path: path, target: resolved}, nil +} + +func resolveCandidateTarget(target string) (string, error) { + if target != "" { + return target, nil + } + return New().resolveExe() +} + +// Cleanup removes a prepared candidate that was not installed. +func (c *Candidate) Cleanup() { + if c != nil && c.path != "" { + _ = vfs.Remove(c.path) + } +} + +// Install promotes the prepared candidate. The returned finalize function +// drops the previous executable's backup after the surrounding update +// commits; on Unix promotion is a single atomic rename and finalize is a +// no-op. Platform mechanics live in candidate_install_{unix,windows}.go. +func (c *Candidate) Install() (func(), error) { + if c == nil || c.path == "" || c.target == "" { + return nil, fmt.Errorf("prepared binary candidate is required") + } + return c.install() +} + +// VerifyCandidateVersion checks the exact opaque version reported by a binary. +func VerifyCandidateVersion(path, version string) error { + ctx, cancel := context.WithTimeout(context.Background(), candidateVerifyTimeout) + defer cancel() + output, err := exec.CommandContext(ctx, path, "--version").Output() //nolint:gosec // path is a checksum-verified staged binary. + if ctx.Err() == context.DeadlineExceeded { + return fmt.Errorf("binary verification timed out after %s", candidateVerifyTimeout) + } + if err != nil { + return fmt.Errorf("run --version: %w", err) + } + if strings.TrimSpace(string(output)) != "lark-cli version "+version { + return fmt.Errorf("binary reported %q, want version %q", strings.TrimSpace(string(output)), version) + } + return nil +} diff --git a/internal/selfupdate/candidate_install_unix.go b/internal/selfupdate/candidate_install_unix.go new file mode 100644 index 0000000000..d363cda2d1 --- /dev/null +++ b/internal/selfupdate/candidate_install_unix.go @@ -0,0 +1,23 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build !windows + +package selfupdate + +import ( + "github.com/larksuite/cli/internal/vfs" +) + +// install promotes the staged candidate with a single atomic rename. Unix +// permits renaming over a running executable (inode semantics, same contract +// as updater_unix.go), so there is no window where the target is missing and +// no backup to roll back. +func (c *Candidate) install() (func(), error) { + _ = vfs.Remove(c.target + ".old") // stale backup from an older two-phase update + if err := vfs.Rename(c.path, c.target); err != nil { + return nil, err + } + c.path = "" + return func() {}, nil +} diff --git a/internal/selfupdate/candidate_install_windows.go b/internal/selfupdate/candidate_install_windows.go new file mode 100644 index 0000000000..1967999788 --- /dev/null +++ b/internal/selfupdate/candidate_install_windows.go @@ -0,0 +1,102 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build windows + +package selfupdate + +import ( + "errors" + "fmt" + "io/fs" + "unsafe" + + "github.com/larksuite/cli/internal/vfs" + "golang.org/x/sys/windows" +) + +var replaceFile = windows.NewLazySystemDLL("kernel32.dll").NewProc("ReplaceFileW") +var replaceFilePath = callReplaceFilePath + +// install uses ReplaceFileW so replacing a running executable and creating its +// backup are one filesystem operation; a crash cannot leave the target absent. +func (c *Candidate) install() (func(), error) { + backup := c.target + ".old" + if _, err := vfs.Stat(c.target); errors.Is(err, fs.ErrNotExist) { + if err := vfs.Rename(c.path, c.target); err != nil { + return nil, err + } + c.path = "" + return func() {}, nil + } else if err != nil { + return nil, err + } + if err := vfs.Remove(backup); err != nil && !errors.Is(err, fs.ErrNotExist) { + return nil, fmt.Errorf("remove stale binary backup: %w", err) + } + if err := replaceFilePath(c.target, c.path, backup); err != nil { + return nil, c.recoverFailedWindowsInstall(backup, err) + } + c.path = "" + return func() { _ = vfs.Remove(backup) }, nil +} + +func callReplaceFilePath(targetPath, replacementPath, backupPath string) error { + target, err := windows.UTF16PtrFromString(targetPath) + if err != nil { + return err + } + replacement, err := windows.UTF16PtrFromString(replacementPath) + if err != nil { + return err + } + var backup uintptr + if backupPath != "" { + ptr, err := windows.UTF16PtrFromString(backupPath) + if err != nil { + return err + } + backup = uintptr(unsafe.Pointer(ptr)) + } + if result, _, callErr := replaceFile.Call( + uintptr(unsafe.Pointer(target)), + uintptr(unsafe.Pointer(replacement)), + backup, + 0, 0, 0, + ); result == 0 { + return fmt.Errorf("ReplaceFileW: %w", callErr) + } + return nil +} + +// recoverFailedWindowsInstall restores the old executable before returning an +// install error. If Windows cannot restore it, preserve both recovery files so +// Candidate.Cleanup cannot remove the only usable copy. +func (c *Candidate) recoverFailedWindowsInstall(backup string, installErr error) error { + if _, err := vfs.Stat(backup); err == nil { + var restoreErr error + if _, targetErr := vfs.Stat(c.target); errors.Is(targetErr, fs.ErrNotExist) { + restoreErr = vfs.Rename(backup, c.target) + } else if targetErr != nil { + restoreErr = targetErr + } else { + restoreErr = replaceFilePath(c.target, backup, "") + } + if restoreErr == nil { + return installErr + } + return c.windowsRecoveryRequired(backup, installErr, restoreErr) + } else if !errors.Is(err, fs.ErrNotExist) { + return c.windowsRecoveryRequired(backup, installErr, err) + } + if _, err := vfs.Stat(c.target); err != nil { + return c.windowsRecoveryRequired(backup, installErr, err) + } + return installErr +} + +func (c *Candidate) windowsRecoveryRequired(backup string, installErr, recoveryErr error) error { + candidate := c.path + c.path = "" // preserve the candidate for manual recovery + return fmt.Errorf("%w; automatic recovery failed: %v; preserved backup %q and candidate %q", installErr, recoveryErr, backup, candidate) +} diff --git a/internal/selfupdate/candidate_install_windows_test.go b/internal/selfupdate/candidate_install_windows_test.go new file mode 100644 index 0000000000..2d3e939a54 --- /dev/null +++ b/internal/selfupdate/candidate_install_windows_test.go @@ -0,0 +1,41 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build windows + +package selfupdate + +import ( + "errors" + "testing" + + "github.com/larksuite/cli/internal/vfs" +) + +func TestCandidateInstallRestoresPartialWindowsReplacement(t *testing.T) { + target := t.TempDir() + `\lark-cli.exe` + candidate := target + ".new" + if err := vfs.WriteFile(target, []byte("old"), 0o755); err != nil { + t.Fatal(err) + } + if err := vfs.WriteFile(candidate, []byte("new"), 0o755); err != nil { + t.Fatal(err) + } + original := replaceFilePath + replaceFilePath = func(targetPath, _ string, backupPath string) error { + if err := vfs.Rename(targetPath, backupPath); err != nil { + t.Fatal(err) + } + return errors.New("simulated partial replacement") + } + t.Cleanup(func() { replaceFilePath = original }) + + prepared := &Candidate{path: candidate, target: target} + if _, err := prepared.Install(); err == nil { + t.Fatal("partial replacement unexpectedly succeeded") + } + got, err := vfs.ReadFile(target) + if err != nil || string(got) != "old" { + t.Fatalf("restored target = %q, %v", got, err) + } +} diff --git a/internal/selfupdate/candidate_test.go b/internal/selfupdate/candidate_test.go new file mode 100644 index 0000000000..7d0c7baf19 --- /dev/null +++ b/internal/selfupdate/candidate_test.go @@ -0,0 +1,53 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package selfupdate + +import ( + "errors" + "io/fs" + "path/filepath" + "runtime" + "testing" + + "github.com/larksuite/cli/internal/vfs" +) + +func TestVerifyCandidateVersionIgnoresStderr(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("test helper uses a POSIX shell script") + } + path := filepath.Join(t.TempDir(), "lark-cli") + script := "#!/bin/sh\nprintf 'Fetching API metadata...\\n' >&2\nprintf 'lark-cli version 1.2.3\\n'\n" + if err := vfs.WriteFile(path, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + if err := VerifyCandidateVersion(path, "1.2.3"); err != nil { + t.Fatal(err) + } +} + +func TestCandidateInstallPromotesStagedBinaryAndCleansBackup(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "lark-cli") + backup := target + ".old" + staged := filepath.Join(root, "staged") + if err := vfs.WriteFile(backup, []byte("old"), 0o755); err != nil { + t.Fatal(err) + } + if err := vfs.WriteFile(staged, []byte("new"), 0o755); err != nil { + t.Fatal(err) + } + finalize, err := (&Candidate{path: staged, target: target}).Install() + if err != nil { + t.Fatal(err) + } + got, err := vfs.ReadFile(target) + if err != nil || string(got) != "new" { + t.Fatalf("target = %q, %v", got, err) + } + finalize() + if _, err := vfs.Stat(backup); !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("backup remains after finalize: %v", err) + } +} diff --git a/internal/skillscheck/check.go b/internal/skillscheck/check.go index d1425ea816..c35b61e063 100644 --- a/internal/skillscheck/check.go +++ b/internal/skillscheck/check.go @@ -3,7 +3,7 @@ package skillscheck -import "strings" +import "github.com/larksuite/cli/internal/versioncheck" // Init runs the synchronous skills version check. Stores a StaleNotice when // the local skills state records a version that does not match currentVersion, @@ -14,15 +14,26 @@ import "strings" // Skip rules: see shouldSkip (CI envs, DEV builds, non-release semver, // LARKSUITE_CLI_NO_SKILLS_NOTIFIER opt-out). func Init(currentVersion string) { + InitForSource(currentVersion, OfficialSourceIdentity, false) +} + +// InitForSource also considers which distribution owns the installed Skills. +// exactTarget is true for manifest distributions, whose versions are opaque +// strings rather than SemVer releases. +func InitForSource(currentVersion, sourceIdentity string, exactTarget bool) { SetPending(nil) - if shouldSkip(currentVersion) { + if shouldSkip(currentVersion, exactTarget) { return } state, ok, err := ReadState() if err != nil || !ok || state.Version == "" { return } - if strings.TrimPrefix(strings.TrimPrefix(state.Version, "v"), "V") == strings.TrimPrefix(strings.TrimPrefix(currentVersion, "v"), "V") && !state.OfficialSkillsUnknown { + versionMatches := versioncheck.Equal(state.Version, currentVersion) + if exactTarget { + versionMatches = state.Version == currentVersion + } + if versionMatches && !state.OfficialSkillsUnknown && MatchesSource(state, sourceIdentity) { return } SetPending(&StaleNotice{ diff --git a/internal/skillscheck/check_test.go b/internal/skillscheck/check_test.go index f3b11890eb..3fe52513b3 100644 --- a/internal/skillscheck/check_test.go +++ b/internal/skillscheck/check_test.go @@ -51,6 +51,45 @@ func TestInit_NormalizedVersion_NoNotice(t *testing.T) { } } +func TestInitForSourceNoticesAtSameVersionWhenSourceChanges(t *testing.T) { + clearSkillsSkipEnv(t) + resetPending(t) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := WriteState(SkillsState{ + Version: "1.0.21", + SourceIdentity: "manifest:first", + }); err != nil { + t.Fatal(err) + } + InitForSource("1.0.21", "manifest:second", true) + if got := GetPending(); got == nil { + t.Fatal("GetPending() = nil, want notice for a changed Skills source") + } +} + +// TestInitForSourceNoticesOpaqueManifestVersion pins the manifest-mode +// regression: an opaque target version must not be suppressed by the +// SemVer/release gate. +func TestInitForSourceNoticesOpaqueManifestVersion(t *testing.T) { + clearSkillsSkipEnv(t) + resetPending(t) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := WriteState(SkillsState{ + Version: "1.0.21", + SourceIdentity: "manifest:dist", + }); err != nil { + t.Fatal(err) + } + InitForSource("v1.0.21", "manifest:dist", true) + got := GetPending() + if got == nil { + t.Fatal("GetPending() = nil, want notice for opaque manifest version drift") + } + if got.Current != "1.0.21" || got.Target != "v1.0.21" { + t.Errorf("notice = %+v", got) + } +} + func TestInit_OfficialSkillsUnknown_NoticeAtSameVersion(t *testing.T) { clearSkillsSkipEnv(t) resetPending(t) diff --git a/internal/skillscheck/prepared.go b/internal/skillscheck/prepared.go new file mode 100644 index 0000000000..1c54542105 --- /dev/null +++ b/internal/skillscheck/prepared.go @@ -0,0 +1,225 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package skillscheck + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "slices" + + "github.com/larksuite/cli/internal/vfs" +) + +// PreparedTreeOptions describes a complete, already-extracted official Skills +// tree. TargetDir overrides automatic agent-directory discovery when non-empty. +type PreparedTreeOptions struct { + Root string + Version string + SourceIdentity string + TargetDir string +} + +// SyncPreparedTree installs a complete official Skills tree and records its +// state. The returned rollback is kept by callers until related update work is +// committed; finalize removes temporary backups after a successful commit. +func SyncPreparedTree(opts PreparedTreeOptions) (rollback func() error, finalize func(), err error) { + official, err := listPreparedSkills(opts.Root) + if err != nil { + return nil, nil, err + } + previous, readable, err := ReadState() + if err != nil && !errors.Is(err, ErrUnreadableState) { + return nil, nil, fmt.Errorf("read Skills state: %w", err) + } + if err != nil { + previous, readable = nil, false + } + restoreState, err := SnapshotState() + if err != nil { + return nil, nil, fmt.Errorf("snapshot Skills state: %w", err) + } + plan := PlanSync(SyncInput{ + Version: opts.Version, + OfficialSkills: official, + PreviousState: previous, + StateReadable: readable, + Force: true, + }) + targets, err := preparedSkillsTargets(opts.TargetDir) + if err != nil { + return nil, nil, err + } + rollbackFiles, finalizeFiles, err := installPreparedToTargets(opts.Root, targets, plan) + if err != nil { + return nil, nil, err + } + rollbackAll := func() error { + return errors.Join(rollbackFiles(), restoreState()) + } + + state := NewCompleteState(opts.Version, LayoutSeparate, official, previous) + state.SourceIdentity = opts.SourceIdentity + if err := WriteState(state); err != nil { + cause := fmt.Errorf("write Skills state: %w", err) + if rollbackErr := rollbackAll(); rollbackErr != nil { + return nil, nil, fmt.Errorf("%w (%w)", cause, rollbackErr) + } + return nil, nil, cause + } + return rollbackAll, finalizeFiles, nil +} + +func listPreparedSkills(root string) ([]string, error) { + entries, err := vfs.ReadDir(root) + if err != nil { + return nil, err + } + names := make([]string, 0, len(entries)) + for _, entry := range entries { + if entry.IsDir() { + names = append(names, entry.Name()) + } + } + if len(names) == 0 { + return nil, fmt.Errorf("skills artifact contains no Skills") + } + slices.Sort(names) + return names, nil +} + +func preparedSkillsTargets(override string) ([]string, error) { + if override != "" { + return []string{override}, nil + } + home, err := vfs.UserHomeDir() + if err != nil { + return nil, err + } + targets := []string{filepath.Join(home, ".agents", "skills")} + targets = appendDetectedTarget(targets, os.Getenv("CLAUDE_CONFIG_DIR"), filepath.Join(home, ".claude")) + targets = appendDetectedTarget(targets, os.Getenv("CODEX_HOME"), filepath.Join(home, ".codex")) + return uniquePaths(targets), nil +} + +func appendDetectedTarget(targets []string, configuredRoot, defaultRoot string) []string { + root := configuredRoot + if root == "" { + root = defaultRoot + if info, err := vfs.Stat(root); err != nil || !info.IsDir() { + return targets + } + } + return append(targets, filepath.Join(root, "skills")) +} + +func uniquePaths(paths []string) []string { + seen := map[string]bool{} + result := make([]string, 0, len(paths)) + for _, path := range paths { + path = filepath.Clean(path) + if !seen[path] { + seen[path] = true + result = append(result, path) + } + } + return result +} + +func installPreparedToTargets(root string, targets []string, plan SyncPlan) (func() error, func(), error) { + rollbacks := make([]func() error, 0, len(targets)) + finalizers := make([]func(), 0, len(targets)) + rollbackAll := func() error { + var errs []error + for i := len(rollbacks) - 1; i >= 0; i-- { + errs = append(errs, rollbacks[i]()) + } + return errors.Join(errs...) + } + for _, target := range targets { + rollback, finalize, err := installPrepared(root, target, plan) + if err != nil { + return nil, nil, failPreparedAfterRollback(fmt.Errorf("install Skills to %s: %w", target, err), rollbackAll) + } + rollbacks = append(rollbacks, rollback) + finalizers = append(finalizers, finalize) + } + return rollbackAll, func() { + for _, finalize := range finalizers { + finalize() + } + }, nil +} + +func installPrepared(root, target string, plan SyncPlan) (func() error, func(), error) { + parent := filepath.Dir(target) + if err := vfs.MkdirAll(parent, 0o755); err != nil { + return nil, nil, err + } + stage, err := vfs.MkdirTemp(parent, ".lark-cli-skills-new-*") + if err != nil { + return nil, nil, err + } + backup, err := vfs.MkdirTemp(parent, ".lark-cli-skills-old-*") + if err != nil { + _ = vfs.RemoveAll(stage) + return nil, nil, err + } + cleanup := func() { _ = vfs.RemoveAll(stage); _ = vfs.RemoveAll(backup) } + for _, name := range plan.ToUpdate { + // Both paths are bounded CLI-managed host directories; the standard + // library preserves the source tree without another copy implementation. + if err := os.CopyFS(filepath.Join(stage, name), os.DirFS(filepath.Join(root, name))); err != nil { //nolint:forbidigo + cleanup() + return nil, nil, err + } + } + if err := vfs.MkdirAll(target, 0o755); err != nil { + cleanup() + return nil, nil, err + } + movedOld, movedNew := []string{}, []string{} + rollback := func() error { + var errs []error + for i := len(movedNew) - 1; i >= 0; i-- { + errs = append(errs, vfs.RemoveAll(filepath.Join(target, movedNew[i]))) + } + for i := len(movedOld) - 1; i >= 0; i-- { + name := movedOld[i] + errs = append(errs, vfs.Rename(filepath.Join(backup, name), filepath.Join(target, name))) + } + _ = vfs.RemoveAll(stage) + if err := errors.Join(errs...); err != nil { + return err // keep the backup for manual recovery + } + _ = vfs.RemoveAll(backup) + return nil + } + for _, name := range plan.CleanupOfficial { + current := filepath.Join(target, name) + if _, err := vfs.Stat(current); err == nil { + if err := vfs.Rename(current, filepath.Join(backup, name)); err != nil { + return nil, nil, failPreparedAfterRollback(err, rollback) + } + movedOld = append(movedOld, name) + } else if !os.IsNotExist(err) { + return nil, nil, failPreparedAfterRollback(err, rollback) + } + if slices.Contains(plan.ToUpdate, name) { + if err := vfs.Rename(filepath.Join(stage, name), current); err != nil { + return nil, nil, failPreparedAfterRollback(err, rollback) + } + movedNew = append(movedNew, name) + } + } + return rollback, cleanup, nil +} + +func failPreparedAfterRollback(cause error, rollback func() error) error { + if err := rollback(); err != nil { + return fmt.Errorf("%w (rollback failed: %w; backup retained)", cause, err) + } + return cause +} diff --git a/internal/skillscheck/prepared_test.go b/internal/skillscheck/prepared_test.go new file mode 100644 index 0000000000..e064a0099a --- /dev/null +++ b/internal/skillscheck/prepared_test.go @@ -0,0 +1,89 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package skillscheck + +import ( + "errors" + "io/fs" + "path/filepath" + "reflect" + "testing" + + "github.com/larksuite/cli/internal/vfs" +) + +func TestSyncPreparedTreeReplacesOfficialSkillsAndPreservesCustom(t *testing.T) { + root := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", filepath.Join(root, "config")) + target := filepath.Join(root, "installed") + writePreparedTestFile(t, filepath.Join(target, "retired", "SKILL.md"), "old") + writePreparedTestFile(t, filepath.Join(target, "custom", "SKILL.md"), "custom") + if err := WriteState(SkillsState{Version: "old", OfficialSkills: []string{"retired"}}); err != nil { + t.Fatal(err) + } + source := filepath.Join(root, "prepared") + writePreparedTestFile(t, filepath.Join(source, "current", "SKILL.md"), "new") + writePreparedTestFile(t, filepath.Join(source, "README.md"), "metadata") + + rollback, finalize, err := SyncPreparedTree(PreparedTreeOptions{ + Root: source, Version: "target", SourceIdentity: "manifest:test", TargetDir: target, + }) + if err != nil { + t.Fatal(err) + } + if rollback == nil || finalize == nil { + t.Fatal("SyncPreparedTree returned incomplete transaction hooks") + } + finalize() + assertPreparedTestFile(t, filepath.Join(target, "current", "SKILL.md"), "new") + assertPreparedTestFile(t, filepath.Join(target, "custom", "SKILL.md"), "custom") + if _, err := vfs.Stat(filepath.Join(target, "retired")); !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("retired Skill remains: %v", err) + } + state, ok, err := ReadState() + if err != nil || !ok || state.Version != "target" || state.SourceIdentity != "manifest:test" || + !reflect.DeepEqual(state.OfficialSkills, []string{"current"}) { + t.Fatalf("state = %#v, %v, %v", state, ok, err) + } +} + +func TestPreparedSkillsTargetsHonorDetectedAgentHomes(t *testing.T) { + root := t.TempDir() + t.Setenv("HOME", root) + t.Setenv("CLAUDE_CONFIG_DIR", filepath.Join(root, "claude")) + t.Setenv("CODEX_HOME", filepath.Join(root, "codex")) + got, err := preparedSkillsTargets("") + if err != nil { + t.Fatal(err) + } + want := []string{ + filepath.Join(root, ".agents", "skills"), + filepath.Join(root, "claude", "skills"), + filepath.Join(root, "codex", "skills"), + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("targets = %#v, want %#v", got, want) + } +} + +func writePreparedTestFile(t *testing.T, path, content string) { + t.Helper() + if err := vfs.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := vfs.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func assertPreparedTestFile(t *testing.T, path, want string) { + t.Helper() + got, err := vfs.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != want { + t.Fatalf("%s = %q, want %q", path, got, want) + } +} diff --git a/internal/skillscheck/skip.go b/internal/skillscheck/skip.go index b4da13d4b0..029e235233 100644 --- a/internal/skillscheck/skip.go +++ b/internal/skillscheck/skip.go @@ -6,22 +6,33 @@ package skillscheck import ( "os" - "github.com/larksuite/cli/internal/update" + "github.com/larksuite/cli/internal/versioncheck" ) // shouldSkip returns true when the skills check should be silently // suppressed. Mirrors internal/update.shouldSkip semantics but uses // a dedicated opt-out env var so users can disable the skills nag // without also disabling the binary update nag. -func shouldSkip(version string) bool { +// +// exactTarget marks a manifest distribution: its version is an opaque string +// chosen by the producer, so the SemVer/release gate (including the DEV +// marker) does not apply — only the opt-out, CI, and a missing version +// suppress the check. +func shouldSkip(version string, exactTarget bool) bool { if os.Getenv("LARKSUITE_CLI_NO_SKILLS_NOTIFIER") != "" { return true } - if update.IsCIEnv() { + if versioncheck.IsCIEnv() { return true } - if version == "DEV" || version == "dev" || version == "" { + if version == "" { return true } - return !update.IsRelease(version) + if exactTarget { + return false + } + if version == "DEV" || version == "dev" { + return true + } + return !versioncheck.IsRelease(version) } diff --git a/internal/skillscheck/skip_test.go b/internal/skillscheck/skip_test.go index 0d9b216553..31045633ce 100644 --- a/internal/skillscheck/skip_test.go +++ b/internal/skillscheck/skip_test.go @@ -20,38 +20,52 @@ func clearSkillsSkipEnv(t *testing.T) { func TestShouldSkip(t *testing.T) { tests := []struct { - name string - setup func(t *testing.T) - version string - want bool + name string + setup func(t *testing.T) + version string + exactTarget bool + want bool }{ - {"release_no_skip", clearSkillsSkipEnv, "1.0.21", false}, - {"dev_uppercase", clearSkillsSkipEnv, "DEV", true}, - {"dev_lowercase", clearSkillsSkipEnv, "dev", true}, - {"empty_version", clearSkillsSkipEnv, "", true}, - {"git_describe", clearSkillsSkipEnv, "1.0.0-12-g9b933f1-dirty", true}, + {"release_no_skip", clearSkillsSkipEnv, "1.0.21", false, false}, + {"dev_uppercase", clearSkillsSkipEnv, "DEV", false, true}, + {"dev_lowercase", clearSkillsSkipEnv, "dev", false, true}, + {"empty_version", clearSkillsSkipEnv, "", false, true}, + {"git_describe", clearSkillsSkipEnv, "1.0.0-12-g9b933f1-dirty", false, true}, + // Manifest distributions carry opaque target strings; the SemVer gate + // and the DEV marker must not suppress their checks. + {"manifest_opaque_version", clearSkillsSkipEnv, "release-channel-7", true, false}, + {"manifest_dev_marker", clearSkillsSkipEnv, "DEV", true, false}, + {"manifest_empty_version", clearSkillsSkipEnv, "", true, true}, {"opt_out", func(t *testing.T) { clearSkillsSkipEnv(t) t.Setenv("LARKSUITE_CLI_NO_SKILLS_NOTIFIER", "1") - }, "1.0.21", true}, + }, "1.0.21", false, true}, + {"manifest_opt_out", func(t *testing.T) { + clearSkillsSkipEnv(t) + t.Setenv("LARKSUITE_CLI_NO_SKILLS_NOTIFIER", "1") + }, "release-channel-7", true, true}, {"ci_env", func(t *testing.T) { clearSkillsSkipEnv(t) t.Setenv("CI", "true") - }, "1.0.21", true}, + }, "1.0.21", false, true}, + {"manifest_ci_env", func(t *testing.T) { + clearSkillsSkipEnv(t) + t.Setenv("CI", "true") + }, "release-channel-7", true, true}, {"build_number_env", func(t *testing.T) { clearSkillsSkipEnv(t) t.Setenv("BUILD_NUMBER", "42") - }, "1.0.21", true}, + }, "1.0.21", false, true}, {"run_id_env", func(t *testing.T) { clearSkillsSkipEnv(t) t.Setenv("RUN_ID", "abc") - }, "1.0.21", true}, + }, "1.0.21", false, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { tt.setup(t) - if got := shouldSkip(tt.version); got != tt.want { - t.Errorf("shouldSkip(%q) = %v, want %v", tt.version, got, tt.want) + if got := shouldSkip(tt.version, tt.exactTarget); got != tt.want { + t.Errorf("shouldSkip(%q, %v) = %v, want %v", tt.version, tt.exactTarget, got, tt.want) } }) } @@ -62,7 +76,7 @@ func TestShouldSkip(t *testing.T) { func TestShouldSkip_OptOutIsIndependent(t *testing.T) { clearSkillsSkipEnv(t) t.Setenv("LARKSUITE_CLI_NO_UPDATE_NOTIFIER", "1") // update opt-out, not us - if shouldSkip("1.0.21") { + if shouldSkip("1.0.21", false) { t.Error("shouldSkip(release) = true with only LARKSUITE_CLI_NO_UPDATE_NOTIFIER set, want false") } } diff --git a/internal/skillscheck/state.go b/internal/skillscheck/state.go index 44d1d76cb6..48d27bc829 100644 --- a/internal/skillscheck/state.go +++ b/internal/skillscheck/state.go @@ -9,6 +9,8 @@ import ( "fmt" "io/fs" "path/filepath" + "slices" + "time" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/validate" @@ -16,13 +18,15 @@ import ( ) const ( - stateFile = "skills-state.json" + stateFile = "skills-state.json" + OfficialSourceIdentity = "official" ) var ErrUnreadableState = errors.New("skills state is unreadable") type SkillsState struct { Version string `json:"version"` + SourceIdentity string `json:"source_identity,omitempty"` Layout Layout `json:"layout,omitempty"` OfficialSkills []string `json:"official_skills"` OfficialSkillsUnknown bool `json:"official_skills_unknown,omitempty"` @@ -32,6 +36,55 @@ type SkillsState struct { UpdatedAt string `json:"updated_at"` } +// MatchesSource reports whether state belongs to the expected Skills source. +// States written before source tracking are treated as the official source. +func MatchesSource(state *SkillsState, expected string) bool { + if state == nil { + return false + } + if state.SourceIdentity == "" { + return expected == OfficialSourceIdentity + } + return state.SourceIdentity == expected +} + +// KnownOfficialSkills returns the previous managed Skill set when the state is +// authoritative. Callers receive a copy so installation planning cannot mutate +// the persisted state in memory. +func KnownOfficialSkills(state *SkillsState) []string { + if state == nil || state.OfficialSkillsUnknown { + return nil + } + return slices.Clone(state.OfficialSkills) +} + +// NewCompleteState builds state for a complete managed Skills replacement. +// Every supplied Skill is installed in this operation, and Skills that were not +// present in the previous authoritative state are recorded as newly added. +func NewCompleteState(version string, layout Layout, official []string, previous *SkillsState) SkillsState { + official = slices.Clone(official) + previousSet := make(map[string]bool) + for _, name := range KnownOfficialSkills(previous) { + previousSet[name] = true + } + added := make([]string, 0, len(official)) + for _, name := range official { + if !previousSet[name] { + added = append(added, name) + } + } + state := SkillsState{ + Version: version, + Layout: layout, + OfficialSkills: official, + UpdatedSkills: slices.Clone(official), + AddedOfficialSkills: added, + UpdatedAt: time.Now().UTC().Format(time.RFC3339), + } + state.ensureNonNilSlices() + return state +} + func statePath() string { return filepath.Join(core.GetBaseConfigDir(), stateFile) } @@ -70,6 +123,32 @@ func WriteState(state SkillsState) error { return validate.AtomicWrite(statePath(), append(data, '\n'), 0o644) } +// SnapshotState captures the exact state file and returns a restore function. +// Distribution installation uses it to roll back a state write together with +// the managed Skills directories when a later binary replacement fails. +func SnapshotState() (restore func() error, err error) { + path := statePath() + data, readErr := vfs.ReadFile(path) + if readErr != nil { + if !errors.Is(readErr, fs.ErrNotExist) { + return nil, readErr + } + return func() error { + removeErr := vfs.Remove(path) + if errors.Is(removeErr, fs.ErrNotExist) { + return nil + } + return removeErr + }, nil + } + return func() error { + if err := vfs.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return err + } + return validate.AtomicWrite(path, data, 0o644) + }, nil +} + func ReadSyncedVersion() (string, bool) { state, ok, err := ReadState() if err != nil || !ok || state.Version == "" { diff --git a/internal/skillscheck/state_test.go b/internal/skillscheck/state_test.go index 9a2b2a5dec..7e9cd7b7eb 100644 --- a/internal/skillscheck/state_test.go +++ b/internal/skillscheck/state_test.go @@ -27,6 +27,31 @@ func TestReadState_Missing(t *testing.T) { } } +func TestNewCompleteStateOwnsManagedStateSemantics(t *testing.T) { + previous := &SkillsState{OfficialSkills: []string{"existing", "retired"}} + got := NewCompleteState("target", LayoutSeparate, []string{"existing", "new"}, previous) + + if got.Version != "target" || got.Layout != LayoutSeparate { + t.Fatalf("state identity = %#v", got) + } + if !reflect.DeepEqual(got.OfficialSkills, []string{"existing", "new"}) || + !reflect.DeepEqual(got.UpdatedSkills, []string{"existing", "new"}) || + !reflect.DeepEqual(got.AddedOfficialSkills, []string{"new"}) { + t.Fatalf("state Skills = %#v", got) + } + if got.SkippedDeletedSkills == nil || got.UpdatedAt == "" { + t.Fatalf("state completeness = %#v", got) + } + if known := KnownOfficialSkills(&SkillsState{OfficialSkillsUnknown: true, OfficialSkills: []string{"existing"}}); known != nil { + t.Fatalf("unknown official Skills = %#v, want nil", known) + } + known := KnownOfficialSkills(&got) + known[0] = "mutated" + if got.OfficialSkills[0] == "mutated" { + t.Fatal("KnownOfficialSkills returned state-owned storage") + } +} + func TestReadState_Valid(t *testing.T) { dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) diff --git a/internal/skillscheck/sync.go b/internal/skillscheck/sync.go index 4239a9e0df..f05d6d2dbc 100644 --- a/internal/skillscheck/sync.go +++ b/internal/skillscheck/sync.go @@ -500,6 +500,7 @@ func finishSync(opts SyncOptions, layout Layout, plan SyncPlan, action, warning } state := SkillsState{ Version: opts.Version, + SourceIdentity: OfficialSourceIdentity, Layout: layout, OfficialSkills: plan.OfficialSkills, OfficialSkillsUnknown: officialUnknown, diff --git a/internal/update/update.go b/internal/update/update.go index 2d0b8bef2f..1cb4b4e542 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -4,22 +4,22 @@ package update import ( + "context" "encoding/json" "fmt" "io" "net/http" "os" "path/filepath" - "regexp" - "strconv" - "strings" "sync/atomic" "time" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/distribution" "github.com/larksuite/cli/internal/transport" "github.com/larksuite/cli/internal/urlrewrite" "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/internal/versioncheck" "github.com/larksuite/cli/internal/vfs" ) @@ -29,13 +29,13 @@ const ( fetchTimeout = 15 * time.Second stateFile = "update-state.json" maxBody = 256 << 10 // 256 KB - ) // UpdateInfo holds version update information. type UpdateInfo struct { Current string `json:"current"` Latest string `json:"latest"` + Source string `json:"source,omitempty"` } // Message returns a concise update notification including the canonical @@ -43,6 +43,9 @@ type UpdateInfo struct { // AI agents can parse a unified "run: lark-cli update" hint across // both notice types. func (u *UpdateInfo) Message() string { + if u.Source != "" { + return fmt.Sprintf("lark-cli target %s configured, current %s, run: lark-cli update", u.Latest, u.Current) + } return fmt.Sprintf("lark-cli %s available, current %s, run: lark-cli update", u.Latest, u.Current) } @@ -70,92 +73,65 @@ func httpClient() *http.Client { type updateState struct { LatestVersion string `json:"latest_version"` CheckedAt int64 `json:"checked_at"` + Source string `json:"source,omitempty"` } // CheckCached checks the local cache only (no network). Always fast. -func CheckCached(currentVersion string) *UpdateInfo { - if shouldSkip(currentVersion) { +func CheckCached(ctx context.Context, currentVersion string) *UpdateInfo { + src, err := distribution.ResolveSource(ctx) + if err != nil || shouldSkip(currentVersion, src.ManifestMode()) { return nil } state, _ := loadState() - if state == nil || state.LatestVersion == "" { + if state == nil || state.LatestVersion == "" || state.Source != src.Identity() { return nil } - if !IsNewer(state.LatestVersion, currentVersion) { + if src.ManifestMode() { + if state.LatestVersion == currentVersion { + return nil + } + return &UpdateInfo{Current: currentVersion, Latest: state.LatestVersion, Source: "manifest"} + } + if !versioncheck.IsNewer(state.LatestVersion, currentVersion) { return nil } return &UpdateInfo{Current: currentVersion, Latest: state.LatestVersion} } -// RefreshCache fetches the latest version from npm and updates the local cache. +// RefreshCache fetches the configured target and updates the local cache. // No-op if the cache is still fresh (< 24h). Safe to call from a goroutine. -func RefreshCache(currentVersion string) { - if shouldSkip(currentVersion) { +func RefreshCache(ctx context.Context, currentVersion string) { + src, err := distribution.ResolveSource(ctx) + if err != nil || shouldSkip(currentVersion, src.ManifestMode()) { return } state, _ := loadState() - if state != nil && time.Since(time.Unix(state.CheckedAt, 0)) < cacheTTL { + if state != nil && state.Source == src.Identity() && time.Since(time.Unix(state.CheckedAt, 0)) < cacheTTL { return // cache is fresh } - latest, err := fetchLatestVersion() - if err != nil { + version, fetchErr := fetchTargetVersion(context.Background(), src) + if fetchErr != nil { return } _ = saveState(&updateState{ - LatestVersion: latest, + LatestVersion: version, CheckedAt: time.Now().Unix(), + Source: src.Identity(), }) } -func shouldSkip(version string) bool { - if os.Getenv("LARKSUITE_CLI_NO_UPDATE_NOTIFIER") != "" { +// shouldSkip suppresses the notifier in CI, when opted out, or without a +// usable version. The npm flow additionally only tracks published releases; +// a manifest distribution may target development builds. +func shouldSkip(version string, manifestMode bool) bool { + if os.Getenv("LARKSUITE_CLI_NO_UPDATE_NOTIFIER") != "" || versioncheck.IsCIEnv() || version == "" { return true } - // Suppress in CI environments. - if IsCIEnv() { - return true - } - // No version info at all — can't compare. - if version == "DEV" || version == "dev" || version == "" { - return true - } - // Skip local dev builds (e.g. v1.0.0-12-g9b933f1-dirty from git describe). - // Only released versions (clean X.Y.Z) should check for updates. - if !isRelease(version) { - return true - } - return false -} - -// isRelease returns true for published versions: clean semver (1.0.0) -// and npm prerelease (1.0.0-beta.1, 1.0.0-rc.1). -// Returns false for git describe dev builds (v1.0.0-12-g9b933f1-dirty). -var gitDescribePattern = regexp.MustCompile(`-\d+-g[0-9a-f]{7,}`) - -func isRelease(version string) bool { - v := strings.TrimPrefix(version, "v") - if ParseVersion(v) == nil { + if manifestMode { return false } - return !gitDescribePattern.MatchString(v) -} - -// IsRelease reports whether version looks like a clean published release -// (semver "1.0.0", or npm prerelease "1.0.0-beta.1") and not a git-describe -// dev build like "1.0.0-12-g9b933f1-dirty". Exported so internal/skillscheck -// can apply the same release-only gating without duplicating the regex. -func IsRelease(version string) bool { return isRelease(version) } - -// IsCIEnv returns true when any of the standard CI environment variables -// is set. Exported for internal/skillscheck so its skip rules track the -// same CI-suppression behavior as the update notifier. -func IsCIEnv() bool { - for _, key := range []string{"CI", "BUILD_NUMBER", "RUN_ID"} { - if os.Getenv(key) != "" { - return true - } - } - return false + // Skip local dev builds (e.g. v1.0.0-12-g9b933f1-dirty from git describe). + return version == "DEV" || version == "dev" || !versioncheck.IsRelease(version) } // --- state file I/O --- @@ -188,10 +164,49 @@ func saveState(s *updateState) error { return validate.AtomicWrite(statePath(), data, 0644) } -// FetchLatest queries the npm registry and returns the latest published version. -// This is a synchronous call with timeout, intended for diagnostic commands (doctor). -func FetchLatest() (string, error) { - return fetchLatestVersion() +// Target describes the active source's desired CLI version. +type Target struct { + Version string + Exact bool +} + +// Available reports whether the target should be offered for current. +func (t Target) Available(current string) bool { + if t.Exact { + return t.Version != "" && t.Version != current + } + return versioncheck.IsNewer(t.Version, current) +} + +// FetchTarget synchronously queries the active update source. It is intended +// for explicit checks such as update and doctor. +func FetchTarget(ctx context.Context) (Target, error) { + src, err := distribution.ResolveSource(ctx) + if err != nil { + return Target{}, err + } + return FetchTargetForSource(ctx, src) +} + +// FetchTargetForSource queries an already-resolved source without consulting +// the extension registry again. +func FetchTargetForSource(ctx context.Context, src distribution.Source) (Target, error) { + version, err := fetchTargetVersion(ctx, src) + if err != nil { + return Target{}, err + } + return Target{Version: version, Exact: src.ManifestMode()}, nil +} + +func fetchTargetVersion(ctx context.Context, src distribution.Source) (string, error) { + if src.ManifestMode() { + manifest, err := src.FetchManifest(ctx) + if err != nil { + return "", err + } + return manifest.Version, nil + } + return fetchLatestVersion(ctx) } // --- npm registry --- @@ -200,8 +215,12 @@ type npmLatestResponse struct { Version string `json:"version"` } -func fetchLatestVersion() (string, error) { - resp, err := httpClient().Get(urlrewrite.Rewrite(registryURL)) +func fetchLatestVersion(ctx context.Context) (string, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, urlrewrite.Rewrite(registryURL), nil) + if err != nil { + return "", err + } + resp, err := httpClient().Do(req) if err != nil { return "", err } @@ -225,135 +244,3 @@ func fetchLatestVersion() (string, error) { } return result.Version, nil } - -// --- semver helpers --- - -// IsNewer returns true if version a should be considered an update over b. -// -// When both parse as semver, standard comparison applies. -// When b cannot be parsed (e.g. bare commit hash "9b933f1"), any valid a -// is considered newer — an unparseable local version is assumed outdated. -// When a cannot be parsed, returns false (can't confirm it's newer). -func IsNewer(a, b string) bool { - ap := parseVersionDetail(a) - bp := parseVersionDetail(b) - if ap == nil { - return false // can't confirm remote is newer - } - if bp == nil { - return true // local version unparseable → assume outdated - } - for i := 0; i < 3; i++ { - if ap.core[i] > bp.core[i] { - return true - } - if ap.core[i] < bp.core[i] { - return false - } - } - return comparePrerelease(ap.prerelease, bp.prerelease) > 0 -} - -// ParseVersion parses "X.Y.Z" (with optional "v" prefix and pre-release suffix) -// into [major, minor, patch]. Returns nil on invalid input. -func ParseVersion(v string) []int { - parsed := parseVersionDetail(v) - if parsed == nil { - return nil - } - return []int{parsed.core[0], parsed.core[1], parsed.core[2]} -} - -type parsedVersion struct { - core [3]int - prerelease string -} - -// validPrerelease matches semver pre-release identifiers (dot-separated). -// Each identifier is either: "0", a non-zero-leading numeric, or alphanumeric with at least one letter/hyphen. -// Rejects empty identifiers ("1.0.0-"), leading-zero numerics ("1.0.0-01"), etc. -var validPrerelease = regexp.MustCompile( - `^(?:0|[1-9]\d*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*)` + - `(?:\.(?:0|[1-9]\d*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*))*$`) - -func parseVersionDetail(v string) *parsedVersion { - v = strings.TrimPrefix(v, "v") - if idx := strings.Index(v, "+"); idx >= 0 { - v = v[:idx] - } - prerelease := "" - if idx := strings.Index(v, "-"); idx >= 0 { - prerelease = v[idx+1:] - v = v[:idx] - if prerelease == "" || !validPrerelease.MatchString(prerelease) { - return nil - } - } - parts := strings.SplitN(v, ".", 3) - if len(parts) != 3 { - return nil - } - var nums [3]int - for i, p := range parts { - if len(p) > 1 && p[0] == '0' { - return nil // leading zero in core part (e.g. "01.0.0") - } - n, err := strconv.Atoi(p) - if err != nil { - return nil - } - nums[i] = n - } - return &parsedVersion{core: nums, prerelease: prerelease} -} - -func comparePrerelease(a, b string) int { - if a == "" && b == "" { - return 0 - } - if a == "" { - return 1 - } - if b == "" { - return -1 - } - ap := strings.Split(a, ".") - bp := strings.Split(b, ".") - for i := 0; i < len(ap) && i < len(bp); i++ { - cmp := comparePrereleaseIdentifier(ap[i], bp[i]) - if cmp != 0 { - return cmp - } - } - switch { - case len(ap) > len(bp): - return 1 - case len(ap) < len(bp): - return -1 - default: - return 0 - } -} - -func comparePrereleaseIdentifier(a, b string) int { - an, aErr := strconv.Atoi(a) - bn, bErr := strconv.Atoi(b) - aNumeric := aErr == nil - bNumeric := bErr == nil - switch { - case aNumeric && bNumeric: - if an > bn { - return 1 - } - if an < bn { - return -1 - } - return 0 - case aNumeric: - return -1 - case bNumeric: - return 1 - default: - return strings.Compare(a, b) - } -} diff --git a/internal/update/update_test.go b/internal/update/update_test.go index 5750840726..739b5c9ee9 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -6,6 +6,7 @@ package update import ( "context" "encoding/json" + "fmt" "io" "net/http" "net/http/httptest" @@ -17,6 +18,8 @@ import ( "time" exttransport "github.com/larksuite/cli/extension/transport" + "github.com/larksuite/cli/internal/distribution" + "github.com/larksuite/cli/internal/vfs" ) // roundTripFunc adapts a function to http.RoundTripper. @@ -26,6 +29,7 @@ func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { re type updateExternalProvider struct { interceptor exttransport.Interceptor + manifestURL string rewriter exttransport.URLRewriter } @@ -35,6 +39,10 @@ func (p updateExternalProvider) ResolveInterceptor(context.Context) exttransport return p.interceptor } +func (p updateExternalProvider) ResolveManifestURL(context.Context) string { + return p.manifestURL +} + func (p updateExternalProvider) ResolveURLRewriter(context.Context) exttransport.URLRewriter { return p.rewriter } @@ -57,6 +65,52 @@ func (i *updateExternalInterceptor) PreRoundTrip(req *http.Request) func(*http.R return nil } +func TestManifestCacheUsesExactTargetAndSourceIdentity(t *testing.T) { + clearSkipEnv(t) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("X-External-Route") != "" { + t.Fatal("manifest request passed through the request interceptor") + } + target := "old-target" + if r.URL.Path == "/second" { + target = "second-target" + } + fmt.Fprintf(w, `{"schema":1,"version":%q,"artifacts":{"skills":{"url":"https://dist.example/skills","checksum":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},%q:{"url":"https://dist.example/binary","checksum":"sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"}}}`, target, distribution.CurrentPlatformKey()) + })) + defer server.Close() + previousProvider := exttransport.GetProvider() + previousClient := distribution.DefaultClient + distribution.DefaultClient = server.Client() + exttransport.Register(updateExternalProvider{interceptor: &updateExternalInterceptor{}, manifestURL: server.URL + "/first"}) + t.Cleanup(func() { + exttransport.Register(previousProvider) + distribution.DefaultClient = previousClient + }) + + RefreshCache(context.Background(), "new-current") + stateBytes, err := vfs.ReadFile(statePath()) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(stateBytes), server.URL) { + t.Fatal("update cache persisted the manifest URL") + } + info := CheckCached(context.Background(), "new-current") + if info == nil || info.Latest != "old-target" || info.Source != "manifest" { + t.Fatalf("CheckCached = %#v", info) + } + + // A different manifest is a different source even while the 24-hour cache + // from the first source is fresh. + exttransport.Register(updateExternalProvider{manifestURL: server.URL + "/second"}) + RefreshCache(context.Background(), "new-current") + info = CheckCached(context.Background(), "new-current") + if info == nil || info.Latest != "second-target" { + t.Fatalf("CheckCached after source switch = %#v", info) + } +} + // clearSkipEnv unsets all env vars that shouldSkip checks, // preventing the host environment (e.g. CI=true) from polluting test results. func clearSkipEnv(t *testing.T) { @@ -75,73 +129,6 @@ func mustParseURL(raw string) *url.URL { return u } -func TestIsNewer(t *testing.T) { - tests := []struct { - a, b string - want bool - }{ - {"1.1.0", "1.0.0", true}, - {"1.0.0", "1.0.0", false}, - {"1.0.0", "1.1.0", false}, - {"2.0.0", "1.9.9", true}, - {"1.0.1", "1.0.0", true}, - {"v1.1.0", "1.0.0", true}, - {"1.1.0", "v1.0.0", true}, - {"0.0.1", "0.0.0", true}, - {"DEV", "1.0.0", false}, // unparseable remote → false - {"1.0.0", "DEV", true}, // unparseable local → assume outdated - {"1.0.0", "9b933f1", true}, // bare commit hash → assume outdated - {"", "1.0.0", false}, // empty remote → false - {"1.1.0", "v1.0.0-12-g9b933f1-dirty", true}, // git describe: 1.1.0 > 1.0.0 - {"1.0.0", "1.0.0-rc.1", true}, // stable release > prerelease - {"1.0.0-rc.2", "1.0.0-rc.1", true}, // prerelease identifiers are ordered - {"1.0.0-rc.1", "1.0.0", false}, // prerelease < stable release - } - for _, tt := range tests { - got := IsNewer(tt.a, tt.b) - if got != tt.want { - t.Errorf("IsNewer(%q, %q) = %v, want %v", tt.a, tt.b, got, tt.want) - } - } -} - -func TestParseVersion(t *testing.T) { - tests := []struct { - input string - want []int - }{ - {"1.2.3", []int{1, 2, 3}}, - {"v1.2.3", []int{1, 2, 3}}, - {"0.0.1", []int{0, 0, 1}}, - {"1.0.0-beta.1", []int{1, 0, 0}}, - {"1.0.0-rc.1", []int{1, 0, 0}}, - {"1.0.0-0", []int{1, 0, 0}}, - {"1.0.0+build.123", []int{1, 0, 0}}, - {"1.0.0-beta.1+build", []int{1, 0, 0}}, - {"1.0.0-", nil}, // empty pre-release - {"1.0.0-01", nil}, // leading zero in numeric pre-release - {"1.0.0-beta..1", nil}, // empty identifier between dots - {"01.0.0", nil}, // leading zero in major - {"1.00.0", nil}, // leading zero in minor - {"1.0.00", nil}, // leading zero in patch - {"DEV", nil}, - {"", nil}, - {"1.2", nil}, - } - for _, tt := range tests { - got := ParseVersion(tt.input) - if tt.want == nil { - if got != nil { - t.Errorf("ParseVersion(%q) = %v, want nil", tt.input, got) - } - continue - } - if got == nil || got[0] != tt.want[0] || got[1] != tt.want[1] || got[2] != tt.want[2] { - t.Errorf("ParseVersion(%q) = %v, want %v", tt.input, got, tt.want) - } - } -} - func TestShouldSkip(t *testing.T) { tests := []struct { name string @@ -170,7 +157,7 @@ func TestShouldSkip(t *testing.T) { for k, v := range tt.env { t.Setenv(k, v) } - got := shouldSkip(tt.version) + got := shouldSkip(tt.version, false) if got != tt.want { t.Errorf("shouldSkip(%q) = %v, want %v", tt.version, got, tt.want) } @@ -178,34 +165,6 @@ func TestShouldSkip(t *testing.T) { } } -func TestIsRelease(t *testing.T) { - tests := []struct { - name string - ver string - want bool - }{ - {"clean_semver", "1.0.0", true}, - {"v_prefix", "v1.0.0", true}, - {"prerelease", "1.0.0-beta.1", true}, - {"rc", "1.0.0-rc.1", true}, - {"alpha_prerelease", "2.0.0-alpha.0", true}, - {"git_describe_dirty", "1.0.0-12-g9b933f1-dirty", false}, - {"git_describe_clean", "1.0.0-12-g9b933f1", false}, - {"bare_commit_hash", "9b933f1", false}, - {"dev_marker", "DEV", false}, - {"incomplete_semver", "1.0", false}, - {"empty", "", false}, - {"invalid", "not-a-version", false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := IsRelease(tt.ver); got != tt.want { - t.Errorf("IsRelease(%q) = %v, want %v", tt.ver, got, tt.want) - } - }) - } -} - func TestUpdateInfoMethods(t *testing.T) { info := &UpdateInfo{Current: "1.0.0", Latest: "2.0.0"} got := info.Message() @@ -221,7 +180,7 @@ func TestCheckCached(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp) // No cache → nil - info := CheckCached("1.0.0") + info := CheckCached(context.Background(), "1.0.0") if info != nil { t.Errorf("expected nil with no cache, got %+v", info) } @@ -231,7 +190,7 @@ func TestCheckCached(t *testing.T) { data, _ := json.Marshal(state) os.WriteFile(filepath.Join(tmp, stateFile), data, 0644) - info = CheckCached("1.0.0") + info = CheckCached(context.Background(), "1.0.0") if info == nil { t.Fatal("expected update info, got nil") } @@ -240,7 +199,7 @@ func TestCheckCached(t *testing.T) { } // Same version → nil - info = CheckCached("2.0.0") + info = CheckCached(context.Background(), "2.0.0") if info != nil { t.Errorf("expected nil when versions match, got %+v", info) } @@ -265,10 +224,10 @@ func TestRefreshCache(t *testing.T) { }) defer func() { DefaultClient = nil }() - RefreshCache("1.0.0") + RefreshCache(context.Background(), "1.0.0") // Verify cache was written - info := CheckCached("1.0.0") + info := CheckCached(context.Background(), "1.0.0") if info == nil { t.Fatal("expected update info after refresh, got nil") } @@ -277,7 +236,7 @@ func TestRefreshCache(t *testing.T) { } // Second refresh should be no-op (cache is fresh) — won't hit network. - RefreshCache("1.0.0") + RefreshCache(context.Background(), "1.0.0") } func TestFetchLatestVersionRewritesRegistryAndUsesExternalClass(t *testing.T) { @@ -311,7 +270,7 @@ func TestFetchLatestVersionRewritesRegistryAndUsesExternalClass(t *testing.T) { }) t.Cleanup(func() { http.DefaultTransport = previousTransport }) - if _, err := fetchLatestVersion(); err != nil { + if _, err := fetchLatestVersion(context.Background()); err != nil { t.Fatal(err) } @@ -340,19 +299,3 @@ func TestPendingAtomicAccess(t *testing.T) { // Clean up for other tests SetPending(nil) } - -func TestIsCIEnv(t *testing.T) { - clearSkipEnv(t) - if IsCIEnv() { - t.Fatal("IsCIEnv() = true after clearSkipEnv, want false") - } - for _, key := range []string{"CI", "BUILD_NUMBER", "RUN_ID"} { - t.Run(key, func(t *testing.T) { - clearSkipEnv(t) - t.Setenv(key, "1") - if !IsCIEnv() { - t.Errorf("IsCIEnv() = false with %s=1, want true", key) - } - }) - } -} diff --git a/internal/versioncheck/versioncheck.go b/internal/versioncheck/versioncheck.go new file mode 100644 index 0000000000..b72f00e4ec --- /dev/null +++ b/internal/versioncheck/versioncheck.go @@ -0,0 +1,90 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +// Package versioncheck owns version and environment predicates shared by +// update and Skills notification checks. +package versioncheck + +import ( + "os" + "regexp" + "strconv" + "strings" + + "golang.org/x/mod/semver" +) + +var gitDescribePattern = regexp.MustCompile(`-\d+-g[0-9a-f]{7,}`) + +// IsRelease reports whether version is a clean published SemVer rather than a +// git-describe development build. +func IsRelease(version string) bool { + canonical, ok := canonical(version) + return ok && !gitDescribePattern.MatchString(canonical) +} + +// IsNewer reports whether a is a SemVer update over b. A valid remote version +// is considered newer than an unparseable local development version. +func IsNewer(a, b string) bool { + remote, remoteOK := canonical(a) + if !remoteOK { + return false + } + local, localOK := canonical(b) + return !localOK || semver.Compare(remote, local) > 0 +} + +// Normalize canonicalizes a version string for comparison: trims whitespace +// and strips a leading "v"/"V" so versions written by the Makefile +// (git describe → "v1.0.0") and npm (no prefix → "1.0.0") compare equal. +func Normalize(version string) string { + version = strings.TrimSpace(version) + version = strings.TrimPrefix(version, "v") + return strings.TrimPrefix(version, "V") +} + +// Equal reports whether two versions are the same after Normalize. +func Equal(a, b string) bool { return Normalize(a) == Normalize(b) } + +// Parse returns the major, minor, and patch components of a SemVer value. +func Parse(version string) []int { + canonicalVersion, ok := canonical(version) + if !ok { + return nil + } + core := strings.SplitN(strings.TrimPrefix(canonicalVersion, "v"), "-", 2)[0] + core = strings.SplitN(core, "+", 2)[0] + parts := strings.Split(core, ".") + result := make([]int, 3) + for i, part := range parts { + result[i], _ = strconv.Atoi(part) + } + return result +} + +func canonical(version string) (string, bool) { + version = strings.TrimPrefix(version, "v") + core := strings.SplitN(strings.SplitN(version, "-", 2)[0], "+", 2)[0] + parts := strings.Split(core, ".") + if len(parts) != 3 { + return "", false + } + for _, part := range parts { + if _, err := strconv.Atoi(part); err != nil { + return "", false + } + } + canonicalVersion := "v" + version + return canonicalVersion, semver.IsValid(canonicalVersion) +} + +// IsCIEnv reports whether the process is running in a supported CI +// environment. +func IsCIEnv() bool { + for _, key := range []string{"CI", "BUILD_NUMBER", "RUN_ID"} { + if os.Getenv(key) != "" { + return true + } + } + return false +} diff --git a/internal/versioncheck/versioncheck_test.go b/internal/versioncheck/versioncheck_test.go new file mode 100644 index 0000000000..d09823b8a4 --- /dev/null +++ b/internal/versioncheck/versioncheck_test.go @@ -0,0 +1,143 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package versioncheck + +import "testing" + +func TestIsRelease(t *testing.T) { + for _, tt := range []struct { + version string + want bool + }{ + {"1.0.0", true}, + {"v1.0.0", true}, + {"1.0.0-beta.1", true}, + {"1.0.0+build.1", true}, + {"1.0.0-12-g9b933f1", false}, + {"1.0", false}, + {"DEV", false}, + } { + if got := IsRelease(tt.version); got != tt.want { + t.Errorf("IsRelease(%q) = %v, want %v", tt.version, got, tt.want) + } + } +} + +func TestIsNewerFollowsSemVerPrecedence(t *testing.T) { + ordered := []string{ + "1.0.0-alpha", + "1.0.0-alpha.1", + "1.0.0-alpha.beta", + "1.0.0-beta", + "1.0.0-beta.2", + "1.0.0-beta.11", + "1.0.0-rc.1", + "1.0.0", + "1.0.1", + "1.1.0", + "2.0.0", + } + for i := 1; i < len(ordered); i++ { + older, newer := ordered[i-1], ordered[i] + t.Run(older+"_to_"+newer, func(t *testing.T) { + if !IsNewer(newer, older) { + t.Fatalf("IsNewer(%q, %q) = false", newer, older) + } + if IsNewer(older, newer) { + t.Fatalf("IsNewer(%q, %q) = true", older, newer) + } + }) + } +} + +func TestIsNewerHandlesVersionInputBoundaries(t *testing.T) { + for _, tt := range []struct { + name string + remote string + local string + want bool + }{ + {name: "v prefix", remote: "v1.2.4", local: "1.2.3", want: true}, + {name: "build metadata ignored", remote: "1.2.3+new", local: "1.2.3+old", want: false}, + {name: "valid remote replaces development build", remote: "1.2.3", local: "DEV", want: true}, + {name: "invalid remote rejected", remote: "latest", local: "1.2.3", want: false}, + {name: "equal version", remote: "1.2.3", local: "1.2.3", want: false}, + } { + t.Run(tt.name, func(t *testing.T) { + if got := IsNewer(tt.remote, tt.local); got != tt.want { + t.Fatalf("IsNewer(%q, %q) = %v, want %v", tt.remote, tt.local, got, tt.want) + } + }) + } +} + +func TestParse(t *testing.T) { + tests := []struct { + input string + want []int + }{ + {"1.2.3", []int{1, 2, 3}}, + {"v1.2.3", []int{1, 2, 3}}, + {"0.0.1", []int{0, 0, 1}}, + {"1.0.0-beta.1", []int{1, 0, 0}}, + {"1.0.0-rc.1", []int{1, 0, 0}}, + {"1.0.0-0", []int{1, 0, 0}}, + {"1.0.0+build.123", []int{1, 0, 0}}, + {"1.0.0-beta.1+build", []int{1, 0, 0}}, + {"1.0.0-", nil}, // empty pre-release + {"1.0.0-01", nil}, // leading zero in numeric pre-release + {"1.0.0-beta..1", nil}, // empty identifier between dots + {"01.0.0", nil}, // leading zero in major + {"1.00.0", nil}, // leading zero in minor + {"1.0.00", nil}, // leading zero in patch + {"DEV", nil}, + {"", nil}, + {"1.2", nil}, + } + for _, tt := range tests { + got := Parse(tt.input) + if tt.want == nil { + if got != nil { + t.Errorf("Parse(%q) = %v, want nil", tt.input, got) + } + continue + } + if got == nil || got[0] != tt.want[0] || got[1] != tt.want[1] || got[2] != tt.want[2] { + t.Errorf("Parse(%q) = %v, want %v", tt.input, got, tt.want) + } + } +} + +func TestNormalizeAndEqual(t *testing.T) { + for _, tt := range []struct { + input string + want string + }{ + {"1.2.3", "1.2.3"}, + {"v1.2.3", "1.2.3"}, + {"V1.2.3", "1.2.3"}, + {" v1.2.3 ", "1.2.3"}, + } { + if got := Normalize(tt.input); got != tt.want { + t.Errorf("Normalize(%q) = %q, want %q", tt.input, got, tt.want) + } + } + if !Equal("v1.2.3", "1.2.3") || Equal("1.2.3", "1.2.4") { + t.Error("Equal mismatch") + } +} + +func TestIsCIEnv(t *testing.T) { + for _, key := range []string{"CI", "BUILD_NUMBER", "RUN_ID"} { + t.Run(key, func(t *testing.T) { + for _, candidate := range []string{"CI", "BUILD_NUMBER", "RUN_ID"} { + t.Setenv(candidate, "") + } + t.Setenv(key, "1") + if !IsCIEnv() { + t.Fatalf("IsCIEnv() = false with %s set", key) + } + }) + } +}