mirror of
https://github.com/larksuite/cli.git
synced 2026-07-06 00:06:28 +08:00
* feat(extension): introduce Plugin / Hook framework with command pruning
Add a single public extension contract under extension/platform: integrators
implement the Plugin interface and register Observers, Wrappers, Lifecycle
handlers, and pruning Rules through the Registrar in one Install call.
Command pruning:
- Rule (Allow / Deny / MaxRisk / Identities) with doublestar globs
- 4-axis AND evaluation, parent-group aggregation, unknown-risk allow
- Sources: Plugin.Restrict (single-rule) and ~/.lark-cli/policy.yml
- Plugin path is fail-closed (envelope on rule error / multiple Restrict);
yaml path is fail-open (warning, CLI continues)
- strict-mode stubs now also write the denial annotation so the hook
layer's denial guard physically isolates Wrap chains on them
- HOME path never leaked through policy_source label
Hook framework:
- Observer (panic-safe, Before/After), Wrapper (middleware, may short-circuit
via AbortError), Lifecycle (Startup + Shutdown only)
- Recover guards every plugin entry point: Capabilities(), Install(),
Wrapper factory composition AND inner Handler, Lifecycle handlers
- namespacedWrap copies AbortError so a plugin's package-level sentinel
is never mutated across concurrent invocations
- Selector unknown-risk uniform: ByExactRisk / ByWrite / ByReadOnly never
match unannotated commands; safety-side hooks opt in via
ByWrite().Or(ByUnknownRisk())
Bootstrap orchestration (cmd/build.go + cmd/policy.go):
- InstallAll uses a staging Registrar + atomic commit
- FailClosed plugin install / Plugin.Restrict conflict / Startup handler
failure each install a structured envelope guard at every dispatch path
- walkGuard neutralises every cobra bypass we know of (PersistentPreRunE
first-wins, ValidateArgs, ParseFlags, legacyArgs, __complete /
__completeNoDesc, non-runnable groups, required-arg subcommands)
- cmd/root.go::Execute calls hook.Emit(Shutdown, runErr) after
rootCmd.Execute; isCompletionCommand skips both __complete and
__completeNoDesc so Tab completion never triggers Shutdown handlers
Capabilities consistency:
- Restricts=true must declare FailurePolicy=FailClosed
- RequiredCLIVersion (semver constraint) is validated against build.Version;
a malformed constraint is treated as untrusted-config and aborts
unconditionally, regardless of FailurePolicy (DEV builds included)
JSON envelope contract:
- error.type closed enum: pruning / strict_mode / hook / plugin_install /
plugin_conflict / plugin_lifecycle
- reason_code closed enums per type, all referenced by structured tests
Bootstrap surfaces (new user commands):
- lark-cli config policy show -- JSON view of the active Rule + source
- lark-cli config policy validate -- parse + schema + glob check, no apply
Coverage:
- extension/platform: every public type has a unit test
- internal/{pruning,hook,platformhost,policydecision,cmdmeta}: full coverage
of denial guard isolation, AbortError sentinel safety, observer panic
safety, lifecycle error/panic typing, staging atomic rollback
- cmd/plugin_integration_test.go: end-to-end through buildInternal with
synthetic and real command trees
- cmd/install_guard_test.go: walkGuard covers auth / config / __complete /
__completeNoDesc / non-runnable parents
* fix(pruning): deny stub must override Args + PersistentPreRunE
The pruning denyStub and the strict-mode stub previously only swapped
RunE plus Hidden + DisableFlagParsing. Cobra's dispatch order means
several pre-RunE gates can fire BEFORE the stub's RunE ever runs:
1. Args validator: shortcut commands often declare cobra.NoArgs.
With DisableFlagParsing=true the user's `--doc xxx --mode append`
looks like positional args, so ValidateArgs surfaces a usage
error instead of the pruning / strict_mode envelope. Observer
hooks also miss the dispatch entirely.
2. Parent PersistentPreRunE: cmd/auth/auth.go declares a
PersistentPreRunE that returns external_provider when env
credentials are set. Cobra's "first PersistentPreRunE wins
walking up from the leaf" then short-circuits with
external_provider instead of the leaf's denial envelope.
Both stubs now also set:
- Args = cobra.ArbitraryArgs (bypass gate 1)
- PersistentPreRunE = no-op leaf hook (bypass gate 2)
- PreRunE / PreRun / PersistentPreRun = nil (defensive)
Effect: dispatch reaches the wrapped RunE, observers fire, the real
pruning / strict_mode envelope is emitted regardless of credential
provider or flag count.
Adds regression tests covering both gates on both stub paths.
* fix(config): policy subcommand bypasses parent's credential check
cmd/config/config.go::NewCmdConfig declares a PersistentPreRunE that
calls f.RequireBuiltinCredentialProvider; with env credentials set,
it returns external_provider for every config subcommand.
`config policy show` and `config policy validate` are READ-ONLY
diagnostic commands -- they inspect or parse the user-layer rule
without touching credentials. They MUST work regardless of which
credential provider is active, otherwise users on env-credential
deployments cannot debug their policy.
Same shape as the codex C11/C13 fix: install a no-op leaf-level
PersistentPreRunE on the `policy` group so cobra's "first walking up
from leaf" rule picks ours over the config parent's.
Regression caught by divergent e2e (F1-F6 all returned external_provider
before this fix; all pass after). Adds a unit test pinning the
PersistentPreRunE override.
* feat(shortcuts): tag service groups with cmdmeta.Domain
RegisterShortcutsWithContext now calls cmdmeta.SetDomain on each
service-level cobra.Command (im, docs, drive, calendar, ...) so the
business-domain axis is actually populated on every shortcut leaf via
parent-chain inheritance.
Before this change, platform.ByDomain("docs") never matched any
command: the domain annotation was unset across the entire shortcut
tree, so the selector's d != "" guard always failed and risk-style
selectors silently degraded to no-op.
The SetDomain call is placed AFTER the create-or-reuse branch so it
fires whether the service command was freshly created here or had
already been added by cmd/service/service.go's OpenAPI auto-
registration (which runs first and creates im, drive, calendar, etc.).
Without this placement only pure-shortcut services like docs would
have been tagged.
Adds a regression test asserting:
- service-group cobra.Command carries the cmdmeta.domain annotation
- leaf shortcuts inherit the domain via parent-chain walk
* feat(diagnostic): add unconditionally allowed command paths for introspection
* feat(plugins): add diagnostic command to inspect installed plugins and their contributions
* fix(cli): surface unknown_subcommand error instead of silent help fallback
When a user passed an unknown subcommand or shortcut (e.g. `lark-cli drive
+bogus`), cobra returned `flag.ErrHelp` for the non-runnable group command,
printed the parent help, and exited 0. AI agents couldn't distinguish a
typo from an intentional help request.
Install a tree-wide guard that attaches a RunE to every group command
without its own Run/RunE. The RunE forwards no-args invocations to help
(preserving prior behavior) and emits a structured unknown_subcommand
ExitError (exit 2) listing available subcommands when args are present.
* refactor(envelope): rename error.type pruning/strict_mode to command_denied
The envelope's `type` field was leaking implementation terms ("pruning",
"strict_mode") that describe enforcement mechanism rather than the user-
facing semantic. It also duplicated `detail.layer`, and forced consumers
to branch on two values for the same conceptual error ("a command was
denied by policy").
Collapse both into a single semantic type "command_denied". The
enforcement layer ("pruning" / "strict_mode") is preserved in
`detail.layer` so debugging and per-layer diagnostics still work.
* feat(platform): fail closed on unannotated/invalid risk when a Rule is active
The pruning engine used to treat any command without a risk annotation as
ALLOW even when a Rule with MaxRisk was set, and would silently skip the
MaxRisk comparison whenever the command's risk string was outside the
closed taxonomy. Both gaps let an unannotated or typo'd write command
slip past an "agent read-only" pruning rule.
Engine now denies before any other axis when a Rule is registered:
- reason_code "risk_not_annotated" for commands with no risk
- reason_code "risk_invalid" for commands whose risk is outside
the read | write | high-risk-write
taxonomy (e.g. typo "wrtie")
Main-flow is preserved: a nil Rule still returns Allowed=true
unconditionally, so a CLI with no pruning plugin behaves identically to
before. ByUnknownRisk() is removed from the public surface since the
Unknown state is no longer reachable through risk-based selectors when
any Rule is active; safety-side widening composition is no longer needed.
* chore(config): hide diagnostic policy/plugins commands from --help
`config policy show`, `config policy validate`, and `config plugins show`
are local-introspection-only commands kept behind the pruning
diagnostic whitelist so operators can always inspect why a command was
denied. They do not need to surface in `--help` for AI agents and were
contributing to help noise.
Hide the `policy` and `plugins` parent groups and both `show` /
`validate` leaves. Commands remain callable by exact name and continue
to bypass user-layer pruning via diagnosticPaths.
* style: gofmt
* fix(platform): nil Selector honours None contract; reject multi-doc policy yaml
- selector.go: And/Or/Not now treat nil Selector as None() per godoc,
preventing runtime panic when composed selectors are invoked.
- schema.go: Parse rejects multi-document YAML input so a stray '---'
separator can't silently drop trailing policy constraints.
* chore: go mod tidy
* feat(extension/platform): plugin SDK with policy engine, hooks, and Builder
Introduces extension/platform — the in-process plugin SDK external
Go forks of lark-cli use to extend or restrict the command surface.
Plugins compile in via blank import; there is no dynamic loading
and no RPC isolation.
Public SDK (extension/platform):
- Plugin interface (Name / Version / Capabilities / Install).
- Registrar verbs: Observe, Wrap, On, Restrict.
- Hook types: Observer (side-effect, panic-safe, fires Before/After
RunE), Wrapper (middleware, may short-circuit via AbortError),
LifecycleHandler (Startup / Shutdown), Selector with nil-safe
And/Or/Not composition.
- Risk / Identity are defined string types with closed taxonomies;
ParseRisk / ParseIdentity convert raw strings with the
absent-vs-invalid distinction the engine relies on.
- Builder ergonomic constructor (NewPlugin().Observer().Wrap()
...MustBuild()) that enforces name/hookName grammar, hookName
uniqueness, and the Restrict ↔ FailClosed pairing regardless of
call order.
- Invocation is a read-only interface; the framework's concrete
invocation type lives in internal/hook so plugins cannot
fabricate denial / strict-mode / identity state. Args() returns
a defensive copy on every call so hook mutation cannot leak
into the original RunE.
- CommandDeniedError + AbortError carry structured fields for the
closed `command_denied` / `hook` envelope contract.
- ResetForTesting gated behind //go:build testing.
- README + godoc examples (Observer / Wrapper / Restrict) + two
runnable example forks (audit-observer, readonly-policy).
Host (internal/platform, internal/hook, internal/cmdpolicy):
- InstallAll: staged plugin registration with atomic commit, panic
isolation, FailOpen / FailClosed semantics, RequiredCLIVersion
semver check, single-Restrict invariant, duplicate-plugin-name
detection.
- hook.Install wraps every runnable cmd.RunE with:
Before observers (panic-safe) → denial guard → composed Wrap
chain → original RunE → After observers (always fire, even on
err). Denied commands physically bypass the Wrap chain so a
plugin Wrapper cannot suppress or rewrite a denial; observers
still see the attempt for audit.
- Recover shim around plugin Wrappers converts panics (including
the factory call) into a structured `hook` envelope with
reason_code=panic; namespacing shim attributes AbortError to
the namespaced hook name.
- cmdpolicy (renamed from internal/pruning) is the user-layer
command policy engine: walks the cobra tree, evaluates each
runnable command against a Rule's four-axis filter (Allow /
Deny / MaxRisk / Identities), produces parent-group aggregate
denials, and installs denyStubs. Rule.AllowUnannotated opts out
of the unannotated-deny gate for gradual adoption; risk_invalid
typos always deny with an edit-distance "did you mean"
suggestion.
- Strict-mode stub in cmd/prune.go composes the shared
detail.* / wrapped CommandDeniedError shape via cmdpolicy
helpers (BuildDenialError / CommandDeniedFromDenial /
DenialDetailMap), so command_denied envelopes from strict-mode
and user-layer policy carry the same closed-enum fields
(detail.layer / reason_code / policy_source). The historical
short Message + independent Hint are preserved unchanged.
- cmdpolicy/yaml: structural parsing of ~/.lark-cli/policy.yml
with KnownFields strict mode, including allow_unannotated.
- `config policy show` / `config policy validate` and the plugin
inventory diagnostic surface the resolved Rule (allow,
deny, max_risk, identities, allow_unannotated) and the hook
contributions per plugin.
Envelope contract (docs/extension/reason-codes.md):
- error.type is a closed set: command_denied, hook, plugin_install,
plugin_conflict, plugin_lifecycle.
- reason_code is a closed enum per error.type, dispatched on by
external agents and CI integrations.
- detail.layer = "policy" | "strict_mode" attributes the rejection.
Build / CI:
- Makefile unit-test / vet / coverage and ci.yml fast-gate +
unit-test + coverage now pass -tags testing so register_testing.go
is visible; ./extension/... is in the package list so the SDK's
own tests actually run.
- fmt-check and examples-build Makefile targets.
- bmatcuk/doublestar/v4 added as a direct dependency for `**` glob
matching in Rule.Allow / Rule.Deny.
Author-facing material:
- docs/extension/ (quickstart, plugin-author-guide, reason-codes)
is provided in the working tree but kept out of git tracking
per repo convention (.gitignore covers docs/).
Change-Id: I3b8ecc2923bd54c2dff19e5dce8a0855a6f9e703
* feat(extension/platform): plugin SDK with policy engine, hooks, and Builder
Introduces extension/platform — the in-process plugin SDK external
Go forks of lark-cli use to extend or restrict the command surface.
Plugins compile in via blank import; there is no dynamic loading
and no RPC isolation.
Public SDK (extension/platform):
- Plugin interface (Name / Version / Capabilities / Install).
- Registrar verbs: Observe, Wrap, On, Restrict.
- Hook types: Observer (side-effect, panic-safe, fires Before/After
RunE), Wrapper (middleware, may short-circuit via AbortError),
LifecycleHandler (Startup / Shutdown), Selector with nil-safe
And/Or/Not composition.
- Risk / Identity are defined string types with closed taxonomies;
ParseRisk / ParseIdentity convert raw strings with the
absent-vs-invalid distinction the engine relies on.
- Builder ergonomic constructor (NewPlugin().Observer().Wrap()
...MustBuild()) that enforces name/hookName grammar, hookName
uniqueness, and the Restrict ↔ FailClosed pairing regardless of
call order.
- Invocation is a read-only interface; the framework's concrete
invocation type lives in internal/hook so plugins cannot
fabricate denial / strict-mode / identity state. Args() returns
a defensive copy on every call so hook mutation cannot leak
into the original RunE.
- CommandDeniedError + AbortError carry structured fields for the
closed `command_denied` / `hook` envelope contract.
- ResetForTesting gated behind //go:build testing.
- README + godoc examples (Observer / Wrapper / Restrict) + two
runnable example forks (audit-observer, readonly-policy).
Host (internal/platform, internal/hook, internal/cmdpolicy):
- InstallAll: staged plugin registration with atomic commit, panic
isolation, FailOpen / FailClosed semantics, RequiredCLIVersion
semver check, single-Restrict invariant, duplicate-plugin-name
detection.
- hook.Install wraps every runnable cmd.RunE with:
Before observers (panic-safe) → denial guard → composed Wrap
chain → original RunE → After observers (always fire, even on
err). Denied commands physically bypass the Wrap chain so a
plugin Wrapper cannot suppress or rewrite a denial; observers
still see the attempt for audit.
- Recover shim around plugin Wrappers converts panics (including
the factory call) into a structured `hook` envelope with
reason_code=panic; namespacing shim attributes AbortError to
the namespaced hook name.
- cmdpolicy (renamed from internal/pruning) is the user-layer
command policy engine: walks the cobra tree, evaluates each
runnable command against a Rule's four-axis filter (Allow /
Deny / MaxRisk / Identities), produces parent-group aggregate
denials, and installs denyStubs. Rule.AllowUnannotated opts out
of the unannotated-deny gate for gradual adoption; risk_invalid
typos always deny with an edit-distance "did you mean"
suggestion.
- Strict-mode stub in cmd/prune.go composes the shared
detail.* / wrapped CommandDeniedError shape via cmdpolicy
helpers (BuildDenialError / CommandDeniedFromDenial /
DenialDetailMap), so command_denied envelopes from strict-mode
and user-layer policy carry the same closed-enum fields
(detail.layer / reason_code / policy_source). The historical
short Message + independent Hint are preserved unchanged.
- cmdpolicy/yaml: structural parsing of ~/.lark-cli/policy.yml
with KnownFields strict mode, including allow_unannotated.
- `config policy show` / `config policy validate` and the plugin
inventory diagnostic surface the resolved Rule (allow,
deny, max_risk, identities, allow_unannotated) and the hook
contributions per plugin.
Envelope contract (docs/extension/reason-codes.md):
- error.type is a closed set: command_denied, hook, plugin_install,
plugin_conflict, plugin_lifecycle.
- reason_code is a closed enum per error.type, dispatched on by
external agents and CI integrations.
- detail.layer = "policy" | "strict_mode" attributes the rejection.
Build / CI:
- Makefile unit-test / vet / coverage and ci.yml fast-gate +
unit-test + coverage now pass -tags testing so register_testing.go
is visible; ./extension/... is in the package list so the SDK's
own tests actually run.
- fmt-check and examples-build Makefile targets.
- bmatcuk/doublestar/v4 added as a direct dependency for `**` glob
matching in Rule.Allow / Rule.Deny.
Author-facing material:
- docs/extension/ (quickstart, plugin-author-guide, reason-codes)
is provided in the working tree but kept out of git tracking
per repo convention (.gitignore covers docs/).
Change-Id: I3b8ecc2923bd54c2dff19e5dce8a0855a6f9e703
* refactor(policy): remove validate command and update diagnostics
* 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
* fix(extension/platform): address CodeRabbit review comments + CI gofmt
- hook/install: propagate wrapper-injected ctx to invokeOriginal so
RunE/Run see context values added by upstream Wrappers
- hook/testing: SetStderrForTesting returns a restore func; tests now
defer it via t.Cleanup to avoid cross-test sink leakage
- cmdpolicy/active: deep-copy ActivePolicy.Rule on SetActive/GetActive
so callers can't mutate the stored global through shared slices
- platform/inventory: deep-copy Inventory + nested Plugins / HookEntry
/ RuleView slices on SetActiveInventory / GetActiveInventory
- platform/staging: Restrict clones the plugin-supplied Rule before
retaining it so the plugin can't mutate it after Install returns
- platform/version: reject RequiredCLIVersion with more than three
numeric components instead of silently truncating 1.2.3.4 to 1.2.3
- cmd/platform_bootstrap: clear cmdpolicy.SetActive on yaml resolver
error so config policy show doesn't surface a stale rule
- cmd/platform_bootstrap_test: tmpHome pins LARKSUITE_CLI_CONFIG_DIR
so host env can't bleed into the policy test fixtures
- cmdpolicy/apply: installDenyStub returns bool; Apply count no longer
over-reports when strict-mode short-circuits the install
- cmdpolicy/engine: aggregateParents now returns the runnable hybrid's
own denial status when all children are placeholder branches
- cmdpolicy/resolver_test: use t.TempDir()-rooted missing path instead
of hardcoded /nonexistent for hermetic missing-file assertion
- cmd/config/plugins: empty-inventory branch emits total: 0 so the
JSON schema stays stable across populated/empty cases
- cmd/platform_guards_test: select leaf by RunE != nil (not Runnable)
so the test doesn't nil-deref on Run-only commands
- gofmt run on previously committed cmdpolicy/path*.go (CI fast-gate)
* fix(cmdpolicy): replace filepath.Abs with filepath.Clean for lint policy
The depguard / forbidigo rule blocks filepath.Abs in internal/ on the
grounds that it accesses the filesystem (Getwd) directly. Switch
RedactHomeDir + foldPrefix to operate on filepath.Clean strings; real
callers pass already-absolute paths (resolver builds yamlPath via
filepath.Join on the absolute config root), so the redaction outcome
is unchanged for production inputs. Relative inputs fall through to
the unchanged branch — filepath.Rel rejects the mixed-absoluteness
case with an error, which the foldPrefix helper already treats as
"not a hit".
* refactor(cmdpolicy): pure Resolve + drop path redaction & verbose comments
- Resolve becomes a pure function; I/O moves to LoadYAMLPolicy so
precedence selection can be unit-tested without vfs mocks
- ActivePolicy drops YAMLPath; config policy show JSON loses yaml_path
and yaml_shadowed (and the TOCTOU stat that surfaced them)
- RedactHomeDir and path_test.go removed: the home-dir folding was only
earning its keep through the now-deleted yaml_path field
- cmd/build.go bootstrap block trimmed from 71 to 39 lines by cutting
PR-rationale comments; one note kept for the fail-CLOSED-vs-fail-OPEN
business rule
- cmd/config/config.go: parent Long no longer hard-codes hidden command
hints, matching their Hidden:true intent
Change-Id: Icfbb818ce3ef523c63286bfbed34c49be08ed6a2
* refactor(platform): drop StrictMode/Identity from Invocation interface
These two accessors were documented in the public SDK as "After observers
always see ok=true" but the framework never plumbed values to them, so they
always returned ("", false). Zero internal/example/test callers; a plugin
author trusting the doc would silently get wrong behaviour.
Identity is also fundamentally unsuited for Before observers (per-command
identity resolves inside RunE via f.AuthFor, after Before fires). StrictMode
is a global value better placed on a Framework/Environment interface than
per-Invocation. Removing is non-breaking now (no callers); adding later is
non-breaking too.
Change-Id: Ice200543e9bca3bda759ad98a6e34a56df69e915
* fix(prune): preserve original metadata on strict-mode denial stubs
strictModeStubFrom built a fresh *cobra.Command from scratch, dropping
the original command's annotations (risk_level, lark:supportedIdentities,
cmdmeta.domain) and help text. cobraCommandView is a live proxy walking
parent annotations, so after the Remove+Add replacement, audit observers
firing on a strict-mode-denied command saw Cmd().Risk()=("",false) and
Cmd().Identities()=nil -- breaking the first-class use case for
audit/compliance plugins.
Copy child.Annotations into the stub (stamping the denial annotations on
top) and propagate Short/Long for help-text parity with
cmdpolicy/apply.go::installDenyStub, which preserves these by virtue of
mutating in place.
Regression test asserts risk_level / supportedIdentities / Short / Long
all survive replacement, alongside the denial annotations.
Change-Id: I19810a34575996344b63e839066888c154d69335
* chore(platform): align docs with implementation; fold home in yaml warnings
Followup cleanup to the previous three refactor commits, addressing review
fallout where public docs / examples / contract notes still pointed at
deleted symbols or unimplemented designs:
- cmd/build.go: Build() docstring now mentions the plugin install + Startup
emit side effects; Shutdown only fires on Execute path
- extension/platform/doc.go, lifecycle.go, invocation.go: drop references
to the deleted StrictMode/Identity methods, restore minimal Godoc on
Cmd/Args/Started
- extension/platform/view.go, cmd/platform_bootstrap.go,
internal/hook/install.go: rewrite "snapshot before pruning" promise to
match the actual contract (live view + strict-mode stub metadata
preservation)
- cmd/platform_guards_test.go: stubInvocation drops the two old methods
- cmd/platform_bootstrap.go: redactHome() last-mile folds $HOME -> ~ in
warnPolicyError so an os.PathError carrying the absolute policy path
does not leak the user's home dir to stderr / agent / CI logs
- examples/readonly-policy/README.md: drop yaml_path from the sample
`config policy show` envelope (the field was removed in 52cbb92)
Change-Id: I2874cc2cf9225dfa44a9c07b2449149181b387cb
* chore(build): drop vestigial -tags testing from Makefile and CI
The `testing` build tag was introduced in 461e3c6 to gate
extension/platform/register_testing.go (ResetForTesting); PR review
0efee93 then dropped the //go:build testing directive from that file
so downstream `go test ./...` would work without the tag, but never
cleaned the matching tag references out of Makefile and ci.yml.
The result: 8 places passing -tags testing for a tag that nothing in
the repo actually gates, plus a Makefile comment that confidently
claims a gate exists. Net behaviour is identical to omitting the flag;
the only effect is misleading developers into believing there is a
test-only surface separation.
Drop the flag from vet / unit-test / lint / coverage / deadcode (head
+ base worktree) and remove the misleading comment. ResetForTesting's
public-API exposure was the conscious trade-off taken in 0efee93 and
is left untouched.
Change-Id: If0cd78c87d4aec2a2533419fe75b01aae6b165fd
* feat(cmdpolicy): enrich denial Reason with attempted value + rule constraint
The envelope reason for command_denied previously told the caller WHAT
axis failed but not the concrete values on each side, so an AI agent
reading the envelope could not tell which command identity / risk /
path was attempted vs. which the rule permits. The natural temptation
was then to recommend modifying the rule -- exactly the wrong nudge,
since policy exists to prevent the agent from rewriting its own limits.
Each Reason now carries both the attempted value and the rule's
constraint:
identity_mismatch:
"command supports identities [user]; rule allows [bot]"
domain_not_allowed:
"command path \"drive/+upload\" not in allow list [docs/** contact/**]"
command_denylisted:
"command path \"docs/+delete-doc\" matched deny pattern \"docs/+delete-*\""
risk_too_high / write_not_allowed:
"command risk \"high-risk-write\" exceeds rule max_risk \"write\""
risk_not_annotated:
"command has no risk_level annotation; rule denies unannotated commands"
(drops the prescriptive "set allow_unannotated=true" hint -- that
belongs in docs, not in the engine's denial path)
Adds firstMatch() helper so command_denylisted can name the specific
glob that fired; matchesAny() now wraps firstMatch.
Regression test pins the substring contract per reason_code so future
"comment cleanup" cannot silently strip the values out again.
Change-Id: I17c7cc9411f58e3e43ade5e1ce875f3b7fe3e5ea
* fix(cmdpolicy): gofmt engine_test.go
CI fast-gate flagged the test added in 2eb0c2b as unformatted. Local
make unit-test had it cached; should have run `make vet` (which runs
gofmt-equivalent check via fmt-check) before pushing. Trivial 3-line
indent fix.
Change-Id: I42297ae59f607b97b32e976c9ec1c9ec4ab7de21
* feat(cmd): annotate risk_level on all hand-written cobra commands
Without this, any non-empty user-layer policy.yml (default
allow_unannotated=false) denies these commands with reason_code
risk_not_annotated -- bricking auth login, config init, profile use
etc. on first contact with a policy.
cmdpolicy/engine evaluation now resolves to the intended axis (deny
list / allow list / max_risk / identities) instead of failing closed
on the unannotated gate. Policy authors can write `max_risk: write`
or `allow: [auth/** config/** ...]` to express real intent.
Classification:
read auth status/check/list/scopes, config show /
policy show / plugins show, doctor, completion,
schema, profile list, event list/status/schema/
consume
write auth login/logout, config init/bind/remove/
default-as/strict-mode, profile add/remove/
rename/use, event stop/_bus, api (raw transit)
high-risk-write update (replaces the CLI binary; failure can
leave the install broken)
Notes:
- api standalone is conservatively `write`; per-call risk is unknown
at parse time (raw transit), so static gating only enforces the
write-class minimum.
- event _bus is the hidden IPC daemon forked by consume; standalone
invocation by users is not expected, but the annotation keeps
policy evaluation consistent with the other event subcommands.
- The two diagnostic-allowlisted commands (config policy show /
plugins show) still bypass the engine via diagnosticPaths; the
read annotation is for consistency with surrounding leaves.
---------
Co-authored-by: liangshuo-1 <266696938+liangshuo-1@users.noreply.github.com>
359 lines
13 KiB
Go
359 lines
13 KiB
Go
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
package hook
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"fmt"
|
|
|
|
"github.com/spf13/cobra"
|
|
|
|
"github.com/larksuite/cli/extension/platform"
|
|
"github.com/larksuite/cli/internal/output"
|
|
)
|
|
|
|
// Install wraps every runnable command's RunE so the hook chain fires
|
|
// around it. The wrapper is:
|
|
//
|
|
// Before observers (always run, panic-safe)
|
|
// denial guard:
|
|
// if cmd is denied -> denyStub returns its CommandDeniedError
|
|
// else -> compose(matched Wrappers)(originalRunE) runs
|
|
// After observers (always run, panic-safe, sees inv.Err)
|
|
//
|
|
// Critical invariants enforced here (constraint #2):
|
|
//
|
|
// - **Denied commands NEVER reach the Wrap chain.** The guard runs
|
|
// denyStub directly so no plugin Wrapper can suppress or rewrite
|
|
// the denial. Observers still fire (audit must see the attempted
|
|
// call), but Wrap is physically out of the path.
|
|
//
|
|
// - **After observers always fire**, even when RunE returned an
|
|
// error. Wrap short-circuits via AbortError get converted to
|
|
// *output.ExitError so cmd/root.go emits the right envelope.
|
|
//
|
|
// - **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
|
|
// 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
|
|
}
|
|
walkTree(root, func(c *cobra.Command) {
|
|
if !c.Runnable() {
|
|
return
|
|
}
|
|
if !c.HasParent() {
|
|
return // do not wrap the binary root itself
|
|
}
|
|
wrapRunE(c, reg, snapshot)
|
|
})
|
|
}
|
|
|
|
// CommandViewSource resolves a *cobra.Command into a CommandView. The
|
|
// default implementation returns a live view over the cobra node;
|
|
// strict-mode's replacement stubs (cmd/prune.go) carry the original
|
|
// command's annotations forward so the view keeps reporting accurate
|
|
// Risk / Identities / Domain after replacement.
|
|
type CommandViewSource interface {
|
|
View(cmd *cobra.Command) platform.CommandView
|
|
}
|
|
|
|
// wrapRunE replaces cmd.RunE with a hook-aware wrapper. The original
|
|
// RunE is captured by closure so the Wrapper chain can still call it
|
|
// as the innermost handler.
|
|
//
|
|
// The wrapper preserves the Run vs RunE distinction: cmd.Run is
|
|
// cleared because RunE wins when both are set and leaving a stale Run
|
|
// around is a hazard for future maintainers.
|
|
func wrapRunE(cmd *cobra.Command, reg *Registry, snapshot CommandViewSource) {
|
|
originalRunE := cmd.RunE
|
|
originalRun := cmd.Run
|
|
cmd.Run = nil
|
|
|
|
cmd.RunE = func(c *cobra.Command, args []string) error {
|
|
view := snapshot.View(c)
|
|
inv := newInvocation(view, args)
|
|
|
|
// Detect denial: a denied command's original RunE was already
|
|
// 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: cmdpolicy.Apply itself marks the command
|
|
// via cobra annotation; install reads the annotation directly.
|
|
populateInvocationDenial(inv, c)
|
|
|
|
ctx := c.Context()
|
|
if ctx == nil {
|
|
ctx = context.Background()
|
|
}
|
|
|
|
// === Before observers (panic-safe, always run) ===
|
|
for _, obs := range reg.MatchingObservers(view, platform.Before) {
|
|
runObserverSafe(ctx, obs, inv)
|
|
}
|
|
|
|
// === Denial guard ===
|
|
// If denied, run the originalRunE directly (it is the denyStub
|
|
// installed by cmdpolicy.Apply). The Wrap chain is bypassed.
|
|
var err error
|
|
if inv.DeniedByPolicy() {
|
|
err = invokeOriginal(ctx, c, args, originalRunE, originalRun)
|
|
} else {
|
|
// Compose matching Wrappers around the originalRunE. Each
|
|
// Wrapper is wrapped with a thin namespacing shim so any
|
|
// *AbortError returned has its HookName replaced with the
|
|
// framework-namespaced WrapperEntry.Name -- a plugin
|
|
// cannot impersonate another plugin's hook even by
|
|
// accident.
|
|
matched := reg.MatchingWrappers(view)
|
|
wrappers := make([]platform.Wrapper, 0, len(matched))
|
|
for _, w := range matched {
|
|
// Each plugin Wrapper is wrapped twice: once by the
|
|
// namespacing shim (AbortError attribution) and once
|
|
// by the panic shim (so a plugin panic becomes a
|
|
// structured hook envelope instead of crashing the
|
|
// process).
|
|
wrappers = append(wrappers, recoverWrap(w.Name, namespacedWrap(w.Name, w.Fn)))
|
|
}
|
|
composed := ComposeWrappers(wrappers)
|
|
// 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)
|
|
}
|
|
|
|
// Convert AbortError -> *output.ExitError so the envelope writer
|
|
// renders the structured "hook" type.
|
|
err = wrapAbortError(err)
|
|
|
|
inv.setErr(err)
|
|
|
|
// === After observers (panic-safe, always run, including
|
|
// when err != nil) ===
|
|
for _, obs := range reg.MatchingObservers(view, platform.After) {
|
|
runObserverSafe(ctx, obs, inv)
|
|
}
|
|
|
|
return err
|
|
}
|
|
}
|
|
|
|
// invokeOriginal runs whatever the original command logic was. If
|
|
// originalRunE is non-nil (the common case), use it; otherwise fall
|
|
// back to the Run variant. Commands without either are a programming
|
|
// error caught at registration time (cmd.Runnable() returns false).
|
|
//
|
|
// The wrapper-propagated ctx is set on cmd via SetContext *before* the
|
|
// inner RunE/Run is invoked, so any context values injected by an
|
|
// upstream Wrapper (auth tokens, request-scoped IDs, trace spans,
|
|
// cancellation deadlines) reach the original handler. Without this
|
|
// hand-off the inner handler would observe c.Context() — the
|
|
// pre-wrapper context — and silently lose every value the Wrap chain
|
|
// added.
|
|
//
|
|
// We restore the previous context on return so a single command's
|
|
// SetContext mutation cannot leak to sibling dispatches that share the
|
|
// same *cobra.Command pointer (cobra reuses the tree across calls in
|
|
// long-running embedders).
|
|
func invokeOriginal(ctx context.Context, c *cobra.Command, args []string, runE func(*cobra.Command, []string) error, run func(*cobra.Command, []string)) error {
|
|
prev := c.Context()
|
|
c.SetContext(ctx)
|
|
defer c.SetContext(prev)
|
|
|
|
if runE != nil {
|
|
return runE(c, args)
|
|
}
|
|
if run != nil {
|
|
run(c, args)
|
|
return nil
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// 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) {
|
|
defer func() {
|
|
if r := recover(); r != nil {
|
|
fmt.Fprintf(stderr(), "warning: hook %q panicked: %v\n", obs.Name, r)
|
|
}
|
|
}()
|
|
obs.Fn(ctx, inv)
|
|
}
|
|
|
|
// wrapAbortError converts *platform.AbortError into the equivalent
|
|
// *output.ExitError so cmd/root.go's envelope writer emits the right
|
|
// JSON structure (type="hook"). Non-AbortError values pass through
|
|
// unchanged.
|
|
func wrapAbortError(err error) error {
|
|
if err == nil {
|
|
return nil
|
|
}
|
|
var ab *platform.AbortError
|
|
if !errors.As(err, &ab) {
|
|
return err
|
|
}
|
|
return &output.ExitError{
|
|
Code: output.ExitValidation,
|
|
Detail: &output.ErrDetail{
|
|
Type: "hook",
|
|
Message: ab.Error(),
|
|
Detail: map[string]any{
|
|
"hook_name": ab.HookName,
|
|
"reason": ab.Reason,
|
|
"reason_code": "aborted",
|
|
"detail": ab.Detail,
|
|
},
|
|
},
|
|
Err: ab,
|
|
}
|
|
}
|
|
|
|
// recoverWrap wraps a Wrapper so any panic anywhere in the plugin's
|
|
// implementation -- including the wrapper FACTORY call (the
|
|
// `func(next Handler) Handler` step) and the inner Handler call -- is
|
|
// recovered and surfaced as a structured *output.ExitError with
|
|
// type="hook" and reason_code="panic". Without this guard, a panicking
|
|
// plugin would crash the entire CLI process and break the structured-
|
|
// error contract (downstream automation cannot parse a stack trace).
|
|
//
|
|
// The recovered panic keeps the fully-qualified hook name (the same
|
|
// namespacing as namespacedWrap below uses) so on-call can pinpoint
|
|
// the offending plugin without grepping logs.
|
|
//
|
|
// **Why the factory call is inside the deferred recover**: a plugin
|
|
// can write something like
|
|
//
|
|
// func(next Handler) Handler {
|
|
// state := mustInit() // panics on bad config
|
|
// return func(...) error { ... use state ... }
|
|
// }
|
|
//
|
|
// If `mustInit` panics, the panic happens during composition
|
|
// (ComposeWrappers -> ws[i](next)) which runs at invocation time inside
|
|
// wrapRunE. Without recovering this branch, the whole CLI crashes.
|
|
// We pay a tiny per-invocation cost (one factory call per command
|
|
// dispatch) in exchange for total panic isolation.
|
|
//
|
|
// **Factory-local state lifetime contract**: any value the plugin's
|
|
// outer factory captures (`state` in the example above) is now created
|
|
// PER INVOCATION of the wrapped command -- it is NOT a one-shot init
|
|
// the way Plugin.Install is. Plugins that need long-lived state (a
|
|
// connection pool, an LRU cache, a metrics counter) MUST hold it on
|
|
// the Plugin struct or in a package-level variable; relying on
|
|
// closure-local memoisation inside the wrapper factory will silently
|
|
// 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) {
|
|
defer func() {
|
|
if r := recover(); r != nil {
|
|
returned = &output.ExitError{
|
|
Code: output.ExitValidation,
|
|
Detail: &output.ErrDetail{
|
|
Type: "hook",
|
|
Message: fmt.Sprintf("hook %q panicked: %v", fullName, r),
|
|
Detail: map[string]any{
|
|
"hook_name": fullName,
|
|
"reason_code": "panic",
|
|
"reason": fmt.Sprintf("%v", r),
|
|
},
|
|
},
|
|
Err: fmt.Errorf("hook %q panic: %v", fullName, r),
|
|
}
|
|
}
|
|
}()
|
|
// Construct AFTER the recover is armed so a panicking
|
|
// factory becomes a hook envelope instead of a process
|
|
// crash.
|
|
inner := w(next)
|
|
return inner(ctx, inv)
|
|
}
|
|
}
|
|
}
|
|
|
|
// namespacedWrap wraps a plugin's Wrapper so any *platform.AbortError it
|
|
// returns is replaced with a fresh copy whose HookName is the
|
|
// framework-namespaced name (e.g. "policy-plugin.policy"). Plugin
|
|
// authors do not need to know their own plugin name; the framework
|
|
// attribution is authoritative.
|
|
//
|
|
// **Why a copy, not mutation**: an AbortError value may be shared
|
|
// across concurrent command invocations (e.g. a plugin's package-level
|
|
// sentinel). Mutating it would race; copy keeps each invocation's
|
|
// attribution isolated.
|
|
//
|
|
// **Why only top-level AbortError, not wrapped**: a wrapped AbortError
|
|
// in a chain via fmt.Errorf("...: %w", ab) would require rebuilding
|
|
// the entire chain to substitute the value. The simpler contract --
|
|
// "plugin returns AbortError directly to short-circuit" -- is what we
|
|
// document, so we only namespace the top-level case. Wrapped
|
|
// AbortErrors keep whatever HookName the plugin set; that is still
|
|
// surfaced unchanged by the envelope writer.
|
|
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 {
|
|
err := inner(ctx, inv)
|
|
if err == nil {
|
|
return nil
|
|
}
|
|
if ab, ok := err.(*platform.AbortError); ok {
|
|
copied := *ab
|
|
copied.HookName = fullName
|
|
return &copied
|
|
}
|
|
return err
|
|
}
|
|
}
|
|
}
|
|
|
|
// stderr returns the stderr writer the wrapper uses for safe warnings.
|
|
// Indirected through a func so tests can substitute it.
|
|
var stderr = func() interface{ Write(p []byte) (int, error) } {
|
|
// Avoid pulling os just for stderr access -- the real impl lives
|
|
// in install_default.go (see file). The function is overridable
|
|
// to keep test isolation tight.
|
|
return defaultStderr
|
|
}
|
|
|
|
// 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: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:policy_denied_source".
|
|
//
|
|
// This indirection lets us avoid an import cycle between hook and
|
|
// pruning packages.
|
|
func populateInvocationDenial(inv *invocation, c *cobra.Command) {
|
|
const layerKey = "lark:policy_denied_layer"
|
|
const sourceKey = "lark:policy_denied_source"
|
|
if c.Annotations == nil {
|
|
return
|
|
}
|
|
layer, ok := c.Annotations[layerKey]
|
|
if !ok || layer == "" {
|
|
return
|
|
}
|
|
source := c.Annotations[sourceKey]
|
|
inv.setDenial(layer, source)
|
|
}
|