From 13ecb4841606dc9fced92dcd9580ea3fa5f9bb91 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]" <github-actions[bot]@users.noreply.github.com>
Date: Tue, 31 Mar 2026 10:08:18 +0200
Subject: [PATCH] fix: resolve env:// scheme in SecureString.fromRaw

resolveKey() only dispatched enc:// and file:// to credential.Resolver.
env:// references were returned as the literal string, causing 401 errors
when API keys are stored as env:// references in config.

Add regression tests in resolve_key_sushi30_test.go documenting the fix
and guarding against future rebase regressions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 pkg/config/config_struct.go            |  2 +-
 pkg/config/resolve_key_sushi30_test.go | 42 ++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 pkg/config/resolve_key_sushi30_test.go

diff --git a/pkg/config/config_struct.go b/pkg/config/config_struct.go
index 418475166..bf8ba31e6 100644
--- a/pkg/config/config_struct.go
+++ b/pkg/config/config_struct.go
@@ -276,7 +276,7 @@ func resolveKey(v string) (string, error) {
 	if resolver == nil {
 		resolver = credential.NewResolver("")
 	}
-	if strings.HasPrefix(v, "enc://") || strings.HasPrefix(v, "file://") {
+	if strings.HasPrefix(v, "enc://") || strings.HasPrefix(v, "file://") || strings.HasPrefix(v, "env://") {
 		decrypted, err := resolver.Resolve(v)
 		if err != nil {
 			logger.Errorf("Resolve error: %v", err)
diff --git a/pkg/config/resolve_key_sushi30_test.go b/pkg/config/resolve_key_sushi30_test.go
new file mode 100644
index 000000000..1d050e6ce
--- /dev/null
+++ b/pkg/config/resolve_key_sushi30_test.go
@@ -0,0 +1,42 @@
+// resolve_key_sushi30_test.go — sushi30 fork regression tests for resolveKey.
+//
+// Background: resolveKey() previously only dispatched enc:// and file:// to
+// credential.Resolver.Resolve(). env:// references were returned as-is (the
+// literal string "env://VAR_NAME"), causing 401 authentication errors when API
+// keys were stored as env:// references in config.
+//
+// Fix: pkg/config/config_struct.go resolveKey() now also dispatches env://.
+// These tests guard against that regression being re-introduced by a future
+// upstream merge or rebase.
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestResolveKey_EnvScheme verifies that a SecureString initialised with an
+// env:// reference resolves to the environment variable value, not the raw
+// reference string.
+func TestResolveKey_EnvScheme(t *testing.T) {
+	t.Setenv("PICOCLAW_TEST_API_KEY", "sk-from-env")
+
+	s := NewSecureString("env://PICOCLAW_TEST_API_KEY")
+	assert.Equal(t, "sk-from-env", s.String(),
+		"env:// reference must resolve to the env var value, not the literal string")
+}
+
+// TestResolveKey_EnvScheme_Unset verifies that an unset env:// reference
+// causes an error (not a silent empty string or the literal reference).
+func TestResolveKey_EnvScheme_Unset(t *testing.T) {
+	// Ensure the variable is not set.
+	t.Setenv("PICOCLAW_TEST_UNSET_KEY", "")
+
+	s := NewSecureString("env://PICOCLAW_TEST_UNSET_KEY_DEFINITELY_MISSING")
+	// resolver.Resolve returns an error for missing/empty env vars;
+	// fromRaw propagates it but resolveKey logs and returns "". The resolved
+	// value must NOT be the raw reference string.
+	assert.NotEqual(t, "env://PICOCLAW_TEST_UNSET_KEY_DEFINITELY_MISSING", s.String(),
+		"env:// reference to an unset variable must not return the literal reference string")
+}