From 3902faaaeec7611fe62c058232800aab4aa366f2 Mon Sep 17 00:00:00 2001 From: Dennis Vink Date: Tue, 19 May 2026 22:36:31 +0200 Subject: [PATCH] Enforce flat broker repo names --- CHANGELOG.md | 7 ++++ broker/aws/template.yaml | 17 +++++++-- broker/gcp/index.js | 20 +++++++++-- broker_commands.go | 77 +++++++++++++++++++++++++++++++--------- broker_commands_test.go | 39 ++++++++++++-------- global_config_test.go | 2 +- main.go | 22 ++++++++---- main_test.go | 30 +++++++++------- ssh.go | 13 +++++-- testsuite/aws/init.sh | 4 +++ testsuite/gcp/init.sh | 4 +++ web.go | 13 +++++-- www/app.js | 4 +-- 13 files changed, 188 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2854191..bfdcfcd 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/broker/aws/template.yaml b/broker/aws/template.yaml index 5afee22..a7257a9 100644 --- a/broker/aws/template.yaml +++ b/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) { diff --git a/broker/gcp/index.js b/broker/gcp/index.js index f351aeb..f1979de 100644 --- a/broker/gcp/index.js +++ b/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); diff --git a/broker_commands.go b/broker_commands.go index 417ccea..64a90b0 100644 --- a/broker_commands.go +++ b/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 [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 [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() diff --git a/broker_commands_test.go b/broker_commands_test.go index 9d9a76d..f9ee920 100644 --- a/broker_commands_test.go +++ b/broker_commands_test.go @@ -31,7 +31,7 @@ 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) } @@ -39,7 +39,7 @@ func TestBrokerInitWritesBrokerGitConfig(t *testing.T) { "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", } { diff --git a/global_config_test.go b/global_config_test.go index 711b306..8b54947 100644 --- a/global_config_test.go +++ b/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", }}, diff --git a/main.go b/main.go index a971d42..f446a9e 100644 --- a/main.go +++ b/main.go @@ -21,7 +21,7 @@ import ( const defaultBranch = "main" const defaultAuthMode = "gcloud" -const brokerVersion = "1.0.0-dev" +const brokerVersion = "1.0.1-dev" var version = "dev" @@ -592,7 +592,11 @@ func readLocalConfig(dir string) (config, error) { } logicalRepo := "" if logicalOut, logicalErr := runGit(dir, "config", "--get", "bucketgit.logicalRepo"); logicalErr == nil { - logicalRepo = strings.Trim(strings.TrimSpace(string(logicalOut)), "/") + var err error + logicalRepo, err = normalizeLogicalRepoName(string(logicalOut)) + if err != nil { + return config{}, err + } } localRegion := "" if regionOut, regionErr := runGit(dir, "config", "--get", "bucketgit.region"); regionErr == nil { @@ -756,7 +760,11 @@ func newRemoteStore(ctx context.Context, cfg config, publicFallback bool) (gitRe } if 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 nil, nil, normalizeErr + } + cfg.logicalRepo = logical } } if cfg.brokerURL != "" { @@ -954,16 +962,16 @@ func helpPages() map[string]string { return map[string]string{ "clone": `usage: bgit clone [directory] - bgit clone https://broker.example.com/team/app.git [directory] - bgit clone --broker https://broker.example.com team/app.git [directory] + bgit clone https://broker.example.com/app.git [directory] + bgit clone --broker https://broker.example.com app.git [directory] Clone a BucketGit repository by logical repo name. Passing a broker URL makes the checkout self-contained and does not require a local profile. Direct object-storage clone moved to bgit direct clone. examples: - bgit clone team/app.git - bgit clone https://bgit-broker.example.com/team/app.git + bgit clone app.git + bgit clone https://bgit-broker.example.com/app.git bgit direct clone gs://my-bucket/repositories/app.git bgit direct clone s3://my-bucket/repositories/app.git --profile aws-profile `, diff --git a/main_test.go b/main_test.go index d571dd2..e652271 100644 --- a/main_test.go +++ b/main_test.go @@ -1253,6 +1253,8 @@ func TestAWSBrokerCloudFormationTemplateHasBrokerOutput(t *testing.T) { "InvokedViaFunctionUrl", "/refs/update", "roleAllows", + "normalizeLogicalRepo", + "logical repo names must be flat", "ConditionalCheckFailedException", "BROKER_VERSION: " + brokerVersion, `version: brokerVersion`, @@ -1284,6 +1286,8 @@ func TestGCPBrokerSourceUsesFirestoreAndSignatureHeaders(t *testing.T) { "/objects/read", "/refs/update", "roleAllows", + "normalizeLogicalRepo", + "logical repo names must be flat", "runTransaction", "process.env.BROKER_VERSION", "version: brokerVersion", @@ -2156,7 +2160,7 @@ func TestLocalBranchLifecycleMaintainsOriginTracking(t *testing.T) { for _, args := range [][]string{ {"config", "user.name", "Ada"}, {"config", "user.email", "ada@example.com"}, - {"remote", "add", "origin", "git@git.bucketgit.com:team/app.git"}, + {"remote", "add", "origin", "git@git.bucketgit.com:app.git"}, } { if _, err := runGit(target, args...); err != nil { t.Fatal(err) @@ -3185,7 +3189,7 @@ func TestOpenWebRepositoryUsesBrokerFromRepoConfig(t *testing.T) { } for _, args := range [][]string{ {"config", "bucketgit.broker", "https://broker.example.test"}, - {"config", "bucketgit.logicalRepo", "team/app.git"}, + {"config", "bucketgit.logicalRepo", "app.git"}, {"config", "bucketgit.provider", "gcs"}, } { if _, err := runGit(target, args...); err != nil { @@ -3212,7 +3216,7 @@ func TestOpenWebRepositoryUsesBrokerFromRepoConfig(t *testing.T) { if !ok { t.Fatalf("api store = %T, want *brokerGitStore", apiRepo.store) } - if store.brokerURL != "https://broker.example.test" || cfg.brokerURL != "https://broker.example.test" || cfg.logicalRepo != "team/app.git" { + if store.brokerURL != "https://broker.example.test" || cfg.brokerURL != "https://broker.example.test" || cfg.logicalRepo != "app.git" { t.Fatalf("store=%#v cfg=%#v", store, cfg) } } @@ -3224,7 +3228,7 @@ func TestOpenWebRepositoryLocalBypassesBroker(t *testing.T) { } for _, args := range [][]string{ {"config", "bucketgit.broker", "https://broker.example.test"}, - {"config", "bucketgit.logicalRepo", "team/app.git"}, + {"config", "bucketgit.logicalRepo", "app.git"}, } { if _, err := runGit(target, args...); err != nil { t.Fatal(err) @@ -3254,17 +3258,17 @@ func TestOpenWebRepositoryLocalBypassesBroker(t *testing.T) { func TestWebClonePanelShowsBrokerCloneCommand(t *testing.T) { server := &webServer{cfg: config{ brokerURL: "https://broker.example.test/", - logicalRepo: "team/app.git", - origin: "git@git.bucketgit.com:team/app.git", + logicalRepo: "app.git", + origin: "git@git.bucketgit.com:app.git", }} html := server.clonePanelHTML() - if !strings.Contains(html, "team/app.git") { + if !strings.Contains(html, "app.git") { t.Fatalf("clone panel missing repo: %s", html) } - if !strings.Contains(html, "bgit clone https://broker.example.test/team/app.git") { + if !strings.Contains(html, "bgit clone https://broker.example.test/app.git") { t.Fatalf("clone panel missing broker clone command: %s", html) } - if !strings.Contains(html, "git@git.bucketgit.com:team/app.git") { + if !strings.Contains(html, "git@git.bucketgit.com:app.git") { t.Fatalf("clone panel missing ssh origin: %s", html) } } @@ -3272,14 +3276,14 @@ func TestWebClonePanelShowsBrokerCloneCommand(t *testing.T) { func TestWebRepoHeaderUsesShortTitleAndBrokerLocationBadge(t *testing.T) { cfg := config{ brokerURL: "https://broker.example.test/", - logicalRepo: "team/app.git", + logicalRepo: "app.git", } title := webRepoTitle(cfg) - if title != "team/app.git" { + if title != "app.git" { t.Fatalf("title = %q", title) } server := &webServer{cfg: cfg, title: title} - if badge := server.repoLocationBadge(); badge != "broker.example.test/team/app.git" { + if badge := server.repoLocationBadge(); badge != "broker.example.test/app.git" { t.Fatalf("badge = %q", badge) } header := server.headerHTML("refs/heads/main", "") @@ -3366,7 +3370,7 @@ func TestWebPullRequestCacheRendersPRTabAndPage(t *testing.T) { handler := newWebHandlerWithAPI( newNativeGitRepoForStore(config{branch: "main"}, &localGitStore{root: bare}), nil, - config{branch: "main", brokerURL: "https://broker.example.test", logicalRepo: "team/app.git", provider: "gcs"}, + config{branch: "main", brokerURL: "https://broker.example.test", logicalRepo: "app.git", provider: "gcs"}, ) if err := handler.writePullRequestCache([]brokerPullRequest{{ ID: 7, diff --git a/ssh.go b/ssh.go index e2b78f6..aafb65a 100644 --- a/ssh.go +++ b/ssh.go @@ -588,11 +588,15 @@ type brokerKeysResponse struct { } func brokerUpsertLogicalRepo(brokerURL, provider, logicalRepo string) error { + logical, err := normalizeLogicalRepoName(logicalRepo) + if err != nil { + return err + } cfg := config{ provider: provider, - prefix: strings.Trim(logicalRepo, "/"), - logicalRepo: strings.Trim(logicalRepo, "/"), - origin: fmt.Sprintf("git@%s:%s", defaultSSHHost, strings.Trim(logicalRepo, "/")), + prefix: logical, + logicalRepo: logical, + origin: fmt.Sprintf("git@%s:%s", defaultSSHHost, logical), } req := brokerRepoRequest{Repo: repoForBroker(cfg)} return brokerPost(brokerURL, "/repos/upsert", req, nil) @@ -723,6 +727,9 @@ func repoForBroker(cfg config) brokerRepo { cfg.origin = originForConfig(cfg) } logical := strings.Trim(firstNonEmpty(cfg.logicalRepo, cfg.prefix), "/") + if normalized, err := normalizeLogicalRepoName(logical); err == nil { + logical = normalized + } return brokerRepo{ Provider: firstNonEmpty(cfg.provider, "gcs"), Bucket: cfg.bucket, diff --git a/testsuite/aws/init.sh b/testsuite/aws/init.sh index dec7066..5aff5f3 100755 --- a/testsuite/aws/init.sh +++ b/testsuite/aws/init.sh @@ -8,3 +8,7 @@ out="$(git -C "$dir" config --get bucketgit.logicalRepo)" assert_contains "$out" "$repo" out="$(git -C "$dir" config --get bucketgit.broker)" assert_contains "$out" "http" + +bad_dir="$(new_workdir aws init-path-rejected)" +out="$(expect_failure "$BGIT" init --noninteractive --repo team/app --profile "$AWS_PROFILE" "${CONFIG_ARGS[@]}" "$bad_dir")" +assert_contains "$out" "logical repo names must be flat" diff --git a/testsuite/gcp/init.sh b/testsuite/gcp/init.sh index d264dda..c8bb228 100755 --- a/testsuite/gcp/init.sh +++ b/testsuite/gcp/init.sh @@ -8,3 +8,7 @@ out="$(git -C "$dir" config --get bucketgit.logicalRepo)" assert_contains "$out" "$repo" out="$(git -C "$dir" config --get bucketgit.broker)" assert_contains "$out" "http" + +bad_dir="$(new_workdir gcp init-path-rejected)" +out="$(expect_failure "$BGIT" init --noninteractive --repo team/app --profile "$GCP_PROFILE" "${CONFIG_ARGS[@]}" "$bad_dir")" +assert_contains "$out" "logical repo names must be flat" diff --git a/web.go b/web.go index 8bcdd2b..4eb7922 100644 --- a/web.go +++ b/web.go @@ -1409,8 +1409,13 @@ func (s *webServer) handleAPIActionSettings(ctx context.Context, w http.Response endpoint = "/owners/transfer/confirm" payload = brokerOwnerTransferRequest{Repo: repoForBroker(s.cfg), BrokerURL: s.cfg.brokerURL} case "repo-rename": + logical, err := normalizeLogicalRepoName(req.Logical) + if err != nil { + s.renderJSONError(w, http.StatusBadRequest, err) + return + } endpoint = "/repo/rename" - payload = brokerRepoInfoRequest{Repo: repoForBroker(s.cfg), Logical: logicalRepoWithGit(req.Logical)} + payload = brokerRepoInfoRequest{Repo: repoForBroker(s.cfg), Logical: logical} case "repo-delete": endpoint = "/repo/delete" payload = brokerRepoInfoRequest{Repo: repoForBroker(s.cfg)} @@ -1455,7 +1460,11 @@ func (s *webServer) handleAPIActionSettings(ctx context.Context, w http.Response return } if endpoint == "/repo/rename" && strings.TrimSpace(req.Logical) != "" { - logical := logicalRepoWithGit(req.Logical) + logical, err := normalizeLogicalRepoName(req.Logical) + if err != nil { + s.renderJSONError(w, http.StatusBadRequest, err) + return + } _, _ = runGit(".", "config", "--local", "bucketgit.logicalRepo", logical) _, _ = runGit(".", "remote", "set-url", "origin", "git@"+defaultSSHHost+":"+logical) s.cfg.logicalRepo = logical diff --git a/www/app.js b/www/app.js index a8be870..edff5fa 100644 --- a/www/app.js +++ b/www/app.js @@ -587,10 +587,10 @@ async function handleSettingsForm(form) { } else if (action === 'repo-rename') { payload.logical = formValue(form, 'logical'); if (!payload.logical) { - setSyncStatus('New logical repository name is required.', 'is-stale'); + setSyncStatus('New repository name is required.', 'is-stale'); return; } - const ok = await confirmModal({title: 'Rename repository?', body: 'Rename this logical repository to ' + payload.logical + '.', confirm: 'OK'}); + const ok = await confirmModal({title: 'Rename repository?', body: 'Rename this repository to ' + payload.logical + '.', confirm: 'OK'}); if (!ok) return; } else if (action === 'repo-delete') { const expected = form.querySelector('[data-confirm-repo]')?.getAttribute('data-confirm-repo') || '';