mirror of
https://github.com/larksuite/cli.git
synced 2026-07-03 14:02:43 +08:00
fix(extension/platform): address PR review must-fix items
- cmdpolicy: skip AnnotationPureGroup commands in EvaluateAll, aggregateParents, and hasRunnableDescendant so user-layer policy no longer blocks `<group> --help` after the unknown-subcommand guard attaches RunE to every parent - cmd/root: tag guarded parent groups with AnnotationPureGroup - extension/platform: drop `//go:build testing` from register_testing.go so `go test ./...` works without an extra build tag - extension/platform/README: inline reason_code reference, fix plugin lifecycle diagram order (init/Register precede RegisteredPlugins) - cmd/platform_bootstrap: route userPolicyPath through core.GetBaseConfigDir so LARKSUITE_CLI_CONFIG_DIR is honoured - cmdpolicy: add RedactHomeDir helper, fold base config dir and $HOME prefixes for config policy show + resolver errors - internal/platform: reject unrecognised FailurePolicy values with invalid_capability instead of silently fail-open - cmd/config: surface diagnostic policy/plugins commands in `config --help` Long text - CHANGELOG: document command_denied error.type rename and unknown_subcommand exit-2 behavior change
This commit is contained in:
41
CHANGELOG.md
41
CHANGELOG.md
@@ -2,6 +2,47 @@
|
||||
|
||||
All notable changes to this project will be documented in this file.
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Features
|
||||
|
||||
- **extension/platform**: Public plugin SDK with policy engine, hooks,
|
||||
and the `NewPlugin` Builder. Plugins ship as a fork of `lark-cli` that
|
||||
blank-imports the plugin package; the host installs hooks (Observe /
|
||||
Wrap / Lifecycle) and a single optional user-layer `Restrict` rule.
|
||||
Diagnostic commands `lark-cli config plugins show` and
|
||||
`lark-cli config policy show` enumerate the resulting inventory (#910).
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
- **error.type rename — `strict_mode` / `pruning` → `command_denied`**.
|
||||
Both the strict-mode pruning pass and the new user-layer policy
|
||||
engine now emit a single envelope `error.type == "command_denied"`.
|
||||
The original layer is preserved on `detail.layer` (`strict_mode` or
|
||||
`policy`), so agents that need to distinguish can still do so.
|
||||
|
||||
- Old: `error.type == "strict_mode"` and `error.type == "pruning"`.
|
||||
- New: `error.type == "command_denied" && detail.layer == "strict_mode"`
|
||||
or `detail.layer == "policy"`.
|
||||
- Migration: any agent / monitor / script that matches
|
||||
`error.type == "strict_mode"` will silently break — update the
|
||||
match to the new shape.
|
||||
|
||||
- **Unknown subcommand returns exit 2 + structured error**. Previously
|
||||
invoking a group with an unknown subcommand (e.g.
|
||||
`lark-cli drive nosuchcommand`) silently printed help and exited 0.
|
||||
It now emits an `error.type == "unknown_subcommand"` envelope and
|
||||
exits with code 2.
|
||||
- Migration: any script / CI step that relied on exit-0 for unknown
|
||||
subcommands must be updated. `lark-cli <group> --help` still
|
||||
returns exit 0 + help as before.
|
||||
|
||||
### Documentation
|
||||
|
||||
- **extension/platform**: Embed the full `reason_code` reference table
|
||||
inside `extension/platform/README.md` so the public SDK landing page
|
||||
no longer links to documents that live outside the repository (#910).
|
||||
|
||||
## [v1.0.31] - 2026-05-14
|
||||
|
||||
### Features
|
||||
|
||||
@@ -14,6 +14,11 @@ func NewCmdConfig(f *cmdutil.Factory) *cobra.Command {
|
||||
cmd := &cobra.Command{
|
||||
Use: "config",
|
||||
Short: "Global CLI configuration management",
|
||||
Long: `Global CLI configuration management.
|
||||
|
||||
Diagnostic (hidden) commands — runnable but omitted from --help:
|
||||
lark-cli config policy show Inspect active user-layer policy
|
||||
lark-cli config plugins show Inspect installed plugins and hooks`,
|
||||
PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
|
||||
// Replicate rootCmd's PersistentPreRun behaviour: cobra stops at the first
|
||||
// PersistentPreRun[E] found walking up the chain, so the root-level
|
||||
|
||||
@@ -74,10 +74,20 @@ func runConfigPolicyShow(f *cmdutil.Factory) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// `yaml_path` and the yaml-source `source_name` leak the user's home
|
||||
// directory in their raw form (e.g. /Users/alice/.lark-cli/policy.yml).
|
||||
// `config policy show` is read by AI agents and CI logs, so we redact
|
||||
// the prefix before emitting -- same rule as policySourceLabel for
|
||||
// envelopes. For plugin sources, Source.Name is the plugin name (no
|
||||
// path) and is surfaced verbatim.
|
||||
sourceName := active.Source.Name
|
||||
if active.Source.Kind == cmdpolicy.SourceYAML {
|
||||
sourceName = cmdpolicy.RedactHomeDir(sourceName)
|
||||
}
|
||||
out := map[string]any{
|
||||
"source": string(active.Source.Kind),
|
||||
"source_name": active.Source.Name,
|
||||
"yaml_path": active.YAMLPath,
|
||||
"source_name": sourceName,
|
||||
"yaml_path": cmdpolicy.RedactHomeDir(active.YAMLPath),
|
||||
"denied_paths": active.DeniedPaths,
|
||||
}
|
||||
if active.Rule != nil {
|
||||
|
||||
@@ -13,9 +13,9 @@ import (
|
||||
|
||||
"github.com/larksuite/cli/extension/platform"
|
||||
"github.com/larksuite/cli/internal/cmdpolicy"
|
||||
"github.com/larksuite/cli/internal/core"
|
||||
"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.
|
||||
@@ -221,14 +221,20 @@ func splitCSV(s string) []string {
|
||||
return out
|
||||
}
|
||||
|
||||
// userPolicyPath returns the absolute path of ~/.lark-cli/policy.yml,
|
||||
// or an error if the user's home directory cannot be determined.
|
||||
// userPolicyPath returns the path of <baseConfigDir>/policy.yml.
|
||||
//
|
||||
// The base directory honours LARKSUITE_CLI_CONFIG_DIR (via
|
||||
// core.GetBaseConfigDir) so that test isolation, container deployments
|
||||
// and per-Agent config overrides all see a consistent policy location.
|
||||
// Using vfs.UserHomeDir directly here would silently bypass the env
|
||||
// override and route every test through the real ~/.lark-cli.
|
||||
//
|
||||
// The error return is retained for caller compatibility but is always
|
||||
// nil today: GetBaseConfigDir falls back to a relative ".lark-cli" when
|
||||
// the home dir can't be resolved, and the resolver already treats a
|
||||
// missing file as "no policy".
|
||||
func userPolicyPath() (string, error) {
|
||||
home, err := vfs.UserHomeDir()
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return filepath.Join(home, ".lark-cli", userPolicyFileName), nil
|
||||
return filepath.Join(core.GetBaseConfigDir(), userPolicyFileName), nil
|
||||
}
|
||||
|
||||
// warnPolicyError writes a one-line stderr warning when the user policy
|
||||
|
||||
12
cmd/root.go
12
cmd/root.go
@@ -19,6 +19,7 @@ import (
|
||||
"github.com/larksuite/cli/extension/platform"
|
||||
internalauth "github.com/larksuite/cli/internal/auth"
|
||||
"github.com/larksuite/cli/internal/build"
|
||||
"github.com/larksuite/cli/internal/cmdpolicy"
|
||||
"github.com/larksuite/cli/internal/cmdutil"
|
||||
"github.com/larksuite/cli/internal/core"
|
||||
"github.com/larksuite/cli/internal/hook"
|
||||
@@ -286,9 +287,20 @@ func writeSecurityPolicyError(w io.Writer, spErr *internalauth.SecurityPolicyErr
|
||||
|
||||
// installUnknownSubcommandGuard replaces cobra's silent help fallback on
|
||||
// group commands (no Run/RunE) with an unknown_subcommand error.
|
||||
//
|
||||
// IMPORTANT: every command modified here is also tagged with
|
||||
// cmdpolicy.AnnotationPureGroup so the user-layer policy engine
|
||||
// continues to treat the command as a pure parent group. Without the
|
||||
// tag, the RunE injection here would flip Runnable()=true and a user
|
||||
// rule like `max_risk: read` would deny every `<group> --help` call
|
||||
// with reason_code=risk_not_annotated.
|
||||
func installUnknownSubcommandGuard(cmd *cobra.Command) {
|
||||
if cmd.HasSubCommands() && cmd.Run == nil && cmd.RunE == nil {
|
||||
cmd.RunE = unknownSubcommandRunE
|
||||
if cmd.Annotations == nil {
|
||||
cmd.Annotations = map[string]string{}
|
||||
}
|
||||
cmd.Annotations[cmdpolicy.AnnotationPureGroup] = "true"
|
||||
}
|
||||
for _, c := range cmd.Commands() {
|
||||
installUnknownSubcommandGuard(c)
|
||||
|
||||
@@ -61,6 +61,42 @@ You should see `audit` in the plugin list.
|
||||
| `On(Startup/Shutdown)` | Process lifecycle | N/A |
|
||||
| `Restrict(Rule)` | Bootstrap-time, single per binary | Denies whole subtrees |
|
||||
|
||||
### Plugin lifecycle
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
participant Host as lark-cli (host)
|
||||
participant SDK as platform (SDK)
|
||||
participant Plugin as your plugin
|
||||
|
||||
Note over Host,Plugin: Process start (before main)
|
||||
Plugin->>Plugin: init() (via blank import)
|
||||
Plugin->>SDK: Register(plugin)
|
||||
|
||||
Note over Host,Plugin: Bootstrap (host main)
|
||||
Host->>SDK: RegisteredPlugins()
|
||||
SDK-->>Host: snapshot in registration order
|
||||
Host->>SDK: InstallAll()
|
||||
SDK->>Plugin: Capabilities()
|
||||
SDK->>Plugin: Install(Registrar)
|
||||
Plugin->>SDK: Observe / Wrap / Restrict / On(Startup,Shutdown)
|
||||
SDK->>Plugin: On(Startup) fire
|
||||
|
||||
Note over Host,Plugin: Each command dispatch
|
||||
Host->>SDK: hook chain (in registration order)
|
||||
SDK->>Plugin: Observer Before
|
||||
SDK->>Plugin: Wrap (around RunE)
|
||||
SDK->>Plugin: Observer After
|
||||
|
||||
Note over Host,Plugin: Process exit
|
||||
Host->>SDK: Emit(Shutdown)
|
||||
SDK->>Plugin: On(Shutdown) fire
|
||||
```
|
||||
|
||||
A `command_denied` decision (from `Restrict` or strict-mode) bypasses
|
||||
the `Wrap` chain entirely — observers still fire so audit plugins see
|
||||
the rejected dispatch.
|
||||
|
||||
## Safety contract (read this)
|
||||
|
||||
- A plugin calling `Restrict()` MUST declare `FailClosed`. The Builder
|
||||
@@ -85,9 +121,66 @@ You should see `audit` in the plugin list.
|
||||
does NOT bypass this — typo is a code bug, not a missing
|
||||
annotation.
|
||||
|
||||
## reason_code reference
|
||||
|
||||
Every install / dispatch failure emits a `command_denied` or
|
||||
`plugin_install` envelope carrying a `detail.reason_code` from the
|
||||
closed enum below. Use the code (not the human-readable message) when
|
||||
matching errors in agents, CI scripts, or downstream tools — the
|
||||
messages are localised and may change between releases.
|
||||
|
||||
### Plugin install (`error.type = plugin_install`)
|
||||
|
||||
| reason_code | When it fires | Honours FailurePolicy? |
|
||||
| --------------------------- | ------------------------------------------------------------------------------ | ---------------------- |
|
||||
| `invalid_plugin_name` | `Plugin.Name()` doesn't match `^[a-z0-9][a-z0-9-]*$` | No — always aborts |
|
||||
| `plugin_name_panic` | `Plugin.Name()` panicked | No — always aborts |
|
||||
| `duplicate_plugin_name` | Two plugins return the same `Name()` | No — always aborts |
|
||||
| `capabilities_panic` | `Plugin.Capabilities()` panicked | Yes |
|
||||
| `invalid_capability` | `Capabilities` malformed: bad `RequiredCLIVersion`, unknown `FailurePolicy` | No — always aborts |
|
||||
| `capability_unmet` | Current CLI version doesn't satisfy `RequiredCLIVersion` | Yes |
|
||||
| `restricts_mismatch` | `Restricts=true` without `FailClosed`, or `Restricts` flag inconsistent w/ Install | No — always aborts |
|
||||
| `invalid_hook_name` | Hook name contains `.` or doesn't match the plugin namespace | Yes |
|
||||
| `duplicate_hook_name` | Same hook name registered twice within a plugin | Yes |
|
||||
| `invalid_hook_registration` | Hook factory returns nil / Wrap chain re-entry / etc. | Yes |
|
||||
| `invalid_rule` | Rule fails ValidateRule (malformed glob, bad MaxRisk, unknown Identity) | Yes |
|
||||
| `double_restrict` | Plugin called `r.Restrict()` more than once in one Install | Yes |
|
||||
| `multiple_restrict_plugins` | Two or more plugins each contributed Restrict | Yes |
|
||||
| `install_failed` | `Plugin.Install` returned a non-nil error | Yes |
|
||||
| `install_panic` | `Plugin.Install` panicked | Yes |
|
||||
|
||||
"No — always aborts" entries are treated as **untrusted-config errors**:
|
||||
the host can't honour the plugin's declared `FailurePolicy` because the
|
||||
declaration itself is suspect (e.g. an `invalid_capability` plugin
|
||||
might also be lying about being `FailOpen`).
|
||||
|
||||
### Command dispatch (`error.type = command_denied`)
|
||||
|
||||
| reason_code | Meaning |
|
||||
| ----------------------- | ---------------------------------------------------------------------------------------------------------------- |
|
||||
| `risk_not_annotated` | Command has no `risk_level` annotation, and the active Rule does not set `allow_unannotated: true` |
|
||||
| `risk_invalid` | Command's `risk_level` is a typo / not in the `read | write | high-risk-write` taxonomy (always fail-closed) |
|
||||
| `command_denylisted` | Command path matched the active Rule's `deny` glob |
|
||||
| `domain_not_allowed` | Active Rule has a non-empty `allow` list and the command path did not match any glob |
|
||||
| `write_not_allowed` | Command risk is `write` / `high-risk-write` and exceeds Rule `max_risk` |
|
||||
| `risk_too_high` | Command risk exceeds Rule `max_risk` but is not a write (reserved for future risk levels) |
|
||||
| `identity_mismatch` | Command's `supportedIdentities` does not intersect Rule `identities` |
|
||||
| `aggregate_all_denied` | Aggregate stub installed on a parent group because every live child was denied |
|
||||
|
||||
The `detail.layer` field distinguishes who rejected the call:
|
||||
`policy` (this SDK's user-layer engine) vs. `strict_mode`
|
||||
(`cmd/prune.go`'s credential-hardening pass). Agents that want to
|
||||
dispatch on "any denial" should match `error.type == "command_denied"`
|
||||
and ignore the layer; agents that only care about user-policy denials
|
||||
should additionally check `detail.layer == "policy"`.
|
||||
|
||||
## 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)
|
||||
- Builder API: see [`builder.go`](./builder.go) for the full DSL
|
||||
(`NewPlugin`, `Observer`, `Wrap`, `Restrict`, `FailOpen`/`FailClosed`,
|
||||
`MustBuild`).
|
||||
- Inventory diagnostic: run `lark-cli config plugins show` after
|
||||
installing your plugin to see hooks/rules attributed to your plugin
|
||||
name.
|
||||
|
||||
@@ -40,6 +40,7 @@ func TestRegister_preservesInsertionOrder(t *testing.T) {
|
||||
|
||||
func TestRegister_resetClears(t *testing.T) {
|
||||
platform.ResetForTesting()
|
||||
t.Cleanup(platform.ResetForTesting)
|
||||
platform.Register(stubPlugin{name: "a"})
|
||||
if len(platform.RegisteredPlugins()) != 1 {
|
||||
t.Fatalf("expected 1 plugin")
|
||||
|
||||
@@ -1,14 +1,16 @@
|
||||
// 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.
|
||||
// ResetForTesting clears the global plugin registry. Exposed for test
|
||||
// isolation only — plugin authors and SDK consumers must NOT call this
|
||||
// from production code. The function is exported (rather than placed in
|
||||
// an internal test-only file) so that `go test ./...` works for every
|
||||
// downstream package without an extra build tag.
|
||||
//
|
||||
// Tests that exercise plugin registration should defer
|
||||
// `t.Cleanup(platform.ResetForTesting)` so subsequent tests start
|
||||
// from a clean slate.
|
||||
// Tests that exercise plugin registration must defer
|
||||
// `t.Cleanup(platform.ResetForTesting)` so subsequent tests start from a
|
||||
// clean slate. The helper is NOT goroutine-safe across concurrent
|
||||
// `t.Parallel()` tests — the global registry is shared process state.
|
||||
func ResetForTesting() { pluginRegistry.reset() }
|
||||
|
||||
@@ -240,6 +240,113 @@ func TestApply_runEReturnsExitErrorAndCommandDeniedError(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Regression: a pure parent group carrying AnnotationPureGroup must be
|
||||
// skipped by both EvaluateAll and aggregateParents. Without the skip,
|
||||
// the cmd.installUnknownSubcommandGuard pass (which attaches a RunE to
|
||||
// every group for cobra's silent-help fallback) would flip Runnable()
|
||||
// to true for `docs`, `drive`, etc., and a yaml rule like
|
||||
// `max_risk: read` would deny every `<group> --help` invocation with
|
||||
// reason_code = risk_not_annotated.
|
||||
func TestEvaluateAll_skipsAnnotatedPureGroup(t *testing.T) {
|
||||
root := &cobra.Command{Use: "lark-cli"}
|
||||
drive := &cobra.Command{
|
||||
Use: "drive",
|
||||
RunE: func(*cobra.Command, []string) error { return nil }, // emulate guard injection
|
||||
Annotations: map[string]string{
|
||||
cmdpolicy.AnnotationPureGroup: "true",
|
||||
},
|
||||
}
|
||||
root.AddCommand(drive)
|
||||
pull := &cobra.Command{Use: "+pull", RunE: noop}
|
||||
cmdutil.SetRisk(pull, "read")
|
||||
drive.AddCommand(pull)
|
||||
|
||||
e := cmdpolicy.New(&platform.Rule{MaxRisk: "read"})
|
||||
got := e.EvaluateAll(root)
|
||||
|
||||
if d, present := got["drive"]; present {
|
||||
t.Errorf("annotated pure group should not appear in Decisions; got %+v", d)
|
||||
}
|
||||
if !got["drive/+pull"].Allowed {
|
||||
t.Errorf("leaf under pure group must still be evaluated; got %+v", got["drive/+pull"])
|
||||
}
|
||||
}
|
||||
|
||||
// Regression: hasRunnableDescendant must also treat
|
||||
// AnnotationPureGroup-tagged commands as non-runnable. Without the
|
||||
// skip, an entire branch consisting of a pure-group placeholder + a
|
||||
// single pure-group leaf would advertise itself as a "live" subtree
|
||||
// and the parent aggregation pass would refuse to install a deny stub
|
||||
// (allLiveChildrenDenied flips to false because the pure group is
|
||||
// neither runnable nor in `denied`).
|
||||
func TestHasRunnableDescendant_ignoresAnnotatedPureGroup(t *testing.T) {
|
||||
root := &cobra.Command{Use: "lark-cli"}
|
||||
docs := &cobra.Command{Use: "docs"}
|
||||
root.AddCommand(docs)
|
||||
|
||||
// A pure-group sibling of a real leaf. The parent must still
|
||||
// aggregate based on the real leaf alone.
|
||||
placeholder := &cobra.Command{
|
||||
Use: "placeholder",
|
||||
RunE: func(*cobra.Command, []string) error { return nil },
|
||||
Annotations: map[string]string{
|
||||
cmdpolicy.AnnotationPureGroup: "true",
|
||||
},
|
||||
}
|
||||
docs.AddCommand(placeholder)
|
||||
noChild := &cobra.Command{
|
||||
Use: "+ghost",
|
||||
RunE: func(*cobra.Command, []string) error { return nil },
|
||||
Annotations: map[string]string{
|
||||
cmdpolicy.AnnotationPureGroup: "true",
|
||||
},
|
||||
}
|
||||
placeholder.AddCommand(noChild)
|
||||
|
||||
fetch := &cobra.Command{Use: "+fetch", RunE: noop}
|
||||
cmdutil.SetRisk(fetch, "write")
|
||||
docs.AddCommand(fetch)
|
||||
|
||||
e := cmdpolicy.New(&platform.Rule{MaxRisk: "read"})
|
||||
decisions := e.EvaluateAll(root)
|
||||
denied := cmdpolicy.BuildDeniedByPath(root, decisions, cmdpolicy.ResolveSource{Kind: cmdpolicy.SourceYAML}, "")
|
||||
|
||||
if _, ok := denied["docs"]; !ok {
|
||||
t.Fatalf("docs should be aggregated as fully denied (pure-group children excluded from live count); map=%+v", denied)
|
||||
}
|
||||
}
|
||||
|
||||
// Regression: aggregateParents must treat an AnnotationPureGroup-tagged
|
||||
// command exactly like a parent-only group. With cmdRunnable accidentally
|
||||
// true (RunE attached by the guard), the aggregator would otherwise look
|
||||
// for an own-RunE denial entry and skip aggregation, leaving `<group>
|
||||
// --help` reachable even when every live child is denied.
|
||||
func TestBuildDeniedByPath_aggregatesAnnotatedPureGroup(t *testing.T) {
|
||||
root := &cobra.Command{Use: "lark-cli"}
|
||||
drive := &cobra.Command{
|
||||
Use: "drive",
|
||||
RunE: func(*cobra.Command, []string) error { return nil },
|
||||
Annotations: map[string]string{
|
||||
cmdpolicy.AnnotationPureGroup: "true",
|
||||
},
|
||||
}
|
||||
root.AddCommand(drive)
|
||||
push := &cobra.Command{Use: "+push", RunE: noop}
|
||||
cmdutil.SetRisk(push, "write")
|
||||
drive.AddCommand(push)
|
||||
pull := &cobra.Command{Use: "+pull", RunE: noop}
|
||||
cmdutil.SetRisk(pull, "write")
|
||||
drive.AddCommand(pull)
|
||||
|
||||
e := cmdpolicy.New(&platform.Rule{MaxRisk: "read"})
|
||||
decisions := e.EvaluateAll(root)
|
||||
denied := cmdpolicy.BuildDeniedByPath(root, decisions, cmdpolicy.ResolveSource{Kind: cmdpolicy.SourceYAML}, "")
|
||||
|
||||
if _, ok := denied["drive"]; !ok {
|
||||
t.Fatalf("aggregator must install drive denial when all children denied; map=%+v", denied)
|
||||
}
|
||||
}
|
||||
|
||||
// The binary root must never receive a denyStub even if every descendant
|
||||
// is denied. cobra still needs root to dispatch help / completion.
|
||||
func TestApply_neverInstallsOnRoot(t *testing.T) {
|
||||
|
||||
@@ -72,8 +72,32 @@ func Apply(root *cobra.Command, deniedByPath map[string]Denial) int {
|
||||
const (
|
||||
AnnotationDenialLayer = "lark:policy_denied_layer"
|
||||
AnnotationDenialSource = "lark:policy_denied_source"
|
||||
|
||||
// AnnotationPureGroup marks a cobra.Command that is logically a
|
||||
// parent-only group but had a RunE attached by the bootstrap-time
|
||||
// unknown-subcommand guard. The engine treats annotated commands
|
||||
// the same as un-annotated parent groups (no RunE): they are not
|
||||
// evaluated against the Rule, and aggregateParents does not treat
|
||||
// them as hybrids.
|
||||
//
|
||||
// Without this signal, a user enabling a policy.yml with
|
||||
// max_risk: read would see every group (`lark-cli drive --help`,
|
||||
// `lark-cli docs --help`) return exit 2 + risk_not_annotated,
|
||||
// because the guard's RunE flips Runnable()=true and the engine
|
||||
// then demands a risk_level annotation on the group itself.
|
||||
AnnotationPureGroup = "lark:cmd_pure_group"
|
||||
)
|
||||
|
||||
// IsPureGroup reports whether cmd carries the AnnotationPureGroup marker.
|
||||
// Used by the engine to skip evaluation and by the aggregator to treat the
|
||||
// command as a parent-only group regardless of cobra's Runnable() answer.
|
||||
func IsPureGroup(cmd *cobra.Command) bool {
|
||||
if cmd == nil || cmd.Annotations == nil {
|
||||
return false
|
||||
}
|
||||
return cmd.Annotations[AnnotationPureGroup] == "true"
|
||||
}
|
||||
|
||||
// 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 {
|
||||
|
||||
@@ -63,6 +63,14 @@ func (e *Engine) EvaluateAll(root *cobra.Command) map[string]Decision {
|
||||
if !c.Runnable() {
|
||||
return
|
||||
}
|
||||
// Pure parent groups carrying the AnnotationPureGroup marker
|
||||
// (installed by cmd.installUnknownSubcommandGuard) look
|
||||
// Runnable to cobra but are not a real leaf: skip them just
|
||||
// like cobra-native parent groups, so a user-level Rule does
|
||||
// not block `<group> --help` discovery.
|
||||
if IsPureGroup(c) {
|
||||
return
|
||||
}
|
||||
path := CanonicalPath(c)
|
||||
if path == "" {
|
||||
return
|
||||
@@ -211,7 +219,12 @@ func aggregateParents(cmd *cobra.Command, denied map[string]Denial) bool {
|
||||
}
|
||||
|
||||
children := cmd.Commands()
|
||||
cmdRunnable := cmd.Runnable()
|
||||
// A pure parent group decorated with the unknown-subcommand guard
|
||||
// looks Runnable() to cobra but is not a true hybrid: treat it
|
||||
// exactly like cobra-native parent groups so the aggregation pass
|
||||
// can still install an aggregate deny stub when every live child
|
||||
// is denied.
|
||||
cmdRunnable := cmd.Runnable() && !IsPureGroup(cmd)
|
||||
cmdPath := CanonicalPath(cmd)
|
||||
|
||||
// Pure leaf
|
||||
@@ -277,7 +290,7 @@ func hasRunnableDescendant(cmd *cobra.Command) bool {
|
||||
if cmd == nil {
|
||||
return false
|
||||
}
|
||||
if cmd.Runnable() {
|
||||
if cmd.Runnable() && !IsPureGroup(cmd) {
|
||||
return true
|
||||
}
|
||||
for _, c := range cmd.Commands() {
|
||||
|
||||
@@ -4,9 +4,13 @@
|
||||
package cmdpolicy
|
||||
|
||||
import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
||||
"github.com/spf13/cobra"
|
||||
|
||||
"github.com/larksuite/cli/internal/core"
|
||||
"github.com/larksuite/cli/internal/vfs"
|
||||
)
|
||||
|
||||
// CanonicalPath returns the rootless slash-separated path used everywhere in
|
||||
@@ -50,3 +54,67 @@ func useName(cmd *cobra.Command) string {
|
||||
}
|
||||
return name
|
||||
}
|
||||
|
||||
// RedactHomeDir collapses environment-rooted prefixes so path strings
|
||||
// can be safely surfaced through `config policy show` and resolver
|
||||
// error messages without leaking the user's filesystem layout to AI
|
||||
// agents / CI logs.
|
||||
//
|
||||
// It folds, in priority order:
|
||||
// 1. core.GetBaseConfigDir() (typically ~/.lark-cli, or a custom
|
||||
// directory under LARKSUITE_CLI_CONFIG_DIR — e.g.
|
||||
// "/private/tmp/sandbox/.lark-cli" in a sandboxed run) → "<config>"
|
||||
// 2. The user's home directory → "~"
|
||||
//
|
||||
// (1) runs first so a `LARKSUITE_CLI_CONFIG_DIR` pointing outside `$HOME`
|
||||
// still produces a stable, non-identifying label. When neither prefix
|
||||
// matches, the input is returned unchanged — those cases don't leak
|
||||
// anything that wasn't already passed in by the caller.
|
||||
func RedactHomeDir(path string) string {
|
||||
if path == "" {
|
||||
return ""
|
||||
}
|
||||
abs, err := filepath.Abs(path)
|
||||
if err != nil {
|
||||
abs = path
|
||||
}
|
||||
|
||||
if rel, ok := foldPrefix(abs, core.GetBaseConfigDir()); ok {
|
||||
if rel == "" {
|
||||
return "<config>"
|
||||
}
|
||||
return "<config>/" + rel
|
||||
}
|
||||
|
||||
home, err := vfs.UserHomeDir()
|
||||
if err != nil || home == "" {
|
||||
return path
|
||||
}
|
||||
if rel, ok := foldPrefix(abs, home); ok {
|
||||
if rel == "" {
|
||||
return "~"
|
||||
}
|
||||
return "~/" + rel
|
||||
}
|
||||
return path
|
||||
}
|
||||
|
||||
// foldPrefix reports whether abs lives at or beneath prefix; on hit it
|
||||
// returns the slash-form relative tail (empty when abs == prefix).
|
||||
func foldPrefix(abs, prefix string) (string, bool) {
|
||||
if prefix == "" {
|
||||
return "", false
|
||||
}
|
||||
absPrefix, err := filepath.Abs(prefix)
|
||||
if err != nil {
|
||||
absPrefix = prefix
|
||||
}
|
||||
rel, err := filepath.Rel(absPrefix, abs)
|
||||
if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
|
||||
return "", false
|
||||
}
|
||||
if rel == "." {
|
||||
return "", true
|
||||
}
|
||||
return filepath.ToSlash(rel), true
|
||||
}
|
||||
|
||||
57
internal/cmdpolicy/path_test.go
Normal file
57
internal/cmdpolicy/path_test.go
Normal file
@@ -0,0 +1,57 @@
|
||||
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package cmdpolicy_test
|
||||
|
||||
import (
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/larksuite/cli/internal/cmdpolicy"
|
||||
)
|
||||
|
||||
// RedactHomeDir folds two prefixes:
|
||||
//
|
||||
// 1. core.GetBaseConfigDir() → "<config>" (covers the
|
||||
// LARKSUITE_CLI_CONFIG_DIR override, which is the only way a real
|
||||
// deployment writes the policy file outside $HOME).
|
||||
// 2. The user's home dir → "~" (catches the conventional
|
||||
// ~/.lark-cli/policy.yml path when no override is set).
|
||||
//
|
||||
// Both folds run in path-prefix space (not string-prefix), so a path
|
||||
// like "/Usersfoo" never gets folded against "/Users".
|
||||
func TestRedactHomeDir_foldsConfigDirOverride(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp)
|
||||
|
||||
policyPath := filepath.Join(tmp, "policy.yml")
|
||||
got := cmdpolicy.RedactHomeDir(policyPath)
|
||||
if got != "<config>/policy.yml" {
|
||||
t.Errorf("override path = %q, want <config>/policy.yml", got)
|
||||
}
|
||||
|
||||
// A path that equals the config dir itself collapses to "<config>".
|
||||
if got := cmdpolicy.RedactHomeDir(tmp); got != "<config>" {
|
||||
t.Errorf("exact-prefix path = %q, want <config>", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A path outside both the config dir and $HOME stays absolute. This is
|
||||
// the "no leak introduced" property: redaction never invents a label
|
||||
// for something it doesn't recognise.
|
||||
func TestRedactHomeDir_unrelatedPathUnchanged(t *testing.T) {
|
||||
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", "/var/lib/lark-cli")
|
||||
|
||||
path := "/etc/random/file.yml"
|
||||
if got := cmdpolicy.RedactHomeDir(path); got != path {
|
||||
t.Errorf("unrelated path = %q, want %q (unchanged)", got, path)
|
||||
}
|
||||
}
|
||||
|
||||
// Empty input round-trips. Callers (e.g. `config policy show` with
|
||||
// no yaml configured) rely on this.
|
||||
func TestRedactHomeDir_emptyStays(t *testing.T) {
|
||||
if got := cmdpolicy.RedactHomeDir(""); got != "" {
|
||||
t.Errorf("empty input = %q, want empty string", got)
|
||||
}
|
||||
}
|
||||
@@ -82,22 +82,27 @@ func Resolve(pluginRules []PluginRule, yamlPath string) (*platform.Rule, Resolve
|
||||
// vfs.Stat lets callers swap in an in-memory FS for tests. The
|
||||
// errors here surface as typed os.ErrNotExist when the file is
|
||||
// absent, just like a direct os.ReadFile call would.
|
||||
//
|
||||
// Error messages use the home-dir-redacted form so the user's
|
||||
// absolute path doesn't reach agents / CI logs through the
|
||||
// warnPolicyError stderr line.
|
||||
display := RedactHomeDir(yamlPath)
|
||||
if _, err := vfs.Stat(yamlPath); err != nil {
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
return nil, ResolveSource{Kind: SourceNone}, nil
|
||||
}
|
||||
return nil, ResolveSource{}, fmt.Errorf("stat policy yaml %q: %w", yamlPath, err)
|
||||
return nil, ResolveSource{}, fmt.Errorf("stat policy yaml %q: %w", display, err)
|
||||
}
|
||||
data, err := vfs.ReadFile(yamlPath)
|
||||
if err != nil {
|
||||
return nil, ResolveSource{}, fmt.Errorf("read policy yaml %q: %w", yamlPath, err)
|
||||
return nil, ResolveSource{}, fmt.Errorf("read policy yaml %q: %w", display, err)
|
||||
}
|
||||
rule, err := pyaml.Parse(data)
|
||||
if err != nil {
|
||||
return nil, ResolveSource{}, fmt.Errorf("policy yaml %q: %w", yamlPath, err)
|
||||
return nil, ResolveSource{}, fmt.Errorf("policy yaml %q: %w", display, err)
|
||||
}
|
||||
if err := ValidateRule(rule); err != nil {
|
||||
return nil, ResolveSource{}, fmt.Errorf("policy yaml %q: %w", yamlPath, err)
|
||||
return nil, ResolveSource{}, fmt.Errorf("policy yaml %q: %w", display, err)
|
||||
}
|
||||
return rule, ResolveSource{Kind: SourceYAML, Name: yamlPath}, nil
|
||||
}
|
||||
|
||||
@@ -116,6 +116,22 @@ func installOne(name string, p platform.Plugin, result *InstallResult) error {
|
||||
return capsErr
|
||||
}
|
||||
|
||||
// FailurePolicy is a closed enum. An out-of-range value almost
|
||||
// always means the plugin author shipped FailurePolicy(2)/etc. by
|
||||
// mistake, and the host's switch on caps.FailurePolicy below would
|
||||
// silently treat the unknown value as FailOpen — defeating the
|
||||
// security boundary the policy was meant to express. Reject up
|
||||
// front with ReasonInvalidCapability (classified as
|
||||
// untrusted-config, so the abort is unconditional).
|
||||
if caps.FailurePolicy != platform.FailOpen && caps.FailurePolicy != platform.FailClosed {
|
||||
return &PluginInstallError{
|
||||
PluginName: name,
|
||||
ReasonCode: ReasonInvalidCapability,
|
||||
Reason: fmt.Sprintf("FailurePolicy=%d is not a recognised value (expected FailOpen or FailClosed)",
|
||||
caps.FailurePolicy),
|
||||
}
|
||||
}
|
||||
|
||||
// Strict consistency check: Restricts=true must pair with
|
||||
// FailClosed (design hard-constraint #6).
|
||||
if caps.Restricts && caps.FailurePolicy != platform.FailClosed {
|
||||
|
||||
Reference in New Issue
Block a user