fix: scope GitHub App token and deep-copy cached remote resources

pkg/provider/github/github.go

@@ -363,6 +363,17 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
 		// If Global and Repo level configurations are not provided then lets not override the provider token.
 		if token != "" {
 			event.Provider.Token = token
+		} else if len(v.RepositoryIDs) > 0 {
+			// We need to keep the token unscoped until ScopeTokenToListOfRepos so that CreateToken can
+			// look up the extra repos from the configmap.
+			// Token is scoped to only the calling repo if no additional scoping repos are configured
+			// so that no unwanted remote tasks are executed.
+			ns := info.GetNS(ctx)
+			scopedToken, err := v.GetAppToken(ctx, run.Clients.Kube, event.Provider.URL, event.InstallationID, ns)
+			if err != nil {
+				return fmt.Errorf("failed to scope token to triggering repository: %w", err)
+			}
+			event.Provider.Token = scopedToken
 		}
 	}
 

pkg/provider/github/parse_payload.go

@@ -131,18 +131,29 @@ type Payload struct {
 	Installation struct {
 		ID *int64 `json:"id"`
 	} `json:"installation"`
+	Repository struct {
+		ID *int64 `json:"id"`
+	} `json:"repository"`
 }
 
-func getInstallationIDFromPayload(payload string) (int64, error) {
+func getInstallationAndRepoIDFromPayload(payload string) (int64, int64, error) {
 	var data Payload
 	err := json.Unmarshal([]byte(payload), &data)
 	if err != nil {
-		return -1, err
+		return -1, -1, err
 	}
+
+	var installationID int64 = -1
+	var repoID int64 = -1
 	if data.Installation.ID != nil {
-		return *data.Installation.ID, nil
+		installationID = *data.Installation.ID
+	}
+
+	if data.Repository.ID != nil {
+		repoID = *data.Repository.ID
 	}
-	return -1, nil
+
+	return installationID, repoID, nil
 }
 
 // ParsePayload will parse the payload and return the event
@@ -177,7 +188,7 @@ func (v *Provider) ParsePayload(ctx context.Context, run *params.Run, request *h
 		return nil, err
 	}
 
-	installationIDFrompayload, err := getInstallationIDFromPayload(payload)
+	installationIDFrompayload, repoIDFromPayload, err := getInstallationAndRepoIDFromPayload(payload)
 	if err != nil {
 		return nil, err
 	}
@@ -189,6 +200,10 @@ func (v *Provider) ParsePayload(ctx context.Context, run *params.Run, request *h
 		}
 	}
 
