fixing security comments and copilot issues

Author: gautambaghel

cmd/terraform-mcp-server/init.go

@@ -10,6 +10,7 @@ import (
 	stdlog "log"
 	"net/http"
 	"os"
+	"path"
 	"strings"
 	"time"
 

e2e/cors_e2e_test.go

@@ -150,8 +150,8 @@ func runCORSTests(t *testing.T, mcpURL, mode, configuredOrigins string) {
 
 	// Define base test cases that apply to all modes
 	baseTestCases := []testCase{
-		{"GET with allowed origin", "GET", "https://example.com", 202, true},
-		{"GET with no origin", "GET", "", 202, false},
+		{"GET with allowed origin", "GET", "https://example.com", 200, true},
+		{"GET with no origin", "GET", "", 200, false},
 		{"OPTIONS preflight with allowed origin", "OPTIONS", "https://example.com", 200, true},
 	}
 
@@ -163,15 +163,15 @@ func runCORSTests(t *testing.T, mcpURL, mode, configuredOrigins string) {
 	}
 
 	developmentModeTests := []testCase{
-		{"GET with localhost origin", "GET", "http://localhost:3000", 202, true},
-		{"GET with IPv4 localhost", "GET", "http://127.0.0.1:3000", 202, true},
-		{"GET with IPv6 localhost", "GET", "http://[::1]:3000", 202, true},
+		{"GET with localhost origin", "GET", "http://localhost:3000", 200, true},
+		{"GET with IPv4 localhost", "GET", "http://127.0.0.1:3000", 200, true},
+		{"GET with IPv6 localhost", "GET", "http://[::1]:3000", 200, true},
 		{"GET with disallowed origin", "GET", "https://evil.com", 403, false},
 		{"OPTIONS with localhost origin", "OPTIONS", "http://localhost:3000", 200, true},
 	}
 
 	disabledModeTests := []testCase{
-		{"GET with any origin", "GET", "https://any-site.com", 202, true},
+		{"GET with any origin", "GET", "https://any-site.com", 200, true},
 		{"OPTIONS with any origin", "OPTIONS", "https://any-site.com", 200, true},
 	}
 
@@ -195,7 +195,7 @@ func runCORSTests(t *testing.T, mcpURL, mode, configuredOrigins string) {
 			var sessionID string
 			if tc.method != "OPTIONS" {
 				// Only try to initialize if we expect it to succeed
-				if tc.expectedStatus == 202 {
+				if tc.expectedStatus == 200 {
 					sessionID = initializeMCPSession(t, mcpURL, tc.origin)
 					require.NotEmpty(t, sessionID, "Expected to get a session ID for allowed origin")
 				} else {

pkg/resources/resource_templates.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"path"
 
 	"github.com/hashicorp/terraform-mcp-server/pkg/client"
 	"github.com/hashicorp/terraform-mcp-server/pkg/utils"
@@ -16,10 +17,16 @@ import (
 )
 
 func RegisterResourceTemplates(hcServer *server.MCPServer, logger *log.Logger) {
-	hcServer.AddResourceTemplate(ProviderResourceTemplate(path.Join(utils.PROVIDER_BASE_PATH, "{namespace}", "name", "{name}", "version", "{version}"), utils.PROVIDER_BASE_PATH), "Provider details", logger))
+	hcServer.AddResourceTemplate(
+		providerResourceTemplate(
+			path.Join(utils.PROVIDER_BASE_PATH, "{namespace}", "name", "{name}", "version", "{version}"),
+			"Provider details",
+			logger,
+		),
+	)
 }
 
-func ProviderResourceTemplate(resourceURI string, description string, logger *log.Logger) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) {
+func providerResourceTemplate(resourceURI string, description string, logger *log.Logger) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) {
 	return mcp.NewResourceTemplate(
 			resourceURI,
 			description,

pkg/tools/dynamic_tool.go

@@ -123,12 +123,11 @@ func (r *DynamicToolRegistry) createDynamicTFETool(toolName string, toolFactory
 			terraformClient := client.GetTerraformClient(sessionID)
 			if terraformClient == nil || terraformClient.TfeClient == nil {
 				r.logger.WithFields(log.Fields{
-					"tool":       toolName,
+					"tool": toolName,
 				}).Warn("TFE tool called but session has no valid TFE client")
 
 				return mcp.NewToolResultError("This tool is not available. This tool requires a valid Terraform Cloud/Enterprise token and configuration. Please ensure TFE_TOKEN and TFE_ADDRESS environment variables are properly set."), nil
 			}
-
 			// If we found a valid client that wasn't registered, register it now
 			r.RegisterSessionWithTFE(sessionID)
 		}

pkg/tools/get_provider_docs.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"path"
 
 	"github.com/hashicorp/terraform-mcp-server/pkg/client"
 	"github.com/hashicorp/terraform-mcp-server/pkg/utils"

pkg/tools/list_terraform_orgs.go

@@ -22,14 +22,15 @@ func ListTerraformOrgs(logger *log.Logger) server.ServerTool {
 			mcp.WithDescription(`Fetches a list of all Terraform organizations.`),
 			mcp.WithTitleAnnotation("List all Terraform organizations"),
 			mcp.WithReadOnlyHintAnnotation(true),
+			utils.WithPagination(),
 		),
 		Handler: func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
-			return listTerraformOrgsHandler(ctx, logger)
+			return listTerraformOrgsHandler(ctx, req, logger)
 		},
 	}
 }
 
-func listTerraformOrgsHandler(ctx context.Context, logger *log.Logger) (*mcp.CallToolResult, error) {
+func listTerraformOrgsHandler(ctx context.Context, request mcp.CallToolRequest, logger *log.Logger) (*mcp.CallToolResult, error) {
 	// Get a Terraform client from context
 	terraformClients, err := client.GetTerraformClientFromContext(ctx, logger)
 	if err != nil {
@@ -41,9 +42,15 @@ func listTerraformOrgsHandler(ctx context.Context, logger *log.Logger) (*mcp.Cal
 		return nil, utils.LogAndReturnError(logger, "TFE client is not available - please ensure TFE_TOKEN and TFE_ADDRESS are properly configured", nil)
 	}
 
+	pagination, err := utils.OptionalPaginationParams(request)
+	if err != nil {
+		return mcp.NewToolResultError(err.Error()), nil
+	}
+
 	orgs, err := tfeClient.Organizations.List(ctx, &tfe.OrganizationListOptions{
 		ListOptions: tfe.ListOptions{
-			PageSize: 100,
+			PageNumber: pagination.Page,
+			PageSize:   pagination.PageSize,
 		},
 	})
 

pkg/tools/list_terraform_projects.go

@@ -26,6 +26,7 @@ func ListTerraformProjects(logger *log.Logger) server.ServerTool {
 				mcp.Required(),
 				mcp.Description("The name of the Terraform organization to list projects for."),
 			),
+			utils.WithPagination(),
 		),
 		Handler: func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
 			return listTerraformProjectsHandler(ctx, req, logger)
@@ -42,6 +43,11 @@ func listTerraformProjectsHandler(ctx context.Context, request mcp.CallToolReque
 		return nil, utils.LogAndReturnError(logger, "terraform_org_name cannot be empty", nil)
 	}
 
+	pagination, err := utils.OptionalPaginationParams(request)
+	if err != nil {
+		return mcp.NewToolResultError(err.Error()), nil
+	}
+
 	// Get a Terraform client from context
 	terraformClients, err := client.GetTerraformClientFromContext(ctx, logger)
 	if err != nil {
@@ -55,7 +61,8 @@ func listTerraformProjectsHandler(ctx context.Context, request mcp.CallToolReque
 	// Fetch the list of projects
 	projects, err := tfeClient.Projects.List(ctx, terraformOrgName, &tfe.ProjectListOptions{
 		ListOptions: tfe.ListOptions{
-			PageSize: 100,
+			PageNumber: pagination.Page,
+			PageSize:   pagination.PageSize,
 		},
 	})
 

pkg/tools/policy_details.go

@@ -7,7 +7,9 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net/url"
 	"strings"
+	"text/template"
 
 	"github.com/hashicorp/terraform-mcp-server/pkg/client"
 	"github.com/hashicorp/terraform-mcp-server/pkg/utils"
@@ -69,11 +71,28 @@ func getPolicyDetailsHandler(ctx context.Context, request mcp.CallToolRequest, l
 	moduleList := ""
 	for _, policy := range policyDetails.Included {
 		if policy.Type == "policy-modules" {
-			moduleList += fmt.Sprintf(`
-module "%s" {
-	source = "https://registry.terraform.io/v2%s/policy-module/%s.sentinel?checksum=sha256:%s"
+			// Use text/template to safely build the module block
+			var moduleBuilder strings.Builder
+			tmpl := `
+module "{{.Name}}" {
+	source = "https://registry.terraform.io/v2{{.PolicyID}}/policy-module/{{.Name}}.sentinel?checksum=sha256:{{.Shasum}}"
 }
-`, policy.Attributes.Name, terraformPolicyID, policy.Attributes.Name, policy.Attributes.Shasum)
+`
+			type moduleData struct {
+				Name     string
+				PolicyID string
+				Shasum   string
+			}
+			t := template.Must(template.New("module").Parse(tmpl))
+			err := t.Execute(&moduleBuilder, moduleData{
+				Name:     policy.Attributes.Name,
+				PolicyID: terraformPolicyID,
+				Shasum:   policy.Attributes.Shasum,
+			})
+			if err != nil {
+				logger.WithError(err).Error("failed to render module template")
+			}
+			moduleList += moduleBuilder.String()
 		}
 
 		if policy.Type == "policies" {
@@ -85,13 +104,30 @@ module "%s" {
 	builder.WriteString("## Usage\n\n")
 	builder.WriteString("Generate the content for a HashiCorp Configuration Language (HCL) file named policies.hcl. This file should define a set of policies. For each policy provided, create a distinct policy block using the following template.\n")
 	builder.WriteString("\n```hcl\n")
-	hclTemplate := fmt.Sprintf(`
-%s
+	// Use text/template to safely build the HCL template for policies
+	hclTmpl := `
+{{- if .ModuleList }}
+{{ .ModuleList }}
+{{- end }}
 policy "<<POLICY_NAME>>" {
-	source = "https://registry.terraform.io/v2%s/policy/<<POLICY_NAME>>.sentinel?checksum=<<POLICY_CHECKSUM>>"
-	enforcement_level = "advisory"
+  source = "https://registry.terraform.io/v2{{ .TerraformPolicyID }}/policy/<<POLICY_NAME>>.sentinel?checksum=<<POLICY_CHECKSUM>>"
+  enforcement_level = "advisory"
 }
-`, moduleList, terraformPolicyID)
+`
+	type hclTemplateData struct {
+		ModuleList        string
+		TerraformPolicyID string
+	}
+	var hclBuilder strings.Builder
+	t := template.Must(template.New("hclPolicy").Parse(hclTmpl))
+	err = t.Execute(&hclBuilder, hclTemplateData{
+		ModuleList:        moduleList,
+		TerraformPolicyID: terraformPolicyID,
+	})
+	if err != nil {
+		logger.WithError(err).Error("failed to render HCL policy template")
+	}
+	hclTemplate := hclBuilder.String()
 	builder.WriteString(hclTemplate)
 	builder.WriteString("\n```\n")
 	builder.WriteString(fmt.Sprintf("Available policies with SHA for %s are: \n\n", terraformPolicyID))

pkg/tools/resolve_provider_doc_id.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"path"
 	"strings"
 
 	"github.com/hashicorp/terraform-mcp-server/pkg/client"
@@ -126,7 +127,7 @@ func resolveProviderDocIDHandler(ctx context.Context, request mcp.CallToolReques
 			cs_pn, err_pn := utils.ContainsSlug(fmt.Sprintf("%s_%s", providerDetail.ProviderName, doc.Slug), serviceSlug)
 			if (cs || cs_pn) && err == nil && err_pn == nil {
 				contentAvailable = true
-				descriptionSnippet, err := getContentSnippet(registryClient, doc.ID, logger)
+				descriptionSnippet, err := getContentSnippet(httpClient, doc.ID, logger)
 				if err != nil {
 					logger.Warnf("Error fetching content snippet for provider doc ID: %s: %v", doc.ID, err)
 				}
@@ -143,7 +144,7 @@ func resolveProviderDocIDHandler(ctx context.Context, request mcp.CallToolReques
 	return mcp.NewToolResultText(builder.String()), nil
 }
 
-func resolveProviderDetails(request mcp.CallToolRequest, registryClient *http.Client, defaultErrorGuide string, logger *log.Logger) (client.ProviderDetail, error) {
+func resolveProviderDetails(request mcp.CallToolRequest, httpClient *http.Client, defaultErrorGuide string, logger *log.Logger) (client.ProviderDetail, error) {
 	providerDetail := client.ProviderDetail{}
 	providerName := request.GetString("provider_name", "")
 	if providerName == "" {
@@ -164,7 +165,7 @@ func resolveProviderDetails(request mcp.CallToolRequest, registryClient *http.Cl
 	if utils.IsValidProviderVersionFormat(providerVersion) {
 		providerVersionValue = providerVersion
 	} else {
-		providerVersionValue, err = client.GetLatestProviderVersion(registryClient, providerNamespace, providerName, logger)
+		providerVersionValue, err = client.GetLatestProviderVersion(httpClient, providerNamespace, providerName, logger)
 		if err != nil {
 			providerVersionValue = ""
 			logger.Debugf("Error getting latest provider version in %s namespace: %v", providerNamespace, err)
@@ -174,7 +175,7 @@ func resolveProviderDetails(request mcp.CallToolRequest, registryClient *http.Cl
 	// If the provider version doesn't exist, try the hashicorp namespace
 	if providerVersionValue == "" {
 		tryProviderNamespace := "hashicorp"
-		providerVersionValue, err = client.GetLatestProviderVersion(registryClient, tryProviderNamespace, providerName, logger)
+		providerVersionValue, err = client.GetLatestProviderVersion(httpClient, tryProviderNamespace, providerName, logger)
 		if err != nil {
 			// Just so we don't print the same namespace twice if they are the same
 			if providerNamespace != tryProviderNamespace {
@@ -198,20 +199,20 @@ func resolveProviderDetails(request mcp.CallToolRequest, registryClient *http.Cl
 }
 
 // get_provider_docsV2 retrieves a list of documentation items for a specific provider category using v2 API with support for pagination using page numbers
-func get_provider_docsV2(registryClient *http.Client, providerDetail client.ProviderDetail, logger *log.Logger) (string, error) {
-	providerVersionID, err := client.GetProviderVersionID(registryClient, providerDetail.ProviderNamespace, providerDetail.ProviderName, providerDetail.ProviderVersion, logger)
+func get_provider_docsV2(httpClient *http.Client, providerDetail client.ProviderDetail, logger *log.Logger) (string, error) {
+	providerVersionID, err := client.GetProviderVersionID(httpClient, providerDetail.ProviderNamespace, providerDetail.ProviderName, providerDetail.ProviderVersion, logger)
 	if err != nil {
 		return "", utils.LogAndReturnError(logger, "getting provider version ID", err)
 	}
 	category := providerDetail.ProviderDataType
 	if category == "overview" {
-		return client.GetProviderOverviewDocs(registryClient, providerVersionID, logger)
+		return client.GetProviderOverviewDocs(httpClient, providerVersionID, logger)
 	}
 
 	uriPrefix := fmt.Sprintf("provider-docs?filter[provider-version]=%s&filter[category]=%s&filter[language]=hcl",
 		providerVersionID, category)
 
-	docs, err := client.SendPaginatedRegistryCall(registryClient, uriPrefix, logger)
+	docs, err := client.SendPaginatedRegistryCall(httpClient, uriPrefix, logger)
 	if err != nil {
 		return "", utils.LogAndReturnError(logger, "getting provider documentation", err)
 	}
@@ -225,7 +226,7 @@ func get_provider_docsV2(registryClient *http.Client, providerDetail client.Prov
 	builder.WriteString("Each result includes:\n- providerDocID: tfprovider-compatible identifier\n- Title: Service or resource name\n- Category: Type of document\n- Description: Brief summary of the document\n")
 	builder.WriteString("For best results, select libraries based on the service_slug match and category of information requested.\n\n---\n\n")
 	for _, doc := range docs {
-		descriptionSnippet, err := getContentSnippet(registryClient, doc.ID, logger)
+		descriptionSnippet, err := getContentSnippet(httpClient, doc.ID, logger)
 		if err != nil {
 			logger.Warnf("Error fetching content snippet for provider doc ID: %s: %v", doc.ID, err)
 		}
@@ -235,8 +236,8 @@ func get_provider_docsV2(registryClient *http.Client, providerDetail client.Prov
 	return builder.String(), nil
 }
 
-func getContentSnippet(registryClient *http.Client, docID string, logger *log.Logger) (string, error) {
-	docContent, err := client.SendRegistryCall(registryClient, "GET", fmt.Sprintf("provider-docs/%s", docID), logger, "v2")
+func getContentSnippet(httpClient *http.Client, docID string, logger *log.Logger) (string, error) {
+	docContent, err := client.SendRegistryCall(httpClient, "GET", fmt.Sprintf("provider-docs/%s", docID), logger, "v2")
 	if err != nil {
 		return "", utils.LogAndReturnError(logger, fmt.Sprintf("error fetching provider-docs/%s within getContentSnippet", docID), err)
 	}

pkg/tools/search_policies.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net/url"
 	"strings"
 
 	"github.com/hashicorp/terraform-mcp-server/pkg/client"
@@ -62,8 +63,14 @@ func getSearchPoliciesHandler(ctx context.Context, request mcp.CallToolRequest,
 	}
 
 	httpClient := terraformClients.HttpClient
-	// static list of 100 is fine for now
-	policyResp, err := client.SendRegistryCall(httpClient, "GET", (&url.URL{Path: "policies", RawQuery: url.Values{"page[size]": {"100"}, "include": {"latest-version"}}.Encode()}).String(), logger, "v2")
+	uri := (&url.URL{
+		Path: "policies",
+		RawQuery: url.Values{
+			"page[size]": {"100"}, // static list of 100 is fine for now
+			"include":    {"latest-version"},
+		}.Encode(),
+	}).String()
+	policyResp, err := client.SendRegistryCall(httpClient, "GET", uri, logger, "v2")
 	if err != nil {
 		return nil, utils.LogAndReturnError(logger, "Failed to fetch policies: registry API did not return a successful response", err)
 	}