Check package installs that a macOS session process starts

configs/policies/agent-default.yaml

@@ -843,6 +843,15 @@ package_rules:
     action: block
     reason: "Known malware detected"
 
+  # Every other finding is allowed. A finding that no rule matches blocks the
+  # install, and the local provider reports "license unknown" for every
+  # package, so without this rule every install was blocked. The daemon
+  # applies this last rule after its package_checks.block_on rules, so a
+  # stricter block_on still blocks.
+  - match: {}
+    action: allow
+    reason: "No critical vulnerability or malware"
+
 # =============================================================================
 # RESOURCE LIMITS
 # =============================================================================

internal/api/core.go

@@ -931,7 +931,14 @@ func (a *App) execInSessionCore(ctx context.Context, id string, req types.ExecRe
 
 	// Package install check (after command policy check and approval, before event emission).
 	if a.pkgChecker != nil && pre.EffectiveDecision != types.DecisionDeny {
-		verdict, pkgErr := a.pkgChecker.Check(ctx, req.Command, req.Args, req.WorkingDir)
+		// The session's own policy rules, as on the macOS exec path.
+		var verdict *pkgcheck.Verdict
+		var pkgErr error
+		if eng := s.PolicyEngine(); eng != nil {
+			verdict, pkgErr = a.pkgChecker.CheckForPolicy(ctx, eng.PackageRules(), req.Command, req.Args, req.WorkingDir)
+		} else {
+			verdict, pkgErr = a.pkgChecker.Check(ctx, req.Command, req.Args, req.WorkingDir)
+		}
 		if pkgErr != nil {
 			slog.Warn("package check error", "err", pkgErr)
 			pre.EffectiveDecision = types.DecisionDeny

internal/pkgcheck/checker.go

@@ -3,7 +3,10 @@ package pkgcheck
 import (
 	"context"
 	"fmt"
+	"path/filepath"
 	"strings"
+	"sync"
+	"time"
 
 	"github.com/diffsec/agentmon/internal/policy"
 )
@@ -13,7 +16,12 @@ type CheckerConfig struct {
 	Scope     string // "new_packages_only" | "all_installs"
 	Resolvers []Resolver
 	Providers map[string]ProviderEntry
-	Rules     []policy.PackageRule
+	// Rules are the package rules of the daemon's default policy. Check uses
+	// them. CheckForPolicy takes a session's own rules instead.
+	Rules []policy.PackageRule
+	// BlockOn are the rules from package_checks.block_on. They apply after a
+	// policy's rules, except its final catch-all. See ComposeRules.
+	BlockOn   []policy.PackageRule
 	Allowlist *Allowlist
 	// Privacy configures which packages are sent to external providers.
 	// An empty PrivacyConfig (the zero value) means no filtering — all
@@ -28,8 +36,24 @@ type Checker struct {
 	cfg  CheckerConfig
 	orch *Orchestrator
 	eval *Evaluator
+
+	// recent holds the verdicts of the last recentTTL, by install. On macOS
+	// one `agentmon exec pip install x` is checked twice: by the exec API,
+	// and again when the system extension sees the exec. Without this, the
+	// second check resolves the same install again.
+	mu     sync.Mutex
+	recent map[string]recentVerdict
+	now    func() time.Time
+}
+
+type recentVerdict struct {
+	verdict Verdict
+	at      time.Time
 }
 
+// recentTTL is how long Check reuses a verdict for the same install.
+const recentTTL = 30 * time.Second
+
 // NewChecker creates a new Checker wired to the given config.
 func NewChecker(cfg CheckerConfig) *Checker {
 	pf := NewPrivacyFilter(cfg.Privacy)
@@ -39,19 +63,83 @@ func NewChecker(cfg CheckerConfig) *Checker {
 			Providers:     cfg.Providers,
 			PrivacyFilter: pf,
 		}),
-		eval: NewEvaluator(cfg.Rules),
+		eval:   NewEvaluator(ComposeRules(cfg.Rules, cfg.BlockOn)),
+		recent: make(map[string]recentVerdict),
+		now:    time.Now,
 	}
 }
 
 // Check evaluates a command. Returns a nil verdict if the command is not a
 // recognised package-install operation.
+//
+// A verdict of allow, warn or block for the same install in the same
+// directory is reused for recentTTL. An approve verdict is not reused,
+// because each one asks a person.
 func (c *Checker) Check(ctx context.Context, command string, args []string, workDir string) (*Verdict, error) {
-	// 1. Classify the command.
+	return c.check(ctx, c.eval, "default", command, args, workDir)
+}
+
+// CheckForPolicy is Check with a session policy's package rules in place of
+// the default policy's. The block_on rules still apply. Before, every check
+// used the daemon's default policy, so an agent-default session never got
+// agent-default's package rules.
+func (c *Checker) CheckForPolicy(ctx context.Context, rules []policy.PackageRule, command string, args []string, workDir string) (*Verdict, error) {
+	eval := NewEvaluator(ComposeRules(rules, c.cfg.BlockOn))
+	return c.check(ctx, eval, fmt.Sprintf("%+v", rules), command, args, workDir)
+}
+
+// check classifies the command and returns a recent verdict for the same
+// install under the same rules, or checks it. rulesKey names the rules.
+func (c *Checker) check(ctx context.Context, eval *Evaluator, rulesKey string, command string, args []string, workDir string) (*Verdict, error) {
+	// `python -m pip install x` is a pip install.
+	command, args = UnwrapInstallCommand(command, args)
 	intent := ClassifyInstallCommand(command, args, c.cfg.Scope)
 	if intent == nil {
 		return nil, nil
 	}
+	key := rulesKey + "\x00" + recentKey(intent, workDir)
+	if v, ok := c.recentVerdict(key); ok {
+		return v, nil
+	}
+	v, err := c.resolveAndEvaluate(ctx, eval, command, args, workDir, intent)
+	if err == nil && v != nil && v.Action != VerdictApprove {
+		c.mu.Lock()
+		c.recent[key] = recentVerdict{verdict: *v, at: c.now()}
+		c.mu.Unlock()
+	}
+	return v, err
+}
 
+// recentKey names an install: the tool, the directory and the arguments.
+// The directory is resolved, because the exec API and macOS ES can name the
+// same one differently, for example /var and /private/var.
+func recentKey(intent *InstallIntent, workDir string) string {
+	if dir, err := filepath.EvalSymlinks(workDir); err == nil {
+		workDir = dir
+	}
+	return intent.Tool + "\x00" + workDir + "\x00" + strings.Join(intent.OrigArgs, "\x00")
+}
+
+// recentVerdict returns a copy of a verdict from the last recentTTL, and
+// drops the ones that are older.
+func (c *Checker) recentVerdict(key string) (*Verdict, bool) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	now := c.now()
+	for k, r := range c.recent {
+		if now.Sub(r.at) > recentTTL {
+			delete(c.recent, k)
+		}
+	}
+	r, ok := c.recent[key]
+	if !ok {
+		return nil, false
+	}
+	v := r.verdict
+	return &v, true
+}
+
+func (c *Checker) resolveAndEvaluate(ctx context.Context, eval *Evaluator, command string, args []string, workDir string, intent *InstallIntent) (*Verdict, error) {
 	// 2. Find a resolver that can handle this tool.
 	// Strip leading global flags so args[0] is the subcommand,
 	// which is what resolvers' CanResolve expects.
@@ -123,7 +211,7 @@ func (c *Checker) Check(ctx context.Context, command string, args []string, work
 	}
 
 	// 6. Evaluate findings against policy rules.
-	verdict := c.eval.EvaluateWithContext(EvalContext{
+	verdict := eval.EvaluateWithContext(EvalContext{
 		Findings:       findings,
 		Ecosystem:      plan.Ecosystem,
 		ProviderErrors: providerErrs,
@@ -193,3 +281,32 @@ func buildCheckerSummary(intent *InstallIntent, plan *InstallPlan, verdict *Verd
 	}
 	return sb.String()
 }
+
+// ComposeRules puts a policy's package rules first, then the block_on rules.
+// A policy's last rule that matches every finding moves to the end.
+// Otherwise it would shadow every block_on rule: agent-default ends with an
+// allow for every finding, and block_on.vulnerability=high must still block
+// a high vulnerability. A policy without a catch-all keeps the evaluator's
+// default deny for a finding that no rule matches.
+func ComposeRules(policyRules, blockOn []policy.PackageRule) []policy.PackageRule {
+	out := make([]policy.PackageRule, 0, len(policyRules)+len(blockOn))
+	var catchAll *policy.PackageRule
+	if n := len(policyRules); n > 0 && IsCatchAllRule(policyRules[n-1]) {
+		catchAll = &policyRules[n-1]
+		policyRules = policyRules[:n-1]
+	}
+	out = append(out, policyRules...)
+	out = append(out, blockOn...)
+	if catchAll != nil {
+		out = append(out, *catchAll)
+	}
+	return out
+}
+
+// IsCatchAllRule reports whether a package rule matches every finding.
+func IsCatchAllRule(r policy.PackageRule) bool {
+	m := r.Match
+	return m.FindingType == "" && m.Severity == "" && len(m.Packages) == 0 &&
+		len(m.NamePatterns) == 0 && len(m.Reasons) == 0 && m.Ecosystem == "" &&
+		m.LicenseSPDX == nil && len(m.Options) == 0
+}

internal/pkgcheck/checker_recent_test.go

@@ -0,0 +1,120 @@
+package pkgcheck
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/diffsec/agentmon/internal/policy"
+)
+
+// countingResolver resolves every pip install to one package and counts the
+// calls, so that a test can see whether Check resolved again.
+type countingResolver struct{ calls int }
+
+func (r *countingResolver) Name() string { return "counting" }
+
+func (r *countingResolver) CanResolve(tool string, _ []string) bool { return tool == "pip" }
+
+func (r *countingResolver) Resolve(context.Context, string, []string) (*InstallPlan, error) {
+	r.calls++
+	return &InstallPlan{
+		Tool:      "pip",
+		Ecosystem: EcosystemPyPI,
+		Direct:    []PackageRef{{Name: "requests", Version: "2.0.0", Direct: true}},
+	}, nil
+}
+
+func TestCheckReusesRecentVerdict(t *testing.T) {
+	res := &countingResolver{}
+	c := NewChecker(CheckerConfig{Resolvers: []Resolver{res}})
+	clock := time.Unix(1000, 0)
+	c.now = func() time.Time { return clock }
+	ctx := context.Background()
+	dir := t.TempDir()
+
+	if _, err := c.Check(ctx, "pip3", []string{"install", "requests"}, dir); err != nil {
+		t.Fatal(err)
+	}
+	// The same install as macOS ES reports it: a versioned name.
+	if _, err := c.Check(ctx, "/opt/homebrew/bin/pip3.15", []string{"install", "requests"}, dir); err != nil {
+		t.Fatal(err)
+	}
+	// The same install as `python3 -m pip`.
+	if _, err := c.Check(ctx, "/opt/homebrew/bin/python3", []string{"-m", "pip", "install", "requests"}, dir); err != nil {
+		t.Fatal(err)
+	}
+	if res.calls != 1 {
+		t.Fatalf("resolved %d times within recentTTL, want 1", res.calls)
+	}
+
+	clock = clock.Add(recentTTL + time.Second)
+	if _, err := c.Check(ctx, "pip3", []string{"install", "requests"}, dir); err != nil {
+		t.Fatal(err)
+	}
+	if res.calls != 2 {
+		t.Errorf("resolved %d times after recentTTL, want 2", res.calls)
+	}
+}
+
+func TestUnwrapInstallCommand(t *testing.T) {
+	for _, c := range []struct {
+		cmd      string
+		args     []string
+		wantCmd  string
+		wantArgs int
+	}{
+		{"/opt/homebrew/bin/python3", []string{"-m", "pip", "install", "x"}, "pip", 2},
+		{"python3.15", []string{"-I", "-m", "pip3", "install", "x"}, "pip", 2},
+		{"python3", []string{"script.py", "-m", "pip"}, "python3", 3},
+		{"python3", []string{"-m", "venv", "x"}, "python3", 3},
+		{"node", []string{"-m", "pip"}, "node", 2},
+	} {
+		cmd, args := UnwrapInstallCommand(c.cmd, c.args)
+		if cmd != c.wantCmd || len(args) != c.wantArgs {
+			t.Errorf("UnwrapInstallCommand(%s %v) = %s %v", c.cmd, c.args, cmd, args)
+		}
+	}
+	if intent := ClassifyInstallCommand("/opt/homebrew/bin/pip3.15", []string{"install", "x"}, ""); intent == nil || intent.Tool != "pip" {
+		t.Errorf("pip3.15 install x: intent = %+v, want a pip install", intent)
+	}
+}
+
+// findingResolver resolves to one package, and findingProvider reports a
+// "license unknown" finding for it, as the local provider does.
+type findingProvider struct{}
+
+func (findingProvider) Name() string                { return "fake" }
+func (findingProvider) Capabilities() []FindingType { return []FindingType{FindingLicense} }
+func (findingProvider) CheckBatch(_ context.Context, req CheckRequest) (*CheckResponse, error) {
+	var out []Finding
+	for _, p := range req.Packages {
+		out = append(out, Finding{Type: FindingLicense, Severity: SeverityInfo, Package: p, Provider: "fake"})
+	}
+	return &CheckResponse{Provider: "fake", Findings: out}, nil
+}
+
+// TestCheckForPolicyUsesSessionRules: the default policy has no package
+// rules, so a license finding blocks. A session policy that allows every
+// other finding, as agent-default does, must allow the same install.
+func TestCheckForPolicyUsesSessionRules(t *testing.T) {
+	c := NewChecker(CheckerConfig{
+		Resolvers: []Resolver{&countingResolver{}},
+		Providers: map[string]ProviderEntry{"fake": {Provider: findingProvider{}}},
+	})
+	ctx := context.Background()
+	dir := t.TempDir()
+
+	v, err := c.Check(ctx, "pip3", []string{"install", "requests"}, dir)
+	if err != nil || v == nil || v.Action != VerdictBlock {
+		t.Fatalf("Check with the default rules = %+v, %v; want block", v, err)
+	}
+	session := []policy.PackageRule{
+		{Match: policy.PackageMatch{FindingType: "malware"}, Action: "block"},
+		{Match: policy.PackageMatch{}, Action: "allow"},
+	}
+	v, err = c.CheckForPolicy(ctx, session, "pip3", []string{"install", "requests"}, dir)
+	if err != nil || v == nil || v.Action != VerdictAllow {
+		t.Errorf("CheckForPolicy with a catch-all allow = %+v, %v; want allow", v, err)
+	}
+}

internal/pkgcheck/compose_rules_test.go

@@ -0,0 +1,49 @@
+package pkgcheck
+
+import (
+	"testing"
+
+	"github.com/diffsec/agentmon/internal/policy"
+)
+
+// TestComposeRules_CatchAllLast: agent-default ends with an allow for
+// every finding. A stricter block_on must still block, so the catch-all moves
+// after the block_on rules.
+func TestComposeRules_CatchAllLast(t *testing.T) {
+	policyRules := []policy.PackageRule{
+		{Match: policy.PackageMatch{FindingType: "malware"}, Action: "block"},
+		{Match: policy.PackageMatch{}, Action: "allow"},
+	}
+	// What config.CompileBlockOnRules gives for block_on.vulnerability=high.
+	blockOn := []policy.PackageRule{
+		{Match: policy.PackageMatch{FindingType: "vulnerability", Severity: "critical"}, Action: "deny"},
+		{Match: policy.PackageMatch{FindingType: "vulnerability", Severity: "high"}, Action: "deny"},
+	}
+	rules := ComposeRules(policyRules, blockOn)
+	if last := rules[len(rules)-1]; last.Action != "allow" || !IsCatchAllRule(last) {
+		t.Fatalf("last rule = %+v, want the policy's catch-all allow", last)
+	}
+
+	e := NewEvaluator(rules)
+	high := Finding{Type: FindingVulnerability, Severity: SeverityHigh,
+		Package: PackageRef{Name: "x"}}
+	if v := e.Evaluate([]Finding{high}, EcosystemPyPI); v.Action != VerdictBlock {
+		t.Errorf("high vulnerability with block_on.vulnerability=high: %s, want block", v.Action)
+	}
+	license := Finding{Type: FindingLicense, Severity: SeverityInfo,
+		Package: PackageRef{Name: "x"}}
+	if v := e.Evaluate([]Finding{license}, EcosystemPyPI); v.Action != VerdictAllow {
+		t.Errorf("license unknown: %s, want allow", v.Action)
+	}
+}
+
+// TestComposeRules_NoCatchAll keeps a policy without a catch-all in
+// order, so the evaluator's default deny still applies to it.
+func TestComposeRules_NoCatchAll(t *testing.T) {
+	policyRules := []policy.PackageRule{{Match: policy.PackageMatch{FindingType: "malware"}, Action: "block"}}
+	blockOn := []policy.PackageRule{{Match: policy.PackageMatch{FindingType: "license"}, Action: "deny"}}
+	rules := ComposeRules(policyRules, blockOn)
+	if len(rules) != 2 || rules[0].Match.FindingType != "malware" || rules[1].Match.FindingType != "license" {
+		t.Errorf("rules = %+v", rules)
+	}
+}