Conversation
Contributor
Author
|
This change is part of the following stack: Change managed by git-spice. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Contributor
tdraier
force-pushed
the
migrate-allowed-tiers-off-groupmodelids
branch
from
September 22, 2026 08:51
7192261 to
86b6d69
Compare
`allowed_tiers` was the last consumer of `Authenticator.groupModelIds()` (and thus of the cached `_groupModelIds` field) for its group-override source. It now resolves the caller's tier-override group memberships live from the user via `GroupResource.dangerouslyListUserGroupsForAuth`, and drops the now-dead `groupModelIds()` accessor. Behavior: group-based `models_tier` overrides are a per-user feature, so they now apply only to auths backed by a workspace-member user. Auths with no user (API keys, system, internal, poke) get no group override and fall back to the user/workspace tiers — previously a key/superuser could inherit a group's override via its construction-time group set. Agent-driven runs are unaffected; they bypass this path via getAgentAllowedTierNamesOverride. The user and workspace tier sources, and the user > group > workspace precedence, are unchanged. Adds a code contract and a test covering the non-user case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rename `_groupModelIds` -> `_requestedGroupModelIds` and persist it ONLY for auths whose group scope can't be re-derived from the auth's own identity: - sandbox tokens (narrowed to the workload's spaces from token claims that aren't retained), - the `fromKey` `requestedGroupIds` override (a system key acting as a user), - internal/system auths (no user, no key). `null` means "derive on refresh". `refresh()` now consolidates all sources: explicit scope -> reuse it; user auth -> live membership; API-key auth -> re-derive from the key (`KeyResource.fetchByWorkspaceAndId` + the key's groups, workspace-scoped so a foreign-workspace key yields none). So user and plain API-key auths no longer carry a stored group snapshot — the user-group snapshot is gone, and keys refresh from `_key` alone. Serialization keeps the field name `groupIds` but makes it nullable to distinguish "derive" (null) from an explicit empty scope ([]); user/plain-key auths serialize null and recompute on rehydrate. Behavior-preserving for `_permissions`; adds a key-refresh test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tdraier
force-pushed
the
migrate-allowed-tiers-off-groupmodelids
branch
from
September 22, 2026 14:48
86b6d69 to
0e84963
Compare
Contributor
|
🟡 1 security issue detected below the blocking threshold. Reviewed everything up to 0e84963. The following issues were found:
Evidence: The PR changes rehydration to set
Security OverviewDetected Code Changes
|
![zeropath-ai[bot]](https://avatars.githubusercontent.com/in/921241?s=60&v=4){kind=small}
Comment on lines
+1836
to
+1841
| // `null` = no explicit scope (derive on refresh). A serialized list rehydrates the explicit | ||
| // scope. (Old payloads that stored the full group set land here as an explicit scope too, which | ||
| // is correct: refresh reuses it instead of re-deriving until the payload cycles out.) | ||
| const requestedGroupModelIds = authType.groupIds | ||
| ? removeNulls(authType.groupIds.map((sId) => getResourceIdFromSId(sId))) | ||
| : null; |
Contributor
There was a problem hiding this comment.
Unauthorized Access via In-Flight Authenticator Rehydration in front/lib/auth.ts (Severity: MEDIUM)
Impact: revoking a key’s group or deleting the key may not remove access for in-flight authenticators, allowing continued workspace/space access until the snapshot expires. The code path rehydrates explicit scopes from legacy serialized groupIds in fromJSON, causing refresh() to trust the old in-snapshot group set instead of deriving from the current key, which leads to stale permissions persisting across key changes.
View details in ZeroPath
Automatic patch generation was not possible for this finding.
💬 Reply @ZeroPath false-positive because … or @ZeroPath accepted-risk because … to triage this finding, or ask it any question.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
allowed_tierswas the last consumer ofAuthenticator.groupModelIds()(the cached_groupModelIdsfield) — it used it to find the caller's tier-override groups. It now resolves that membership live from the user viaGroupResource.dangerouslyListUserGroupsForAuth, and the now-deadgroupModelIds()accessor is removed.Why not governance grants:
GroupPermissionsdeliberately holds no group ids and flattens sources, so it can't express the tier resolver'suser > highest group > workspaceprecedence (a flat union would over-grant). Tier overrides are fundamentally a group-membership feature, so the migration replaces the field read with a membership query rather than a governance lookup.Behavior change (intentional): group-based
models_tieroverrides now apply only to auths backed by a workspace-member user. Auths with no user — API keys, system, internal, poke — get no group override and fall back to the user/workspace tiers; previously a key/superuser could inherit a group override via its construction-time group set. Agent-driven runs are unaffected (they bypass this path viagetAgentAllowedTierNamesOverride). The user/workspace sources and precedence are unchanged. Documented via the newmodels-tier-group-override-follows-user-membershipcode contract.Tests
lib/model_tiers/allowed_tiers.test.ts— 11/11 pass, including a new case asserting a no-user internal auth (carrying all workspace groups) does not inherit a group override.tsgoclean on changed files.Risk
Low. The group-tier-override path is user-facing (agent runs use the override bypass). The only behavior delta is non-user auths no longer inheriting group overrides, which is the intended semantics.
Deploy Plan
Standard.
🤖 Generated with Claude Code