finos / finos/architecture-as-code

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

Open
#3,092 0 comments 0 reactions 0 assignees View on GitHub
calm-hub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.