Repository navigation
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
f01ff35 to
62fb6f4
Compare
Review finding (P2): decodePathParam ran unconditionally, so a name whose
only escape is a literal %XX (e.g. "a%41", sent as "a%2541") left RawPath
empty, chi matched the already-decoded Path, and the second decode deleted
a different keyring entry ("aA"). Decode only when RawPath is set, and add
a regression test covering literal-percent names.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Verified the bug on main and the fix; thank you! The review found one gap, so I pushed a small maintainer commit on top of your branch: decoding unconditionally decodes twice when Go leaves I will merge once CI is green. |
Dumbris
left a comment
Dumbris
left a comment
There was a problem hiding this comment.
Verified: the bug reproduces on main and the PR fixes it. The review found one gap, closed by the maintainer commit on top (regression test included). Race tests and CI lint for the touched packages are clean on this branch merged with main, and the result was cross-reviewed clean.
|
Merged. Thank you, @rudycelekli! Deleting a secret whose name the Web UI percent-encodes (like Contributions like this are what keep MCPProxy moving, and they matter a lot to the project. One small ask, only if you feel like it: a short public note about your change (a tweet, a LinkedIn post, a line in a community you are part of) helps more people discover MCPProxy. No pressure at all, and I would be glad to repost it. |
Problem
Secret names aren't restricted. The reference parser accepts anything but
}after the type, so${keyring:openai:prod}is a valid reference andPOST /api/v1/secretsstores it as-is. The Web UI deletes with/api/v1/secrets/${encodeURIComponent(name)}, which sendsopenai%3Aprod. chi routes onRawPath, sochi.URLParam(r, "name")returns the encoded value andhandleDeleteSecretasks the keyring to deleteopenai%3Aprod, an entry that doesn't exist. The real secret stays in the keyring.Fix
Decode
{name}with the existingdecodePathParam, matching the other path-param handlers.Test
TestDeleteSecret_DecodesNameParamregisters a recording keyring provider and sendsDELETE /api/v1/secrets/openai%3Aprod. On main the provider receivesopenai%3Aprod; with this change it receivesopenai:prod.go test -count=1 ./internal/httpapi/passes.Made with Cursor