sec(auth): unscoped user API key silently inherits owner's full permissions #61

Summary

After PR LeanerCloud/cloud-commitments-cli#1226 wires user API keys into requirePermission, a user API key created without any explicit scopes silently inherits the owning user's full permission set on the request path. This is the exact "no-scope == full-user-permission" backward-compat trap flagged in the adversarial review brief: dormant before PR LeanerCloud/cloud-commitments-cli#1226 (unscoped keys 401'd), live after (unscoped key = full user perms on every endpoint).

Where it comes from

auth.Service.ComputeEffectivePermissions:

// If API key has no specific permissions, use user's permissions
if len(apiKey.Permissions) == 0 {
    return authCtx.Permissions, nil
}

And CreateAPIKey happily accepts a zero-length Permissions slice:

• APICreateAPIKeyRequest.Permissions is json:"permissions,omitempty".
• validateAPIKeyPermissions iterates the requested permissions; an empty slice passes the loop without error.
• CreateAPIKey stores the key with Permissions: [] (or nil).

PR LeanerCloud/cloud-commitments-cli#1226 documents this as intentional in the HasAPIKeyPermissionAPI docstring:

"A key created without explicit permissions inherits the owner's full permission set."

This is a deliberate policy, but it makes "create the key, leave scope blank" the most permissive option in the UI, and the existing test at service_apikeys_test.go:719-733 already encodes "admin with unscoped key returns full group permissions" as the expected behavior.

Why it's a problem

1. Privilege-escalation surface: a user with a UI that defaults scopes empty (or a CLI that omits --scope) creates a key that, post-PR fix(api): enforce user API key scoped permissions in requirePermission cloud-commitments-cli#1226, can do everything the owner can do. There is no warning at creation time and no way to tell from listAPIKeys output whether a key has zero scopes vs. full scopes (the field is json:"omitempty").
2. Defies the principle of least privilege the per-key scope mechanism was built to enforce.
3. Defeats containment if the user's account is later promoted to admin: an old "view recommendations" key is still bounded, but a no-scope key issued years ago automatically becomes an admin-capable key the moment the owner joins the Administrators group.
4. Audit gap: revoking a leaked no-scope key looks indistinguishable from revoking a leaked admin-scoped key in the access log.

Proposed fix

Make unscoped keys fail closed. Two complementary changes:

1. ComputeEffectivePermissions: when len(apiKey.Permissions) == 0, return []Permission{} (zero perms) rather than the user's full set. An unscoped key authenticates the owner but authorizes nothing. Updates the existing "unscoped key returns full group permissions" tests to assert zero permissions instead.
2. CreateAPIKey: reject len(permissions) == 0 with "at least one permission is required". Forces UI/CLI to make scoping explicit. Add a one-shot migration that flags existing zero-scope keys (or simply lets them keep working post-build: publish the self-hosted platform as a standalone repository #1 above, since Update Deployment Model to Terraform cloud-commitments-cli#1 already neutralizes them).

Either change alone closes the immediate hole; doing both gives defense in depth.

Acceptance

• Unscoped key against any endpoint -> 403.
• Attempt to create a key with permissions: [] -> 400 with a clear error.
• Updated test TestService_ComputeEffectivePermissions / "admin with unscoped key" asserts an empty effective set.
• New test TestService_CreateAPIKey / "fail when permissions is empty".

Surfaced by

Adversarial review of PR LeanerCloud/cloud-commitments-cli#1226. Briefing explicitly flagged this:

"existing customers with API keys without scopes — do they get rejected now? Migration path? Or grandfathered via 'no-scope == full-user-permission' (DANGEROUS — silently keeps the old over-broad behavior)?"

PR LeanerCloud/cloud-commitments-cli#1226 chose the grandfathered path; this issue captures the policy fix needed to close it.