update: Set remeber-ok-to-test settings to false by default

`remeber-ok-to-test` setting is set to false by default to mitigate
security risks of enabling an attacker to gain trust of repo owner
and push malicious code.

fixes: #1798

https://issues.redhat.com/browse/SRVKP-7238

Signed-off-by: Zaki Shaikh <[email protected]>

Author: zakisk

config/302-pac-configmap.yaml

@@ -129,7 +129,7 @@ data:
   # pull request if ok-to-test is done once
   #
   # you may want to disable this if ok-to-test should be done on each iteration
-  remember-ok-to-test: "true"
+  remember-ok-to-test: "false"
 
   # Configure a custom console here, the driver support custom parameters from
   # Repo CR along a few other template variable, see documentation for more

docs/content/docs/install/settings.md

@@ -130,7 +130,13 @@ There is a few things you can configure through the config map
   case of push event on pull request either through new commit or amend, then CI will
   re-run automatically
 
-  You can disable by setting false if you want to provide `ok-to-test` on every iteration
+  By default, the `remember-ok-to-test` setting is set to false in Pipelines-as-Code to mitigate serious security risks.
+  An attacker could submit a seemingly harmless PR to gain the repository owner's trust, and later
+  inject malicious code designed to compromise the build system, such as exfiltrating secrets.
+
+  Enabling this feature increases the risk of unauthorized access and is therefore strongly discouraged
+  unless absolutely necessary. If you choose to enable it you can set it to true, you do so at your own
+  risk and should be aware of the potential security vulnerabilities.
   (only GitHub and Gitea is supported at the moment).
 
 ### Tekton Hub support

pkg/formatting/starting.go

@@ -7,13 +7,22 @@ import (
 )
 
 //go:embed templates/starting.go.tmpl
-var StartingPipelineRunText string
+var StartingPipelineRunHTML string
+
+//go:embed templates/starting.markdown.go.tmpl
+var StartingPipelineRunMarkdown string
 
 //go:embed templates/queuing.go.tmpl
-var QueuingPipelineRunText string
+var QueuingPipelineRunHTML string
+
+//go:embed templates/queuing.markdown.go.tmpl
+var QueuingPipelineRunMarkdown string
 
 //go:embed templates/pipelinerunstatus.tmpl
