diff --git a/cmd/build.go b/cmd/build.go index b7db05c2d..0e2a24d81 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -133,26 +133,13 @@ func buildInternal(ctx context.Context, inv cmdutil.InvocationContext, opts ...B installUnknownSubcommandGuard(rootCmd) - // Prune commands incompatible with strict mode. if mode := f.ResolveStrictMode(ctx); mode.IsActive() { pruneForStrictMode(rootCmd, mode) } - // Run the platform host: install registered plugins, collecting - // their Restrict() contributions and their hooks. FailClosed - // failures (and untrusted-config failures like restricts_mismatch) - // are abort-worthy: InstallAll returns an error in those cases. - // We honour that by installing a PersistentPreRunE that emits - // the structured envelope at command-dispatch time -- buildInternal - // itself cannot return an error without breaking its assembly - // contract, but cobra runs PersistentPreRunE before any RunE so - // the user sees the error on their very next invocation. installResult, installErr := installPluginsAndHooks(cfg.streams.ErrOut) if installErr != nil { installPluginInstallErrorGuard(rootCmd, installErr) - // Stop wiring more state from a failed install -- the rest of - // the function would only matter if the CLI is allowed to - // proceed normally, which it isn't. return f, rootCmd, nil } var pluginRules []cmdpolicy.PluginRule @@ -162,16 +149,9 @@ func buildInternal(ctx context.Context, inv cmdutil.InvocationContext, opts ...B registry = installResult.Registry } - // Apply user-layer command pruning: yaml + Plugin.Restrict. - // - // **Error policy splits by source**: - // - Plugin path (any pluginRules contributed): a validation or - // conflict error is a HARD failure -- the plugin author asked - // for a security policy, silently dropping it would leave the - // CLI more permissive than intended. Abort via the conflict - // guard so every command surfaces the structured envelope. - // - yaml-only path: stays fail-OPEN with a warning. A user typo - // in policy.yml must not lock them out of every command. + // Policy errors fail-CLOSED when a plugin contributed (security + // intent must not be silently dropped); yaml-only errors fail-OPEN + // with a warning so a typo can't lock the user out. if err := applyUserPolicyPruning(rootCmd, pluginRules); err != nil { if len(pluginRules) > 0 { installPluginConflictGuard(rootCmd, err) @@ -180,15 +160,6 @@ func buildInternal(ctx context.Context, inv cmdutil.InvocationContext, opts ...B warnPolicyError(cfg.streams.ErrOut, err) } - // Install hooks onto the (now-pruned) command tree and emit the - // Startup lifecycle event so Plugin.On(Startup) handlers can run. - // - // Startup handler error or panic is a HARD failure: a plugin's - // Startup logic is part of its install contract, and silently - // continuing would mean the plugin's invariants do not hold while - // the rest of its hooks (Wrap / Observe) still fire. Install the - // plugin_lifecycle guard and short-circuit so every subsequent - // dispatch surfaces the envelope. if registry != nil { if err := wireHooks(ctx, rootCmd, registry); err != nil { installPluginLifecycleErrorGuard(rootCmd, err) @@ -196,10 +167,6 @@ func buildInternal(ctx context.Context, inv cmdutil.InvocationContext, opts ...B } } - // Snapshot the plugin inventory so `config plugins show` can answer - // "what plugins / hooks / rules are currently in effect" without - // re-calling plugin methods at display time. recordInventory(installResult) - return f, rootCmd, registry } diff --git a/cmd/config/config.go b/cmd/config/config.go index ddcb10723..c99f6b482 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -14,11 +14,6 @@ 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 diff --git a/cmd/config/policy.go b/cmd/config/policy.go index bb1e38a3a..76b7374a2 100644 --- a/cmd/config/policy.go +++ b/cmd/config/policy.go @@ -4,9 +4,6 @@ package config import ( - "fmt" - "os" - "github.com/spf13/cobra" "github.com/larksuite/cli/internal/cmdpolicy" @@ -14,24 +11,13 @@ import ( "github.com/larksuite/cli/internal/output" ) -// NewCmdConfigPolicy returns the `config policy` group. Subcommands: -// -// show - print the resolved user-layer Rule + source + denied count -// -// The command writes a structured JSON envelope so AI agents and CI -// integrations can parse the result. func NewCmdConfigPolicy(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "policy", - Hidden: true, // diagnostic-only; kept callable, omitted from --help to reduce noise + Hidden: true, Short: "Inspect the user-layer command policy", - // The parent `config` group has a PersistentPreRunE that calls - // RequireBuiltinCredentialProvider, which returns external_provider - // when env credentials are set. `policy show` is a READ-ONLY - // diagnostic command and does not modify credentials, so it must - // work regardless of which credential provider is active. A - // leaf-level no-op PersistentPreRunE wins under cobra's "first - // walking up" rule and bypasses the parent check. + // Override parent's RequireBuiltinCredentialProvider check; this + // group is read-only diagnostic and must work under any provider. PersistentPreRunE: func(c *cobra.Command, _ []string) error { c.SilenceUsage = true return nil @@ -44,17 +30,8 @@ func NewCmdConfigPolicy(f *cmdutil.Factory) *cobra.Command { func newCmdConfigPolicyShow(f *cmdutil.Factory) *cobra.Command { return &cobra.Command{ Use: "show", - Hidden: true, // diagnostic-only; kept callable, omitted from --help to reduce noise - Short: "Show the active user-layer policy (Plugin.Restrict / yaml / none)", - Long: `Print the policy currently in effect after bootstrap, including: - - - source: "plugin:" / "yaml" / "none" - - rule: the resolved Rule (Allow / Deny / MaxRisk / Identities) - - yaml_path: the file location that was examined (informational) - - yaml_shadowed: true when a plugin Restrict overrides the yaml - -A "denied_paths" count reflects the number of commands the engine -marked as denied after father-group aggregation.`, + Hidden: true, + Short: "Show the active user-layer policy (plugin / yaml / none)", RunE: func(cmd *cobra.Command, args []string) error { return runConfigPolicyShow(f) }, @@ -64,9 +41,6 @@ marked as denied after father-group aggregation.`, func runConfigPolicyShow(f *cmdutil.Factory) error { 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(cmdpolicy.SourceNone), "note": "no policy recorded; bootstrap did not run pruning", @@ -74,20 +48,13 @@ 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) + sourceName := "" + if active.Source.Kind == cmdpolicy.SourcePlugin { + sourceName = active.Source.Name } out := map[string]any{ "source": string(active.Source.Kind), "source_name": sourceName, - "yaml_path": cmdpolicy.RedactHomeDir(active.YAMLPath), "denied_paths": active.DeniedPaths, } if active.Rule != nil { @@ -101,15 +68,6 @@ func runConfigPolicyShow(f *cmdutil.Factory) error { "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 == cmdpolicy.SourcePlugin && active.YAMLPath != "" { - if _, err := os.Stat(active.YAMLPath); err == nil { - out["yaml_shadowed"] = true - fmt.Fprintln(f.IOStreams.ErrOut, - "note: a plugin contributed Restrict(); yaml IGNORED") - } - } output.PrintJson(f.IOStreams.Out, out) return nil } diff --git a/cmd/config/policy_test.go b/cmd/config/policy_test.go index d4ec82361..05d8a180b 100644 --- a/cmd/config/policy_test.go +++ b/cmd/config/policy_test.go @@ -6,8 +6,6 @@ package config import ( "bytes" "encoding/json" - "os" - "path/filepath" "testing" "github.com/larksuite/cli/extension/platform" @@ -48,8 +46,7 @@ func TestConfigPolicyShow_NoActivePolicy(t *testing.T) { } // When bootstrap recorded an active plugin Rule, `show` emits the rule -// plus its source. yaml_shadowed is true when a yaml file exists but a -// plugin overrode it; verified separately below. +// plus its source. func TestConfigPolicyShow_PluginActive(t *testing.T) { cmdpolicy.ResetActiveForTesting() t.Cleanup(cmdpolicy.ResetActiveForTesting) @@ -95,34 +92,39 @@ func TestConfigPolicyShow_PluginActive(t *testing.T) { } } -// When a yaml file exists AND a plugin Rule won, show should warn the -// user "yaml IGNORED" so they're not surprised that their yaml is -// inert. -func TestConfigPolicyShow_YamlShadowedWarning(t *testing.T) { +// `source_name` must be empty when source=yaml. The yaml path is +// deliberately not surfaced (matches engine envelope convention, +// avoids leaking the user's home dir to AI agents / CI logs). The +// rule's "name:" field is the disambiguator users should rely on. +func TestConfigPolicyShow_YamlSourceNameIsEmpty(t *testing.T) { cmdpolicy.ResetActiveForTesting() t.Cleanup(cmdpolicy.ResetActiveForTesting) - dir := t.TempDir() - yamlPath := filepath.Join(dir, "policy.yml") - if err := os.WriteFile(yamlPath, []byte("name: shadowed\n"), 0o644); err != nil { - t.Fatalf("write yaml: %v", err) - } - cmdpolicy.SetActive(&cmdpolicy.ActivePolicy{ - Rule: &platform.Rule{Name: "plug"}, + Rule: &platform.Rule{Name: "my-yaml-rule"}, Source: cmdpolicy.ResolveSource{ - Kind: cmdpolicy.SourcePlugin, - Name: "plug", + Kind: cmdpolicy.SourceYAML, + Name: "/Users/alice/.lark-cli/policy.yml", }, - YAMLPath: yamlPath, }) - f, _, errOut := newPolicyTestFactory() + f, out, _ := newPolicyTestFactory() if err := runConfigPolicyShow(f); err != nil { t.Fatalf("show: %v", err) } - if !bytes.Contains(errOut.Bytes(), []byte("yaml IGNORED")) { - t.Errorf("expected 'yaml IGNORED' warning, got: %q", errOut.String()) + var got map[string]any + if err := json.Unmarshal(out.Bytes(), &got); err != nil { + t.Fatalf("not json: %v\n%s", err, out.String()) + } + if got["source"] != "yaml" { + t.Errorf("source = %v, want yaml", got["source"]) + } + if got["source_name"] != "" { + t.Errorf("source_name = %q, want empty (yaml path must not leak)", got["source_name"]) + } + // The path must not appear anywhere in the envelope. + if bytes.Contains(out.Bytes(), []byte("/Users/alice")) { + t.Errorf("envelope leaked yaml path: %s", out.String()) } } diff --git a/cmd/platform_bootstrap.go b/cmd/platform_bootstrap.go index 1b09ed53f..116101527 100644 --- a/cmd/platform_bootstrap.go +++ b/cmd/platform_bootstrap.go @@ -42,7 +42,7 @@ func applyUserPolicyPruning(rootCmd *cobra.Command, pluginRules []cmdpolicy.Plug yamlPath = "" } - rule, source, err := cmdpolicy.Resolve(pluginRules, yamlPath) + yamlRule, err := cmdpolicy.LoadYAMLPolicy(yamlPath) if err != nil { // Yaml-only failures are fail-OPEN at the caller (warn and // continue), but the active-policy snapshot is process-global @@ -53,11 +53,18 @@ func applyUserPolicyPruning(rootCmd *cobra.Command, pluginRules []cmdpolicy.Plug cmdpolicy.SetActive(nil) return err } + + rule, source, err := cmdpolicy.Resolve(cmdpolicy.Sources{ + PluginRules: pluginRules, + YAMLRule: yamlRule, + YAMLPath: yamlPath, + }) + if err != nil { + cmdpolicy.SetActive(nil) + return err + } if rule == nil { - cmdpolicy.SetActive(&cmdpolicy.ActivePolicy{ - Source: source, - YAMLPath: yamlPath, - }) + cmdpolicy.SetActive(&cmdpolicy.ActivePolicy{Source: source}) return nil } @@ -66,11 +73,9 @@ func applyUserPolicyPruning(rootCmd *cobra.Command, pluginRules []cmdpolicy.Plug 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 diff --git a/internal/cmdpolicy/active.go b/internal/cmdpolicy/active.go index b40d29289..488d641c1 100644 --- a/internal/cmdpolicy/active.go +++ b/internal/cmdpolicy/active.go @@ -18,8 +18,7 @@ import ( type ActivePolicy struct { Rule *platform.Rule Source ResolveSource - YAMLPath string // path examined, populated even when yaml was shadowed by a plugin Rule - DeniedPaths int // number of commands the engine marked as denied (post-aggregation) + DeniedPaths int // number of commands the engine marked as denied (post-aggregation) } var ( @@ -58,8 +57,8 @@ func GetActive() *ActivePolicy { } // cloneActivePolicy deep-copies the top-level struct plus the embedded -// Rule's slice fields. Other fields (Source, YAMLPath, DeniedPaths) -// are value types so the struct copy already disjoints them. +// Rule's slice fields. Other fields (Source, DeniedPaths) are value +// types so the struct copy already disjoints them. func cloneActivePolicy(in *ActivePolicy) *ActivePolicy { if in == nil { return nil diff --git a/internal/cmdpolicy/path.go b/internal/cmdpolicy/path.go index e090ed95a..fe0124db2 100644 --- a/internal/cmdpolicy/path.go +++ b/internal/cmdpolicy/path.go @@ -4,28 +4,15 @@ 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 // the pruning framework. Cobra's CommandPath() yields space-separated // segments ("lark-cli docs +update"); doublestar globs ("docs/**") require // slashes, so all internal lookups go through this conversion. -// -// Algorithm: -// -// 1. Collect cmd.Use first words from the command up to (but not including) -// the root, in reverse order. -// 2. Reverse the collection and join with "/". -// -// The root (the binary's own command, no parent) is stripped. For a command -// with no parent, the returned path is just its own Use word. func CanonicalPath(cmd *cobra.Command) string { if cmd == nil { return "" @@ -34,19 +21,15 @@ func CanonicalPath(cmd *cobra.Command) string { for c := cmd; c != nil && c.HasParent(); c = c.Parent() { parts = append(parts, useName(c)) } - // reverse for i, j := 0, len(parts)-1; i < j; i, j = i+1, j-1 { parts[i], parts[j] = parts[j], parts[i] } if len(parts) == 0 { - // orphan command -- return its own name so callers still see - // something stable. return useName(cmd) } return strings.Join(parts, "/") } -// useName extracts the first word of cmd.Use ("update [flags] " -> "update"). func useName(cmd *cobra.Command) string { name := cmd.Use if i := strings.IndexByte(name, ' '); i >= 0 { @@ -54,73 +37,3 @@ 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) → "" -// 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. -// -// The implementation operates on the cleaned strings (no -// `filepath.Abs`) because the depguard / forbidigo lint policy bans -// direct filesystem access from internal/. All real call sites pass -// already-absolute paths (`core.GetBaseConfigDir()` returns absolute -// when LARKSUITE_CLI_CONFIG_DIR or $HOME is set; resolver builds -// yamlPath via filepath.Join on that absolute root). A relative input -// simply falls through the prefix checks and is returned unchanged. -func RedactHomeDir(path string) string { - if path == "" { - return "" - } - clean := filepath.Clean(path) - - if rel, ok := foldPrefix(clean, core.GetBaseConfigDir()); ok { - if rel == "" { - return "" - } - return "/" + rel - } - - home, err := vfs.UserHomeDir() - if err != nil || home == "" { - return path - } - if rel, ok := foldPrefix(clean, home); ok { - if rel == "" { - return "~" - } - return "~/" + rel - } - return path -} - -// foldPrefix reports whether path lives at or beneath prefix; on hit -// it returns the slash-form relative tail (empty when path == prefix). -// filepath.Rel itself rejects the relative-vs-absolute mismatch case -// with an error, so a relative input against an absolute prefix (or -// vice versa) falls through to the "not a hit" branch — no extra -// validation needed. -func foldPrefix(path, prefix string) (string, bool) { - if prefix == "" { - return "", false - } - cleanPrefix := filepath.Clean(prefix) - rel, err := filepath.Rel(cleanPrefix, path) - if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - return "", false - } - if rel == "." { - return "", true - } - return filepath.ToSlash(rel), true -} diff --git a/internal/cmdpolicy/path_test.go b/internal/cmdpolicy/path_test.go deleted file mode 100644 index 939a86c09..000000000 --- a/internal/cmdpolicy/path_test.go +++ /dev/null @@ -1,57 +0,0 @@ -// 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() → "" (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 != "/policy.yml" { - t.Errorf("override path = %q, want /policy.yml", got) - } - - // A path that equals the config dir itself collapses to "". - if got := cmdpolicy.RedactHomeDir(tmp); got != "" { - t.Errorf("exact-prefix path = %q, want ", 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) - } -} diff --git a/internal/cmdpolicy/resolver.go b/internal/cmdpolicy/resolver.go index dea23b62c..d70335f58 100644 --- a/internal/cmdpolicy/resolver.go +++ b/internal/cmdpolicy/resolver.go @@ -13,9 +13,6 @@ import ( "github.com/larksuite/cli/internal/vfs" ) -// SourceKind describes which source contributed the active Rule. Surfaced -// by `config policy show` so users can tell at a glance whether their yaml -// is being shadowed by a plugin. type SourceKind string const ( @@ -24,88 +21,72 @@ const ( SourceNone SourceKind = "none" ) -// ResolveSource is the metadata about which rule won. type ResolveSource struct { Kind SourceKind - Name string // plugin name when Kind=plugin; file path when Kind=yaml; "" otherwise + Name string } -// PluginRule represents a single Restrict() contribution. The Hook surface -// (next milestone) will collect these via Plugin.Install -> r.Restrict; for -// now the package consumer (Bootstrap pipeline) just hands in the slice. -// -// More than one entry is a configuration error (single-rule policy) -- the -// resolver reports it as a typed error so the bootstrap can abort. type PluginRule struct { PluginName string Rule *platform.Rule } -// ErrMultipleRestricts is returned when 2+ plugins both contribute a Rule. -// The bootstrap pipeline must treat this as fail-closed (start-up abort); -// resolving by silent priority would mask a configuration mistake. +type Sources struct { + PluginRules []PluginRule + YAMLRule *platform.Rule + YAMLPath string +} + var ErrMultipleRestricts = errors.New("multiple plugins called Restrict; only one is permitted") -// Resolve picks the active Rule from the configured sources. Precedence: -// -// plugin contribution > yaml file at yamlPath > no rule -// -// pluginRules may be nil/empty. yamlPath may be "" (skip yaml). -// -// The chosen Rule is validated through ValidateRule before being returned -// -- bad MaxRisk strings, malformed globs, or unknown identities all -// abort the resolve with a typed error so the bootstrap pipeline can -// honour the plugin's FailurePolicy. A typo in a policy plugin must -// never silently fail-open by reaching the engine. -// -// The returned Rule pointer is owned by the caller; resolver does not -// retain a reference. -func Resolve(pluginRules []PluginRule, yamlPath string) (*platform.Rule, ResolveSource, error) { - switch len(pluginRules) { - case 0: - // fall through to yaml - case 1: - rule := pluginRules[0].Rule - if err := ValidateRule(rule); err != nil { - return nil, ResolveSource{}, fmt.Errorf("plugin %q rule invalid: %w", pluginRules[0].PluginName, err) - } - return rule, ResolveSource{Kind: SourcePlugin, Name: pluginRules[0].PluginName}, nil - default: - names := make([]string, len(pluginRules)) - for i, pr := range pluginRules { +// Resolve picks by precedence: plugin > yaml > none. Pure function; load +// yaml via LoadYAMLPolicy first. Winner is validated. +func Resolve(s Sources) (*platform.Rule, ResolveSource, error) { + if len(s.PluginRules) > 1 { + names := make([]string, len(s.PluginRules)) + for i, pr := range s.PluginRules { names[i] = pr.PluginName } return nil, ResolveSource{}, fmt.Errorf("%w: %v", ErrMultipleRestricts, names) } - if yamlPath != "" { - // 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", display, err) + if len(s.PluginRules) == 1 { + pr := s.PluginRules[0] + if err := ValidateRule(pr.Rule); err != nil { + return nil, ResolveSource{}, fmt.Errorf("plugin %q rule invalid: %w", pr.PluginName, err) } - data, err := vfs.ReadFile(yamlPath) - if err != nil { - return nil, ResolveSource{}, fmt.Errorf("read policy yaml %q: %w", display, err) + return pr.Rule, ResolveSource{Kind: SourcePlugin, Name: pr.PluginName}, nil + } + + if s.YAMLRule != nil { + if err := ValidateRule(s.YAMLRule); err != nil { + return nil, ResolveSource{}, fmt.Errorf("policy yaml %q: %w", s.YAMLPath, err) } - rule, err := pyaml.Parse(data) - if err != nil { - 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", display, err) - } - return rule, ResolveSource{Kind: SourceYAML, Name: yamlPath}, nil + return s.YAMLRule, ResolveSource{Kind: SourceYAML, Name: s.YAMLPath}, nil } return nil, ResolveSource{Kind: SourceNone}, nil } + +// LoadYAMLPolicy returns (nil, nil) when path is empty or file is absent, +// so callers can pass the result straight into Sources.YAMLRule. +func LoadYAMLPolicy(path string) (*platform.Rule, error) { + if path == "" { + return nil, nil + } + if _, err := vfs.Stat(path); err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + return nil, fmt.Errorf("stat policy yaml %q: %w", path, err) + } + data, err := vfs.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("read policy yaml %q: %w", path, err) + } + rule, err := pyaml.Parse(data) + if err != nil { + return nil, fmt.Errorf("policy yaml %q: %w", path, err) + } + return rule, nil +} diff --git a/internal/cmdpolicy/resolver_test.go b/internal/cmdpolicy/resolver_test.go index d4ff7db0c..1631cb6c7 100644 --- a/internal/cmdpolicy/resolver_test.go +++ b/internal/cmdpolicy/resolver_test.go @@ -15,9 +15,9 @@ import ( func TestResolve_singlePluginWins(t *testing.T) { rule := &platform.Rule{Name: "secaudit"} - got, src, err := cmdpolicy.Resolve([]cmdpolicy.PluginRule{ - {PluginName: "secaudit", Rule: rule}, - }, "") + got, src, err := cmdpolicy.Resolve(cmdpolicy.Sources{ + PluginRules: []cmdpolicy.PluginRule{{PluginName: "secaudit", Rule: rule}}, + }) if err != nil { t.Fatalf("Resolve err: %v", err) } @@ -27,17 +27,13 @@ func TestResolve_singlePluginWins(t *testing.T) { } func TestResolve_pluginShadowsYaml(t *testing.T) { - dir := t.TempDir() - yamlPath := filepath.Join(dir, "policy.yml") - if err := os.WriteFile(yamlPath, []byte("name: from-yaml\n"), 0o644); err != nil { - t.Fatalf("write yaml: %v", err) - } - pluginRule := &platform.Rule{Name: "from-plugin"} - got, src, err := cmdpolicy.Resolve( - []cmdpolicy.PluginRule{{PluginName: "secaudit", Rule: pluginRule}}, - yamlPath, - ) + yamlRule := &platform.Rule{Name: "from-yaml"} + got, src, err := cmdpolicy.Resolve(cmdpolicy.Sources{ + PluginRules: []cmdpolicy.PluginRule{{PluginName: "secaudit", Rule: pluginRule}}, + YAMLRule: yamlRule, + YAMLPath: "/some/policy.yml", + }) if err != nil { t.Fatalf("Resolve err: %v", err) } @@ -47,49 +43,24 @@ func TestResolve_pluginShadowsYaml(t *testing.T) { } func TestResolve_yamlWhenNoPlugin(t *testing.T) { - dir := t.TempDir() - yamlPath := filepath.Join(dir, "policy.yml") - if err := os.WriteFile(yamlPath, []byte("name: from-yaml\nmax_risk: read\n"), 0o644); err != nil { - t.Fatalf("write yaml: %v", err) - } - - got, src, err := cmdpolicy.Resolve(nil, yamlPath) + yamlRule := &platform.Rule{Name: "from-yaml", MaxRisk: "read"} + got, src, err := cmdpolicy.Resolve(cmdpolicy.Sources{ + YAMLRule: yamlRule, + YAMLPath: "/some/policy.yml", + }) if err != nil { t.Fatalf("Resolve err: %v", err) } 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) { - // A guaranteed-missing path under t.TempDir() keeps the test - // hermetic — a stray `/nonexistent/policy.yml` could in principle - // exist on some sandbox runners and make the assertion misleading. - missing := filepath.Join(t.TempDir(), "absent-policy.yml") - got, src, err := cmdpolicy.Resolve(nil, missing) - if err != nil { - t.Fatalf("missing yaml should not error, got %v", err) - } - if got != nil || src.Kind != cmdpolicy.SourceNone { - t.Fatalf("expected (nil, SourceNone), got (%v, %+v)", got, src) - } -} - -// 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 := cmdpolicy.Resolve([]cmdpolicy.PluginRule{ - {PluginName: "a", Rule: &platform.Rule{Name: "a"}}, - {PluginName: "b", Rule: &platform.Rule{Name: "b"}}, - }, "") - if !errors.Is(err, cmdpolicy.ErrMultipleRestricts) { - t.Fatalf("err = %v, want ErrMultipleRestricts", err) + if src.Name != "/some/policy.yml" { + t.Errorf("yaml source Name should carry path, got %q", src.Name) } } func TestResolve_emptyEverythingIsNone(t *testing.T) { - got, src, err := cmdpolicy.Resolve(nil, "") + got, src, err := cmdpolicy.Resolve(cmdpolicy.Sources{}) if err != nil { t.Fatalf("Resolve err: %v", err) } @@ -97,3 +68,56 @@ func TestResolve_emptyEverythingIsNone(t *testing.T) { t.Fatalf("expected (nil, SourceNone), got (%v, %+v)", got, src) } } + +// 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 := cmdpolicy.Resolve(cmdpolicy.Sources{ + PluginRules: []cmdpolicy.PluginRule{ + {PluginName: "a", Rule: &platform.Rule{Name: "a"}}, + {PluginName: "b", Rule: &platform.Rule{Name: "b"}}, + }, + }) + if !errors.Is(err, cmdpolicy.ErrMultipleRestricts) { + t.Fatalf("err = %v, want ErrMultipleRestricts", err) + } +} + +// LoadYAMLPolicy: missing file returns (nil, nil) silently so callers +// can pass the result straight into Sources.YAMLRule without special- +// casing not-exist. +func TestLoadYAMLPolicy_missingIsSilent(t *testing.T) { + missing := filepath.Join(t.TempDir(), "absent-policy.yml") + rule, err := cmdpolicy.LoadYAMLPolicy(missing) + if err != nil { + t.Fatalf("missing yaml should not error, got %v", err) + } + if rule != nil { + t.Fatalf("missing yaml should return nil rule, got %+v", rule) + } +} + +func TestLoadYAMLPolicy_emptyPathIsNoop(t *testing.T) { + rule, err := cmdpolicy.LoadYAMLPolicy("") + if err != nil { + t.Fatalf("empty path should not error, got %v", err) + } + if rule != nil { + t.Fatalf("empty path should return nil rule, got %+v", rule) + } +} + +func TestLoadYAMLPolicy_parsesValid(t *testing.T) { + dir := t.TempDir() + yamlPath := filepath.Join(dir, "policy.yml") + if err := os.WriteFile(yamlPath, []byte("name: from-yaml\nmax_risk: read\n"), 0o644); err != nil { + t.Fatalf("write yaml: %v", err) + } + rule, err := cmdpolicy.LoadYAMLPolicy(yamlPath) + if err != nil { + t.Fatalf("LoadYAMLPolicy err: %v", err) + } + if rule == nil || rule.Name != "from-yaml" { + t.Fatalf("expected rule with name=from-yaml, got %+v", rule) + } +}