From d5b0de12cb6fe874742917ec1270f788ed671241 Mon Sep 17 00:00:00 2001
From: Lukasz Raczylo <lukasz@raczylo.com>
Date: Sat, 23 May 2026 12:15:06 +0100
Subject: [PATCH] fix(secrets): pass through unset env vars so harness-resolved
 ${VARS} survive

---
 internal/secrets/envsub.go      | 21 ++++++++++++++++++++-
 internal/secrets/envsub_test.go | 29 +++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/internal/secrets/envsub.go b/internal/secrets/envsub.go
index 885d680..46ecd20 100644
--- a/internal/secrets/envsub.go
+++ b/internal/secrets/envsub.go
@@ -10,8 +10,27 @@ import (
 // Lookup resolves a placeholder name to a value. Returns false if missing.
 type Lookup func(name string) (string, bool)
 
-// OSEnv resolves via os.LookupEnv.
+// OSEnv resolves via os.LookupEnv. Returns (value, true) when the variable
+// is set, otherwise (placeholder, true) so the literal ${NAME} reaches the
+// downstream config unchanged.
+//
+// Rationale: many harness MCP configs reference variables that the harness
+// itself injects at MCP launch time (e.g. ${CLAUDE_PROJECT} for claude-code,
+// ${GITHUB_TOKEN} when the user keeps it out of their login shell). Aborting
+// the apply for an unset variable would block valid configurations, so unset
+// names are passed through. Real secrets the user wants resolved at apply
+// time must be exported in the process environment.
 func OSEnv(name string) (string, bool) {
+	if v, ok := os.LookupEnv(name); ok {
+		return v, true
+	}
+	return "${" + name + "}", true
+}
+
+// StrictOSEnv resolves via os.LookupEnv and reports false for unset
+// variables, causing Substitute to return an error. Use when an empty
+// resolution is unacceptable (e.g. dedicated secrets pipeline).
+func StrictOSEnv(name string) (string, bool) {
 	return os.LookupEnv(name)
 }
 
diff --git a/internal/secrets/envsub_test.go b/internal/secrets/envsub_test.go
index 3f7f1ad..9a9c94c 100644
--- a/internal/secrets/envsub_test.go
+++ b/internal/secrets/envsub_test.go
@@ -30,3 +30,32 @@ func TestSubstituteEscaped(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, "${LITERAL}", out)
 }
+
+func TestOSEnvPassThroughOnMissing(t *testing.T) {
+	t.Setenv("HARNESS_SYNC_TEST_SET", "value-set")
+	v, ok := OSEnv("HARNESS_SYNC_TEST_SET")
+	assert.True(t, ok)
+	assert.Equal(t, "value-set", v)
+
+	// Unset variable returns the literal placeholder so downstream configs
+	// preserve runtime-resolved references like ${CLAUDE_PROJECT}.
+	v, ok = OSEnv("HARNESS_SYNC_TEST_NEVER_SET_XYZ")
+	assert.True(t, ok)
+	assert.Equal(t, "${HARNESS_SYNC_TEST_NEVER_SET_XYZ}", v)
+}
+
+func TestStrictOSEnvErrorsOnMissing(t *testing.T) {
+	t.Setenv("HARNESS_SYNC_TEST_SET2", "x")
+	v, ok := StrictOSEnv("HARNESS_SYNC_TEST_SET2")
+	assert.True(t, ok)
+	assert.Equal(t, "x", v)
+
+	_, ok = StrictOSEnv("HARNESS_SYNC_TEST_NEVER_SET_XYZ2")
+	assert.False(t, ok)
+}
+
+func TestSubstituteWithPassThroughOSEnv(t *testing.T) {
+	out, err := Substitute("token=${HARNESS_SYNC_NEVER_SET}", OSEnv)
+	assert.NoError(t, err)
+	assert.Equal(t, "token=${HARNESS_SYNC_NEVER_SET}", out)
+}