enforce project scoping in the gerrit client

Every change query gets the allowlist clause injected around whatever
the caller composed, so agent-supplied project terms can only narrow.
Direct operations check the target's project first: project~number
identifiers locally, bare identifiers via one resolving fetch; the
refusal happens before the content request leaves the process.
GetChange checks its own response instead of double-fetching.

Refs: #11

Author: xobotyi

internal/gerritclient/changes.go

@@ -2,6 +2,8 @@ package gerritclient
 
 import (
 	"context"
+	"slices"
+	"strings"
 
 	"dev.gaijin.team/go/golib/e"
 	"dev.gaijin.team/go/golib/fields"
@@ -15,6 +17,9 @@ var (
 	ErrListFiles    = e.New("list change files")
 	ErrGetDiff      = e.New("get file diff")
 	ErrListComments = e.New("list change comments")
+
+	// ErrProjectScope reports an operation refused by the project allowlist.
+	ErrProjectScope = e.New("change is outside the configured project scope")
 )
 
 // CurrentRevision addresses the latest patch set of a change.
@@ -47,9 +52,76 @@ func (c *Client) GetChange(ctx context.Context, id string) (*gerrit.ChangeInfo,
 		return nil, ErrGetChange.Wrap(errEmptyResponse, fields.F("change", id))
 	}
 
+	if !c.projectAllowed(info.Project) {
+		return nil, c.scopeError(id)
+	}
+
 	return info, nil
 }
 
