Skip to content

Commit 145da03

Browse files
dmoernerclaude
andcommitted
fix: fail closed on negative v2 feature-permission masks
bin() renders a negative int as "-0b111...", so slicing off two characters left a stray "b" followed by the magnitude's bits. Walking that granted permissions that were never assigned -- 63 of them for a mask of -9223372036854775807. The backend builds these masks in a signed 64-bit integer, so an action at bit index 63 overflows into exactly that shape. Reject any mask that is not a non-negative ASCII decimal, mirroring decimalToBinaryBits in @clerk/shared. Large positive masks are unaffected: Python ints are arbitrary-precision and already decode any width exactly. Part of CORE-3701 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 86216c3 commit 145da03

2 files changed

Lines changed: 103 additions & 3 deletions

File tree

src/clerk_backend_api/security/authenticaterequest.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import re
12
from http.cookies import SimpleCookie
23
from typing import Any, Dict, List, Optional
34

@@ -11,6 +12,11 @@
1112
)
1213

1314

15+
# ASCII digits only: str.isdigit() and the regex \d class both accept other
16+
# Unicode digit forms, which are not valid in a JWT claim.
17+
_DECIMAL_MASK = re.compile(r"[0-9]+")
18+
19+
1420
def _compute_org_permissions(claims: Dict[str, Any]) -> List[str]:
1521
features_str = claims.get("fea")
1622
if features_str is None:
@@ -40,11 +46,17 @@ def _compute_org_permissions(claims: Dict[str, Any]) -> List[str]:
4046
if "o" not in scope:
4147
continue
4248

43-
try:
44-
binary = bin(int(mapping))[2:].lstrip("0")
45-
except ValueError:
49+
# Only a non-negative decimal integer is a valid mask. A negative value
50+
# means the issuer overflowed a signed integer while encoding, and
51+
# bin() renders it as "-0b111...", so the [2:] slice would leave a
52+
# stray "b" and the remaining digits would grant the magnitude's bits --
53+
# permissions that were never assigned. Reject rather than guess,
54+
# mirroring decimalToBinaryBits in @clerk/shared.
55+
if _DECIMAL_MASK.fullmatch(mapping) is None:
4656
continue
4757

58+
binary = bin(int(mapping))[2:].lstrip("0")
59+
4860
reversed_binary = binary[::-1]
4961

5062
for i, bit in enumerate(reversed_binary):

tests/test_org_permissions_v2.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
"""Tests for decoding v2 session token org permissions (`fea` / `o.per` / `o.fpm`).
2+
3+
A permission key is `org:<feature>:<action>`. The token factors these into an
4+
ordered feature list, a shared action vocabulary, and one bitmask per feature
5+
over that vocabulary. Bit j of mask i means feature i grants action j, with
6+
bit 0 least significant.
7+
"""
8+
9+
import pytest
10+
11+
from clerk_backend_api.security.authenticaterequest import _compute_org_permissions
12+
13+
14+
def claims(fea: str, per: str, fpm: str):
15+
return {"fea": fea, "o": {"per": per, "fpm": fpm}}
16+
17+
18+
def test_decodes_a_normal_mask():
19+
assert _compute_org_permissions(
20+
claims("o:leads,o:whatsapp", "read,manage", "3,1")
21+
) == ["org:leads:read", "org:leads:manage", "org:whatsapp:read"]
22+
23+
24+
def test_decodes_bit_62():
25+
perms = ",".join(f"p{i:02d}" for i in range(80))
26+
assert _compute_org_permissions(
27+
claims("o:repositories", perms, str((1 << 62) | 1))
28+
) == ["org:repositories:p00", "org:repositories:p62"]
29+
30+
31+
def test_decodes_a_positive_bignum_at_bit_63():
32+
"""Python ints are arbitrary-precision, so a wide positive mask is exact."""
33+
perms = ",".join(f"p{i:02d}" for i in range(80))
34+
assert _compute_org_permissions(
35+
claims("o:repositories", perms, str((1 << 63) | 1))
36+
) == ["org:repositories:p00", "org:repositories:p63"]
37+
38+
39+
def test_decodes_beyond_64_bits():
40+
perms = ",".join(f"p{i:02d}" for i in range(80))
41+
assert _compute_org_permissions(
42+
claims("o:repositories", perms, str((1 << 72) | 1))
43+
) == ["org:repositories:p00", "org:repositories:p72"]
44+
45+
46+
@pytest.mark.parametrize(
47+
"mask",
48+
[
49+
"-9223372036854775807",
50+
"-9223372036854775808",
51+
"-1",
52+
],
53+
)
54+
def test_negative_masks_grant_nothing(mask):
55+
"""A negative mask can only come from an issuer that overflowed a signed
56+
integer. bin() renders it as "-0b111...", so slicing off two characters
57+
leaves a stray "b" and the magnitude's bits, which previously granted
58+
permissions that were never assigned -- 63 of them for the first case."""
59+
perms = ",".join(f"p{i:02d}" for i in range(80))
60+
assert _compute_org_permissions(claims("o:repositories", perms, mask)) == []
61+
62+
63+
@pytest.mark.parametrize("mask", ["0x1f", "1.5", "", " 3", "3 ", "abc", "+3", "1"])
64+
def test_non_decimal_masks_grant_nothing(mask):
65+
assert _compute_org_permissions(claims("o:repositories", "read,manage", mask)) == []
66+
67+
68+
def test_zero_mask_grants_nothing():
69+
assert _compute_org_permissions(claims("o:repositories", "read,manage", "0")) == []
70+
71+
72+
def test_mask_wider_than_the_vocabulary_is_bounded():
73+
"""Extra high bits must not emit permissions with no action name."""
74+
assert _compute_org_permissions(
75+
claims("o:repositories", "read,manage", str((1 << 40) | 1))
76+
) == ["org:repositories:read"]
77+
78+
79+
def test_user_scoped_features_are_skipped():
80+
assert _compute_org_permissions(
81+
claims("u:impersonation,o:leads", "read,manage", "1,1")
82+
) == ["org:leads:read"]
83+
84+
85+
def test_more_masks_than_features_are_ignored():
86+
assert _compute_org_permissions(claims("o:leads", "read,manage", "1,3")) == [
87+
"org:leads:read"
88+
]

0 commit comments

Comments
 (0)