-var PipelineRunStatusText string
+var PipelineRunStatusHTML string
+
+//go:embed templates/pipelinerunstatus_markdown.tmpl
+var PipelineRunStatusMarkDown string
 
 type MessageTemplate struct {
 	PipelineRunName string

pkg/formatting/templates/pipelinerunstatus_markdown.tmpl

@@ -0,0 +1,17 @@
+- **Namespace**: [{{ .Mt.Namespace }}]({{ .Mt.NamespaceURL }})
+- **PipelineRun**: [{{ .Mt.PipelineRunName }}]({{ .Mt.ConsoleURL }})
+
+---
+
+### Task Statuses:
+
+| **Status** | **Name** | **Duration** |
+|------------|----------|--------------|
+{{ .Mt.TaskStatus }}
+
+{{- if not (eq .Mt.FailureSnippet "")}}
+---
+
+### Failure snippet:
+{{ .Mt.FailureSnippet }}
+{{- end }}

pkg/formatting/templates/queuing.markdown.go.tmpl

@@ -0,0 +1 @@
+PipelineRun **{{ .Mt.PipelineRunName }}** has been queued in namespace **{{ .Mt.Namespace }}**

pkg/formatting/templates/starting.markdown.go.tmpl

@@ -0,0 +1,5 @@
+Starting PipelineRun **{{ .Mt.PipelineRunName }}** in namespace **{{ .Mt.Namespace }}**
+
+You can monitor the execution using the [{{ .Mt.ConsoleName }}]({{ .Mt.ConsoleURL }}) PipelineRun viewer or through the command line by using the [{{ .Mt.TknBinary }}]({{ .Mt.TknBinaryURL }}) CLI with the following command:
+
+`{{ .Mt.TknBinary }} pr logs -n {{ .Mt.Namespace }} {{ .Mt.PipelineRunName }} -f`

pkg/params/settings/config.go

@@ -69,7 +69,7 @@ type Settings struct {
 	CustomConsolePRTaskLog    string `json:"custom-console-url-pr-tasklog"`
 	CustomConsoleNamespaceURL string `json:"custom-console-url-namespace"`
 
-	RememberOKToTest bool `default:"true" json:"remember-ok-to-test"`
+	RememberOKToTest bool `json:"remember-ok-to-test"`
 }
 
 func (s *Settings) DeepCopy(out *Settings) {

pkg/params/settings/config_test.go

@@ -43,7 +43,7 @@ func TestSyncConfig(t *testing.T) {
 				CustomConsolePRdetail:              "",
 				CustomConsolePRTaskLog:             "",
 				CustomConsoleNamespaceURL:          "",
-				RememberOKToTest:                   true,
+				RememberOKToTest:                   false,
 			},
 		},
 		{

pkg/params/settings/convert_test.go

@@ -36,7 +36,7 @@ func TestConvert(t *testing.T) {
 				"hub-catalog-name":                       "tekton",
 				"hub-url":                                "https://api.hub.tekton.dev/v1",
 				"max-keep-run-upper-limit":               "0",
-				"remember-ok-to-test":                    "true",
+				"remember-ok-to-test":                    "false",
 				"remote-tasks":                           "true",
 				"secret-auto-create":                     "true",
 				"secret-github-app-scope-extra-repos":    "",
@@ -74,7 +74,7 @@ func TestConvert(t *testing.T) {
 				"hub-catalog-name":                       "tekton",
 				"hub-url":                                "https://api.hub.tekton.dev/v1",
 				"max-keep-run-upper-limit":               "0",
-				"remember-ok-to-test":                    "true",
+				"remember-ok-to-test":                    "false",
 				"remote-tasks":                           "true",
 				"secret-auto-create":                     "true",
 				"secret-github-app-scope-extra-repos":    "",
@@ -113,7 +113,7 @@ func TestConvert(t *testing.T) {
 				"hub-catalog-name":                       "test tekton",
 				"hub-url":                                "https://api.hub.tekton.dev/v2",
 				"max-keep-run-upper-limit":               "0",
-				"remember-ok-to-test":                    "true",
+				"remember-ok-to-test":                    "false",
 				"remote-tasks":                           "true",
 				"secret-auto-create":                     "true",
 				"secret-github-app-scope-extra-repos":    "",
@@ -164,7 +164,7 @@ func TestConvert(t *testing.T) {
 				"hub-catalog-name":                       "test tekton",
 				"hub-url":                                "https://api.hub.tekton.dev/v2",
 				"max-keep-run-upper-limit":               "0",
-				"remember-ok-to-test":                    "true",
+				"remember-ok-to-test":                    "false",
 				"remote-tasks":                           "true",
 				"secret-auto-create":                     "true",
 				"secret-github-app-scope-extra-repos":    "",

pkg/pipelineascode/pipelineascode.go

@@ -226,7 +226,8 @@ func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.Pi
 		TknBinary:       settings.TknBinaryName,
 		TknBinaryURL:    settings.TknBinaryURL,
 	}
-	msg, err := mt.MakeTemplate(formatting.StartingPipelineRunText)
+
+	msg, err := mt.MakeTemplate(p.vcx.GetTemplate(provider.StartingPipelineType))
 	if err != nil {
 		return nil, fmt.Errorf("cannot create message template: %w", err)
 	}
@@ -243,7 +244,7 @@ func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.Pi
 	// if pipelineRun is in pending state then report status as queued
 	if pr.Spec.Status == tektonv1.PipelineRunSpecStatusPending {
 		status.Status = queuedStatus
-		if status.Text, err = mt.MakeTemplate(formatting.QueuingPipelineRunText); err != nil {
+		if status.Text, err = mt.MakeTemplate(p.vcx.GetTemplate(provider.QueueingPipelineType)); err != nil {
 			return nil, fmt.Errorf("cannot create message template: %w", err)
 		}
 	}

pkg/provider/bitbucketcloud/bitbucket.go

@@ -29,6 +29,8 @@ type Provider struct {
 	Token, APIURL *string
 	Username      *string
 	provenance    string
+	eventEmitter  *events.EventEmitter
+	repo          *v1alpha1.Repository
 }
 
 // CheckPolicyAllowing TODO: Implement ME.
@@ -45,9 +47,7 @@ func (v *Provider) SetPacInfo(pacInfo *info.PacOpts) {
 	v.pacInfo = pacInfo
 }
 
-const taskStatusTemplate = `| **Status** | **Duration** | **Name** |
-| --- | --- | --- |
-{{range $taskrun := .TaskRunList }}|{{ formatCondition $taskrun.PipelineRunTaskRunStatus.Status.Conditions }}|{{ formatDuration $taskrun.PipelineRunTaskRunStatus.Status.StartTime $taskrun.PipelineRunTaskRunStatus.Status.CompletionTime }}|{{ $taskrun.ConsoleLogURL }}|
+const taskStatusTemplate = `{{range $taskrun := .TaskRunList }} | **{{ formatCondition $taskrun.PipelineRunTaskRunStatus.Status.Conditions }}** | {{ $taskrun.ConsoleLogURL }} | *{{ formatDuration $taskrun.PipelineRunTaskRunStatus.Status.StartTime $taskrun.PipelineRunTaskRunStatus.Status.CompletionTime }}* |
 {{ end }}`
 
 func (v *Provider) Validate(_ context.Context, _ *params.Run, _ *info.Event) error {
@@ -110,7 +110,10 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts
 
 	_, err := v.Client.Repositories.Commits.CreateCommitStatus(cmo, cso)
 	if err != nil {
-		return err
+		// Only emit an event to notify the user that something went wrong with the commit status API,
+		// and proceed with creating the comment (if applicable).
+		v.eventEmitter.EmitMessage(v.repo, zap.ErrorLevel, "FailedToSetCommitStatus",
+			"cannot set status with the Bitbucket Cloud token because of: "+err.Error())
 	}
 
 	eventType := triggertype.IsPullRequestType(event.EventType)
@@ -176,7 +179,7 @@ func (v *Provider) GetFileInsideRepo(_ context.Context, event *info.Event, path,
 	return v.getBlob(event, revision, path)
 }
 
-func (v *Provider) SetClient(_ context.Context, run *params.Run, event *info.Event, _ *v1alpha1.Repository, _ *events.EventEmitter) error {
+func (v *Provider) SetClient(_ context.Context, run *params.Run, event *info.Event, repo *v1alpha1.Repository, eventEmitter *events.EventEmitter) error {
 	if event.Provider.Token == "" {
 		return fmt.Errorf("no git_provider.secret has been set in the repo crd")
 	}
@@ -187,6 +190,8 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, event *info.Eve
 	v.Token = &event.Provider.Token
 	v.Username = &event.Provider.User
 	v.run = run
+	v.eventEmitter = eventEmitter
+	v.repo = repo
 	return nil
 }
 
@@ -297,3 +302,7 @@ func (v *Provider) GetFiles(_ context.Context, _ *info.Event) (changedfiles.Chan
 func (v *Provider) CreateToken(_ context.Context, _ []string, _ *info.Event) (string, error) {
 	return "", nil
 }
+
+func (v *Provider) GetTemplate(commentType provider.CommentType) string {
+	return provider.GetMarkdownTemplate(commentType)
+}

pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go

@@ -23,8 +23,7 @@ import (
 	"go.uber.org/zap"
 )
 
-const taskStatusTemplate = `
-{{range $taskrun := .TaskRunList }}* **{{ formatCondition $taskrun.PipelineRunTaskRunStatus.Status.Conditions }}**  {{ $taskrun.ConsoleLogURL }} *{{ formatDuration $taskrun.Status.StartTime $taskrun.Status.CompletionTime }}*
+const taskStatusTemplate = `{{range $taskrun := .TaskRunList }}| **{{ formatCondition $taskrun.PipelineRunTaskRunStatus.Status.Conditions }}** | {{ $taskrun.ConsoleLogURL }} | *{{ formatDuration $taskrun.Status.StartTime $taskrun.Status.CompletionTime }}* |
 {{ end }}`
 const apiResponseLimit = 100
 
@@ -418,3 +417,7 @@ func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedf
 func (v *Provider) CreateToken(_ context.Context, _ []string, _ *info.Event) (string, error) {
 	return "", nil
 }
+
+func (v *Provider) GetTemplate(commentType provider.CommentType) string {
+	return provider.GetMarkdownTemplate(commentType)
+}

pkg/provider/gitea/gitea.go

@@ -403,3 +403,7 @@ func (v *Provider) GetFiles(_ context.Context, runevent *info.Event) (changedfil
 func (v *Provider) CreateToken(_ context.Context, _ []string, _ *info.Event) (string, error) {
 	return "", nil
 }
+
+func (v *Provider) GetTemplate(commentType provider.CommentType) string {
+	return provider.GetHTMLTemplate(commentType)
+}

pkg/provider/github/github.go

@@ -603,3 +603,7 @@ func (v *Provider) isHeadCommitOfBranch(ctx context.Context, runevent *info.Even
 	}
 	return fmt.Errorf("provided SHA %s is not the HEAD commit of the branch %s", runevent.SHA, branchName)
 }
+
+func (v *Provider) GetTemplate(commentType provider.CommentType) string {
+	return provider.GetHTMLTemplate(commentType)
+}

pkg/provider/gitlab/gitlab.go

@@ -460,3 +460,7 @@ func (v *Provider) isHeadCommitOfBranch(runevent *info.Event, branchName string)
 
 	return fmt.Errorf("provided SHA %s is not the HEAD commit of the branch %s", runevent.SHA, branchName)
 }
+
+func (v *Provider) GetTemplate(commentType provider.CommentType) string {
+	return provider.GetHTMLTemplate(commentType)
+}

pkg/provider/interface.go

@@ -45,6 +45,7 @@ type Interface interface {
 	GetTaskURI(ctx context.Context, event *info.Event, uri string) (bool, string, error)
 	CreateToken(context.Context, []string, *info.Event) (string, error)
 	CheckPolicyAllowing(context.Context, *info.Event, []string) (bool, string)
+	GetTemplate(CommentType) string
 }
 
 const DefaultProviderAPIUser = "git"

pkg/provider/provider.go

@@ -6,6 +6,7 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/formatting"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
 	"gopkg.in/yaml.v2"
 )
@@ -28,6 +29,38 @@ const (
 	GitHubApp = "GitHubApp"
 )
 
+type CommentType int
+
+const (
+	StartingPipelineType CommentType = iota
+	PipelineRunStatusType
+	QueueingPipelineType
+)
+
+func GetHTMLTemplate(commentType CommentType) string {
+	switch commentType {
+	case StartingPipelineType:
+		return formatting.StartingPipelineRunHTML
+	case PipelineRunStatusType:
+		return formatting.PipelineRunStatusHTML
+	case QueueingPipelineType:
+		return formatting.QueuingPipelineRunHTML
+	}
+	return ""
+}
+
+func GetMarkdownTemplate(commentType CommentType) string {
+	switch commentType {
+	case StartingPipelineType:
+		return formatting.StartingPipelineRunMarkdown
+	case PipelineRunStatusType:
+		return formatting.PipelineRunStatusMarkDown
+	case QueueingPipelineType:
+		return formatting.QueuingPipelineRunMarkdown
+	}
+	return ""
+}
+
 func Valid(value string, validValues []string) bool {
 	for _, v := range validValues {
 		if v == value {

pkg/reconciler/reconciler.go

@@ -276,7 +276,7 @@ func (r *Reconciler) updatePipelineRunToInProgress(ctx context.Context, logger *
 		TknBinary:       settings.TknBinaryName,
 		TknBinaryURL:    settings.TknBinaryURL,
 	}
-	msg, err := mt.MakeTemplate(formatting.StartingPipelineRunText)
+	msg, err := mt.MakeTemplate(detectedProvider.GetTemplate(provider.StartingPipelineType))
 	if err != nil {
 		return fmt.Errorf("cannot create message template: %w", err)
 	}

pkg/reconciler/status.go

@@ -138,7 +138,7 @@ func (r *Reconciler) postFinalStatus(ctx context.Context, logger *zap.SugaredLog
 		}
 	}
 	var tmplStatusText string
-	if tmplStatusText, err = mt.MakeTemplate(formatting.PipelineRunStatusText); err != nil {
+	if tmplStatusText, err = mt.MakeTemplate(vcx.GetTemplate(provider.PipelineRunStatusType)); err != nil {
 		return nil, fmt.Errorf("cannot create message template: %w", err)
 	}
 

pkg/test/provider/testwebvcs.go

@@ -120,3 +120,7 @@ func (v *TestProviderImp) GetFiles(_ context.Context, _ *info.Event) (changedfil
 func (v *TestProviderImp) CreateToken(_ context.Context, _ []string, _ *info.Event) (string, error) {
 	return "", nil
 }
+
+func (v *TestProviderImp) GetTemplate(commentType provider.CommentType) string {
+	return provider.GetHTMLTemplate(commentType)
+}