From ab12ebdc02b6bf0e76037bb1df213e10657ea418 Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Wed, 27 May 2026 11:56:50 +0200 Subject: [PATCH] feat(remote-tasks): gate remote annotation URLs Check remote task and pipeline URLs before fetching them. HTTPS URLs continue to work by default, and operators can restrict them with the remote task URL allowlist. HTTP URLs stay blocked unless an operator explicitly allows that host with an http:// allowlist entry. Apply the same checks before same-provider fetches, and keep large provider responses from being parsed as pipeline resources. Jira: https://redhat.atlassian.net/browse/SRVKP-12181 Signed-off-by: Chmouel Boudjnah Co-Authored-By: Claude --- config/302-pac-configmap.yaml | 17 + docs/content/docs/api/configmap.md | 30 ++ .../docs/guides/creating-pipelines/_index.md | 2 +- .../docs/guides/pipeline-resolution/_index.md | 18 +- pkg/matcher/annotation_tasks_install.go | 53 ++- pkg/matcher/annotation_tasks_install_test.go | 191 +++++++--- pkg/params/clients/clients.go | 2 + pkg/params/clients/clients_test.go | 356 +++++++++++++++++ pkg/params/clients/remote_resource.go | 359 ++++++++++++++++++ pkg/params/settings/config.go | 67 ++++ pkg/params/settings/config_test.go | 37 ++ pkg/resolve/remote_test.go | 33 +- ...remote-pipeline-with-relative-tasks-1.yaml | 2 +- ...ine-with-relative-tasks-same-pipeline.yaml | 2 +- .../remote-pipeline-with-relative-tasks.yaml | 2 +- ...peline-with-remote-task-from-pipeline.yaml | 2 +- ...ine-with-remote-task-from-pipelinerun.yaml | 4 +- ...pipelinerun-annotations-and-tektondir.yaml | 4 +- ...n-annotations-and-pipeline-annotation.yaml | 6 +- ...pipelinerun-annotations-and-tektondir.yaml | 4 +- pkg/test/http/http.go | 79 ++++ pkg/test/provider/testwebvcs.go | 3 +- test/gitea_test.go | 36 ++ 23 files changed, 1209 insertions(+), 100 deletions(-) create mode 100644 pkg/params/clients/remote_resource.go diff --git a/config/302-pac-configmap.yaml b/config/302-pac-configmap.yaml index 5c150cfe3c..68ec3ababb 100644 --- a/config/302-pac-configmap.yaml +++ b/config/302-pac-configmap.yaml @@ -70,6 +70,23 @@ data: # Allow fetching remote tasks remote-tasks: "true" + # Optional comma-separated host allowlist for remote annotation URLs. Bare + # hosts and https:// hosts allow HTTPS fetches. HTTP fetches are disabled + # unless the host is explicitly listed with http://. + # Wildcards such as "*.example.com" match host suffixes; broad patterns such + # as "*.com" are allowed but risky. + remote-tasks-url-allowlist: "" + + # Optional comma-separated CIDRs to block in addition to built-in unsafe + # ranges such as loopback, link-local, private, multicast, and shared address + # space. These built-in checks block typical Kubernetes service targets when + # they resolve to private cluster IPs. Use this to block additional + # cluster-specific service and pod CIDRs. + remote-tasks-url-blocked-cidrs: "" + + # Maximum response size, in bytes, for remote annotation fetches. + remote-tasks-url-max-response-size: "1048576" + # Using the URL of the Tekton dashboard, Pipelines-as-Code generates a URL to the # PipelineRun on the Tekton dashboard tekton-dashboard-url: "" diff --git a/docs/content/docs/api/configmap.md b/docs/content/docs/api/configmap.md index 1c47256d97..de2edc65e0 100644 --- a/docs/content/docs/api/configmap.md +++ b/docs/content/docs/api/configmap.md @@ -116,6 +116,33 @@ remote-tasks: "true" {{< /param >}} +{{< param name="remote-tasks-url-allowlist" type="string" default="" id="param-remote-tasks-url-allowlist" >}} +Restricts remote annotation URLs to a comma-separated list of hosts. Leave empty to allow HTTPS fetches from any public host after IP-range safety checks. Bare hosts and `https://` entries allow HTTPS; plaintext HTTP requires an explicit `http://` entry for the host. Wildcards such as `*.example.com` match host suffixes; broad patterns such as `*.com` are allowed but risky and should be used only when the operator intends that scope. + +```yaml +remote-tasks-url-allowlist: "raw.githubusercontent.com,https://gitlab.example.com,http://internal.example.com" +``` + +{{< /param >}} + +{{< param name="remote-tasks-url-blocked-cidrs" type="string" default="" id="param-remote-tasks-url-blocked-cidrs" >}} +Adds comma-separated CIDRs that remote annotation URLs must not reach. Pipelines-as-Code always blocks loopback, link-local, private, multicast, unspecified, and shared address space. These built-in checks block typical Kubernetes service targets when they resolve to private cluster IPs; use this setting for any additional cluster-specific pod or service CIDRs. + +```yaml +remote-tasks-url-blocked-cidrs: "10.128.0.0/14,172.30.0.0/16" +``` + +{{< /param >}} + +{{< param name="remote-tasks-url-max-response-size" type="integer" default="1048576" id="param-remote-tasks-url-max-response-size" >}} +Maximum response size, in bytes, for remote annotation fetches. + +```yaml +remote-tasks-url-max-response-size: "1048576" +``` + +{{< /param >}} + ### Dashboard Integration {{< param name="tekton-dashboard-url" type="string" id="param-tekton-dashboard-url" >}} @@ -364,6 +391,9 @@ data: hub-url: "https://artifacthub.io/api/v1" hub-catalog-type: "artifacthub" remote-tasks: "true" + remote-tasks-url-allowlist: "" + remote-tasks-url-blocked-cidrs: "" + remote-tasks-url-max-response-size: "1048576" tekton-dashboard-url: "https://tekton.example.com" diff --git a/docs/content/docs/guides/creating-pipelines/_index.md b/docs/content/docs/guides/creating-pipelines/_index.md index b5af8d0ebb..28cd9d5b49 100644 --- a/docs/content/docs/guides/creating-pipelines/_index.md +++ b/docs/content/docs/guides/creating-pipelines/_index.md @@ -5,7 +5,7 @@ weight: 1 This page covers how to write PipelineRun definitions that Pipelines-as-Code picks up from your `.tekton/` directory. Use it when you need to define CI/CD pipelines triggered by Git events. -Pipelines-as-Code follows the standard Tekton template format as closely as possible. You write your templates as `.yaml` files in the `.tekton/` directory at the top level of your repository, and Pipelines-as-Code runs them. You can reference YAML files in other repositories using [remote HTTP URLs]({{< relref "/docs/guides/pipeline-resolution#remote-http-url" >}}), but PipelineRuns only trigger from events in the repository that contains the `.tekton/` directory. +Pipelines-as-Code follows the standard Tekton template format as closely as possible. You write your templates as `.yaml` files in the `.tekton/` directory at the top level of your repository, and Pipelines-as-Code runs them. You can reference YAML files in other repositories using [remote URLs]({{< relref "/docs/guides/pipeline-resolution#remote-url" >}}), but PipelineRuns only trigger from events in the repository that contains the `.tekton/` directory. Using its [resolver]({{< relref "/docs/guides/pipeline-resolution" >}}), Pipelines-as-Code bundles each PipelineRun with all its referenced Tasks into a single self-contained PipelineRun with no external dependencies. diff --git a/docs/content/docs/guides/pipeline-resolution/_index.md b/docs/content/docs/guides/pipeline-resolution/_index.md index 2de23b72bd..971bb6f480 100644 --- a/docs/content/docs/guides/pipeline-resolution/_index.md +++ b/docs/content/docs/guides/pipeline-resolution/_index.md @@ -3,7 +3,7 @@ title: Resolver weight: 3 --- -This page explains how the Pipelines-as-Code resolver processes your `.tekton/` directory and assembles self-contained PipelineRuns. It covers the overall resolution process and the `pipelinesascode.tekton.dev/task` annotation for fetching tasks from Artifact Hub, HTTP URLs, or your repository. For the `pipelinesascode.tekton.dev/pipeline` annotation that references remote Pipelines, see [Remote Pipelines]({{< relref "remote-pipelines" >}}). +This page explains how the Pipelines-as-Code resolver processes your `.tekton/` directory and assembles self-contained PipelineRuns. It covers the overall resolution process and the `pipelinesascode.tekton.dev/task` annotation for fetching tasks from Artifact Hub, remote URLs, or your repository. For the `pipelinesascode.tekton.dev/pipeline` annotation that references remote Pipelines, see [Remote Pipelines]({{< relref "remote-pipelines" >}}). The resolver exists to solve a practical problem: Tekton PipelineRuns can reference external Tasks and Pipelines by name, but those references must be available on the cluster at runtime. Rather than requiring you to pre-install every Task, Pipelines-as-Code resolves all references at submission time and embeds everything into a single PipelineRun. This ensures your pipeline is fully self-contained and portable. @@ -36,7 +36,7 @@ If you need to test your PipelineRun locally before sending it in a pull request ## Remote task annotations -Remote task annotations let you pull Task and Pipeline definitions from external sources -- such as Artifact Hub, HTTP URLs, or other repositories -- without committing them to your `.tekton/` directory. Pipelines-as-Code fetches and inlines them during resolution. +Remote task annotations let you pull Task and Pipeline definitions from external sources -- such as Artifact Hub, remote URLs, or other repositories -- without committing them to your `.tekton/` directory. Pipelines-as-Code fetches and inlines them during resolution. If the resolver finds a PipelineRun referencing a remote task or Pipeline through an annotation, it automatically fetches and inlines the resource. @@ -57,7 +57,7 @@ pipelinesascode.tekton.dev/task: "[git-clone, pylint]" ### Hub support for tasks {{< callout type="warning" >}} -**Deprecated**: Tekton Hub integration (the `tektonhub` catalog type) is deprecated and will be removed in a future release. If you are using a self-hosted Tekton Hub instance, please migrate to [Artifact Hub](https://artifacthub.io) or fetch tasks directly from a [remote URL](#remote-http-url) or [git repository](#tasks-inside-the-repository). The default Artifact Hub integration is unaffected. +**Deprecated**: Tekton Hub integration (the `tektonhub` catalog type) is deprecated and will be removed in a future release. If you are using a self-hosted Tekton Hub instance, please migrate to [Artifact Hub](https://artifacthub.io) or fetch tasks directly from a [remote URL](#remote-url) or [git repository](#tasks-inside-the-repository). The default Artifact Hub integration is unaffected. {{< /callout >}} [Artifact Hub](https://artifacthub.io/packages/search?kind=7&kind=11) is a public registry where the Tekton community publishes reusable Tasks and Pipelines. When you reference a task by name alone, Pipelines-as-Code fetches it from Artifact Hub by default. @@ -108,15 +108,17 @@ There is no fallback between different hubs. If Pipelines-as-Code does not find There is no support for custom hubs from the CLI when using the `tkn pac resolve` command. -### Remote HTTP URL +### Remote URL -If the annotation value starts with `http://` or `https://`, Pipelines-as-Code fetches the task directly from that remote URL: +If the annotation value starts with `https://`, Pipelines-as-Code fetches the task directly after applying the remote URL safety checks. + +Plaintext `http://` URLs are disabled by default. A cluster operator can opt in for a specific host by adding an `http://` entry to `remote-tasks-url-allowlist`. ```yaml pipelinesascode.tekton.dev/task: "[https://remote.url/task.yaml]" ``` -### Remote HTTP URL from a private repository +### Remote URLs from a private repository If you use the GitHub or GitLab provider and the remote task URL uses the same host as the Repository CR, Pipelines-as-Code uses the provided token to fetch the URL through the GitHub or GitLab API. This lets you reference tasks from private repositories without exposing credentials. @@ -126,11 +128,11 @@ When you use the GitHub provider and your repository URL looks like this: -and the remote HTTP URL is a GitHub "blob" URL: +and the remote URL is a GitHub "blob" URL: -If the remote HTTP URL has a slash (`/`) in the branch name, you need to URL-encode it with the `%2F` character: +If the remote URL has a slash (`/`) in the branch name, you need to URL-encode it with the `%2F` character: diff --git a/pkg/matcher/annotation_tasks_install.go b/pkg/matcher/annotation_tasks_install.go index dc659b6e8f..17cf9b5c19 100644 --- a/pkg/matcher/annotation_tasks_install.go +++ b/pkg/matcher/annotation_tasks_install.go @@ -12,6 +12,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/hub" hubtypes "github.com/openshift-pipelines/pipelines-as-code/pkg/hub/vars" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -34,6 +35,19 @@ type RemoteTasks struct { DeprecatedHubResources []string // tracks resources resolved from deprecated tektonhub catalogs } +func (rt RemoteTasks) remoteResourceFetchOptions() clients.RemoteResourceFetchOptions { + if rt.Run == nil || rt.Run.Info.Pac == nil { + return clients.RemoteResourceFetchOptions{ + MaxResponseBytes: clients.DefaultRemoteResourceMaxResponseBytes, + } + } + return clients.RemoteResourceFetchOptions{ + AllowedHosts: rt.Run.Info.Pac.RemoteTasksURLAllowlist, + BlockedCIDRs: rt.Run.Info.Pac.RemoteTasksURLBlockedCIDRs, + MaxResponseBytes: int64(rt.Run.Info.Pac.RemoteTasksURLMaxResponseSize), + } +} + // nolint: dupl func (rt *RemoteTasks) convertToPipeline(ctx context.Context, uri, data string) (*tektonv1.Pipeline, error) { decoder := k8scheme.Codecs.UniversalDeserializer() @@ -101,20 +115,43 @@ func (rt *RemoteTasks) convertTotask(ctx context.Context, uri, data string) (*te func (rt *RemoteTasks) getRemote(ctx context.Context, uri string, fromHub bool, kind string) (string, error) { rt.Logger.Debugf("getRemote: uri=%s kind=%s fromHub=%t", uri, kind, fromHub) - if fetchedFromURIFromProvider, task, err := rt.ProviderInterface.GetTaskURI(ctx, rt.Event, uri); fetchedFromURIFromProvider { - rt.Logger.Debugf("getRemote: fetched %s via provider hook for uri=%s", kind, uri) - return task, err - } + fetchOptions := rt.remoteResourceFetchOptions() + loweredURI := strings.ToLower(uri) - switch { - case strings.HasPrefix(uri, "https://"), strings.HasPrefix(uri, "http://"): // if it starts with http(s)://, it is a remote resource + if strings.HasPrefix(loweredURI, "http://") || strings.HasPrefix(loweredURI, "https://") { + if err := clients.ValidateRemoteResourceProviderURL(uri, fetchOptions); err != nil { + return "", err + } + if fetchedFromURIFromProvider, task, err := rt.ProviderInterface.GetTaskURI(ctx, rt.Event, uri); fetchedFromURIFromProvider { + rt.Logger.Debugf("getRemote: fetched %s via provider hook for uri=%s", kind, uri) + if err != nil { + return task, err + } + if err := clients.CheckRemoteResourceResponseSize(int64(len(task)), fetchOptions); err != nil { + return "", err + } + return task, nil + } rt.Logger.Debugf("getRemote: fetching %s from http(s) url", kind) - data, err := rt.Run.Clients.GetURL(ctx, uri) + data, err := rt.Run.Clients.GetRemoteResourceURL(ctx, uri, fetchOptions) if err != nil { return "", err } - rt.Logger.Infof("successfully fetched %s from remote HTTPS URL", uri) + rt.Logger.Infof("successfully fetched %s from remote URL", uri) return string(data), nil + } + if fetchedFromURIFromProvider, task, err := rt.ProviderInterface.GetTaskURI(ctx, rt.Event, uri); fetchedFromURIFromProvider { + rt.Logger.Debugf("getRemote: fetched %s via provider hook for uri=%s", kind, uri) + if err != nil { + return task, err + } + if err := clients.CheckRemoteResourceResponseSize(int64(len(task)), fetchOptions); err != nil { + return "", err + } + return task, nil + } + + switch { case fromHub && strings.Contains(uri, "://"): // if it contains ://, it is a remote custom catalog split := strings.Split(uri, "://") catalogID := split[0] diff --git a/pkg/matcher/annotation_tasks_install_test.go b/pkg/matcher/annotation_tasks_install_test.go index 12b3b013fd..b84dc94ecc 100644 --- a/pkg/matcher/annotation_tasks_install_test.go +++ b/pkg/matcher/annotation_tasks_install_test.go @@ -234,45 +234,76 @@ func TestGetTaskFromAnnotationName(t *testing.T) { Type: hubtype.ArtifactHubType, }) tests := []struct { - task string - filesInsideRepo map[string]string - gotTaskName string - name string - remoteURLS map[string]map[string]string - runevent info.Event - wantErr string - wantLog string - wantProviderRemoteTask bool - wantDeprecated bool + task string + filesInsideRepo map[string]string + gotTaskName string + name string + remoteURLS map[string]map[string]string + runevent info.Event + wantErr string + wantLog string + remoteTasksURLAllowlist string + remoteTasksURLMaxSize int + providerRemoteTask string + wantProviderRemoteTask bool + wantDeprecated bool }{ { - name: "test-annotations-error-remote-http-not-k8", - task: "http://remote.task", + name: "blocked/remote task http before provider", + task: "http://provider/remote.task", + wantProviderRemoteTask: true, + wantErr: "scheme \"http\" is not allowed", + }, + { + name: "blocked/remote task uppercase http before provider", + task: "HTTP://provider/remote.task", + wantProviderRemoteTask: true, + wantErr: "scheme \"http\" is not allowed", + }, + { + name: "test-annotations-remote-http-with-allowlist", + task: "http://93.184.216.34/remote.task", + gotTaskName: "task", + remoteTasksURLAllowlist: "http://93.184.216.34", remoteURLS: map[string]map[string]string{ - "http://remote.task": { - "body": "", + "http://93.184.216.34/remote.task": { + "body": readTDfile(t, "task-good"), "code": "200", }, }, - wantErr: "not found", + }, + { + name: "test-provider-remote-http-with-allowlist", + task: "http://93.184.216.34/provider/remote.task", + gotTaskName: "task", + remoteTasksURLAllowlist: "http://93.184.216.34", + providerRemoteTask: readTDfile(t, "task-good"), + wantProviderRemoteTask: true, }, { name: "test-good-coming-from-provider", - task: "http://provider/remote.task", + task: "https://93.184.216.34/provider/remote.task", wantProviderRemoteTask: true, wantErr: "not found", }, + { + name: "test-provider-remote-https-with-loopback-dns", + task: "https://localhost/provider/remote.task", + gotTaskName: "task", + providerRemoteTask: readTDfile(t, "task-good"), + wantProviderRemoteTask: true, + }, { name: "test-bad-coming-from-provider", - task: "http://provider/remote.task", + task: "https://93.184.216.34/provider/remote.task", wantProviderRemoteTask: false, wantErr: "error getting remote task", }, { - name: "test-annotations-remote-http", - task: "http://remote.task", + name: "test-annotations-remote-https", + task: "https://93.184.216.34/remote.task", remoteURLS: map[string]map[string]string{ - "http://remote.task": { + "https://93.184.216.34/remote.task": { "body": readTDfile(t, "task-good"), "code": "200", }, @@ -294,26 +325,47 @@ func TestGetTaskFromAnnotationName(t *testing.T) { // wantErr: "cannot be validated properly", // }, { - name: "test-annotations-remote-https", - task: "https://remote.task", + name: "test-annotations-remote-https-hostname", + task: "https://93.184.216.34/task.yaml", gotTaskName: "task", remoteURLS: map[string]map[string]string{ - "https://remote.task": { + "https://93.184.216.34/task.yaml": { "body": readTDfile(t, "task-good"), "code": "200", }, }, }, + { + name: "blocked/remote task loopback", + task: "https://127.0.0.1/task.yaml", + wantErr: "loopback", + }, + { + name: "blocked/provider remote task by allowlist", + task: "https://93.184.216.34/provider/remote.task", + remoteTasksURLAllowlist: "example.com", + providerRemoteTask: readTDfile(t, "task-good"), + wantProviderRemoteTask: true, + wantErr: "not in the allowlist", + }, + { + name: "blocked/provider remote task response too large", + task: "https://93.184.216.34/provider/remote.task", + remoteTasksURLMaxSize: 5, + providerRemoteTask: "123456", + wantProviderRemoteTask: true, + wantErr: "exceeds 5 bytes", + }, { name: "bad/not a tasl", - task: "http://remote.task", + task: "https://93.184.216.34/not-a-task.yaml", remoteURLS: map[string]map[string]string{ - "http://remote.task": { + "https://93.184.216.34/not-a-task.yaml": { "body": readTDfile(t, "pipeline-good"), "code": "200", }, }, - wantErr: "remote task from URI http://remote.task has not been recognized as a Tekton task", + wantErr: "remote task from URI https://93.184.216.34/not-a-task.yaml has not been recognized as a Tekton task", }, { name: "test-annotations-inside-repo", @@ -431,7 +483,7 @@ func TestGetTaskFromAnnotationName(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - httpTestClient := httptesthelper.MakeHTTPTestClient(tt.remoteURLS) + httpTestClient := httptesthelper.MakeHTTPTransportTestClient(t, tt.remoteURLS) observer, fakelog := zapobserver.New(zap.InfoLevel) logger := zap.New(observer).Sugar() cs := ¶ms.Run{ @@ -442,7 +494,9 @@ func TestGetTaskFromAnnotationName(t *testing.T) { Info: info.Info{ Pac: &info.PacOpts{ Settings: settings.Settings{ - HubCatalogs: &hubCatalogs, + HubCatalogs: &hubCatalogs, + RemoteTasksURLAllowlist: tt.remoteTasksURLAllowlist, + RemoteTasksURLMaxResponseSize: tt.remoteTasksURLMaxSize, }, }, }, @@ -453,6 +507,7 @@ func TestGetTaskFromAnnotationName(t *testing.T) { Logger: logger, ProviderInterface: &provider.TestProviderImp{ FilesInsideRepo: tt.filesInsideRepo, + ProviderRemoteTask: tt.providerRemoteTask, WantProviderRemoteTask: tt.wantProviderRemoteTask, }, Event: &tt.runevent, @@ -514,22 +569,25 @@ func TestGetPipelineFromAnnotationName(t *testing.T) { Type: hubtype.ArtifactHubType, }) tests := []struct { - pipeline string - filesInsideRepo map[string]string - gotPipelineName string - name string - remoteURLS map[string]map[string]string - runevent info.Event - wantErr string - wantLog string - wantDeprecated bool + pipeline string + filesInsideRepo map[string]string + gotPipelineName string + name string + remoteURLS map[string]map[string]string + runevent info.Event + wantErr string + wantLog string + remoteTasksURLAllowlist string + providerRemoteTask string + wantProviderRemoteTask bool + wantDeprecated bool }{ { - name: "good/fetching from remote http", + name: "good/fetching from remote https", gotPipelineName: "pipeline", - pipeline: "http://remote.pipeline", + pipeline: "https://93.184.216.34/remote.pipeline", remoteURLS: map[string]map[string]string{ - "http://remote.pipeline": { + "https://93.184.216.34/remote.pipeline": { "body": readTDfile(t, "pipeline-good"), "code": "200", }, @@ -538,14 +596,33 @@ func TestGetPipelineFromAnnotationName(t *testing.T) { { name: "good/fetching with bundle", gotPipelineName: "pipeline", - pipeline: "http://remote.pipeline", + pipeline: "https://93.184.216.34/remote-pipeline-bundle", remoteURLS: map[string]map[string]string{ - "http://remote.pipeline": { + "https://93.184.216.34/remote-pipeline-bundle": { "body": readTDfile(t, "pipeline-good-bundle"), "code": "200", }, }, }, + { + name: "blocked/remote pipeline http before provider", + pipeline: "http://provider/remote.pipeline", + providerRemoteTask: readTDfile(t, "pipeline-good"), + wantProviderRemoteTask: true, + wantErr: "scheme \"http\" is not allowed", + }, + { + name: "good/fetching from remote http with allowlist", + gotPipelineName: "pipeline", + pipeline: "http://93.184.216.34/remote.pipeline", + remoteURLS: map[string]map[string]string{ + "http://93.184.216.34/remote.pipeline": { + "body": readTDfile(t, "pipeline-good"), + "code": "200", + }, + }, + remoteTasksURLAllowlist: "http://93.184.216.34", + }, // TODO: to uncomment in the future when fixing the Valdiate bug issue // { // name: "invalid-pipeline-validaton-failed", @@ -575,9 +652,9 @@ func TestGetPipelineFromAnnotationName(t *testing.T) { // }, { name: "bad/error getting pipeline", - pipeline: "http://remote.pipeline", + pipeline: "https://93.184.216.34/error.pipeline", remoteURLS: map[string]map[string]string{ - "http://remote.pipeline": { + "https://93.184.216.34/error.pipeline": { "code": "501", }, }, @@ -585,25 +662,30 @@ func TestGetPipelineFromAnnotationName(t *testing.T) { }, { name: "bad/not a pipeline", - pipeline: "http://remote.pipeline", + pipeline: "https://93.184.216.34/not-a-pipeline.yaml", remoteURLS: map[string]map[string]string{ - "http://remote.pipeline": { + "https://93.184.216.34/not-a-pipeline.yaml": { "body": readTDfile(t, "task-good"), "code": "200", }, }, - wantErr: "remote pipeline from URI http://remote.pipeline has not been recognized as a Tekton pipeline", + wantErr: "remote pipeline from URI https://93.184.216.34/not-a-pipeline.yaml has not been recognized as a Tekton pipeline", }, { name: "bad/could not get remote", - pipeline: "http://nowhere.pipeline", + pipeline: "https://93.184.216.34/nowhere.pipeline", wantErr: "error getting remote pipeline", }, + { + name: "blocked/remote pipeline loopback", + pipeline: "https://127.0.0.1/pipeline.yaml", + wantErr: "loopback", + }, { name: "bad/not found", - pipeline: "http://remote.pipeline", + pipeline: "https://93.184.216.34/not-found.pipeline", remoteURLS: map[string]map[string]string{ - "http://remote.pipeline": { + "https://93.184.216.34/not-found.pipeline": { "body": "", "code": "200", }, @@ -701,7 +783,7 @@ func TestGetPipelineFromAnnotationName(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - httpTestClient := httptesthelper.MakeHTTPTestClient(tt.remoteURLS) + httpTestClient := httptesthelper.MakeHTTPTransportTestClient(t, tt.remoteURLS) observer, fakelog := zapobserver.New(zap.InfoLevel) logger := zap.New(observer).Sugar() @@ -713,7 +795,8 @@ func TestGetPipelineFromAnnotationName(t *testing.T) { Info: info.Info{ Pac: &info.PacOpts{ Settings: settings.Settings{ - HubCatalogs: &hubCatalogs, + HubCatalogs: &hubCatalogs, + RemoteTasksURLAllowlist: tt.remoteTasksURLAllowlist, }, }, }, @@ -723,7 +806,9 @@ func TestGetPipelineFromAnnotationName(t *testing.T) { Run: cs, Logger: logger, ProviderInterface: &provider.TestProviderImp{ - FilesInsideRepo: tt.filesInsideRepo, + FilesInsideRepo: tt.filesInsideRepo, + ProviderRemoteTask: tt.providerRemoteTask, + WantProviderRemoteTask: tt.wantProviderRemoteTask, }, Event: &tt.runevent, } diff --git a/pkg/params/clients/clients.go b/pkg/params/clients/clients.go index db3465ec7e..6592c353d8 100644 --- a/pkg/params/clients/clients.go +++ b/pkg/params/clients/clients.go @@ -26,6 +26,8 @@ const ( // of 100 seconds so using that value. ConnectMaxWaitTime = 100 * time.Second RequestMaxWaitTime = 100 * time.Second + + DefaultRemoteResourceMaxResponseBytes int64 = 1024 * 1024 ) type Clients struct { diff --git a/pkg/params/clients/clients_test.go b/pkg/params/clients/clients_test.go index 1a8f239b4e..48dd577646 100644 --- a/pkg/params/clients/clients_test.go +++ b/pkg/params/clients/clients_test.go @@ -1,6 +1,14 @@ package clients import ( + "bufio" + "bytes" + "context" + "io" + "net" + "net/http" + "net/http/httptest" + "strconv" "testing" httptesthelper "github.com/openshift-pipelines/pipelines-as-code/pkg/test/http" @@ -54,3 +62,351 @@ func TestClientsGetURL(t *testing.T) { }) } } + +func TestClientsGetRemoteResourceURL(t *testing.T) { + tests := []struct { + name string + client *http.Client + opts RemoteResourceFetchOptions + url string + want string + wantErr string + }{ + { + name: "good public host", + client: makeRemoteResourceHTTPSTestClient(t, map[string]map[string]string{ + "https://93.184.216.34/task.yaml": { + "body": "task", + "code": "200", + }, + }), + url: "https://93.184.216.34/task.yaml", + want: "task", + }, + { + name: "blocked plaintext http", + client: httptesthelper.MakeHTTPTestClient(map[string]map[string]string{ + "http://example.com/task.yaml": { + "body": "task", + "code": "200", + }, + }), + url: "http://example.com/task.yaml", + wantErr: "scheme \"http\" is not allowed", + }, + { + name: "blocked plaintext http with bare allowlist host", + client: httptesthelper.MakeHTTPTestClient(map[string]map[string]string{ + "http://93.184.216.34/task.yaml": { + "body": "task", + "code": "200", + }, + }), + opts: RemoteResourceFetchOptions{ + AllowedHosts: "93.184.216.34", + }, + url: "http://93.184.216.34/task.yaml", + wantErr: "explicitly allowlisted with http://", + }, + { + name: "allowed plaintext http with scheme-qualified allowlist host", + client: makeRemoteResourceHTTPTestClient(map[string]map[string]string{ + "http://93.184.216.34/task.yaml": { + "body": "task", + "code": "200", + }, + }), + opts: RemoteResourceFetchOptions{ + AllowedHosts: "http://93.184.216.34", + }, + url: "http://93.184.216.34/task.yaml", + want: "task", + }, + { + name: "https blocked by http-only allowlist host", + client: httptesthelper.MakeHTTPTestClient(map[string]map[string]string{ + "https://93.184.216.34/task.yaml": { + "body": "task", + "code": "200", + }, + }), + opts: RemoteResourceFetchOptions{ + AllowedHosts: "http://93.184.216.34", + }, + url: "https://93.184.216.34/task.yaml", + wantErr: "not in the allowlist", + }, + { + name: "blocked loopback literal", + client: httptesthelper.MakeHTTPTestClient(map[string]map[string]string{ + "https://127.0.0.1/task.yaml": { + "body": "task", + "code": "200", + }, + }), + url: "https://127.0.0.1/task.yaml", + wantErr: "loopback", + }, + { + name: "blocked loopback from dns", + client: httptesthelper.MakeHTTPTestClient(map[string]map[string]string{ + "https://localhost/task.yaml": { + "body": "task", + "code": "200", + }, + }), + url: "https://localhost/task.yaml", + wantErr: "loopback", + }, + { + name: "blocked private literal", + client: httptesthelper.MakeHTTPTestClient(map[string]map[string]string{ + "https://10.0.0.1/task.yaml": { + "body": "task", + "code": "200", + }, + }), + url: "https://10.0.0.1/task.yaml", + wantErr: "private", + }, + { + name: "blocked shared address space literal", + client: httptesthelper.MakeHTTPTestClient(map[string]map[string]string{ + "https://100.64.0.1/task.yaml": { + "body": "task", + "code": "200", + }, + }), + url: "https://100.64.0.1/task.yaml", + wantErr: "shared address space", + }, + { + name: "blocked by allowlist", + client: httptesthelper.MakeHTTPTestClient(map[string]map[string]string{ + "https://example.com/task.yaml": { + "body": "task", + "code": "200", + }, + }), + opts: RemoteResourceFetchOptions{ + AllowedHosts: "allowed.example.com", + }, + url: "https://example.com/task.yaml", + wantErr: "not in the allowlist", + }, + { + name: "response too large", + client: makeRemoteResourceHTTPSTestClient(t, map[string]map[string]string{ + "https://93.184.216.34/task.yaml": { + "body": "123456", + "code": "200", + }, + }), + opts: RemoteResourceFetchOptions{ + MaxResponseBytes: 5, + }, + url: "https://93.184.216.34/task.yaml", + wantErr: "exceeds 5 bytes", + }, + { + name: "redirect to blocked host", + client: makeRemoteResourceHTTPSTestClient(t, map[string]map[string]string{ + "https://93.184.216.34/task.yaml": { + "code": "302", + "location": "http://169.254.169.254/latest/meta-data", + }, + }), + url: "https://93.184.216.34/task.yaml", + wantErr: "blocked redirect target", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, _ := rtesting.SetupFakeContext(t) + c := &Clients{ + HTTP: *tt.client, + } + got, err := c.GetRemoteResourceURL(ctx, tt.url, tt.opts) + if tt.wantErr != "" { + assert.ErrorContains(t, err, tt.wantErr) + return + } + assert.NilError(t, err) + assert.Equal(t, string(got), tt.want) + }) + } +} + +type remoteResourceHTTPTestConn struct { + net.Conn + remoteAddr net.Addr +} + +func (c remoteResourceHTTPTestConn) RemoteAddr() net.Addr { + return c.remoteAddr +} + +type remoteResourceHTTPTestAddr string + +func (a remoteResourceHTTPTestAddr) Network() string { + return "tcp" +} + +func (a remoteResourceHTTPTestAddr) String() string { + return string(a) +} + +func makeRemoteResourceHTTPTestClient(config map[string]map[string]string) *http.Client { + return &http.Client{ + Transport: &http.Transport{ + DialContext: func(context.Context, string, string) (net.Conn, error) { + clientConn, serverConn := net.Pipe() + go serveRemoteResourceHTTPTestConn(serverConn, config) + return remoteResourceHTTPTestConn{ + Conn: clientConn, + remoteAddr: remoteResourceHTTPTestAddr("93.184.216.34:80"), + }, nil + }, + }, + } +} + +func makeRemoteResourceHTTPSTestClient(t *testing.T, config map[string]map[string]string) *http.Client { + t.Helper() + + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + code, body, location := remoteResourceHTTPTestResponse(req, "https", config) + if location != "" { + w.Header().Set("Location", location) + } + w.WriteHeader(code) + _, _ = io.WriteString(w, body) + })) + t.Cleanup(server.Close) + + httpClient := server.Client() + baseTransport, ok := httpClient.Transport.(*http.Transport) + assert.Assert(t, ok) + transport := baseTransport.Clone() + transport.TLSClientConfig = transport.TLSClientConfig.Clone() + transport.TLSClientConfig.ServerName = "example.com" + transport.DialContext = func(ctx context.Context, network, _ string) (net.Conn, error) { + conn, err := (&net.Dialer{}).DialContext(ctx, network, server.Listener.Addr().String()) + if err != nil { + return nil, err + } + return remoteResourceHTTPTestConn{ + Conn: conn, + remoteAddr: remoteResourceHTTPTestAddr("93.184.216.34:443"), + }, nil + } + return &http.Client{ + Transport: transport, + } +} + +func serveRemoteResourceHTTPTestConn(conn net.Conn, config map[string]map[string]string) { + defer conn.Close() + + req, err := http.ReadRequest(bufio.NewReader(conn)) + if err != nil { + return + } + defer req.Body.Close() + + response := &http.Response{ + StatusCode: http.StatusNotFound, + ProtoMajor: 1, + ProtoMinor: 1, + Header: make(http.Header), + Body: http.NoBody, + Request: req, + } + code, body, location := remoteResourceHTTPTestResponse(req, "http", config) + response.StatusCode = code + if body != "" { + response.Body = io.NopCloser(bytes.NewBufferString(body)) + } + if location != "" { + response.Header.Set("Location", location) + } + _ = response.Write(conn) +} + +func remoteResourceHTTPTestResponse(req *http.Request, scheme string, config map[string]map[string]string) (int, string, string) { + values, ok := config[scheme+"://"+req.Host+req.URL.RequestURI()] + if !ok { + return http.StatusNotFound, "", "" + } + code, _ := strconv.Atoi(values["code"]) + return code, values["body"], values["location"] +} + +func TestSafeRemoteResourceHTTPClientReplacesNonStandardTransport(t *testing.T) { + policy, err := newRemoteResourcePolicy(RemoteResourceFetchOptions{}) + assert.NilError(t, err) + + mockTransport := httptesthelper.RoundTripFunc(func(req *http.Request) *http.Response { + return &http.Response{ + StatusCode: http.StatusOK, + Body: http.NoBody, + Request: req, + } + }) + + c := &Clients{ + HTTP: http.Client{ + Transport: mockTransport, + }, + } + + httpClient := c.safeRemoteResourceHTTPClient(policy) + transport, ok := httpClient.Transport.(*http.Transport) + assert.Assert(t, ok, "non-standard transport should fall back to a guarded http.Transport") + assert.Assert(t, transport.DialContext != nil) +} + +func TestSafeRemoteResourceHTTPClientWrapsDefaultTransport(t *testing.T) { + policy, err := newRemoteResourcePolicy(RemoteResourceFetchOptions{}) + assert.NilError(t, err) + + c := &Clients{ + HTTP: http.Client{}, + } + + httpClient := c.safeRemoteResourceHTTPClient(policy) + transport, ok := httpClient.Transport.(*http.Transport) + assert.Assert(t, ok, "nil transport should fall back to http.DefaultTransport and be wrapped") + assert.Assert(t, transport.DialContext != nil) +} + +func TestSafeRemoteResourceHTTPClientClearsTLSDialHooks(t *testing.T) { + policy, err := newRemoteResourcePolicy(RemoteResourceFetchOptions{}) + assert.NilError(t, err) + + baseTransport := &http.Transport{ + DialContext: func(context.Context, string, string) (net.Conn, error) { + return nil, nil + }, + DialTLSContext: func(context.Context, string, string) (net.Conn, error) { + return nil, nil + }, + DialTLS: func(string, string) (net.Conn, error) { + return nil, nil + }, + } + c := &Clients{ + HTTP: http.Client{ + Transport: baseTransport, + }, + } + + httpClient := c.safeRemoteResourceHTTPClient(policy) + transport, ok := httpClient.Transport.(*http.Transport) + assert.Assert(t, ok) + assert.Assert(t, transport.DialContext != nil) + assert.Assert(t, transport.DialTLSContext == nil) + assert.Assert(t, transport.DialTLS == nil) //nolint:staticcheck + assert.Assert(t, baseTransport.DialTLSContext != nil) + assert.Assert(t, baseTransport.DialTLS != nil) //nolint:staticcheck +} diff --git a/pkg/params/clients/remote_resource.go b/pkg/params/clients/remote_resource.go new file mode 100644 index 0000000000..b6d684db2d --- /dev/null +++ b/pkg/params/clients/remote_resource.go @@ -0,0 +1,359 @@ +package clients + +import ( + "context" + "fmt" + "io" + "net" + "net/http" + "net/netip" + "net/url" + "strings" +) + +type RemoteResourceFetchOptions struct { + AllowedHosts string + BlockedCIDRs string + MaxResponseBytes int64 +} + +type remoteResourcePolicy struct { + allowedHTTPSHosts map[string]bool + allowedHTTPHosts map[string]bool + hasAllowlist bool + blockedCIDRs []netip.Prefix + maxBytes int64 +} + +var cgnatPrefix = netip.MustParsePrefix("100.64.0.0/10") + +func (c *Clients) GetRemoteResourceURL(ctx context.Context, rawURL string, opts RemoteResourceFetchOptions) ([]byte, error) { + policy, err := validateRemoteResourceURL(ctx, rawURL, opts) + if err != nil { + return nil, err + } + + nctx, cancel := context.WithTimeout(ctx, RequestMaxWaitTime) + defer cancel() + + req, err := http.NewRequestWithContext(nctx, http.MethodGet, rawURL, nil) + if err != nil { + return nil, err + } + + httpClient := c.safeRemoteResourceHTTPClient(policy) + res, err := httpClient.Do(req) + if err != nil { + return nil, err + } + defer res.Body.Close() + + statusOK := res.StatusCode >= 200 && res.StatusCode < 300 + if !statusOK { + return nil, fmt.Errorf("Non-OK HTTP status: %d", res.StatusCode) + } + + limitedBody := &io.LimitedReader{R: res.Body, N: policy.maxBytes + 1} + data, err := io.ReadAll(limitedBody) + if err != nil { + return nil, err + } + if int64(len(data)) > policy.maxBytes { + return nil, fmt.Errorf("remote resource response exceeds %d bytes", policy.maxBytes) + } + return data, nil +} + +// ValidateRemoteResourceURL checks whether a remote resource URL is allowed by +// the same policy used by GetRemoteResourceURL, without fetching its content. +func ValidateRemoteResourceURL(ctx context.Context, rawURL string, opts RemoteResourceFetchOptions) error { + _, err := validateRemoteResourceURL(ctx, rawURL, opts) + return err +} + +// ValidateRemoteResourceProviderURL checks whether a provider-backed remote +// resource URL is allowed without resolving the URL host. Provider-backed +// fetches use the provider API instead of dialing the annotation URL directly. +func ValidateRemoteResourceProviderURL(rawURL string, opts RemoteResourceFetchOptions) error { + policy, err := newRemoteResourcePolicy(opts) + if err != nil { + return err + } + parsedURL, err := url.Parse(rawURL) + if err != nil { + return err + } + return policy.validateURL(parsedURL) +} + +// CheckRemoteResourceResponseSize checks a response size against the configured +// remote resource limit. +func CheckRemoteResourceResponseSize(responseSize int64, opts RemoteResourceFetchOptions) error { + policy, err := newRemoteResourcePolicy(opts) + if err != nil { + return err + } + if responseSize > policy.maxBytes { + return fmt.Errorf("remote resource response exceeds %d bytes", policy.maxBytes) + } + return nil +} + +func validateRemoteResourceURL(ctx context.Context, rawURL string, opts RemoteResourceFetchOptions) (remoteResourcePolicy, error) { + policy, err := newRemoteResourcePolicy(opts) + if err != nil { + return policy, err + } + + parsedURL, err := url.Parse(rawURL) + if err != nil { + return policy, err + } + if err := policy.validateURL(parsedURL); err != nil { + return policy, err + } + if err := policy.validateResolvedHost(ctx, parsedURL.Hostname()); err != nil { + return policy, err + } + return policy, nil +} + +func (c *Clients) safeRemoteResourceHTTPClient(policy remoteResourcePolicy) http.Client { + httpClient := c.HTTP + httpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return fmt.Errorf("stopped after 10 redirects") + } + if err := policy.validateURL(req.URL); err != nil { + return fmt.Errorf("blocked redirect target: %w", err) + } + return nil + } + + transport := httpClient.Transport + if transport == nil { + transport = http.DefaultTransport + } + + baseTransport, ok := transport.(*http.Transport) + if !ok { + defaultTransport, defaultOK := http.DefaultTransport.(*http.Transport) + if !defaultOK { + defaultTransport = &http.Transport{} + } + baseTransport = defaultTransport + } + + safeTransport := baseTransport.Clone() + originalDialContext := safeTransport.DialContext + if originalDialContext == nil { + originalDialContext = (&net.Dialer{Timeout: ConnectMaxWaitTime}).DialContext + } + safeTransport.DialTLSContext = nil + safeTransport.DialTLS = nil //nolint:staticcheck // Clear deprecated hook so HTTPS cannot bypass DialContext. + safeTransport.DialContext = func(ctx context.Context, network, address string) (net.Conn, error) { + host, _, err := net.SplitHostPort(address) + if err != nil { + return nil, err + } + if err := policy.validateResolvedHost(ctx, host); err != nil { + return nil, err + } + + conn, err := originalDialContext(ctx, network, address) + if err != nil { + return nil, err + } + if err := policy.validateRemoteAddr(conn.RemoteAddr()); err != nil { + _ = conn.Close() + return nil, err + } + return conn, nil + } + httpClient.Transport = safeTransport + return httpClient +} + +func newRemoteResourcePolicy(opts RemoteResourceFetchOptions) (remoteResourcePolicy, error) { + allowedHTTPSHosts, allowedHTTPHosts, hasAllowlist := parseAllowedHostList(opts.AllowedHosts) + policy := remoteResourcePolicy{ + allowedHTTPSHosts: allowedHTTPSHosts, + allowedHTTPHosts: allowedHTTPHosts, + hasAllowlist: hasAllowlist, + maxBytes: opts.MaxResponseBytes, + } + if policy.maxBytes <= 0 { + policy.maxBytes = DefaultRemoteResourceMaxResponseBytes + } + + blockedCIDRs, err := parseCIDRList(opts.BlockedCIDRs) + if err != nil { + return policy, err + } + policy.blockedCIDRs = blockedCIDRs + return policy, nil +} + +func (p remoteResourcePolicy) validateURL(parsedURL *url.URL) error { + if parsedURL == nil { + return fmt.Errorf("remote resource URL is empty") + } + scheme := strings.ToLower(parsedURL.Scheme) + if scheme != "http" && scheme != "https" { + return fmt.Errorf("remote resource URL scheme %q is not allowed", parsedURL.Scheme) + } + host := parsedURL.Hostname() + if host == "" { + return fmt.Errorf("remote resource URL host is empty") + } + + switch { + case scheme == "http" && !p.isAllowedHost(host, p.allowedHTTPHosts): + return fmt.Errorf("remote resource URL scheme %q is not allowed unless host %q is explicitly allowlisted with http://", scheme, host) + case scheme == "https" && p.hasAllowlist && !p.isAllowedHost(host, p.allowedHTTPSHosts): + return fmt.Errorf("remote resource URL host %q is not in the allowlist", host) + } + + if addr, err := netip.ParseAddr(host); err == nil { + return p.validateAddr(addr) + } + return nil +} + +func (p remoteResourcePolicy) isAllowedHost(host string, allowedHosts map[string]bool) bool { + host = normalizeHost(host) + if allowedHosts[host] { + return true + } + for allowedHost := range allowedHosts { + if strings.HasPrefix(allowedHost, "*.") && strings.HasSuffix(host, strings.TrimPrefix(allowedHost, "*")) { + return true + } + } + return false +} + +func (p remoteResourcePolicy) validateResolvedHost(ctx context.Context, host string) error { + if addr, err := netip.ParseAddr(host); err == nil { + return p.validateAddr(addr) + } + ips, err := net.DefaultResolver.LookupIPAddr(ctx, host) + if err != nil { + return err + } + for _, ip := range ips { + addr, ok := netip.AddrFromSlice(ip.IP) + if !ok { + return fmt.Errorf("cannot parse resolved IP address %q", ip.IP.String()) + } + if err := p.validateAddr(addr); err != nil { + return err + } + } + return nil +} + +func (p remoteResourcePolicy) validateRemoteAddr(remoteAddr net.Addr) error { + if remoteAddr == nil { + return fmt.Errorf("remote address is empty") + } + host, _, err := net.SplitHostPort(remoteAddr.String()) + if err != nil { + return err + } + addr, err := netip.ParseAddr(host) + if err != nil { + return err + } + return p.validateAddr(addr) +} + +func (p remoteResourcePolicy) validateAddr(addr netip.Addr) error { + addr = addr.Unmap() + switch { + case !addr.IsValid(): + return fmt.Errorf("remote resource address is invalid") + case addr.IsUnspecified(): + return fmt.Errorf("remote resource address %s is unspecified", addr) + case addr.IsLoopback(): + return fmt.Errorf("remote resource address %s is loopback", addr) + case addr.IsPrivate(): + return fmt.Errorf("remote resource address %s is private", addr) + case addr.IsLinkLocalUnicast() || addr.IsLinkLocalMulticast(): + return fmt.Errorf("remote resource address %s is link-local", addr) + case addr.IsMulticast(): + return fmt.Errorf("remote resource address %s is multicast", addr) + case cgnatPrefix.Contains(addr): + return fmt.Errorf("remote resource address %s is shared address space", addr) + } + for _, prefix := range p.blockedCIDRs { + if prefix.Contains(addr) { + return fmt.Errorf("remote resource address %s is in blocked CIDR %s", addr, prefix) + } + } + return nil +} + +func parseAllowedHostList(value string) (map[string]bool, map[string]bool, bool) { + httpsHosts := map[string]bool{} + httpHosts := map[string]bool{} + hasAllowlist := false + for _, item := range strings.Split(value, ",") { + item = strings.TrimSpace(item) + if item == "" { + continue + } + hasAllowlist = true + scheme := "https" + host := item + if strings.Contains(item, "://") { + if parsedURL, err := url.Parse(item); err == nil { + scheme = strings.ToLower(parsedURL.Scheme) + host = parsedURL.Host + } + } + host = normalizeHost(host) + if host == "" { + continue + } + switch scheme { + case "http": + httpHosts[host] = true + case "https": + httpsHosts[host] = true + } + } + return httpsHosts, httpHosts, hasAllowlist +} + +func parseCIDRList(value string) ([]netip.Prefix, error) { + var prefixes []netip.Prefix + for _, item := range strings.Split(value, ",") { + item = strings.TrimSpace(item) + if item == "" { + continue + } + prefix, err := netip.ParsePrefix(item) + if err != nil { + return nil, fmt.Errorf("invalid blocked CIDR %q: %w", item, err) + } + prefixes = append(prefixes, prefix.Masked()) + } + return prefixes, nil +} + +func normalizeHost(host string) string { + host = strings.TrimSpace(strings.ToLower(host)) + if host == "" { + return "" + } + if strings.Contains(host, "://") { + if parsedURL, err := url.Parse(host); err == nil { + host = parsedURL.Host + } + } + if splitHost, _, err := net.SplitHostPort(host); err == nil { + host = splitHost + } + return strings.TrimSuffix(host, ".") +} diff --git a/pkg/params/settings/config.go b/pkg/params/settings/config.go index 1420627db5..f31d010582 100644 --- a/pkg/params/settings/config.go +++ b/pkg/params/settings/config.go @@ -3,8 +3,10 @@ package settings import ( "fmt" "net/http" + "net/netip" "net/url" "regexp" + "strconv" "strings" "sync" @@ -50,6 +52,9 @@ type Settings struct { ApplicationName string `default:"Pipelines as Code CI" json:"application-name"` HubCatalogs *sync.Map RemoteTasks bool `default:"true" json:"remote-tasks"` + RemoteTasksURLAllowlist string `json:"remote-tasks-url-allowlist"` + RemoteTasksURLBlockedCIDRs string `json:"remote-tasks-url-blocked-cidrs"` + RemoteTasksURLMaxResponseSize int `default:"1048576" json:"remote-tasks-url-max-response-size"` MaxKeepRunsUpperLimit int `json:"max-keep-run-upper-limit"` DefaultMaxKeepRuns int `json:"default-max-keep-runs"` BitbucketCloudCheckSourceIP bool `default:"true" json:"bitbucket-cloud-check-source-ip"` @@ -115,6 +120,11 @@ func DefaultValidators() map[string]func(string) error { "CustomConsoleURL": isValidURL, "CustomConsolePRTaskLog": startWithHTTPorHTTPS, "CustomConsolePRDetail": startWithHTTPorHTTPS, + "RemoteTasksURLAllowlist": isValidHostList, + "RemoteTasksURLBlockedCIDRs": isValidCIDRList, + "RemoteTasksURLMaxResponseSize": func(value string) error { + return positiveInt(value, "remote tasks URL max response size") + }, } } @@ -164,3 +174,60 @@ func startWithHTTPorHTTPS(url string) error { } return nil } + +func isValidHostList(value string) error { + for _, host := range strings.Split(value, ",") { + host = strings.TrimSpace(host) + if host == "" { + continue + } + if strings.Contains(host, "://") { + parsedURL, err := url.Parse(host) + if err != nil { + return fmt.Errorf("host %q has an invalid URL scheme: %w", host, err) + } + if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { + return fmt.Errorf("host %q must use http:// or https:// when including a URL scheme", host) + } + if parsedURL.User != nil || parsedURL.RawQuery != "" || parsedURL.Fragment != "" || (parsedURL.Path != "" && parsedURL.Path != "/") { + return fmt.Errorf("host %q must not include a URL path, query, fragment, or userinfo", host) + } + host = parsedURL.Host + } + if strings.Contains(host, "*") && !strings.HasPrefix(host, "*.") { + return fmt.Errorf("host %q has an invalid wildcard", host) + } + trimmedHost := strings.TrimPrefix(host, "*.") + if trimmedHost == "" || strings.ContainsAny(trimmedHost, "/?#") { + return fmt.Errorf("host %q is invalid", host) + } + } + return nil +} + +func isValidCIDRList(value string) error { + for _, cidr := range strings.Split(value, ",") { + cidr = strings.TrimSpace(cidr) + if cidr == "" { + continue + } + if _, err := netip.ParsePrefix(cidr); err != nil { + return fmt.Errorf("invalid CIDR %q: %w", cidr, err) + } + } + return nil +} + +func positiveInt(value, name string) error { + if strings.TrimSpace(value) == "" { + return nil + } + parsedValue, err := strconv.Atoi(value) + if err != nil { + return err + } + if parsedValue <= 0 { + return fmt.Errorf("%s must be greater than 0", name) + } + return nil +} diff --git a/pkg/params/settings/config_test.go b/pkg/params/settings/config_test.go index 7ccdbfedc0..4d5726adb3 100644 --- a/pkg/params/settings/config_test.go +++ b/pkg/params/settings/config_test.go @@ -25,6 +25,9 @@ func TestSyncConfig(t *testing.T) { ApplicationName: "Pipelines as Code CI", HubCatalogs: nil, RemoteTasks: true, + RemoteTasksURLAllowlist: "", + RemoteTasksURLBlockedCIDRs: "", + RemoteTasksURLMaxResponseSize: 1048576, MaxKeepRunsUpperLimit: 0, DefaultMaxKeepRuns: 0, BitbucketCloudCheckSourceIP: true, @@ -56,6 +59,9 @@ func TestSyncConfig(t *testing.T) { configMap: map[string]string{ "application-name": "pac-pac", "remote-tasks": "false", + "remote-tasks-url-allowlist": "raw.githubusercontent.com,https://*.example.com,http://internal.example.com", + "remote-tasks-url-blocked-cidrs": "10.0.0.0/8,fd00::/8", + "remote-tasks-url-max-response-size": "2048", "max-keep-run-upper-limit": "10", "default-max-keep-runs": "5", "bitbucket-cloud-check-source-ip": "false", @@ -84,6 +90,9 @@ func TestSyncConfig(t *testing.T) { ApplicationName: "pac-pac", HubCatalogs: nil, RemoteTasks: false, + RemoteTasksURLAllowlist: "raw.githubusercontent.com,https://*.example.com,http://internal.example.com", + RemoteTasksURLBlockedCIDRs: "10.0.0.0/8,fd00::/8", + RemoteTasksURLMaxResponseSize: 2048, MaxKeepRunsUpperLimit: 10, DefaultMaxKeepRuns: 5, BitbucketCloudCheckSourceIP: false, @@ -126,6 +135,34 @@ func TestSyncConfig(t *testing.T) { }, expectedError: "invalid value for int field MaxKeepRunsUpperLimit: strconv.ParseInt: parsing \"invalid\": invalid syntax", }, + { + name: "invalid remote tasks url max response size", + configMap: map[string]string{ + "remote-tasks-url-max-response-size": "0", + }, + expectedError: "custom validation failed for field RemoteTasksURLMaxResponseSize: remote tasks URL max response size must be greater than 0", + }, + { + name: "invalid remote tasks url blocked cidrs", + configMap: map[string]string{ + "remote-tasks-url-blocked-cidrs": "10.0.0.0/99", + }, + expectedError: "custom validation failed for field RemoteTasksURLBlockedCIDRs: invalid CIDR", + }, + { + name: "invalid remote tasks url allowlist scheme", + configMap: map[string]string{ + "remote-tasks-url-allowlist": "ftp://example.com", + }, + expectedError: "custom validation failed for field RemoteTasksURLAllowlist: host \"ftp://example.com\" must use http:// or https:// when including a URL scheme", + }, + { + name: "invalid remote tasks url allowlist path", + configMap: map[string]string{ + "remote-tasks-url-allowlist": "http://example.com/path", + }, + expectedError: "custom validation failed for field RemoteTasksURLAllowlist: host \"http://example.com/path\" must not include a URL path, query, fragment, or userinfo", + }, { name: "invalid value regex", configMap: map[string]string{ diff --git a/pkg/resolve/remote_test.go b/pkg/resolve/remote_test.go index c14f039e76..497ec5a3b0 100644 --- a/pkg/resolve/remote_test.go +++ b/pkg/resolve/remote_test.go @@ -23,14 +23,15 @@ import ( func TestRemote(t *testing.T) { randomPipelineRunName := "pipelinerun-abc" + remoteBaseURL := "https://93.184.216.34" remotePipelineName := "remote-pipeline" - remotePipelineURL := "http://remote/" + remotePipelineName + remotePipelineURL := remoteBaseURL + "/" + remotePipelineName taskFromPipelineRunName := "task-from-pipelinerun" - taskFromPipelineRunURL := "http://remote/" + taskFromPipelineRunName + taskFromPipelineRunURL := remoteBaseURL + "/" + taskFromPipelineRunName remoteTaskName := "remote-task" - remoteTaskURL := "http://remote/" + remoteTaskName + remoteTaskURL := remoteBaseURL + "/" + remoteTaskName taskFromPipelineSpec := tektonv1.TaskSpec{ Steps: []tektonv1.Step{ { @@ -165,7 +166,7 @@ func TestRemote(t *testing.T) { ), }, remoteURLS: map[string]map[string]string{ - "http://remote/embedpipeline": { + remoteBaseURL + "/embedpipeline": { "body": string(pipelinewithTaskEmbeddedB), "code": "200", }, @@ -180,8 +181,8 @@ func TestRemote(t *testing.T) { }, expectedTaskSpec: taskFromPipelineSpec, expectedLogsSnippets: []string{ - fmt.Sprintf("successfully fetched %s from remote HTTPS URL", remotePipelineURL), - fmt.Sprintf("successfully fetched %s from remote HTTPS URL", remoteTaskURL), + fmt.Sprintf("successfully fetched %s from remote URL", remotePipelineURL), + fmt.Sprintf("successfully fetched %s from remote URL", remoteTaskURL), }, expectedPipelineRun: []string{"remote-pipeline-with-remote-task-from-pipeline.yaml"}, }, @@ -228,7 +229,7 @@ func TestRemote(t *testing.T) { "body": string(singleRelativeTaskBc), "code": "200", }, - "http://remote/utils/remote-task-c": { + remoteBaseURL + "/utils/remote-task-c": { "body": string(singleRelativeTaskBc), "code": "200", }, @@ -321,8 +322,8 @@ func TestRemote(t *testing.T) { }, }, expectedLogsSnippets: []string{ - fmt.Sprintf("successfully fetched %s from remote HTTPS URL", remotePipelineURL), - fmt.Sprintf("successfully fetched %s from remote HTTPS URL", taskFromPipelineRunURL), + fmt.Sprintf("successfully fetched %s from remote URL", remotePipelineURL), + fmt.Sprintf("successfully fetched %s from remote URL", taskFromPipelineRunURL), }, expectedPipelineRun: []string{"remote-pipeline-with-remote-task-from-pipelinerun.yaml"}, }, @@ -381,8 +382,8 @@ func TestRemote(t *testing.T) { }, expectedTaskSpec: taskFromPipelineSpec, expectedLogsSnippets: []string{ - fmt.Sprintf("successfully fetched %s from remote HTTPS URL", remotePipelineURL), - fmt.Sprintf("successfully fetched %s from remote HTTPS URL", remoteTaskURL), + fmt.Sprintf("successfully fetched %s from remote URL", remotePipelineURL), + fmt.Sprintf("successfully fetched %s from remote URL", remoteTaskURL), }, expectedPipelineRun: []string{"skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-pipeline-annotation.yaml"}, }, @@ -415,8 +416,8 @@ func TestRemote(t *testing.T) { }, expectedTaskSpec: taskFromPipelineSpec, expectedLogsSnippets: []string{ - fmt.Sprintf("successfully fetched %s from remote HTTPS URL", remotePipelineURL), - fmt.Sprintf("successfully fetched %s from remote HTTPS URL", remoteTaskURL), + fmt.Sprintf("successfully fetched %s from remote URL", remotePipelineURL), + fmt.Sprintf("successfully fetched %s from remote URL", remoteTaskURL), fmt.Sprintf("skipping remote task %s as already fetched task %s for pipelinerun %s", remoteTaskURL, remoteTaskName, randomPipelineRunName), fmt.Sprintf("overriding task %s coming from .tekton directory by an annotation task for pipelinerun %s", remoteTaskName, randomPipelineRunName), }, @@ -451,8 +452,8 @@ func TestRemote(t *testing.T) { }, expectedTaskSpec: taskFromPipelineSpec, expectedLogsSnippets: []string{ - fmt.Sprintf("successfully fetched %s from remote HTTPS URL", remotePipelineURL), - fmt.Sprintf("successfully fetched %s from remote HTTPS URL", remoteTaskURL), + fmt.Sprintf("successfully fetched %s from remote URL", remotePipelineURL), + fmt.Sprintf("successfully fetched %s from remote URL", remoteTaskURL), fmt.Sprintf("skipping remote task %s as already fetched task %s for pipelinerun %s", remoteTaskURL, remoteTaskName, randomPipelineRunName), }, expectedPipelineRun: []string{"skip-fetching-multiple-pipelines-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml"}, @@ -470,7 +471,7 @@ func TestRemote(t *testing.T) { } tprovider := &testprovider.TestProviderImp{} - httpTestClient := httptesthelper.MakeHTTPTestClient(tt.remoteURLS) + httpTestClient := httptesthelper.MakeHTTPTransportTestClient(t, tt.remoteURLS) rt := &matcher.RemoteTasks{ ProviderInterface: tprovider, Logger: logger, diff --git a/pkg/resolve/testdata/remote-pipeline-with-relative-tasks-1.yaml b/pkg/resolve/testdata/remote-pipeline-with-relative-tasks-1.yaml index 514906afb2..8dd8a910db 100644 --- a/pkg/resolve/testdata/remote-pipeline-with-relative-tasks-1.yaml +++ b/pkg/resolve/testdata/remote-pipeline-with-relative-tasks-1.yaml @@ -2,7 +2,7 @@ apiVersion: tekton.dev/v1 kind: PipelineRun metadata: annotations: - pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline-1 + pipelinesascode.tekton.dev/pipeline: https://93.184.216.34/remote-pipeline-1 generateName: pipelinerun-abc- spec: pipelineSpec: diff --git a/pkg/resolve/testdata/remote-pipeline-with-relative-tasks-same-pipeline.yaml b/pkg/resolve/testdata/remote-pipeline-with-relative-tasks-same-pipeline.yaml index e4a6bdeed2..2ecf9c1058 100644 --- a/pkg/resolve/testdata/remote-pipeline-with-relative-tasks-same-pipeline.yaml +++ b/pkg/resolve/testdata/remote-pipeline-with-relative-tasks-same-pipeline.yaml @@ -2,7 +2,7 @@ apiVersion: tekton.dev/v1 kind: PipelineRun metadata: annotations: - pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline + pipelinesascode.tekton.dev/pipeline: https://93.184.216.34/remote-pipeline generateName: pipelinerun-abc-second- spec: pipelineSpec: diff --git a/pkg/resolve/testdata/remote-pipeline-with-relative-tasks.yaml b/pkg/resolve/testdata/remote-pipeline-with-relative-tasks.yaml index 54179ac36f..625e89850b 100644 --- a/pkg/resolve/testdata/remote-pipeline-with-relative-tasks.yaml +++ b/pkg/resolve/testdata/remote-pipeline-with-relative-tasks.yaml @@ -2,7 +2,7 @@ apiVersion: tekton.dev/v1 kind: PipelineRun metadata: annotations: - pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline + pipelinesascode.tekton.dev/pipeline: https://93.184.216.34/remote-pipeline generateName: pipelinerun-abc- spec: pipelineSpec: diff --git a/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipeline.yaml b/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipeline.yaml index 84ccf0a6ba..9a25bac659 100644 --- a/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipeline.yaml +++ b/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipeline.yaml @@ -2,7 +2,7 @@ apiVersion: tekton.dev/v1 kind: PipelineRun metadata: annotations: - pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline + pipelinesascode.tekton.dev/pipeline: https://93.184.216.34/remote-pipeline generateName: pipelinerun-abc- spec: pipelineSpec: diff --git a/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipelinerun.yaml b/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipelinerun.yaml index 4ebb11c56c..2722439c4d 100644 --- a/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipelinerun.yaml +++ b/pkg/resolve/testdata/remote-pipeline-with-remote-task-from-pipelinerun.yaml @@ -2,8 +2,8 @@ apiVersion: tekton.dev/v1 kind: PipelineRun metadata: annotations: - pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline - pipelinesascode.tekton.dev/task: http://remote/task-from-pipelinerun + pipelinesascode.tekton.dev/pipeline: https://93.184.216.34/remote-pipeline + pipelinesascode.tekton.dev/task: https://93.184.216.34/task-from-pipelinerun generateName: pipelinerun-abc- spec: pipelineSpec: diff --git a/pkg/resolve/testdata/skip-fetching-multiple-pipelines-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml b/pkg/resolve/testdata/skip-fetching-multiple-pipelines-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml index 91561ba876..1e585a09e0 100644 --- a/pkg/resolve/testdata/skip-fetching-multiple-pipelines-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml +++ b/pkg/resolve/testdata/skip-fetching-multiple-pipelines-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml @@ -2,8 +2,8 @@ apiVersion: tekton.dev/v1 kind: PipelineRun metadata: annotations: - pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline - pipelinesascode.tekton.dev/task: http://remote/remote-task + pipelinesascode.tekton.dev/pipeline: https://93.184.216.34/remote-pipeline + pipelinesascode.tekton.dev/task: https://93.184.216.34/remote-task generateName: pipelinerun-abc- spec: pipelineSpec: diff --git a/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-pipeline-annotation.yaml b/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-pipeline-annotation.yaml index e7b6edd44b..1360aa67f9 100644 --- a/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-pipeline-annotation.yaml +++ b/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-pipeline-annotation.yaml @@ -2,9 +2,9 @@ apiVersion: tekton.dev/v1 kind: PipelineRun metadata: annotations: - pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline - pipelinesascode.tekton.dev/task: http://remote/remote-task - pipelinesascode.tekton.dev/task-1: http://remote/remote-task + pipelinesascode.tekton.dev/pipeline: https://93.184.216.34/remote-pipeline + pipelinesascode.tekton.dev/task: https://93.184.216.34/remote-task + pipelinesascode.tekton.dev/task-1: https://93.184.216.34/remote-task generateName: pipelinerun-abc- spec: pipelineSpec: diff --git a/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml b/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml index 91561ba876..1e585a09e0 100644 --- a/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml +++ b/pkg/resolve/testdata/skip-fetching-multiple-tasks-of-the-same-name-from-pipelinerun-annotations-and-tektondir.yaml @@ -2,8 +2,8 @@ apiVersion: tekton.dev/v1 kind: PipelineRun metadata: annotations: - pipelinesascode.tekton.dev/pipeline: http://remote/remote-pipeline - pipelinesascode.tekton.dev/task: http://remote/remote-task + pipelinesascode.tekton.dev/pipeline: https://93.184.216.34/remote-pipeline + pipelinesascode.tekton.dev/task: https://93.184.216.34/remote-task generateName: pipelinerun-abc- spec: pipelineSpec: diff --git a/pkg/test/http/http.go b/pkg/test/http/http.go index 729064136c..9b5d663bba 100644 --- a/pkg/test/http/http.go +++ b/pkg/test/http/http.go @@ -2,9 +2,14 @@ package http import ( "bytes" + "context" + "crypto/tls" "io" + "net" "net/http" + "net/http/httptest" "strconv" + "testing" ) // NewTestClient returns *http.Client with Transport replaced to avoid making real calls. @@ -34,6 +39,80 @@ func MakeHTTPTestClient(config map[string]map[string]string) *http.Client { }) } +// MakeHTTPTransportTestClient creates a test HTTP client backed by a real +// *http.Transport, while routing all dials to local test servers. +func MakeHTTPTransportTestClient(t testing.TB, config map[string]map[string]string) *http.Client { + t.Helper() + + handler := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + scheme := "http" + if req.TLS != nil { + scheme = "https" + } + values, ok := config[scheme+"://"+req.Host+req.URL.RequestURI()] + if !ok { + w.WriteHeader(http.StatusNotFound) + return + } + code, _ := strconv.Atoi(values["code"]) + w.WriteHeader(code) + if body, ok := values["body"]; ok { + _, _ = io.WriteString(w, body) + } + }) + + httpServer := httptest.NewServer(handler) + t.Cleanup(httpServer.Close) + httpsServer := httptest.NewTLSServer(handler) + t.Cleanup(httpsServer.Close) + + return &http.Client{ + Transport: &http.Transport{ + DialContext: func(ctx context.Context, network, address string) (net.Conn, error) { + _, port, err := net.SplitHostPort(address) + if err != nil { + return nil, err + } + if port == "443" { + return dialHTTPTransportTestConn(ctx, network, address, httpsServer.Listener.Addr().String()) + } + return dialHTTPTransportTestConn(ctx, network, address, httpServer.Listener.Addr().String()) + }, + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec + }, + } +} + +func dialHTTPTransportTestConn(ctx context.Context, network, remoteAddress, testServerAddress string) (net.Conn, error) { + conn, err := (&net.Dialer{}).DialContext(ctx, network, testServerAddress) + if err != nil { + return nil, err + } + return httpTransportTestConn{ + Conn: conn, + remoteAddr: httpTransportTestAddr(remoteAddress), + }, nil +} + +type httpTransportTestConn struct { + net.Conn + remoteAddr net.Addr +} + +func (c httpTransportTestConn) RemoteAddr() net.Addr { + return c.remoteAddr +} + +type httpTransportTestAddr string + +func (a httpTransportTestAddr) Network() string { + return "tcp" +} + +func (a httpTransportTestAddr) String() string { + return string(a) +} + // RoundTripFunc is a function adapter to implement http.RoundTripper interface. type RoundTripFunc func(req *http.Request) *http.Response diff --git a/pkg/test/provider/testwebvcs.go b/pkg/test/provider/testwebvcs.go index a096988778..e6d7150209 100644 --- a/pkg/test/provider/testwebvcs.go +++ b/pkg/test/provider/testwebvcs.go @@ -24,6 +24,7 @@ type TestProviderImp struct { CreateStatusErorring bool FilesInsideRepo map[string]string WantProviderRemoteTask bool + ProviderRemoteTask string PolicyDisallowing bool AllowedInOwnersFile bool WantAllChangedFiles []string @@ -102,7 +103,7 @@ func (v *TestProviderImp) IsAllowed(_ context.Context, _ *info.Event) (bool, err } func (v *TestProviderImp) GetTaskURI(_ context.Context, _ *info.Event, _ string) (bool, string, error) { - return v.WantProviderRemoteTask, "", nil + return v.WantProviderRemoteTask, v.ProviderRemoteTask, nil } func (v *TestProviderImp) CreateStatus(_ context.Context, _ *info.Event, _ providerstatus.StatusOpts) error { diff --git a/test/gitea_test.go b/test/gitea_test.go index 271dc1ce45..39e39215ed 100644 --- a/test/gitea_test.go +++ b/test/gitea_test.go @@ -160,6 +160,42 @@ func TestGiteaGetTaskURI(t *testing.T) { } } +func TestGiteaRemoteTasksURLAllowlist(t *testing.T) { + ctx, _ := rtesting.SetupFakeContext(t) + topts := &tgitea.TestOpts{ + TargetEvent: triggertype.PullRequest.String(), + YAMLFiles: map[string]string{ + ".tekton/pipeline.yaml": "testdata/pipeline_in_tektondir.yaml", + ".other-tasks/task-referenced-internally.yaml": "testdata/task_referenced_internally.yaml", + ".tekton/pr.yaml": "testdata/pipelinerun_remote_task_annotations.yaml", + }, + CheckForStatus: "failure", + ExpectEvents: true, + SkipEventsCheck: true, + ExtraArgs: map[string]string{ + "RemoteTaskURL": options.RemoteTaskURL, + "RemoteTaskName": options.RemoteTaskName, + }, + } + topts.TargetRefName = names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test") + topts.TargetNS = topts.TargetRefName + topts.ParamsRun, topts.Opts, topts.GiteaCNX, _ = tgitea.Setup(ctx) + assert.NilError(t, topts.ParamsRun.Clients.NewClients(ctx, &topts.ParamsRun.Info)) + ctx, err := cctx.GetControllerCtxInfo(ctx, topts.ParamsRun) + assert.NilError(t, err) + assert.NilError(t, pacrepo.CreateNS(ctx, topts.TargetNS, topts.ParamsRun)) + cfgMapData := map[string]string{ + "remote-tasks-url-allowlist": "nonexistent.invalid", + } + defer configmap.ChangeGlobalConfig(ctx, t, topts.ParamsRun, "pipelines-as-code", cfgMapData)() + + _, f := tgitea.TestPR(t, topts) + defer f() + + topts.Regexp = regexp.MustCompile(`not in the allowlist`) + tgitea.WaitForPullRequestCommentMatch(t, topts) +} + func TestGiteaUseDisplayName(t *testing.T) { topts := &tgitea.TestOpts{ Regexp: regexp.MustCompile(`.*The Task name is Task.*`),