From c67601c7f61c40d3325cb1ab09309cf8d6dca40c Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Wed, 25 Mar 2026 23:02:53 -0700 Subject: [PATCH] Allow all authenticated users to push/pull (not just human+user role) The previous default policy required both AccountTypes=["human"] and Roles=["user"], but MCIAS validate responses don't reliably include these fields. For a private registry, any successfully authenticated caller should have content access. Admin-only operations (policy management) still require the admin role. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/policy/defaults.go | 10 +++---- internal/policy/engine_test.go | 24 +++++++-------- internal/policy/policy_test.go | 55 +++++++++++++++++++++------------- 3 files changed, 51 insertions(+), 38 deletions(-) diff --git a/internal/policy/defaults.go b/internal/policy/defaults.go index ee9a590..4a505e8 100644 --- a/internal/policy/defaults.go +++ b/internal/policy/defaults.go @@ -23,12 +23,10 @@ func DefaultRules() []Rule { Actions: allActions, }, { - ID: -2, - Priority: 0, - Description: "human users have full content access", - Effect: Allow, - Roles: []string{"user"}, - AccountTypes: []string{"human"}, + ID: -2, + Priority: 0, + Description: "authenticated users have full content access", + Effect: Allow, Actions: []Action{ ActionPull, ActionPush, diff --git a/internal/policy/engine_test.go b/internal/policy/engine_test.go index 8019e09..ebe1913 100644 --- a/internal/policy/engine_test.go +++ b/internal/policy/engine_test.go @@ -27,15 +27,15 @@ func TestEngineDefaultsOnly(t *testing.T) { t.Fatalf("admin push with defaults: got %s, want allow", effect) } - // System account should be denied. + // System account should be allowed (rule -2: authenticated users). effect, _ = e.Evaluate(PolicyInput{ Subject: "system-uuid", AccountType: "system", Action: ActionPull, Repository: "myapp", }) - if effect != Deny { - t.Fatalf("system pull with defaults: got %s, want deny", effect) + if effect != Allow { + t.Fatalf("system pull with defaults: got %s, want allow", effect) } } @@ -62,15 +62,15 @@ func TestEngineWithCustomRules(t *testing.T) { t.Fatalf("ci pull with custom rule: got %s, want allow", effect) } - // Different subject should still be denied. + // Different subject is still allowed via default rule -2 (authenticated users). effect, _ = e.Evaluate(PolicyInput{ Subject: "other-uuid", AccountType: "system", Action: ActionPull, Repository: "myapp", }) - if effect != Deny { - t.Fatalf("other pull: got %s, want deny", effect) + if effect != Allow { + t.Fatalf("other pull: got %s, want allow", effect) } } @@ -117,15 +117,15 @@ func TestEngineReload(t *testing.T) { t.Fatalf("Reload (second): %v", err) } - // ci-uuid should now be denied (old rule gone). + // ci-uuid custom rule is gone, but still allowed via default rule -2. effect, _ = e.Evaluate(PolicyInput{ Subject: "ci-uuid", AccountType: "system", Action: ActionPull, Repository: "myapp", }) - if effect != Deny { - t.Fatalf("ci pull after second reload: got %s, want deny", effect) + if effect != Allow { + t.Fatalf("ci pull after second reload: got %s, want allow", effect) } // deploy-uuid should now be allowed. @@ -149,14 +149,14 @@ func TestEngineReloadDisabledExcluded(t *testing.T) { t.Fatalf("Reload: %v", err) } - // No custom rules, so system account should be denied. + // No custom rules, but system account is allowed via default rule -2. effect, _ := e.Evaluate(PolicyInput{ Subject: "ci-uuid", AccountType: "system", Action: ActionPull, Repository: "myapp", }) - if effect != Deny { - t.Fatalf("system pull with no custom rules: got %s, want deny", effect) + if effect != Allow { + t.Fatalf("system pull with no custom rules: got %s, want allow", effect) } } diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go index 7977d64..e0ec103 100644 --- a/internal/policy/policy_test.go +++ b/internal/policy/policy_test.go @@ -38,7 +38,7 @@ func TestEvaluateUserAllow(t *testing.T) { } } -func TestEvaluateSystemAccountDeny(t *testing.T) { +func TestEvaluateSystemAccountAllow(t *testing.T) { rules := DefaultRules() input := PolicyInput{ Subject: "system-uuid", @@ -47,11 +47,11 @@ func TestEvaluateSystemAccountDeny(t *testing.T) { Repository: "myapp", } effect, rule := Evaluate(input, rules) - if effect != Deny { - t.Fatalf("system pull: got %s, want deny", effect) + if effect != Allow { + t.Fatalf("system pull: got %s, want allow", effect) } - if rule != nil { - t.Fatalf("system pull: expected default deny (nil rule), got %+v", rule) + if rule == nil || rule.ID != -2 { + t.Fatalf("system pull: expected authenticated content rule (ID -2), got %+v", rule) } } @@ -75,11 +75,11 @@ func TestEvaluateExactRepoMatch(t *testing.T) { t.Fatalf("exact repo match: got %s, want allow", effect) } - // Different repo should deny. + // Different repo still allowed via default rule -2 (authenticated users). input.Repository = "other" effect, _ = Evaluate(input, rules) - if effect != Deny { - t.Fatalf("different repo: got %s, want deny", effect) + if effect != Allow { + t.Fatalf("different repo: got %s, want allow", effect) } } @@ -103,11 +103,12 @@ func TestEvaluateGlobMatch(t *testing.T) { t.Fatalf("glob match production/myapp: got %s, want allow", effect) } - // Nested repo should not match (path.Match: * doesn't cross /). + // Nested repo does not match the glob rule, but default rule -2 + // (authenticated users) allows it. input.Repository = "production/team/myapp" effect, _ = Evaluate(input, rules) - if effect != Deny { - t.Fatalf("glob no-match production/team/myapp: got %s, want deny", effect) + if effect != Allow { + t.Fatalf("glob no-match production/team/myapp: got %s, want allow", effect) } } @@ -185,8 +186,9 @@ func TestEvaluateEmptyRepoGlobalOperation(t *testing.T) { t.Fatalf("admin catalog: got %s, want allow", effect) } - // System account with a repo-scoped rule should be denied for catalog - // (global op doesn't match repo-scoped rules). + // System account catalog: default rule -2 (authenticated users) + // allows catalog with no repo restriction, so this is allowed even + // though the custom repo-scoped rule would not match. rules = append(rules, Rule{ ID: 1, Priority: 50, @@ -201,8 +203,8 @@ func TestEvaluateEmptyRepoGlobalOperation(t *testing.T) { Action: ActionCatalog, } effect, _ = Evaluate(input, rules) - if effect != Deny { - t.Fatalf("system catalog with repo-scoped rule: got %s, want deny", effect) + if effect != Allow { + t.Fatalf("system catalog: got %s, want allow", effect) } } @@ -289,10 +291,12 @@ func TestDefaultRulesUserContentAccess(t *testing.T) { } } -func TestDefaultRulesSystemAccountDeny(t *testing.T) { +func TestDefaultRulesSystemAccountAccess(t *testing.T) { rules := DefaultRules() - actions := []Action{ActionPull, ActionPush, ActionDelete, ActionCatalog, ActionPolicyManage} - for _, a := range actions { + + // System accounts are now allowed for content actions via rule -2. + allowed := []Action{ActionPull, ActionPush, ActionDelete, ActionCatalog} + for _, a := range allowed { input := PolicyInput{ Subject: "system-uuid", AccountType: "system", @@ -300,10 +304,21 @@ func TestDefaultRulesSystemAccountDeny(t *testing.T) { Repository: "myapp", } effect, _ := Evaluate(input, rules) - if effect != Deny { - t.Errorf("system %s: got %s, want deny", a, effect) + if effect != Allow { + t.Errorf("system %s: got %s, want allow", a, effect) } } + + // policy:manage should still be denied for system accounts. + input := PolicyInput{ + Subject: "system-uuid", + AccountType: "system", + Action: ActionPolicyManage, + } + effect, _ := Evaluate(input, rules) + if effect != Deny { + t.Errorf("system policy:manage: got %s, want deny", effect) + } } func TestDefaultRulesVersionCheckAlwaysAllowed(t *testing.T) {