Skip to content

Commit d6c0c58

Browse files
authored
Merge branch 'master' into scraps/form/hash-iv
2 parents 9b8862b + 4022fa9 commit d6c0c58

File tree

55 files changed

+375
-338
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+375
-338
lines changed

.claude/skills/sentry-security/SKILL.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ The most common vulnerability. An endpoint accepts an ID from the request but do
6767
- Query includes `organization_id=organization.id` where `organization` comes from `convert_args()`
6868
- Uses `self.get_projects()` which scopes by org internally
6969
- Object is fetched via URL kwargs resolved by `convert_args()`
70+
- Unscoped query is a guard that only raises an error (never returns data), AND
71+
a downstream query in the same flow IS org-scoped and raises the same error —
72+
no differential behavior means no information leak
7073

7174
### Check 2: Missing Authorization Checks — 10 patches last year
7275

@@ -137,10 +140,19 @@ For each potential finding, trace the **complete** request flow end-to-end. Do n
137140
7. Serializer → do validate_*() methods enforce it?
138141
```
139142

140-
**A check at ANY layer is enforcement.** Before marking HIGH, confirm the check is absent from all 7 layers using the checklist in `enforcement-layers.md`.
143+
**A check at ANY layer is enforcement.** Before marking HIGH, confirm the check is absent from all layers using the checklist in `enforcement-layers.md`.
141144

142145
If you cannot confirm the check is absent from every layer, mark the finding as **MEDIUM** (needs verification), not HIGH.
143146

147+
**Cross-flow enforcement for token issuance:** For token/credential issuance flows, also check whether the issued credential is blocked at **usage time** (e.g., `determine_access()` rejects it at all endpoints in the relevant scope). Classify based on the enforcement scope:
148+
149+
- **Centralized enforcement** (check runs in a permission class inherited by all endpoints in the affected scope) → the credential is effectively inert → **LOW** (do not report)
150+
- **Scattered enforcement** (only some endpoints or serializers check, others may not) → **MEDIUM** (report as needs verification)
151+
152+
See `enforcement-layers.md` "Cross-Flow Enforcement."
153+
154+
**Non-DRF views:** OAuth views are plain Django views — the 7-layer DRF model does not apply to the view itself. Check the view's own decorators and handler logic. But tokens issued by these views are later used at DRF endpoints where the full enforcement chain applies.
155+
144156
## Step 4: Report Findings
145157

146158
````markdown

.claude/skills/sentry-security/references/endpoint-patterns.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,49 @@ class BaseDataConditionGroupValidator(serializers.Serializer):
107107
□ For PUT/POST/DELETE: the object being modified is scoped by org/project
108108
```
109109

110+
## Guard Queries vs. Data Queries
111+
112+
Not all unscoped queries are exploitable. Before flagging an unscoped query, determine
113+
whether it is a **data query** or a **guard query**:
114+
115+
**Data query** — fetches an object whose attributes are returned to the caller (in a
116+
Response, serializer, or side effect). An unscoped data query is a real IDOR because the
117+
attacker receives cross-org information.
118+
119+
**Guard query** — checks for the existence of a record only to raise an error or block
120+
access. The query result is never returned to the caller.
121+
122+
**A guard query is not exploitable when:**
123+
124+
1. It only raises an error (e.g., `ResourceDoesNotExist`, `PermissionDenied`)
125+
2. A downstream query in the same request flow IS org-scoped
126+
3. The downstream query raises the same error class for the same input
127+
4. Therefore the attacker observes identical responses regardless of the guard
128+
129+
**Example (not exploitable):**
130+
131+
```python
132+
# Guard query — no org scope, but only raises an error
133+
invite = OrganizationMemberInvite.objects.filter(organization_member_id=om_id).first()
134+
if invite is not None:
135+
raise ResourceDoesNotExist # ← same error as below
136+
137+
# Data query — properly org-scoped
138+
return OrganizationMember.objects.filter(id=om_id, organization_id=org.id).get()
139+
# ↑ DoesNotExist → ResourceDoesNotExist
140+
```
141+
142+
**Example (exploitable — still flag this):**
143+
144+
```python
145+
# Unscoped query whose result is RETURNED to the caller
146+
widget = DashboardWidget.objects.get(id=widget_id) # ← no org scope
147+
return Response(serialize(widget)) # ← attacker gets cross-org data
148+
```
149+
150+
**Note:** Even when a guard query is not exploitable, adding org scoping is valid
151+
defense-in-depth. But it should not be reported as a finding.
152+
110153
## Using self.get_projects()
111154

112155
When project IDs come from the request, always use `self.get_projects()`:

