chore: replace em dashes with hyphens, add no-em-dash rule to CLAUDE.md
This commit is contained in:
@@ -35,7 +35,7 @@ var ErrConfirmRequired = errors.New("confirmation required")
|
||||
var ErrOutOfScope = errors.New("targets outside token scope")
|
||||
|
||||
// logEvent is services.LogEvent behind a package variable so tests can
|
||||
// observe what would have been audited without a live database connection —
|
||||
// observe what would have been audited without a live database connection -
|
||||
// services.LogEvent talks straight to Mongo via db.Col, which panics on a nil
|
||||
// client outside a real boot.
|
||||
var logEvent = services.LogEvent
|
||||
@@ -117,7 +117,7 @@ func LogDenied(c Caller, toolName, gate string) {
|
||||
}
|
||||
|
||||
// LogFailure records a write tool call that reached a service and that
|
||||
// service returned an error — as opposed to LogDenied, which records a
|
||||
// service returned an error - as opposed to LogDenied, which records a
|
||||
// policy refusal that never reached one. Distinguishing the two in
|
||||
// audit_logs is what lets a human reading it tell "the agent was stopped"
|
||||
// from "the agent tried and the machine failed".
|
||||
@@ -130,7 +130,7 @@ func LogFailure(c Caller, t Tool, args map[string]any, err error) {
|
||||
//
|
||||
// It is a distinct event type rather than another mcp.tool_call row because of
|
||||
// the question a human will actually ask, which is "what has this agent added
|
||||
// to my instance" — an answer buried among hundreds of read rows is not an
|
||||
// to my instance" - an answer buried among hundreds of read rows is not an
|
||||
// answer.
|
||||
func LogCreated(c Caller, kind, id, name string) {
|
||||
logEvent(c.InstanceID, "mcp.created", c.TokenName, "", "",
|
||||
|
||||
@@ -3,8 +3,8 @@
|
||||
// It is a presentation layer over the service layer and introduces no authority
|
||||
// of its own: every tool calls the same service functions the REST handlers
|
||||
// call, and every decision about who may do what is made by machinery that
|
||||
// already exists. Three gates apply to every call — the licence feature, the
|
||||
// mcp:* scope, and the tool's own resource scope — and all three must pass.
|
||||
// already exists. Three gates apply to every call - the licence feature, the
|
||||
// mcp:* scope, and the tool's own resource scope - and all three must pass.
|
||||
package mcp
|
||||
|
||||
import (
|
||||
@@ -50,7 +50,7 @@ const (
|
||||
// what tagArg decodes.
|
||||
ArgTagMap ArgType = "tag_map"
|
||||
// ArgObject is a free-form object whose inner shape the tool documents in
|
||||
// the argument description — create_monitor's target, whose fields differ
|
||||
// the argument description - create_monitor's target, whose fields differ
|
||||
// per monitor type.
|
||||
ArgObject ArgType = "object"
|
||||
)
|
||||
@@ -83,7 +83,7 @@ type Tool struct {
|
||||
Write bool
|
||||
// Args declares every argument the handler reads, in the order a client
|
||||
// should see them. A tool taking none declares an empty slice, which is
|
||||
// distinct from "nobody has written the schema yet" — see the registry
|
||||
// distinct from "nobody has written the schema yet" - see the registry
|
||||
// tests, which require the declaration to be deliberate.
|
||||
Args []ToolArg
|
||||
// TouchesServers marks a tool that returns or acts on server-derived data:
|
||||
@@ -92,7 +92,7 @@ type Tool struct {
|
||||
// ResolveTargetsScoped, ListServersFiltered or VisibleServerIDs.
|
||||
//
|
||||
// Like serverScopedRoutes in the api package, this can only ever assert
|
||||
// that a declaration exists, never that the handler honours it — get_run_logs
|
||||
// that a declaration exists, never that the handler honours it - get_run_logs
|
||||
// proved the run's instance and the server's membership in the run and then
|
||||
// read production stdout for a staging token. What it does buy is that
|
||||
// adding a tool forces an answer to "does this touch server data?", and the
|
||||
|
||||
@@ -72,8 +72,8 @@ func TestInputSchemaMarshals(t *testing.T) {
|
||||
// serverTouchingTools names every tool that returns or acts on server-derived
|
||||
// data. The test below pins the registry against it, so a tool added that
|
||||
// reads a hostname, a server ID, a package list or a run's per-server output
|
||||
// fails until somebody declares TouchesServers and — the point of the exercise
|
||||
// — decides how it applies Caller.TokenScope.
|
||||
// fails until somebody declares TouchesServers and - the point of the exercise
|
||||
// - decides how it applies Caller.TokenScope.
|
||||
//
|
||||
// This is the assertion that would have caught get_run_logs, which proved the
|
||||
// run's instance and the named server's membership in the run and then read
|
||||
|
||||
@@ -141,7 +141,7 @@ func buildMonitor(args map[string]any) (models.Monitor, error) {
|
||||
// named agent, and a silently ignored argument would leave a model
|
||||
// believing it had. services.CreateMonitor now validates a runner
|
||||
// through GetServerScoped as well, so this is a second line rather than
|
||||
// the only one — but the clearer answer belongs here.
|
||||
// the only one - but the clearer answer belongs here.
|
||||
if _, present := args["runner"]; present {
|
||||
return models.Monitor{}, fmt.Errorf("runner cannot be set from here; monitors created this way always run on the control plane")
|
||||
}
|
||||
@@ -176,7 +176,7 @@ func init() {
|
||||
Write: true,
|
||||
Scope: "workflows:write",
|
||||
Description: "Create a reusable workflow step: a named script with an interpreter. " +
|
||||
"The step is SAVED to this Vantage instance but is not run by creating it — " +
|
||||
"The step is SAVED to this Vantage instance but is not run by creating it - " +
|
||||
"add it to a workflow with create_workflow, then run that with run_workflow. " +
|
||||
"Steps created this way cannot reference secrets.",
|
||||
Handler: func(ctx context.Context, c Caller, args map[string]any) (any, error) {
|
||||
|
||||
@@ -16,7 +16,7 @@ import (
|
||||
// Runner in particular is not merely omitted as noise: for an agent-pushed
|
||||
// monitor it is literally a server ID, and REST's listMonitors/getMonitor
|
||||
// redact it to models.RunnerRestricted when that server is outside the
|
||||
// caller's scope. This projection never had a runner field to redact — the
|
||||
// caller's scope. This projection never had a runner field to redact - the
|
||||
// same outcome, reached by never including it rather than by filtering it
|
||||
// out, so this tool and get_monitor_status cannot disagree with the REST
|
||||
// surface about what a restricted token learns.
|
||||
|
||||
@@ -60,7 +60,7 @@ func TestServerSummaryStaysSmall(t *testing.T) {
|
||||
}
|
||||
|
||||
// The real status vocabulary is "pending" / "active" / "offline" (see
|
||||
// internal/services/servers.go) — "online" is never assigned anywhere. A
|
||||
// internal/services/servers.go) - "online" is never assigned anywhere. A
|
||||
// server carrying the live status ("active") must project as Online: true,
|
||||
// or list_servers/get_server misreport the entire fleet as down.
|
||||
func TestSummariseServerReportsActiveAsOnline(t *testing.T) {
|
||||
|
||||
@@ -39,7 +39,7 @@ type workflowDetail struct {
|
||||
Tags map[string]string `json:"target_tags,omitempty"`
|
||||
Schedule string `json:"schedule,omitempty"`
|
||||
// TargetsRestricted is set, with no count, when Targets omits at least
|
||||
// one server ID outside this token's scope — mirroring
|
||||
// one server ID outside this token's scope - mirroring
|
||||
// WorkflowResponse's REST field, so a model reading this alongside a
|
||||
// run_workflow refusal for the same workflow is not left to conclude the
|
||||
// refusal invented a problem this tool never mentioned.
|
||||
@@ -341,7 +341,7 @@ func init() {
|
||||
//
|
||||
// The refusal reuses the membership message verbatim so that
|
||||
// "in the run but out of your scope" and "not in the run at all"
|
||||
// are indistinguishable — otherwise the difference between the
|
||||
// are indistinguishable - otherwise the difference between the
|
||||
// two answers enumerates hosts the token cannot see.
|
||||
if _, err := services.GetServerScoped(c.InstanceID, serverID, c.TokenScope); err != nil {
|
||||
return nil, fmt.Errorf("server %q is not part of run %q", serverID, runID)
|
||||
@@ -461,7 +461,7 @@ func init() {
|
||||
}
|
||||
|
||||
// Go randomises map iteration order, so truncating a ranged map
|
||||
// to a page made two identical calls return different CVEs — a
|
||||
// to a page made two identical calls return different CVEs - a
|
||||
// model comparing its own two answers would see the fleet change
|
||||
// under it. Sorting by CVE ID (then package, since the key is a
|
||||
// pair) makes the page deterministic.
|
||||
@@ -541,7 +541,7 @@ func init() {
|
||||
},
|
||||
TouchesServers: true,
|
||||
Scope: "vulns:read",
|
||||
Description: "Search every server's installed packages by name across the whole fleet — " +
|
||||
Description: "Search every server's installed packages by name across the whole fleet - " +
|
||||
"answers questions like \"which hosts still run OpenSSL 1.1\". version_below is not " +
|
||||
"currently supported: filtering package versions correctly requires knowing each " +
|
||||
"distribution's own version-ordering scheme (dpkg/rpm/apk), which this tool cannot " +
|
||||
@@ -555,7 +555,7 @@ func init() {
|
||||
if stringArg(args, "version_below") != "" {
|
||||
return nil, fmt.Errorf("version_below is not supported: correct version ordering " +
|
||||
"depends on each host's distribution (dpkg/rpm/apk each order differently), " +
|
||||
"which this tool cannot resolve here — omit version_below and every matching " +
|
||||
"which this tool cannot resolve here - omit version_below and every matching " +
|
||||
"install is returned instead")
|
||||
}
|
||||
|
||||
@@ -616,7 +616,7 @@ func init() {
|
||||
// This tool reads no arguments at all.
|
||||
Args: []ToolArg{},
|
||||
Scope: "secrets:read",
|
||||
Description: "List secret group and key names on this instance. Metadata only — no " +
|
||||
Description: "List secret group and key names on this instance. Metadata only - no " +
|
||||
"tool ever returns a secret's plaintext value to a model.",
|
||||
Handler: func(ctx context.Context, c Caller, args map[string]any) (any, error) {
|
||||
groups, err := services.ListSecretGroups(c.InstanceID)
|
||||
|
||||
@@ -43,8 +43,8 @@ type runStartedResult struct {
|
||||
// workflow's own configured target_server_ids/target_tags via
|
||||
// services.ResolveTargets (unscoped) and runs against exactly that set. There
|
||||
// is no per-call server_ids/tags override to plumb through, so this tool takes
|
||||
// only workflow_id. To keep the token's scope meaningful — TriggerWorkflow
|
||||
// itself does not consult it — this handler first loads the workflow and
|
||||
// only workflow_id. To keep the token's scope meaningful - TriggerWorkflow
|
||||
// itself does not consult it - this handler first loads the workflow and
|
||||
// resolves its configured targets through ResolveTargetsScoped with the
|
||||
// caller's TokenScope, and refuses the run outright if that scoped view does
|
||||
// not cover every server the unscoped resolution would touch. That is the
|
||||
@@ -61,7 +61,7 @@ func init() {
|
||||
Write: true,
|
||||
Scope: "workflows:write",
|
||||
Description: "Run a workflow against the servers it is already configured to target " +
|
||||
"(its saved server list and tags — this call does not let you pick different " +
|
||||
"(its saved server list and tags - this call does not let you pick different " +
|
||||
"targets). This EXECUTES COMMANDS on real machines and cannot be undone from " +
|
||||
"here. Returns a run ID immediately; poll get_run for progress and get_run_logs " +
|
||||
"for output. Refused if the workflow's targets reach outside this token's own " +
|
||||
@@ -121,7 +121,7 @@ type cancelledResult struct {
|
||||
// cancel_run. The REST cancel route (workflows.go's cancelRun) calls
|
||||
// services.CancelRun(instanceID, runID) directly; that call is already scoped
|
||||
// to the caller's instance by instanceID, which is what "verifies the run
|
||||
// belongs to the caller's instance" reduces to here — there is no separate
|
||||
// belongs to the caller's instance" reduces to here - there is no separate
|
||||
// per-server scope to check, since cancelling touches the run record, not a
|
||||
// server.
|
||||
func init() {
|
||||
@@ -164,7 +164,7 @@ type updateBatchResult struct {
|
||||
// There is no fleet-wide variant of that service call to invoke once, so this
|
||||
// tool resolves the requested targets through ResolveTargetsScoped exactly as
|
||||
// the brief describes, then calls the same DispatchApplyUpdates the REST route
|
||||
// calls, once per resolved server — the identical dispatch, just looped
|
||||
// calls, once per resolved server - the identical dispatch, just looped
|
||||
// instead of hardcoded to one server_id from the URL.
|
||||
func init() {
|
||||
All().Register(Tool{
|
||||
@@ -275,12 +275,12 @@ type assignKeyResult struct {
|
||||
|
||||
// assign_key. The REST route (handlers.go's assignKey) takes one server_id in
|
||||
// the body and calls services.AssignKey(instanceID, keyID, serverID) directly
|
||||
// — AssignKey itself resolves the server with the unscoped services.GetServer,
|
||||
// - AssignKey itself resolves the server with the unscoped services.GetServer,
|
||||
// not GetServerScoped, so the REST route carries no token-scope check of its
|
||||
// own (session auth has no server-scope restriction; only API tokens do). For
|
||||
// the MCP surface, this tool resolves every named target through
|
||||
// ResolveTargetsScoped first — the same chokepoint every other target-
|
||||
// resolving write tool goes through — so a token whose scope excludes a server
|
||||
// ResolveTargetsScoped first - the same chokepoint every other target-
|
||||
// resolving write tool goes through - so a token whose scope excludes a server
|
||||
// cannot reach it here even though the REST handler's own server lookup would
|
||||
// not have stopped it. Then it calls the identical AssignKey once per resolved
|
||||
// server.
|
||||
@@ -298,7 +298,7 @@ func init() {
|
||||
Scope: "keys:write",
|
||||
Description: "Assign an SSH key to real servers, selected by server_ids and/or " +
|
||||
"tags. The agent rewrites /root/.ssh/authorized_keys on each targeted machine " +
|
||||
"and this cannot be undone from here — use revoke to remove it afterward.",
|
||||
"and this cannot be undone from here - use revoke to remove it afterward.",
|
||||
Handler: func(ctx context.Context, c Caller, args map[string]any) (any, error) {
|
||||
keyID := stringArg(args, "key_id")
|
||||
if keyID == "" {
|
||||
|
||||
@@ -31,7 +31,7 @@ func callerFromContext(c *gin.Context) Caller {
|
||||
// because a stateless server has no session to open the server-to-client SSE
|
||||
// stream against. The GET route is still registered deliberately (see
|
||||
// handlers.go) so a client probing for the endpoint sees a protocol-correct
|
||||
// 405 rather than gin's 404 — the MCP spec expects exactly that response from
|
||||
// 405 rather than gin's 404 - the MCP spec expects exactly that response from
|
||||
// a server that does not offer the GET/SSE leg. Nothing here should route GET
|
||||
// requests differently or try to make them do anything else.
|
||||
func Handler() gin.HandlerFunc {
|
||||
@@ -69,7 +69,7 @@ func Handler() gin.HandlerFunc {
|
||||
func registerSDKTool(srv *sdk.Server, tool Tool, caller Caller) {
|
||||
// InputSchema is set explicitly rather than inferred from the handler's
|
||||
// argument type. The SDK can infer one from a typed In parameter, which is
|
||||
// cleaner where it fits — but every ToolFunc here takes map[string]any, and
|
||||
// cleaner where it fits - but every ToolFunc here takes map[string]any, and
|
||||
// inference over that yields a bare open object saying nothing. Giving each
|
||||
// tool its own Go argument struct would mean twenty-odd structs and a
|
||||
// generic registry that could no longer hold them in one map, losing the
|
||||
@@ -86,8 +86,8 @@ func registerSDKTool(srv *sdk.Server, tool Tool, caller Caller) {
|
||||
}
|
||||
|
||||
// callTool is the gate check, dispatch and audit write registerSDKTool wraps
|
||||
// onto the SDK's call signature. It is a separate function — rather than the
|
||||
// closure body inline — so it can be exercised directly in tests without
|
||||
// onto the SDK's call signature. It is a separate function - rather than the
|
||||
// closure body inline - so it can be exercised directly in tests without
|
||||
// standing up an sdk.Server and driving a real MCP request through it.
|
||||
func callTool(ctx context.Context, tool Tool, caller Caller, args map[string]any) (*sdk.CallToolResult, any, error) {
|
||||
if ok, gate := Allowed(tool, caller); !ok {
|
||||
@@ -98,14 +98,14 @@ func callTool(ctx context.Context, tool Tool, caller Caller, args map[string]any
|
||||
out, err := tool.Handler(ctx, caller, args)
|
||||
if err != nil {
|
||||
// A write tool's own handler never gets a chance to audit its own
|
||||
// refusal or failure — it returns before reaching its LogCall, and
|
||||
// refusal or failure - it returns before reaching its LogCall, and
|
||||
// unlike a successful write, this layer does not know a resolved
|
||||
// server count to pass along anyway. So every write failure is
|
||||
// audited here instead: a policy refusal (fan-out or tag scope) as
|
||||
// mcp.tool_denied naming the gate, everything else as
|
||||
// mcp.tool_failed, so a human reading audit_logs can tell "the agent
|
||||
// was stopped" from "the agent tried and the machine failed". Read
|
||||
// tools are unaffected — a failed read was never going to change
|
||||
// tools are unaffected - a failed read was never going to change
|
||||
// anything and carries no gate to name.
|
||||
if tool.Write {
|
||||
switch {
|
||||
|
||||
@@ -7,7 +7,7 @@ import (
|
||||
|
||||
// TestRefusedWriteIsAudited exercises the real dispatch path (callTool, which
|
||||
// registerSDKTool wraps) for a write tool whose handler refuses the call
|
||||
// before it ever reaches its own LogCall — a fan-out refusal, in this case,
|
||||
// before it ever reaches its own LogCall - a fan-out refusal, in this case,
|
||||
// which run_workflow, apply_updates, update_agent and assign_key all reach
|
||||
// the same way via CheckFanOut. The refusal must still produce an audit row:
|
||||
// a blocked mutation attempt is the single most audit-worthy event a write
|
||||
@@ -77,8 +77,8 @@ func TestOutOfScopeWriteIsAudited(t *testing.T) {
|
||||
}
|
||||
|
||||
// TestServiceFailureIsAuditedDistinctly makes sure a write tool failing for a
|
||||
// reason that is not a policy refusal — the underlying service call itself
|
||||
// erroring — is still audited, but as mcp.tool_failed rather than
|
||||
// reason that is not a policy refusal - the underlying service call itself
|
||||
// erroring - is still audited, but as mcp.tool_failed rather than
|
||||
// mcp.tool_denied, so a human reading audit_logs can tell the two apart.
|
||||
func TestServiceFailureIsAuditedDistinctly(t *testing.T) {
|
||||
var events []string
|
||||
|
||||
Reference in New Issue
Block a user