fix(authz): count the permission-boundary 403 in denials_total

🎯 Summary

gitlab_artifact_registry_authz_denials_total gains one denial_reason value, exists_but_forbidden, incremented at exactly one site: denialMapping's arm where GLAZ denied a non-read action but the follow-up read Check allowed, so the service answers 403.

That refusal — a principal attempting an operation their role does not permit — is the commonest one in normal operation and was counted nowhere, so it was chartable only from logs. Staging measured 42 occurrences in a 24-hour window against zero on any counter.

No response status, body, header, or control flow changes. The commit adds an Inc() inside an existing branch, above an unmoved return.

🔧 What changed

  • denialReasonExistsButForbidden = "exists_but_forbidden" in internal/authz/metrics.go. The name is this repository's own term for the case, already used in internal/authz/decision.go, the S09 spec, and several test names.
  • The increment in internal/authz/decision.go, gated on booking so Evaluator.Probe still books nothing.
  • internal/metrics/cardinality.go: the denial_reason budget moves 8 to 9 and the value joins the pinned closed set.
  • The counter is no longer masked-404 only. Filter denial_reason!="exists_but_forbidden" to recover the previous reading.
  • The rule for which 403 belongs is now the one that still discriminates: the counter carries the 403 the decision layer reaches by a GLAZ verdict. RouteUnmapped's 403 and the suspended-namespace write gate's are decided without one and stay out; RouteUnmapped's arm says so in place.

⚠️ Two deviations from the issue

The issue asks to retire glaz_denied and remove anonymous as unreachable. Both claims hold only for the staging wiring they were measured against, so both values stay and instead gain a bound in the metric catalog.

  • glaz_denied is unreachable only while every built-in role grants both read_artifact and read_repository — all four role columns of ADR-021's default permission buckets table tick both. ## Resolutions in the S09 spec already rules that a custom role holding one without the other reaches it, so retiring the label would delete coverage the spec has committed to.
  • anonymous fires today on a bootstrap-token deployment, where Validate returns auth.AnonymousIdentity() on a successful static-token compare. Work item #982 (closed) names this counter as one of only two signals for diagnosing that posture.

Both bounds are written as a condition and its event, so they read correctly on either side of the condition changing.

🧪 Tests

Four assertions were added because the suite stayed green against deliberately wrong code without them, each confirmed by mutation:

  • the booking gate — removing it left the entire repository green;
  • the masked-read arm's silence on the new reason;
  • RouteUnmapped's arm booking no reason, now checked across all nine values rather than one;
  • the decision log the new arm inherits from evaluate, asserted field by field. The action field is the discriminator: a coarser check passes against the broken version, because denialMapping's own masking-read log survives the mutation.

📄 Spec amendment

### Observability in docs/specs/S09-authorization.md travels with this change: the value set gains exists_but_forbidden, and "403 and 503 are excluded" narrows to every other 403. The amendment describes what this MR does, so it ships here rather than in a preceding spec MR.

🔍 e2e scenarios

No scenario is added or affected. e2e.oci.access.reader-cannot-push in docs/testing/e2e/oci.md is exactly this denial and stays blocked on that catalog's standing rationale: role assignment is a GitLab UI and API surface this repository does not own, so a journey starting from an assigned role has no path through the rig. No scenario keys on a counter.

🔐 Security

Reviewed for the change's own surface. No response or control-flow change, so ADR-021's masking is untouched; the counter carries no tenant-identifying label and is served on the probe listener rather than the public one; the label value is a compile-time constant, so cardinality growth is exactly +1; and no log statement is added.

Related to #1141 (closed)

Merge request reports

Loading
Loading