Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a672066
feat(coderd/rbac): compare scopes by permission coverage
BobbyHo Aug 14, 2026
4315706
Merge branch 'main' into plat479-1-rbac-scope-coverage
BobbyHo Aug 17, 2026
787c461
docs(coderd/rbac): shorten the ScopesCover doc comment
BobbyHo Aug 17, 2026
2d39e04
Merge branch 'main' into plat479-1-rbac-scope-coverage
BobbyHo Aug 18, 2026
2f6c44e
fix(coderd/rbac): guard allowed-side org and user permissions
BobbyHo Aug 18, 2026
bd40270
refactor(coderd/rbac): drop the unreachable negative skip in coverage
BobbyHo Aug 18, 2026
3139c54
test(coderd/rbac): pin the scope coverage table's weak assertions
BobbyHo Aug 18, 2026
9276bb8
refactor(coderd/rbac): make the coverage guards reachable from tests
BobbyHo Aug 18, 2026
865eb9a
refactor(coderd/rbac): share the alias table and name the canonical c…
BobbyHo Aug 18, 2026
26a6bed
docs(coderd/rbac): document the expansion invariant where it can be b…
BobbyHo Aug 18, 2026
7cca7b3
Merge branch 'main' into plat479-1-rbac-scope-coverage
BobbyHo Aug 19, 2026
1678a77
test(coderd/rbac): pin the coverage guards at one strength
BobbyHo Aug 19, 2026
aba3c6c
fix(coderd/rbac): name the scope once in expansion errors
BobbyHo Aug 19, 2026
9a08105
docs(coderd/rbac): correct the external scope list contract
BobbyHo Aug 19, 2026
2fcce8b
docs(coderd/rbac): trim the restated coverage invariant
BobbyHo Aug 19, 2026
a775a48
docs(coderd/rbac): correct the negative permission cross-reference
BobbyHo Aug 19, 2026
e0c0d4a
docs(coderd/rbac): name every category IsExternalScope admits
BobbyHo Aug 19, 2026
189740d
test(coderd/rbac): pin the alias list invariants on the alias table
BobbyHo Aug 19, 2026
7d08e49
Merge branch 'main' into plat479-1-rbac-scope-coverage
BobbyHo Aug 19, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions coderd/rbac/scopes.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,89 @@ func expandLowLevel(resource string, action policy.Action) Scope {
AllowIDList: []AllowListElement{{Type: policy.WildcardSymbol, ID: policy.WildcardSymbol}},
}
}

