test(config): build api_key reference configs with encoding/json

Formatting a Windows path (C:\Users\...) into a JSON string produced
invalid escapes on the Windows runner. The api_key reference tests now
write mcp_config.json through json.Marshal, compare the stored reference
by decoding the file, and a new test pins the backslash round trip. The
symlink read test skips on Windows when the account cannot create one.
Docs say what an absolute path is on Windows.

Author: james-crowley

docs/features/keyring-integration.md

@@ -54,7 +54,10 @@ MCPProxy supports three types of secret references that can be used anywhere in
 (`docker inspect`, `/proc/<pid>/environ`). The same rules apply wherever it is used:
 
 - The path must be absolute. A symlink is followed (Kubernetes secret volumes are symlinks) but must
-  point at a regular file.
+  point at a regular file. On Windows an absolute path has a drive letter (`C:\ProgramData\mcpproxy\key`
+  or `C:/ProgramData/mcpproxy/key`); a path that starts with `/` and has no drive is not absolute. In
+  `mcp_config.json` a backslash must be escaped (`"${file:C:\\ProgramData\\mcpproxy\\key}"`), so forward
+  slashes are easier.
 - At most 64 KiB is read. A larger file is an error, never a truncated secret.
 - One trailing newline is removed, LF or CRLF, so a file written on Windows resolves to the same value
   as one written on Linux. Leading and inner whitespace are kept.

internal/config/api_key_env_test.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"context"
+	"encoding/json"
 	"os"
 	"path/filepath"
 	"testing"
@@ -19,6 +20,21 @@ func writeKeyFile(t *testing.T, content string) string {
 	return path
 }
 
+// writeAPIKeyConfig writes a minimal mcp_config.json whose api_key is apiKey.
+// The JSON is built with encoding/json, never by formatting apiKey into a
+// string: a reference such as ${file:C:\Users\x\key} carries backslashes that
+// a Windows path puts into the document and that must be escaped.
+func writeAPIKeyConfig(t *testing.T, apiKey string) string {
+	t.Helper()
+	raw, err := json.Marshal(map[string]any{
+		"listen":     "127.0.0.1:8080",
+		"api_key":    apiKey,
+		"mcpServers": []any{},
+	})
+	require.NoError(t, err)
+	return writeOverrideTestFile(t, string(raw))
+}
+
 // unsetEnv removes name for the test and restores it afterwards (t.Setenv
 // records the original value; the Unsetenv that follows leaves it unset).
 func unsetEnv(t *testing.T, name string) {
@@ -275,7 +291,7 @@ func TestLoad_APIKeyReferenceResolvesAsProcessOverride(t *testing.T) {
 	ResetProcessOverrides()
 	keyFile := writeKeyFile(t, "ref-secret-key\r\n")
 	ref := "${file:" + keyFile + "}"
-	path := writeOverrideTestFile(t, `{"listen": "127.0.0.1:8080", "api_key": "`+ref+`", "mcpServers": []}`)
+	path := writeAPIKeyConfig(t, ref)
 	unsetEnv(t, EnvAPIKey)
 	t.Setenv(EnvAPIKeyFile, "")
 
@@ -307,7 +323,7 @@ func TestPersistableConfig_APIKeyReferenceSurvivesAnUnrelatedEdit(t *testing.T)
 	ResetProcessOverrides()
 	keyFile := writeKeyFile(t, "ref-secret-key\n")
 	ref := "${file:" + keyFile + "}"
-	path := writeOverrideTestFile(t, `{"listen": "127.0.0.1:8080", "api_key": "`+ref+`", "mcpServers": []}`)
+	path := writeAPIKeyConfig(t, ref)
 	unsetEnv(t, EnvAPIKey)
 	t.Setenv(EnvAPIKeyFile, "")
 
@@ -325,7 +341,7 @@ func TestPersistableConfig_EditingAPIKeyUnderAReferenceIsPersisted(t *testing.T)
 	t.Cleanup(ResetProcessOverrides)
 	ResetProcessOverrides()
 	keyFile := writeKeyFile(t, "ref-secret-key\n")
-	path := writeOverrideTestFile(t, `{"listen": "127.0.0.1:8080", "api_key": "${file:`+keyFile+`}", "mcpServers": []}`)
+	path := writeAPIKeyConfig(t, "${file:"+keyFile+"}")
 	unsetEnv(t, EnvAPIKey)
 	t.Setenv(EnvAPIKeyFile, "")
 
@@ -348,16 +364,15 @@ func TestLoad_APIKeyReferenceThatCannotBeResolvedFailsTheLoad(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			t.Cleanup(ResetProcessOverrides)
 			ResetProcessOverrides()
-			path := writeOverrideTestFile(t, `{"listen": "127.0.0.1:8080", "api_key": "`+ref+`", "mcpServers": []}`)
+			path := writeAPIKeyConfig(t, ref)
 			unsetEnv(t, EnvAPIKey)
 			t.Setenv(EnvAPIKeyFile, "")
 
 			_, err := LoadFromFile(path)
 			require.Error(t, err, "no key may be generated over a reference that does not resolve")
 			assert.Contains(t, err.Error(), "api_key")
-			raw, readErr := os.ReadFile(path)
-			require.NoError(t, readErr)
-			assert.Contains(t, string(raw), ref, "the file is untouched")
+			assert.NotContains(t, err.Error(), ref, "the error never quotes the reference")
+			assert.Equal(t, ref, readJSON(t, path)["api_key"], "the file is untouched")
 		})
 	}
 }
