feat(policy): add pkg/policy shared validator and capiscio policy CLI by beonde · Pull Request #46 · capiscio/capiscio-core

beonde · 2026-03-22T06:22:30Z

PR 6 of POLICY-CONFIG-001: Shared Policy Validator + CLI Commands

Overview

Adds the canonical policy configuration validator as a shared package (pkg/policy/) and two new CLI commands for policy management.

New Package: `pkg/policy/`

Config types: Config, RateLimitRule, OperationRule, MCPToolRule
Parse(data []byte) — YAML parsing with full semantic validation
Validate(cfg *Config) — version, trust levels, DID formats, rate limits, operations, MCP tools
ToMap(cfg *Config) — converts to OPA data document format for bundle embedding

This is the shared validator used by both the CLI and the server. The server's internal/pdp/policy_config.go can be updated to import from this package in a follow-up.

New CLI Commands

capiscio policy validate [-f file] [--json]

Local-only YAML config validation (no network access)
Non-zero exit on validation failure (PM non-negotiable: "silent-allow is worst failure mode")
--json flag for machine-readable output
Default file: capiscio-policy.yaml

capiscio policy context [--registry url] [--api-key key] [-o file]

Calls GET /v1/sdk/policy-context (requires PR 5 on server)
Supports CAPISCIO_API_KEY env var or --api-key flag
Writes pretty-printed JSON to stdout or file

Tests

14 new tests covering all validation paths (valid configs, invalid versions, bad DIDs, duplicates, conflicting lists, rate limits, operations, MCP tools, malformed YAML, ToMap round-trip, multi-error reporting)

Verification

go test ./pkg/policy/ -v     # All 14 tests pass
go build ./...                # Clean build
./bin/capiscio policy validate --help  # Smoke test

Part of

POLICY-CONFIG-001 implementation sequence (Step 6 of 8)

PR 6 of POLICY-CONFIG-001: Shared validator + CLI commands New package: pkg/policy/ - Config types (Config, RateLimitRule, OperationRule, MCPToolRule) - Parse() - YAML parsing with validation - Validate() - semantic validation (version, trust levels, DIDs, etc) - ToMap() - convert to OPA data document format New CLI commands: - capiscio policy validate [-f file] [--json] Local-only YAML config validation, non-zero exit on failure - capiscio policy context [--registry url] [--api-key key] [-o file] Fetch GET /v1/sdk/policy-context from registry Tests: 14 new tests for pkg/policy validator

Copilot

Pull request overview

Adds a shared pkg/policy YAML policy configuration validator (intended for reuse by CLI and server) and introduces new capiscio policy CLI commands to validate configs locally and fetch registry policy context.

Changes:

Introduce pkg/policy with config types plus Parse, Validate, and ToMap.
Add unit tests for parsing/validation and ToMap.
Add capiscio policy validate and capiscio policy context commands; add YAML dependency.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.

File	Description
pkg/policy/config.go	Implements policy config types, parsing, semantic validation, and OPA data conversion
pkg/policy/config_test.go	Adds tests for parse/validate behavior and `ToMap` output
cmd/capiscio/policy.go	Adds CLI commands for local validation and fetching policy context from registry
go.mod	Promotes `gopkg.in/yaml.v3` to a direct dependency

Copilot · 2026-03-22T06:25:50Z

pkg/policy/config.go