// ScopesCover reports whether every permission the requested scope grants is
// also granted by at least one of the allowed scopes. It compares expanded
// permissions, not names, so `coder:workspaces.access` covers `workspace:read`
// and `coder:all` covers everything.
//
// Names must be canonical (see CanonicalScopeName). An unknown name is an
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
// error rather than a false, since a caller cannot tell those two apart.
//
// Coverage models site-level grants only. Anything else returns an error
// instead of being skipped, because skipping it could report "covered" about
// authority that was never compared. The one exception is an unmodelled grant
// on the allowed side, which is dropped. Dropping it only shrinks the ceiling,
// so at worst it rejects a request that would have been allowed. A negative
// permission or an allow list is never dropped, on either side, since
// dropping one would widen the ceiling rather than shrink it.
func ScopesCover(allowed []ScopeName, requested ScopeName) (bool, error) {
want, err := ExpandScope(requested)
if err != nil {
return false, xerrors.Errorf("expand requested scope: %w", err)
Comment thread
BobbyHo marked this conversation as resolved.
}
// Scope expansion populates Site only, with a wildcard allow list and no
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
// negative permissions. These guards hold that invariant: if a future
// scope breaks it, coverage stops being decidable here and the request is
// refused rather than approved on an incomplete comparison.
if len(want.User) > 0 || len(want.ByOrgID) > 0 {
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
return false, xerrors.Errorf("scope %q grants org or user permissions, which coverage does not model", requested)
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
}
for _, perm := range want.Site {
if perm.Negate {
return false, xerrors.Errorf("scope %q carries a negative permission, which coverage does not model", requested)
}
}
if !allowListContainsAll(want.AllowIDList) {
return false, xerrors.Errorf("scope %q carries a resource allow list, which coverage does not model", requested)
}

granted := make([]Permission, 0, len(allowed)*4)
Comment thread
BobbyHo marked this conversation as resolved.
for _, name := range allowed {
expanded, err := ExpandScope(name)
if err != nil {
return false, xerrors.Errorf("expand allowed scope %q: %w", name, err)
}
// A narrower allow list on the allowed side would make these
// permissions conditional, and treating them as unconditional would
// overstate the ceiling.
if !allowListContainsAll(expanded.AllowIDList) {
return false, xerrors.Errorf("allowed scope %q carries a resource allow list, which coverage does not model", name)
}
// A negative permission is the one thing on this side that cannot be
// dropped safely. Ignoring an unmodelled grant narrows the ceiling,
// but ignoring an anti-grant widens it: an "everything except delete"
// scope would otherwise cover a request for delete.
for _, perm := range expanded.Site {
if perm.Negate {
return false, xerrors.Errorf("allowed scope %q carries a negative permission, which coverage does not model", name)
}
}
granted = append(granted, expanded.Site...)
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
}

for _, needed := range want.Site {
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
if !permissionCovered(needed, granted) {
return false, nil
}
}
return true, nil
}

// permissionCovered reports whether any granted permission subsumes needed,
// treating the wildcard resource type and action as covering every value.
func permissionCovered(needed Permission, granted []Permission) bool {
Comment thread
BobbyHo marked this conversation as resolved.
for _, perm := range granted {
if perm.Negate {
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
continue
}
if perm.ResourceType != needed.ResourceType && perm.ResourceType != policy.WildcardSymbol {
continue
}
if perm.Action != needed.Action && perm.Action != policy.WildcardSymbol {
continue
}
return true
}
return false
}
18 changes: 18 additions & 0 deletions coderd/rbac/scopes_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,24 @@ func IsExternalScope(name ScopeName) bool {
return false
}

// CanonicalScopeName maps the backward-compatibility aliases IsExternalScope
// accepts onto the names the api_key_scope enum stores. Any other name is
// returned unchanged.
//
// IsExternalScope answers whether a name may be requested; it does not answer
// how that name is spelled once persisted. The aliases `all` and
// `application_connect` are accepted but are not enum members, so a caller
// that stores what it validated must canonicalize in between.
func CanonicalScopeName(name ScopeName) ScopeName {
Comment thread
BobbyHo marked this conversation as resolved.
Comment thread
BobbyHo marked this conversation as resolved.
switch name {
case "all":
return ScopeAll
case "application_connect":
return ScopeApplicationConnect
}
return name
}

// ExternalScopeNames returns a sorted list of all public scopes, which
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
// includes the `all` and `application_connect` special scopes, curated
// low-level resource:action names, and curated composite coder:* scopes.
Expand Down
148 changes: 148 additions & 0 deletions coderd/rbac/scopes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,151 @@ func TestExpandScope(t *testing.T) {
}
})
}

