fix(trust): enforce scoped repository defaults

docs/history.md

@@ -161,17 +161,17 @@ Severity is low: an attacker with read access to process memory already has full
 access to the process. The Go runtime does not guarantee memory zeroing and
 GC-managed runtimes inherently have this limitation.
 
-### Finding F3: Empty ScopedRepos Bypasses Scope Check on Tier 2 (Medium) — Open
+### Finding F3: Empty ScopedRepos Bypasses Scope Check on Tier 2 (Medium) — RESOLVED
 
-In `policy.go`, the repo scope check is conditioned on `len(agent.ScopedRepos) > 0`.
-A Tier 2 agent with empty `ScopedRepos` (nil or `[]string{}`) is treated as
-unrestricted rather than as having no access. If an admin registers a Tier 2
-agent without explicitly setting `ScopedRepos`, it gets access to all repositories
-for repo-scoped capabilities (`repo.push`, `pr.create`, `pr.merge`, `secrets.read`).
+In `policy.go`, repo-scoped capability access previously skipped checks when
+`len(agent.ScopedRepos) == 0`.
+A Tier 2 agent with empty `ScopedRepos` (nil or `[]string{}`) was previously treated as
+unrestricted rather than as having no access.
 
-Potential remediation: treat empty `ScopedRepos` as no access for Tier 2 agents,
-requiring explicit `["*"]` or `["org/**"]` for unrestricted access. This is a
-design decision with backward-compatibility implications.
+Resolved by requiring an explicit scope for repo-scoped capabilities:
+- `[]string{}` / `nil` now denies all repo-scoped access by default.
+- `[]string{"*"}` grants unrestricted repo access.
+- Pattern matching with `host-uk/*` and `host-uk/**` still applies as before.
 
 ### Finding F4: `go vet` Clean — Passed
 
@@ -224,8 +224,6 @@ callers that need structured logs should wrap or replace the cleanup goroutine.
   `crypt/chachapoly` into a single implementation.
 - **Hardware key backends**: implement `HardwareKey` for PKCS#11 (via
   `miekg/pkcs11` or `ThalesIgnite/crypto11`) and YubiKey (via `go-piv`).
-- **Resolve Finding F3**: require explicit wildcard for unrestricted Tier 2
-  access; treat empty `ScopedRepos` as no-access.
 - **Structured logging**: replace `fmt.Printf` in `StartCleanup` with an
   `slog.Logger` option on `Authenticator`.
 - **Rate limiting enforcement**: the `Agent.RateLimit` field is stored in the

trust/policy.go