@@ -368,7 +383,7 @@ func TestLoad_EnvironmentKeyWinsOverAnAPIKeyReferenceWithoutReadingIt(t *testing
 	// The referenced file does not exist: with an env key the reference is not
 	// resolved at all, so that is not an error.
 	ref := "${file:" + filepath.Join(t.TempDir(), "missing") + "}"
-	path := writeOverrideTestFile(t, `{"listen": "127.0.0.1:8080", "api_key": "`+ref+`", "mcpServers": []}`)
+	path := writeAPIKeyConfig(t, ref)
 	t.Setenv(EnvAPIKey, "env-key")
 	t.Setenv(EnvAPIKeyFile, "")
 
@@ -405,3 +420,27 @@ func TestServerConfig_FileReferencesExpandOnACopy(t *testing.T) {
 	assert.Contains(t, orig.Env["GH_TOKEN"], "${file:", "the stored config keeps the reference")
 	assert.Contains(t, orig.Headers["Authorization"], "${file:")
 }
+
+// Both variables set stays an error when api_key is a reference: the reference
+// is not read, and the error is the same fixed text.
+func TestLoad_BothSetFailsTheLoadEvenWithAnAPIKeyReference(t *testing.T) {
+	t.Cleanup(ResetProcessOverrides)
+	ResetProcessOverrides()
+	ref := "${file:" + writeKeyFile(t, "ref-secret-key\n") + "}"
+	path := writeAPIKeyConfig(t, ref)
+	t.Setenv(EnvAPIKey, "env-secret-value-1234")
+	t.Setenv(EnvAPIKeyFile, writeKeyFile(t, "file-secret-value-5678\n"))
+
+	_, err := LoadFromFile(path)
+	require.ErrorIs(t, err, ErrAPIKeyBothSet)
+	assert.NotContains(t, err.Error(), ref)
+	assert.NotContains(t, err.Error(), "ref-secret-key")
+}
+
+// The helper must produce valid JSON for a Windows-style path, which is what
+// the tests above feed it when they run on Windows.
+func TestWriteAPIKeyConfig_EscapesBackslashes(t *testing.T) {
+	ref := `${file:C:\Users\ci\AppData\Local\Temp\key}`
+	path := writeAPIKeyConfig(t, ref)
+	assert.Equal(t, ref, readJSON(t, path)["api_key"])
+}

internal/secret/file_read_test.go

@@ -103,7 +103,12 @@ func TestReadFile_FollowsSymlinkToRegularFile(t *testing.T) {
 	// Kubernetes secret volumes expose each key as a symlink into ..data/.
 	target := writeSecretFile(t, "from-target\n")
 	link := filepath.Join(t.TempDir(), "link")
-	require.NoError(t, os.Symlink(target, link))
+	if err := os.Symlink(target, link); err != nil {
+		if runtime.GOOS == "windows" {
+			t.Skipf("creating a symlink needs a privilege this Windows account lacks: %v", err)
+		}
+		require.NoError(t, err)
+	}
 
 	got, err := ReadFile(link)
 	require.NoError(t, err)