diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ffa82c3..6ae1b03e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,7 +30,7 @@ jobs: - name: Build run: go build ./... - name: Vet - run: go vet ./... + run: go vet -tags testing ./... - name: Check formatting run: | unformatted=$(gofmt -l .) @@ -63,7 +63,7 @@ jobs: - name: Fetch meta data run: python3 scripts/fetch_meta.py - name: Run tests - run: go test -v -race -count=1 -timeout=5m ./cmd/... ./internal/... ./shortcuts/... + run: go test -tags testing -v -race -count=1 -timeout=5m ./cmd/... ./internal/... ./shortcuts/... ./extension/... lint: needs: fast-gate @@ -81,7 +81,7 @@ jobs: - name: Fetch meta data run: python3 scripts/fetch_meta.py - name: Run golangci-lint - run: go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/main + run: go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --build-tags=testing --new-from-rev=origin/main coverage: needs: fast-gate @@ -99,7 +99,7 @@ jobs: - name: Run tests with coverage run: | packages=$(go list ./... | grep -v '^github.com/larksuite/cli/tests/cli_e2e$' | grep -v '^github.com/larksuite/cli/tests/cli_e2e/') - go test -race -coverprofile=coverage.txt -covermode=atomic $packages + go test -tags testing -race -coverprofile=coverage.txt -covermode=atomic $packages - name: Upload coverage to Codecov uses: codecov/codecov-action@3f20e214133d0983f9a10f3d63b0faf9241a3daa # v6 with: @@ -153,14 +153,14 @@ jobs: run: | # Analyze current HEAD (strip line:col for stable diff across line shifts) # Filter "go: downloading ..." lines to avoid false diffs from module cache state - go run golang.org/x/tools/cmd/deadcode@v0.31.0 -test ./... 2>&1 | \ + go run golang.org/x/tools/cmd/deadcode@v0.31.0 -tags=testing -test ./... 2>&1 | \ grep -v '^go: ' | \ sed 's/:[0-9][0-9]*:[0-9][0-9]*:/:/' | sort > /tmp/dc-head.txt # Analyze base branch via worktree git worktree add -q /tmp/dc-base "origin/${{ github.base_ref }}" (cd /tmp/dc-base && python3 scripts/fetch_meta.py && \ - go run golang.org/x/tools/cmd/deadcode@v0.31.0 -test ./... 2>&1 | \ + go run golang.org/x/tools/cmd/deadcode@v0.31.0 -tags=testing -test ./... 2>&1 | \ grep -v '^go: ' | \ sed 's/:[0-9][0-9]*:[0-9][0-9]*:/:/' | sort > /tmp/dc-base.txt) || { echo "::warning::Failed to analyze base branch — skipping incremental dead code check" diff --git a/Makefile b/Makefile index 7733335b..e0a8b5fd 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ DATE := $(shell date +%Y-%m-%d) LDFLAGS := -s -w -X $(MODULE)/internal/build.Version=$(VERSION) -X $(MODULE)/internal/build.Date=$(DATE) PREFIX ?= /usr/local -.PHONY: all build vet test unit-test integration-test install uninstall clean fetch_meta gitleaks +.PHONY: all build vet fmt-check test unit-test integration-test examples-build install uninstall clean fetch_meta gitleaks all: test @@ -19,15 +19,37 @@ build: fetch_meta go build -trimpath -ldflags "$(LDFLAGS)" -o $(BINARY) . vet: fetch_meta - go vet ./... + go vet -tags testing ./... +# fmt-check fails when any file would be reformatted by gofmt. Keep this +# in sync with the fast-gate "Check formatting" step in CI. +fmt-check: + @unformatted=$$(gofmt -l . | grep -v '^\.claude/' || true); \ + if [ -n "$$unformatted" ]; then \ + echo "Unformatted Go files:"; \ + echo "$$unformatted"; \ + echo "Run 'gofmt -w .' and commit."; \ + exit 1; \ + fi + +# unit-test passes -tags testing because public-SDK packages gate test-only +# helpers (e.g. platform.ResetForTesting) behind //go:build testing. The +# ./extension/... package list keeps the public plugin SDK in the default +# test matrix. unit-test: fetch_meta - go test -race -gcflags="all=-N -l" -count=1 ./cmd/... ./internal/... ./shortcuts/... + go test -tags testing -race -gcflags="all=-N -l" -count=1 \ + ./cmd/... ./internal/... ./shortcuts/... ./extension/... + +# examples-build keeps the shipped plugin-SDK examples compilable. If this +# breaks, the plugin author guide's "go build ./..." path is broken. +examples-build: + go build ./extension/platform/examples/audit-observer + go build ./extension/platform/examples/readonly-policy integration-test: build go test -v -count=1 ./tests/... -test: vet unit-test integration-test +test: vet fmt-check unit-test examples-build integration-test install: build install -d $(PREFIX)/bin diff --git a/cmd/build.go b/cmd/build.go index 7ab55604..b7db05c2 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -19,10 +19,10 @@ import ( cmdupdate "github.com/larksuite/cli/cmd/update" _ "github.com/larksuite/cli/events" "github.com/larksuite/cli/internal/build" + "github.com/larksuite/cli/internal/cmdpolicy" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/hook" "github.com/larksuite/cli/internal/keychain" - "github.com/larksuite/cli/internal/pruning" "github.com/larksuite/cli/shortcuts" "github.com/spf13/cobra" ) @@ -155,7 +155,7 @@ func buildInternal(ctx context.Context, inv cmdutil.InvocationContext, opts ...B // proceed normally, which it isn't. return f, rootCmd, nil } - var pluginRules []pruning.PluginRule + var pluginRules []cmdpolicy.PluginRule var registry *hook.Registry if installResult != nil { pluginRules = installResult.PluginRules diff --git a/cmd/config/plugins.go b/cmd/config/plugins.go index 9dab5c21..96c0537b 100644 --- a/cmd/config/plugins.go +++ b/cmd/config/plugins.go @@ -8,21 +8,20 @@ import ( "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/output" - "github.com/larksuite/cli/internal/plugininventory" + "github.com/larksuite/cli/internal/platform" ) // NewCmdConfigPlugins exposes the plugin inventory diagnostic command. // // `config policy show` is intentionally focused on the user-layer Rule // (Restrict). Plugins also contribute hooks (Observe / Wrap / Lifecycle) -// that are not policy in the pruning sense but still mutate the CLI's -// runtime behaviour. This command surfaces both halves so an operator -// can answer "what is this binary doing differently from stock lark-cli?" -// in one place. +// that are not policy gates but still mutate the CLI's runtime behaviour. +// This command surfaces both halves so an operator can answer "what is +// this binary doing differently from stock lark-cli?" in one place. // -// Like config policy show, the dispatch path is exempt from pruning -// (see internal/pruning/diagnostic.go) so it remains usable under any -// Rule. +// Like config policy show, the dispatch path is exempt from policy +// enforcement (see internal/cmdpolicy/diagnostic.go) so it remains +// usable under any Rule. func NewCmdConfigPlugins(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "plugins", @@ -60,7 +59,7 @@ the plugin name as the prefix at registration time, so an entry } func runConfigPluginsShow(f *cmdutil.Factory) error { - inv := plugininventory.GetActive() + inv := internalplatform.GetActiveInventory() if inv == nil { output.PrintJson(f.IOStreams.Out, map[string]any{ "plugins": []any{}, diff --git a/cmd/config/policy.go b/cmd/config/policy.go index bd1f06ec..d3623e5e 100644 --- a/cmd/config/policy.go +++ b/cmd/config/policy.go @@ -9,10 +9,10 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/internal/cmdpolicy" + pyaml "github.com/larksuite/cli/internal/cmdpolicy/yaml" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/output" - "github.com/larksuite/cli/internal/pruning" - pyaml "github.com/larksuite/cli/internal/pruning/yaml" ) // NewCmdConfigPolicy returns the `config policy` group. Subcommands: @@ -65,13 +65,13 @@ marked as denied after father-group aggregation.`, } func runConfigPolicyShow(f *cmdutil.Factory) error { - active := pruning.GetActive() + active := cmdpolicy.GetActive() if active == nil { // Bootstrap not yet recorded -- happens when the command is // invoked from a context that bypassed buildInternal (only test // shells should hit this). output.PrintJson(f.IOStreams.Out, map[string]any{ - "source": string(pruning.SourceNone), + "source": string(cmdpolicy.SourceNone), "note": "no policy recorded; bootstrap did not run pruning", }) return nil @@ -85,17 +85,18 @@ func runConfigPolicyShow(f *cmdutil.Factory) error { } if active.Rule != nil { out["rule"] = map[string]any{ - "name": active.Rule.Name, - "description": active.Rule.Description, - "allow": active.Rule.Allow, - "deny": active.Rule.Deny, - "max_risk": active.Rule.MaxRisk, - "identities": active.Rule.Identities, + "name": active.Rule.Name, + "description": active.Rule.Description, + "allow": active.Rule.Allow, + "deny": active.Rule.Deny, + "max_risk": active.Rule.MaxRisk, + "identities": active.Rule.Identities, + "allow_unannotated": active.Rule.AllowUnannotated, } } // Surface the yaml-shadowed case so a user wondering "why is my // yaml ignored?" sees it immediately. - if active.Source.Kind == pruning.SourcePlugin && active.YAMLPath != "" { + if active.Source.Kind == cmdpolicy.SourcePlugin && active.YAMLPath != "" { if _, err := os.Stat(active.YAMLPath); err == nil { out["yaml_shadowed"] = true fmt.Fprintln(f.IOStreams.ErrOut, @@ -129,17 +130,18 @@ func runConfigPolicyValidate(f *cmdutil.Factory, path string) error { return output.Errorf(output.ExitValidation, "validation", "parse policy yaml %q: %v", path, err) } - if err := pruning.ValidateRule(rule); err != nil { + if err := cmdpolicy.ValidateRule(rule); err != nil { return output.Errorf(output.ExitValidation, "validation", "invalid rule in %q: %v", path, err) } output.PrintJson(f.IOStreams.Out, map[string]any{ - "ok": true, - "path": path, - "rule_name": rule.Name, - "allow": rule.Allow, - "deny": rule.Deny, - "max_risk": rule.MaxRisk, + "ok": true, + "path": path, + "rule_name": rule.Name, + "allow": rule.Allow, + "deny": rule.Deny, + "max_risk": rule.MaxRisk, + "allow_unannotated": rule.AllowUnannotated, }) return nil } diff --git a/cmd/config/policy_test.go b/cmd/config/policy_test.go index d2f9c186..524cab13 100644 --- a/cmd/config/policy_test.go +++ b/cmd/config/policy_test.go @@ -11,8 +11,8 @@ import ( "testing" "github.com/larksuite/cli/extension/platform" + "github.com/larksuite/cli/internal/cmdpolicy" "github.com/larksuite/cli/internal/cmdutil" - "github.com/larksuite/cli/internal/pruning" ) func newPolicyTestFactory() (*cmdutil.Factory, *bytes.Buffer, *bytes.Buffer) { @@ -28,8 +28,8 @@ func newPolicyTestFactory() (*cmdutil.Factory, *bytes.Buffer, *bytes.Buffer) { // When nothing is recorded the command must still produce a JSON // envelope with source=none and a note explaining the missing context. func TestConfigPolicyShow_NoActivePolicy(t *testing.T) { - pruning.ResetActiveForTesting() - t.Cleanup(pruning.ResetActiveForTesting) + cmdpolicy.ResetActiveForTesting() + t.Cleanup(cmdpolicy.ResetActiveForTesting) f, out, _ := newPolicyTestFactory() if err := runConfigPolicyShow(f); err != nil { @@ -51,18 +51,18 @@ func TestConfigPolicyShow_NoActivePolicy(t *testing.T) { // plus its source. yaml_shadowed is true when a yaml file exists but a // plugin overrode it; verified separately below. func TestConfigPolicyShow_PluginActive(t *testing.T) { - pruning.ResetActiveForTesting() - t.Cleanup(pruning.ResetActiveForTesting) + cmdpolicy.ResetActiveForTesting() + t.Cleanup(cmdpolicy.ResetActiveForTesting) rule := &platform.Rule{ Name: "secaudit", Allow: []string{"docs/**"}, MaxRisk: "read", } - pruning.SetActive(&pruning.ActivePolicy{ + cmdpolicy.SetActive(&cmdpolicy.ActivePolicy{ Rule: rule, - Source: pruning.ResolveSource{ - Kind: pruning.SourcePlugin, + Source: cmdpolicy.ResolveSource{ + Kind: cmdpolicy.SourcePlugin, Name: "secaudit", }, DeniedPaths: 42, @@ -99,8 +99,8 @@ func TestConfigPolicyShow_PluginActive(t *testing.T) { // user "yaml IGNORED" so they're not surprised that their yaml is // inert. func TestConfigPolicyShow_YamlShadowedWarning(t *testing.T) { - pruning.ResetActiveForTesting() - t.Cleanup(pruning.ResetActiveForTesting) + cmdpolicy.ResetActiveForTesting() + t.Cleanup(cmdpolicy.ResetActiveForTesting) dir := t.TempDir() yamlPath := filepath.Join(dir, "policy.yml") @@ -108,10 +108,10 @@ func TestConfigPolicyShow_YamlShadowedWarning(t *testing.T) { t.Fatalf("write yaml: %v", err) } - pruning.SetActive(&pruning.ActivePolicy{ + cmdpolicy.SetActive(&cmdpolicy.ActivePolicy{ Rule: &platform.Rule{Name: "plug"}, - Source: pruning.ResolveSource{ - Kind: pruning.SourcePlugin, + Source: cmdpolicy.ResolveSource{ + Kind: cmdpolicy.SourcePlugin, Name: "plug", }, YAMLPath: yamlPath, diff --git a/cmd/platform_bootstrap.go b/cmd/platform_bootstrap.go new file mode 100644 index 00000000..3f21f74f --- /dev/null +++ b/cmd/platform_bootstrap.go @@ -0,0 +1,244 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package cmd + +import ( + "context" + "fmt" + "io" + "path/filepath" + + "github.com/spf13/cobra" + + "github.com/larksuite/cli/extension/platform" + "github.com/larksuite/cli/internal/cmdpolicy" + "github.com/larksuite/cli/internal/hook" + internalplatform "github.com/larksuite/cli/internal/platform" + "github.com/larksuite/cli/internal/vfs" +) + +// userPolicyFileName is the conventional filename for the user-layer Rule. +// Lives under ~/.lark-cli/ to match the rest of the CLI's user-state +// directory. +const userPolicyFileName = "policy.yml" + +// applyUserPolicyPruning resolves the user-layer Rule from plugin +// contributions and/or ~/.lark-cli/policy.yml and installs denyStubs +// for commands it rejects. +// +// Missing yaml is not an error -- the CLI runs with no user-layer +// restriction. A malformed Rule (bad MaxRisk enum, malformed glob, etc.) +// surfaces via the returned error; the caller decides how to handle it. +// +// pluginRules carries Plugin.Restrict() contributions collected from +// the InstallAll phase; nil/empty is fine. +func applyUserPolicyPruning(rootCmd *cobra.Command, pluginRules []cmdpolicy.PluginRule) error { + yamlPath, err := userPolicyPath() + if err != nil { + // No user home dir means we cannot locate the policy. Treat + // the same as "file missing": no pruning, no error. This keeps + // non-interactive CI environments (no HOME set) running. + yamlPath = "" + } + + rule, source, err := cmdpolicy.Resolve(pluginRules, yamlPath) + if err != nil { + return err + } + if rule == nil { + cmdpolicy.SetActive(&cmdpolicy.ActivePolicy{ + Source: source, + YAMLPath: yamlPath, + }) + return nil + } + + engine := cmdpolicy.New(rule) + decisions := engine.EvaluateAll(rootCmd) + denied := cmdpolicy.BuildDeniedByPath(rootCmd, decisions, source, rule.Name) + cmdpolicy.Apply(rootCmd, denied) + + // Record the active policy so `config policy show` can read it. + cmdpolicy.SetActive(&cmdpolicy.ActivePolicy{ + Rule: rule, + Source: source, + YAMLPath: yamlPath, + DeniedPaths: len(denied), + }) + return nil +} + +// installPluginsAndHooks runs the InstallAll phase on the globally- +// registered plugins, returning the Plugin.Restrict contributions for +// cmdpolicy and the populated hook.Registry for the runtime wrapper. +// Errors from FailClosed plugins propagate; FailOpen failures are +// warned to errOut and the loop continues. +func installPluginsAndHooks(errOut io.Writer) (*internalplatform.InstallResult, error) { + plugins := platform.RegisteredPlugins() + if len(plugins) == 0 { + return &internalplatform.InstallResult{Registry: nil}, nil + } + return internalplatform.InstallAll(plugins, errOut) +} + +// recordInventory builds and stores the plugin inventory snapshot for +// diagnostic commands (config plugins show) to read at runtime. Called +// once from build.go after applyUserPolicyPruning + wireHooks succeed. +func recordInventory(installResult *internalplatform.InstallResult) { + if installResult == nil { + internalplatform.SetActiveInventory(nil) + return + } + pluginSrcs := make([]internalplatform.PluginInventorySource, 0, len(installResult.Plugins)) + for _, p := range installResult.Plugins { + pluginSrcs = append(pluginSrcs, internalplatform.PluginInventorySource{ + Name: p.Name, + Version: p.Version, + Capabilities: p.Capabilities, + }) + } + ruleSrcs := make([]internalplatform.RuleInventorySource, 0, len(installResult.PluginRules)) + for _, r := range installResult.PluginRules { + if r.Rule == nil { + continue + } + idents := make([]string, len(r.Rule.Identities)) + for i, id := range r.Rule.Identities { + idents[i] = string(id) + } + ruleSrcs = append(ruleSrcs, internalplatform.RuleInventorySource{ + PluginName: r.PluginName, + Allow: r.Rule.Allow, + Deny: r.Rule.Deny, + MaxRisk: string(r.Rule.MaxRisk), + Identities: idents, + RuleName: r.Rule.Name, + Desc: r.Rule.Description, + AllowUnannotated: r.Rule.AllowUnannotated, + }) + } + internalplatform.SetActiveInventory(internalplatform.BuildInventory(pluginSrcs, installResult.Registry, ruleSrcs)) +} + +// wireHooks installs Observer/Wrapper hooks onto every runnable command +// and emits the Startup lifecycle event. The registry may be nil when +// no plugin contributed any hook -- the function short-circuits in +// that case to avoid useless RunE wrapping. +func wireHooks(ctx context.Context, rootCmd *cobra.Command, reg *hook.Registry) error { + if reg == nil { + return nil + } + hook.Install(rootCmd, reg, cobraCommandViewSource{}) + return hook.Emit(ctx, reg, platform.Startup, nil) +} + +// cobraCommandViewSource is the default CommandViewSource: it builds a +// CommandView directly from a *cobra.Command on demand. A future PR +// will snapshot views at registration time so the view survives +// strict-mode's RemoveCommand+AddCommand replacement of the +// underlying *cobra.Command pointer. For now this is acceptable +// because user-layer cmdpolicy preserves the pointer (only strict-mode +// swaps it), and strict-mode-pruned commands are already unreachable +// by the hook chain. +type cobraCommandViewSource struct{} + +func (cobraCommandViewSource) View(cmd *cobra.Command) platform.CommandView { + return cobraCommandView{cmd: cmd} +} + +// cobraCommandView adapts *cobra.Command to the CommandView interface. +type cobraCommandView struct { + cmd *cobra.Command +} + +func (v cobraCommandView) Path() string { + return cmdpolicy.CanonicalPath(v.cmd) +} + +func (v cobraCommandView) Domain() string { + for c := v.cmd; c != nil; c = c.Parent() { + if c.Annotations == nil { + continue + } + if v, ok := c.Annotations["cmdmeta.domain"]; ok && v != "" { + return v + } + } + return "" +} + +func (v cobraCommandView) Risk() (platform.Risk, bool) { + for c := v.cmd; c != nil; c = c.Parent() { + if c.Annotations == nil { + continue + } + if r, ok := c.Annotations["risk_level"]; ok && r != "" { + return platform.Risk(r), true + } + } + return "", false +} + +func (v cobraCommandView) Identities() []platform.Identity { + for c := v.cmd; c != nil; c = c.Parent() { + if c.Annotations == nil { + continue + } + if raw, ok := c.Annotations["lark:supportedIdentities"]; ok && raw != "" { + parts := splitCSV(raw) + out := make([]platform.Identity, len(parts)) + for i, p := range parts { + out[i] = platform.Identity(p) + } + return out + } + } + return nil +} + +func (v cobraCommandView) Annotation(key string) (string, bool) { + if v.cmd.Annotations == nil { + return "", false + } + s, ok := v.cmd.Annotations[key] + return s, ok +} + +// splitCSV is a tiny csv-without-quotes helper. The +// lark:supportedIdentities annotation is always plain +// "user" / "bot" / "user,bot" without escaping. +func splitCSV(s string) []string { + out := []string{} + start := 0 + for i := 0; i < len(s); i++ { + if s[i] == ',' { + out = append(out, s[start:i]) + start = i + 1 + } + } + out = append(out, s[start:]) + return out +} + +// userPolicyPath returns the absolute path of ~/.lark-cli/policy.yml, +// or an error if the user's home directory cannot be determined. +func userPolicyPath() (string, error) { + home, err := vfs.UserHomeDir() + if err != nil { + return "", err + } + return filepath.Join(home, ".lark-cli", userPolicyFileName), nil +} + +// warnPolicyError writes a one-line stderr warning when the user policy +// fails to load. V1 yaml errors are fail-OPEN -- the CLI keeps running +// without policy enforcement so the user can fix the typo. Plugin-supplied +// rules are fail-CLOSED instead because integrators take a code-level +// responsibility for them. +func warnPolicyError(errOut io.Writer, err error) { + if err == nil { + return + } + fmt.Fprintf(errOut, "warning: user policy not applied: %v\n", err) +} diff --git a/cmd/policy_test.go b/cmd/platform_bootstrap_test.go similarity index 100% rename from cmd/policy_test.go rename to cmd/platform_bootstrap_test.go diff --git a/cmd/policy.go b/cmd/platform_guards.go similarity index 51% rename from cmd/policy.go rename to cmd/platform_guards.go index 305a8886..714d147f 100644 --- a/cmd/policy.go +++ b/cmd/platform_guards.go @@ -4,133 +4,16 @@ package cmd import ( - "context" "errors" - "fmt" - "io" - "path/filepath" "github.com/spf13/cobra" - "github.com/larksuite/cli/extension/platform" + "github.com/larksuite/cli/internal/cmdpolicy" "github.com/larksuite/cli/internal/hook" "github.com/larksuite/cli/internal/output" - "github.com/larksuite/cli/internal/platformhost" - "github.com/larksuite/cli/internal/plugininventory" - "github.com/larksuite/cli/internal/pruning" - "github.com/larksuite/cli/internal/vfs" + internalplatform "github.com/larksuite/cli/internal/platform" ) -// userPolicyFileName is the conventional filename for the user-layer Rule. -// Lives under ~/.lark-cli/ to match the rest of the CLI's user-state -// directory. -const userPolicyFileName = "policy.yml" - -// applyUserPolicyPruning resolves the user-layer Rule from plugin -// contributions and/or ~/.lark-cli/policy.yml and installs denyStubs -// for commands it rejects. -// -// Missing yaml is not an error -- the CLI runs with no user-layer -// restriction. A malformed Rule (bad MaxRisk enum, malformed glob, etc.) -// surfaces via the returned error; the caller decides how to handle it. -// -// pluginRules carries Plugin.Restrict() contributions collected from -// the platformhost InstallAll phase; nil/empty is fine. -func applyUserPolicyPruning(rootCmd *cobra.Command, pluginRules []pruning.PluginRule) error { - yamlPath, err := userPolicyPath() - if err != nil { - // No user home dir means we cannot locate the policy. Treat - // the same as "file missing": no pruning, no error. This keeps - // non-interactive CI environments (no HOME set) running. - yamlPath = "" - } - - rule, source, err := pruning.Resolve(pluginRules, yamlPath) - if err != nil { - return err - } - if rule == nil { - pruning.SetActive(&pruning.ActivePolicy{ - Source: source, - YAMLPath: yamlPath, - }) - return nil - } - - engine := pruning.New(rule) - decisions := engine.EvaluateAll(rootCmd) - denied := pruning.BuildDeniedByPath(rootCmd, decisions, source, rule.Name) - pruning.Apply(rootCmd, denied) - - // Record the active policy so `config policy show` can read it. - pruning.SetActive(&pruning.ActivePolicy{ - Rule: rule, - Source: source, - YAMLPath: yamlPath, - DeniedPaths: len(denied), - }) - return nil -} - -// installPluginsAndHooks runs the platformhost.InstallAll phase on the -// globally-registered plugins, returning the Plugin.Restrict -// contributions for pruning and the populated hook.Registry for the -// runtime wrapper. Errors from FailClosed plugins propagate; FailOpen -// failures are warned to errOut and the loop continues. -func installPluginsAndHooks(errOut io.Writer) (*platformhost.InstallResult, error) { - plugins := platform.RegisteredPlugins() - if len(plugins) == 0 { - return &platformhost.InstallResult{Registry: nil}, nil - } - return platformhost.InstallAll(plugins, errOut) -} - -// recordInventory builds and stores the plugin inventory snapshot for -// diagnostic commands (config plugins show) to read at runtime. Called -// once from build.go after applyUserPolicyPruning + wireHooks succeed. -func recordInventory(installResult *platformhost.InstallResult) { - if installResult == nil { - plugininventory.SetActive(nil) - return - } - pluginSrcs := make([]plugininventory.PluginSource, 0, len(installResult.Plugins)) - for _, p := range installResult.Plugins { - pluginSrcs = append(pluginSrcs, plugininventory.PluginSource{ - Name: p.Name, - Version: p.Version, - Capabilities: p.Capabilities, - }) - } - ruleSrcs := make([]plugininventory.RuleSource, 0, len(installResult.PluginRules)) - for _, r := range installResult.PluginRules { - if r.Rule == nil { - continue - } - ruleSrcs = append(ruleSrcs, plugininventory.RuleSource{ - PluginName: r.PluginName, - Allow: r.Rule.Allow, - Deny: r.Rule.Deny, - MaxRisk: r.Rule.MaxRisk, - Identities: r.Rule.Identities, - RuleName: r.Rule.Name, - Desc: r.Rule.Description, - }) - } - plugininventory.SetActive(plugininventory.Build(pluginSrcs, installResult.Registry, ruleSrcs)) -} - -// wireHooks installs Observer/Wrapper hooks onto every runnable command -// and emits the Startup lifecycle event. The registry may be nil when -// no plugin contributed any hook -- the function short-circuits in -// that case to avoid useless RunE wrapping. -func wireHooks(ctx context.Context, rootCmd *cobra.Command, reg *hook.Registry) error { - if reg == nil { - return nil - } - hook.Install(rootCmd, reg, cobraCommandViewSource{}) - return hook.Emit(ctx, reg, platform.Startup, nil) -} - // installFatalGuard wires a fail-closed guard at every cobra dispatch // path on rootCmd. Used by the three abort-side fatal paths: // @@ -194,7 +77,7 @@ func installFatalGuard(rootCmd *cobra.Command, makeErr func() *output.ExitError) // runs. func installPluginInstallErrorGuard(rootCmd *cobra.Command, installErr error) { makeErr := func() *output.ExitError { - var pi *platformhost.PluginInstallError + var pi *internalplatform.PluginInstallError if errors.As(installErr, &pi) { return &output.ExitError{ Code: output.ExitValidation, @@ -216,7 +99,7 @@ func installPluginInstallErrorGuard(rootCmd *cobra.Command, installErr error) { Type: "plugin_install", Message: installErr.Error(), Detail: map[string]any{ - "reason_code": platformhost.ReasonInstallFailed, + "reason_code": internalplatform.ReasonInstallFailed, }, }, Err: installErr, @@ -227,7 +110,7 @@ func installPluginInstallErrorGuard(rootCmd *cobra.Command, installErr error) { // installPluginConflictGuard surfaces a Plugin.Restrict() configuration // error (single plugin invalid Rule or multiple plugins each contributing -// Restrict). The tech doc separates the envelope type: +// Restrict). The design separates the envelope type: // // - "plugin_install" with reason_code "invalid_rule" - single bad rule // - "plugin_conflict" with reason_code "multiple_restrict_plugins" - multi @@ -236,10 +119,10 @@ func installPluginInstallErrorGuard(rootCmd *cobra.Command, installErr error) { func installPluginConflictGuard(rootCmd *cobra.Command, err error) { makeErr := func() *output.ExitError { envelopeType := "plugin_install" - reasonCode := platformhost.ReasonInvalidRule - if errors.Is(err, pruning.ErrMultipleRestricts) { + reasonCode := internalplatform.ReasonInvalidRule + if errors.Is(err, cmdpolicy.ErrMultipleRestricts) { envelopeType = "plugin_conflict" - reasonCode = platformhost.ReasonMultipleRestricts + reasonCode = internalplatform.ReasonMultipleRestricts } return &output.ExitError{ Code: output.ExitValidation, @@ -260,9 +143,6 @@ func installPluginConflictGuard(rootCmd *cobra.Command, err error) { // failure as a plugin_lifecycle envelope. The reason_code splits // returned-error vs panic so consumers (audit / on-call) can tell the // two failure modes apart. -// -// Per tech-doc table line 523: type=plugin_lifecycle, reason_code in -// {lifecycle_failed, lifecycle_panic}. func installPluginLifecycleErrorGuard(rootCmd *cobra.Command, err error) { makeErr := func() *output.ExitError { reasonCode := "lifecycle_failed" @@ -365,113 +245,3 @@ func walkGuard(cmd *cobra.Command, makeErr func() *output.ExitError) { walkGuard(c, makeErr) } } - -// cobraCommandViewSource is the default CommandViewSource: it builds a -// CommandView directly from a *cobra.Command on demand. A future PR -// will snapshot views at registration time (constraint #1 fully) so -// the view survives strict-mode's RemoveCommand+AddCommand replacement -// of the underlying *cobra.Command pointer. For now this is acceptable -// because user-layer pruning preserves the pointer (only strict-mode -// swaps it), and strict-mode-pruned commands are already unreachable -// by the hook chain. -type cobraCommandViewSource struct{} - -func (cobraCommandViewSource) View(cmd *cobra.Command) platform.CommandView { - return cobraCommandView{cmd: cmd} -} - -// cobraCommandView adapts *cobra.Command to the CommandView interface. -type cobraCommandView struct { - cmd *cobra.Command -} - -func (v cobraCommandView) Path() string { - return pruning.CanonicalPath(v.cmd) -} - -func (v cobraCommandView) Domain() string { - // cmdmeta inheritance is implemented in internal/cmdmeta; we - // re-read annotations directly here to keep the import surface - // small. Future PR may pull cmdmeta into the View. - for c := v.cmd; c != nil; c = c.Parent() { - if c.Annotations == nil { - continue - } - if v, ok := c.Annotations["cmdmeta.domain"]; ok && v != "" { - return v - } - } - return "" -} - -func (v cobraCommandView) Risk() (string, bool) { - for c := v.cmd; c != nil; c = c.Parent() { - if c.Annotations == nil { - continue - } - if r, ok := c.Annotations["risk_level"]; ok && r != "" { - return r, true - } - } - return "", false -} - -func (v cobraCommandView) Identities() []string { - for c := v.cmd; c != nil; c = c.Parent() { - if c.Annotations == nil { - continue - } - if raw, ok := c.Annotations["lark:supportedIdentities"]; ok && raw != "" { - return splitCSV(raw) - } - } - return nil -} - -func (v cobraCommandView) Annotation(key string) (string, bool) { - if v.cmd.Annotations == nil { - return "", false - } - s, ok := v.cmd.Annotations[key] - return s, ok -} - -// splitCSV is a tiny csv-without-quotes helper. CommandView is on the -// hot path (one lookup per command invocation) and we want to avoid -// pulling strings.Split's allocation cost; the lark:supportedIdentities -// annotation is always plain "user" / "bot" / "user,bot" without -// escaping. -func splitCSV(s string) []string { - out := []string{} - start := 0 - for i := 0; i < len(s); i++ { - if s[i] == ',' { - out = append(out, s[start:i]) - start = i + 1 - } - } - out = append(out, s[start:]) - return out -} - -// userPolicyPath returns the absolute path of ~/.lark-cli/policy.yml, -// or an error if the user's home directory cannot be determined. -func userPolicyPath() (string, error) { - home, err := vfs.UserHomeDir() - if err != nil { - return "", err - } - return filepath.Join(home, ".lark-cli", userPolicyFileName), nil -} - -// warnPolicyError writes a one-line stderr warning when the user policy -// fails to load. V1 yaml errors are fail-OPEN -- the CLI keeps running -// without pruning so the user can fix the typo. Plugin-supplied rules -// (Hook surface, future) will be fail-CLOSED instead because integrators -// take a code-level responsibility for them. -func warnPolicyError(errOut io.Writer, err error) { - if err == nil { - return - } - fmt.Fprintf(errOut, "warning: user policy not applied: %v\n", err) -} diff --git a/cmd/install_guard_test.go b/cmd/platform_guards_test.go similarity index 83% rename from cmd/install_guard_test.go rename to cmd/platform_guards_test.go index 6acd7868..d8babe20 100644 --- a/cmd/install_guard_test.go +++ b/cmd/platform_guards_test.go @@ -8,13 +8,14 @@ import ( "errors" "sync" "testing" + "time" "github.com/spf13/cobra" "github.com/larksuite/cli/extension/platform" "github.com/larksuite/cli/internal/hook" "github.com/larksuite/cli/internal/output" - "github.com/larksuite/cli/internal/platformhost" + "github.com/larksuite/cli/internal/platform" ) // failClosedAbortingPlugin returns a PluginInstallError on Install, @@ -107,7 +108,7 @@ func checkGuardError(t *testing.T, err error) { if detail["plugin"] != "policy" { t.Errorf("detail.plugin = %v, want policy", detail["plugin"]) } - if detail["reason_code"] != platformhost.ReasonInstallFailed { + if detail["reason_code"] != internalplatform.ReasonInstallFailed { t.Errorf("detail.reason_code = %v, want install_failed", detail["reason_code"]) } } @@ -135,7 +136,7 @@ func TestNamespacedWrap_doesNotMutateSharedAbortError(t *testing.T) { makeWrapper := func(name string) platform.Wrapper { return func(next platform.Handler) platform.Handler { - return func(context.Context, *platform.Invocation) error { return shared } + return func(context.Context, platform.Invocation) error { return shared } } } @@ -160,8 +161,8 @@ func TestNamespacedWrap_doesNotMutateSharedAbortError(t *testing.T) { i, m := i, m go func() { defer wg.Done() - err := m.Fn(func(context.Context, *platform.Invocation) error { return nil })( - context.Background(), &platform.Invocation{}) + err := m.Fn(func(context.Context, platform.Invocation) error { return nil })( + context.Background(), stubInvocation{}) if ab, ok := err.(*platform.AbortError); ok { results[i] = ab.HookName } @@ -187,6 +188,21 @@ type stubView struct{} func (stubView) Path() string { return "x" } func (stubView) Domain() string { return "" } -func (stubView) Risk() (string, bool) { return "", false } -func (stubView) Identities() []string { return nil } +func (stubView) Risk() (platform.Risk, bool) { return "", false } +func (stubView) Identities() []platform.Identity { return nil } func (stubView) Annotation(string) (string, bool) { return "", false } + +// stubInvocation is the minimal platform.Invocation implementation +// used by tests that need to drive a Wrap without going through the +// full hook.Install pipeline. +type stubInvocation struct{} + +func (stubInvocation) Cmd() platform.CommandView { return stubView{} } +func (stubInvocation) Args() []string { return nil } +func (stubInvocation) Started() time.Time { return time.Time{} } +func (stubInvocation) Err() error { return nil } +func (stubInvocation) DeniedByPolicy() bool { return false } +func (stubInvocation) DenialLayer() string { return "" } +func (stubInvocation) DenialPolicySource() string { return "" } +func (stubInvocation) StrictMode() (string, bool) { return "", false } +func (stubInvocation) Identity() (string, bool) { return "", false } diff --git a/cmd/plugin_integration_test.go b/cmd/plugin_integration_test.go index 1435fff6..13480aa6 100644 --- a/cmd/plugin_integration_test.go +++ b/cmd/plugin_integration_test.go @@ -14,11 +14,11 @@ import ( "github.com/spf13/cobra" "github.com/larksuite/cli/extension/platform" + "github.com/larksuite/cli/internal/cmdpolicy" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/hook" "github.com/larksuite/cli/internal/output" - "github.com/larksuite/cli/internal/platformhost" - "github.com/larksuite/cli/internal/pruning" + "github.com/larksuite/cli/internal/platform" ) // These integration tests exercise the Hook framework's plumbing @@ -53,16 +53,16 @@ func (p *fakeIntegrationPlugin) Install(r platform.Registrar) error { r.Restrict(p.rule) } r.Observe(platform.Before, "audit-pre", platform.All(), - func(context.Context, *platform.Invocation) { + func(context.Context, platform.Invocation) { atomic.AddInt64(&p.beforeCount, 1) }) r.Observe(platform.After, "audit-post", platform.All(), - func(context.Context, *platform.Invocation) { + func(context.Context, platform.Invocation) { atomic.AddInt64(&p.afterCount, 1) }) r.Wrap("policy", platform.ByWrite(), func(next platform.Handler) platform.Handler { - return func(ctx context.Context, inv *platform.Invocation) error { + return func(ctx context.Context, inv platform.Invocation) error { atomic.AddInt64(&p.wrapCount, 1) if p.wrapDeniesWrite { return &platform.AbortError{ @@ -97,7 +97,7 @@ func syntheticTree() (*cobra.Command, *cobra.Command) { } // End-to-end through the public install pipeline: register a plugin, -// run platformhost.InstallAll (the same function buildInternal calls), +// run internalplatform.InstallAll (the same function buildInternal calls), // wire hooks onto a synthetic tree, invoke the leaf, and confirm // observers fired. func TestPluginPipeline_observersWired(t *testing.T) { @@ -109,7 +109,7 @@ func TestPluginPipeline_observersWired(t *testing.T) { } platform.Register(plugin) - result, err := platformhost.InstallAll(platform.RegisteredPlugins(), nil) + result, err := internalplatform.InstallAll(platform.RegisteredPlugins(), nil) if err != nil { t.Fatalf("InstallAll: %v", err) } @@ -145,7 +145,7 @@ func TestPluginPipeline_wrapAbortReachesEnvelope(t *testing.T) { } platform.Register(plugin) - result, err := platformhost.InstallAll(platform.RegisteredPlugins(), nil) + result, err := internalplatform.InstallAll(platform.RegisteredPlugins(), nil) if err != nil { t.Fatalf("InstallAll: %v", err) } @@ -182,7 +182,7 @@ func TestPluginPipeline_wrapAbortReachesEnvelope(t *testing.T) { // Plugin.Restrict() contribution must reach the pruning resolver and // take precedence over a yaml file (single-rule, plugin wins). This // goes through the REAL Build() pipeline so the wiring between -// installPluginsAndHooks -> applyUserPolicyPruning -> pruning.Resolve +// installPluginsAndHooks -> applyUserPolicyPruning -> cmdpolicy.Resolve // is covered. func TestPluginPipeline_restrictBeatsYaml(t *testing.T) { cfgDir := tmpHome(t) @@ -251,20 +251,20 @@ func TestPluginPipeline_denialGuardIntegrated(t *testing.T) { } platform.Register(malicious) - result, err := platformhost.InstallAll(platform.RegisteredPlugins(), nil) + result, err := internalplatform.InstallAll(platform.RegisteredPlugins(), nil) if err != nil { t.Fatalf("InstallAll: %v", err) } root, leaf := syntheticTree() - // Simulate pruning.Apply marking leaf as denied. + // Simulate cmdpolicy.Apply marking leaf as denied. leaf.Hidden = true leaf.DisableFlagParsing = true if leaf.Annotations == nil { leaf.Annotations = map[string]string{} } - leaf.Annotations["lark:pruning_denied_layer"] = "pruning" - leaf.Annotations["lark:pruning_denied_source"] = "plugin:other" + leaf.Annotations["lark:policy_denied_layer"] = "policy" + leaf.Annotations["lark:policy_denied_source"] = "plugin:other" denyStubCalled := false leaf.RunE = func(*cobra.Command, []string) error { denyStubCalled = true @@ -302,7 +302,7 @@ func (p *mockMaliciousPlugin) Capabilities() platform.Capabilities { func (p *mockMaliciousPlugin) Install(r platform.Registrar) error { r.Wrap("hijack", platform.All(), func(_ platform.Handler) platform.Handler { - return func(context.Context, *platform.Invocation) error { + return func(context.Context, platform.Invocation) error { if p.invokedFlag != nil { *p.invokedFlag = true } @@ -386,8 +386,8 @@ func TestPluginConflictGuard_MultipleRestrictAbortsCLI(t *testing.T) { tmpHome(t) platform.ResetForTesting() t.Cleanup(platform.ResetForTesting) - pruning.ResetActiveForTesting() - t.Cleanup(pruning.ResetActiveForTesting) + cmdpolicy.ResetActiveForTesting() + t.Cleanup(cmdpolicy.ResetActiveForTesting) rule := &platform.Rule{Name: "any", Allow: []string{"**"}} platform.Register(&fakeIntegrationPlugin{ @@ -430,8 +430,8 @@ func TestPluginConflictGuard_InvalidRuleAbortsCLI(t *testing.T) { tmpHome(t) platform.ResetForTesting() t.Cleanup(platform.ResetForTesting) - pruning.ResetActiveForTesting() - t.Cleanup(pruning.ResetActiveForTesting) + cmdpolicy.ResetActiveForTesting() + t.Cleanup(cmdpolicy.ResetActiveForTesting) // MaxRisk "nukem" is rejected by ValidateRule -> Resolve returns // an error that is NOT ErrMultipleRestricts. @@ -472,8 +472,8 @@ func TestPluginLifecycleGuard_StartupErrorAbortsCLI(t *testing.T) { tmpHome(t) platform.ResetForTesting() t.Cleanup(platform.ResetForTesting) - pruning.ResetActiveForTesting() - t.Cleanup(pruning.ResetActiveForTesting) + cmdpolicy.ResetActiveForTesting() + t.Cleanup(cmdpolicy.ResetActiveForTesting) platform.Register(&startupFailingPlugin{ name: "lc", @@ -508,8 +508,8 @@ func TestPluginLifecycleGuard_StartupPanicAbortsCLI(t *testing.T) { tmpHome(t) platform.ResetForTesting() t.Cleanup(platform.ResetForTesting) - pruning.ResetActiveForTesting() - t.Cleanup(pruning.ResetActiveForTesting) + cmdpolicy.ResetActiveForTesting() + t.Cleanup(cmdpolicy.ResetActiveForTesting) platform.Register(&startupFailingPlugin{ name: "lc", @@ -566,7 +566,7 @@ func TestWrapperPanic_BecomesHookPanicEnvelope(t *testing.T) { platform.Register(&panickingWrapPlugin{name: "p"}) - result, err := platformhost.InstallAll(platform.RegisteredPlugins(), nil) + result, err := internalplatform.InstallAll(platform.RegisteredPlugins(), nil) if err != nil { t.Fatalf("InstallAll: %v", err) } @@ -606,7 +606,7 @@ func (p *panickingWrapPlugin) Capabilities() platform.Capabilities { return plat func (p *panickingWrapPlugin) Install(r platform.Registrar) error { r.Wrap("boom", platform.All(), func(_ platform.Handler) platform.Handler { - return func(context.Context, *platform.Invocation) error { + return func(context.Context, platform.Invocation) error { panic("intentional panic for test") } }) @@ -640,7 +640,7 @@ func TestWrapperFactoryPanic_BecomesHookPanicEnvelope(t *testing.T) { platform.Register(&factoryPanicWrapPlugin{name: "fac"}) - result, err := platformhost.InstallAll(platform.RegisteredPlugins(), nil) + result, err := internalplatform.InstallAll(platform.RegisteredPlugins(), nil) if err != nil { t.Fatalf("InstallAll: %v", err) } diff --git a/cmd/prune.go b/cmd/prune.go index b1b6c5f4..5d7d1882 100644 --- a/cmd/prune.go +++ b/cmd/prune.go @@ -7,12 +7,12 @@ import ( "fmt" "slices" + "github.com/spf13/cobra" + + "github.com/larksuite/cli/internal/cmdpolicy" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/output" - "github.com/larksuite/cli/internal/policydecision" - "github.com/larksuite/cli/internal/pruning" - "github.com/spf13/cobra" ) // pruneForStrictMode removes commands incompatible with the active strict mode. @@ -52,7 +52,7 @@ func strictModeStubFrom(child *cobra.Command, mode core.StrictMode) *cobra.Comma // against platform.All() could intercept and silently swallow the // strict-mode error -- breaking strict-mode's "hard boundary" contract. // - // Args + PersistentPreRunE overrides mirror pruning/apply.go::installDenyStub: + // Args + PersistentPreRunE overrides mirror cmdpolicy/apply.go::installDenyStub: // // - Args=ArbitraryArgs: with DisableFlagParsing the user's flags // look like positional args; the original child's Args validator @@ -64,6 +64,21 @@ func strictModeStubFrom(child *cobra.Command, mode core.StrictMode) *cobra.Comma // credentials are set. Cobra's "first wins walking up" would // pick auth's instead of our denial. A leaf-level no-op makes // cobra stop here and proceed to the wrapped RunE. + // + // strict-mode keeps its short Message + independent Hint and + // composes the shared detail.* / wrapped-CommandDeniedError shape + // by hand; BuildDenialError would override Message with the + // CommandDeniedError.Error() long form. + stubMessage := fmt.Sprintf( + "strict mode is %q, only %s-identity commands are available", + mode, mode.ForcedIdentity()) + const stubHint = "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)" + denial := cmdpolicy.Denial{ + Layer: cmdpolicy.LayerStrictMode, + PolicySource: "strict-mode", + ReasonCode: "identity_not_supported", + Reason: stubMessage, + } return &cobra.Command{ Use: child.Use, Aliases: append([]string(nil), child.Aliases...), @@ -71,17 +86,25 @@ func strictModeStubFrom(child *cobra.Command, mode core.StrictMode) *cobra.Comma DisableFlagParsing: true, Args: cobra.ArbitraryArgs, Annotations: map[string]string{ - pruning.AnnotationDenialLayer: policydecision.LayerStrictMode, - pruning.AnnotationDenialSource: "strict-mode", + cmdpolicy.AnnotationDenialLayer: cmdpolicy.LayerStrictMode, + cmdpolicy.AnnotationDenialSource: "strict-mode", }, PersistentPreRunE: func(c *cobra.Command, _ []string) error { c.SilenceUsage = true return nil }, - RunE: func(cmd *cobra.Command, args []string) error { - return output.ErrWithHint(output.ExitValidation, "command_denied", - fmt.Sprintf("strict mode is %q, only %s-identity commands are available", mode, mode.ForcedIdentity()), - "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)") + RunE: func(c *cobra.Command, _ []string) error { + cd := cmdpolicy.CommandDeniedFromDenial(cmdpolicy.CanonicalPath(c), denial) + return &output.ExitError{ + Code: output.ExitValidation, + Detail: &output.ErrDetail{ + Type: "command_denied", + Message: stubMessage, + Hint: stubHint, + Detail: cmdpolicy.DenialDetailMap(cd), + }, + Err: cd, + } }, } } diff --git a/cmd/prune_test.go b/cmd/prune_test.go index 37a49ef9..c002de22 100644 --- a/cmd/prune_test.go +++ b/cmd/prune_test.go @@ -4,13 +4,15 @@ package cmd import ( + "errors" "strings" "testing" + "github.com/larksuite/cli/extension/platform" + "github.com/larksuite/cli/internal/cmdpolicy" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" - "github.com/larksuite/cli/internal/policydecision" - "github.com/larksuite/cli/internal/pruning" + "github.com/larksuite/cli/internal/output" "github.com/spf13/cobra" ) @@ -245,6 +247,66 @@ func TestStrictModeStub_BypassesArgsValidator(t *testing.T) { } } +// Pins the strict-mode envelope shape: structured detail.* / wrapped +// CommandDeniedError for external agents, AND the historical short +// Message + independent Hint for existing consumers. +func TestStrictModeStub_StructuredEnvelope(t *testing.T) { + root := newTestTree() + pruneForStrictMode(root, core.StrictModeBot) + stub := findCmd(root, "im", "+search") + if stub == nil { + t.Fatalf("expected im/+search stub") + } + err := stub.RunE(stub, nil) + if err == nil { + t.Fatalf("strict-mode stub RunE should return error") + } + + var ee *output.ExitError + if !errors.As(err, &ee) { + t.Fatalf("err is not *output.ExitError: %T", err) + } + if ee.Detail == nil { + t.Fatalf("ExitError.Detail is nil; envelope writer cannot emit JSON") + } + if ee.Detail.Type != "command_denied" { + t.Errorf("Detail.Type = %q, want command_denied", ee.Detail.Type) + } + dm, ok := ee.Detail.Detail.(map[string]any) + if !ok { + t.Fatalf("Detail.Detail = %T, want map[string]any", ee.Detail.Detail) + } + if got, _ := dm["layer"].(string); got != cmdpolicy.LayerStrictMode { + t.Errorf("Detail.Detail[layer] = %q, want %q", got, cmdpolicy.LayerStrictMode) + } + if got, _ := dm["reason_code"].(string); got != "identity_not_supported" { + t.Errorf("Detail.Detail[reason_code] = %q, want identity_not_supported", got) + } + if got, _ := dm["policy_source"].(string); got != "strict-mode" { + t.Errorf("Detail.Detail[policy_source] = %q, want strict-mode", got) + } + + var cd *platform.CommandDeniedError + if !errors.As(err, &cd) { + t.Fatalf("err does not unwrap to *platform.CommandDeniedError") + } + if cd.Layer != cmdpolicy.LayerStrictMode { + t.Errorf("CommandDeniedError.Layer = %q, want %q", cd.Layer, cmdpolicy.LayerStrictMode) + } + if cd.ReasonCode != "identity_not_supported" { + t.Errorf("CommandDeniedError.ReasonCode = %q, want identity_not_supported", cd.ReasonCode) + } + if !strings.Contains(cd.Reason, `strict mode is "bot"`) { + t.Errorf("CommandDeniedError.Reason = %q, want substring 'strict mode is \"bot\"'", cd.Reason) + } + if ee.Detail.Message != `strict mode is "bot", only bot-identity commands are available` { + t.Errorf("Detail.Message = %q, want short historical form", ee.Detail.Message) + } + if !strings.HasPrefix(ee.Detail.Hint, "if the user explicitly wants to switch policy") { + t.Errorf("Detail.Hint = %q, want historical hint", ee.Detail.Hint) + } +} + // strictModeStubFrom must write the denial annotations so the hook // layer's populateInvocationDenial recognises the command as denied // and physically isolates the Wrap chain. Without this, a plugin @@ -259,13 +321,13 @@ func TestStrictModeStub_HasDenialAnnotation(t *testing.T) { if stub == nil { t.Fatalf("expected im/+search stub to exist") } - got := stub.Annotations[pruning.AnnotationDenialLayer] - if got != policydecision.LayerStrictMode { + got := stub.Annotations[cmdpolicy.AnnotationDenialLayer] + if got != cmdpolicy.LayerStrictMode { t.Errorf("stub annotation %q = %q, want %q", - pruning.AnnotationDenialLayer, got, policydecision.LayerStrictMode) + cmdpolicy.AnnotationDenialLayer, got, cmdpolicy.LayerStrictMode) } - if src := stub.Annotations[pruning.AnnotationDenialSource]; src != "strict-mode" { + if src := stub.Annotations[cmdpolicy.AnnotationDenialSource]; src != "strict-mode" { t.Errorf("stub annotation %q = %q, want %q", - pruning.AnnotationDenialSource, src, "strict-mode") + cmdpolicy.AnnotationDenialSource, src, "strict-mode") } } diff --git a/cmd/root_integration_test.go b/cmd/root_integration_test.go index 2f2d4a10..a8919d1c 100644 --- a/cmd/root_integration_test.go +++ b/cmd/root_integration_test.go @@ -27,6 +27,14 @@ import ( "github.com/spf13/cobra" ) +// Canonical strict-mode envelope strings shared across fixtures +// (reflect.DeepEqual pins them; keep in sync with strictModeStubFrom). +const ( + strictModeBotMessage = `strict mode is "bot", only bot-identity commands are available` + strictModeUserMessage = `strict mode is "user", only user-identity commands are available` + strictModeHint = "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)" +) + // buildIntegrationRootCmd creates a root command with api, service, and shortcut // subcommands wired to a test factory, simulating the real CLI command tree. func buildIntegrationRootCmd(t *testing.T, f *cmdutil.Factory) *cobra.Command { @@ -354,8 +362,16 @@ func TestIntegration_StrictModeBot_ProfileOverride_DirectAuthLoginReturnsEnvelop OK: false, Error: &output.ErrDetail{ Type: "command_denied", - Message: `strict mode is "bot", only bot-identity commands are available`, - Hint: "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)", + Message: strictModeBotMessage, + Hint: strictModeHint, + Detail: map[string]any{ + "path": "auth/login", + "layer": "strict_mode", + "policy_source": "strict-mode", + "rule_name": "", + "reason_code": "identity_not_supported", + "reason": strictModeBotMessage, + }, }, }) } @@ -372,8 +388,16 @@ func TestIntegration_StrictModeBot_ProfileOverride_DirectUserShortcutReturnsEnve OK: false, Error: &output.ErrDetail{ Type: "command_denied", - Message: `strict mode is "bot", only bot-identity commands are available`, - Hint: "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)", + Message: strictModeBotMessage, + Hint: strictModeHint, + Detail: map[string]any{ + "path": "im/+messages-search", + "layer": "strict_mode", + "policy_source": "strict-mode", + "rule_name": "", + "reason_code": "identity_not_supported", + "reason": strictModeBotMessage, + }, }, }) } @@ -447,8 +471,16 @@ func TestIntegration_StrictModeUser_ProfileOverride_ServiceBotOnlyMethodReturnsE OK: false, Error: &output.ErrDetail{ Type: "command_denied", - Message: `strict mode is "user", only user-identity commands are available`, - Hint: "if the user explicitly wants to switch policy, see `lark-cli config strict-mode --help` (confirm with the user before switching; switching does NOT require re-bind)", + Message: strictModeUserMessage, + Hint: strictModeHint, + Detail: map[string]any{ + "path": "im/images/create", + "layer": "strict_mode", + "policy_source": "strict-mode", + "rule_name": "", + "reason_code": "identity_not_supported", + "reason": strictModeUserMessage, + }, }, }) } diff --git a/extension/platform/README.md b/extension/platform/README.md new file mode 100644 index 00000000..1c3f168e --- /dev/null +++ b/extension/platform/README.md @@ -0,0 +1,93 @@ +# lark-cli Plugin SDK + +`extension/platform` is the **in-process plugin SDK** for lark-cli. +Plugins compile into a **fork** of the lark-cli binary via a blank +import; there is no `.so` loading, no RPC, no subprocess isolation. +A plugin shares the binary's address space and lifecycle. + +## 5-minute hello world + +```go +// myplugin/audit.go +package myplugin + +import ( + "context" + "log" + + "github.com/larksuite/cli/extension/platform" +) + +func init() { + platform.Register( + platform.NewPlugin("audit", "0.1.0"). + Observer(platform.After, "log-cmd", platform.All(), + func(ctx context.Context, inv platform.Invocation) { + log.Printf("cmd=%s err=%v", inv.Cmd().Path(), inv.Err()) + }). + FailOpen(). + MustBuild()) +} +``` + +Wire into a fork: + +```go +// cmd/larkx/main.go in your fork +package main + +import ( + _ "github.com/me/myplugin" // blank import → init() runs + + "github.com/larksuite/cli/cmd" + "os" +) + +func main() { os.Exit(cmd.Execute()) } +``` + +```sh +go build -o larkx ./cmd/larkx && ./larkx config plugins show +``` + +You should see `audit` in the plugin list. + +## What you can hook + +| Hook | Fires | Can block? | +| -------------------------- | ---------------------------------- | -------------------------------- | +| `Observer` | Before / After each command | No (fire-and-forget audit) | +| `Wrap` | Around each command's RunE | Yes (return `*AbortError`) | +| `On(Startup/Shutdown)` | Process lifecycle | N/A | +| `Restrict(Rule)` | Bootstrap-time, single per binary | Denies whole subtrees | + +## Safety contract (read this) + +- A plugin calling `Restrict()` MUST declare `FailClosed`. The Builder + flips it automatically; the lower-level `Plugin` interface rejects + the mismatch with `restricts_mismatch`. +- Only ONE plugin per binary can call `Restrict()`. Multi-plugin + Restrict is a deliberate `plugin_conflict` error (single-rule + ecosystem assumption). YAML policy at `~/.lark-cli/policy.yml` is + shadowed by any plugin Restrict. +- The `Wrap` factory runs **once per command dispatch**, not at + install time. Long-lived state (clients, caches, metrics counters) + must live on the Plugin struct or in package-level variables. +- Plugins cannot suppress a `command_denied`: the framework + physically isolates denied commands from the Wrap chain (Observers + still fire). +- Commands missing a `risk_level` annotation are denied by default + when a Rule is active. Set `Rule.AllowUnannotated = true` (or + `allow_unannotated: true` in yaml) to opt out during gradual + adoption. +- Risk annotation typos (e.g. `"wrtie"`) are always denied with + `risk_invalid` plus a "did you mean" suggestion. `AllowUnannotated` + does NOT bypass this — typo is a code bug, not a missing + annotation. + +## Where to go next + +- [Runnable example: audit observer](./examples/audit-observer/) +- [Runnable example: read-only policy](./examples/readonly-policy/) +- [Plugin author guide](../../docs/extension/plugin-author-guide.md) +- [reason_code reference](../../docs/extension/reason-codes.md) diff --git a/extension/platform/builder.go b/extension/platform/builder.go new file mode 100644 index 00000000..1bcba749 --- /dev/null +++ b/extension/platform/builder.go @@ -0,0 +1,215 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package platform + +import ( + "errors" + "fmt" + "regexp" +) + +// Builder is the ergonomic constructor for Plugin. Use it from init(): +// +// func init() { +// platform.Register( +// platform.NewPlugin("audit", "0.1.0"). +// Observer(platform.After, "log", platform.All(), auditFn). +// FailOpen(). +// MustBuild()) +// } +// +// The lower-level Plugin interface remains available for cases that +// need finer control (state on a struct, complex Install logic). The +// Builder enforces: +// +// - Name format (^[a-z0-9][a-z0-9-]*$) +// - hookName format and uniqueness within a plugin +// - Restricts ↔ FailClosed consistency (calling Restrict() implies +// FailClosed, so plugin authors cannot accidentally ship a policy +// plugin under FailOpen) +// - Rule validation via ValidateRule analogues (delegated to +// internal/cmdpolicy at install time; Builder only fast-fails +// blatantly bad input) +type Builder struct { + name string + version string + caps Capabilities + + actions []func(Registrar) + rule *Rule + + hookNames map[string]bool + errs []error +} + +var pluginNamePattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`) + +// NewPlugin starts a Builder. Name format is validated lazily — errors +// surface at Build()/MustBuild() time, allowing chained calls without +// intermediate error handling. +func NewPlugin(name, version string) *Builder { + b := &Builder{ + name: name, + version: version, + hookNames: map[string]bool{}, + } + if !pluginNamePattern.MatchString(name) { + b.errs = append(b.errs, fmt.Errorf("invalid plugin name %q: must match ^[a-z0-9][a-z0-9-]*$", name)) + } + return b +} + +// RequireCLI sets Capabilities.RequiredCLIVersion (semver constraint, +// e.g. ">=1.1.0"). Empty string means no requirement. +func (b *Builder) RequireCLI(constraint string) *Builder { + b.caps.RequiredCLIVersion = constraint + return b +} + +// FailOpen sets Capabilities.FailurePolicy = FailOpen. Default when +// neither FailOpen nor FailClosed is called and Restrict is not used. +func (b *Builder) FailOpen() *Builder { + b.caps.FailurePolicy = FailOpen + return b +} + +// FailClosed sets Capabilities.FailurePolicy = FailClosed. Implicit +// when Restrict() is called. +func (b *Builder) FailClosed() *Builder { + b.caps.FailurePolicy = FailClosed + return b +} + +// Observer registers an Observer. Multiple calls accumulate. +func (b *Builder) Observer(when When, hookName string, sel Selector, fn Observer) *Builder { + if !b.validateHookName(hookName, "observer") { + return b + } + // Capture by value so the action closure doesn't share state with + // subsequent Observer() calls (Go ≥1.22 already gives each call + // its own copies of parameter values, but pinning is explicit). + w, n, s, f := when, hookName, sel, fn + b.actions = append(b.actions, func(r Registrar) { + r.Observe(w, n, s, f) + }) + return b +} + +// Wrap registers a Wrapper. Multiple calls accumulate; the host +// composes them in registration order (outermost first). +func (b *Builder) Wrap(hookName string, sel Selector, wrap Wrapper) *Builder { + if !b.validateHookName(hookName, "wrap") { + return b + } + n, s, w := hookName, sel, wrap + b.actions = append(b.actions, func(r Registrar) { + r.Wrap(n, s, w) + }) + return b +} + +// On registers a LifecycleHandler. +func (b *Builder) On(event LifecycleEvent, hookName string, fn LifecycleHandler) *Builder { + if !b.validateHookName(hookName, "on") { + return b + } + e, n, f := event, hookName, fn + b.actions = append(b.actions, func(r Registrar) { + r.On(e, n, f) + }) + return b +} + +// Restrict contributes a pruning Rule. Calling Restrict implicitly +// sets Restricts=true and FailurePolicy=FailClosed (the framework +// requires both to coexist; the builder enforces the pairing so the +// plugin author cannot accidentally ship a policy plugin under +// FailOpen). +func (b *Builder) Restrict(rule *Rule) *Builder { + if rule == nil { + b.errs = append(b.errs, errors.New("Restrict(nil): rule must not be nil")) + return b + } + b.caps.Restricts = true + b.caps.FailurePolicy = FailClosed + b.rule = rule + return b +} + +// Build returns the configured Plugin, or an error if any builder +// step found a fault. MustBuild panics on the same error. +// +// The Restrict + FailOpen mismatch is checked here, not in the chained +// setters, because the two methods may be called in either order. +func (b *Builder) Build() (Plugin, error) { + if b.rule != nil && b.caps.FailurePolicy == FailOpen { + b.errs = append(b.errs, errors.New( + "Restrict() requires FailClosed; do not call FailOpen() after Restrict()")) + } + if len(b.errs) > 0 { + return nil, errors.Join(b.errs...) + } + return &builtPlugin{ + name: b.name, + version: b.version, + caps: b.caps, + actions: b.actions, + rule: b.rule, + }, nil +} + +// MustBuild panics if Build() would return an error. Designed for +// init(): +// +// func init() { platform.Register(platform.NewPlugin(...).MustBuild()) } +// +// A panic in init runs before the framework's recover guard is +// installed and will crash the binary. That is the intended +// behaviour: a misconfigured plugin must NOT be silently registered. +func (b *Builder) MustBuild() Plugin { + p, err := b.Build() + if err != nil { + panic(fmt.Sprintf("plugin %q: %v", b.name, err)) + } + return p +} + +// validateHookName checks the grammar and uniqueness; returns false +// when the name was rejected (caller skips the action). +func (b *Builder) validateHookName(hookName, kind string) bool { + if !pluginNamePattern.MatchString(hookName) { + b.errs = append(b.errs, fmt.Errorf( + "%s %q: hookName must match ^[a-z0-9][a-z0-9-]*$", kind, hookName)) + return false + } + if b.hookNames[hookName] { + b.errs = append(b.errs, fmt.Errorf( + "%s %q: hookName already used in this plugin", kind, hookName)) + return false + } + b.hookNames[hookName] = true + return true +} + +// builtPlugin is the Plugin implementation the builder emits. +type builtPlugin struct { + name string + version string + caps Capabilities + actions []func(Registrar) + rule *Rule +} + +func (p *builtPlugin) Name() string { return p.name } +func (p *builtPlugin) Version() string { return p.version } +func (p *builtPlugin) Capabilities() Capabilities { return p.caps } +func (p *builtPlugin) Install(r Registrar) error { + if p.rule != nil { + r.Restrict(p.rule) + } + for _, action := range p.actions { + action(r) + } + return nil +} diff --git a/extension/platform/builder_test.go b/extension/platform/builder_test.go new file mode 100644 index 00000000..541271a1 --- /dev/null +++ b/extension/platform/builder_test.go @@ -0,0 +1,180 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package platform_test + +import ( + "context" + "strings" + "testing" + + "github.com/larksuite/cli/extension/platform" +) + +// recorder Registrar captures everything a builder schedules so the +// test can assert what Install produced without involving the host. +type recorder struct { + observers int + wrappers int + lifecycles int + rule *platform.Rule +} + +func (r *recorder) Observe(platform.When, string, platform.Selector, platform.Observer) { + r.observers++ +} +func (r *recorder) Wrap(string, platform.Selector, platform.Wrapper) { r.wrappers++ } +func (r *recorder) On(platform.LifecycleEvent, string, platform.LifecycleHandler) { r.lifecycles++ } +func (r *recorder) Restrict(rule *platform.Rule) { r.rule = rule } + +func TestBuilder_basicAssembly(t *testing.T) { + p, err := platform.NewPlugin("audit", "0.1.0"). + Observer(platform.Before, "pre", platform.All(), + func(context.Context, platform.Invocation) {}). + Observer(platform.After, "post", platform.All(), + func(context.Context, platform.Invocation) {}). + Wrap("policy", platform.All(), + func(next platform.Handler) platform.Handler { return next }). + On(platform.Startup, "boot", + func(context.Context, *platform.LifecycleContext) error { return nil }). + FailOpen(). + Build() + if err != nil { + t.Fatalf("Build: %v", err) + } + if p.Name() != "audit" || p.Version() != "0.1.0" { + t.Errorf("metadata = %q/%q", p.Name(), p.Version()) + } + if p.Capabilities().FailurePolicy != platform.FailOpen { + t.Errorf("FailurePolicy = %v, want FailOpen", p.Capabilities().FailurePolicy) + } + + r := &recorder{} + if err := p.Install(r); err != nil { + t.Fatalf("Install: %v", err) + } + if r.observers != 2 || r.wrappers != 1 || r.lifecycles != 1 { + t.Errorf("Install dispatch = observers=%d wrappers=%d lifecycles=%d", + r.observers, r.wrappers, r.lifecycles) + } +} + +// Restrict() flips Restricts=true and FailClosed automatically — a +// policy plugin can't accidentally ship under FailOpen. +func TestBuilder_restrictForcesFailClosed(t *testing.T) { + p, err := platform.NewPlugin("policy-plugin", "0.1.0"). + Restrict(&platform.Rule{Name: "read-only", MaxRisk: platform.RiskRead}). + Build() + if err != nil { + t.Fatalf("Build: %v", err) + } + caps := p.Capabilities() + if !caps.Restricts { + t.Errorf("Restricts = false, want true (Restrict() should flip it)") + } + if caps.FailurePolicy != platform.FailClosed { + t.Errorf("FailurePolicy = %v, want FailClosed (Restrict() implies it)", caps.FailurePolicy) + } + + r := &recorder{} + if err := p.Install(r); err != nil { + t.Fatalf("Install: %v", err) + } + if r.rule == nil || r.rule.Name != "read-only" { + t.Errorf("Install did not propagate Rule: %+v", r.rule) + } +} + +// Invalid name surfaces at Build time, not at NewPlugin. +func TestBuilder_invalidPluginName(t *testing.T) { + _, err := platform.NewPlugin("Has_Underscore_And_Caps", "0.1").Build() + if err == nil { + t.Fatalf("Build must reject malformed plugin name") + } + if !strings.Contains(err.Error(), "invalid plugin name") { + t.Errorf("error should mention plugin name, got: %v", err) + } +} + +// Duplicate hookName within the same builder is rejected. +func TestBuilder_duplicateHookName(t *testing.T) { + noopObs := func(context.Context, platform.Invocation) {} + _, err := platform.NewPlugin("dup", "0"). + Observer(platform.Before, "h", platform.All(), noopObs). + Observer(platform.After, "h", platform.All(), noopObs). + Build() + if err == nil { + t.Fatalf("Build must reject duplicate hookName") + } + if !strings.Contains(err.Error(), "already used") { + t.Errorf("error should mention duplicate hookName, got %v", err) + } +} + +func TestBuilder_invalidHookName(t *testing.T) { + _, err := platform.NewPlugin("p", "0"). + Observer(platform.Before, "Bad.Name", platform.All(), + func(context.Context, platform.Invocation) {}). + Build() + if err == nil { + t.Fatalf("Build must reject hookName with dot") + } +} + +// MustBuild panics on builder error. +func TestBuilder_mustBuildPanicsOnError(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Fatalf("MustBuild must panic when Build would fail") + } + }() + _ = platform.NewPlugin("BadName", "0").MustBuild() +} + +func TestBuilder_restrictNilRejected(t *testing.T) { + _, err := platform.NewPlugin("p", "0").Restrict(nil).Build() + if err == nil { + t.Fatalf("Restrict(nil) must produce error") + } +} + +func TestBuilder_capabilitiesSetters(t *testing.T) { + p, err := platform.NewPlugin("p", "0.1"). + RequireCLI(">=1.0.0"). + FailClosed(). + Build() + if err != nil { + t.Fatalf("Build: %v", err) + } + caps := p.Capabilities() + if caps.RequiredCLIVersion != ">=1.0.0" { + t.Errorf("RequiredCLIVersion = %q, want >=1.0.0", caps.RequiredCLIVersion) + } + if caps.FailurePolicy != platform.FailClosed { + t.Errorf("FailurePolicy = %v, want FailClosed", caps.FailurePolicy) + } +} + +func TestBuilder_restrictThenFailOpenRejected(t *testing.T) { + rule := &platform.Rule{Name: "r", MaxRisk: platform.RiskRead} + _, err := platform.NewPlugin("p", "0").Restrict(rule).FailOpen().Build() + if err == nil { + t.Fatalf("Build must reject Restrict()+FailOpen() mismatch") + } + if !strings.Contains(err.Error(), "FailClosed") { + t.Errorf("error should mention FailClosed, got: %v", err) + } +} + +// Restrict() flips FailurePolicy to FailClosed; the previous FailOpen() +// is overridden. Pin it so the Build-time validation does not over-reject. +func TestBuilder_failOpenThenRestrictOK(t *testing.T) { + rule := &platform.Rule{Name: "r", MaxRisk: platform.RiskRead} + p, err := platform.NewPlugin("p", "0").FailOpen().Restrict(rule).Build() + if err != nil { + t.Fatalf("FailOpen()+Restrict() must succeed (Restrict flips to FailClosed): %v", err) + } + if p.Capabilities().FailurePolicy != platform.FailClosed { + t.Errorf("FailurePolicy = %v, want FailClosed", p.Capabilities().FailurePolicy) + } +} diff --git a/extension/platform/doc.go b/extension/platform/doc.go index f68a9dbc..f6241c36 100644 --- a/extension/platform/doc.go +++ b/extension/platform/doc.go @@ -23,15 +23,17 @@ // - Invocation - per-call context passed to handlers (Cmd view + DeniedByPolicy / StrictMode / Identity) // - AbortError - structured short-circuit error from a Wrapper; framework namespaces HookName // -// Pruning surface (what Restrict contributes, also consumable from yaml policy): +// Policy surface (what Restrict contributes, also consumable from yaml policy): // -// - Rule - declarative pruning rule (Allow / Deny / MaxRisk / Identities) +// - Rule - declarative policy rule (Allow / Deny / MaxRisk / Identities / AllowUnannotated) // - CommandView - read-only command metadata view (Path / Domain / Risk / Identities) -// - Risk constants - the closed risk taxonomy (read < write < high-risk-write) + RiskRank +// - Risk / Identity - defined string types with closed taxonomies; ParseRisk / ParseIdentity +// convert raw strings (yaml, cobra annotation) into typed values; r.Rank() +// gives a comparable rank for the read < write < high-risk-write ordering // - CommandDeniedError - structured error returned to denied callers // // Stability: every exported symbol here is part of the contract. Internal // orchestration (staging, validation, RunE wrapping, denial guard) lives -// under internal/platformhost, internal/hook and internal/pruning and is not +// under internal/platform, internal/hook and internal/cmdpolicy and is not // importable by third parties. package platform diff --git a/extension/platform/errors.go b/extension/platform/errors.go index 490e3f8a..7bd99f2d 100644 --- a/extension/platform/errors.go +++ b/extension/platform/errors.go @@ -14,7 +14,7 @@ import "fmt" // Layer values: // // - "strict_mode" -- credential strict-mode rejected the command -// - "pruning" -- user-layer Rule rejected the command +// - "policy" -- user-layer Rule rejected the command // // PolicySource is a free-form identifier such as "plugin:secaudit", // "yaml:mywork", or "strict-mode". Reason fields: diff --git a/extension/platform/types_test.go b/extension/platform/errors_test.go similarity index 52% rename from extension/platform/types_test.go rename to extension/platform/errors_test.go index 4e2ffc01..767e00d8 100644 --- a/extension/platform/types_test.go +++ b/extension/platform/errors_test.go @@ -10,46 +10,10 @@ import ( "github.com/larksuite/cli/extension/platform" ) -func TestRiskRank_orderedTaxonomy(t *testing.T) { - cases := []struct { - level platform.Risk - want int - }{ - {platform.RiskRead, 0}, - {platform.RiskWrite, 1}, - {platform.RiskHighRiskWrite, 2}, - } - for _, c := range cases { - got, ok := platform.RiskRank(c.level) - if !ok || got != c.want { - t.Errorf("RiskRank(%q) = (%d,%v), want (%d,true)", c.level, got, ok, c.want) - } - } - - if _, ok := platform.RiskRank("unknown-level"); ok { - t.Fatalf("RiskRank('unknown-level') ok should be false") - } - if _, ok := platform.RiskRank(""); ok { - t.Fatalf("RiskRank('') ok should be false (signals 'no risk annotation')") - } -} - -// The Risk ordering must be strict: read < write < high-risk-write. The -// pruning engine compares ranks; a regression that swaps the order would -// silently let high-risk commands pass under MaxRisk=write. -func TestRiskRank_strictlyMonotonic(t *testing.T) { - r1, _ := platform.RiskRank(platform.RiskRead) - r2, _ := platform.RiskRank(platform.RiskWrite) - r3, _ := platform.RiskRank(platform.RiskHighRiskWrite) - if !(r1 < r2 && r2 < r3) { - t.Fatalf("Risk ranks not monotonic: read=%d write=%d high=%d", r1, r2, r3) - } -} - func TestCommandDeniedError_messageFormats(t *testing.T) { withReason := &platform.CommandDeniedError{ Path: "docs/+update", - Layer: "pruning", + Layer: "policy", ReasonCode: "write_not_allowed", Reason: "write disabled by policy", } diff --git a/extension/platform/example_test.go b/extension/platform/example_test.go new file mode 100644 index 00000000..07839825 --- /dev/null +++ b/extension/platform/example_test.go @@ -0,0 +1,63 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package platform_test + +import ( + "context" + "fmt" + + "github.com/larksuite/cli/extension/platform" +) + +// ExampleNewPlugin_observer registers an audit Observer that fires +// after every command, regardless of success or failure. +func ExampleNewPlugin_observer() { + p, _ := platform.NewPlugin("audit", "0.1.0"). + Observer(platform.After, "log", platform.All(), + func(ctx context.Context, inv platform.Invocation) { + _ = inv.Cmd().Path() // do something useful with the command + }). + FailOpen(). + Build() + fmt.Println(p.Name(), p.Version()) + // Output: audit 0.1.0 +} + +// ExampleNewPlugin_wrapper registers a Wrap that short-circuits any +// write-class command. The framework converts the returned +// *AbortError into a structured "hook" envelope; observers still +// fire on the After stage so audit sees the attempt. +func ExampleNewPlugin_wrapper() { + p, _ := platform.NewPlugin("policy-plugin", "0.1.0"). + Wrap("block-writes", platform.ByWrite(), + func(next platform.Handler) platform.Handler { + return func(ctx context.Context, inv platform.Invocation) error { + return &platform.AbortError{ + HookName: "block-writes", + Reason: "writes are disabled for this session", + } + } + }). + FailOpen(). + Build() + fmt.Println(p.Capabilities().FailurePolicy == platform.FailOpen) + // Output: true +} + +// ExampleNewPlugin_restrict registers a policy plugin that allows +// only docs/* read commands. Note that Restrict() implicitly sets +// FailClosed — a policy plugin must abort the binary if it fails to +// install, not silently disappear. +func ExampleNewPlugin_restrict() { + p, _ := platform.NewPlugin("readonly-docs", "0.1.0"). + Restrict(&platform.Rule{ + Name: "docs-only", + Allow: []string{"docs/**"}, + MaxRisk: platform.RiskRead, + }). + Build() + caps := p.Capabilities() + fmt.Println(caps.Restricts, caps.FailurePolicy == platform.FailClosed) + // Output: true true +} diff --git a/extension/platform/examples/.gitignore b/extension/platform/examples/.gitignore new file mode 100644 index 00000000..6c34736f --- /dev/null +++ b/extension/platform/examples/.gitignore @@ -0,0 +1,2 @@ +audit-observer/audit-observer +readonly-policy/readonly-policy diff --git a/extension/platform/examples/README.md b/extension/platform/examples/README.md new file mode 100644 index 00000000..c7eab33d --- /dev/null +++ b/extension/platform/examples/README.md @@ -0,0 +1,13 @@ +# lark-cli plugin examples + +Runnable fork-and-blank-import examples that demonstrate the Plugin +SDK in production-shape. Each subdirectory is a complete `main` +package: `go build .` produces a working CLI. + +| Example | What it shows | +| --- | --- | +| [audit-observer](./audit-observer/) | Simplest possible plugin: one Observer matching every command, logs to stderr. | +| [readonly-policy](./readonly-policy/) | Policy plugin: `Restrict()` with `MaxRisk=read`, demonstrates the `FailClosed` + `Restricts=true` auto-pairing. | + +All examples are built by CI (`make examples-build`) so they cannot +silently drift from the SDK. diff --git a/extension/platform/examples/audit-observer/README.md b/extension/platform/examples/audit-observer/README.md new file mode 100644 index 00000000..a860a4dd --- /dev/null +++ b/extension/platform/examples/audit-observer/README.md @@ -0,0 +1,26 @@ +# Example: audit observer + +The simplest possible lark-cli plugin: one After observer that logs +every dispatched command to stderr (success or failure). + +## Build & run + +```sh +cd extension/platform/examples/audit-observer +go build -o audit-cli . +./audit-cli config plugins show +# {"plugins":[{"name":"audit", ...}], "total":1} + +./audit-cli api GET /open-apis/contact/v3/users/me +# [audit] api ok (on stderr) +``` + +## Key points + +- `platform.NewPlugin(...).MustBuild()` from `init()`. The blank + import of this package in `main.go` triggers `init()`. +- `Observer(platform.After, ...)` runs **after** the command's RunE, + even on failure (Observers cannot prevent execution). +- `FailOpen()` means: if Install ever fails, the binary logs a + warning and continues without this plugin. Right default for + audit-only plugins. diff --git a/extension/platform/examples/audit-observer/main.go b/extension/platform/examples/audit-observer/main.go new file mode 100644 index 00000000..2c3c3053 --- /dev/null +++ b/extension/platform/examples/audit-observer/main.go @@ -0,0 +1,44 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +// Command audit-observer is a runnable fork of lark-cli that logs +// every dispatched command to stderr. Demonstrates the simplest +// possible plugin: one After observer matching All commands. +// +// Build & run: +// +// cd extension/platform/examples/audit-observer +// go build -o audit-cli . +// ./audit-cli config plugins show # see "audit" in the list +// ./audit-cli api GET /open-apis/... # observer logs to stderr +package main + +import ( + "context" + "fmt" + "log" + "os" + + "github.com/larksuite/cli/cmd" + "github.com/larksuite/cli/extension/platform" +) + +func init() { + platform.Register( + platform.NewPlugin("audit", "0.1.0"). + Observer(platform.After, "log", platform.All(), + func(ctx context.Context, inv platform.Invocation) { + path := inv.Cmd().Path() + if err := inv.Err(); err != nil { + fmt.Fprintf(os.Stderr, "[audit] %s FAILED: %v\n", path, err) + } else { + log.Printf("[audit] %s ok", path) + } + }). + FailOpen(). + MustBuild()) +} + +func main() { + os.Exit(cmd.Execute()) +} diff --git a/extension/platform/examples/readonly-policy/README.md b/extension/platform/examples/readonly-policy/README.md new file mode 100644 index 00000000..638c7686 --- /dev/null +++ b/extension/platform/examples/readonly-policy/README.md @@ -0,0 +1,62 @@ +# Example: read-only policy + +A policy plugin that installs a `Rule` allowing only `docs/*` and +`im/*` read commands. Any write command produces a structured +`command_denied` envelope. + +## Build & run + +```sh +cd extension/platform/examples/readonly-policy +go build -o readonly-cli . + +./readonly-cli config policy show +# { +# "source": "plugin", +# "source_name": "readonly", +# "yaml_path": "/Users/you/.lark-cli/policy.yml", +# "denied_paths": N, +# "rule": { +# "name": "agent-readonly", +# "allow": ["docs/**", "im/**"], +# "deny": [], +# "max_risk": "read", +# "identities": [], +# "allow_unannotated": false +# } +# } + +./readonly-cli docs +update --doc-token X --content Y +# {"ok":false,"error":{ +# "type":"command_denied", +# "detail":{ +# "layer":"policy", +# "policy_source":"plugin:readonly", +# "rule_name":"agent-readonly", +# "reason_code":"write_not_allowed" +# } +# }} + +./readonly-cli docs +fetch --doc-token X +# Normal read response (assuming credentials) +``` + +## Key points + +- `Restrict(&Rule{...})` is the only call needed — the Builder + flips Capabilities to `Restricts=true, FailurePolicy=FailClosed` + automatically. A policy plugin that silently fails to install + would erase the security boundary, so FailClosed is enforced. +- `MaxRisk: platform.RiskRead` rejects any command annotated + write / high-risk-write. +- `AllowUnannotated` is left default (false): unannotated commands + are denied with `risk_not_annotated`. Set it to true if you need + a gradual-adoption window for the lark-cli main tree. + +## Caveats + +- A binary may have **only one** plugin calling `Restrict()`. Two + policy plugins is a deliberate `plugin_conflict` configuration + error. +- This Rule shadows any `~/.lark-cli/policy.yml` — plugin Rule + wins per the resolver precedence. diff --git a/extension/platform/examples/readonly-policy/main.go b/extension/platform/examples/readonly-policy/main.go new file mode 100644 index 00000000..21b674bd --- /dev/null +++ b/extension/platform/examples/readonly-policy/main.go @@ -0,0 +1,45 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +// Command readonly-policy is a runnable fork of lark-cli that +// installs a Rule permitting only docs/* and im/* read commands. +// Any write command produces a structured command_denied envelope. +// +// Build & run: +// +// cd extension/platform/examples/readonly-policy +// go build -o readonly-cli . +// ./readonly-cli docs +update --doc-token X --content Y +// # {"ok":false,"error":{"type":"command_denied", ...}} +// +// ./readonly-cli config policy show +// # shows the active Rule with source=plugin:readonly +package main + +import ( + "os" + + "github.com/larksuite/cli/cmd" + "github.com/larksuite/cli/extension/platform" +) + +func init() { + platform.Register( + platform.NewPlugin("readonly", "0.1.0"). + Restrict(&platform.Rule{ + Name: "agent-readonly", + Description: "Only read-class docs/im commands. Suitable for AI-agent sessions.", + Allow: []string{"docs/**", "im/**"}, + MaxRisk: platform.RiskRead, + // AllowUnannotated stays default false (fail-closed): + // unannotated commands are denied, surfacing missing + // risk_level annotations early in adoption. + }). + MustBuild()) + // Note: Restrict() implicitly sets Restricts=true and FailClosed. + // No need to call FailClosed() explicitly. +} + +func main() { + os.Exit(cmd.Execute()) +} diff --git a/extension/platform/handler.go b/extension/platform/handler.go index 2a0cc802..c0863596 100644 --- a/extension/platform/handler.go +++ b/extension/platform/handler.go @@ -9,20 +9,28 @@ import "context" // "command business logic" from the Wrapper's perspective -- calling // next(ctx, inv) inside a Wrapper means "let the command proceed"; // returning early without calling next short-circuits. -type Handler func(ctx context.Context, inv *Invocation) error +type Handler func(ctx context.Context, inv Invocation) error // Observer is a side-effect-only command hook. No return value, no // next-chain control: an Observer can read Invocation but cannot prevent // the command from running. Used for audit, metrics, and completion // logs. After-stage Observers fire even when the command failed -// (Invocation.Err is populated in that case). -type Observer func(ctx context.Context, inv *Invocation) +// (Invocation.Err() is populated in that case). +type Observer func(ctx context.Context, inv Invocation) // Wrapper is a middleware-style hook: it receives the rest of the // handler chain and returns a wrapped version. The Wrapper decides // whether to call next (allow), abstain (deny, return an AbortError), // or transform the result. Multiple Wrappers compose left-to-right by // registration order; the outermost runs first. +// +// ⚠️ IMPORTANT: The factory function `func(next Handler) Handler` is +// invoked ONCE PER COMMAND DISPATCH, not once at plugin install. This +// lets the framework recover from a panicking factory and convert it +// to a structured envelope, but it means any state captured by the +// outer closure is rebuilt on every command. Long-lived state (HTTP +// clients, caches, metrics counters) MUST live on the Plugin struct +// or in package-level variables, never in factory-local captures. type Wrapper func(next Handler) Handler // LifecycleHandler runs at one of the process-level LifecycleEvent diff --git a/extension/platform/identity.go b/extension/platform/identity.go new file mode 100644 index 00000000..1354f37d --- /dev/null +++ b/extension/platform/identity.go @@ -0,0 +1,40 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package platform + +import "fmt" + +// Identity is the identity taxonomy a command supports. +// +// Defined type (not alias) so plugin authors get compile-time + +// IDE help; raw-string boundaries (yaml, cobra annotation) cross +// through ParseIdentity. +type Identity string + +const ( + IdentityUser Identity = "user" + IdentityBot Identity = "bot" +) + +// ParseIdentity converts a raw string into an Identity. Returns +// ("", nil) for empty input ("not specified"), error for unrecognised +// values. Matching is strict (case-sensitive, no trim). +func ParseIdentity(s string) (Identity, error) { + if s == "" { + return "", nil + } + id := Identity(s) + if id != IdentityUser && id != IdentityBot { + return "", fmt.Errorf("invalid identity %q: must be user|bot", s) + } + return id, nil +} + +// IsValid reports whether i is one of the two recognised values. +func (i Identity) IsValid() bool { + return i == IdentityUser || i == IdentityBot +} + +// String returns the underlying string. +func (i Identity) String() string { return string(i) } diff --git a/extension/platform/invocation.go b/extension/platform/invocation.go index f25edd1f..80fa6b53 100644 --- a/extension/platform/invocation.go +++ b/extension/platform/invocation.go @@ -5,106 +5,68 @@ package platform import "time" -// Invocation carries the per-command context a Wrapper or Observer needs. -// Cmd is the read-only snapshot taken before any RunE replacement (see -// CommandView); Args is the actual user input; Started is when the -// outermost RunE wrapper began. Err is populated for After hooks and -// the post-next portion of a Wrapper. +// Invocation is the per-command data a Wrapper / Observer receives. It +// is a read-only interface: the framework implementation lives in +// internal/hook and is never visible to plugins, so there is no way +// for plugin code to mutate denial / strict-mode / identity state. // -// The struct is deliberately NOT a context.Context -- it is data only, -// no cancellation. ctx (from the function signature) is the -// context.Context for cancellation/timeout/trace propagation. +// The struct is deliberately NOT a context.Context — it is data only, +// no cancellation. ctx (from the handler signature) carries +// cancellation / timeout / trace propagation. // -// Implementation note: the lazy fields (DeniedByPolicy, Identity, etc.) -// are populated by the framework before any hook fires. Plugins must -// not depend on these being non-zero at construction; they always read -// through the accessor methods which centralise the "is this populated -// yet?" logic. -type Invocation struct { - Cmd CommandView - Args []string - Started time.Time - Err error - - // Unexported state populated by the framework. Plugins read it via - // the methods below; direct field access is impossible. - deniedByPolicy bool - denialLayer string // "strict_mode" / "pruning" / "" - denialSource string // "plugin:secaudit" / "yaml" / "strict-mode" / "" - - // strictMode is the resolved credential strict-mode value, or - // the empty string when no strict-mode is active. We do not use - // a separate "resolved?" bool: the StrictMode() accessor returns - // ok=false when the lifecycle has not yet resolved this. - strictMode string - strictModeKnown bool - - identity string - identityResolved bool -} - -// DeniedByPolicy reports whether the command was rejected by either -// strict-mode or user-layer pruning before the chain reached the -// hook. Observers fire even for denied commands (audit case); Wrap is -// physically isolated by the framework so plugins do not need to check -// this themselves before calling next. -func (inv *Invocation) DeniedByPolicy() bool { return inv.deniedByPolicy } - -// DenialLayer returns the layer that rejected the command: +// Accessor semantics: // -// "" - not denied -// "strict_mode" - credential strict-mode -// "pruning" - user-layer Rule (Plugin.Restrict() or yaml) -// -// Matches the error.type field in the envelope so consumers can route -// recovery logic by this value alone. -func (inv *Invocation) DenialLayer() string { return inv.denialLayer } +// - Cmd / Args / Started are populated before the first hook fires +// - Err is populated for After observers and the post-next portion of +// a Wrapper (the value the wrapped handler returned) +// - DeniedByPolicy / DenialLayer / DenialPolicySource are populated by +// the framework's denial guard before any hook runs +// - StrictMode / Identity may return ok=false in Before observers if +// the bootstrap pipeline has not yet resolved them; After observers +// always see ok=true +type Invocation interface { + // Cmd is the read-only snapshot of the dispatched command. + Cmd() CommandView -// DenialPolicySource returns the specific source identifier -// ("plugin:secaudit", "yaml", "strict-mode") corresponding to the -// denial. Empty when the command was not denied. -func (inv *Invocation) DenialPolicySource() string { return inv.denialSource } + // Args is the positional args slice the user invoked the command with. + Args() []string -// StrictMode returns the active credential strict-mode value -// ("user", "bot", "off"). ok=false signals "not yet resolved" -- the -// Bootstrap pipeline resolves strict-mode before any hook fires, so in -// practice hooks always see ok=true; the bool exists to keep this -// safe under future reordering. -func (inv *Invocation) StrictMode() (mode string, ok bool) { - return inv.strictMode, inv.strictModeKnown -} - -// Identity returns the resolved identity ("user"/"bot") for the -// current command. resolved=false means the framework has not yet -// resolved identity at the call site (Before observers and Wrap entry -// may see this; After observers always see resolved=true). -func (inv *Invocation) Identity() (id string, resolved bool) { - return inv.identity, inv.identityResolved -} - -// --- internal setters (lower-case, package-internal) --- -// -// Public callers cannot mutate these fields; the framework uses -// targeted helpers exposed only to internal/hook. - -// SetDenial is called by the framework before the hook chain runs. -// Exported with "Internal" prefix to mark "framework-only" intent; it -// is technically importable but lives outside the contract surface. -// Renaming or removing it is not a breaking change. -func (inv *Invocation) InternalSetDenial(deniedByPolicy bool, layer, source string) { - inv.deniedByPolicy = deniedByPolicy - inv.denialLayer = layer - inv.denialSource = source -} - -// InternalSetStrictMode populates the strict-mode accessor. -func (inv *Invocation) InternalSetStrictMode(mode string, known bool) { - inv.strictMode = mode - inv.strictModeKnown = known -} - -// InternalSetIdentity populates the identity accessor. -func (inv *Invocation) InternalSetIdentity(id string, resolved bool) { - inv.identity = id - inv.identityResolved = resolved + // Started is the wall-clock time the outermost RunE wrapper began. + Started() time.Time + + // Err is the error the wrapped handler returned. Populated for + // After observers and the post-next portion of a Wrapper. nil + // before the handler runs. + Err() error + + // DeniedByPolicy reports whether the command was rejected by either + // strict-mode or user-layer policy before the chain reached the + // hook. Observers fire even for denied commands (audit case); Wrap + // is physically isolated by the framework so plugins do not need + // to check this themselves before calling next. + DeniedByPolicy() bool + + // DenialLayer returns the layer that rejected the command: + // + // "" - not denied + // "strict_mode" - credential strict-mode + // "policy" - user-layer Rule (Plugin.Restrict() or yaml) + // + // Matches the detail.layer field in the envelope so consumers can + // route recovery logic by this value alone. + DenialLayer() string + + // DenialPolicySource returns the specific source identifier + // ("plugin:secaudit", "yaml", "strict-mode") corresponding to the + // denial. Empty when the command was not denied. + DenialPolicySource() string + + // StrictMode returns the active credential strict-mode value + // ("user", "bot", "off"). ok=false signals "not yet resolved". + StrictMode() (mode string, ok bool) + + // Identity returns the resolved identity ("user"/"bot") for the + // current command. resolved=false means the framework has not yet + // resolved identity at the call site. + Identity() (id string, resolved bool) } diff --git a/extension/platform/register.go b/extension/platform/register.go index bc95027d..fe22059d 100644 --- a/extension/platform/register.go +++ b/extension/platform/register.go @@ -27,13 +27,6 @@ func RegisteredPlugins() []Plugin { return pluginRegistry.snapshot() } -// ResetForTesting clears the registry. Test code uses this to isolate -// test cases that register plugins. It is exported to test packages -// only by convention; production code never calls it. -func ResetForTesting() { - pluginRegistry.reset() -} - // pluginRegistry is the package-level singleton. The mutex protects // concurrent Register calls -- harmless in practice (init runs // serially) but cheap insurance. diff --git a/extension/platform/register_testing.go b/extension/platform/register_testing.go new file mode 100644 index 00000000..878d6d66 --- /dev/null +++ b/extension/platform/register_testing.go @@ -0,0 +1,14 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build testing + +package platform + +// ResetForTesting clears the global plugin registry. Available only +// under `-tags testing`; not part of the public API. +// +// Tests that exercise plugin registration should defer +// `t.Cleanup(platform.ResetForTesting)` so subsequent tests start +// from a clean slate. +func ResetForTesting() { pluginRegistry.reset() } diff --git a/extension/platform/risk.go b/extension/platform/risk.go new file mode 100644 index 00000000..287c5ff8 --- /dev/null +++ b/extension/platform/risk.go @@ -0,0 +1,71 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package platform + +import "fmt" + +// Risk is the three-tier risk taxonomy declared on every command. +// +// A defined type (not an alias of string) so plugin authors get +// compile-time + IDE candidate help when passing the constants below. +// Crossing the string boundary (yaml, cobra annotation) goes through +// ParseRisk so typos surface as `risk_invalid` rather than silently +// flowing through. +type Risk string + +const ( + RiskRead Risk = "read" + RiskWrite Risk = "write" + RiskHighRiskWrite Risk = "high-risk-write" +) + +// riskOrder maps the Risk taxonomy to a comparable rank. The pruning +// engine compares ranks for the MaxRisk axis. +var riskOrder = map[Risk]int{ + RiskRead: 0, + RiskWrite: 1, + RiskHighRiskWrite: 2, +} + +// ParseRisk converts a raw string (yaml, cobra annotation) into a Risk. +// +// - s == "" → ("", nil) "not specified" +// - s 在闭合枚举 → (Risk(s), nil) OK +// - s 不在枚举内 → ("", error) invalid +// +// The (absent vs invalid) split mirrors the cmdpolicy engine's +// risk_not_annotated vs risk_invalid reason codes — callers can treat +// the "" + nil case as "not specified" without losing the distinction +// from a typo. +// +// Matching is strict: "Read" / "READ" / " read " are all rejected. +// annotation is developer code, not user input — strict matching is +// the typo-catch mechanism, not a normalisation opportunity. +func ParseRisk(s string) (Risk, error) { + if s == "" { + return "", nil + } + r := Risk(s) + if _, ok := riskOrder[r]; !ok { + return "", fmt.Errorf("invalid risk %q: must be read|write|high-risk-write", s) + } + return r, nil +} + +// IsValid reports whether r is one of the three recognised values. +func (r Risk) IsValid() bool { + _, ok := riskOrder[r] + return ok +} + +// Rank returns the comparable rank of r. ok=false when r is not in the +// closed taxonomy. +func (r Risk) Rank() (rank int, ok bool) { + rank, ok = riskOrder[r] + return rank, ok +} + +// String returns the underlying string. Useful for yaml/json output +// and cobra annotation injection. +func (r Risk) String() string { return string(r) } diff --git a/extension/platform/risk_test.go b/extension/platform/risk_test.go new file mode 100644 index 00000000..d934a03c --- /dev/null +++ b/extension/platform/risk_test.go @@ -0,0 +1,120 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package platform_test + +import ( + "testing" + + "github.com/larksuite/cli/extension/platform" +) + +func TestRisk_Rank_orderedTaxonomy(t *testing.T) { + cases := []struct { + level platform.Risk + want int + }{ + {platform.RiskRead, 0}, + {platform.RiskWrite, 1}, + {platform.RiskHighRiskWrite, 2}, + } + for _, c := range cases { + got, ok := c.level.Rank() + if !ok || got != c.want { + t.Errorf("Risk(%q).Rank() = (%d,%v), want (%d,true)", c.level, got, ok, c.want) + } + } + + if _, ok := platform.Risk("unknown-level").Rank(); ok { + t.Fatalf("unknown-level.Rank() ok should be false") + } + if _, ok := platform.Risk("").Rank(); ok { + t.Fatalf("empty.Rank() ok should be false (signals 'no risk annotation')") + } +} + +// The Risk ordering must be strict: read < write < high-risk-write. The +// policy engine compares ranks; a regression that swaps the order would +// silently let high-risk commands pass under MaxRisk=write. +func TestRisk_Rank_strictlyMonotonic(t *testing.T) { + r1, _ := platform.RiskRead.Rank() + r2, _ := platform.RiskWrite.Rank() + r3, _ := platform.RiskHighRiskWrite.Rank() + if !(r1 < r2 && r2 < r3) { + t.Fatalf("Risk ranks not monotonic: read=%d write=%d high=%d", r1, r2, r3) + } +} + +func TestRisk_IsValid(t *testing.T) { + valid := []platform.Risk{platform.RiskRead, platform.RiskWrite, platform.RiskHighRiskWrite} + for _, r := range valid { + if !r.IsValid() { + t.Errorf("%q.IsValid() = false, want true", r) + } + } + invalid := []platform.Risk{"", "wrtie", "Read", "READ", " read "} + for _, r := range invalid { + if r.IsValid() { + t.Errorf("%q.IsValid() = true, want false", r) + } + } +} + +// ParseRisk distinguishes absent (empty input) from invalid (typo). +// The absent / invalid split mirrors the cmdpolicy engine's +// risk_not_annotated vs risk_invalid reason codes. +func TestParseRisk(t *testing.T) { + // Empty -> ("", nil) — "not specified" + got, err := platform.ParseRisk("") + if err != nil || got != "" { + t.Errorf(`ParseRisk("") = (%q,%v), want ("",nil)`, got, err) + } + + // Valid values pass through + for _, want := range []platform.Risk{platform.RiskRead, platform.RiskWrite, platform.RiskHighRiskWrite} { + got, err := platform.ParseRisk(string(want)) + if err != nil || got != want { + t.Errorf("ParseRisk(%q) = (%q,%v), want (%q,nil)", want, got, err, want) + } + } + + // Typo -> error, strict matching (case-sensitive, no trim) + bad := []string{"wrtie", "Read", "READ", " read ", "high_risk_write"} + for _, s := range bad { + got, err := platform.ParseRisk(s) + if err == nil { + t.Errorf("ParseRisk(%q) succeeded (got %q), want error", s, got) + } + if got != "" { + t.Errorf("ParseRisk(%q) returned %q, want empty Risk on error", s, got) + } + } +} + +func TestParseIdentity(t *testing.T) { + got, err := platform.ParseIdentity("") + if err != nil || got != "" { + t.Errorf(`ParseIdentity("") = (%q,%v), want ("",nil)`, got, err) + } + for _, want := range []platform.Identity{platform.IdentityUser, platform.IdentityBot} { + got, err := platform.ParseIdentity(string(want)) + if err != nil || got != want { + t.Errorf("ParseIdentity(%q) = (%q,%v)", want, got, err) + } + } + if _, err := platform.ParseIdentity("admin"); err == nil { + t.Fatalf(`ParseIdentity("admin") want error`) + } +} + +func TestIdentity_IsValid(t *testing.T) { + if !platform.IdentityUser.IsValid() { + t.Error("user.IsValid() = false") + } + if !platform.IdentityBot.IsValid() { + t.Error("bot.IsValid() = false") + } + if platform.Identity("admin").IsValid() { + t.Error("admin.IsValid() = true") + } +} diff --git a/extension/platform/rule.go b/extension/platform/rule.go index 0d40c8df..cf5eceba 100644 --- a/extension/platform/rule.go +++ b/extension/platform/rule.go @@ -3,17 +3,17 @@ package platform -// Rule is the declarative pruning rule data structure. yaml files and (once -// the Hook surface lands) Plugin.Restrict() both produce the same Rule. +// Rule is the declarative policy rule data structure. yaml files and +// Plugin.Restrict() both produce the same Rule. // // At any moment there is at most one effective Rule -- the resolver decides // which source wins (Plugin > yaml > none). This package only defines the -// shape; selection lives in internal/pruning. +// shape; selection lives in internal/cmdpolicy. // // The four filter fields are joined by AND. See the engine's Evaluate for // the full semantics. JSON tags are used by `config policy show`; yaml -// parsing lives in internal/pruning/yaml so the public API does not depend -// on a yaml library. +// parsing lives in internal/cmdpolicy/yaml so the public API does not +// depend on a yaml library. type Rule struct { Name string `json:"name"` Description string `json:"description,omitempty"` @@ -36,4 +36,25 @@ type Rule struct { // the intersection with the command's own supported identities is // non-empty. Empty slice means "no identity restriction". Identities []Identity `json:"identities,omitempty"` + + // AllowUnannotated controls how commands missing a risk_level + // annotation are handled when this Rule is active. + // + // Default (false, fail-closed): unannotated commands are rejected + // with reason_code=risk_not_annotated. This is the safe default + // — a typo'd or forgotten annotation cannot slip past an + // "agent read-only" rule. + // + // Set to true to opt out during gradual adoption: lark-cli main + // has hundreds of service commands that may not yet carry + // risk_level annotations, and a brand-new policy plugin would + // otherwise lock the binary to nothing. + // + // This flag does NOT affect risk_invalid (typos): a command that + // claims a risk but mis-spells it is always denied, regardless of + // AllowUnannotated. Typo is a code bug, not a migration phase. + // + // No yaml tag: yaml decoding lives in internal/cmdpolicy/yaml so + // platform stays free of a yaml library dependency. + AllowUnannotated bool `json:"allow_unannotated,omitempty"` } diff --git a/extension/platform/selector.go b/extension/platform/selector.go index 29412af9..0e632537 100644 --- a/extension/platform/selector.go +++ b/extension/platform/selector.go @@ -47,7 +47,7 @@ func ByCommandPath(patterns ...string) Selector { // ByIdentity matches when the command's supported identities include // the supplied id. Unknown identities never match. -func ByIdentity(id string) Selector { +func ByIdentity(id Identity) Selector { return func(cmd CommandView) bool { for _, x := range cmd.Identities() { if x == id { @@ -61,9 +61,10 @@ func ByIdentity(id string) Selector { // Risk-based selectors below match only commands whose declared risk // equals the selector's target level. The closed taxonomy is read / // write / high-risk-write — there is no "unknown" branch in the public -// API. When any pruning Rule is registered, the pruning engine treats -// unannotated commands as implicit deny, so risk-based selectors never -// see them in hook dispatch. +// API. When a Rule without AllowUnannotated=true is registered, the +// policy engine treats unannotated commands as implicit deny, so risk- +// based selectors never see them in hook dispatch under that +// configuration. // ByExactRisk matches commands whose declared risk level is exactly level. func ByExactRisk(level Risk) Selector { @@ -114,7 +115,8 @@ func (s Selector) Or(other Selector) Selector { } } -// Not negates the selector. +// Not negates the selector. A nil receiver is treated as None(), so +// nil.Not() behaves as All(). func (s Selector) Not() Selector { inner := normalize(s) return func(cmd CommandView) bool { diff --git a/extension/platform/selector_test.go b/extension/platform/selector_test.go index ae05c314..f08b0c66 100644 --- a/extension/platform/selector_test.go +++ b/extension/platform/selector_test.go @@ -18,10 +18,16 @@ type fakeView struct { identities []string } -func (v fakeView) Path() string { return v.path } -func (v fakeView) Domain() string { return v.domain } -func (v fakeView) Risk() (string, bool) { return v.risk, v.riskOK } -func (v fakeView) Identities() []string { return v.identities } +func (v fakeView) Path() string { return v.path } +func (v fakeView) Domain() string { return v.domain } +func (v fakeView) Risk() (platform.Risk, bool) { return platform.Risk(v.risk), v.riskOK } +func (v fakeView) Identities() []platform.Identity { + out := make([]platform.Identity, len(v.identities)) + for i, x := range v.identities { + out[i] = platform.Identity(x) + } + return out +} func (v fakeView) Annotation(key string) (string, bool) { return "", false } func TestAll_None(t *testing.T) { @@ -50,8 +56,8 @@ func TestByDomain(t *testing.T) { // Risk-based selectors match only against the closed taxonomy // (read / write / high-risk-write). Commands without a risk annotation -// never match; the pruning engine guarantees such commands cannot reach -// hook dispatch when any Rule is registered. +// never match; the policy engine guarantees such commands cannot reach +// hook dispatch when a Rule without AllowUnannotated=true is registered. func TestByExactRisk_unknownDoesNotMatch(t *testing.T) { sel := platform.ByExactRisk("write") if !sel(fakeView{risk: "write", riskOK: true}) { diff --git a/extension/platform/types.go b/extension/platform/types.go deleted file mode 100644 index b6ec7629..00000000 --- a/extension/platform/types.go +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright (c) 2026 Lark Technologies Pte. Ltd. -// SPDX-License-Identifier: MIT - -package platform - -// Risk is the three-tier risk taxonomy. Aliased to string (not a defined -// type) so plugin authors can use either the constants below or raw literals -// without conversion friction. -type Risk = string - -const ( - RiskRead Risk = "read" - RiskWrite Risk = "write" - RiskHighRiskWrite Risk = "high-risk-write" -) - -// Identity values supported by the framework. Aliased to string for the same -// reason as Risk. -type Identity = string - -const ( - IdentityUser Identity = "user" - IdentityBot Identity = "bot" -) - -// riskOrder maps the Risk taxonomy to a comparable rank. Used by the pruning -// engine's MaxRisk check: c.Risk <= MaxRisk holds when riskOrder[c.Risk] <= -// riskOrder[MaxRisk]. Defined here so the public taxonomy and the comparable -// ordering live next to each other; unknown levels return -1 so callers -// can detect "this is not a recognised risk". -var riskOrder = map[Risk]int{ - RiskRead: 0, - RiskWrite: 1, - RiskHighRiskWrite: 2, -} - -// RiskRank returns a comparable rank for a Risk value. ok=false when the -// value is not one of the three recognised constants. -func RiskRank(r Risk) (rank int, ok bool) { - rank, ok = riskOrder[r] - return rank, ok -} diff --git a/extension/platform/view.go b/extension/platform/view.go index 22d2d1d6..67c68a4e 100644 --- a/extension/platform/view.go +++ b/extension/platform/view.go @@ -4,7 +4,7 @@ package platform // CommandView is the read-only view of a cobra.Command exposed to plugins -// and the pruning engine. *cobra.Command is deliberately NOT reachable +// and the policy engine. *cobra.Command is deliberately NOT reachable // through this interface -- a plugin should never mutate the command tree. // // snapshot rules (enforced by hard-constraint #1 in the tech doc): @@ -17,10 +17,11 @@ package platform // - Path() is the canonical slash form ("docs/+fetch"), matching the // doublestar glob semantics used by Rule.Allow / Rule.Deny. // -// - Risk() returns ok=false when the command is unannotated. The -// pruning engine treats an unannotated command as implicit deny -// whenever any Rule is registered, so risk-based Selectors never see -// unannotated commands during normal hook dispatch. +// - Risk() returns ok=false when the command is unannotated. The policy +// engine treats an unannotated command as implicit deny whenever any +// Rule without AllowUnannotated=true is registered, so risk-based +// Selectors never see unannotated commands during normal hook dispatch +// under that configuration. type CommandView interface { // Path is the canonical slash-separated path, rootless ("docs/+update"). Path() string diff --git a/internal/cmdmeta/meta.go b/internal/cmdmeta/meta.go index 9355102a..f0a9ea6b 100644 --- a/internal/cmdmeta/meta.go +++ b/internal/cmdmeta/meta.go @@ -2,8 +2,8 @@ // SPDX-License-Identifier: MIT // Package cmdmeta is the single source of truth for command metadata that the -// pruning engine and (later) the hook selector both consume. It wraps the -// existing cmdutil annotations (risk_level, supportedIdentities) and adds the +// policy engine and the hook selector both consume. It wraps the existing +// cmdutil annotations (risk_level, supportedIdentities) and adds the // "domain" axis that the hook selector and Rule path globs need. // // Three axes: @@ -21,10 +21,11 @@ // GetSupportedIdentities. // // Missing values are returned as the zero value with ok=false (where the -// signature exposes it). Interpretation is up to the consumer: the pruning +// signature exposes it). Interpretation is up to the consumer: the policy // engine treats a missing risk as fail-closed when a Rule is registered -// and as allow when no Rule is registered. Identities still defaults to -// ALLOW. Do not synthesise defaults here -- let each consumer decide. +// without AllowUnannotated=true, and as allow otherwise. Identities still +// defaults to ALLOW. Do not synthesise defaults here -- let each consumer +// decide. package cmdmeta import ( @@ -38,8 +39,8 @@ import ( // disturbing existing readers. const domainAnnotationKey = "cmdmeta.domain" -// Meta groups the three command-level metadata axes consumed by pruning and -// hook selectors. +// Meta groups the three command-level metadata axes consumed by the policy +// engine and hook selectors. type Meta struct { Domain string Risk string @@ -94,7 +95,7 @@ func SetDomain(cmd *cobra.Command, domain string) { // Domain returns the nearest-ancestor domain for the command. Empty string // when no ancestor has the annotation -- this is the "unknown" state the -// pruning engine must treat as ALLOW. +// policy engine must treat as ALLOW. func Domain(cmd *cobra.Command) string { for c := cmd; c != nil; c = c.Parent() { if c.Annotations == nil { @@ -108,9 +109,9 @@ func Domain(cmd *cobra.Command) string { } // Risk returns the nearest-ancestor risk level (via cmdutil.GetRisk). -// ok=false signals "unknown" -- the pruning engine treats this as -// fail-closed (deny with risk_not_annotated) whenever a Rule is active, -// and as allow when no Rule is registered. +// ok=false signals "unknown" -- the policy engine treats this as +// fail-closed (deny with risk_not_annotated) whenever a Rule without +// AllowUnannotated=true is active, and as allow otherwise. func Risk(cmd *cobra.Command) (level string, ok bool) { for c := cmd; c != nil; c = c.Parent() { if level, ok = cmdutil.GetRisk(c); ok { @@ -121,7 +122,8 @@ func Risk(cmd *cobra.Command) (level string, ok bool) { } // Identities returns the first non-nil identity set found while walking up -// the parent chain. nil signals "unknown" -- pruning treats this as ALLOW. +// the parent chain. nil signals "unknown" -- the policy engine treats this +// as ALLOW. // // cmdutil.GetSupportedIdentities returns nil when the annotation is absent // or empty; an explicit non-empty set (even ["user"] alone) stops the walk. diff --git a/internal/cmdmeta/meta_test.go b/internal/cmdmeta/meta_test.go index 0990cbd4..61e83131 100644 --- a/internal/cmdmeta/meta_test.go +++ b/internal/cmdmeta/meta_test.go @@ -97,7 +97,7 @@ func TestGet_nearestAncestorWins(t *testing.T) { } } -// Unknown axes return zero / nil so the pruning engine can apply the +// Unknown axes return zero / nil so the policy engine can apply the // "unknown => ALLOW" contract. func TestGet_unknownReturnsZero(t *testing.T) { cmd := &cobra.Command{Use: "orphan"} diff --git a/internal/pruning/active.go b/internal/cmdpolicy/active.go similarity index 98% rename from internal/pruning/active.go rename to internal/cmdpolicy/active.go index 052824ae..d30d7a51 100644 --- a/internal/pruning/active.go +++ b/internal/cmdpolicy/active.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning +package cmdpolicy import ( "sync" diff --git a/internal/pruning/aggregation_test.go b/internal/cmdpolicy/aggregation_test.go similarity index 86% rename from internal/pruning/aggregation_test.go rename to internal/cmdpolicy/aggregation_test.go index 60caf53a..f0d36e52 100644 --- a/internal/pruning/aggregation_test.go +++ b/internal/cmdpolicy/aggregation_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning_test +package cmdpolicy_test import ( "encoding/json" @@ -12,10 +12,9 @@ import ( "github.com/spf13/cobra" "github.com/larksuite/cli/extension/platform" + "github.com/larksuite/cli/internal/cmdpolicy" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/output" - "github.com/larksuite/cli/internal/policydecision" - "github.com/larksuite/cli/internal/pruning" ) // EvaluateAll must skip non-runnable parent groups (their decision is @@ -24,7 +23,7 @@ import ( // because the parent's own path "docs" did not match "docs/**". func TestEvaluateAll_skipsPureGroups(t *testing.T) { root := buildTree() // docs and im are pure groups, +fetch / +update / +send are leaves - e := pruning.New(&platform.Rule{Allow: []string{"docs/**"}}) + e := cmdpolicy.New(&platform.Rule{Allow: []string{"docs/**"}}) got := e.EvaluateAll(root) if _, present := got["docs"]; present { @@ -56,7 +55,7 @@ func TestBuildDeniedByPath_parentAggregationAllChildrenDenied(t *testing.T) { // Risk is set on both leaves so the rejection comes from the Allow // axis (the contract this test pins), not from the risk gate. - e := pruning.New(&platform.Rule{Allow: []string{"docs/**"}}) // none of im/* matches + e := cmdpolicy.New(&platform.Rule{Allow: []string{"docs/**"}}) // none of im/* matches decisions := e.EvaluateAll(root) // Pin the rejection axis: both leaves are rejected by Allow miss, @@ -70,8 +69,8 @@ func TestBuildDeniedByPath_parentAggregationAllChildrenDenied(t *testing.T) { t.Errorf("im/+search ReasonCode = %q, want domain_not_allowed", rc) } - denied := pruning.BuildDeniedByPath(root, decisions, - pruning.ResolveSource{Kind: pruning.SourceYAML, Name: "/policy.yml"}, "agent") + denied := cmdpolicy.BuildDeniedByPath(root, decisions, + cmdpolicy.ResolveSource{Kind: cmdpolicy.SourceYAML, Name: "/policy.yml"}, "agent") // Both leaves denied. if _, ok := denied["im/+send"]; !ok { @@ -85,7 +84,7 @@ func TestBuildDeniedByPath_parentAggregationAllChildrenDenied(t *testing.T) { if !ok { t.Fatalf("parent 'im' should be aggregated into denied map") } - if parent.Layer != "pruning" { + if parent.Layer != "policy" { t.Errorf("parent.Layer = %q, want pruning", parent.Layer) } } @@ -106,12 +105,12 @@ func TestBuildDeniedByPath_partialDenialKeepsParent(t *testing.T) { cmdutil.SetRisk(delete, "high-risk-write") docs.AddCommand(delete) // denied by Deny - e := pruning.New(&platform.Rule{ + e := cmdpolicy.New(&platform.Rule{ Allow: []string{"docs/**"}, Deny: []string{"docs/+delete"}, }) - denied := pruning.BuildDeniedByPath(root, e.EvaluateAll(root), - pruning.ResolveSource{Kind: pruning.SourcePlugin, Name: "secaudit"}, "secaudit-policy") + denied := cmdpolicy.BuildDeniedByPath(root, e.EvaluateAll(root), + cmdpolicy.ResolveSource{Kind: cmdpolicy.SourcePlugin, Name: "secaudit"}, "secaudit-policy") if _, ok := denied["docs"]; ok { t.Errorf("parent 'docs' must NOT be denied when some children are allowed") @@ -128,9 +127,9 @@ func TestBuildDeniedByPath_partialDenialKeepsParent(t *testing.T) { // descendants are denied -- the entry point must remain dispatchable. func TestBuildDeniedByPath_rootNeverDenied(t *testing.T) { root := buildTree() - e := pruning.New(&platform.Rule{Allow: []string{"nonexistent/**"}}) - denied := pruning.BuildDeniedByPath(root, e.EvaluateAll(root), - pruning.ResolveSource{Kind: pruning.SourceYAML, Name: "/p.yml"}, "") + e := cmdpolicy.New(&platform.Rule{Allow: []string{"nonexistent/**"}}) + denied := cmdpolicy.BuildDeniedByPath(root, e.EvaluateAll(root), + cmdpolicy.ResolveSource{Kind: cmdpolicy.SourceYAML, Name: "/p.yml"}, "") // Every leaf should be denied. We do not assert on the root entry // because Apply skips the root regardless; the contract is "root @@ -153,11 +152,11 @@ func TestBuildDeniedByPath_hybridParentOwnAllowedKeepsAlive(t *testing.T) { docs.AddCommand(delete) // Allow "docs" (parent) but deny "+delete" child. - e := pruning.New(&platform.Rule{ + e := cmdpolicy.New(&platform.Rule{ Allow: []string{"docs"}, }) - denied := pruning.BuildDeniedByPath(root, e.EvaluateAll(root), - pruning.ResolveSource{Kind: pruning.SourceYAML, Name: ""}, "") + denied := cmdpolicy.BuildDeniedByPath(root, e.EvaluateAll(root), + cmdpolicy.ResolveSource{Kind: cmdpolicy.SourceYAML, Name: ""}, "") // docs/+delete denied (path doesn't match Allow=["docs"]). if _, ok := denied["docs/+delete"]; !ok { @@ -175,16 +174,16 @@ func TestBuildDeniedByPath_hybridParentOwnAllowedKeepsAlive(t *testing.T) { // 2. in-process consumers extracting the platform.CommandDeniedError func TestApply_runEReturnsExitErrorAndCommandDeniedError(t *testing.T) { root := buildTree() - denied := map[string]policydecision.Denial{ + denied := map[string]cmdpolicy.Denial{ "docs/+update": { - Layer: "pruning", + Layer: "policy", PolicySource: "plugin:secaudit", RuleName: "secaudit-policy", ReasonCode: "write_not_allowed", Reason: "write disabled", }, } - pruning.Apply(root, denied) + cmdpolicy.Apply(root, denied) update := findChild(t, root, "docs", "+update") err := update.RunE(update, []string{}) @@ -245,10 +244,10 @@ func TestApply_runEReturnsExitErrorAndCommandDeniedError(t *testing.T) { // is denied. cobra still needs root to dispatch help / completion. func TestApply_neverInstallsOnRoot(t *testing.T) { root := buildTree() - denied := map[string]policydecision.Denial{ - "lark-cli": {Layer: "pruning", ReasonCode: "all_children_denied"}, + denied := map[string]cmdpolicy.Denial{ + "lark-cli": {Layer: "policy", ReasonCode: "all_children_denied"}, } - pruning.Apply(root, denied) + cmdpolicy.Apply(root, denied) if root.RunE != nil { t.Errorf("root.RunE should remain nil; got a denyStub installed") } diff --git a/internal/pruning/apply.go b/internal/cmdpolicy/apply.go similarity index 68% rename from internal/pruning/apply.go rename to internal/cmdpolicy/apply.go index f8aa5e20..6b28993d 100644 --- a/internal/pruning/apply.go +++ b/internal/cmdpolicy/apply.go @@ -1,18 +1,17 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning +package cmdpolicy import ( "github.com/spf13/cobra" "github.com/larksuite/cli/extension/platform" "github.com/larksuite/cli/internal/output" - "github.com/larksuite/cli/internal/policydecision" ) // Apply walks the command tree and installs denyStubs for every path in -// deniedByPath whose Denial.Layer == "pruning". It is the user-layer +// deniedByPath whose Denial.Layer == "policy". It is the user-layer // counterpart to applyStrictModeDenials in cmd/prune.go; both consume the // same deniedByPath map produced by the bootstrap pipeline, neither // re-evaluates rules. @@ -39,7 +38,7 @@ import ( // cobra.Execute. It mutates the command tree in place and is not safe to // call concurrently with command dispatch. Returns the number of commands // modified. -func Apply(root *cobra.Command, deniedByPath map[string]policydecision.Denial) int { +func Apply(root *cobra.Command, deniedByPath map[string]Denial) int { if root == nil || len(deniedByPath) == 0 { return 0 } @@ -58,7 +57,7 @@ func Apply(root *cobra.Command, deniedByPath map[string]policydecision.Denial) i return } d, ok := deniedByPath[path] - if !ok || d.Layer != policydecision.LayerPruning { + if !ok || d.Layer != LayerPolicy { return } installDenyStub(c, path, d) @@ -67,17 +66,57 @@ func Apply(root *cobra.Command, deniedByPath map[string]policydecision.Denial) i return count } -// AnnotationDenialLayer is the cobra annotation key written by -// installDenyStub to signal "this command is denied" to layers above -// the pruning package (specifically internal/hook reads it to populate -// Invocation.DeniedByPolicy without importing pruning, avoiding an -// import cycle). -const AnnotationDenialLayer = "lark:pruning_denied_layer" +// AnnotationDenialLayer / AnnotationDenialSource carry the denial +// signal to internal/hook through cobra annotations, avoiding an +// import cycle between hook and cmdpolicy. +const ( + AnnotationDenialLayer = "lark:policy_denied_layer" + AnnotationDenialSource = "lark:policy_denied_source" +) -// AnnotationDenialSource records the matching PolicySource so the hook -// layer can populate Invocation.DenialPolicySource() with the right -// value. -const AnnotationDenialSource = "lark:pruning_denied_source" +// CommandDeniedFromDenial materialises the wrapped error type carried +// on ExitError.Err so errors.As works for in-process consumers. +func CommandDeniedFromDenial(path string, d Denial) *platform.CommandDeniedError { + return &platform.CommandDeniedError{ + Path: path, + Layer: d.Layer, + PolicySource: d.PolicySource, + RuleName: d.RuleName, + ReasonCode: d.ReasonCode, + Reason: d.Reason, + } +} + +// DenialDetailMap is the canonical detail.* shape every `command_denied` +// envelope shares (see docs/extension/reason-codes.md). Use it as +// ErrDetail.Detail when constructing an envelope outside BuildDenialError. +func DenialDetailMap(cd *platform.CommandDeniedError) map[string]any { + return map[string]any{ + "path": cd.Path, + "layer": cd.Layer, + "policy_source": cd.PolicySource, + "rule_name": cd.RuleName, + "reason_code": cd.ReasonCode, + "reason": cd.Reason, + } +} + +// BuildDenialError is the default envelope for user-layer denials: +// Message comes from CommandDeniedError.Error(), no Hint. Callers that +// need a custom Message or an independent Hint (strict-mode) should +// compose CommandDeniedFromDenial + DenialDetailMap themselves. +func BuildDenialError(path string, d Denial) *output.ExitError { + cd := CommandDeniedFromDenial(path, d) + return &output.ExitError{ + Code: output.ExitValidation, + Detail: &output.ErrDetail{ + Type: "command_denied", + Message: cd.Error(), + Detail: DenialDetailMap(cd), + }, + Err: cd, + } +} // installDenyStub mutates a cobra.Command in place. Unlike cmd/prune.go // which does RemoveCommand+AddCommand (changing the pointer), we modify @@ -91,9 +130,9 @@ const AnnotationDenialSource = "lark:pruning_denied_source" // Two cobra Annotations are set as a denial signal that internal/hook // reads (without taking a dependency on this package): // -// - AnnotationDenialLayer -> "pruning" or "strict_mode" +// - AnnotationDenialLayer -> "policy" or "strict_mode" // - AnnotationDenialSource -> the PolicySource ("yaml", "plugin:foo", ...) -func installDenyStub(cmd *cobra.Command, path string, d policydecision.Denial) { +func installDenyStub(cmd *cobra.Command, path string, d Denial) { // strict-mode wins over user-layer pruning. If the command was // already replaced by a strict-mode stub (cmd/prune.go::strictModeStubFrom // writes layer=strict_mode), do NOT overwrite -- the user-layer @@ -102,9 +141,9 @@ func installDenyStub(cmd *cobra.Command, path string, d policydecision.Denial) { // Behaviour without this guard (pre-fix): a user yaml rule matching // a strict-mode stub's path would replace the RunE with the pruning // denyStub, hiding the original strict-mode error message AND - // re-labelling detail.layer from "strict_mode" to "pruning". + // re-labelling detail.layer from "strict_mode" to "policy". if cmd.Annotations != nil && - cmd.Annotations[AnnotationDenialLayer] == policydecision.LayerStrictMode { + cmd.Annotations[AnnotationDenialLayer] == LayerStrictMode { return } cmd.Hidden = true @@ -146,33 +185,10 @@ func installDenyStub(cmd *cobra.Command, path string, d policydecision.Denial) { denial := d // capture by value for the closure cmd.RunE = func(c *cobra.Command, args []string) error { - cd := &platform.CommandDeniedError{ - Path: path, - Layer: denial.Layer, - PolicySource: denial.PolicySource, - RuleName: denial.RuleName, - ReasonCode: denial.ReasonCode, - Reason: denial.Reason, - } // error.type is the user-facing semantic ("a command was denied by // policy"). detail.layer carries the implementation distinction - // ("pruning" vs "strict_mode") for debugging. - return &output.ExitError{ - Code: output.ExitValidation, - Detail: &output.ErrDetail{ - Type: "command_denied", - Message: cd.Error(), - Detail: map[string]any{ - "path": cd.Path, - "layer": cd.Layer, - "policy_source": cd.PolicySource, - "rule_name": cd.RuleName, - "reason_code": cd.ReasonCode, - "reason": cd.Reason, - }, - }, - Err: cd, // preserved for errors.As-style consumers - } + // ("policy" vs "strict_mode") for debugging. + return BuildDenialError(path, denial) } // Clear any pre-existing Run hook: cobra prefers RunE when both are // set, but leaving a stale Run around is a foot-gun for future diff --git a/internal/cmdpolicy/denial.go b/internal/cmdpolicy/denial.go new file mode 100644 index 00000000..3411984d --- /dev/null +++ b/internal/cmdpolicy/denial.go @@ -0,0 +1,130 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package cmdpolicy + +import "sort" + +// Layer values match CommandDeniedError.Layer and the detail.layer +// field of the JSON envelope (under error.type = "command_denied"). +const ( + LayerStrictMode = "strict_mode" + // LayerPolicy is the user-layer enforcement label. The string value + // is "policy" — the package name "cmdpolicy" matches it. This + // replaces the older "pruning" label. + LayerPolicy = "policy" +) + +// Denial is the merged record for a single rejected command path. It +// is distinct from the user-layer-only Decision type: Denial only +// exists when the command is rejected (the Allowed bool would be +// wasted here, hence not reusing Decision). +type Denial struct { + Layer string // "strict_mode" | "policy" + PolicySource string // "plugin:secaudit" | "yaml:mywork" | "strict-mode" | "" + RuleName string // matched Rule.Name (if any) + ReasonCode string // closed enum, see docs/extension/reason-codes.md + Reason string // human-readable +} + +// ChildDenial is what AggregateChildren consumes — it pairs a Denial +// with the child command's path so the aggregate can carry that +// breakdown for envelope.detail.children_denied. +type ChildDenial struct { + Path string + Denial Denial +} + +// AggregateChildren produces the parent-group Denial when every child +// of a command group is itself denied. The rules: +// +// - all children share Layer "strict_mode" → parent Layer = +// strict_mode, parent ReasonCode = single child's ReasonCode (if +// consistent) or "mixed_children_strict_mode" otherwise. +// - all children share Layer "policy" → parent Layer = policy, +// ReasonCode behaves analogously. +// - mixed layers across children → parent Layer = "policy", +// ReasonCode = "all_children_denied", PolicySource = "mixed". +// +// Calling with an empty slice returns a zero Denial — callers should +// treat this as "no aggregation needed". +func AggregateChildren(children []ChildDenial) Denial { + if len(children) == 0 { + return Denial{} + } + + layers := map[string]struct{}{} + reasonCodes := map[string]struct{}{} + sources := map[string]struct{}{} + ruleNames := map[string]struct{}{} + for _, c := range children { + layers[c.Denial.Layer] = struct{}{} + reasonCodes[c.Denial.ReasonCode] = struct{}{} + if c.Denial.PolicySource != "" { + sources[c.Denial.PolicySource] = struct{}{} + } + if c.Denial.RuleName != "" { + ruleNames[c.Denial.RuleName] = struct{}{} + } + } + + // Mixed: layers differ across children. Parent goes to Layer=policy + // (the more "user-recoverable" of the two — swapping policy can + // flip children, swapping credential cannot). + if len(layers) > 1 { + return Denial{ + Layer: LayerPolicy, + PolicySource: "mixed", + ReasonCode: "all_children_denied", + Reason: "all child commands are denied (mixed reasons)", + } + } + + var layer string + for l := range layers { + layer = l + } + + d := Denial{Layer: layer} + + switch len(reasonCodes) { + case 1: + for rc := range reasonCodes { + d.ReasonCode = rc + } + default: + switch layer { + case LayerStrictMode: + d.ReasonCode = "mixed_children_strict_mode" + default: + d.ReasonCode = "mixed_children_policy" + } + } + + if len(sources) == 1 { + for s := range sources { + d.PolicySource = s + } + } + if layer == LayerStrictMode { + d.PolicySource = "strict-mode" + } + + if len(ruleNames) == 1 { + for n := range ruleNames { + d.RuleName = n + } + } + + d.Reason = "all child commands are denied" + return d +} + +// SortChildren orders children by Path. The aggregate output of +// AggregateChildren is deterministic regardless of slice order, but +// tests and the envelope's children_denied list want a stable order. +func SortChildren(children []ChildDenial) { + sort.Slice(children, func(i, j int) bool { + return children[i].Path < children[j].Path + }) +} diff --git a/internal/cmdpolicy/denial_test.go b/internal/cmdpolicy/denial_test.go new file mode 100644 index 00000000..6c66665c --- /dev/null +++ b/internal/cmdpolicy/denial_test.go @@ -0,0 +1,98 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package cmdpolicy_test + +import ( + "testing" + + "github.com/larksuite/cli/internal/cmdpolicy" +) + +func TestAggregateChildren_allSameLayerAndReason(t *testing.T) { + got := cmdpolicy.AggregateChildren([]cmdpolicy.ChildDenial{ + {Path: "docs/+update", Denial: cmdpolicy.Denial{ + Layer: cmdpolicy.LayerPolicy, PolicySource: "yaml:agent", + ReasonCode: "write_not_allowed", RuleName: "agent-policy", + }}, + {Path: "docs/+delete", Denial: cmdpolicy.Denial{ + Layer: cmdpolicy.LayerPolicy, PolicySource: "yaml:agent", + ReasonCode: "write_not_allowed", RuleName: "agent-policy", + }}, + }) + if got.Layer != cmdpolicy.LayerPolicy || got.ReasonCode != "write_not_allowed" { + t.Fatalf("got %+v, want layer=policy reason=write_not_allowed", got) + } + if got.PolicySource != "yaml:agent" || got.RuleName != "agent-policy" { + t.Fatalf("Source / RuleName should propagate when consistent, got %+v", got) + } +} + +func TestAggregateChildren_sameLayerMixedReasons(t *testing.T) { + got := cmdpolicy.AggregateChildren([]cmdpolicy.ChildDenial{ + {Denial: cmdpolicy.Denial{Layer: cmdpolicy.LayerPolicy, ReasonCode: "write_not_allowed"}}, + {Denial: cmdpolicy.Denial{Layer: cmdpolicy.LayerPolicy, ReasonCode: "domain_not_allowed"}}, + }) + if got.Layer != cmdpolicy.LayerPolicy || got.ReasonCode != "mixed_children_policy" { + t.Fatalf("got %+v, want layer=policy reason=mixed_children_policy", got) + } +} + +func TestAggregateChildren_strictModeBranch(t *testing.T) { + got := cmdpolicy.AggregateChildren([]cmdpolicy.ChildDenial{ + {Denial: cmdpolicy.Denial{Layer: cmdpolicy.LayerStrictMode, ReasonCode: "identity_not_supported"}}, + {Denial: cmdpolicy.Denial{Layer: cmdpolicy.LayerStrictMode, ReasonCode: "identity_not_supported"}}, + }) + if got.Layer != cmdpolicy.LayerStrictMode || got.ReasonCode != "identity_not_supported" { + t.Fatalf("got %+v", got) + } + if got.PolicySource != "strict-mode" { + t.Fatalf("PolicySource = %q, want strict-mode", got.PolicySource) + } +} + +// Mixed layers (some strict_mode, some policy) collapse to Layer=policy +// per the design rule — a parent group failing for "both" reasons is +// most actionable framed as a user-policy issue (swappable) rather than +// a credential capability one (not swappable). +func TestAggregateChildren_mixedLayersFallsToPolicy(t *testing.T) { + got := cmdpolicy.AggregateChildren([]cmdpolicy.ChildDenial{ + {Path: "docs/+update", Denial: cmdpolicy.Denial{ + Layer: cmdpolicy.LayerStrictMode, ReasonCode: "identity_not_supported", + }}, + {Path: "docs/+fetch", Denial: cmdpolicy.Denial{ + Layer: cmdpolicy.LayerPolicy, ReasonCode: "domain_not_allowed", + }}, + }) + if got.Layer != cmdpolicy.LayerPolicy { + t.Fatalf("Layer = %q, want policy (mixed-children rule)", got.Layer) + } + if got.ReasonCode != "all_children_denied" { + t.Fatalf("ReasonCode = %q, want all_children_denied", got.ReasonCode) + } + if got.PolicySource != "mixed" { + t.Fatalf("PolicySource = %q, want mixed", got.PolicySource) + } +} + +func TestAggregateChildren_emptySlice(t *testing.T) { + got := cmdpolicy.AggregateChildren(nil) + if (got != cmdpolicy.Denial{}) { + t.Fatalf("empty slice should produce zero Denial, got %+v", got) + } +} + +func TestSortChildren_stableOrder(t *testing.T) { + children := []cmdpolicy.ChildDenial{ + {Path: "docs/+update"}, + {Path: "docs/+delete"}, + {Path: "docs/+create"}, + } + cmdpolicy.SortChildren(children) + want := []string{"docs/+create", "docs/+delete", "docs/+update"} + for i, c := range children { + if c.Path != want[i] { + t.Fatalf("children[%d].Path = %q, want %q", i, c.Path, want[i]) + } + } +} diff --git a/internal/pruning/diagnostic.go b/internal/cmdpolicy/diagnostic.go similarity index 98% rename from internal/pruning/diagnostic.go rename to internal/cmdpolicy/diagnostic.go index 49aedd8a..9e8f4825 100644 --- a/internal/pruning/diagnostic.go +++ b/internal/cmdpolicy/diagnostic.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning +package cmdpolicy // diagnosticPaths lists command paths that are unconditionally allowed, // regardless of any user-layer Rule. Entries must satisfy two properties: diff --git a/internal/pruning/diagnostic_test.go b/internal/cmdpolicy/diagnostic_test.go similarity index 93% rename from internal/pruning/diagnostic_test.go rename to internal/cmdpolicy/diagnostic_test.go index 46997713..48843ed7 100644 --- a/internal/pruning/diagnostic_test.go +++ b/internal/cmdpolicy/diagnostic_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning_test +package cmdpolicy_test import ( "testing" @@ -9,7 +9,7 @@ import ( "github.com/spf13/cobra" "github.com/larksuite/cli/extension/platform" - "github.com/larksuite/cli/internal/pruning" + "github.com/larksuite/cli/internal/cmdpolicy" ) // configPolicyTree builds the minimal slice of the real command tree @@ -33,7 +33,7 @@ func configPolicyTree() *cobra.Command { func TestEvaluate_diagnosticAllowedDespiteStrictAllow(t *testing.T) { root := configPolicyTree() // Rule that allows ONLY docs/** -- normally locks out everything else. - e := pruning.New(&platform.Rule{ + e := cmdpolicy.New(&platform.Rule{ Allow: []string{"docs/**"}, }) got := e.EvaluateAll(root) @@ -59,7 +59,7 @@ func TestEvaluate_diagnosticAllowedDespiteExplicitDeny(t *testing.T) { // sensitive deployment needs to block introspection, they should // strip the binary, not rely on Rule. root := configPolicyTree() - e := pruning.New(&platform.Rule{ + e := cmdpolicy.New(&platform.Rule{ Allow: []string{"**"}, Deny: []string{"config/policy/**"}, }) @@ -85,7 +85,7 @@ func TestIsDiagnosticPath(t *testing.T) { {"", false}, } for _, tc := range cases { - if got := pruning.IsDiagnosticPath(tc.path); got != tc.want { + if got := cmdpolicy.IsDiagnosticPath(tc.path); got != tc.want { t.Errorf("IsDiagnosticPath(%q) = %v, want %v", tc.path, got, tc.want) } } diff --git a/internal/pruning/engine.go b/internal/cmdpolicy/engine.go similarity index 77% rename from internal/pruning/engine.go rename to internal/cmdpolicy/engine.go index 7311f712..b5ad70e4 100644 --- a/internal/pruning/engine.go +++ b/internal/cmdpolicy/engine.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -// Package pruning is the user-layer command pruning engine. It consumes a +// Package cmdpolicy is the user-layer command policy engine. It consumes a // platform.Rule and the cobra command tree, evaluates each runnable command // against the rule's four-axis filter (Allow / Deny / MaxRisk / Identities), // and produces a path -> Decision map. A separate BuildDeniedByPath step @@ -9,23 +9,24 @@ // aggregation), which the Apply step consumes to install denyStubs. // // This package only implements the user-layer half. Strict-mode is handled -// by cmd/prune.go::applyStrictModeDenials, which consumes the same merged -// deniedByPath produced by the bootstrap pipeline. The two layers share -// the decision-map data structure (internal/policydecision.Denial) but -// keep distinct apply functions so error.type stays accurate. -package pruning +// by cmd/prune.go, which produces command_denied envelopes of the same +// shape via BuildDenialError so external agents can dispatch on +// detail.layer / reason_code uniformly regardless of which layer rejected +// the call. +package cmdpolicy import ( + "fmt" + "github.com/bmatcuk/doublestar/v4" "github.com/spf13/cobra" "github.com/larksuite/cli/extension/platform" "github.com/larksuite/cli/internal/cmdmeta" - "github.com/larksuite/cli/internal/policydecision" ) // Decision is the user-layer single-rule evaluation result. Distinct from -// policydecision.Denial: Decision carries Allowed=true/false and the +// Denial: Decision carries Allowed=true/false and the // rejection reason when Allowed=false; Denial only ever exists when the // command is rejected. Keeping them separate avoids a perpetually-false // Allowed field on Denial. @@ -86,23 +87,33 @@ func (e *Engine) EvaluateOne(cmd *cobra.Command) Decision { // A registered Rule expresses intent over the closed risk taxonomy // (read / write / high-risk-write). Two ways a command can fall - // outside that taxonomy -- both are fail-closed before any other - // axis runs, so an unreasoned command never slips past an - // "agent read-only" rule. - cmdRisk, hasRisk := cmdmeta.Risk(cmd) - if !hasRisk { + // outside that taxonomy: + // + // - "absent" (no risk_level annotation) — fail-closed by default, + // but Rule.AllowUnannotated=true opts out for gradual adoption. + // - "invalid" (annotation exists but is a typo / not in the + // closed enum) — always fail-closed regardless of + // AllowUnannotated. Typo is a code bug, not a migration phase. + cmdRiskStr, hasRisk := cmdmeta.Risk(cmd) + cmdRisk := platform.Risk(cmdRiskStr) + var ( + cmdRank int + cmdRankOk bool + ) + if hasRisk { + cmdRank, cmdRankOk = cmdRisk.Rank() + if !cmdRankOk { + return Decision{ + Allowed: false, + ReasonCode: "risk_invalid", + Reason: fmt.Sprintf("invalid risk %q; did you mean %q?", cmdRiskStr, suggestRisk(cmdRiskStr)), + } + } + } else if !r.AllowUnannotated { return Decision{ Allowed: false, ReasonCode: "risk_not_annotated", - Reason: "command has no risk annotation; required when a pruning rule is active", - } - } - cmdRank, cmdRankOk := platform.RiskRank(cmdRisk) - if !cmdRankOk { - return Decision{ - Allowed: false, - ReasonCode: "risk_invalid", - Reason: "command has invalid risk annotation; must be one of read|write|high-risk-write", + Reason: "command has no risk_level annotation; required when a Rule is active (set rule.allow_unannotated=true to opt out during gradual adoption)", } } @@ -124,9 +135,11 @@ func (e *Engine) EvaluateOne(cmd *cobra.Command) Decision { } } - // Axis 3: MaxRisk. - if r.MaxRisk != "" { - if limit, limitOk := platform.RiskRank(r.MaxRisk); limitOk && cmdRank > limit { + // Axis 3: MaxRisk. Skipped when cmd risk is absent + AllowUnannotated: + // the engine has no rank to compare against, and AllowUnannotated + // is the explicit "allow this through" opt-in. + if r.MaxRisk != "" && cmdRankOk { + if limit, limitOk := r.MaxRisk.Rank(); limitOk && cmdRank > limit { return Decision{ Allowed: false, ReasonCode: reasonCodeForRisk(cmdRisk), @@ -138,7 +151,7 @@ func (e *Engine) EvaluateOne(cmd *cobra.Command) Decision { // Axis 4: Identities. Unknown command identities is treated as ALLOW. if len(r.Identities) > 0 { cmdIdents := cmdmeta.Identities(cmd) - if cmdIdents != nil && !hasIntersection(r.Identities, cmdIdents) { + if cmdIdents != nil && !hasIdentityIntersection(r.Identities, cmdIdents) { return Decision{ Allowed: false, ReasonCode: "identity_mismatch", @@ -153,7 +166,7 @@ func (e *Engine) EvaluateOne(cmd *cobra.Command) Decision { // BuildDeniedByPath converts engine Decisions to a deniedByPath map keyed // by canonical path. It performs the parent-group aggregation defined in // the tech doc: a non-runnable parent whose every runnable descendant is -// denied gets an aggregate denial (via policydecision.AggregateChildren); +// denied gets an aggregate denial (via AggregateChildren); // hybrid commands (own RunE + children) get one only when both their own // RunE and all children are denied. // @@ -163,14 +176,14 @@ func (e *Engine) EvaluateOne(cmd *cobra.Command) Decision { // // source / ruleName populate PolicySource and RuleName on the produced // Denial values, so envelope output can attribute denials. -func BuildDeniedByPath(root *cobra.Command, decisions map[string]Decision, source ResolveSource, ruleName string) map[string]policydecision.Denial { - out := map[string]policydecision.Denial{} +func BuildDeniedByPath(root *cobra.Command, decisions map[string]Decision, source ResolveSource, ruleName string) map[string]Denial { + out := map[string]Denial{} sourceLabel := policySourceLabel(source) for path, d := range decisions { if !d.Allowed { - out[path] = policydecision.Denial{ - Layer: policydecision.LayerPruning, + out[path] = Denial{ + Layer: LayerPolicy, PolicySource: sourceLabel, RuleName: ruleName, ReasonCode: d.ReasonCode, @@ -192,7 +205,7 @@ func BuildDeniedByPath(root *cobra.Command, decisions map[string]Decision, sourc // "Live" children are those with at least one runnable descendant; pure // non-runnable placeholders neither count toward "all denied" nor block // the aggregation. -func aggregateParents(cmd *cobra.Command, denied map[string]policydecision.Denial) bool { +func aggregateParents(cmd *cobra.Command, denied map[string]Denial) bool { if cmd == nil { return false } @@ -212,7 +225,7 @@ func aggregateParents(cmd *cobra.Command, denied map[string]policydecision.Denia // Has children: recurse first, collect direct-child denials for the // aggregation message. - childDenials := make([]policydecision.ChildDenial, 0, len(children)) + childDenials := make([]ChildDenial, 0, len(children)) liveChildSeen := false allLiveChildrenDenied := true for _, child := range children { @@ -225,7 +238,7 @@ func aggregateParents(cmd *cobra.Command, denied map[string]policydecision.Denia } if cp := CanonicalPath(child); cp != "" { if d, ok := denied[cp]; ok { - childDenials = append(childDenials, policydecision.ChildDenial{Path: cp, Denial: d}) + childDenials = append(childDenials, ChildDenial{Path: cp, Denial: d}) } } } @@ -251,8 +264,8 @@ func aggregateParents(cmd *cobra.Command, denied map[string]policydecision.Denia // skip the binary root. if cmd.HasParent() && cmdPath != "" { if _, exists := denied[cmdPath]; !exists { - policydecision.SortChildren(childDenials) - denied[cmdPath] = policydecision.AggregateChildren(childDenials) + SortChildren(childDenials) + denied[cmdPath] = AggregateChildren(childDenials) } } return true @@ -320,13 +333,14 @@ func matchesAny(globs []string, path string) bool { return false } -// hasIntersection reports whether two string slices share any element. -// Both slices are short (usually 1-2 identities) so a nested loop beats -// allocating a set. -func hasIntersection(a, b []string) bool { - for _, x := range a { - for _, y := range b { - if x == y { +// hasIdentityIntersection reports whether the rule's typed identities +// share any value with the command's raw identity strings. Both slices +// are short (usually 1-2 identities) so a nested loop beats allocating +// a set. +func hasIdentityIntersection(rule []platform.Identity, cmd []string) bool { + for _, x := range rule { + for _, y := range cmd { + if string(x) == y { return true } } diff --git a/internal/pruning/engine_test.go b/internal/cmdpolicy/engine_test.go similarity index 73% rename from internal/pruning/engine_test.go rename to internal/cmdpolicy/engine_test.go index c66b0b17..c102b58f 100644 --- a/internal/pruning/engine_test.go +++ b/internal/cmdpolicy/engine_test.go @@ -1,19 +1,19 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning_test +package cmdpolicy_test import ( "errors" + "strings" "testing" "github.com/spf13/cobra" "github.com/larksuite/cli/extension/platform" "github.com/larksuite/cli/internal/cmdmeta" + "github.com/larksuite/cli/internal/cmdpolicy" "github.com/larksuite/cli/internal/cmdutil" - "github.com/larksuite/cli/internal/policydecision" - "github.com/larksuite/cli/internal/pruning" ) // buildTree assembles a tiny realistic tree for engine tests: @@ -62,7 +62,7 @@ func noop(*cobra.Command, []string) error { return nil } func TestEvaluate_nilRuleAllowsAll(t *testing.T) { root := buildTree() - got := pruning.New(nil).EvaluateAll(root) + got := cmdpolicy.New(nil).EvaluateAll(root) for path, d := range got { if !d.Allowed { t.Fatalf("nil rule should allow all, got Allowed=false for %s", path) @@ -72,7 +72,7 @@ func TestEvaluate_nilRuleAllowsAll(t *testing.T) { func TestEvaluate_allowGlob(t *testing.T) { root := buildTree() - e := pruning.New(&platform.Rule{ + e := cmdpolicy.New(&platform.Rule{ Allow: []string{"docs/**"}, }) got := e.EvaluateAll(root) @@ -91,7 +91,7 @@ func TestEvaluate_allowGlob(t *testing.T) { func TestEvaluate_denyTakesPriorityOverAllow(t *testing.T) { root := buildTree() - e := pruning.New(&platform.Rule{ + e := cmdpolicy.New(&platform.Rule{ Allow: []string{"docs/**"}, Deny: []string{"docs/+delete-doc"}, }) @@ -111,7 +111,7 @@ func TestEvaluate_denyTakesPriorityOverAllow(t *testing.T) { func TestEvaluate_maxRiskCutoff(t *testing.T) { root := buildTree() - e := pruning.New(&platform.Rule{ + e := cmdpolicy.New(&platform.Rule{ MaxRisk: "write", // allow read+write, deny high-risk-write }) got := e.EvaluateAll(root) @@ -144,7 +144,7 @@ func TestEvaluate_unannotatedRiskIsDeny(t *testing.T) { docs.AddCommand(orphan) // Rule without MaxRisk still triggers the implicit deny. - e := pruning.New(&platform.Rule{Allow: []string{"docs/**"}}) + e := cmdpolicy.New(&platform.Rule{Allow: []string{"docs/**"}}) got := e.EvaluateAll(root) if got["docs/+orphan"].Allowed { t.Fatalf("unannotated risk must be denied when a Rule is registered") @@ -155,7 +155,7 @@ func TestEvaluate_unannotatedRiskIsDeny(t *testing.T) { // And with MaxRisk it still uses risk_not_annotated (the missing- // annotation gate runs before the MaxRisk axis). - e = pruning.New(&platform.Rule{MaxRisk: "read"}) + e = cmdpolicy.New(&platform.Rule{MaxRisk: "read"}) got = e.EvaluateAll(root) if got["docs/+orphan"].ReasonCode != "risk_not_annotated" { t.Errorf("ReasonCode under MaxRisk = %q, want risk_not_annotated", got["docs/+orphan"].ReasonCode) @@ -165,7 +165,7 @@ func TestEvaluate_unannotatedRiskIsDeny(t *testing.T) { // triggers the implicit deny. "any registered Rule = enter the safety // boundary" is the design contract; pin it so future edits cannot // silently weaken it. - e = pruning.New(&platform.Rule{}) + e = cmdpolicy.New(&platform.Rule{}) got = e.EvaluateAll(root) if got["docs/+orphan"].Allowed { t.Fatalf("empty Rule{} must still deny unannotated commands") @@ -175,14 +175,85 @@ func TestEvaluate_unannotatedRiskIsDeny(t *testing.T) { } // Without any Rule, unannotated commands are still allowed (no - // pruning engine is invoked when no plugin registers a Rule). - e = pruning.New(nil) + // policy engine is invoked when no plugin registers a Rule). + e = cmdpolicy.New(nil) got = e.EvaluateAll(root) if !got["docs/+orphan"].Allowed { t.Fatalf("nil Rule must allow unannotated commands (no main-flow impact)") } } +// AllowUnannotated=true opts out of the "unannotated = deny" rule for +// gradual adoption. The flag does NOT loosen any other axis: Deny still +// rejects, MaxRisk is skipped (no rank to compare), Allow/Identities still +// apply. +func TestEvaluate_allowUnannotatedOptsOutOfDeny(t *testing.T) { + root := &cobra.Command{Use: "lark-cli"} + docs := &cobra.Command{Use: "docs"} + root.AddCommand(docs) + orphan := &cobra.Command{Use: "+orphan", RunE: noop} + docs.AddCommand(orphan) + + // Without opt-in: still denied + e := cmdpolicy.New(&platform.Rule{Allow: []string{"docs/**"}}) + if got := e.EvaluateAll(root); got["docs/+orphan"].Allowed { + t.Fatalf("default behaviour must deny unannotated; AllowUnannotated should be opt-in") + } + + // With opt-in: allowed + e = cmdpolicy.New(&platform.Rule{ + Allow: []string{"docs/**"}, + AllowUnannotated: true, + }) + got := e.EvaluateAll(root) + if !got["docs/+orphan"].Allowed { + t.Fatalf("AllowUnannotated=true must allow unannotated commands; got %+v", got["docs/+orphan"]) + } + + // AllowUnannotated does NOT bypass Deny: an unannotated command + // hitting a Deny glob is still rejected. + e = cmdpolicy.New(&platform.Rule{ + Deny: []string{"docs/+orphan"}, + AllowUnannotated: true, + }) + got = e.EvaluateAll(root) + if got["docs/+orphan"].Allowed { + t.Fatalf("AllowUnannotated must not bypass Deny; got %+v", got["docs/+orphan"]) + } + if got["docs/+orphan"].ReasonCode != "command_denylisted" { + t.Errorf("ReasonCode under Deny+AllowUnannotated = %q, want command_denylisted", + got["docs/+orphan"].ReasonCode) + } +} + +// risk_invalid (typo) is unaffected by AllowUnannotated and emits a +// "did you mean" suggestion in the reason text. +func TestEvaluate_invalidRiskAlwaysDeny_andSuggests(t *testing.T) { + root := &cobra.Command{Use: "lark-cli"} + docs := &cobra.Command{Use: "docs"} + root.AddCommand(docs) + typo := &cobra.Command{Use: "+typo", RunE: noop} + cmdutil.SetRisk(typo, "wrtie") + docs.AddCommand(typo) + + // AllowUnannotated=true must NOT bypass risk_invalid — typo is a + // code bug, not a missing annotation. + e := cmdpolicy.New(&platform.Rule{ + MaxRisk: "read", + AllowUnannotated: true, + }) + got := e.EvaluateAll(root) + if got["docs/+typo"].Allowed { + t.Fatalf("AllowUnannotated must not bypass risk_invalid; got %+v", got["docs/+typo"]) + } + if got["docs/+typo"].ReasonCode != "risk_invalid" { + t.Errorf("ReasonCode = %q, want risk_invalid", got["docs/+typo"].ReasonCode) + } + if !strings.Contains(got["docs/+typo"].Reason, "write") { + t.Errorf("Reason should contain suggestion 'write', got %q", got["docs/+typo"].Reason) + } +} + // Invalid risk annotations (typos like "wrtie" or anything outside the // read|write|high-risk-write taxonomy) are denied with reason_code // "risk_invalid". Without this gate they used to pass the MaxRisk axis @@ -197,7 +268,7 @@ func TestEvaluate_invalidRiskIsDeny(t *testing.T) { docs.AddCommand(typo) // Even under MaxRisk=read the typo command must not slip through. - e := pruning.New(&platform.Rule{MaxRisk: "read"}) + e := cmdpolicy.New(&platform.Rule{MaxRisk: "read"}) got := e.EvaluateAll(root) if got["docs/+typo"].Allowed { t.Fatalf("invalid risk must be denied under MaxRisk=read, got allowed") @@ -208,7 +279,7 @@ func TestEvaluate_invalidRiskIsDeny(t *testing.T) { // Same when no MaxRisk is set -- the taxonomy check runs unconditionally // once a Rule is present. - e = pruning.New(&platform.Rule{Allow: []string{"docs/**"}}) + e = cmdpolicy.New(&platform.Rule{Allow: []string{"docs/**"}}) got = e.EvaluateAll(root) if got["docs/+typo"].ReasonCode != "risk_invalid" { t.Errorf("ReasonCode without MaxRisk = %q, want risk_invalid", got["docs/+typo"].ReasonCode) @@ -217,7 +288,7 @@ func TestEvaluate_invalidRiskIsDeny(t *testing.T) { // The risk_invalid gate must fire BEFORE Deny matching, otherwise a // typo command landing in the deny list would surface as // command_denylisted and mask the underlying taxonomy violation. - e = pruning.New(&platform.Rule{Deny: []string{"docs/+typo"}}) + e = cmdpolicy.New(&platform.Rule{Deny: []string{"docs/+typo"}}) got = e.EvaluateAll(root) if got["docs/+typo"].ReasonCode != "risk_invalid" { t.Errorf("ReasonCode under Deny match = %q, want risk_invalid (taxonomy gate must precede Deny)", got["docs/+typo"].ReasonCode) @@ -225,7 +296,7 @@ func TestEvaluate_invalidRiskIsDeny(t *testing.T) { // Without any Rule, invalid risk is not policed (same main-flow // no-impact rule as risk_not_annotated). - e = pruning.New(nil) + e = cmdpolicy.New(nil) got = e.EvaluateAll(root) if !got["docs/+typo"].Allowed { t.Fatalf("nil Rule must allow invalid risk (no main-flow impact)") @@ -234,8 +305,8 @@ func TestEvaluate_invalidRiskIsDeny(t *testing.T) { func TestEvaluate_identitiesIntersection(t *testing.T) { root := buildTree() - e := pruning.New(&platform.Rule{ - Identities: []string{"bot"}, // bot-only rule + e := cmdpolicy.New(&platform.Rule{ + Identities: []platform.Identity{"bot"}, // bot-only rule }) got := e.EvaluateAll(root) @@ -262,24 +333,24 @@ func TestEvaluate_unknownIdentitiesIsAllow(t *testing.T) { root.AddCommand(cmd) // no SetSupportedIdentities - e := pruning.New(&platform.Rule{Identities: []string{"bot"}}) + e := cmdpolicy.New(&platform.Rule{Identities: []platform.Identity{"bot"}}) got := e.EvaluateAll(root) if !got["+x"].Allowed { t.Fatalf("unknown identities must pass any identity rule") } } -// Apply must install denyStubs only on Layer="pruning" entries. A +// Apply must install denyStubs only on Layer="policy" entries. A // "strict_mode" denial in the same map must be left for // applyStrictModeDenials in cmd/. func TestApply_onlyTouchesPruningLayer(t *testing.T) { root := buildTree() - denied := map[string]policydecision.Denial{ - "docs/+update": {Layer: "pruning", ReasonCode: "write_not_allowed"}, + denied := map[string]cmdpolicy.Denial{ + "docs/+update": {Layer: "policy", ReasonCode: "write_not_allowed"}, "docs/+fetch": {Layer: "strict_mode", ReasonCode: "identity_not_supported"}, } - count := pruning.Apply(root, denied) + count := cmdpolicy.Apply(root, denied) if count != 1 { t.Fatalf("Apply count = %d, want 1 (only pruning-layer entries)", count) } @@ -295,7 +366,7 @@ func TestApply_onlyTouchesPruningLayer(t *testing.T) { // strict-mode entry must NOT have been touched here. fetch := findChild(t, root, "docs", "+fetch") if fetch.Hidden || fetch.DisableFlagParsing { - t.Errorf("+fetch (strict_mode layer) should NOT be touched by pruning.Apply") + t.Errorf("+fetch (strict_mode layer) should NOT be touched by cmdpolicy.Apply") } } @@ -304,9 +375,9 @@ func TestApply_onlyTouchesPruningLayer(t *testing.T) { // (agent, integration) depends on. func TestApply_runEReturnsTypedError(t *testing.T) { root := buildTree() - pruning.Apply(root, map[string]policydecision.Denial{ + cmdpolicy.Apply(root, map[string]cmdpolicy.Denial{ "docs/+update": { - Layer: "pruning", + Layer: "policy", PolicySource: "plugin:secaudit", RuleName: "secaudit-policy", ReasonCode: "write_not_allowed", @@ -323,7 +394,7 @@ func TestApply_runEReturnsTypedError(t *testing.T) { if !errors.As(err, &denied) { t.Fatalf("error should be *platform.CommandDeniedError, got %T", err) } - if denied.Layer != "pruning" || denied.ReasonCode != "write_not_allowed" { + if denied.Layer != "policy" || denied.ReasonCode != "write_not_allowed" { t.Errorf("denial = %+v, want layer=pruning code=write_not_allowed", denied) } if denied.Path != "docs/+update" { @@ -336,7 +407,7 @@ func TestApply_runEReturnsTypedError(t *testing.T) { func TestApply_emptyMapNoop(t *testing.T) { root := buildTree() - if got := pruning.Apply(root, nil); got != 0 { + if got := cmdpolicy.Apply(root, nil); got != 0 { t.Fatalf("nil deniedByPath should yield count=0, got %d", got) } } @@ -346,10 +417,10 @@ func TestApply_emptyMapNoop(t *testing.T) { func TestCanonicalPath(t *testing.T) { root := buildTree() update := findChild(t, root, "docs", "+update") - if got := pruning.CanonicalPath(update); got != "docs/+update" { + if got := cmdpolicy.CanonicalPath(update); got != "docs/+update" { t.Fatalf("CanonicalPath = %q, want docs/+update", got) } - if got := pruning.CanonicalPath(root); got != "lark-cli" { + if got := cmdpolicy.CanonicalPath(root); got != "lark-cli" { t.Fatalf("CanonicalPath(root) = %q, want lark-cli (orphan fallback)", got) } } diff --git a/internal/pruning/path.go b/internal/cmdpolicy/path.go similarity index 98% rename from internal/pruning/path.go rename to internal/cmdpolicy/path.go index 64681ced..6ce4f198 100644 --- a/internal/pruning/path.go +++ b/internal/cmdpolicy/path.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning +package cmdpolicy import ( "strings" diff --git a/internal/pruning/resolver.go b/internal/cmdpolicy/resolver.go similarity index 98% rename from internal/pruning/resolver.go rename to internal/cmdpolicy/resolver.go index b2e91228..29309709 100644 --- a/internal/pruning/resolver.go +++ b/internal/cmdpolicy/resolver.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning +package cmdpolicy import ( "errors" @@ -9,7 +9,7 @@ import ( "os" "github.com/larksuite/cli/extension/platform" - pyaml "github.com/larksuite/cli/internal/pruning/yaml" + pyaml "github.com/larksuite/cli/internal/cmdpolicy/yaml" "github.com/larksuite/cli/internal/vfs" ) diff --git a/internal/pruning/resolver_test.go b/internal/cmdpolicy/resolver_test.go similarity index 70% rename from internal/pruning/resolver_test.go rename to internal/cmdpolicy/resolver_test.go index ca16f1cd..7209eb27 100644 --- a/internal/pruning/resolver_test.go +++ b/internal/cmdpolicy/resolver_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning_test +package cmdpolicy_test import ( "errors" @@ -10,18 +10,18 @@ import ( "testing" "github.com/larksuite/cli/extension/platform" - "github.com/larksuite/cli/internal/pruning" + "github.com/larksuite/cli/internal/cmdpolicy" ) func TestResolve_singlePluginWins(t *testing.T) { rule := &platform.Rule{Name: "secaudit"} - got, src, err := pruning.Resolve([]pruning.PluginRule{ + got, src, err := cmdpolicy.Resolve([]cmdpolicy.PluginRule{ {PluginName: "secaudit", Rule: rule}, }, "") if err != nil { t.Fatalf("Resolve err: %v", err) } - if got != rule || src.Kind != pruning.SourcePlugin || src.Name != "secaudit" { + if got != rule || src.Kind != cmdpolicy.SourcePlugin || src.Name != "secaudit" { t.Fatalf("Resolve = (%v, %+v)", got, src) } } @@ -34,14 +34,14 @@ func TestResolve_pluginShadowsYaml(t *testing.T) { } pluginRule := &platform.Rule{Name: "from-plugin"} - got, src, err := pruning.Resolve( - []pruning.PluginRule{{PluginName: "secaudit", Rule: pluginRule}}, + got, src, err := cmdpolicy.Resolve( + []cmdpolicy.PluginRule{{PluginName: "secaudit", Rule: pluginRule}}, yamlPath, ) if err != nil { t.Fatalf("Resolve err: %v", err) } - if got.Name != "from-plugin" || src.Kind != pruning.SourcePlugin { + if got.Name != "from-plugin" || src.Kind != cmdpolicy.SourcePlugin { t.Fatalf("plugin should shadow yaml, got %+v / %+v", got, src) } } @@ -53,21 +53,21 @@ func TestResolve_yamlWhenNoPlugin(t *testing.T) { t.Fatalf("write yaml: %v", err) } - got, src, err := pruning.Resolve(nil, yamlPath) + got, src, err := cmdpolicy.Resolve(nil, yamlPath) if err != nil { t.Fatalf("Resolve err: %v", err) } - if got.Name != "from-yaml" || src.Kind != pruning.SourceYAML { + if got.Name != "from-yaml" || src.Kind != cmdpolicy.SourceYAML { t.Fatalf("yaml should win when no plugin, got %+v / %+v", got, src) } } func TestResolve_missingYamlIsNoRule(t *testing.T) { - got, src, err := pruning.Resolve(nil, "/nonexistent/policy.yml") + got, src, err := cmdpolicy.Resolve(nil, "/nonexistent/policy.yml") if err != nil { t.Fatalf("missing yaml should not error, got %v", err) } - if got != nil || src.Kind != pruning.SourceNone { + if got != nil || src.Kind != cmdpolicy.SourceNone { t.Fatalf("expected (nil, SourceNone), got (%v, %+v)", got, src) } } @@ -75,21 +75,21 @@ func TestResolve_missingYamlIsNoRule(t *testing.T) { // Two plugins both contributing a Rule must produce the typed error so the // bootstrap pipeline aborts (hard-constraint #7). func TestResolve_multipleRestrictIsError(t *testing.T) { - _, _, err := pruning.Resolve([]pruning.PluginRule{ + _, _, err := cmdpolicy.Resolve([]cmdpolicy.PluginRule{ {PluginName: "a", Rule: &platform.Rule{Name: "a"}}, {PluginName: "b", Rule: &platform.Rule{Name: "b"}}, }, "") - if !errors.Is(err, pruning.ErrMultipleRestricts) { + if !errors.Is(err, cmdpolicy.ErrMultipleRestricts) { t.Fatalf("err = %v, want ErrMultipleRestricts", err) } } func TestResolve_emptyEverythingIsNone(t *testing.T) { - got, src, err := pruning.Resolve(nil, "") + got, src, err := cmdpolicy.Resolve(nil, "") if err != nil { t.Fatalf("Resolve err: %v", err) } - if got != nil || src.Kind != pruning.SourceNone { + if got != nil || src.Kind != cmdpolicy.SourceNone { t.Fatalf("expected (nil, SourceNone), got (%v, %+v)", got, src) } } diff --git a/internal/pruning/source_label_test.go b/internal/cmdpolicy/source_label_test.go similarity index 85% rename from internal/pruning/source_label_test.go rename to internal/cmdpolicy/source_label_test.go index 60b69f83..dbd31d56 100644 --- a/internal/pruning/source_label_test.go +++ b/internal/cmdpolicy/source_label_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning_test +package cmdpolicy_test import ( "errors" @@ -11,8 +11,8 @@ import ( "github.com/spf13/cobra" "github.com/larksuite/cli/extension/platform" + "github.com/larksuite/cli/internal/cmdpolicy" "github.com/larksuite/cli/internal/output" - "github.com/larksuite/cli/internal/pruning" ) // The envelope's policy_source must never leak the absolute home path. @@ -26,17 +26,17 @@ func TestEnvelope_yamlPolicySourceDoesNotLeakHomePath(t *testing.T) { leaf := &cobra.Command{Use: "+write", RunE: func(*cobra.Command, []string) error { return nil }} docs.AddCommand(leaf) - e := pruning.New(&platform.Rule{ + e := cmdpolicy.New(&platform.Rule{ Name: "my-readonly-rule", Allow: []string{"contact/**"}, // docs/* falls outside, denied }) - denied := pruning.BuildDeniedByPath(root, e.EvaluateAll(root), - pruning.ResolveSource{ - Kind: pruning.SourceYAML, + denied := cmdpolicy.BuildDeniedByPath(root, e.EvaluateAll(root), + cmdpolicy.ResolveSource{ + Kind: cmdpolicy.SourceYAML, Name: "/Users/alice/.lark-cli/policy.yml", // simulate an absolute path }, "my-readonly-rule") - pruning.Apply(root, denied) + cmdpolicy.Apply(root, denied) err := leaf.RunE(leaf, nil) var exitErr *output.ExitError @@ -70,14 +70,14 @@ func TestEnvelope_pluginPolicySourceCarriesName(t *testing.T) { leaf := &cobra.Command{Use: "+block", RunE: func(*cobra.Command, []string) error { return nil }} root.AddCommand(leaf) - e := pruning.New(&platform.Rule{ + e := cmdpolicy.New(&platform.Rule{ Name: "secaudit-policy", Deny: []string{"+block"}, }) - denied := pruning.BuildDeniedByPath(root, e.EvaluateAll(root), - pruning.ResolveSource{Kind: pruning.SourcePlugin, Name: "secaudit"}, + denied := cmdpolicy.BuildDeniedByPath(root, e.EvaluateAll(root), + cmdpolicy.ResolveSource{Kind: cmdpolicy.SourcePlugin, Name: "secaudit"}, "secaudit-policy") - pruning.Apply(root, denied) + cmdpolicy.Apply(root, denied) err := leaf.RunE(leaf, nil) var exitErr *output.ExitError diff --git a/internal/pruning/strict_mode_skip_test.go b/internal/cmdpolicy/strict_mode_skip_test.go similarity index 80% rename from internal/pruning/strict_mode_skip_test.go rename to internal/cmdpolicy/strict_mode_skip_test.go index d25148bb..90276cab 100644 --- a/internal/pruning/strict_mode_skip_test.go +++ b/internal/cmdpolicy/strict_mode_skip_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning_test +package cmdpolicy_test import ( "errors" @@ -9,11 +9,10 @@ import ( "github.com/spf13/cobra" - "github.com/larksuite/cli/internal/policydecision" - "github.com/larksuite/cli/internal/pruning" + "github.com/larksuite/cli/internal/cmdpolicy" ) -// pruning.Apply MUST NOT overwrite the denial annotation on a command +// cmdpolicy.Apply MUST NOT overwrite the denial annotation on a command // already marked as strict-mode denied. strict-mode is a hard boundary // (credential-derived); a user-layer rule cannot relabel or replace // the error path. @@ -29,29 +28,29 @@ func TestApply_PreservesStrictModeAnnotation(t *testing.T) { Use: "victim", Hidden: true, Annotations: map[string]string{ - pruning.AnnotationDenialLayer: policydecision.LayerStrictMode, - pruning.AnnotationDenialSource: "strict-mode", + cmdpolicy.AnnotationDenialLayer: cmdpolicy.LayerStrictMode, + cmdpolicy.AnnotationDenialSource: "strict-mode", }, RunE: func(*cobra.Command, []string) error { return nil }, } root.AddCommand(stub) // User-layer pruning denies the same path. - denied := map[string]policydecision.Denial{ + denied := map[string]cmdpolicy.Denial{ "victim": { - Layer: policydecision.LayerPruning, + Layer: cmdpolicy.LayerPolicy, PolicySource: "yaml", Reason: "denied by user yaml", ReasonCode: "command_denylisted", }, } - pruning.Apply(root, denied) + cmdpolicy.Apply(root, denied) - if got := stub.Annotations[pruning.AnnotationDenialLayer]; got != policydecision.LayerStrictMode { + if got := stub.Annotations[cmdpolicy.AnnotationDenialLayer]; got != cmdpolicy.LayerStrictMode { t.Errorf("strict-mode layer overwritten by pruning: got %q want %q", - got, policydecision.LayerStrictMode) + got, cmdpolicy.LayerStrictMode) } - if got := stub.Annotations[pruning.AnnotationDenialSource]; got != "strict-mode" { + if got := stub.Annotations[cmdpolicy.AnnotationDenialSource]; got != "strict-mode" { t.Errorf("strict-mode source overwritten: got %q", got) } } @@ -75,15 +74,15 @@ func TestApply_DenyStubBypassesArgsValidator(t *testing.T) { } root.AddCommand(leaf) - denied := map[string]policydecision.Denial{ + denied := map[string]cmdpolicy.Denial{ "+update": { - Layer: policydecision.LayerPruning, + Layer: cmdpolicy.LayerPolicy, PolicySource: "yaml", ReasonCode: "command_denylisted", Reason: "denied by user yaml", }, } - pruning.Apply(root, denied) + cmdpolicy.Apply(root, denied) if leaf.Args == nil { t.Fatal("denied command must have non-nil Args validator after Apply") @@ -119,15 +118,15 @@ func TestApply_DenyStubBypassesParentPersistentPreRunE(t *testing.T) { } parent.AddCommand(leaf) - denied := map[string]policydecision.Denial{ + denied := map[string]cmdpolicy.Denial{ "auth/login": { - Layer: policydecision.LayerPruning, + Layer: cmdpolicy.LayerPolicy, PolicySource: "yaml", ReasonCode: "identity_mismatch", Reason: "denied", }, } - pruning.Apply(root, denied) + cmdpolicy.Apply(root, denied) if leaf.PersistentPreRunE == nil { t.Fatal("denied command must have leaf-level PersistentPreRunE") @@ -148,17 +147,17 @@ func TestApply_NonStrictCommandStillGetsPruningAnnotation(t *testing.T) { } root.AddCommand(leaf) - denied := map[string]policydecision.Denial{ + denied := map[string]cmdpolicy.Denial{ "normal": { - Layer: policydecision.LayerPruning, + Layer: cmdpolicy.LayerPolicy, PolicySource: "yaml", Reason: "denied", ReasonCode: "command_denylisted", }, } - pruning.Apply(root, denied) + cmdpolicy.Apply(root, denied) - if got := leaf.Annotations[pruning.AnnotationDenialLayer]; got != policydecision.LayerPruning { + if got := leaf.Annotations[cmdpolicy.AnnotationDenialLayer]; got != cmdpolicy.LayerPolicy { t.Errorf("expected pruning layer annotation, got %q", got) } } diff --git a/internal/cmdpolicy/suggest.go b/internal/cmdpolicy/suggest.go new file mode 100644 index 00000000..2f7362e3 --- /dev/null +++ b/internal/cmdpolicy/suggest.go @@ -0,0 +1,86 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package cmdpolicy + +import ( + "github.com/larksuite/cli/extension/platform" +) + +// suggestRisk returns the closest valid Risk literal by edit distance +// for risk_invalid diagnostics; input is never silently substituted. +// Case-insensitive ("WRITE" → "write"); empty in, empty out (the +// absent-annotation case goes to risk_not_annotated, not here). +func suggestRisk(bad string) string { + if bad == "" { + return "" + } + lowered := toLower(bad) + candidates := []platform.Risk{ + platform.RiskRead, platform.RiskWrite, platform.RiskHighRiskWrite, + } + best := string(candidates[0]) + bestDist := levenshtein(lowered, best) + for _, c := range candidates[1:] { + if d := levenshtein(lowered, string(c)); d < bestDist { + bestDist, best = d, string(c) + } + } + return best +} + +// toLower is an ASCII-only lowercase. Risk taxonomy values are +// ASCII; pulling in unicode here would be overkill. +func toLower(s string) string { + b := []byte(s) + for i, c := range b { + if c >= 'A' && c <= 'Z' { + b[i] = c + ('a' - 'A') + } + } + return string(b) +} + +// levenshtein computes the classic edit distance between two strings. +// O(len(a)*len(b)) time, O(min(a,b)) space. Three-element string set +// makes raw performance irrelevant — clarity beats trickiness here. +func levenshtein(a, b string) int { + if len(a) == 0 { + return len(b) + } + if len(b) == 0 { + return len(a) + } + prev := make([]int, len(b)+1) + curr := make([]int, len(b)+1) + for j := 0; j <= len(b); j++ { + prev[j] = j + } + for i := 1; i <= len(a); i++ { + curr[0] = i + for j := 1; j <= len(b); j++ { + cost := 1 + if a[i-1] == b[j-1] { + cost = 0 + } + curr[j] = min3( + prev[j]+1, // deletion + curr[j-1]+1, // insertion + prev[j-1]+cost, // substitution + ) + } + prev, curr = curr, prev + } + return prev[len(b)] +} + +func min3(a, b, c int) int { + m := a + if b < m { + m = b + } + if c < m { + m = c + } + return m +} diff --git a/internal/cmdpolicy/suggest_test.go b/internal/cmdpolicy/suggest_test.go new file mode 100644 index 00000000..da91495a --- /dev/null +++ b/internal/cmdpolicy/suggest_test.go @@ -0,0 +1,51 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package cmdpolicy + +import "testing" + +// suggest is unexported, so the test lives in the same package. + +func TestSuggestRisk(t *testing.T) { + cases := []struct { + input string + want string + }{ + {"wrtie", "write"}, + {"WRITE", "write"}, + {"reed", "read"}, + {"rad", "read"}, + {"high-rik-write", "high-risk-write"}, + // "highrisk" is genuinely ambiguous between "write" and + // "high-risk-write" — not testing it. + {"", ""}, // empty input has no meaningful suggestion; the engine + // routes the absent case to risk_not_annotated, not risk_invalid. + } + for _, c := range cases { + got := suggestRisk(c.input) + if got != c.want { + t.Errorf("suggestRisk(%q) = %q, want %q", c.input, got, c.want) + } + } +} + +func TestLevenshtein(t *testing.T) { + cases := []struct { + a, b string + want int + }{ + {"", "", 0}, + {"", "abc", 3}, + {"abc", "", 3}, + {"abc", "abc", 0}, + {"wrtie", "write", 2}, + {"kitten", "sitting", 3}, + } + for _, c := range cases { + got := levenshtein(c.a, c.b) + if got != c.want { + t.Errorf("levenshtein(%q,%q) = %d, want %d", c.a, c.b, got, c.want) + } + } +} diff --git a/internal/pruning/validate.go b/internal/cmdpolicy/validate.go similarity index 94% rename from internal/pruning/validate.go rename to internal/cmdpolicy/validate.go index 1579bcef..21bb168f 100644 --- a/internal/pruning/validate.go +++ b/internal/cmdpolicy/validate.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning +package cmdpolicy import ( "fmt" @@ -34,13 +34,13 @@ func ValidateRule(r *platform.Rule) error { } if r.MaxRisk != "" { - if _, ok := platform.RiskRank(r.MaxRisk); !ok { + if !r.MaxRisk.IsValid() { return fmt.Errorf("invalid max_risk %q: must be one of read|write|high-risk-write", r.MaxRisk) } } for _, id := range r.Identities { - if id != platform.IdentityUser && id != platform.IdentityBot { + if !id.IsValid() { return fmt.Errorf("invalid identities entry %q: must be 'user' or 'bot'", id) } } diff --git a/internal/pruning/validate_test.go b/internal/cmdpolicy/validate_test.go similarity index 83% rename from internal/pruning/validate_test.go rename to internal/cmdpolicy/validate_test.go index a1563e48..3961f12a 100644 --- a/internal/pruning/validate_test.go +++ b/internal/cmdpolicy/validate_test.go @@ -1,19 +1,19 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package pruning_test +package cmdpolicy_test import ( "strings" "testing" "github.com/larksuite/cli/extension/platform" - "github.com/larksuite/cli/internal/pruning" + "github.com/larksuite/cli/internal/cmdpolicy" ) // nil rule is "no restriction" everywhere -- validation must agree. func TestValidateRule_nilIsOk(t *testing.T) { - if err := pruning.ValidateRule(nil); err != nil { + if err := cmdpolicy.ValidateRule(nil); err != nil { t.Fatalf("nil rule should validate, got %v", err) } } @@ -23,9 +23,9 @@ func TestValidateRule_validRule(t *testing.T) { Allow: []string{"docs/**", "contact/+search-*"}, Deny: []string{"docs/+delete-doc"}, MaxRisk: "write", - Identities: []string{"user", "bot"}, + Identities: []platform.Identity{"user", "bot"}, } - if err := pruning.ValidateRule(r); err != nil { + if err := cmdpolicy.ValidateRule(r); err != nil { t.Fatalf("valid rule rejected: %v", err) } } @@ -36,8 +36,8 @@ func TestValidateRule_validRule(t *testing.T) { func TestValidateRule_badMaxRisk(t *testing.T) { cases := []string{"readd", "Read", "high_risk_write", "anything"} for _, bad := range cases { - r := &platform.Rule{MaxRisk: bad} - err := pruning.ValidateRule(r) + r := &platform.Rule{MaxRisk: platform.Risk(bad)} + err := cmdpolicy.ValidateRule(r) if err == nil { t.Errorf("ValidateRule should reject MaxRisk=%q", bad) continue @@ -52,8 +52,8 @@ func TestValidateRule_badMaxRisk(t *testing.T) { // like "users" would silently lock out everyone (no command intersects // the typo), so it must abort. func TestValidateRule_badIdentity(t *testing.T) { - r := &platform.Rule{Identities: []string{"user", "admin"}} - err := pruning.ValidateRule(r) + r := &platform.Rule{Identities: []platform.Identity{"user", "admin"}} + err := cmdpolicy.ValidateRule(r) if err == nil { t.Fatalf("ValidateRule should reject identity 'admin'") } @@ -75,7 +75,7 @@ func TestValidateRule_malformedGlob(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - err := pruning.ValidateRule(c.rule) + err := cmdpolicy.ValidateRule(c.rule) if err == nil { t.Fatalf("ValidateRule should reject %+v", c.rule) } @@ -91,7 +91,7 @@ func TestValidateRule_emptyFieldsAreOk(t *testing.T) { MaxRisk: "", Identities: nil, } - if err := pruning.ValidateRule(r); err != nil { + if err := cmdpolicy.ValidateRule(r); err != nil { t.Fatalf("empty optional fields should validate, got %v", err) } } diff --git a/internal/pruning/yaml/reader.go b/internal/cmdpolicy/yaml/reader.go similarity index 100% rename from internal/pruning/yaml/reader.go rename to internal/cmdpolicy/yaml/reader.go diff --git a/internal/pruning/yaml/schema.go b/internal/cmdpolicy/yaml/schema.go similarity index 61% rename from internal/pruning/yaml/schema.go rename to internal/cmdpolicy/yaml/schema.go index cb856880..718d2a8b 100644 --- a/internal/pruning/yaml/schema.go +++ b/internal/cmdpolicy/yaml/schema.go @@ -9,7 +9,7 @@ // This package does **structural** parsing only (yaml syntax + unknown-field // rejection). Semantic validation (valid MaxRisk enum, valid identity // values, valid doublestar glob syntax) is centralised in -// internal/pruning.ValidateRule so a single contract is enforced regardless +// internal/cmdpolicy.ValidateRule so a single contract is enforced regardless // of whether the Rule came from yaml or from Plugin.Restrict. package yaml @@ -26,12 +26,13 @@ import ( // schema is the internal yaml-tagged shape. Mirrors platform.Rule but lives // here so the public Rule has no yaml tag baggage. type schema struct { - Name string `yaml:"name"` - Description string `yaml:"description,omitempty"` - Allow []string `yaml:"allow,omitempty"` - Deny []string `yaml:"deny,omitempty"` - MaxRisk string `yaml:"max_risk,omitempty"` - Identities []string `yaml:"identities,omitempty"` + Name string `yaml:"name"` + Description string `yaml:"description,omitempty"` + Allow []string `yaml:"allow,omitempty"` + Deny []string `yaml:"deny,omitempty"` + MaxRisk string `yaml:"max_risk,omitempty"` + Identities []string `yaml:"identities,omitempty"` + AllowUnannotated bool `yaml:"allow_unannotated,omitempty"` } // Parse decodes yaml bytes into a *platform.Rule. Unknown fields are @@ -40,7 +41,7 @@ type schema struct { // // Semantic validation (MaxRisk taxonomy, identity values, glob syntax) is // the caller's responsibility -- run the result through -// internal/pruning.ValidateRule before handing it to the engine. +// internal/cmdpolicy.ValidateRule before handing it to the engine. func Parse(data []byte) (*platform.Rule, error) { var s schema dec := gopkgyaml.NewDecoder(bytesReader(data)) @@ -49,10 +50,9 @@ func Parse(data []byte) (*platform.Rule, error) { return nil, fmt.Errorf("parse policy yaml: %w", err) } - // Reject multi-document input. yaml.v3 only decodes one document per - // call; silently dropping trailing docs would let a typo'd "---" hide - // real policy constraints (e.g. a stray separator followed by the - // intended deny list would leave enforcement empty). + // Reject multi-document input: yaml.v3 only decodes one document + // per call, so a stray "---" followed by another document would + // silently drop the trailing rule. var extra schema if err := dec.Decode(&extra); !errors.Is(err, io.EOF) { if err == nil { @@ -61,12 +61,17 @@ func Parse(data []byte) (*platform.Rule, error) { return nil, fmt.Errorf("parse policy yaml: %w", err) } + idents := make([]platform.Identity, len(s.Identities)) + for i, id := range s.Identities { + idents[i] = platform.Identity(id) + } return &platform.Rule{ - Name: s.Name, - Description: s.Description, - Allow: s.Allow, - Deny: s.Deny, - MaxRisk: s.MaxRisk, - Identities: s.Identities, + Name: s.Name, + Description: s.Description, + Allow: s.Allow, + Deny: s.Deny, + MaxRisk: platform.Risk(s.MaxRisk), + Identities: idents, + AllowUnannotated: s.AllowUnannotated, }, nil } diff --git a/internal/pruning/yaml/schema_test.go b/internal/cmdpolicy/yaml/schema_test.go similarity index 63% rename from internal/pruning/yaml/schema_test.go rename to internal/cmdpolicy/yaml/schema_test.go index 70bde3a1..912c8b2a 100644 --- a/internal/pruning/yaml/schema_test.go +++ b/internal/cmdpolicy/yaml/schema_test.go @@ -8,7 +8,7 @@ import ( "testing" "github.com/larksuite/cli/extension/platform" - pyaml "github.com/larksuite/cli/internal/pruning/yaml" + pyaml "github.com/larksuite/cli/internal/cmdpolicy/yaml" ) func TestParse_validRule(t *testing.T) { @@ -34,13 +34,52 @@ identities: Allow: []string{"docs/**", "contact/**"}, Deny: []string{"docs/+update"}, MaxRisk: "read", - Identities: []string{"user"}, + Identities: []platform.Identity{"user"}, } if !reflect.DeepEqual(rule, want) { t.Fatalf("rule = %+v, want %+v", rule, want) } } +// allow_unannotated is documented in the README / author guide as the +// gradual-adoption opt-in. The yaml schema must carry it through to +// platform.Rule, otherwise a user following the docs would either hit +// "unknown field" (under KnownFields strict mode) or silently lose the +// opt-in and end up with a safer-but-broken policy. +func TestParse_allowUnannotatedPassesThrough(t *testing.T) { + data := []byte(` +name: agent-readonly +max_risk: read +allow_unannotated: true +`) + rule, err := pyaml.Parse(data) + if err != nil { + t.Fatalf("Parse failed: %v", err) + } + if !rule.AllowUnannotated { + t.Fatalf("AllowUnannotated = false, want true (yaml field must propagate)") + } + if rule.MaxRisk != "read" || rule.Name != "agent-readonly" { + t.Errorf("other fields lost: %+v", rule) + } +} + +// Default is false when the key is absent: pin the fail-closed default so +// future schema edits cannot accidentally flip it. +func TestParse_allowUnannotatedDefaultsFalse(t *testing.T) { + data := []byte(` +name: x +max_risk: read +`) + rule, err := pyaml.Parse(data) + if err != nil { + t.Fatalf("Parse failed: %v", err) + } + if rule.AllowUnannotated { + t.Fatalf("AllowUnannotated must default to false when key is absent") + } +} + // Unknown fields must be rejected so the old binary cannot silently ignore // new schema additions (forward-compat safeguard). func TestParse_rejectsUnknownFields(t *testing.T) { @@ -53,7 +92,7 @@ mystery_field: oh no } } -// Semantic validation lives in pruning.ValidateRule. Parse only checks +// Semantic validation lives in cmdpolicy.ValidateRule. Parse only checks // structural yaml; an invalid max_risk passes through (validation happens // downstream). func TestParse_doesNotValidateSemantics(t *testing.T) { diff --git a/internal/hook/doc.go b/internal/hook/doc.go index 19595527..6993cb1b 100644 --- a/internal/hook/doc.go +++ b/internal/hook/doc.go @@ -15,6 +15,6 @@ // // Plugins NEVER import this package -- they only ever see // extension/platform. The Registrar contract is implemented inside -// internal/platformhost, which delegates to this Registry after -// validating the plugin's calls (staging + atomic commit). +// internal/platform, which delegates to this Registry after validating +// the plugin's calls (staging + atomic commit). package hook diff --git a/internal/hook/install.go b/internal/hook/install.go index f8989533..733a1fd1 100644 --- a/internal/hook/install.go +++ b/internal/hook/install.go @@ -7,7 +7,6 @@ import ( "context" "errors" "fmt" - "time" "github.com/spf13/cobra" @@ -35,15 +34,14 @@ import ( // error. Wrap short-circuits via AbortError get converted to // *output.ExitError so cmd/root.go emits the right envelope. // -// - **Identity is resolved by the time After observers run.** The -// framework calls invocation.InternalSetIdentity from inside the -// wrapper as soon as the command runner resolves it (today the -// wrapper does not have access to identity resolution, so this is -// stubbed to "" / false for V1 -- future PR will plumb it). +// - **Denial layer / source are populated from cobra annotations +// before any hook fires.** populateInvocationDenial reads the +// annotations attached by cmdpolicy.Apply and strictModeStubFrom, +// avoiding an import cycle between hook and cmdpolicy. // // Install must be called once during the Bootstrap pipeline after -// pruning has finished. Calling it twice on the same tree is a bug -// (each command's RunE would be wrapped multiple times). +// policy pruning has finished. Calling it twice on the same tree is a +// bug (each command's RunE would be wrapped multiple times). func Install(root *cobra.Command, reg *Registry, snapshot CommandViewSource) { if root == nil || reg == nil { return @@ -82,19 +80,15 @@ func wrapRunE(cmd *cobra.Command, reg *Registry, snapshot CommandViewSource) { cmd.RunE = func(c *cobra.Command, args []string) error { view := snapshot.View(c) - inv := &platform.Invocation{ - Cmd: view, - Args: args, - Started: time.Now(), - } + inv := newInvocation(view, args) // Detect denial: a denied command's original RunE was already - // replaced by pruning.Apply with a denyStub that returns + // replaced by cmdpolicy.Apply with a denyStub that returns // *output.ExitError wrapping *platform.CommandDeniedError. We // invoke originalRunE once with a probe-only context (no args // matter because DisableFlagParsing is set on denied commands) // to extract its CommandDeniedError, but for V1 we use a - // simpler shortcut: pruning.Apply itself marks the command + // simpler shortcut: cmdpolicy.Apply itself marks the command // via cobra annotation; install reads the annotation directly. populateInvocationDenial(inv, c) @@ -110,7 +104,7 @@ func wrapRunE(cmd *cobra.Command, reg *Registry, snapshot CommandViewSource) { // === Denial guard === // If denied, run the originalRunE directly (it is the denyStub - // installed by pruning.Apply). The Wrap chain is bypassed. + // installed by cmdpolicy.Apply). The Wrap chain is bypassed. var err error if inv.DeniedByPolicy() { err = invokeOriginal(ctx, c, args, originalRunE, originalRun) @@ -132,8 +126,11 @@ func wrapRunE(cmd *cobra.Command, reg *Registry, snapshot CommandViewSource) { wrappers = append(wrappers, recoverWrap(w.Name, namespacedWrap(w.Name, w.Fn))) } composed := ComposeWrappers(wrappers) - finalHandler := composed(func(c2 context.Context, i *platform.Invocation) error { - return invokeOriginal(c2, c, i.Args, originalRunE, originalRun) + // Pass the wrapRunE-local args, not i.Args(): the original + // RunE must see what cobra parsed, not what a hook may have + // observed via the read-only interface. + finalHandler := composed(func(c2 context.Context, _ platform.Invocation) error { + return invokeOriginal(c2, c, args, originalRunE, originalRun) }) err = finalHandler(ctx, inv) } @@ -142,7 +139,7 @@ func wrapRunE(cmd *cobra.Command, reg *Registry, snapshot CommandViewSource) { // renders the structured "hook" type. err = wrapAbortError(err) - inv.Err = err + inv.setErr(err) // === After observers (panic-safe, always run, including // when err != nil) === @@ -172,7 +169,7 @@ func invokeOriginal(ctx context.Context, c *cobra.Command, args []string, runE f // runObserverSafe invokes an Observer with panic recovery. Observers // must not break the main flow; their job is side-effect-only and a // broken plugin should not cascade into a failed CLI run. -func runObserverSafe(ctx context.Context, obs ObserverEntry, inv *platform.Invocation) { +func runObserverSafe(ctx context.Context, obs ObserverEntry, inv platform.Invocation) { defer func() { if r := recover(); r != nil { fmt.Fprintf(stderr(), "warning: hook %q panicked: %v\n", obs.Name, r) @@ -245,7 +242,7 @@ func wrapAbortError(err error) error { // reset on every command dispatch. func recoverWrap(fullName string, w platform.Wrapper) platform.Wrapper { return func(next platform.Handler) platform.Handler { - return func(ctx context.Context, inv *platform.Invocation) (returned error) { + return func(ctx context.Context, inv platform.Invocation) (returned error) { defer func() { if r := recover(); r != nil { returned = &output.ExitError{ @@ -293,7 +290,7 @@ func recoverWrap(fullName string, w platform.Wrapper) platform.Wrapper { func namespacedWrap(fullName string, w platform.Wrapper) platform.Wrapper { return func(next platform.Handler) platform.Handler { inner := w(next) - return func(ctx context.Context, inv *platform.Invocation) error { + return func(ctx context.Context, inv platform.Invocation) error { err := inner(ctx, inv) if err == nil { return nil @@ -317,22 +314,21 @@ var stderr = func() interface{ Write(p []byte) (int, error) } { return defaultStderr } -// PopulateInvocationDenial is exported for tests so they can simulate -// the denial signal without a full pruning pipeline. Production code -// goes through populateInvocationDenial which reads the cobra -// annotation set by pruning.Apply. +// populateInvocationDenial reads the cobra annotation set by +// cmdpolicy.Apply and propagates it onto the framework-internal +// invocation. // // V1 contract: a denial is signalled by the cobra annotation -// "lark:pruning_denied_layer" being set on the command. The layer -// value is the enforcement layer ("pruning" / "strict_mode") that +// "lark:policy_denied_layer" being set on the command. The layer +// value is the enforcement layer ("policy" / "strict_mode") that // gets emitted as detail.layer in the envelope; the source follows -// the annotation "lark:pruning_denied_source". +// the annotation "lark:policy_denied_source". // // This indirection lets us avoid an import cycle between hook and // pruning packages. -func populateInvocationDenial(inv *platform.Invocation, c *cobra.Command) { - const layerKey = "lark:pruning_denied_layer" - const sourceKey = "lark:pruning_denied_source" +func populateInvocationDenial(inv *invocation, c *cobra.Command) { + const layerKey = "lark:policy_denied_layer" + const sourceKey = "lark:policy_denied_source" if c.Annotations == nil { return } @@ -341,5 +337,5 @@ func populateInvocationDenial(inv *platform.Invocation, c *cobra.Command) { return } source := c.Annotations[sourceKey] - inv.InternalSetDenial(true, layer, source) + inv.setDenial(layer, source) } diff --git a/internal/hook/install_test.go b/internal/hook/install_test.go index df17f154..b99b5a87 100644 --- a/internal/hook/install_test.go +++ b/internal/hook/install_test.go @@ -30,8 +30,8 @@ type fakeView struct { func (v fakeView) Path() string { return v.path } func (v fakeView) Domain() string { return "" } -func (v fakeView) Risk() (string, bool) { return v.risk, v.risk != "" } -func (v fakeView) Identities() []string { return nil } +func (v fakeView) Risk() (platform.Risk, bool) { return platform.Risk(v.risk), v.risk != "" } +func (v fakeView) Identities() []platform.Identity { return nil } func (v fakeView) Annotation(string) (string, bool) { return "", false } func makeLeaf(use string) *cobra.Command { @@ -53,14 +53,14 @@ func TestInstall_observersBeforeAndAfterAlwaysRun(t *testing.T) { var seen []string reg.AddObserver(hook.ObserverEntry{ Name: "before", When: platform.Before, Selector: platform.All(), - Fn: func(_ context.Context, inv *platform.Invocation) { - seen = append(seen, fmt.Sprintf("before:err=%v", inv.Err)) + Fn: func(_ context.Context, inv platform.Invocation) { + seen = append(seen, fmt.Sprintf("before:err=%v", inv.Err())) }, }) reg.AddObserver(hook.ObserverEntry{ Name: "after", When: platform.After, Selector: platform.All(), - Fn: func(_ context.Context, inv *platform.Invocation) { - seen = append(seen, fmt.Sprintf("after:err=%v", inv.Err)) + Fn: func(_ context.Context, inv platform.Invocation) { + seen = append(seen, fmt.Sprintf("after:err=%v", inv.Err())) }, }) @@ -94,7 +94,7 @@ func TestInstall_wrapperChainOrder(t *testing.T) { reg.AddWrapper(hook.WrapperEntry{ Name: "outer", Selector: platform.All(), Fn: func(next platform.Handler) platform.Handler { - return func(ctx context.Context, inv *platform.Invocation) error { + return func(ctx context.Context, inv platform.Invocation) error { order = append(order, "outer-before") err := next(ctx, inv) order = append(order, "outer-after") @@ -105,7 +105,7 @@ func TestInstall_wrapperChainOrder(t *testing.T) { reg.AddWrapper(hook.WrapperEntry{ Name: "inner", Selector: platform.All(), Fn: func(next platform.Handler) platform.Handler { - return func(ctx context.Context, inv *platform.Invocation) error { + return func(ctx context.Context, inv platform.Invocation) error { order = append(order, "inner-before") err := next(ctx, inv) order = append(order, "inner-after") @@ -142,8 +142,8 @@ func TestInstall_denialGuard_physicalIsolation(t *testing.T) { return errors.New("CommandPruned: this is the denyStub") }, Annotations: map[string]string{ - "lark:pruning_denied_layer": "pruning", - "lark:pruning_denied_source": "yaml", + "lark:policy_denied_layer": "policy", + "lark:policy_denied_source": "yaml", }, } root.AddCommand(leaf) @@ -154,7 +154,7 @@ func TestInstall_denialGuard_physicalIsolation(t *testing.T) { reg.AddWrapper(hook.WrapperEntry{ Name: "malicious", Selector: platform.All(), Fn: func(next platform.Handler) platform.Handler { - return func(ctx context.Context, inv *platform.Invocation) error { + return func(ctx context.Context, inv platform.Invocation) error { maliciousWrapCalled = true return nil // suppress the denial } @@ -189,7 +189,7 @@ func TestInstall_observerPanicIsolated(t *testing.T) { reg := hook.NewRegistry() reg.AddObserver(hook.ObserverEntry{ Name: "buggy", When: platform.Before, Selector: platform.All(), - Fn: func(context.Context, *platform.Invocation) { + Fn: func(context.Context, platform.Invocation) { panic("plugin author wrote bad code") }, }) @@ -217,7 +217,7 @@ func TestInstall_abortErrorBecomesExitError(t *testing.T) { reg.AddWrapper(hook.WrapperEntry{ Name: "rejecter", Selector: platform.All(), Fn: func(_ platform.Handler) platform.Handler { - return func(context.Context, *platform.Invocation) error { + return func(context.Context, platform.Invocation) error { return &platform.AbortError{ HookName: "rejecter", Reason: "policy says no", @@ -276,14 +276,14 @@ func TestInstall_namespacedWrap_doesNotMutateSentinel(t *testing.T) { Name: "plugin-a.wrap", Selector: platform.ByCommandPath("+a"), Fn: func(platform.Handler) platform.Handler { - return func(context.Context, *platform.Invocation) error { return sentinel } + return func(context.Context, platform.Invocation) error { return sentinel } }, }) reg.AddWrapper(hook.WrapperEntry{ Name: "plugin-b.wrap", Selector: platform.ByCommandPath("+b"), Fn: func(platform.Handler) platform.Handler { - return func(context.Context, *platform.Invocation) error { return sentinel } + return func(context.Context, platform.Invocation) error { return sentinel } }, }) @@ -325,6 +325,45 @@ func checkHookName(t *testing.T, err error, want string) { } } +// A Before observer mutating inv.Args() must not affect what the +// original RunE sees: pins the slice-level read-only contract. +func TestInstall_argsNotMutableByObserver(t *testing.T) { + root := &cobra.Command{Use: "lark-cli"} + + var seenByRunE []string + leaf := &cobra.Command{ + Use: "+echo", + RunE: func(_ *cobra.Command, args []string) error { + seenByRunE = append([]string(nil), args...) + return nil + }, + } + root.AddCommand(leaf) + + reg := hook.NewRegistry() + reg.AddObserver(hook.ObserverEntry{ + Name: "tamper", When: platform.Before, Selector: platform.All(), + Fn: func(_ context.Context, inv platform.Invocation) { + got := inv.Args() + if len(got) > 0 { + got[0] = "HIJACKED" + } + }, + }) + hook.Install(root, reg, fakeViewSource{view: fakeView{path: "+echo"}}) + + originalArgs := []string{"hello", "world"} + if err := leaf.RunE(leaf, originalArgs); err != nil { + t.Fatalf("RunE returned %v", err) + } + if !equalStrings(seenByRunE, originalArgs) { + t.Fatalf("RunE saw mutated args: got %v, want %v", seenByRunE, originalArgs) + } + if originalArgs[0] != "hello" { + t.Fatalf("caller's original args were mutated: %v", originalArgs) + } +} + // Root command (no parent) must never be wrapped -- it dispatches help // and other framework concerns. The root has no RunE so we instead // verify the root's children are wrapped while the root itself remains diff --git a/internal/hook/invocation.go b/internal/hook/invocation.go new file mode 100644 index 00000000..fceb1b60 --- /dev/null +++ b/internal/hook/invocation.go @@ -0,0 +1,87 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package hook + +import ( + "time" + + "github.com/larksuite/cli/extension/platform" +) + +// invocation is the framework-side concrete implementation of +// platform.Invocation. All setters are unexported so plugin code +// (which only sees the platform.Invocation interface) cannot mutate +// state. +// +// The "denial" / "strict_mode" / "identity" fields are populated by +// the framework's bootstrap pipeline before any hook fires; plugins +// only read them through the interface. +type invocation struct { + cmd platform.CommandView + args []string + started time.Time + err error + + denied bool + layer string + source string + + strictMode string + strictModeKnown bool + + identity string + identityResolved bool +} + +// newInvocation copies args so the read-only platform.Invocation +// contract holds at the slice level: a hook cannot mutate the args +// the original RunE will see. +func newInvocation(cmd platform.CommandView, args []string) *invocation { + argsCopy := append([]string(nil), args...) + return &invocation{ + cmd: cmd, + args: argsCopy, + started: time.Now(), + } +} + +// --- platform.Invocation read interface --- + +func (i *invocation) Cmd() platform.CommandView { return i.cmd } + +// Args returns a fresh copy every call; see newInvocation. +func (i *invocation) Args() []string { + out := make([]string, len(i.args)) + copy(out, i.args) + return out +} +func (i *invocation) Started() time.Time { return i.started } +func (i *invocation) Err() error { return i.err } + +func (i *invocation) DeniedByPolicy() bool { return i.denied } +func (i *invocation) DenialLayer() string { return i.layer } +func (i *invocation) DenialPolicySource() string { + return i.source +} + +func (i *invocation) StrictMode() (string, bool) { return i.strictMode, i.strictModeKnown } +func (i *invocation) Identity() (string, bool) { return i.identity, i.identityResolved } + +// --- framework-internal setters (unexported) --- + +func (i *invocation) setDenial(layer, source string) { + i.denied = true + i.layer = layer + i.source = source +} + +// StrictMode and Identity setters are intentionally absent in V1: the +// framework does not yet plumb either value to the invocation, and +// platform.Invocation.StrictMode() / Identity() therefore return zero +// values. Add the setters when the bootstrap pipeline starts resolving +// them. + +func (i *invocation) setErr(err error) { + i.err = err +} diff --git a/internal/hook/registry.go b/internal/hook/registry.go index f0588270..90235c27 100644 --- a/internal/hook/registry.go +++ b/internal/hook/registry.go @@ -39,7 +39,7 @@ type LifecycleEntry struct { // Registry holds all registered hooks. The framework constructs one // Registry per binary execution; concurrent reads after Install // commits are safe because the maps are not mutated thereafter. Writes -// (during Install) are serialised by the platformhost. +// (during Install) are serialised by the internalplatform. type Registry struct { mu sync.RWMutex @@ -178,7 +178,7 @@ func ComposeWrappers(ws []platform.Wrapper) platform.Wrapper { // Wrappers for a command -- callers can always compose into // next(ctx, inv) without a nil check. func identityWrapper(next platform.Handler) platform.Handler { - return func(ctx context.Context, inv *platform.Invocation) error { + return func(ctx context.Context, inv platform.Invocation) error { return next(ctx, inv) } } diff --git a/internal/platformhost/doc.go b/internal/platform/doc.go similarity index 88% rename from internal/platformhost/doc.go rename to internal/platform/doc.go index 1daf1cac..1a70e594 100644 --- a/internal/platformhost/doc.go +++ b/internal/platform/doc.go @@ -5,7 +5,7 @@ // global plugin registry (extension/platform.RegisteredPlugins) into: // // - a populated internal/hook.Registry (Observer / Wrapper / Lifecycle) -// - a list of pruning.PluginRule contributions (one per plugin that +// - a list of cmdpolicy.PluginRule contributions (one per plugin that // called r.Restrict) // // Two key invariants: @@ -26,6 +26,6 @@ // The host returns: // // - a *hook.Registry ready to install on the command tree -// - a []pruning.PluginRule for the pruning resolver +// - a []cmdpolicy.PluginRule for the pruning resolver // - an error when a FailClosed plugin failed -package platformhost +package internalplatform diff --git a/internal/platformhost/error.go b/internal/platform/error.go similarity index 98% rename from internal/platformhost/error.go rename to internal/platform/error.go index 4d84a330..8ee037aa 100644 --- a/internal/platformhost/error.go +++ b/internal/platform/error.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package platformhost +package internalplatform import "fmt" diff --git a/internal/platformhost/host.go b/internal/platform/host.go similarity index 97% rename from internal/platformhost/host.go rename to internal/platform/host.go index c87b8aa3..f7944db8 100644 --- a/internal/platformhost/host.go +++ b/internal/platform/host.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package platformhost +package internalplatform import ( "errors" @@ -9,8 +9,8 @@ import ( "io" "github.com/larksuite/cli/extension/platform" + "github.com/larksuite/cli/internal/cmdpolicy" "github.com/larksuite/cli/internal/hook" - "github.com/larksuite/cli/internal/pruning" ) // PluginInfo is the metadata of a successfully-installed plugin, @@ -24,13 +24,13 @@ type PluginInfo struct { } // InstallResult is the output of InstallAll. Registry is ready for -// hook.Install; PluginRules feeds into pruning.Resolve as the +// hook.Install; PluginRules feeds into cmdpolicy.Resolve as the // "plugin contribution" half of the resolver input. Plugins lists // every plugin that committed successfully (FailOpen-skipped plugins // are absent), for downstream diagnostics. type InstallResult struct { Registry *hook.Registry - PluginRules []pruning.PluginRule + PluginRules []cmdpolicy.PluginRule Plugins []PluginInfo } @@ -186,7 +186,7 @@ func installOne(name string, p platform.Plugin, result *InstallResult) error { result.Registry.AddLifecycle(e) } if staging.rule != nil { - result.PluginRules = append(result.PluginRules, pruning.PluginRule{ + result.PluginRules = append(result.PluginRules, cmdpolicy.PluginRule{ PluginName: name, Rule: staging.rule, }) diff --git a/internal/platformhost/host_test.go b/internal/platform/host_test.go similarity index 82% rename from internal/platformhost/host_test.go rename to internal/platform/host_test.go index 9d092a3c..c1a9d663 100644 --- a/internal/platformhost/host_test.go +++ b/internal/platform/host_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package platformhost_test +package internalplatform_test import ( "bytes" @@ -11,7 +11,7 @@ import ( "testing" "github.com/larksuite/cli/extension/platform" - "github.com/larksuite/cli/internal/platformhost" + "github.com/larksuite/cli/internal/platform" ) // happyPlugin is a textbook plugin: declares Capabilities, calls a few @@ -27,10 +27,10 @@ func (p happyPlugin) Capabilities() platform.Capabilities { } func (p happyPlugin) Install(r platform.Registrar) error { r.Observe(platform.Before, "audit-pre", platform.All(), - func(context.Context, *platform.Invocation) {}) + func(context.Context, platform.Invocation) {}) r.Wrap("policy", platform.All(), func(next platform.Handler) platform.Handler { - return func(ctx context.Context, inv *platform.Invocation) error { + return func(ctx context.Context, inv platform.Invocation) error { return next(ctx, inv) } }) @@ -40,7 +40,7 @@ func (p happyPlugin) Install(r platform.Registrar) error { } func TestInstallAll_happyPlugin(t *testing.T) { - result, err := platformhost.InstallAll([]platform.Plugin{happyPlugin{name: "audit"}}, nil) + result, err := internalplatform.InstallAll([]platform.Plugin{happyPlugin{name: "audit"}}, nil) if err != nil { t.Fatalf("InstallAll: %v", err) } @@ -69,8 +69,8 @@ type fakeView struct{} func (fakeView) Path() string { return "" } func (fakeView) Domain() string { return "" } -func (fakeView) Risk() (string, bool) { return "", false } -func (fakeView) Identities() []string { return nil } +func (fakeView) Risk() (platform.Risk, bool) { return "", false } +func (fakeView) Identities() []platform.Identity { return nil } func (fakeView) Annotation(string) (string, bool) { return "", false } // A FailClosed plugin whose Install returns an error must abort @@ -89,15 +89,15 @@ func (failClosedPlugin) Install(platform.Registrar) error { } func TestInstallAll_failClosedAborts(t *testing.T) { - _, err := platformhost.InstallAll([]platform.Plugin{failClosedPlugin{}}, nil) + _, err := internalplatform.InstallAll([]platform.Plugin{failClosedPlugin{}}, nil) if err == nil { t.Fatalf("FailClosed install error should abort") } - var pi *platformhost.PluginInstallError + var pi *internalplatform.PluginInstallError if !errors.As(err, &pi) { t.Fatalf("error must be *PluginInstallError, got %T", err) } - if pi.ReasonCode != platformhost.ReasonInstallFailed { + if pi.ReasonCode != internalplatform.ReasonInstallFailed { t.Errorf("ReasonCode = %q, want install_failed", pi.ReasonCode) } } @@ -121,7 +121,7 @@ func TestInstallAll_failOpenSkips(t *testing.T) { failOpenPlugin{}, happyPlugin{name: "audit"}, } - result, err := platformhost.InstallAll(plugins, &buf) + result, err := internalplatform.InstallAll(plugins, &buf) if err != nil { t.Fatalf("FailOpen failure must not abort, got %v", err) } @@ -151,12 +151,12 @@ func (misconfiguredRestrictPlugin) Capabilities() platform.Capabilities { func (misconfiguredRestrictPlugin) Install(platform.Registrar) error { return nil } func TestInstallAll_restrictsRequiresFailClosed(t *testing.T) { - _, err := platformhost.InstallAll([]platform.Plugin{misconfiguredRestrictPlugin{}}, nil) + _, err := internalplatform.InstallAll([]platform.Plugin{misconfiguredRestrictPlugin{}}, nil) if err == nil { t.Fatalf("Restricts+FailOpen must abort") } - var pi *platformhost.PluginInstallError - if !errors.As(err, &pi) || pi.ReasonCode != platformhost.ReasonRestrictsMismatch { + var pi *internalplatform.PluginInstallError + if !errors.As(err, &pi) || pi.ReasonCode != internalplatform.ReasonRestrictsMismatch { t.Fatalf("ReasonCode = %v, want restricts_mismatch", pi) } } @@ -178,12 +178,12 @@ func (lyingRestrictPlugin) Install(platform.Registrar) error { } func TestInstallAll_restrictsDeclaredButNotCalled(t *testing.T) { - _, err := platformhost.InstallAll([]platform.Plugin{lyingRestrictPlugin{}}, nil) + _, err := internalplatform.InstallAll([]platform.Plugin{lyingRestrictPlugin{}}, nil) if err == nil { t.Fatalf("missing Restrict call when declared must fail") } - var pi *platformhost.PluginInstallError - if !errors.As(err, &pi) || pi.ReasonCode != platformhost.ReasonRestrictsMismatch { + var pi *internalplatform.PluginInstallError + if !errors.As(err, &pi) || pi.ReasonCode != internalplatform.ReasonRestrictsMismatch { t.Fatalf("ReasonCode = %v, want restricts_mismatch", pi) } } @@ -202,27 +202,27 @@ func (panicInstallPlugin) Install(platform.Registrar) error { } func TestInstallAll_installPanicRecovered(t *testing.T) { - _, err := platformhost.InstallAll([]platform.Plugin{panicInstallPlugin{}}, nil) + _, err := internalplatform.InstallAll([]platform.Plugin{panicInstallPlugin{}}, nil) if err == nil { t.Fatalf("Install panic should surface as error") } - var pi *platformhost.PluginInstallError - if !errors.As(err, &pi) || pi.ReasonCode != platformhost.ReasonInstallPanic { + var pi *internalplatform.PluginInstallError + if !errors.As(err, &pi) || pi.ReasonCode != internalplatform.ReasonInstallPanic { t.Fatalf("ReasonCode = %v, want install_panic", pi) } } // Two plugins with the same Name must abort before any Install runs. func TestInstallAll_duplicatePluginName(t *testing.T) { - _, err := platformhost.InstallAll([]platform.Plugin{ + _, err := internalplatform.InstallAll([]platform.Plugin{ happyPlugin{name: "audit"}, happyPlugin{name: "audit"}, }, nil) if err == nil { t.Fatalf("duplicate Plugin.Name must abort") } - var pi *platformhost.PluginInstallError - if !errors.As(err, &pi) || pi.ReasonCode != platformhost.ReasonDuplicatePluginName { + var pi *internalplatform.PluginInstallError + if !errors.As(err, &pi) || pi.ReasonCode != internalplatform.ReasonDuplicatePluginName { t.Fatalf("ReasonCode = %v, want duplicate_plugin_name", pi) } } @@ -244,12 +244,12 @@ func TestInstallAll_invalidPluginName(t *testing.T) { cases := []string{"with.dot", "", "-leading-hyphen", "UPPER"} for _, name := range cases { t.Run(name, func(t *testing.T) { - _, err := platformhost.InstallAll([]platform.Plugin{badNamePlugin{n: name}}, nil) + _, err := internalplatform.InstallAll([]platform.Plugin{badNamePlugin{n: name}}, nil) if err == nil { t.Fatalf("invalid name %q should abort", name) } - var pi *platformhost.PluginInstallError - if !errors.As(err, &pi) || pi.ReasonCode != platformhost.ReasonInvalidPluginName { + var pi *internalplatform.PluginInstallError + if !errors.As(err, &pi) || pi.ReasonCode != internalplatform.ReasonInvalidPluginName { t.Fatalf("ReasonCode = %v, want invalid_plugin_name", pi) } }) @@ -266,18 +266,18 @@ func (duplicateHookPlugin) Capabilities() platform.Capabilities { return platform.Capabilities{FailurePolicy: platform.FailClosed} } func (duplicateHookPlugin) Install(r platform.Registrar) error { - r.Observe(platform.Before, "x", platform.All(), func(context.Context, *platform.Invocation) {}) - r.Observe(platform.After, "x", platform.All(), func(context.Context, *platform.Invocation) {}) + r.Observe(platform.Before, "x", platform.All(), func(context.Context, platform.Invocation) {}) + r.Observe(platform.After, "x", platform.All(), func(context.Context, platform.Invocation) {}) return nil } func TestInstallAll_duplicateHookName(t *testing.T) { - _, err := platformhost.InstallAll([]platform.Plugin{duplicateHookPlugin{}}, nil) + _, err := internalplatform.InstallAll([]platform.Plugin{duplicateHookPlugin{}}, nil) if err == nil { t.Fatalf("duplicate hookName within same plugin must abort") } - var pi *platformhost.PluginInstallError - if !errors.As(err, &pi) || pi.ReasonCode != platformhost.ReasonDuplicateHookName { + var pi *internalplatform.PluginInstallError + if !errors.As(err, &pi) || pi.ReasonCode != internalplatform.ReasonDuplicateHookName { t.Fatalf("ReasonCode = %v, want duplicate_hook_name", pi) } } @@ -301,7 +301,7 @@ func (p restrictPlugin) Install(r platform.Registrar) error { func TestInstallAll_restrictPropagatesRule(t *testing.T) { rule := &platform.Rule{Name: "secaudit-policy", MaxRisk: "read"} - result, err := platformhost.InstallAll([]platform.Plugin{restrictPlugin{rule: rule}}, nil) + result, err := internalplatform.InstallAll([]platform.Plugin{restrictPlugin{rule: rule}}, nil) if err != nil { t.Fatalf("InstallAll: %v", err) } @@ -331,14 +331,14 @@ func (partiallyRegisterThenFailPlugin) Capabilities() platform.Capabilities { } func (partiallyRegisterThenFailPlugin) Install(r platform.Registrar) error { r.Observe(platform.Before, "would-leak", platform.All(), - func(context.Context, *platform.Invocation) {}) + func(context.Context, platform.Invocation) {}) // validateSelf will fail because Restricts=true but Restrict // was not called -- this is the atomic-rollback case. return nil } func TestInstallAll_atomicRollback(t *testing.T) { - _, err := platformhost.InstallAll( + _, err := internalplatform.InstallAll( []platform.Plugin{partiallyRegisterThenFailPlugin{}, happyPlugin{name: "audit"}}, nil, ) @@ -351,7 +351,7 @@ func TestInstallAll_atomicRollback(t *testing.T) { // proven by the fact that we got nil back. A weaker but useful // check: even if we passed a happy second plugin, the loop must // have stopped at the first FailClosed failure. - var pi *platformhost.PluginInstallError + var pi *internalplatform.PluginInstallError if !errors.As(err, &pi) { t.Fatalf("error must be *PluginInstallError, got %T", err) } diff --git a/internal/platform/inventory.go b/internal/platform/inventory.go new file mode 100644 index 00000000..c89c9da1 --- /dev/null +++ b/internal/platform/inventory.go @@ -0,0 +1,226 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package internalplatform + +import ( + "strings" + "sync" + + "github.com/larksuite/cli/extension/platform" + "github.com/larksuite/cli/internal/hook" +) + +// HookEntry is the displayable form of one registered hook. +type HookEntry struct { + Name string `json:"name"` + When string `json:"when,omitempty"` // observers only + Event string `json:"event,omitempty"` // lifecycle only +} + +// PluginEntry collects everything one plugin contributed. +type PluginEntry struct { + Name string + Version string + Capabilities CapabilitiesView + + // Rule is non-nil only when the plugin called r.Restrict. + Rule *RuleView + + Observers []HookEntry + Wrappers []HookEntry + Lifecycles []HookEntry +} + +// CapabilitiesView mirrors platform.Capabilities for display. We keep a +// separate struct so the JSON shape stays under our control and does +// not drift with extension/platform. +type CapabilitiesView struct { + Restricts bool `json:"restricts"` + FailurePolicy string `json:"failure_policy"` + RequiredCLIVersion string `json:"required_cli_version,omitempty"` +} + +// NewCapabilitiesView converts a platform.Capabilities value into the +// display struct. +func NewCapabilitiesView(c platform.Capabilities) CapabilitiesView { + return CapabilitiesView{ + Restricts: c.Restricts, + FailurePolicy: failurePolicyLabel(c.FailurePolicy), + RequiredCLIVersion: c.RequiredCLIVersion, + } +} + +func failurePolicyLabel(p platform.FailurePolicy) string { + switch p { + case platform.FailOpen: + return "FailOpen" + case platform.FailClosed: + return "FailClosed" + } + return "" +} + +// RuleView is the displayable form of a Plugin.Restrict contribution. +type RuleView struct { + Name string `json:"name"` + Description string `json:"description,omitempty"` + Allow []string `json:"allow"` + Deny []string `json:"deny"` + MaxRisk string `json:"max_risk"` + Identities []string `json:"identities"` + AllowUnannotated bool `json:"allow_unannotated"` +} + +// Inventory is the full snapshot. +type Inventory struct { + Plugins []PluginEntry +} + +// PluginInventorySource is the minimum slice of PluginInfo BuildInventory needs. +type PluginInventorySource struct { + Name string + Version string + Capabilities platform.Capabilities +} + +// RuleInventorySource is the minimum slice of cmdpolicy.PluginRule +// BuildInventory needs. Kept as plain strings to avoid an import +// cycle with cmdpolicy (the caller converts platform.Risk / Identity +// to string at the boundary). +type RuleInventorySource struct { + PluginName string + Allow []string + Deny []string + MaxRisk string + Identities []string + RuleName string + Desc string + AllowUnannotated bool +} + +// BuildInventory assembles an Inventory from the parts produced by +// InstallAll: the plugin metadata list, the hook registry (may be nil +// when no hooks were registered), and the plugin rules. +// +// Hooks are attributed to plugins by the namespaced name convention: +// each entry's Name starts with ".", and we group by the +// leading segment up to the first dot. +func BuildInventory(plugins []PluginInventorySource, registry *hook.Registry, rules []RuleInventorySource) *Inventory { + byPlugin := make(map[string]*PluginEntry, len(plugins)) + out := &Inventory{Plugins: make([]PluginEntry, 0, len(plugins))} + for _, p := range plugins { + entry := PluginEntry{ + Name: p.Name, + Version: p.Version, + Capabilities: NewCapabilitiesView(p.Capabilities), + } + out.Plugins = append(out.Plugins, entry) + } + for i := range out.Plugins { + byPlugin[out.Plugins[i].Name] = &out.Plugins[i] + } + + if registry != nil { + for _, e := range registry.Observers() { + if entry := byPlugin[ownerOf(e.Name)]; entry != nil { + entry.Observers = append(entry.Observers, HookEntry{ + Name: e.Name, + When: whenLabel(e.When), + }) + } + } + for _, e := range registry.Wrappers() { + if entry := byPlugin[ownerOf(e.Name)]; entry != nil { + entry.Wrappers = append(entry.Wrappers, HookEntry{ + Name: e.Name, + }) + } + } + for _, e := range registry.Lifecycles() { + if entry := byPlugin[ownerOf(e.Name)]; entry != nil { + entry.Lifecycles = append(entry.Lifecycles, HookEntry{ + Name: e.Name, + Event: eventLabel(e.Event), + }) + } + } + } + + for _, r := range rules { + if entry := byPlugin[r.PluginName]; entry != nil { + entry.Rule = &RuleView{ + Name: r.RuleName, + Description: r.Desc, + Allow: r.Allow, + Deny: r.Deny, + MaxRisk: r.MaxRisk, + Identities: r.Identities, + AllowUnannotated: r.AllowUnannotated, + } + } + } + return out +} + +// ownerOf extracts the plugin name from a namespaced hook name. The +// platform forbids "." in plugin names, so the first dot is always the +// namespace separator. Names without a dot are returned as-is. +func ownerOf(hookName string) string { + if i := strings.IndexByte(hookName, '.'); i >= 0 { + return hookName[:i] + } + return hookName +} + +func whenLabel(w platform.When) string { + switch w { + case platform.Before: + return "Before" + case platform.After: + return "After" + } + return "" +} + +func eventLabel(e platform.LifecycleEvent) string { + switch e { + case platform.Startup: + return "Startup" + case platform.Shutdown: + return "Shutdown" + } + return "" +} + +// --- Active inventory storage (process-global) --- + +var ( + inventoryMu sync.RWMutex + activeInventory *Inventory +) + +// SetActiveInventory records the inventory built at bootstrap. Called +// once from cmd/policy.go after install + wireHooks complete. +func SetActiveInventory(inv *Inventory) { + inventoryMu.Lock() + defer inventoryMu.Unlock() + if inv == nil { + activeInventory = nil + return + } + cp := *inv + activeInventory = &cp +} + +// GetActiveInventory returns a copy of the inventory, or nil if +// bootstrap has not finished. +func GetActiveInventory() *Inventory { + inventoryMu.RLock() + defer inventoryMu.RUnlock() + if activeInventory == nil { + return nil + } + cp := *activeInventory + return &cp +} diff --git a/internal/plugininventory/build_test.go b/internal/platform/inventory_test.go similarity index 81% rename from internal/plugininventory/build_test.go rename to internal/platform/inventory_test.go index e55ca9d0..a9d8d8b5 100644 --- a/internal/plugininventory/build_test.go +++ b/internal/platform/inventory_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package plugininventory_test +package internalplatform_test import ( "context" @@ -9,11 +9,11 @@ import ( "github.com/larksuite/cli/extension/platform" "github.com/larksuite/cli/internal/hook" - "github.com/larksuite/cli/internal/plugininventory" + internalplatform "github.com/larksuite/cli/internal/platform" ) -func TestBuild_groupsByPluginName(t *testing.T) { - plugins := []plugininventory.PluginSource{ +func TestBuildInventory_groupsByPluginName(t *testing.T) { + plugins := []internalplatform.PluginInventorySource{ {Name: "a", Version: "1.0", Capabilities: platform.Capabilities{ Restricts: true, FailurePolicy: platform.FailClosed, }}, @@ -21,7 +21,7 @@ func TestBuild_groupsByPluginName(t *testing.T) { } r := hook.NewRegistry() - obs := func(context.Context, *platform.Invocation) {} + obs := func(context.Context, platform.Invocation) {} wrap := func(next platform.Handler) platform.Handler { return next } lc := func(context.Context, *platform.LifecycleContext) error { return nil } @@ -32,11 +32,11 @@ func TestBuild_groupsByPluginName(t *testing.T) { r.AddLifecycle(hook.LifecycleEntry{Name: "a.boot", Event: platform.Startup, Fn: lc}) r.AddLifecycle(hook.LifecycleEntry{Name: "b.bye", Event: platform.Shutdown, Fn: lc}) - rules := []plugininventory.RuleSource{ + rules := []internalplatform.RuleInventorySource{ {PluginName: "a", RuleName: "a-rule", Allow: []string{"docs/**"}, MaxRisk: "read"}, } - inv := plugininventory.Build(plugins, r, rules) + inv := internalplatform.BuildInventory(plugins, r, rules) if got := len(inv.Plugins); got != 2 { t.Fatalf("Plugins len = %d, want 2", got) @@ -74,14 +74,14 @@ func TestBuild_groupsByPluginName(t *testing.T) { } } -func TestBuild_emptyRegistry(t *testing.T) { - inv := plugininventory.Build(nil, nil, nil) +func TestBuildInventory_empty(t *testing.T) { + inv := internalplatform.BuildInventory(nil, nil, nil) if got := len(inv.Plugins); got != 0 { t.Errorf("Plugins len = %d, want 0", got) } } -func findPlugin(inv *plugininventory.Inventory, name string) *plugininventory.PluginEntry { +func findPlugin(inv *internalplatform.Inventory, name string) *internalplatform.PluginEntry { for i := range inv.Plugins { if inv.Plugins[i].Name == name { return &inv.Plugins[i] diff --git a/internal/platformhost/staging.go b/internal/platform/staging.go similarity index 99% rename from internal/platformhost/staging.go rename to internal/platform/staging.go index 81676780..0d9f46b2 100644 --- a/internal/platformhost/staging.go +++ b/internal/platform/staging.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package platformhost +package internalplatform import ( "fmt" diff --git a/internal/platformhost/version.go b/internal/platform/version.go similarity index 99% rename from internal/platformhost/version.go rename to internal/platform/version.go index 4935b5b4..6a7c940e 100644 --- a/internal/platformhost/version.go +++ b/internal/platform/version.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package platformhost +package internalplatform import ( "fmt" diff --git a/internal/platformhost/version_test.go b/internal/platform/version_test.go similarity index 99% rename from internal/platformhost/version_test.go rename to internal/platform/version_test.go index 9f28dcda..fec37bf0 100644 --- a/internal/platformhost/version_test.go +++ b/internal/platform/version_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package platformhost +package internalplatform import ( "errors" diff --git a/internal/plugininventory/build.go b/internal/plugininventory/build.go deleted file mode 100644 index bb1c4ee2..00000000 --- a/internal/plugininventory/build.go +++ /dev/null @@ -1,127 +0,0 @@ -// Copyright (c) 2026 Lark Technologies Pte. Ltd. -// SPDX-License-Identifier: MIT - -package plugininventory - -import ( - "strings" - - "github.com/larksuite/cli/extension/platform" - "github.com/larksuite/cli/internal/hook" -) - -// PluginSource is the minimum slice of platformhost.PluginInfo we need -// here. Declared as an interface to avoid importing platformhost -// (which itself depends on hook, pruning -- keeping plugininventory at -// a lower level of the dependency graph). -type PluginSource struct { - Name string - Version string - Capabilities platform.Capabilities -} - -// RuleSource is the minimum slice of pruning.PluginRule we need. -type RuleSource struct { - PluginName string - Allow []string - Deny []string - MaxRisk string - Identities []string - RuleName string - Desc string -} - -// Build assembles an Inventory from the parts produced by -// platformhost.InstallAll: the plugin metadata list, the hook registry -// (may be nil when no hooks were registered), and the plugin rules. -// -// Hooks are attributed to plugins by the namespaced name convention: -// each entry's Name starts with ".", and we group by the -// leading segment up to the first dot. -func Build(plugins []PluginSource, registry *hook.Registry, rules []RuleSource) *Inventory { - byPlugin := make(map[string]*PluginEntry, len(plugins)) - out := &Inventory{Plugins: make([]PluginEntry, 0, len(plugins))} - for _, p := range plugins { - entry := PluginEntry{ - Name: p.Name, - Version: p.Version, - Capabilities: NewCapabilitiesView(p.Capabilities), - } - out.Plugins = append(out.Plugins, entry) - } - for i := range out.Plugins { - byPlugin[out.Plugins[i].Name] = &out.Plugins[i] - } - - if registry != nil { - for _, e := range registry.Observers() { - if entry := byPlugin[ownerOf(e.Name)]; entry != nil { - entry.Observers = append(entry.Observers, HookEntry{ - Name: e.Name, - When: whenLabel(e.When), - }) - } - } - for _, e := range registry.Wrappers() { - if entry := byPlugin[ownerOf(e.Name)]; entry != nil { - entry.Wrappers = append(entry.Wrappers, HookEntry{ - Name: e.Name, - }) - } - } - for _, e := range registry.Lifecycles() { - if entry := byPlugin[ownerOf(e.Name)]; entry != nil { - entry.Lifecycles = append(entry.Lifecycles, HookEntry{ - Name: e.Name, - Event: eventLabel(e.Event), - }) - } - } - } - - for _, r := range rules { - if entry := byPlugin[r.PluginName]; entry != nil { - entry.Rule = &RuleView{ - Name: r.RuleName, - Description: r.Desc, - Allow: r.Allow, - Deny: r.Deny, - MaxRisk: r.MaxRisk, - Identities: r.Identities, - } - } - } - return out -} - -// ownerOf extracts the plugin name from a namespaced hook name. The -// platform forbids "." in plugin names, so the first dot is always the -// namespace separator. Names without a dot are returned as-is (best- -// effort: an unregistered or pre-namespaced legacy hook still surfaces -// under its own name). -func ownerOf(hookName string) string { - if i := strings.IndexByte(hookName, '.'); i >= 0 { - return hookName[:i] - } - return hookName -} - -func whenLabel(w platform.When) string { - switch w { - case platform.Before: - return "Before" - case platform.After: - return "After" - } - return "" -} - -func eventLabel(e platform.LifecycleEvent) string { - switch e { - case platform.Startup: - return "Startup" - case platform.Shutdown: - return "Shutdown" - } - return "" -} diff --git a/internal/plugininventory/inventory.go b/internal/plugininventory/inventory.go deleted file mode 100644 index fcd8e1b1..00000000 --- a/internal/plugininventory/inventory.go +++ /dev/null @@ -1,121 +0,0 @@ -// Copyright (c) 2026 Lark Technologies Pte. Ltd. -// SPDX-License-Identifier: MIT - -// Package plugininventory holds a runtime-readable snapshot of the -// plugins that successfully installed during bootstrap. It powers -// diagnostic commands (config plugins show) without forcing them to -// re-call plugin methods at display time. -// -// The snapshot is built once, after platformhost.InstallAll commits, -// and read-only thereafter. Mutex is belt-and-braces for tests that -// reset state between cases. -package plugininventory - -import ( - "sync" - - "github.com/larksuite/cli/extension/platform" -) - -// HookEntry is the displayable form of one registered hook. -type HookEntry struct { - Name string `json:"name"` - When string `json:"when,omitempty"` // observers only - Event string `json:"event,omitempty"` // lifecycle only -} - -// PluginEntry collects everything one plugin contributed. -type PluginEntry struct { - Name string - Version string - Capabilities CapabilitiesView - - // Rule is non-nil only when the plugin called r.Restrict. - Rule *RuleView - - Observers []HookEntry - Wrappers []HookEntry - Lifecycles []HookEntry -} - -// CapabilitiesView mirrors platform.Capabilities for display. We keep a -// separate struct so the JSON shape stays under our control and does -// not drift with extension/platform. -type CapabilitiesView struct { - Restricts bool `json:"restricts"` - FailurePolicy string `json:"failure_policy"` - RequiredCLIVersion string `json:"required_cli_version,omitempty"` -} - -// NewCapabilitiesView converts a platform.Capabilities value into the -// display struct. -func NewCapabilitiesView(c platform.Capabilities) CapabilitiesView { - return CapabilitiesView{ - Restricts: c.Restricts, - FailurePolicy: failurePolicyLabel(c.FailurePolicy), - RequiredCLIVersion: c.RequiredCLIVersion, - } -} - -func failurePolicyLabel(p platform.FailurePolicy) string { - switch p { - case platform.FailOpen: - return "FailOpen" - case platform.FailClosed: - return "FailClosed" - } - return "" -} - -// RuleView is the displayable form of a Plugin.Restrict contribution. -type RuleView struct { - Name string `json:"name"` - Description string `json:"description,omitempty"` - Allow []string `json:"allow"` - Deny []string `json:"deny"` - MaxRisk string `json:"max_risk"` - Identities []string `json:"identities"` -} - -// Inventory is the full snapshot. -type Inventory struct { - Plugins []PluginEntry -} - -var ( - mu sync.RWMutex - active *Inventory -) - -// SetActive records the inventory built at bootstrap. Called once from -// cmd/policy.go after install + wireHooks complete. -func SetActive(inv *Inventory) { - mu.Lock() - defer mu.Unlock() - if inv == nil { - active = nil - return - } - cp := *inv - active = &cp -} - -// GetActive returns a copy of the inventory, or nil if bootstrap has -// not finished. -func GetActive() *Inventory { - mu.RLock() - defer mu.RUnlock() - if active == nil { - return nil - } - cp := *active - return &cp -} - -// ResetForTesting clears the snapshot. Tests must call this in cleanup -// when they exercise the bootstrap path. -func ResetForTesting() { - mu.Lock() - defer mu.Unlock() - active = nil -} diff --git a/internal/policydecision/denial.go b/internal/policydecision/denial.go deleted file mode 100644 index 01219ee3..00000000 --- a/internal/policydecision/denial.go +++ /dev/null @@ -1,144 +0,0 @@ -// Copyright (c) 2026 Lark Technologies Pte. Ltd. -// SPDX-License-Identifier: MIT - -// Package policydecision holds the merged-denial decision type that both -// strict-mode and user-layer pruning produce. It lives below both consumers -// (strict-mode apply in cmd/, user-layer engine in internal/pruning) so -// neither has to import the other. -// -// The bootstrap pipeline produces a single deniedByPath map keyed by -// canonical slash path; strict-mode and user-layer apply functions each -// filter the map by Layer and install denyStubs accordingly. -package policydecision - -import "sort" - -// Layer values match CommandDeniedError.Layer and the error.type field of -// the JSON envelope. -const ( - LayerStrictMode = "strict_mode" - LayerPruning = "pruning" -) - -// Denial is the merged record for a single rejected command path. It is -// distinct from the user-layer-only pruning.Decision type: Denial only -// exists when the command is rejected (the Allowed bool would be wasted -// here, hence not reusing pruning.Decision). -type Denial struct { - Layer string // "strict_mode" | "pruning" - PolicySource string // "plugin:secaudit" | "yaml:mywork" | "strict-mode" | "" - RuleName string // matched Rule.Name (if any) - ReasonCode string // closed enum, see tech-doc 5.3 - Reason string // human-readable -} - -// ChildDenial is what AggregateChildren consumes -- it pairs a Denial with -// the child command's path so the aggregate can carry that breakdown for -// envelope.detail.children_denied. -type ChildDenial struct { - Path string - Denial Denial -} - -// AggregateChildren produces the parent-group Denial when every child of a -// command group is itself denied. The rules: -// -// - all children share Layer "strict_mode" -> parent Layer = strict_mode, -// parent ReasonCode = single child's ReasonCode (if consistent) or -// "mixed_children_strict_mode" otherwise. -// - all children share Layer "pruning" -> parent Layer = pruning, -// ReasonCode behaves analogously. -// - mixed layers across children -> parent Layer = "pruning", -// ReasonCode = "all_children_denied", -// PolicySource = "mixed". -// -// Calling with an empty slice returns a zero Denial -- callers should treat -// this as "no aggregation needed". -func AggregateChildren(children []ChildDenial) Denial { - if len(children) == 0 { - return Denial{} - } - - // Detect layer mix and reasonCode consistency. - layers := map[string]struct{}{} - reasonCodes := map[string]struct{}{} - sources := map[string]struct{}{} - ruleNames := map[string]struct{}{} - for _, c := range children { - layers[c.Denial.Layer] = struct{}{} - reasonCodes[c.Denial.ReasonCode] = struct{}{} - if c.Denial.PolicySource != "" { - sources[c.Denial.PolicySource] = struct{}{} - } - if c.Denial.RuleName != "" { - ruleNames[c.Denial.RuleName] = struct{}{} - } - } - - // Mixed: layers differ across children. Parent goes to Layer=pruning - // (the more "user-recoverable" of the two -- swapping policy can flip - // children, swapping credential cannot). - if len(layers) > 1 { - return Denial{ - Layer: LayerPruning, - PolicySource: "mixed", - ReasonCode: "all_children_denied", - Reason: "all child commands are denied (mixed reasons)", - } - } - - // Single layer for all children. - var layer string - for l := range layers { - layer = l - } - - d := Denial{Layer: layer} - - // ReasonCode: collapse when consistent, otherwise prefix with - // "mixed_children_". - switch len(reasonCodes) { - case 1: - for rc := range reasonCodes { - d.ReasonCode = rc - } - default: - switch layer { - case LayerStrictMode: - d.ReasonCode = "mixed_children_strict_mode" - default: - d.ReasonCode = "mixed_children_pruning" - } - } - - // PolicySource: identical across children -> carry it; otherwise leave - // blank (the caller can still see per-child sources via children_denied - // in the envelope detail). - if len(sources) == 1 { - for s := range sources { - d.PolicySource = s - } - } - if layer == LayerStrictMode { - d.PolicySource = "strict-mode" - } - - // RuleName: same idea. - if len(ruleNames) == 1 { - for n := range ruleNames { - d.RuleName = n - } - } - - d.Reason = "all child commands are denied" - return d -} - -// SortChildren orders children by Path. The aggregate output of -// AggregateChildren is deterministic regardless of slice order, but tests -// and the envelope's children_denied list want a stable order. -func SortChildren(children []ChildDenial) { - sort.Slice(children, func(i, j int) bool { - return children[i].Path < children[j].Path - }) -} diff --git a/internal/policydecision/denial_test.go b/internal/policydecision/denial_test.go deleted file mode 100644 index 270e98b8..00000000 --- a/internal/policydecision/denial_test.go +++ /dev/null @@ -1,98 +0,0 @@ -// Copyright (c) 2026 Lark Technologies Pte. Ltd. -// SPDX-License-Identifier: MIT - -package policydecision_test - -import ( - "testing" - - "github.com/larksuite/cli/internal/policydecision" -) - -func TestAggregateChildren_allSameLayerAndReason(t *testing.T) { - got := policydecision.AggregateChildren([]policydecision.ChildDenial{ - {Path: "docs/+update", Denial: policydecision.Denial{ - Layer: "pruning", PolicySource: "yaml:agent", - ReasonCode: "write_not_allowed", RuleName: "agent-policy", - }}, - {Path: "docs/+delete", Denial: policydecision.Denial{ - Layer: "pruning", PolicySource: "yaml:agent", - ReasonCode: "write_not_allowed", RuleName: "agent-policy", - }}, - }) - if got.Layer != "pruning" || got.ReasonCode != "write_not_allowed" { - t.Fatalf("got %+v, want layer=pruning reason=write_not_allowed", got) - } - if got.PolicySource != "yaml:agent" || got.RuleName != "agent-policy" { - t.Fatalf("Source / RuleName should propagate when consistent, got %+v", got) - } -} - -func TestAggregateChildren_sameLayerMixedReasons(t *testing.T) { - got := policydecision.AggregateChildren([]policydecision.ChildDenial{ - {Denial: policydecision.Denial{Layer: "pruning", ReasonCode: "write_not_allowed"}}, - {Denial: policydecision.Denial{Layer: "pruning", ReasonCode: "domain_not_allowed"}}, - }) - if got.Layer != "pruning" || got.ReasonCode != "mixed_children_pruning" { - t.Fatalf("got %+v, want layer=pruning reason=mixed_children_pruning", got) - } -} - -func TestAggregateChildren_strictModeBranch(t *testing.T) { - got := policydecision.AggregateChildren([]policydecision.ChildDenial{ - {Denial: policydecision.Denial{Layer: "strict_mode", ReasonCode: "identity_not_supported"}}, - {Denial: policydecision.Denial{Layer: "strict_mode", ReasonCode: "identity_not_supported"}}, - }) - if got.Layer != "strict_mode" || got.ReasonCode != "identity_not_supported" { - t.Fatalf("got %+v", got) - } - if got.PolicySource != "strict-mode" { - t.Fatalf("PolicySource = %q, want strict-mode", got.PolicySource) - } -} - -// Mixed layers (some strict_mode, some pruning) collapse to Layer=pruning -// per the tech-doc rule -- a parent group failing for "both" reasons is -// most actionable framed as a user-policy issue (swappable) rather than a -// credential capability one (not swappable). -func TestAggregateChildren_mixedLayersFallsToPruning(t *testing.T) { - got := policydecision.AggregateChildren([]policydecision.ChildDenial{ - {Path: "docs/+update", Denial: policydecision.Denial{ - Layer: "strict_mode", ReasonCode: "identity_not_supported", - }}, - {Path: "docs/+fetch", Denial: policydecision.Denial{ - Layer: "pruning", ReasonCode: "domain_not_allowed", - }}, - }) - if got.Layer != "pruning" { - t.Fatalf("Layer = %q, want pruning (mixed-children rule)", got.Layer) - } - if got.ReasonCode != "all_children_denied" { - t.Fatalf("ReasonCode = %q, want all_children_denied", got.ReasonCode) - } - if got.PolicySource != "mixed" { - t.Fatalf("PolicySource = %q, want mixed", got.PolicySource) - } -} - -func TestAggregateChildren_emptySlice(t *testing.T) { - got := policydecision.AggregateChildren(nil) - if (got != policydecision.Denial{}) { - t.Fatalf("empty slice should produce zero Denial, got %+v", got) - } -} - -func TestSortChildren_stableOrder(t *testing.T) { - children := []policydecision.ChildDenial{ - {Path: "docs/+update"}, - {Path: "docs/+delete"}, - {Path: "docs/+create"}, - } - policydecision.SortChildren(children) - want := []string{"docs/+create", "docs/+delete", "docs/+update"} - for i, c := range children { - if c.Path != want[i] { - t.Fatalf("children[%d].Path = %q, want %q", i, c.Path, want[i]) - } - } -}