From 3c483e20718550a9dedd96de0117856da3386412 Mon Sep 17 00:00:00 2001 From: pratap0007 Date: Fri, 11 Sep 2026 14:00:30 +0530 Subject: [PATCH] fix(hub): restrict tkn hub install resources fetching tkn hub install applied Hub YAML with the user's kube credentials after fetching it over an unrestricted HTTP client. --api-server and httpGet only used url.ParseRequestURI, so remote HTTP, file://, and arbitrary schemes were accepted. The installer also did not check GVK, so a compromised Hub response could apply a TaskRun or PipelineRun and run immediately. Require HTTPS for Hub URLs (HTTP only to loopback for a local Hub), reject non-https redirects, and time out fetches. Before apply, allow only tekton.dev Task, Pipeline, and StepAction and runnable kinds stay rejected. All the above cases are handled. TLS pinning is not required: default Hub URLs are already HTTPS and Go verifies public CA certificates. Cosign/sigstore and SHA digest checks are not required as they need Hub-published signatures or hashes, and a digest from the same Hub API is the same trust root as the YAML. A malicious Task from a trusted catalog is still a catalog-trust issue, not a CLI URL bug. Signed-off-by: pratap0007 Assisted-by: Cursor Grok 4.6 (via Cursor) Co-authored-by: Cursor --- pkg/cmd/hub/hub/hub.go | 70 ++++++++++++++++++-- pkg/cmd/hub/hub/hub_test.go | 45 +++++++++++-- pkg/cmd/hub/installer/action.go | 38 +++++++++++ pkg/cmd/hub/installer/action_test.go | 96 ++++++++++++++++++++++++++++ pkg/cmd/hub/test/config.go | 2 +- 5 files changed, 239 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/hub/hub/hub.go b/pkg/cmd/hub/hub/hub.go index d4cde80762..2cc5eeec9a 100644 --- a/pkg/cmd/hub/hub/hub.go +++ b/pkg/cmd/hub/hub/hub.go @@ -17,9 +17,12 @@ package hub import ( "fmt" "io" + "net" "net/http" "net/url" "os/user" + "strings" + "time" "github.com/joho/godotenv" "github.com/spf13/viper" @@ -39,6 +42,9 @@ const ( artifactHubCatInfoEndpoint = "/api/v1/packages/tekton" artifactHubTaskType = 7 artifactHubPipelineType = 11 + + httpClientTimeout = 30 * time.Second + maxRedirects = 10 ) type Client interface { @@ -126,8 +132,7 @@ func (t *tektonHubClient) Get(endpoint string) ([]byte, int, error) { func resolveUrl(apiURL, envVariable, defaultUrl string) (string, error) { if apiURL != "" { - _, err := url.ParseRequestURI(apiURL) - if err != nil { + if err := validateHubURLString(apiURL); err != nil { return "", err } @@ -140,8 +145,7 @@ func resolveUrl(apiURL, envVariable, defaultUrl string) (string, error) { viper.AutomaticEnv() if apiURL := viper.GetString(envVariable); apiURL != "" { - _, err := url.ParseRequestURI(apiURL) - if err != nil { + if err := validateHubURLString(apiURL); err != nil { return "", fmt.Errorf("invalid url set for %s: %s : %v", envVariable, apiURL, err) } return apiURL, nil @@ -170,15 +174,20 @@ func get(url string) ([]byte, int, error) { return data, status, err } -// httpGet gets raw data given the url -func httpGet(url string) ([]byte, int, error) { +// httpGet gets raw data given the url. +// Only https is allowed, except http to loopback addresses used by a local Hub. +func httpGet(rawURL string) ([]byte, int, error) { err := loadConfigFile() if err != nil { return nil, 0, err } - resp, err := http.Get(url) + if err := validateHubURLString(rawURL); err != nil { + return nil, 0, err + } + + resp, err := hubHTTPClient.Get(rawURL) if err != nil { return nil, 0, err } @@ -192,6 +201,53 @@ func httpGet(url string) ([]byte, int, error) { return data, resp.StatusCode, err } +var hubHTTPClient = &http.Client{ + Timeout: httpClientTimeout, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= maxRedirects { + return fmt.Errorf("stopped after %d redirects", maxRedirects) + } + if !strings.EqualFold(req.URL.Scheme, "https") { + return fmt.Errorf("refusing non-HTTPS redirect to %q", req.URL.Redacted()) + } + return validateHubURL(req.URL) + }, +} + +func validateHubURLString(rawURL string) error { + u, err := url.ParseRequestURI(rawURL) + if err != nil { + return err + } + return validateHubURL(u) +} + +func validateHubURL(u *url.URL) error { + if u == nil { + return fmt.Errorf("invalid url") + } + + switch strings.ToLower(u.Scheme) { + case "https": + return nil + case "http": + if isLoopbackHost(u.Hostname()) { + return nil + } + return fmt.Errorf("refusing insecure HTTP URL %q; use HTTPS", u.Redacted()) + default: + return fmt.Errorf("unsupported URL scheme %q; only https is allowed", u.Scheme) + } +} + +func isLoopbackHost(host string) bool { + if strings.EqualFold(host, "localhost") { + return true + } + ip := net.ParseIP(host) + return ip != nil && ip.IsLoopback() +} + // Looks for config file at $HOME/.tekton/hub-config and loads into // in the environment func loadConfigFile() error { diff --git a/pkg/cmd/hub/hub/hub_test.go b/pkg/cmd/hub/hub/hub_test.go index 22137a5f0e..9529df8d83 100644 --- a/pkg/cmd/hub/hub/hub_test.go +++ b/pkg/cmd/hub/hub/hub_test.go @@ -1,6 +1,7 @@ package hub import ( + "net/url" "testing" "github.com/stretchr/testify/assert" @@ -11,10 +12,10 @@ func TestSetURL_TektonHub(t *testing.T) { err := tHub.SetURL("http://localhost:80000") assert.NoError(t, err) - err = tHub.SetURL("localhost:8000") + err = tHub.SetURL("https://api.hub.tekton.dev") assert.NoError(t, err) - err = tHub.SetURL("http://80.80.79.9:80") + err = tHub.SetURL("http://127.0.0.1:8080") assert.NoError(t, err) // default url @@ -28,10 +29,10 @@ func TestSetURL_ArtifactHub(t *testing.T) { err := aHub.SetURL("http://localhost:80000") assert.NoError(t, err) - err = aHub.SetURL("localhost:8000") + err = aHub.SetURL("https://artifacthub.io") assert.NoError(t, err) - err = aHub.SetURL("http://80.80.79.9:80") + err = aHub.SetURL("http://127.0.0.1:8080") assert.NoError(t, err) // default url @@ -46,4 +47,40 @@ func TestSetURL_InvalidCase(t *testing.T) { err := hub.SetURL("abc") assert.Error(t, err) assert.EqualError(t, err, "parse \"abc\": invalid URI for request") + + err = hub.SetURL("localhost:8000") + assert.EqualError(t, err, "unsupported URL scheme \"localhost\"; only https is allowed") + + err = hub.SetURL("http://80.80.79.9:80") + assert.EqualError(t, err, "refusing insecure HTTP URL \"http://80.80.79.9:80\"; use HTTPS") +} + +func TestValidateHubURL(t *testing.T) { + tests := []struct { + raw string + wantErr string + }{ + {raw: "https://artifacthub.io"}, + {raw: "https://api.hub.tekton.dev/v1/resource/tekton/task/git-clone/0.9/yaml"}, + {raw: "http://localhost:8080"}, + {raw: "http://127.0.0.1:8080"}, + {raw: "http://[::1]:8080"}, + {raw: "http://evil.example", wantErr: "refusing insecure HTTP URL"}, + {raw: "file:///etc/passwd", wantErr: "unsupported URL scheme"}, + {raw: "ftp://artifacthub.io", wantErr: "unsupported URL scheme"}, + } + + for _, tc := range tests { + t.Run(tc.raw, func(t *testing.T) { + u, err := url.ParseRequestURI(tc.raw) + assert.NoError(t, err) + err = validateHubURL(u) + if tc.wantErr == "" { + assert.NoError(t, err) + return + } + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + }) + } } diff --git a/pkg/cmd/hub/installer/action.go b/pkg/cmd/hub/installer/action.go index f780d9ce66..29161c3a62 100644 --- a/pkg/cmd/hub/installer/action.go +++ b/pkg/cmd/hub/installer/action.go @@ -36,8 +36,18 @@ const ( communitySupportTier = "Community" verifiedSupportTier = "Verified" verifiedCatOrg = "tektoncd" + tektonAPIGroup = "tekton.dev" ) +// installableKinds are catalog resource definitions. Runnable kinds such as +// TaskRun or PipelineRun are rejected so a compromised Hub response cannot +// execute workloads immediately. +var installableKinds = map[string]struct{}{ + "Task": {}, + "Pipeline": {}, + "StepAction": {}, +} + // Errors var ( ErrAlreadyExist = errors.New("resource already exists") @@ -99,6 +109,11 @@ func (i *Installer) Install(data []byte, hubType, org, catalog, namespace string return nil, errors } + if err := validateInstallableResource(newRes); err != nil { + errors = append(errors, err) + return nil, errors + } + newResPipMinVersion := newRes.GetAnnotations()[ResourceMinVersion] err = i.checkVersion("v" + newResPipMinVersion) if err != nil { @@ -187,6 +202,11 @@ func (i *Installer) updateByAction(data []byte, catalog, namespace string, actio return nil, errors } + if err := validateInstallableResource(newRes); err != nil { + errors = append(errors, err) + return nil, errors + } + newResPipMinVersion := newRes.GetAnnotations()[ResourceMinVersion] err = i.checkVersion("v" + newResPipMinVersion) @@ -315,6 +335,24 @@ func (i *Installer) updateRes(existing, new *unstructured.Unstructured, catalog, return res, nil } +func validateInstallableResource(res *unstructured.Unstructured) error { + if res == nil { + return fmt.Errorf("refusing to install resource: empty manifest") + } + + gvk := res.GroupVersionKind() + if gvk.Group != tektonAPIGroup { + return fmt.Errorf("refusing to install %s: hub can only install tekton.dev resources", gvk.String()) + } + if gvk.Kind == "" { + return fmt.Errorf("refusing to install resource: missing kind") + } + if _, ok := installableKinds[gvk.Kind]; !ok { + return fmt.Errorf("refusing to install kind %s: hub can only install Task, Pipeline, or StepAction resources", gvk.Kind) + } + return nil +} + func toUnstructured(data []byte) (*unstructured.Unstructured, error) { r := bytes.NewReader(data) diff --git a/pkg/cmd/hub/installer/action_test.go b/pkg/cmd/hub/installer/action_test.go index f9f74b3b87..9cf4629d3f 100644 --- a/pkg/cmd/hub/installer/action_test.go +++ b/pkg/cmd/hub/installer/action_test.go @@ -103,6 +103,102 @@ func TestToUnstructuredAndAddLabel(t *testing.T) { } } +func TestValidateInstallableResource(t *testing.T) { + const catalogKindErr = "hub can only install Task, Pipeline, or StepAction resources" + tests := []struct { + name string + data string + wantErr string + }{ + { + name: "allows Task", + data: res, + }, + { + name: "allows Pipeline", + data: `apiVersion: tekton.dev/v1 +kind: Pipeline +metadata: + name: foo +spec: {} +`, + }, + { + name: "allows StepAction", + data: `apiVersion: tekton.dev/v1beta1 +kind: StepAction +metadata: + name: git-clone +spec: + image: alpine +`, + }, + { + name: "rejects ClusterTask", + data: `apiVersion: tekton.dev/v1beta1 +kind: ClusterTask +metadata: + name: foo +spec: {} +`, + wantErr: "refusing to install kind ClusterTask: " + catalogKindErr, + }, + { + name: "rejects TaskRun", + data: `apiVersion: tekton.dev/v1beta1 +kind: TaskRun +metadata: + name: foo +spec: {} +`, + wantErr: "refusing to install kind TaskRun: " + catalogKindErr, + }, + { + name: "rejects PipelineRun", + data: `apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + name: foo +spec: {} +`, + wantErr: "refusing to install kind PipelineRun: " + catalogKindErr, + }, + { + name: "rejects core Secret", + data: `apiVersion: v1 +kind: Secret +metadata: + name: evil +`, + wantErr: "refusing to install /v1, Kind=Secret: hub can only install tekton.dev resources", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + obj, err := toUnstructured([]byte(tc.data)) + assert.NoError(t, err) + err = validateInstallableResource(obj) + if tc.wantErr == "" { + assert.NoError(t, err) + return + } + assert.EqualError(t, err, tc.wantErr) + }) + } +} + +func TestInstall_RejectsClusterTask(t *testing.T) { + clusterTask := `apiVersion: tekton.dev/v1beta1 +kind: ClusterTask +metadata: + name: foo +spec: {} +` + _, errs := New(nil).Install([]byte(clusterTask), hub.TektonHubType, "", "tekton", "hub") + assert.EqualError(t, errs[0], "refusing to install kind ClusterTask: hub can only install Task, Pipeline, or StepAction resources") +} + func TestListInstalled(t *testing.T) { existingTask := &v1beta1.Task{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/cmd/hub/test/config.go b/pkg/cmd/hub/test/config.go index 97aa246338..76ee35a62d 100644 --- a/pkg/cmd/hub/test/config.go +++ b/pkg/cmd/hub/test/config.go @@ -23,7 +23,7 @@ import ( ) // API is test URL -const API string = "http://test.hub.cli" +const API string = "https://test.hub.cli" type cli struct { hub hub.Client