+func Parse(data []byte) (*Config, error) {
+	var cfg Config
+	if err := yaml.Unmarshal(data, &cfg); err != nil {
+		return nil, fmt.Errorf("parse policy config: %w", err)
+	}


Parse uses yaml.Unmarshal, which ignores unknown fields by default. For a security-sensitive policy file this can lead to silent misconfiguration (typos are dropped). Consider switching to a yaml.Decoder with KnownFields(true) so unknown keys fail validation.

Copilot · 2026-03-22T06:25:50Z

pkg/policy/config.go

+// Validate checks a parsed Config for semantic correctness.
+func Validate(cfg *Config) error {
+	var errs []string
+
+	if cfg.Version != "1" {
+		errs = append(errs, fmt.Sprintf("version must be \"1\", got %q", cfg.Version))
+	}


Validate assumes cfg is non-nil; calling Validate(nil) will panic when accessing cfg.Version. Since Validate is exported, it should return a descriptive error for nil input instead of panicking.

Copilot · 2026-03-22T06:25:50Z

pkg/policy/config.go

+	for _, did := range cfg.AllowedDIDs {
+		if !strings.HasPrefix(did, "did:") {
+			errs = append(errs, fmt.Sprintf("allowed_dids: invalid DID format %q", did))
+		}


DID validation here only checks strings.HasPrefix(did, "did:"). This will accept many invalid/unsupported DIDs. Prefer using the repo’s canonical DID parser (pkg/did.Parse) so policy configs reject malformed/unsupported identifiers consistently.

Copilot · 2026-03-22T06:25:50Z

pkg/policy/config.go

+	deniedSeen := make(map[string]bool)
+	for _, did := range cfg.DeniedDIDs {
+		if !strings.HasPrefix(did, "did:") {
+			errs = append(errs, fmt.Sprintf("denied_dids: invalid DID format %q", did))
+		}


Denied DID validation only checks the "did:" prefix; this can accept malformed/unsupported DIDs. Use pkg/did.Parse for consistent validation, and keep the existing overlap check with allowed_dids.

Copilot · 2026-03-22T06:25:50Z

pkg/policy/config.go

+	// Validate rate limits
+	for i, rl := range cfg.RateLimits {
+		if !strings.HasPrefix(rl.DID, "did:") {
+			errs = append(errs, fmt.Sprintf("rate_limits[%d]: invalid DID format %q", i, rl.DID))
+		}
+		if rl.RPM <= 0 {
+			errs = append(errs, fmt.Sprintf("rate_limits[%d]: rpm must be positive, got %d", i, rl.RPM))
+		}
+	}


rate_limits validation does not check for duplicate entries for the same DID, so later consumers may get ambiguous behavior. Consider tracking seen DIDs (similar to allowed_dids/denied_dids) and erroring on duplicates.

Copilot · 2026-03-22T06:25:51Z

pkg/policy/config.go

+	for i, rl := range cfg.RateLimits {
+		if !strings.HasPrefix(rl.DID, "did:") {
+			errs = append(errs, fmt.Sprintf("rate_limits[%d]: invalid DID format %q", i, rl.DID))
+		}


rate_limits[%d] DID validation only checks the "did:" prefix. To avoid accepting malformed/unsupported identifiers, validate rl.DID using pkg/did.Parse (or equivalent shared validator).

Copilot · 2026-03-22T06:25:51Z

pkg/policy/config.go

+		}
+		if !validTrustLevels[op.MinTrustLevel] {
+			errs = append(errs, fmt.Sprintf("operations[%d]: invalid min_trust_level %q", i, op.MinTrustLevel))
+		}


operations validation checks pattern and min_trust_level but does not validate op.AllowedDIDs/op.DeniedDIDs (format, duplicates, or overlap between allowed/denied). That leaves operation-scoped rules semantically unchecked even though they’re part of the config schema.

Suggested change

}

}

// Validate operation-scoped allowed DIDs

opAllowedSeen := make(map[string]bool)

for j, did := range op.AllowedDIDs {

if !strings.HasPrefix(did, "did:") {

errs = append(errs, fmt.Sprintf("operations[%d]: allowed_dids[%d]: invalid DID format %q", i, j, did))

}

if opAllowedSeen[did] {

errs = append(errs, fmt.Sprintf("operations[%d]: allowed_dids[%d]: duplicate DID %q", i, j, did))

}

opAllowedSeen[did] = true

}

// Validate operation-scoped denied DIDs

opDeniedSeen := make(map[string]bool)

for j, did := range op.DeniedDIDs {

if !strings.HasPrefix(did, "did:") {

errs = append(errs, fmt.Sprintf("operations[%d]: denied_dids[%d]: invalid DID format %q", i, j, did))

}

if opDeniedSeen[did] {

errs = append(errs, fmt.Sprintf("operations[%d]: denied_dids[%d]: duplicate DID %q", i, j, did))

}

opDeniedSeen[did] = true

if opAllowedSeen[did] {

errs = append(errs, fmt.Sprintf("operations[%d]: DID %q appears in both allowed_dids and denied_dids", i, did))

}

}

Copilot · 2026-03-22T06:25:51Z

pkg/policy/config.go

+		}
+		if !validTrustLevels[tool.MinTrustLevel] {
+			errs = append(errs, fmt.Sprintf("mcp_tools[%d]: invalid min_trust_level %q", i, tool.MinTrustLevel))
+		}


mcp_tools validation checks tool name and min_trust_level but does not validate AllowedDIDs/DeniedDIDs (format, duplicates, overlap). This can allow invalid tool-scoped rules to pass validation.

Suggested change

}

}

// Validate MCP tool allowed DIDs

