feat: add new KV tools and improve existing ones#80
Conversation
f7be9c7 to
453853f
Compare
453853f to
a7dfafe
Compare
There was a problem hiding this comment.
Pull request overview
This PR expands the Vault KV toolset (KV v2 version/metadata operations + patch semantics) and refactors some KV mount detection logic via a shared helper, while also changing write_secret to accept a full data object (replacing the prior key/value interface) and removing delete_secret.
Changes:
- Added new KV v2 tools for metadata read/write, patching, and version delete/undelete/destroy operations.
- Refactored KV v1/v2 mount detection in updated tools via a shared
getMountInfo()helper. - Updated existing KV tools (
read_secret,write_secret) and removeddelete_secret, with accompanying schema-focused unit tests.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/tools/tools.go | Registers new KV tools and replaces removed delete_secret registration with version/metadata/patch tools. |
| pkg/tools/kv/helpers.go | Adds shared mount existence + KV v2 detection helper used by updated KV tools. |
| pkg/tools/kv/read_secret.go | Adds optional version parameter (KV v2 only) and uses shared mount detection. |
| pkg/tools/kv/write_secret.go | Breaking change: writes full data object; removes read-before-write merge; uses shared mount detection. |
| pkg/tools/kv/read_secret_metadata.go | New KV v2 tool to read secret metadata. |
| pkg/tools/kv/write_secret_metadata.go | New KV v2 tool to write secret metadata configuration/custom metadata. |
| pkg/tools/kv/patch_secret.go | New KV v2 tool to JSON-merge-patch secret data. |
| pkg/tools/kv/delete_secret_versions.go | New KV v2 tool to soft-delete versions (or latest). |
| pkg/tools/kv/undelete_secret.go | New KV v2 tool to undelete previously soft-deleted versions. |
| pkg/tools/kv/destroy_secret_versions.go | New KV v2 tool to permanently destroy versions. |
| pkg/tools/kv/*_test.go | Adds schema/annotation/unit tests for the new/updated KV tools. |
| README.md | Removes delete_secret documentation section (but other KV tool docs need updating). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| m, ok := mounts[mount+"/"] | ||
| if !ok { | ||
| return false, fmt.Errorf("mount path '%s' does not exist. Use 'create_mount' with the type kv2 to create the mount", mount) |
There was a problem hiding this comment.
getMountInfo is used by tools that support both KV v1 and KV v2 (e.g., write_secret), but the missing-mount error message only suggests creating a kv2 mount. Consider changing the guidance to mention kv or kv2 (or omit the version) so the message is accurate for all callers.
| return false, fmt.Errorf("mount path '%s' does not exist. Use 'create_mount' with the type kv2 to create the mount", mount) | |
| return false, fmt.Errorf("mount path '%s' does not exist. Use 'create_mount' with type 'kv' or 'kv2' to create the mount", mount) |
| }, | ||
| ), | ||
| mcp.WithDescription("Writes a secret value to a KV store in Vault using the specified path and mount. Supports both KV v1 and v2 mounts. If a KV v2 mount is detected, the currently stored version of the secret will be returned."), | ||
| mcp.WithDescription("Writes a secret to a KV store in Vault using the specified path and mount. The data parameter is a complete key-value map that will replace the secret at the given path. Supports both KV v1 and v2 mounts. If a KV v2 mount is detected, the currently stored version of the secret will be returned."), |
There was a problem hiding this comment.
write_secret description says that on KV v2 "the currently stored version of the secret will be returned", but the handler no longer reads the existing secret and the response version (if present) corresponds to the newly written version. Please update the description to avoid misleading users about what version is returned.
| mcp.WithDescription("Writes a secret to a KV store in Vault using the specified path and mount. The data parameter is a complete key-value map that will replace the secret at the given path. Supports both KV v1 and v2 mounts. If a KV v2 mount is detected, the currently stored version of the secret will be returned."), | |
| mcp.WithDescription("Writes a secret to a KV store in Vault using the specified path and mount. The data parameter is a complete key-value map that will replace the secret at the given path. Supports both KV v1 and v2 mounts. On KV v2 mounts, a new version of the secret is written and any version information in the response (if present) corresponds to that newly written version."), |
| "path": path, | ||
| }).Info("Successfully soft-deleted latest secret version") | ||
|
|
||
| return mcp.NewToolResultText(fmt.Sprintf("Successfully soft-deleted the latest version of secret at path '%s' in mount '%s'. Use undelete_secret to recover it.", path, mount)), nil |
There was a problem hiding this comment.
Success message for deleting the latest version references undelete_secret, but the tool exposed by this PR is undelete_secret_versions. Please update the guidance to the correct tool name.
| return mcp.NewToolResultText(fmt.Sprintf("Successfully soft-deleted the latest version of secret at path '%s' in mount '%s'. Use undelete_secret to recover it.", path, mount)), nil | |
| return mcp.NewToolResultText(fmt.Sprintf("Successfully soft-deleted the latest version of secret at path '%s' in mount '%s'. Use undelete_secret_versions to recover it.", path, mount)), nil |
| deleteSecretVersionsTool := kv.DeleteSecretVersions(logger) | ||
| hcServer.AddTool(deleteSecretVersionsTool.Tool, deleteSecretVersionsTool.Handler) | ||
|
|
There was a problem hiding this comment.
PR description says unit tests were added for delete_secret, but this PR removes the delete_secret tool entirely and there’s no corresponding test file. Also, the description claims mount-checking is deduplicated across KV tools via getMountInfo(), but list_secrets still has its own mount-checking logic. Please align the PR description with the actual changes (or update the remaining tool(s) if the description is the intended scope).
| - `path`: (Optional) The path to list secrets from (defaults to root) | ||
|
|
||
| #### delete_secret | ||
| Delete secrets (or keys) in a KV mount under a specific path in Vault. | ||
| - `mount`: The mount path of the secret engine | ||
| - `path`: The path to the secret to delete | ||
| - `key`: (Optional) The key name to delete from the entire secret (defaults to deleting the entire secret) | ||
|
|
||
| #### write_secret | ||
| Writes a secret to a KV mount in Vault. | ||
| - `mount`: The mount path of the secret engine |
There was a problem hiding this comment.
This README section is now inaccurate after the KV tool changes in this PR: write_secret no longer takes key/value and instead requires a data object, read_secret gained an optional version (KV v2 only), and several new KV tools were added/renamed. Please update the “Available Tools” docs to match the current tool schemas/names.
| mcp.WithDescription("Soft-delete specific versions of a secret in a KV v2 mount in Vault. The secret data is marked as deleted but can be recovered using undelete_secret. Only supported on KV v2 mounts."), | ||
| mcp.WithString("mount", |
There was a problem hiding this comment.
Tool description references undelete_secret, but the tool added in this PR is undelete_secret_versions. Please update the wording to reference the correct tool name to avoid directing users to a non-existent tool.
| mcp.WithArray("versions", | ||
| mcp.Description("An array of version numbers to soft-delete. For example: [1, 3, 5]. If not specified, the latest version is soft-deleted. Soft-deleted versions can be recovered with undelete_secret."), | ||
| ), |
There was a problem hiding this comment.
The versions parameter description says soft-deleted versions can be recovered with undelete_secret, but this PR introduces undelete_secret_versions. Update the description to point at the correct recovery tool name.
| return mcp.NewToolResultText(fmt.Sprintf("Successfully soft-deleted versions %v of secret at path '%s' in mount '%s'. Use undelete_secret to recover them.", versions, path, mount)), nil | ||
| } |
There was a problem hiding this comment.
Success message instructs users to run undelete_secret to recover versions, but that tool name doesn’t exist after this PR (it’s undelete_secret_versions). Please update the message so the CLI/agent guidance is actionable.
| } | ||
|
|
||
| func undeleteSecretHandler(ctx context.Context, req mcp.CallToolRequest, logger *log.Logger) (*mcp.CallToolResult, error) { | ||
| logger.Debug("Handling undelete_secret request") |
There was a problem hiding this comment.
Log message still says "Handling undelete_secret request" even though the tool name is undelete_secret_versions. For consistency/debuggability, please update the log text to the actual tool name.
| logger.Debug("Handling undelete_secret request") | |
| logger.Debug("Handling undelete_secret_versions request") |
| } | ||
|
|
||
| if !isV2 { | ||
| return mcp.NewToolResultError("undelete_secret is only supported on KV v2 mounts"), nil |
There was a problem hiding this comment.
Error message says undelete_secret is only supported on KV v2 mounts, but the tool registered/exported is undelete_secret_versions. Please update the message to match the tool name so users know which tool is failing.
| return mcp.NewToolResultError("undelete_secret is only supported on KV v2 mounts"), nil | |
| return mcp.NewToolResultError("undelete_secret_versions is only supported on KV v2 mounts"), nil |
This PR add missing KV tools and improving existing ones.
New tools
read_secret_metadata: Read metadata for a secret from a KV v2 mount (version history, custom metadata, configuration like max_versions, cas_required). Read-only and idempotent.write_secret_metadata: Write metadata configuration for a secret in a KV v2 mount (max_versions, cas_required, delete_version_after, custom_metadata).patch_secret: Merge provided data with existing secret data in a KV v2 mount using JSON merge patch semantics. Only specified keys are updated, unspecified keys are preserved.delete_secret_versions: Soft delete versions of a secret in a KV v2 mount by specifying version numbers to delete.undelete_secret_versions: Restore previously soft-deleted versions of a secret in a KV v2 mount by specifying version numbers to recover.destroy_secret_versions: Permanently destroy specific versions of a secret in a KV v2 mount. Unlike soft-delete, destroyed versions cannot be recovered.Modified tools
read_secret:write_secret:Deleted tools
delete_secret:write_secretshould be used to delete a key in a secret.delete_secret_versionswhich also support to delete a specific versionOther Changes