From 4f6e21bdefc2a4264041b65738f92f7c827d6691 Mon Sep 17 00:00:00 2001 From: Akshay Pant Date: Fri, 24 Apr 2026 16:12:43 +0530 Subject: [PATCH 1/2] fix(github): scope App token to triggering repo Reissue a scoped token in SetClient when no extra scope config is present. The initial token is created before RepositoryIDs are populated, so it has access to all repos in the installation. After ScopeTokenToListOfRepos returns empty, fall back to GetAppToken which now uses the populated RepositoryIDs. Signed-off-by: Akshay Pant --- pkg/provider/github/github.go | 11 +++++++++++ pkg/provider/github/parse_payload.go | 28 +++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/pkg/provider/github/github.go b/pkg/provider/github/github.go index 09af833e6..2a1dc3b12 100644 --- a/pkg/provider/github/github.go +++ b/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 } } diff --git a/pkg/provider/github/parse_payload.go b/pkg/provider/github/parse_payload.go index b1011cdac..1523420c9 100644 --- a/pkg/provider/github/parse_payload.go +++ b/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 { From 092b5edd49632718d7316353ecd0a4b2901793ad Mon Sep 17 00:00:00 2001 From: Akshay Pant Date: Fri, 24 Apr 2026 16:13:43 +0530 Subject: [PATCH 2/2] fix(resolve): deep-copy cached resources before inlining Use DeepCopy when reusing cached Pipeline and Task objects across PipelineRuns. Without this, inlineTasks mutates the cached original, contaminating subsequent runs that reference the same remote pipeline. Assisted-by: Claude Opus 4.6 Signed-off-by: Akshay Pant --- pkg/resolve/remote.go | 8 ++-- pkg/resolve/remote_test.go | 78 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/pkg/resolve/remote.go b/pkg/resolve/remote.go index 6986a53d2..521cfb0b9 100644 --- a/pkg/resolve/remote.go +++ b/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 diff --git a/pkg/resolve/remote_test.go b/pkg/resolve/remote_test.go index c14f039e7..cd5a08eb2 100644 --- a/pkg/resolve/remote_test.go +++ b/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: ¶ms.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