Skip to content

calm-hub: GitHub-mode access/namespace resolution needs a single authoritative path #3092

Description

@jpgough-ms

GitHub-mode access resolution has no single authoritative implementation. Six symptoms of that one root cause:

  1. Two independent access-control mechanisms. Namespace visibility inside GitHubControlStore/GitHubDomainStore goes through NamespaceAccessFilter.getAccessibleNamespaces() (group membership vs GitHubCloneManager.getAccessGroupsForNamespace + OidcRoleResolver). REST-layer @PermissionsAllowed gates go through GitHubUserAccessStore.getGrantsForUser, independently re-deriving the same access from the same inputs. They agree today only because the inputs match — nothing enforces that going forward.
  2. Domain matching disagrees between call sites. GitHubControlStore matches by substring (path.contains("controls/" + domain + "/")); GitHubControlDomains.extractDomain matches by path position. Disagree on a nested path like controls/security/legacy/x.json.
  3. extractDomain returns the raw filename for a control with no domain subdirectory (controls/root-control.json → domain "root-control.json", not "default").
  4. GitHubUserAccessStore.getGrantsForUser(String username) ignores its own argument — resolves against SecurityIdentity instead. Harmless today (only caller passes its own name), wrong for any future admin/introspection use.
  5. Dotted namespaces are unreadable unless every ancestor prefix is separately registered. NAMESPACE_REGEX permits dots; hasNamespaceAccess's READ path requires ALL ancestors to independently have a grant; GitHub mode's namespace model has no concept of nesting.
  6. proxy-auth + calm.database.mode=github together silently deny every userproxy-auth has no groups claim, every rule above is group-based, so access resolves to NONE for everyone. Fails closed, but silent.

Fix: one authoritative access-resolution path called by both the store layer and the REST-permission layer; fix extractDomain's no-subdirectory case; guard or fix getGrantsForUser's unused argument; decide how (or whether) to support dotted namespaces without phantom ancestor registrations; add a startup warning for the proxy-auth + github combination.

(Surfaced during #3066's review, building the end-to-end DOMAIN_READ integration test.)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    calm-hubAffects `calm-hub`

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions