diff --git a/README.md b/README.md index 585beb3..4697197 100644 --- a/README.md +++ b/README.md @@ -129,16 +129,29 @@ Every other provider's purchase tool (`cudly_aws_savingsplans_purchase`, `cudly_ - Every real purchase is tagged with a source identifying it came from this MCP server (never a user-suppliable string) and a deterministic idempotency token derived from the request's own parameters. By default, retrying an identical tool call -- however long after the original, and regardless of any clock boundary -- always derives the same token, so the provider dedupes the retry instead of buying twice; this is a fail-safe default, since the worst case of a false dedupe is a skipped intentional repeat, never a double purchase. To deliberately make a second, otherwise-identical purchase (e.g. "buy 3 RIs now" and "buy 3 more next week"), pass a fresh `idempotency_nonce` value on the second call; passing the same nonce on a retry of that same call still dedupes correctly. - Provider/SDK failures surface their full error text back to the caller; nothing is swallowed. - A **completed** real purchase carries an `archera` block in the response: an optional underutilization-insurance offer (Archera covers the gap if committed capacity goes unused), the signup link, the enrollment window in days, and both partnership disclosures. It is attached only when the purchase actually succeeded, never to a dry run or a failed purchase, because neither started an enrollment window. Archera sponsors CUDly's development from a fraction of their insurance premiums, and CUDly works fully without it; both facts travel with the link in every response so a client rendering this payload cannot present the offer as a neutral recommendation. -- Every real purchase writes an `mcp purchase ATTEMPT` line and a matching `mcp purchase OK` / `mcp purchase FAILED` line to **stderr**, recording provider, target account, region, resource, count, term, payment option, the resulting commitment ID, and a masked idempotency token. Dry runs are not logged (they spend nothing). Capture your MCP client's stderr if you want this trail retained. Nothing is written to stdout, which the MCP stdio transport owns for JSON-RPC framing. +- Every real purchase writes an `mcp purchase ATTEMPT` line and a matching `mcp purchase OK` / `mcp purchase FAILED` line to **stderr**, recording provider, target account, region, resource, count, term, payment option, the resulting commitment ID, and a masked idempotency token. Dry runs emit no purchase diagnostic to stderr because they spend nothing, but when auditing is enabled and the append succeeds, they are persisted in the JSONL audit log as `status: "skipped"`. Capture your MCP client's stderr if you want the diagnostic trail retained. Nothing is written to stdout, which the MCP stdio transport owns for JSON-RPC framing. ### What this server does NOT give you Understand these before enabling real purchases, especially in a shared or production account: - **No scheduled/4-eyes approval workflow.** The web UI routes a purchase through `purchase_executions` with a scheduled date and, under 4-eyes mode, a second approver who cannot be the creator. This server has no such workflow: once `CUDLY_MCP_ENABLE_REAL_PURCHASES=1` is set, `dry_run=false` plus `confirm=true` in a single tool call executes immediately. `confirm` is still a guardrail against an accidental call rather than an authorization control -- it is supplied by the model driving the client, not the operator -- but `CUDLY_MCP_ENABLE_REAL_PURCHASES` is the operator-side authorization control layered underneath it: it must be explicitly enabled before *any* tool call, confirmed or not, can spend money on this server. -- **No persisted audit record.** The CLI writes a `common.AuditRecord` per purchase and the web path persists an execution row; this server writes only the stderr lines above. An MCP purchase does not appear in CUDly's own purchase history, so reconcile against the provider's console/billing data rather than against CUDly. +- **The persisted audit record is a local JSONL file, not CUDly's purchase history.** When auditing is enabled and a write succeeds, each purchase attempt -- including previews -- is appended as one `common.AuditRecord` JSON line to the path from [Audit log](#audit-log) below. An MCP purchase still does not appear in the CLI/web paths' `purchase_history`, so reconcile against the provider's console/billing data or this file rather than against CUDly's own database. - **Credentials are whatever launched the process.** `aws_profile` / `azure_subscription_id` / `gcp_project_id` are per-call arguments chosen by the model, so any account reachable from the ambient credentials is reachable from any tool call. Scope the credentials you launch `cudly-mcp` with to what you are willing to let it spend, rather than relying on the tool arguments to constrain it. +## Audit log + +Auditing is **on by default**. When auditing is enabled and a write succeeds, each purchase attempt -- previews included -- appends one `common.AuditRecord` JSON line to a local file, independent of the stderr trail in [Safety model](#safety-model) above. + +- **Default path**: `$XDG_STATE_HOME/cudly/mcp-audit.jsonl`, falling back to `~/.local/state/cudly/mcp-audit.jsonl` when `XDG_STATE_HOME` is unset, empty, or relative. +- **`CUDLY_MCP_AUDIT_LOG`** overrides the path. Setting it to an **empty or whitespace-only string disables the log entirely** -- that is the explicit opt-out; leaving the variable unset is not the same thing and still uses the default path. +- A preview (`dry_run=true`) is recorded with `status: "skipped"`, `dry_run: true` -- it spent nothing, but it was still a decision worth reconstructing later. +- When a credential scope is supplied, `credential_scope` records that routing identifier: an AWS profile, Azure subscription, or GCP project. It identifies how CUDly selected the target; it is not a verified provider account ID. Previews may omit it because they do not require a target. +- A real purchase is recorded `"success"` only when the provider reports `Success: true` with no embedded error. A failed provider call, `Success: false`, or a result containing an error is recorded as `"error"`. +- Every purchase in one server process shares a single `run_id`, so a log spanning many purchases can be grouped by session. +- **A write failure never changes the purchase result.** It logs a warning to stderr naming the path and error; the tool response returned to the caller is unaffected. +- The path is probed for read, append, and directory durability at server startup. MCP creates missing parent directories as owner-only `0700` subject to umask, then opens and syncs every directory edge from the filesystem root. Each ancestor therefore needs search and read permission, every creation parent also needs write permission, and the filesystem must support directory `fsync`. A path that fails these checks stops server construction instead of silently dropping every record for the session. + ## Caveats and known gaps These are pre-existing behaviours in the underlying purchase clients, not something introduced by or specific to the MCP server -- flagged here so you know what to expect: diff --git a/cmd/cudly-mcp/main_test.go b/cmd/cudly-mcp/main_test.go index a4f422d..539119f 100644 --- a/cmd/cudly-mcp/main_test.go +++ b/cmd/cudly-mcp/main_test.go @@ -1,20 +1,53 @@ package main import ( + "bytes" "context" + "encoding/json" + "log" + "os" + "os/exec" "path/filepath" "strings" "testing" "time" + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" gosdk "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" cudlymcp "github.com/LeanerCloud/cloud-commitments-mcp" "github.com/LeanerCloud/cloud-commitments-mcp/tools" ) +const runAsMCPServerEnv = "CUDLY_MCP_TEST_HELPER_PROCESS" + +func TestMain(m *testing.M) { + if os.Getenv(runAsMCPServerEnv) == "1" { + main() + os.Exit(0) + } + + auditDir, err := os.MkdirTemp("", "cudly-mcp-audit-testmain") + if err != nil { + log.Printf("create MCP audit test directory: %v", err) + os.Exit(1) + } + if err := os.Setenv(tools.EnvAuditLog, filepath.Join(auditDir, "mcp-audit.jsonl")); err != nil { + log.Printf("set MCP audit log test path: %v", err) + if cleanupErr := os.RemoveAll(auditDir); cleanupErr != nil { + log.Printf("remove MCP audit test directory after setup failure: %v", cleanupErr) + } + os.Exit(1) + } + + code := m.Run() + os.RemoveAll(auditDir) + os.Exit(code) +} + // isolateFromAmbientAWS points the AWS SDK at deliberately nonexistent // profile/config/credentials so config.LoadDefaultConfig cannot resolve any // real credentials -- neither from a dev machine's ~/.aws files nor from the @@ -38,6 +71,208 @@ func isolateFromAmbientAWS(t *testing.T) { t.Setenv("AWS_WEB_IDENTITY_TOKEN_FILE", "") } +func holdMCPAuditLog(t *testing.T, path string) func() { + t.Helper() + f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) + require.NoError(t, err) + release := func() { + if f != nil { + assert.NoError(t, f.Close()) + f = nil + } + } + t.Cleanup(release) + require.NoError(t, unix.Flock(int(f.Fd()), unix.LOCK_EX|unix.LOCK_NB)) + return release +} + +func mcpChildEnv(auditPath string) []string { + childEnv := make([]string, 0, len(os.Environ())+3) + for _, entry := range os.Environ() { + if strings.HasPrefix(entry, tools.EnvAuditLog+"=") || strings.HasPrefix(entry, tools.EnvEnableRealPurchases+"=") { + continue + } + childEnv = append(childEnv, entry) + } + return append(childEnv, + runAsMCPServerEnv+"=1", + tools.EnvAuditLog+"="+auditPath, + tools.EnvEnableRealPurchases+"=", + ) +} + +func callMCPPreview(t *testing.T, session *gosdk.ClientSession) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + result, err := session.CallTool(ctx, &gosdk.CallToolParams{ + Name: "cudly_aws_ec2_ri_purchase", + Arguments: map[string]any{ + "region": "us-east-1", + "instance_type": "m5.large", + "count": 1, + "term_years": 1, + "payment_option": "no-upfront", + "aws_profile": "cudly-mcp-synthetic-profile", + "dry_run": true, + "confirm": false, + }, + }) + require.NoError(t, err) + require.False(t, result.IsError) + structured, err := json.Marshal(result.StructuredContent) + require.NoError(t, err) + var response tools.PurchaseResponse + require.NoError(t, json.Unmarshal(structured, &response)) + require.True(t, response.Success) + require.True(t, response.DryRun) +} + +func TestMainAuditLockTimeout(t *testing.T) { + t.Run("startup", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "mcp-audit.jsonl") + original := []byte("existing-record\n") + require.NoError(t, os.WriteFile(path, original, 0o600)) + holdMCPAuditLog(t, path) + + exe, err := os.Executable() + require.NoError(t, err) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, exe) + cmd.Env = mcpChildEnv(path) + var stderr bytes.Buffer + cmd.Stderr = &stderr + client := gosdk.NewClient(&gosdk.Implementation{Name: "test-client"}, nil) + session, err := client.Connect(ctx, &gosdk.CommandTransport{Command: cmd}, nil) + if session != nil { + defer func() { assert.NoError(t, session.Close()) }() + } + require.Error(t, err) + require.NoError(t, ctx.Err()) + assert.Contains(t, stderr.String(), "timed out acquiring audit lock") + data, readErr := os.ReadFile(path) + require.NoError(t, readErr) + require.Equal(t, original, data) + }) + + t.Run("preview", func(t *testing.T) { + isolateFromAmbientAWS(t) + path := filepath.Join(t.TempDir(), "mcp-audit.jsonl") + original := []byte("existing-record\n") + require.NoError(t, os.WriteFile(path, original, 0o600)) + exe, err := os.Executable() + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, exe) + cmd.Env = mcpChildEnv(path) + var stderr bytes.Buffer + cmd.Stderr = &stderr + client := gosdk.NewClient(&gosdk.Implementation{Name: "test-client"}, nil) + session, err := client.Connect(ctx, &gosdk.CommandTransport{Command: cmd}, nil) + if session != nil { + defer func() { assert.NoError(t, session.Close()) }() + } + require.NoError(t, err) + releaseHolder := holdMCPAuditLog(t, path) + callMCPPreview(t, session) + require.NoError(t, session.Close()) + require.NoError(t, ctx.Err()) + assert.Contains(t, stderr.String(), "timed out acquiring audit lock") + data, readErr := os.ReadFile(path) + require.NoError(t, readErr) + require.Equal(t, original, data) + releaseHolder() + + freshCmd := exec.CommandContext(ctx, exe) + freshCmd.Env = mcpChildEnv(path) + var freshStderr bytes.Buffer + freshCmd.Stderr = &freshStderr + freshClient := gosdk.NewClient(&gosdk.Implementation{Name: "test-client"}, nil) + freshSession, err := freshClient.Connect(ctx, &gosdk.CommandTransport{Command: freshCmd}, nil) + if freshSession != nil { + defer func() { assert.NoError(t, freshSession.Close()) }() + } + require.NoError(t, err) + callMCPPreview(t, freshSession) + require.NoError(t, freshSession.Close()) + require.NoError(t, ctx.Err()) + + data, readErr = os.ReadFile(path) + require.NoError(t, readErr) + require.True(t, bytes.HasPrefix(data, original)) + lines := bytes.Split(bytes.TrimSuffix(data, []byte{'\n'}), []byte{'\n'}) + require.Len(t, lines, 2) + var record common.AuditRecord + require.NoError(t, json.Unmarshal(lines[1], &record)) + assert.Equal(t, "skipped", record.Status) + assert.True(t, record.DryRun) + assert.Equal(t, "cudly-mcp-synthetic-profile", record.CredentialScope) + assert.Equal(t, common.PurchaseSourceMCP, record.Source) + assert.Equal(t, common.ProviderAWS, record.Provider) + assert.Equal(t, string(common.ServiceEC2), record.Service) + }) +} + +func TestMainRejectsStdoutAuditLogBeforeProtocolTraffic(t *testing.T) { + info, err := os.Stat("/dev/stdout") + if err != nil { + t.Skipf("/dev/stdout unavailable: %v", err) + } + if info.Mode().IsRegular() { + t.Skip("/dev/stdout is a regular file in this environment") + } + + exe, err := os.Executable() + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, exe) + childEnv := make([]string, 0, len(os.Environ())+2) + for _, entry := range os.Environ() { + if strings.HasPrefix(entry, tools.EnvAuditLog+"=") { + continue + } + childEnv = append(childEnv, entry) + } + cmd.Env = append(childEnv, runAsMCPServerEnv+"=1", tools.EnvAuditLog+"=/dev/stdout") + var stderr bytes.Buffer + cmd.Stderr = &stderr + + client := gosdk.NewClient(&gosdk.Implementation{Name: "test-client"}, nil) + session, err := client.Connect(ctx, &gosdk.CommandTransport{ + Command: cmd, + TerminateDuration: time.Second, + }, nil) + if err == nil { + _, err = session.CallTool(ctx, &gosdk.CallToolParams{ + Name: "cudly_aws_ec2_ri_purchase", + Arguments: map[string]any{ + "region": "us-east-1", + "instance_type": "m5.large", + "count": 1, + "term_years": 1, + "payment_option": "no-upfront", + }, + }) + closeErr := session.Close() + if err == nil { + err = closeErr + } + } + + require.Error(t, err) + assert.NotContains(t, err.Error(), "invalid message version tag") + assert.NoError(t, ctx.Err()) + stderrText := stderr.String() + assert.Contains(t, stderrText, "failed to build server") + assert.Contains(t, stderrText, "non-regular audit log target") +} + // TestRealPurchasePastProviderRegistration is the regression guard for the // bug this file's blank imports fix: cudly-mcp never imported // providers/aws|azure|gcp, so their init()-registered factories were never diff --git a/go.mod b/go.mod index cd3ddde..a1c0af6 100644 --- a/go.mod +++ b/go.mod @@ -64,7 +64,7 @@ require ( golang.org/x/net v0.58.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.22.0 // indirect - golang.org/x/sys v0.47.0 // indirect + golang.org/x/sys v0.47.0 golang.org/x/text v0.41.0 // indirect golang.org/x/time v0.15.0 // indirect google.golang.org/api v0.274.0 // indirect @@ -77,11 +77,12 @@ require ( ) require ( - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20260928074610-6168f8b5360d - github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20260928074610-6168f8b5360d - github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928074610-6168f8b5360d - github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928074610-6168f8b5360d + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20260928132534-fe940a89483d + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20260928132534-fe940a89483d + github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928132534-fe940a89483d + github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928132534-fe940a89483d github.com/google/jsonschema-go v0.4.3 + github.com/google/uuid v1.6.0 github.com/modelcontextprotocol/go-sdk v1.6.1 ) @@ -106,7 +107,6 @@ require ( github.com/envoyproxy/go-control-plane/envoy v1.37.0 // indirect github.com/envoyproxy/protoc-gen-validate v1.3.3 // indirect github.com/go-jose/go-jose/v4 v4.1.4 // indirect - github.com/google/uuid v1.6.0 // indirect github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect github.com/segmentio/asm v1.1.3 // indirect github.com/segmentio/encoding v0.5.4 // indirect diff --git a/go.sum b/go.sum index 77bc98a..94b7f4f 100644 --- a/go.sum +++ b/go.sum @@ -76,14 +76,14 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.54.0/go.mod h1:vB2GH9GAYYJTO3mEn8oYwzEdhlayZIdQz6zdzgUIRvA= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0 h1:s0WlVbf9qpvkh1c/uDAPElam0WrL7fHRIidgZJ7UqZI= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0/go.mod h1:Mf6O40IAyB9zR/1J8nGDDPirZQQPbYJni8Yisy7NTMc= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20260928074610-6168f8b5360d h1:kWXws3XgZUwRWZj1dZTP9KxrFrAZKoxi6JsvPIyGqhs= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20260928074610-6168f8b5360d/go.mod h1:pYpkdSOCe6cnmhe7t+3B6S0Txjnbm/2hyc/EAlQn8PI= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20260928074610-6168f8b5360d h1:XtyLtFR23EjV1jRK+bSuEimq6a+s7VMw2pbw2ZD9QSA= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20260928074610-6168f8b5360d/go.mod h1:CTwoaiQJNefXp5W0AoQcGokMMcCDJQ9m+ML5PUHU6KQ= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928074610-6168f8b5360d h1:UnZ/7Hz3/UdWtIR8HSBptnEoFjPo36DyYdJLALg6zFQ= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928074610-6168f8b5360d/go.mod h1:SPzd/neHw+jTyyKY0rrCVgh38DsYSs0VVQAvc2OJy+s= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928074610-6168f8b5360d h1:dxfx+OpaGKy/7dzr+XaUpICyWFxEb85uV63rpl7ApCE= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928074610-6168f8b5360d/go.mod h1:KNRux6gPe5WpG0U3P0wCEEe6+Su00+dnaXpooNBKQGk= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20260928132534-fe940a89483d h1:cdVqYayr8z0juNLKM5y3IopcbT8o1vMkouIDcIDN2tw= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20260928132534-fe940a89483d/go.mod h1:ApWBliDXe099f3oDXBz41K/I9v4bHvn1dG/BGoRmHlw= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20260928132534-fe940a89483d h1:vL3eBWycBeAYV1vwJxs4knDzse9v6n9VgrbfN+vkl38= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20260928132534-fe940a89483d/go.mod h1:CTwoaiQJNefXp5W0AoQcGokMMcCDJQ9m+ML5PUHU6KQ= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928132534-fe940a89483d h1:UnZ5LTWNZoT1xlehBXwhoehTOcQsh4bPk6i0Qx5P/1E= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928132534-fe940a89483d/go.mod h1:SPzd/neHw+jTyyKY0rrCVgh38DsYSs0VVQAvc2OJy+s= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928132534-fe940a89483d h1:Exy2yM3fcguKVEuHlEwM7+WHhdrhzfxX8MysNCI9IM4= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928132534-fe940a89483d/go.mod h1:KNRux6gPe5WpG0U3P0wCEEe6+Su00+dnaXpooNBKQGk= github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= github.com/aws/aws-sdk-go-v2/config v1.29.12 h1:Y/2a+jLPrPbHpFkpAAYkVEtJmxORlXoo5k2g1fa2sUo= diff --git a/server.go b/server.go index 1428ae5..90658f5 100644 --- a/server.go +++ b/server.go @@ -43,6 +43,14 @@ func registrations() []tools.Registration { // server, err := mcp.NewServer("1.0.0") // server.Run(ctx, &gosdk.StdioTransport{}) func NewServer(version string) (*gosdk.Server, error) { + // A server that cannot durably write its purchase audit trail should not accept + // purchase calls at all. Failing here surfaces a misconfigured + // CUDLY_MCP_AUDIT_LOG at startup instead of silently dropping every + // audit record for the session. + if err := tools.EnsureAuditLogWritable(); err != nil { + return nil, fmt.Errorf("audit log is not readable, writable, and directory-durable: %w", err) + } + s := gosdk.NewServer(&gosdk.Implementation{Name: ServerName, Version: version}, nil) regs := registrations() diff --git a/server.json b/server.json index a6886ee..cbfc6e8 100644 --- a/server.json +++ b/server.json @@ -23,6 +23,13 @@ "isRequired": false, "isSecret": false, "format": "string" + }, + { + "name": "CUDLY_MCP_AUDIT_LOG", + "description": "Overrides the local JSONL purchase audit path after trimming. When unset, writes to $XDG_STATE_HOME/cudly/mcp-audit.jsonl; if XDG_STATE_HOME is unset, empty, or relative, it is ignored and the fallback is ~/.local/state/cudly/mcp-audit.jsonl. An empty or whitespace-only CUDLY_MCP_AUDIT_LOG value disables auditing.", + "isRequired": false, + "isSecret": false, + "format": "string" } ] } diff --git a/server_test.go b/server_test.go index 1b84c52..55eb5e6 100644 --- a/server_test.go +++ b/server_test.go @@ -3,6 +3,9 @@ package mcp import ( "context" "encoding/json" + "log" + "os" + "path/filepath" "strings" "testing" @@ -13,6 +16,34 @@ import ( "github.com/LeanerCloud/cloud-commitments-mcp/tools" ) +// TestMain pins tools.EnvAuditLog to a path inside a run-scoped temp +// directory for this package's test binary. Auditing is on by default (see +// tools.EnvAuditLog), and every test in this file calls NewServer, which now +// probes the audit log path at construction time -- without this, the whole +// suite would touch the developer's real ~/.local/state/cudly/mcp-audit.jsonl. +// Tests that need a specific audit configuration override this with their +// own t.Setenv. +func TestMain(m *testing.M) { + auditDir, err := os.MkdirTemp("", "cudly-mcp-server-audit-testmain") + if err != nil { + log.Printf("create MCP server audit test directory: %v", err) + os.Exit(1) + } + if err := os.Setenv(tools.EnvAuditLog, filepath.Join(auditDir, "mcp-audit.jsonl")); err != nil { + log.Printf("set MCP server audit log test path: %v", err) + if cleanupErr := os.RemoveAll(auditDir); cleanupErr != nil { + log.Printf("remove MCP server audit test directory after setup failure: %v", cleanupErr) + } + os.Exit(1) + } + + // os.Exit skips deferred calls, so the temp dir is removed explicitly + // before exiting rather than via defer. + code := m.Run() + os.RemoveAll(auditDir) + os.Exit(code) +} + func TestNewServerBuildsWithoutError(t *testing.T) { t.Parallel() s, err := NewServer("test") @@ -20,6 +51,54 @@ func TestNewServerBuildsWithoutError(t *testing.T) { require.NotNil(t, s) } +// TestNewServerFailsOnBadAuditPath proves NewServer refuses to build when +// the resolved audit log path is unwritable, rather than silently dropping +// every audit record for the session. The path's parent is a regular file +// (not a directory), so descriptor-relative directory preparation fails. +// Not parallel: t.Setenv forbids it. +func TestNewServerFailsOnBadAuditPath(t *testing.T) { + dir := t.TempDir() + blockingFile := filepath.Join(dir, "not-a-directory") + require.NoError(t, os.WriteFile(blockingFile, []byte("x"), 0o600)) + t.Setenv(tools.EnvAuditLog, filepath.Join(blockingFile, "nested.jsonl")) + + s, err := NewServer("test") + require.Error(t, err) + assert.Nil(t, s) + assert.Contains(t, err.Error(), "audit log") +} + +// TestNewServerRejectsNonRegularAuditPath proves startup rejects paths like +// /dev/null before the server accepts MCP traffic and loses every audit line. +// Not parallel: t.Setenv forbids it. +func TestNewServerRejectsNonRegularAuditPath(t *testing.T) { + info, err := os.Stat("/dev/null") + if err != nil { + t.Skipf("/dev/null unavailable: %v", err) + } + if info.Mode().IsRegular() { + t.Skip("/dev/null is a regular file in this environment") + } + t.Setenv(tools.EnvAuditLog, "/dev/null") + + s, err := NewServer("test") + require.Error(t, err) + assert.Nil(t, s) + assert.Contains(t, err.Error(), "audit log") + assert.Contains(t, err.Error(), "non-regular audit log target") +} + +// TestNewServerSucceedsWhenAuditDisabled proves the audit gate does not +// interfere with server construction when auditing is explicitly disabled. +// Not parallel: t.Setenv forbids it. +func TestNewServerSucceedsWhenAuditDisabled(t *testing.T) { + t.Setenv(tools.EnvAuditLog, "") + + s, err := NewServer("test") + require.NoError(t, err) + assert.NotNil(t, s) +} + // TestRegistryNonEmpty proves the tool registry is never accidentally empty: // cudly_list_commitment_actions is always present, even before any // purchase/search tool has been registered. diff --git a/tools/audit.go b/tools/audit.go new file mode 100644 index 0000000..d809018 --- /dev/null +++ b/tools/audit.go @@ -0,0 +1,149 @@ +package tools + +import ( + "fmt" + "log" + "os" + "path/filepath" + "strings" + + "github.com/google/uuid" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" +) + +// EnvAuditLog names the operator-controlled path for the MCP server's +// purchase audit log (one JSON line per successful audit write, including +// previews). Auditing is ON BY DEFAULT -- unlike EnvEnableRealPurchases, +// which fails closed, this fails open toward keeping a record: an unset +// variable resolves to a default path under the user's XDG state directory +// (see AuditLogPath), not to "no logging". Setting this variable to an empty +// or whitespace-only string is the explicit operator opt-out that disables +// the log entirely; any other value overrides the default path after trimming. +// For real purchases this overlaps with logPurchaseAttempt and +// logPurchaseOutcome's stderr trail. The JSONL file is the durable, +// machine-readable counterpart and additionally records previews: without +// it, an MCP purchase left no record anywhere once the process exited, +// unlike the CLI (cmd/multi_service.go) and web (purchase_executions) paths. +const EnvAuditLog = "CUDLY_MCP_AUDIT_LOG" + +// auditRunID identifies every purchase made by this server process in one +// audit trail. Initialized once at process startup (not per purchase) so an +// operator reading the log can correlate every purchase in one server +// lifetime, matching how a single CLI invocation shares one run. +var auditRunID = uuid.NewString() + +// auditStatusSuccess, auditStatusError, and auditStatusSkipped are the three +// statuses this server ever writes. A dry run is skipped; a provider result +// is successful only when Success is true and Error is nil; every other +// provider outcome is an error. The fourth status common.NewAuditRecord +// documents, "skipped_covered", belongs to the CLI's recent-duplicate guard, +// which this server does not run yet. +const ( + auditStatusSuccess = "success" + auditStatusError = "error" + auditStatusSkipped = "skipped" +) + +// auditStatusFor maps a provider PurchaseResult to the audit status it +// represents. A provider result counts as success only when it reports +// Success=true and carries no embedded error detail. +func auditStatusFor(result common.PurchaseResult) string { + if purchaseSucceeded(result) { + return auditStatusSuccess + } + return auditStatusError +} + +// AuditLogPath resolves where the MCP purchase audit log is written. +// enabled is false only when EnvAuditLog is set to an empty or whitespace-only +// string -- the explicit operator opt-out. An unset variable selects the +// default path, +// $XDG_STATE_HOME/cudly/mcp-audit.jsonl (falling back to +// ~/.local/state/cudly/mcp-audit.jsonl when XDG_STATE_HOME is unset, +// empty, or relative). +// +// os.LookupEnv, not os.Getenv, is required here: it is the only way to +// distinguish "the operator set this to empty or whitespace-only on purpose" +// from "the operator never set this at all", and those two cases must resolve +// to opposite outcomes (disabled vs. the default path). +func AuditLogPath() (path string, enabled bool, err error) { + if v, isSet := os.LookupEnv(EnvAuditLog); isSet { + // Trimmed, matching how every other operator env var in this package + // is read: a value of " " is a typo, not a request to create a file + // whose name is a space. + if trimmed := strings.TrimSpace(v); trimmed != "" { + return trimmed, true, nil + } + return "", false, nil + } + + stateDir := os.Getenv("XDG_STATE_HOME") + if stateDir == "" || !filepath.IsAbs(stateDir) { + home, homeErr := os.UserHomeDir() + if homeErr != nil { + return "", false, fmt.Errorf("resolve default audit log path: %w", homeErr) + } + stateDir = filepath.Join(home, ".local", "state") + } + return filepath.Join(stateDir, "cudly", "mcp-audit.jsonl"), true, nil +} + +// EnsureAuditLogWritable resolves the audit path, durably creates its parent +// directory hierarchy, and probes the file and parent directories. Returns nil +// when auditing is disabled. Called once from mcp.NewServer so a misconfigured +// path fails server construction loudly, rather than silently dropping every +// audit record for the life of the process. +func EnsureAuditLogWritable() error { + path, enabled, err := AuditLogPath() + if err != nil { + return err + } + if !enabled { + return nil + } + // 0700: the default path lives under the user's own state directory, and + // this is a per-user record of money-spending decisions. + if err := ensureAuditLogDirectory(path); err != nil { + return fmt.Errorf("prepare audit log directory for %s: %w", path, err) + } + if err := common.CheckAuditLogWritable(path); err != nil { + return fmt.Errorf("audit log: %w", err) + } + return nil +} + +// recordPurchaseAudit attempts to append one JSONL record for a purchase +// attempt (preview or real) when auditing is enabled. Resolves the path per +// call, not once at startup, so a test or operator override of EnvAuditLog +// after process start still takes effect. +// +// A write failure warns on stderr and returns: the caller's purchase result +// is authoritative and must never change because the audit trail hiccuped +// -- losing one line is a mundane operational problem, silently turning a +// completed purchase into a reported failure would not be. +func recordPurchaseAudit( + rec common.Recommendation, + credentialScope string, + result common.PurchaseResult, + status string, + dryRun bool, +) { + path, enabled, err := AuditLogPath() + if err != nil { + log.Printf("mcp audit log: %v", err) + return + } + if !enabled { + return + } + if err := ensureAuditLogDirectory(path); err != nil { + log.Printf("mcp audit log: prepare directory for %s: %v", path, err) + return + } + record := common.NewAuditRecord(auditRunID, rec, result, status, dryRun, common.PurchaseSourceMCP) + record.CredentialScope = credentialScope + if err := common.WriteAuditRecord(record, path); err != nil { + log.Printf("mcp audit log: %v", err) + } +} diff --git a/tools/audit_directory_unix.go b/tools/audit_directory_unix.go new file mode 100644 index 0000000..898337a --- /dev/null +++ b/tools/audit_directory_unix.go @@ -0,0 +1,161 @@ +//go:build linux || darwin + +package tools + +import ( + "errors" + "fmt" + "io/fs" + "math" + "os" + "path/filepath" + "strings" + + "golang.org/x/sys/unix" +) + +type auditDirectoryHandle interface { + Fd() uintptr + Sync() error + Close() error +} + +type auditDirectoryOps struct { + openRoot func() (auditDirectoryHandle, error) + mkdirAt func(auditDirectoryHandle, string, uint32) error + openAt func(auditDirectoryHandle, string) (auditDirectoryHandle, error) +} + +func ensureAuditLogDirectory(path string) error { + return ensureAuditLogDirectoryWithOps(path, productionAuditDirectoryOps()) +} + +func ensureAuditLogDirectoryWithOps(path string, ops auditDirectoryOps) error { + parentPath, err := filepath.Abs(filepath.Dir(path)) + if err != nil { + return fmt.Errorf("resolve audit log directory for %s: %w", path, err) + } + relativePath, err := filepath.Rel(string(filepath.Separator), parentPath) + if err != nil { + return fmt.Errorf("resolve audit log directory components for %s: %w", path, err) + } + + current, err := ops.openRoot() + if err != nil { + return fmt.Errorf("open audit log directory / for durability sync: %w", err) + } + currentPath := string(filepath.Separator) + + components := strings.Split(relativePath, string(filepath.Separator)) + if relativePath == "." { + components = nil + } + for _, component := range components { + current, currentPath, err = walkAuditDirectoryComponent(current, currentPath, component, ops) + if err != nil { + return err + } + } + + return closeAuditDirectory(currentPath, current, nil) +} + +func walkAuditDirectoryComponent( + parent auditDirectoryHandle, + parentPath string, + component string, + ops auditDirectoryOps, +) (auditDirectoryHandle, string, error) { + childPath := filepath.Join(parentPath, component) + if err := createAuditDirectoryComponent(parent, component, ops); err != nil { + return nil, "", closeAuditDirectory( + parentPath, + parent, + fmt.Errorf("create audit log directory %s: %w", childPath, err), + ) + } + + child, err := ops.openAt(parent, component) + if err != nil { + return nil, "", closeAuditDirectory( + parentPath, + parent, + fmt.Errorf("open audit log directory %s for durability sync: %w", childPath, err), + ) + } + + if err := syncAndCloseAuditDirectory(parentPath, parent); err != nil { + return nil, "", closeAuditDirectory( + childPath, + child, + err, + ) + } + return child, childPath, nil +} + +func createAuditDirectoryComponent(parent auditDirectoryHandle, component string, ops auditDirectoryOps) error { + err := ops.mkdirAt(parent, component, 0o700) + if errors.Is(err, fs.ErrExist) { + return nil + } + return err +} + +func syncAndCloseAuditDirectory(path string, directory auditDirectoryHandle) error { + var opErr error + if err := directory.Sync(); err != nil { + opErr = fmt.Errorf("sync audit log directory %s for durability: %w", path, err) + } + if err := directory.Close(); err != nil { + opErr = errors.Join(opErr, fmt.Errorf("close audit log directory %s: %w", path, err)) + } + return opErr +} + +func closeAuditDirectory(path string, handle auditDirectoryHandle, opErr error) error { + if err := handle.Close(); err != nil { + opErr = errors.Join(opErr, fmt.Errorf("close audit log directory %s: %w", path, err)) + } + return opErr +} + +func auditDirectoryFD(handle auditDirectoryHandle) (int, error) { + fd := handle.Fd() + if fd > uintptr(math.MaxInt) { + return 0, fmt.Errorf("audit directory file descriptor %d exceeds int range", fd) + } + return int(fd), nil // #nosec G115 -- fd is range-checked above +} + +func productionAuditDirectoryOps() auditDirectoryOps { + return auditDirectoryOps{ + openRoot: func() (auditDirectoryHandle, error) { + return os.Open(string(filepath.Separator)) + }, + mkdirAt: func(parent auditDirectoryHandle, name string, mode uint32) error { + fd, err := auditDirectoryFD(parent) + if err != nil { + return err + } + return unix.Mkdirat(fd, name, mode) + }, + openAt: func(parent auditDirectoryHandle, name string) (auditDirectoryHandle, error) { + parentFD, err := auditDirectoryFD(parent) + if err != nil { + return nil, err + } + fd, err := unix.Openat( + parentFD, + name, + unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, + 0, + ) + if err != nil { + return nil, err + } + file := os.NewFile(uintptr(fd), name) // #nosec G115 -- successful unix.Openat fd fits uintptr + return file, nil + }, + } +} diff --git a/tools/audit_directory_unix_test.go b/tools/audit_directory_unix_test.go new file mode 100644 index 0000000..aa105c8 --- /dev/null +++ b/tools/audit_directory_unix_test.go @@ -0,0 +1,276 @@ +//go:build linux || darwin + +package tools + +import ( + "errors" + "fmt" + "io/fs" + "math" + "os" + "path/filepath" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" +) + +var ( + errAuditDirectorySync = errors.New("directory sync failed") + errAuditDirectoryClose = errors.New("directory close failed") +) + +func TestProductionAuditDirectoryOpsRejectsUnrepresentableDescriptor(t *testing.T) { + t.Parallel() + parent := &tracedAuditDirectory{fd: uintptr(math.MaxInt) + 1} + ops := productionAuditDirectoryOps() + tests := []struct { + name string + run func() (auditDirectoryHandle, error) + }{ + { + name: "mkdirat", + run: func() (auditDirectoryHandle, error) { + return nil, ops.mkdirAt(parent, "", 0o700) + }, + }, + { + name: "openat", + run: func() (auditDirectoryHandle, error) { + return ops.openAt(parent, "") + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + handle, err := test.run() + if handle != nil { + require.NoError(t, handle.Close()) + t.Fatal("operation returned an unexpected directory handle") + } + require.EqualError(t, err, fmt.Sprintf( + "audit directory file descriptor %d exceeds int range", + parent.fd, + )) + }) + } +} + +func TestEnsureAuditLogDirectorySyncsEveryPathEdge(t *testing.T) { + t.Parallel() + trace := &auditDirectoryTrace{} + + require.NoError(t, ensureAuditLogDirectoryWithOps("/one/two/three/audit.jsonl", trace.ops())) + + assert.Equal(t, []string{ + "open:/", "mkdir:one", "open:one", "sync:/", "close:/", + "mkdir:two", "open:two", "sync:one", "close:one", + "mkdir:three", "open:three", "sync:two", "close:two", "close:three", + }, trace.operations) +} + +func TestEnsureAuditLogDirectoryJoinsSyncAndCloseErrors(t *testing.T) { + t.Parallel() + trace := &auditDirectoryTrace{ + syncErrors: map[string]error{"/": errAuditDirectorySync}, + closeErrors: map[string]error{"/": errAuditDirectoryClose}, + } + + err := ensureAuditLogDirectoryWithOps("/one/audit.jsonl", trace.ops()) + + require.ErrorIs(t, err, errAuditDirectorySync) + require.ErrorIs(t, err, errAuditDirectoryClose) + assert.Equal(t, []string{"open:/", "mkdir:one", "open:one", "sync:/", "close:/", "close:one"}, trace.operations) +} + +func TestEnsureAuditLogDirectoryRetriesExistingHierarchyAfterSyncFailure(t *testing.T) { + t.Parallel() + first := &auditDirectoryTrace{syncErrors: map[string]error{"/": errAuditDirectorySync}} + require.ErrorIs(t, ensureAuditLogDirectoryWithOps("/one/audit.jsonl", first.ops()), errAuditDirectorySync) + + second := &auditDirectoryTrace{mkdirErrors: map[string]error{"one": fs.ErrExist}} + require.NoError(t, ensureAuditLogDirectoryWithOps("/one/audit.jsonl", second.ops())) + assert.Contains(t, second.operations, "sync:/") +} + +func TestEnsureAuditLogDirectoryRejectsExistingNonDirectory(t *testing.T) { + t.Parallel() + blocking := filepath.Join(t.TempDir(), "not-a-directory") + require.NoError(t, os.WriteFile(blocking, []byte("x"), 0o600)) + + err := ensureAuditLogDirectory(filepath.Join(blocking, "audit.jsonl")) + + require.Error(t, err) + assert.ErrorContains(t, err, blocking) +} + +func TestEnsureAuditLogDirectorySupportsConcurrentCreators(t *testing.T) { + t.Parallel() + path := filepath.Join(t.TempDir(), "one", "two", "three", "audit.jsonl") + start := make(chan struct{}) + errs := make(chan error, 2) + var wg sync.WaitGroup + for range 2 { + wg.Add(1) + go func() { + defer wg.Done() + <-start + errs <- ensureAuditLogDirectory(path) + }() + } + close(start) + wg.Wait() + close(errs) + for err := range errs { + require.NoError(t, err) + } +} + +func TestEnsureAuditLogDirectoryCreatesPrivateNestedHierarchy(t *testing.T) { + root := t.TempDir() + components := []string{ + filepath.Join(root, "one"), + filepath.Join(root, "one", "two"), + filepath.Join(root, "one", "two", "three"), + } + previousUmask := unix.Umask(0) + t.Cleanup(func() { unix.Umask(previousUmask) }) + + require.NoError(t, ensureAuditLogDirectory(filepath.Join(components[2], "audit.jsonl"))) + + for _, component := range components { + info, err := os.Stat(component) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o700), info.Mode().Perm(), component) + } +} + +func TestEnsureAuditLogDirectoryPreservesExistingMode(t *testing.T) { + t.Parallel() + parent := filepath.Join(t.TempDir(), "audit-parent") + require.NoError(t, os.Mkdir(parent, 0o750)) + require.NoError(t, os.Chmod(parent, 0o750)) + + require.NoError(t, ensureAuditLogDirectory(filepath.Join(parent, "audit.jsonl"))) + + info, err := os.Stat(parent) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o750), info.Mode().Perm()) +} + +func TestEnsureAuditLogWritableCreatesNestedHierarchy(t *testing.T) { + path := filepath.Join(t.TempDir(), "one", "two", "three", "audit.jsonl") + t.Setenv(EnvAuditLog, path) + + require.NoError(t, EnsureAuditLogWritable()) + + info, err := os.Stat(path) + require.NoError(t, err) + assert.True(t, info.Mode().IsRegular()) +} + +func TestEnsureAuditLogDirectoryRejectsUnreadableWalkAncestor(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses directory read permission checks") + } + + ancestor := filepath.Join(t.TempDir(), "ancestor") + require.NoError(t, os.Mkdir(ancestor, 0o700)) + require.NoError(t, os.Chmod(ancestor, 0o300)) + t.Cleanup(func() { require.NoError(t, os.Chmod(ancestor, 0o700)) }) + + err := ensureAuditLogDirectory(filepath.Join(ancestor, "child", "audit.jsonl")) + + require.Error(t, err) + assert.ErrorContains(t, err, ancestor) + assert.ErrorContains(t, err, "durability") +} + +func TestEnsureAuditLogWritableRejectsUnreadableWalkAncestor(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses directory read permission checks") + } + + ancestor, path := auditPathBehindUnreadableAncestor(t) + t.Setenv(EnvAuditLog, path) + + err := EnsureAuditLogWritable() + + require.Error(t, err) + assert.ErrorContains(t, err, ancestor) + assert.ErrorContains(t, err, "durability") +} + +func TestPurchaseAuditRejectsUnreadableWalkAncestorWithoutChangingResult(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses directory read permission checks") + } + + ancestor, path := auditPathBehindUnreadableAncestor(t) + t.Setenv(EnvAuditLog, path) + + resp, output := executeSuccessfulPurchaseWithCapturedAuditWarning(t) + + assert.True(t, resp.Success) + assert.Equal(t, "ri-unwritable", resp.CommitmentID) + assert.Contains(t, output, "mcp audit log") + assert.Contains(t, output, path) + assert.Contains(t, output, ancestor) + assert.Contains(t, output, "durability") + _, err := os.Stat(path) + require.ErrorIs(t, err, os.ErrNotExist) +} + +func auditPathBehindUnreadableAncestor(t *testing.T) (string, string) { + t.Helper() + ancestor := filepath.Join(t.TempDir(), "ancestor") + immediateParent := filepath.Join(ancestor, "immediate") + require.NoError(t, os.MkdirAll(immediateParent, 0o700)) + require.NoError(t, os.Chmod(ancestor, 0o300)) + t.Cleanup(func() { require.NoError(t, os.Chmod(ancestor, 0o700)) }) + return ancestor, filepath.Join(immediateParent, "audit.jsonl") +} + +type auditDirectoryTrace struct { + operations []string + mkdirErrors map[string]error + syncErrors map[string]error + closeErrors map[string]error +} + +func (t *auditDirectoryTrace) ops() auditDirectoryOps { + return auditDirectoryOps{ + openRoot: func() (auditDirectoryHandle, error) { + t.operations = append(t.operations, "open:/") + return &tracedAuditDirectory{name: "/", trace: t}, nil + }, + mkdirAt: func(_ auditDirectoryHandle, name string, _ uint32) error { + t.operations = append(t.operations, "mkdir:"+name) + return t.mkdirErrors[name] + }, + openAt: func(_ auditDirectoryHandle, name string) (auditDirectoryHandle, error) { + t.operations = append(t.operations, "open:"+name) + return &tracedAuditDirectory{name: name, trace: t}, nil + }, + } +} + +type tracedAuditDirectory struct { + name string + trace *auditDirectoryTrace + fd uintptr +} + +func (d *tracedAuditDirectory) Fd() uintptr { return d.fd } + +func (d *tracedAuditDirectory) Sync() error { + d.trace.operations = append(d.trace.operations, "sync:"+d.name) + return d.trace.syncErrors[d.name] +} + +func (d *tracedAuditDirectory) Close() error { + d.trace.operations = append(d.trace.operations, "close:"+d.name) + return d.trace.closeErrors[d.name] +} diff --git a/tools/audit_test.go b/tools/audit_test.go new file mode 100644 index 0000000..d725a97 --- /dev/null +++ b/tools/audit_test.go @@ -0,0 +1,462 @@ +package tools + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "log" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" +) + +// auditTestRecommendation mirrors testRecommendation but uses a 1yr term so +// tests here can assert the raw AuditRecord.Term (months) conversion +// independently of the purchase_test.go fixtures, which use "3yr". +func auditTestRecommendation() common.Recommendation { + rec := testRecommendation() + rec.Term = "1yr" + return rec +} + +// readAuditLines reads path and splits it into non-empty JSONL lines. +func readAuditLines(t *testing.T, path string) []string { + t.Helper() + data, err := os.ReadFile(path) // #nosec G304 -- test-controlled temp path + require.NoError(t, err) + var lines []string + for _, l := range strings.Split(string(data), "\n") { + if l != "" { + lines = append(lines, l) + } + } + return lines +} + +// TestAuditLogPathDefaultWhenUnset proves an unset EnvAuditLog resolves to +// the default path under XDG_STATE_HOME, not to "disabled". Not parallel: +// t.Setenv forbids it. +func TestAuditLogPathDefaultWhenUnset(t *testing.T) { + stateDir := t.TempDir() + t.Setenv("XDG_STATE_HOME", stateDir) + prev, had := os.LookupEnv(EnvAuditLog) + require.NoError(t, os.Unsetenv(EnvAuditLog)) + t.Cleanup(func() { + if had { + _ = os.Setenv(EnvAuditLog, prev) + } + }) + + path, enabled, err := AuditLogPath() + require.NoError(t, err) + assert.True(t, enabled, "an unset variable must select the default path, not disable auditing") + assert.Equal(t, filepath.Join(stateDir, "cudly", "mcp-audit.jsonl"), path) +} + +func TestAuditLogPathIgnoresRelativeXDGStateHome(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_STATE_HOME", "relative-state") + prev, had := os.LookupEnv(EnvAuditLog) + require.NoError(t, os.Unsetenv(EnvAuditLog)) + t.Cleanup(func() { + if had { + _ = os.Setenv(EnvAuditLog, prev) + } + }) + + path, enabled, err := AuditLogPath() + require.NoError(t, err) + assert.True(t, enabled) + assert.Equal(t, filepath.Join(home, ".local", "state", "cudly", "mcp-audit.jsonl"), path) + assert.NotContains(t, path, "relative-state") +} + +// TestAuditLogEmptyValueDisables proves EnvAuditLog="" is the explicit +// operator opt-out: AuditLogPath reports disabled, and a full real purchase +// through ExecutePurchase writes no file anywhere under a temp +// XDG_STATE_HOME. Not parallel: t.Setenv forbids it. +func TestAuditLogEmptyValueDisables(t *testing.T) { + stateDir := t.TempDir() + t.Setenv("XDG_STATE_HOME", stateDir) + t.Setenv(EnvAuditLog, "") + + _, enabled, err := AuditLogPath() + require.NoError(t, err) + assert.False(t, enabled) + + fake := &fakeServiceClient{purchaseResult: common.PurchaseResult{Success: true, CommitmentID: "ri-disabled"}} + resp, err := ExecutePurchase(context.Background(), PurchaseRequest{ + Region: "us-east-1", Recommendation: auditTestRecommendation(), DryRun: false, Confirm: true, + CredentialScope: "test-scope", + ResolveClient: func(_ context.Context) (provider.ServiceClient, error) { return fake, nil }, + }) + require.NoError(t, err) + require.NotNil(t, resp) + + entries, err := os.ReadDir(stateDir) + require.NoError(t, err) + assert.Empty(t, entries, "disabled auditing must not create anything under the state dir") +} + +// TestAuditLogWhitespaceValueDisables covers the typo case: a value that is +// only whitespace disables auditing rather than resolving to a file whose +// name is a space. +func TestAuditLogWhitespaceValueDisables(t *testing.T) { + t.Setenv("XDG_STATE_HOME", t.TempDir()) + t.Setenv(EnvAuditLog, " ") + + path, enabled, err := AuditLogPath() + require.NoError(t, err) + assert.False(t, enabled) + assert.Empty(t, path) +} + +// TestAuditLogPathIsTrimmed proves surrounding whitespace in the override is +// stripped rather than becoming part of the resolved filename. +func TestAuditLogPathIsTrimmed(t *testing.T) { + want := filepath.Join(t.TempDir(), "audit.jsonl") + t.Setenv(EnvAuditLog, " "+want+" ") + + path, enabled, err := AuditLogPath() + require.NoError(t, err) + assert.True(t, enabled) + assert.Equal(t, want, path) +} + +// TestAuditLogOverrideWins proves an explicit EnvAuditLog path wins over +// XDG_STATE_HOME. Not parallel: t.Setenv forbids it. +func TestAuditLogOverrideWins(t *testing.T) { + t.Setenv("XDG_STATE_HOME", t.TempDir()) + custom := filepath.Join(t.TempDir(), "custom-audit.jsonl") + t.Setenv(EnvAuditLog, custom) + + path, enabled, err := AuditLogPath() + require.NoError(t, err) + assert.True(t, enabled) + assert.Equal(t, custom, path) +} + +// TestPreviewWritesSkippedRecord proves a preview purchase (dry_run=true) is +// recorded with status "skipped", dry_run=true, the cudly-mcp source, a +// non-empty run_id, and the term converted to months. Not parallel: +// t.Setenv forbids it. +func TestPreviewWritesSkippedRecord(t *testing.T) { + path := filepath.Join(t.TempDir(), "nested", "audit.jsonl") + t.Setenv(EnvAuditLog, path) + + resp, err := ExecutePurchase(context.Background(), PurchaseRequest{ + Region: "us-east-1", Recommendation: auditTestRecommendation(), DryRun: true, + CredentialScope: "audit-scope-preview", + }) + require.NoError(t, err) + require.NotNil(t, resp) + + lines := readAuditLines(t, path) + require.Len(t, lines, 1, "a preview must write exactly one audit line") + var raw map[string]any + require.NoError(t, json.Unmarshal([]byte(lines[0]), &raw)) + assert.Equal(t, "audit-scope-preview", raw["credential_scope"]) + + var record common.AuditRecord + require.NoError(t, json.Unmarshal([]byte(lines[0]), &record)) + assert.Equal(t, "skipped", record.Status) + assert.True(t, record.DryRun) + assert.Equal(t, common.PurchaseSourceMCP, record.Source) + assert.Equal(t, "audit-scope-preview", record.CredentialScope) + assert.NotEmpty(t, record.RunID) + assert.Equal(t, 12, record.Term, `a "1yr" term must convert to 12 months`) +} + +func TestPreviewAuditOmitsAmbientCredentialScopeWhenOverrideOmitted(t *testing.T) { + path := filepath.Join(t.TempDir(), "audit.jsonl") + t.Setenv(EnvAuditLog, path) + t.Setenv("AWS_PROFILE", "ambient-aws-profile") + t.Setenv("AZURE_SUBSCRIPTION_ID", "ABC12345-1234-1234-1234-1234567890AB") + + simpleTools := simpleToolConstructors() + cases := make([]struct { + name string + call func(*testing.T) + }, 0, 5+len(simpleTools)) + cases = append(cases, []struct { + name string + call func(*testing.T) + }{ + { + name: "aws ec2", + call: func(t *testing.T) { + args := validEC2Args() + args.AWSProfile = "" + _, resp, err := (&awsEC2RIPurchaseTool{}).handle(context.Background(), nil, args) + require.NoError(t, err) + assert.True(t, resp.DryRun) + }, + }, + { + name: "aws elasticache", + call: func(t *testing.T) { + args := validElastiCacheArgs() + args.AWSProfile = "" + _, resp, err := (&awsElastiCacheRIPurchaseTool{}).handle(context.Background(), nil, args) + require.NoError(t, err) + assert.True(t, resp.DryRun) + }, + }, + { + name: "aws rds", + call: func(t *testing.T) { + args := validRDSArgs() + args.AWSProfile = "" + _, resp, err := (&awsRDSRIPurchaseTool{}).handle(context.Background(), nil, args) + require.NoError(t, err) + assert.True(t, resp.DryRun) + }, + }, + { + name: "aws savings plans", + call: func(t *testing.T) { + args := validSavingsPlansArgs() + args.AWSProfile = "" + _, resp, err := (&awsSavingsPlansPurchaseTool{}).handle(context.Background(), nil, args) + require.NoError(t, err) + assert.True(t, resp.DryRun) + }, + }, + { + name: "azure compute", + call: func(t *testing.T) { + args := validAzureComputeArgs() + args.AzureSubscriptionID = "" + _, resp, err := (&azureComputeRIPurchaseTool{}).handle(context.Background(), nil, args) + require.NoError(t, err) + assert.True(t, resp.DryRun) + }, + }, + }...) + for product, ctor := range simpleTools { + cases = append(cases, struct { + name string + call func(*testing.T) + }{ + name: "aws " + product, + call: func(t *testing.T) { + args := validSimpleArgs() + args.AWSProfile = "" + tool := ctor().(*simpleAWSRIPurchaseTool) + _, resp, err := tool.handle(context.Background(), nil, args) + require.NoError(t, err) + assert.True(t, resp.DryRun) + }, + }) + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + before := 0 + if _, err := os.Stat(path); err == nil { + before = len(readAuditLines(t, path)) + } else { + require.ErrorIs(t, err, os.ErrNotExist) + } + + tc.call(t) + + lines := readAuditLines(t, path) + require.Len(t, lines, before+1) + + var raw map[string]any + require.NoError(t, json.Unmarshal([]byte(lines[len(lines)-1]), &raw)) + assert.NotContains(t, raw, "credential_scope") + }) + } +} + +func TestPreviewAuditLowercasesExplicitAzureCredentialScope(t *testing.T) { + path := filepath.Join(t.TempDir(), "audit.jsonl") + t.Setenv(EnvAuditLog, path) + t.Setenv("AZURE_SUBSCRIPTION_ID", "99999999-9999-9999-9999-999999999999") + + args := validAzureComputeArgs() + args.AzureSubscriptionID = "ABC12345-1234-1234-1234-1234567890AB" + _, resp, err := (&azureComputeRIPurchaseTool{}).handle(context.Background(), nil, args) + require.NoError(t, err) + require.True(t, resp.DryRun) + + lines := readAuditLines(t, path) + require.Len(t, lines, 1) + + var raw map[string]any + require.NoError(t, json.Unmarshal([]byte(lines[0]), &raw)) + assert.Equal(t, "abc12345-1234-1234-1234-1234567890ab", raw["credential_scope"]) +} + +func TestRequestCredentialScopeRespectsPurchaseMode(t *testing.T) { + const envVar = "CUDLY_TEST_PURCHASE_SCOPE" + t.Setenv(envVar, "ambient-scope") + + assert.Empty(t, requestCredentialScope("", true, envVar), + "a preview without an explicit scope must not record an ambient provider fallback") + assert.Equal(t, "explicit-scope", requestCredentialScope(" explicit-scope ", true, envVar)) + assert.Equal(t, "ambient-scope", requestCredentialScope("", false, envVar), + "a real purchase must keep the same ambient fallback used by provider resolution and idempotency") + assert.Equal(t, "explicit-scope", requestCredentialScope(" explicit-scope ", false, envVar)) +} + +// TestSuccessfulPurchaseWritesSuccessRecord proves a completed real purchase +// is recorded with status "success" and the provider's CommitmentID. Not +// parallel: t.Setenv forbids it. +func TestSuccessfulPurchaseWritesSuccessRecord(t *testing.T) { + path := filepath.Join(t.TempDir(), "audit.jsonl") + t.Setenv(EnvAuditLog, path) + + fake := &fakeServiceClient{purchaseResult: common.PurchaseResult{Success: true, CommitmentID: "ri-success-1"}} + resp, err := ExecutePurchase(context.Background(), PurchaseRequest{ + Region: "us-east-1", Recommendation: auditTestRecommendation(), DryRun: false, Confirm: true, + CredentialScope: "test-scope", + ResolveClient: func(_ context.Context) (provider.ServiceClient, error) { return fake, nil }, + }) + require.NoError(t, err) + require.NotNil(t, resp) + + lines := readAuditLines(t, path) + require.Len(t, lines, 1) + var record common.AuditRecord + require.NoError(t, json.Unmarshal([]byte(lines[0]), &record)) + assert.Equal(t, "success", record.Status) + assert.False(t, record.DryRun) + assert.Equal(t, "ri-success-1", record.CommitmentID) + assert.Equal(t, "test-scope", record.CredentialScope) +} + +// TestProviderErrorWritesErrorRecord proves a provider-side purchase failure +// (Go error from PurchaseCommitment) is recorded with status "error" and a +// non-empty error message, and that ExecutePurchase still returns its error +// to the caller unchanged. Not parallel: t.Setenv forbids it. +func TestProviderErrorWritesErrorRecord(t *testing.T) { + path := filepath.Join(t.TempDir(), "audit.jsonl") + t.Setenv(EnvAuditLog, path) + + fake := &fakeServiceClient{purchaseErr: errors.New("AWS API: InsufficientInstanceCapacity")} + resp, err := ExecutePurchase(context.Background(), PurchaseRequest{ + Region: "us-east-1", Recommendation: auditTestRecommendation(), DryRun: false, Confirm: true, + CredentialScope: "test-scope", + ResolveClient: func(_ context.Context) (provider.ServiceClient, error) { return fake, nil }, + }) + require.Error(t, err, "the audit write must not swallow or change the provider error") + assert.Nil(t, resp) + assert.Contains(t, err.Error(), "InsufficientInstanceCapacity") + + lines := readAuditLines(t, path) + require.Len(t, lines, 1) + var record common.AuditRecord + require.NoError(t, json.Unmarshal([]byte(lines[0]), &record)) + assert.Equal(t, "error", record.Status) + assert.NotEmpty(t, record.ErrorMessage) + assert.Equal(t, "test-scope", record.CredentialScope) +} + +// TestProviderReportedFailureMapsToErrorStatus proves a PurchaseResult with +// Success=false (no Go error) is recorded as "error", never "success". Not +// parallel: t.Setenv forbids it. +func TestProviderReportedFailureMapsToErrorStatus(t *testing.T) { + path := filepath.Join(t.TempDir(), "audit.jsonl") + t.Setenv(EnvAuditLog, path) + + fake := &fakeServiceClient{purchaseResult: common.PurchaseResult{Success: false}} + resp, err := ExecutePurchase(context.Background(), PurchaseRequest{ + Region: "us-east-1", Recommendation: auditTestRecommendation(), DryRun: false, Confirm: true, + CredentialScope: "test-scope", + ResolveClient: func(_ context.Context) (provider.ServiceClient, error) { return fake, nil }, + }) + require.NoError(t, err, "a provider-reported failure surfaces via the response, not a Go error") + require.NotNil(t, resp) + assert.False(t, resp.Success) + assert.Empty(t, resp.Error, "audit enrichment must not alter the provider response") + + lines := readAuditLines(t, path) + require.Len(t, lines, 1) + var record common.AuditRecord + require.NoError(t, json.Unmarshal([]byte(lines[0]), &record)) + assert.Equal(t, "error", record.Status, "Success=false must never be recorded as success") + assert.Equal(t, "provider reported failure with no error detail", record.ErrorMessage) + assert.Equal(t, "test-scope", record.CredentialScope) +} + +// TestTwoPurchasesShareOneRunID proves every purchase in one process +// correlates through the same auditRunID. Not parallel: t.Setenv forbids it. +func TestTwoPurchasesShareOneRunID(t *testing.T) { + path := filepath.Join(t.TempDir(), "audit.jsonl") + t.Setenv(EnvAuditLog, path) + + for i := 0; i < 2; i++ { + fake := &fakeServiceClient{purchaseResult: common.PurchaseResult{Success: true, CommitmentID: "ri-run"}} + _, err := ExecutePurchase(context.Background(), PurchaseRequest{ + Region: "us-east-1", Recommendation: auditTestRecommendation(), DryRun: false, Confirm: true, + CredentialScope: "test-scope", + ResolveClient: func(_ context.Context) (provider.ServiceClient, error) { return fake, nil }, + }) + require.NoError(t, err) + } + + lines := readAuditLines(t, path) + require.Len(t, lines, 2) + + var first, second common.AuditRecord + require.NoError(t, json.Unmarshal([]byte(lines[0]), &first)) + require.NoError(t, json.Unmarshal([]byte(lines[1]), &second)) + assert.NotEmpty(t, first.RunID) + assert.Equal(t, first.RunID, second.RunID) +} + +// TestUnwritablePathWarnsAndDoesNotChangeResult proves that when the audit +// log cannot be written, recordPurchaseAudit warns on stderr naming the path +// and error, and ExecutePurchase still returns its normal, unmodified +// result. The path's parent is a regular file (not a directory), so +// descriptor-relative directory preparation fails regardless of process +// privileges (a chmod-based unwritable directory is ineffective when running +// as root, and CI may run as root). Not parallel: t.Setenv forbids it, and +// this test swaps the shared standard-logger output. +func TestUnwritablePathWarnsAndDoesNotChangeResult(t *testing.T) { + dir := t.TempDir() + blockingFile := filepath.Join(dir, "not-a-directory") + require.NoError(t, os.WriteFile(blockingFile, []byte("x"), 0o600)) + badPath := filepath.Join(blockingFile, "mcp-audit.jsonl") + t.Setenv(EnvAuditLog, badPath) + + resp, out := executeSuccessfulPurchaseWithCapturedAuditWarning(t) + assert.True(t, resp.Success) + assert.Equal(t, "ri-unwritable", resp.CommitmentID) + assert.Contains(t, out, "mcp audit log") + assert.Contains(t, out, badPath) +} + +func executeSuccessfulPurchaseWithCapturedAuditWarning(t *testing.T) (*PurchaseResponse, string) { + t.Helper() + var buf bytes.Buffer + prevOut, prevFlags := log.Writer(), log.Flags() + log.SetOutput(&buf) + log.SetFlags(0) + t.Cleanup(func() { + log.SetOutput(prevOut) + log.SetFlags(prevFlags) + }) + + fake := &fakeServiceClient{purchaseResult: common.PurchaseResult{Success: true, CommitmentID: "ri-unwritable"}} + resp, err := ExecutePurchase(context.Background(), PurchaseRequest{ + Region: "us-east-1", Recommendation: auditTestRecommendation(), DryRun: false, Confirm: true, + CredentialScope: "test-scope", + ResolveClient: func(_ context.Context) (provider.ServiceClient, error) { return fake, nil }, + }) + require.NoError(t, err) + require.NotNil(t, resp) + return resp, buf.String() +} diff --git a/tools/aws_ec2_ri.go b/tools/aws_ec2_ri.go index bc36e25..67110ef 100644 --- a/tools/aws_ec2_ri.go +++ b/tools/aws_ec2_ri.go @@ -103,7 +103,7 @@ func (t *awsEC2RIPurchaseTool) handle(ctx context.Context, _ *mcp.CallToolReques Confirm: confirm, ResolveClient: t.resolveClient(args, region), Nonce: args.IdempotencyNonce, - CredentialScope: CredentialScope(args.AWSProfile, "AWS_PROFILE"), + CredentialScope: requestCredentialScope(args.AWSProfile, dryRun, "AWS_PROFILE"), }) if err != nil { return nil, PurchaseResponse{}, err diff --git a/tools/aws_elasticache_ri.go b/tools/aws_elasticache_ri.go index c1d0695..008fa0d 100644 --- a/tools/aws_elasticache_ri.go +++ b/tools/aws_elasticache_ri.go @@ -95,7 +95,7 @@ func (t *awsElastiCacheRIPurchaseTool) handle(ctx context.Context, _ *mcp.CallTo Confirm: confirm, ResolveClient: t.resolveClient(args, region), Nonce: args.IdempotencyNonce, - CredentialScope: CredentialScope(args.AWSProfile, "AWS_PROFILE"), + CredentialScope: requestCredentialScope(args.AWSProfile, dryRun, "AWS_PROFILE"), }) if err != nil { return nil, PurchaseResponse{}, err diff --git a/tools/aws_rds_ri.go b/tools/aws_rds_ri.go index 5fbe393..3aeace0 100644 --- a/tools/aws_rds_ri.go +++ b/tools/aws_rds_ri.go @@ -98,7 +98,7 @@ func (t *awsRDSRIPurchaseTool) handle(ctx context.Context, _ *mcp.CallToolReques Confirm: confirm, ResolveClient: t.resolveClient(args, region), Nonce: args.IdempotencyNonce, - CredentialScope: CredentialScope(args.AWSProfile, "AWS_PROFILE"), + CredentialScope: requestCredentialScope(args.AWSProfile, dryRun, "AWS_PROFILE"), }) if err != nil { return nil, PurchaseResponse{}, err diff --git a/tools/aws_savingsplans.go b/tools/aws_savingsplans.go index 400382d..1b74e4d 100644 --- a/tools/aws_savingsplans.go +++ b/tools/aws_savingsplans.go @@ -113,7 +113,7 @@ func (t *awsSavingsPlansPurchaseTool) handle(ctx context.Context, _ *mcp.CallToo Confirm: confirm, ResolveClient: t.resolveClient(args, region, rec.Service), Nonce: args.IdempotencyNonce, - CredentialScope: CredentialScope(args.AWSProfile, "AWS_PROFILE"), + CredentialScope: requestCredentialScope(args.AWSProfile, dryRun, "AWS_PROFILE"), }) if err != nil { return nil, PurchaseResponse{}, err diff --git a/tools/aws_simple_ri.go b/tools/aws_simple_ri.go index 9f83445..10a69d2 100644 --- a/tools/aws_simple_ri.go +++ b/tools/aws_simple_ri.go @@ -157,7 +157,7 @@ func (t *simpleAWSRIPurchaseTool) handle(ctx context.Context, _ *mcp.CallToolReq Confirm: confirm, ResolveClient: t.resolveClient(args, region), Nonce: args.IdempotencyNonce, - CredentialScope: CredentialScope(args.AWSProfile, "AWS_PROFILE"), + CredentialScope: requestCredentialScope(args.AWSProfile, dryRun, "AWS_PROFILE"), }) if err != nil { return nil, PurchaseResponse{}, err diff --git a/tools/azure_compute_ri.go b/tools/azure_compute_ri.go index 6492c0f..48dd2e7 100644 --- a/tools/azure_compute_ri.go +++ b/tools/azure_compute_ri.go @@ -130,6 +130,10 @@ func azureCredentialScope(explicit string, envVars ...string) string { return strings.ToLower(CredentialScope(explicit, envVars...)) } +func azureRequestCredentialScope(explicit string, dryRun bool, envVars ...string) string { + return strings.ToLower(requestCredentialScope(explicit, dryRun, envVars...)) +} + func (t *azureComputeRIPurchaseTool) handle(ctx context.Context, _ *mcp.CallToolRequest, args azureComputeRIPurchaseArgs) (*mcp.CallToolResult, PurchaseResponse, error) { rec, region, dryRun, confirm, err := azureComputeRecommendationFromArgs(args) if err != nil { @@ -143,7 +147,7 @@ func (t *azureComputeRIPurchaseTool) handle(ctx context.Context, _ *mcp.CallTool Confirm: confirm, ResolveClient: t.resolveClient(args, region), Nonce: args.IdempotencyNonce, - CredentialScope: azureCredentialScope(args.AzureSubscriptionID, "AZURE_SUBSCRIPTION_ID"), + CredentialScope: azureRequestCredentialScope(args.AzureSubscriptionID, dryRun, "AZURE_SUBSCRIPTION_ID"), }) if err != nil { return nil, PurchaseResponse{}, err diff --git a/tools/purchase.go b/tools/purchase.go index 06ac5aa..e4fea13 100644 --- a/tools/purchase.go +++ b/tools/purchase.go @@ -2,6 +2,7 @@ package tools import ( "context" + "errors" "fmt" "log" "os" @@ -94,8 +95,9 @@ type PurchaseRequest struct { // Azure subscription, or GCP project the call is billed to. It is folded // into the idempotency key so two purchases that are identical in every // product dimension but target different accounts derive DIFFERENT - // tokens. Populated by each tool via CredentialScope(). See - // idempotencyKeyFor for why omitting it is a double-spend/skipped-spend + // tokens. Real purchases include the same ambient fallback as provider + // resolution; previews record only an explicit caller-supplied scope. + // See idempotencyKeyFor for why omitting it is a double-spend/skipped-spend // hazard on Azure specifically. CredentialScope string } @@ -131,6 +133,16 @@ func CredentialScope(explicit string, envVars ...string) string { return "" } +// requestCredentialScope resolves the audit/idempotency scope for the selected +// mode: previews must not record an ambient account they never resolved, while +// real purchases must keep the provider-env fallback used by authorization. +func requestCredentialScope(explicit string, dryRun bool, envVars ...string) string { + if dryRun { + return CredentialScope(explicit) + } + return CredentialScope(explicit, envVars...) +} + // credentialScopeSource describes where a provider's credential scope can come // from: the tool argument that supplies it, and the ambient environment // variable that can supply it instead, if the provider has one. @@ -397,6 +409,10 @@ func rfc3339OrNil(t time.Time) *string { return &s } +func purchaseSucceeded(result common.PurchaseResult) bool { + return result.Success && result.Error == nil +} + // idempotencyKeyFor derives a stable per-request key from every field that // identifies what is being bought: provider, region, service, resource type, // count, term, payment option, plus every service-specific dimension held in @@ -514,21 +530,18 @@ func savingsPlanDetailsKey(d *common.SavingsPlanDetails) string { d.PlanType, d.HourlyCommitment, d.Coverage, d.InstanceFamily, d.Region, d.OfferingID) } -// logPurchaseAttempt and logPurchaseOutcome write the MCP server's audit -// trail for real, money-spending purchases. Without them an MCP purchase -// left no record anywhere: the CLI path emits a common.AuditRecord per -// purchase (cmd/multi_service.go) and the web path persists a -// purchase_executions row that also carries the approval history, but this -// server has neither, so an operator asking "what did the assistant buy?" -// had nothing to read. These lines are that record. +// logPurchaseAttempt and logPurchaseOutcome write the MCP server's stderr +// diagnostic trail for real, money-spending purchases. These lines are the +// live human-readable counterpart to the server's durable JSONL audit log. // // They go to the standard logger, which writes to STDERR. That is load // bearing: the MCP stdio transport owns stdout for JSON-RPC framing, so // anything written there would corrupt the protocol stream. // -// Preview calls are deliberately not logged: they contact no provider and -// spend nothing, and logging every dry run would bury the real purchases in -// the noise they need to stand out from. +// Preview calls deliberately omit purchase attempt/outcome diagnostics on +// stderr: they contact no provider and spend nothing, and logging every dry +// run would bury real purchases. recordPurchaseAudit still runs for each +// preview; audit path or write failures may emit an "mcp audit log" warning. // // The idempotency token is masked (common.MaskToken) rather than written in // full, matching how every provider client logs it: it is a stable @@ -540,6 +553,8 @@ func logPurchaseAttempt(req PurchaseRequest, rec common.Recommendation, token st rec.Term, rec.PaymentOption, common.MaskToken(token)) } +const providerFailureWithoutDetail = "provider reported failure with no error detail" + // logPurchaseOutcome logs FAILED whenever success is false, even if err is // nil: a provider can report PurchaseResult{Success: false, Error: nil} (no // Go error, no result.Error, just a plain "did not buy anything"), and that @@ -547,7 +562,7 @@ func logPurchaseAttempt(req PurchaseRequest, rec common.Recommendation, token st func logPurchaseOutcome(rec common.Recommendation, token, commitmentID string, success bool, err error) { if !success { if err == nil { - err = fmt.Errorf("provider reported failure with no error detail") + err = errors.New(providerFailureWithoutDetail) } log.Printf("mcp purchase FAILED: provider=%s resource=%s token=%s: %v", rec.Provider, rec.ResourceType, common.MaskToken(token), err) @@ -603,6 +618,7 @@ func ExecutePurchase(ctx context.Context, req PurchaseRequest) (*PurchaseRespons rec := req.Recommendation if mode == modePreview { + recordPurchaseAudit(rec, req.CredentialScope, common.PurchaseResult{DryRun: true}, auditStatusSkipped, true) return &PurchaseResponse{ Success: true, DryRun: true, @@ -638,14 +654,20 @@ func ExecutePurchase(ctx context.Context, req PurchaseRequest) (*PurchaseRespons result, err := client.PurchaseCommitment(ctx, rec, opts) if err != nil { logPurchaseOutcome(rec, token, "", false, err) + recordPurchaseAudit(rec, req.CredentialScope, common.PurchaseResult{Error: err}, auditStatusError, false) // Full provider error text surfaces to the caller (feedback: // providers must never swallow the underlying SDK/HTTP error). return nil, fmt.Errorf("purchase commitment failed: %w", err) } - logPurchaseOutcome(rec, token, result.CommitmentID, result.Success, result.Error) + auditResult := result + if !purchaseSucceeded(auditResult) && auditResult.Error == nil { + auditResult.Error = errors.New(providerFailureWithoutDetail) + } + logPurchaseOutcome(rec, token, auditResult.CommitmentID, purchaseSucceeded(auditResult), auditResult.Error) + recordPurchaseAudit(rec, req.CredentialScope, auditResult, auditStatusFor(auditResult), false) resp := &PurchaseResponse{ - Success: result.Success, + Success: purchaseSucceeded(result), DryRun: result.DryRun, CommitmentID: result.CommitmentID, Cost: nonZeroCostPtr(result.Cost), @@ -663,7 +685,7 @@ func ExecutePurchase(ctx context.Context, req PurchaseRequest) (*PurchaseRespons // Success=false (or carries an Error) bought nothing, so pitching a // 7-day window against a purchase that did not happen would be wrong on // the facts, not merely premature. - if result.Success && result.Error == nil { + if purchaseSucceeded(result) { resp.Archera = archeraOffer() } return resp, nil diff --git a/tools/purchase_test.go b/tools/purchase_test.go index 8772982..b2a5c44 100644 --- a/tools/purchase_test.go +++ b/tools/purchase_test.go @@ -7,6 +7,7 @@ import ( "errors" "log" "os" + "path/filepath" "testing" "time" @@ -28,11 +29,36 @@ import ( // TestExecutePurchaseRealPurchaseGate is the one test that deliberately // overrides this default, and it does so non-parallel (see its doc comment) // so no parallel test in this package ever observes a transient override. +// +// It also pins EnvAuditLog to a path inside a run-scoped temp directory. +// Auditing is on by default (see EnvAuditLog), so without this every test in +// this file that calls ExecutePurchase would append to the developer's real +// ~/.local/state/cudly/mcp-audit.jsonl. Individual tests that need to assert +// on audit file contents override this with their own t.Setenv. func TestMain(m *testing.M) { if err := os.Setenv(EnvEnableRealPurchases, "1"); err != nil { - panic(err) + log.Printf("enable real purchases for MCP tests: %v", err) + os.Exit(1) + } + + auditDir, err := os.MkdirTemp("", "cudly-mcp-audit-testmain") + if err != nil { + log.Printf("create MCP tool audit test directory: %v", err) + os.Exit(1) + } + if err := os.Setenv(EnvAuditLog, filepath.Join(auditDir, "mcp-audit.jsonl")); err != nil { + log.Printf("set MCP tool audit log test path: %v", err) + if cleanupErr := os.RemoveAll(auditDir); cleanupErr != nil { + log.Printf("remove MCP tool audit test directory after setup failure: %v", cleanupErr) + } + os.Exit(1) } - os.Exit(m.Run()) + + // os.Exit skips deferred calls, so the temp dir is removed explicitly + // before exiting rather than via defer. + code := m.Run() + os.RemoveAll(auditDir) + os.Exit(code) } // fakeServiceClient is a minimal provider.ServiceClient test double. Only @@ -71,6 +97,22 @@ func (f *fakeServiceClient) GetValidResourceTypes(_ context.Context) ([]string, var _ provider.ServiceClient = (*fakeServiceClient)(nil) +func captureStandardLogger(t *testing.T, fn func()) string { + t.Helper() + + var buf bytes.Buffer + prevOut, prevFlags := log.Writer(), log.Flags() + log.SetOutput(&buf) + log.SetFlags(0) + defer func() { + log.SetOutput(prevOut) + log.SetFlags(prevFlags) + }() + + fn() + return buf.String() +} + // testRecommendation mirrors what a real purchase tool's *FromArgs // constructor actually builds: none of them populate // OnDemandCost/CommitmentCost/EstimatedSavings/SavingsPercentage (they build @@ -776,46 +818,31 @@ func TestCredentialScopeResolution(t *testing.T) { }) } -// TestExecutePurchaseAuditLogging pins the MCP server's only record of a -// real purchase. The CLI path emits a common.AuditRecord per purchase -// (cmd/multi_service.go) and the web path persists a purchase_executions row -// carrying the approval history; this server has neither, so before these -// log lines an operator asking "what did the assistant actually buy?" had -// nothing at all to read. +// TestExecutePurchaseAuditLogging pins the MCP server's stderr diagnostic +// trail for a real purchase. The durable JSONL audit log is covered +// separately in audit_test.go. // -// It also pins that a preview stays silent (it contacts no provider and -// spends nothing, so logging every dry run would bury the real purchases) -// and that the idempotency token is masked rather than written in full. +// It also pins that a preview emits no purchase diagnostics on stderr and that +// the idempotency token is masked rather than written in full. +// recordPurchaseAudit still runs; TestUnwritablePathWarnsAndDoesNotChangeResult +// covers its "mcp audit log" warning when the audit path cannot be written. func TestExecutePurchaseAuditLogging(t *testing.T) { // Not parallel: this test swaps the shared standard-logger output. - capture := func(fn func()) string { - var buf bytes.Buffer - prevOut, prevFlags := log.Writer(), log.Flags() - log.SetOutput(&buf) - log.SetFlags(0) - defer func() { - log.SetOutput(prevOut) - log.SetFlags(prevFlags) - }() - fn() - return buf.String() - } - rec := testRecommendation() t.Run("a preview logs nothing", func(t *testing.T) { - out := capture(func() { + out := captureStandardLogger(t, func() { _, err := ExecutePurchase(context.Background(), PurchaseRequest{ Region: "us-east-1", Recommendation: rec, DryRun: true, }) require.NoError(t, err) }) - assert.Empty(t, out, "a dry run spends nothing and must not pollute the purchase audit trail") + assert.Empty(t, out, "a dry run spends nothing and must not pollute the stderr purchase diagnostic trail") }) t.Run("a real purchase logs the attempt and the outcome", func(t *testing.T) { fake := &fakeServiceClient{purchaseResult: common.PurchaseResult{Success: true, CommitmentID: "ri-abc123"}} - out := capture(func() { + out := captureStandardLogger(t, func() { _, err := ExecutePurchase(context.Background(), PurchaseRequest{ Region: "us-east-1", Recommendation: rec, DryRun: false, Confirm: true, CredentialScope: "subscription-a", @@ -837,7 +864,7 @@ func TestExecutePurchaseAuditLogging(t *testing.T) { t.Run("a failed purchase logs the failure", func(t *testing.T) { fake := &fakeServiceClient{purchaseErr: errors.New("insufficient capacity")} - out := capture(func() { + out := captureStandardLogger(t, func() { _, err := ExecutePurchase(context.Background(), PurchaseRequest{ Region: "us-east-1", Recommendation: rec, DryRun: false, Confirm: true, CredentialScope: "test-scope", ResolveClient: func(_ context.Context) (provider.ServiceClient, error) { return fake, nil }, @@ -858,7 +885,7 @@ func TestExecutePurchaseAuditLogging(t *testing.T) { // result.Success. t.Run("a provider-reported failure is never logged as OK", func(t *testing.T) { fake := &fakeServiceClient{purchaseResult: common.PurchaseResult{Success: false, Error: nil}} - out := capture(func() { + out := captureStandardLogger(t, func() { _, err := ExecutePurchase(context.Background(), PurchaseRequest{ Region: "us-east-1", Recommendation: rec, DryRun: false, Confirm: true, CredentialScope: "test-scope", ResolveClient: func(_ context.Context) (provider.ServiceClient, error) { return fake, nil }, @@ -871,6 +898,51 @@ func TestExecutePurchaseAuditLogging(t *testing.T) { }) } +// TestExecutePurchaseContradictoryProviderResultIsFailure pins the +// contradictory provider result found in review: a provider can return a +// nil Go error while embedding a non-nil result.Error alongside +// Success=true. Every consumer-facing signal must still classify that as a +// failed purchase. +func TestExecutePurchaseContradictoryProviderResultIsFailure(t *testing.T) { + // Not parallel: this test swaps the shared standard-logger output and + // sets a process environment override for the audit path. + path := filepath.Join(t.TempDir(), "audit.jsonl") + t.Setenv(EnvAuditLog, path) + + injectedErr := errors.New("provider returned contradictory success with error detail") + fake := &fakeServiceClient{purchaseResult: common.PurchaseResult{ + Success: true, + CommitmentID: "ri-contradictory", + Error: injectedErr, + }} + + var resp *PurchaseResponse + out := captureStandardLogger(t, func() { + var err error + resp, err = ExecutePurchase(context.Background(), PurchaseRequest{ + Region: "us-east-1", Recommendation: testRecommendation(), DryRun: false, Confirm: true, + CredentialScope: "test-scope", + ResolveClient: func(_ context.Context) (provider.ServiceClient, error) { return fake, nil }, + }) + require.NoError(t, err, "provider-level contradictory result must surface via the structured response") + }) + + require.NotNil(t, resp) + assert.False(t, resp.Success) + assert.Equal(t, injectedErr.Error(), resp.Error) + assert.Nil(t, resp.Archera) + assert.Contains(t, out, "mcp purchase FAILED") + assert.Contains(t, out, injectedErr.Error()) + assert.NotContains(t, out, "mcp purchase OK") + + lines := readAuditLines(t, path) + require.Len(t, lines, 1) + var record common.AuditRecord + require.NoError(t, json.Unmarshal([]byte(lines[0]), &record)) + assert.Equal(t, "error", record.Status) + assert.Equal(t, injectedErr.Error(), record.ErrorMessage) +} + // TestResolveDryRunConfirm pins the shared default resolution now used by // every purchase tool. This is the gate that decides whether real money // moves, and it was previously hand-copied into seven files; the single