.claude/skills/sentry-security/references/enforcement-layers.md

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,31 @@ Classes like `Validator`, `ManualTokenRefresher`, `GrantExchanger`, and `Refresh
6969

7070
`validate_*()` methods and field-level validators may enforce org/project scoping on FK references.
7171

72+
## Non-DRF Views
73+
74+
OAuth views (`OAuthAuthorizeView`, `OAuthDeviceView`, `OAuthTokenView`) are plain Django views, **not** DRF endpoints. Layers 1–4 do not apply to the view itself. Check the view's own authentication decorators, `dispatch()`, and handler logic directly.
75+
76+
However, tokens issued by these views are later used at DRF API endpoints where layers 1–7 **do** apply. See "Cross-Flow Enforcement" below.
77+
78+
## Cross-Flow Enforcement
79+
80+
For token and credential issuance, enforcement may exist in a **different request flow** than the one being reviewed:
81+
82+
- **Issuance flow**: The OAuth authorize/token view that creates the credential
83+
- **Usage flow**: The DRF API endpoints where the credential is subsequently used
84+
85+
If the issued credential cannot be used because a separate enforcement point blocks it, classify based on where the enforcement lives:
86+
87+
- **Centralized enforcement** — the check runs in a permission class inherited by all endpoints within the affected scope. The credential cannot reach any endpoint that lacks the check. Classify as **LOW** (do not report).
88+
- **Scattered enforcement** — the check exists in some endpoints or serializers but not all. The credential may be usable against unchecked endpoints. Classify as **MEDIUM** (report as needs verification).
89+
90+
**Example (LOW — centralized):** OAuth authorize view issues a token to a `member-limit:restricted` member. The token exists, but `is_member_disabled_from_limit()` in `OrganizationPermission.determine_access()` rejects it at every organization-scoped DRF endpoint. Since the token is only usable against organization endpoints (which all inherit this permission class), the enforcement covers all relevant paths. Do not report.
91+
92+
**Example (MEDIUM — scattered):** A token is issued without checking X, and X is only validated in specific endpoint subclasses (not the base). Some endpoints may not inherit the check. Report as needs verification.
93+
7294
## Tracing Requirements
7395

74-
Before marking a finding as **HIGH**, confirm the check is absent from **all** layers:
96+
Before marking a finding as **HIGH**, confirm the check is absent from **all** layers AND from cross-flow enforcement:
7597

7698
```
7799
□ Authentication class does not enforce it
@@ -81,6 +103,7 @@ Before marking a finding as **HIGH**, confirm the check is absent from **all** l
81103
□ Handler method does not enforce it
82104
□ Business logic classes do not enforce it
83105
□ Serializer does not enforce it
106+
□ Cross-flow: the issued credential is not blocked at usage time
84107
```
85108

86-
If the check exists in any layer, the finding is either invalid or at most **MEDIUM** (if the enforcement is indirect or fragile).
109+
If the check exists in any layer or in a cross-flow enforcement point, the finding is either invalid, **LOW** (if enforcement is centralized in a base class), or at most **MEDIUM** (if enforcement is scattered or fragile).

.claude/skills/sentry-security/references/token-lifecycle.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,16 @@ OAuth applications with org-level access did not require `organization_id`, allo
4545

4646
## Member Status Checks
4747

48+
### Member disabled states in Sentry
49+
50+
| State | Field / Flag | Can log in? | Reachable in OAuth? | Where enforced |
51+
| --------------------- | ----------------------------------------------------- | ----------- | ------------------- | --------------------------------------------------------------------------------- |
52+
| Account deactivated | `OrganizationMember.user_is_active=False` | No | No — login blocked | Login flow |
53+
| Pending invitation | `OrganizationMember.is_pending` | No | No — requires login | Login flow |
54+
| Seat-limit restricted | `OrganizationMember.flags["member-limit:restricted"]` | **Yes** | **Yes** | `OrganizationPermission.determine_access()` via `is_member_disabled_from_limit()` |
55+
56+
The seat-limit restricted state is the one that matters for OAuth and token issuance reviews. The user can still log in and complete an OAuth flow, but all organization-scoped DRF endpoints block the resulting token via `is_member_disabled_from_limit()` in `OrganizationPermission`.
57+
4858
### Real vulnerability: Disabled member tokens (PR #92616)
4959

5060
Disabled organization members could still request auth tokens.
@@ -61,6 +71,8 @@ if member.is_pending or not member.user_is_active:
6171
return Response({"detail": "Member is not active"}, status=403)
6272
```
6373

74+
**Known downstream enforcement:** PR #92616 added `is_member_disabled_from_limit()` checks via the organization permission base class. This is **centralized enforcement** — the check runs for every organization-scoped DRF endpoint via `OrganizationPermission.determine_access()`. Tokens held by seat-limit restricted members are blocked at all organization API endpoints. Because the enforcement covers all endpoints the token can be used against, this pattern is **LOW** (do not report). See `enforcement-layers.md` "Cross-Flow Enforcement."
75+
6476
### Real vulnerability: Personal tokens managing org tokens (PR #99457)
6577

