Skip to content

search: filter sub-repo perms in run.Aggregator

Warren Gifford requested to merge 503/search-results-streaming into main

Created by: bobheadxi

Closes #27139 (closed) - alternative approach to #27012 (see update in RFC 503)

Filters sub-repo perms in the results Aggregator, used by both streaming and graphql search. The entire check is omitted if the enableSubRepoPerms site config is not enabled.

Also see https://github.com/sourcegraph/sourcegraph/pull/27189#issuecomment-967300062 for closing remarks

Original summary

From my understanding of the search API code:

  • Streaming search is handled by streamHandler, which uses graphqlbackend.NewSearchImplementer => searchResolver. The search implementer providers a resovler via Results() that is used to get results for streaming.
  • It appears SearchResultsResolver from the searchResolver is also used in the search GraphQL API
  • From there, results come from resultsBatch or resultsStreaming, both of which use resultsRecursive, which seems to handle doing things to the query until we get a newResult from evaluate()
  • Within evaluate(), the code branches off into a bunch of different things again

From this understanding:

  • it seems that placing the sub-repo filtering in resultsRecursive is the right thing to do, so I've gone ahead and drafted what this might look like here. Another idea could be placing it in searchResolver.Results().
  • the actor will likely be available here because this seems to all live in the frontend.

We don't have docs yet, but some details about SubRepoPermsClient:

Merge request reports

Loading