diff --git a/server/internal/mcp/audit.go b/server/internal/mcp/audit.go index 4bfbd58..2dcd031 100644 --- a/server/internal/mcp/audit.go +++ b/server/internal/mcp/audit.go @@ -136,3 +136,10 @@ func LogCreated(c Caller, kind, id, name string) { logEvent(c.InstanceID, "mcp.created", c.TokenName, "", "", fmt.Sprintf("created %s %q (%s)", kind, name, id)) } + +// LogUpdated records a change an agent made to something that already existed, +// as its own event type for the same reason as LogCreated. +func LogUpdated(c Caller, kind, id, name string) { + logEvent(c.InstanceID, "mcp.updated", c.TokenName, "", "", + fmt.Sprintf("updated %s %q (%s)", kind, name, id)) +} diff --git a/server/internal/mcp/tools_create.go b/server/internal/mcp/tools_create.go index 80221e1..2f9d229 100644 --- a/server/internal/mcp/tools_create.go +++ b/server/internal/mcp/tools_create.go @@ -360,22 +360,22 @@ func init() { Scope: "secrets:write", Description: "Add new secret keys to a group, creating the group if needed. " + "Existing keys are never overwritten - the call is refused if any key already " + - "exists, so rotate or replace a secret in the Vantage UI. Values are not returned.", + "exists, so use update_secret to change one. Values are not returned.", Handler: func(ctx context.Context, c Caller, args map[string]any) (any, error) { group, values, err := buildSecrets(args) if err != nil { return nil, err } - // UpsertSecrets overwrites silently. A model replacing a live - // credential would break whatever uses it, so only additions - // are allowed from here. + // UpsertSecrets overwrites silently. Replacing a live credential + // must be a deliberate update_secret call, not a side effect of + // adding a key. existing, err := services.GetSecretGroup(c.InstanceID, group) if err != nil { return nil, fmt.Errorf("could not read secret group: %w", err) } for _, s := range existing { if _, clash := values[s.Key]; clash { - return nil, fmt.Errorf("key %q already exists in group %q; change it in the Vantage UI", s.Key, group) + return nil, fmt.Errorf("key %q already exists in group %q; use update_secret to change it", s.Key, group) } } if err := services.UpsertSecrets(c.InstanceID, group, values); err != nil { @@ -390,4 +390,56 @@ func init() { }, nil }, }) + + All().Register(Tool{ + Name: "update_secret", + Args: []ToolArg{ + {Name: "group", Type: ArgString, Description: "Name of an existing secret group.", Required: true}, + {Name: "values", Type: ArgTagMap, Description: "Existing keys and their new values, encrypted at rest.", Required: true}, + }, + Write: true, + Scope: "secrets:write", + Description: "Replace the values of existing secret keys. Whatever uses these keys gets the " + + "new value from then on. The call is refused, changing nothing, if any key does not " + + "exist - use create_secret to add one. Values are not returned.", + Handler: func(ctx context.Context, c Caller, args map[string]any) (any, error) { + group, values, err := buildSecrets(args) + if err != nil { + return nil, err + } + existing, err := services.GetSecretGroup(c.InstanceID, group) + if err != nil { + return nil, fmt.Errorf("could not read secret group: %w", err) + } + if missing := missingSecretKeys(existing, values); len(missing) > 0 { + return nil, fmt.Errorf("key(s) %s do not exist in group %q; use create_secret to add them", + strings.Join(missing, ", "), group) + } + if err := services.UpsertSecrets(c.InstanceID, group, values); err != nil { + return nil, fmt.Errorf("could not save the secrets: %w", err) + } + keys := services.SortedKeys(values) + LogUpdated(c, "secret", group, strings.Join(keys, ", ")) + return map[string]any{ + "group": group, + "keys": keys, + "note": "Updated. Values are not readable through MCP.", + }, nil + }, + }) +} + +// missingSecretKeys returns the sorted keys in values that existing lacks. +func missingSecretKeys(existing []models.Secret, values map[string]string) []string { + present := make(map[string]bool, len(existing)) + for _, s := range existing { + present[s.Key] = true + } + var missing []string + for _, k := range services.SortedKeys(values) { + if !present[k] { + missing = append(missing, k) + } + } + return missing } diff --git a/server/internal/mcp/tools_create_test.go b/server/internal/mcp/tools_create_test.go index 422bf05..fcfb7cd 100644 --- a/server/internal/mcp/tools_create_test.go +++ b/server/internal/mcp/tools_create_test.go @@ -3,6 +3,8 @@ package mcp import ( "strings" "testing" + + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models" ) func TestCreationToolsAreRegisteredAsWrites(t *testing.T) { @@ -20,10 +22,12 @@ func TestCreationToolsAreRegisteredAsWrites(t *testing.T) { // An agent may add a definition. It may never alter or remove one a human // wrote, and the cheapest guard against that is the tool simply not existing. +// update_secret is the one deliberate exception: it replaces a value, which is +// never readable through MCP, and leaves steps, workflows and monitors alone. func TestNoUpdateOrDeleteTools(t *testing.T) { for _, tool := range All().Tools() { n := tool.Name - if strings.HasPrefix(n, "update_") && n != "update_agent" { + if strings.HasPrefix(n, "update_") && n != "update_agent" && n != "update_secret" { t.Errorf("tool %q edits an existing definition", n) } if strings.HasPrefix(n, "delete_") || strings.HasPrefix(n, "remove_") { @@ -242,3 +246,14 @@ func TestBuildSecretsValidates(t *testing.T) { t.Errorf("valid input rejected: %v", err) } } + +func TestMissingSecretKeys(t *testing.T) { + existing := []models.Secret{{Key: "DB_PASS"}, {Key: "API_KEY"}} + if got := missingSecretKeys(existing, map[string]string{"DB_PASS": "x"}); len(got) != 0 { + t.Errorf("existing key reported missing: %v", got) + } + got := missingSecretKeys(existing, map[string]string{"DB_PASS": "x", "Z": "y", "A": "y"}) + if strings.Join(got, ",") != "A,Z" { + t.Errorf("got %v, want [A Z]", got) + } +}