fix(repository): validate request remotes before cache access (BUG-001)

config/config.go

@@ -99,6 +99,9 @@ func ParseBytes(yamlBytes []byte) (*Config, error) {
 		if remote == "" {
 			return nil, fmt.Errorf("repository[%d].remote must not be empty", i)
 		}
+		if config.Repository[i].RepositoryID == "" {
+			return nil, fmt.Errorf("repository[%d].repository_id must not be empty", i)
+		}
 		if _, exists := config.repositoryByRemote[remote]; exists {
 			return nil, fmt.Errorf("duplicate repository remote %q", remote)
 		}

config/config_test.go

@@ -28,6 +28,7 @@ func minimal() string {
 	return `
 repository:
   - remote: "https://example.com/repo.git"
+    repository_id: "test-repository"
 service:
   max_worker_pool_size: 2
   workspaces_root_path: "/tmp/tango-repo-manager"
@@ -49,6 +50,7 @@ func TestParseBytes_ExplicitValues(t *testing.T) {
 	yamlStr := `
 repository:
   - remote: "https://example.com/repo.git"
+    repository_id: "test-repository"
     query_timeout_seconds: 60
     bzlmod_enabled: false
     full_hash_repos: ["//"]
@@ -88,6 +90,7 @@ storage:
   type: "memory"
 repository:
   - remote: "https://example.com/r.git"
+    repository_id: "test-repository"
 service:
   max_worker_pool_size: 1
   workspaces_root_path: "/tmp/tango-repo-manager"
@@ -98,6 +101,7 @@ service:
 			yaml: `
 repository:
   - remote: "https://example.com/r.git"
+    repository_id: "test-repository"
 service:
   max_worker_pool_size: 1
   workspaces_root_path: "/tmp/tango-repo-manager"
@@ -112,6 +116,7 @@ storage:
     root_path: "/tmp/store"
 repository:
   - remote: "https://example.com/r.git"
+    repository_id: "test-repository"
 service:
   max_worker_pool_size: 1
   workspaces_root_path: "/tmp/tango-repo-manager"
@@ -125,6 +130,7 @@ storage:
   type: "disk"
 repository:
   - remote: "https://example.com/r.git"
+    repository_id: "test-repository"
 service:
   max_worker_pool_size: 1
   workspaces_root_path: "/tmp/tango-repo-manager"
@@ -140,6 +146,7 @@ storage:
     root_path: ""
 repository:
   - remote: "https://example.com/r.git"
+    repository_id: "test-repository"
 service:
   max_worker_pool_size: 1
   workspaces_root_path: "/tmp/tango-repo-manager"
@@ -153,6 +160,7 @@ storage:
   type: "s3"
 repository:
   - remote: "https://example.com/r.git"
+    repository_id: "test-repository"
 service:
   max_worker_pool_size: 1
   workspaces_root_path: "/tmp/tango-repo-manager"
@@ -175,6 +183,7 @@ func TestParseBytes_UnknownFieldsRejected(t *testing.T) {
 	yamlStr := `
 repository:
   - remote: "https://example.com/repo.git"
+    repository_id: "test-repository"
 service:
   max_worker_pool_size: 1
   workspaces_root_path: "/tmp/tango-repo-manager"
@@ -188,6 +197,7 @@ func TestParseBytes_WorkerPoolSizeRequired(t *testing.T) {
 	yamlStr := `
 repository:
   - remote: "https://example.com/repo.git"
+    repository_id: "test-repository"
 service:
   max_worker_pool_size: 0
   workspaces_root_path: "/tmp/tango-repo-manager"
@@ -200,6 +210,19 @@ func TestParseBytes_EmptyRemoteRejected(t *testing.T) {
 	yamlStr := `
 repository:
   - remote: ""
+    repository_id: "test-repository"
+service:
+  max_worker_pool_size: 1
+  workspaces_root_path: "/tmp/tango-repo-manager"
+`
+	_, err := ParseBytes([]byte(yamlStr))
+	require.Error(t, err)
+}
+
+func TestParseBytes_RepositoryIDRequired(t *testing.T) {
+	yamlStr := `
+repository:
+  - remote: "https://example.com/repo.git"
 service:
   max_worker_pool_size: 1
   workspaces_root_path: "/tmp/tango-repo-manager"
@@ -212,7 +235,9 @@ func TestParseBytes_DuplicateRemoteRejected(t *testing.T) {
 	yamlStr := `
 repository:
   - remote: "https://example.com/repo.git"
+    repository_id: "test-repository"
   - remote: "https://example.com/repo.git"
+    repository_id: "test-repository"
 service:
   max_worker_pool_size: 1
   workspaces_root_path: "/tmp/tango-repo-manager"
@@ -225,6 +250,7 @@ func TestParseBytes_WorkspacesRootPathRequired(t *testing.T) {
 	yamlStr := `
 repository:
   - remote: "https://example.com/repo.git"
+    repository_id: "test-repository"
 service:
   max_worker_pool_size: 1
 `
@@ -254,6 +280,7 @@ func TestGetRepositoryConfig(t *testing.T) {
 	repo, ok := cfg.GetRepositoryConfig("https://example.com/repo.git")
 	assert.True(t, ok)
 	assert.Equal(t, "https://example.com/repo.git", repo.Remote)
+	assert.Equal(t, "test-repository", repo.RepositoryID)
 
 	_, ok = cfg.GetRepositoryConfig("https://missing.com/repo.git")
 	assert.False(t, ok)

config/repository_config.go

@@ -29,6 +29,10 @@ type RepositoryConfig struct {
 	// unique across all entries and match exactly what clients send in
 	// BuildDescription.remote.
 	Remote string `yaml:"remote"`
+	// RepositoryID is the required operator-provided name used for metrics,
+	// repository workspaces, and cache keys. It must be safe for all three and
+	// uniquely identify this repository across Tango installations.
+	RepositoryID string `yaml:"repository_id"`
 	// TODO: FullHashRepos, ExcludedFiles, and StreamBazelLogs are not
 	// documented in config/README.md. Delete them if they turn out to be
 	// unneeded, otherwise document them there.

controller/BUILD.bazel

@@ -26,7 +26,6 @@ go_library(
         "//internal/targetdiff",
         "//internal/tgb",
         "//internal/tgbdiff",
-        "//internal/url",
         "//observability/metrics",
         "//orchestrator",
         "//tangopb",

controller/controller.go

@@ -17,9 +17,11 @@ package controller
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	"github.com/uber-go/tally"
 	"github.com/uber/tango/config"
+	tangoerrors "github.com/uber/tango/core/errors"
 	"github.com/uber/tango/core/storage"
 	"github.com/uber/tango/observability/metrics"
 	"github.com/uber/tango/orchestrator"
@@ -28,15 +30,19 @@ import (
 	"go.uber.org/zap"
 )
 
+const unknownRepositoryMetricLabel = "unknown"
+
 // Params are the parameters for the controller.
 type Params struct {
 	fx.In
 	Logger          *zap.Logger
 	Storage         storage.Storage
 	Orchestrator    orchestrator.Orchestrator
-	Scope           tally.Scope                     `optional:"true"`
-	MaxMessageBytes int                             `optional:"true"`
-	RepoConfig      config.RepositoryConfigProvider `optional:"true"`
+	Scope           tally.Scope `optional:"true"`
+	MaxMessageBytes int         `optional:"true"`
+	// RepoConfig is the authoritative repository allowlist. RPC remotes must
+	// match it exactly before the controller performs cache I/O.
+	RepoConfig config.RepositoryConfigProvider
 	// GraphFormat mirrors ServiceConfig.GraphFormat; empty defaults to gob.
 	// It must match the orchestrator's configured format — both are wired
 	// from the same ServiceConfig.
@@ -47,6 +53,23 @@ type Params struct {
 	ShadowCompare bool `optional:"true"`
 }
 
+// resolveRequestRepository returns the configured repository and metric label
+// for a validated request. Invalid requests retain the common unknown label and
+// their existing error; valid requests must exactly match the configured
+// repository allowlist before controller cache I/O.
+func (c *controller) resolveRequestRepository(remote string, requestErr error) (config.RepositoryConfig, string, error) {
+	if requestErr != nil {
+		return config.RepositoryConfig{}, unknownRepositoryMetricLabel, requestErr
+	}
+	repo, ok := c.repoConfig.GetRepositoryConfig(remote)
+	if !ok {
+		return config.RepositoryConfig{}, unknownRepositoryMetricLabel, tangoerrors.NewUser(
+			fmt.Errorf("repository remote %q is not configured", remote),
+		)
+	}
+	return repo, repo.RepositoryID, nil
+}
+
 type controller struct {
 	logger          *zap.Logger
 	storage         storage.Storage

controller/controller_test.go

@@ -19,12 +19,38 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/uber/tango/config"
 	tangoerrors "github.com/uber/tango/core/errors"
 	orchestratormock "github.com/uber/tango/orchestrator/orchestratormock"
 	"go.uber.org/mock/gomock"
 	"go.uber.org/zap"
 )
 
+type rejectAllRepositoryConfigProvider struct{}
+
+func (rejectAllRepositoryConfigProvider) GetRepositoryConfig(string) (config.RepositoryConfig, bool) {
+	return config.RepositoryConfig{}, false
+}
+
+func TestResolveRequestRepository(t *testing.T) {
+	c := &controller{repoConfig: rejectAllRepositoryConfigProvider{}}
+
+	t.Run("request error uses unknown repository", func(t *testing.T) {
+		repo, label, err := c.resolveRequestRepository("ignored", assert.AnError)
+		assert.Empty(t, repo)
+		assert.Equal(t, unknownRepositoryMetricLabel, label)
+		assert.ErrorIs(t, err, assert.AnError)
+	})
+
+	t.Run("plain remote", func(t *testing.T) {
+		_, label, err := c.resolveRequestRepository("git@github.com:other/repo.git", nil)
+		require.Error(t, err)
+		assert.Equal(t, unknownRepositoryMetricLabel, label)
+		assert.Equal(t, tangoerrors.ErrorUser, tangoerrors.GetErrorCode(err))
+	})
+}
+
 // TestNewController_StoresAppContext verifies the caller-supplied context is
 // retained and is the one observed by background goroutines.
 func TestNewController_StoresAppContext(t *testing.T) {
@@ -33,6 +59,7 @@ func TestNewController_StoresAppContext(t *testing.T) {
 	defer cancel()
 
 	c := NewController(appCtx, Params{
+		RepoConfig:   allowAnyRepositoryConfigProvider{},
 		Logger:       zap.NewNop(),
 		Orchestrator: orchestratormock.NewMockOrchestrator(ctrl),
 	}).(*controller)