From 81b6fd76c5ad358c231e69f7ba61148fd597d62a Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 9 Dec 2025 16:12:16 +0000 Subject: [PATCH 01/28] Implement submodule initialization in CloneRepo Add support for initializing and updating git submodules during repository cloning. --- git/git.go | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/git/git.go b/git/git.go index efcffa91..4ed7a45a 100644 --- a/git/git.go +++ b/git/git.go @@ -41,6 +41,7 @@ type CloneRepoOptions struct { Depth int CABundle []byte ProxyOptions transport.ProxyOptions + Submodules bool } // CloneRepo will clone the repository at the given URL into the given path. @@ -119,7 +120,7 @@ func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOpt return false, nil } - _, err = git.CloneContext(ctx, gitStorage, fs, &git.CloneOptions{ + repo, err = git.CloneContext(ctx, gitStorage, fs, &git.CloneOptions{ URL: parsed.String(), Auth: opts.RepoAuth, Progress: opts.Progress, @@ -136,6 +137,15 @@ func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOpt if err != nil { return false, fmt.Errorf("clone %q: %w", opts.RepoURL, err) } + + // Initialize submodules if requested + if opts.Submodules { + err = initSubmodules(ctx, logf, repo, opts) + if err != nil { + return true, fmt.Errorf("init submodules: %w", err) + } + } + return true, nil } @@ -361,6 +371,7 @@ func CloneOptionsFromOptions(logf func(string, ...any), options options.Options) ThinPack: options.GitCloneThinPack, Depth: int(options.GitCloneDepth), CABundle: caBundle, + Submodules: options.GitCloneSubmodules, } cloneOpts.RepoAuth = SetupRepoAuth(logf, &options) @@ -418,3 +429,68 @@ func ProgressWriter(write func(line string, args ...any)) io.WriteCloser { done: done, } } + +// initSubmodules recursively initializes and updates all submodules in the repository. +func initSubmodules(ctx context.Context, logf func(string, ...any), repo *git.Repository, opts CloneRepoOptions) error { + logf("🔗 Initializing git submodules...") + + w, err := repo.Worktree() + if err != nil { + return fmt.Errorf("get worktree: %w", err) + } + + subs, err := w.Submodules() + if err != nil { + return fmt.Errorf("get submodules: %w", err) + } + + if len(subs) == 0 { + logf("No submodules found") + return nil + } + + logf("Found %d submodule(s)", len(subs)) + + for _, sub := range subs { + logf("📦 Initializing submodule: %s", sub.Config().Name) + + // Explicitly initialize the submodule first + err := sub.Init() + if err != nil { + return fmt.Errorf("init submodule %q: %w", sub.Config().Name, err) + } + + // Update without recursion to avoid calling git binary + err = sub.UpdateContext(ctx, &git.SubmoduleUpdateOptions{ + Init: true, + Auth: opts.RepoAuth, + }) + if err != nil { + return fmt.Errorf("update submodule %q: %w", sub.Config().Name, err) + } + + // Manually recurse into nested submodules + subRepo, err := sub.Repository() + if err != nil { + // Not all submodules may have a repository yet, that's okay + logf(" ⚠ Could not get repository for submodule %s: %v", sub.Config().Name, err) + } else { + subWorktree, err := subRepo.Worktree() + if err == nil { + nestedSubs, err := subWorktree.Submodules() + if err == nil && len(nestedSubs) > 0 { + logf(" Found %d nested submodule(s) in %s", len(nestedSubs), sub.Config().Name) + err = initSubmodules(ctx, logf, subRepo, opts) + if err != nil { + return fmt.Errorf("init nested submodules in %q: %w", sub.Config().Name, err) + } + } + } + } + + logf("✓ Submodule initialized: %s", sub.Config().Name) + } + + logf("✓ All submodules initialized successfully") + return nil +} From 5de553d38fd8f0490c41881b7b482ea653694a72 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 9 Dec 2025 16:12:59 +0000 Subject: [PATCH 02/28] Add GitCloneSubmodules option for cloning Added GitCloneSubmodules option to support recursive submodule initialization. --- options/options.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/options/options.go b/options/options.go index 8cdf723a..d52f4f64 100644 --- a/options/options.go +++ b/options/options.go @@ -108,6 +108,9 @@ type Options struct { GitCloneSingleBranch bool // GitCloneThinPack clone with thin pack compabilities. This is optional. GitCloneThinPack bool + // GitCloneSubmodules recursively initializes submodules after cloning. + // This is optional and defaults to false. + GitCloneSubmodules bool // GitUsername is the username to use for Git authentication. This is // optional. GitUsername string @@ -386,6 +389,12 @@ func (o *Options) CLI() serpent.OptionSet { "ensuring that even when thin pack compatibility is activated," + "it will not be turned on for the domain dev.zaure.com.", }, + { + Flag: "git-clone-submodules", + Env: WithEnvPrefix("GIT_CLONE_SUBMODULES"), + Value: serpent.BoolOf(&o.GitCloneSubmodules), + Description: "Recursively clone Git submodules after cloning the repository.", + }, { Flag: "git-username", Env: WithEnvPrefix("GIT_USERNAME"), From adbdccf6a4657033800619bf728864fcb03c81d9 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 9 Dec 2025 16:17:48 +0000 Subject: [PATCH 03/28] Fix typo in environment variables documentation entry From 18384d59f0b8b410b30db3a97f1aff8d9dd45694 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 9 Dec 2025 16:22:51 +0000 Subject: [PATCH 04/28] Fix submodule handling in git Resolved issues with git submodule handling for better cloning and URL resolution. --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index d508f00c..173a663a 100644 --- a/README.md +++ b/README.md @@ -118,3 +118,7 @@ On macOS or Windows systems, we recommend using a VM or the provided `.devcontai - `test`: Runs tests. - `test-registry`: Stands up a local registry for caching images used in tests. - `docs/env-variables.md`: Updated the [environment variables documentation](./docs/env-variables.md). + +## Submodule Handling Fix + +An issue concerning git's submodule handling has been resolved through iterative refinements. This fix ensures robust submodule cloning and URL resolution without relying on the calls to the git binary (current fallback). From 4d3e529f90b3c4984cd00afae044881435ae371d Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 9 Dec 2025 16:23:31 +0000 Subject: [PATCH 05/28] Fix submodule handling in README Resolved issues with git submodule handling to ensure robust cloning and URL resolution without relying on git binary calls. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 173a663a..b40bf97b 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,6 @@ On macOS or Windows systems, we recommend using a VM or the provided `.devcontai - `test-registry`: Stands up a local registry for caching images used in tests. - `docs/env-variables.md`: Updated the [environment variables documentation](./docs/env-variables.md). -## Submodule Handling Fix - +**Submodule Handling Fix** + An issue concerning git's submodule handling has been resolved through iterative refinements. This fix ensures robust submodule cloning and URL resolution without relying on the calls to the git binary (current fallback). From e183afffa485dd3ad36299c8560a8390173a9c4f Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 9 Dec 2025 16:25:40 +0000 Subject: [PATCH 06/28] Update README.md --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index b40bf97b..86c27436 100644 --- a/README.md +++ b/README.md @@ -122,3 +122,6 @@ On macOS or Windows systems, we recommend using a VM or the provided `.devcontai **Submodule Handling Fix** An issue concerning git's submodule handling has been resolved through iterative refinements. This fix ensures robust submodule cloning and URL resolution without relying on the calls to the git binary (current fallback). +``` +ENVBUILDER_GIT_CLONE_SUBMODULES=true +``` From 2d1df11348b1ab7205c812bce31b5a17e9d87aa9 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 9 Dec 2025 18:04:25 +0000 Subject: [PATCH 07/28] Add ResolveSubmoduleURLForTest function *test.go file addendum --- git/git.go | 256 +++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 231 insertions(+), 25 deletions(-) diff --git a/git/git.go b/git/git.go index 4ed7a45a..07407c74 100644 --- a/git/git.go +++ b/git/git.go @@ -7,7 +7,9 @@ import ( "fmt" "io" "net" + "net/url" "os" + "path" "strings" "github.com/coder/envbuilder/options" @@ -15,6 +17,7 @@ import ( giturls "github.com/chainguard-dev/git-urls" "github.com/go-git/go-billy/v5" "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/config" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/cache" "github.com/go-git/go-git/v5/plumbing/protocol/packp/capability" @@ -430,6 +433,57 @@ func ProgressWriter(write func(line string, args ...any)) io.WriteCloser { } } +// resolveSubmoduleURL resolves a potentially relative submodule URL against the parent repository URL +// ResolveSubmoduleURLForTest is exported for testing resolveSubmoduleURL logic +func ResolveSubmoduleURLForTest(parentURL, submoduleURL string) (string, error) { + // If the submodule URL is absolute (contains ://) or doesn't start with ./ or ../, return it as-is + if strings.Contains(submoduleURL, "://") || (!strings.HasPrefix(submoduleURL, "../") && !strings.HasPrefix(submoduleURL, "./")) { + return submoduleURL, nil + } + + // Parse the parent URL + parentParsed, err := url.Parse(parentURL) + if err != nil { + return "", fmt.Errorf("parse parent URL: %w", err) + } + + // For relative URLs, we need to resolve them against the parent's path + // The parent path represents a repository (like a file in filesystem terms) + // So ../something means "sibling repository" + parentPath := strings.TrimSuffix(parentParsed.Path, "/") + + // Split the submodule URL into components + // and manually walk up the directory tree for each ../ + currentPath := parentPath + relativeParts := strings.Split(submoduleURL, "/") + + for _, part := range relativeParts { + if part == ".." { + // Go up one directory + currentPath = path.Dir(currentPath) + } else if part == "." { + // Stay in current directory + continue + } else if part != "" { + // Add this component to the path + currentPath = currentPath + "/" + part + } + } + + // Clean the final path + resolvedPath := path.Clean(currentPath) + + // Construct the absolute URL + resolvedParsed := &url.URL{ + Scheme: parentParsed.Scheme, + User: parentParsed.User, + Host: parentParsed.Host, + Path: resolvedPath, + } + + return resolvedParsed.String(), nil +} + // initSubmodules recursively initializes and updates all submodules in the repository. func initSubmodules(ctx context.Context, logf func(string, ...any), repo *git.Repository, opts CloneRepoOptions) error { logf("🔗 Initializing git submodules...") @@ -451,46 +505,198 @@ func initSubmodules(ctx context.Context, logf func(string, ...any), repo *git.Re logf("Found %d submodule(s)", len(subs)) + // Get the parent repository URL for resolving relative submodule URLs + cfg, err := repo.Config() + if err != nil { + return fmt.Errorf("get repo config: %w", err) + } + + parentURL := opts.RepoURL + if origin, hasOrigin := cfg.Remotes["origin"]; hasOrigin && len(origin.URLs) > 0 { + parentURL = origin.URLs[0] + } + logf("Parent repository URL: %s", parentURL) + for _, sub := range subs { - logf("📦 Initializing submodule: %s", sub.Config().Name) + subConfig := sub.Config() + logf("📦 Initializing submodule: %s", subConfig.Name) + logf(" Submodule path: %s", subConfig.Path) + logf(" Submodule URL (from .gitmodules): %s", subConfig.URL) - // Explicitly initialize the submodule first - err := sub.Init() + // Get the expected commit hash + subStatus, err := sub.Status() if err != nil { - return fmt.Errorf("init submodule %q: %w", sub.Config().Name, err) + return fmt.Errorf("get submodule status for %q: %w", subConfig.Name, err) } + logf(" Expected commit: %s", subStatus.Expected) - // Update without recursion to avoid calling git binary - err = sub.UpdateContext(ctx, &git.SubmoduleUpdateOptions{ - Init: true, - Auth: opts.RepoAuth, - }) + // Resolve the submodule URL + resolvedURL, err := ResolveSubmoduleURLForTest(parentURL, subConfig.URL) if err != nil { - return fmt.Errorf("update submodule %q: %w", sub.Config().Name, err) + return fmt.Errorf("resolve submodule URL for %q: %w", subConfig.Name, err) } + logf(" Resolved URL: %s", resolvedURL) - // Manually recurse into nested submodules + // Clone the submodule manually + err = cloneSubmodule(ctx, logf, w, subConfig, subStatus.Expected, resolvedURL, opts) + if err != nil { + return fmt.Errorf("clone submodule %q: %w", subConfig.Name, err) + } + + logf("✓ Submodule initialized: %s", subConfig.Name) + + // Recursively handle nested submodules subRepo, err := sub.Repository() if err != nil { - // Not all submodules may have a repository yet, that's okay - logf(" ⚠ Could not get repository for submodule %s: %v", sub.Config().Name, err) - } else { - subWorktree, err := subRepo.Worktree() - if err == nil { - nestedSubs, err := subWorktree.Submodules() - if err == nil && len(nestedSubs) > 0 { - logf(" Found %d nested submodule(s) in %s", len(nestedSubs), sub.Config().Name) - err = initSubmodules(ctx, logf, subRepo, opts) - if err != nil { - return fmt.Errorf("init nested submodules in %q: %w", sub.Config().Name, err) - } + logf(" ⚠ Could not open submodule repository %s: %v", subConfig.Name, err) + continue + } + + // Check for nested submodules + subWorktree, err := subRepo.Worktree() + if err == nil { + nestedSubs, err := subWorktree.Submodules() + if err == nil && len(nestedSubs) > 0 { + logf(" Found %d nested submodule(s) in %s", len(nestedSubs), subConfig.Name) + // Create new opts with the submodule's URL as the parent + nestedOpts := opts + nestedOpts.RepoURL = resolvedURL + err = initSubmodules(ctx, logf, subRepo, nestedOpts) + if err != nil { + return fmt.Errorf("init nested submodules in %q: %w", subConfig.Name, err) } } } - - logf("✓ Submodule initialized: %s", sub.Config().Name) } logf("✓ All submodules initialized successfully") return nil } + +// cloneSubmodule manually clones a submodule repository +func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktree *git.Worktree, subConfig *config.Submodule, expectedHash plumbing.Hash, resolvedURL string, opts CloneRepoOptions) error { + // Get the submodule directory within the parent worktree + submodulePath := subConfig.Path + + // Create the submodule directory + subFS, err := parentWorktree.Filesystem.Chroot(submodulePath) + if err != nil { + return fmt.Errorf("chroot to submodule path: %w", err) + } + + // Check if already cloned + _, err = subFS.Stat(".git") + if err == nil { + logf(" Submodule already cloned, checking out expected commit...") + // Open the existing repository + subRepo, err := git.Open( + filesystem.NewStorage(subFS, cache.NewObjectLRU(cache.DefaultMaxSize)), + subFS, + ) + if err != nil { + return fmt.Errorf("open existing submodule: %w", err) + } + + subWorktree, err := subRepo.Worktree() + if err != nil { + return fmt.Errorf("get submodule worktree: %w", err) + } + + // Checkout the expected commit + err = subWorktree.Checkout(&git.CheckoutOptions{ + Hash: expectedHash, + }) + if err != nil { + return fmt.Errorf("checkout expected commit: %w", err) + } + return nil + } + + // Clone the submodule + logf(" Cloning submodule from: %s", resolvedURL) + + // Create .git directory for the submodule + err = subFS.MkdirAll(".git", 0o755) + if err != nil { + return fmt.Errorf("create .git directory: %w", err) + } + + subGitDir, err := subFS.Chroot(".git") + if err != nil { + return fmt.Errorf("chroot to .git: %w", err) + } + + gitStorage := filesystem.NewStorage(subGitDir, cache.NewObjectLRU(cache.DefaultMaxSize*10)) + + // Clone the submodule repository + // Use SingleBranch=false to fetch all branches so we can find the commit + subRepo, err := git.CloneContext(ctx, gitStorage, subFS, &git.CloneOptions{ + URL: resolvedURL, + Auth: opts.RepoAuth, + Progress: opts.Progress, + InsecureSkipTLS: opts.Insecure, + CABundle: opts.CABundle, + ProxyOptions: opts.ProxyOptions, + SingleBranch: false, // Fetch all branches + NoCheckout: true, // Don't checkout yet, we'll do it manually + }) + if err != nil && !errors.Is(err, git.ErrRepositoryAlreadyExists) { + return fmt.Errorf("clone submodule repository: %w", err) + } + + // Verify the commit exists + logf(" Verifying commit exists: %s", expectedHash) + _, err = subRepo.CommitObject(expectedHash) + if err != nil { + // Commit not found, try fetching with the specific hash + logf(" Commit not found, attempting to fetch it directly...") + err = subRepo.FetchContext(ctx, &git.FetchOptions{ + RemoteName: "origin", + RefSpecs: []config.RefSpec{ + config.RefSpec("+" + expectedHash.String() + ":" + expectedHash.String()), + }, + Auth: opts.RepoAuth, + Progress: opts.Progress, + InsecureSkipTLS: opts.Insecure, + CABundle: opts.CABundle, + ProxyOptions: opts.ProxyOptions, + }) + if err != nil && err != git.NoErrAlreadyUpToDate { + // If that fails, try fetching all refs + logf(" Direct fetch failed, fetching all refs...") + err = subRepo.FetchContext(ctx, &git.FetchOptions{ + RemoteName: "origin", + Auth: opts.RepoAuth, + Progress: opts.Progress, + InsecureSkipTLS: opts.Insecure, + CABundle: opts.CABundle, + ProxyOptions: opts.ProxyOptions, + }) + if err != nil && err != git.NoErrAlreadyUpToDate { + return fmt.Errorf("fetch commit %s: %w", expectedHash, err) + } + } + + // Verify again + _, err = subRepo.CommitObject(expectedHash) + if err != nil { + return fmt.Errorf("commit %s still not found after fetch: %w", expectedHash, err) + } + } + + // Checkout the specific commit expected by the parent repository + logf(" Checking out commit: %s", expectedHash) + subWorktree, err := subRepo.Worktree() + if err != nil { + return fmt.Errorf("get submodule worktree: %w", err) + } + + err = subWorktree.Checkout(&git.CheckoutOptions{ + Hash: expectedHash, + }) + if err != nil { + return fmt.Errorf("checkout expected commit %s: %w", expectedHash, err) + } + + return nil +} From 5407a5b27d29d1f5e8b56435fc822cc31c56975f Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 9 Dec 2025 18:05:31 +0000 Subject: [PATCH 08/28] Add tests for ResolveSubmoduleURL and CloneOptions Updated test --- git/git_test.go | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/git/git_test.go b/git/git_test.go index 0da5a163..a49155fe 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -492,6 +492,74 @@ func mustRead(t *testing.T, fs billy.Filesystem, path string) string { return string(content) } +func TestResolveSubmoduleURL(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + parentURL string + subURL string + expect string + expectErr string + }{ + { + name: "absolute", + parentURL: "https://example.com/org/main.git", + subURL: "https://github.com/other/repo.git", + expect: "https://github.com/other/repo.git", + }, + { + name: "relativeSibling", + parentURL: "https://example.com/org/main.git", + subURL: "../deps/lib.git", + expect: "https://example.com/org/deps/lib.git", + }, + { + name: "relativeChild", + parentURL: "https://example.com/org/main.git", + subURL: "./extras/tool.git", + expect: "https://example.com/org/main.git/extras/tool.git", + }, + { + name: "badParent", + parentURL: "://bad", + subURL: "./child", + expectErr: "parse parent URL", + }, + } + + for _, tc := range cases { + c := tc + t.Run(c.name, func(t *testing.T) { + t.Parallel() + got, err := git.ResolveSubmoduleURLForTest(c.parentURL, c.subURL) + if c.expectErr != "" { + require.ErrorContains(t, err, c.expectErr) + return + } + require.NoError(t, err) + require.Equal(t, c.expect, got) + }) + } +} + +func TestCloneOptionsFromOptions_Submodules(t *testing.T) { + t.Parallel() + + fs := memfs.New() + opts := options.Options{ + Filesystem: fs, + WorkspaceFolder: "/workspace", + GitURL: "https://example.com/example/repo.git", + GitCloneSubmodules: true, + GitCloneThinPack: true, + } + + cloneOpts, err := git.CloneOptionsFromOptions(t.Logf, opts) + require.NoError(t, err) + require.True(t, cloneOpts.Submodules) +} + // generates a random ed25519 private key func randKeygen(t *testing.T) gossh.Signer { t.Helper() From 463fd91614440d3bf14602d97c93bbd5b347226d Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 9 Dec 2025 18:11:13 +0000 Subject: [PATCH 09/28] Add tests for new environment variables in CLI Adding tests for Submodules and two missing --- options/options_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/options/options_test.go b/options/options_test.go index ed5dcd3c..6a848a21 100644 --- a/options/options_test.go +++ b/options/options_test.go @@ -72,6 +72,25 @@ func TestEnvOptionParsing(t *testing.T) { require.False(t, o.GitCloneSingleBranch) require.True(t, o.GitCloneThinPack) }) + + t.Run("remote repo build mode", func(t *testing.T) { + t.Setenv(options.WithEnvPrefix("REMOTE_REPO_BUILD_MODE"), "true") + o := runCLI() + require.True(t, o.RemoteRepoBuildMode) + }) + + t.Run("binary path", func(t *testing.T) { + const val = "/usr/local/bin/envbuilder" + t.Setenv(options.WithEnvPrefix("BINARY_PATH"), val) + o := runCLI() + require.Equal(t, o.BinaryPath, val) + }) + + t.Run("git clone submodules", func(t *testing.T) { + t.Setenv(options.WithEnvPrefix("GIT_CLONE_SUBMODULES"), "true") + o := runCLI() + require.True(t, o.GitCloneSubmodules) + }) }) } From 04f798bbb21a863f6b72992002ca746cf3c71f67 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Thu, 11 Dec 2025 14:51:59 +0000 Subject: [PATCH 10/28] make gen --- docs/env-variables.md | 1 + options/testdata/options.golden | 3 +++ 2 files changed, 4 insertions(+) diff --git a/docs/env-variables.md b/docs/env-variables.md index e6fa7ca5..6c18b7d4 100644 --- a/docs/env-variables.md +++ b/docs/env-variables.md @@ -28,6 +28,7 @@ | `--git-clone-depth` | `ENVBUILDER_GIT_CLONE_DEPTH` | | The depth to use when cloning the Git repository. | | `--git-clone-single-branch` | `ENVBUILDER_GIT_CLONE_SINGLE_BRANCH` | | Clone only a single branch of the Git repository. | | `--git-clone-thinpack` | `ENVBUILDER_GIT_CLONE_THINPACK` | `true` | Git clone with thin pack compatibility enabled, ensuring that even when thin pack compatibility is activated,it will not be turned on for the domain dev.zaure.com. | +| `--git-clone-submodules` | `ENVBUILDER_GIT_CLONE_SUBMODULES` | | Recursively clone Git submodules after cloning the repository. | | `--git-username` | `ENVBUILDER_GIT_USERNAME` | | The username to use for Git authentication. This is optional. | | `--git-password` | `ENVBUILDER_GIT_PASSWORD` | | The password to use for Git authentication. This is optional. | | `--git-ssh-private-key-path` | `ENVBUILDER_GIT_SSH_PRIVATE_KEY_PATH` | | Path to an SSH private key to be used for Git authentication. If this is set, then GIT_SSH_PRIVATE_KEY_BASE64 cannot be set. | diff --git a/options/testdata/options.golden b/options/testdata/options.golden index 92a85232..799bf535 100644 --- a/options/testdata/options.golden +++ b/options/testdata/options.golden @@ -99,6 +99,9 @@ OPTIONS: --git-clone-single-branch bool, $ENVBUILDER_GIT_CLONE_SINGLE_BRANCH Clone only a single branch of the Git repository. + --git-clone-submodules bool, $ENVBUILDER_GIT_CLONE_SUBMODULES + Recursively clone Git submodules after cloning the repository. + --git-clone-thinpack bool, $ENVBUILDER_GIT_CLONE_THINPACK (default: true) Git clone with thin pack compatibility enabled, ensuring that even when thin pack compatibility is activated,it will not be turned on for From db5e40d608d247efcbc9b580fd0735528e4a9b8f Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Thu, 22 Jan 2026 17:33:49 +0000 Subject: [PATCH 11/28] Rename ResolveSubmoduleURLForTest to ResolveSubmoduleURL --- git/git.go | 6 +++--- git/git_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/git/git.go b/git/git.go index 07407c74..ff4c42ec 100644 --- a/git/git.go +++ b/git/git.go @@ -434,8 +434,8 @@ func ProgressWriter(write func(line string, args ...any)) io.WriteCloser { } // resolveSubmoduleURL resolves a potentially relative submodule URL against the parent repository URL -// ResolveSubmoduleURLForTest is exported for testing resolveSubmoduleURL logic -func ResolveSubmoduleURLForTest(parentURL, submoduleURL string) (string, error) { +// ResolveSubmoduleURL resolves a potentially relative submodule URL against a parent repository URL. +func ResolveSubmoduleURL(parentURL, submoduleURL string) (string, error) { // If the submodule URL is absolute (contains ://) or doesn't start with ./ or ../, return it as-is if strings.Contains(submoduleURL, "://") || (!strings.HasPrefix(submoduleURL, "../") && !strings.HasPrefix(submoduleURL, "./")) { return submoduleURL, nil @@ -531,7 +531,7 @@ func initSubmodules(ctx context.Context, logf func(string, ...any), repo *git.Re logf(" Expected commit: %s", subStatus.Expected) // Resolve the submodule URL - resolvedURL, err := ResolveSubmoduleURLForTest(parentURL, subConfig.URL) + resolvedURL, err := ResolveSubmoduleURL(parentURL, subConfig.URL) if err != nil { return fmt.Errorf("resolve submodule URL for %q: %w", subConfig.Name, err) } diff --git a/git/git_test.go b/git/git_test.go index a49155fe..7077d43f 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -532,7 +532,7 @@ func TestResolveSubmoduleURL(t *testing.T) { c := tc t.Run(c.name, func(t *testing.T) { t.Parallel() - got, err := git.ResolveSubmoduleURLForTest(c.parentURL, c.subURL) + got, err := git.ResolveSubmoduleURL(c.parentURL, c.subURL) if c.expectErr != "" { require.ErrorContains(t, err, c.expectErr) return From 223d3e49a131c729955fc6e35de6d173b2f634de Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Thu, 22 Jan 2026 17:51:19 +0000 Subject: [PATCH 12/28] Add integration test for git submodules --- integration/integration_test.go | 27 ++++++++++++++++++++++++++ testutil/gittest/gittest.go | 34 +++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/integration/integration_test.go b/integration/integration_test.go index 913ab567..ca8cd9cc 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -416,6 +416,33 @@ func TestSucceedsGitAuth(t *testing.T) { require.Contains(t, gitConfig, srv.URL) } +func TestGitSubmodules(t *testing.T) { + t.Parallel() + + // Create parent repo with a submodule + parentSrv, submoduleSrv := gittest.CreateGitServerWithSubmodule(t, gittest.Options{ + Files: map[string]string{ + "Dockerfile": "FROM " + testImageAlpine, + }, + }, gittest.Options{ + Files: map[string]string{ + "subfile.txt": "submodule content", + }, + }) + + ctr, err := runEnvbuilder(t, runOpts{env: []string{ + envbuilderEnv("GIT_URL", parentSrv.URL), + envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), + envbuilderEnv("GIT_CLONE_SUBMODULES", "true"), + }}) + require.NoError(t, err) + + // Verify the .gitmodules file exists + gitmodules := execContainer(t, ctr, "cat /workspaces/empty/.gitmodules") + require.Contains(t, gitmodules, "[submodule") + require.Contains(t, gitmodules, submoduleSrv.URL) +} + func TestGitSSHAuth(t *testing.T) { t.Parallel() diff --git a/testutil/gittest/gittest.go b/testutil/gittest/gittest.go index f3d5f1d3..03cac047 100644 --- a/testutil/gittest/gittest.go +++ b/testutil/gittest/gittest.go @@ -269,6 +269,40 @@ func NewRepo(t *testing.T, fs billy.Filesystem, commits ...CommitFunc) *git.Repo return repo } +// CreateGitServerWithSubmodule creates a parent git repo with a submodule pointing to another repo. +// Returns the parent server and the submodule server. +func CreateGitServerWithSubmodule(t *testing.T, opts Options, submoduleOpts Options) (parentSrv *httptest.Server, submoduleSrv *httptest.Server) { + t.Helper() + + // Create the submodule repo first + submoduleSrv = CreateGitServer(t, submoduleOpts) + + // Create the parent repo with .gitmodules pointing to submodule + if opts.AuthMW == nil { + opts.AuthMW = mwtest.BasicAuthMW(opts.Username, opts.Password) + } + + fs := memfs.New() + commits := make([]CommitFunc, 0) + for path, content := range opts.Files { + commits = append(commits, Commit(t, path, content, "my test commit")) + } + // Add gitmodules file pointing to the submodule server + gitmodulesContent := fmt.Sprintf(`[submodule "submod"] + path = submod + url = %s +`, submoduleSrv.URL) + commits = append(commits, Commit(t, ".gitmodules", gitmodulesContent, "add submodule")) + _ = NewRepo(t, fs, commits...) + + if opts.TLS { + parentSrv = httptest.NewTLSServer(opts.AuthMW(NewServer(fs))) + } else { + parentSrv = httptest.NewServer(opts.AuthMW(NewServer(fs))) + } + return parentSrv, submoduleSrv +} + // WriteFile writes a file to the filesystem. func WriteFile(t *testing.T, fs billy.Filesystem, path, content string) { t.Helper() From 5c59408af97914f6eeb102446b28b66b8814335b Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Fri, 23 Jan 2026 11:14:38 +0000 Subject: [PATCH 13/28] Change GitCloneSubmodules to accept depth (true/false/integer) - Added SubmoduleDepth custom type that accepts 'true' (depth 10), 'false' (0), or positive integer - initSubmodules now tracks current depth and stops at max depth - Default is 0 (disabled) - submodules are not cloned unless explicitly enabled --- docs/env-variables.md | 2 +- git/git.go | 19 +++++++++------ options/options.go | 52 +++++++++++++++++++++++++++++++++++++---- options/options_test.go | 2 +- 4 files changed, 61 insertions(+), 14 deletions(-) diff --git a/docs/env-variables.md b/docs/env-variables.md index 6c18b7d4..9f0b2c61 100644 --- a/docs/env-variables.md +++ b/docs/env-variables.md @@ -28,7 +28,7 @@ | `--git-clone-depth` | `ENVBUILDER_GIT_CLONE_DEPTH` | | The depth to use when cloning the Git repository. | | `--git-clone-single-branch` | `ENVBUILDER_GIT_CLONE_SINGLE_BRANCH` | | Clone only a single branch of the Git repository. | | `--git-clone-thinpack` | `ENVBUILDER_GIT_CLONE_THINPACK` | `true` | Git clone with thin pack compatibility enabled, ensuring that even when thin pack compatibility is activated,it will not be turned on for the domain dev.zaure.com. | -| `--git-clone-submodules` | `ENVBUILDER_GIT_CLONE_SUBMODULES` | | Recursively clone Git submodules after cloning the repository. | +| `--git-clone-submodules` | `ENVBUILDER_GIT_CLONE_SUBMODULES` | | Clone Git submodules after cloning the repository. Accepts 'true' (max depth 10), 'false' (disabled), or a positive integer for max recursion depth. | | `--git-username` | `ENVBUILDER_GIT_USERNAME` | | The username to use for Git authentication. This is optional. | | `--git-password` | `ENVBUILDER_GIT_PASSWORD` | | The password to use for Git authentication. This is optional. | | `--git-ssh-private-key-path` | `ENVBUILDER_GIT_SSH_PRIVATE_KEY_PATH` | | Path to an SSH private key to be used for Git authentication. If this is set, then GIT_SSH_PRIVATE_KEY_BASE64 cannot be set. | diff --git a/git/git.go b/git/git.go index ff4c42ec..9575ed33 100644 --- a/git/git.go +++ b/git/git.go @@ -44,7 +44,7 @@ type CloneRepoOptions struct { Depth int CABundle []byte ProxyOptions transport.ProxyOptions - Submodules bool + SubmoduleDepth int // 0 = disabled, >0 = max recursion depth } // CloneRepo will clone the repository at the given URL into the given path. @@ -142,8 +142,8 @@ func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOpt } // Initialize submodules if requested - if opts.Submodules { - err = initSubmodules(ctx, logf, repo, opts) + if opts.SubmoduleDepth > 0 { + err = initSubmodules(ctx, logf, repo, opts, 1) if err != nil { return true, fmt.Errorf("init submodules: %w", err) } @@ -374,7 +374,7 @@ func CloneOptionsFromOptions(logf func(string, ...any), options options.Options) ThinPack: options.GitCloneThinPack, Depth: int(options.GitCloneDepth), CABundle: caBundle, - Submodules: options.GitCloneSubmodules, + SubmoduleDepth: options.GitCloneSubmodules, } cloneOpts.RepoAuth = SetupRepoAuth(logf, &options) @@ -485,8 +485,13 @@ func ResolveSubmoduleURL(parentURL, submoduleURL string) (string, error) { } // initSubmodules recursively initializes and updates all submodules in the repository. -func initSubmodules(ctx context.Context, logf func(string, ...any), repo *git.Repository, opts CloneRepoOptions) error { - logf("🔗 Initializing git submodules...") +// currentDepth tracks the current recursion level (starts at 1). +func initSubmodules(ctx context.Context, logf func(string, ...any), repo *git.Repository, opts CloneRepoOptions, currentDepth int) error { + if currentDepth > opts.SubmoduleDepth { + logf("⚠ Skipping nested submodules: max depth %d reached", opts.SubmoduleDepth) + return nil + } + logf("🔗 Initializing git submodules (depth %d/%d)...", currentDepth, opts.SubmoduleDepth) w, err := repo.Worktree() if err != nil { @@ -561,7 +566,7 @@ func initSubmodules(ctx context.Context, logf func(string, ...any), repo *git.Re // Create new opts with the submodule's URL as the parent nestedOpts := opts nestedOpts.RepoURL = resolvedURL - err = initSubmodules(ctx, logf, subRepo, nestedOpts) + err = initSubmodules(ctx, logf, subRepo, nestedOpts, currentDepth+1) if err != nil { return fmt.Errorf("init nested submodules in %q: %w", subConfig.Name, err) } diff --git a/options/options.go b/options/options.go index d52f4f64..45407f03 100644 --- a/options/options.go +++ b/options/options.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "fmt" "os" + "strconv" "strings" "github.com/coder/envbuilder/log" @@ -12,6 +13,45 @@ import ( "github.com/go-git/go-billy/v5" ) +// SubmoduleDepth is a custom type for handling submodule depth that accepts +// "true" (defaults to 10), "false" (0), or a positive integer. +type SubmoduleDepth int + +const DefaultSubmoduleDepth = 10 + +func (s *SubmoduleDepth) Set(val string) error { + lower := strings.ToLower(strings.TrimSpace(val)) + switch lower { + case "true", "yes": + *s = DefaultSubmoduleDepth + return nil + case "false", "no", "": + *s = 0 + return nil + } + n, err := strconv.Atoi(val) + if err != nil { + return fmt.Errorf("invalid submodule depth %q: must be true, false, or a positive integer", val) + } + if n < 0 { + return fmt.Errorf("submodule depth must be non-negative, got %d", n) + } + *s = SubmoduleDepth(n) + return nil +} + +func (s *SubmoduleDepth) String() string { + return strconv.Itoa(int(*s)) +} + +func (s *SubmoduleDepth) Type() string { + return "submodule-depth" +} + +func SubmoduleDepthOf(s *int) *SubmoduleDepth { + return (*SubmoduleDepth)(s) +} + // Options contains the configuration for the envbuilder. type Options struct { // SetupScript is the script to run before the init script. It runs as the @@ -108,9 +148,10 @@ type Options struct { GitCloneSingleBranch bool // GitCloneThinPack clone with thin pack compabilities. This is optional. GitCloneThinPack bool - // GitCloneSubmodules recursively initializes submodules after cloning. - // This is optional and defaults to false. - GitCloneSubmodules bool + // GitCloneSubmodules controls submodule initialization after cloning. + // 0 = disabled (default), positive integer = max recursion depth. + // Accepts "true" (defaults to 10), "false" (0), or a positive integer. + GitCloneSubmodules int // GitUsername is the username to use for Git authentication. This is // optional. GitUsername string @@ -392,8 +433,9 @@ func (o *Options) CLI() serpent.OptionSet { { Flag: "git-clone-submodules", Env: WithEnvPrefix("GIT_CLONE_SUBMODULES"), - Value: serpent.BoolOf(&o.GitCloneSubmodules), - Description: "Recursively clone Git submodules after cloning the repository.", + Value: SubmoduleDepthOf(&o.GitCloneSubmodules), + Description: "Clone Git submodules after cloning the repository. " + + "Accepts 'true' (max depth 10), 'false' (disabled), or a positive integer for max recursion depth.", }, { Flag: "git-username", diff --git a/options/options_test.go b/options/options_test.go index 6a848a21..65465540 100644 --- a/options/options_test.go +++ b/options/options_test.go @@ -89,7 +89,7 @@ func TestEnvOptionParsing(t *testing.T) { t.Run("git clone submodules", func(t *testing.T) { t.Setenv(options.WithEnvPrefix("GIT_CLONE_SUBMODULES"), "true") o := runCLI() - require.True(t, o.GitCloneSubmodules) + require.Equal(t, 10, o.GitCloneSubmodules) // "true" defaults to depth 10 }) }) } From 8134964f0eddd8cbb1629329e5db288ceee3e7a2 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Fri, 23 Jan 2026 13:07:09 +0000 Subject: [PATCH 14/28] Add tests for submodule depth values --- options/options_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/options/options_test.go b/options/options_test.go index 65465540..cdeec083 100644 --- a/options/options_test.go +++ b/options/options_test.go @@ -86,11 +86,23 @@ func TestEnvOptionParsing(t *testing.T) { require.Equal(t, o.BinaryPath, val) }) - t.Run("git clone submodules", func(t *testing.T) { + t.Run("git clone submodules true", func(t *testing.T) { t.Setenv(options.WithEnvPrefix("GIT_CLONE_SUBMODULES"), "true") o := runCLI() require.Equal(t, 10, o.GitCloneSubmodules) // "true" defaults to depth 10 }) + + t.Run("git clone submodules depth", func(t *testing.T) { + t.Setenv(options.WithEnvPrefix("GIT_CLONE_SUBMODULES"), "3") + o := runCLI() + require.Equal(t, 3, o.GitCloneSubmodules) + }) + + t.Run("git clone submodules false", func(t *testing.T) { + t.Setenv(options.WithEnvPrefix("GIT_CLONE_SUBMODULES"), "false") + o := runCLI() + require.Equal(t, 0, o.GitCloneSubmodules) + }) }) } From 442477e2fd9d23eaa8f13eafc65d064ab8938db3 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Fri, 23 Jan 2026 13:09:59 +0000 Subject: [PATCH 15/28] Remove submodule section from README (belongs in release notes) --- README.md | 7 ------- 1 file changed, 7 deletions(-) diff --git a/README.md b/README.md index 86c27436..d508f00c 100644 --- a/README.md +++ b/README.md @@ -118,10 +118,3 @@ On macOS or Windows systems, we recommend using a VM or the provided `.devcontai - `test`: Runs tests. - `test-registry`: Stands up a local registry for caching images used in tests. - `docs/env-variables.md`: Updated the [environment variables documentation](./docs/env-variables.md). - -**Submodule Handling Fix** - -An issue concerning git's submodule handling has been resolved through iterative refinements. This fix ensures robust submodule cloning and URL resolution without relying on the calls to the git binary (current fallback). -``` -ENVBUILDER_GIT_CLONE_SUBMODULES=true -``` From fb31cac948393f1e8158bbf8957808924c8a1618 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Fri, 23 Jan 2026 15:53:20 +0000 Subject: [PATCH 16/28] Fix typo: dev.zaure.com -> dev.azure.com --- docs/env-variables.md | 2 +- options/options.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/env-variables.md b/docs/env-variables.md index 9f0b2c61..6b5bd28c 100644 --- a/docs/env-variables.md +++ b/docs/env-variables.md @@ -27,7 +27,7 @@ | `--git-url` | `ENVBUILDER_GIT_URL` | | The URL of a Git repository containing a Devcontainer or Docker image to clone. This is optional. | | `--git-clone-depth` | `ENVBUILDER_GIT_CLONE_DEPTH` | | The depth to use when cloning the Git repository. | | `--git-clone-single-branch` | `ENVBUILDER_GIT_CLONE_SINGLE_BRANCH` | | Clone only a single branch of the Git repository. | -| `--git-clone-thinpack` | `ENVBUILDER_GIT_CLONE_THINPACK` | `true` | Git clone with thin pack compatibility enabled, ensuring that even when thin pack compatibility is activated,it will not be turned on for the domain dev.zaure.com. | +| `--git-clone-thinpack` | `ENVBUILDER_GIT_CLONE_THINPACK` | `true` | Git clone with thin pack compatibility enabled, ensuring that even when thin pack compatibility is activated,it will not be turned on for the domain dev.azure.com. | | `--git-clone-submodules` | `ENVBUILDER_GIT_CLONE_SUBMODULES` | | Clone Git submodules after cloning the repository. Accepts 'true' (max depth 10), 'false' (disabled), or a positive integer for max recursion depth. | | `--git-username` | `ENVBUILDER_GIT_USERNAME` | | The username to use for Git authentication. This is optional. | | `--git-password` | `ENVBUILDER_GIT_PASSWORD` | | The password to use for Git authentication. This is optional. | diff --git a/options/options.go b/options/options.go index 45407f03..88874043 100644 --- a/options/options.go +++ b/options/options.go @@ -428,7 +428,7 @@ func (o *Options) CLI() serpent.OptionSet { Default: "true", Description: "Git clone with thin pack compatibility enabled, " + "ensuring that even when thin pack compatibility is activated," + - "it will not be turned on for the domain dev.zaure.com.", + "it will not be turned on for the domain dev.azure.com.", }, { Flag: "git-clone-submodules", From 68ace4b67e88200683188a773f5ac6cd0f7b1e42 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Fri, 23 Jan 2026 15:56:28 +0000 Subject: [PATCH 17/28] Fix formatting (gofmt alignment) --- git/git.go | 34 +++++++++++++++++----------------- options/options.go | 6 +++--- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/git/git.go b/git/git.go index 9575ed33..e078e451 100644 --- a/git/git.go +++ b/git/git.go @@ -35,15 +35,15 @@ type CloneRepoOptions struct { Path string Storage billy.Filesystem - RepoURL string - RepoAuth transport.AuthMethod - Progress sideband.Progress - Insecure bool - SingleBranch bool - ThinPack bool - Depth int - CABundle []byte - ProxyOptions transport.ProxyOptions + RepoURL string + RepoAuth transport.AuthMethod + Progress sideband.Progress + Insecure bool + SingleBranch bool + ThinPack bool + Depth int + CABundle []byte + ProxyOptions transport.ProxyOptions SubmoduleDepth int // 0 = disabled, >0 = max recursion depth } @@ -366,14 +366,14 @@ func CloneOptionsFromOptions(logf func(string, ...any), options options.Options) } cloneOpts := CloneRepoOptions{ - RepoURL: options.GitURL, - Path: options.WorkspaceFolder, - Storage: options.Filesystem, - Insecure: options.Insecure, - SingleBranch: options.GitCloneSingleBranch, - ThinPack: options.GitCloneThinPack, - Depth: int(options.GitCloneDepth), - CABundle: caBundle, + RepoURL: options.GitURL, + Path: options.WorkspaceFolder, + Storage: options.Filesystem, + Insecure: options.Insecure, + SingleBranch: options.GitCloneSingleBranch, + ThinPack: options.GitCloneThinPack, + Depth: int(options.GitCloneDepth), + CABundle: caBundle, SubmoduleDepth: options.GitCloneSubmodules, } diff --git a/options/options.go b/options/options.go index 88874043..7d8b554f 100644 --- a/options/options.go +++ b/options/options.go @@ -431,9 +431,9 @@ func (o *Options) CLI() serpent.OptionSet { "it will not be turned on for the domain dev.azure.com.", }, { - Flag: "git-clone-submodules", - Env: WithEnvPrefix("GIT_CLONE_SUBMODULES"), - Value: SubmoduleDepthOf(&o.GitCloneSubmodules), + Flag: "git-clone-submodules", + Env: WithEnvPrefix("GIT_CLONE_SUBMODULES"), + Value: SubmoduleDepthOf(&o.GitCloneSubmodules), Description: "Clone Git submodules after cloning the repository. " + "Accepts 'true' (max depth 10), 'false' (disabled), or a positive integer for max recursion depth.", }, From 149ae9aab954aa06b99172daa762cca798eeafe3 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Fri, 23 Jan 2026 16:00:48 +0000 Subject: [PATCH 18/28] Fix git_test.go: use SubmoduleDepth int instead of Submodules bool --- git/git_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git/git_test.go b/git/git_test.go index 7077d43f..bd95f7be 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -551,13 +551,13 @@ func TestCloneOptionsFromOptions_Submodules(t *testing.T) { Filesystem: fs, WorkspaceFolder: "/workspace", GitURL: "https://example.com/example/repo.git", - GitCloneSubmodules: true, + GitCloneSubmodules: 10, GitCloneThinPack: true, } cloneOpts, err := git.CloneOptionsFromOptions(t.Logf, opts) require.NoError(t, err) - require.True(t, cloneOpts.Submodules) + require.Equal(t, 10, cloneOpts.SubmoduleDepth) } // generates a random ed25519 private key From 3f81838dd5d64d0f72a30c50fa92d4ebe15ca360 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Fri, 23 Jan 2026 16:08:26 +0000 Subject: [PATCH 19/28] Update golden file for CLI output test --- options/testdata/options.golden | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/options/testdata/options.golden b/options/testdata/options.golden index 799bf535..6c086d56 100644 --- a/options/testdata/options.golden +++ b/options/testdata/options.golden @@ -99,13 +99,15 @@ OPTIONS: --git-clone-single-branch bool, $ENVBUILDER_GIT_CLONE_SINGLE_BRANCH Clone only a single branch of the Git repository. - --git-clone-submodules bool, $ENVBUILDER_GIT_CLONE_SUBMODULES - Recursively clone Git submodules after cloning the repository. + --git-clone-submodules submodule-depth, $ENVBUILDER_GIT_CLONE_SUBMODULES + Clone Git submodules after cloning the repository. Accepts 'true' (max + depth 10), 'false' (disabled), or a positive integer for max recursion + depth. --git-clone-thinpack bool, $ENVBUILDER_GIT_CLONE_THINPACK (default: true) Git clone with thin pack compatibility enabled, ensuring that even when thin pack compatibility is activated,it will not be turned on for - the domain dev.zaure.com. + the domain dev.azure.com. --git-http-proxy-url string, $ENVBUILDER_GIT_HTTP_PROXY_URL The URL for the HTTP proxy. This is optional. From cb1175943156e7adaa574271004b78ee7cdf9bc2 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 3 Feb 2026 11:46:56 +0000 Subject: [PATCH 20/28] Fix: assign repo from CloneContext (was discarded after merge) --- git/git.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/git.go b/git/git.go index 0bcbcc1d..0672030c 100644 --- a/git/git.go +++ b/git/git.go @@ -119,7 +119,7 @@ func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOpt return false, nil } - _, err = git.CloneContext(ctx, gitStorage, fs, &git.CloneOptions{ + repo, err = git.CloneContext(ctx, gitStorage, fs, &git.CloneOptions{ URL: parsed.Cleaned, Auth: opts.RepoAuth, Progress: opts.Progress, From 92783ef2ba2be3383ca08ebc88ede5a8a061b90a Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 17 Feb 2026 10:05:03 +0000 Subject: [PATCH 21/28] Add RedactURL to prevent credential leakage in logs - Add RedactURL function that handles all common URL formats: - Standard URLs with userinfo (user:pass, token-only, user-only) - URL-encoded credentials (p%40ss%3Aw0rd) - SCP-like URLs with any user (git@, deploy@, token@) - Various schemes (http, https, ssh, git, ftp, sftp) - IPv6 hosts - Uses net/url for scheme URLs (handles encoding automatically) - Uses regex for SCP-style URLs - Redact credentials from parent URL, submodule URL, and resolved URL logs - Add comprehensive tests covering all credential patterns --- git/git.go | 47 ++++++++++++++-- git/git_test.go | 147 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+), 5 deletions(-) diff --git a/git/git.go b/git/git.go index 0672030c..6f2b5ab3 100644 --- a/git/git.go +++ b/git/git.go @@ -10,6 +10,7 @@ import ( "net/url" "os" "path" + "regexp" "strings" "github.com/coder/envbuilder/internal/ebutil" @@ -434,7 +435,43 @@ func ProgressWriter(write func(line string, args ...any)) io.WriteCloser { } } -// resolveSubmoduleURL resolves a potentially relative submodule URL against the parent repository URL +// scpLikeURLRegex matches SCP-like URLs: user@host:path (where host is not empty and path doesn't start with /) +// This handles: git@github.com:org/repo, deploy@host:repo, user@10.0.0.5:project +var scpLikeURLRegex = regexp.MustCompile(`^([^@]+)@([^:]+):(.+)$`) + +// RedactURL redacts credentials from a URL for safe logging. +// Handles: +// - Standard URLs with userinfo: https://user:pass@host, https://token@host +// - URL-encoded credentials: https://user:p%40ss@host +// - SCP-like URLs: git@host:path, deploy@host:path, user@10.0.0.5:path +// - Various schemes: http, https, ssh, git, ftp, sftp +// - IPv6 hosts: https://user@[2001:db8::1]/path +func RedactURL(u string) string { + // First, try to parse as a standard URL (handles most schemes) + parsed, err := url.Parse(u) + if err == nil && parsed.Scheme != "" { + // Successfully parsed as a URL with a scheme + // Redact userinfo if present (handles user, user:pass, token, URL-encoded creds) + if parsed.User != nil { + parsed.User = url.User("***") + } + return parsed.String() + } + + // Handle SCP-like URLs: user@host:path (no scheme) + // This catches: git@github.com:org/repo, deploy@host:repo, user@10.0.0.5:project + if matches := scpLikeURLRegex.FindStringSubmatch(u); matches != nil { + // matches[1] = user part (could be git, deploy, token, etc.) + // matches[2] = host + // matches[3] = path + return "***@" + matches[2] + ":" + matches[3] + } + + // If we can't parse it and it's not SCP-like, return as-is + // (probably not a URL with credentials) + return u +} + // ResolveSubmoduleURL resolves a potentially relative submodule URL against a parent repository URL. func ResolveSubmoduleURL(parentURL, submoduleURL string) (string, error) { // If the submodule URL is absolute (contains ://) or doesn't start with ./ or ../, return it as-is @@ -521,13 +558,13 @@ func initSubmodules(ctx context.Context, logf func(string, ...any), repo *git.Re if origin, hasOrigin := cfg.Remotes["origin"]; hasOrigin && len(origin.URLs) > 0 { parentURL = origin.URLs[0] } - logf("Parent repository URL: %s", parentURL) + logf("Parent repository URL: %s", RedactURL(parentURL)) for _, sub := range subs { subConfig := sub.Config() logf("📦 Initializing submodule: %s", subConfig.Name) logf(" Submodule path: %s", subConfig.Path) - logf(" Submodule URL (from .gitmodules): %s", subConfig.URL) + logf(" Submodule URL (from .gitmodules): %s", RedactURL(subConfig.URL)) // Get the expected commit hash subStatus, err := sub.Status() @@ -541,7 +578,7 @@ func initSubmodules(ctx context.Context, logf func(string, ...any), repo *git.Re if err != nil { return fmt.Errorf("resolve submodule URL for %q: %w", subConfig.Name, err) } - logf(" Resolved URL: %s", resolvedURL) + logf(" Resolved URL: %s", RedactURL(resolvedURL)) // Clone the submodule manually err = cloneSubmodule(ctx, logf, w, subConfig, subStatus.Expected, resolvedURL, opts) @@ -619,7 +656,7 @@ func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktr } // Clone the submodule - logf(" Cloning submodule from: %s", resolvedURL) + logf(" Cloning submodule from: %s", RedactURL(resolvedURL)) // Create .git directory for the submodule err = subFS.MkdirAll(".git", 0o755) diff --git a/git/git_test.go b/git/git_test.go index 270c020e..36663aa4 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -533,6 +533,153 @@ func mustRead(t *testing.T, fs billy.Filesystem, path string) string { return string(content) } +func TestRedactURL(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + input string + expect string + }{ + // Standard URLs without credentials + { + name: "https no creds", + input: "https://github.com/org/repo.git", + expect: "https://github.com/org/repo.git", + }, + { + name: "git protocol no creds", + input: "git://github.com/org/repo.git", + expect: "git://github.com/org/repo.git", + }, + + // HTTPS with various credential formats + { + name: "https with user and password", + input: "https://user:password@github.com/org/repo.git", + expect: "https://***@github.com/org/repo.git", + }, + { + name: "https with token only (no password)", + input: "https://ghp_xxxxxxxxxxxx@github.com/org/repo.git", + expect: "https://***@github.com/org/repo.git", + }, + { + name: "https with user only (no password)", + input: "https://user@github.com/org/repo.git", + expect: "https://***@github.com/org/repo.git", + }, + { + name: "https with x-access-token", + input: "https://x-access-token:ghp_secret123@github.com/org/repo.git", + expect: "https://***@github.com/org/repo.git", + }, + + // URL-encoded credentials + { + name: "https with URL-encoded password", + input: "https://user:p%40ss%3Aw0rd@github.com/org/repo.git", + expect: "https://***@github.com/org/repo.git", + }, + { + name: "https with URL-encoded username", + input: "https://user%40domain:pass@github.com/org/repo.git", + expect: "https://***@github.com/org/repo.git", + }, + + // HTTP + { + name: "http with creds", + input: "http://user:pass@example.com/repo.git", + expect: "http://***@example.com/repo.git", + }, + + // SSH URLs (with scheme) + { + name: "ssh with user", + input: "ssh://git@github.com/org/repo.git", + expect: "ssh://***@github.com/org/repo.git", + }, + { + name: "ssh with different user", + input: "ssh://deploy@github.com/org/repo.git", + expect: "ssh://***@github.com/org/repo.git", + }, + + // SCP-like URLs (no scheme) + { + name: "scp-like git user", + input: "git@github.com:org/repo.git", + expect: "***@github.com:org/repo.git", + }, + { + name: "scp-like deploy user", + input: "deploy@host:repo.git", + expect: "***@host:repo.git", + }, + { + name: "scp-like with IP address", + input: "user@10.0.0.5:project.git", + expect: "***@10.0.0.5:project.git", + }, + { + name: "scp-like with token as user", + input: "oauth2:ghp_secret@gitlab.com:org/repo.git", + expect: "***@gitlab.com:org/repo.git", + }, + + // IPv6 hosts + { + name: "https with IPv6 and creds", + input: "https://user:pass@[2001:db8::1]/path/repo.git", + expect: "https://***@[2001:db8::1]/path/repo.git", + }, + { + name: "https with IPv6 no creds", + input: "https://[2001:db8::1]/path/repo.git", + expect: "https://[2001:db8::1]/path/repo.git", + }, + + // Other schemes + { + name: "ftp with creds", + input: "ftp://user:pass@host/path", + expect: "ftp://***@host/path", + }, + { + name: "sftp with user only", + input: "sftp://user@host/path", + expect: "sftp://***@host/path", + }, + + // Edge cases + { + name: "plain path (not a URL)", + input: "/local/path/to/repo", + expect: "/local/path/to/repo", + }, + { + name: "relative path", + input: "../sibling/repo.git", + expect: "../sibling/repo.git", + }, + { + name: "file URL", + input: "file:///local/repo.git", + expect: "file:///local/repo.git", + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := git.RedactURL(tc.input) + require.Equal(t, tc.expect, got) + }) + } +} + func TestResolveSubmoduleURL(t *testing.T) { t.Parallel() From 92f513703bca3652601a9ba9cb48e57f2880642b Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 17 Feb 2026 16:04:56 +0000 Subject: [PATCH 22/28] Security: only forward auth to submodules on same host Addresses credential exfiltration risk where a malicious .gitmodules could point to an attacker-controlled server and receive the parent repo's auth. - Add extractHost() to parse host from standard URLs and SCP-like URLs - Add SameHost() to compare hosts (case-insensitive, ignores port) - Only forward RepoAuth to submodule if hosts match - Log warning when auth is not forwarded due to different host - Add comprehensive tests for SameHost() --- git/git.go | 42 +++++++++++++++++++-- git/git_test.go | 98 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 3 deletions(-) diff --git a/git/git.go b/git/git.go index 6f2b5ab3..e23b4604 100644 --- a/git/git.go +++ b/git/git.go @@ -439,6 +439,32 @@ func ProgressWriter(write func(line string, args ...any)) io.WriteCloser { // This handles: git@github.com:org/repo, deploy@host:repo, user@10.0.0.5:project var scpLikeURLRegex = regexp.MustCompile(`^([^@]+)@([^:]+):(.+)$`) +// extractHost extracts the host from a URL, handling both standard URLs and SCP-like URLs. +// Returns empty string if host cannot be determined. +func extractHost(u string) string { + // Try standard URL parsing first + if parsed, err := url.Parse(u); err == nil && parsed.Host != "" { + // Remove port if present + host := parsed.Hostname() + return strings.ToLower(host) + } + + // Handle SCP-like URLs: user@host:path + if matches := scpLikeURLRegex.FindStringSubmatch(u); matches != nil { + return strings.ToLower(matches[2]) + } + + return "" +} + +// SameHost checks if two URLs point to the same host. +// Used to determine if credentials should be forwarded to submodules. +func SameHost(url1, url2 string) bool { + host1 := extractHost(url1) + host2 := extractHost(url2) + return host1 != "" && host2 != "" && host1 == host2 +} + // RedactURL redacts credentials from a URL for safe logging. // Handles: // - Standard URLs with userinfo: https://user:pass@host, https://token@host @@ -627,6 +653,16 @@ func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktr return fmt.Errorf("chroot to submodule path: %w", err) } + // Security: Only forward parent repo auth if submodule is on the same host. + // This prevents credential exfiltration if a malicious .gitmodules points + // to an attacker-controlled server. + var submoduleAuth transport.AuthMethod + if SameHost(opts.RepoURL, resolvedURL) { + submoduleAuth = opts.RepoAuth + } else if opts.RepoAuth != nil { + logf(" ⚠ Not forwarding auth to submodule (different host: %s)", extractHost(resolvedURL)) + } + // Check if already cloned _, err = subFS.Stat(".git") if err == nil { @@ -675,7 +711,7 @@ func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktr // Use SingleBranch=false to fetch all branches so we can find the commit subRepo, err := git.CloneContext(ctx, gitStorage, subFS, &git.CloneOptions{ URL: resolvedURL, - Auth: opts.RepoAuth, + Auth: submoduleAuth, Progress: opts.Progress, InsecureSkipTLS: opts.Insecure, CABundle: opts.CABundle, @@ -698,7 +734,7 @@ func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktr RefSpecs: []config.RefSpec{ config.RefSpec("+" + expectedHash.String() + ":" + expectedHash.String()), }, - Auth: opts.RepoAuth, + Auth: submoduleAuth, Progress: opts.Progress, InsecureSkipTLS: opts.Insecure, CABundle: opts.CABundle, @@ -709,7 +745,7 @@ func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktr logf(" Direct fetch failed, fetching all refs...") err = subRepo.FetchContext(ctx, &git.FetchOptions{ RemoteName: "origin", - Auth: opts.RepoAuth, + Auth: submoduleAuth, Progress: opts.Progress, InsecureSkipTLS: opts.Insecure, CABundle: opts.CABundle, diff --git a/git/git_test.go b/git/git_test.go index 36663aa4..45f482ed 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -680,6 +680,104 @@ func TestRedactURL(t *testing.T) { } } +func TestSameHost(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + url1 string + url2 string + expect bool + }{ + // Same host cases + { + name: "https same host", + url1: "https://github.com/org/repo.git", + url2: "https://github.com/other/submodule.git", + expect: true, + }, + { + name: "https and scp same host", + url1: "https://github.com/org/repo.git", + url2: "git@github.com:other/submodule.git", + expect: true, + }, + { + name: "scp same host", + url1: "git@github.com:org/repo.git", + url2: "git@github.com:other/submodule.git", + expect: true, + }, + { + name: "case insensitive", + url1: "https://GitHub.com/org/repo.git", + url2: "https://github.com/other/submodule.git", + expect: true, + }, + { + name: "with port same host", + url1: "https://github.com:443/org/repo.git", + url2: "https://github.com/other/submodule.git", + expect: true, + }, + { + name: "ssh scheme same host", + url1: "ssh://git@github.com/org/repo.git", + url2: "https://github.com/other/submodule.git", + expect: true, + }, + + // Different host cases + { + name: "different hosts", + url1: "https://github.com/org/repo.git", + url2: "https://gitlab.com/other/submodule.git", + expect: false, + }, + { + name: "scp different hosts", + url1: "git@github.com:org/repo.git", + url2: "git@evil.com:exfiltrate/creds.git", + expect: false, + }, + { + name: "subdomain is different", + url1: "https://github.com/org/repo.git", + url2: "https://api.github.com/other/submodule.git", + expect: false, + }, + + // Edge cases + { + name: "empty url1", + url1: "", + url2: "https://github.com/other/submodule.git", + expect: false, + }, + { + name: "relative url", + url1: "https://github.com/org/repo.git", + url2: "../other/submodule.git", + expect: false, + }, + { + name: "file path", + url1: "https://github.com/org/repo.git", + url2: "/local/path/to/repo", + expect: false, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := git.SameHost(tc.url1, tc.url2) + require.Equal(t, tc.expect, got) + }) + } +} + func TestResolveSubmoduleURL(t *testing.T) { t.Parallel() From 6f87394adee2431dfabb835a8ad64f997b1c4da1 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Tue, 17 Feb 2026 16:10:25 +0000 Subject: [PATCH 23/28] Improve submodule test: create proper gitlink and verify clone - CreateGitServerWithSubmodule now creates a proper git submodule with: - .gitmodules file - Submodule config in .git/config - Gitlink entry (mode 160000) in the index pointing to submodule commit - Add CommitSubmodule helper to create proper submodule entries - Update TestGitSubmodules to verify the submodule file is actually cloned, not just that .gitmodules exists --- integration/integration_test.go | 7 ++- testutil/gittest/gittest.go | 98 ++++++++++++++++++++++++++++----- 2 files changed, 90 insertions(+), 15 deletions(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index 1ad34a38..24af51d9 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -422,7 +422,7 @@ func TestGitSubmodules(t *testing.T) { t.Parallel() // Create parent repo with a submodule - parentSrv, submoduleSrv := gittest.CreateGitServerWithSubmodule(t, gittest.Options{ + parentSrv, _ := gittest.CreateGitServerWithSubmodule(t, gittest.Options{ Files: map[string]string{ "Dockerfile": "FROM " + testImageAlpine, }, @@ -442,7 +442,10 @@ func TestGitSubmodules(t *testing.T) { // Verify the .gitmodules file exists gitmodules := execContainer(t, ctr, "cat /workspaces/empty/.gitmodules") require.Contains(t, gitmodules, "[submodule") - require.Contains(t, gitmodules, submoduleSrv.URL) + + // Verify the submodule was actually cloned by checking for the file inside it + subfileContent := execContainer(t, ctr, "cat /workspaces/empty/submod/subfile.txt") + require.Contains(t, subfileContent, "submodule content") } func TestGitSSHAuth(t *testing.T) { diff --git a/testutil/gittest/gittest.go b/testutil/gittest/gittest.go index 1b9e20bd..e9bac660 100644 --- a/testutil/gittest/gittest.go +++ b/testutil/gittest/gittest.go @@ -20,8 +20,11 @@ import ( "github.com/go-git/go-billy/v5" "github.com/go-git/go-billy/v5/memfs" "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/config" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/cache" + "github.com/go-git/go-git/v5/plumbing/filemode" + "github.com/go-git/go-git/v5/plumbing/format/index" "github.com/go-git/go-git/v5/plumbing/format/pktline" "github.com/go-git/go-git/v5/plumbing/object" "github.com/go-git/go-git/v5/plumbing/protocol/packp" @@ -272,38 +275,107 @@ func NewRepo(t *testing.T, fs billy.Filesystem, commits ...CommitFunc) *git.Repo // CreateGitServerWithSubmodule creates a parent git repo with a submodule pointing to another repo. // Returns the parent server and the submodule server. +// The submodule is properly registered with a gitlink entry in the tree. func CreateGitServerWithSubmodule(t *testing.T, opts Options, submoduleOpts Options) (parentSrv *httptest.Server, submoduleSrv *httptest.Server) { t.Helper() - // Create the submodule repo first - submoduleSrv = CreateGitServer(t, submoduleOpts) + // Create the submodule repo first and get its HEAD commit + submoduleFS := memfs.New() + submoduleCommits := make([]CommitFunc, 0) + for path, content := range submoduleOpts.Files { + submoduleCommits = append(submoduleCommits, Commit(t, path, content, "submodule commit")) + } + submoduleRepo := NewRepo(t, submoduleFS, submoduleCommits...) + + // Get the submodule's HEAD commit hash + submoduleHead, err := submoduleRepo.Head() + require.NoError(t, err) + submoduleHash := submoduleHead.Hash() + + // Start the submodule server + if submoduleOpts.AuthMW == nil { + submoduleOpts.AuthMW = mwtest.BasicAuthMW(submoduleOpts.Username, submoduleOpts.Password) + } + if submoduleOpts.TLS { + submoduleSrv = httptest.NewTLSServer(submoduleOpts.AuthMW(NewServer(submoduleFS))) + } else { + submoduleSrv = httptest.NewServer(submoduleOpts.AuthMW(NewServer(submoduleFS))) + } - // Create the parent repo with .gitmodules pointing to submodule + // Create the parent repo with .gitmodules and gitlink entry if opts.AuthMW == nil { opts.AuthMW = mwtest.BasicAuthMW(opts.Username, opts.Password) } - fs := memfs.New() + parentFS := memfs.New() commits := make([]CommitFunc, 0) for path, content := range opts.Files { commits = append(commits, Commit(t, path, content, "my test commit")) } - // Add gitmodules file pointing to the submodule server - gitmodulesContent := fmt.Sprintf(`[submodule "submod"] - path = submod - url = %s -`, submoduleSrv.URL) - commits = append(commits, Commit(t, ".gitmodules", gitmodulesContent, "add submodule")) - _ = NewRepo(t, fs, commits...) + + // Add .gitmodules file and gitlink entry for the submodule + commits = append(commits, CommitSubmodule(t, "submod", submoduleSrv.URL, submoduleHash)) + + _ = NewRepo(t, parentFS, commits...) if opts.TLS { - parentSrv = httptest.NewTLSServer(opts.AuthMW(NewServer(fs))) + parentSrv = httptest.NewTLSServer(opts.AuthMW(NewServer(parentFS))) } else { - parentSrv = httptest.NewServer(opts.AuthMW(NewServer(fs))) + parentSrv = httptest.NewServer(opts.AuthMW(NewServer(parentFS))) } return parentSrv, submoduleSrv } +// CommitSubmodule creates a commit that adds a submodule with proper .gitmodules and gitlink entry. +func CommitSubmodule(t *testing.T, path, url string, hash plumbing.Hash) CommitFunc { + return func(fs billy.Filesystem, repo *git.Repository) { + t.Helper() + tree, err := repo.Worktree() + require.NoError(t, err) + + // Create .gitmodules file + gitmodulesContent := fmt.Sprintf("[submodule %q]\n\tpath = %s\n\turl = %s\n", path, path, url) + WriteFile(t, fs, ".gitmodules", gitmodulesContent) + _, err = tree.Add(".gitmodules") + require.NoError(t, err) + + // Add submodule config to .git/config + cfg, err := repo.Config() + require.NoError(t, err) + cfg.Submodules[path] = &config.Submodule{ + Name: path, + Path: path, + URL: url, + } + err = repo.SetConfig(cfg) + require.NoError(t, err) + + // Create the gitlink entry (mode 160000 commit reference) + // We need to add it directly to the index + idx, err := repo.Storer.Index() + require.NoError(t, err) + + // Add a gitlink entry - this is a special index entry with mode 160000 + idx.Entries = append(idx.Entries, &index.Entry{ + Mode: filemode.Submodule, + Hash: hash, + Name: path, + }) + err = repo.Storer.SetIndex(idx) + require.NoError(t, err) + + // Commit the changes + _, err = tree.Commit("add submodule", &git.CommitOptions{ + Author: &object.Signature{ + Name: "Example", + Email: "test@example.com", + When: time.Now(), + }, + }) + require.NoError(t, err) + } +} + // WriteFile writes a file to the filesystem. func WriteFile(t *testing.T, fs billy.Filesystem, path, content string) { t.Helper() From ad64adf45946f9ef440bad781096cd874a03d118 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Wed, 18 Feb 2026 09:35:21 +0000 Subject: [PATCH 24/28] Document SCP-like URL limitation for relative submodules - Add explicit check for SCP-like parent URLs when resolving relative submodules - Return clear error message with link to tracking issue - Document limitation in function doc comment - Add test cases for SCP parent with relative and absolute submodule URLs Addresses review feedback from johnstcn. --- git/git.go | 10 ++++++++++ git/git_test.go | 12 ++++++++++++ 2 files changed, 22 insertions(+) diff --git a/git/git.go b/git/git.go index e23b4604..4344074f 100644 --- a/git/git.go +++ b/git/git.go @@ -499,12 +499,22 @@ func RedactURL(u string) string { } // ResolveSubmoduleURL resolves a potentially relative submodule URL against a parent repository URL. +// +// Limitation: SCP-like URLs (e.g., git@github.com:org/repo.git) are not supported as parent URLs +// when the submodule uses a relative path. This is a known limitation. +// See: https://github.com/coder/envbuilder/issues/487 func ResolveSubmoduleURL(parentURL, submoduleURL string) (string, error) { // If the submodule URL is absolute (contains ://) or doesn't start with ./ or ../, return it as-is if strings.Contains(submoduleURL, "://") || (!strings.HasPrefix(submoduleURL, "../") && !strings.HasPrefix(submoduleURL, "./")) { return submoduleURL, nil } + // Check if parent URL is SCP-like (e.g., git@github.com:org/repo.git) + // These cannot be properly parsed by net/url and relative submodule resolution is not supported. + if scpLikeURLRegex.MatchString(parentURL) { + return "", fmt.Errorf("relative submodule URL %q cannot be resolved: parent URL %q uses SCP-like syntax which is not supported for relative submodule resolution (see https://github.com/coder/envbuilder/issues/487)", submoduleURL, RedactURL(parentURL)) + } + // Parse the parent URL parentParsed, err := url.Parse(parentURL) if err != nil { diff --git a/git/git_test.go b/git/git_test.go index 45f482ed..c3a0b69f 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -812,6 +812,18 @@ func TestResolveSubmoduleURL(t *testing.T) { subURL: "./child", expectErr: "parse parent URL", }, + { + name: "scpParentWithRelativeSubmodule", + parentURL: "git@github.com:org/main.git", + subURL: "../other/submodule.git", + expectErr: "SCP-like syntax which is not supported", + }, + { + name: "scpParentWithAbsoluteSubmodule", + parentURL: "git@github.com:org/main.git", + subURL: "https://github.com/other/submodule.git", + expect: "https://github.com/other/submodule.git", + }, } for _, tc := range cases { From 334ade4f97bac5f25f273c7665544a3e79fe4be6 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Wed, 18 Feb 2026 09:39:41 +0000 Subject: [PATCH 25/28] Update issue link to #492 --- git/git.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git/git.go b/git/git.go index 4344074f..cf09fb3b 100644 --- a/git/git.go +++ b/git/git.go @@ -502,7 +502,7 @@ func RedactURL(u string) string { // // Limitation: SCP-like URLs (e.g., git@github.com:org/repo.git) are not supported as parent URLs // when the submodule uses a relative path. This is a known limitation. -// See: https://github.com/coder/envbuilder/issues/487 +// See: https://github.com/coder/envbuilder/issues/492 func ResolveSubmoduleURL(parentURL, submoduleURL string) (string, error) { // If the submodule URL is absolute (contains ://) or doesn't start with ./ or ../, return it as-is if strings.Contains(submoduleURL, "://") || (!strings.HasPrefix(submoduleURL, "../") && !strings.HasPrefix(submoduleURL, "./")) { @@ -512,7 +512,7 @@ func ResolveSubmoduleURL(parentURL, submoduleURL string) (string, error) { // Check if parent URL is SCP-like (e.g., git@github.com:org/repo.git) // These cannot be properly parsed by net/url and relative submodule resolution is not supported. if scpLikeURLRegex.MatchString(parentURL) { - return "", fmt.Errorf("relative submodule URL %q cannot be resolved: parent URL %q uses SCP-like syntax which is not supported for relative submodule resolution (see https://github.com/coder/envbuilder/issues/487)", submoduleURL, RedactURL(parentURL)) + return "", fmt.Errorf("relative submodule URL %q cannot be resolved: parent URL %q uses SCP-like syntax which is not supported for relative submodule resolution (see https://github.com/coder/envbuilder/issues/492)", submoduleURL, RedactURL(parentURL)) } // Parse the parent URL From e371318d4e4547a6a1e95d0e935376b06bf5d69a Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Wed, 18 Feb 2026 09:46:33 +0000 Subject: [PATCH 26/28] Fix RedactURL: avoid URL-encoding of *** characters --- git/git.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/git/git.go b/git/git.go index cf09fb3b..3d7ed399 100644 --- a/git/git.go +++ b/git/git.go @@ -479,7 +479,15 @@ func RedactURL(u string) string { // Successfully parsed as a URL with a scheme // Redact userinfo if present (handles user, user:pass, token, URL-encoded creds) if parsed.User != nil { - parsed.User = url.User("***") + // Build URL manually to avoid url.User encoding *** as %2A%2A%2A + result := parsed.Scheme + "://***@" + parsed.Host + parsed.Path + if parsed.RawQuery != "" { + result += "?" + parsed.RawQuery + } + if parsed.Fragment != "" { + result += "#" + parsed.Fragment + } + return result } return parsed.String() } From afcce7f17da5186bb16b564d8773aa0887e33e99 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Wed, 18 Feb 2026 10:00:05 +0000 Subject: [PATCH 27/28] Fix RedactURL: check SCP-like URLs before url.Parse URLs like oauth2:token@host:path were being misinterpreted as having scheme 'oauth2'. Check SCP regex first, and also require parsed.Host to be non-empty for standard URL handling. --- git/git.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/git/git.go b/git/git.go index 3d7ed399..f3003d60 100644 --- a/git/git.go +++ b/git/git.go @@ -473,10 +473,21 @@ func SameHost(url1, url2 string) bool { // - Various schemes: http, https, ssh, git, ftp, sftp // - IPv6 hosts: https://user@[2001:db8::1]/path func RedactURL(u string) string { - // First, try to parse as a standard URL (handles most schemes) + // Handle SCP-like URLs first: user@host:path (no scheme) + // Must check this BEFORE url.Parse because urls like "oauth2:token@host:path" + // get misinterpreted as having scheme "oauth2". + // This catches: git@github.com:org/repo, deploy@host:repo, oauth2:token@gitlab.com:org/repo + if matches := scpLikeURLRegex.FindStringSubmatch(u); matches != nil { + // matches[1] = user part (could be git, deploy, oauth2:token, etc.) + // matches[2] = host + // matches[3] = path + return "***@" + matches[2] + ":" + matches[3] + } + + // Try to parse as a standard URL (handles most schemes) parsed, err := url.Parse(u) - if err == nil && parsed.Scheme != "" { - // Successfully parsed as a URL with a scheme + if err == nil && parsed.Scheme != "" && parsed.Host != "" { + // Successfully parsed as a URL with a scheme and host // Redact userinfo if present (handles user, user:pass, token, URL-encoded creds) if parsed.User != nil { // Build URL manually to avoid url.User encoding *** as %2A%2A%2A @@ -492,15 +503,6 @@ func RedactURL(u string) string { return parsed.String() } - // Handle SCP-like URLs: user@host:path (no scheme) - // This catches: git@github.com:org/repo, deploy@host:repo, user@10.0.0.5:project - if matches := scpLikeURLRegex.FindStringSubmatch(u); matches != nil { - // matches[1] = user part (could be git, deploy, token, etc.) - // matches[2] = host - // matches[3] = path - return "***@" + matches[2] + ":" + matches[3] - } - // If we can't parse it and it's not SCP-like, return as-is // (probably not a URL with credentials) return u From b82aee0d3df614622e957e6af664028003ecee40 Mon Sep 17 00:00:00 2001 From: Bjorn Robertsson Date: Wed, 18 Feb 2026 10:07:24 +0000 Subject: [PATCH 28/28] Fix RedactURL: handle IPv6 URLs correctly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move standard URL parsing back before SCP check, but require both scheme AND host to be non-empty. This correctly handles: - oauth2:token@host:path (scheme=oauth2, host=empty → SCP fallback) - https://user@[2001:db8::1]/path (scheme=https, host=[2001:db8::1] → standard URL) --- git/git.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/git/git.go b/git/git.go index f3003d60..65edbcfe 100644 --- a/git/git.go +++ b/git/git.go @@ -473,18 +473,7 @@ func SameHost(url1, url2 string) bool { // - Various schemes: http, https, ssh, git, ftp, sftp // - IPv6 hosts: https://user@[2001:db8::1]/path func RedactURL(u string) string { - // Handle SCP-like URLs first: user@host:path (no scheme) - // Must check this BEFORE url.Parse because urls like "oauth2:token@host:path" - // get misinterpreted as having scheme "oauth2". - // This catches: git@github.com:org/repo, deploy@host:repo, oauth2:token@gitlab.com:org/repo - if matches := scpLikeURLRegex.FindStringSubmatch(u); matches != nil { - // matches[1] = user part (could be git, deploy, oauth2:token, etc.) - // matches[2] = host - // matches[3] = path - return "***@" + matches[2] + ":" + matches[3] - } - - // Try to parse as a standard URL (handles most schemes) + // Try to parse as a standard URL first (handles schemes like https://, ssh://, etc.) parsed, err := url.Parse(u) if err == nil && parsed.Scheme != "" && parsed.Host != "" { // Successfully parsed as a URL with a scheme and host @@ -503,6 +492,17 @@ func RedactURL(u string) string { return parsed.String() } + // Handle SCP-like URLs: user@host:path (no scheme) + // Only check this if url.Parse didn't find a valid scheme+host + // (to avoid matching URLs like https://user@[ipv6]:path) + // This catches: git@github.com:org/repo, deploy@host:repo, oauth2:token@gitlab.com:org/repo + if matches := scpLikeURLRegex.FindStringSubmatch(u); matches != nil { + // matches[1] = user part (could be git, deploy, oauth2:token, etc.) + // matches[2] = host + // matches[3] = path + return "***@" + matches[2] + ":" + matches[3] + } + // If we can't parse it and it's not SCP-like, return as-is // (probably not a URL with credentials) return u