+// projectAllowed reports whether a project passes the allowlist; an empty
+// allowlist admits everything.
+func (c *Client) projectAllowed(project string) bool {
+	return len(c.projects) == 0 || slices.Contains(c.projects, project)
+}
+
+func (c *Client) scopeError(changeID string) error {
+	return ErrProjectScope.WithFields(
+		fields.F("change", changeID),
+		fields.F("projects", strings.Join(c.projects, ",")),
+	)
+}
+
+// scopedQuery forces the project allowlist into a change query regardless of
+// what the caller composed; agent-supplied clauses can only narrow further.
+func (c *Client) scopedQuery(query string) string {
+	if len(c.projects) == 0 {
+		return query
+	}
+
+	clauses := make([]string, 0, len(c.projects))
+	for _, p := range c.projects {
+		clauses = append(clauses, "project:"+p)
+	}
+
+	scope := "(" + strings.Join(clauses, " OR ") + ")"
+
+	if strings.TrimSpace(query) == "" {
+		return scope
+	}
+
+	return scope + " (" + query + ")"
+}
+
+// checkProjectScope refuses operations on changes outside the allowlist
+// before any request for their content leaves the process. A project~number
+// identifier is checked directly; a bare identifier costs one resolving
+// fetch.
+func (c *Client) checkProjectScope(ctx context.Context, changeID string) error {
+	if len(c.projects) == 0 {
+		return nil
+	}
+
+	if project, _, ok := strings.Cut(changeID, "~"); ok {
+		if !c.projectAllowed(project) {
+			return c.scopeError(changeID)
+		}
+
+		return nil
+	}
+
+	info, resp, err := c.gerrit.Changes.GetChange(ctx, changeID, nil)
+	if err != nil {
+		return ErrGetChange.Wrap(apiError(resp, err), fields.F("change", changeID))
+	}
+
+	if info == nil || !c.projectAllowed(info.Project) {
+		return c.scopeError(changeID)
+	}
+
+	return nil
+}
+
 // QueryResult is one page of a change query.
 type QueryResult struct {
 	Changes []gerrit.ChangeInfo
@@ -62,7 +134,7 @@ type QueryResult struct {
 func (c *Client) QueryChanges(ctx context.Context, query string, limit, start int) (*QueryResult, error) {
 	opt := &gerrit.QueryChangeOptions{}
 
-	opt.Query = []string{query}
+	opt.Query = []string{c.scopedQuery(query)}
 	opt.Limit = limit
 	opt.Start = start
 	opt.AdditionalFields = []string{"DETAILED_ACCOUNTS"}
@@ -89,6 +161,10 @@ func (c *Client) ListFiles(ctx context.Context, changeID, revision string) (map[
 		revision = CurrentRevision
 	}
 
+	if err := c.checkProjectScope(ctx, changeID); err != nil {
+		return nil, ErrListFiles.Wrap(err)
+	}
+
 	files, resp, err := c.gerrit.Changes.ListFiles(ctx, changeID, revision, nil)
 	if err != nil {
 		return nil, ErrListFiles.Wrap(apiError(resp, err),
@@ -105,6 +181,10 @@ func (c *Client) GetDiff(ctx context.Context, changeID, revision, path string) (
 		revision = CurrentRevision
 	}
 
+	if err := c.checkProjectScope(ctx, changeID); err != nil {
+		return nil, ErrGetDiff.Wrap(err)
+	}
+
 	diff, resp, err := c.gerrit.Changes.GetDiff(ctx, changeID, revision, path, nil)
 	if err != nil {
 		return nil, ErrGetDiff.Wrap(apiError(resp, err),
@@ -121,6 +201,10 @@ func (c *Client) GetDiff(ctx context.Context, changeID, revision, path string) (
 // ListChangeComments lists all published comments of a change across
 // revisions, grouped by file path.
 func (c *Client) ListChangeComments(ctx context.Context, changeID string) (map[string][]gerrit.CommentInfo, error) {
+	if err := c.checkProjectScope(ctx, changeID); err != nil {
+		return nil, ErrListComments.Wrap(err)
+	}
+
 	comments, resp, err := c.gerrit.Changes.ListChangeComments(ctx, changeID)
 	if err != nil {
 		return nil, ErrListComments.Wrap(apiError(resp, err), fields.F("change", changeID))

internal/gerritclient/gerritclient.go

@@ -36,6 +36,9 @@ var (
 type Client struct {
 	gerrit *gerrit.Client
 	self   gerrit.AccountInfo
+	// projects, when non-empty, confines every operation to the listed
+	// Gerrit projects (see docs/glossary.md: Project scoping).
+	projects []string
 }
 
 // New builds an authenticated client and validates the credentials against
@@ -58,7 +61,7 @@ func New(ctx context.Context, cfg *config.Config) (*Client, error) {
 		return nil, ErrCredentials.Wrap(errEmptyResponse)
 	}
 
-	return &Client{gerrit: g, self: *self}, nil
+	return &Client{gerrit: g, self: *self, projects: cfg.Projects}, nil
 }
 
 // Self reports the authenticated account as validated at startup.

internal/gerritclient/gerritclient_test.go

@@ -15,10 +15,13 @@ import (
 
 func testConfig(url string) *config.Config {
 	return &config.Config{
-		GerritURL: url,
-		Username:  "bot",
-		Token:     "s3cret",
-		Groups:    []config.Group{config.GroupRead},
+		GerritURL:    url,
+		Username:     "bot",
+		Token:        "s3cret",
+		Groups:       []config.Group{config.GroupRead},
+		IncludeTools: nil,
+		ExcludeTools: nil,
+		Projects:     nil,
 	}
 }
 

internal/gerritclient/scoping_test.go

@@ -0,0 +1,179 @@
+package gerritclient_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"dev.gaijin.team/go/go-gerrit-mcp/internal/gerritclient"
+)
+
+const scopedSelfJSON = ")]}'\n" + `{"_account_id":42,"name":"Review Bot","username":"bot"}`
+
+// scopedClient builds a client with a project allowlist against a recording
+// fake Gerrit. requests collects every API path hit besides the self check.
+func scopedClient(t *testing.T, handler http.HandlerFunc) (*gerritclient.Client, *[]string) {
+	t.Helper()
+
+	var (
+		mu       sync.Mutex
+		requests []string
+	)
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/a/accounts/self" {
+			_, _ = w.Write([]byte(scopedSelfJSON))
+
+			return
+		}
+
+		mu.Lock()
+
+		requests = append(requests, r.URL.String())
+
+		mu.Unlock()
+
+		handler(w, r)
+	}))
+	t.Cleanup(srv.Close)
+
+	cfg := testConfig(srv.URL)
+
+	cfg.Projects = []string{"core", "infra"}
+
+	client, err := gerritclient.New(t.Context(), cfg)
+	require.NoError(t, err)
+
+	return client, &requests
+}
+
+func Test_ProjectScoping(t *testing.T) {
+	t.Parallel()
+
+	t.Run("query gets the allowlist clause injected", func(t *testing.T) {
+		t.Parallel()
+
+		var gotQuery string
+
+		client, _ := scopedClient(t, func(w http.ResponseWriter, r *http.Request) {
+			gotQuery = r.URL.Query().Get("q")
+
+			_, _ = w.Write([]byte(")]}'\n[]"))
+		})
+
+		_, err := client.QueryChanges(t.Context(), "status:open project:secret", 10, 0)
+		require.NoError(t, err)
+
+		assert.Equal(t, "(project:core OR project:infra) (status:open project:secret)", gotQuery)
+	})
+
+	t.Run("empty query becomes the allowlist clause", func(t *testing.T) {
+		t.Parallel()
+
+		var gotQuery string
+
+		client, _ := scopedClient(t, func(w http.ResponseWriter, r *http.Request) {
+			gotQuery = r.URL.Query().Get("q")
+
+			_, _ = w.Write([]byte(")]}'\n[]"))
+		})
+
+		_, err := client.QueryChanges(t.Context(), "", 10, 0)
+		require.NoError(t, err)
+
+		assert.Equal(t, "(project:core OR project:infra)", gotQuery)
+	})
+
+	t.Run("get_change refuses out-of-scope project", func(t *testing.T) {
+		t.Parallel()
+
+		client, _ := scopedClient(t, func(w http.ResponseWriter, _ *http.Request) {
+			_, _ = w.Write([]byte(")]}'\n{\"_number\":9,\"project\":\"secret\",\"branch\":\"main\"}"))
+		})
+
+		_, err := client.GetChange(t.Context(), "9")
+
+		require.Error(t, err)
+		require.ErrorIs(t, err, gerritclient.ErrProjectScope)
+	})
+
+	t.Run("scoped id refused without any request", func(t *testing.T) {
+		t.Parallel()
+
+		client, requests := scopedClient(t, func(w http.ResponseWriter, _ *http.Request) {
+			_, _ = w.Write([]byte(")]}'\n{}"))
+		})
+
+		_, err := client.ListFiles(t.Context(), "secret~123", "")
+
+		require.Error(t, err)
+		require.ErrorIs(t, err, gerritclient.ErrProjectScope)
+		assert.Empty(t, *requests, "no API request may leave the process")
+	})
+
+	t.Run("bare id resolves project then refuses", func(t *testing.T) {
+		t.Parallel()
+
+		client, requests := scopedClient(t, func(w http.ResponseWriter, r *http.Request) {
+			if strings.HasSuffix(r.URL.Path, "/comments") {
+				t.Error("comments endpoint must not be reached")
+			}
+
+			_, _ = w.Write([]byte(")]}'\n{\"_number\":9,\"project\":\"secret\",\"branch\":\"main\"}"))
+		})
+
+		_, err := client.ListChangeComments(t.Context(), "9")
+
+		require.Error(t, err)
+		require.ErrorIs(t, err, gerritclient.ErrProjectScope)
+		require.Len(t, *requests, 1, "exactly the resolving fetch")
+	})
+
+	t.Run("in-scope scoped id proceeds directly", func(t *testing.T) {
+		t.Parallel()
+
+		client, requests := scopedClient(t, func(w http.ResponseWriter, _ *http.Request) {
+			_, _ = w.Write([]byte(")]}'\n{}"))
+		})
+
+		files, err := client.ListFiles(t.Context(), "core~123", "")
+		require.NoError(t, err)
+
+		assert.NotNil(t, files)
+		require.Len(t, *requests, 1)
+		assert.Contains(t, (*requests)[0], "/files")
+	})
+
+	t.Run("unscoped client passes everything through", func(t *testing.T) {
+		t.Parallel()
+
+		var gotQuery string
+
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if r.URL.Path == "/a/accounts/self" {
+				_, _ = w.Write([]byte(scopedSelfJSON))
+
+				return
+			}
+
+			gotQuery = r.URL.Query().Get("q")
+
+			_, _ = w.Write([]byte(")]}'\n[]"))
+		}))
+		t.Cleanup(srv.Close)
+
+		client, err := gerritclient.New(t.Context(), testConfig(srv.URL))
+		require.NoError(t, err)
+
+		_, err = client.QueryChanges(t.Context(), "status:open", 10, 0)
+		require.NoError(t, err)
+		require.NotErrorIs(t, err, gerritclient.ErrProjectScope)
+
+		assert.Equal(t, "status:open", gotQuery)
+	})
+}