finos / finos/architecture-as-code
calm-hub: GitHub-mode access/namespace resolution needs a single authoritative path
- Dominant language
- TypeScript
- Stars
- 399
- Forks
- 138
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 38
Description
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 user** — `proxy-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.)
Contributor guide
Research direction
Start by tracing access resolution through GitHubControlStore, GitHubDomainStore, NamespaceAccessFilter, GitHubCloneManager, OidcRoleResolver, and GitHubUserAccessStore, including the REST @PermissionsAllowed path. Review GitHubControlDomains.extractDomain and the namespace access checks, then define how the shared path handles dotted namespaces and proxy-auth. Done means both layers use one resolver, the listed edge cases are addressed, and coverage or startup behavior verifies them.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- authorization, backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100