Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 30 additions & 10 deletions docs/AUTH.md
Original file line number Diff line number Diff line change
Expand Up @@ -277,32 +277,52 @@ Before a record is written, parameters are walked and any of the following keys

The redaction is shallow (top-level keys only). Tool authors who add new sensitive fields must add them to `sensitiveParamKeys` in `internal/mcp/server.go`.

### Limitations
### Signed audit log (opt-in)

- **No per-operator identity is captured.** The audit log says "the credential called users_delete," not "Jane called users_delete." Two operators sharing one API key are indistinguishable in the log.
- **No signature.** A laptop-local attacker who can write to the log file can forge or scrub entries.
- **Stdio transport carries no session metadata** — there is no client identifier in the record beyond the tool call itself.
Enable `mcp.sign_destructive_ops: true` (or pass `--sign-destructive` to `jc mcp serve`) to emit a tamper-evident manifest to `~/.config/jc/mcp-audit-signed.log` for every successful destructive op. Each manifest carries an Ed25519 signature over the tool name, redacted args, target, timestamp, and a 32-byte nonce.

Storage:

- A per-profile keypair is generated lazily on the first signed op.
- Private key lives in the OS keychain at `service="jc"`, account `"<profile>:signing_key"`. No plaintext fallback — fail closed if the keychain is unavailable.
- Public key is persisted to config at `profiles.<name>.signing_pubkey` so verifiers can trust the chain on first use without consulting the keychain.

