From 76ada04442f2e7f207145f52751d046f60a81a72 Mon Sep 17 00:00:00 2001 From: Steve Cliff Date: Tue, 23 Jun 2026 07:18:27 +0100 Subject: [PATCH] refactor(cli): wire commandRole into dispatch; doc + comment cleanup Resolve final-review findings: commandRole is now the single source of truth (Run resolves role once and threads it to handlers, replacing hardcoded openStore roles). Tighten crypto/SKILL/SPEC/USER-MANUAL wording and document init's agent-key-on-first-init-only semantics. Co-Authored-By: Claude Opus 4.8 (1M context) --- USER-MANUAL.md | 3 ++- internal/cli/admin.go | 16 ++++++++-------- internal/cli/run.go | 27 ++++++++++++++------------- internal/crypto/crypto.go | 2 +- internal/store/keys.go | 3 +++ skills/emcli/SKILL.md | 5 ++--- specifications/SPEC.md | 3 ++- 7 files changed, 32 insertions(+), 27 deletions(-) diff --git a/USER-MANUAL.md b/USER-MANUAL.md index 0c7ee5b..4442125 100644 --- a/USER-MANUAL.md +++ b/USER-MANUAL.md @@ -38,7 +38,8 @@ This manual is for **using and administering** `emcli`. It assumes you have the - **Agent commands** (`list`, `get`, `search`, `ack`, `send`, `doctor`) require `EMCLI_KEY` (or `EMCLI_ADMIN_KEY` as a superset) and are for the *agent*. They print one line of JSON and nothing else, so a program can consume them reliably. (`doctor` prints human-readable text but - is authorised by the agent key — the agent or a human with either key can run it.) + is authorised by the agent key — `EMCLI_KEY` alone is sufficient; `EMCLI_ADMIN_KEY` also works + as a superset, so either key suffices for agent commands.) **Accounts** are named (e.g. `gmail`, `work`). The agent refers to an account by name and never sees its password. diff --git a/internal/cli/admin.go b/internal/cli/admin.go index eb96a8e..fc48089 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -11,7 +11,7 @@ import ( ) // runAccount handles `account add|list`. Human-readable output (never JSON). -func runAccount(args []string, out, errOut io.Writer) int { +func runAccount(args []string, role store.Role, out, errOut io.Writer) int { if len(args) == 0 || helpRequested(args[0]) { printCmdUsage(out, "account") fmt.Fprintln(out, "\nSubcommands: add, edit, remove, list") @@ -21,7 +21,7 @@ func runAccount(args []string, out, errOut io.Writer) int { return 2 } sub, rest := args[0], args[1:] - st, err := openStore(store.RoleAdmin) + st, err := openStore(role) if err != nil { fmt.Fprintf(errOut, "emcli: %v\n", err) return 1 @@ -191,7 +191,7 @@ func auditList(st *store.Store, account string, limit int, out io.Writer) error } // runConfig handles `config set ` and `config get `. -func runConfig(args []string, out, errOut io.Writer) int { +func runConfig(args []string, role store.Role, out, errOut io.Writer) int { if len(args) == 0 || helpRequested(args[0]) { printCmdUsage(out, "config") if len(args) > 0 { @@ -204,7 +204,7 @@ func runConfig(args []string, out, errOut io.Writer) int { return 2 } sub, key := args[0], args[1] - st, err := openStore(store.RoleAdmin) + st, err := openStore(role) if err != nil { fmt.Fprintf(errOut, "emcli: %v\n", err) return 1 @@ -246,7 +246,7 @@ func runConfig(args []string, out, errOut io.Writer) int { } // runAudit handles `audit list [--account ] [--limit N]`. -func runAudit(args []string, out, errOut io.Writer) int { +func runAudit(args []string, role store.Role, out, errOut io.Writer) int { if len(args) > 0 && helpRequested(args[0]) { printCmdUsage(out, "audit") return 0 @@ -262,7 +262,7 @@ func runAudit(args []string, out, errOut io.Writer) int { if err := fs.Parse(args[1:]); err != nil { return 2 } - st, err := openStore(store.RoleAdmin) + st, err := openStore(role) if err != nil { fmt.Fprintf(errOut, "emcli: %v\n", err) return 1 @@ -276,7 +276,7 @@ func runAudit(args []string, out, errOut io.Writer) int { } // runWhitelist handles `whitelist add --account NAME --address A`. -func runWhitelist(args []string, out, errOut io.Writer) int { +func runWhitelist(args []string, role store.Role, out, errOut io.Writer) int { if len(args) == 0 || helpRequested(args[0]) { printCmdUsage(out, "whitelist") if len(args) > 0 { @@ -301,7 +301,7 @@ func runWhitelist(args []string, out, errOut io.Writer) int { fmt.Fprintln(errOut, "--account is required") return 2 } - st, err := openStore(store.RoleAdmin) + st, err := openStore(role) if err != nil { fmt.Fprintf(errOut, "emcli: %v\n", err) return 1 diff --git a/internal/cli/run.go b/internal/cli/run.go index 8fd7af0..abeb5c0 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -99,7 +99,7 @@ func newDepsLive(st *store.Store, out io.Writer) Deps { } // runDoctor handles `doctor [--account ]` (human-readable diagnostics). -func runDoctor(args []string, out, errOut io.Writer) int { +func runDoctor(args []string, role store.Role, out, errOut io.Writer) int { fs := flag.NewFlagSet("doctor", flag.ContinueOnError) fs.SetOutput(errOut) usageFlags(fs, "doctor", errOut) @@ -110,7 +110,7 @@ func runDoctor(args []string, out, errOut io.Writer) int { } return 2 } - st, err := openStore(store.RoleAgent) + st, err := openStore(role) if err != nil { fmt.Fprintf(errOut, "emcli: %v\n", err) return 1 @@ -135,21 +135,22 @@ func Run(args []string, out, errOut io.Writer) int { return 0 } cmd, rest := args[0], args[1:] + role := commandRole(cmd) switch cmd { case "list", "get", "search", "ack": - return runAgent(cmd, rest, out, errOut) + return runAgent(cmd, rest, role, out, errOut) case "send": - return runSend(rest, out, errOut) + return runSend(rest, role, out, errOut) case "account": - return runAccount(rest, out, errOut) + return runAccount(rest, role, out, errOut) case "whitelist": - return runWhitelist(rest, out, errOut) + return runWhitelist(rest, role, out, errOut) case "config": - return runConfig(rest, out, errOut) + return runConfig(rest, role, out, errOut) case "audit": - return runAudit(rest, out, errOut) + return runAudit(rest, role, out, errOut) case "doctor": - return runDoctor(rest, out, errOut) + return runDoctor(rest, role, out, errOut) case "init": return runInit(rest, out, errOut) default: @@ -159,7 +160,7 @@ func Run(args []string, out, errOut io.Writer) int { } // runAgent handles JSON-emitting commands. Errors are emitted as JSON envelopes. -func runAgent(cmd string, args []string, out, errOut io.Writer) int { +func runAgent(cmd string, args []string, role store.Role, out, errOut io.Writer) int { fs := flag.NewFlagSet(cmd, flag.ContinueOnError) fs.SetOutput(errOut) usageFlags(fs, cmd, errOut) @@ -190,7 +191,7 @@ func runAgent(cmd string, args []string, out, errOut io.Writer) int { _ = Failure(CodeUsage, "--account is required").Write(out) return 2 } - st, err := openStore(store.RoleAgent) + st, err := openStore(role) if err != nil { _ = Failure(CodeConfig, err.Error()).Write(out) return 1 @@ -255,7 +256,7 @@ func (s *stringSlice) Set(v string) error { } // runSend handles the `send` agent command (JSON envelope output). -func runSend(args []string, out, errOut io.Writer) int { +func runSend(args []string, role store.Role, out, errOut io.Writer) int { fs := flag.NewFlagSet("send", flag.ContinueOnError) fs.SetOutput(errOut) usageFlags(fs, "send", errOut) @@ -280,7 +281,7 @@ func runSend(args []string, out, errOut io.Writer) int { _ = Failure(CodeUsage, "--account is required").Write(out) return 2 } - st, err := openStore(store.RoleAgent) + st, err := openStore(role) if err != nil { _ = Failure(CodeConfig, err.Error()).Write(out) return 1 diff --git a/internal/crypto/crypto.go b/internal/crypto/crypto.go index 04ce2ec..dc76f2d 100644 --- a/internal/crypto/crypto.go +++ b/internal/crypto/crypto.go @@ -1,4 +1,4 @@ -// Package crypto provides AES-256-GCM field encryption keyed from EMCLI_KEY. +// Package crypto provides AES-256-GCM field encryption; keys are loaded from EMCLI_KEY (agent) or EMCLI_ADMIN_KEY (admin). package crypto import ( diff --git a/internal/store/keys.go b/internal/store/keys.go index fcebc4d..57823eb 100644 --- a/internal/store/keys.go +++ b/internal/store/keys.go @@ -37,6 +37,9 @@ func (s *Store) dbPath() string { // NOT regenerate the DEK — it unlocks via the admin slot (idempotent re-init). func (s *Store) InitKeys(adminKey, agentKey []byte) error { if _, err := s.GetSetting(settingDEKWrapAdmin); err == nil { + // Already initialised: the DEK and both wrap slots already exist, so the + // agent key is not consumed here. Only the admin key is used to unlock the + // existing dek_wrap_admin slot; the DEK itself is preserved unchanged. return s.Unlock(RoleAdmin, adminKey, nil) } dek, err := crypto.NewDEK() diff --git a/skills/emcli/SKILL.md b/skills/emcli/SKILL.md index f62ce45..c12d0fa 100644 --- a/skills/emcli/SKILL.md +++ b/skills/emcli/SKILL.md @@ -21,9 +21,8 @@ sets its exit code to match. provided only `EMCLI_KEY` (the agent key), which authorises these commands and nothing else. Account setup, passwords, whitelists, and config are the **user's** job (admin commands that require `EMCLI_ADMIN_KEY`) — do not run or suggest running `account`, `whitelist`, `config`, - `audit`, or `init` unless the user explicitly asks you to help administer and confirms they have - provided `EMCLI_ADMIN_KEY` in your environment. Attempting admin commands with only `EMCLI_KEY` - will be refused by `emcli` with a privilege error. + `audit`, or `init`. You have only `EMCLI_KEY` (agent key); `emcli` will refuse admin commands + with a privilege error. - **Never touch the secret key.** `EMCLI_KEY` is supplied in the environment by whoever launched you. Do not read it, print it, log it, pass it as an argument, or try to generate one. If it is missing, stop and tell the user (see "Files & first run"). diff --git a/specifications/SPEC.md b/specifications/SPEC.md index 02b9271..278af70 100644 --- a/specifications/SPEC.md +++ b/specifications/SPEC.md @@ -73,8 +73,9 @@ the DEK-wrapping scheme: commands (`list`, `get`, `search`, `ack`, `send`, `doctor`) only. `EMCLI_ADMIN_KEY` is a superset: a process with only the admin key can also run agent commands. - Agent commands use `EMCLI_KEY`; if only `EMCLI_ADMIN_KEY` is set, they fall back to it. - If neither key satisfies the required slot, `emcli` exits with: + If a process holding only `EMCLI_KEY` attempts an admin command, `emcli` exits with: `emcli: this command requires EMCLI_ADMIN_KEY (admin privilege)`. + (An agent command with no key set at all yields a different `config` error: `EMCLI_KEY is not set`.) - `EMCLI_KEY` is supplied by the orchestrator that launches `emcli`, never as an argument the agent constructs. The agent has no command that reveals secret values. - All policy decisions happen inside `emcli`; the agent cannot bypass them because it has no other