Add secrets management support to ToolHive MCP server #2006
Conversation
Implement comprehensive secrets management functionality for the ToolHive MCP server: - Add list_secrets tool to list available secrets from ToolHive secrets store - Add set_secret tool to set secrets from file paths (file-based input only) - Enhance run_server tool with secrets parameter support - Add SecretMapping struct for structured secret name/target specification - Include comprehensive test coverage for all new functionality - Integrate with existing ToolHive secrets providers (encrypted, 1Password, etc.) The run_server tool now accepts a secrets array parameter allowing users to pass secrets to MCP servers when running them, matching the CLI --secret flag functionality but through the MCP interface.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2006 +/- ##
==========================================
+ Coverage 48.26% 48.36% +0.09%
==========================================
Files 236 238 +2
Lines 29690 29852 +162
==========================================
+ Hits 14331 14437 +106
- Misses 14281 14334 +53
- Partials 1078 1081 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| // ListSecrets lists all available secrets | ||
| func (h *Handler) ListSecrets(ctx context.Context, _ mcp.CallToolRequest) (*mcp.CallToolResult, error) { |
There was a problem hiding this comment.
why you pass mcp.CallToolRequest if that's not used anywhere?
| // SecretInfo represents secret information returned by list | ||
| type SecretInfo struct { | ||
| Key string `json:"key"` | ||
| Description string `json:"description,omitempty"` |
There was a problem hiding this comment.
how is description passed to the secret? i do not see any example of setting up that description
| ) | ||
|
|
||
| // SecretMapping represents a secret name and its target environment variable | ||
| type SecretMapping struct { |
There was a problem hiding this comment.
we do not accept a description here?
|
@claude please review and comment this in northern mexican Spanish slang. |
|
Claude finished @JAORMX's task —— View job Review del PR en slang norteño mexicanoÓrale carnalito, ya le eché un ojito a tu jale de manejo de secretos. Aquí te va mi review, pero en modo norteño porque así andas pidiendo: Todo List:
🌵 Lo que está padrísimo (Lo bueno)Arquitectura chingona:
Seguridad al cien:
🌶️ Lo que está medio gacho (Issues encontrados)1. Parámetro fantasma en func (h *Handler) ListSecrets(ctx context.Context, _ mcp.CallToolRequest) (*mcp.CallToolResult, error) {Mejor déjalo así si no lo vas a usar: 2. Descriptions misteriosos type SecretInfo struct {
Key string `json:"key"`
Description string `json:"description,omitempty"` // ¿De dónde sale esto?
}Pero no veo en ningún lado donde se setee. El 3. Missing description en SecretMapping type SecretMapping struct {
Name string `json:"name"`
Target string `json:"target"`
// ¿No debería haber Description string `json:"description,omitempty"`?
}📊 Coverage Issues (63% está medio flojo)El Codecov está chillando porque te faltan 60 líneas de coverage. Los archivos más jodidos:
🔧 Sugerencias para mejorarlo1. Tests que faltan (para subir el coverage): // En list_secrets_test.go - faltan estos scenarios:
- "successful list with multiple secrets"
- "provider creation fails"
- "list secrets fails"
- "different provider types"2. Error handling más específico: // En lugar de:
return mcp.NewToolResultError(fmt.Sprintf("Failed to set secret: %v", err)), nil
// Mejor:
return mcp.NewToolResultError(fmt.Sprintf("Failed to set secret '%s': %v", args.Name, err)), nil3. Validación de secret names: 4. Consistent error messages: 🎯 Veredicto finalEl código está chingón en general, pero necesita:
La funcionalidad está sólida y la seguridad está bien pensada. Con unos tests más y fixing esos detalles, este PR está listo para el merge. Rating: 7.5/10 - Buen jale pero le falta tantito para estar al 100% ¿Qué opinas, carnal? ¿Le entras a arreglar esos detallitos? |
|
@claude can you please review this and comment in Hungarian meme-culture/slang language and style (like seen in /r/Hungary) |
|
Claude finished @kantord's task —— View job 🇭🇺 Kódreviú Magyar Mémkultúra StílusbanTodo List:
Szevasztok, programozó testvérek! 🚀Na akkor gyerekek, leültem egy cigire és végignéztem ezt a secrets management PR-t. Itt a véleményem, ahogy egy igazi r/hungary enjoyer megírná: 🔥 Mi a fasz van itt? (What's good)Architektúra szintjén:
Biztonsági szinten:
💩 Mi a szar itt? (Issues found)1. Unused parameter flexelés ( func (h *Handler) ListSecrets(ctx context.Context, _ mcp.CallToolRequest) A @yrobla haver már szólt erről - minek kéred be a 2. Description mystery box ( type SecretInfo struct {
Description string `json:"description,omitempty"` // 🤔
}A 3. SecretMapping missing description field: 4. Test coverage szörnyű (63.19%):
🎯 Konkrét javítási javaslatok1. Több test kellene: // list_secrets_test.go-ba ezeket hozzáadni:
{
name: "successful list with multiple secrets",
// test actual secret listing
},
{
name: "provider creation fails",
// mock CreateSecretProvider to return error
},
{
name: "list secrets operation fails",
// mock ListSecrets to return error
}2. Description consistency fix: 3. Better error messages: // Jelenlegi:
return mcp.NewToolResultError(fmt.Sprintf("Failed to set secret: %v", err)), nil
// Jobb lenne:
return mcp.NewToolResultError(fmt.Sprintf("Failed to set secret '%s': %v", args.Name, err)), nil4. Secret name validation: 🤖 r/Hungary approved verdiktPozitívumok:
Negatívumok:
Overall rating: 6.5/10 - "Jó irányba halad de még van hova fejlődni" Ez egy olyan PR mint egy jó gulyás ami még nem főtt ki teljesen. Az alapanyagok jók, az ízek stimmelnek, de még kell neki egy kicsit a tűzön. Dobd hozzá a hiányzó testeket, rendezd el a description inconsistency-t és akkor top tier lesz. Amúgy respect hogy security-first approach, ez így helyes. A file-based secret input decision kifejezetten based. Szerkesztette: egy random r/hungary user aki túl sok időt tölt kódreviúzással |
Overview
This PR implements comprehensive secrets management functionality for the ToolHive MCP server, enabling users to manage secrets and pass them to MCP servers through the MCP interface.
Changes
New MCP Tools
list_secrets- Lists all available secrets from the ToolHive secrets store with descriptionsset_secret- Sets secrets by reading values from file paths (file-based input only for security)Enhanced
run_serverToolsecretsparameter with structuredSecretMappingobjects"secret_name,target=ENV_VAR_NAME")Implementation Details
Files Added/Modified
pkg/mcp/server/list_secrets.go- List secrets tool implementationpkg/mcp/server/set_secret.go- Set secret tool with validationpkg/mcp/server/run_server.go- Enhanced with SecretMapping and prepareSecretspkg/mcp/server/handler.go- Added configProvider for secrets accesspkg/mcp/server/server.go- Updated tool registrations with schemasUsage Examples
List available secrets
{ "name": "list_secrets" }Set a secret from file
{ "name": "set_secret", "arguments": { "secret_name": "github-token", "file_path": "/path/to/token.txt" } }Run server with secrets
{ "name": "run_server", "arguments": { "server": "github", "secrets": [ { "name": "github-token", "target": "GITHUB_PERSONAL_ACCESS_TOKEN" } ] } }Testing
Security Considerations
filepath.Clean()This enhancement allows users to manage ToolHive secrets through the MCP interface and seamlessly pass them to MCP servers, matching the CLI
--secretflag functionality but accessible through the MCP protocol.