Skip to content

Chained intersectScopes re-widens: an emptied intersection is re-read as "no scope requested" and grants the next layer's full allow-list #305

Description

@saucam

Found while verifying the scope-mechanism review in highflame-ai/highflame-sdk#143. Not reported by any of the six sub-issues; it lives in the same function #301 proposes to rewrite, so it should land as part of that change rather than separately.

The bug

intersectScopes returns the allowed set when requested is empty — the RFC 6749 §3.3 "pre-defined default":

func intersectScopes(requested, allowed []string) []string {
	if len(allowed) == 0 {
		return requested
	}
	if len(requested) == 0 {
		return allowed      // <-- RFC 6749 §3.3 default
	}
	...

Correct for a single call. apiKeyGrant chains four of them (internal/service/oauth.go:1367-1386):

scopes := parseScopeString(req.Scope)
scopes = intersectScopes(scopes, sk.Scopes)                  // layer 1
scopes = intersectScopes(scopes, kp.AllowedScopes)           // layer 2
scopes = intersectScopes(scopes, identityPolicyScopes)       // layer 3
scopes = intersectScopes(scopes, identity.AllowedScopes)     // layer 4

An emptied intersection and "no scope requested" are the same value — nil. So the moment any layer narrows to nothing, the next layer reads that as "caller asked for nothing, grant the default" and hands back its full allow-list.

Reproduction

Integration probe, real Postgres via testcontainers:

key.scopes = [data:read]        key policy allows [data:read, data:write]

requested "data:read"   (key holds it)      -> 200  [data:read]
requested "data:write"  (key does NOT hold) -> 200  [data:read data:write]   <-- broader

Requesting a scope you are forbidden yields a strictly broader token than requesting a permitted one, and escapes the key's own scopes[] restriction entirely. EnforcePolicy then passes it, because the re-widened set is by construction a subset of the policy it was re-widened from. The per-key scopes[] field is never checked at the issuance chokepoint — only in this resolution chain — so nothing else catches it.

apiKeyGrant is the only chained call site. jwtBearer (oauth.go:727), clientCredentials (oauth.go:593), IssueAuthCode (oauth.go:1614) and cimd.go:524 each call it once and are unaffected.

Live exposure: currently zero, and worth saying so plainly

Measured on dev1 and prod (highflame_authn):

dev1 prod
Active service keys 823 845
Keys using the per-key scopes[] field 0 0
Keys with a key policy ceiling + a distinct scope-bearing identity policy 573 198

The escalating variant needs a key that populates scopes[]. No key in either environment does. So this is latent, not live.

The second row is the shape that does exist — layer 2 empties, layer 3 re-widens to the identity policy's full set. That one is caught by IssueCredential's dual enforcement, because the re-widened set exceeds the narrower key policy:

requested "data:read"     -> 200  [data:read]
requested "billing:write" -> 400  policy_violation:
                             scope "admin:all" is not permitted by policy

Contained — but note the error names admin:all, a scope the caller never mentioned. That is the re-widening surfacing as a genuinely baffling diagnostic on 573 dev1 / 198 prod keys today.

Ask

Make "narrowed to nothing" distinguishable from "nothing requested". Options:

  1. Apply the §3.3 default once, up front — resolve the empty-request case before the chain starts, then make the chained calls pure intersection with no defaulting.
  2. Return an explicit sentinel (e.g. ([]string, bool) for "restricted to empty") so subsequent layers can tell the two apart.

Option 1 is the smaller change and keeps intersectScopes honest as a set operation.

This interacts directly with #301: that issue already proposes refusing to issue on an empty intersection, which would also close this. Landing them together avoids touching the same function twice.

Verification

Reproduced by TestProbe_P3_EmptiedIntersectionReWidensAtNextLayer and TestProbe_P6_LiveShape_KeyPolicyNarrowerThanIdentityPolicy in a local probe suite (tests/integration/, not yet committed — happy to land it as regression coverage alongside the fix).

Activity

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't workinggoPull requests that update go codesecuritySecurity issue

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions