Enforce flat broker repo names

CHANGELOG.md

@@ -4,6 +4,13 @@ All notable changes to `bgit` are documented in this file.
 
 This project follows semantic versioning.
 
+## 1.0.1
+
+Changed
+
+- Broker logical repository names are now flat. Path-shaped names are rejected
+  in the CLI, web settings, and broker to reserve URL paths for team routing.
+
 ## 1.0.0
 
 Breaking changes

broker/aws/template.yaml

@@ -165,11 +165,23 @@ Resources:
           function cleanName(value) {
             return String(value || "repo").toLowerCase().replace(/[^a-z0-9.-]+/g, "-").replace(/^-+|-+$/g, "").slice(0, 40) || "repo";
           }
+          function normalizeLogicalRepo(value) {
+            const base = String(value || "").trim().replace(/\.git$/, "");
+            if (!base) throw new Error("logical repo name is required");
+            if (base.includes("/") || base.includes("\\")) throw new Error("logical repo names must be flat");
+            if (base === "." || base === "..") throw new Error("logical repo name is invalid");
+            return `${base}.git`;
+          }
+          function validateRepo(repo) {
+            if (!repo || (!repo.logical && (!repo.bucket || !repo.prefix))) throw new Error("repo is required");
+            if (repo.logical) repo.logical = normalizeLogicalRepo(repo.logical);
+            return repo;
+          }
           function randomSuffix() {
             return crypto.randomBytes(5).toString("hex");
           }
           async function loadRepo(repo) {
-            if (!repo || (!repo.logical && (!repo.bucket || !repo.prefix))) throw new Error("repo is required");
+            repo = validateRepo(repo);
             const id = docID(repo);
             const out = await db.send(new GetItemCommand({TableName: table, Key: {id: {S: id}}}));
             let data = {repo, keys: [], audit: []};
@@ -840,8 +852,7 @@ Resources:
                 const entry = await loadRepo(body.repo);
                 const key = signedKey(event, entry);
                 if (!key || key.role !== "owner") throw Object.assign(new Error("owner SSH signature required"), {statusCode: 403});
-                const logical = String(body.logical || "").trim().replace(/^\/+|\/+$/g, "");
-                if (!logical) throw new Error("logical repo name is required");
+                const logical = normalizeLogicalRepo(body.logical);
                 const newRepo = {...(entry.data.repo || body.repo), logical};
                 const newID = docID(newRepo);
                 if (entry.id !== newID) {

broker/gcp/index.js

@@ -26,11 +26,26 @@ function cleanName(value) {
   return String(value || 'repo').toLowerCase().replace(/[^a-z0-9.-]+/g, '-').replace(/^-+|-+$/g, '').slice(0, 40) || 'repo';
 }
 
+function normalizeLogicalRepo(value) {
+  const base = String(value || '').trim().replace(/\.git$/, '');
+  if (!base) throw new Error('logical repo name is required');
+  if (base.includes('/') || base.includes('\\')) throw new Error('logical repo names must be flat');
+  if (base === '.' || base === '..') throw new Error('logical repo name is invalid');
+  return `${base}.git`;
+}
+
+function validateRepo(repo) {
+  if (!repo || (!repo.logical && (!repo.bucket || !repo.prefix))) throw new Error('repo is required');
+  if (repo.logical) repo.logical = normalizeLogicalRepo(repo.logical);
+  return repo;
+}
+
 function randomSuffix() {
   return crypto.randomBytes(5).toString('hex');
 }
 
 async function loadRepo(repo) {
+  repo = validateRepo(repo);
   const ref = repos.doc(docID(repo));
   const snap = await ref.get();
   if (!snap.exists) return {ref, data: {repo, keys: [], audit: []}};
@@ -688,7 +703,7 @@ function countApprovals(pr) {
 }
 
 async function ensureRepo(repo) {
-  if (!repo || (!repo.logical && (!repo.bucket || !repo.prefix))) throw new Error('repo is required');
+  repo = validateRepo(repo);
   const entry = await loadRepo(repo);
   const owners = await loadOwners();
   for (const owner of owners.data.keys || []) {
@@ -774,8 +789,7 @@ exports.broker = async (req, res) => {
       const entry = await ensureRepo(body.repo);
       const key = signedKey(req, entry);
       if (!key || key.role !== 'owner') throw Object.assign(new Error('owner SSH signature required'), {status: 403});
-      const logical = String(body.logical || '').trim().replace(/^\/+|\/+$/g, '');
-      if (!logical) throw new Error('logical repo name is required');
+      const logical = normalizeLogicalRepo(body.logical);
       const newRepo = {...(entry.data.repo || body.repo), logical};
       const newRef = repos.doc(docID(newRepo));
       const oldID = docID(entry.data.repo || body.repo);

broker_commands.go

@@ -114,7 +114,10 @@ func brokerAdminRepoCommand(cfg config, args []string, stdout io.Writer) error {
 		if len(args) != 2 {
 			return errors.New("usage: bgit admin repo rename NEW_LOGICAL_NAME")
 		}
-		logical := logicalRepoWithGit(args[1])
+		logical, err := normalizeLogicalRepoName(args[1])
+		if err != nil {
+			return err
+		}
 		if err := brokerPost(cfg.brokerURL, "/repo/rename", brokerRepoAdminRequest{Repo: repoForBroker(cfg), Logical: logical}, nil); err != nil {
 			return err
 		}
@@ -281,7 +284,7 @@ func brokerCloneCommand(args []string, stdin io.Reader, stdout io.Writer) error
 		}
 	}
 	if strings.TrimSpace(repoName) == "" {
-		return errors.New("usage: bgit clone <repo> [directory] [--profile PROFILE]\n       bgit clone https://broker.example.com/team/app.git [directory]\n       bgit clone --broker https://broker.example.com team/app.git [directory]")
+		return errors.New("usage: bgit clone <repo> [directory] [--profile PROFILE]\n       bgit clone https://broker.example.com/app.git [directory]\n       bgit clone --broker https://broker.example.com app.git [directory]")
 	}
 	if opts.brokerURL != "" {
 		profile, err := brokerProfileForCloneURL(opts.brokerURL)
@@ -455,8 +458,18 @@ func configForBrokerCommand(base config) (config, error) {
 	}
 	if strings.TrimSpace(cfg.logicalRepo) == "" {
 		if out, err := runGit(".", "config", "--get", "bucketgit.logicalRepo"); err == nil {
-			cfg.logicalRepo = strings.Trim(strings.TrimSpace(string(out)), "/")
+			logical, normalizeErr := normalizeLogicalRepoName(string(out))
+			if normalizeErr != nil {
+				return config{}, normalizeErr
+			}
+			cfg.logicalRepo = logical
 		}
+	} else {
+		logical, normalizeErr := normalizeLogicalRepoName(cfg.logicalRepo)
+		if normalizeErr != nil {
+			return config{}, normalizeErr
+		}
+		cfg.logicalRepo = logical
 	}
 	if cfg.origin == "" {
 		cfg.origin = originForConfig(cfg)
@@ -510,7 +523,11 @@ func brokerConfirmOwnershipTransferCommand(cfg config, args []string, stdout io.
 	if err != nil {
 		return err
 	}
-	repo := brokerRepo{Provider: "gcs", Logical: logicalRepoWithGit(repoName), Origin: "git@" + defaultSSHHost + ":" + logicalRepoWithGit(repoName)}
+	logical, err := normalizeLogicalRepoName(repoName)
+	if err != nil {
+		return err
+	}
+	repo := brokerRepo{Provider: "gcs", Logical: logical, Origin: "git@" + defaultSSHHost + ":" + logical}
 	var resp brokerOwnerTransferResponse
 	if err := brokerPost(brokerURL, "/owners/transfer/confirm", brokerOwnerTransferRequest{Repo: repo, BrokerURL: brokerURL}, &resp); err != nil {
 		return err
@@ -540,7 +557,11 @@ func brokerCancelOwnershipTransferCommand(cfg config, args []string, stdout io.W
 	if err != nil {
 		return err
 	}
-	repo := brokerRepo{Provider: "gcs", Logical: logicalRepoWithGit(repoName), Origin: "git@" + defaultSSHHost + ":" + logicalRepoWithGit(repoName)}
+	logical, err := normalizeLogicalRepoName(repoName)
+	if err != nil {
+		return err
+	}
+	repo := brokerRepo{Provider: "gcs", Logical: logical, Origin: "git@" + defaultSSHHost + ":" + logical}
 	if err := brokerPost(brokerURL, "/owners/transfer/cancel", brokerOwnerTransferRequest{Repo: repo}, nil); err != nil {
 		return err
 	}
@@ -653,7 +674,11 @@ func brokerInviteUserCommand(cfg config, args []string, stdout io.Writer) error
 	if !validBrokerRole(role) || role == "owner" {
 		return fmt.Errorf("invalid role %q", role)
 	}
-	repo := brokerRepo{Provider: "gcs", Logical: logicalRepoWithGit(repoName), Origin: "git@" + defaultSSHHost + ":" + logicalRepoWithGit(repoName)}
+	logical, err := normalizeLogicalRepoName(repoName)
+	if err != nil {
+		return err
+	}
+	repo := brokerRepo{Provider: "gcs", Logical: logical, Origin: "git@" + defaultSSHHost + ":" + logical}
 	var resp brokerOwnerTransferResponse
 	if err := brokerPost(brokerURL, "/keys/invite/create", brokerOwnerTransferRequest{Repo: repo, BrokerURL: brokerURL, User: user, Role: role}, &resp); err != nil {
 		return err
@@ -683,7 +708,11 @@ func brokerCancelInviteCommand(cfg config, args []string, stdout io.Writer) erro
 	if err != nil {
 		return err
 	}
-	repo := brokerRepo{Provider: "gcs", Logical: logicalRepoWithGit(repoName), Origin: "git@" + defaultSSHHost + ":" + logicalRepoWithGit(repoName)}
+	logical, err := normalizeLogicalRepoName(repoName)
+	if err != nil {
+		return err
+	}
+	repo := brokerRepo{Provider: "gcs", Logical: logical, Origin: "git@" + defaultSSHHost + ":" + logical}
 	if err := brokerPost(brokerURL, "/keys/invite/cancel", brokerOwnerTransferRequest{Repo: repo, User: user}, nil); err != nil {
 		return err
 	}
@@ -1359,7 +1388,11 @@ func parseBrokerCloneURL(raw string) (string, string, bool, error) {
 	if repoName == "" {
 		return "", "", true, errors.New("broker clone URL must include a logical repository path")
 	}
-	return parsed.Scheme + "://" + parsed.Host, repoName, true, nil
+	logical, err := normalizeLogicalRepoName(repoName)
+	if err != nil {
+		return "", "", true, err
+	}
+	return parsed.Scheme + "://" + parsed.Host, logical, true, nil
 }
 
 func brokerProfileForCloneURL(brokerURL string) (brokerProfile, error) {
@@ -1660,14 +1693,26 @@ func defaultInitRepoName() string {
 }
 
 func logicalRepoWithGit(name string) string {
-	name = strings.Trim(strings.TrimSpace(name), "/")
-	if name == "" {
+	logical, err := normalizeLogicalRepoName(name)
+	if err != nil {
 		return "repo.git"
 	}
-	if !strings.HasSuffix(name, ".git") {
-		name += ".git"
+	return logical
+}
+
+func normalizeLogicalRepoName(name string) (string, error) {
+	name = strings.TrimSpace(name)
+	name = strings.TrimSuffix(name, ".git")
+	if name == "" {
+		return "", errors.New("logical repo name is required")
 	}
-	return name
+	if strings.ContainsAny(name, `/\`) {
+		return "", fmt.Errorf("logical repo names must be flat; use %q instead of a path", filepath.Base(name))
+	}
+	if name == "." || name == ".." {
+		return "", errors.New("logical repo name is invalid")
+	}
+	return name + ".git", nil
 }
 
 func logicalRepoDisplayName(name string) string {
@@ -2001,9 +2046,9 @@ func initBrokerWorktree(target, repoName string, profile brokerProfile, identity
 			return err
 		}
 	}
-	repoName = strings.Trim(repoName, "/")
-	if !strings.HasSuffix(repoName, ".git") {
-		repoName += ".git"
+	repoName, err = normalizeLogicalRepoName(repoName)
+	if err != nil {
+		return err
 	}
 	remoteURL := fmt.Sprintf("git@%s:%s", defaultSSHHost, repoName)
 	sshCommand := gitSSHCommandForExecutable()

broker_commands_test.go

@@ -31,15 +31,15 @@ func TestBrokerInitWritesBrokerGitConfig(t *testing.T) {
 	}
 	target := filepath.Join(root, "app")
 	var stdout bytes.Buffer
-	err := brokerInitCommand([]string{"--noninteractive", "--repo", "team/app", target, "--profile", "gcp:work/europe-west1", "--config", configPath}, strings.NewReader(""), &stdout)
+	err := brokerInitCommand([]string{"--noninteractive", "--repo", "app", target, "--profile", "gcp:work/europe-west1", "--config", configPath}, strings.NewReader(""), &stdout)
 	if err != nil {
 		t.Fatal(err)
 	}
 	for key, want := range map[string]string{
 		"bucketgit.broker":      "https://broker.example.test",
 		"bucketgit.profile":     "gcp:work/europe-west1",
 		"bucketgit.region":      "europe-west1",
-		"bucketgit.logicalRepo": "team/app.git",
+		"bucketgit.logicalRepo": "app.git",
 		"branch.main.remote":    "origin",
 		"branch.main.merge":     "refs/heads/main",
 		"core.autocrlf":         "false",
@@ -67,7 +67,7 @@ func TestBrokerInitWritesBrokerGitConfig(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if strings.TrimSpace(string(remote)) != "git@git.bucketgit.com:team/app.git" {
+	if strings.TrimSpace(string(remote)) != "git@git.bucketgit.com:app.git" {
 		t.Fatalf("origin = %q", strings.TrimSpace(string(remote)))
 	}
 }
@@ -89,7 +89,7 @@ func TestShellQuoteForGitSSHCommand(t *testing.T) {
 
 func TestInitBrokerWorktreeOmitsIdentityWhenUnset(t *testing.T) {
 	target := filepath.Join(t.TempDir(), "app")
-	err := initBrokerWorktree(target, "team/app", brokerProfile{
+	err := initBrokerWorktree(target, "app", brokerProfile{
 		Provider:      "gcs",
 		QualifiedName: "broker:https://broker.example.test",
 		BrokerURL:     "",
@@ -106,7 +106,7 @@ func TestInitBrokerWorktreeOmitsIdentityWhenUnset(t *testing.T) {
 }
 
 func TestBrokerInitNoninteractiveRequiresProfileAndRepo(t *testing.T) {
-	err := brokerInitCommand([]string{"--noninteractive", "--repo", "team/app"}, strings.NewReader(""), ioDiscard{})
+	err := brokerInitCommand([]string{"--noninteractive", "--repo", "app"}, strings.NewReader(""), ioDiscard{})
 	if err == nil || !strings.Contains(err.Error(), "requires --profile") {
 		t.Fatalf("err = %v", err)
 	}
@@ -125,7 +125,7 @@ func TestAdminKeysListUsesLogicalBrokerRepo(t *testing.T) {
 		if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
 			t.Fatal(err)
 		}
-		if req.Repo.Logical != "team/app.git" {
+		if req.Repo.Logical != "app.git" {
 			t.Fatalf("logical repo = %q", req.Repo.Logical)
 		}
 		_, _ = w.Write([]byte(`{"keys":[{"user":"owner","role":"owner","public_key":"ssh-ed25519 AAAA owner"}]}`))
@@ -176,7 +176,7 @@ func TestTopLevelBrokerInitForwardsGlobalProfile(t *testing.T) {
 	}
 	target := filepath.Join(root, "app")
 	var stdout bytes.Buffer
-	err := run([]string{"init", "--noninteractive", "--repo", "team/app", target, "--config", configPath, "--profile", "gcp:work/europe-west1"}, strings.NewReader(""), &stdout, ioDiscard{})
+	err := run([]string{"init", "--noninteractive", "--repo", "app", target, "--config", configPath, "--profile", "gcp:work/europe-west1"}, strings.NewReader(""), &stdout, ioDiscard{})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -319,15 +319,26 @@ func TestBrokerProfileDotRegionSelectsProfile(t *testing.T) {
 }
 
 func TestParseBrokerCloneURL(t *testing.T) {
-	brokerURL, repo, ok, err := parseBrokerCloneURL("https://broker.example.test/team/app.git")
+	brokerURL, repo, ok, err := parseBrokerCloneURL("https://broker.example.test/app.git")
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !ok || brokerURL != "https://broker.example.test" || repo != "team/app.git" {
+	if !ok || brokerURL != "https://broker.example.test" || repo != "app.git" {
 		t.Fatalf("brokerURL=%q repo=%q ok=%v", brokerURL, repo, ok)
 	}
 }
 
+func TestLogicalRepoNamesMustBeFlat(t *testing.T) {
+	for _, name := range []string{"team/app", "team/app.git", `team\app`} {
+		if _, err := normalizeLogicalRepoName(name); err == nil {
+			t.Fatalf("normalizeLogicalRepoName(%q) succeeded", name)
+		}
+	}
+	if _, _, _, err := parseBrokerCloneURL("https://broker.example.test/team/app.git"); err == nil {
+		t.Fatal("parseBrokerCloneURL accepted path-shaped logical repo")
+	}
+}
+
 func TestBrokerProfileForCloneURL(t *testing.T) {
 	profile, err := brokerProfileForCloneURL("https://broker.example.test/")
 	if err != nil {
@@ -392,7 +403,7 @@ func TestBrokerInitInteractiveIdentityOnlyUpdatesRepoConfig(t *testing.T) {
 		t.Fatal(err)
 	}
 	for _, args := range [][]string{
-		{"config", "bucketgit.logicalRepo", "team/app.git"},
+		{"config", "bucketgit.logicalRepo", "app.git"},
 		{"config", "bucketgit.profile", "gcp:work/europe-west1"},
 		{"config", "user.name", "Repo User"},
 		{"config", "user.email", "old@example.com"},
@@ -403,7 +414,7 @@ func TestBrokerInitInteractiveIdentityOnlyUpdatesRepoConfig(t *testing.T) {
 	}
 	var stdout bytes.Buffer
 	input := strings.NewReader("\x1b[B\x1b[B\n" + strings.Repeat("\x7f", len("old@example.com")) + "new@example.com\n\x04")
-	err := brokerInitCommand([]string{"--config", configPath, "--repo", "team/app.git", target}, input, &stdout)
+	err := brokerInitCommand([]string{"--config", configPath, "--repo", "app.git", target}, input, &stdout)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -531,7 +542,7 @@ func TestInitDialogInitialStateUsesRepoThenGlobalIdentity(t *testing.T) {
 		t.Fatal(err)
 	}
 	for _, args := range [][]string{
-		{"config", "bucketgit.logicalRepo", "team/app.git"},
+		{"config", "bucketgit.logicalRepo", "app.git"},
 		{"config", "bucketgit.profile", "gcp:work/europe-west1"},
 		{"config", "user.name", "Repo User"},
 		{"config", "user.email", "repo@example.com"},
@@ -541,7 +552,7 @@ func TestInitDialogInitialStateUsesRepoThenGlobalIdentity(t *testing.T) {
 		}
 	}
 	initial := initDialogInitialState(target, globalConfig{Identity: globalIdentityConfig{Name: "Global User", Email: "global@example.com"}}, "", "")
-	if !initial.Existing || initial.RepoName != "team/app.git" || initial.ProfileName != "gcp:work/europe-west1" ||
+	if !initial.Existing || initial.RepoName != "app.git" || initial.ProfileName != "gcp:work/europe-west1" ||
 		initial.IdentityName != "Repo User" || initial.IdentityEmail != "repo@example.com" {
 		t.Fatalf("initial = %#v", initial)
 	}
@@ -739,7 +750,7 @@ func setupBrokerCommandTestRepo(t *testing.T, handler http.HandlerFunc) (string,
 	}))
 	for key, value := range map[string]string{
 		"bucketgit.broker":      server.URL,
-		"bucketgit.logicalRepo": "team/app.git",
+		"bucketgit.logicalRepo": "app.git",
 		"bucketgit.provider":    "gcs",
 		"bucketgit.branch":      "main",
 	} {

global_config_test.go

@@ -40,7 +40,7 @@ func TestGlobalConfigRoundTrip(t *testing.T) {
 			}},
 		}},
 		Repos: []globalRepoConfig{{
-			Name:      "team/app.git",
+			Name:      "app.git",
 			Profile:   "gcp:work",
 			BrokerURL: "https://gcp.example.test",
 		}},