From e7cd08fb4cc43802f6f98aabf880b84dc9f364f8 Mon Sep 17 00:00:00 2001
From: Javan lacerda <javanlacerda@google.com>
Date: Wed, 26 Jun 2024 16:46:33 +0000
Subject: [PATCH] adding check for parsing templates

Signed-off-by: Javan lacerda <javanlacerda@google.com>
---
 pkg/config/config.go                 | 36 +++++++++++++++++++++++-
 pkg/config/config_network_test.go    | 42 ++++++++++++++++++++++++++++
 pkg/identity/ciprovider/principal.go |  2 ++
 3 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/pkg/config/config.go b/pkg/config/config.go
index de0927946..d80b3fb7f 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"html/template"
 	"net/http"
 	"net/url"
 	"os"
@@ -30,6 +31,7 @@ import (
 	"time"
 
 	"github.com/coreos/go-oidc/v3/oidc"
+	"github.com/fatih/structs"
 	lru "github.com/hashicorp/golang-lru"
 	"github.com/sigstore/fulcio/pkg/certificate"
 	fulciogrpc "github.com/sigstore/fulcio/pkg/generated/protobuf"
@@ -455,6 +457,32 @@ func FromContext(ctx context.Context) *FulcioConfig {
 	return untyped.(*FulcioConfig)
 }
 
+// It checks that the templates defined are parseable
+// We should check it during the service bootstrap to avoid errors further
+func CheckParseTemplates(fulcioConfig *FulcioConfig) error {
+
+	checkParse := func(temp interface{}) error {
+		t := template.New("").Option("missingkey=error")
+		_, err := t.Parse(temp.(string))
+		return err
+	}
+
+	for _, ciIssuerMetadata := range fulcioConfig.CIIssuerMetadata {
+		claimsTemplates := structs.Map(ciIssuerMetadata.ClaimsTemplates)
+		for _, temp := range claimsTemplates {
+			err := checkParse(temp)
+			if err != nil {
+				return err
+			}
+		}
+		err := checkParse(ciIssuerMetadata.SubjectAlternativeNameTemplate)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // Load a config from disk, or use defaults
 func Load(configPath string) (*FulcioConfig, error) {
 	if _, err := os.Stat(configPath); os.IsNotExist(err) {
@@ -469,7 +497,13 @@ func Load(configPath string) (*FulcioConfig, error) {
 	if err != nil {
 		return nil, fmt.Errorf("read file: %w", err)
 	}
-	return Read(b)
+
+	fulcioConfig, err := Read(b)
+	if err != nil {
+		return fulcioConfig, err
+	}
+	err = CheckParseTemplates(fulcioConfig)
+	return fulcioConfig, err
 }
 
 // Read parses the bytes of a config
diff --git a/pkg/config/config_network_test.go b/pkg/config/config_network_test.go
index 52808181a..99405b594 100644
--- a/pkg/config/config_network_test.go
+++ b/pkg/config/config_network_test.go
@@ -25,6 +25,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/sigstore/fulcio/pkg/certificate"
 )
 
 func TestLoad(t *testing.T) {
@@ -68,6 +69,47 @@ func TestLoad(t *testing.T) {
 	}
 }
 
+func TestParseTemplate(t *testing.T) {
+
+	validTemplate := "{{.foobar}}"
+	invalidTemplate := "{{.foobar}"
+	ciissuerMetadata := make(map[string]DefaultTemplateValues)
+	ciissuerMetadata["github"] = DefaultTemplateValues{
+		ClaimsTemplates: certificate.Extensions{
+			BuildTrigger: invalidTemplate,
+		},
+	}
+	fulcioConfig := &FulcioConfig{
+		CIIssuerMetadata: ciissuerMetadata,
+	}
+	err := CheckParseTemplates(fulcioConfig)
+	if err == nil {
+		t.Error("It should raise an error")
+	}
+	ciissuerMetadata["github"] = DefaultTemplateValues{
+		ClaimsTemplates: certificate.Extensions{
+			BuildTrigger: validTemplate,
+		},
+	}
+	fulcioConfig = &FulcioConfig{
+		CIIssuerMetadata: ciissuerMetadata,
+	}
+	err = CheckParseTemplates(fulcioConfig)
+	if err != nil {
+		t.Error("It shouldn't raise an error")
+	}
+	ciissuerMetadata["github"] = DefaultTemplateValues{
+		SubjectAlternativeNameTemplate: invalidTemplate,
+	}
+	fulcioConfig = &FulcioConfig{
+		CIIssuerMetadata: ciissuerMetadata,
+	}
+	err = CheckParseTemplates(fulcioConfig)
+	if err == nil {
+		t.Error("It should raise an error")
+	}
+}
+
 func TestLoadDefaults(t *testing.T) {
 	td := t.TempDir()
 
diff --git a/pkg/identity/ciprovider/principal.go b/pkg/identity/ciprovider/principal.go
index 86e8494bd..c54231d3e 100644
--- a/pkg/identity/ciprovider/principal.go
+++ b/pkg/identity/ciprovider/principal.go
@@ -69,6 +69,8 @@ func applyTemplateOrReplace(extValueTemplate string, tokenClaims map[string]stri
 		// This option forces to having the claim that is required
 		// for the template
 		t := template.New("").Option("missingkey=error")
+		// It shouldn't raise error since we already checked all
+		// templates in CheckParseTemplates functions in config.go
 		p, err := t.Parse(extValueTemplate)
 		if err != nil {
 			return "", err