feat(authz): BatchCheck client part 1/2 (S09 Enforcement plan: 3/20)

David Fernandezrequested to merge
10io/s09-enforcement-step-3-part1of2 into main

Stacked MRs

Step 3 ships as 2 stacked MRs to keep each part within the review size limit. Part 1 targets main; part 2 targets part 1. Review and merge bottom-up.

What this MR does

Part 1 of 2 of the S09 Enforcement plan — Step 3: GLAZ BatchCheck client method, per S09 (authorization).

Adds the client-side BatchCheck core contract in internal/authz/glaz:

  • The BatchChecker seam and its domain types (BatchCheckRequest, BatchCheckItem, BatchCheckResult) — no gRPC or proto type crosses the request/response boundary — plus the exported GLAZ contract bounds (MaxBatchCheck*).
  • Client.BatchCheck: dedupe the shared relationship list, validate every cap and identifier client-side before any RPC (wrapping ErrInvalidRequest), issue one RPC under the batch deadline.
  • Response mapping that refuses a lying or partial reply: verdicts are matched by (item_index, action); an unrequested action, a duplicate pair, an out-of-range index, an object echo that is absent or names a different object, or an incomplete response fails the whole call. Each result's object comes from the request, so a missing verdict can never read as a denial, nor a partial list as complete, nor a misattributed verdict as this item's.
  • ErrUnimplemented (sidecar does not serve the RPC) and a ResourceExhausted → ErrUnavailable mapping.
  • A separate deadline for the batch path: glaz.batch_timeout (default 10s), validated like glaz.timeout and applied only to BatchCheck. A full batch is up to 10,000 objects against up to 10,000 shared tuples, so glaz.timeout's 1s — chosen for a single decision — cannot size both, and raising it would hold every point Check longer during a sidecar stall.
  • Operational metrics: the BatchCheck rpc label on the existing RPC vectors, and glaz_batch_rejections_total, which counts the rejections the client makes itself. Neither class is visible on the RPC metrics — a pre-RPC ErrInvalidRequest never reaches the interceptor, and a response-contract refusal is recorded there under grpc_code="OK" — so a partial-batch regression would otherwise 503 listings while the dashboards read healthy. Both label sets are pinned in the internal/metrics cardinality catalog.
  • Per-item ABAC context is produced by the existing marshalAttributes helper ({} on empty, never concatenation), and BatchCheckItem.AncestorIDs carries the trust-boundary contract in its doc comment: GLAZ trusts ancestor ids unverified, so values must come from the datastore hierarchy, never request input.

The stacked part 2 hardens the client: gRPC receive-cap and retry-replay-buffer sizing, at-cap boundary tests, the response-shape rejection suite, and error-code table parity with Check.

Spec coverage

Obligation Tests
One RPC carrying one deduped relationship list and per-item ancestor ids TestClient_BatchCheck_RequestMapping, TestClient_BatchCheck_Dedup
Item and relationship caps (and per-item bounds, empty identifiers, duplicate actions) rejected client-side, no RPC issued TestClient_BatchCheck_ValidationRejectsBeforeRPC
Per-item context via the marshalAttributes-shaped helper, empty map encodes {} TestClient_BatchCheck_RequestMapping, TestClient_BatchCheck_NilAttributesAndAncestorIDs
UNIMPLEMENTED mapped distinctly so an older sidecar contract is diagnosable TestClient_BatchCheck_UnimplementedMapping
Client timeout maps to ErrUnavailable TestClient_BatchCheck_ClientTimeoutMapsToErrUnavailable
The batch path is bounded by glaz.batch_timeout, the point check by glaz.timeout TestClient_BatchCheck_UsesItsOwnDeadline, TestNewClient_RejectsNonPositiveTimeout, TestGLAZFromProto, TestLoad_GLAZ_HappyPath, TestLoad_GLAZ_RejectsNonPositiveBatchTimeout
Duplicate-action rejection is per item, not per request TestClient_BatchCheck_SameActionAcrossDifferentItems
Every mapped verdict field (ItemIndex, Object, Action, Allowed, Reason) carried through, in server order TestClient_BatchCheck_ResponseMapping
The object echo is required: one naming another object is refused, and so is an absent one TestClient_BatchCheck_ObjectEcho
Every client-side rejection is counted under its own reason TestBatchRejections_RecordTheirReason, TestBatchRejections_NoDisallowedLabels, TestRegisterMetrics_PassesCardinalityAudit
Results matched back by item_index, not object; the remaining response-contract violations refused part 2: TestClient_BatchCheck_ItemIndexMatching, TestClient_BatchCheck_OutOfRangeItemIndex, TestClient_BatchCheck_ResponseValidation, TestClient_BatchCheck_ObjectComesFromRequest
Ancestor-ids trust boundary documented on the domain type BatchCheckItem.AncestorIDs doc comment (code, not a test)

Size

~1,535 reviewable LoC, over the 500 guideline. Production is ~438 LoC, almost all in one package: internal/authz/glaz ~400, plus internal/config/glaz.go, internal/metrics/cardinality.go, and cmd/artifact-registry/wire_glaz.go for the config field and its wiring. Tests are ~1,083 LoC across the same packages, and config.example.yaml, the configuration reference, the observability catalog, and the config proto add 14 between them. The regenerated config stub (18 LoC) is excluded as generated.

Splitting further would separate functions from their only tests. The one seam that could split — the config field and its wiring — is deliberately in this MR: glaz.timeout is published here as bounding both RPCs, and correcting a published config surface later is a user-visible change.

e2e scenarios

Client-only seam with no consumer wired in this MR (the listing filter consumes it in a later step); no request-path behavior changes, so no docs/testing/ scenario is added or affected. glaz.batch_timeout is a new configuration field with a default, so an existing deployment's behavior is unchanged unless it sets one.

Related to #652 (closed)

Stacked MRs (review/merge bottom-up)

Edited by David Fernandez

Merge request reports