fix: guard nil debug request in bulk check by ivanauth · Pull Request #3174 · authzed/spicedb

ivanauth · 2026-06-11T16:48:54Z

CheckBulkPermissions panics when a DebugInformation entry with a nil
Check.Request reaches the resource-ID matching loop. This happens when a
tracing-enabled request shares a singleflight dispatch with a non-tracing
bulk check call — the combined metadata node produced by
combineResponseMetadata never sets Check.Request.

Add a nil guard to skip such entries.

Closes #3159

codecov · 2026-06-11T16:54:33Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

josephschorr

LGTM

miparnisari

Can you explain this solution? The bug report said

Our CheckPermission calls have with_tracing: true but CheckBulkPermissions has with_tracing: false

how does this translate to if di.Check == nil || di.Check.Request == nil? if i understood correctly, in this particular issue, they would NOT be nil?

You also said

the combined metadata node produced by combineResponseMetadata never sets Check.Request.

is this not the root cause then? Why are we not fixing that?

ivanauth · 2026-06-11T21:23:41Z

Thank you for the feedback!

di.Check.Request actually is nil in this case. The node reaching that loop isn't the original per-resource trace node (those do have Request set). It's the wrapper node created by combineResponseMetadata, which intentionally sets fields like TraceId, SourceId, and SubProblems but leaves Request nil. The real trace entries are nested under SubProblems. The panic is consistent with that: slices.Contains(di.Check.Request.ResourceIds, ...) can only nil-deref if di.Check.Request is nil.

I don't think combineResponseMetadata is the bug. A combined node represents multiple sub-checks, so there isn't a single request to attach to it. The code also relies on Request == nil when deciding whether to flatten subproblems, so populating it would change that behavior.

The actual issue is one layer up: the singleflight key doesn't include the debug flag. That allows a with_tracing=true CheckPermission request and a with_tracing=false CheckBulkPermissions request to share the same flight, which can leak debug metadata into the bulk response.

I've reworked the fix so debug-enabled checks no longer share a singleflight flight and instead dispatch directly. That prevents debug metadata from leaking into non-debug requests and fixes the panic at the source. It also fixes the reverse case, where a tracing request could lose trace data because it shared a flight with a non-tracing request.

I kept the nil check as a defensive guard since Request == nil is still a valid shape for combined metadata nodes.

Happy to adjust the approach if you'd prefer it handled differently or close this PR completely since it's getting too complex.

A DebugInformation entry with a nil Check.Request can reach a non-tracing CheckBulkPermissions caller when an identical tracing-enabled request shares its dispatch, causing a nil pointer dereference. Skip such entries when matching debug info by resource.

The dispatch/singleflight key does not depend on req.Debug, so a tracing-enabled check and an otherwise-identical non-tracing check collapse into one flight. The shared response is cloned back to both callers, which caused two bugs from the same root cause: - A tracing leader leaked its debug trace into a non-tracing follower. The trace contains combined nodes (Check.Request == nil, synthesized by combineResponseMetadata), which panicked the CheckBulkPermissions resource-ID matching loop. (authzed#3159) - A non-tracing leader denied a tracing follower its trace entirely, making with_tracing nondeterministic under concurrent load. Dispatch debug-enabled checks directly instead of sharing a flight. The dispatch cache key is unchanged, so cache sharing across debug levels is preserved. The bulk loop keeps a nil-Request guard as defense-in-depth, since combineResponseMetadata legitimately produces such nodes.

miparnisari

Lgtm, just a couple nits

miparnisari · 2026-07-03T21:32:15Z

 - Server handles: `GRPCDialContext` as a handle on the server used deprecated gRPC methods. We modernized it and renamed it to `NewClient` (https://github.com/authzed/spicedb/pull/3147)

 ### Fixed
+- Fixed a nil pointer dereference panic in `CheckBulkPermissions` that could occur under concurrent load when a tracing-enabled check shared a singleflight dispatch with a non-tracing bulk check. Debug-enabled checks are no longer singleflighted together with non-debug checks. (https://github.com/authzed/spicedb/pull/3174)


Move to unreleased section

miparnisari · 2026-07-03T21:35:29Z

+	// latter has panicked CheckBulkPermissions, see #3159). Dispatch directly
+	// so the response carries exactly the debug info this caller asked for.
+	if req.Debug != v1.DispatchCheckRequest_NO_DEBUG {
+		singleFlightCount.WithLabelValues("DispatchCheck", "debug").Inc()


Why are we incrementing the sf counter if we didn't use sf?

miparnisari · 2026-07-03T21:40:26Z

+}
+
+// TestCheckBulkPermissionsWithNilDebugRequest ensures that a debug information entry
+// with a nil Check.Request does not panic CheckBulkPermissions when tracing was not


Is this supposed to test the "continue" line? If so, it's not getting hit according to codecov

ivanauth requested a review from a team as a code owner June 11, 2026 16:48

github-actions Bot added area/api v1 Affects the v1 API area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Jun 11, 2026

josephschorr previously approved these changes Jun 11, 2026

View reviewed changes

miparnisari requested changes Jun 11, 2026

View reviewed changes

ivanauth dismissed josephschorr’s stale review via 52ea926 June 11, 2026 19:07

ivanauth force-pushed the fix/issue-3159-bulkcheck-nil-debug branch from 52ea926 to 1759b8f Compare June 11, 2026 19:31

github-actions Bot added the area/dispatch Affects dispatching of requests label Jun 11, 2026

ivanauth force-pushed the fix/issue-3159-bulkcheck-nil-debug branch from 1759b8f to 541b949 Compare June 11, 2026 20:26

ivanauth added 3 commits June 23, 2026 13:30

Address review: tighten changelog, drop unreachable nil guard

216f90d

ivanauth force-pushed the fix/issue-3159-bulkcheck-nil-debug branch from c4cc995 to 216f90d Compare June 23, 2026 17:31

ci: re-trigger flaky CI checks

8b1db1d

miparnisari requested changes Jul 3, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: guard nil debug request in bulk check#3174

fix: guard nil debug request in bulk check#3174
ivanauth wants to merge 4 commits into
authzed:mainfrom
ivanauth:fix/issue-3159-bulkcheck-nil-debug

ivanauth commented Jun 11, 2026

Uh oh!

codecov Bot commented Jun 11, 2026 •

edited

Loading

Uh oh!

josephschorr left a comment

Uh oh!

miparnisari left a comment •

edited

Loading

Uh oh!

ivanauth commented Jun 11, 2026

Uh oh!

miparnisari left a comment

Uh oh!

miparnisari Jul 3, 2026

Uh oh!

miparnisari Jul 3, 2026 •

edited

Loading

Uh oh!

miparnisari Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ivanauth commented Jun 11, 2026

Uh oh!

codecov Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

josephschorr left a comment

Choose a reason for hiding this comment

Uh oh!

miparnisari left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivanauth commented Jun 11, 2026

Uh oh!

miparnisari left a comment

Choose a reason for hiding this comment

Uh oh!

miparnisari Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

miparnisari Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miparnisari Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jun 11, 2026 •

edited

Loading

miparnisari left a comment •

edited

Loading

miparnisari Jul 3, 2026 •

edited

Loading