Skip to content

Commit f01ff35

Browse files
fix(httpapi): percent-decode the secret name on DELETE /secrets/{name}
Secret names may contain any character but '}', so ${keyring:openai:prod} is a valid reference. The Web UI builds the delete URL with encodeURIComponent and chi routes on RawPath, so {name} arrived as openai%3Aprod and the keyring delete targeted an entry that does not exist, leaving the real secret in place. Decode it with decodePathParam like the other path-param handlers. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent c82d515 commit f01ff35

2 files changed

Lines changed: 50 additions & 1 deletion

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package httpapi
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
"go.uber.org/zap/zaptest"
12+
13+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/secret"
14+
)
15+
16+
// recordingKeyring is a keyring provider that records deleted names.
17+
type recordingKeyring struct{ deleted []string }
18+
19+
func (p *recordingKeyring) CanResolve(t string) bool { return t == secretTypeKeyring }
20+
func (p *recordingKeyring) Resolve(context.Context, secret.Ref) (string, error) {
21+
return "", nil
22+
}
23+
func (p *recordingKeyring) Store(context.Context, secret.Ref, string) error { return nil }
24+
func (p *recordingKeyring) Delete(_ context.Context, ref secret.Ref) error {
25+
p.deleted = append(p.deleted, ref.Name)
26+
return nil
27+
}
28+
func (p *recordingKeyring) List(context.Context) ([]secret.Ref, error) { return nil, nil }
29+
func (p *recordingKeyring) IsAvailable() bool { return true }
30+
31+
// Secret names may contain any character but '}' (${keyring:openai:prod} is a
32+
// valid reference). The Web UI encodes the name with encodeURIComponent and chi
33+
// routes on RawPath, so {name} arrives encoded and must be decoded before the
34+
// keyring delete, otherwise the wrong (nonexistent) entry is targeted.
35+
func TestDeleteSecret_DecodesNameParam(t *testing.T) {
36+
kr := &recordingKeyring{}
37+
res := secret.NewResolver()
38+
res.RegisterProvider(secretTypeKeyring, kr)
39+
ctrl := &secretResolverController{MockServerController: &MockServerController{}, resolver: res}
40+
srv := NewServer(ctrl, zaptest.NewLogger(t).Sugar(), nil)
41+
42+
req := httptest.NewRequest(http.MethodDelete, "/api/v1/secrets/openai%3Aprod?type=keyring", http.NoBody)
43+
req.Header.Set("X-API-Key", mockControllerAPIKey)
44+
w := httptest.NewRecorder()
45+
srv.ServeHTTP(w, req)
46+
47+
require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
48+
assert.Equal(t, []string{"openai:prod"}, kr.deleted)
49+
}

‎internal/httpapi/server.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5059,7 +5059,7 @@ func (s *Server) handleDeleteSecret(w http.ResponseWriter, r *http.Request) {
50595059
return
50605060
}
50615061

5062-
name := chi.URLParam(r, "name")
5062+
name := decodePathParam(chi.URLParam(r, "name"))
50635063
if name == "" {
50645064
s.writeError(w, r, http.StatusBadRequest, "Secret name is required")
50655065
return

0 commit comments

Comments
 (0)