+	if repoIDFromPayload > 0 {
+		v.RepositoryIDs = []int64{repoIDFromPayload}
+	}
+
 	eventInt, err := github.ParseWebHook(event.EventType, []byte(payload))
 	if err != nil {
 		return nil, err
@@ -537,6 +552,7 @@ func (v *Provider) handleReRequestEvent(ctx context.Context, event *github.Check
 		// fine because you can't do a rereq without being a github owner?
 		runevent.Sender = event.GetSender().GetLogin()
 		v.userType = event.GetSender().GetType()
+		v.RepositoryIDs = []int64{event.GetRepo().GetID()}
 		return runevent, nil
 	}
 	runevent.PullRequestNumber = event.GetCheckRun().GetCheckSuite().PullRequests[0].GetNumber()
@@ -584,6 +600,7 @@ func (v *Provider) handleCheckSuites(ctx context.Context, event *github.CheckSui
 		// fine because you can't do a rereq without being a github owner?
 		runevent.Sender = event.GetSender().GetLogin()
 		v.userType = event.GetSender().GetType()
+		v.RepositoryIDs = []int64{event.GetRepo().GetID()}
 		return runevent, nil
 	}
 	runevent.PullRequestNumber = event.GetCheckSuite().PullRequests[0].GetNumber()
@@ -699,6 +716,7 @@ func (v *Provider) handleCommitCommentEvent(ctx context.Context, event *github.C
 	runevent.BaseURL = runevent.HeadURL
 	runevent.TriggerTarget = triggertype.Push
 	v.userType = event.GetSender().GetType()
+	v.RepositoryIDs = []int64{event.GetRepo().GetID()}
 
 	repo, err := MatchEventURLRepo(ctx, v.Run, runevent, "")
 	if err != nil {

pkg/resolve/remote.go

@@ -122,8 +122,8 @@ func resolveRemoteResources(ctx context.Context, rt *matcher.RemoteTasks, types
 				// making sure that the pipeline with same annotation name is not fetched
 				if alreadyFetchedResource(fetchedResourcesForEvent.Pipelines, remotePipeline) {
 					rt.Logger.Debugf("skipping already fetched pipeline %s in annotations on pipelinerun %s", remotePipeline, pipelinerun.GetName())
-					// already fetched, then just get the pipeline to add to run specific Resources
-					pipeline = fetchedResourcesForEvent.Pipelines[remotePipeline]
+					// already fetched, deep-copy so inlining for this run doesn't mutate the cached original
+					pipeline = fetchedResourcesForEvent.Pipelines[remotePipeline].DeepCopy()
 				} else {
 					// seems like a new pipeline, fetch it based on name in annotation
 					pipeline, err = rt.GetPipelineFromAnnotationName(ctx, remotePipeline)
@@ -181,7 +181,7 @@ func resolveRemoteResources(ctx context.Context, rt *matcher.RemoteTasks, types
 				// if task is already fetched in the event, then just copy the task
 				if alreadyFetchedResource(fetchedResourcesForEvent.Tasks, remoteTask) {
 					rt.Logger.Debugf("skipping already fetched task %s in annotations on pipelinerun %s", remoteTask, pipelinerun.GetName())
-					task = fetchedResourcesForEvent.Tasks[remoteTask]
+					task = fetchedResourcesForEvent.Tasks[remoteTask].DeepCopy()
 				} else {
 					// get the task from annotation name
 					task, err = rt.GetTaskFromAnnotationName(ctx, remoteTask)
@@ -213,7 +213,7 @@ func resolveRemoteResources(ctx context.Context, rt *matcher.RemoteTasks, types
 
 		// if PipelineRef is used then, first resolve pipeline and replace all taskRef{Finally/Task} of Pipeline, then put inlinePipeline in PipelineRun
 		if pipelinerun.Spec.PipelineRef != nil && pipelinerun.Spec.PipelineRef.Resolver == "" {
-			pipelineResolved := fetchedResourcesForPipelineRun.Pipeline
+			pipelineResolved := fetchedResourcesForPipelineRun.Pipeline.DeepCopy()
 			turns, err := inlineTasks(pipelineResolved.Spec.Tasks, ropt, fetchedResourcesForPipelineRun)
 			if err != nil {
 				return nil, err

pkg/resolve/remote_test.go

@@ -512,6 +512,84 @@ func TestRemote(t *testing.T) {
 	}
 }
 
+// Verifies that cached remote pipelines are deep-copied before modification.
+// Without deep copy, when the first PipelineRun applies its task annotation,
+// it would mutate the cached pipeline and leak that task into the second run.
+func TestSharedRemotePipelineCacheNotMutated(t *testing.T) {
+	taskName := "shared-task"
+
+	pipelineTask := tektonv1.TaskSpec{
+		Steps: []tektonv1.Step{
+			{Name: "from-pipeline", Image: "alpine", Command: []string{"true"}},
+		},
+	}
+	pipelinerunTask := tektonv1.TaskSpec{
+		Steps: []tektonv1.Step{
+			{Name: "from-pipelinerun", Image: "busybox", Command: []string{"false"}},
+		},
+	}
+
+	// remote pipeline with a single TaskRef
+	pipeline := ttkn.MakePipeline("shared-pipeline", []tektonv1.PipelineTask{
+		{Name: taskName, TaskRef: &tektonv1.TaskRef{Name: taskName}},
+	}, map[string]string{
+		apipac.Task: "http://remote/shared-task",
+	})
+	pipelineB, err := yaml.Marshal(pipeline)
+	assert.NilError(t, err)
+
+	pipelineTaskB, err := ttkn.MakeTaskB(taskName, pipelineTask)
+	assert.NilError(t, err)
+	pipelinerunTaskB, err := ttkn.MakeTaskB(taskName, pipelinerunTask)
+	assert.NilError(t, err)
+
+	remoteURLS := map[string]map[string]string{
+		"http://remote/shared-pipeline": {"body": string(pipelineB), "code": "200"},
+		"http://remote/shared-task":     {"body": string(pipelineTaskB), "code": "200"},
+		"http://remote/pr-task":         {"body": string(pipelinerunTaskB), "code": "200"},
+	}
+
+	// first run: overrides the task via pipelinerun annotation
+	// second run: same pipeline, no override — should get the original task
+	pipelineruns := []*tektonv1.PipelineRun{
+		ttkn.MakePR("first-run", map[string]string{
+			apipac.Pipeline: "http://remote/shared-pipeline",
+			apipac.Task:     "http://remote/pr-task",
+		}, tektonv1.PipelineRunSpec{
+			PipelineRef: &tektonv1.PipelineRef{Name: "shared-pipeline"},
+		}),
+		ttkn.MakePR("second-run", map[string]string{
+			apipac.Pipeline: "http://remote/shared-pipeline",
+		}, tektonv1.PipelineRunSpec{
+			PipelineRef: &tektonv1.PipelineRef{Name: "shared-pipeline"},
+		}),
+	}
+
+	observer, _ := zapobserver.New(zap.InfoLevel)
+	logger := zap.New(observer).Sugar()
+	ctx, _ := rtesting.SetupFakeContext(t)
+	httpTestClient := httptesthelper.MakeHTTPTestClient(remoteURLS)
+	rt := &matcher.RemoteTasks{
+		ProviderInterface: &testprovider.TestProviderImp{},
+		Logger:            logger,
+		Run: &params.Run{
+			Clients: clients.Clients{HTTP: *httpTestClient},
+		},
+	}
+
+	ret, err := resolveRemoteResources(ctx, rt, TektonTypes{PipelineRuns: pipelineruns}, &Opts{RemoteTasks: true, GenerateName: true})
+	assert.NilError(t, err)
+	assert.Equal(t, len(ret), 2)
+
+	firstStep := ret[0].Spec.PipelineSpec.Tasks[0].TaskSpec.Steps[0]
+	assert.Equal(t, firstStep.Name, "from-pipelinerun")
+	assert.Equal(t, firstStep.Image, "busybox")
+
+	secondStep := ret[1].Spec.PipelineSpec.Tasks[0].TaskSpec.Steps[0]
+	assert.Equal(t, secondStep.Name, "from-pipeline")
+	assert.Equal(t, secondStep.Image, "alpine")
+}
+
 func TestAssembleTaskFQDNs(t *testing.T) {
 	tests := []struct {
 		name        string