From 20560f5ba6d7d256307869780bed956a8bc267e5 Mon Sep 17 00:00:00 2001 From: hongming-codex-laptop Date: Thu, 14 May 2026 09:27:58 -0700 Subject: [PATCH] fix(handlers): keep embedded missing env refs literal --- .../internal/handlers/org_helpers.go | 5 +-- .../handlers/org_helpers_pure_test.go | 32 +++++++++++-------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/workspace-server/internal/handlers/org_helpers.go b/workspace-server/internal/handlers/org_helpers.go index 3dd569f7..1a88e99b 100644 --- a/workspace-server/internal/handlers/org_helpers.go +++ b/workspace-server/internal/handlers/org_helpers.go @@ -145,10 +145,7 @@ func expandEnvRef(key, ref, whole string, env map[string]string) string { if ref == whole { return os.Getenv(key) } - if os.Getenv(key) != "" { - return ref - } - return "" + return ref } func isEnvIdentStart(c byte) bool { diff --git a/workspace-server/internal/handlers/org_helpers_pure_test.go b/workspace-server/internal/handlers/org_helpers_pure_test.go index ccdc9345..34296abd 100644 --- a/workspace-server/internal/handlers/org_helpers_pure_test.go +++ b/workspace-server/internal/handlers/org_helpers_pure_test.go @@ -104,8 +104,8 @@ func TestHasUnresolvedVarRef_Resolved(t *testing.T) { // documents this design choice; callers who need empty=resolved should // pre-process the output before calling hasUnresolvedVarRef. {"${VAR}", "", true}, - {"${VAR}", "value", false}, // var replaced - {"$VAR", "value", false}, // bare var replaced + {"${VAR}", "value", false}, // var replaced + {"$VAR", "value", false}, // bare var replaced {"prefix${VAR}suffix", "prefixvaluesuffix", false}, {"${A}${B}", "ab", false}, // FOO=FOO and BAR=BAR — both vars found and replaced. Expanded output @@ -125,14 +125,14 @@ func TestHasUnresolvedVarRef_Resolved(t *testing.T) { func TestHasUnresolvedVarRef_Unresolved(t *testing.T) { // Expansion left the refs intact → unresolved. cases := []struct { - orig string + orig string expanded string }{ - {"${VAR}", "${VAR}"}, // untouched - {"$VAR", "$VAR"}, // bare untouched + {"${VAR}", "${VAR}"}, // untouched + {"$VAR", "$VAR"}, // bare untouched {"prefix${VAR}suffix", "prefix${VAR}suffix"}, - {"${A}${B}", "${A}${B}"}, // both unresolved - {"${FOO}", ""}, // empty result with var ref in original + {"${A}${B}", "${A}${B}"}, // both unresolved + {"${FOO}", ""}, // empty result with var ref in original } for _, tc := range cases { t.Run(tc.orig, func(t *testing.T) { @@ -205,8 +205,8 @@ func TestMergeCategoryRouting_WorkspaceOverrides(t *testing.T) { "ui": {"Frontend Engineer"}, } ws := map[string][]string{ - "security": {"SRE Team"}, // narrows - "ui": {}, // drops + "security": {"SRE Team"}, // narrows + "ui": {}, // drops "infra": {"Platform Team"}, // adds } r := mergeCategoryRouting(defaults, ws) @@ -462,8 +462,14 @@ func TestExpandWithEnv_LiteralDollar(t *testing.T) { func TestExpandWithEnv_PartiallyPresent(t *testing.T) { env := map[string]string{"SET": "yes"} result := expandWithEnv("${SET} and ${NOT_SET}", env) - // ${SET} resolved; ${NOT_SET} -> "" via empty fallback. - assert.Equal(t, "yes and ", result) + assert.Equal(t, "yes and ${NOT_SET}", result) +} + +func TestExpandWithEnv_EmbeddedMissingProcessEnvStaysLiteral(t *testing.T) { + t.Setenv("MOL_TEST_EMBEDDED_MISSING", "") + + result := expandWithEnv("prefix/${MOL_TEST_EMBEDDED_MISSING}/suffix", map[string]string{}) + assert.Equal(t, "prefix/${MOL_TEST_EMBEDDED_MISSING}/suffix", result) } // POSIX identifier guard regression tests (CWE-78 fix). @@ -576,8 +582,8 @@ func TestRenderCategoryRoutingYAML_SingleCategory(t *testing.T) { func TestRenderCategoryRoutingYAML_MultipleCategoriesSorted(t *testing.T) { routing := map[string][]string{ - "zebra": {"RoleZ"}, - "alpha": {"RoleA"}, + "zebra": {"RoleZ"}, + "alpha": {"RoleA"}, "middleware": {"RoleM"}, } result, err := renderCategoryRoutingYAML(routing)