From 0eaddd66939074bf9a7d234ffeb5a110a776c5d6 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Fri, 20 Mar 2026 09:28:31 +0100 Subject: [PATCH 1/8] Consolidate gogs test helpers into shared utils package Move the gogs container setup, URL helpers, and SSH key generation out of gitcloner/clone_test.go and gitjob/git/git_test.go into a shared integrationtests/utils/gogs.go package. The helpers are used by both test suites; centralising them removes the duplication and makes them available for the bundlereader integration tests added in the next commit. --- integrationtests/gitcloner/clone_test.go | 224 ++--------------------- integrationtests/gitjob/git/git_test.go | 33 +--- integrationtests/utils/gogs.go | 219 ++++++++++++++++++++++ 3 files changed, 238 insertions(+), 238 deletions(-) create mode 100644 integrationtests/utils/gogs.go diff --git a/integrationtests/gitcloner/clone_test.go b/integrationtests/gitcloner/clone_test.go index a9c1246c22..b39132577f 100644 --- a/integrationtests/gitcloner/clone_test.go +++ b/integrationtests/gitcloner/clone_test.go @@ -2,22 +2,14 @@ package apply import ( "context" - "crypto/rand" - "crypto/rsa" - "crypto/tls" - "crypto/x509" - "encoding/pem" "fmt" "io" - "net/http" "os" - "path/filepath" "strings" "time" "github.com/rancher/fleet/internal/cmd/cli/gitcloner" - dockercontainer "github.com/docker/docker/api/types/container" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/config" "github.com/go-git/go-git/v5/plumbing/object" @@ -25,8 +17,6 @@ import ( "github.com/gogits/go-gogs-client" cp "github.com/otiai10/copy" "github.com/testcontainers/testcontainers-go" - "github.com/testcontainers/testcontainers-go/wait" - "golang.org/x/crypto/ssh" "golang.org/x/crypto/ssh/knownhosts" . "github.com/onsi/ginkgo/v2" @@ -45,13 +35,7 @@ and uses data and conf from assets/gitserver. These files are mounted into the g It contains an already created user whose credentials are provided as constants. */ const ( - gogsUser = "test" - gogsPass = "pass" - gogsHTTPSPort = "3000" - gogsSSHPort = "22" - testRepoName = "test-repo" - timeout = 240 * time.Second - interval = 10 * time.Second + testRepoName = "test-repo" ) var ( @@ -76,7 +60,7 @@ var _ = Describe("Applying a git job gets content from git repo", Label("network BeforeAll(func() { var err error - container, err = createGogsContainerWithHTTPS() + container, gogsCABundle, gogsClient, err = utils.CreateGogsContainerWithHTTPS(context.Background(), "assets/gitserver") Expect(err).NotTo(HaveOccurred()) DeferCleanup(func() { if container != nil { @@ -87,12 +71,12 @@ var _ = Describe("Applying a git job gets content from git repo", Label("network When("Cloning an https repo", func() { JustBeforeEach(func() { - url, err := getHTTPSURL(context.Background(), container) + url, err := utils.GetGogsHTTPSURL(context.Background(), container) Expect(err).NotTo(HaveOccurred()) repoName, initialCommit, err = createRepo(url, "assets/repo", private) Expect(err).NotTo(HaveOccurred()) opts.Repo = url + "/test/" + repoName - tmp = createTempFolder() + tmp = utils.CreateTempFolder("gogs") opts.Path = tmp }) @@ -167,7 +151,7 @@ var _ = Describe("Applying a git job gets content from git repo", Label("network private = true opts = &gitcloner.GitCloner{ InsecureSkipTLS: true, - Username: gogsUser, + Username: utils.GogsUser, PasswordFile: "assets/gogs/password", } }) @@ -238,21 +222,21 @@ var _ = Describe("Applying a git job gets content from git repo", Label("network var tmpKey string JustBeforeEach(func() { - url, err := getHTTPSURL(context.Background(), container) + url, err := utils.GetGogsHTTPSURL(context.Background(), container) Expect(err).NotTo(HaveOccurred()) repoName, _, err = createRepo(url, "assets/repo", private) Expect(err).NotTo(HaveOccurred()) - sshURL, err := getSSHURL(context.Background(), container) + sshURL, err := utils.GetGogsSSHURL(context.Background(), container) Expect(err).NotTo(HaveOccurred()) opts.Repo = sshURL + "/test/" + repoName - tmp = createTempFolder() + tmp = utils.CreateTempFolder("gogs") opts.Path = tmp // create private key, and register public key in gogs key, err := createAndAddKeys() Expect(err).NotTo(HaveOccurred()) tmpFile, err := os.CreateTemp("", "testkey") Expect(err).NotTo(HaveOccurred()) - _, err = tmpFile.Write([]byte(key)) + _, err = tmpFile.WriteString(key) Expect(err).NotTo(HaveOccurred()) tmpKey = tmpFile.Name() opts.SSHPrivateKeyFile = tmpKey @@ -349,121 +333,6 @@ var _ = Describe("Applying a git job gets content from git repo", Label("network }) }) -// createGogsContainerWithHTTPS creates a container that runs gogs. It creates a certificate for tls, and stores the ca bundle in gogsCABundle -func createGogsContainerWithHTTPS() (testcontainers.Container, error) { - tmpDir := createTempFolder() - err := cp.Copy("assets/gitserver", tmpDir) - if err != nil { - return nil, err - } - // The .ssh directory may end up group-writable depending on the system - // umask, which causes OpenSSH StrictModes to reject authorized_keys. - err = os.Chmod(filepath.Join(tmpDir, "git"), 0700) - if err != nil { - return nil, fmt.Errorf("failed to change permissions for .ssh directory: %w", err) - } - err = os.Chmod(filepath.Join(tmpDir, "git", ".ssh"), 0700) - if err != nil { - return nil, fmt.Errorf("failed to change permissions for .ssh directory: %w", err) - } - req := testcontainers.ContainerRequest{ - Image: "gogs/gogs:0.13", - ExposedPorts: []string{gogsHTTPSPort + "/tcp", gogsSSHPort + "/tcp"}, - WaitingFor: wait.ForListeningPort("22/tcp").WithStartupTimeout(timeout), - HostConfigModifier: func(hostConfig *dockercontainer.HostConfig) { - // Use Binds instead of Mounts to support SELinux relabeling with :z flag - // The :z flag allows the bind mount to be shared between containers and - // automatically relabels the content for SELinux - hostConfig.Binds = []string{tmpDir + ":/data:z"} - }, - } - container, err := testcontainers.GenericContainer(context.Background(), testcontainers.GenericContainerRequest{ - ContainerRequest: req, - Started: true, - }) - if err != nil { - return nil, err - } - - // create ca bundle and certs needed for https - _, _, err = container.Exec(context.Background(), []string{"./gogs", "cert", "-ca=true", "-duration=8760h0m0s", "-host=localhost"}) - if err != nil { - return container, err - } - _, _, err = container.Exec(context.Background(), []string{"chown", "git:git", "cert.pem", "key.pem"}) - if err != nil { - return container, err - } - caReader, err := container.CopyFileFromContainer(context.Background(), "/app/gogs/cert.pem") - if err != nil { - return container, err - } - gogsCABundle, err = io.ReadAll(caReader) - if err != nil { - return container, err - } - - // restart gogs container to make sure https certs are picked - err = container.Stop(context.Background(), &[]time.Duration{timeout}[0]) - if err != nil { - return container, err - } - err = container.Start(context.Background()) - if err != nil { - return container, err - } - - url, err := getHTTPSURL(context.Background(), container) - if err != nil { - return container, err - } - - // create access token, we need to wait until the https server is available. We can't check this in testcontainers.WaitFor - // because we need to create the certificates first. - Eventually(func() error { - c := gogs.NewClient(url, "") - - conf := &tls.Config{InsecureSkipVerify: true} - - // only continue if it's a TLS connection - addr := strings.Replace(url, "https://", "", 1) - dialer := &tls.Dialer{Config: conf} - if _, err := dialer.DialContext(context.Background(), "tcp", addr); err != nil { - GinkgoWriter.Printf("error dialing: %v", err) - orgErr := err - - // debug the connection - dialer := &tls.Dialer{Config: nil} - conn, err := dialer.DialContext(context.Background(), "tcp", addr) - if err != nil { - GinkgoWriter.Printf("error dialing without tls: %v", err) - return orgErr - } - body, _ := io.ReadAll(conn) - GinkgoWriter.Printf("tcp connection response: %s", body) - - return orgErr - } - - tr := &http.Transport{TLSClientConfig: conf} - httpClient := &http.Client{Transport: tr} // #nosec G402 - c.SetHTTPClient(httpClient) - token, err := c.CreateAccessToken(gogsUser, gogsPass, gogs.CreateAccessTokenOption{ - Name: "test", - }) - if err != nil { - GinkgoWriter.Printf("error creating access token: %v", err) - return err - } - gogsClient = gogs.NewClient(url, token.Sha1) - gogsClient.SetHTTPClient(httpClient) - - return nil - }, timeout, interval).ShouldNot(HaveOccurred()) - - return container, nil -} - // createRepo creates a git repo for testing // returns the initial commit and a unique repo name. func createRepo(url string, path string, private bool) (string, string, error) { @@ -476,7 +345,7 @@ func createRepo(url string, path string, private bool) (string, string, error) { if err != nil { return "", "", err } - repoURL := url + "/" + gogsUser + "/" + repoName + repoURL := url + "/" + utils.GogsUser + "/" + repoName // add initial commit tmp, err := os.MkdirTemp("", repoName) @@ -531,8 +400,8 @@ func createRepo(url string, path string, private bool) (string, string, error) { RemoteName: "upstream", RemoteURL: repoURL, Auth: &httpgit.BasicAuth{ - Username: gogsUser, - Password: gogsPass, + Username: utils.GogsUser, + Password: utils.GogsPass, }, InsecureSkipTLS: true, }) @@ -543,50 +412,10 @@ func createRepo(url string, path string, private bool) (string, string, error) { return repoName, commit.String(), nil } -func getHTTPSURL(ctx context.Context, container testcontainers.Container) (string, error) { - mappedPort, err := container.MappedPort(ctx, gogsHTTPSPort) - if err != nil { - return "", err - } - host, err := container.Host(ctx) - if err != nil { - return "", err - } - url := "https://" + host + ":" + mappedPort.Port() - - return url, nil -} - -func getSSHURL(ctx context.Context, container testcontainers.Container) (string, error) { - mappedPort, err := container.MappedPort(ctx, gogsSSHPort) - if err != nil { - return "", err - } - host, err := container.Host(ctx) - if err != nil { - return "", err - } - url := "ssh://git@" + host + ":" + mappedPort.Port() - - return url, nil -} - -// createTempFolder uses testing tempDir if running in local, which will cleanup the files at the end of the tests. -// cleanup fails in github actions, that's why we use os.MkdirTemp instead. Container will be removed at the end in -// github actions, so no resources are left orphaned. -func createTempFolder() string { - if os.Getenv("GITHUB_ACTIONS") == "true" { - tmp, err := os.MkdirTemp("", "gogs") - Expect(err).ToNot(HaveOccurred()) - return tmp - } - - return GinkgoT().TempDir() -} - -// createAndAddKeys creates a public private key pair. It adds the public key to gogs, and returns the private key. +// createAndAddKeys creates a public/private key pair, registers the public key in gogs, +// and returns the private key in PEM format. func createAndAddKeys() (string, error) { - publicKey, privateKey, err := makeSSHKeyPair() + publicKey, privateKey, err := utils.MakeSSHKeyPair() if err != nil { return "", err } @@ -603,7 +432,7 @@ func createAndAddKeys() (string, error) { } func getKnownHostEntry(container testcontainers.Container) (string, error) { - mappedPort, err := container.MappedPort(context.Background(), gogsSSHPort) + mappedPort, err := container.MappedPort(context.Background(), utils.GogsSSHPort) if err != nil { return "", err } @@ -623,24 +452,3 @@ func getKnownHostEntry(container testcontainers.Container) (string, error) { return fmt.Sprintf("[localhost]:%s %s %s", port, algorithm, hostKey), nil } - -func makeSSHKeyPair() (string, string, error) { - privateKey, err := rsa.GenerateKey(rand.Reader, 4096) - if err != nil { - return "", "", err - } - - var privKeyBuf strings.Builder - privateKeyPEM := &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(privateKey)} - if err := pem.Encode(&privKeyBuf, privateKeyPEM); err != nil { - return "", "", err - } - pub, err := ssh.NewPublicKey(&privateKey.PublicKey) - if err != nil { - return "", "", err - } - var pubKeyBuf strings.Builder - pubKeyBuf.Write(ssh.MarshalAuthorizedKey(pub)) - - return pubKeyBuf.String(), privKeyBuf.String(), nil -} diff --git a/integrationtests/gitjob/git/git_test.go b/integrationtests/gitjob/git/git_test.go index c2ed06300b..1288158507 100644 --- a/integrationtests/gitjob/git/git_test.go +++ b/integrationtests/gitjob/git/git_test.go @@ -2,16 +2,11 @@ package integrationtests import ( "context" - "crypto/rand" - "crypto/rsa" - "crypto/x509" - "encoding/pem" "errors" "fmt" "io" "os" "path/filepath" - "strings" "testing" "time" @@ -28,7 +23,6 @@ import ( "github.com/testcontainers/testcontainers-go" "github.com/testcontainers/testcontainers-go/wait" "go.uber.org/mock/gomock" - "golang.org/x/crypto/ssh" v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -469,14 +463,14 @@ func createGogsContainer(ctx context.Context, tmpDir string) (testcontainers.Con return nil, "", err } if latestCommitPublicRepo == "" { - return nil, "", fmt.Errorf("latestCommitPublicRepo is empty after initRepo") + return nil, "", errors.New("latestCommitPublicRepo is empty after initRepo") } latestCommitPrivateRepo, err = initRepo(url, "private-repo", true) if err != nil { return nil, "", err } if latestCommitPrivateRepo == "" { - return nil, "", fmt.Errorf("latestCommitPrivateRepo is empty after initRepo") + return nil, "", errors.New("latestCommitPrivateRepo is empty after initRepo") } return container, url, nil @@ -669,7 +663,7 @@ func getURL(ctx context.Context, container testcontainers.Container) (string, er // createAndAddKeys creates a public private key pair. It adds the public key to gogs, and returns the private key. func createAndAddKeys() (string, error) { - publicKey, privateKey, err := makeSSHKeyPair() + publicKey, privateKey, err := utils.MakeSSHKeyPair() if err != nil { return "", err } @@ -685,27 +679,6 @@ func createAndAddKeys() (string, error) { return privateKey, nil } -func makeSSHKeyPair() (string, string, error) { - privateKey, err := rsa.GenerateKey(rand.Reader, 4096) - if err != nil { - return "", "", err - } - - var privKeyBuf strings.Builder - privateKeyPEM := &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(privateKey)} - if err := pem.Encode(&privKeyBuf, privateKeyPEM); err != nil { - return "", "", err - } - pub, err := ssh.NewPublicKey(&privateKey.PublicKey) - if err != nil { - return "", "", err - } - var pubKeyBuf strings.Builder - pubKeyBuf.Write(ssh.MarshalAuthorizedKey(pub)) - - return pubKeyBuf.String(), privKeyBuf.String(), nil -} - type mockKnownHostsGetter struct { isStrict bool knownHosts string diff --git a/integrationtests/utils/gogs.go b/integrationtests/utils/gogs.go new file mode 100644 index 0000000000..c89bf09fe5 --- /dev/null +++ b/integrationtests/utils/gogs.go @@ -0,0 +1,219 @@ +package utils + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "encoding/pem" + "fmt" + "io" + "net" + "net/http" + "os" + "path/filepath" + "strings" + "time" + + dockercontainer "github.com/docker/docker/api/types/container" + gogs "github.com/gogits/go-gogs-client" + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + cp "github.com/otiai10/copy" + "github.com/testcontainers/testcontainers-go" + "github.com/testcontainers/testcontainers-go/wait" + "golang.org/x/crypto/ssh" +) + +const ( + GogsUser = "test" + GogsPass = "pass" + GogsHTTPSPort = "3000" + GogsSSHPort = "22" + + gogsContainerTimeout = 240 * time.Second + gogsContainerInterval = 10 * time.Second +) + +// MakeSSHKeyPair generates an RSA-4096 key pair for git SSH authentication. +// Returns the public key in authorized_keys format and the private key in PEM format. +func MakeSSHKeyPair() (publicKey, privateKey string, _ error) { + priv, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + return "", "", err + } + var privBuf strings.Builder + if err := pem.Encode(&privBuf, &pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(priv), + }); err != nil { + return "", "", err + } + pub, err := ssh.NewPublicKey(&priv.PublicKey) + if err != nil { + return "", "", err + } + return string(ssh.MarshalAuthorizedKey(pub)), privBuf.String(), nil +} + +// CreateTempFolder returns a temporary directory for test data. +// On GitHub Actions, os.MkdirTemp is used to avoid cleanup failures; +// locally GinkgoT().TempDir() is used so Ginkgo cleans up automatically. +func CreateTempFolder(prefix string) string { + if os.Getenv("GITHUB_ACTIONS") == "true" { + tmp, err := os.MkdirTemp("", prefix) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + return tmp + } + return ginkgo.GinkgoT().TempDir() +} + +// GetGogsHTTPSURL returns the mapped HTTPS base URL for a running gogs container. +func GetGogsHTTPSURL(ctx context.Context, container testcontainers.Container) (string, error) { + port, err := container.MappedPort(ctx, GogsHTTPSPort) + if err != nil { + return "", err + } + host, err := container.Host(ctx) + if err != nil { + return "", err + } + return "https://" + host + ":" + port.Port(), nil +} + +// GetGogsSSHURL returns the mapped SSH base URL for a running gogs container. +func GetGogsSSHURL(ctx context.Context, container testcontainers.Container) (string, error) { + port, err := container.MappedPort(ctx, GogsSSHPort) + if err != nil { + return "", err + } + host, err := container.Host(ctx) + if err != nil { + return "", err + } + return "ssh://git@" + host + ":" + port.Port(), nil +} + +// CreateGogsContainerWithHTTPS starts a gogs container backed by assetPath, generates a +// TLS certificate, and waits until the HTTPS endpoint accepts connections. +// It returns the container, the CA bundle PEM bytes, and an authenticated gogs client. +// assetPath must be the gitserver directory to bind-mount into the container at /data. +func CreateGogsContainerWithHTTPS(ctx context.Context, assetPath string) (testcontainers.Container, []byte, *gogs.Client, error) { + tmpDir := CreateTempFolder("gogs") + if err := cp.Copy(assetPath, tmpDir); err != nil { + return nil, nil, nil, err + } + for _, dir := range []string{"git", filepath.Join("git", ".ssh")} { + if err := os.Chmod(filepath.Join(tmpDir, dir), 0700); err != nil { + return nil, nil, nil, fmt.Errorf("chmod %s: %w", dir, err) + } + } + + req := testcontainers.ContainerRequest{ + Image: "gogs/gogs:0.13", + ExposedPorts: []string{GogsHTTPSPort + "/tcp", GogsSSHPort + "/tcp"}, + WaitingFor: wait.ForListeningPort("22/tcp").WithStartupTimeout(gogsContainerTimeout), + HostConfigModifier: func(hc *dockercontainer.HostConfig) { + hc.Binds = []string{tmpDir + ":/data:z"} + }, + } + container, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{ + ContainerRequest: req, + Started: true, + }) + if err != nil { + return nil, nil, nil, err + } + + if _, _, err = container.Exec(ctx, []string{"./gogs", "cert", "-ca=true", "-duration=8760h0m0s", "-host=localhost"}); err != nil { + return container, nil, nil, err + } + if _, _, err = container.Exec(ctx, []string{"chown", "git:git", "cert.pem", "key.pem"}); err != nil { + return container, nil, nil, err + } + caReader, err := container.CopyFileFromContainer(ctx, "/app/gogs/cert.pem") + if err != nil { + return container, nil, nil, err + } + defer caReader.Close() + caBundle, err := io.ReadAll(caReader) + if err != nil { + return container, nil, nil, err + } + + stopTimeout := gogsContainerTimeout + if err = container.Stop(ctx, &stopTimeout); err != nil { + return container, nil, nil, err + } + if err = container.Start(ctx); err != nil { + return container, nil, nil, err + } + + httpsURL, err := GetGogsHTTPSURL(ctx, container) + if err != nil { + return container, caBundle, nil, err + } + + var client *gogs.Client + gomega.Eventually(func() error { + c := gogs.NewClient(httpsURL, "") + conf := &tls.Config{InsecureSkipVerify: true} + addr := strings.TrimPrefix(httpsURL, "https://") + if _, err := (&tls.Dialer{Config: conf}).DialContext(ctx, "tcp", addr); err != nil { + ginkgo.GinkgoWriter.Printf("error dialing %s: %v\n", addr, err) + // debug: try plain TCP to distinguish TLS negotiation from connectivity failures + conn, tcpErr := (&net.Dialer{}).DialContext(ctx, "tcp", addr) + if tcpErr != nil { + ginkgo.GinkgoWriter.Printf("error dialing without TLS: %v\n", tcpErr) + } else { + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(5 * time.Second)) + buf := make([]byte, 512) + n, _ := conn.Read(buf) + ginkgo.GinkgoWriter.Printf("plain TCP response: %s\n", buf[:n]) + } + return err + } + tr := &http.Transport{TLSClientConfig: conf} + httpClient := &http.Client{Transport: tr} + c.SetHTTPClient(httpClient) + token, tokenErr := c.CreateAccessToken(GogsUser, GogsPass, gogs.CreateAccessTokenOption{Name: "test"}) + if tokenErr != nil { + ginkgo.GinkgoWriter.Printf("error creating access token: %v\n", tokenErr) + return tokenErr + } + client = gogs.NewClient(httpsURL, token.Sha1) + client.SetHTTPClient(httpClient) + return nil + }, gogsContainerTimeout, gogsContainerInterval).ShouldNot(gomega.HaveOccurred()) + + return container, caBundle, client, nil +} + +// GetGogsKnownHostEntry reads the container's SSH ECDSA host public key and returns +// a known_hosts format line suitable for use with CreateKnownHostsCallBack. +func GetGogsKnownHostEntry(ctx context.Context, container testcontainers.Container) (string, error) { + port, err := container.MappedPort(ctx, GogsSSHPort) + if err != nil { + return "", err + } + host, err := container.Host(ctx) + if err != nil { + return "", err + } + pubKeyReader, err := container.CopyFileFromContainer(ctx, "/data/ssh/ssh_host_ecdsa_key.pub") + if err != nil { + return "", err + } + defer pubKeyReader.Close() + pubKeyBytes, err := io.ReadAll(pubKeyReader) + if err != nil { + return "", err + } + fields := strings.Fields(string(pubKeyBytes)) + if len(fields) < 2 { + return "", fmt.Errorf("unexpected known_hosts key format: %q", string(pubKeyBytes)) + } + return fmt.Sprintf("[%s]:%s %s %s", host, port.Port(), fields[0], fields[1]), nil +} From c696ec1e4729882bcd293ba306dd5fd25af23277 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Fri, 20 Mar 2026 09:34:03 +0100 Subject: [PATCH 2/8] Add bundlereader integration tests for git content fetching Add integration tests exercising GetContent for a range of scenarios that were supported before the go-getter removal: HTTPS with InsecureSkipVerify, no-TLS certificate rejection, private repos with embedded credentials, branch selection via ?ref=, subdir extraction via //, and plain SSH with a private key. --- .../bundlereader/getcontent_test.go | 372 ++++++++++++++++++ integrationtests/bundlereader/suite_test.go | 13 + 2 files changed, 385 insertions(+) create mode 100644 integrationtests/bundlereader/getcontent_test.go create mode 100644 integrationtests/bundlereader/suite_test.go diff --git a/integrationtests/bundlereader/getcontent_test.go b/integrationtests/bundlereader/getcontent_test.go new file mode 100644 index 0000000000..d3207338cb --- /dev/null +++ b/integrationtests/bundlereader/getcontent_test.go @@ -0,0 +1,372 @@ +package bundlereader_test + +import ( + "context" + "fmt" + "net/url" + "os" + "path/filepath" + "time" + + gogit "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/config" + "github.com/go-git/go-git/v5/plumbing/object" + httpgit "github.com/go-git/go-git/v5/plumbing/transport/http" + gogs "github.com/gogits/go-gogs-client" + cp "github.com/otiai10/copy" + "github.com/testcontainers/testcontainers-go" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/rancher/fleet/integrationtests/utils" + "github.com/rancher/fleet/internal/bundlereader" +) + +func init() { + utils.DisableReaper() +} + +const ( + testRepoName = "test-repo" + timeout = 240 * time.Second + interval = 10 * time.Second +) + +var ( + gogsClient *gogs.Client + gogsCABundle []byte +) + +var _ = Describe("GetContent fetches files from a git repository", Label("networking"), Ordered, func() { + var container testcontainers.Container + + BeforeAll(func() { + // Suppress interactive git prompts when running against the go-getter + // implementation. + // go-git never reads these env vars, so the later go-git based + // implementation should be unaffected by those. + DeferCleanup(os.Unsetenv, "GIT_TERMINAL_PROMPT") + DeferCleanup(os.Unsetenv, "GIT_SSH_COMMAND") + os.Setenv("GIT_TERMINAL_PROMPT", "0") + os.Setenv("GIT_SSH_COMMAND", "ssh -o StrictHostKeyChecking=accept-new") + + var err error + container, gogsCABundle, gogsClient, err = utils.CreateGogsContainerWithHTTPS(context.Background(), "../gitcloner/assets/gitserver") + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(func() { + if container != nil { + _ = container.Terminate(context.Background()) + } + }) + }) + + When("fetching from an https git repo", func() { + var ( + repoName string + httpsBase string + ) + + JustBeforeEach(func() { + var err error + httpsBase, err = utils.GetGogsHTTPSURL(context.Background(), container) + Expect(err).NotTo(HaveOccurred()) + repoName, _, err = createRepo(httpsBase, false) + Expect(err).NotTo(HaveOccurred()) + }) + + JustAfterEach(func() { + Expect(gogsClient.DeleteRepo(utils.GogsUser, repoName)).To(Succeed()) + }) + + When("insecureSkipVerify is true", func() { + It("returns the repository files", func() { + source := fmt.Sprintf("git::%s/%s/%s", httpsBase, utils.GogsUser, repoName) + files, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{InsecureSkipVerify: true}, false, nil, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(files).To(HaveKey("README.md")) + Expect(files).To(HaveKey(filepath.Join("subdir", "config.yaml"))) + }) + }) + + When("no TLS configuration is provided", func() { + It("fails with a certificate error", func() { + source := fmt.Sprintf("git::%s/%s/%s", httpsBase, utils.GogsUser, repoName) + _, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{}, false, nil, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("certificate")) + }) + }) + + When("the repo is private and credentials are embedded in the URL", func() { + var privateRepoName string + + JustBeforeEach(func() { + var err error + privateRepoName, _, err = createPrivateRepo(httpsBase) + Expect(err).NotTo(HaveOccurred()) + }) + + JustAfterEach(func() { + Expect(gogsClient.DeleteRepo(utils.GogsUser, privateRepoName)).To(Succeed()) + }) + + It("returns the repository files when credentials are embedded in URL", func() { + u, err := url.Parse(fmt.Sprintf("%s/%s/%s", httpsBase, utils.GogsUser, privateRepoName)) + Expect(err).NotTo(HaveOccurred()) + u.User = url.UserPassword(utils.GogsUser, utils.GogsPass) + source := "git::" + u.String() + + files, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{InsecureSkipVerify: true}, false, nil, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(files).To(HaveKey("README.md")) + }) + + It("fails when no credentials are provided", func() { + source := fmt.Sprintf("git::%s/%s/%s", httpsBase, utils.GogsUser, privateRepoName) + _, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{InsecureSkipVerify: true}, false, nil, + ) + Expect(err).To(HaveOccurred()) + }) + }) + + When("fetching a specific branch via ?ref=", func() { + var branchRepoName string + + JustBeforeEach(func() { + var err error + branchRepoName, _, err = createRepoWithBranch(httpsBase) + Expect(err).NotTo(HaveOccurred()) + }) + + JustAfterEach(func() { + Expect(gogsClient.DeleteRepo(utils.GogsUser, branchRepoName)).To(Succeed()) + }) + + It("returns files from the specified branch", func() { + source := fmt.Sprintf("git::%s/%s/%s?ref=feature", httpsBase, utils.GogsUser, branchRepoName) + files, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{InsecureSkipVerify: true}, false, nil, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(files).To(HaveKey("feature.yaml")) + Expect(files).NotTo(HaveKey("README.md")) + }) + }) + + When("fetching a subdirectory via // path", func() { + It("returns only the files in the subdirectory", func() { + source := fmt.Sprintf("git::%s/%s/%s//subdir", httpsBase, utils.GogsUser, repoName) + files, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{InsecureSkipVerify: true}, false, nil, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(files).To(HaveKey("config.yaml")) + Expect(files).NotTo(HaveKey("README.md")) + }) + }) + }) + + When("fetching from an ssh git repo", Label("ssh"), func() { + var ( + repoName string + httpsBase string + sshBase string + tmpKeyFile string + ) + + JustBeforeEach(func() { + var err error + httpsBase, err = utils.GetGogsHTTPSURL(context.Background(), container) + Expect(err).NotTo(HaveOccurred()) + sshBase, err = utils.GetGogsSSHURL(context.Background(), container) + Expect(err).NotTo(HaveOccurred()) + + repoName, _, err = createPrivateRepo(httpsBase) + Expect(err).NotTo(HaveOccurred()) + + publicKey, privateKey, err := utils.MakeSSHKeyPair() + Expect(err).NotTo(HaveOccurred()) + _, err = gogsClient.CreatePublicKey(gogs.CreateKeyOption{Title: "test", Key: publicKey}) + Expect(err).NotTo(HaveOccurred()) + + tmpKeyFile = filepath.Join(GinkgoT().TempDir(), "id_rsa") + Expect(os.WriteFile(tmpKeyFile, []byte(privateKey), 0600)).To(Succeed()) + }) + + JustAfterEach(func() { + Expect(gogsClient.DeleteRepo(utils.GogsUser, repoName)).To(Succeed()) + keys, err := gogsClient.ListMyPublicKeys() + Expect(err).NotTo(HaveOccurred()) + for _, key := range keys { + _ = gogsClient.DeletePublicKey(key.ID) + } + }) + + It("returns the repository files when an SSH private key is provided", func() { + privateKey, err := os.ReadFile(tmpKeyFile) + Expect(err).NotTo(HaveOccurred()) + + source := fmt.Sprintf("%s/%s/%s", sshBase, utils.GogsUser, repoName) + Eventually(func() error { + _, err = bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{SSHPrivateKey: privateKey}, false, nil, + ) + return err + }, timeout, interval).Should(Succeed()) + + files, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{SSHPrivateKey: privateKey}, false, nil, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(files).To(HaveKey("README.md")) + }) + }) +}) + +// createRepo creates a public or private git repo in gogs, pushes test files, and returns its name and commit SHA. +func createRepo(baseURL string, private bool) (string, string, error) { + repoName := fmt.Sprintf("%s%d", testRepoName, time.Now().UTC().UnixMicro()) + _, err := gogsClient.CreateRepo(gogs.CreateRepoOption{Name: repoName, Private: private}) + if err != nil { + return "", "", err + } + commit, err := pushAssets(baseURL, repoName, "../gitcloner/assets/repo") + return repoName, commit, err +} + +// createPrivateRepo creates a private git repo in gogs. +func createPrivateRepo(baseURL string) (string, string, error) { + return createRepo(baseURL, true) +} + +// createRepoWithBranch creates a repo with a "master" branch (from assets) and an additional "feature" branch. +func createRepoWithBranch(baseURL string) (string, string, error) { + repoName := fmt.Sprintf("%s%d", testRepoName, time.Now().UTC().UnixMicro()) + _, err := gogsClient.CreateRepo(gogs.CreateRepoOption{Name: repoName, Private: false}) + if err != nil { + return "", "", err + } + + _, err = pushAssets(baseURL, repoName, "../gitcloner/assets/repo") + if err != nil { + return "", "", err + } + + repoURL := fmt.Sprintf("%s/%s/%s", baseURL, utils.GogsUser, repoName) + tmp, err := os.MkdirTemp("", repoName) + if err != nil { + return "", "", err + } + defer os.RemoveAll(tmp) + + r, err := gogit.PlainClone(tmp, false, &gogit.CloneOptions{ + URL: repoURL, + InsecureSkipTLS: true, + Auth: &httpgit.BasicAuth{Username: utils.GogsUser, Password: utils.GogsPass}, + }) + if err != nil { + return "", "", fmt.Errorf("clone for branch creation: %w", err) + } + w, err := r.Worktree() + if err != nil { + return "", "", err + } + if err := w.Checkout(&gogit.CheckoutOptions{Branch: "refs/heads/feature", Create: true}); err != nil { + return "", "", fmt.Errorf("checkout feature branch: %w", err) + } + + // Remove master files and add a feature-specific one so the branch is distinct. + _ = os.Remove(filepath.Join(tmp, "README.md")) + if err := os.WriteFile(filepath.Join(tmp, "feature.yaml"), []byte("feature: true\n"), 0600); err != nil { + return "", "", err + } + if _, err := w.Add("."); err != nil { + return "", "", err + } + commit, err := w.Commit("feature commit", &gogit.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com", When: time.Now()}, + }) + if err != nil { + return "", "", err + } + if err := r.Push(&gogit.PushOptions{ + Auth: &httpgit.BasicAuth{Username: utils.GogsUser, Password: utils.GogsPass}, + InsecureSkipTLS: true, + RefSpecs: []config.RefSpec{"refs/heads/feature:refs/heads/feature"}, + }); err != nil { + return "", "", fmt.Errorf("push feature branch: %w", err) + } + + return repoName, commit.String(), nil +} + +// pushAssets initialises a local repo from path, adds all files, and pushes to gogs. +// It also creates a subdir/config.yaml file to enable subdirectory tests. +func pushAssets(baseURL, repoName, path string) (string, error) { + repoURL := fmt.Sprintf("%s/%s/%s", baseURL, utils.GogsUser, repoName) + tmp, err := os.MkdirTemp("", repoName) + if err != nil { + return "", err + } + defer os.RemoveAll(tmp) + + r, err := gogit.PlainInit(tmp, false) + if err != nil { + return "", err + } + w, err := r.Worktree() + if err != nil { + return "", err + } + if err := cp.Copy(path, tmp); err != nil { + return "", err + } + if err := os.MkdirAll(filepath.Join(tmp, "subdir"), 0755); err != nil { + return "", err + } + if err := os.WriteFile(filepath.Join(tmp, "subdir", "config.yaml"), []byte("key: value\n"), 0600); err != nil { + return "", err + } + if _, err := w.Add("."); err != nil { + return "", err + } + commit, err := w.Commit("initial commit", &gogit.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com", When: time.Now()}, + }) + if err != nil { + return "", err + } + cfg, err := r.Config() + if err != nil { + return "", err + } + cfg.Remotes["upstream"] = &config.RemoteConfig{Name: "upstream", URLs: []string{repoURL}} + if err := r.SetConfig(cfg); err != nil { + return "", err + } + if err := r.Push(&gogit.PushOptions{ + RemoteName: "upstream", + RemoteURL: repoURL, + Auth: &httpgit.BasicAuth{Username: utils.GogsUser, Password: utils.GogsPass}, + InsecureSkipTLS: true, + }); err != nil { + return "", err + } + return commit.String(), nil +} diff --git a/integrationtests/bundlereader/suite_test.go b/integrationtests/bundlereader/suite_test.go new file mode 100644 index 0000000000..851abe8ae9 --- /dev/null +++ b/integrationtests/bundlereader/suite_test.go @@ -0,0 +1,13 @@ +package bundlereader_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestBundleReader(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "BundleReader Suite") +} From 36ee83c5a7611acce98d7c26e22575c83d5668f9 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Fri, 20 Mar 2026 09:34:27 +0100 Subject: [PATCH 3/8] Extract SSH public key auth into internal/ssh Centralise SSH private-key loading and public-key auth creation into internal/ssh so both bundlereader and gitcloner share a single implementation. pkg/git/netutils.go gains SSHPublicKeysFromFile for callers in pkg/git that already use a file path directly. Prefer known_hosts from the secret; fall back to the cluster-wide value. Add unit tests for NewSSHPublicKeys. --- internal/ssh/auth.go | 29 ++++++++++++ internal/ssh/auth_test.go | 94 +++++++++++++++++++++++++++++++++++++++ pkg/git/netutils.go | 24 +++------- 3 files changed, 129 insertions(+), 18 deletions(-) create mode 100644 internal/ssh/auth.go create mode 100644 internal/ssh/auth_test.go diff --git a/internal/ssh/auth.go b/internal/ssh/auth.go new file mode 100644 index 0000000000..d167992c3a --- /dev/null +++ b/internal/ssh/auth.go @@ -0,0 +1,29 @@ +package ssh + +import ( + gossh "github.com/go-git/go-git/v5/plumbing/transport/ssh" + golangssh "golang.org/x/crypto/ssh" +) + +// NewSSHPublicKeys creates a go-git SSH auth method from the given PEM key and +// optional known_hosts bytes. If knownHosts is empty, InsecureIgnoreHostKey is +// used as the host key callback, matching Fleet's default behaviour across all +// git cloning code paths. +func NewSSHPublicKeys(user string, keyPEM []byte, knownHosts []byte) (*gossh.PublicKeys, error) { + pubKeys, err := gossh.NewPublicKeys(user, keyPEM, "") + if err != nil { + return nil, err + } + if len(knownHosts) > 0 { + pubKeys.HostKeyCallback, err = CreateKnownHostsCallBack(knownHosts) + if err != nil { + return nil, err + } + } else { + //nolint:gosec // G106: InsecureIgnoreHostKey is the intentional fallback + // when no known_hosts are provided; matches behaviour across gitcloner, + // bundlereader, and pkg/git. + pubKeys.HostKeyCallback = golangssh.InsecureIgnoreHostKey() + } + return pubKeys, nil +} diff --git a/internal/ssh/auth_test.go b/internal/ssh/auth_test.go new file mode 100644 index 0000000000..7e7f69c778 --- /dev/null +++ b/internal/ssh/auth_test.go @@ -0,0 +1,94 @@ +package ssh_test + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "encoding/pem" + "fmt" + "net" + "testing" + + golangssh "golang.org/x/crypto/ssh" + + "github.com/rancher/fleet/internal/ssh" +) + +func generateECPrivateKeyPEM(t *testing.T) []byte { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generating key: %v", err) + } + der, err := x509.MarshalECPrivateKey(key) + if err != nil { + t.Fatalf("marshalling key: %v", err) + } + return pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: der}) +} + +func generateKnownHostsEntry(t *testing.T, hostname string) (golangssh.PublicKey, string) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generating host key: %v", err) + } + sshPubKey, err := golangssh.NewPublicKey(&key.PublicKey) + if err != nil { + t.Fatalf("creating SSH public key: %v", err) + } + entry := fmt.Sprintf("%s %s", hostname, string(golangssh.MarshalAuthorizedKey(sshPubKey))) + return sshPubKey, entry +} + +// fakeAddr implements net.Addr for use in HostKeyCallback tests. +type fakeAddr struct{ s string } + +func (a fakeAddr) Network() string { return "tcp" } +func (a fakeAddr) String() string { return a.s } + +func TestNewSSHPublicKeys_NoKnownHosts(t *testing.T) { + keyPEM := generateECPrivateKeyPEM(t) + pubKeys, err := ssh.NewSSHPublicKeys("git", keyPEM, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if pubKeys == nil { + t.Fatal("expected non-nil PublicKeys") + } + // With no known hosts, InsecureIgnoreHostKey must accept any host. + if err := pubKeys.HostKeyCallback("any-host:22", fakeAddr{"any-host:22"}, nil); err != nil { + t.Fatalf("InsecureIgnoreHostKey should accept any host, got: %v", err) + } +} + +func TestNewSSHPublicKeys_WithKnownHosts(t *testing.T) { + keyPEM := generateECPrivateKeyPEM(t) + hostPubKey, knownHostsEntry := generateKnownHostsEntry(t, "myhost.example.com") + + pubKeys, err := ssh.NewSSHPublicKeys("git", keyPEM, []byte(knownHostsEntry)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Matching key for the known host must be accepted. + addr := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 22} + if err := pubKeys.HostKeyCallback("myhost.example.com:22", addr, hostPubKey); err != nil { + t.Fatalf("expected callback to accept matching key, got: %v", err) + } + + // An unknown host must be rejected. + if err := pubKeys.HostKeyCallback("unknown-host:22", addr, hostPubKey); err == nil { + t.Fatal("expected error for unknown host, got nil") + } +} + +func TestNewSSHPublicKeys_InvalidKnownHosts(t *testing.T) { + keyPEM := generateECPrivateKeyPEM(t) + // Malformed known_hosts must be rejected at construction time. + _, err := ssh.NewSSHPublicKeys("git", keyPEM, []byte("not-valid-known-hosts @@@@")) + if err == nil { + t.Fatal("expected error for invalid known_hosts, got nil") + } +} diff --git a/pkg/git/netutils.go b/pkg/git/netutils.go index 5249fd7675..73e7dc709e 100644 --- a/pkg/git/netutils.go +++ b/pkg/git/netutils.go @@ -9,9 +9,7 @@ import ( "github.com/go-git/go-git/v5/plumbing/transport" httpgit "github.com/go-git/go-git/v5/plumbing/transport/http" - gossh "github.com/go-git/go-git/v5/plumbing/transport/ssh" giturls "github.com/rancher/fleet/pkg/git-urls" - "golang.org/x/crypto/ssh" corev1 "k8s.io/api/core/v1" fleetgithub "github.com/rancher/fleet/internal/github" @@ -51,25 +49,15 @@ func GetAuthFromSecret(url string, creds *corev1.Secret, knownHosts string) (tra if err != nil { return nil, err } - auth, err := gossh.NewPublicKeys(gitURL.User.Username(), creds.Data[corev1.SSHAuthPrivateKey], "") + // Prefer known_hosts from the secret; fall back to the cluster-wide value. + knownHostsData := creds.Data["known_hosts"] + if len(knownHostsData) == 0 { + knownHostsData = []byte(knownHosts) + } + auth, err := fleetssh.NewSSHPublicKeys(gitURL.User.Username(), creds.Data[corev1.SSHAuthPrivateKey], knownHostsData) if err != nil { return nil, err } - switch { - case creds.Data["known_hosts"] != nil: - auth.HostKeyCallback, err = fleetssh.CreateKnownHostsCallBack(creds.Data["known_hosts"]) - if err != nil { - return nil, err - } - case len(knownHosts) > 0: - auth.HostKeyCallback, err = fleetssh.CreateKnownHostsCallBack([]byte(knownHosts)) - if err != nil { - return nil, err - } - default: - //nolint:gosec // G106: Use of ssh InsecureIgnoreHostKey should be audited - auth.HostKeyCallback = ssh.InsecureIgnoreHostKey() - } return auth, nil default: auth, err := fleetgithub.GetGithubAppAuthFromSecret(url, creds, GitHubAppGetter) From 10c8e4fbf38f0a2a195ae35f0d51e3464c27bfa1 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Mon, 23 Mar 2026 15:13:23 +0100 Subject: [PATCH 4/8] Remove go-getter from GetContent Remove the go-getter dependency from bundlereader and replace its use with go-git for git sources and stdlib HTTP for HTTP archive extraction. CA bundles augment the system pool rather than replacing it; go-git creates an isolated transport per clone so concurrent clones with different CAs need no mutex. SCP-style SSH sources accept any username, and forced-scheme prefixes (git::, ssh::) are normalised to proper URLs. Source shorthands cover GitHub, GitLab, and Bitbucket. HTTP archives (tar.gz, tar.bz2, tar.xz, tar.zst, tar, gz, bz2, xz, zst, zip) are extracted inline; ?checksum=: verifies the body via a TeeReader; ?archive= overrides extension detection. Symlink and path traversal in tar and zip archives are rejected. Error messages redact any embedded credentials in clone URLs. --- .../go_getter_custom_ca_test.go | 21 +- go.mod | 12 +- go.sum | 21 +- .../bundlereader/getcontent_test.go | 53 ++ .../bundlereader/httpcontent_test.go | 146 +++++ integrationtests/gitcloner/clone_test.go | 49 ++ integrationtests/go.mod | 10 +- integrationtests/go.sum | 21 +- internal/bundlereader/archive.go | 297 ++++++++++ internal/bundlereader/auth.go | 2 + internal/bundlereader/gitclone.go | 234 ++++++++ internal/bundlereader/gitclone_test.go | 119 ++++ internal/bundlereader/httpdownload.go | 151 +++++ .../httpdownload_internal_test.go | 243 ++++++++ internal/bundlereader/loaddirectory.go | 246 ++++---- .../loaddirectory_internal_test.go | 525 +++++++++++++----- internal/bundlereader/loaddirectory_test.go | 65 +-- internal/bundlereader/source.go | 245 ++++++++ internal/cmd/cli/gitcloner/cloner.go | 43 +- pkg/git/netutils.go | 6 +- 20 files changed, 2083 insertions(+), 426 deletions(-) create mode 100644 integrationtests/bundlereader/httpcontent_test.go create mode 100644 internal/bundlereader/archive.go create mode 100644 internal/bundlereader/gitclone.go create mode 100644 internal/bundlereader/gitclone_test.go create mode 100644 internal/bundlereader/httpdownload.go create mode 100644 internal/bundlereader/httpdownload_internal_test.go create mode 100644 internal/bundlereader/source.go diff --git a/e2e/single-cluster/go_getter_custom_ca_test.go b/e2e/single-cluster/go_getter_custom_ca_test.go index 2bcf29258e..840ff98a3d 100644 --- a/e2e/single-cluster/go_getter_custom_ca_test.go +++ b/e2e/single-cluster/go_getter_custom_ca_test.go @@ -21,15 +21,12 @@ import ( "sigs.k8s.io/yaml" ) -// Those tests are specifically targeting one feature of go-getter, namely cloning of git -// repositories using HTTPS. That is, because TLS certificates are only used in HTTPS URLs, not if -// SSH keys are used to clone those repositories. Since go-getter shells out to the `git` CLI for -// cloning git repositories, the configuration of TLS certificates or ignoring those needs to be -// configured with `git` typical environment variables. Those are the tests for that implementation. -// For go-getter to be used, the `helm.chart` field in `fleet.yaml` needs to point to a URL that -// tells go-getter to use the git protocol over HTTPS. Such an URL is prefixed with `git::https://`. -// The contents fetched from those repositories are expected to be helm charts. -var _ = Describe("Testing go-getter CA bundles and insecureSkipVerify for cloning git repositories", Label("infra-setup"), func() { +// These tests cover HTTPS cloning of git repositories via the `git::https://` URL prefix in +// `helm.chart`, including TLS certificate configuration (custom CA bundles and InsecureSkipVerify). +// The implementation uses go-git directly; the `git::` prefix is detected by Fleet's source parser +// and dispatched to the go-git–based fetcher. The contents fetched from those repositories are +// expected to be helm charts. +var _ = Describe("Testing CA bundles and insecureSkipVerify for cloning git repositories", Label("infra-setup"), func() { const ( sleeper = "sleeper" entrypoint = "entrypoint" @@ -167,7 +164,7 @@ var _ = Describe("Testing go-getter CA bundles and insecureSkipVerify for clonin tmpAssetDir := path.Join(tmpDir, "entryPoint") err = os.Mkdir(tmpAssetDir, 0755) Expect(err).ToNot(HaveOccurred()) - url := "git::" + gh.GetInClusterURL(host, HTTPSPort, "repo?ref="+sleeper) + url := "git::" + gh.GetInClusterURL(host, HTTPSPort, "repo//"+sleeper) err = os.WriteFile( path.Join(tmpAssetDir, "fleet.yaml"), fmt.Appendf([]byte{}, "helm:\n chart: %s\n", url), @@ -185,7 +182,7 @@ var _ = Describe("Testing go-getter CA bundles and insecureSkipVerify for clonin _, _ = k.Delete("ns", targetNamespace) }) - When("testing custom CA bundles for cloning git with HTTPS using go-getter (fleet.yaml)", func() { + When("testing custom CA bundles for cloning git with HTTPS (fleet.yaml)", func() { It("should succeed when not configuring any CA", func() { // Create and apply GitRepo, don't configure CABundle, InsecureSkipTLSVerify, // helmSecretName, or helmSecretNameForPath in GitRepo.spec which makes it fall back to @@ -267,7 +264,7 @@ var _ = Describe("Testing go-getter CA bundles and insecureSkipVerify for clonin }) }) - When("testing ignoring insecure certificates for cloning git repositories with HTTPS using go-getter (fleet.yaml)", func() { + When("testing ignoring insecure certificates for cloning git repositories with HTTPS (fleet.yaml)", func() { It("should succeed when using the incorrect CA bundle provided in GitRepo's CABundle field but setting InsecureSkipTLSVerify to true", func() { // But it should fail in gitcloner already, not later in fleet apply. encodedCert := base64.StdEncoding.EncodeToString([]byte("invalid-ca-bundle")) diff --git a/go.mod b/go.mod index c2fbd15c2c..7ec0237283 100644 --- a/go.mod +++ b/go.mod @@ -26,8 +26,8 @@ require ( github.com/gogits/go-gogs-client v0.0.0-20210131175652-1d7215cd8d85 github.com/google/go-cmp v0.7.0 github.com/google/go-containerregistry v0.21.3 - github.com/hashicorp/go-getter/v2 v2.2.3 github.com/jpillora/backoff v1.0.0 + github.com/klauspost/compress v1.18.5 github.com/onsi/ginkgo/v2 v2.28.1 github.com/onsi/gomega v1.39.0 github.com/opencontainers/go-digest v1.0.0 @@ -44,6 +44,7 @@ require ( github.com/sirupsen/logrus v1.9.4 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 + github.com/ulikunitz/xz v0.5.15 go.uber.org/mock v0.6.0 go.uber.org/zap v1.27.1 golang.org/x/crypto v0.49.0 @@ -82,7 +83,6 @@ require ( github.com/ProtonMail/go-crypto v1.3.0 // indirect github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect github.com/beorn7/perks v1.0.1 // indirect - github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/chai2010/gettext-go v1.0.3 // indirect @@ -135,11 +135,6 @@ require ( github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect github.com/gosuri/uitable v0.0.4 // indirect github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect - github.com/hashicorp/errwrap v1.1.0 // indirect - github.com/hashicorp/go-cleanhttp v0.5.2 // indirect - github.com/hashicorp/go-multierror v1.1.1 // indirect - github.com/hashicorp/go-safetemp v1.0.0 // indirect - github.com/hashicorp/go-version v1.6.0 // indirect github.com/huandu/xstrings v1.5.0 // indirect github.com/ianlancetaylor/demangle v0.0.0-20251118225945-96ee0021ea0f // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect @@ -147,7 +142,6 @@ require ( github.com/jmoiron/sqlx v1.4.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/kevinburke/ssh_config v1.2.0 // indirect - github.com/klauspost/compress v1.18.4 // indirect github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/lib/pq v1.10.9 // indirect @@ -157,7 +151,6 @@ require ( github.com/mattn/go-runewidth v0.0.13 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect github.com/mitchellh/go-homedir v1.1.0 // indirect - github.com/mitchellh/go-testing-interface v1.14.1 // indirect github.com/mitchellh/go-wordwrap v1.0.1 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/moby/spdystream v0.5.0 // indirect @@ -183,7 +176,6 @@ require ( github.com/spf13/pflag v1.0.10 // indirect github.com/tetratelabs/wabin v0.0.0-20230304001439-f6f874872834 // indirect github.com/tetratelabs/wazero v1.11.0 // indirect - github.com/ulikunitz/xz v0.5.15 // indirect github.com/vbatts/tar-split v0.12.2 // indirect github.com/x448/float16 v0.8.4 // indirect github.com/xanzy/ssh-agent v0.3.3 // indirect diff --git a/go.sum b/go.sum index d599e2d490..b0d7e3c329 100644 --- a/go.sum +++ b/go.sum @@ -37,8 +37,6 @@ github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3d github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= -github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d h1:xDfNPAt8lFiC1UJrqV3uuy861HCTo708pDMbjHHdCas= -github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d/go.mod h1:6QX/PXZ00z/TKoufEY6K/a0k6AhaJrQKdFe6OfVXsa4= github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/bradleyfalzon/ghinstallation/v2 v2.16.0 h1:B91r9bHtXp/+XRgS5aZm6ZzTdz3ahgJYmkt4xZkgDz8= @@ -237,19 +235,6 @@ github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 h1:+ngKgrYPPJr github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA= github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.7 h1:X+2YciYSxvMQK0UZ7sg45ZVabVZBeBuvMkmuI2V3Fak= github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.7/go.mod h1:lW34nIZuQ8UDPdkon5fmfp2l3+ZkQ2me/+oecHYLOII= -github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= -github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= -github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= -github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= -github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= -github.com/hashicorp/go-getter/v2 v2.2.3 h1:6CVzhT0KJQHqd9b0pK3xSP0CM/Cv+bVhk+jcaRJ2pGk= -github.com/hashicorp/go-getter/v2 v2.2.3/go.mod h1:hp5Yy0GMQvwWVUmwLs3ygivz1JSLI323hdIE9J9m7TY= -github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= -github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= -github.com/hashicorp/go-safetemp v1.0.0 h1:2HR189eFNrjHQyENnQMMpCiBAsRxzbTMIgBhEyExpmo= -github.com/hashicorp/go-safetemp v1.0.0/go.mod h1:oaerMy3BhqiTbVye6QuFhFtIceqFoDHxNAB65b+Rj1I= -github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek= -github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/golang-lru/arc/v2 v2.0.5 h1:l2zaLDubNhW4XO3LnliVj0GXO3+/CGNJAg1dcN2Fpfw= github.com/hashicorp/golang-lru/arc/v2 v2.0.5/go.mod h1:ny6zBSQZi2JxIeYcv7kt2sH2PXJtirBN7RDhRpxPkxU= github.com/hashicorp/golang-lru/v2 v2.0.5 h1:wW7h1TG88eUIJ2i69gaE3uNVtEPIagzhGvHgwfx2Vm4= @@ -272,8 +257,8 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/kevinburke/ssh_config v1.2.0 h1:x584FjTGwHzMwvHx18PXxbBVzfnxogHaAReU4gf13a4= github.com/kevinburke/ssh_config v1.2.0/go.mod h1:CT57kijsi8u/K/BOFA39wgDQJ9CxiF4nAY/ojJ6r6mM= -github.com/klauspost/compress v1.18.4 h1:RPhnKRAQ4Fh8zU2FY/6ZFDwTVTxgJ/EMydqSTzE9a2c= -github.com/klauspost/compress v1.18.4/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4= +github.com/klauspost/compress v1.18.5 h1:/h1gH5Ce+VWNLSWqPzOVn6XBO+vJbCNGvjoaGBFW2IE= +github.com/klauspost/compress v1.18.5/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= @@ -311,8 +296,6 @@ github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa1 github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= -github.com/mitchellh/go-testing-interface v1.14.1 h1:jrgshOhYAUVNMAJiKbEu7EqAwgJJ2JqpQmpLJOu07cU= -github.com/mitchellh/go-testing-interface v1.14.1/go.mod h1:gfgS7OtZj6MA4U1UrDRp04twqAjfvlZyCfX3sDjEym8= github.com/mitchellh/go-wordwrap v1.0.1 h1:TLuKupo69TCn6TQSyGxwI1EblZZEsQ0vMlAFQflz0v0= github.com/mitchellh/go-wordwrap v1.0.1/go.mod h1:R62XHJLzvMFRBbcrT7m7WgmE1eOyTSsCt+hzestvNj0= github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ= diff --git a/integrationtests/bundlereader/getcontent_test.go b/integrationtests/bundlereader/getcontent_test.go index d3207338cb..7d09be6f28 100644 --- a/integrationtests/bundlereader/getcontent_test.go +++ b/integrationtests/bundlereader/getcontent_test.go @@ -92,6 +92,18 @@ var _ = Describe("GetContent fetches files from a git repository", Label("networ }) }) + When("a CA bundle is provided", func() { + It("returns the repository files", func() { + source := fmt.Sprintf("git::%s/%s/%s", httpsBase, utils.GogsUser, repoName) + files, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{CABundle: gogsCABundle}, false, nil, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(files).To(HaveKey("README.md")) + }) + }) + When("no TLS configuration is provided", func() { It("fails with a certificate error", func() { source := fmt.Sprintf("git::%s/%s/%s", httpsBase, utils.GogsUser, repoName) @@ -236,6 +248,47 @@ var _ = Describe("GetContent fetches files from a git repository", Label("networ Expect(err).NotTo(HaveOccurred()) Expect(files).To(HaveKey("README.md")) }) + + It("returns the repository files when a correct known host entry is provided", func() { + privateKey, err := os.ReadFile(tmpKeyFile) + Expect(err).NotTo(HaveOccurred()) + + knownHost, err := utils.GetGogsKnownHostEntry(context.Background(), container) + Expect(err).NotTo(HaveOccurred()) + + source := fmt.Sprintf("%s/%s/%s", sshBase, utils.GogsUser, repoName) + Eventually(func() error { + _, err = bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{SSHPrivateKey: privateKey, SSHKnownHosts: []byte(knownHost)}, false, nil, + ) + return err + }, timeout, interval).Should(Succeed()) + + files, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{SSHPrivateKey: privateKey, SSHKnownHosts: []byte(knownHost)}, false, nil, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(files).To(HaveKey("README.md")) + }) + + It("fails when a wrong known host entry is provided", func() { + privateKey, err := os.ReadFile(tmpKeyFile) + Expect(err).NotTo(HaveOccurred()) + + // A valid-looking but wrong known_hosts entry: the key is for github.com, not our gogs server. + wrongKnownHost := "github.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl" + + source := fmt.Sprintf("%s/%s/%s", sshBase, utils.GogsUser, repoName) + Eventually(func() error { + _, err = bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), source, "", + bundlereader.Auth{SSHPrivateKey: privateKey, SSHKnownHosts: []byte(wrongKnownHost)}, false, nil, + ) + return err + }, timeout, interval).Should(HaveOccurred()) + }) }) }) diff --git a/integrationtests/bundlereader/httpcontent_test.go b/integrationtests/bundlereader/httpcontent_test.go new file mode 100644 index 0000000000..1afdaeb26f --- /dev/null +++ b/integrationtests/bundlereader/httpcontent_test.go @@ -0,0 +1,146 @@ +package bundlereader_test + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/hex" + "net/http" + "net/http/httptest" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/rancher/fleet/internal/bundlereader" +) + +// newTarGz builds a tar.gz archive in memory containing the given files +// (map of relative name → content) and returns the bytes. +func newTarGz(files map[string][]byte) []byte { + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + for name, data := range files { + _ = tw.WriteHeader(&tar.Header{ + Typeflag: tar.TypeReg, + Name: name, + Size: int64(len(data)), + Mode: 0600, + }) + _, _ = tw.Write(data) + } + _ = tw.Close() + _ = gw.Close() + return buf.Bytes() +} + +var _ = Describe("GetContent fetches HTTP sources", func() { + var ( + srv *httptest.Server + archiveBytes []byte + ) + + BeforeEach(func() { + archiveBytes = newTarGz(map[string][]byte{ + "README.md": []byte("hello"), + "subdir/config.yaml": []byte("key: value"), + }) + }) + + AfterEach(func() { + if srv != nil { + srv.Close() + srv = nil + } + }) + + When("the URL points to a tar.gz archive", func() { + It("extracts the archive and returns the files", func() { + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(archiveBytes) + })) + + files, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), srv.URL+"/bundle.tar.gz", "", + bundlereader.Auth{}, false, nil, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(files).To(HaveKey("README.md")) + Expect(files).To(HaveKey(filepath.Join("subdir", "config.yaml"))) + }) + }) + + When("the URL has no recognisable extension but ?archive= is provided", func() { + It("extracts using the override extension", func() { + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // The server must NOT receive the ?archive= parameter. + Expect(r.URL.Query().Get("archive")).To(BeEmpty()) + _, _ = w.Write(archiveBytes) + })) + + files, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), srv.URL+"/download?archive=tar.gz", "", + bundlereader.Auth{}, false, nil, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(files).To(HaveKey("README.md")) + }) + }) + + When("a correct ?checksum= is provided", func() { + It("succeeds when the hash matches", func() { + h := sha256.New() + h.Write(archiveBytes) + hexHash := hex.EncodeToString(h.Sum(nil)) + + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // The server must NOT receive the ?checksum= parameter. + Expect(r.URL.Query().Get("checksum")).To(BeEmpty()) + _, _ = w.Write(archiveBytes) + })) + + _, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), + srv.URL+"/bundle.tar.gz?checksum=sha256:"+hexHash, "", + bundlereader.Auth{}, false, nil, + ) + Expect(err).NotTo(HaveOccurred()) + }) + + It("fails when the hash does not match", func() { + wrongHash := "0000000000000000000000000000000000000000000000000000000000000000" + + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(archiveBytes) + })) + + _, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), + srv.URL+"/bundle.tar.gz?checksum=sha256:"+wrongHash, "", + bundlereader.Auth{}, false, nil, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("checksum mismatch")) + }) + }) + + When("the URL points to a plain file", func() { + It("writes the file under its URL base name", func() { + content := []byte("plain content") + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(content) + })) + + files, err := bundlereader.GetContent( + context.Background(), GinkgoT().TempDir(), srv.URL+"/data.txt", "", + bundlereader.Auth{}, false, nil, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(files).To(HaveKey("data.txt")) + Expect(files["data.txt"]).To(Equal(content)) + }) + }) +}) diff --git a/integrationtests/gitcloner/clone_test.go b/integrationtests/gitcloner/clone_test.go index b39132577f..56987a2a36 100644 --- a/integrationtests/gitcloner/clone_test.go +++ b/integrationtests/gitcloner/clone_test.go @@ -216,6 +216,55 @@ var _ = Describe("Applying a git job gets content from git repo", Label("network Expect(err).NotTo(HaveOccurred()) }) }) + + // This test verifies that the HTTPS transport is restored to the default + // after a CA bundle clone. Without transport restoration, a subsequent + // clone without a CA bundle would erroneously trust the custom CA still + // installed in the global transport. + // + // This is the integration-level regression test for the exclusive cert pool + // fix: a Rancher CA bundle mounted in gitjob pods must not prevent subsequent + // connections to servers signed by well-known CAs (e.g. GitHub). + When("CA bundle clone is followed by a no-bundle clone", func() { + It("restores the default transport so the no-bundle clone fails TLS", func() { + url, err := utils.GetGogsHTTPSURL(context.Background(), container) + Expect(err).NotTo(HaveOccurred()) + repoName, _, err := createRepo(url, "assets/repo", false) + Expect(err).NotTo(HaveOccurred()) + + caBundleFile, err := os.CreateTemp("", "gitcloner-ca") + Expect(err).NotTo(HaveOccurred()) + defer func() { _ = os.Remove(caBundleFile.Name()) }() + _, err = caBundleFile.Write(gogsCABundle) + Expect(err).NotTo(HaveOccurred()) + + tmp1 := utils.CreateTempFolder("gogs-ca") + defer func() { _ = os.RemoveAll(tmp1) }() + + // First: clone with CA bundle — must succeed. + c := gitcloner.New() + Expect(c.CloneRepo(&gitcloner.GitCloner{ + Repo: url + "/test/" + repoName, + Path: tmp1, + Branch: "master", + CABundleFile: caBundleFile.Name(), + })).NotTo(HaveOccurred(), "CA bundle clone should succeed") + + // Second: clone without CA bundle — must fail TLS. + // If the transport is not restored, the gogs self-signed cert would + // still be trusted and the clone would incorrectly succeed. + tmp2 := utils.CreateTempFolder("gogs-nocert") + defer func() { _ = os.RemoveAll(tmp2) }() + + noCertErr := c.CloneRepo(&gitcloner.GitCloner{ + Repo: url + "/test/" + repoName, + Path: tmp2, + Branch: "master", + }) + Expect(noCertErr).To(HaveOccurred(), "clone without CA bundle should fail TLS") + Expect(noCertErr.Error()).To(ContainSubstring("certificate")) + }) + }) }) When("Cloning an ssh repo", func() { diff --git a/integrationtests/go.mod b/integrationtests/go.mod index ba86178dad..8f17951899 100644 --- a/integrationtests/go.mod +++ b/integrationtests/go.mod @@ -48,7 +48,6 @@ require ( github.com/ProtonMail/go-crypto v1.3.0 // indirect github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect github.com/beorn7/perks v1.0.1 // indirect - github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/bradleyfalzon/ghinstallation/v2 v2.16.0 // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect @@ -120,12 +119,6 @@ require ( github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect github.com/gosuri/uitable v0.0.4 // indirect github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect - github.com/hashicorp/errwrap v1.1.0 // indirect - github.com/hashicorp/go-cleanhttp v0.5.2 // indirect - github.com/hashicorp/go-getter/v2 v2.2.3 // indirect - github.com/hashicorp/go-multierror v1.1.1 // indirect - github.com/hashicorp/go-safetemp v1.0.0 // indirect - github.com/hashicorp/go-version v1.6.0 // indirect github.com/huandu/xstrings v1.5.0 // indirect github.com/ianlancetaylor/demangle v0.0.0-20251118225945-96ee0021ea0f // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect @@ -134,7 +127,7 @@ require ( github.com/jpillora/backoff v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/kevinburke/ssh_config v1.2.0 // indirect - github.com/klauspost/compress v1.18.4 // indirect + github.com/klauspost/compress v1.18.5 // indirect github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/lib/pq v1.10.9 // indirect @@ -146,7 +139,6 @@ require ( github.com/mattn/go-runewidth v0.0.16 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect github.com/mitchellh/go-homedir v1.1.0 // indirect - github.com/mitchellh/go-testing-interface v1.14.1 // indirect github.com/mitchellh/go-wordwrap v1.0.1 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/moby/docker-image-spec v1.3.1 // indirect diff --git a/integrationtests/go.sum b/integrationtests/go.sum index 7b80d8d192..43985183cc 100644 --- a/integrationtests/go.sum +++ b/integrationtests/go.sum @@ -33,8 +33,6 @@ github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3d github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= -github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d h1:xDfNPAt8lFiC1UJrqV3uuy861HCTo708pDMbjHHdCas= -github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d/go.mod h1:6QX/PXZ00z/TKoufEY6K/a0k6AhaJrQKdFe6OfVXsa4= github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/bradleyfalzon/ghinstallation/v2 v2.16.0 h1:B91r9bHtXp/+XRgS5aZm6ZzTdz3ahgJYmkt4xZkgDz8= @@ -246,19 +244,6 @@ github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 h1:+ngKgrYPPJr github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA= github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.7 h1:X+2YciYSxvMQK0UZ7sg45ZVabVZBeBuvMkmuI2V3Fak= github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.7/go.mod h1:lW34nIZuQ8UDPdkon5fmfp2l3+ZkQ2me/+oecHYLOII= -github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= -github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= -github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= -github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= -github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= -github.com/hashicorp/go-getter/v2 v2.2.3 h1:6CVzhT0KJQHqd9b0pK3xSP0CM/Cv+bVhk+jcaRJ2pGk= -github.com/hashicorp/go-getter/v2 v2.2.3/go.mod h1:hp5Yy0GMQvwWVUmwLs3ygivz1JSLI323hdIE9J9m7TY= -github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= -github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= -github.com/hashicorp/go-safetemp v1.0.0 h1:2HR189eFNrjHQyENnQMMpCiBAsRxzbTMIgBhEyExpmo= -github.com/hashicorp/go-safetemp v1.0.0/go.mod h1:oaerMy3BhqiTbVye6QuFhFtIceqFoDHxNAB65b+Rj1I= -github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek= -github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/golang-lru/arc/v2 v2.0.5 h1:l2zaLDubNhW4XO3LnliVj0GXO3+/CGNJAg1dcN2Fpfw= github.com/hashicorp/golang-lru/arc/v2 v2.0.5/go.mod h1:ny6zBSQZi2JxIeYcv7kt2sH2PXJtirBN7RDhRpxPkxU= github.com/hashicorp/golang-lru/v2 v2.0.5 h1:wW7h1TG88eUIJ2i69gaE3uNVtEPIagzhGvHgwfx2Vm4= @@ -281,8 +266,8 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/kevinburke/ssh_config v1.2.0 h1:x584FjTGwHzMwvHx18PXxbBVzfnxogHaAReU4gf13a4= github.com/kevinburke/ssh_config v1.2.0/go.mod h1:CT57kijsi8u/K/BOFA39wgDQJ9CxiF4nAY/ojJ6r6mM= -github.com/klauspost/compress v1.18.4 h1:RPhnKRAQ4Fh8zU2FY/6ZFDwTVTxgJ/EMydqSTzE9a2c= -github.com/klauspost/compress v1.18.4/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4= +github.com/klauspost/compress v1.18.5 h1:/h1gH5Ce+VWNLSWqPzOVn6XBO+vJbCNGvjoaGBFW2IE= +github.com/klauspost/compress v1.18.5/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= @@ -323,8 +308,6 @@ github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa1 github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= -github.com/mitchellh/go-testing-interface v1.14.1 h1:jrgshOhYAUVNMAJiKbEu7EqAwgJJ2JqpQmpLJOu07cU= -github.com/mitchellh/go-testing-interface v1.14.1/go.mod h1:gfgS7OtZj6MA4U1UrDRp04twqAjfvlZyCfX3sDjEym8= github.com/mitchellh/go-wordwrap v1.0.1 h1:TLuKupo69TCn6TQSyGxwI1EblZZEsQ0vMlAFQflz0v0= github.com/mitchellh/go-wordwrap v1.0.1/go.mod h1:R62XHJLzvMFRBbcrT7m7WgmE1eOyTSsCt+hzestvNj0= github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ= diff --git a/internal/bundlereader/archive.go b/internal/bundlereader/archive.go new file mode 100644 index 0000000000..1e410c7c57 --- /dev/null +++ b/internal/bundlereader/archive.go @@ -0,0 +1,297 @@ +package bundlereader + +import ( + "archive/tar" + "archive/zip" + "compress/bzip2" + "compress/gzip" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/klauspost/compress/zstd" + "github.com/ulikunitz/xz" +) + +// extractResponse writes the content of r into dst. +// filename is used to detect the archive type by extension. +// archiveOverride, when non-empty, takes precedence over the filename extension. +func extractResponse(dst, filename, archiveOverride string, r io.Reader) error { + lower := archiveOverride + if lower == "" { + lower = strings.ToLower(filename) + } + switch { + case strings.HasSuffix(lower, ".tar.gz"), strings.HasSuffix(lower, ".tgz"): + return extractTarGz(dst, r) + case strings.HasSuffix(lower, ".tar.bz2"), strings.HasSuffix(lower, ".tbz2"): + return extractTarBz2(dst, r) + case strings.HasSuffix(lower, ".tar.zst"), strings.HasSuffix(lower, ".tar.zstd"), + strings.HasSuffix(lower, ".tzst"): + return extractTarZst(dst, r) + case strings.HasSuffix(lower, ".tar"): + return extractTar(dst, r) + case strings.HasSuffix(lower, ".gz"): + return extractGz(dst, filepath.Base(filename), r) + case strings.HasSuffix(lower, ".bz2"): + return extractBz2(dst, filepath.Base(filename), r) + case strings.HasSuffix(lower, ".zst"), strings.HasSuffix(lower, ".zstd"): + return extractZst(dst, filepath.Base(filename), r) + case strings.HasSuffix(lower, ".zip"): + return extractZipFromReader(dst, r) + case strings.HasSuffix(lower, ".tar.xz"), strings.HasSuffix(lower, ".txz"): + return extractTarXz(dst, r) + case strings.HasSuffix(lower, ".xz"): + return extractXz(dst, filepath.Base(filename), r) + default: + // Plain file: write it under its base name inside dst. + // filepath.Base returns "/" for a trailing-slash URL and "." for an + // empty string; both would be unsafe as file names. + name := filepath.Base(filename) + if name == "/" || name == "." || name == "" { + name = "file" + } + out, err := os.Create(filepath.Join(dst, name)) + if err != nil { + return err + } + defer out.Close() + _, err = io.Copy(out, r) + return err + } +} + +// extractTarGz decompresses a gzip-compressed tar archive into dst. +func extractTarGz(dst string, r io.Reader) error { + gz, err := gzip.NewReader(r) + if err != nil { + return fmt.Errorf("opening gzip reader: %w", err) + } + defer gz.Close() + return extractTar(dst, gz) +} + +// extractTarBz2 decompresses a bzip2-compressed tar archive into dst. +func extractTarBz2(dst string, r io.Reader) error { + return extractTar(dst, bzip2.NewReader(r)) +} + +// extractTarZst decompresses a zstd-compressed tar archive into dst. +func extractTarZst(dst string, r io.Reader) error { + zr, err := zstd.NewReader(r) + if err != nil { + return fmt.Errorf("opening zstd reader: %w", err) + } + defer zr.Close() + return extractTar(dst, zr) +} + +// extractTarXz decompresses an xz-compressed tar archive into dst. +func extractTarXz(dst string, r io.Reader) error { + xr, err := xz.NewReader(r) + if err != nil { + return fmt.Errorf("opening xz reader: %w", err) + } + return extractTar(dst, xr) +} + +// extractGz decompresses a single gzip-compressed file into dst/name. +func extractGz(dst, name string, r io.Reader) error { + gz, err := gzip.NewReader(r) + if err != nil { + return fmt.Errorf("opening gzip reader: %w", err) + } + defer gz.Close() + return extractSingleFile(dst, strings.TrimSuffix(name, ".gz"), gz) +} + +// extractBz2 decompresses a single bzip2-compressed file into dst/name. +func extractBz2(dst, name string, r io.Reader) error { + return extractSingleFile(dst, strings.TrimSuffix(name, ".bz2"), bzip2.NewReader(r)) +} + +// extractZst decompresses a single zstd-compressed file into dst/name. +func extractZst(dst, name string, r io.Reader) error { + zr, err := zstd.NewReader(r) + if err != nil { + return fmt.Errorf("opening zstd reader: %w", err) + } + defer zr.Close() + // Handle both .zst and .zstd extensions (e.g. when ?archive=.zstd is used). + return extractSingleFile(dst, strings.TrimSuffix(strings.TrimSuffix(name, ".zstd"), ".zst"), zr) +} + +// extractXz decompresses a single xz-compressed file into dst/name. +func extractXz(dst, name string, r io.Reader) error { + xr, err := xz.NewReader(r) + if err != nil { + return fmt.Errorf("opening xz reader: %w", err) + } + return extractSingleFile(dst, strings.TrimSuffix(name, ".xz"), xr) +} + +// extractSingleFile writes the content of r into a single file under dst named +// outName. An empty outName is replaced with "file". +func extractSingleFile(dst, outName string, r io.Reader) error { + if outName == "" { + outName = "file" + } + out, err := os.Create(filepath.Join(dst, outName)) + if err != nil { + return err + } + defer out.Close() + + _, err = io.Copy(out, r) + return err +} + +// extractTar extracts a tar archive into dst. +func extractTar(dst string, r io.Reader) error { + tr := tar.NewReader(r) + for { + hdr, err := tr.Next() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return fmt.Errorf("reading tar: %w", err) + } + + target, err := safeJoin(dst, hdr.Name) + if err != nil { + return err + } + + switch hdr.Typeflag { + case tar.TypeDir: + if err := os.MkdirAll(target, 0750); err != nil { + return err + } + case tar.TypeReg: + if err := os.MkdirAll(filepath.Dir(target), 0750); err != nil { + return err + } + f, err := os.OpenFile(target, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, hdr.FileInfo().Mode()) + if err != nil { + return err + } + //nolint:gosec // G110: archive content is sourced from a trusted server configured by the cluster admin + if _, err := io.Copy(f, tr); err != nil { + f.Close() + return err + } + f.Close() + case tar.TypeSymlink: + // Reject absolute symlink targets outright; they bypass path-safety checks. + if filepath.IsAbs(hdr.Linkname) { + return fmt.Errorf("symlink %q: absolute target not allowed in archive", hdr.Name) + } + // Validate the target resolves within the extraction root when expanded + // relative to the directory containing the link (not relative to dst itself). + //nolint:gosec // G305: path traversal is prevented by the filepath.Rel check below. + resolved := filepath.Join(filepath.Dir(target), hdr.Linkname) + rel, err := filepath.Rel(filepath.Clean(dst), filepath.Clean(resolved)) + if err != nil || strings.HasPrefix(rel, "..") { + return fmt.Errorf("symlink %q: target %q escapes destination directory", hdr.Name, hdr.Linkname) + } + if err := os.MkdirAll(filepath.Dir(target), 0750); err != nil { + return err + } + if err := os.Symlink(hdr.Linkname, target); err != nil && !os.IsExist(err) { + return err + } + case tar.TypeXHeader, tar.TypeXGlobalHeader, tar.TypeGNULongName, tar.TypeGNULongLink: + // Metadata-only entries produced by some tar implementations. + // Go's archive/tar resolves these in Next(), but handle them + // defensively in case an unusual archive surfaces them directly. + continue + default: + return fmt.Errorf("unsupported tar entry type %d for %q", hdr.Typeflag, hdr.Name) + } + } + return nil +} + +// safeJoin joins base and name, returning an error if the result would escape base. +func safeJoin(base, name string) (string, error) { + // filepath.Clean("/"+name) produces an absolute path; filepath.Join then + // discards base on Unix-like systems. Strip the leading separator after + // cleaning so that the result stays relative to base. + clean := filepath.Clean(string(os.PathSeparator) + name) + rel := strings.TrimPrefix(clean, string(os.PathSeparator)) + target := filepath.Join(base, rel) + cleanBase := filepath.Clean(base) + if !strings.HasPrefix(target, cleanBase+string(os.PathSeparator)) && + target != cleanBase { + return "", fmt.Errorf("archive path %q escapes destination directory", name) + } + return target, nil +} + +// archive/zip requires a ReaderAt, so we use an *os.File as the backing store +// to avoid buffering the entire archive in memory. +func extractZipFromReader(dst string, r io.Reader) error { + f, err := os.CreateTemp("", "bundle-*.zip") + if err != nil { + return fmt.Errorf("creating temp file for zip: %w", err) + } + defer func() { + f.Close() + os.Remove(f.Name()) + }() + + if _, err := io.Copy(f, r); err != nil { + return fmt.Errorf("writing zip body to temp file: %w", err) + } + + size, err := f.Seek(0, io.SeekEnd) + if err != nil { + return fmt.Errorf("seeking zip temp file: %w", err) + } + + zr, err := zip.NewReader(f, size) + if err != nil { + return fmt.Errorf("opening zip reader: %w", err) + } + + for _, entry := range zr.File { + target, err := safeJoin(dst, entry.Name) + if err != nil { + return err + } + + if entry.FileInfo().IsDir() { + if err := os.MkdirAll(target, 0750); err != nil { + return err + } + continue + } + + if err := os.MkdirAll(filepath.Dir(target), 0750); err != nil { + return err + } + + rc, err := entry.Open() + if err != nil { + return fmt.Errorf("opening zip entry %q: %w", entry.Name, err) + } + out, err := os.OpenFile(target, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, entry.Mode()) + if err != nil { + rc.Close() + return err + } + //nolint:gosec // G110: archive content is sourced from a trusted server configured by the cluster admin + if _, err := io.Copy(out, rc); err != nil { + rc.Close() + out.Close() + return fmt.Errorf("extracting zip entry %q: %w", entry.Name, err) + } + rc.Close() + out.Close() + } + return nil +} diff --git a/internal/bundlereader/auth.go b/internal/bundlereader/auth.go index 54dd82b36f..edcb59ce30 100644 --- a/internal/bundlereader/auth.go +++ b/internal/bundlereader/auth.go @@ -18,6 +18,7 @@ type Auth struct { Password string `json:"password,omitempty"` CABundle []byte `json:"caBundle,omitempty"` SSHPrivateKey []byte `json:"sshPrivateKey,omitempty"` + SSHKnownHosts []byte `json:"sshKnownHosts,omitempty"` InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"` BasicHTTP bool `json:"basicHTTP,omitempty"` // remember to update Hash() when adding/modifying fields @@ -46,6 +47,7 @@ func (a Auth) Hash() string { []byte(a.Password), a.CABundle, a.SSHPrivateKey, + a.SSHKnownHosts, {toByte(a.InsecureSkipVerify)}, {toByte(a.BasicHTTP)}, } { diff --git a/internal/bundlereader/gitclone.go b/internal/bundlereader/gitclone.go new file mode 100644 index 0000000000..c919381f19 --- /dev/null +++ b/internal/bundlereader/gitclone.go @@ -0,0 +1,234 @@ +package bundlereader + +import ( + "context" + "crypto/x509" + "encoding/base64" + "errors" + "fmt" + "net/url" + "os" + "strconv" + "strings" + + gogit "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing" + httpgit "github.com/go-git/go-git/v5/plumbing/transport/http" + + fleetssh "github.com/rancher/fleet/internal/ssh" + fleetgit "github.com/rancher/fleet/pkg/git" +) + +// gitDownload clones rawURL into dst using go-git. +// +// rawURL may include ?ref=, ?sshkey=, and ?depth= query parameters. The auth +// struct provides credentials, TLS settings, and optional SSH known-hosts. +// +// When auth.CABundle is non-empty, TLS verification uses the system cert pool +// augmented with those CA certificates, so that both the explicit CA and any +// system-trusted CA are accepted. +func gitDownload(ctx context.Context, dst, rawURL string, auth Auth) error { + u, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("parsing git URL %q: %w", rawURL, err) + } + + ref, sshKeyPEM, depth, err := extractQueryParams(u) + if err != nil { + return err + } + + // extractQueryParams already removed only the Fleet-specific params (?ref=, + // ?sshkey=, ?depth=) from u.RawQuery. Copy the URL so any remaining query + // params (e.g. signed-URL tokens) are preserved for the clone. + cloneURL := *u + + d := depth + if d == 0 { + d = 1 + } + + // go-git applies CABundle per-clone via CloneOptions: it clones the base + // transport in newSession and configures TLS on that copy without touching + // the process-global gitclient.Protocols map. Concurrent HTTPS clones with + // different CA bundles are therefore fully independent and need no mutex. + // + // Validate the PEM upfront: an invalid bundle would otherwise surface as a + // generic TLS handshake failure, which is harder to diagnose. + if len(auth.CABundle) > 0 && !auth.InsecureSkipVerify { + if !x509.NewCertPool().AppendCertsFromPEM(auth.CABundle) { + return errors.New("CA bundle contains no valid PEM certificates") + } + } + + cloneOpts := &gogit.CloneOptions{ + URL: cloneURL.String(), + InsecureSkipTLS: auth.InsecureSkipVerify, + CABundle: auth.CABundle, + ProxyOptions: fleetgit.ProxyOptsFromEnvironment(cloneURL.String()), + } + if err := setGitAuth(cloneOpts, &cloneURL, sshKeyPEM, auth); err != nil { + return err + } + + if err := resetDir(dst); err != nil { + return err + } + + if ref == "" { + cloneOpts.Depth = d + if _, err := gogit.PlainCloneContext(ctx, dst, false, cloneOpts); err != nil { + return fmt.Errorf("shallow clone of %s: %w", cloneURL.Redacted(), err) + } + return nil + } + + // Try shallow branch clone. + branchOpts := *cloneOpts + branchOpts.ReferenceName = plumbing.NewBranchReferenceName(ref) + branchOpts.SingleBranch = true + branchOpts.Depth = d + if _, err := gogit.PlainCloneContext(ctx, dst, false, &branchOpts); err == nil { + return nil + } + + // Try shallow tag clone. + if err := resetDir(dst); err != nil { + return err + } + tagOpts := *cloneOpts + tagOpts.ReferenceName = plumbing.NewTagReferenceName(ref) + tagOpts.SingleBranch = true + tagOpts.Depth = d + if _, err := gogit.PlainCloneContext(ctx, dst, false, &tagOpts); err == nil { + return nil + } + + // Fall back: full clone then check out ref as commit SHA or arbitrary revision. + if err := resetDir(dst); err != nil { + return err + } + r, err := gogit.PlainCloneContext(ctx, dst, false, cloneOpts) + if err != nil { + return fmt.Errorf("cloning %s: %w", cloneURL.Redacted(), err) + } + h, err := r.ResolveRevision(plumbing.Revision(ref)) + if err != nil { + return fmt.Errorf("resolving ref %q: %w", ref, err) + } + w, err := r.Worktree() + if err != nil { + return err + } + return w.Checkout(&gogit.CheckoutOptions{Hash: *h}) +} + +// setGitAuth configures the authentication method on opts. +// sshKeyPEM (from the URL ?sshkey= param) takes precedence over auth.SSHPrivateKey. +// For HTTPS, credentials are taken from the URL userinfo first, then from auth. +func setGitAuth(opts *gogit.CloneOptions, u *url.URL, sshKeyPEM []byte, auth Auth) error { + // Prefer the key from the URL query param over auth.SSHPrivateKey. + keyPEM := sshKeyPEM + if len(keyPEM) == 0 { + keyPEM = auth.SSHPrivateKey + } + + if len(keyPEM) > 0 { + user := "git" + if u.User != nil && u.User.Username() != "" { + user = u.User.Username() + } + pubKeys, err := fleetssh.NewSSHPublicKeys(user, keyPEM, auth.SSHKnownHosts) + if err != nil { + return fmt.Errorf("configuring SSH auth: %w", err) + } + opts.Auth = pubKeys + return nil + } + + if u.Scheme == "https" || u.Scheme == "http" { + var username, password string + if u.User != nil { + username = u.User.Username() + password, _ = u.User.Password() + } + if username == "" { + username = auth.Username + password = auth.Password + } + if username != "" { + opts.Auth = &httpgit.BasicAuth{Username: username, Password: password} + } + } + return nil +} + +// extractQueryParams strips the known Fleet query params (?ref=, ?sshkey=, +// ?depth=) from u and returns them as typed values. +func extractQueryParams(u *url.URL) (ref string, sshKey []byte, depth int, err error) { + q := u.Query() + ref = q.Get("ref") + q.Del("ref") + + if raw := q.Get("sshkey"); raw != "" { + // Prefer raw URL-safe Base64 (no padding). Fall back to standard/padded + // Base64, normalising spaces to '+' first because URL query parsing + // converts '+' to space in standard-encoded values. + sshKey, err = base64.RawURLEncoding.DecodeString(raw) + if err != nil { + normalized := strings.ReplaceAll(raw, " ", "+") + sshKey, err = base64.StdEncoding.DecodeString(normalized) + if err != nil { + sshKey, err = base64.URLEncoding.DecodeString(normalized) + if err != nil { + return "", nil, 0, fmt.Errorf("decoding sshkey query param: %w", err) + } + } + } + } + q.Del("sshkey") + + if rawDepth := q.Get("depth"); rawDepth != "" { + n, e := strconv.Atoi(rawDepth) + if e != nil { + return "", nil, 0, fmt.Errorf("invalid ?depth=%q: %w", rawDepth, e) + } + if n <= 0 { + return "", nil, 0, fmt.Errorf("invalid ?depth=%d: must be > 0", n) + } + depth = n + } + q.Del("depth") + + // Strip only the Fleet-specific params from the raw query string, + // preserving the encoding and ordering of all other parameters. + // url.Values.Encode() would reorder keys and normalise escaping, which + // breaks presigned or otherwise byte-stable URLs that contain non-Fleet + // params (e.g. ?token=ab+cd%2Fef must survive unchanged). + if u.RawQuery != "" { + var kept []string + for pair := range strings.SplitSeq(u.RawQuery, "&") { + if pair == "" { + continue + } + rawKey, _, _ := strings.Cut(pair, "=") + key, _ := url.QueryUnescape(rawKey) + switch key { + case "ref", "sshkey", "depth": + // Fleet-only params; already extracted above. + default: + kept = append(kept, pair) + } + } + u.RawQuery = strings.Join(kept, "&") + } + return ref, sshKey, depth, nil +} + +// resetDir removes path if it exists and recreates it as an empty directory. +func resetDir(path string) error { + if err := os.RemoveAll(path); err != nil { + return err + } + return os.MkdirAll(path, 0750) +} diff --git a/internal/bundlereader/gitclone_test.go b/internal/bundlereader/gitclone_test.go new file mode 100644 index 0000000000..b705f7334c --- /dev/null +++ b/internal/bundlereader/gitclone_test.go @@ -0,0 +1,119 @@ +package bundlereader + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "net" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// newSelfSignedTLSServer returns an HTTPS test server with a freshly generated +// self-signed certificate and the PEM-encoded certificate. +// Every call returns a unique cert so that different servers can be told apart. +func newSelfSignedTLSServer(t *testing.T) (*httptest.Server, []byte) { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "test"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1)}, + } + certDER, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + require.NoError(t, err) + + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) + + keyDER, err := x509.MarshalECPrivateKey(key) + require.NoError(t, err) + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) + + tlsCert, err := tls.X509KeyPair(certPEM, keyPEM) + require.NoError(t, err) + + srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + srv.TLS = &tls.Config{Certificates: []tls.Certificate{tlsCert}} + srv.StartTLS() + t.Cleanup(srv.Close) + + return srv, certPEM +} + +// TestGitDownloadCABundle verifies TLS certificate handling for gitDownload: +// +// - No CA bundle: TLS fails because the server's self-signed cert is not in +// the system pool. +// - Correct CA bundle: TLS handshake succeeds; only a git-protocol error +// follows (the httptest server does not speak git HTTP protocol). +// - Valid but mismatched CA bundle: providing a valid cert from a different +// server still fails TLS because the target's cert is trusted by neither the +// mismatched cert's CA nor the system pool. +// - Invalid CA bundle: rejected immediately before any TLS attempt. +// +// Each server uses a freshly generated self-signed certificate so that the +// test is independent of any system CA store. +func TestGitDownloadCABundle(t *testing.T) { + t.Parallel() + + // srv is the target clone server. + srv, srvCertPEM := newSelfSignedTLSServer(t) + + // otherSrv is a second TLS server whose cert we use as a "wrong" CA bundle. + // Because each call to newSelfSignedTLSServer generates a unique key pair, + // srvCertPEM != otherCertPEM. + _, otherCertPEM := newSelfSignedTLSServer(t) + + t.Run("no CA bundle fails with TLS error", func(t *testing.T) { + t.Parallel() + dst := t.TempDir() + err := gitDownload(context.Background(), dst, srv.URL, Auth{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "certificate") + }) + + t.Run("correct CA bundle gets past TLS", func(t *testing.T) { + t.Parallel() + dst := t.TempDir() + err := gitDownload(context.Background(), dst, srv.URL, Auth{CABundle: srvCertPEM}) + require.Error(t, err) + // TLS succeeded; expect a git-protocol error, not a TLS error. + assert.NotContains(t, err.Error(), "certificate") + }) + + t.Run("valid but mismatched CA bundle fails with TLS error", func(t *testing.T) { + t.Parallel() + dst := t.TempDir() + // otherCertPEM is a valid PEM cert from a different server. + // Neither it nor the system pool covers srv's cert, so TLS must fail. + err := gitDownload(context.Background(), dst, srv.URL, Auth{CABundle: otherCertPEM}) + require.Error(t, err) + assert.Contains(t, err.Error(), "certificate") + }) + + t.Run("invalid CA bundle is rejected immediately", func(t *testing.T) { + t.Parallel() + dst := t.TempDir() + err := gitDownload(context.Background(), dst, srv.URL, Auth{CABundle: []byte("not-a-cert")}) + require.Error(t, err) + assert.Contains(t, err.Error(), "no valid PEM certificates") + }) +} diff --git a/internal/bundlereader/httpdownload.go b/internal/bundlereader/httpdownload.go new file mode 100644 index 0000000000..35f8753cec --- /dev/null +++ b/internal/bundlereader/httpdownload.go @@ -0,0 +1,151 @@ +package bundlereader + +import ( + "bytes" + "context" + "crypto/md5" //nolint:gosec // md5 is only used as a data-integrity check per user-supplied ?checksum=, not for security + "crypto/sha1" //nolint:gosec // sha1 is only used as a data-integrity check per user-supplied ?checksum=, not for security + "crypto/sha256" + "crypto/sha512" + "encoding/hex" + "errors" + "fmt" + "hash" + "io" + "net/http" + "net/url" + "os" + "strings" +) + +// httpDownload downloads src to dst. +// +// Supported archive extensions (extracted into dst as a directory): +// +// .tar.gz / .tgz, .tar.bz2 / .tbz2, .tar.zst / .tzst, .tar.xz / .txz, .tar, .gz, .bz2, .zst, .xz, .zip +// +// A plain URL with no recognised extension is written as a single file. +// +// go-getter query parameters handled: +// +// ?checksum=: verify the response body against the given hash +// (md5, sha1, sha256, sha512). The "file:" variant +// (hash fetched from a URL) is not supported; provide +// an inline hash instead. +// ?archive= override format detection (e.g. "tar.gz" when the +// URL path has no recognisable extension). +// +// Unrecognised query parameters are forwarded to the server unchanged. +func httpDownload(ctx context.Context, dst, src string, auth Auth) error { + u, err := url.Parse(src) + if err != nil { + return fmt.Errorf("parsing URL %q: %w", src, err) + } + + q := u.Query() + + // Parse and strip ?checksum= before sending the request. + var checksum *checksumSpec + if cv := q.Get("checksum"); cv != "" { + checksum, err = parseChecksumParam(cv) + if err != nil { + return fmt.Errorf("invalid ?checksum= in %q: %w", u.Redacted(), err) + } + q.Del("checksum") + } + + // Parse and strip ?archive= before sending the request. + archiveOverride := "" + if av := q.Get("archive"); av != "" { + if !strings.HasPrefix(av, ".") { + av = "." + av + } + archiveOverride = strings.ToLower(av) + q.Del("archive") + } + + u.RawQuery = q.Encode() + src = u.String() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, src, nil) + if err != nil { + return fmt.Errorf("building request for %q: %w", src, err) + } + if auth.Username != "" && auth.Password != "" { + req.SetBasicAuth(auth.Username, auth.Password) + } + + resp, err := getHTTPClient(auth).Do(req) + if err != nil { + return fmt.Errorf("downloading %q: %w", src, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("downloading %q: unexpected status %d", src, resp.StatusCode) + } + + if err := os.MkdirAll(dst, 0750); err != nil { + return err + } + + body := io.Reader(resp.Body) + if checksum != nil { + checksum.hash.Reset() + body = io.TeeReader(resp.Body, checksum.hash) + } + + if err := extractResponse(dst, u.Path, archiveOverride, body); err != nil { + return err + } + + if checksum != nil { + actual := checksum.hash.Sum(nil) + if !bytes.Equal(actual, checksum.expected) { + return fmt.Errorf("checksum mismatch for %q: expected %s:%x, got %x", + u.Redacted(), checksum.hashType, checksum.expected, actual) + } + } + return nil +} + +// checksumSpec holds the parsed ?checksum= parameter. +type checksumSpec struct { + hashType string + hash hash.Hash + expected []byte +} + +// parseChecksumParam parses a ?checksum=: value. +// +// Supported hash types: md5, sha1, sha256, sha512. +// The "file:" variant (hash fetched from a remote URL) is not supported; +// that would require a recursive HTTP fetch and credential forwarding with +// no meaningful security benefit over an inline hash. +func parseChecksumParam(v string) (*checksumSpec, error) { + typePart, valuePart, ok := strings.Cut(v, ":") + if !ok { + return nil, fmt.Errorf("must be in type:value format (e.g. sha256:), got %q", v) + } + if strings.EqualFold(typePart, "file") { + return nil, errors.New(`the "file:" checksum variant is not supported; provide an inline hash (e.g. sha256:)`) + } + expected, err := hex.DecodeString(valuePart) + if err != nil { + return nil, fmt.Errorf("hex value: %w", err) + } + spec := &checksumSpec{hashType: strings.ToLower(typePart), expected: expected} + switch spec.hashType { + case "md5": + spec.hash = md5.New() //nolint:gosec + case "sha1": + spec.hash = sha1.New() //nolint:gosec + case "sha256": + spec.hash = sha256.New() + case "sha512": + spec.hash = sha512.New() + default: + return nil, fmt.Errorf("unsupported hash type %q; use md5, sha1, sha256, or sha512", typePart) + } + return spec, nil +} diff --git a/internal/bundlereader/httpdownload_internal_test.go b/internal/bundlereader/httpdownload_internal_test.go new file mode 100644 index 0000000000..57f5ffef64 --- /dev/null +++ b/internal/bundlereader/httpdownload_internal_test.go @@ -0,0 +1,243 @@ +package bundlereader + +import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/gzip" + "crypto/sha256" + "encoding/hex" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestExtractTar_UnsupportedTypes verifies that extractTar returns an error +// for tar entry types that are not dir, regular file, or symlink. Hard links, +// device nodes, and FIFOs must be rejected to prevent unexpected side effects +// during archive extraction. Metadata-only entries (PAX headers, GNU long-name +// entries) are skipped rather than rejected, but Go's tar.Writer refuses to +// encode them, so that path can't be exercised via a synthetic archive here. +func TestExtractTar_UnsupportedTypes(t *testing.T) { + rejected := []struct { + name string + typeflag byte + }{ + {"hard link", tar.TypeLink}, + {"char device", tar.TypeChar}, + {"block device", tar.TypeBlock}, + {"FIFO", tar.TypeFifo}, + {"contiguous file", tar.TypeCont}, + } + for _, tc := range rejected { + t.Run(tc.name, func(t *testing.T) { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + require.NoError(t, tw.WriteHeader(&tar.Header{ + Typeflag: tc.typeflag, + Name: "entry", + Linkname: "target", + })) + require.NoError(t, tw.Close()) + + err := extractTar(t.TempDir(), &buf) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsupported tar entry type") + }) + } +} + +// TestExtractTar_SymlinkTraversalRejected verifies that a symlink pointing +// outside the extraction root is rejected. +func TestExtractTar_SymlinkTraversalRejected(t *testing.T) { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + require.NoError(t, tw.WriteHeader(&tar.Header{ + Typeflag: tar.TypeSymlink, + Name: "link", + Linkname: "../../etc/passwd", + })) + require.NoError(t, tw.Close()) + + err := extractTar(t.TempDir(), &buf) + require.Error(t, err) + assert.Contains(t, err.Error(), "escapes destination directory") +} + +// TestExtractZipFromReader verifies that extractZipFromReader correctly +// extracts a zip archive via the temp-file code path (not an in-memory buffer). +func TestExtractZipFromReader(t *testing.T) { + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + w, err := zw.Create("hello.txt") + require.NoError(t, err) + _, err = w.Write([]byte("hello world")) + require.NoError(t, err) + require.NoError(t, zw.Close()) + + dst := t.TempDir() + require.NoError(t, extractZipFromReader(dst, &buf)) + + data, err := os.ReadFile(filepath.Join(dst, "hello.txt")) + require.NoError(t, err) + assert.Equal(t, "hello world", string(data)) +} + +// TestExtractResponse_Bzip2Dispatch verifies that extractResponse routes +// .tar.bz2, .tbz2, and .bz2 files to the bzip2 extraction path. +// Since compress/bzip2 is read-only in the stdlib (no writer), we confirm +// dispatch by checking that invalid bzip2 content produces a bzip2 error +// rather than an "unsupported" or other unrelated error. +func TestExtractResponse_Bzip2Dispatch(t *testing.T) { + invalid := []byte("not bzip2") + for _, ext := range []string{".tar.bz2", ".tbz2", ".bz2"} { + t.Run(ext, func(t *testing.T) { + err := extractResponse(t.TempDir(), "archive"+ext, "", bytes.NewReader(invalid)) + require.Error(t, err) + // bzip2.NewReader returns an error on the first read when the magic + // bytes are wrong; the message contains "bzip2". + assert.Contains(t, err.Error(), "bzip2", "expected a bzip2 error for %s, got: %v", ext, err) + }) + } +} + +// TestExtractResponse_ZstdDispatch verifies that extractResponse routes +// .tar.zst, .tar.zstd, .tzst, .zst, and .zstd files to the zstd extraction path. +func TestExtractResponse_ZstdDispatch(t *testing.T) { + invalid := []byte("not zstd") + for _, ext := range []string{".tar.zst", ".tar.zstd", ".tzst", ".zst", ".zstd"} { + t.Run(ext, func(t *testing.T) { + err := extractResponse(t.TempDir(), "archive"+ext, "", bytes.NewReader(invalid)) + require.Error(t, err) + // zstd.NewReader returns an error when the magic bytes are wrong. + assert.Contains(t, err.Error(), "magic number mismatch", "expected a zstd error for %s, got: %v", ext, err) + }) + } +} + +// TestExtractResponse_XzDispatch verifies that extractResponse routes +// .tar.xz, .txz, and .xz files to the xz extraction path. +func TestExtractResponse_XzDispatch(t *testing.T) { + invalid := []byte("not xz") + for _, ext := range []string{".tar.xz", ".txz", ".xz"} { + t.Run(ext, func(t *testing.T) { + err := extractResponse(t.TempDir(), "archive"+ext, "", bytes.NewReader(invalid)) + require.Error(t, err) + assert.Contains(t, err.Error(), "xz", "expected an xz error for %s, got: %v", ext, err) + }) + } +} + +// TestParseChecksumParam covers valid and invalid ?checksum= inputs. +func TestParseChecksumParam(t *testing.T) { + tests := []struct { + name string + input string + wantErr string + }{ + {"sha256 valid", "sha256:2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824", ""}, + {"sha1 valid", "sha1:aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", ""}, + {"md5 valid", "md5:5d41402abc4b2a76b9719d911017c592", ""}, + {"sha512 valid", "sha512:9b71d224bd62f3785d96d46ad3ea3d73319bfbc2890caadae2dff72519673ca72323c3d99ba5c11d7c7acc6e14b8c5da0c4663475c2e5c3adef46f73bcdec043", ""}, + {"file variant", "file:http://example.com/sha.txt", "not supported"}, + {"no colon", "sha256deadbeef", "type:value format"}, + {"odd hex", "sha256:abc", "hex value"}, + {"unknown type", "md2:deadbeef", "unsupported hash type"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := parseChecksumParam(tc.input) + if tc.wantErr == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + } + }) + } +} + +// TestHttpDownload_ChecksumVerification checks that a matching ?checksum= +// parameter succeeds and that the server does not receive the checksum param. +func TestHttpDownload_ChecksumVerification(t *testing.T) { + body := []byte("hello world") + h := sha256.New() + h.Write(body) + hexHash := hex.EncodeToString(h.Sum(nil)) + + var receivedURL string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedURL = r.URL.String() + _, _ = w.Write(body) + })) + defer srv.Close() + + dst := t.TempDir() + err := httpDownload(t.Context(), dst, srv.URL+"/file.txt?checksum=sha256:"+hexHash+"&token=xyz", Auth{}) + require.NoError(t, err) + + assert.NotContains(t, receivedURL, "checksum", "?checksum= must not be forwarded to the server") + assert.Contains(t, receivedURL, "token=xyz", "non-checksum query params must be preserved") + + data, err := os.ReadFile(filepath.Join(dst, "file.txt")) + require.NoError(t, err) + assert.Equal(t, body, data) +} + +// TestHttpDownload_ChecksumMismatch verifies that a wrong ?checksum= value +// causes httpDownload to return a "checksum mismatch" error. +func TestHttpDownload_ChecksumMismatch(t *testing.T) { + body := []byte("hello world") + wrongHash := "0000000000000000000000000000000000000000000000000000000000000000" + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(body) + })) + defer srv.Close() + + dst := t.TempDir() + err := httpDownload(t.Context(), dst, srv.URL+"/file.txt?checksum=sha256:"+wrongHash, Auth{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "checksum mismatch") +} + +// TestHttpDownload_ArchiveOverride verifies that ?archive= forces format +// detection to use the given extension, allowing archives without a +// recognisable URL extension to be extracted correctly. +func TestHttpDownload_ArchiveOverride(t *testing.T) { + // Build a minimal tar.gz in memory. + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + content := []byte("override content") + require.NoError(t, tw.WriteHeader(&tar.Header{ + Typeflag: tar.TypeReg, + Name: "inner.txt", + Size: int64(len(content)), + Mode: 0600, + })) + _, err := tw.Write(content) + require.NoError(t, err) + require.NoError(t, tw.Close()) + require.NoError(t, gw.Close()) + archiveBytes := buf.Bytes() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(archiveBytes) + })) + defer srv.Close() + + dst := t.TempDir() + // URL has no recognisable extension; ?archive=tar.gz triggers extraction. + err = httpDownload(t.Context(), dst, srv.URL+"/download/bundle?archive=tar.gz", Auth{}) + require.NoError(t, err) + + data, err := os.ReadFile(filepath.Join(dst, "inner.txt")) + require.NoError(t, err) + assert.Equal(t, content, data) +} diff --git a/internal/bundlereader/loaddirectory.go b/internal/bundlereader/loaddirectory.go index aeb4736a5f..5c11b99f2c 100644 --- a/internal/bundlereader/loaddirectory.go +++ b/internal/bundlereader/loaddirectory.go @@ -3,20 +3,17 @@ package bundlereader import ( "bufio" "context" - "encoding/base64" "fmt" + "io" "io/fs" - "net/http" "net/url" "os" "path/filepath" "regexp" "slices" "strings" - "sync" "unicode/utf8" - "github.com/hashicorp/go-getter/v2" "helm.sh/helm/v4/pkg/downloader" helmgetter "helm.sh/helm/v4/pkg/getter" "helm.sh/helm/v4/pkg/registry" @@ -26,15 +23,6 @@ import ( fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" ) -var ( - gitHTTPSCloneMutex sync.Mutex -) - -// Getter abstracts the go-getter client for testing. -type Getter interface { - Get(ctx context.Context, req *getter.Request) (*getter.GetResult, error) -} - // ignoreTree represents a tree of ignored paths (read from .fleetignore files), each node being a directory. // It provides a means for ignored paths to be propagated down the tree, but not between subdirectories of a same // directory. @@ -202,7 +190,7 @@ func loadDirectory(ctx context.Context, opts loadOpts, dir directory) ([]fleet.B return resources, nil } -// GetContent uses go-getter (and Helm for OCI) to read the files from directories and servers. +// GetContent uses fetchToDir (and Helm for OCI) to read the files from directories and servers. func GetContent(ctx context.Context, base, source, version string, auth Auth, disableDepsUpdate bool, ignoreApplyConfigs []string) (map[string][]byte, error) { temp, err := os.MkdirTemp("", "fleet") if err != nil { @@ -212,9 +200,7 @@ func GetContent(ctx context.Context, base, source, version string, auth Auth, di orgSource := source - // go-getter does not support downloading OCI registry based files yet - // until this is implemented we use Helm to download charts from OCI based registries - // and provide the downloaded file to go-getter locally + // OCI registries are handled via Helm; the downloaded chart is then read locally. if strings.HasPrefix(source, ociURLPrefix) { source, err = downloadOCIChart(source, version, temp, auth) if err != nil { @@ -229,40 +215,8 @@ func GetContent(ctx context.Context, base, source, version string, auth Auth, di return nil, err } - if auth.SSHPrivateKey != nil { - if !strings.ContainsAny(source, "?") { - source += "?" - } else { - source += "&" - } - source += fmt.Sprintf("sshkey=%s", base64.StdEncoding.EncodeToString(auth.SSHPrivateKey)) - } - - customGetters := []getter.Getter{} - for _, g := range getter.Getters { - // Replace default HTTP(S) getter with our customized one - if _, ok := g.(*getter.HttpGetter); ok { - continue - } - customGetters = append(customGetters, g) - } - - httpGetter := newHttpGetter(auth) - customGetters = append(customGetters, httpGetter) - - client := &getter.Client{ - Getters: customGetters, - } - - req := &getter.Request{ - Src: source, - Dst: temp, - Pwd: base, - GetMode: getter.ModeDir, - } - - if err := get(ctx, client, req, auth); err != nil { - return nil, fmt.Errorf("retrieving file from %q: %w", source, err) + if err := fetchToDir(ctx, temp, source, base, auth); err != nil { + return nil, fmt.Errorf("retrieving file from %q: %w", orgSource, err) } files := map[string][]byte{} @@ -349,10 +303,9 @@ func downloadOCIChart(name, version, path string, auth Auth) (string, error) { } defer os.RemoveAll(temp) - tmpGetter := newHttpGetter(auth) clientOptions := []registry.ClientOption{ registry.ClientOptCredentialsFile(filepath.Join(temp, "creds.json")), - registry.ClientOptHTTPClient(tmpGetter.Client), + registry.ClientOptHTTPClient(getHTTPClient(auth)), } if auth.BasicHTTP { clientOptions = append(clientOptions, registry.ClientOptPlainHTTP()) @@ -418,75 +371,154 @@ func downloadOCIChart(name, version, path string, auth Auth) (string, error) { return saved, nil } -// get performs the actual get operation, handling the mutex and environment variables for git-over-HTTPS operations. -func get(ctx context.Context, client Getter, req *getter.Request, auth Auth) error { - if needsGitSSLEnvVars(req) { - gitHTTPSCloneMutex.Lock() - defer gitHTTPSCloneMutex.Unlock() +// safeJoinSubDir joins base and sub, returning an error if sub is absolute or +// escapes base (e.g., due to ".." components). +func safeJoinSubDir(base, sub string) (string, error) { + cleanSub := filepath.Clean(filepath.FromSlash(sub)) + if filepath.IsAbs(cleanSub) { + return "", fmt.Errorf("subdir must be relative, got %q", sub) + } + if cleanSub == "." { + return base, nil + } + joined := filepath.Join(base, cleanSub) + rel, err := filepath.Rel(base, joined) + if err != nil { + return "", err + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { + return "", fmt.Errorf("subdir %q escapes base directory", sub) + } + return joined, nil +} + +// fetchToDir resolves source (relative to pwd), downloads or copies it, and +// places the resulting files into dst. +func fetchToDir(ctx context.Context, dst, source, pwd string, auth Auth) error { + si, err := parseSource(source, pwd) + if err != nil { + return err } - if auth.CABundle != nil { - file, err := os.CreateTemp("", "cabundle-*") + // If there's a subdirectory, download into a temp directory then move the + // subdirectory into dst. + if si.subDir != "" { + // For local directory sources, compose the subdir path directly without + // a temp directory. fetchScheme would try os.Symlink(rawURL, td) which + // fails because td already exists as a directory from os.MkdirTemp. + if si.scheme == "local" { + fi, err := os.Stat(si.rawURL) + if err != nil { + return err + } + if fi.IsDir() { + src, err := safeJoinSubDir(si.rawURL, si.subDir) + if err != nil { + return fmt.Errorf("invalid subdir %q: %w", si.subDir, err) + } + if err := os.MkdirAll(dst, 0750); err != nil { + return err + } + return copyDir(src, dst) + } + // Local file (e.g. an archive): fall through to the temp-dir path. + } + + td, err := os.MkdirTemp("", "fleet-subdir") if err != nil { return err } - defer func() { - file.Close() - os.Remove(file.Name()) - }() + defer os.RemoveAll(td) - if _, err := file.Write(auth.CABundle); err != nil { + if err := fetchScheme(ctx, td, si.scheme, si.rawURL, auth); err != nil { return err } - os.Setenv("GIT_SSL_CAINFO", file.Name()) - defer os.Unsetenv("GIT_SSL_CAINFO") - } - - if auth.InsecureSkipVerify { - os.Setenv("GIT_SSL_NO_VERIFY", "true") - defer os.Unsetenv("GIT_SSL_NO_VERIFY") + src, err := safeJoinSubDir(td, si.subDir) + if err != nil { + return fmt.Errorf("invalid subdir %q: %w", si.subDir, err) + } + if err := os.MkdirAll(dst, 0750); err != nil { + return err + } + if err := copyDir(src, dst); err != nil { + return fmt.Errorf("copying subdir %q: %w", si.subDir, err) + } + return nil } - _, err := client.Get(ctx, req) - return err + return fetchScheme(ctx, dst, si.scheme, si.rawURL, auth) } -// needsGitSSLEnvVars checks whether the request will use git over HTTPS with custom TLS settings that require -// environment variables (and thus mutex protection). -func needsGitSSLEnvVars(req *getter.Request) bool { - src := req.Src - - // Check for explicit git::https:// prefix - if strings.HasPrefix(src, "git::https://") { - return true - } - - // Check for shorthand URLs that go-getter's {BitBucket,GitHub,GitLab}Detector will transform to git::https:// - // internally. The GitGetter.Detect() method strips the "git::" prefix when storing back to req.Src, so we need to - // detect these patterns directly. These patterns match what go-getter's detectors recognize. - if strings.HasPrefix(src, "github.com/") || strings.HasPrefix(src, "gitlab.com/") || strings.HasPrefix(src, "bitbucket.org/") { - return true +func fetchScheme(ctx context.Context, dst, scheme, rawURL string, auth Auth) error { + switch scheme { + case "git": + return gitDownload(ctx, dst, rawURL, auth) + case "http": + return httpDownload(ctx, dst, rawURL, auth) + case "local": + fi, err := os.Stat(rawURL) + if err != nil { + return err + } + if fi.IsDir() { + // Mirror go-getter's FileGetter behaviour: create a symlink at dst + // pointing to rawURL so that WalkDir and helmupdater operate on the + // original directory (with writes going back to the source). + // GetContent's os.Readlink call then resolves dst to the source path. + return os.Symlink(rawURL, dst) + } + // A single file (e.g. an OCI-downloaded .tgz chart) — extract into dst. + if err := os.MkdirAll(dst, 0750); err != nil { + return err + } + f, err := os.Open(rawURL) + if err != nil { + return err + } + defer f.Close() + return extractResponse(dst, filepath.Base(rawURL), "", f) + default: + return fmt.Errorf("unsupported scheme %q", scheme) } - - return false } -func newHttpGetter(auth Auth) *getter.HttpGetter { - httpGetter := &getter.HttpGetter{ - Client: getHTTPClient(auth), - } - - if auth.Username != "" && auth.Password != "" { - header := http.Header{} - header.Add("Authorization", "Basic "+basicAuth(auth.Username, auth.Password)) - httpGetter.Header = header - } - - return httpGetter -} +// copyDir recursively copies the content of src into dst. +func copyDir(src, dst string) error { + return filepath.WalkDir(src, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + rel, err := filepath.Rel(src, path) + if err != nil { + return err + } + target := filepath.Join(dst, rel) + if d.IsDir() { + return os.MkdirAll(target, 0750) + } + // Reject symlinks: following them could copy data from outside the + // cloned tree if the repository contains malicious symlinks. + if d.Type()&os.ModeSymlink != 0 { + return fmt.Errorf("copyDir: symlink not supported: %s", path) + } + info, err := d.Info() + if err != nil { + return err + } + //nolint:gosec // G304: path is derived from a go-git controlled temp directory + srcFile, err := os.Open(path) + if err != nil { + return err + } + defer srcFile.Close() -func basicAuth(username, password string) string { - auth := username + ":" + password - return base64.StdEncoding.EncodeToString([]byte(auth)) + dstFile, err := os.OpenFile(target, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, info.Mode().Perm()) + if err != nil { + return err + } + defer dstFile.Close() + _, err = io.Copy(dstFile, srcFile) + return err + }) } diff --git a/internal/bundlereader/loaddirectory_internal_test.go b/internal/bundlereader/loaddirectory_internal_test.go index 81701272a1..d36d127746 100644 --- a/internal/bundlereader/loaddirectory_internal_test.go +++ b/internal/bundlereader/loaddirectory_internal_test.go @@ -1,207 +1,426 @@ package bundlereader import ( - "context" - "sync" + "encoding/base64" + "net/url" "testing" - "time" - "github.com/hashicorp/go-getter/v2" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -// mockGetter is a test double for the Getter interface that allows -// controlling timing to test synchronization behavior. -type mockGetter struct { - inGet chan struct{} // signals when Get is entered - canFinish chan struct{} // signals when Get can return +func TestSplitForcedScheme(t *testing.T) { + tests := []struct { + name string + src string + wantScheme string + wantStripped string + }{ + {"git:: prefix", "git::https://example.com/repo.git", "git", "https://example.com/repo.git"}, + {"ssh:: prefix", "ssh::git@example.com:org/repo", "ssh", "git@example.com:org/repo"}, + {"http:: prefix", "http::http://example.com/file.tar.gz", "http", "http://example.com/file.tar.gz"}, + {"no prefix", "https://example.com/repo.git", "", "https://example.com/repo.git"}, + {"local path", "/some/local/path", "", "/some/local/path"}, + {"unknown prefix is ignored", "foo::https://example.com", "", "foo::https://example.com"}, + {"no double colons", "https://example.com/repo", "", "https://example.com/repo"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme, stripped := splitForcedScheme(tt.src) + assert.Equal(t, tt.wantScheme, scheme) + assert.Equal(t, tt.wantStripped, stripped) + }) + } } -func newMockGetter() *mockGetter { - return &mockGetter{ - inGet: make(chan struct{}, 1), - canFinish: make(chan struct{}), +func TestSplitSubdir(t *testing.T) { + tests := []struct { + name string + src string + wantBase string + wantSubdir string + }{ + { + name: "no subdir", + src: "https://host/repo.git", + wantBase: "https://host/repo.git", + wantSubdir: "", + }, + { + name: "subdir without query string", + src: "https://host/repo.git//path/to/dir", + wantBase: "https://host/repo.git", + wantSubdir: "path/to/dir", + }, + { + name: "subdir with query string on subdir part", + src: "https://host/repo.git//subdir?ref=main", + wantBase: "https://host/repo.git?ref=main", + wantSubdir: "subdir", + }, + { + name: "no subdir but has query string", + src: "https://host/repo?ref=main", + wantBase: "https://host/repo?ref=main", + wantSubdir: "", + }, + { + name: "git:: prefix stripped, subdir present", + src: "https://host/repo//charts", + wantBase: "https://host/repo", + wantSubdir: "charts", + }, + { + name: "scheme separator does not count as subdir marker", + src: "ssh://git@host/repo", + wantBase: "ssh://git@host/repo", + wantSubdir: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + base, subdir := splitSubdir(tt.src) + assert.Equal(t, tt.wantBase, base, "base URL") + assert.Equal(t, tt.wantSubdir, subdir, "subdir") + }) } } -func (g *mockGetter) Get(ctx context.Context, req *getter.Request) (*getter.GetResult, error) { - g.inGet <- struct{}{} // signal that we're inside Get - <-g.canFinish // wait for permission to finish - return &getter.GetResult{}, nil +func TestParseSource_ForcedGetters(t *testing.T) { + tests := []struct { + name string + src string + wantScheme string + wantURL string + }{ + { + name: "git::https URL resolves to git scheme", + src: "git::https://example.com/repo.git", + wantScheme: "git", + wantURL: "https://example.com/repo.git", + }, + { + name: "git::ssh URL resolves to git scheme", + src: "git::ssh://git@example.com/repo.git", + wantScheme: "git", + wantURL: "ssh://git@example.com/repo.git", + }, + { + name: "ssh::ssh URL resolves to git scheme", + src: "ssh::ssh://git@example.com/repo.git", + wantScheme: "git", + wantURL: "ssh://git@example.com/repo.git", + }, + { + name: "http:: prefix resolves to http scheme", + src: "http::https://example.com/file.tar.gz", + wantScheme: "http", + wantURL: "https://example.com/file.tar.gz", + }, + { + name: "git:: with SCP-style SSH normalizes to ssh:// URL", + src: "git::git@github.com:org/repo", + wantScheme: "git", + wantURL: "ssh://git@github.com/org/repo", + }, + { + name: "ssh:: with non-git user normalizes to ssh:// URL", + src: "ssh::alice@example.com:org/repo", + wantScheme: "git", + wantURL: "ssh://alice@example.com/org/repo", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseSource(tt.src, "") + require.NoError(t, err) + assert.Equal(t, tt.wantScheme, got.scheme) + assert.Equal(t, tt.wantURL, got.rawURL) + }) + } } -func TestGetMutexForGitHTTPS(t *testing.T) { - mg := newMockGetter() - ctx := context.Background() - - // Auth with CABundle triggers mutex acquisition for git::https:// URLs - auth := Auth{CABundle: []byte("test-ca-bundle")} - - // Request that will be detected as git::https:// - req := &getter.Request{Src: "git::https://example.com/repo.git"} - - var wg sync.WaitGroup - wg.Add(2) - - // Start first call - go func() { - defer wg.Done() - _ = get(ctx, mg, req, auth) - }() - - // Wait for first call to enter Get (it now holds the mutex) - <-mg.inGet - - // Start second call - secondEntered := make(chan struct{}) - go func() { - defer wg.Done() - close(secondEntered) // signal that goroutine started - _ = get(ctx, mg, req, auth) - }() - - // Wait for second goroutine to start - <-secondEntered - - // Give second goroutine time to potentially enter Get (if mutex wasn't working) - time.Sleep(50 * time.Millisecond) - - // Verify second call hasn't entered Get yet (should be blocked on mutex) - select { - case <-mg.inGet: - t.Fatal("second call entered Get while first still running - mutex not working!") - default: - // Good - second call is blocked as expected +func TestParseSource_SchemeURLs(t *testing.T) { + tests := []struct { + name string + src string + wantScheme string + }{ + {"ssh:// scheme is git", "ssh://git@example.com/repo.git", "git"}, + {"git:// scheme is git", "git://example.com/repo.git", "git"}, + {"https:// scheme without forced is http", "https://example.com/file.tar.gz", "http"}, + {"http:// scheme is http", "http://example.com/file.tar.gz", "http"}, } - - // Let first call finish - mg.canFinish <- struct{}{} - - // Now second call should enter Get - select { - case <-mg.inGet: - // Good - second call entered after first finished - case <-time.After(1 * time.Second): - t.Fatal("second call never entered Get after first finished") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseSource(tt.src, "") + require.NoError(t, err) + assert.Equal(t, tt.wantScheme, got.scheme) + }) } - - // Let second call finish - mg.canFinish <- struct{}{} - - // Wait for both goroutines to complete - wg.Wait() } -func TestGetNoMutexForNonGitHTTPS(t *testing.T) { - ctx := context.Background() - - // Auth without CABundle or InsecureSkipVerify - no mutex needed - auth := Auth{} - - // Regular HTTP request (not git::https://) - req := &getter.Request{Src: "https://example.com/file.tar.gz"} - - // Create two mock getters to track concurrent access - var concurrentCalls int - var mu sync.Mutex - var maxConcurrent int - - concurrentGetter := &concurrencyTrackingGetter{ - onGet: func() { - mu.Lock() - concurrentCalls++ - if concurrentCalls > maxConcurrent { - maxConcurrent = concurrentCalls +func TestParseSource_Shorthands(t *testing.T) { + tests := []struct { + name string + src string + wantScheme string + wantOK bool + }{ + { + name: "github.com shorthand is git", + src: "github.com/foo/bar", + wantScheme: "git", + wantOK: true, + }, + { + name: "gitlab.com shorthand is git", + src: "gitlab.com/foo/bar", + wantScheme: "git", + wantOK: true, + }, + { + name: "bitbucket.org shorthand is git", + src: "bitbucket.org/foo/bar", + wantScheme: "git", + wantOK: true, + }, + { + name: "SCP SSH shorthand is git", + src: "git@example.com:org/repo", + wantScheme: "git", + wantOK: true, + }, + { + name: "SCP SSH shorthand with non-git user is git", + src: "alice@example.com:org/repo", + wantScheme: "git", + wantOK: true, + }, + { + name: "unknown host falls back to local", + src: "example.com/foo/bar", + wantScheme: "local", + wantOK: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseSource(tt.src, "") + require.NoError(t, err) + assert.Equal(t, tt.wantScheme, got.scheme) + if tt.wantOK { + assert.NotEmpty(t, got.rawURL) } - mu.Unlock() - - time.Sleep(50 * time.Millisecond) + }) + } +} - mu.Lock() - concurrentCalls-- - mu.Unlock() +func TestParseSource_SubdirExtraction(t *testing.T) { + tests := []struct { + name string + src string + wantSubdir string + wantURL string + }{ + { + name: "git::https with //subdir", + src: "git::https://host/repo.git//charts", + wantSubdir: "charts", + wantURL: "https://host/repo.git", + }, + { + name: "git::https with //subdir and ?ref=", + src: "git::https://host/repo.git//charts?ref=main", + wantSubdir: "charts", + wantURL: "https://host/repo.git?ref=main", + }, + { + name: "github.com shorthand with subpath becomes subdir", + src: "github.com/foo/bar/deploy", + wantSubdir: "deploy", + }, + { + name: "no subdir", + src: "git::https://host/repo.git", + wantSubdir: "", }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseSource(tt.src, "") + require.NoError(t, err) + assert.Equal(t, tt.wantSubdir, got.subDir, "subdir") + if tt.wantURL != "" { + assert.Equal(t, tt.wantURL, got.rawURL, "rawURL") + } + }) + } +} - var wg sync.WaitGroup - wg.Add(2) - - go func() { - defer wg.Done() - _ = get(ctx, concurrentGetter, req, auth) - }() - - go func() { - defer wg.Done() - _ = get(ctx, concurrentGetter, req, auth) - }() - - wg.Wait() +func TestParseSource_EmptySource(t *testing.T) { + got, err := parseSource("", "/my/pwd") + require.NoError(t, err) + assert.Equal(t, "local", got.scheme) + assert.Equal(t, "/my/pwd", got.rawURL) +} - // Without mutex protection, both calls should run concurrently - assert.Equal(t, 2, maxConcurrent, "expected concurrent execution when mutex is not needed") +func TestParseSource_LocalRelativePaths(t *testing.T) { + tests := []struct { + name string + src string + pwd string + wantURL string + }{ + {"dot resolves to pwd", ".", "/my/pwd", "/my/pwd"}, + {"relative path joined with pwd", "charts", "/my/pwd", "/my/pwd/charts"}, + {"absolute path unchanged", "/abs/path", "/my/pwd", "/abs/path"}, + {"empty pwd leaves relative as-is", "charts", "", "charts"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseSource(tt.src, tt.pwd) + require.NoError(t, err) + assert.Equal(t, "local", got.scheme) + assert.Equal(t, tt.wantURL, got.rawURL) + }) + } } -// concurrencyTrackingGetter tracks concurrent calls to Get -type concurrencyTrackingGetter struct { - onGet func() +func TestSafeJoinSubDir(t *testing.T) { + tests := []struct { + name string + base string + sub string + want string + wantErr bool + }{ + {"normal subdir", "/base", "charts", "/base/charts", false}, + {"nested subdir", "/base", "a/b/c", "/base/a/b/c", false}, + {"dot returns base", "/base", ".", "/base", false}, + {"absolute path rejected", "/base", "/etc/passwd", "", true}, + {"single traversal rejected", "/base", "../etc", "", true}, + {"deep traversal rejected", "/base", "charts/../../etc", "", true}, + {"traversal in nested path", "/base", "a/../../../etc", "", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := safeJoinSubDir(tt.base, tt.sub) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } } -func (g *concurrencyTrackingGetter) Get(ctx context.Context, req *getter.Request) (*getter.GetResult, error) { - if g.onGet != nil { - g.onGet() +func TestSafeJoin(t *testing.T) { + tests := []struct { + name string + base string + input string + want string + wantErr bool + }{ + {"plain file", "/base", "file.txt", "/base/file.txt", false}, + {"nested path", "/base", "sub/dir/file.txt", "/base/sub/dir/file.txt", false}, + // Traversal attempts are neutralised by prepending "/" before filepath.Clean, + // placing the result safely inside base rather than rejecting it. + {"path traversal sanitised", "/base", "../etc/passwd", "/base/etc/passwd", false}, + {"deep traversal sanitised", "/base", "sub/../../etc/passwd", "/base/etc/passwd", false}, + // An absolute path inside an archive is rewritten safely within base. + {"absolute treated as relative", "/base", "/etc/passwd", "/base/etc/passwd", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := safeJoin(tt.base, tt.input) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.want, got) + } + }) } - return &getter.GetResult{}, nil } -func TestNeedsGitSSLEnvVars(t *testing.T) { +func TestExtractQueryParams(t *testing.T) { + keyBytes := []byte("fake-ssh-key") + keyB64 := base64.RawURLEncoding.EncodeToString(keyBytes) + tests := []struct { - name string - src string - expected bool + name string + rawURL string + wantRef string + wantKey []byte + wantDepth int + wantErr bool + wantRawQuery string // if non-empty, u.RawQuery must equal this after extraction }{ { - name: "git https needs env vars", - src: "git::https://github.com/example/repo.git", - expected: true, + name: "no params", + rawURL: "https://host/repo.git", + wantRef: "", }, { - name: "regular https does not need env vars", - src: "https://example.com/file.tar.gz", - expected: false, + name: "ref only", + rawURL: "https://host/repo.git?ref=main", + wantRef: "main", }, { - name: "git ssh does not need env vars", - src: "git::ssh://git@github.com/example/repo.git", - expected: false, + name: "depth only", + rawURL: "https://host/repo.git?depth=5", + wantDepth: 5, }, { - name: "github.com shorthand needs env vars", - src: "github.com/foo/bar", - expected: true, + name: "sshkey only", + rawURL: "https://host/repo.git?sshkey=" + keyB64, + wantKey: keyBytes, }, { - name: "gitlab.com shorthand needs env vars", - src: "gitlab.com/foo/bar", - expected: true, + name: "all params", + rawURL: "https://host/repo.git?ref=v1.0&depth=3&sshkey=" + keyB64, + wantRef: "v1.0", + wantDepth: 3, + wantKey: keyBytes, }, { - name: "bitbucket.org shorthand needs env vars", - src: "bitbucket.org/foo/bar", - expected: true, + name: "invalid sshkey base64 returns error", + rawURL: "https://host/repo.git?sshkey=!!!invalid!!!", + wantErr: true, }, { - name: "unknown domain without protocol does not need env vars", - src: "example.com/foo/bar", - expected: false, + // Non-Fleet params must survive with their original encoding and + // ordering; url.Values.Encode() would reorder and re-encode them. + name: "non-Fleet params preserve original encoding and ordering", + rawURL: "https://host/repo.git?token=ab+cd%2Fef&ref=main&extra=1%202", + wantRef: "main", + wantRawQuery: "token=ab+cd%2Fef&extra=1%202", }, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - req := &getter.Request{Src: tt.src} - result := needsGitSSLEnvVars(req) - assert.Equal(t, tt.expected, result) + u, err := url.Parse(tt.rawURL) + require.NoError(t, err) + + ref, key, depth, err := extractQueryParams(u) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantRef, ref, "ref") + assert.Equal(t, tt.wantKey, key, "sshKey") + assert.Equal(t, tt.wantDepth, depth, "depth") + // Fleet params must be stripped from the URL. + assert.Empty(t, u.Query().Get("ref")) + assert.Empty(t, u.Query().Get("sshkey")) + assert.Empty(t, u.Query().Get("depth")) + if tt.wantRawQuery != "" { + assert.Equal(t, tt.wantRawQuery, u.RawQuery, "rawQuery") + } }) } } diff --git a/internal/bundlereader/loaddirectory_test.go b/internal/bundlereader/loaddirectory_test.go index 291f501390..240038638a 100644 --- a/internal/bundlereader/loaddirectory_test.go +++ b/internal/bundlereader/loaddirectory_test.go @@ -3,8 +3,6 @@ package bundlereader_test import ( "context" "io" - "net/http" - "net/http/httptest" "os" "path/filepath" "regexp" @@ -585,10 +583,7 @@ func TestGetContent(t *testing.T) { }, } - base, err := os.MkdirTemp("", "test-fleet") - require.NoError(t, err) - - defer os.RemoveAll(base) + base := t.TempDir() ignoreApplyConfigs := []string{"fleet.yaml", "chart/myvalues.yaml"} @@ -615,59 +610,6 @@ func TestGetContent(t *testing.T) { } } -type authTester struct { - t *testing.T - want string -} - -func (a *authTester) ServeHTTP(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet { - http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) - return - } - url, err := r.URL.Parse(r.URL.String()) - if err != nil { - http.Error(w, "Invalid URL", http.StatusBadRequest) - return - } - if sskey := url.Query().Get("sshkey"); sskey != a.want { - a.t.Errorf("wrong or no sshkey query parameter: want %s but got %s", a.want, sskey) - } - w.WriteHeader(http.StatusOK) -} - -func TestGetContentSSHKey(t *testing.T) { - cases := []struct { - name, want string - auth bundlereader.Auth - }{ - { - name: "any URL with SSHPrivateKey set should be queried with sshkey query parameter", - auth: bundlereader.Auth{ - SSHPrivateKey: []byte("foo"), - }, - want: "Zm9v", // base64 encoding of "foo" - }, - { - name: "no query parameter if SSHPrivateKey is not set", - auth: bundlereader.Auth{}, - }, - } - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - authTester := &authTester{t: t, want: c.want} - s := httptest.NewServer(authTester) - defer s.Close() - - base, err := os.MkdirTemp("", "test-fleet") - require.NoError(t, err) - defer os.RemoveAll(base) - - _, _ = bundlereader.GetContent(context.Background(), base, s.URL, "", c.auth, false, []string{}) - }) - } -} - func TestGetContentOCI(t *testing.T) { cases := []struct { name string @@ -733,10 +675,7 @@ func TestGetContentOCI(t *testing.T) { assert := assert.New(t) - base, err := os.MkdirTemp("", "test-fleet") - require.NoError(t, err) - - defer os.RemoveAll(base) + base := t.TempDir() for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/internal/bundlereader/source.go b/internal/bundlereader/source.go new file mode 100644 index 0000000000..31d04f93fe --- /dev/null +++ b/internal/bundlereader/source.go @@ -0,0 +1,245 @@ +package bundlereader + +import ( + "fmt" + "net/url" + "path/filepath" + "strings" +) + +// sourceInfo describes a fully-resolved source for GetContent. +type sourceInfo struct { + // scheme is the resolved scheme: "git", "http", or "local". + scheme string + // rawURL is the URL or local path to fetch (forced-scheme prefix and "//" removed). + rawURL string + // subDir is the subdirectory to copy out of the downloaded content (from the "//" notation). + subDir string +} + +// parseSource resolves a source string into a sourceInfo. +// +// It recognises: +// - forced-scheme prefixes: "git::https://...", "ssh::...", "http::" +// - the "//" subdirectory separator: "git::https://host/repo.git//path/to/dir" +// - shorthand URLs: "github.com/user/repo", "gitlab.com/user/repo" +// - SCP-style SSH: "git@host:org/repo" +// +// Anything that cannot be resolved to an HTTP or git URL is treated as a local path. +func parseSource(src, pwd string) (sourceInfo, error) { + if src == "" { + return sourceInfo{scheme: "local", rawURL: pwd}, nil + } + + // 1. Strip forced-scheme prefix (e.g. "git::", "ssh::", "http::"). + forcedScheme, stripped := splitForcedScheme(src) + + // 2. Extract the "//" subdirectory separator from the stripped URL. + cleanSrc, subDir := splitSubdir(stripped) + + // 3. A forced getter scheme determines the handler directly. + switch forcedScheme { + case "git", "ssh": + // Normalise SCP-style SSH (e.g. "git@host:org/repo" or "user@host:org/repo") + // so that downstream url.Parse works correctly. + return sourceInfo{scheme: "git", rawURL: normalizeSCPToSSH(cleanSrc), subDir: subDir}, nil + case "http", "https": + return sourceInfo{scheme: "http", rawURL: cleanSrc, subDir: subDir}, nil + } + + // 4. Parse the URL scheme. + if u, err := url.Parse(cleanSrc); err == nil && u.Scheme != "" { + switch u.Scheme { + case "git", "ssh": + return sourceInfo{scheme: "git", rawURL: cleanSrc, subDir: subDir}, nil + case "http", "https": + return sourceInfo{scheme: "http", rawURL: cleanSrc, subDir: subDir}, nil + } + } + + // 5. Try shorthand detectors (GitHub, GitLab, SCP-SSH). + if detected, ok, err := detectShorthand(cleanSrc); err != nil { + return sourceInfo{}, fmt.Errorf("detecting source URL: %w", err) + } else if ok { + // detected is "git::..." — recurse to normalise, then merge subdirs. + info, err := parseSource(detected, pwd) + if err != nil { + return sourceInfo{}, err + } + // Outer subDir appended after any subDir extracted by the detector. + if subDir != "" { + if info.subDir != "" { + info.subDir = info.subDir + "/" + subDir + } else { + info.subDir = subDir + } + } + return info, nil + } + + // 6. Fall back to local path. Resolve relative paths against pwd so that the + // caller does not need to track the working directory separately. + rawPath := cleanSrc + if !filepath.IsAbs(rawPath) && pwd != "" { + rawPath = filepath.Join(pwd, rawPath) + } + return sourceInfo{scheme: "local", rawURL: rawPath, subDir: subDir}, nil +} + +// splitForcedScheme splits a "scheme::url" string into the forced scheme and the rest. +// Recognised schemes are "git", "ssh", "http", and "https". Returns an empty scheme +// and the original src unchanged when no known forced-scheme prefix is present. +func splitForcedScheme(src string) (string, string) { + scheme, rest, ok := strings.Cut(src, "::") + if !ok { + return "", src + } + switch scheme { + case "git", "ssh", "http", "https": + return scheme, rest + } + return "", src +} + +// splitSubdir extracts the "//" subdirectory separator from src, mirroring +// go-getter's SourceDirSubdir semantics. +// +// "https://host/repo?ref=main" → ("https://host/repo?ref=main", "") +// "https://host/repo.git//subdir" → ("https://host/repo.git", "subdir") +// "https://host/repo.git//sub?ref=main" → ("https://host/repo.git?ref=main", "sub") +func splitSubdir(src string) (string, string) { + // Do not search inside the query string. + stop := len(src) + if idx := strings.Index(src, "?"); idx > -1 { + stop = idx + } + + // Skip past "://" to avoid treating the scheme separator as a subdir marker. + offset := 0 + if idx := strings.Index(src[:stop], "://"); idx > -1 { + offset = idx + 3 + } + + idx := strings.Index(src[offset:stop], "//") + if idx == -1 { + return src, "" + } + + idx += offset + subDir := src[idx+2:] + base := src[:idx] + + // Move any query string from the subdir portion back onto the base URL. + if qi := strings.Index(subDir, "?"); qi > -1 { + base += subDir[qi:] + subDir = subDir[:qi] + } + return base, subDir +} + +// detectShorthand runs the known shorthand detectors (GitHub, GitLab, BitBucket, +// SCP-SSH) against src. Returns the resolved URL string, true, nil on a match. +func detectShorthand(src string) (string, bool, error) { + if s, ok, err := detectGitHub(src); err != nil || ok { + return s, ok, err + } + if s, ok, err := detectGitLab(src); err != nil || ok { + return s, ok, err + } + if s, ok, err := detectBitBucket(src); err != nil || ok { + return s, ok, err + } + if s, ok := detectSCPSSH(src); ok { + return s, true, nil + } + return "", false, nil +} + +// detectGitHub turns "github.com/user/repo[/subpath]" into a git:: HTTPS URL. +func detectGitHub(src string) (string, bool, error) { + if !strings.HasPrefix(src, "github.com/") { + return "", false, nil + } + return buildGitHTTPS(src, "github.com") +} + +// detectGitLab turns "gitlab.com/user/repo[/subpath]" into a git:: HTTPS URL. +func detectGitLab(src string) (string, bool, error) { + if !strings.HasPrefix(src, "gitlab.com/") { + return "", false, nil + } + return buildGitHTTPS(src, "gitlab.com") +} + +// detectBitBucket turns "bitbucket.org/user/repo[/subpath]" into a git:: HTTPS URL. +// Bitbucket dropped Mercurial support in 2020; all repositories are git today, +// so the VCS type can be inferred without a live API call. +func detectBitBucket(src string) (string, bool, error) { + if !strings.HasPrefix(src, "bitbucket.org/") { + return "", false, nil + } + return buildGitHTTPS(src, "bitbucket.org") +} + +// buildGitHTTPS constructs "git::https://host/user/repo[.git][//subpath]" from +// "host/user/repo[/subpath]". The fourth path component and beyond become the +// subdirectory within the repo. +func buildGitHTTPS(src, host string) (string, bool, error) { + // parts: [host, user, repo, rest...] + parts := strings.SplitN(src, "/", 4) + if len(parts) < 3 { + return "", false, fmt.Errorf("%s URLs must have the form %s/user/repo", host, host) + } + + repoURL, err := url.Parse(fmt.Sprintf("https://%s/%s/%s", parts[0], parts[1], parts[2])) + if err != nil { + return "", true, fmt.Errorf("parsing %s URL: %w", host, err) + } + if !strings.HasSuffix(repoURL.Path, ".git") { + repoURL.Path += ".git" + } + + result := "git::" + repoURL.String() + if len(parts) == 4 && parts[3] != "" { + result += "//" + parts[3] + } + return result, true, nil +} + +// normalizeSCPToSSH converts SCP-style addresses like "user@host:path" to +// "ssh://user@host/path". Returns src unchanged if it is not SCP-style. +func normalizeSCPToSSH(src string) string { + if strings.Contains(src, "://") { + return src + } + userHost, path, ok := strings.Cut(src, ":") + if !ok || path == "" || !strings.Contains(userHost, "@") { + return src + } + return "ssh://" + userHost + "/" + strings.TrimPrefix(path, "/") +} + +// detectSCPSSH detects SCP-style SSH addresses ("user@host:org/repo") and converts +// them to "git::ssh://user@host/org/repo". Requires an explicit user ("@" present) +// to avoid false positives against local "drive:path" or "host:port" strings. +func detectSCPSSH(src string) (string, bool) { + if strings.Contains(src, "://") { + return "", false + } + userHost, path, ok := strings.Cut(src, ":") + if !ok || path == "" { + return "", false + } + path = strings.TrimPrefix(path, "/") + user, host, ok := strings.Cut(userHost, "@") + if !ok || user == "" { + return "", false + } + repoURL := &url.URL{ + Scheme: "ssh", + User: url.User(user), + Host: host, + Path: "/" + path, + } + return "git::" + repoURL.String(), true +} diff --git a/internal/cmd/cli/gitcloner/cloner.go b/internal/cmd/cli/gitcloner/cloner.go index 851e0da008..7865b6c099 100644 --- a/internal/cmd/cli/gitcloner/cloner.go +++ b/internal/cmd/cli/gitcloner/cloner.go @@ -10,9 +10,7 @@ import ( "github.com/go-git/go-git/v5/plumbing/protocol/packp/capability" "github.com/go-git/go-git/v5/plumbing/transport" httpgit "github.com/go-git/go-git/v5/plumbing/transport/http" - gossh "github.com/go-git/go-git/v5/plumbing/transport/ssh" "github.com/sirupsen/logrus" - "golang.org/x/crypto/ssh" "github.com/rancher/fleet/internal/cmd/cli/gitcloner/submodule" fleetgithub "github.com/rancher/fleet/internal/github" @@ -94,23 +92,16 @@ func cloneBranch(opts *GitCloner, auth transport.AuthMethod, caBundle []byte) er Tags: git.NoTags, ProxyOptions: fleetgit.ProxyOptsFromEnvironment(opts.Repo), }) - if err != nil { return fmt.Errorf("failed to clone main repo from branch %s: %w, skipping submodule clone", repo(opts), err) } - submoduleUpdateOptions := &git.SubmoduleUpdateOptions{ + return updateSubmodules(r, &git.SubmoduleUpdateOptions{ Init: true, RecurseSubmodules: git.DefaultSubmoduleRecursionDepth, Depth: 1, Auth: auth, - } - - if err := updateSubmodules(r, submoduleUpdateOptions); err != nil { - return err - } - - return nil + }) } func cloneRevision(opts *GitCloner, auth transport.AuthMethod, caBundle []byte) error { @@ -135,23 +126,16 @@ func cloneRevision(opts *GitCloner, auth transport.AuthMethod, caBundle []byte) if err != nil { return fmt.Errorf("failed to get filesystem worktree for %s: %w", repo(opts), err) } - if err := w.Checkout(&git.CheckoutOptions{Hash: *h}); err != nil { return fmt.Errorf("failed to checkout in worktree %s: %w", repo(opts), err) } - submoduleUpdateOptions := &git.SubmoduleUpdateOptions{ + return updateSubmodules(r, &git.SubmoduleUpdateOptions{ Init: true, RecurseSubmodules: git.DefaultSubmoduleRecursionDepth, Depth: 1, Auth: auth, - } - - if err := updateSubmodules(r, submoduleUpdateOptions); err != nil { - return err - } - - return nil + }) } func getCABundleFromFile(path string) ([]byte, error) { @@ -177,22 +161,15 @@ func createAuthFromOpts(opts *GitCloner) (transport.AuthMethod, error) { if err != nil { return nil, err } - auth, err := gossh.NewPublicKeys(gitURL.User.Username(), privateKey, "") - if err != nil { - return nil, err + username := "git" + if gitURL.User != nil && gitURL.User.Username() != "" { + username = gitURL.User.Username() } + var knownHostsData []byte if isKnownHostsSet { - knownHostsCallBack, err := fleetssh.CreateKnownHostsCallBack([]byte(knownHosts)) - if err != nil { - return nil, fmt.Errorf("could not create known_hosts callback: %w", err) - } - - auth.HostKeyCallback = knownHostsCallBack - } else { - //nolint:gosec // G106: Use of ssh InsecureIgnoreHostKey should be audited - this will run in an init-container, so there is no persistence - auth.HostKeyCallback = ssh.InsecureIgnoreHostKey() + knownHostsData = []byte(knownHosts) } - return auth, nil + return fleetssh.NewSSHPublicKeys(username, privateKey, knownHostsData) } if opts.GitHubAppID != 0 && opts.GitHubAppInstallation != 0 && opts.GitHubAppKeyFile != "" { diff --git a/pkg/git/netutils.go b/pkg/git/netutils.go index 73e7dc709e..a5a464c537 100644 --- a/pkg/git/netutils.go +++ b/pkg/git/netutils.go @@ -49,12 +49,16 @@ func GetAuthFromSecret(url string, creds *corev1.Secret, knownHosts string) (tra if err != nil { return nil, err } + username := "git" + if gitURL.User != nil && gitURL.User.Username() != "" { + username = gitURL.User.Username() + } // Prefer known_hosts from the secret; fall back to the cluster-wide value. knownHostsData := creds.Data["known_hosts"] if len(knownHostsData) == 0 { knownHostsData = []byte(knownHosts) } - auth, err := fleetssh.NewSSHPublicKeys(gitURL.User.Username(), creds.Data[corev1.SSHAuthPrivateKey], knownHostsData) + auth, err := fleetssh.NewSSHPublicKeys(username, creds.Data[corev1.SSHAuthPrivateKey], knownHostsData) if err != nil { return nil, err } From f1b39b7c6d24aa3749ed970f2853e0fc1153e316 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Mon, 23 Mar 2026 08:54:34 +0100 Subject: [PATCH 5/8] Forward SSH known-hosts and InsecureSkipTLSverify to fleet apply Pass FLEET_KNOWN_HOSTS and FLEET_INSECURE_SKIP_TLS_VERIFY through to the bundlereader.Auth so that fleet apply CLI and the GitOps reconciler honour the same TLS and SSH host-key settings as the rest of the pipeline. Default the SSH username to 'git' when the URL has no user component, matching the controller behaviour. Add tests for createAuthFromOpts and addAuthToOpts, and update the gitjob test to assert that InsecureSkipTLSverify is forwarded. --- internal/cmd/cli/apply.go | 103 +----------- internal/cmd/cli/apply_test.go | 151 +++--------------- internal/cmd/cli/gitcloner/cloner_test.go | 135 ++++++++++++++++ .../controller/gitops/reconciler/gitjob.go | 24 +-- .../gitops/reconciler/gitjob_test.go | 22 +-- 5 files changed, 168 insertions(+), 267 deletions(-) diff --git a/internal/cmd/cli/apply.go b/internal/cmd/cli/apply.go index f03ff58f6f..3237133e93 100644 --- a/internal/cmd/cli/apply.go +++ b/internal/cmd/cli/apply.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "os" - "strings" gogit "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" @@ -134,13 +133,6 @@ func (a *Apply) run(cmd *cobra.Command, args []string) error { ImagescanEnabled: a.ImagescanEnabled, } - knownHostsPath, err := writeTmpKnownHosts() - if err != nil { - return err - } - - defer os.RemoveAll(knownHostsPath) - if err := a.addAuthToOpts(&opts, os.ReadFile, a.HelmBasicHTTP, a.HelmInsecureSkipTLS); err != nil { return fmt.Errorf("adding auth to opts: %w", err) } @@ -170,13 +162,6 @@ func (a *Apply) run(cmd *cobra.Command, args []string) error { args = args[1:] } - restoreEnv, err := setEnv(knownHostsPath) - if err != nil { - return fmt.Errorf("setting git SSH command env var for known hosts: %w", err) - } - - defer restoreEnv() //nolint: errcheck // best-effort - ctx := cmd.Context() cfg := ctrl.GetConfigOrDie() client, err := client.New(cfg, client.Options{Scheme: scheme}) @@ -240,6 +225,9 @@ func (a *Apply) addAuthToOpts(opts *apply.Options, readFile readFile, helmBasicH opts.Auth.BasicHTTP = helmBasicHTTP opts.Auth.InsecureSkipVerify = helmInsecureSkipTLS + if raw := os.Getenv(ssh.KnownHostsEnvVar); raw != "" { + opts.Auth.SSHKnownHosts = []byte(raw) + } return nil } @@ -261,91 +249,6 @@ func currentCommit(dir string) string { // writeTmpKnownHosts creates a temporary file and writes known_hosts data to it, if such data is available from // environment variable `FLEET_KNOWN_HOSTS`. // It returns the name of the file and any error which may have happened while creating the file or writing to it. -func writeTmpKnownHosts() (string, error) { - knownHosts, isSet := os.LookupEnv(ssh.KnownHostsEnvVar) - if !isSet || knownHosts == "" { - return "", nil - } - - f, err := os.CreateTemp("", "known_hosts") - if err != nil { - return "", err - } - - knownHostsPath := f.Name() - - if err := os.WriteFile(knownHostsPath, []byte(knownHosts), 0600); err != nil { - return "", fmt.Errorf( - "failed to write value of %q env var to known_hosts file %s: %w", - ssh.KnownHostsEnvVar, - knownHostsPath, - err, - ) - } - - return knownHostsPath, nil -} - -// setEnv sets the `GIT_SSH_COMMAND` environment variable with a known_hosts flag pointing to the provided -// knownHostsPath. It takes care of preserving existing flags in the existing value of the environment variable, if any, -// except for other user known_hosts file flags. -// It returns a function to restore the environment variable to its initial value, and any error that might have -// occurred in the process. -func setEnv(knownHostsPath string) (func() error, error) { - commandEnvVar := "GIT_SSH_COMMAND" - flagName := "UserKnownHostsFile" - - initialCommand, isSet := os.LookupEnv(commandEnvVar) - - fail := func(err error) (func() error, error) { - return func() error { return nil }, err - } - - if !isSet { - if err := os.Setenv(commandEnvVar, fmt.Sprintf("ssh -o %s=%s", flagName, knownHostsPath)); err != nil { - return fail(err) - } - - return func() error { return os.Unsetenv(commandEnvVar) }, nil - } - - // Check if `UserKnownHostsFile` is already present (case-insensitive), even multiple times, and skip it if so. - var newSSHCommand strings.Builder - options := strings.Split(initialCommand, " -o ") - for _, opt := range options { - kv := strings.Split(opt, "=") - if len(kv) != 2 { // first element, pre `-o`, or other flag - if _, err := newSSHCommand.WriteString(opt); err != nil { - return fail(err) - } - - continue - } - - if strings.EqualFold(kv[0], flagName) { // case-insensitive comparison - continue - } - - if _, err := fmt.Fprintf(&newSSHCommand, " -o %s", opt); err != nil { - return fail(err) - } - } - - if _, err := fmt.Fprintf(&newSSHCommand, " -o %s=%s", flagName, knownHostsPath); err != nil { - return fail(err) - } - - if err := os.Setenv(commandEnvVar, newSSHCommand.String()); err != nil { - return fail(err) - } - - restore := func() error { - return os.Setenv(commandEnvVar, initialCommand) - } - - return restore, nil -} - func getEventRecorder(config *rest.Config, componentName string) (record.EventRecorder, error) { clientset, err := kubernetes.NewForConfig(config) if err != nil { diff --git a/internal/cmd/cli/apply_test.go b/internal/cmd/cli/apply_test.go index f6eab52448..8b960587fb 100644 --- a/internal/cmd/cli/apply_test.go +++ b/internal/cmd/cli/apply_test.go @@ -15,6 +15,7 @@ import ( "github.com/rancher/fleet/internal/bundlereader" "github.com/rancher/fleet/internal/cmd/cli/apply" + ssh "github.com/rancher/fleet/internal/ssh" ) const ( @@ -34,146 +35,25 @@ const ( var helmSecretsNameByPath_content = map[string]bundlereader.Auth{"path": {Username: username, Password: password_content}} -func TestSetEnv(t *testing.T) { - tests := map[string]struct { - envValue string - knownHostsPath string - expectedGitSSHCommand string - expectedErr error - }{ - "unset env var": { - knownHostsPath: "/foo/bar", - expectedGitSSHCommand: "ssh -o UserKnownHostsFile=/foo/bar", - }, - "set env var without options": { - envValue: "ssh", - knownHostsPath: "/foo/bar", - expectedGitSSHCommand: "ssh -o UserKnownHostsFile=/foo/bar", - }, - "set env var with other options": { - envValue: "ssh -o stricthostkeychecking=yes", - knownHostsPath: "/foo/bar", - expectedGitSSHCommand: "ssh -o stricthostkeychecking=yes -o UserKnownHostsFile=/foo/bar", - }, - "set env var with other options and known hosts file option": { - envValue: "ssh -o stricthostkeychecking=yes -o userknownhostsFile=/another/file", - knownHostsPath: "/foo/bar", - expectedGitSSHCommand: "ssh -o stricthostkeychecking=yes -o UserKnownHostsFile=/foo/bar", - }, - "set env var with other options and known hosts file option specified multiple times": { - envValue: "ssh -o userknownhostsFile=/another/file -o UserKnownHostsFile=/yet/another/file -o stricthostkeychecking=yes", - knownHostsPath: "/foo/bar", - expectedGitSSHCommand: "ssh -o stricthostkeychecking=yes -o UserKnownHostsFile=/foo/bar", - }, - } - - bkpEnv := os.Getenv("GIT_SSH_COMMAND") - defer os.Setenv("GIT_SSH_COMMAND", bkpEnv) - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - defer os.Unsetenv("GIT_SSH_COMMAND") - - if test.envValue != "" { - os.Setenv("GIT_SSH_COMMAND", test.envValue) - } else { - os.Unsetenv("GIT_SSH_COMMAND") - } - - restore, err := setEnv(test.knownHostsPath) - if !errors.Is(err, test.expectedErr) { - t.Errorf("expected err %v, got %v", test.expectedErr, err) - } - - if gitSSHCommand := os.Getenv("GIT_SSH_COMMAND"); gitSSHCommand != test.expectedGitSSHCommand { - t.Errorf("expected GIT_SSH_COMMAND %q, got %q", test.expectedGitSSHCommand, gitSSHCommand) - } - - if restoreErr := restore(); restoreErr != nil { - t.Errorf("expected nil restore error, got %v", restoreErr) - } - - restoredEnvValue, isSet := os.LookupEnv("GIT_SSH_COMMAND") - if restoredEnvValue != test.envValue { - t.Errorf( - "expected restored GIT_SSH_COMMAND value to be %q, got %t/%q", - test.envValue, - isSet, - restoredEnvValue, - ) - } - }) - } -} - -func TestWriteTmpKnownHosts(t *testing.T) { - tests := map[string]struct { - knownHosts string - isSet bool - expectFileExists bool - }{ - "does not write to known hosts file if FLEET_KNOWN_HOSTS is unset": {}, - "does not write to known hosts file if FLEET_KNOWN_HOSTS is empty": {isSet: true}, - "writes FLEET_KNOWN_HOSTS to custom known hosts file if set": { - knownHosts: "foo", - isSet: true, - expectFileExists: true, - }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - if test.isSet { - if err := os.Setenv("FLEET_KNOWN_HOSTS", test.knownHosts); err != nil { - t.Errorf("failed to set FLEET_KNOWN_HOSTS env var: %v", err) - } - - defer os.Unsetenv("FLEET_KNOWN_HOSTS") - } - - khPath, err := writeTmpKnownHosts() - if err != nil { - t.Errorf("expected nil error from writeTmpKnownHosts, got: %v", err) - } - - if !test.expectFileExists { - return - } - - gotKnownHosts, err := os.ReadFile(khPath) - if err != nil { - t.Errorf("failed to read known_hosts file: %v", err) - } - - defer os.RemoveAll(khPath) - - if test.knownHosts != "" { - if string(gotKnownHosts) != test.knownHosts { - t.Errorf("known_hosts mismatch: expected\n\t%s\ngot:\n\t%s", test.knownHosts, gotKnownHosts) - } - } - }) - } -} - func TestAddAuthToOpts(t *testing.T) { tests := map[string]struct { - name string - apply Apply - knownHosts string - expectedOpts *apply.Options - expectedErr error + name string + apply Apply + knownHosts string + helmInsecureSkipTLS bool + expectedOpts *apply.Options + expectedErr error }{ "Auth is empty if no arguments are provided": { apply: Apply{}, expectedOpts: &apply.Options{}, expectedErr: nil, }, - "known_hosts file is populated if the env var is set": { + "FLEET_KNOWN_HOSTS env var sets SSHKnownHosts in opts": { apply: Apply{}, - expectedOpts: &apply.Options{}, + knownHosts: "some-known-host", + expectedOpts: &apply.Options{Auth: bundlereader.Auth{SSHKnownHosts: []byte("some-known-host")}}, expectedErr: nil, - knownHosts: "foo", }, "Auth contains values from username, password, caCerts and sshPrivatey when helmSecretsNameByPath not provided": { apply: Apply{PasswordFile: password_file, Username: username, CACertsFile: caCerts_file, SSHPrivateKeyFile: sshPrivateKey_file}, @@ -190,6 +70,12 @@ func TestAddAuthToOpts(t *testing.T) { expectedOpts: &apply.Options{AuthByPath: helmSecretsNameByPath_content}, expectedErr: nil, }, + "HelmInsecureSkipTLS sets InsecureSkipVerify in opts": { + apply: Apply{}, + helmInsecureSkipTLS: true, + expectedOpts: &apply.Options{Auth: bundlereader.Auth{InsecureSkipVerify: true}}, + expectedErr: nil, + }, "Error if file doesn't exist": { apply: Apply{HelmCredentialsByPathFile: "notfound"}, expectedOpts: &apply.Options{}, @@ -199,8 +85,11 @@ func TestAddAuthToOpts(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { + if test.knownHosts != "" { + t.Setenv(ssh.KnownHostsEnvVar, test.knownHosts) + } opts := &apply.Options{} - err := test.apply.addAuthToOpts(opts, mockReadFile, false, false) + err := test.apply.addAuthToOpts(opts, mockReadFile, false, test.helmInsecureSkipTLS) if !cmp.Equal(opts, test.expectedOpts) { t.Errorf("opts don't match: expected %v, got %v", test.expectedOpts, opts) } diff --git a/internal/cmd/cli/gitcloner/cloner_test.go b/internal/cmd/cli/gitcloner/cloner_test.go index 99634b6b41..e27333a01d 100644 --- a/internal/cmd/cli/gitcloner/cloner_test.go +++ b/internal/cmd/cli/gitcloner/cloner_test.go @@ -1,7 +1,13 @@ package gitcloner import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "encoding/pem" "errors" + "net" "os" "strings" "testing" @@ -15,6 +21,8 @@ import ( gossh "github.com/go-git/go-git/v5/plumbing/transport/ssh" "github.com/google/go-cmp/cmp" "github.com/rancher/fleet/internal/cmd/cli/gitcloner/submodule" + fleetssh "github.com/rancher/fleet/internal/ssh" + golangssh "golang.org/x/crypto/ssh" ) type fakeGetter struct{} @@ -616,3 +624,130 @@ func TestCloneRevision_SubmoduleUpdateError(t *testing.T) { t.Fatalf("expected 'submodule update failed', got: %s", err.Error()) } } + +// generateECPrivateKeyPEM creates a PEM-encoded EC private key for use in tests. +func generateECPrivateKeyPEM(t *testing.T) []byte { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generating key: %v", err) + } + der, err := x509.MarshalECPrivateKey(key) + if err != nil { + t.Fatalf("marshalling key: %v", err) + } + return pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: der}) +} + +// generateKnownHostsEntry creates a host key and returns the matching +// known_hosts line for use in tests. +func generateKnownHostsEntry(t *testing.T, hostname string) (golangssh.PublicKey, string) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generating host key: %v", err) + } + sshPubKey, err := golangssh.NewPublicKey(&key.PublicKey) + if err != nil { + t.Fatalf("creating SSH public key: %v", err) + } + entry := golangssh.MarshalAuthorizedKey(sshPubKey) + return sshPubKey, hostname + " " + string(entry) +} + +func TestCreateAuthFromOpts(t *testing.T) { + // Ensure real file I/O regardless of which mock a previous test left installed. + origReadFile := readFile + readFile = os.ReadFile + t.Cleanup(func() { readFile = origReadFile }) + keyPEM := generateECPrivateKeyPEM(t) + hostKey, knownHostsLine := generateKnownHostsEntry(t, "expected-host") + + writeTempKey := func(t *testing.T) string { + t.Helper() + f, err := os.CreateTemp(t.TempDir(), "ssh-key-*.pem") + if err != nil { + t.Fatalf("creating temp key file: %v", err) + } + if _, err := f.Write(keyPEM); err != nil { + t.Fatalf("writing key: %v", err) + } + f.Close() + return f.Name() + } + + addr := &net.TCPAddr{} + + t.Run("no SSH key returns nil auth", func(t *testing.T) { + opts := &GitCloner{Repo: "https://github.com/example/repo"} + auth, err := createAuthFromOpts(opts) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if auth != nil { + t.Fatalf("expected nil auth, got %v", auth) + } + }) + + t.Run("SSH key without known hosts uses InsecureIgnoreHostKey", func(t *testing.T) { + opts := &GitCloner{ + Repo: "ssh://git@example.com/org/repo", + SSHPrivateKeyFile: writeTempKey(t), + } + auth, err := createAuthFromOpts(opts) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + pubKeys, ok := auth.(*gossh.PublicKeys) + if !ok { + t.Fatalf("expected *gossh.PublicKeys, got %T", auth) + } + // InsecureIgnoreHostKey never returns an error. + if err := pubKeys.HostKeyCallback("any-host:22", addr, hostKey); err != nil { + t.Fatalf("expected InsecureIgnoreHostKey to accept any host, got: %v", err) + } + }) + + t.Run("SSH key with FLEET_KNOWN_HOSTS enforces host key", func(t *testing.T) { + t.Setenv(fleetssh.KnownHostsEnvVar, knownHostsLine) + opts := &GitCloner{ + Repo: "ssh://git@example.com/org/repo", + SSHPrivateKeyFile: writeTempKey(t), + } + auth, err := createAuthFromOpts(opts) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + pubKeys, ok := auth.(*gossh.PublicKeys) + if !ok { + t.Fatalf("expected *gossh.PublicKeys, got %T", auth) + } + // Known host passes. + if err := pubKeys.HostKeyCallback("expected-host:22", addr, hostKey); err != nil { + t.Fatalf("expected-host should be accepted: %v", err) + } + // Unknown host key is rejected. + otherHostKey, _ := generateKnownHostsEntry(t, "other-host") + if err := pubKeys.HostKeyCallback("expected-host:22", addr, otherHostKey); err == nil { + t.Fatal("unexpected host key should be rejected") + } + }) + + t.Run("SSH URL without user defaults to git", func(t *testing.T) { + opts := &GitCloner{ + Repo: "ssh://example.com/org/repo", + SSHPrivateKeyFile: writeTempKey(t), + } + auth, err := createAuthFromOpts(opts) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + pubKeys, ok := auth.(*gossh.PublicKeys) + if !ok { + t.Fatalf("expected *gossh.PublicKeys, got %T", auth) + } + if pubKeys.User != "git" { + t.Fatalf("expected user 'git', got %q", pubKeys.User) + } + }) +} diff --git a/internal/cmd/controller/gitops/reconciler/gitjob.go b/internal/cmd/controller/gitops/reconciler/gitjob.go index a6ef56bf78..8f78d06b74 100644 --- a/internal/cmd/controller/gitops/reconciler/gitjob.go +++ b/internal/cmd/controller/gitops/reconciler/gitjob.go @@ -397,6 +397,10 @@ func (r *GitJobReconciler) newJobSpec(ctx context.Context, gitrepo *v1alpha1.Git volumeMounts = append(volumeMounts, volMnts...) } + // spec.InsecureSkipTLSverify applies to all git operations, including + // Helm-chart-from-git fetches in fleet apply. + helmInsecure = helmInsecure || gitrepo.Spec.InsecureSkipTLSverify + // In case no Helm secret volume has been created, because Helm secrets don't exist or don't contain a CA // bundle, mount a volume with a Rancher CA bundle. if !certVolCreated { @@ -448,7 +452,6 @@ func (r *GitJobReconciler) newJobSpec(ctx context.Context, gitrepo *v1alpha1.Git gitrepo, logger, CACertsFilePathOverride, - r.KnownHosts, drivenScanSeparator, helmInsecure, helmBasicHTTP, @@ -658,7 +661,6 @@ func argsAndEnvs( gitrepo *v1alpha1.GitRepo, logger logr.Logger, pathOverrideCACerts string, - knownHosts KnownHostsGetter, drivenScanSeparator string, helmInsecureSkipTLS bool, helmBasicHTTP bool, @@ -742,8 +744,6 @@ func argsAndEnvs( } args = append(args, helmArgs...) - // for ssh go-getter - env = append(env, gitSSHCommandEnvVar(knownHosts.IsStrict())) } else if gitrepo.Spec.HelmSecretName != "" { helmArgs := []string{ "--password-file", @@ -763,8 +763,6 @@ func argsAndEnvs( helmArgs = append(helmArgs, "--helm-repo-url-regex", gitrepo.Spec.HelmRepoURLRegex) } args = append(args, helmArgs...) - // for ssh go-getter - env = append(env, gitSSHCommandEnvVar(knownHosts.IsStrict())) env = append(env, corev1.EnvVar{ Name: "HELM_USERNAME", @@ -789,7 +787,6 @@ func argsAndEnvs( helmArgs = append(helmArgs, "--helm-repo-url-regex", gitrepo.Spec.HelmRepoURLRegex) } args = append(args, helmArgs...) - env = append(env, gitSSHCommandEnvVar(knownHosts.IsStrict())) } if !ocistorage.OCIIsEnabled() { @@ -969,19 +966,6 @@ func proxyEnvVars() []corev1.EnvVar { return envVars } -func gitSSHCommandEnvVar(strictChecks bool) corev1.EnvVar { - strictVal := "no" - - if strictChecks { - strictVal = "yes" - } - - return corev1.EnvVar{ - Name: "GIT_SSH_COMMAND", - Value: fmt.Sprintf("ssh -o stricthostkeychecking=%s", strictVal), - } -} - // getDrivenScanSeparator returns a separator that is valid for all the Bundle // definitions in the given GitRepo. // Since we cannot disregard the possibility that a user might have an diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_test.go b/internal/cmd/controller/gitops/reconciler/gitjob_test.go index 9356712b17..37b864c4da 100644 --- a/internal/cmd/controller/gitops/reconciler/gitjob_test.go +++ b/internal/cmd/controller/gitops/reconciler/gitjob_test.go @@ -1427,6 +1427,12 @@ func TestNewJob(t *testing.T) { }, }, }, + expectedContainers: []corev1.Container{ + { + Name: "fleet", + Args: []string{"--helm-insecure-skip-tls"}, + }, + }, expectedVolumes: []corev1.Volume{ { Name: gitClonerVolumeName, @@ -1700,10 +1706,6 @@ func TestGenerateJob_EnvVars(t *testing.T) { Name: "FLEET_BUNDLE_CREATION_MAX_CONCURRENCY", Value: "4", }, - { - Name: "GIT_SSH_COMMAND", - Value: "ssh -o stricthostkeychecking=no", - }, { Name: "HELM_USERNAME", ValueFrom: &corev1.EnvVarSource{ @@ -1773,10 +1775,6 @@ func TestGenerateJob_EnvVars(t *testing.T) { Name: "FLEET_BUNDLE_CREATION_MAX_CONCURRENCY", Value: "4", }, - { - Name: "GIT_SSH_COMMAND", - Value: "ssh -o stricthostkeychecking=yes", - }, { Name: "HELM_USERNAME", ValueFrom: &corev1.EnvVarSource{ @@ -1830,10 +1828,6 @@ func TestGenerateJob_EnvVars(t *testing.T) { Name: "FLEET_BUNDLE_CREATION_MAX_CONCURRENCY", Value: "4", }, - { - Name: "GIT_SSH_COMMAND", - Value: "ssh -o stricthostkeychecking=no", - }, { Name: "COMMIT", Value: "commit", @@ -1891,10 +1885,6 @@ func TestGenerateJob_EnvVars(t *testing.T) { Name: "FLEET_BUNDLE_CREATION_MAX_CONCURRENCY", Value: "4", }, - { - Name: "GIT_SSH_COMMAND", - Value: "ssh -o stricthostkeychecking=yes", - }, { Name: "COMMIT", Value: "commit", From b08f6516cf93998e03166ac7c37a7304c6c7baf6 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Mon, 23 Mar 2026 10:16:46 +0100 Subject: [PATCH 6/8] Guard DefaultTransport assertion in transportForAuth Fall back to a transport with stdlib-matching defaults if DefaultTransport is not a *http.Transport, to avoid a panic when another component has replaced the global default. The fallback sets Proxy: http.ProxyFromEnvironment, 30s dial/keep-alive, and the standard idle/TLS/expect-continue timeouts so runtime behaviour stays close to the baseline. --- internal/bundlereader/charturl.go | 21 ++++++- .../bundlereader/charturl_internal_test.go | 55 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 internal/bundlereader/charturl_internal_test.go diff --git a/internal/bundlereader/charturl.go b/internal/bundlereader/charturl.go index 8866f837c7..b3d45b6bf6 100644 --- a/internal/bundlereader/charturl.go +++ b/internal/bundlereader/charturl.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io" + "net" "net/http" "net/url" "strings" @@ -290,7 +291,25 @@ func transportForAuth(insecureSkipVerify bool, caBundle []byte) http.RoundTrippe } // Create new transport - transport := http.DefaultTransport.(*http.Transport).Clone() + baseTransport, ok := http.DefaultTransport.(*http.Transport) + if !ok { + // Another component has replaced the global default transport. + // Construct a transport that preserves the standard proxy and timeout + // defaults so runtime behaviour stays close to the stdlib baseline. + baseTransport = &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + } + } + transport := baseTransport.Clone() transport.TLSClientConfig = &tls.Config{ InsecureSkipVerify: insecureSkipVerify, //nolint:gosec } diff --git a/internal/bundlereader/charturl_internal_test.go b/internal/bundlereader/charturl_internal_test.go new file mode 100644 index 0000000000..ae7890883d --- /dev/null +++ b/internal/bundlereader/charturl_internal_test.go @@ -0,0 +1,55 @@ +package bundlereader + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeRoundTripper is a RoundTripper that is not *http.Transport, so substituting +// it for http.DefaultTransport triggers the type-assertion path in transportForAuth. +type fakeRoundTripper struct{} + +func (fakeRoundTripper) RoundTrip(*http.Request) (*http.Response, error) { return nil, nil } + +// TestTransportForAuthNonDefaultTransport verifies that transportForAuth does not +// panic when http.DefaultTransport has been replaced by a value that is not +// *http.Transport, and that the resulting transport has proxy and timeout defaults. +// +// Not parallel: the test mutates the process-global http.DefaultTransport. +func TestTransportForAuthNonDefaultTransport(t *testing.T) { + orig := http.DefaultTransport + t.Cleanup(func() { http.DefaultTransport = orig }) + + http.DefaultTransport = fakeRoundTripper{} + + // The transport cache is package-level; clear it so a fresh entry is built. + transportsCacheMutex.Lock() + transportsCache = map[string]http.RoundTripper{} + transportsCacheMutex.Unlock() + + var result http.RoundTripper + require.NotPanics(t, func() { + result = transportForAuth(false, nil) + }, "transportForAuth must not panic when http.DefaultTransport is not *http.Transport") + require.NotNil(t, result) + + // The fallback transport must carry proxy and timeout settings, not bare &http.Transport{}. + tr, ok := result.(*http.Transport) + require.True(t, ok, "result must be *http.Transport") + assert.NotNil(t, tr.Proxy, "fallback transport must have Proxy set") + assert.NotNil(t, tr.DialContext, "fallback transport must have DialContext set") + assert.Greater(t, tr.TLSHandshakeTimeout, tr.IdleConnTimeout*0, "TLSHandshakeTimeout must be positive") + + // Also verify the custom-CA path does not panic. + transportsCacheMutex.Lock() + transportsCache = map[string]http.RoundTripper{} + transportsCacheMutex.Unlock() + + require.NotPanics(t, func() { + result = transportForAuth(false, []byte("not-a-real-cert")) + assert.NotNil(t, result) + }, "transportForAuth must not panic with a CA bundle when DefaultTransport is not *http.Transport") +} From c73552ba11e46c1d907e6a0bc925f50bc2fa549a Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Mon, 23 Mar 2026 10:17:04 +0100 Subject: [PATCH 7/8] Restrict SSH auth to SSH/git URL schemes in setGitAuth go-git's HTTP transport does not accept *ssh.PublicKeys, so applying SSH auth to an https:// URL causes the clone to fail with a confusing transport error. Guard the SSH auth path with an isSSH check. If ?sshkey= is explicitly provided for an HTTP(S) URL, return a clear error. If Auth.SSHPrivateKey is set alongside an HTTP(S) URL, silently ignore it and fall through to HTTP basic auth. --- internal/bundlereader/gitclone.go | 14 +++++ internal/bundlereader/gitclone_test.go | 71 ++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/internal/bundlereader/gitclone.go b/internal/bundlereader/gitclone.go index c919381f19..1bb659e9bc 100644 --- a/internal/bundlereader/gitclone.go +++ b/internal/bundlereader/gitclone.go @@ -127,12 +127,26 @@ func gitDownload(ctx context.Context, dst, rawURL string, auth Auth) error { // sshKeyPEM (from the URL ?sshkey= param) takes precedence over auth.SSHPrivateKey. // For HTTPS, credentials are taken from the URL userinfo first, then from auth. func setGitAuth(opts *gogit.CloneOptions, u *url.URL, sshKeyPEM []byte, auth Auth) error { + isSSH := u.Scheme == "ssh" || u.Scheme == "git" + // Prefer the key from the URL query param over auth.SSHPrivateKey. keyPEM := sshKeyPEM if len(keyPEM) == 0 { keyPEM = auth.SSHPrivateKey } + if len(keyPEM) > 0 { + if !isSSH { + // ?sshkey= in an HTTP(S) URL is a misconfiguration. + if len(sshKeyPEM) > 0 { + return fmt.Errorf("?sshkey= is not supported for %s URLs", u.Scheme) + } + // Auth.SSHPrivateKey alongside an HTTP(S) URL: ignore and fall + // through to HTTP basic auth below. + keyPEM = nil + } + } + if len(keyPEM) > 0 { user := "git" if u.User != nil && u.User.Username() != "" { diff --git a/internal/bundlereader/gitclone_test.go b/internal/bundlereader/gitclone_test.go index b705f7334c..938f291932 100644 --- a/internal/bundlereader/gitclone_test.go +++ b/internal/bundlereader/gitclone_test.go @@ -13,9 +13,11 @@ import ( "net" "net/http" "net/http/httptest" + "net/url" "testing" "time" + gogit "github.com/go-git/go-git/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -117,3 +119,72 @@ func TestGitDownloadCABundle(t *testing.T) { assert.Contains(t, err.Error(), "no valid PEM certificates") }) } + +// generateEd25519PEM returns a PEM-encoded Ed25519 private key for use in tests. +func generateEd25519PEM(t *testing.T) []byte { + t.Helper() + + // Use ECDSA since crypto/ed25519 and go-git's SSH key parsing both work, + // but PEM encoding of Ed25519 requires the x/crypto package. ECDSA P-256 + // is simpler to produce inline. + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + der, err := x509.MarshalECPrivateKey(key) + require.NoError(t, err) + return pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: der}) +} + +// TestSetGitAuth verifies that setGitAuth respects URL scheme when choosing +// which auth type to configure. +func TestSetGitAuth(t *testing.T) { + t.Parallel() + + keyPEM := generateEd25519PEM(t) + + t.Run("sshKey in URL for HTTPS URL returns error", func(t *testing.T) { + t.Parallel() + u, _ := url.Parse("https://example.com/repo.git") + opts := &gogit.CloneOptions{} + err := setGitAuth(opts, u, keyPEM, Auth{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "?sshkey= is not supported") + assert.Nil(t, opts.Auth) + }) + + t.Run("Auth.SSHPrivateKey alongside HTTPS URL is ignored", func(t *testing.T) { + t.Parallel() + u, _ := url.Parse("https://example.com/repo.git") + opts := &gogit.CloneOptions{} + err := setGitAuth(opts, u, nil, Auth{SSHPrivateKey: keyPEM}) + require.NoError(t, err) + // No auth configured: callers that need basic auth supply URL credentials. + assert.Nil(t, opts.Auth) + }) + + t.Run("sshKey in URL for SSH URL configures SSH auth", func(t *testing.T) { + t.Parallel() + u, _ := url.Parse("ssh://git@example.com/repo.git") + opts := &gogit.CloneOptions{} + err := setGitAuth(opts, u, keyPEM, Auth{}) + require.NoError(t, err) + assert.NotNil(t, opts.Auth) + }) + + t.Run("Auth.SSHPrivateKey for SSH URL configures SSH auth", func(t *testing.T) { + t.Parallel() + u, _ := url.Parse("ssh://git@example.com/repo.git") + opts := &gogit.CloneOptions{} + err := setGitAuth(opts, u, nil, Auth{SSHPrivateKey: keyPEM}) + require.NoError(t, err) + assert.NotNil(t, opts.Auth) + }) + + t.Run("basic auth from URL userinfo for HTTPS URL", func(t *testing.T) { + t.Parallel() + u, _ := url.Parse("https://user:pass@example.com/repo.git") + opts := &gogit.CloneOptions{} + err := setGitAuth(opts, u, nil, Auth{}) + require.NoError(t, err) + assert.NotNil(t, opts.Auth) + }) +} From e02372944a16ef4ac6279fcc1fe63a663b1494e4 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Tue, 24 Mar 2026 12:00:11 +0100 Subject: [PATCH 8/8] Switch controller image to bci-busybox with static tini Remove openssh-clients and git-core: all git and SSH operations use go-git and golang.org/x/crypto/ssh; no system git or ssh binary is called anywhere in the controller. Switch the base image from bci-base to bci-busybox, matching the agent image. Drop useradd; bci-busybox lacks useradd, and USER 1000 without prior user-creation is already the pattern used by Dockerfile.agent. --- package/Dockerfile | 20 +++++++++++--------- package/log.sh | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/package/Dockerfile b/package/Dockerfile index 1a69dd5523..cad1e45cd4 100644 --- a/package/Dockerfile +++ b/package/Dockerfile @@ -3,15 +3,18 @@ ARG TARGETPLATFORM ARG TARGETARCH ARG ARCH=${TARGETARCH} -FROM --platform=linux/${ARCH} registry.suse.com/bci/bci-base:15.7 AS base +FROM --platform=linux/${ARCH} registry.suse.com/bci/bci-busybox:15.7 AS base +ARG ARCH +# Download tini-static v0.19.0 (stable final release) and verify via embedded SHA256 checksum. +RUN set -eux; \ + wget -qO /usr/bin/tini "https://github.com/krallin/tini/releases/download/v0.19.0/tini-static-${ARCH}"; \ + case "${ARCH}" in \ + amd64) echo "c5b0666b4cb676901f90dfcb37106783c5fe2077b04590973b885950611b30ee /usr/bin/tini" | sha256sum -c - ;; \ + arm64) echo "eae1d3aa50c48fb23b8cbdf4e369d0910dfc538566bfd09df89a774aa84a48b9 /usr/bin/tini" | sha256sum -c - ;; \ + *) echo "Unsupported architecture: ${ARCH}" >&2; exit 1 ;; \ + esac; \ + chmod 0755 /usr/bin/tini COPY package/log.sh /usr/bin/ -RUN zypper rm -y container-suseconnect && \ - zypper ar --priority=500 https://download.opensuse.org/repositories/Virtualization:containers/5.5/Virtualization:containers.repo && \ - zypper --gpg-auto-import-keys ref && \ - zypper -n update && \ - zypper -n install --no-recommends openssh-clients tini git-core && \ - zypper -n clean -a && \ - rm -fr /var/log/zypp* /usr/share/doc FROM base AS copy_docker ONBUILD ARG ARCH @@ -29,7 +32,6 @@ COPY ${TARGETPLATFORM}/fleetcontroller-linux-* /usr/bin/fleetcontroller COPY ${TARGETPLATFORM}/fleet-linux-* /usr/bin/fleet FROM copy_${BUILD_ENV} -RUN useradd -u 1000 user USER 1000 ENTRYPOINT ["tini", "--"] CMD ["fleetcontroller"] diff --git a/package/log.sh b/package/log.sh index 3eeaf1ac11..c81b269f74 100755 --- a/package/log.sh +++ b/package/log.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh set -o pipefail env "$@" 2>&1 | tee /dev/termination-log