@@ -117,9 +117,9 @@ func (pe *PolicyEngine) Evaluate(agentName string, cap Capability, repo string)
 	// Check if capability is allowed.
 	for _, allowed := range policy.Allowed {
 		if allowed == cap {
-			// For repo-scoped capabilities, verify repo access.
-			if isRepoScoped(cap) && len(agent.ScopedRepos) > 0 {
-				if !repoAllowed(agent.ScopedRepos, repo) {
+			// For repo-scoped capabilities, verify repo access for restricted tiers.
+			if isRepoScoped(cap) && agent.Tier != TierFull {
+				if len(agent.ScopedRepos) == 0 || !repoAllowed(agent.ScopedRepos, repo) {
 					return EvalResult{
 						Decision: Deny,
 						Agent:    agentName,
@@ -247,6 +247,11 @@ func matchScope(pattern, repo string) bool {
 		return true
 	}
 
+	// Star means unrestricted access for all repos.
+	if pattern == "*" {
+		return true
+	}
+
 	// Check for wildcard patterns.
 	if !strings.Contains(pattern, "*") {
 		return false

trust/policy_test.go

@@ -270,34 +270,49 @@ func TestDefaultRateLimit(t *testing.T) {
 
 // --- Phase 0 Additions ---
 
-// TestEvaluate_Good_Tier2EmptyScopedReposAllowsAll verifies that a Tier 2
-// agent with empty ScopedRepos is treated as "unrestricted" for repo-scoped
-// capabilities. NOTE: This is a potential security concern documented in
-// FINDINGS.md — empty ScopedRepos bypasses the repo scope check entirely.
-func TestEvaluate_Good_Tier2EmptyScopedReposAllowsAll(t *testing.T) {
+// TestEvaluate_Bad_Tier2EmptyScopedReposDeniesAll verifies that an empty
+// scoped-repo list blocks repo-scoped capabilities by default.
+func TestEvaluate_Bad_Tier2EmptyScopedReposDeniesAll(t *testing.T) {
 	r := NewRegistry()
 	require.NoError(t, r.Register(Agent{
 		Name:        "Hypnos",
 		Tier:        TierVerified,
-		ScopedRepos: []string{}, // empty — currently means "unrestricted"
+		ScopedRepos: []string{},
 	}))
 	pe := NewPolicyEngine(r)
 
-	// Current behaviour: empty ScopedRepos skips scope check (len == 0)
 	result := pe.Evaluate("Hypnos", CapPushRepo, "host-uk/core")
-	assert.Equal(t, Allow, result.Decision,
-		"empty ScopedRepos currently allows all repos (potential security finding)")
+	assert.Equal(t, Deny, result.Decision,
+		"empty ScopedRepos should deny repo-scoped operations by default")
 
 	result = pe.Evaluate("Hypnos", CapReadSecrets, "host-uk/core")
-	assert.Equal(t, Allow, result.Decision)
+	assert.Equal(t, Deny, result.Decision)
 
 	result = pe.Evaluate("Hypnos", CapCreatePR, "host-uk/core")
 	assert.Equal(t, Allow, result.Decision)
 
-	// Non-repo-scoped capabilities should still work
 	result = pe.Evaluate("Hypnos", CapCreateIssue, "")
 	assert.Equal(t, Allow, result.Decision)
-	result = pe.Evaluate("Hypnos", CapCommentIssue, "")
+}
+
+func TestEvaluate_Good_Tier2WildcardAllowsAll(t *testing.T) {
+	r := NewRegistry()
+	require.NoError(t, r.Register(Agent{
+		Name:        "Hydrus",
+		Tier:        TierVerified,
+		ScopedRepos: []string{"*"},
+	}))
+	pe := NewPolicyEngine(r)
+
+	result := pe.Evaluate("Hydrus", CapPushRepo, "host-uk/core")
+	assert.Equal(t, Allow, result.Decision)
+
+	result = pe.Evaluate("Hydrus", CapReadSecrets, "host-uk/any")
+	assert.Equal(t, Allow, result.Decision)
+
+	result = pe.Evaluate("Hydrus", CapCreateIssue, "")
+	assert.Equal(t, Allow, result.Decision)
+	result = pe.Evaluate("Hydrus", CapCommentIssue, "")
 	assert.Equal(t, Allow, result.Decision)
 }
 

trust/scope_test.go

@@ -13,6 +13,11 @@ func TestMatchScope_Good_ExactMatch(t *testing.T) {
 	assert.True(t, matchScope("host-uk/core", "host-uk/core"))
 }
 
+func TestMatchScope_Good_StarWildcard(t *testing.T) {
+	assert.True(t, matchScope("*", "host-uk/core"))
+	assert.True(t, matchScope("*", "core/php/sub"))
+}
+
 func TestMatchScope_Good_SingleWildcard(t *testing.T) {
 	assert.True(t, matchScope("core/*", "core/php"))
 	assert.True(t, matchScope("core/*", "core/go-crypt"))

trust/trust.go

@@ -71,7 +71,9 @@ type Agent struct {
 	Name string
 	// Tier is the agent's trust level.
 	Tier Tier
-	// ScopedRepos limits repo access for Tier 2 agents. Empty means no repo access.
+	// ScopedRepos limits repo access for Tier 2 agents.
+	// Empty means no repo access.
+	// Use ["*"] for unrestricted repo scope.
 	// Tier 3 agents ignore this field (they have access to all repos).
 	ScopedRepos []string
 	// RateLimit is the maximum requests per minute. 0 means unlimited.