toolAllowedSeen := make(map[string]bool)

for _, did := range tool.AllowedDIDs {

if !strings.HasPrefix(did, "did:") {

errs = append(errs, fmt.Sprintf("mcp_tools[%d].allowed_dids: invalid DID format %q", i, did))

}

if toolAllowedSeen[did] {

errs = append(errs, fmt.Sprintf("mcp_tools[%d].allowed_dids: duplicate DID %q", i, did))

}

toolAllowedSeen[did] = true

}

// Validate MCP tool denied DIDs

toolDeniedSeen := make(map[string]bool)

for _, did := range tool.DeniedDIDs {

if !strings.HasPrefix(did, "did:") {

errs = append(errs, fmt.Sprintf("mcp_tools[%d].denied_dids: invalid DID format %q", i, did))

}

if toolDeniedSeen[did] {

errs = append(errs, fmt.Sprintf("mcp_tools[%d].denied_dids: duplicate DID %q", i, did))

}

toolDeniedSeen[did] = true

if toolAllowedSeen[did] {

errs = append(errs, fmt.Sprintf("mcp_tools[%d]: DID %q appears in both allowed_dids and denied_dids", i, did))

}

}

Copilot · 2026-03-22T06:25:51Z

cmd/capiscio/policy.go

+	cfg, err := policy.Parse(data)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "❌ Validation failed for %s:\n%s\n", policyFile, err.Error())
+		os.Exit(1)


runPolicyValidate calls os.Exit(1) inside a Cobra RunE handler. This bypasses Cobra’s normal error handling, makes the command hard to test, and can skip deferred cleanup. Prefer returning an error (and optionally set cmd.SilenceUsage/SilenceErrors as needed) and let main() exit non-zero.

Suggested change

os.Exit(1)

return fmt.Errorf("validation failed for %s: %w", policyFile, err)

Copilot · 2026-03-22T06:25:51Z

cmd/capiscio/policy.go

+		return nil
+	}
+
+	fmt.Printf("✅ %s is valid (version %s, trust_level: %s)\n", policyFile, cfg.Version, cfg.MinTrustLevel)


Success output says "trust_level" but the config field is "min_trust_level". Consider updating the message to match the actual YAML key to avoid confusing users when troubleshooting configs.

Suggested change

fmt.Printf("✅ %s is valid (version %s, trust_level: %s)\n", policyFile, cfg.Version, cfg.MinTrustLevel)

fmt.Printf("✅ %s is valid (version %s, min_trust_level: %s)\n", policyFile, cfg.Version, cfg.MinTrustLevel)

…ist helper

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

Copilot · 2026-03-22T06:32:49Z

pkg/policy/config.go