This is tracked in [KLA-408](https://linear.app/klaassenconsulting/issue/KLA-408): the proposed Slice 2 is a signed manifest per destructive op (Ed25519 signature over `{tool, args, target_ids, timestamp, nonce, operator_pubkey}`).
Verification:

```bash
jc audit verify # active profile, default log path
jc audit verify --profile production # named profile
jc audit verify --pubkey <base64> # override the configured pubkey
jc audit verify --log /path/to/audit.log # alternate log path
```

A successful run reports the count of verified records and exits 0; any signature mismatch, truncation, or decode error exits non-zero with the offending manifest's nonce + tool so operators can grep the file.

This is detection, not prevention: a successful credential exfiltration still produces forensic record signed with the original keypair, which is hard to forge without also stealing the keychain entry. Pair it with TTY step-up (`mcp.require_step_up_for_destructive`) for in-the-loop pause + signed trail.

### Other audit-log limitations

- **No per-operator identity at the JumpCloud Console level.** The signed manifest binds an op to *the local Ed25519 keypair*, not to a specific person — multiple operators sharing one jc profile are still indistinguishable. Real per-user audit identity needs OAuth Device Flow, tracked in [KLA-414](https://linear.app/klaassenconsulting/issue/KLA-414).
- **Stdio transport carries no session metadata** — there is no client identifier in the record beyond the tool call itself.

## Threat model summary

| Scenario | What protects you today | What doesn't |
|---|---|---|
| **Laptop is online, operator is at the keyboard** | TTY confirmation, `--plan`, `--read-only` MCP mode, keychain storage | — |
| **Laptop is stolen with screen unlocked** | Credentials are in the keychain, not on disk; the OS lock screen is the boundary | jc itself does not require re-auth on resume |
| **API key exfiltrated** | Validated keys hit the org's normal API rate limits and audit log on JumpCloud | jc has no key-rotation reminder, no per-op fingerprinting, no anomaly detection |
| **Compromised MCP agent prompt** | Read-only mode, tool allow/block lists, rate limiting, the `execute: true` gate | The agent controls `execute: true`; nothing forces a human in the loop |
| **API key exfiltrated** | Validated keys hit the org's normal API rate limits and audit log on JumpCloud; `mcp.sign_destructive_ops` produces a tamper-evident local trail | jc has no key-rotation reminder; signed manifests detect rather than prevent |
| **Compromised MCP agent prompt** | Read-only mode, tool allow/block lists, rate limiting, the `execute: true` gate, optional TTY step-up, optional signed-manifest audit trail | The agent controls `execute: true` unless step-up is enabled; signing is post-hoc detection, not prevention |
| **MCP server exposed via tunnel without `--require-auth`** | Loopback default + explicit warning when binding non-loopback without TLS | Once exposed, anyone reaching the URL has full credential privilege |
| **Plaintext config used (`--allow-plaintext`)** | Mode-`0600` config file in a `0700` dir | jc cannot prevent backups, sync clients, or `cat ~/.config/jc/config.yaml` from leaking it |

## What's coming

Active backlog tickets that close gaps called out in this document:

- **[KLA-407](https://linear.app/klaassenconsulting/issue/KLA-407) — Default to OAuth + roadmap to user-bound auth (device flow).** Slice A flips `jc auth login` to recommend OAuth client credentials over personal API keys; Slice B adds OAuth Device Flow once the JumpCloud platform exposes the endpoint.
- **[KLA-408](https://linear.app/klaassenconsulting/issue/KLA-408) — Step-up auth gate for MCP-invoked destructive ops.** Touch ID / PIN reprompt before any `execute: true` destructive call, plus signed-manifest audit entries.
- **[KLA-409](https://linear.app/klaassenconsulting/issue/KLA-409) — MCP server tool allowlist + read-only mode.** Most of this already shipped (allow/block lists, `--read-only`); the remaining slice is profile-level read-only enforcement so a profile bound to a read-only OAuth client can't be misconfigured into a mutation-capable session.
- **[KLA-412](https://linear.app/klaassenconsulting/issue/KLA-412) — Touch ID / WebAuthn step-up authenticator.** macOS Touch ID via `LocalAuthentication.framework` so the prompt works in stdio transport (Claude Desktop). Today's TTY step-up only works in HTTP/SSE transport with a real terminal attached.
- **[KLA-413](https://linear.app/klaassenconsulting/issue/KLA-413) — Out-of-band approval for MCP destructive ops.** Webhook-based dual-control for "delete in production" workflows.
- **[KLA-414](https://linear.app/klaassenconsulting/issue/KLA-414) — OAuth Device Flow.** Per-operator audit identity once JumpCloud exposes `/oauth2/device`. Currently platform-blocked.

## Quick reference

Expand Down
92 changes: 92 additions & 0 deletions internal/cmd/audit.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package cmd

import (
"crypto/ed25519"
"encoding/base64"
"fmt"
"os"
"path/filepath"

"github.com/spf13/cobra"

"github.com/klaassen-consulting/jc/internal/config"
"github.com/klaassen-consulting/jc/internal/mcp"
)

func newAuditCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "audit",
Short: "Inspect and verify MCP audit logs",
Long: "Subcommands for working with MCP audit logs, including signed-manifest verification (KLA-411).",
}

cmd.AddCommand(newAuditVerifyCmd())
return cmd
}

func newAuditVerifyCmd() *cobra.Command {
var profileFlag, logPath, pubkeyOverride string

cmd := &cobra.Command{
Use: "verify",
Short: "Verify signatures in the signed MCP audit log",
Long: `Read every signed manifest in the MCP signed-audit-log and verify the
chain against the configured per-profile public key.

Used to catch tampering with the audit trail. A successful run reports
the count of verified records and exits 0; any signature mismatch,
truncation, or decode error exits with a non-zero code.

By default, reads ~/.config/jc/mcp-audit-signed.log and uses the active
profile's signing pubkey from config (set the first time
'mcp.sign_destructive_ops' is enabled and a destructive op fires).

Pass --pubkey to verify against an externally-attested key — useful when
investigating a host whose config might itself be tampered with.`,
RunE: func(cmd *cobra.Command, args []string) error {
profile := profileFlag
if profile == "" {
profile = config.ActiveProfile()
}

pubkeyB64 := pubkeyOverride
if pubkeyB64 == "" {
pubkeyB64 = config.SigningPubkey(profile)
}
if pubkeyB64 == "" {
return fmt.Errorf("no signing public key for profile %q. Either run a signed destructive op first to bootstrap, or pass --pubkey explicitly", profile)
}
pubBytes, err := base64.StdEncoding.DecodeString(pubkeyB64)
if err != nil {
return fmt.Errorf("decoding stored pubkey: %w", err)
}
if len(pubBytes) != ed25519.PublicKeySize {
return fmt.Errorf("stored pubkey is %d bytes, want %d", len(pubBytes), ed25519.PublicKeySize)
}

path := logPath
if path == "" {
path = filepath.Join(config.ConfigDir(), "mcp-audit-signed.log")
}
f, err := os.Open(path)
if err != nil {
return fmt.Errorf("opening signed audit log %s: %w", path, err)
}
defer f.Close()

verified, verr := mcp.VerifyManifestStream(f, ed25519.PublicKey(pubBytes))
if verr != nil {
fmt.Fprintf(cmd.ErrOrStderr(), "Verified %d manifest(s) before failure: %v\n", verified, verr)
return &ExitError{Code: 1, Err: verr}
}
fmt.Fprintf(cmd.OutOrStdout(), "OK — verified %d signed manifest(s) for profile %q.\n", verified, profile)
return nil
},
}

cmd.Flags().StringVar(&profileFlag, "profile", "", "Profile whose signing pubkey to use (defaults to active profile)")
cmd.Flags().StringVar(&logPath, "log", "", "Path to the signed audit log (defaults to ~/.config/jc/mcp-audit-signed.log)")
cmd.Flags().StringVar(&pubkeyOverride, "pubkey", "", "Verify against this base64 Ed25519 pubkey instead of the configured one (override when investigating a possibly-tampered host)")

return cmd
}
179 changes: 179 additions & 0 deletions internal/cmd/audit_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
package cmd

import (
"bytes"
"crypto/ed25519"
"crypto/rand"
"encoding/base64"
"encoding/json"
"errors"
"os"
"path/filepath"
"strings"
"testing"

"github.com/spf13/cobra"
)

// writeTestManifest writes one JSON-encoded signed manifest line to path.
// The shape mirrors mcp.signedManifest; we hand-roll it here to avoid an
// import cycle with the mcp package's tests.
func writeTestManifest(t *testing.T, path string, priv ed25519.PrivateKey) {
t.Helper()
pub := priv.Public().(ed25519.PublicKey)

type manifest struct {
Tool string `json:"tool"`
ArgsRedacted json.RawMessage `json:"args_redacted"`
Target string `json:"target,omitempty"`
Timestamp string `json:"timestamp"`
Nonce string `json:"nonce"`
OperatorPubkey string `json:"operator_pubkey"`
Signature string `json:"signature,omitempty"`
}

m := manifest{
Tool: "users_delete",
ArgsRedacted: json.RawMessage(`{"identifier":"alice"}`),
Target: "alice",
Timestamp: "2026-04-28T20:00:00Z",
Nonce: base64.StdEncoding.EncodeToString([]byte("nonce-fixture-12345678901234567")),
OperatorPubkey: base64.StdEncoding.EncodeToString(pub),
}
canonical, _ := json.Marshal(m)
m.Signature = base64.StdEncoding.EncodeToString(ed25519.Sign(priv, canonical))

out, _ := json.Marshal(m)
out = append(out, '\n')

if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil {
t.Fatalf("mkdir: %v", err)
}
if err := os.WriteFile(path, out, 0600); err != nil {
t.Fatalf("write: %v", err)
}
}

func TestAuditVerify_OK(t *testing.T) {
setupTestConfig(t, `active_profile: default
profiles:
default:
api_key: ""
`)

pub, priv, _ := ed25519.GenerateKey(rand.Reader)
pubB64 := base64.StdEncoding.EncodeToString(pub)

tmp := t.TempDir()
logPath := filepath.Join(tmp, "mcp-audit-signed.log")
writeTestManifest(t, logPath, priv)

rootCmd := NewRootCmd()
stdout := new(bytes.Buffer)
stderr := new(bytes.Buffer)
rootCmd.SetOut(stdout)
rootCmd.SetErr(stderr)
rootCmd.SetArgs([]string{"audit", "verify", "--log", logPath, "--pubkey", pubB64})

if err := rootCmd.Execute(); err != nil {
t.Fatalf("audit verify failed: %v\nstderr: %s", err, stderr.String())
}
if !strings.Contains(stdout.String(), "OK") {
t.Errorf("expected OK message, got: %s", stdout.String())
}
if !strings.Contains(stdout.String(), "1 signed manifest") {
t.Errorf("expected count=1 in output, got: %s", stdout.String())
}
}

func TestAuditVerify_DetectsTamper(t *testing.T) {
setupTestConfig(t, `active_profile: default
profiles:
default:
api_key: ""
`)

pub, priv, _ := ed25519.GenerateKey(rand.Reader)
pubB64 := base64.StdEncoding.EncodeToString(pub)

tmp := t.TempDir()
logPath := filepath.Join(tmp, "mcp-audit-signed.log")
writeTestManifest(t, logPath, priv)

// Tamper with the on-disk manifest.
data, _ := os.ReadFile(logPath)
tampered := bytes.Replace(data, []byte(`"target":"alice"`), []byte(`"target":"bob"`), 1)
_ = os.WriteFile(logPath, tampered, 0600)

rootCmd := NewRootCmd()
rootCmd.SilenceUsage = true
rootCmd.SilenceErrors = true
rootCmd.SetOut(new(bytes.Buffer))
rootCmd.SetErr(new(bytes.Buffer))
rootCmd.SetArgs([]string{"audit", "verify", "--log", logPath, "--pubkey", pubB64})

err := rootCmd.Execute()
if err == nil {
t.Fatal("expected verify to fail on tampered manifest, got nil")
}
var exitErr *ExitError
if !errors.As(err, &exitErr) {
t.Fatalf("expected ExitError, got %T: %v", err, err)
}
if exitErr.Code == 0 {
t.Errorf("expected non-zero exit code on tamper, got %d", exitErr.Code)
}
}

func TestAuditVerify_NoPubkey(t *testing.T) {
setupTestConfig(t, `active_profile: default
profiles:
default:
api_key: ""
`)

tmp := t.TempDir()
logPath := filepath.Join(tmp, "mcp-audit-signed.log")
if err := os.WriteFile(logPath, []byte(""), 0600); err != nil {
t.Fatal(err)
}

rootCmd := NewRootCmd()
rootCmd.SilenceUsage = true
rootCmd.SilenceErrors = true
rootCmd.SetOut(new(bytes.Buffer))
rootCmd.SetErr(new(bytes.Buffer))
rootCmd.SetArgs([]string{"audit", "verify", "--log", logPath})

err := rootCmd.Execute()
if err == nil {
t.Fatal("expected error when no pubkey configured and no --pubkey passed, got nil")
}
if !strings.Contains(err.Error(), "no signing public key") {
t.Errorf("error should mention missing pubkey, got: %v", err)
}
}

func TestAuditCommandRegistered(t *testing.T) {
rootCmd := NewRootCmd()
var found *cobra.Command
for _, c := range rootCmd.Commands() {
if c.Use == "audit" {
found = c
break
}
}
if found == nil {
t.Fatal("expected 'audit' command to be registered")
}
var verifyFound bool
for _, sub := range found.Commands() {
if sub.Name() == "verify" {
verifyFound = true
break
}
}
if !verifyFound {
t.Error("expected 'verify' subcommand under 'audit'")
}
}
Loading
Loading