[object Object]

← back to Cli Printing Press

fix(cli): keep template-var env names out of verifier auth discovery

a0e1bb4aae8d8bf40d2db99210fc48615551e413 · 2026-04-29 21:48:55 -0500 · Cathryn Lavery

discoverCLIEnvVars reads the CLI's config.go and treats every os.Getenv()
result (besides _BASE_URL/_CONFIG suffixes) as an auth env var. After PR-2
adds Config.TemplateVars reads for SHOPIFY_SHOP / SHOPIFY_API_VERSION,
those names leaked into authEnvVars — and the verifier's --api-key path
overwrites every auth name with the API key value, which would route live
requests at the API-key string instead of the configured shop.

Match the template-var assignment shape (cfg.TemplateVars["..."] = v
immediately after the os.Getenv call) and exclude those names from the
discovered set. Add discoverCLITemplateVarEnvs to recover them separately
so mock-mode buildEnv can inject "mock" placeholder values — without
injection, mock verification of a templated GraphQL path returns
TemplateVarError before ever hitting the test server.

Files touched

Diff

commit a0e1bb4aae8d8bf40d2db99210fc48615551e413
Author: Cathryn Lavery <50469282+cathrynlavery@users.noreply.github.com>
Date:   Wed Apr 29 21:48:55 2026 -0500

    fix(cli): keep template-var env names out of verifier auth discovery
    
    discoverCLIEnvVars reads the CLI's config.go and treats every os.Getenv()
    result (besides _BASE_URL/_CONFIG suffixes) as an auth env var. After PR-2
    adds Config.TemplateVars reads for SHOPIFY_SHOP / SHOPIFY_API_VERSION,
    those names leaked into authEnvVars — and the verifier's --api-key path
    overwrites every auth name with the API key value, which would route live
    requests at the API-key string instead of the configured shop.
    
    Match the template-var assignment shape (cfg.TemplateVars["..."] = v
    immediately after the os.Getenv call) and exclude those names from the
    discovered set. Add discoverCLITemplateVarEnvs to recover them separately
    so mock-mode buildEnv can inject "mock" placeholder values — without
    injection, mock verification of a templated GraphQL path returns
    TemplateVarError before ever hitting the test server.
---
 internal/pipeline/runtime.go      | 70 ++++++++++++++++++++++++++++++++++++++-
 internal/pipeline/runtime_test.go | 55 ++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/internal/pipeline/runtime.go b/internal/pipeline/runtime.go
index 926dae1c..887385a1 100644
--- a/internal/pipeline/runtime.go
+++ b/internal/pipeline/runtime.go
@@ -166,6 +166,15 @@ func RunVerify(cfg VerifyConfig) (*VerifyReport, error) {
 		}
 	}
 