+func Validate(cfg *Config) error {
+	var errs []string
+
+	if cfg.Version != "1" {
+		errs = append(errs, fmt.Sprintf("version must be \"1\", got %q", cfg.Version))
+	}


Validate will panic if called with a nil cfg (cfg.Version dereference). Since Validate is exported and can be called independently of Parse, it should return a clear error when cfg is nil instead of crashing.

Copilot · 2026-03-22T06:32:49Z

pkg/policy/config.go

+func Parse(data []byte) (*Config, error) {
+	var cfg Config
+	if err := yaml.Unmarshal(data, &cfg); err != nil {
+		return nil, fmt.Errorf("parse policy config: %w", err)
+	}


Parse uses yaml.Unmarshal, which silently ignores unknown YAML keys. For a config validator (and given the “silent-allow is worst failure mode” requirement), typos like allowed_did: would be dropped without error and could change policy behavior. Consider decoding with yaml.Decoder and enabling KnownFields(true) so unknown fields fail validation.

Copilot · 2026-03-22T06:32:50Z

pkg/policy/config.go

+	for _, did := range dids {
+		if !strings.HasPrefix(did, "did:") {
+			errs = append(errs, fmt.Sprintf("%s: invalid DID format %q", name, did))
+		}


DID validation currently only checks strings.HasPrefix("did:") (including for rate_limits). This accepts many invalid values (e.g., "did:" or unsupported methods) even though the repo has a canonical DID parser in pkg/did.Parse that enforces did:web/did:key rules. Using the shared DID parser here would make “DID formats” validation accurate and consistent.

Copilot · 2026-03-22T06:32:50Z

pkg/policy/config.go

+	// Validate operations
+	for i, op := range cfg.Operations {
+		if op.Pattern == "" {
+			errs = append(errs, fmt.Sprintf("operations[%d]: pattern must not be empty", i))
+		}
+		if !validTrustLevels[op.MinTrustLevel] {
+			errs = append(errs, fmt.Sprintf("operations[%d]: invalid min_trust_level %q", i, op.MinTrustLevel))
+		}
+	}
+
+	// Validate MCP tools
+	for i, tool := range cfg.MCPTools {
+		if tool.Tool == "" {
+			errs = append(errs, fmt.Sprintf("mcp_tools[%d]: tool must not be empty", i))
+		}
+		if !validTrustLevels[tool.MinTrustLevel] {
+			errs = append(errs, fmt.Sprintf("mcp_tools[%d]: invalid min_trust_level %q", i, tool.MinTrustLevel))
+		}
+	}


OperationRule and MCPToolRule both include allowed_dids/denied_dids fields, but Validate does not validate their DID lists at all (format, duplicates, or allowed-vs-denied conflicts). This means invalid or conflicting DIDs inside operations/mcp_tools can pass validation despite the policy file containing those lists. Extend validation to run the same DID list checks per operation/tool entry.

Copilot · 2026-03-22T06:32:50Z

pkg/policy/config_test.go

+func TestParse_FullConfig(t *testing.T) {
+	yaml := `version: "1"
+min_trust_level: DV
+allowed_dids:
+  - did:web:agent1.example.com
+  - did:web:agent2.example.com
+denied_dids:
+  - did:web:evil.example.com
+rate_limits:
+  - did: did:web:agent1.example.com
+    rpm: 100
+operations:
+  - pattern: "POST /tasks"
+    min_trust_level: OV
+    allowed_dids:
+      - did:web:agent1.example.com
+mcp_tools:
+  - tool: file_read
+    min_trust_level: EV
+    denied_dids:
+      - did:web:untrusted.example.com
+`


There are no tests covering validation of allowed_dids/denied_dids inside operations[] or mcp_tools[] entries (e.g., invalid DID format, duplicates, or appearing in both lists). Once per-entry validation is added, add table cases to ensure these nested DID lists are enforced (and that errors are attributed to the right index).

codecov · 2026-03-22T06:35:49Z

Codecov Report

❌ Patch coverage is 90.90909% with 10 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/policy/config.go	90.90%	5 Missing and 5 partials ⚠️

📢 Thoughts on this report? Let us know!

- Add nil guard in Validate() - Add duplicate rate limit DID check - Validate DIDs in operation/MCP tool rules (format, duplicates) - Replace os.Exit(1) with return err in RunE handler - Fix output label: trust_level → min_trust_level - Add 4 new tests for nil, rate limit dedup, op/tool DID validation

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Copilot · 2026-03-22T06:50:53Z

pkg/policy/config.go

+func Parse(data []byte) (*Config, error) {
+	var cfg Config
+	if err := yaml.Unmarshal(data, &cfg); err != nil {
+		return nil, fmt.Errorf("parse policy config: %w", err)
+	}
+	if err := Validate(&cfg); err != nil {
+		return nil, err
+	}


Parse uses yaml.Unmarshal, which will silently ignore unknown YAML keys. For a canonical config validator this is risky (typos like min_trsut_level would be dropped without error). Consider switching to yaml.NewDecoder with KnownFields(true) so unknown fields fail validation.

Copilot · 2026-03-22T06:50:53Z

pkg/policy/config.go

+	for i, op := range cfg.Operations {
+		if op.Pattern == "" {
+			errs = append(errs, fmt.Sprintf("operations[%d]: pattern must not be empty", i))
+		}
+		if !validTrustLevels[op.MinTrustLevel] {
+			errs = append(errs, fmt.Sprintf("operations[%d]: invalid min_trust_level %q", i, op.MinTrustLevel))
+		}
+		_, opDIDErrs := validateDIDList(fmt.Sprintf("operations[%d].allowed_dids", i), op.AllowedDIDs, nil)
+		errs = append(errs, opDIDErrs...)
+		_, opDeniedErrs := validateDIDList(fmt.Sprintf("operations[%d].denied_dids", i), op.DeniedDIDs, nil)
+		errs = append(errs, opDeniedErrs...)
+	}


Top-level allowed_dids vs denied_dids are cross-checked for conflicts, but operation-scoped lists are not. As a result, the same DID can appear in both operations[i].allowed_dids and operations[i].denied_dids without error, making evaluation order ambiguous and contradicting the PR description’s "conflicting lists" validation.

Copilot · 2026-03-22T06:50:53Z

pkg/policy/config.go

+		_, toolDIDErrs := validateDIDList(fmt.Sprintf("mcp_tools[%d].allowed_dids", i), tool.AllowedDIDs, nil)
+		errs = append(errs, toolDIDErrs...)
+		_, toolDeniedErrs := validateDIDList(fmt.Sprintf("mcp_tools[%d].denied_dids", i), tool.DeniedDIDs, nil)


Similarly, MCP tool rules validate allowed/denied lists independently but do not detect a DID that appears in both mcp_tools[i].allowed_dids and mcp_tools[i].denied_dids. If conflicts are meant to be rejected (as with top-level lists), add a cross-check here as well to avoid ambiguous policy behavior.

Suggested change

_, toolDIDErrs := validateDIDList(fmt.Sprintf("mcp_tools[%d].allowed_dids", i), tool.AllowedDIDs, nil)

errs = append(errs, toolDIDErrs...)

_, toolDeniedErrs := validateDIDList(fmt.Sprintf("mcp_tools[%d].denied_dids", i), tool.DeniedDIDs, nil)

toolAllowedSeen, toolDIDErrs := validateDIDList(fmt.Sprintf("mcp_tools[%d].allowed_dids", i), tool.AllowedDIDs, nil)

errs = append(errs, toolDIDErrs...)

_, toolDeniedErrs := validateDIDList(fmt.Sprintf("mcp_tools[%d].denied_dids", i), tool.DeniedDIDs, toolAllowedSeen)

…l rules - Cross-check allowed vs denied DID lists within each operation rule - Cross-check allowed vs denied DID lists within each MCP tool rule - Add tests for operation and MCP tool DID conflicts - Fix DID conflict error messages to include scoped name

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Copilot · 2026-03-22T07:02:49Z

pkg/policy/config.go

+}
+
+// ToMap converts a Config to the map format used in OPA data documents.
+func ToMap(cfg *Config) map[string]interface{} {


ToMap dereferences cfg without a nil check; calling ToMap(nil) will panic. Since this is an exported helper, consider returning an empty map (or nil) and/or mirroring Validate by returning an error when cfg is nil.

Suggested change

func ToMap(cfg *Config) map[string]interface{} {

func ToMap(cfg *Config) map[string]interface{} {

if cfg == nil {

return map[string]interface{}{}

}

Copilot · 2026-03-22T07:02:50Z

cmd/capiscio/policy.go

+var policyValidateCmd = &cobra.Command{
+	Use:   "validate",
+	Short: "Validate a YAML policy config file",
+	Long: `Validate a CapiscIO YAML policy configuration file locally.
+No network access is required. Checks schema version, trust levels,


This command does not declare an Args validator, so extra positional args will be silently accepted/ignored by cobra. Consider adding Args: cobra.NoArgs (and similarly for policy context) to fail fast on unexpected input.

Copilot · 2026-03-22T07:02:50Z

cmd/capiscio/policy.go

+var policyContextCmd = &cobra.Command{
+	Use:   "context",
+	Short: "Fetch policy context from registry",
+	Long: `Fetch the aggregate policy context from the CapiscIO registry.
+This calls GET /v1/sdk/policy-context and writes the result as JSON.


This command does not declare an Args validator, so extra positional args will be silently accepted/ignored by cobra. Consider adding Args: cobra.NoArgs to ensure misuse is caught early.

- ToMap returns nil for nil input instead of panicking - Add Args: cobra.NoArgs to validate and context commands

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot · 2026-03-22T07:07:44Z

pkg/policy/config.go

+		}
+		seen[did] = true
+		if crossCheck != nil && crossCheck[did] {
+			errs = append(errs, fmt.Sprintf("%s: DID %q conflicts with allowed_dids", name, did))


The conflict error message is hard-coded as "conflicts with allowed_dids" even when validating nested scopes like operations[i].denied_dids or mcp_tools[i].denied_dids. Consider including the fully-qualified allowed list name in the message (e.g., operations[i].allowed_dids) so users can quickly identify what it conflicts with.

Suggested change

errs = append(errs, fmt.Sprintf("%s: DID %q conflicts with allowed_dids", name, did))

allowedName := strings.Replace(name, "denied_dids", "allowed_dids", 1)

errs = append(errs, fmt.Sprintf("%s: DID %q conflicts with %s", name, did, allowedName))

Copilot AI review requested due to automatic review settings March 22, 2026 06:22

Copilot started reviewing on behalf of beonde March 22, 2026 06:23 View session