Compare commits
18 Commits
76ada04442
...
main
| Author | SHA1 | Date | |
|---|---|---|---|
| 3c5e0a26f3 | |||
| 456d25d4f3 | |||
| 3bea73f857 | |||
| c651b00d08 | |||
| 8ed10dd503 | |||
| 2140d9e173 | |||
| 64ff32ab29 | |||
| 7039371f70 | |||
| e1b4ec38e5 | |||
| bd06b4b900 | |||
| 8e5c06a4cb | |||
| 32f5a8d933 | |||
| b6e68ddeae | |||
| 6a99e5bb6e | |||
| c5e42ffbae | |||
| cdffb15004 | |||
| a4c49d4aca | |||
| 852bb1dc5b |
+27
-10
@@ -33,8 +33,9 @@ This manual is for **using and administering** `emcli`. It assumes you have the
|
||||
## 1. Key concepts
|
||||
|
||||
**Two kinds of commands.**
|
||||
- **Admin commands** (`init`, `account`, `whitelist`, `config`, `audit`) require `EMCLI_ADMIN_KEY`
|
||||
and are for *you*, the human. They print human-readable text or open an interactive form.
|
||||
- **Admin commands** (`init`, `account add/edit/remove`, `whitelist`, `config`, `audit`) require
|
||||
`EMCLI_ADMIN_KEY` and are for *you*, the human. They print human-readable text or open an
|
||||
interactive form. (`account list` is the one exception — it is also an agent command; see below.)
|
||||
- **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
|
||||
@@ -124,8 +125,12 @@ DEK for an admin command, even if it somehow knows the agent key.
|
||||
|
||||
| Command | Role required |
|
||||
|---|---|
|
||||
| `list`, `get`, `search`, `ack`, `send`, `doctor` | Agent (`EMCLI_KEY` or `EMCLI_ADMIN_KEY`) |
|
||||
| `account`, `whitelist`, `config`, `audit` | Admin (`EMCLI_ADMIN_KEY` required) |
|
||||
| `list`, `get`, `search`, `ack`, `send`, `doctor`, `account list` | Agent (`EMCLI_KEY` or `EMCLI_ADMIN_KEY`) |
|
||||
| `account add/edit/remove`, `whitelist`, `config`, `audit` | Admin (`EMCLI_ADMIN_KEY` required) |
|
||||
|
||||
`account list` is dual-role: with the admin key it prints the full `NAME MODE IMAP USER` table;
|
||||
with only `EMCLI_KEY` (an agent) it prints a JSON envelope exposing just `name`, `from`, and
|
||||
`can_send` — no host or login username.
|
||||
| `init` | Both keys required (writes both wrap slots) |
|
||||
|
||||
### Agent launcher guidance
|
||||
@@ -191,6 +196,7 @@ emcli account add --name work --mode RW \
|
||||
| `--smtp-security` | `tls` | `tls` or `starttls` |
|
||||
| `--username` | — | Login username, usually your full email (required) |
|
||||
| `--password` | — | Login password or app password |
|
||||
| `--from` | — | Send-as address (blank = use username); bare or `"Display Name <addr>"` |
|
||||
| `--subject-regex` | — | Inbound subject filter (optional) |
|
||||
| `--whitelist-in` | off | Enable inbound whitelist |
|
||||
| `--whitelist-out` | off | Enable outbound whitelist |
|
||||
@@ -260,9 +266,15 @@ emcli account edit --name work --mode RW --smtp-host smtp.example.com --smtp-por
|
||||
emcli account edit --name gmail --password 'new-app-password' # rotate the app password
|
||||
```
|
||||
|
||||
> Note: the flag form of `account edit` covers connection/auth fields and `--subject-regex`. To
|
||||
> toggle whitelists or `process-backlog`, use the interactive form (`emcli account edit --name X`
|
||||
> with no other flags), or the `whitelist` commands in section 6.
|
||||
```bash
|
||||
emcli account edit --name work --from 'Work Team <you@yourdomain.com>' # set the send-as address
|
||||
emcli account edit --name work --from '' # clear it (revert to username)
|
||||
```
|
||||
|
||||
> Note: the flag form of `account edit` covers connection/auth fields, `--from`, and
|
||||
> `--subject-regex`. Passing `--from ''` clears the send-as address so mail falls back to the login
|
||||
> username. To toggle whitelists or `process-backlog`, use the interactive form (`emcli account edit
|
||||
> --name X` with no other flags), or the `whitelist` commands in section 6.
|
||||
|
||||
**Remove an account** (requires `--yes`):
|
||||
|
||||
@@ -420,6 +432,11 @@ emcli send --account gmail --to a@x.com --subject "Re: Hi" --body "thanks" \
|
||||
client. The source is subject to the inbound whitelist — you can't reply to mail you aren't allowed
|
||||
to see.
|
||||
|
||||
The message's `From:` is the account's send-as address (`--from`, set on `account add`/`edit`); if
|
||||
none is configured it falls back to the login username. A display-name address like
|
||||
`Steve Cliff <me@example.com>` shows the name in the recipient's client while the bare address is
|
||||
used for the SMTP envelope.
|
||||
|
||||
---
|
||||
|
||||
## 8. The JSON envelope
|
||||
@@ -540,8 +557,8 @@ base64-encoded 32-byte key (section 2). Agent commands (`list`, `get`, `search`,
|
||||
`doctor`) need this key.
|
||||
|
||||
**"this command requires EMCLI_ADMIN_KEY (admin privilege)".** Set `EMCLI_ADMIN_KEY` (section 2).
|
||||
Admin commands (`account`, `whitelist`, `config`, `audit`, `init`) need this key; `EMCLI_KEY`
|
||||
alone is not enough for them.
|
||||
Admin commands (`account add/edit/remove`, `whitelist`, `config`, `audit`, `init`) need this key;
|
||||
`EMCLI_KEY` alone is not enough for them. (`account list` is the exception — an agent can run it.)
|
||||
|
||||
**A command fails to decrypt / wrong key.** The key doesn't match the one used when the database
|
||||
was initialised. Restore the original key, or re-run `emcli init` (idempotent — it won't regenerate
|
||||
@@ -582,7 +599,7 @@ emcli <command> --help # usage and flags for one command
|
||||
# Admin (requires EMCLI_ADMIN_KEY)
|
||||
emcli init # create DB + add first account (form)
|
||||
emcli account add [flags | none for form] # add an account
|
||||
emcli account list # list accounts (no secrets)
|
||||
emcli account list # full table (admin) / name+from+can_send JSON (agent)
|
||||
emcli account edit --name N [flags | none for form] # change an account
|
||||
emcli account remove --name N --yes # delete an account
|
||||
emcli whitelist in|out add|remove|list --account N [--address A]
|
||||
|
||||
@@ -0,0 +1,497 @@
|
||||
# Agent-readable `account list` Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Let an agent holding only `EMCLI_KEY` run `emcli account list` and get a reduced JSON view (name, from, can_send), while admin keeps the full text table and `account add/edit/remove` stay admin-only.
|
||||
|
||||
**Architecture:** Make `commandRole` subcommand-aware so `account list` routes to the agent role; branch the `list` renderer on whether the admin key is present (admin → existing text table; agent → standard `Success` JSON envelope). No schema change; `ListAccounts` already avoids decrypting secrets.
|
||||
|
||||
**Tech Stack:** Go, standard library (`flag`, `encoding/json`), existing `internal/cli` envelope helpers and `internal/crypto` key loaders.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- Agent output is the existing JSON envelope shape: `{"error":bool,"error_detail":{...},"data":{...}}` via `Success(...)` / `Failure(...)` in `internal/cli/envelope.go`.
|
||||
- Admin `account list` output stays byte-for-byte the current human-readable table (`NAME MODE IMAP USER`).
|
||||
- The agent (reduced) view exposes only `name`, `from`, `can_send` — never the IMAP host/port or login username.
|
||||
- `from = Account.SendFrom()` (explicit `FromAddress`, else `Username`). `can_send = (Mode == "RW")`.
|
||||
- `account add/edit/remove` remain admin-only (hard-require `EMCLI_ADMIN_KEY`, no fallback).
|
||||
- Privilege detection: a caller is "admin" iff `crypto.AdminKeyFromEnv()` returns no error.
|
||||
- Spec: `docs/superpowers/specs/2026-06-23-agent-account-list-design.md`.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Route `account list` to the agent role and render by privilege
|
||||
|
||||
**Files:**
|
||||
- Modify: `internal/cli/run.go` (`commandRole`, its call site in `Run`)
|
||||
- Modify: `internal/cli/admin.go` (the `list` branch of `runAccount`; add `crypto` import)
|
||||
- Modify: `internal/cli/role_test.go` (`TestCommandRole`)
|
||||
- Modify: `internal/cli/security_invariant_test.go` (refused-commands set)
|
||||
- Create/Test: `internal/cli/account_list_test.go`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `store.Account.SendFrom() string`, `store.Account.Mode string`, `store.Store.ListAccounts() ([]store.Account, error)`, `crypto.AdminKeyFromEnv() ([]byte, error)`, `Success(map[string]any) Envelope`, `Failure(code, msg string) Envelope`, `Envelope.Write(io.Writer) error`, test helpers `adminEnv(t)`, `run(t, args...)`.
|
||||
- Produces: `commandRole(args []string) store.Role` (signature changes from `commandRole(cmd string)`). Agent `account list` emits `{"data":{"accounts":[{"name":string,"from":string,"can_send":bool}]}}`.
|
||||
|
||||
- [ ] **Step 1: Rewrite `TestCommandRole` for the new signature and subcommand routing**
|
||||
|
||||
Replace the body of `TestCommandRole` in `internal/cli/role_test.go` with:
|
||||
|
||||
```go
|
||||
func TestCommandRole(t *testing.T) {
|
||||
adminCmds := [][]string{
|
||||
{"whitelist"}, {"config"}, {"audit"},
|
||||
{"account"}, {"account", "add"}, {"account", "edit"}, {"account", "remove"},
|
||||
}
|
||||
agentCmds := [][]string{
|
||||
{"list"}, {"get"}, {"search"}, {"ack"}, {"send"}, {"doctor"},
|
||||
{"account", "list"},
|
||||
}
|
||||
for _, c := range adminCmds {
|
||||
if commandRole(c) != store.RoleAdmin {
|
||||
t.Errorf("%v should be admin", c)
|
||||
}
|
||||
}
|
||||
for _, c := range agentCmds {
|
||||
if commandRole(c) != store.RoleAgent {
|
||||
t.Errorf("%v should be agent", c)
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Note: `init` is intentionally absent from this table. `commandRole({"init"})` falls through to the agent arm, but `Run` dispatches `init` via its own bootstrap path (which requires both keys), so its `commandRole` result is never used — asserting a role for it here would be both wrong and meaningless.
|
||||
|
||||
- [ ] **Step 2: Run the routing test to verify it fails to compile**
|
||||
|
||||
Run: `go test ./internal/cli/ -run TestCommandRole`
|
||||
Expected: FAIL — compile error, `commandRole` takes `string`, called with `[]string`.
|
||||
|
||||
- [ ] **Step 3: Make `commandRole` subcommand-aware, update its call site, and fix the security invariant**
|
||||
|
||||
In `internal/cli/run.go`, replace:
|
||||
|
||||
```go
|
||||
func commandRole(cmd string) store.Role {
|
||||
switch cmd {
|
||||
case "account", "whitelist", "config", "audit":
|
||||
return store.RoleAdmin
|
||||
default: // list, get, search, ack, send, doctor
|
||||
return store.RoleAgent
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```go
|
||||
func commandRole(args []string) store.Role {
|
||||
switch args[0] {
|
||||
case "account":
|
||||
// account list is a read-only discovery view available to agents;
|
||||
// add/edit/remove mutate config and require admin.
|
||||
if len(args) >= 2 && args[1] == "list" {
|
||||
return store.RoleAgent
|
||||
}
|
||||
return store.RoleAdmin
|
||||
case "whitelist", "config", "audit":
|
||||
return store.RoleAdmin
|
||||
default: // list, get, search, ack, send, doctor
|
||||
return store.RoleAgent
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
In `Run`, change the call site from `role := commandRole(cmd)` to `role := commandRole(args)`.
|
||||
|
||||
In `internal/cli/security_invariant_test.go`, in `TestAgentKeyCannotRunAdminCommands`, replace the `adminAttempts` entry `{"account", "list"}` so the set covers a *mutating* account command instead (account list is now allowed for agents):
|
||||
|
||||
```go
|
||||
adminAttempts := [][]string{
|
||||
{"account", "add", "--name", "x", "--imap-host", "h", "--username", "u@x.com"},
|
||||
{"config", "set", "audit_retention_days", "30"},
|
||||
{"audit"},
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the cli package tests to verify routing + invariant pass**
|
||||
|
||||
Run: `go test ./internal/cli/ -run 'TestCommandRole|TestAgentKeyCannotRunAdminCommands'`
|
||||
Expected: PASS (both). The agent can no longer be proven to refuse `account list` — that is intended; the invariant now proves `account add` is refused and the DB is unchanged.
|
||||
|
||||
- [ ] **Step 5: Add the rendering tests (agent JSON view + admin text view)**
|
||||
|
||||
Create `internal/cli/account_list_test.go`:
|
||||
|
||||
```go
|
||||
package cli
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// With only EMCLI_KEY set, `account list` emits the reduced JSON envelope:
|
||||
// name/from/can_send, and never the IMAP host or login username.
|
||||
func TestAccountListAgentJSONView(t *testing.T) {
|
||||
adminEnv(t) // both keys + initialized temp DB
|
||||
run(t, "account", "add", "--name", "work", "--mode", "RW",
|
||||
"--imap-host", "imap.example.com", "--smtp-host", "smtp.example.com",
|
||||
"--username", "login@example.com", "--from", "me@example.com")
|
||||
run(t, "account", "add", "--name", "alerts", "--mode", "RO",
|
||||
"--imap-host", "imap.example.com", "--username", "alerts@example.com")
|
||||
|
||||
// Drop the admin key → caller is an agent.
|
||||
t.Setenv("EMCLI_ADMIN_KEY", "")
|
||||
code, out, errOut := run(t, "account", "list")
|
||||
if code != 0 {
|
||||
t.Fatalf("agent account list should succeed: code=%d err=%q", code, errOut)
|
||||
}
|
||||
|
||||
var env struct {
|
||||
Error bool `json:"error"`
|
||||
Data struct {
|
||||
Accounts []struct {
|
||||
Name string `json:"name"`
|
||||
From string `json:"from"`
|
||||
CanSend bool `json:"can_send"`
|
||||
} `json:"accounts"`
|
||||
} `json:"data"`
|
||||
}
|
||||
if err := json.Unmarshal([]byte(out), &env); err != nil {
|
||||
t.Fatalf("output is not the agent envelope: %v\n%s", err, out)
|
||||
}
|
||||
if env.Error || len(env.Data.Accounts) != 2 {
|
||||
t.Fatalf("want 2 accounts and no error, got %+v", env)
|
||||
}
|
||||
// The reduced view must not leak the IMAP host or the login username.
|
||||
if strings.Contains(out, "imap.example.com") || strings.Contains(out, "login@example.com") {
|
||||
t.Fatalf("agent view leaked host/username:\n%s", out)
|
||||
}
|
||||
|
||||
got := map[string]struct {
|
||||
from string
|
||||
canSend bool
|
||||
}{}
|
||||
for _, a := range env.Data.Accounts {
|
||||
got[a.Name] = struct {
|
||||
from string
|
||||
canSend bool
|
||||
}{a.From, a.CanSend}
|
||||
}
|
||||
if g := got["work"]; g.from != "me@example.com" || !g.canSend {
|
||||
t.Errorf("work: want from=me@example.com can_send=true, got %+v", g)
|
||||
}
|
||||
// alerts has no --from → SendFrom() falls back to the username.
|
||||
if g := got["alerts"]; g.from != "alerts@example.com" || g.canSend {
|
||||
t.Errorf("alerts: want from=alerts@example.com can_send=false, got %+v", g)
|
||||
}
|
||||
}
|
||||
|
||||
// With the admin key present, `account list` stays the full human-readable table.
|
||||
func TestAccountListAdminTextView(t *testing.T) {
|
||||
adminEnv(t)
|
||||
run(t, "account", "add", "--name", "work", "--mode", "RW",
|
||||
"--imap-host", "imap.example.com", "--smtp-host", "smtp.example.com",
|
||||
"--username", "login@example.com", "--from", "me@example.com")
|
||||
|
||||
code, out, _ := run(t, "account", "list")
|
||||
if code != 0 {
|
||||
t.Fatalf("admin account list failed: code=%d", code)
|
||||
}
|
||||
for _, want := range []string{"NAME", "MODE", "IMAP", "USER", "imap.example.com:993", "login@example.com"} {
|
||||
if !strings.Contains(out, want) {
|
||||
t.Fatalf("admin view missing %q:\n%s", want, out)
|
||||
}
|
||||
}
|
||||
if strings.Contains(out, `"accounts"`) {
|
||||
t.Fatalf("admin view should be text, not JSON:\n%s", out)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Run the rendering tests to verify the agent view fails**
|
||||
|
||||
Run: `go test ./internal/cli/ -run 'TestAccountListAgentJSONView|TestAccountListAdminTextView'`
|
||||
Expected: `TestAccountListAdminTextView` PASS (already text); `TestAccountListAgentJSONView` FAIL — output is still the text table, so `json.Unmarshal` errors.
|
||||
|
||||
- [ ] **Step 7: Split the `list` branch by privilege in `runAccount`**
|
||||
|
||||
In `internal/cli/admin.go`, add the crypto import. Change the import block:
|
||||
|
||||
```go
|
||||
import (
|
||||
"flag"
|
||||
"fmt"
|
||||
"io"
|
||||
"strconv"
|
||||
|
||||
"git.dcglab.co.uk/steve/emcli/internal/crypto"
|
||||
"git.dcglab.co.uk/steve/emcli/internal/store"
|
||||
"git.dcglab.co.uk/steve/emcli/internal/tui"
|
||||
)
|
||||
```
|
||||
|
||||
Replace the `case "list":` block (currently):
|
||||
|
||||
```go
|
||||
case "list":
|
||||
accs, err := st.ListAccounts()
|
||||
if err != nil {
|
||||
fmt.Fprintf(errOut, "list: %v\n", err)
|
||||
return 1
|
||||
}
|
||||
fmt.Fprintf(out, "%-16s %-4s %-28s %s\n", "NAME", "MODE", "IMAP", "USER")
|
||||
for _, a := range accs {
|
||||
fmt.Fprintf(out, "%-16s %-4s %-28s %s\n",
|
||||
a.Name, a.Mode, fmt.Sprintf("%s:%d", a.IMAPHost, a.IMAPPort), a.Username)
|
||||
}
|
||||
return 0
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```go
|
||||
case "list":
|
||||
// Holding the admin key means the caller is the human admin (full
|
||||
// detail). An agent holds only EMCLI_KEY and gets a reduced JSON view.
|
||||
_, adminErr := crypto.AdminKeyFromEnv()
|
||||
isAdmin := adminErr == nil
|
||||
accs, err := st.ListAccounts()
|
||||
if err != nil {
|
||||
if isAdmin {
|
||||
fmt.Fprintf(errOut, "list: %v\n", err)
|
||||
} else {
|
||||
_ = Failure(CodeDB, err.Error()).Write(out)
|
||||
}
|
||||
return 1
|
||||
}
|
||||
if !isAdmin {
|
||||
items := make([]map[string]any, 0, len(accs))
|
||||
for _, a := range accs {
|
||||
items = append(items, map[string]any{
|
||||
"name": a.Name,
|
||||
"from": a.SendFrom(),
|
||||
"can_send": a.Mode == "RW",
|
||||
})
|
||||
}
|
||||
_ = Success(map[string]any{"accounts": items}).Write(out)
|
||||
return 0
|
||||
}
|
||||
fmt.Fprintf(out, "%-16s %-4s %-28s %s\n", "NAME", "MODE", "IMAP", "USER")
|
||||
for _, a := range accs {
|
||||
fmt.Fprintf(out, "%-16s %-4s %-28s %s\n",
|
||||
a.Name, a.Mode, fmt.Sprintf("%s:%d", a.IMAPHost, a.IMAPPort), a.Username)
|
||||
}
|
||||
return 0
|
||||
```
|
||||
|
||||
- [ ] **Step 8: Run the full cli package test suite**
|
||||
|
||||
Run: `go test ./internal/cli/`
|
||||
Expected: PASS (all tests, including the two new rendering tests, the routing test, and the security invariant).
|
||||
|
||||
- [ ] **Step 9: Run the whole module to confirm nothing else regressed**
|
||||
|
||||
Run: `go build ./... && go test ./...`
|
||||
Expected: build clean; all packages PASS.
|
||||
|
||||
- [ ] **Step 10: Commit**
|
||||
|
||||
```bash
|
||||
git add internal/cli/run.go internal/cli/admin.go internal/cli/role_test.go \
|
||||
internal/cli/security_invariant_test.go internal/cli/account_list_test.go
|
||||
git commit -m "feat(cli): agent-readable account list (reduced JSON view)
|
||||
|
||||
account list now routes to the agent role; an agent (EMCLI_KEY only) gets a
|
||||
JSON envelope of name/from/can_send, while the admin keeps the full text
|
||||
table. account add/edit/remove stay admin-only.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Update user and agent documentation
|
||||
|
||||
**Files:**
|
||||
- Modify: `USER-MANUAL.md` (command-kinds note, role table, cheatsheet)
|
||||
- Modify: `skills/emcli/SKILL.md` (allowed-commands note, command table, do/don't)
|
||||
- Modify: `skills/emcli/AGENTIC-MANUAL.md` (§4 account discovery)
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: behavior shipped in Task 1 (agent `account list` → `{"data":{"accounts":[{name,from,can_send}]}}`).
|
||||
- Produces: docs only; no code interface.
|
||||
|
||||
- [ ] **Step 1: USER-MANUAL — note that `account list` is the one agent-readable admin view**
|
||||
|
||||
In `USER-MANUAL.md`, in the "Two kinds of commands" block, change the Admin bullet (line ~36) from:
|
||||
|
||||
```
|
||||
- **Admin commands** (`init`, `account`, `whitelist`, `config`, `audit`) require `EMCLI_ADMIN_KEY`
|
||||
and are for *you*, the human. They print human-readable text or open an interactive form.
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```
|
||||
- **Admin commands** (`init`, `account add/edit/remove`, `whitelist`, `config`, `audit`) require
|
||||
`EMCLI_ADMIN_KEY` and are for *you*, the human. They print human-readable text or open an
|
||||
interactive form. (`account list` is the one exception — it is also an agent command; see below.)
|
||||
```
|
||||
|
||||
- [ ] **Step 2: USER-MANUAL — update the role table**
|
||||
|
||||
Replace the role table rows (lines ~127-128):
|
||||
|
||||
```
|
||||
| `list`, `get`, `search`, `ack`, `send`, `doctor` | Agent (`EMCLI_KEY` or `EMCLI_ADMIN_KEY`) |
|
||||
| `account`, `whitelist`, `config`, `audit` | Admin (`EMCLI_ADMIN_KEY` required) |
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```
|
||||
| `list`, `get`, `search`, `ack`, `send`, `doctor`, `account list` | Agent (`EMCLI_KEY` or `EMCLI_ADMIN_KEY`) |
|
||||
| `account add/edit/remove`, `whitelist`, `config`, `audit` | Admin (`EMCLI_ADMIN_KEY` required) |
|
||||
|
||||
`account list` is dual-role: with the admin key it prints the full `NAME MODE IMAP USER` table;
|
||||
with only `EMCLI_KEY` (an agent) it prints a JSON envelope exposing just `name`, `from`, and
|
||||
`can_send` — no host or login username.
|
||||
```
|
||||
|
||||
- [ ] **Step 3: USER-MANUAL — annotate the cheatsheet**
|
||||
|
||||
In the cheatsheet (line ~597), change:
|
||||
|
||||
```
|
||||
emcli account list # list accounts (no secrets)
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```
|
||||
emcli account list # full table (admin) / name+from+can_send JSON (agent)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: SKILL.md — carve `account list` out of the forbidden-commands rule**
|
||||
|
||||
In `skills/emcli/SKILL.md`, change the first bullet (lines ~20-24):
|
||||
|
||||
```
|
||||
- **You only run agent commands:** `list`, `get`, `search`, `ack`, `send`, `doctor`. You are
|
||||
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`. You have only `EMCLI_KEY` (agent key); `emcli` will refuse admin commands
|
||||
with a privilege error.
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```
|
||||
- **You only run agent commands:** `list`, `get`, `search`, `ack`, `send`, `doctor`, and
|
||||
`account list` (to discover accounts). You are provided only `EMCLI_KEY` (the agent key), which
|
||||
authorises these and nothing else. Account *setup* (`account add/edit/remove`), passwords,
|
||||
whitelists, and config are the **user's** job (admin commands that require `EMCLI_ADMIN_KEY`) —
|
||||
do not run or suggest running `account add/edit/remove`, `whitelist`, `config`, `audit`, or
|
||||
`init`. `emcli` will refuse those with a privilege error.
|
||||
```
|
||||
|
||||
- [ ] **Step 5: SKILL.md — add `account list` to the command table**
|
||||
|
||||
In the command table (after the `send` row, line ~122), add:
|
||||
|
||||
```
|
||||
| `emcli account list` | Discover accounts: JSON `name` / `from` / `can_send` per account |
|
||||
```
|
||||
|
||||
- [ ] **Step 6: SKILL.md — fix the "don't" bullet**
|
||||
|
||||
Change (lines ~147-148):
|
||||
|
||||
```
|
||||
- ❌ Don't run admin commands (`account`/`whitelist`/`config`/`audit`/`init`) — you have only
|
||||
`EMCLI_KEY` (agent key); `emcli` will refuse admin commands with a privilege error.
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```
|
||||
- ❌ Don't run admin commands (`account add/edit/remove`, `whitelist`, `config`, `audit`, `init`) —
|
||||
you have only `EMCLI_KEY` (agent key); `emcli` will refuse them with a privilege error.
|
||||
(`account list` is allowed — use it to discover accounts.)
|
||||
```
|
||||
|
||||
Also change the ✅ bullet (line ~145) from `Ask the user for the account name; keep bodies plain text.` to:
|
||||
|
||||
```
|
||||
- ✅ Discover accounts with `emcli account list`, or ask the user; keep bodies plain text.
|
||||
```
|
||||
|
||||
- [ ] **Step 7: AGENTIC-MANUAL — document discovery via `account list`**
|
||||
|
||||
In `skills/emcli/AGENTIC-MANUAL.md`, replace the body of `## 4. Find the account(s)` (lines ~88-97):
|
||||
|
||||
```
|
||||
You refer to an account by name (e.g. `gmail`, `work`). Ask the user which account to use.
|
||||
`emcli doctor` is an agent command (authorised by `EMCLI_KEY`), so you can run it to check that
|
||||
configured accounts connect and authenticate:
|
||||
|
||||
```bash
|
||||
emcli doctor # all accounts
|
||||
emcli doctor --account gmail
|
||||
```
|
||||
|
||||
Just take the account name from the user and start with the workflow in `SKILL.md`.
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```
|
||||
You refer to an account by name (e.g. `gmail`, `work`). Discover the configured accounts yourself
|
||||
with `emcli account list` (an agent command authorised by `EMCLI_KEY`); it prints a JSON envelope
|
||||
with one entry per account:
|
||||
|
||||
```bash
|
||||
emcli account list
|
||||
# {"error":false,"error_detail":{},"data":{"accounts":[
|
||||
# {"name":"gmail","from":"me@gmail.com","can_send":true},
|
||||
# {"name":"alerts","from":"alerts@x.com","can_send":false}]}}
|
||||
```
|
||||
|
||||
`name` is what you pass to `--account`; `from` is the send-as identity; `can_send` is false for
|
||||
read-only accounts (they reject `send`). If unsure which to use, ask the user. `emcli doctor`
|
||||
(also an agent command) checks that accounts connect and authenticate:
|
||||
|
||||
```bash
|
||||
emcli doctor # all accounts
|
||||
emcli doctor --account gmail
|
||||
```
|
||||
|
||||
Then start with the workflow in `SKILL.md`.
|
||||
```
|
||||
|
||||
- [ ] **Step 8: Sanity-check the docs render and reference reality**
|
||||
|
||||
Run: `grep -n "account list" USER-MANUAL.md skills/emcli/SKILL.md skills/emcli/AGENTIC-MANUAL.md`
|
||||
Expected: each file shows the updated `account list` references; no remaining text claims the agent cannot run `account list`.
|
||||
|
||||
- [ ] **Step 9: Commit**
|
||||
|
||||
```bash
|
||||
git add USER-MANUAL.md skills/emcli/SKILL.md skills/emcli/AGENTIC-MANUAL.md
|
||||
git commit -m "docs: agent can discover accounts via account list
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Notes for the implementer
|
||||
|
||||
- Run all Go commands from the repo root (`/home/steve/src/emcli`).
|
||||
- The two intentional red states are Step 2 (compile error) and Step 6 (agent JSON test) in Task 1. Every other test run must be green.
|
||||
- Do not change the admin text table or add columns to it — admin output must stay identical.
|
||||
- `adminEnv(t)` and `run(t, ...)` live in `internal/cli/admin_test.go`; `b64Key`/`b64AgentKey` in `internal/cli/run_test.go`. No new helpers are needed.
|
||||
@@ -0,0 +1,688 @@
|
||||
# Send-as "From" Address Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Let each account configure the email address used as the `From:` when sending, instead of always reusing the login username.
|
||||
|
||||
**Architecture:** Add a single freeform RFC 5322 `from_address` field to the account (bare address or `Display Name <addr>`). When blank, sending falls back to the login username — no migration of existing data. The header `From:` carries the full identity; the SMTP envelope sender is derived as the bare address. A version-gated `ALTER TABLE` migration adds the column to existing databases.
|
||||
|
||||
**Tech Stack:** Go, SQLite (`modernc.org/sqlite`), `github.com/emersion/go-message/mail` for MIME, `net/mail` (stdlib) for address validation, bubbletea TUI.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- Module path: `git.dcglab.co.uk/steve/emcli`.
|
||||
- The `from_address` field is **not** a secret — store as plaintext (like `username`), never encrypted.
|
||||
- A blank from-address is always valid and means "fall back to `Account.Username`".
|
||||
- Follow existing patterns: `nullStr` for nullable text columns, `sql.NullString` in `scanAccount`, `fs.Visit` overlay for `account edit` flags.
|
||||
- Tests are Go table/unit tests in the same package; reuse the existing `openTemp(t)` helper where keys are needed.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Store — field, migration, persistence
|
||||
|
||||
**Files:**
|
||||
- Modify: `internal/store/account.go` (Account struct, AddAccount, GetAccount, ListAccounts, UpdateAccount, scanAccount)
|
||||
- Modify: `internal/store/schema.go` (add column, bump schemaVersion)
|
||||
- Modify: `internal/store/store.go` (run migration in Open)
|
||||
- Modify: `internal/store/store_test.go` (update schema_version expectations to "2")
|
||||
- Test: `internal/store/account_test.go` (SendFrom + round-trip), `internal/store/store_test.go` (migration)
|
||||
|
||||
**Interfaces:**
|
||||
- Produces: `store.Account.FromAddress string` field; method `func (a Account) SendFrom() string`; schema at version 2 with `accounts.from_address TEXT` column.
|
||||
|
||||
- [ ] **Step 1: Write the failing test for SendFrom + round-trip**
|
||||
|
||||
Add to `internal/store/account_test.go`:
|
||||
|
||||
```go
|
||||
func TestSendFromFallsBackToUsername(t *testing.T) {
|
||||
a := Account{Username: "login@example.com"}
|
||||
if got := a.SendFrom(); got != "login@example.com" {
|
||||
t.Fatalf("blank from-address should fall back to username, got %q", got)
|
||||
}
|
||||
a.FromAddress = "Steve Cliff <me@stevecliff.com>"
|
||||
if got := a.SendFrom(); got != "Steve Cliff <me@stevecliff.com>" {
|
||||
t.Fatalf("set from-address should win, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAddGetAccountRoundTripsFromAddress(t *testing.T) {
|
||||
s := openTemp(t)
|
||||
a := sampleAccount()
|
||||
a.FromAddress = "Steve Cliff <me@stevecliff.com>"
|
||||
if _, err := s.AddAccount(a); err != nil {
|
||||
t.Fatalf("AddAccount: %v", err)
|
||||
}
|
||||
got, err := s.GetAccount("work")
|
||||
if err != nil {
|
||||
t.Fatalf("GetAccount: %v", err)
|
||||
}
|
||||
if got.FromAddress != "Steve Cliff <me@stevecliff.com>" {
|
||||
t.Fatalf("FromAddress not round-tripped: %q", got.FromAddress)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they fail**
|
||||
|
||||
Run: `go test ./internal/store/ -run 'TestSendFrom|TestAddGetAccountRoundTripsFromAddress' -v`
|
||||
Expected: FAIL — `a.SendFrom undefined` and `a.FromAddress undefined`.
|
||||
|
||||
- [ ] **Step 3: Add the field and SendFrom method**
|
||||
|
||||
In `internal/store/account.go`, add `FromAddress` to the struct (right after `Username`) and the method. The struct becomes:
|
||||
|
||||
```go
|
||||
type Account struct {
|
||||
ID int64
|
||||
Name string
|
||||
Mode string // RO | RW
|
||||
IMAPHost string
|
||||
IMAPPort int
|
||||
IMAPSecurity string // tls | starttls
|
||||
SMTPHost string // nullable for RO accounts
|
||||
SMTPPort int
|
||||
SMTPSecurity string // tls | starttls
|
||||
AuthType string // password | oauth2
|
||||
Username string
|
||||
FromAddress string // send-as identity; blank ⇒ fall back to Username
|
||||
Password string // decrypted; empty in ListAccounts
|
||||
WhitelistInEnabled bool
|
||||
WhitelistOutEnabled bool
|
||||
SubjectRegex string
|
||||
ProcessBacklog bool
|
||||
}
|
||||
|
||||
// SendFrom returns the From identity for outgoing mail, falling back to the
|
||||
// login username when no explicit from-address is configured.
|
||||
func (a Account) SendFrom() string {
|
||||
if a.FromAddress != "" {
|
||||
return a.FromAddress
|
||||
}
|
||||
return a.Username
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Thread from_address through persistence**
|
||||
|
||||
In `internal/store/account.go`:
|
||||
|
||||
AddAccount — add `from_address` to the column list and a value placeholder. The INSERT becomes:
|
||||
|
||||
```go
|
||||
res, err := s.db.Exec(`
|
||||
INSERT INTO accounts
|
||||
(name,mode,imap_host,imap_port,imap_security,smtp_host,smtp_port,smtp_security,
|
||||
auth_type,username,from_address,
|
||||
enc_password,whitelist_in_enabled,whitelist_out_enabled,subject_regex,process_backlog)
|
||||
VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)`,
|
||||
a.Name, a.Mode, a.IMAPHost, a.IMAPPort, a.IMAPSecurity,
|
||||
nullStr(a.SMTPHost), nullInt(a.SMTPPort), nullStr(a.SMTPSecurity),
|
||||
a.AuthType, a.Username, nullStr(a.FromAddress),
|
||||
encPw, b2i(a.WhitelistInEnabled), b2i(a.WhitelistOutEnabled),
|
||||
nullStr(a.SubjectRegex), b2i(a.ProcessBacklog))
|
||||
```
|
||||
|
||||
GetAccount and ListAccounts — add `from_address` to both SELECT column lists, right after `username`:
|
||||
|
||||
```go
|
||||
SELECT id,name,mode,imap_host,imap_port,imap_security,smtp_host,smtp_port,smtp_security,
|
||||
auth_type,username,from_address,
|
||||
enc_password,whitelist_in_enabled,whitelist_out_enabled,subject_regex,process_backlog
|
||||
```
|
||||
|
||||
UpdateAccount — add `from_address=?` to the SET clause and its arg (after `username=?` / `a.Username`):
|
||||
|
||||
```go
|
||||
set := `mode=?, imap_host=?, imap_port=?, imap_security=?,
|
||||
smtp_host=?, smtp_port=?, smtp_security=?,
|
||||
auth_type=?, username=?, from_address=?,
|
||||
whitelist_in_enabled=?, whitelist_out_enabled=?, subject_regex=?, process_backlog=?`
|
||||
args := []any{
|
||||
a.Mode, a.IMAPHost, a.IMAPPort, a.IMAPSecurity,
|
||||
nullStr(a.SMTPHost), nullInt(a.SMTPPort), nullStr(a.SMTPSecurity),
|
||||
a.AuthType, a.Username, nullStr(a.FromAddress),
|
||||
b2i(a.WhitelistInEnabled), b2i(a.WhitelistOutEnabled),
|
||||
nullStr(a.SubjectRegex), b2i(a.ProcessBacklog),
|
||||
}
|
||||
```
|
||||
|
||||
scanAccount — add a `fromAddr sql.NullString` local, scan it after `&a.Username`, and assign. The var block gains `fromAddr sql.NullString`; the Scan call becomes:
|
||||
|
||||
```go
|
||||
err := sc.Scan(&a.ID, &a.Name, &a.Mode, &a.IMAPHost, &a.IMAPPort, &a.IMAPSecurity,
|
||||
&smtpHost, &smtpPort, &smtpSec,
|
||||
&a.AuthType, &a.Username, &fromAddr, &encPw, &wlIn, &wlOut, &subj, &backlog)
|
||||
```
|
||||
|
||||
and after the existing assignments add:
|
||||
|
||||
```go
|
||||
a.FromAddress = fromAddr.String
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Add the column to the schema and bump the version**
|
||||
|
||||
In `internal/store/schema.go`, change `const schemaVersion = 1` to `const schemaVersion = 2`, and add the column to the `accounts` CREATE TABLE, right after the `username` line:
|
||||
|
||||
```sql
|
||||
username TEXT NOT NULL,
|
||||
from_address TEXT,
|
||||
enc_password BLOB,
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Run the round-trip + SendFrom tests to verify they pass**
|
||||
|
||||
Run: `go test ./internal/store/ -run 'TestSendFrom|TestAddGetAccountRoundTripsFromAddress' -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 7: Write the failing migration test**
|
||||
|
||||
The existing `TestOpenCreatesSchemaAndIsIdempotent` will now fail because it expects `schema_version == "1"`. Update both assertions in `internal/store/store_test.go` from `"1"` to `"2"`. Then add a new migration test in `internal/store/store_test.go`:
|
||||
|
||||
```go
|
||||
func TestOpenMigratesV1AddsFromAddress(t *testing.T) {
|
||||
p := filepath.Join(t.TempDir(), "emcli.db")
|
||||
|
||||
// Hand-build a v1 database: accounts table WITHOUT from_address, a settings
|
||||
// table pinned at schema_version=1, and one pre-existing account row.
|
||||
raw, err := sql.Open("sqlite", p)
|
||||
if err != nil {
|
||||
t.Fatalf("sql.Open: %v", err)
|
||||
}
|
||||
const v1Schema = `
|
||||
CREATE TABLE settings (key TEXT PRIMARY KEY, value TEXT NOT NULL);
|
||||
CREATE TABLE accounts (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
name TEXT UNIQUE NOT NULL,
|
||||
mode TEXT NOT NULL,
|
||||
imap_host TEXT NOT NULL,
|
||||
imap_port INTEGER NOT NULL,
|
||||
imap_security TEXT NOT NULL,
|
||||
smtp_host TEXT, smtp_port INTEGER, smtp_security TEXT,
|
||||
auth_type TEXT NOT NULL,
|
||||
username TEXT NOT NULL,
|
||||
enc_password BLOB,
|
||||
enc_oauth_client_id BLOB, enc_oauth_client_secret BLOB, enc_oauth_refresh_token BLOB,
|
||||
whitelist_in_enabled INTEGER NOT NULL DEFAULT 0,
|
||||
whitelist_out_enabled INTEGER NOT NULL DEFAULT 0,
|
||||
subject_regex TEXT,
|
||||
process_backlog INTEGER NOT NULL DEFAULT 0
|
||||
);
|
||||
INSERT INTO settings(key,value) VALUES ('schema_version','1');
|
||||
INSERT INTO accounts(name,mode,imap_host,imap_port,imap_security,auth_type,username)
|
||||
VALUES ('legacy','RO','imap.example.com',993,'tls','password','login@example.com');
|
||||
`
|
||||
if _, err := raw.Exec(v1Schema); err != nil {
|
||||
t.Fatalf("seed v1 schema: %v", err)
|
||||
}
|
||||
raw.Close()
|
||||
|
||||
// Open via the store: the migration must add from_address and bump to v2.
|
||||
s, err := Open(p)
|
||||
if err != nil {
|
||||
t.Fatalf("Open (migrate): %v", err)
|
||||
}
|
||||
defer s.Close()
|
||||
|
||||
if v, _ := s.GetSetting("schema_version"); v != "2" {
|
||||
t.Fatalf("schema_version after migrate: %q, want 2", v)
|
||||
}
|
||||
// ListAccounts SELECTs from_address; it would error if the column were missing.
|
||||
accs, err := s.ListAccounts()
|
||||
if err != nil {
|
||||
t.Fatalf("ListAccounts after migrate: %v", err)
|
||||
}
|
||||
if len(accs) != 1 || accs[0].FromAddress != "" {
|
||||
t.Fatalf("legacy account wrong after migrate: %+v", accs)
|
||||
}
|
||||
if got := accs[0].SendFrom(); got != "login@example.com" {
|
||||
t.Fatalf("legacy account should send from username, got %q", got)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Ensure `internal/store/store_test.go` imports `"database/sql"` (add it to the import block).
|
||||
|
||||
- [ ] **Step 8: Run the migration test to verify it fails**
|
||||
|
||||
Run: `go test ./internal/store/ -run 'TestOpenMigratesV1AddsFromAddress|TestOpenCreatesSchemaAndIsIdempotent' -v`
|
||||
Expected: migration test FAILS with a "no such column: from_address" error from `ListAccounts` (the column is in the schema for new DBs but not added to the seeded v1 DB).
|
||||
|
||||
- [ ] **Step 9: Add the migration runner to Open**
|
||||
|
||||
In `internal/store/store.go`, replace the post-schema version block with a call to a new `migrate` method. Change the tail of `Open` from:
|
||||
|
||||
```go
|
||||
s := &Store{db: db}
|
||||
if _, err := s.GetSetting("schema_version"); err != nil {
|
||||
if err := s.SetSetting("schema_version", strconv.Itoa(schemaVersion)); err != nil {
|
||||
db.Close()
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
return s, nil
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```go
|
||||
s := &Store{db: db}
|
||||
if err := s.migrate(); err != nil {
|
||||
db.Close()
|
||||
return nil, err
|
||||
}
|
||||
return s, nil
|
||||
```
|
||||
|
||||
and add the method:
|
||||
|
||||
```go
|
||||
// migrate brings an existing database up to the current schemaVersion. A brand-
|
||||
// new database (no schema_version yet) already has every column from schemaSQL,
|
||||
// so it is simply stamped at the current version. Each older version runs its
|
||||
// forward step. The version gate makes every step idempotent across reopens.
|
||||
func (s *Store) migrate() error {
|
||||
v, err := s.GetSetting("schema_version")
|
||||
if err != nil {
|
||||
// Fresh database: schemaSQL created all columns already.
|
||||
return s.SetSetting("schema_version", strconv.Itoa(schemaVersion))
|
||||
}
|
||||
ver, _ := strconv.Atoi(v)
|
||||
if ver < 2 {
|
||||
if _, err := s.db.Exec(`ALTER TABLE accounts ADD COLUMN from_address TEXT`); err != nil {
|
||||
return fmt.Errorf("migrate to v2: %w", err)
|
||||
}
|
||||
if err := s.SetSetting("schema_version", "2"); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
```
|
||||
|
||||
Confirm `internal/store/store.go` already imports `fmt` and `strconv` (it does); no import changes needed.
|
||||
|
||||
- [ ] **Step 10: Run the full store test suite to verify it passes**
|
||||
|
||||
Run: `go test ./internal/store/ -v`
|
||||
Expected: PASS (migration, idempotency, round-trip, and existing tests all green).
|
||||
|
||||
- [ ] **Step 11: Commit**
|
||||
|
||||
```bash
|
||||
git add internal/store/account.go internal/store/schema.go internal/store/store.go internal/store/account_test.go internal/store/store_test.go
|
||||
git commit -m "feat(store): add account from_address field + v2 migration
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Mail — envelope sender vs header From
|
||||
|
||||
**Files:**
|
||||
- Modify: `internal/mail/send.go` (add `envelopeFrom` helper, use it in `SendSMTP`)
|
||||
- Test: `internal/mail/send_test.go` (envelopeFrom table test + BuildMIME display-name assertion)
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `OutgoingMessage.From` may now hold `Display Name <addr>`.
|
||||
- Produces: `func envelopeFrom(from string) string` (package-private) — bare address for the SMTP envelope.
|
||||
|
||||
- [ ] **Step 1: Write the failing test for envelopeFrom and the display-name header**
|
||||
|
||||
Add to `internal/mail/send_test.go`:
|
||||
|
||||
```go
|
||||
func TestEnvelopeFromStripsDisplayName(t *testing.T) {
|
||||
cases := map[string]string{
|
||||
"Steve Cliff <me@stevecliff.com>": "me@stevecliff.com",
|
||||
"me@stevecliff.com": "me@stevecliff.com",
|
||||
"<me@stevecliff.com>": "me@stevecliff.com",
|
||||
"not a valid address": "not a valid address", // unparseable ⇒ passthrough
|
||||
}
|
||||
for in, want := range cases {
|
||||
if got := envelopeFrom(in); got != want {
|
||||
t.Fatalf("envelopeFrom(%q) = %q, want %q", in, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildMIMEKeepsDisplayNameInHeader(t *testing.T) {
|
||||
raw, err := BuildMIME(OutgoingMessage{
|
||||
From: "Steve Cliff <me@stevecliff.com>",
|
||||
To: []string{"you@example.com"},
|
||||
Subject: "hi",
|
||||
BodyText: "body",
|
||||
Date: time.Date(2026, 6, 23, 12, 0, 0, 0, time.UTC),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("BuildMIME: %v", err)
|
||||
}
|
||||
if !strings.Contains(string(raw), "Steve Cliff") {
|
||||
t.Fatalf("From header lost display name:\n%s", raw)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the tests to verify they fail**
|
||||
|
||||
Run: `go test ./internal/mail/ -run 'TestEnvelopeFromStripsDisplayName|TestBuildMIMEKeepsDisplayNameInHeader' -v`
|
||||
Expected: FAIL — `envelopeFrom` undefined. (The BuildMIME test may already pass, since `SetAddressList` renders display names; the envelopeFrom test is the gating failure.)
|
||||
|
||||
- [ ] **Step 3: Add the envelopeFrom helper and use it in SendSMTP**
|
||||
|
||||
In `internal/mail/send.go`, add the helper (near `addrList`):
|
||||
|
||||
```go
|
||||
// envelopeFrom returns the bare address for the SMTP envelope sender, stripping
|
||||
// any display name. A display-name From (e.g. "Name <addr>") is a valid header
|
||||
// but an invalid envelope sender, so it must be reduced to the bare address.
|
||||
// Unparseable input is passed through unchanged (preserves prior behaviour for
|
||||
// plain addresses).
|
||||
func envelopeFrom(from string) string {
|
||||
if a, err := gomail.ParseAddress(from); err == nil {
|
||||
return a.Address
|
||||
}
|
||||
return from
|
||||
}
|
||||
```
|
||||
|
||||
In `SendSMTP`, change the send line from:
|
||||
|
||||
```go
|
||||
if err := c.SendMail(m.From, m.Recipients(), bytes.NewReader(raw)); err != nil {
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```go
|
||||
if err := c.SendMail(envelopeFrom(m.From), m.Recipients(), bytes.NewReader(raw)); err != nil {
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the tests to verify they pass**
|
||||
|
||||
Run: `go test ./internal/mail/ -run 'TestEnvelopeFromStripsDisplayName|TestBuildMIMEKeepsDisplayNameInHeader' -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 5: Run the full mail suite**
|
||||
|
||||
Run: `go test ./internal/mail/`
|
||||
Expected: PASS (`imap_integration_test` may skip without a live server — that is fine).
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add internal/mail/send.go internal/mail/send_test.go
|
||||
git commit -m "feat(mail): derive bare envelope sender from display-name From
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: CLI + TUI — inputs, validation, and send wiring
|
||||
|
||||
**Files:**
|
||||
- Modify: `internal/cli/send.go:26` (use `acc.SendFrom()`)
|
||||
- Modify: `internal/cli/admin.go` (`--from` flag on `account add` and `account edit`)
|
||||
- Modify: `internal/tui/account.go` (Fields field, fieldDef, ToAccount, FieldsFromAccount, fieldValue, collect, validation helper, Validate)
|
||||
- Test: `internal/tui/account_test.go` (validation + round-trip), `internal/cli/send_test.go` (send uses configured from)
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `store.Account.FromAddress`, `store.Account.SendFrom()` (Task 1).
|
||||
- Produces: `func ValidFromAddress(s string) error` exported from `tui` package, used by both `Fields.Validate` and `internal/cli/admin.go`.
|
||||
|
||||
- [ ] **Step 1: Write the failing TUI validation + round-trip tests**
|
||||
|
||||
Add to `internal/tui/account_test.go`:
|
||||
|
||||
```go
|
||||
func TestValidateRejectsBadFromAddress(t *testing.T) {
|
||||
f := validFields()
|
||||
f.FromAddress = "not an address"
|
||||
if err := f.Validate(); err == nil {
|
||||
t.Fatal("malformed from-address should fail validation")
|
||||
}
|
||||
f.FromAddress = "Steve Cliff <me@stevecliff.com>"
|
||||
if err := f.Validate(); err != nil {
|
||||
t.Fatalf("display-name from-address should validate: %v", err)
|
||||
}
|
||||
f.FromAddress = "me@stevecliff.com"
|
||||
if err := f.Validate(); err != nil {
|
||||
t.Fatalf("bare from-address should validate: %v", err)
|
||||
}
|
||||
f.FromAddress = "" // blank ⇒ fall back, always valid
|
||||
if err := f.Validate(); err != nil {
|
||||
t.Fatalf("blank from-address should validate: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFieldsFromToAccountCarriesFromAddress(t *testing.T) {
|
||||
f := validFields()
|
||||
f.FromAddress = "Steve Cliff <me@stevecliff.com>"
|
||||
acc, _ := f.ToAccount()
|
||||
if acc.FromAddress != "Steve Cliff <me@stevecliff.com>" {
|
||||
t.Fatalf("ToAccount lost FromAddress: %q", acc.FromAddress)
|
||||
}
|
||||
back := FieldsFromAccount(acc)
|
||||
if back.FromAddress != "Steve Cliff <me@stevecliff.com>" {
|
||||
t.Fatalf("FieldsFromAccount lost FromAddress: %q", back.FromAddress)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the tests to verify they fail**
|
||||
|
||||
Run: `go test ./internal/tui/ -run 'TestValidateRejectsBadFromAddress|TestFieldsFromToAccountCarriesFromAddress' -v`
|
||||
Expected: FAIL — `f.FromAddress` undefined.
|
||||
|
||||
- [ ] **Step 3: Add the field, validation helper, and wiring in tui/account.go**
|
||||
|
||||
In `internal/tui/account.go`:
|
||||
|
||||
Add `"net/mail"` to the import block.
|
||||
|
||||
Add `FromAddress` to `Fields` (after the `Username, Password` line):
|
||||
|
||||
```go
|
||||
type Fields struct {
|
||||
Name, Mode string
|
||||
IMAPHost, IMAPPort, IMAPSecurity string
|
||||
SMTPHost, SMTPPort, SMTPSecurity string
|
||||
Username, Password string
|
||||
FromAddress string
|
||||
WhitelistIn, WhitelistOut, ProcessBacklog bool
|
||||
SubjectRegex string
|
||||
}
|
||||
```
|
||||
|
||||
Add the exported validator:
|
||||
|
||||
```go
|
||||
// ValidFromAddress returns an error if s is set but is not a valid RFC 5322
|
||||
// address (bare or "Display Name <addr>"). A blank value is valid: sending
|
||||
// falls back to the login username.
|
||||
func ValidFromAddress(s string) error {
|
||||
if strings.TrimSpace(s) == "" {
|
||||
return nil
|
||||
}
|
||||
if _, err := mail.ParseAddress(s); err != nil {
|
||||
return errors.New("from address must be a valid email address")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
```
|
||||
|
||||
In `Fields.Validate`, add before the final `return nil`:
|
||||
|
||||
```go
|
||||
if err := ValidFromAddress(f.FromAddress); err != nil {
|
||||
return err
|
||||
}
|
||||
```
|
||||
|
||||
In `ToAccount`, set the field on the assembled account (add to the struct literal, after `Username/Password`):
|
||||
|
||||
```go
|
||||
AuthType: "password", Username: f.Username, Password: f.Password,
|
||||
FromAddress: f.FromAddress,
|
||||
```
|
||||
|
||||
In `FieldsFromAccount`, prefill it (after `Username: a.Username,`):
|
||||
|
||||
```go
|
||||
Username: a.Username,
|
||||
FromAddress: a.FromAddress,
|
||||
```
|
||||
|
||||
Add a `fieldDef` to `fieldDefs`, immediately after the `username` entry (so it appears next to it in the form):
|
||||
|
||||
```go
|
||||
{key: "username", label: "Username"},
|
||||
{key: "from_address", label: "From address (optional)"},
|
||||
{key: "password", label: "Password", password: true},
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Wire from_address through fieldValue and collect**
|
||||
|
||||
In `internal/tui/account.go`, find `fieldValue` (≈ line 147) and add a `case "from_address": return f.FromAddress` alongside the other string cases. Find `collect` (≈ line 228) and add the inverse mapping so the typed value is written back to `f.FromAddress` (mirror exactly how `username` is handled in that function's switch).
|
||||
|
||||
- [ ] **Step 5: Run the tui tests to verify they pass**
|
||||
|
||||
Run: `go test ./internal/tui/ -v`
|
||||
Expected: PASS (new validation/round-trip tests plus existing form tests).
|
||||
|
||||
- [ ] **Step 6: Write the failing CLI send test**
|
||||
|
||||
The harness in `internal/cli/send_test.go` records every sent message into `*sent`, so assert directly on `m.From`. Add:
|
||||
|
||||
```go
|
||||
func TestSendUsesConfiguredFromAddress(t *testing.T) {
|
||||
acc := rwAccount()
|
||||
acc.FromAddress = "Steve Cliff <me@stevecliff.com>"
|
||||
d, sent, _ := sendDeps(t, acc, nil)
|
||||
if err := SendCmd(d, "send", []string{"me@stevecliff.com"}, nil, nil, "hi", "body", nil, 0, "INBOX"); err != nil {
|
||||
t.Fatalf("SendCmd: %v", err)
|
||||
}
|
||||
if len(*sent) != 1 {
|
||||
t.Fatalf("want 1 send, got %d", len(*sent))
|
||||
}
|
||||
if got := (*sent)[0].From; got != "Steve Cliff <me@stevecliff.com>" {
|
||||
t.Fatalf("From = %q, want configured from-address", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSendFallsBackToUsernameAsFrom(t *testing.T) {
|
||||
// rwAccount has no FromAddress, so From must be the login username.
|
||||
d, sent, _ := sendDeps(t, rwAccount(), nil)
|
||||
if err := SendCmd(d, "send", []string{"me@stevecliff.com"}, nil, nil, "hi", "body", nil, 0, "INBOX"); err != nil {
|
||||
t.Fatalf("SendCmd: %v", err)
|
||||
}
|
||||
if got := (*sent)[0].From; got != "emcli@stevecliff.com" {
|
||||
t.Fatalf("From = %q, want username fallback", got)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 7: Run the CLI send test to verify it fails**
|
||||
|
||||
Run: `go test ./internal/cli/ -run 'TestSendUsesConfiguredFromAddress' -v`
|
||||
Expected: FAIL — `send.go` still sets `From: acc.Username`.
|
||||
|
||||
- [ ] **Step 8: Wire send.go and add the --from flags**
|
||||
|
||||
In `internal/cli/send.go`, change:
|
||||
|
||||
```go
|
||||
msg := mail.OutgoingMessage{
|
||||
From: acc.Username, To: to, Cc: cc, Bcc: bcc,
|
||||
Subject: subject, BodyText: body,
|
||||
}
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```go
|
||||
msg := mail.OutgoingMessage{
|
||||
From: acc.SendFrom(), To: to, Cc: cc, Bcc: bcc,
|
||||
Subject: subject, BodyText: body,
|
||||
}
|
||||
```
|
||||
|
||||
In `internal/cli/admin.go`, `account add`: register the flag and validate it.
|
||||
|
||||
Add alongside the other `add` flags:
|
||||
|
||||
```go
|
||||
from := fs.String("from", "", "send-as address (blank = use username)")
|
||||
```
|
||||
|
||||
After the required-fields check, before building `acc`:
|
||||
|
||||
```go
|
||||
if err := tui.ValidFromAddress(*from); err != nil {
|
||||
fmt.Fprintln(errOut, err)
|
||||
return 2
|
||||
}
|
||||
```
|
||||
|
||||
Add `FromAddress: *from,` to the `store.Account{...}` literal.
|
||||
|
||||
In `account edit`: register the flag:
|
||||
|
||||
```go
|
||||
from := fs.String("from", "", "send-as address (blank keeps existing)")
|
||||
```
|
||||
|
||||
Add a case to the `fs.Visit` switch:
|
||||
|
||||
```go
|
||||
case "from":
|
||||
if err := tui.ValidFromAddress(*from); err != nil {
|
||||
fmt.Fprintln(errOut, err)
|
||||
return // see note below
|
||||
}
|
||||
acc.FromAddress = *from
|
||||
```
|
||||
|
||||
Because `fs.Visit`'s callback cannot return an exit code, instead validate `--from` before the `fs.Visit` block (the flag value is available regardless of Visit) and set the field inside Visit:
|
||||
|
||||
```go
|
||||
if err := tui.ValidFromAddress(*from); err != nil {
|
||||
fmt.Fprintln(errOut, err)
|
||||
return 2
|
||||
}
|
||||
// ... existing GetAccount + fs.Visit ...
|
||||
case "from":
|
||||
acc.FromAddress = *from
|
||||
```
|
||||
|
||||
Use this pre-Visit validation form (not the in-callback `return`).
|
||||
|
||||
- [ ] **Step 9: Run the CLI suite to verify it passes**
|
||||
|
||||
Run: `go test ./internal/cli/ -v`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 10: Build and vet the whole module**
|
||||
|
||||
Run: `go build ./... && go vet ./... && go test ./...`
|
||||
Expected: clean build, no vet complaints, all tests PASS.
|
||||
|
||||
- [ ] **Step 11: Commit**
|
||||
|
||||
```bash
|
||||
git add internal/cli/send.go internal/cli/admin.go internal/cli/send_test.go internal/tui/account.go internal/tui/account_test.go
|
||||
git commit -m "feat(cli): configurable send-as From address (flags, TUI, validation)
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Notes for the implementer
|
||||
|
||||
- The `account list` output (`admin.go`, `case "list"`) shows NAME/MODE/IMAP/USER. Adding a FROM column is optional polish, not required — leave it unless asked.
|
||||
- `USER-MANUAL.md` / `README.md` mention `account add` flags; if they enumerate flags explicitly, add `--from` there in the relevant commit. Grep first: `grep -rn 'account add\|--username' README.md USER-MANUAL.md docs/`.
|
||||
- Existing send tests in `internal/cli/send_test.go` define the harness shape — read them before writing Task 3 Step 6 rather than inventing a new fake.
|
||||
@@ -0,0 +1,141 @@
|
||||
# Agent-readable `account list` — design
|
||||
|
||||
**Date:** 2026-06-23
|
||||
**Status:** Approved (brainstorm), ready for implementation plan
|
||||
**Author:** Steve + Claude
|
||||
|
||||
## Problem
|
||||
|
||||
An agent process is launched with only `EMCLI_KEY` (the two-key model — see
|
||||
`2026-06-22-two-key-privilege-design.md`). Every agent command takes
|
||||
`--account NAME`, but the agent has no way to *discover* which accounts exist:
|
||||
`account list` is classified admin-only and is refused under the agent key. So
|
||||
account names must be supplied to the agent out of band, which is brittle and
|
||||
defeats the point of a self-directed agent.
|
||||
|
||||
The two-key spec deliberately gated the whole `account` command to admin because
|
||||
`account add/edit/remove` mutate configuration. But `account list` is read-only
|
||||
and exposes no secrets — `store.ListAccounts` never decrypts the password
|
||||
(`enc_password` is scanned and discarded). Gating *discovery* behind admin is
|
||||
stricter than the threat model requires.
|
||||
|
||||
## Goal
|
||||
|
||||
Let an agent holding only `EMCLI_KEY` run `account list` to discover the
|
||||
accounts it may operate on, while:
|
||||
|
||||
- keeping `account add/edit/remove` admin-only (mutation stays gated);
|
||||
- exposing to the agent only what it needs — **account name, the send-as From
|
||||
address, and whether the account can send** — and *not* the IMAP host/port or
|
||||
login username;
|
||||
- preserving the admin's existing full-detail view unchanged.
|
||||
|
||||
## Constraints / decisions
|
||||
|
||||
Settled during brainstorming:
|
||||
|
||||
1. **Scope is exactly `account list`.** `whitelist list`, `config get`, and
|
||||
`audit list` stay admin-only. `audit` in particular is oversight data and
|
||||
must remain invisible to the agent.
|
||||
2. **Privilege-dependent rendering.** The admin keeps the current full table
|
||||
(`NAME MODE IMAP USER`, human-readable). The agent gets a *reduced* view
|
||||
containing only name, From, and send-capability.
|
||||
3. **Agent output is JSON.** The agent is a machine consumer, so its
|
||||
`account list` emits the standard agent envelope (like `list`/`get`/`search`),
|
||||
not a text table. The admin path stays human-readable text.
|
||||
4. **No secret exposure, no schema change.** `ListAccounts` already avoids
|
||||
decrypting passwords; nothing about the data model changes.
|
||||
|
||||
## Approach
|
||||
|
||||
Reclassify `account list` to the agent role, and branch rendering on the
|
||||
caller's actual privilege (presence of the admin key).
|
||||
|
||||
### Routing (`internal/cli/run.go`)
|
||||
|
||||
`commandRole` becomes subcommand-aware for `account`:
|
||||
|
||||
- `account list` → `RoleAgent`
|
||||
- `account add | edit | remove` (and bare `account`) → `RoleAdmin`
|
||||
- all other commands unchanged.
|
||||
|
||||
`commandRole` currently takes `cmd string`; it changes to take the full
|
||||
`args []string` so it can peek at the `account` subcommand. `Run` passes
|
||||
`args` through. This keeps `commandRole` the single source of truth for the
|
||||
classification table.
|
||||
|
||||
Authorization mechanics are unchanged: `openStore(RoleAgent)` requires
|
||||
`EMCLI_KEY` (falling back to the admin key for a human who holds only that and
|
||||
runs `account list`). `account add/edit/remove` still hard-require
|
||||
`EMCLI_ADMIN_KEY` with no fallback.
|
||||
|
||||
### Rendering (`internal/cli/admin.go`, the `list` branch)
|
||||
|
||||
Determine privilege from the environment: `_, err := crypto.AdminKeyFromEnv()`;
|
||||
`isAdmin := err == nil`. (Holding the admin key *is* being the admin in this
|
||||
trust model. A human with only the admin key still gets the admin view; an agent
|
||||
with only `EMCLI_KEY` gets the reduced view.)
|
||||
|
||||
- **Admin** → existing full table, unchanged:
|
||||
```
|
||||
NAME MODE IMAP USER
|
||||
work RW imap.example.com:993 me@example.com
|
||||
```
|
||||
- **Agent** → JSON envelope to stdout:
|
||||
```json
|
||||
{"error":false,"error_detail":{},"data":{"accounts":[
|
||||
{"name":"work","from":"me@example.com","can_send":true},
|
||||
{"name":"alerts","from":"alerts@example.com","can_send":false}
|
||||
]}}
|
||||
```
|
||||
where `from = Account.SendFrom()` (the configured From address, falling back
|
||||
to the username) and `can_send = (Mode == "RW")` (RW accounts have SMTP
|
||||
configured; RO cannot send). The IMAP host/port and the raw login username are
|
||||
**not** emitted (when From falls back to the username it may coincide with it,
|
||||
which is acceptable — the user asked for From specifically).
|
||||
|
||||
The reduced view reuses the existing `Success(...)` envelope and `Envelope.Write`
|
||||
helper; no new output machinery.
|
||||
|
||||
## Error handling
|
||||
|
||||
- Agent key on `account add/edit/remove` → unchanged:
|
||||
`emcli: this command requires EMCLI_ADMIN_KEY (admin privilege)`, non-zero.
|
||||
- Agent `account list` with neither key set → the agent-command config error
|
||||
(`EMCLI_KEY is not set`), surfaced the same way other agent commands surface a
|
||||
missing key. Because the agent path now emits JSON, a missing-key failure on
|
||||
`account list` is a JSON `Failure(CodeConfig, …)` envelope, consistent with the
|
||||
other agent commands.
|
||||
- DB/list errors on the agent path → `Failure(CodeDB, …)` envelope; on the admin
|
||||
path → existing `list: <err>` text to stderr.
|
||||
|
||||
## Testing
|
||||
|
||||
- **Routing:** extend the `commandRole` table test — `account list` → agent;
|
||||
`account add` / `account edit` / `account remove` → admin.
|
||||
- **Agent view:** with only `EMCLI_KEY`, `account list` exits 0, emits a valid
|
||||
envelope, and the `data.accounts` entries carry `name`/`from`/`can_send` — and
|
||||
the output does **not** contain the IMAP host or the login username.
|
||||
- **Admin view:** with `EMCLI_ADMIN_KEY`, `account list` still prints the full
|
||||
`NAME MODE IMAP USER` table (regression guard).
|
||||
- **`can_send`:** an RW account yields `can_send:true`, an RO account
|
||||
`can_send:false`; `from` reflects `SendFrom()` (explicit From, else username).
|
||||
- **Security invariant (`security_invariant_test.go`):** remove
|
||||
`{"account","list"}` from the refused-commands set (it is now allowed) and
|
||||
replace it with a *mutating* `account add …` attempt, so the "forced agent
|
||||
cannot run admin commands and the DB is byte-for-byte unchanged" invariant
|
||||
still covers the `account` family.
|
||||
|
||||
## Documentation updates
|
||||
|
||||
- **USER-MANUAL:** role/command table — `account list` is agent-readable
|
||||
(reduced JSON view); `account add/edit/remove` remain admin.
|
||||
- **`skills/emcli` (SKILL.md / AGENTIC-MANUAL.md):** document that the agent
|
||||
discovers accounts via `account list`, including the JSON shape
|
||||
(`name`, `from`, `can_send`).
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Agent access to `whitelist list`, `config get`, or `audit list`.
|
||||
- Any change to the admin `account list` columns or to the data model.
|
||||
- JSON output for the admin `account list` path (stays human-readable text).
|
||||
@@ -0,0 +1,118 @@
|
||||
# Send-as "From" address — design
|
||||
|
||||
**Date:** 2026-06-23
|
||||
**Status:** Approved (pending spec review)
|
||||
|
||||
## Problem
|
||||
|
||||
An account's configuration has no field for the email address used as the
|
||||
`From:` when sending mail. Today the From is silently aliased to the login
|
||||
username (`internal/cli/send.go:26`, `From: acc.Username`), and neither
|
||||
`store.Account` nor the `accounts` table has any `from`/`address`/`email`
|
||||
column.
|
||||
|
||||
This works only when the login username is exactly the desired send-as
|
||||
address. It breaks for:
|
||||
|
||||
- providers where the login is an account ID rather than an email,
|
||||
- sending from an **alias** of the mailbox,
|
||||
- wanting a **display name** (`Steve Cliff <me@…>`) rather than a bare address,
|
||||
- Gmail App Passwords where envelope-from and header-from may differ.
|
||||
|
||||
## Decisions
|
||||
|
||||
- **Field shape:** a single freeform RFC 5322 From identity — bare
|
||||
(`me@stevecliff.com`) or with a display name
|
||||
(`Steve Cliff <me@stevecliff.com>`). One field, not a split
|
||||
address/display-name pair.
|
||||
- **Fallback:** when the from-address is blank, fall back to
|
||||
`Account.Username` (current behaviour). No migration/backfill of existing
|
||||
accounts required; they keep working unchanged.
|
||||
|
||||
## Design
|
||||
|
||||
### 1. Data model
|
||||
|
||||
Add `FromAddress string` to `store.Account` and a `from_address TEXT` column to
|
||||
the `accounts` table. **Not encrypted** — it is not a secret (it appears in
|
||||
every outgoing header), so it is stored as plaintext like `username`.
|
||||
|
||||
### 2. Fallback in one place
|
||||
|
||||
Add a method so the fallback rule lives in exactly one spot:
|
||||
|
||||
```go
|
||||
// SendFrom returns the From identity for outgoing mail, falling back to the
|
||||
// login username when no explicit from-address is configured.
|
||||
func (a Account) SendFrom() string {
|
||||
if a.FromAddress != "" {
|
||||
return a.FromAddress
|
||||
}
|
||||
return a.Username
|
||||
}
|
||||
```
|
||||
|
||||
`internal/cli/send.go` changes `From: acc.Username` → `From: acc.SendFrom()`.
|
||||
|
||||
### 3. Envelope sender vs header From
|
||||
|
||||
A display-name From breaks the SMTP envelope: `c.SendMail("Steve <me@…>", …)`
|
||||
is invalid — the envelope sender must be the **bare** address.
|
||||
|
||||
- `mail.BuildMIME` keeps using the full `m.From` for the `From:` header
|
||||
(go-message renders `Name <addr>` correctly via `SetAddressList`).
|
||||
- `mail.SendSMTP` derives the envelope sender as the bare address:
|
||||
`gomail.ParseAddress(m.From).Address`. If parsing fails, fall back to the raw
|
||||
`m.From` (preserves today's behaviour for plain addresses).
|
||||
|
||||
Header carries the display name; envelope carries the bare address.
|
||||
|
||||
### 4. Migration
|
||||
|
||||
The schema is v1, applied via `CREATE TABLE … IF NOT EXISTS`, which will not add
|
||||
a column to an existing DB, and there is no migration runner yet.
|
||||
|
||||
- Add the `from_address` column to the `CREATE TABLE accounts` statement (new
|
||||
DBs get it directly).
|
||||
- In `store.Open`, after applying the schema, run a version-gated migration: if
|
||||
the stored `schema_version` is `< 2`, execute
|
||||
`ALTER TABLE accounts ADD COLUMN from_address TEXT` and set `schema_version`
|
||||
to `2`.
|
||||
- Bump the `schemaVersion` constant to `2`.
|
||||
|
||||
SQLite `ALTER TABLE … ADD COLUMN` is cheap and safe. The migration is
|
||||
idempotent under the version gate.
|
||||
|
||||
### 5. Inputs & validation
|
||||
|
||||
- **CLI:** add a `--from` flag to `account add` and `account edit`. On `edit`,
|
||||
follow the existing `fs.Visit` overlay pattern (only set when the flag was
|
||||
passed).
|
||||
- **TUI:** add a `FromAddress` field to `tui.Fields`, a `fieldDef`
|
||||
(`{key: "from_address", label: "From address (optional)"}`), and wire it
|
||||
through `ToAccount`, `FieldsFromAccount`, `fieldValue`, and `collect`.
|
||||
- **Validation:** when the from-address is non-empty, reject it unless
|
||||
`gomail.ParseAddress` accepts it (covers bare and display-name forms). A
|
||||
blank from-address is always valid (→ fallback). Applied in
|
||||
`Fields.Validate` and on the `account add`/`edit` flag path.
|
||||
- **Persistence:** thread `from_address` through `AddAccount`, `UpdateAccount`,
|
||||
`scanAccount`, and the three `SELECT` column lists in
|
||||
`internal/store/account.go`. Stored via `nullStr` (blank → NULL).
|
||||
|
||||
### 6. Tests
|
||||
|
||||
- `SendFrom()`: returns the from-address when set; returns username when blank.
|
||||
- `SendSMTP` envelope: when From is `Name <addr>`, the envelope sender passed to
|
||||
the server is the bare `addr` (table test on the extraction helper).
|
||||
- Migration: open a v1 DB with no `from_address` column → column is added,
|
||||
`schema_version` becomes 2, and an existing account still sends from its
|
||||
username.
|
||||
- `Fields.Validate`: rejects a malformed from-address; accepts bare and
|
||||
display-name forms; accepts blank.
|
||||
- Round-trip: `FieldsFromAccount` then `ToAccount` preserves `FromAddress`.
|
||||
|
||||
## Out of scope (YAGNI)
|
||||
|
||||
- Separate envelope-from override field (derive it from From instead).
|
||||
- Per-message From override at send time.
|
||||
- Multiple aliases per account.
|
||||
@@ -0,0 +1,85 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// With only EMCLI_KEY set, `account list` emits the reduced JSON envelope:
|
||||
// name/from/can_send, and never the IMAP host or login username.
|
||||
func TestAccountListAgentJSONView(t *testing.T) {
|
||||
adminEnv(t) // both keys + initialized temp DB
|
||||
run(t, "account", "add", "--name", "work", "--mode", "RW",
|
||||
"--imap-host", "imap.example.com", "--smtp-host", "smtp.example.com",
|
||||
"--username", "login@example.com", "--from", "me@example.com")
|
||||
run(t, "account", "add", "--name", "alerts", "--mode", "RO",
|
||||
"--imap-host", "imap.example.com", "--username", "alerts@example.com")
|
||||
|
||||
// Drop the admin key → caller is an agent.
|
||||
t.Setenv("EMCLI_ADMIN_KEY", "")
|
||||
code, out, errOut := run(t, "account", "list")
|
||||
if code != 0 {
|
||||
t.Fatalf("agent account list should succeed: code=%d err=%q", code, errOut)
|
||||
}
|
||||
|
||||
var env struct {
|
||||
Error bool `json:"error"`
|
||||
Data struct {
|
||||
Accounts []struct {
|
||||
Name string `json:"name"`
|
||||
From string `json:"from"`
|
||||
CanSend bool `json:"can_send"`
|
||||
} `json:"accounts"`
|
||||
} `json:"data"`
|
||||
}
|
||||
if err := json.Unmarshal([]byte(out), &env); err != nil {
|
||||
t.Fatalf("output is not the agent envelope: %v\n%s", err, out)
|
||||
}
|
||||
if env.Error || len(env.Data.Accounts) != 2 {
|
||||
t.Fatalf("want 2 accounts and no error, got %+v", env)
|
||||
}
|
||||
// The reduced view must not leak the IMAP host or the login username.
|
||||
if strings.Contains(out, "imap.example.com") || strings.Contains(out, "login@example.com") {
|
||||
t.Fatalf("agent view leaked host/username:\n%s", out)
|
||||
}
|
||||
|
||||
got := map[string]struct {
|
||||
from string
|
||||
canSend bool
|
||||
}{}
|
||||
for _, a := range env.Data.Accounts {
|
||||
got[a.Name] = struct {
|
||||
from string
|
||||
canSend bool
|
||||
}{a.From, a.CanSend}
|
||||
}
|
||||
if g := got["work"]; g.from != "me@example.com" || !g.canSend {
|
||||
t.Errorf("work: want from=me@example.com can_send=true, got %+v", g)
|
||||
}
|
||||
// alerts has no --from → SendFrom() falls back to the username.
|
||||
if g := got["alerts"]; g.from != "alerts@example.com" || g.canSend {
|
||||
t.Errorf("alerts: want from=alerts@example.com can_send=false, got %+v", g)
|
||||
}
|
||||
}
|
||||
|
||||
// With the admin key present, `account list` stays the full human-readable table.
|
||||
func TestAccountListAdminTextView(t *testing.T) {
|
||||
adminEnv(t)
|
||||
run(t, "account", "add", "--name", "work", "--mode", "RW",
|
||||
"--imap-host", "imap.example.com", "--smtp-host", "smtp.example.com",
|
||||
"--username", "login@example.com", "--from", "me@example.com")
|
||||
|
||||
code, out, _ := run(t, "account", "list")
|
||||
if code != 0 {
|
||||
t.Fatalf("admin account list failed: code=%d", code)
|
||||
}
|
||||
for _, want := range []string{"NAME", "MODE", "IMAP", "USER", "imap.example.com:993", "login@example.com"} {
|
||||
if !strings.Contains(out, want) {
|
||||
t.Fatalf("admin view missing %q:\n%s", want, out)
|
||||
}
|
||||
}
|
||||
if strings.Contains(out, `"accounts"`) {
|
||||
t.Fatalf("admin view should be text, not JSON:\n%s", out)
|
||||
}
|
||||
}
|
||||
+55
-5
@@ -6,6 +6,7 @@ import (
|
||||
"io"
|
||||
"strconv"
|
||||
|
||||
"git.dcglab.co.uk/steve/emcli/internal/crypto"
|
||||
"git.dcglab.co.uk/steve/emcli/internal/store"
|
||||
"git.dcglab.co.uk/steve/emcli/internal/tui"
|
||||
)
|
||||
@@ -23,7 +24,14 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
|
||||
sub, rest := args[0], args[1:]
|
||||
st, err := openStore(role)
|
||||
if err != nil {
|
||||
fmt.Fprintf(errOut, "emcli: %v\n", err)
|
||||
// account list is an agent command (a JSON consumer), so its
|
||||
// open/key failures are emitted as an envelope, like the other agent
|
||||
// commands; the admin subcommands stay human-readable.
|
||||
if sub == "list" {
|
||||
_ = Failure(CodeConfig, err.Error()).Write(out)
|
||||
} else {
|
||||
fmt.Fprintf(errOut, "emcli: %v\n", err)
|
||||
}
|
||||
return 1
|
||||
}
|
||||
defer st.Close()
|
||||
@@ -45,6 +53,7 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
|
||||
smtpSec := fs.String("smtp-security", "tls", "tls|starttls")
|
||||
user := fs.String("username", "", "login username")
|
||||
pass := fs.String("password", "", "login password")
|
||||
from := fs.String("from", "", "send-as address (blank = use username)")
|
||||
subj := fs.String("subject-regex", "", "inbound subject filter")
|
||||
wlIn := fs.Bool("whitelist-in", false, "enable inbound whitelist")
|
||||
wlOut := fs.Bool("whitelist-out", false, "enable outbound whitelist")
|
||||
@@ -56,9 +65,14 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
|
||||
fmt.Fprintln(errOut, "name, imap-host, and username are required")
|
||||
return 2
|
||||
}
|
||||
if err := tui.ValidFromAddress(*from); err != nil {
|
||||
fmt.Fprintln(errOut, err)
|
||||
return 2
|
||||
}
|
||||
acc := store.Account{
|
||||
Name: *name, Mode: *mode, IMAPHost: *host, IMAPPort: *port, IMAPSecurity: *sec,
|
||||
AuthType: "password", Username: *user, Password: *pass,
|
||||
FromAddress: *from,
|
||||
SubjectRegex: *subj, WhitelistInEnabled: *wlIn, WhitelistOutEnabled: *wlOut,
|
||||
ProcessBacklog: *backlog,
|
||||
}
|
||||
@@ -85,6 +99,7 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
|
||||
smtpSec := fs.String("smtp-security", "", "tls|starttls")
|
||||
user := fs.String("username", "", "login username")
|
||||
pass := fs.String("password", "", "login password (blank keeps existing)")
|
||||
from := fs.String("from", "", "send-as address (empty reverts to username)")
|
||||
subj := fs.String("subject-regex", "", "inbound subject filter")
|
||||
if err := fs.Parse(rest); err != nil {
|
||||
return 2
|
||||
@@ -96,6 +111,10 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
|
||||
if fs.NFlag() == 1 { // only --name → interactive TUI form, prefilled
|
||||
return editInteractive(st, *name, out, errOut)
|
||||
}
|
||||
if err := tui.ValidFromAddress(*from); err != nil {
|
||||
fmt.Fprintln(errOut, err)
|
||||
return 2
|
||||
}
|
||||
acc, err := st.GetAccount(*name)
|
||||
if err != nil {
|
||||
fmt.Fprintf(errOut, "edit: %v\n", err)
|
||||
@@ -122,6 +141,8 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
|
||||
acc.Username = *user
|
||||
case "password":
|
||||
acc.Password = *pass
|
||||
case "from":
|
||||
acc.FromAddress = *from
|
||||
case "subject-regex":
|
||||
acc.SubjectRegex = *subj
|
||||
}
|
||||
@@ -158,11 +179,31 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
|
||||
fmt.Fprintf(out, "account %q removed\n", *name)
|
||||
return 0
|
||||
case "list":
|
||||
// Holding the admin key means the caller is the human admin (full
|
||||
// detail). An agent holds only EMCLI_KEY and gets a reduced JSON view.
|
||||
_, adminErr := crypto.AdminKeyFromEnv()
|
||||
isAdmin := adminErr == nil
|
||||
accs, err := st.ListAccounts()
|
||||
if err != nil {
|
||||
fmt.Fprintf(errOut, "list: %v\n", err)
|
||||
if isAdmin {
|
||||
fmt.Fprintf(errOut, "list: %v\n", err)
|
||||
} else {
|
||||
_ = Failure(CodeDB, err.Error()).Write(out)
|
||||
}
|
||||
return 1
|
||||
}
|
||||
if !isAdmin {
|
||||
items := make([]map[string]any, 0, len(accs))
|
||||
for _, a := range accs {
|
||||
items = append(items, map[string]any{
|
||||
"name": a.Name,
|
||||
"from": a.SendFrom(),
|
||||
"can_send": a.Mode == "RW",
|
||||
})
|
||||
}
|
||||
_ = Success(map[string]any{"accounts": items}).Write(out)
|
||||
return 0
|
||||
}
|
||||
fmt.Fprintf(out, "%-16s %-4s %-28s %s\n", "NAME", "MODE", "IMAP", "USER")
|
||||
for _, a := range accs {
|
||||
fmt.Fprintf(out, "%-16s %-4s %-28s %s\n",
|
||||
@@ -289,7 +330,19 @@ func runWhitelist(args []string, role store.Role, out, errOut io.Writer) int {
|
||||
return 2
|
||||
}
|
||||
dir := store.Direction(args[0])
|
||||
if dir != store.DirIn && dir != store.DirOut {
|
||||
fmt.Fprintf(errOut, "whitelist direction must be \"in\" or \"out\", got %q\n", args[0])
|
||||
fmt.Fprintln(errOut, "usage: emcli whitelist <in|out> <add|remove|list> [flags]")
|
||||
return 2
|
||||
}
|
||||
sub, rest := args[1], args[2:]
|
||||
switch sub {
|
||||
case "add", "remove", "list": // valid
|
||||
default:
|
||||
fmt.Fprintf(errOut, "unknown whitelist subcommand %q (want add|remove|list)\n", sub)
|
||||
fmt.Fprintln(errOut, "usage: emcli whitelist <in|out> <add|remove|list> [flags]")
|
||||
return 2
|
||||
}
|
||||
fs := flag.NewFlagSet("whitelist", flag.ContinueOnError)
|
||||
fs.SetOutput(errOut)
|
||||
account := fs.String("account", "", "account name")
|
||||
@@ -330,9 +383,6 @@ func runWhitelist(args []string, role store.Role, out, errOut io.Writer) int {
|
||||
for _, a := range addrs {
|
||||
fmt.Fprintln(out, a)
|
||||
}
|
||||
default:
|
||||
fmt.Fprintf(errOut, "unknown whitelist subcommand %q\n", sub)
|
||||
return 2
|
||||
}
|
||||
return 0
|
||||
}
|
||||
|
||||
@@ -110,6 +110,51 @@ func TestAccountEditPartialPreservesOtherFields(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A missing direction (e.g. `whitelist list`) must report the real problem —
|
||||
// the in|out direction — not the misleading "--account is required".
|
||||
func TestWhitelistMissingDirectionReported(t *testing.T) {
|
||||
adminEnv(t)
|
||||
code, _, errOut := run(t, "whitelist", "list", "--account", "bobby")
|
||||
if code == 0 {
|
||||
t.Fatal("missing direction must be a usage error")
|
||||
}
|
||||
if strings.Contains(errOut, "--account is required") {
|
||||
t.Fatalf("misleading error; want a direction complaint, got: %q", errOut)
|
||||
}
|
||||
if !strings.Contains(errOut, "in") || !strings.Contains(errOut, "out") {
|
||||
t.Fatalf("error should name the in|out direction, got: %q", errOut)
|
||||
}
|
||||
}
|
||||
|
||||
// A missing subcommand (e.g. `whitelist out --account x`) must report the real
|
||||
// problem — the add|remove|list subcommand — not "--account is required".
|
||||
func TestWhitelistMissingSubcommandReported(t *testing.T) {
|
||||
adminEnv(t)
|
||||
code, _, errOut := run(t, "whitelist", "out", "--account", "bobby")
|
||||
if code == 0 {
|
||||
t.Fatal("missing subcommand must be a usage error")
|
||||
}
|
||||
if strings.Contains(errOut, "--account is required") {
|
||||
t.Fatalf("misleading error; want a subcommand complaint, got: %q", errOut)
|
||||
}
|
||||
if !strings.Contains(errOut, "add") || !strings.Contains(errOut, "list") {
|
||||
t.Fatalf("error should name the add|remove|list subcommand, got: %q", errOut)
|
||||
}
|
||||
}
|
||||
|
||||
// The happy path still works after the direction/subcommand validation.
|
||||
func TestWhitelistListWorks(t *testing.T) {
|
||||
adminEnv(t)
|
||||
run(t, "account", "add", "--name", "bobby", "--imap-host", "h", "--username", "u@x.com")
|
||||
if code, _, e := run(t, "whitelist", "out", "add", "--account", "bobby", "--address", "@x.com"); code != 0 {
|
||||
t.Fatalf("add failed: %s", e)
|
||||
}
|
||||
code, out, _ := run(t, "whitelist", "out", "list", "--account", "bobby")
|
||||
if code != 0 || !strings.Contains(out, "@x.com") {
|
||||
t.Fatalf("list: code=%d out=%q", code, out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuditListCoreRenders(t *testing.T) {
|
||||
st, err := store.Open(filepath.Join(t.TempDir(), "e.db"))
|
||||
if err != nil {
|
||||
@@ -132,3 +177,17 @@ func TestAuditListCoreRenders(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestAccountEditFromValidationRejectsMalformed(t *testing.T) {
|
||||
adminEnv(t)
|
||||
// Seed an account so the failure is from --from validation, not a missing account.
|
||||
run(t, "account", "add", "--name", "valacc", "--imap-host", "imap.x.com", "--username", "u@x.com")
|
||||
// A malformed --from value must be rejected with exit code 2 before touching the account.
|
||||
code, _, errStr := run(t, "account", "edit", "--name", "valacc", "--from", "not an address")
|
||||
if code != 2 {
|
||||
t.Fatalf("expected exit code 2 for malformed --from, got %d (stderr: %q)", code, errStr)
|
||||
}
|
||||
if errStr == "" {
|
||||
t.Fatal("expected an error message on stderr for malformed --from, got none")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -9,16 +9,22 @@ import (
|
||||
)
|
||||
|
||||
func TestCommandRole(t *testing.T) {
|
||||
admin := []string{"account", "whitelist", "config", "audit"}
|
||||
agent := []string{"list", "get", "search", "ack", "send", "doctor"}
|
||||
for _, c := range admin {
|
||||
adminCmds := [][]string{
|
||||
{"whitelist"}, {"config"}, {"audit"},
|
||||
{"account"}, {"account", "add"}, {"account", "edit"}, {"account", "remove"},
|
||||
}
|
||||
agentCmds := [][]string{
|
||||
{"list"}, {"get"}, {"search"}, {"ack"}, {"send"}, {"doctor"},
|
||||
{"account", "list"},
|
||||
}
|
||||
for _, c := range adminCmds {
|
||||
if commandRole(c) != store.RoleAdmin {
|
||||
t.Errorf("%s should be admin", c)
|
||||
t.Errorf("%v should be admin", c)
|
||||
}
|
||||
}
|
||||
for _, c := range agent {
|
||||
for _, c := range agentCmds {
|
||||
if commandRole(c) != store.RoleAgent {
|
||||
t.Errorf("%s should be agent", c)
|
||||
t.Errorf("%v should be agent", c)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+11
-4
@@ -27,9 +27,16 @@ func realMailer(acc store.Account) (Mailer, error) {
|
||||
|
||||
// commandRole maps a command to the privilege it requires. Admin commands
|
||||
// mutate configuration or expose oversight data; everything else is agent.
|
||||
func commandRole(cmd string) store.Role {
|
||||
switch cmd {
|
||||
case "account", "whitelist", "config", "audit":
|
||||
func commandRole(args []string) store.Role {
|
||||
switch args[0] {
|
||||
case "account":
|
||||
// account list is a read-only discovery view available to agents;
|
||||
// add/edit/remove mutate config and require admin.
|
||||
if len(args) >= 2 && args[1] == "list" {
|
||||
return store.RoleAgent
|
||||
}
|
||||
return store.RoleAdmin
|
||||
case "whitelist", "config", "audit":
|
||||
return store.RoleAdmin
|
||||
default: // list, get, search, ack, send, doctor
|
||||
return store.RoleAgent
|
||||
@@ -135,7 +142,7 @@ func Run(args []string, out, errOut io.Writer) int {
|
||||
return 0
|
||||
}
|
||||
cmd, rest := args[0], args[1:]
|
||||
role := commandRole(cmd)
|
||||
role := commandRole(args)
|
||||
switch cmd {
|
||||
case "list", "get", "search", "ack":
|
||||
return runAgent(cmd, rest, role, out, errOut)
|
||||
|
||||
@@ -18,9 +18,9 @@ func TestRunUnknownCommand(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunVersionIsJSONForAgentButTextHere(t *testing.T) {
|
||||
// `account list` with no DB key should fail closed with a usage/config error,
|
||||
// proving the key check happens before any DB work.
|
||||
func TestAccountListMissingKeyFailsClosedAsJSON(t *testing.T) {
|
||||
// `account list` is an agent command: with no DB key it fails closed before
|
||||
// any DB work, emitting a JSON config-error envelope that names EMCLI_KEY.
|
||||
var out, errOut bytes.Buffer
|
||||
t.Setenv("EMCLI_KEY", "")
|
||||
t.Setenv("EMCLI_ADMIN_KEY", "")
|
||||
@@ -28,8 +28,15 @@ func TestRunVersionIsJSONForAgentButTextHere(t *testing.T) {
|
||||
if code == 0 {
|
||||
t.Fatal("missing EMCLI_KEY must fail")
|
||||
}
|
||||
if !strings.Contains(out.String()+errOut.String(), "EMCLI_ADMIN_KEY") {
|
||||
t.Fatalf("should mention EMCLI_ADMIN_KEY, got out=%q err=%q", out.String(), errOut.String())
|
||||
var env map[string]any
|
||||
if err := json.Unmarshal(out.Bytes(), &env); err != nil {
|
||||
t.Fatalf("agent account list error must be JSON, got out=%q err=%q", out.String(), errOut.String())
|
||||
}
|
||||
if env["error"] != true {
|
||||
t.Fatalf("want error envelope: %v", env)
|
||||
}
|
||||
if !strings.Contains(out.String(), "EMCLI_KEY") {
|
||||
t.Fatalf("should name the missing EMCLI_KEY, got %q", out.String())
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -49,7 +49,7 @@ func TestAgentKeyCannotRunAdminCommands(t *testing.T) {
|
||||
|
||||
before := dbBytes(t, db)
|
||||
adminAttempts := [][]string{
|
||||
{"account", "list"},
|
||||
{"account", "add", "--name", "x", "--imap-host", "h", "--username", "u@x.com"},
|
||||
{"config", "set", "audit_retention_days", "30"},
|
||||
{"audit"},
|
||||
}
|
||||
|
||||
@@ -23,7 +23,7 @@ func SendCmd(d Deps, account string, to, cc, bcc []string, subject, body string,
|
||||
}
|
||||
|
||||
msg := mail.OutgoingMessage{
|
||||
From: acc.Username, To: to, Cc: cc, Bcc: bcc,
|
||||
From: acc.SendFrom(), To: to, Cc: cc, Bcc: bcc,
|
||||
Subject: subject, BodyText: body,
|
||||
}
|
||||
recipients := msg.Recipients()
|
||||
|
||||
@@ -139,6 +139,32 @@ func TestSendReplyToThreadsHeaders(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSendUsesConfiguredFromAddress(t *testing.T) {
|
||||
acc := rwAccount()
|
||||
acc.FromAddress = "Steve Cliff <me@stevecliff.com>"
|
||||
d, sent, _ := sendDeps(t, acc, nil)
|
||||
if err := SendCmd(d, "send", []string{"me@stevecliff.com"}, nil, nil, "hi", "body", nil, 0, "INBOX"); err != nil {
|
||||
t.Fatalf("SendCmd: %v", err)
|
||||
}
|
||||
if len(*sent) != 1 {
|
||||
t.Fatalf("want 1 send, got %d", len(*sent))
|
||||
}
|
||||
if got := (*sent)[0].From; got != "Steve Cliff <me@stevecliff.com>" {
|
||||
t.Fatalf("From = %q, want configured from-address", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSendFallsBackToUsernameAsFrom(t *testing.T) {
|
||||
// rwAccount has no FromAddress, so From must be the login username.
|
||||
d, sent, _ := sendDeps(t, rwAccount(), nil)
|
||||
if err := SendCmd(d, "send", []string{"me@stevecliff.com"}, nil, nil, "hi", "body", nil, 0, "INBOX"); err != nil {
|
||||
t.Fatalf("SendCmd: %v", err)
|
||||
}
|
||||
if got := (*sent)[0].From; got != "emcli@stevecliff.com" {
|
||||
t.Fatalf("From = %q, want username fallback", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSendReplyToFilteredSourceNotFound(t *testing.T) {
|
||||
acc := rwAccount()
|
||||
acc.WhitelistInEnabled = true // inbound filter active
|
||||
|
||||
+13
-1
@@ -46,6 +46,18 @@ func (m OutgoingMessage) Recipients() []string {
|
||||
return out
|
||||
}
|
||||
|
||||
// envelopeFrom returns the bare address for the SMTP envelope sender, stripping
|
||||
// any display name. A display-name From (e.g. "Name <addr>") is a valid header
|
||||
// but an invalid envelope sender, so it must be reduced to the bare address.
|
||||
// Unparseable input is passed through unchanged (preserves prior behaviour for
|
||||
// plain addresses).
|
||||
func envelopeFrom(from string) string {
|
||||
if a, err := gomail.ParseAddress(from); err == nil {
|
||||
return a.Address
|
||||
}
|
||||
return from
|
||||
}
|
||||
|
||||
func addrList(addrs []string) []*gomail.Address {
|
||||
out := make([]*gomail.Address, 0, len(addrs))
|
||||
for _, a := range addrs {
|
||||
@@ -163,7 +175,7 @@ func SendSMTP(cfg SMTPConfig, m OutgoingMessage) error {
|
||||
if err := c.Auth(auth); err != nil {
|
||||
return fmt.Errorf("smtp auth: %w", err)
|
||||
}
|
||||
if err := c.SendMail(m.From, m.Recipients(), bytes.NewReader(raw)); err != nil {
|
||||
if err := c.SendMail(envelopeFrom(m.From), m.Recipients(), bytes.NewReader(raw)); err != nil {
|
||||
return fmt.Errorf("smtp send: %w", err)
|
||||
}
|
||||
return c.Quit()
|
||||
|
||||
@@ -100,6 +100,36 @@ func TestRecipientsCombinesAllFields(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnvelopeFromStripsDisplayName(t *testing.T) {
|
||||
cases := map[string]string{
|
||||
"Steve Cliff <me@stevecliff.com>": "me@stevecliff.com",
|
||||
"me@stevecliff.com": "me@stevecliff.com",
|
||||
"<me@stevecliff.com>": "me@stevecliff.com",
|
||||
"not a valid address": "not a valid address", // unparseable ⇒ passthrough
|
||||
}
|
||||
for in, want := range cases {
|
||||
if got := envelopeFrom(in); got != want {
|
||||
t.Errorf("envelopeFrom(%q) = %q, want %q", in, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildMIMEKeepsDisplayNameInHeader(t *testing.T) {
|
||||
raw, err := BuildMIME(OutgoingMessage{
|
||||
From: "Steve Cliff <me@stevecliff.com>",
|
||||
To: []string{"you@example.com"},
|
||||
Subject: "hi",
|
||||
BodyText: "body",
|
||||
Date: time.Date(2026, 6, 23, 12, 0, 0, 0, time.UTC),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("BuildMIME: %v", err)
|
||||
}
|
||||
if !strings.Contains(string(raw), "Steve Cliff") {
|
||||
t.Fatalf("From header lost display name:\n%s", raw)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadHeaderParsesReferences(t *testing.T) {
|
||||
raw := "From: a@x.com\r\n" +
|
||||
"To: b@x.com\r\n" +
|
||||
|
||||
+25
-14
@@ -23,6 +23,7 @@ type Account struct {
|
||||
SMTPSecurity string // tls | starttls
|
||||
AuthType string // password | oauth2
|
||||
Username string
|
||||
FromAddress string // send-as identity; blank ⇒ fall back to Username
|
||||
Password string // decrypted; empty in ListAccounts
|
||||
WhitelistInEnabled bool
|
||||
WhitelistOutEnabled bool
|
||||
@@ -30,6 +31,15 @@ type Account struct {
|
||||
ProcessBacklog bool
|
||||
}
|
||||
|
||||
// SendFrom returns the From identity for outgoing mail, falling back to the
|
||||
// login username when no explicit from-address is configured.
|
||||
func (a Account) SendFrom() string {
|
||||
if a.FromAddress != "" {
|
||||
return a.FromAddress
|
||||
}
|
||||
return a.Username
|
||||
}
|
||||
|
||||
func (s *Store) AddAccount(a Account) (int64, error) {
|
||||
var encPw []byte
|
||||
if a.Password != "" {
|
||||
@@ -42,12 +52,12 @@ func (s *Store) AddAccount(a Account) (int64, error) {
|
||||
res, err := s.db.Exec(`
|
||||
INSERT INTO accounts
|
||||
(name,mode,imap_host,imap_port,imap_security,smtp_host,smtp_port,smtp_security,
|
||||
auth_type,username,
|
||||
auth_type,username,from_address,
|
||||
enc_password,whitelist_in_enabled,whitelist_out_enabled,subject_regex,process_backlog)
|
||||
VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)`,
|
||||
VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)`,
|
||||
a.Name, a.Mode, a.IMAPHost, a.IMAPPort, a.IMAPSecurity,
|
||||
nullStr(a.SMTPHost), nullInt(a.SMTPPort), nullStr(a.SMTPSecurity),
|
||||
a.AuthType, a.Username,
|
||||
a.AuthType, a.Username, nullStr(a.FromAddress),
|
||||
encPw, b2i(a.WhitelistInEnabled), b2i(a.WhitelistOutEnabled),
|
||||
nullStr(a.SubjectRegex), b2i(a.ProcessBacklog))
|
||||
if err != nil {
|
||||
@@ -59,7 +69,7 @@ func (s *Store) AddAccount(a Account) (int64, error) {
|
||||
func (s *Store) GetAccount(name string) (Account, error) {
|
||||
row := s.db.QueryRow(`
|
||||
SELECT id,name,mode,imap_host,imap_port,imap_security,smtp_host,smtp_port,smtp_security,
|
||||
auth_type,username,
|
||||
auth_type,username,from_address,
|
||||
enc_password,whitelist_in_enabled,whitelist_out_enabled,subject_regex,process_backlog
|
||||
FROM accounts WHERE name = ?`, name)
|
||||
a, encPw, err := scanAccount(row)
|
||||
@@ -82,7 +92,7 @@ func (s *Store) GetAccount(name string) (Account, error) {
|
||||
func (s *Store) ListAccounts() ([]Account, error) {
|
||||
rows, err := s.db.Query(`
|
||||
SELECT id,name,mode,imap_host,imap_port,imap_security,smtp_host,smtp_port,smtp_security,
|
||||
auth_type,username,
|
||||
auth_type,username,from_address,
|
||||
enc_password,whitelist_in_enabled,whitelist_out_enabled,subject_regex,process_backlog
|
||||
FROM accounts ORDER BY name`)
|
||||
if err != nil {
|
||||
@@ -108,12 +118,12 @@ func (s *Store) UpdateAccount(a Account) error {
|
||||
// Build the SET clause, conditionally including secret columns.
|
||||
set := `mode=?, imap_host=?, imap_port=?, imap_security=?,
|
||||
smtp_host=?, smtp_port=?, smtp_security=?,
|
||||
auth_type=?, username=?,
|
||||
auth_type=?, username=?, from_address=?,
|
||||
whitelist_in_enabled=?, whitelist_out_enabled=?, subject_regex=?, process_backlog=?`
|
||||
args := []any{
|
||||
a.Mode, a.IMAPHost, a.IMAPPort, a.IMAPSecurity,
|
||||
nullStr(a.SMTPHost), nullInt(a.SMTPPort), nullStr(a.SMTPSecurity),
|
||||
a.AuthType, a.Username,
|
||||
a.AuthType, a.Username, nullStr(a.FromAddress),
|
||||
b2i(a.WhitelistInEnabled), b2i(a.WhitelistOutEnabled),
|
||||
nullStr(a.SubjectRegex), b2i(a.ProcessBacklog),
|
||||
}
|
||||
@@ -152,16 +162,16 @@ type scanner interface{ Scan(dest ...any) error }
|
||||
|
||||
func scanAccount(sc scanner) (Account, []byte, error) {
|
||||
var (
|
||||
a Account
|
||||
encPw []byte
|
||||
subj, smtpHost, smtpSec sql.NullString
|
||||
smtpPort sql.NullInt64
|
||||
wlIn, wlOut int
|
||||
backlog int
|
||||
a Account
|
||||
encPw []byte
|
||||
subj, smtpHost, smtpSec, fromAddr sql.NullString
|
||||
smtpPort sql.NullInt64
|
||||
wlIn, wlOut int
|
||||
backlog int
|
||||
)
|
||||
err := sc.Scan(&a.ID, &a.Name, &a.Mode, &a.IMAPHost, &a.IMAPPort, &a.IMAPSecurity,
|
||||
&smtpHost, &smtpPort, &smtpSec,
|
||||
&a.AuthType, &a.Username, &encPw, &wlIn, &wlOut, &subj, &backlog)
|
||||
&a.AuthType, &a.Username, &fromAddr, &encPw, &wlIn, &wlOut, &subj, &backlog)
|
||||
if err != nil {
|
||||
return Account{}, nil, err
|
||||
}
|
||||
@@ -172,6 +182,7 @@ func scanAccount(sc scanner) (Account, []byte, error) {
|
||||
a.WhitelistOutEnabled = wlOut != 0
|
||||
a.ProcessBacklog = backlog != 0
|
||||
a.SubjectRegex = subj.String
|
||||
a.FromAddress = fromAddr.String
|
||||
return a, encPw, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -84,3 +84,30 @@ func TestListAccountsOmitsSecrets(t *testing.T) {
|
||||
t.Fatal("ListAccounts must not return secrets")
|
||||
}
|
||||
}
|
||||
|
||||
func TestSendFromFallsBackToUsername(t *testing.T) {
|
||||
a := Account{Username: "login@example.com"}
|
||||
if got := a.SendFrom(); got != "login@example.com" {
|
||||
t.Fatalf("blank from-address should fall back to username, got %q", got)
|
||||
}
|
||||
a.FromAddress = "Steve Cliff <me@stevecliff.com>"
|
||||
if got := a.SendFrom(); got != "Steve Cliff <me@stevecliff.com>" {
|
||||
t.Fatalf("set from-address should win, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAddGetAccountRoundTripsFromAddress(t *testing.T) {
|
||||
s := openTemp(t)
|
||||
a := sampleAccount()
|
||||
a.FromAddress = "Steve Cliff <me@stevecliff.com>"
|
||||
if _, err := s.AddAccount(a); err != nil {
|
||||
t.Fatalf("AddAccount: %v", err)
|
||||
}
|
||||
got, err := s.GetAccount("work")
|
||||
if err != nil {
|
||||
t.Fatalf("GetAccount: %v", err)
|
||||
}
|
||||
if got.FromAddress != "Steve Cliff <me@stevecliff.com>" {
|
||||
t.Fatalf("FromAddress not round-tripped: %q", got.FromAddress)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
package store
|
||||
|
||||
const schemaVersion = 1
|
||||
const schemaVersion = 2
|
||||
|
||||
// schemaSQL is the full v1 schema. All statements are idempotent via IF NOT EXISTS.
|
||||
// schemaSQL is the full current schema. All statements are idempotent via IF NOT EXISTS.
|
||||
const schemaSQL = `
|
||||
CREATE TABLE IF NOT EXISTS settings (
|
||||
key TEXT PRIMARY KEY,
|
||||
@@ -21,6 +21,7 @@ CREATE TABLE IF NOT EXISTS accounts (
|
||||
smtp_security TEXT,
|
||||
auth_type TEXT NOT NULL CHECK (auth_type IN ('password','oauth2')),
|
||||
username TEXT NOT NULL,
|
||||
from_address TEXT,
|
||||
enc_password BLOB,
|
||||
enc_oauth_client_id BLOB,
|
||||
enc_oauth_client_secret BLOB,
|
||||
|
||||
+45
-6
@@ -8,6 +8,7 @@ import (
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
_ "modernc.org/sqlite"
|
||||
)
|
||||
@@ -42,21 +43,59 @@ func Open(path string) (*Store, error) {
|
||||
return nil, fmt.Errorf("apply schema: %w", err)
|
||||
}
|
||||
s := &Store{db: db}
|
||||
if _, err := s.GetSetting("schema_version"); err != nil {
|
||||
if err := s.SetSetting("schema_version", strconv.Itoa(schemaVersion)); err != nil {
|
||||
db.Close()
|
||||
return nil, err
|
||||
}
|
||||
if err := s.migrate(); err != nil {
|
||||
db.Close()
|
||||
return nil, err
|
||||
}
|
||||
return s, nil
|
||||
}
|
||||
|
||||
// migrate brings an existing database up to the current schemaVersion. A brand-
|
||||
// new database (no schema_version yet) already has every column from schemaSQL,
|
||||
// so it is simply stamped at the current version. Each older version runs its
|
||||
// forward step. The version gate makes every step idempotent across reopens.
|
||||
func (s *Store) migrate() error {
|
||||
v, err := s.GetSetting("schema_version")
|
||||
if err != nil {
|
||||
// Fresh database: schemaSQL created all columns already.
|
||||
return s.SetSetting("schema_version", strconv.Itoa(schemaVersion))
|
||||
}
|
||||
var ver int
|
||||
ver, err = strconv.Atoi(v)
|
||||
if err != nil {
|
||||
return fmt.Errorf("invalid schema_version %q: %w", v, err)
|
||||
}
|
||||
if ver < 2 {
|
||||
if _, err := s.db.Exec(`ALTER TABLE accounts ADD COLUMN from_address TEXT`); err != nil {
|
||||
return fmt.Errorf("migrate to v2: %w", err)
|
||||
}
|
||||
if err := s.SetSetting("schema_version", "2"); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *Store) Close() error { return s.db.Close() }
|
||||
|
||||
// expandUserHome replaces a leading "~" or "~/" in p with the user's home
|
||||
// directory. Only a leading tilde is expanded (the usual shell convention) —
|
||||
// "~user" and a tilde elsewhere in the path are left untouched. This guards
|
||||
// against an EMCLI_DB set to a literal "~/..." (no shell to expand it), which
|
||||
// would otherwise be opened relative to the cwd and create a stray "~" dir.
|
||||
func expandUserHome(p string) string {
|
||||
if p == "~" || strings.HasPrefix(p, "~/") {
|
||||
if home, err := os.UserHomeDir(); err == nil {
|
||||
return filepath.Join(home, strings.TrimPrefix(p[1:], "/"))
|
||||
}
|
||||
}
|
||||
return p
|
||||
}
|
||||
|
||||
// DefaultDBPath resolves EMCLI_DB or the per-OS default location.
|
||||
func DefaultDBPath() (string, error) {
|
||||
if p := os.Getenv("EMCLI_DB"); p != "" {
|
||||
return p, nil
|
||||
return expandUserHome(p), nil
|
||||
}
|
||||
if runtime.GOOS == "windows" {
|
||||
if dir := os.Getenv("AppData"); dir != "" {
|
||||
|
||||
@@ -1,10 +1,54 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// A leading "~" in EMCLI_DB must be expanded to the home dir, so a literal
|
||||
// tilde (no shell to expand it) can't be opened relative to the cwd and
|
||||
// silently create a stray "~" directory.
|
||||
func TestDefaultDBPathExpandsLeadingTilde(t *testing.T) {
|
||||
home, err := os.UserHomeDir()
|
||||
if err != nil {
|
||||
t.Skipf("no home dir: %v", err)
|
||||
}
|
||||
cases := map[string]string{
|
||||
"~/.config/emcli/emcli.db": filepath.Join(home, ".config", "emcli", "emcli.db"),
|
||||
"~": home,
|
||||
}
|
||||
for in, want := range cases {
|
||||
t.Setenv("EMCLI_DB", in)
|
||||
got, err := DefaultDBPath()
|
||||
if err != nil {
|
||||
t.Fatalf("DefaultDBPath(%q): %v", in, err)
|
||||
}
|
||||
if got != want {
|
||||
t.Fatalf("EMCLI_DB=%q -> %q, want %q", in, got, want)
|
||||
}
|
||||
if strings.Contains(got, "~") {
|
||||
t.Fatalf("EMCLI_DB=%q left a literal tilde: %q", in, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A non-leading tilde or "~user" is NOT a path we should rewrite — leave it be.
|
||||
func TestDefaultDBPathLeavesOtherPathsUntouched(t *testing.T) {
|
||||
for _, p := range []string{"/var/lib/emcli.db", "./rel/emcli.db", "~user/db"} {
|
||||
t.Setenv("EMCLI_DB", p)
|
||||
got, err := DefaultDBPath()
|
||||
if err != nil {
|
||||
t.Fatalf("DefaultDBPath(%q): %v", p, err)
|
||||
}
|
||||
if got != p {
|
||||
t.Fatalf("EMCLI_DB=%q was rewritten to %q", p, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// openTemp opens a fresh store in a temp dir and initialises keys so that
|
||||
// account tests (which do crypto) work without needing their own setup.
|
||||
func openTemp(t *testing.T) *Store {
|
||||
@@ -28,7 +72,7 @@ func TestOpenCreatesSchemaAndIsIdempotent(t *testing.T) {
|
||||
t.Fatalf("first Open: %v", err)
|
||||
}
|
||||
v, err := s.GetSetting("schema_version")
|
||||
if err != nil || v != "1" {
|
||||
if err != nil || v != "2" {
|
||||
t.Fatalf("schema_version: %q err=%v", v, err)
|
||||
}
|
||||
s.Close()
|
||||
@@ -39,7 +83,7 @@ func TestOpenCreatesSchemaAndIsIdempotent(t *testing.T) {
|
||||
t.Fatalf("second Open: %v", err)
|
||||
}
|
||||
defer s2.Close()
|
||||
if v, _ := s2.GetSetting("schema_version"); v != "1" {
|
||||
if v, _ := s2.GetSetting("schema_version"); v != "2" {
|
||||
t.Fatalf("schema_version after reopen: %q", v)
|
||||
}
|
||||
}
|
||||
@@ -104,3 +148,66 @@ func TestForeignKeyCascade(t *testing.T) {
|
||||
t.Fatalf("whitelist_in row not cascade-deleted: count=%d err=%v", count, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOpenMigratesV1AddsFromAddress(t *testing.T) {
|
||||
p := filepath.Join(t.TempDir(), "emcli.db")
|
||||
|
||||
// Hand-build a v1 database: accounts table WITHOUT from_address, a settings
|
||||
// table pinned at schema_version=1, and one pre-existing account row.
|
||||
raw, err := sql.Open("sqlite", p)
|
||||
if err != nil {
|
||||
t.Fatalf("sql.Open: %v", err)
|
||||
}
|
||||
const v1Schema = `
|
||||
CREATE TABLE settings (key TEXT PRIMARY KEY, value TEXT NOT NULL);
|
||||
CREATE TABLE accounts (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
name TEXT UNIQUE NOT NULL,
|
||||
mode TEXT NOT NULL,
|
||||
imap_host TEXT NOT NULL,
|
||||
imap_port INTEGER NOT NULL,
|
||||
imap_security TEXT NOT NULL,
|
||||
smtp_host TEXT, smtp_port INTEGER, smtp_security TEXT,
|
||||
auth_type TEXT NOT NULL,
|
||||
username TEXT NOT NULL,
|
||||
enc_password BLOB,
|
||||
enc_oauth_client_id BLOB, enc_oauth_client_secret BLOB, enc_oauth_refresh_token BLOB,
|
||||
whitelist_in_enabled INTEGER NOT NULL DEFAULT 0,
|
||||
whitelist_out_enabled INTEGER NOT NULL DEFAULT 0,
|
||||
subject_regex TEXT,
|
||||
process_backlog INTEGER NOT NULL DEFAULT 0
|
||||
);
|
||||
INSERT INTO settings(key,value) VALUES ('schema_version','1');
|
||||
INSERT INTO accounts(name,mode,imap_host,imap_port,imap_security,auth_type,username)
|
||||
VALUES ('legacy','RO','imap.example.com',993,'tls','password','login@example.com');
|
||||
`
|
||||
if _, err := raw.Exec(v1Schema); err != nil {
|
||||
t.Fatalf("seed v1 schema: %v", err)
|
||||
}
|
||||
raw.Close()
|
||||
|
||||
// Open via the store: the migration must add from_address and bump to v2.
|
||||
s, err := Open(p)
|
||||
if err != nil {
|
||||
t.Fatalf("Open (migrate): %v", err)
|
||||
}
|
||||
defer s.Close()
|
||||
|
||||
if v, _ := s.GetSetting("schema_version"); v != "2" {
|
||||
t.Fatalf("schema_version after migrate: %q, want 2", v)
|
||||
}
|
||||
// ListAccounts SELECTs from_address; it would error if the column were missing.
|
||||
accs, err := s.ListAccounts()
|
||||
if err != nil {
|
||||
t.Fatalf("ListAccounts after migrate: %v", err)
|
||||
}
|
||||
if len(accs) != 1 {
|
||||
t.Fatalf("want 1 account after migrate, got %d", len(accs))
|
||||
}
|
||||
if accs[0].FromAddress != "" {
|
||||
t.Fatalf("legacy account FromAddress should be empty, got %q", accs[0].FromAddress)
|
||||
}
|
||||
if got := accs[0].SendFrom(); got != "login@example.com" {
|
||||
t.Fatalf("legacy account should send from username, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
+26
-1
@@ -6,6 +6,7 @@ package tui
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/mail"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
@@ -22,10 +23,24 @@ type Fields struct {
|
||||
IMAPHost, IMAPPort, IMAPSecurity string
|
||||
SMTPHost, SMTPPort, SMTPSecurity string
|
||||
Username, Password string
|
||||
FromAddress string
|
||||
WhitelistIn, WhitelistOut, ProcessBacklog bool
|
||||
SubjectRegex string
|
||||
}
|
||||
|
||||
// ValidFromAddress returns an error if s is set but is not a valid RFC 5322
|
||||
// address (bare or "Display Name <addr>"). A blank value is valid: sending
|
||||
// falls back to the login username.
|
||||
func ValidFromAddress(s string) error {
|
||||
if strings.TrimSpace(s) == "" {
|
||||
return nil
|
||||
}
|
||||
if _, err := mail.ParseAddress(s); err != nil {
|
||||
return errors.New("from address must be a valid email address or \"Name <email>\"")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func validSecurity(s string) bool { return s == "tls" || s == "starttls" }
|
||||
|
||||
// Validate checks required fields, enum fields, and numeric ports. RW accounts
|
||||
@@ -60,6 +75,9 @@ func (f Fields) Validate() error {
|
||||
return errors.New("smtp port must be a number")
|
||||
}
|
||||
}
|
||||
if err := ValidFromAddress(f.FromAddress); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -71,6 +89,7 @@ func (f Fields) ToAccount() (store.Account, bool) {
|
||||
Name: strings.TrimSpace(f.Name), Mode: f.Mode,
|
||||
IMAPHost: f.IMAPHost, IMAPPort: ip, IMAPSecurity: f.IMAPSecurity,
|
||||
AuthType: "password", Username: f.Username, Password: f.Password,
|
||||
FromAddress: f.FromAddress,
|
||||
WhitelistInEnabled: f.WhitelistIn, WhitelistOutEnabled: f.WhitelistOut,
|
||||
SubjectRegex: f.SubjectRegex, ProcessBacklog: f.ProcessBacklog,
|
||||
}
|
||||
@@ -95,7 +114,8 @@ func FieldsFromAccount(a store.Account) Fields {
|
||||
Name: a.Name, Mode: a.Mode,
|
||||
IMAPHost: a.IMAPHost, IMAPPort: itoaPort(a.IMAPPort), IMAPSecurity: a.IMAPSecurity,
|
||||
SMTPHost: a.SMTPHost, SMTPPort: itoaPort(a.SMTPPort), SMTPSecurity: a.SMTPSecurity,
|
||||
Username: a.Username,
|
||||
Username: a.Username,
|
||||
FromAddress: a.FromAddress,
|
||||
WhitelistIn: a.WhitelistInEnabled,
|
||||
WhitelistOut: a.WhitelistOutEnabled,
|
||||
ProcessBacklog: a.ProcessBacklog,
|
||||
@@ -122,6 +142,7 @@ var fieldDefs = []fieldDef{
|
||||
{key: "smtp_port", label: "SMTP port (RW)"},
|
||||
{key: "smtp_security", label: "SMTP security (tls/starttls)"},
|
||||
{key: "username", label: "Username"},
|
||||
{key: "from_address", label: "From address (optional)"},
|
||||
{key: "password", label: "Password", password: true},
|
||||
{key: "whitelist_in", label: "Whitelist inbound (y/n)", isBool: true},
|
||||
{key: "whitelist_out", label: "Whitelist outbound (y/n)", isBool: true},
|
||||
@@ -164,6 +185,8 @@ func fieldValue(f Fields, key string) string {
|
||||
return f.SMTPSecurity
|
||||
case "username":
|
||||
return f.Username
|
||||
case "from_address":
|
||||
return f.FromAddress
|
||||
case "password":
|
||||
return f.Password
|
||||
case "whitelist_in":
|
||||
@@ -249,6 +272,8 @@ func (m AccountForm) collect() Fields {
|
||||
f.SMTPSecurity = strings.ToLower(v)
|
||||
case "username":
|
||||
f.Username = v
|
||||
case "from_address":
|
||||
f.FromAddress = v
|
||||
case "password":
|
||||
f.Password = m.inputs[i].Value() // do not trim a password
|
||||
case "whitelist_in":
|
||||
|
||||
@@ -157,3 +157,36 @@ func TestAccountFormCancel(t *testing.T) {
|
||||
t.Fatal("esc should cancel the form")
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateRejectsBadFromAddress(t *testing.T) {
|
||||
f := validFields()
|
||||
f.FromAddress = "not an address"
|
||||
if err := f.Validate(); err == nil {
|
||||
t.Fatal("malformed from-address should fail validation")
|
||||
}
|
||||
f.FromAddress = "Steve Cliff <me@stevecliff.com>"
|
||||
if err := f.Validate(); err != nil {
|
||||
t.Fatalf("display-name from-address should validate: %v", err)
|
||||
}
|
||||
f.FromAddress = "me@stevecliff.com"
|
||||
if err := f.Validate(); err != nil {
|
||||
t.Fatalf("bare from-address should validate: %v", err)
|
||||
}
|
||||
f.FromAddress = "" // blank ⇒ fall back, always valid
|
||||
if err := f.Validate(); err != nil {
|
||||
t.Fatalf("blank from-address should validate: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFieldsFromToAccountCarriesFromAddress(t *testing.T) {
|
||||
f := validFields()
|
||||
f.FromAddress = "Steve Cliff <me@stevecliff.com>"
|
||||
acc, _ := f.ToAccount()
|
||||
if acc.FromAddress != "Steve Cliff <me@stevecliff.com>" {
|
||||
t.Fatalf("ToAccount lost FromAddress: %q", acc.FromAddress)
|
||||
}
|
||||
back := FieldsFromAccount(acc)
|
||||
if back.FromAddress != "Steve Cliff <me@stevecliff.com>" {
|
||||
t.Fatalf("FieldsFromAccount lost FromAddress: %q", back.FromAddress)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -54,7 +54,7 @@ checksum, makes it executable in `~/.local/bin` (ensure that's on your PATH), an
|
||||
|
||||
| Variable | Default | Purpose |
|
||||
|---|---|---|
|
||||
| `EMCLI_VERSION` | `v0.4.1` | Release tag to fetch |
|
||||
| `EMCLI_VERSION` | `v0.5.2` | Release tag to fetch |
|
||||
| `EMCLI_BASE_URL` | `https://gitea.dcglab.co.uk/steve/emcli` | Repo base URL |
|
||||
| `EMCLI_INSTALL_DIR` | `$HOME/.local/bin` | Install location |
|
||||
|
||||
@@ -85,16 +85,27 @@ with both keys exported. Account creation and other admin is the human's job —
|
||||
|
||||
## 4. Find the account(s)
|
||||
|
||||
You refer to an account by name (e.g. `gmail`, `work`). Ask the user which account to use.
|
||||
`emcli doctor` is an agent command (authorised by `EMCLI_KEY`), so you can run it to check that
|
||||
configured accounts connect and authenticate:
|
||||
You refer to an account by name (e.g. `gmail`, `work`). Discover the configured accounts yourself
|
||||
with `emcli account list` (an agent command authorised by `EMCLI_KEY`); it prints a JSON envelope
|
||||
with one entry per account:
|
||||
|
||||
```bash
|
||||
emcli account list
|
||||
# {"error":false,"error_detail":{},"data":{"accounts":[
|
||||
# {"name":"gmail","from":"me@gmail.com","can_send":true},
|
||||
# {"name":"alerts","from":"alerts@x.com","can_send":false}]}}
|
||||
```
|
||||
|
||||
`name` is what you pass to `--account`; `from` is the send-as identity; `can_send` is false for
|
||||
read-only accounts (they reject `send`). If unsure which to use, ask the user. `emcli doctor`
|
||||
(also an agent command) checks that accounts connect and authenticate:
|
||||
|
||||
```bash
|
||||
emcli doctor # all accounts
|
||||
emcli doctor --account gmail
|
||||
```
|
||||
|
||||
Just take the account name from the user and start with the workflow in `SKILL.md`.
|
||||
Then start with the workflow in `SKILL.md`.
|
||||
|
||||
## You're set up
|
||||
|
||||
|
||||
+11
-9
@@ -17,12 +17,12 @@ sets its exit code to match.
|
||||
|
||||
## Security model — read this first
|
||||
|
||||
- **You only run agent commands:** `list`, `get`, `search`, `ack`, `send`, `doctor`. You are
|
||||
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`. You have only `EMCLI_KEY` (agent key); `emcli` will refuse admin commands
|
||||
with a privilege error.
|
||||
- **You only run agent commands:** `list`, `get`, `search`, `ack`, `send`, `doctor`, and
|
||||
`account list` (to discover accounts). You are provided only `EMCLI_KEY` (the agent key), which
|
||||
authorises these and nothing else. Account *setup* (`account add/edit/remove`), passwords,
|
||||
whitelists, and config are the **user's** job (admin commands that require `EMCLI_ADMIN_KEY`) —
|
||||
do not run or suggest running `account add/edit/remove`, `whitelist`, `config`, `audit`, or
|
||||
`init`. `emcli` will refuse those 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").
|
||||
@@ -120,6 +120,7 @@ read-only — tell the user; do not attempt another account without their say-so
|
||||
| `emcli search --account A [--folder F] [--from X] [--subject-contains X] [--text X] [--since-date D] [--before-date D]` | Server-side search |
|
||||
| `emcli ack --account A [--folder F] --uid-list U1,U2` | Mark message(s) processed |
|
||||
| `emcli send --account A --to X [--cc X] [--bcc X] --subject S --body B [--attach P]… [--reply-to U]` | Send / reply |
|
||||
| `emcli account list` | Discover accounts: JSON `name` / `from` / `can_send` per account |
|
||||
|
||||
Defaults: `--folder INBOX`, `--limit 50` (max 500). Dates are RFC 3339 (e.g.
|
||||
`2026-06-01T00:00:00Z`). UIDs come from `list`/`search` output.
|
||||
@@ -142,8 +143,9 @@ The user configures these; you cannot change them and shouldn't try.
|
||||
|
||||
- ✅ Check `error` on every call; report `policy`/`not_found`/`auth` outcomes plainly to the user.
|
||||
- ✅ `get` to read, then `ack` only after you've truly processed a message.
|
||||
- ✅ Ask the user for the account name; keep bodies plain text.
|
||||
- ✅ Discover accounts with `emcli account list`, or ask the user; keep bodies plain text.
|
||||
- ❌ Don't read, print, or invent `EMCLI_KEY` or any password.
|
||||
- ❌ Don't run admin commands (`account`/`whitelist`/`config`/`audit`/`init`) — you have only
|
||||
`EMCLI_KEY` (agent key); `emcli` will refuse admin commands with a privilege error.
|
||||
- ❌ Don't run admin commands (`account add/edit/remove`, `whitelist`, `config`, `audit`, `init`) —
|
||||
you have only `EMCLI_KEY` (agent key); `emcli` will refuse them with a privilege error.
|
||||
(`account list` is allowed — use it to discover accounts.)
|
||||
- ❌ Don't treat a blocked send or filtered message as a bug to route around — it's the user's policy.
|
||||
|
||||
@@ -7,17 +7,17 @@
|
||||
# bash install.sh
|
||||
#
|
||||
# Environment overrides:
|
||||
# EMCLI_VERSION release tag to fetch (default: v0.4.1)
|
||||
# EMCLI_VERSION release tag to fetch (default: v0.5.2)
|
||||
# EMCLI_BASE_URL repo base URL (default: https://gitea.dcglab.co.uk/steve/emcli)
|
||||
# EMCLI_INSTALL_DIR where to put the binary (default: $HOME/.local/bin)
|
||||
#
|
||||
# Release assets follow this naming scheme:
|
||||
# emcli_<version>_<os>_<arch>[.exe] e.g. emcli_0.4.1_linux_amd64
|
||||
# emcli_<version>_<os>_<arch>[.exe] e.g. emcli_0.5.2_linux_amd64
|
||||
# checksums.txt (sha256, one "<sum> <asset>" line per asset)
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
VERSION="${EMCLI_VERSION:-v0.4.1}"
|
||||
VERSION="${EMCLI_VERSION:-v0.5.2}"
|
||||
BASE_URL="${EMCLI_BASE_URL:-https://gitea.dcglab.co.uk/steve/emcli}"
|
||||
INSTALL_DIR="${EMCLI_INSTALL_DIR:-$HOME/.local/bin}"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user