6678
Personal API tokens could perform actions on organization auth tokens, bypassing org-level authorization.
@@ -82,6 +94,8 @@ Impersonated sessions (staff acting as a user) had no rate limiting, allowing un
8294
□ Token issuance/refresh endpoints for org-scoped tokens require and validate organization_id
8395
(authentication classes reading existing tokens are exempt — org scoping is enforced by from_rpc_auth())
8496
□ Member active status is checked before token issuance
97+
If missing at issuance but enforced at usage via is_member_disabled_from_limit() in OrganizationPermission → LOW (centralized, do not report)
98+
If enforced only in specific endpoint subclasses → MEDIUM
8599
□ Auth method is appropriate for the operation (org token vs personal token)
86100
□ Impersonated sessions are rate-limited
87101
□ Token revocation cascades properly (revoking app revokes all its tokens)

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
.warden/logs
12
.env
23
.cache/
34
.code-workspace

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
"@sentry-internal/rrweb": "2.40.0",
6969
"@sentry-internal/rrweb-player": "2.40.0",
7070
"@sentry-internal/rrweb-snapshot": "2.40.0",
71-
"@sentry/conventions": "^0.3.1",
71+
"@sentry/conventions": "^0.4.0",
7272
"@sentry/core": "10.27.0",
7373
"@sentry/node": "10.27.0",
7474
"@sentry/react": "10.27.0",

pnpm-lock.yaml

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/sentry/api/endpoints/organization_sessions.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,11 @@ def build_sessions_query(
116116
except NoProjects:
117117
raise NoProjects("No projects available") # give it a description
118118

119-
query_config = release_health.backend.sessions_query_config(organization)
120-
121119
return QueryDefinition(
122120
query=request.GET,
123121
params=params,
124122
offset=offset,
125123
limit=limit,
126-
query_config=query_config,
127124
)
128125

129126
@contextmanager

src/sentry/api/endpoints/release_thresholds/utils/fetch_sessions_data.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,10 @@ def fetch_sessions_data(
4242
query_params["start"] = start.isoformat()
4343
query_params["end"] = end.isoformat()
4444

45-
# crash free rates are on a dynamic INTERVAL basis
46-
# TODO: determine how this affects results for new releases
47-
query_config = release_health.backend.sessions_query_config(organization)
48-
4945
# NOTE: params start/end are overwritten by query start/end
5046
query = QueryDefinition(
5147
query=query_params,
5248
params=params,
53-
query_config=query_config,
5449
)
5550

5651
return release_health.backend.run_sessions_query(

src/sentry/eventstream/item_helpers.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@
1313

1414
from sentry.models.project import Project
1515
from sentry.services.eventstore.models import Event, GroupEvent
16+
from sentry.utils import json
1617
from sentry.utils.eap import hex_to_item_id
1718

19+
# Max depth for recursive encoding to protobuf AnyValue.
20+
_ENCODE_MAX_DEPTH = 6
21+
1822

1923
def serialize_event_data_as_item(
2024
event: Event | GroupEvent, event_data: Mapping[str, Any], project: Project
@@ -31,12 +35,16 @@ def serialize_event_data_as_item(
3135
),
3236
retention_days=event_data.get("retention_days", 90),
3337
attributes=encode_attributes(
34-
event, event_data, ignore_fields={"event_id", "timestamp", "tags"}
38+
event, event_data, ignore_fields={"event_id", "timestamp", "tags", "spans", "'spans'"}
3539
),
3640
)
3741

3842

39-
def _encode_value(value: Any) -> AnyValue:
43+
def _encode_value(value: Any, _depth: int = 0) -> AnyValue:
44+
if _depth > _ENCODE_MAX_DEPTH:
45+
# Beyond max depth, stringify to prevent protobuf nesting limit errors.
46+
return AnyValue(string_value=json.dumps(value))
47+
4048
if isinstance(value, str):
4149
return AnyValue(string_value=value)
4250
elif isinstance(value, bool):
@@ -53,14 +61,16 @@ def _encode_value(value: Any) -> AnyValue:
5361
elif isinstance(value, list) or isinstance(value, tuple):
5462
# Not yet processed on EAP side
5563
return AnyValue(
56-
array_value=ArrayValue(values=[_encode_value(v) for v in value if v is not None])
64+
array_value=ArrayValue(
65+
values=[_encode_value(v, _depth + 1) for v in value if v is not None]
66+
)
5767
)
5868
elif isinstance(value, dict):
5969
# Not yet processed on EAP side
6070
return AnyValue(
6171
kvlist_value=KeyValueList(
6272
values=[
63-
KeyValue(key=str(kv[0]), value=_encode_value(kv[1]))
73+
KeyValue(key=str(kv[0]), value=_encode_value(kv[1], _depth + 1))
6474
for kv in value.items()
6575
if kv[1] is not None
6676
]

0 commit comments

Comments
 (0)