From 270786226bf0d1d8ea81529c40146c3b3a86093f Mon Sep 17 00:00:00 2001
From: Vincent Demeester <vincent@sbr.pm>
Date: Thu, 7 May 2026 12:42:05 +0200
Subject: [PATCH] tekton: validate workspace configs reject ambiguous volume
 sources

A workspace with multiple sources set (e.g., both storage and pvc)
was silently picking whichever matched first in the switch. Now
parseTektonWorkflowConfig rejects workspaces with zero or multiple
sources, and workspaces with empty names, at parse time with clear
error messages.
---
 provider_tekton.go      | 40 ++++++++++++++++++++++++++++++++-
 provider_tekton_test.go | 49 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/provider_tekton.go b/provider_tekton.go
index 30fdab9..41f1587 100644
--- a/provider_tekton.go
+++ b/provider_tekton.go
@@ -89,8 +89,46 @@ func parseTektonWorkflowConfig(raw string) (*tektonWorkflowConfig, error) {
 	}
 	cfg := doc.Tack.Tekton
 	if cfg.Pipeline == "" {
-		return nil, errors.New("workflow yaml: `tack.tekton.pipeline` is required")
+		return nil, errors.New(
+			"workflow yaml: `tack.tekton.pipeline` is required",
+		)
 	}
+
+	// Validate workspaces: each must have exactly one volume source
+	// so the resulting PipelineRun is unambiguous. Without this,
+	// a workspace with e.g. both `storage` and `pvc` set silently
+	// picks whichever the switch statement matches first.
+	for _, ws := range cfg.Workspaces {
+		if ws.Name == "" {
+			return nil, errors.New("workspace: name is required")
+		}
+		sources := 0
+		if ws.Storage != nil {
+			sources++
+		}
+		if ws.PVC != nil {
+			sources++
+		}
+		if ws.Secret != nil {
+			sources++
+		}
+		if ws.ConfigMap != nil {
+			sources++
+		}
+		if sources == 0 {
+			return nil, fmt.Errorf(
+				"workspace %q: no volume source specified", ws.Name,
+			)
+		}
+		if sources > 1 {
+			return nil, fmt.Errorf(
+				"workspace %q: multiple volume sources specified"+
+					" (pick one of storage, pvc, secret, config_map)",
+				ws.Name,
+			)
+		}
+	}
+
 	return &cfg, nil
 }
 
diff --git a/provider_tekton_test.go b/provider_tekton_test.go
index eb62de8..1021ab6 100644
--- a/provider_tekton_test.go
+++ b/provider_tekton_test.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"log/slog"
+	"strings"
 	"testing"
 	"time"
 
@@ -40,6 +41,54 @@ func TestTektonWorkflowConfig(t *testing.T) {
 	}
 }
 
+func TestTektonWorkspaceValidation(t *testing.T) {
+	tests := []struct {
+		name    string
+		yaml    string
+		wantErr string
+	}{
+		{
+			name:    "empty name",
+			yaml:    "tack:\n  tekton:\n    pipeline: ci\n    workspaces:\n      - storage: 1Gi\n",
+			wantErr: "name is required",
+		},
+		{
+			name:    "no source",
+			yaml:    "tack:\n  tekton:\n    pipeline: ci\n    workspaces:\n      - name: scratch\n",
+			wantErr: "no volume source",
+		},
+		{
+			name:    "multiple sources",
+			yaml:    "tack:\n  tekton:\n    pipeline: ci\n    workspaces:\n      - name: data\n        storage: 5Gi\n        pvc: my-pvc\n",
+			wantErr: "multiple volume sources",
+		},
+		{
+			name: "valid single source",
+			yaml: "tack:\n  tekton:\n    pipeline: ci\n    workspaces:\n      - name: scratch\n        storage: 1Gi\n",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := parseTektonWorkflowConfig(tt.yaml)
+			if tt.wantErr == "" {
+				if err != nil {
+					t.Fatalf("unexpected error: %v", err)
+				}
+				return
+			}
+			if err == nil {
+				t.Fatalf("expected error containing %q", tt.wantErr)
+			}
+			if !strings.Contains(err.Error(), tt.wantErr) {
+				t.Fatalf(
+					"error %q does not contain %q",
+					err.Error(), tt.wantErr,
+				)
+			}
+		})
+	}
+}
+
 func TestTektonBuildPipelineRun(t *testing.T) {
 	cfg := &tektonWorkflowConfig{
 		Pipeline:       "repo-ci",
-- 
2.51.2