Skip to content

Commit 8953f21

Browse files
matt-aitkenclaude
andcommitted
fix(rbac): preserve colons in scope resource ids
`buildJwtAbility` parsed scopes with `[a, b, c] = scope.split(":")` which captures only the first three segments. A scope like `read:tags:env:staging` (a tag id containing a colon) lost everything after the second colon — `scopeId` became `"env"`, and the tag `{ type: "tags", id: "env:staging" }` silently failed to match. Fix: split into all parts, then take everything after the second colon as the resource id (`parts.slice(2).join(":")`). Two-segment scopes (`read:tags`) still produce `scopeId === undefined`, preserving the type-level wildcard semantic. Test coverage added for the multi-colon-id path. The bug was pre-existing in the legacy `checkAuthorization` too — system-generated ids (friendlyIds like `run_abc`) use underscores, so the issue only surfaced for user-provided strings (tags), which made it easy to miss. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ee13f2b commit 8953f21

2 files changed

Lines changed: 20 additions & 1 deletion

File tree

internal-packages/rbac/src/ability.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,17 @@ describe("buildJwtAbility", () => {
5353
expect(ability.can("read", { type: "runs" })).toBe(false);
5454
});
5555

56+
it("preserves colons in the resource id (everything after the 2nd colon)", () => {
57+
// Resource ids can contain colons (e.g. user-provided tags like
58+
// `env:staging`). The naive `[a, b, c] = scope.split(":")` form
59+
// truncated `read:tags:env:staging` → scopeId="env" and silently
60+
// mis-matched. Regression coverage for the multi-colon id path.
61+
const ability = buildJwtAbility(["read:tags:env:staging"]);
62+
expect(ability.can("read", { type: "tags", id: "env:staging" })).toBe(true);
63+
expect(ability.can("read", { type: "tags", id: "env" })).toBe(false);
64+
expect(ability.can("read", { type: "tags", id: "env:prod" })).toBe(false);
65+
});
66+
5667
it("allows any read with read:all scope", () => {
5768
const ability = buildJwtAbility(["read:all"]);
5869
expect(ability.can("read", { type: "runs" })).toBe(true);

internal-packages/rbac/src/ability.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,15 @@ export function buildFallbackAbility(isAdmin: boolean): RbacAbility {
2626
export function buildJwtAbility(scopes: string[]): RbacAbility {
2727
const matches = (action: string, r: RbacResource): boolean =>
2828
scopes.some((scope) => {
29-
const [scopeAction, scopeType, scopeId] = scope.split(":");
29+
// Only the first two colons are delimiters — everything after the
30+
// second colon is the resource id (which may itself contain colons,
31+
// e.g. user-provided tags like "env:staging"). Naive
32+
// `split(":")` + 3-tuple destructuring truncated such ids to the
33+
// first segment and silently failed to match.
34+
const parts = scope.split(":");
35+
const scopeAction = parts[0];
36+
const scopeType = parts[1];
37+
const scopeId = parts.length > 2 ? parts.slice(2).join(":") : undefined;
3038
// Bare `admin` is the universal wildcard. `admin:<type>` is *not* —
3139
// it falls through to normal matching as action="admin" against
3240
// resources of that type. Pre-RBAC, the legacy checkAuthorization

0 commit comments

Comments
 (0)