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 new file mode 100644 index 0000000000..7d09be6f28 --- /dev/null +++ b/integrationtests/bundlereader/getcontent_test.go @@ -0,0 +1,425 @@ +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("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) + _, 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")) + }) + + 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()) + }) + }) +}) + +// 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/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/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") +} diff --git a/integrationtests/gitcloner/clone_test.go b/integrationtests/gitcloner/clone_test.go index a9c1246c22..56987a2a36 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", } }) @@ -232,27 +216,76 @@ 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() { 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 +382,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 +394,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 +449,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 +461,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 +481,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 +501,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/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/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 +} 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/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") +} diff --git a/internal/bundlereader/gitclone.go b/internal/bundlereader/gitclone.go new file mode 100644 index 0000000000..1bb659e9bc --- /dev/null +++ b/internal/bundlereader/gitclone.go @@ -0,0 +1,248 @@ +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 { + 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() != "" { + 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..938f291932 --- /dev/null +++ b/internal/bundlereader/gitclone_test.go @@ -0,0 +1,190 @@ +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" + "net/url" + "testing" + "time" + + gogit "github.com/go-git/go-git/v5" + "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") + }) +} + +// 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) + }) +} 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/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.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/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", 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/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 diff --git a/pkg/git/netutils.go b/pkg/git/netutils.go index 5249fd7675..a5a464c537 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,19 @@ 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], "") + 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(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)