+	// EndpointTemplateVars env names live in their own bucket so the
+	// --api-key overwrite path doesn't rewrite SHOPIFY_SHOP into the
+	// API key, and so mock mode can inject placeholder values that
+	// satisfy buildURL without leaking into authEnvVars. Live mode
+	// inherits whatever the operator already exported; we don't mirror
+	// the env into the subprocess again because os.Environ() above
+	// already carries it.
+	templateVarEnvs := discoverCLITemplateVarEnvs(cfg.Dir)
+
 	// buildEnv constructs the environment for test subprocesses, passing
 	// all auth-related env vars so auth-requiring commands can complete.
 	buildEnv := func() []string {
@@ -188,6 +197,15 @@ func RunVerify(cfg VerifyConfig) (*VerifyReport, error) {
 			for _, ev := range authEnvVars {
 				env = append(env, ev+"=mock-token-for-testing")
 			}
+			// Templated URLs (e.g. /admin/api/{api_version}/graphql.json)
+			// need every {var} resolved before buildURL succeeds. Without
+			// injecting safe values here mock-mode requests never leave the
+			// generated CLI — TemplateVarError fires first. The string
+			// "mock" is opaque to the test server, which echoes whatever
+			// path it receives.
+			for _, ev := range templateVarEnvs {
+				env = append(env, ev+"=mock")
+			}
 			// Defense-in-depth: every mock-mode subprocess inherits this
 			// env var. Generated commands that perform visible side
 			// effects (open browser tabs, send notifications) MUST check
@@ -602,6 +620,14 @@ func startMockServer(spec *openAPISpec) (*httptest.Server, string) {
 	return server, server.URL
 }
 
+// templateVarReadRe matches the shape config.go.tmpl emits for each
+// EndpointTemplateVars entry: `os.Getenv("X")` immediately followed by a
+// `cfg.TemplateVars["..."] = v` assignment. Auth-bearing env reads land in
+// named cfg fields; template-var reads land in this map. Used by both
+// discoverCLIEnvVars (to exclude template names from the auth set) and
+// discoverCLITemplateVarEnvs (to recover them for mock-mode injection).
+var templateVarReadRe = regexp.MustCompile(`(?s)os\.Getenv\("([^"]+)"\)[^{]*\{\s*cfg\.TemplateVars\[`)
+
 // discoverCLIEnvVars reads the CLI's config.go and extracts env var names
 // from os.Getenv() calls. This discovers what the CLI actually reads, which
 // may differ from what the spec declares or the API name implies.
@@ -611,8 +637,23 @@ func discoverCLIEnvVars(dir string) []string {
 	if err != nil {
 		return nil
 	}
+	body := string(data)
+
+	// Endpoint template vars (Shopify's SHOPIFY_SHOP / SHOPIFY_API_VERSION
+	// shape) feed Config.TemplateVars, NOT the auth header. The verifier's
+	// --api-key path overwrites every discovered auth env var with the API
+	// key, so leaking a template-var name into authEnvVars rewrites the
+	// resolved hostname to the API key string and routes the live request
+	// at the wrong URL. Match the template-var shape and exclude those
+	// names from the discovered set; mock-mode injection lives separately
+	// in discoverCLITemplateVarEnvs.
+	templateVarNames := map[string]bool{}
+	for _, m := range templateVarReadRe.FindAllStringSubmatch(body, -1) {
+		templateVarNames[m[1]] = true
+	}
+
 	re := regexp.MustCompile(`os\.Getenv\("([^"]+)"\)`)
-	matches := re.FindAllStringSubmatch(string(data), -1)
+	matches := re.FindAllStringSubmatch(body, -1)
 	seen := map[string]bool{}
 	var envVars []string
 	for _, m := range matches {
@@ -621,6 +662,9 @@ func discoverCLIEnvVars(dir string) []string {
 		if strings.HasSuffix(name, "_BASE_URL") || strings.HasSuffix(name, "_CONFIG") {
 			continue
 		}
+		if templateVarNames[name] {
+			continue
+		}
 		if !seen[name] {
 			seen[name] = true
 			envVars = append(envVars, name)
@@ -629,4 +673,28 @@ func discoverCLIEnvVars(dir string) []string {
 	return envVars
 }
 
+// discoverCLITemplateVarEnvs returns the env var names that feed
+// Config.TemplateVars (the {placeholder} markers in BaseURL or the request
+// path). Mock-mode verification needs these so buildURL doesn't fail on
+// unresolved {var} markers — without injection, a templated GraphQL path
+// like /admin/api/{api_version}/graphql.json returns TemplateVarError
+// before the mock server ever sees a request.
+func discoverCLITemplateVarEnvs(dir string) []string {
+	configPath := filepath.Join(dir, "internal", "config", "config.go")
+	data, err := os.ReadFile(configPath)
+	if err != nil {
+		return nil
+	}
+	seen := map[string]bool{}
+	var names []string
+	for _, m := range templateVarReadRe.FindAllStringSubmatch(string(data), -1) {
+		name := m[1]
+		if !seen[name] {
+			seen[name] = true
+			names = append(names, name)
+		}
+	}
+	return names
+}
+
 // camelToKebab is defined in verify.go
diff --git a/internal/pipeline/runtime_test.go b/internal/pipeline/runtime_test.go
index 88091ef2..98602df4 100644
--- a/internal/pipeline/runtime_test.go
+++ b/internal/pipeline/runtime_test.go
@@ -787,3 +787,58 @@ func initRoot() {
 	assert.Equal(t, "PASS", report.Verdict)
 	assert.FileExists(t, report.Binary)
 }
+
+// TestDiscoverCLIEnvVars_SkipsTemplateVarReads guards the verifier integration
+// for endpoint template vars: the helper must NOT report env vars that feed
+// Config.TemplateVars (Shopify's SHOPIFY_SHOP / SHOPIFY_API_VERSION shape) as
+// auth env vars, because the verifier's --api-key path overwrites every
+// discovered name with the API key value, which would route requests at the
+// API-key string instead of the configured shop.
+func TestDiscoverCLIEnvVars_SkipsTemplateVarReads(t *testing.T) {
+	dir := t.TempDir()
+	configDir := filepath.Join(dir, "internal", "config")
+	require.NoError(t, os.MkdirAll(configDir, 0o755))
+
+	// Mirror the shape that config.go.tmpl emits for a spec with both auth
+	// env vars and EndpointTemplateVars: auth reads land in named fields,
+	// template-var reads land in Config.TemplateVars.
+	configBody := `package config
+
+import "os"
+
+type Config struct {
+	BaseURL      string
+	AccessToken  string
+	TemplateVars map[string]string
+}
+
+func Load() *Config {
+	cfg := &Config{}
+	if v := os.Getenv("SHOPIFY_ACCESS_TOKEN"); v != "" {
+		cfg.AccessToken = v
+	}
+	if v := os.Getenv("SHOPIFY_BASE_URL"); v != "" {
+		cfg.BaseURL = v
+	}
+	if cfg.TemplateVars == nil {
+		cfg.TemplateVars = map[string]string{}
+	}
+	if v := os.Getenv("SHOPIFY_SHOP"); v != "" {
+		cfg.TemplateVars["shop"] = v
+	}
+	if v := os.Getenv("SHOPIFY_API_VERSION"); v != "" {
+		cfg.TemplateVars["api_version"] = v
+	}
+	return cfg
+}
+`
+	require.NoError(t, os.WriteFile(filepath.Join(configDir, "config.go"), []byte(configBody), 0o644))
+
+	got := discoverCLIEnvVars(dir)
+	assert.Equal(t, []string{"SHOPIFY_ACCESS_TOKEN"}, got,
+		"discoverCLIEnvVars must report only auth env vars; template-var reads must be excluded")
+
+	gotTemplate := discoverCLITemplateVarEnvs(dir)
+	assert.ElementsMatch(t, []string{"SHOPIFY_SHOP", "SHOPIFY_API_VERSION"}, gotTemplate,
+		"discoverCLITemplateVarEnvs must return the template-var env names so mock mode can inject placeholder values")
+}

← 8cb57db3 feat(cli): substitute EndpointTemplateVars in client.do() +  ·  back to Cli Printing Press  ·  test(cli): cover EndpointTemplateVars runtime substitution + 717a4de3 →