Do not memoize a nil user in ApplicationContext

What this does

The current user is pushed into the application context as a lazy lambda that resolves to nil until authentication completes. The lazy reader memoized the first resolution, so any read before authentication (for example a log line emitted during pre-authentication token resolution) cached nil for the rest of the request. That dropped user attribution from meta.user, meta.user_id, client_id, and the Gitaly RPC metadata. Gitaly then treated the RPC as anonymous, and under its per-repository concurrency limit those requests were shed as ResourceExhausted, surfacing to callers as HTTP 503.

This change makes the user attribute re-resolve instead of caching a nil.

Changes

  • gems/gitlab-utils/lib/gitlab/utils/lazy_attributes.rb: lazy_attr_reader gains a cache_nil: option, default true, so every existing reader is unchanged. With cache_nil: false, a nil result is re-resolved on each read, and only a non-nil value is cached.
  • lib/gitlab/application_context.rb: the user reader opts into cache_nil: false. user_id and username derive from it, so they are covered too.
  • Specs added in both files.

How to reproduce and verify

The confirmed trigger is PersonalAccessTokens::LastUsedService, which runs during pre-authentication token resolution. When it cannot obtain its update lease, it emits a log line, and that log line resolves the context and caches a nil user before authentication has set the user. Full analysis is in the linked issue.

To reproduce on an instance, in a Rails console, make the token due for an update and hold its lease:

id = TOKEN_ID   # PersonalAccessToken id
PersonalAccessToken.find(id).update_columns(last_used_at: 11.minutes.ago)
Gitlab::ExclusiveLease.new("pat:last_used_update_lock:#{id}", timeout: 60).try_obtain

Then, within 60 seconds, make an authenticated request:

curl -H "PRIVATE-TOKEN: <token>" \
  "https://<host>/api/v4/projects/<pid>/repository/files/README%2Emd/raw?ref=master"

Find that request's log line by its correlation_id. Before this change, json.username is set but json.meta.user is missing. After this change, json.meta.user is populated.

Testing

  • New spec in gems/gitlab-utils/spec/gitlab/utils/lazy_attributes_spec.rb: with cache_nil: false, the reader re-resolves while nil, then caches the first non-nil value.
  • New spec in spec/lib/gitlab/application_context_spec.rb: the user attribute re-resolves after an earlier read returned nil.
  • spec/lib/gitlab/application_context_spec.rb passes locally (41 examples, 0 failures).
  • Verified by console reproduction that meta.user is populated after authentication with this change, where it stayed nil before.

Notes

No feature flag. This is a targeted correctness fix scoped to the user reader, and the default behavior of every other lazy attribute is unchanged.

References

Resolves #626851 (closed)

Edited by Stan Hu

Merge request reports

Loading
Loading