func TestScopesCover(t *testing.T) {
t.Parallel()

tests := []struct {
name string
allowed []rbac.ScopeName
requested rbac.ScopeName
want bool
wantErr bool
}{
{
name: "IdenticalName",
allowed: []rbac.ScopeName{"workspace:read"},
requested: "workspace:read",
want: true,
},
{
// The case name matching cannot answer: the composite expands to
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
// include the requested permission, so the request is within the
// authority the composite already grants.
name: "CompositeCoversItsMember",
allowed: []rbac.ScopeName{"coder:workspaces.access"},
requested: "workspace:ssh",
want: true,
},
{
name: "CompositeDoesNotCoverNonMember",
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
allowed: []rbac.ScopeName{"coder:workspaces.access"},
requested: "workspace:delete",
want: false,
},
{
// Same resource, different action. Coverage compares the pair,
// not the resource alone.
name: "CompositeDoesNotCoverWiderActionOnCoveredResource",
allowed: []rbac.ScopeName{"coder:workspaces.access"},
requested: "template:update",
want: false,
},
{
name: "AllCoversEverything",
allowed: []rbac.ScopeName{rbac.ScopeAll},
requested: "user_secret:delete",
want: true,
},
{
name: "NarrowScopeDoesNotCoverAll",
allowed: []rbac.ScopeName{"workspace:read"},
requested: rbac.ScopeAll,
want: false,
},
{
name: "ResourceWildcardCoversOneAction",
allowed: []rbac.ScopeName{"workspace:*"},
requested: "workspace:ssh",
want: true,
},
{
name: "OneActionDoesNotCoverResourceWildcard",
allowed: []rbac.ScopeName{"workspace:ssh"},
requested: "workspace:*",
want: false,
},
{
// A composite is covered only when every permission it expands
// to is granted, so a strict subset of them is not enough.
name: "PartialUnionDoesNotCoverComposite",
allowed: []rbac.ScopeName{"template:read", "file:create"},
requested: "coder:templates.build",
want: false,
},
{
// The allowed side is a union rather than a set of independent
// candidates, so one composite's permissions may be drawn from
// several allowed entries at once.
name: "UnionOfAllowedScopesCoversComposite",
allowed: []rbac.ScopeName{"template:read", "file:*", "provisioner_jobs:read"},
requested: "coder:templates.build",
want: true,
},
{
name: "EmptyAllowedCoversNothing",
allowed: nil,
requested: "workspace:read",
want: false,
},
{
// Not a false: a caller cannot distinguish "known and not
// covered" from "we could not tell", so an undecidable
// comparison is surfaced rather than answered.
name: "UnknownRequestedScopeErrors",
allowed: []rbac.ScopeName{rbac.ScopeAll},
requested: "not_a_real_scope",
wantErr: true,
},
{
name: "UnknownAllowedScopeErrors",
allowed: []rbac.ScopeName{"not_a_real_scope"},
requested: "workspace:read",
wantErr: true,
},
{
// The aliases IsExternalScope accepts are not expandable names,
// so callers must canonicalize before asking about coverage.
name: "NonCanonicalAliasErrors",
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
allowed: []rbac.ScopeName{rbac.ScopeAll},
requested: "all",
wantErr: true,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()

got, err := rbac.ScopesCover(test.allowed, test.requested)
if test.wantErr {
require.Error(t, err)
Comment thread
BobbyHo marked this conversation as resolved.
Outdated
require.False(t, got, "an undecided comparison must not report coverage")
return
}
require.NoError(t, err)
require.Equal(t, test.want, got)
})
}
}

// TestScopesCoverEveryExternalScope asserts the property the OAuth2 allowlist
// check depends on: coder:all is a ceiling over the whole external catalog, and
// every catalog name covers itself. A name that cannot be compared at all would
// otherwise reject every request naming it, which is a rejection no app owner
// could act on.
func TestScopesCoverEveryExternalScope(t *testing.T) {
t.Parallel()

for _, name := range rbac.ExternalScopeNames() {
canonical := rbac.CanonicalScopeName(rbac.ScopeName(name))

covered, err := rbac.ScopesCover([]rbac.ScopeName{rbac.ScopeAll}, canonical)
require.NoErrorf(t, err, "coder:all vs %q", canonical)
require.Truef(t, covered, "coder:all must cover %q", canonical)

covered, err = rbac.ScopesCover([]rbac.ScopeName{canonical}, canonical)
require.NoErrorf(t, err, "%q vs itself", canonical)
require.Truef(t, covered, "%q must cover itself", canonical)
}
}
Loading