fix: handle url encoded file name in url params for single file serving endpoint

transports/bifrost-http/handlers/plugins.go

@@ -561,36 +561,71 @@ func (h *PluginsHandler) deletePlugin(ctx *fasthttp.RequestCtx) {
 	})
 }
 
-// restoreRedactedFromExisting walks the incoming config map and, for any field that
-// looks like an EnvVar object whose value ShouldPreserveStored (i.e. it is a redacted
-// placeholder like "***"), replaces the value with the corresponding field from the
-// existing DB config. This mirrors the mergeUpdatedKey pattern used by provider keys.
+// restoreRedactedFromExisting walks the incoming config map and, for any field whose
+// value is a redacted placeholder (a masked EnvVar object, or a masked plain string),
+// replaces it with the corresponding value from the existing DB
+// config so client-side redaction never overwrites real credentials. It descends into
+// nested maps AND slices (e.g. the OTEL `profiles` array), and handles header values that
+// are stored as plain strings rather than EnvVar objects. Mirrors the mergeUpdatedKey
+// pattern used by provider keys.
 func restoreRedactedFromExisting(incoming, existing map[string]any) map[string]any {
 	if len(incoming) == 0 {
 		return incoming
 	}
 	result := make(map[string]any, len(incoming))
 	for k, v := range incoming {
-		switch val := v.(type) {
-		case map[string]any:
-			if isEnvVarObject(val) {
-				ev := schemas.NewEnvVar(marshalEnvVarObject(val))
-				if ev.ShouldPreserveStored() {
-					if existingVal, ok := existing[k]; ok {
-						result[k] = existingVal
-						continue
-					}
-				}
-			} else if existingNested, ok := existing[k].(map[string]any); ok {
-				result[k] = restoreRedactedFromExisting(val, existingNested)
-				continue
+		result[k] = restoreRedactedValue(v, existing[k])
+	}
+	return result
+}
+
+// restoreRedactedValue restores a single incoming value against its corresponding existing
+// value. It recurses through maps and slices, and treats both EnvVar-shaped objects and
+// plain redacted strings as placeholders to swap back to the stored original. Returns the
+// incoming value unchanged when it is not a redaction placeholder or has no stored match.
+func restoreRedactedValue(incoming, existing any) any {
+	switch val := incoming.(type) {
+	case map[string]any:
+		if isEnvVarObject(val) {
+			if schemas.NewEnvVar(marshalEnvVarObject(val)).ShouldPreserveStored() && existing != nil {
+				return existing
 			}
-			result[k] = val
-		default:
-			result[k] = v
+			return val
+		}
+		if existingNested, ok := existing.(map[string]any); ok {
+			return restoreRedactedFromExisting(val, existingNested)
+		}
+		return val
+	case []any:
+		// Restore element-by-element against the existing slice (index-aligned). New
+		// elements beyond the existing length carry user-supplied values, so keep them.
+		existingSlice, ok := existing.([]any)
+		if !ok {
+			return val
 		}
+		out := make([]any, len(val))
+		for i, item := range val {
+			if i < len(existingSlice) {
+				out[i] = restoreRedactedValue(item, existingSlice[i])
+			} else {
+				out[i] = item
+			}
+		}
+		return out
+	case string:
+		// Plain-string secrets (e.g. OTEL headers): restore only when the incoming string
+		// is a redaction artifact and not an intentional env reference. Empty strings are
+		// left as-is so clearing a value works.
+		if existingStr, ok := existing.(string); ok {
+			envVal := schemas.NewEnvVar(val)
+			if !envVal.IsFromEnv() && envVal.IsRedacted() {
+				return existingStr
+			}
+		}
+		return val
+	default:
+		return incoming
 	}
-	return result
 }
 
 // isEnvVarObject returns true if m has exactly the shape of a serialised EnvVar:

transports/bifrost-http/handlers/plugins_test.go

@@ -77,6 +77,69 @@ func buildUpdateRequest(t *testing.T, body any) *fasthttp.RequestCtx {
 // config over the existing DB config, preserving fields the caller did not send.
 // This is critical for the plugin_span_filter field: the OTEL config form in the
 // UI does not send plugin_span_filter, so it must survive a save without being wiped.
+// TestRestoreRedacted_OTELProfilesHeaders covers the two gaps that broke OTEL header
+// round-trips after the multi-profile change: (1) headers live inside the `profiles`
+// array (slice traversal), and (2) header values are plain redacted strings, not EnvVar
+// objects. Saving a config whose headers came back redacted must not overwrite the
+// stored credentials.
+func TestRestoreRedacted_OTELProfilesHeaders(t *testing.T) {
+	realAuth := "Basic-REAL-SUPER-SECRET-VALUE"
+	realVersion := "4"
+	maskedAuth := schemas.NewEnvVar(realAuth).Redacted().GetValue()       // long -> first4 + **** + last4
+	maskedVersion := schemas.NewEnvVar(realVersion).Redacted().GetValue() // "4" -> "*"
+
+	mkConfig := func(auth, version string) map[string]any {
+		return map[string]any{
+			"profiles": []any{
+				map[string]any{
+					"service_name": "langfuse",
+					"headers": map[string]any{
+						"Authorization":                auth,
+						"x-langfuse-ingestion-version": version,
+					},
+				},
+			},
+		}
+	}
+
+	existing := mkConfig(realAuth, realVersion)
+	incoming := mkConfig(maskedAuth, maskedVersion) // what the UI sends back after a redacted GET
+
+	got := restoreRedactedFromExisting(incoming, existing)
+	headers := got["profiles"].([]any)[0].(map[string]any)["headers"].(map[string]any)
+
+	if headers["Authorization"] != realAuth {
+		t.Errorf("Authorization not restored: got %q, want %q", headers["Authorization"], realAuth)
+	}
+	if headers["x-langfuse-ingestion-version"] != realVersion {
+		t.Errorf("version not restored: got %q, want %q", headers["x-langfuse-ingestion-version"], realVersion)
+	}
+
+	// A genuinely changed (non-redacted) header value must pass through untouched.
+	changed := mkConfig("Basic-A-BRAND-NEW-KEY-VALUE-1234", "3")
+	got2 := restoreRedactedFromExisting(changed, existing)
+	headers2 := got2["profiles"].([]any)[0].(map[string]any)["headers"].(map[string]any)
+	if headers2["Authorization"] != "Basic-A-BRAND-NEW-KEY-VALUE-1234" {
+		t.Errorf("new Authorization should pass through, got %q", headers2["Authorization"])
+	}
+	if headers2["x-langfuse-ingestion-version"] != "3" {
+		t.Errorf("new version should pass through, got %q", headers2["x-langfuse-ingestion-version"])
+	}
+
+	// An intentional env.* reference (e.g. credential rotation) must pass through.
+	// NewEnvVar parses the "env." prefix as FromEnv=true, which IsRedacted reports as
+	// redacted; the IsFromEnv guard must let it through rather than restoring the stored value.
+	rotated := mkConfig("env.NEW_TOKEN", "env.NEW_VERSION")
+	got3 := restoreRedactedFromExisting(rotated, existing)
+	headers3 := got3["profiles"].([]any)[0].(map[string]any)["headers"].(map[string]any)
+	if headers3["Authorization"] != "env.NEW_TOKEN" {
+		t.Errorf("env.* Authorization should pass through, got %q", headers3["Authorization"])
+	}
+	if headers3["x-langfuse-ingestion-version"] != "env.NEW_VERSION" {
+		t.Errorf("env.* version should pass through, got %q", headers3["x-langfuse-ingestion-version"])
+	}
+}
+
 func TestUpdatePlugin_ConfigMerge(t *testing.T) {
 	SetLogger(&mockLogger{})
 

transports/bifrost-http/handlers/skills_serving.go

@@ -1053,12 +1053,8 @@ func (h *SkillsServingHandler) doServeFileContent(ctx *fasthttp.RequestCtx) {
 		return
 	}
 
-	filePath := ""
-	if val := ctx.UserValue("filepath"); val != nil {
-		filePath, _ = val.(string)
-	}
-	if filePath == "" {
-		SendError(ctx, fasthttp.StatusBadRequest, "file path is required")
+	filePath, ok := decodeStringPathParam(ctx, "filepath", "file path")
+	if !ok {
 		return
 	}
 
@@ -1357,14 +1353,8 @@ func buildSkillFilePath(skillName string, file *tables.TableSkillFile) string {
 
 // lookupSkillByPathParam extracts the skill-name path parameter and fetches the skill.
 func (h *SkillsServingHandler) lookupSkillByPathParam(ctx *fasthttp.RequestCtx) (*tables.TableSkill, bool) {
-	val := ctx.UserValue("skill-name")
-	if val == nil {
-		SendError(ctx, fasthttp.StatusBadRequest, "skill name is required")
-		return nil, false
-	}
-	name, ok := val.(string)
-	if !ok || name == "" {
-		SendError(ctx, fasthttp.StatusBadRequest, "invalid skill name")
+	name, ok := decodeStringPathParam(ctx, "skill-name", "skill name")
+	if !ok {
 		return nil, false
 	}
 
@@ -1381,6 +1371,25 @@ func (h *SkillsServingHandler) lookupSkillByPathParam(ctx *fasthttp.RequestCtx)
 	return skill, true
 }
 
+func decodeStringPathParam(ctx *fasthttp.RequestCtx, paramName, displayName string) (string, bool) {
+	val := ctx.UserValue(paramName)
+	if val == nil {
+		SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("%s is required", displayName))
+		return "", false
+	}
+	raw, ok := val.(string)
+	if !ok || raw == "" {
+		SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("invalid %s", displayName))
+		return "", false
+	}
+	decoded, err := url.PathUnescape(raw)
+	if err != nil || decoded == "" {
+		SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("invalid %s", displayName))
+		return "", false
+	}
+	return decoded, true
+}
+
 // listAllSkills fetches all skills for marketplace generation.
 func (h *SkillsServingHandler) listAllSkills(ctx *fasthttp.RequestCtx) ([]tables.TableSkill, error) {
 	// Use a large limit to get all skills for the marketplace catalog

transports/bifrost-http/handlers/skills_serving_test.go

@@ -0,0 +1,75 @@
+package handlers
+
+import (
+	"context"
+	"net"
+	"testing"
+	"time"
+
+	"github.com/fasthttp/router"
+	"github.com/maximhq/bifrost/framework/configstore/tables"
+	"github.com/valyala/fasthttp"
+	"github.com/valyala/fasthttp/fasthttputil"
+)
+
+func TestSkillsServingGenericFileDownloadDecodesEncodedPathParams(t *testing.T) {
+	ctx := context.Background()
+	store := newTestConfigStore(t)
+	blobID := "encoded-file-blob"
+	content := []byte("encoded file content")
+
+	if err := store.CreateSkillFileBlob(ctx, &tables.TableSkillFileBlob{ID: blobID, Data: content}); err != nil {
+		t.Fatalf("create blob: %v", err)
+	}
+	if err := store.CreateSkill(ctx, &tables.TableSkill{
+		Name:        "encoded-file-skill",
+		Description: "skill with encoded file paths",
+		SkillMDBody: "body",
+		Files: []tables.TableSkillFile{{
+			Path:          "nested dir/file with spaces.txt",
+			SourceType:    tables.SkillSourceTypeText,
+			BlobID:        &blobID,
+			MimeType:      "text/plain",
+			FileSizeBytes: int64(len(content)),
+		}},
+	}, "1.0.0", nil); err != nil {
+		t.Fatalf("create skill: %v", err)
+	}
+
+	handler := NewSkillsServingHandler(store, nil)
+	r := router.New()
+	handler.RegisterRoutes(r)
+
+	server := &fasthttp.Server{Handler: r.Handler}
+	ln := fasthttputil.NewInmemoryListener()
+	go server.Serve(ln) //nolint:errcheck
+	defer ln.Close()
+	defer server.Shutdown()
+
+	client := &fasthttp.Client{
+		Dial: func(addr string) (net.Conn, error) {
+			return ln.Dial()
+		},
+		ReadTimeout:  5 * time.Second,
+		WriteTimeout: 5 * time.Second,
+	}
+
+	req := fasthttp.AcquireRequest()
+	resp := fasthttp.AcquireResponse()
+	defer fasthttp.ReleaseRequest(req)
+	defer fasthttp.ReleaseResponse(resp)
+
+	req.Header.SetMethod(fasthttp.MethodGet)
+	req.SetRequestURI("http://test.local/api/skills/serve/encoded-file-skill/files/nested%20dir/file%20with%20spaces.txt")
+
+	if err := client.Do(req, resp); err != nil {
+		t.Fatalf("request failed: %v", err)
+	}
+
+	if resp.StatusCode() != fasthttp.StatusOK {
+		t.Fatalf("status got %d, want %d; body=%s", resp.StatusCode(), fasthttp.StatusOK, string(resp.Body()))
+	}
+	if got := string(resp.Body()); got != string(content) {
+		t.Fatalf("body got %q, want %q", got, string(content))
+	}
+}