## Summary `GET /search/opensearch.xml` can raise `NoMethodError: undefined method 'id' for nil`. The action itself does no search. The error comes from Rails post-response instrumentation, which pulls in `search_service` and dereferences `Current.organization` unconditionally. `SearchController` already excludes `:opensearch` from every other path that needs `search_service`. It does not exclude it from `append_info_to_payload`. ## Mechanism Verified against `master`. 1. `SearchHelper#search_service` builds the service with `params.merge(organization_id: Current.organization.id)`. There is no guard on `Current.organization`. 2. `SearchController#abuse_payload_metadata` calls `search_service.abuse_detected?`. 3. `SearchController#append_info_to_payload` merges `abuse_payload_metadata` into the payload. Rails runs `append_info_to_payload` for every action on the controller, including `:opensearch`. 4. `:opensearch` is the action least likely to have an organization resolved, because it is deliberately anonymous and session-free. The controller shows the intent to keep `:opensearch` away from `search_service`: | guard | effect on `:opensearch` | |---|---| | `skip_before_action :authenticate_user!, unless: :authenticate?` | excluded, `authenticate?` returns `false` when `action_name == 'opensearch'` | | `before_action :check_scope_global_search_enabled, except: :opensearch` | excluded explicitly | | `before_action :skip_session, only: :opensearch` | session commit skipped only for this action | | `append_info_to_payload` | **not** excluded | The controller also carries this pair with a comment: ```ruby # Ensure organization is resolved before authenticate_user! because # authenticate? calls search_service which needs Current.organization. skip_before_action :set_current_organization prepend_before_action :set_current_organization ``` That fixes the before_action ordering. It does nothing when `set_current_organization` resolves to `nil`. `append_info_to_payload` already has a safety net, but only for one failure mode: ```ruby rescue ActiveRecord::QueryCanceled # rubocop:disable Database/RescueQueryCanceled -- Safetynet on timeout during instrumentation payload ``` A timeout during instrumentation is handled. A nil dereference is not. ## Impact **Production.** `/search/opensearch.xml` is public and unauthenticated. Browsers fetch it opportunistically after a page load, because the signed-out page carries `<link rel="search">`. Every such fetch that resolves no organization produces a 500 on a public endpoint and an entry in error tracking. **CI.** The same error fails unrelated feature specs. Capybara runs with `raise_server_errors == true`, so the server-side error fails whichever example is racing the `opensearch.xml` fetch. It presents as a flaky failure of `spec/features/users/logout_spec.rb:13`, "Logout/Sign out sign out redirects to sign in page", which matches `gitlab-org/quality/test-failure-issues#42009` (`failure::flaky-test`, `flakiness::4`, `test-health:pass-after-retry`). The CI symptom is misleading twice over. The spec fails on post-response instrumentation rather than on its own assertion, so it looks like a logout defect. And because the in-job retry pass recovers it, the job can exit as a successfully-retried test rather than as a failure. ## Steps to reproduce 1. Request `GET /search/opensearch.xml` in a context where `set_current_organization` resolves to `nil`. 2. The response raises `NoMethodError: undefined method 'id' for nil` from `SearchHelper#search_service`. In CI the same path is reached without any deliberate action, because the browser fetches the document during a signed-out feature spec. ## What is the expected correct behaviour `GET /search/opensearch.xml` returns the static OpenSearch description document and does not require an organization. The action renders a template and performs no search, so instrumentation should not be the thing that makes it fail. ## Possible fixes Two directions. Neither is validated against `.ai/principles`, so treat both as starting points rather than recommendations. 1. Make the instrumentation tolerate a missing organization, either by guarding `abuse_payload_metadata` or by guarding `search_service` on `Current.organization`. 2. Exclude `:opensearch` from the abuse metadata, which matches how the other three guards already treat the action. Option 1 also protects any future action that reaches `append_info_to_payload` without an organization. Option 2 is narrower and is consistent with the existing pattern in this controller. ## Routing Found while diagnosing an unrelated pipeline failure on gitlab-org/gitlab!255632, which is a Sec change with no connection to search. Filing it here because the code belongs to global search rather than to the group that found it. @ryaanwells could you route this to the right owners, please.