From 8b7d9968aa3dacb4a1a38d4160b8ea0f107b30dd Mon Sep 17 00:00:00 2001 From: Zachary Lowden Date: Sat, 31 Oct 2026 23:08:23 -0500 Subject: [PATCH 1/3] feat(attention): GET /api/attention/summary, read-only API tokens, `muster attention summary` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit What a status bar polls, and a credential it can hold safely. GET /api/attention/summary (personal token only, requireAPIToken): every item waiting on the token's owner — exactly s.attentionItems, no second "waiting" rule — each with its host, title (textplain.Title of its source) and why (the first line of attentionPushBody: secrets redacted, a waiting session summary only, a reply draft who-and-where only), age, and its link (the tap route /attention/open/ for an ask or a session, the item's own page otherwise; absolute on MUSTER_PUBLIC_URL). Per host: online, status_report, busy and idle Claude windows (an offline host counts nothing). Global: nudge_mode (off | shadow | live for this owner), ts, and partial when a queue source could not be read. No per-item lookups, no stream. Contract in docs/attention.md#the-summary-api. A reply draft's item now carries its draftPushBody, and attentionPushBody returns it for a plugin item, so "the push body" is defined for every kind and the summary never quotes an inbound message. Read-only personal tokens (migration 0068, api_tokens.read_only default false): minted with a checkbox on /account/tokens (unchecked = full, as before); the list, /api/tokens and whoami show the scope. The check is at the one token door (admitAPIToken), by an allowlist of METHODS (GET, HEAD): any other method on any route a personal token reaches is 403 before its handler runs. Existing tokens stay full access. Rollback note in the migration. CLI: `muster attention summary` (verb 24; ledgers moved), JSON passthrough, existing exit codes (401/403 -> 3). A poller keeps its own token in its own file via the existing --env-file. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/muster/attention.go | 49 ++++ cmd/muster/attention_test.go | 71 +++++ cmd/muster/output_test.go | 1 + cmd/muster/root.go | 2 +- cmd/muster/token_seam_test.go | 4 +- cmd/muster/tree_test.go | 5 +- docs/attention.md | 50 ++++ docs/auth.md | 10 + docs/cli.md | 1 + docs/schema.md | 1 + internal/api/api_tokens.go | 36 ++- internal/api/api_tokens_readonly_test.go | 179 +++++++++++++ internal/api/api_tokens_test.go | 14 +- internal/api/attention.go | 18 +- internal/api/attention_summary.go | 175 +++++++++++++ internal/api/attention_summary_pg_test.go | 247 ++++++++++++++++++ internal/api/attention_summary_test.go | 240 +++++++++++++++++ internal/api/flows_test.go | 8 +- internal/api/isolation_test.go | 9 +- internal/api/nostore_ledger_test.go | 1 + internal/api/principal_test.go | 2 + internal/api/session_routes_pg_test.go | 4 +- internal/api/testdata/routes.golden | 1 + internal/apitokens/apitokens.go | 27 +- internal/apitokens/apitokens_test.go | 81 ++++-- .../migrations/0068_api_token_read_only.sql | 24 ++ tests/verb-ledger.sh | 3 +- 27 files changed, 1207 insertions(+), 56 deletions(-) create mode 100644 cmd/muster/attention.go create mode 100644 cmd/muster/attention_test.go create mode 100644 internal/api/api_tokens_readonly_test.go create mode 100644 internal/api/attention_summary.go create mode 100644 internal/api/attention_summary_pg_test.go create mode 100644 internal/api/attention_summary_test.go create mode 100644 internal/db/migrations/0068_api_token_read_only.sql diff --git a/cmd/muster/attention.go b/cmd/muster/attention.go new file mode 100644 index 000000000..200953b32 --- /dev/null +++ b/cmd/muster/attention.go @@ -0,0 +1,49 @@ +package main + +import ( + "net/http" + + "github.com/spf13/cobra" +) + +// The attention verb: `attention summary` over GET /api/attention/summary — +// what is waiting on you, your mirror hosts' busy/idle counts and your chief +// nudge mode, as one JSON document (docs/attention.md#the-summary-api). +// +// 🔴 IT WORKS ONLY WITH A PERSONAL TOKEN IN envToken, like `flow …`: the route +// (api.requireAPIToken) refuses the shared hook secret, because the summary is +// one person's queue and the hook secret names nobody. A read-only token is +// enough. A poller with its own token keeps it in its own file and passes +// --env-file, so that token never has to live in the default env file. + +const attentionSummaryRoute = "/api/attention/summary" + +func newAttentionCmd(a *app) *cobra.Command { + cmd := &cobra.Command{ + Use: "attention", + Short: "Read what is waiting on you (needs a personal API token; read-only is enough)", + } + cmd.AddCommand(newAttentionSummaryCmd(a)) + return cmd +} + +func newAttentionSummaryCmd(a *app) *cobra.Command { + return &cobra.Command{ + Use: "summary", + Short: "GET /api/attention/summary — what waits on you, your hosts, your nudge mode", + Long: `Print what is waiting on you — the same items as the Asks page — with each +item's title, why (the text its notification shows), age and link; each of your +mirror hosts with whether it is online and its busy and idle Claude windows; +and your chief nudge mode (off, shadow or live). The output is the server's +JSON document. "partial": true means part of the queue could not be read, so +the list may be short.`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + body, err := a.call(cmd.Context(), request{method: http.MethodGet, path: attentionSummaryRoute, cred: credHook}) + if err != nil { + return err + } + return a.emit(body) + }, + } +} diff --git a/cmd/muster/attention_test.go b/cmd/muster/attention_test.go new file mode 100644 index 000000000..72fa2ef13 --- /dev/null +++ b/cmd/muster/attention_test.go @@ -0,0 +1,71 @@ +package main + +import ( + "encoding/json" + "net/http" + "os" + "path/filepath" + "strings" + "testing" +) + +// summaryBody is a summary as the server writes it (compact, one line). +const summaryBody = `{"ts":1800000000,"nudge_mode":"live","partial":false,` + + `"hosts":[{"host":"host-a","online":true,"status_report":true,"busy":2,"idle":1}],` + + `"items":[{"key":"ask-5","kind":"ask","host":"","title":"chief","why":"Ship it?","since":1799999940,"age_secs":60,"offline":false,"path":"/attention/open/ask-5","url":""}]}` + +// TestAttentionSummaryReadsItsTokenFromItsOwnEnvFile is the status bar's call: +// no credential flags, the URL and a personal token in a DEDICATED env file +// (--env-file), so the bar's token never has to live in the default one. The +// request is GET /api/attention/summary bearing that token; stdout is the +// server's document (indented), and the token is printed nowhere. +func TestAttentionSummaryReadsItsTokenFromItsOwnEnvFile(t *testing.T) { + h := newHarness(t) + h.json("GET /api/attention/summary", http.StatusOK, summaryBody) + const barToken = "mst_bar-token-from-its-own-file-never-printed" + envFile := filepath.Join(t.TempDir(), "bar.env") + if err := os.WriteFile(envFile, []byte("MUSTER_API_URL="+h.srv.URL+"\nMUSTER_TOKEN="+barToken+"\n"), 0o600); err != nil { + t.Fatal(err) + } + got := h.runCLINoCredFlags(nil, envFile, "attention", "summary") + if got.code != exitOK { + t.Fatalf("exit %d, stderr %q", got.code, got.stderr) + } + reqs := h.nonHealthRequests() + if len(reqs) != 1 || reqs[0].method != http.MethodGet || reqs[0].uri != "/api/attention/summary" || reqs[0].auth != "Bearer "+barToken { + t.Fatalf("requests %+v; want one GET /api/attention/summary bearing the env file's token", reqs) + } + if mustCompact(t, got.stdout) != mustCompact(t, summaryBody) { + t.Fatalf("stdout is not the server's document:\n%s", got.stdout) + } + if !strings.Contains(got.stdout, "\n \"hosts\": [") { + t.Errorf("stdout is not indented: %q", got.stdout) + } + if strings.Contains(got.stdout+got.stderr, barToken) { + t.Fatal("the token was printed") + } +} + +func mustCompact(t *testing.T, s string) string { + t.Helper() + var v any + if err := json.Unmarshal([]byte(s), &v); err != nil { + t.Fatal(err) + } + b, _ := json.Marshal(v) + return string(b) +} + +// TestAttentionSummaryAuthFailureExitsThree: a 401 (a hook secret, an unknown +// or revoked token) and a 403 are exit 3, the auth code a poller branches on +// to show "?" rather than "nothing waits"; nothing is written to stdout. +func TestAttentionSummaryAuthFailureExitsThree(t *testing.T) { + for _, code := range []int{http.StatusUnauthorized, http.StatusForbidden} { + h := newHarness(t) + h.json("GET /api/attention/summary", code, `{"error":"invalid or revoked API token"}`) + got := h.runCLI("attention", "summary") + if got.code != exitAuth || got.stdout != "" { + t.Errorf("HTTP %d: exit %d stdout %q; want exit %d and no stdout", code, got.code, got.stdout, exitAuth) + } + } +} diff --git a/cmd/muster/output_test.go b/cmd/muster/output_test.go index 55ae1306b..0ed853e41 100644 --- a/cmd/muster/output_test.go +++ b/cmd/muster/output_test.go @@ -181,6 +181,7 @@ func TestStdoutIsIndentedJSONForEveryVerb(t *testing.T) { h.json("POST /api/plugins/messages/6/retry", http.StatusAccepted, `{"id":6,"status":"queued","nested":{"a":1}}`) h.json("POST /api/chief/replace", http.StatusOK, `{"deleted":null,"created":true,"nested":{"a":1}}`) h.json("GET /api/doctor", http.StatusOK, `{"hook_token_armed":true,"push":true,"dictation":true,"mirror":{"enabled":true,"hosts":[{"host":"alpha","token":true,"online":true}]}}`) + h.json("GET /api/attention/summary", http.StatusOK, `{"ts":1,"nudge_mode":"off","partial":false,"hosts":[],"items":[],"nested":{"a":1}}`) got := h.runCLI(w.args...) if got.code != exitOK { diff --git a/cmd/muster/root.go b/cmd/muster/root.go index 9905fbd2d..c5f93cac2 100644 --- a/cmd/muster/root.go +++ b/cmd/muster/root.go @@ -163,6 +163,6 @@ func newRootCmd(a *app) *cobra.Command { root.PersistentFlags().StringVar(&a.flagEnvFile, "env-file", defaultEnvFile, "KEY=VALUE config file") root.PersistentFlags().DurationVar(&a.flagTimeout, "timeout", rootDefaultTimeout, "per-request timeout") - root.AddCommand(newTaskCmd(a), newAgentCmd(a), newChiefCmd(a), newTokenCmd(a), newWhoAmICmd(a), newFlowCmd(a), newRouteCmd(a), newPluginsCmd(a), newDoctorCmd(a)) + root.AddCommand(newTaskCmd(a), newAgentCmd(a), newChiefCmd(a), newTokenCmd(a), newWhoAmICmd(a), newFlowCmd(a), newRouteCmd(a), newPluginsCmd(a), newDoctorCmd(a), newAttentionCmd(a)) return root } diff --git a/cmd/muster/token_seam_test.go b/cmd/muster/token_seam_test.go index 0ac0b730b..8eff371c1 100644 --- a/cmd/muster/token_seam_test.go +++ b/cmd/muster/token_seam_test.go @@ -53,11 +53,11 @@ func TestSeamPersonalTokensThroughTheRealServer(t *testing.T) { // is no CLI or API mint — so this test mints its two through the store, // as that page does. asUser := authz.WithPrincipal(ctx, authz.Principal{UserID: uid, Role: authz.RoleMember, Via: authz.ViaSession}) - first, _, err := apitokens.NewPG(pool).Create(asUser, "browser-minted", 0) + first, _, err := apitokens.NewPG(pool).Create(asUser, "browser-minted", 0, false) if err != nil { t.Fatal(err) } - secondTok, secondRec, err := apitokens.NewPG(pool).Create(asUser, "laptop", 0) + secondTok, secondRec, err := apitokens.NewPG(pool).Create(asUser, "laptop", 0, false) if err != nil { t.Fatal(err) } diff --git a/cmd/muster/tree_test.go b/cmd/muster/tree_test.go index 01f618c60..ff29d17da 100644 --- a/cmd/muster/tree_test.go +++ b/cmd/muster/tree_test.go @@ -44,7 +44,7 @@ import ( // status/comment, agent ls/resolve/messages, agent task get/comment/status, and // chief ask. If a thirteenth is genuinely wanted, change this number, add its // ledger row, and say so in the commit — do not discover it in a diff. -const wantVerbCount = 23 +const wantVerbCount = 24 // minRunnableCommands is a hardcoded floor BELOW wantVerbCount. It exists so // that a tree wired to NOTHING cannot pass this file vacuously: a tree that lost @@ -101,6 +101,8 @@ var wiring = []struct { // serves plain http, so `server` would FAIL; it is skipped here, as are the // checks that need a sign-in. What this row pins is the route. {"doctor", []string{"doctor", "--skip", "server,manifest,sign-in,push,dictation"}, "GET /api/doctor"}, + // The 24th verb: the attention summary a status bar polls (attention.go). + {"attention summary", []string{"attention", "summary"}, "GET /api/attention/summary"}, } // runnableCommands walks the tree and returns the full path of every command @@ -210,6 +212,7 @@ func TestEveryCommandReachesItsRoute(t *testing.T) { h.json("POST /api/chief/replace", http.StatusOK, `{"deleted":null,"created":true,"agent":{"id":2,"name":"chief"}}`) h.json("GET /api/doctor", http.StatusOK, `{"auth_mode":"password","hook_token_armed":true,`+ `"push":true,"dictation":true,"mirror":{"enabled":true,"hosts":[{"host":"alpha","token":true,"online":true}]}}`) + h.json("GET /api/attention/summary", http.StatusOK, `{"ts":1,"nudge_mode":"off","partial":false,"hosts":[],"items":[]}`) got := h.runCLI(w.args...) if got.code != exitOK { diff --git a/docs/attention.md b/docs/attention.md index 5738ff13a..d94414779 100644 --- a/docs/attention.md +++ b/docs/attention.md @@ -600,3 +600,53 @@ message. Only your own messages count: a message routed by the chief, the chief's hand-back turn, a message over the machine door (`POST /api/agents/{name}/messages`) and anything an agent sends do not. A reply draft decided outside muster is not cleared by this (it is not an ask). + +## The summary API + +`GET /api/attention/summary` is what a status bar polls: everything waiting on +you, your mirror hosts' busy and idle Claude windows, and your +[chief nudge](chief-nudges.md) mode, in one JSON document. From the CLI it is +`muster attention summary` ([cli.md](cli.md)), which prints the same document. + +- **Credential:** a [personal API token](auth.md) only (the hook token and a + browser session are refused with 401). It reads as the token's owner, and + nothing of another user's is ever in it. A **read-only** token is enough, and + is the one to give a poller. +- **No stream, no ETag.** Poll it; the body is small. It is never cached + (`Cache-Control: no-store`). +- **What "waiting" means:** exactly the items of the Asks page above — the + same function builds both, so the bar and the page cannot disagree. Busy and + idle never make anything waiting. + +```json +{ + "ts": 1800000000, + "nudge_mode": "off", + "partial": false, + "hosts": [ + {"host": "host-a", "online": true, "status_report": true, "busy": 1, "idle": 0} + ], + "items": [ + {"key": "ask-5", "kind": "ask", "host": "", "title": "chief", "why": "Ship it?", + "since": 1799999940, "age_secs": 60, "offline": false, + "path": "/attention/open/ask-5", "url": "https://muster.example.test/attention/open/ask-5"} + ] +} +``` + +| field | meaning | +|---|---| +| `ts` | The server's clock when the summary was built, Unix seconds. | +| `nudge_mode` | `off` (chief nudges do not run on this server), `shadow` or `live` for you. | +| `partial` | `true` when part of the queue could not be read, so `items` may be short. Do not read a partial summary as "nothing waits". | +| `hosts[]` | Each of your mirror hosts, by the name it reports (`MUSTER_MIRROR_HOST`), in the hub's order. `online`; `status_report` (it reports its windows' Claude status, so its zeros are measurements); `busy` and `idle`, its Claude (and other agent runtime) windows by status. An offline host counts `0` and `0`: a count is a claim about now. | +| `items[]` | Everything waiting on you, newest first. | +| `items[].key` | The item's id (`ask-`, `session-`, `draft-`, `task-`). A held question has its window's `session-…` key. | +| `items[].kind` | `ask`, `session`, `question`, `plugin` (a reply draft) or `task`. | +| `items[].host` | The mirror host it is on; `""` for an agent's ask, muster's own, a reply draft and a task. | +| `items[].title`, `items[].why` | What its notification says: the source as a title, and the first line of the notification body. Secrets are redacted, a waiting session is `Waiting: · @ `, a reply draft is who and where only. Never Claude's last words, an ask's options or its suggested replies. | +| `items[].since`, `items[].age_secs` | When it started waiting (Unix seconds) and `ts` minus that; `0` and `0` when unknown. | +| `items[].offline` | A session item whose host dropped within the last 5 minutes, shown as last seen. | +| `items[].path`, `items[].url` | Where to act on it: the [tap route](push.md) `/attention/open/` for an ask or a session (resolved when opened), the item's own page for a draft or a task. `url` is `path` on `MUSTER_PUBLIC_URL`, `""` when that is not set. | + +Fields may be added; none is renamed or removed. diff --git a/docs/auth.md b/docs/auth.md index 14fbb7d9e..f69fb927f 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -287,6 +287,16 @@ Two endpoints help with personal tokens: - `GET /api/tokens` and `DELETE /api/tokens/{id}` manage personal tokens. They accept a personal token only. +**Read-only tokens.** Tick **Read-only** when you mint a token and it may only +read: a `GET` or `HEAD` request is admitted as usual, and any other method +(`POST`, `PUT`, `PATCH`, `DELETE`) on any route a personal token reaches is +refused **403** before it runs. The check is by method, so it covers routes +added later. The token page and `muster token ls` (`"readOnly": true`) show +each token's scope, and `muster whoami` shows the presented token's. A token +minted before read-only tokens existed is full access, as is one minted with +the box left unticked. Give a poller (a status bar reading +[the attention summary](attention.md#the-summary-api)) a read-only token. + From the CLI these are `muster whoami`, `muster token ls` and `muster token revoke ` ([cli.md](cli.md)). diff --git a/docs/cli.md b/docs/cli.md index adce34598..df31ac871 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -18,6 +18,7 @@ The verb set is pinned in two places: | agents | `agent ls`, `agent messages`, `agent resolve`, `chief ask`, `chief replace` ([replacing the chief](chief.md#replacing-the-chief-on-another-kind)) | | browser flows | `flow enqueue`, `flow ls`, `flow status`, `flow cancel` | | credentials | `token ls`, `token revoke`, `whoami` | +| attention | `attention summary` (what is waiting on you, for a status bar: [the summary API](attention.md#the-summary-api)) | | session routing | `route reply` (a mirrored session's answer to a [routed message](plugins.md#routing-a-message-to-a-session)) | | plugins | `plugins retry ` ([retry an unconfirmed message](plugins.md#retrying-an-unconfirmed-message)) | | setup | `doctor` ([below](#muster-doctor)) | diff --git a/docs/schema.md b/docs/schema.md index f2e6cdc55..331bb0b51 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -94,6 +94,7 @@ buys only a word. | `0064` | `chief_nudges`: one row per [stall-nudge](chief-nudges.md) decision — the session (`session_kind` `mirror`/`agent`, `session_key`, its window, or its agent and `chat_session_id` — the one conversation of the agent that stalled), the idle streak it was taken on (`streak_key`, unique per owner and session: the sweep's claim), the decision (`nudge`, `ask`, `leave`, `invalid`), the nudge's message and the quoted owner message (`quote_id`, `quote_at`, `quote_text`) and the action it covers, the ask's question and options, the chief's reason, the `mode` (`shadow`/`live`; a shadow row can hold no ask and no queued message, by CHECK), the `outcome` (a live state, or `suppressed:`), the live ask and queued message it raised, and the owner's relayed answer. `send_queue.chief_nudge_id` marks a queued nudge or relay, gated by its decision rather than a sign-in (never together with `route_id`). Owned, under row-level security, and re-applied by `db.RLSOn`. Rollback: in the file's header. | | `0065` | `agent_focus.cwd`: the working directory the session's transcript tail last named when its focus was written, cleaned and absolute, `''` for unknown (`agent_focus_cwd_check`: empty, or starting with `/` and at most 512 characters). Read only by the fleet report's mail door, to decide whether a session is under `MUSTER_FLEET_REPORT_PATHS` ([focus](focus.md#the-working-directory)); never rendered or sent. The column inherits `agent_focus`'s ownership and row-level security; no policy change, not re-applied by `db.RLSOn`. Must apply after `0064`. Rollback: in the file's header. | | `0067` | `connectors.auth_scheme` gains `postgres`; `connectors.settings` (JSONB, default `{}`): a [postgres connector](connectors.md#postgres-connectors)'s non-secret settings — `database`, `sslmode`, `require_tls`, `statement_timeout_ms`, `max_rows`, `max_bytes` — and `{}` for every other scheme (`connectors_settings_scheme`); `connector_audit.kind` gains `query` (one row per agent query: the SQL's fingerprint and first 120 characters, rows, duration, outcome class; never a parameter value or the password). The columns inherit their tables' ownership and row-level security; not re-applied by `db.RLSOn`. Rollback: in the file's header (delete postgres connectors first). | +| `0068` | `api_tokens.read_only` (default `false`): a [read-only personal token](auth.md) may only make `GET` and `HEAD` requests, enforced at the token door. Every token minted before it is full access. Inherits the table's row-level security; not re-applied by `db.RLSOn`. 🔴 Rollback: an older image would admit a read-only token with full access, so revoke every live read-only token first (in the file's header). | To change the schema, add a new numbered file. diff --git a/internal/api/api_tokens.go b/internal/api/api_tokens.go index 183d5d9e5..e17974c64 100644 --- a/internal/api/api_tokens.go +++ b/internal/api/api_tokens.go @@ -42,6 +42,12 @@ import ( // disabling its owner takes effect on the very next request — tighter than the // gotrue session tier's DisableSweepInterval, which bounds how long an already // OPEN stream survives a disable. No route a token can open streams. +// +// 🔴 A READ-ONLY TOKEN (migration 0068) IS REFUSED AT THE SAME DOOR, BY METHOD. +// admitAPIToken lets it through only on readOnlyTokenMethods (GET, HEAD) and +// answers anything else 403 ReadOnlyTokenRefusal before the handler runs. It is +// an allowlist of METHODS, not of routes: a route added later is covered the +// day it is registered, and a write route is a write whatever its path. // apiTokensPath is the token-management API (list, revoke; requireAPIToken). const apiTokensPath = "/api/tokens" @@ -132,8 +138,18 @@ func (s *Server) requireAPIToken(next http.HandlerFunc) http.HandlerFunc { // nothing about which. const APITokenRefusal = "invalid or revoked API token" +// ReadOnlyTokenRefusal is the 403 body a read-only token gets on any method but +// readOnlyTokenMethods. +const ReadOnlyTokenRefusal = "this API token is read-only: it may only make GET and HEAD requests (mint a full-access token at " + accountTokensPath + " to change anything)" + +// readOnlyTokenMethods is every method a read-only token may use. An +// allowlist: a method not named here (POST, PUT, PATCH, DELETE, and any other) +// is refused. +var readOnlyTokenMethods = map[string]bool{http.MethodGet: true, http.MethodHead: true} + // admitAPIToken authenticates a personal token and admits the request as its -// owner, or refuses it 401. See the file header. +// owner, or refuses it 401 — or 403 for a read-only token on a method it may +// not use. See the file header. // // ⚠ NO FAILURE LIMITER, ON PURPOSE: a 256-bit token cannot be guessed, so a // limiter would only turn 401 into 429. The one gate before the lookup is @@ -160,6 +176,10 @@ func (s *Server) admitAPIToken(w http.ResponseWriter, r *http.Request, next http s.writeJSON(w, http.StatusServiceUnavailable, map[string]any{"error": "could not check the API token"}) return } + if t.ReadOnly && !readOnlyTokenMethods[r.Method] { + s.writeJSON(w, http.StatusForbidden, map[string]any{"error": ReadOnlyTokenRefusal}) + return + } ctx := context.WithValue(r.Context(), apiTokenCtxKey{}, t) s.admit(w, r.WithContext(ctx), next, s.principalFor(r.Context(), t.UserID, authz.ViaToken)) } @@ -181,7 +201,7 @@ func (s *Server) handleWhoAmI(w http.ResponseWriter, r *http.Request) { p := authz.MustFrom(r.Context()) out := map[string]any{"userId": p.UserID, "role": p.Role, "via": p.Via} if t, ok := apiTokenFromContext(r.Context()); ok { - out["token"] = map[string]any{"id": t.ID, "name": t.Name, "createdAt": t.CreatedAt} + out["token"] = map[string]any{"id": t.ID, "name": t.Name, "createdAt": t.CreatedAt, "readOnly": t.ReadOnly} } if actor, ok := serviceActorFromContext(r.Context()); ok { out["actor"] = actor @@ -236,7 +256,9 @@ func (s *Server) handleAccountTokenCreate(w http.ResponseWriter, r *http.Request s.renderAccountTokens(w, r, http.StatusInternalServerError, "", "The token could not be created.") return } - tok, _, err := s.ext.APITokens.Create(r.Context(), r.PostFormValue("name"), iat) + // The checkbox: absent (unchecked) is a full-access token, as before 0068. + readOnly := r.PostFormValue("read_only") != "" + tok, _, err := s.ext.APITokens.Create(r.Context(), r.PostFormValue("name"), iat, readOnly) if errors.Is(err, apitokens.ErrNameTooLong) { s.renderAccountTokens(w, r, http.StatusBadRequest, "", err.Error()) return @@ -307,6 +329,8 @@ func (s *Server) renderAccountTokens(w http.ResponseWriter, r *http.Request, cod +
    @@ -319,13 +343,17 @@ func (s *Server) renderAccountTokens(w http.ResponseWriter, r *http.Request, cod if name == "" { name = "(unnamed)" } + scope := "full access" + if t.ReadOnly { + scope = "read-only" + } used := "never used" if t.LastUsedAt != nil { used = "last used " + t.LastUsedAt.UTC().Format("2006-01-02 15:04") + " UTC" } b.WriteString(`
  • ` + html.EscapeString(name) + ` -created ` + t.CreatedAt.UTC().Format("2006-01-02 15:04") + ` UTC · ` + used + ` +` + scope + ` · created ` + t.CreatedAt.UTC().Format("2006-01-02 15:04") + ` UTC · ` + used + ` `) if t.Revoked() { b.WriteString(`revoked`) diff --git a/internal/api/api_tokens_readonly_test.go b/internal/api/api_tokens_readonly_test.go new file mode 100644 index 000000000..69d6705c9 --- /dev/null +++ b/internal/api/api_tokens_readonly_test.go @@ -0,0 +1,179 @@ +package api + +import ( + "context" + "encoding/json" + "io" + "log" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "testing" + + "github.com/ZacxDev/muster/internal/apitokens" +) + +// scopedTokens is fakeAPITokens with a read-only token beside the full one, +// and a List and Revoke that COUNT, so a test can tell "refused at the door" +// from "the handler ran and refused". +type scopedTokens struct { + *fakeAPITokens + mu sync.Mutex + revokes int + lists int +} + +func (s *scopedTokens) addReadOnly(t *testing.T, uid int64) string { + t.Helper() + tok := s.add(t, uid) + s.fakeAPITokens.mu.Lock() + defer s.fakeAPITokens.mu.Unlock() + rec := s.byTok[tok] + rec.ReadOnly = true + s.byTok[tok] = rec + return tok +} + +func (s *scopedTokens) List(context.Context) ([]apitokens.Token, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.lists++ + return []apitokens.Token{}, nil +} + +func (s *scopedTokens) Revoke(context.Context, int64) (bool, bool, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.revokes++ + return true, true, nil +} + +func (s *scopedTokens) counts() (int, int) { + s.mu.Lock() + defer s.mu.Unlock() + return s.lists, s.revokes +} + +// TestAReadOnlyTokenMayOnlyRead: a read-only token reaches GET routes (whoami, +// the token list, the attention summary) and is refused 403, with the +// read-only sentence, on every write — a token revoke (DELETE), a flow enqueue +// and cancel and a task create (POST), a task status (PATCH) — BEFORE the +// handler runs (the store's Revoke is never called). The same requests with +// the same owner's FULL token are not refused (the control: the 403 is the +// scope, not the route), and the full token's revoke reaches the store. +func TestAReadOnlyTokenMayOnlyRead(t *testing.T) { + f := &scopedTokens{fakeAPITokens: newFakeAPITokens()} + full := f.add(t, 7) + ro := f.addReadOnly(t, 7) + s := New(nil, AuthConfig{HookToken: tokTestHook}, log.New(io.Discard, "", 0)) + s.UseExtensions(Extensions{APITokens: f, Users: stubUsers{}}) + mux := http.NewServeMux() + s.registerAll(mux) + do := func(method, path, tok string) *httptest.ResponseRecorder { + r := httptest.NewRequest(method, path, strings.NewReader(`{}`)) + r.Header.Set("Authorization", "Bearer "+tok) + r.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, r) + return rec + } + for _, path := range []string{"/api/whoami", apiTokensPath, attentionSummaryPath} { + if rec := do(http.MethodGet, path, ro); rec.Code != http.StatusOK { + t.Errorf("GET %s with a read-only token: %d %s; want 200", path, rec.Code, truncate(rec.Body.String(), 200)) + } + if rec := do(http.MethodHead, path, ro); rec.Code == http.StatusForbidden { + t.Errorf("HEAD %s with a read-only token: 403; HEAD is a read", path) + } + } + _, body := whoami(t, mux, ro, "") + if tk, _ := body["token"].(map[string]any); tk == nil || tk["readOnly"] != true { + t.Errorf("whoami does not say the token is read-only: %v", body) + } + writes := []struct{ method, path string }{ + {http.MethodDelete, apiTokensPath + "/1"}, + {http.MethodPost, "/api/flows/jobs"}, + {http.MethodPost, "/api/flows/jobs/1/cancel"}, + {http.MethodPost, "/api/tasks"}, + {http.MethodPatch, "/api/tasks/1/status"}, + } + for _, w := range writes { + rec := do(w.method, w.path, ro) + var out map[string]any + _ = json.Unmarshal(rec.Body.Bytes(), &out) + if rec.Code != http.StatusForbidden || out["error"] != ReadOnlyTokenRefusal { + t.Errorf("%s %s with a read-only token: %d %s; want 403 %q", w.method, w.path, rec.Code, truncate(rec.Body.String(), 200), ReadOnlyTokenRefusal) + } + } + if _, revokes := f.counts(); revokes != 0 { + t.Fatalf("a read-only token's DELETE reached the store's Revoke %d time(s); the door must refuse it first", revokes) + } + for _, w := range writes { + if rec := do(w.method, w.path, full); rec.Code == http.StatusForbidden { + t.Errorf("CONTROL: %s %s with the FULL token: 403 %s; a full token is unchanged", w.method, w.path, truncate(rec.Body.String(), 200)) + } + } + if _, revokes := f.counts(); revokes != 1 { + t.Fatalf("CONTROL: the full token's DELETE reached Revoke %d time(s); want 1", revokes) + } +} + +// mintScopeTokens records the scope each mint asked for, and lists one token +// of each scope. +type mintScopeTokens struct { + apitokens.Store + got *[]bool +} + +func (m mintScopeTokens) Create(_ context.Context, _ string, _ int64, readOnly bool) (string, apitokens.Token, error) { + *m.got = append(*m.got, readOnly) + tok, _ := apitokens.New() + return tok, apitokens.Token{ID: 1, ReadOnly: readOnly}, nil +} + +func (mintScopeTokens) List(context.Context) ([]apitokens.Token, error) { + return []apitokens.Token{{ID: 2, Name: "bar", ReadOnly: true}, {ID: 3, Name: "cli"}}, nil +} + +// TestTheMintPageOffersAReadOnlyToken: the page's form has a read-only +// checkbox, UNCHECKED by default (a mint that leaves it alone is full access, +// as before); a mint with it ticked asks the store for a read-only token, one +// without for a full one; and the list says each token's scope. +func TestTheMintPageOffersAReadOnlyToken(t *testing.T) { + var got []bool + s := New(nil, AuthConfig{HookToken: tokTestHook, UIPassword: "mint-scope-operator-pw"}, log.New(io.Discard, "", 0)) + s.UseExtensions(Extensions{APITokens: mintScopeTokens{got: &got}, Users: stubUsers{}}) + mux := http.NewServeMux() + s.registerAll(mux) + do := func(method, body string) string { + r := httptest.NewRequest(method, accountTokensPath, strings.NewReader(body)) + r.Host = "muster.test" + r.Header.Set("Cookie", sessionCookieFor(s)) + r.Header.Set("Origin", "http://muster.test") + if body != "" { + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + } + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, r) + if rec.Code != http.StatusOK { + t.Fatalf("%s %s: %d", method, accountTokensPath, rec.Code) + } + return rec.Body.String() + } + page := do(http.MethodGet, "") + const box = `` + if !strings.Contains(page, box) { + t.Fatalf("the token page has no unchecked read-only checkbox %q:\n%s", box, truncate(page, 1200)) + } + for _, want := range []string{`data-token-scope="read-only">read-only · created`, `data-token-scope="full access">full access · created`} { + if !strings.Contains(page, want) { + t.Errorf("the token list does not say %q", want) + } + } + do(http.MethodPost, url.Values{"name": {"bar"}, "read_only": {"1"}}.Encode()) + do(http.MethodPost, url.Values{"name": {"cli"}}.Encode()) + if len(got) != 2 || !got[0] || got[1] { + t.Fatalf("the store was asked for scopes %v; want [read-only, full]", got) + } +} diff --git a/internal/api/api_tokens_test.go b/internal/api/api_tokens_test.go index e9b6ff7dd..265c28ab5 100644 --- a/internal/api/api_tokens_test.go +++ b/internal/api/api_tokens_test.go @@ -200,7 +200,7 @@ type mintCountingTokens struct { mints int } -func (m *mintCountingTokens) Create(context.Context, string, int64) (string, apitokens.Token, error) { +func (m *mintCountingTokens) Create(context.Context, string, int64, bool) (string, apitokens.Token, error) { m.mints++ tok, _ := apitokens.New() return tok, apitokens.Token{ID: 99, UserID: 7}, nil @@ -351,7 +351,7 @@ func TestPersonalTokenLifecycleOverHTTP(t *testing.T) { if !strings.Contains(list, aTask.Title) || strings.Contains(list, bTask.Title) { t.Fatalf("A's token's task list: %s", truncate(list, 400)) } - bTok, _, err := apitokens.NewPG(pool).Create(asUser(b), "b", 0) + bTok, _, err := apitokens.NewPG(pool).Create(asUser(b), "b", 0, false) if err != nil { t.Fatal(err) } @@ -491,7 +491,7 @@ func TestAPasswordResetRevokesTheUsersAPITokens(t *testing.T) { }) ts := apitokens.NewPG(w.pool) asUser := authz.WithPrincipal(context.Background(), authz.Principal{UserID: uid, Role: authz.RoleMember, Via: authz.ViaSession}) - secret, _, err := ts.Create(asUser, "before-reset", 0) + secret, _, err := ts.Create(asUser, "before-reset", 0, false) if err != nil { t.Fatal(err) } @@ -537,7 +537,7 @@ func TestSignOutEverywhereSaysItKeepsAPITokens(t *testing.T) { _, _ = w.pool.Exec(authz.System(context.Background()), `DELETE FROM api_tokens WHERE user_id = 1`) }) ts := apitokens.NewPG(w.pool) - secret, _, err := ts.Create(authz.Operator(context.Background()), "kept", 0) + secret, _, err := ts.Create(authz.Operator(context.Background()), "kept", 0, false) if err != nil { t.Fatal(err) } @@ -557,7 +557,7 @@ type refusingTokens struct { err error } -func (r refusingTokens) Create(context.Context, string, int64) (string, apitokens.Token, error) { +func (r refusingTokens) Create(context.Context, string, int64, bool) (string, apitokens.Token, error) { return "", apitokens.Token{}, r.err } func (refusingTokens) List(context.Context) ([]apitokens.Token, error) { return nil, nil } @@ -604,7 +604,7 @@ func TestAResetWhoseSignInFailsStillSaysTheTokensWereRevoked(t *testing.T) { }) ts := apitokens.NewPG(w.pool) secret, _, err := ts.Create(authz.WithPrincipal(context.Background(), - authz.Principal{UserID: uid, Role: authz.RoleMember, Via: authz.ViaSession}), "before", 0) + authz.Principal{UserID: uid, Role: authz.RoleMember, Via: authz.ViaSession}), "before", 0, false) if err != nil { t.Fatal(err) } @@ -626,7 +626,7 @@ type iatCapturingTokens struct { got *[]int64 } -func (c iatCapturingTokens) Create(_ context.Context, _ string, iat int64) (string, apitokens.Token, error) { +func (c iatCapturingTokens) Create(_ context.Context, _ string, iat int64, _ bool) (string, apitokens.Token, error) { *c.got = append(*c.got, iat) tok, _ := apitokens.New() return tok, apitokens.Token{ID: 1}, nil diff --git a/internal/api/attention.go b/internal/api/attention.go index d7512e756..8624a4583 100644 --- a/internal/api/attention.go +++ b/internal/api/attention.go @@ -202,7 +202,8 @@ type attentionItem struct { // chips. Not in the item's JSON (attention_list), which predates them. suggestions []string // pushText, when set, replaces Text in the notification body (muster's - // own unrouted-message ask: unroutedPushText); the page shows Text. + // own unrouted-message ask: unroutedPushText; a reply draft's + // draftPushBody); the page shows Text. pushText string // choices are a held question's choice labels, listed in its push only // (never in attention_list's options). @@ -506,7 +507,7 @@ func (s *Server) pendingDraftItems(ctx context.Context) ([]attentionItem, error) } it := attentionItem{Key: draftKey(c.Draft.ID), Kind: attentionKindPlugin, Source: c.PluginLabel, Text: draftInboundText(c.Message), Detail: "Draft reply: " + mirror.OneLine(c.Draft.Body, draftReplyMax), - Since: c.Draft.CreatedAt, Link: draftLink(c.Draft.ID)} + Since: c.Draft.CreatedAt, Link: draftLink(c.Draft.ID), pushText: draftPushBody(c.Message, c.PluginLabel)} // The agent that drafted it (none once that agent is deleted). if c.Draft.AgentID != nil { it.card.agentID = *c.Draft.AgentID @@ -704,10 +705,18 @@ func attentionPushTitle(it attentionItem) string { // command a question asks about is exactly what must not vanish from it. // // Other notifications are unchanged: a reply draft's (draftPushBody) still -// says who and where only. +// says who and where only — and so does its body here, so this is every +// item's lock-screen text (the attention summary reads it, +// attention_summary.go): a draft item carries its draftPushBody in pushText, +// and one without (built elsewhere) says DraftPushBody, never its message. func attentionPushBody(it attentionItem) string { var lines []string switch it.Kind { + case attentionKindPlugin: + if it.pushText != "" { + return it.pushText + } + return DraftPushBody case attentionKindSession: body := "Waiting: " + textplain.Title(it.Source) if tool := mirror.OneLine(it.tool, mirror.MaxPendingTool); tool != "" { @@ -962,6 +971,9 @@ func (s *Server) registerAttentionRoutes(mux Mux) { mux.HandleFunc("GET "+attentionOpenPath+"{key}", s.requireSession(s.handleAttentionOpen)) mux.HandleFunc("GET "+askPagePrefix+"{id}", s.requireSession(s.handleAskPage)) mux.HandleFunc("GET "+ui.AttentionBadgePath, s.requireSession(s.handleAttentionBadge)) + // What a status bar polls (attention_summary.go): a PERSONAL token only, so + // it reads as that token's owner; a read-only token is enough. + mux.HandleFunc("GET "+attentionSummaryPath, s.requireAPIToken(s.handleAttentionSummary)) mux.HandleFunc("POST "+attentionPath+"/asks/{id}/answer", s.requireOperatorWrite(s.handleAttentionAnswerForm)) mux.HandleFunc("POST "+attentionPath+"/asks/{id}/cancel", s.requireOperatorWrite(s.handleAttentionCancelForm)) mux.HandleFunc("POST "+attentionPath+"/asks/{id}/redeliver", s.requireOperatorWrite(s.handleAttentionRedeliverForm)) diff --git a/internal/api/attention_summary.go b/internal/api/attention_summary.go new file mode 100644 index 000000000..3391fb6f8 --- /dev/null +++ b/internal/api/attention_summary.go @@ -0,0 +1,175 @@ +package api + +import ( + "net/http" + "strings" + "time" + + "github.com/ZacxDev/muster/internal/mirror" + "github.com/ZacxDev/muster/internal/textplain" +) + +// --------------------------------------------------------------------------- +// THE ATTENTION SUMMARY: GET /api/attention/summary (docs/attention.md#the-summary-api). +// +// What a status bar polls: everything waiting on the token's owner, each +// host's online state and its Claude windows' busy/idle counts, and whether the +// owner's chief nudges are off, shadow or live. One JSON document, no stream +// (no route a token can open streams: api_tokens.go). +// +// 🔴 "WAITING ON ME" IS attentionItems, AND NOTHING ELSE. The summary adds no +// predicate of its own: its items are exactly what the Asks page, the badge +// and attention_list read, so the bar and the page cannot disagree about what +// waits. Busy and idle are context only; they never make anything "waiting". +// +// 🔴 EACH ITEM SAYS WHAT ITS PUSH SAYS, AND NO MORE. title is the item's +// source as the title rule makes it (textplain.Title, the sink's own rule for +// a notification title); why is the first line of attentionPushBody — secrets +// redacted, a waiting session as summary only, a reply draft as who and where +// only. Claude's last words (Detail), the asker's options and the suggested +// replies are never in it. That is the disclosure the operator already chose +// for the lock screen. +// +// ⚠ NO PER-ITEM LOOKUPS. An item's link is the tap route (attentionOpenURL), +// which resolves the target when it is opened, so building the summary costs +// what one badge render costs: no ByAsk, no agent read, no focus read. +// --------------------------------------------------------------------------- + +// attentionSummaryPath is the summary's route (requireAPIToken). +const attentionSummaryPath = "/api/attention/summary" + +// The values of attentionSummary.NudgeMode. +const ( + nudgeModeOff = "off" +) + +// attentionSummary is the summary document. Its field names are a contract +// with pollers (docs/attention.md): add fields, never rename one. +type attentionSummary struct { + // TS is the server's clock when the summary was built, Unix seconds. + TS int64 `json:"ts"` + // NudgeMode is "off" (chief nudges do not run here), "shadow" or "live" + // for the token's owner. + NudgeMode string `json:"nudge_mode"` + // Partial: a source of the queue could not be read, so items may be + // missing (attentionItems' error). A poller must not read a partial + // summary as a quiet one. + Partial bool `json:"partial"` + Hosts []attentionSummaryHost `json:"hosts"` + Items []attentionSummaryItem `json:"items"` +} + +// attentionSummaryHost is one of the owner's mirror hosts, by the name it +// reports (MUSTER_MIRROR_HOST). +type attentionSummaryHost struct { + Host string `json:"host"` + Online bool `json:"online"` + // StatusReport: the host reports its windows' Claude status, so a zero + // count (and no waiting session) from it is a measurement. + StatusReport bool `json:"status_report"` + Busy int `json:"busy"` + Idle int `json:"idle"` +} + +// attentionSummaryItem is one item waiting on the owner. +type attentionSummaryItem struct { + Key string `json:"key"` + Kind string `json:"kind"` + // Host is the mirror host the item is on, "" when none (an agent's ask, + // muster's own, a reply draft, a task). + Host string `json:"host"` + Title string `json:"title"` + Why string `json:"why"` + // Since is when it started waiting, Unix seconds (0 unknown); AgeSecs is + // TS minus Since (0 unknown). + Since int64 `json:"since"` + AgeSecs int64 `json:"age_secs"` + Offline bool `json:"offline"` + // Path is where the owner acts on it, on muster's origin; URL is it made + // absolute with MUSTER_PUBLIC_URL, "" when that is not set. + Path string `json:"path"` + URL string `json:"url"` +} + +func (s *Server) handleAttentionSummary(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + owner := requestOwner(ctx) + if owner <= 0 { + s.writeJSON(w, http.StatusForbidden, map[string]any{"error": "the attention summary is a user's own; this credential names none"}) + return + } + items, err := s.attentionItems(ctx) + if err != nil { + s.logger.Printf("attention: summary: %v", err) + } + var hosts []mirror.HostView + if s.ext.Mirror != nil { + hosts = s.ext.Mirror.Hosts(ctx) + } + w.Header().Set("Cache-Control", "no-store") + s.writeJSON(w, http.StatusOK, s.attentionSummaryOf(owner, items, err != nil, hosts, s.now())) +} + +// attentionSummaryOf builds owner's summary from its queue and its hosts. +func (s *Server) attentionSummaryOf(owner int64, items []attentionItem, partial bool, hosts []mirror.HostView, now time.Time) attentionSummary { + out := attentionSummary{TS: now.Unix(), NudgeMode: nudgeModeOff, Partial: partial, + Hosts: []attentionSummaryHost{}, Items: []attentionSummaryItem{}} + if s.chiefNudgesOn() { + out.NudgeMode = s.chiefNudgeMode(owner) + } + for _, h := range hosts { + out.Hosts = append(out.Hosts, attentionSummaryHostOf(h)) + } + base := strings.TrimRight(s.auth.PublicURL, "/") + for _, it := range items { + out.Items = append(out.Items, attentionSummaryItemOf(it, base, now)) + } + return out +} + +// attentionSummaryHostOf is h's line: its Claude windows (and other agent +// runtimes', Window.Spec) counted by status. +// +// ⚠ AN OFFLINE HOST COUNTS NOTHING: a count is a claim about NOW, and a host +// that is gone cannot vouch for it (the rule HostView.Work follows). +func attentionSummaryHostOf(h mirror.HostView) attentionSummaryHost { + out := attentionSummaryHost{Host: h.Host, Online: h.Online, StatusReport: h.StatusReport} + if !h.Online { + return out + } + for _, w := range h.Windows { + if _, agent := w.Spec(); !w.Claude && !agent { + continue + } + switch w.Status { + case mirror.StatusBusy: + out.Busy++ + case mirror.StatusIdle: + out.Idle++ + } + } + return out +} + +// attentionSummaryItemOf is it as the summary lists it. base is the public +// origin without a trailing slash, "" for none. +func attentionSummaryItemOf(it attentionItem, base string, now time.Time) attentionSummaryItem { + out := attentionSummaryItem{Key: it.Key, Kind: it.Kind, Host: it.host, Title: textplain.Title(it.Source), + Why: firstLine(attentionPushBody(it)), Offline: it.Offline, Path: it.Link} + if out.Host == "" { + out.Host = it.card.ref.Host // an ask from a mirrored session + } + if attentionOpenKey.MatchString(it.Key) { + out.Path = attentionOpenURL(it.Key) + } + if !it.Since.IsZero() { + out.Since = it.Since.Unix() + if age := now.Unix() - out.Since; age > 0 { + out.AgeSecs = age + } + } + if base != "" && out.Path != "" { + out.URL = base + out.Path + } + return out +} diff --git a/internal/api/attention_summary_pg_test.go b/internal/api/attention_summary_pg_test.go new file mode 100644 index 000000000..99abfd9bf --- /dev/null +++ b/internal/api/attention_summary_pg_test.go @@ -0,0 +1,247 @@ +package api + +import ( + "context" + "encoding/json" + "net/http" + "net/url" + "sort" + "strconv" + "strings" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/ZacxDev/muster/internal/apitokens" + "github.com/ZacxDev/muster/internal/attention" + "github.com/ZacxDev/muster/internal/authz" + "github.com/ZacxDev/muster/internal/mirror" + "github.com/ZacxDev/muster/internal/plugins" + "github.com/ZacxDev/muster/internal/users" +) + +// THE ATTENTION SUMMARY over real Postgres: GET /api/attention/summary with a +// personal token, against the queue's real sources (asks, the hub's waiting +// windows, reply drafts). + +// newSummaryFixture is newAttnFixture with the personal-token store and the +// users store wired over the same pool. +func newSummaryFixture(t *testing.T) *attnFixture { + t.Helper() + return newAttnFixture(t, func(ext *Extensions, pool *pgxpool.Pool) { + ext.APITokens, ext.Users = apitokens.NewPG(pool), users.NewPG(pool) + }) +} + +// summaryAs is GET /api/attention/summary with tok: the status and the +// decoded body. +func summaryAs(t *testing.T, f *attnFixture, tok string) (int, attentionSummary, string) { + t.Helper() + w := f.call(t, http.MethodGet, attentionSummaryPath, nil, map[string]string{"Authorization": "Bearer " + tok}) + var out attentionSummary + if w.Code == http.StatusOK { + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatalf("summary body: %v\n%s", err, w.Body) + } + } + return w.Code, out, w.Body.String() +} + +func summaryKeys(s attentionSummary) []string { + out := []string{} + for _, it := range s.Items { + out = append(out, it.Key) + } + sort.Strings(out) + return out +} + +func itemKeys(items []attentionItem) []string { + out := []string{} + for _, it := range items { + out = append(out, it.Key) + } + sort.Strings(out) + return out +} + +// TestTheSummaryIsTheOwnersQueueAndHosts: with a mirrored session's ask (a +// secret in its question), an agent's ask, a window waiting at a prompt and a +// pending reply draft, the operator's summary lists EXACTLY attentionItems' +// keys (the seam: no second "waiting" rule), each with its host, its push text +// and its tap link; each host with online and its busy/idle counts (an online +// host's, an offline one's zero). Another owner's token sees its own ask and +// none of the operator's items or hosts. +func TestTheSummaryIsTheOwnersQueueAndHosts(t *testing.T) { + f := newSummaryFixture(t) + op := asUser(1) + opTok, _, err := apitokens.NewPG(f.pool).Create(op, "bar", 0, false) + if err != nil { + t.Fatal(err) + } + memberTok, _, err := apitokens.NewPG(f.pool).Create(asUser(f.member), "member-bar", 0, false) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _, _ = f.pool.Exec(authz.System(context.Background()), `DELETE FROM api_tokens WHERE user_id = ANY($1)`, []int64{1, f.member}) + }) + mirrored, err := f.attn.Create(op, attention.New{Asker: attention.Asker{MirrorHost: "ws", MirrorSession: focusSID, Label: "Gold:1"}, + Question: "Use ghp_0123abcd9 for the mirror?", Options: []string{"yes", "no"}}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _, _ = f.pool.Exec(authz.System(context.Background()), `DELETE FROM attention_asks WHERE id = $1`, mirrored.ID) + }) + agentAsk := f.ask(t, f.opTok, "Which bucket?", "a", "b") + memberAsk := f.ask(t, f.workerTok, "The member's own question?") + // A pending reply draft of the operator's, as TestTheReadModelHasEachSource files one. + ps, err := f.plugins.List(op) + if err != nil { + t.Fatal(err) + } + var p plugins.Plugin + if len(ps) > 0 { + p = ps[0] + } else if _, p, err = f.plugins.Create(op, "attn-inbox", "Attention inbox", "attn-inbox"); err != nil { + t.Fatal(err) + } + m, _, err := f.plugins.Submit(op, p.ID, plugins.MessageIn{ExternalID: "sum-" + strconv.FormatInt(time.Now().UnixNano(), 36), + Sender: "Dana ", SenderVerified: true, Body: "the secret plan"}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _, _ = f.pool.Exec(authz.System(context.Background()), `DELETE FROM plugin_messages WHERE id = $1`, m.ID) + }) + if ok, err := f.plugins.BeginDelivery(op, m.ID, f.op.ID); err != nil || !ok { + t.Fatalf("begin delivery: %v %v", ok, err) + } + if ok, err := f.plugins.RecordDelivery(op, m.ID, f.op.ID, plugins.StatusDelivered); err != nil || !ok { + t.Fatalf("record delivery: %v %v", ok, err) + } + d, _, err := f.plugins.FileDraft(op, f.op.ID, 1, f.op.Name, m.ID, "sure, sending the plan") + if err != nil { + t.Fatal(err) + } + // A second host online with two busy Claude windows and a busy shell, and + // a third that dropped (its windows count nothing). + f.mirror.mu.Lock() + f.mirror.others = []mirror.HostView{ + {Host: "lt", Online: true, StatusReport: true, Windows: mirror.NormalizeWindows([]mirror.Window{ + {Session: "x", ID: "@1", Index: 1, Claude: true, Status: mirror.StatusBusy}, + {Session: "x", ID: "@2", Index: 2, Claude: true, Status: mirror.StatusBusy}, + {Session: "x", ID: "@3", Index: 3, Status: mirror.StatusBusy}}, 0)}, + {Host: "gone", OfflineSince: time.Now().Add(-time.Hour), Windows: mirror.NormalizeWindows([]mirror.Window{ + {Session: "y", ID: "@1", Index: 1, Claude: true, Status: mirror.StatusBusy}}, 0)}, + } + f.mirror.mu.Unlock() + + code, sum, raw := summaryAs(t, f, opTok) + if code != http.StatusOK { + t.Fatalf("the operator's summary: %d %s", code, raw) + } + items, err := f.srv.attentionItems(op) + if err != nil { + t.Fatal(err) + } + if got, want := strings.Join(summaryKeys(sum), ","), strings.Join(itemKeys(items), ","); got != want { + t.Fatalf("the summary's keys %s; attentionItems' %s — the summary must list exactly the queue", got, want) + } + byKey := map[string]attentionSummaryItem{} + for _, it := range sum.Items { + byKey[it.Key] = it + } + sess := sessionKey("ws", "main", "@1") + for key, want := range map[string]struct{ kind, host, title, why, path string }{ + attentionAskKey(mirrored.ID): {"ask", "ws", "Gold:1", "Use «redacted» for the mirror?", attentionOpenPath + attentionAskKey(mirrored.ID)}, + attentionAskKey(agentAsk): {"ask", "", f.op.Name, "Which bucket?", attentionOpenPath + attentionAskKey(agentAsk)}, + sess: {"session", "ws", "main:1 @ ws", "Waiting: main:1 @ ws", attentionOpenPath + sess}, + draftKey(d.ID): {"plugin", "", "Attention inbox", "Reply draft: Dana (Attention inbox)", draftLink(d.ID)}, + } { + it, ok := byKey[key] + if !ok { + t.Errorf("%s is not in the summary: %s", key, raw) + continue + } + if it.Kind != want.kind || it.Host != want.host || it.Title != want.title || it.Why != want.why || it.Path != want.path { + t.Errorf("%s: %+v; want %+v", key, it, want) + } + } + for _, leak := range []string{"ghp_0123abcd9", "the secret plan", "sending the plan", `"yes"`} { + if strings.Contains(raw, leak) { + t.Errorf("the summary carries %q: %s", leak, raw) + } + } + if _, ok := byKey[attentionAskKey(memberAsk)]; ok { + t.Errorf("the operator's summary lists the member's ask") + } + hosts := map[string]attentionSummaryHost{} + for _, h := range sum.Hosts { + hosts[h.Host] = h + } + for name, want := range map[string]attentionSummaryHost{ + "ws": {Host: "ws", Online: true, StatusReport: true, Busy: 0, Idle: 1}, + "lt": {Host: "lt", Online: true, StatusReport: true, Busy: 2, Idle: 0}, + "gone": {Host: "gone"}, + } { + if hosts[name] != want { + t.Errorf("host %s: %+v; want %+v", name, hosts[name], want) + } + } + if len(sum.Hosts) != 3 || sum.NudgeMode != "off" || sum.Partial || sum.TS < time.Now().Add(-time.Minute).Unix() { + t.Errorf("summary %+v", sum) + } + + // Another owner: its own ask, none of the operator's items, none of its hosts. + code, msum, mraw := summaryAs(t, f, memberTok) + if code != http.StatusOK { + t.Fatalf("the member's summary: %d %s", code, mraw) + } + if got := strings.Join(summaryKeys(msum), ","); got != attentionAskKey(memberAsk) || len(msum.Hosts) != 0 { + t.Fatalf("the member's summary: keys %s hosts %+v; want only %s and no hosts", got, msum.Hosts, attentionAskKey(memberAsk)) + } + for _, leak := range []string{"Which bucket?", "Gold:1", "Dana", `"ws"`} { + if strings.Contains(mraw, leak) { + t.Errorf("the member's summary carries the operator's %q", leak) + } + } +} + +// TestAReadOnlyTokenMintedOnThePageReadsAndCannotWrite drives the whole loop +// over the real stores: mint with the box ticked (the list says read-only), +// read the summary with it, and be refused 403 revoking a token — its own +// included — which stays live. +func TestAReadOnlyTokenMintedOnThePageReadsAndCannotWrite(t *testing.T) { + f := newSummaryFixture(t) + t.Cleanup(func() { + _, _ = f.pool.Exec(authz.System(context.Background()), `DELETE FROM api_tokens WHERE user_id = 1 AND name = 'bar-ro'`) + }) + body, hdr := form(url.Values{"name": {"bar-ro"}, "read_only": {"1"}}) + rec := f.call(t, http.MethodPost, accountTokensPath, body, merge(f.operator(), hdr)) + m := mintedRE.FindStringSubmatch(rec.Body.String()) + if rec.Code != http.StatusOK || m == nil { + t.Fatalf("minting a read-only token: %d %s", rec.Code, truncate(rec.Body.String(), 400)) + } + tok := m[1] + if page := f.page(t, accountTokensPath); !strings.Contains(page, `data-token-scope="read-only"`) { + t.Fatalf("the token page does not list the token as read-only") + } + var id int64 + if err := f.pool.QueryRow(authz.System(f.ctx), `SELECT id FROM api_tokens WHERE token_hash = $1 AND read_only`, apitokens.Hash(tok)).Scan(&id); err != nil { + t.Fatalf("the minted token is not stored read-only: %v", err) + } + if code, _, raw := summaryAs(t, f, tok); code != http.StatusOK { + t.Fatalf("the summary with a read-only token: %d %s", code, raw) + } + auth := map[string]string{"Authorization": "Bearer " + tok} + if w := f.call(t, http.MethodDelete, apiTokensPath+"/"+strconv.FormatInt(id, 10), nil, auth); w.Code != http.StatusForbidden || + !strings.Contains(w.Body.String(), "read-only") { + t.Fatalf("a read-only token revoking itself: %d %s; want 403", w.Code, w.Body) + } + if w := f.call(t, http.MethodGet, "/api/whoami", nil, auth); w.Code != http.StatusOK { + t.Fatalf("the read-only token after its refused revoke: %d; it must still be live", w.Code) + } +} diff --git a/internal/api/attention_summary_test.go b/internal/api/attention_summary_test.go new file mode 100644 index 000000000..f5363f3b3 --- /dev/null +++ b/internal/api/attention_summary_test.go @@ -0,0 +1,240 @@ +package api + +import ( + "context" + "encoding/json" + "errors" + "io" + "log" + "net/http" + "net/http/httptest" + "reflect" + "strings" + "testing" + "time" + + "github.com/ZacxDev/muster/internal/attention" + "github.com/ZacxDev/muster/internal/chiefnudge" + "github.com/ZacxDev/muster/internal/mirror" + "github.com/ZacxDev/muster/internal/plugins" + "github.com/ZacxDev/muster/internal/ui" +) + +// summaryNow is the summary fixtures' clock: every item's age is measured +// from it, and each item's Since is a different distance before it. +var summaryNow = time.Unix(1_800_000_000, 0) + +// TestTheSummaryItemSaysWhatItsPushSays pins every field of one item of each +// kind, WHOLE, as literals: its host, its title (the title rule), its why (the +// first line of its push body — a secret redacted, a waiting session summary +// only, a draft who-and-where only), its age, and its link (the tap route for +// an ask's and a session's key, the item's own link otherwise). Every fixture +// carries a Detail, options or a message body that must NOT appear: the +// summary says what the lock screen says and no more. +func TestTheSummaryItemSaysWhatItsPushSays(t *testing.T) { + sessKey := sessionKey("host-b", "main", "@3") + mirrorAsk := askItem(attention.Ask{ID: 41, Asker: attention.Asker{MirrorHost: "ws", MirrorSession: focusSID, Label: "Gold:1"}, + Question: "Use ghp_0123abcd9 for the mirror?\n\nIt is the only key.", Options: []string{"yes", "no"}, CreatedAt: summaryNow.Add(-90 * time.Second)}) + agentAsk := askItem(attention.Ask{ID: 42, Asker: attention.Asker{AgentID: 7, Label: "deploy-bot"}, + Question: "Deploy the importer?", Options: []string{"deploy now", "wait"}, Suggestions: []string{"go ahead"}, CreatedAt: summaryNow.Add(-2 * time.Hour)}) + draft := attentionItem{Key: draftKey(3), Kind: attentionKindPlugin, Source: "Discord DMs", + Text: "Dana : the secret plan", Detail: "Draft reply: sure, sending the plan", + Since: summaryNow.Add(-10 * time.Minute), Link: draftLink(3), + pushText: draftPushBody(plugins.Message{ID: 9, Sender: "Dana ", SenderVerified: true, Body: "the secret plan"}, "Discord DMs")} + for _, c := range []struct { + name string + it attentionItem + want attentionSummaryItem + }{ + {"an ask from a mirrored session: its host, the question redacted, never its options", mirrorAsk, attentionSummaryItem{ + Key: "ask-41", Kind: "ask", Host: "ws", Title: "Gold:1", Why: "Use «redacted» for the mirror? It is the only key.", + Since: 1_799_999_910, AgeSecs: 90, Path: "/attention/open/ask-41", URL: "https://muster.example.test/attention/open/ask-41"}}, + {"an agent's ask: no host, never its options or suggestions", agentAsk, attentionSummaryItem{ + Key: "ask-42", Kind: "ask", Title: "deploy-bot", Why: "Deploy the importer?", + Since: 1_799_992_800, AgeSecs: 7200, Path: "/attention/open/ask-42", URL: "https://muster.example.test/attention/open/ask-42"}}, + {"a session waiting at a prompt: summary only, never Claude's last words", attentionItem{Key: sessKey, Kind: attentionKindSession, + Source: "**Fix** [auth](https://example.com/a) for C# @ host-b", Text: "Bash: curl -H 'Authorization: x' https://x.test", tool: "Bash", + Detail: "Tests pass.\nPublishing now.", host: "host-b", Offline: true, Since: summaryNow.Add(-30 * time.Second), + Link: "/mirror/view?host=host-b&session=main&window=%403", card: attentionCard{ref: ui.MirrorRef{Host: "host-b", Session: "main", Window: "@3"}}}, + attentionSummaryItem{Key: sessKey, Kind: "session", Host: "host-b", Title: "Fix auth for C# @ host-b", Why: "Waiting: Bash · Fix auth for C# @ host-b", + Since: 1_799_999_970, AgeSecs: 30, Offline: true, Path: "/attention/open/" + sessKey, URL: "https://muster.example.test/attention/open/" + sessKey}}, + {"a held question: its first question, never its choices", attentionItem{Key: sessKey, Kind: attentionKindQuestion, Source: "main:3 @ host-b", + Text: "1. Which branch?\n2. Push it?", choices: []string{"main", "dev"}, host: "host-b", Since: summaryNow.Add(-5 * time.Second), + Link: "/mirror/view?host=host-b&session=main&window=%403"}, + attentionSummaryItem{Key: sessKey, Kind: "question", Host: "host-b", Title: "main:3 @ host-b", Why: "1. Which branch?", + Since: 1_799_999_995, AgeSecs: 5, Path: "/attention/open/" + sessKey, URL: "https://muster.example.test/attention/open/" + sessKey}}, + {"a reply draft: who and where only, never the message or the reply", draft, attentionSummaryItem{ + Key: "draft-3", Kind: "plugin", Title: "Discord DMs", Why: "Reply draft: Dana (Discord DMs)", + Since: 1_799_999_400, AgeSecs: 600, Path: "/attention?tab=drafts#draft-3", URL: "https://muster.example.test/attention?tab=drafts#draft-3"}}, + {"a task nobody routed: its own link", attentionItem{Key: unroutedTaskKey(55), Kind: attentionKindTask, Source: "your chief has not routed it", + Text: "Task #9: Rotate the build key", Since: summaryNow.Add(-24 * time.Hour), Link: canonicalTaskPath(9)}, + attentionSummaryItem{Key: "task-55", Kind: "task", Title: "your chief has not routed it", Why: "Task #9: Rotate the build key", + Since: 1_799_913_600, AgeSecs: 86400, Path: "/tasks/9", URL: "https://muster.example.test/tasks/9"}}, + } { + if got := attentionSummaryItemOf(c.it, "https://muster.example.test", summaryNow); got != c.want { + t.Errorf("%s:\n got %+v\nwant %+v", c.name, got, c.want) + } + } + // No public URL: the path alone, and no since is no age. + if got := attentionSummaryItemOf(attentionItem{Key: "ask-1", Kind: attentionKindAsk, Source: "x", Text: "q"}, "", summaryNow); got.URL != "" || got.Path != "/attention/open/ask-1" || got.Since != 0 || got.AgeSecs != 0 { + t.Errorf("no public URL, no since: %+v", got) + } +} + +// TestTheSummaryRedactsExactlyAsThePushDoes pins one item's why as a WHOLE +// normalised string beside its push body's first line, so a summary that took +// its text from anywhere else (the raw Text, the Detail) is a visible diff. +func TestTheSummaryRedactsExactlyAsThePushDoes(t *testing.T) { + it := askItem(attention.Ask{ID: 8, Asker: attention.Asker{AgentID: 3, Label: "ci"}, + Question: "Rotate sk-ant-api03-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\tand ghp_0123abcd9\n\nnow?", Options: []string{"ghp_0123abcd9", "no"}}) + const want = "Rotate «redacted» and «redacted» now?" + push := firstLine(attentionPushBody(it)) + got := attentionSummaryItemOf(it, "", summaryNow).Why + if got != want || push != want { + t.Fatalf("summary why %q, push first line %q; want both %q", got, push, want) + } +} + +// TestTheSummaryCountsEachHostsClaudeWindows: an online host's Claude windows +// (and another agent runtime's) by status — busy and idle counted, waiting and +// a plain shell not — and an OFFLINE host counts nothing (its statuses are a +// claim it can no longer vouch for), with its online and status_report as the +// hub reports them. +func TestTheSummaryCountsEachHostsClaudeWindows(t *testing.T) { + ws := []mirror.Window{ + {Session: "a", ID: "@1", Claude: true, Status: mirror.StatusBusy}, + {Session: "a", ID: "@2", Claude: true, Status: mirror.StatusBusy}, + {Session: "a", ID: "@3", Claude: true, Status: mirror.StatusIdle}, + {Session: "a", ID: "@4", Claude: true, Status: mirror.StatusWaiting}, + {Session: "a", ID: "@5", Claude: true, Status: mirror.StatusBusy}, + {Session: "b", ID: "@6", Status: mirror.StatusBusy}, // a shell: not an agent window + {Session: "b", ID: "@7", Claude: true, Status: mirror.StatusIdle}, + {Session: "b", ID: "@8", Claude: true, Status: mirror.StatusIdle}, + } + for _, c := range []struct { + h mirror.HostView + want attentionSummaryHost + }{ + {mirror.HostView{Host: "host-a", Online: true, StatusReport: true, Windows: ws}, + attentionSummaryHost{Host: "host-a", Online: true, StatusReport: true, Busy: 3, Idle: 3}}, + {mirror.HostView{Host: "host-c", Online: false, OfflineSince: summaryNow, Windows: ws}, + attentionSummaryHost{Host: "host-c"}}, + {mirror.HostView{Host: "old", Online: true, Windows: ws[:1]}, + attentionSummaryHost{Host: "old", Online: true, Busy: 1}}, + } { + if got := attentionSummaryHostOf(c.h); got != c.want { + t.Errorf("%s: %+v; want %+v", c.h.Host, got, c.want) + } + } +} + +// TestTheSummarysNudgeModeIsTheOwnersOwn: off when chief nudges do not run +// here; otherwise live for an owner on the live list and shadow for everyone +// else — the boot banner's source (ChiefNudges.Live). +func TestTheSummarysNudgeModeIsTheOwnersOwn(t *testing.T) { + s := New(nil, AuthConfig{}, log.New(io.Discard, "", 0)) + if got := s.attentionSummaryOf(7, nil, false, nil, summaryNow).NudgeMode; got != "off" { + t.Fatalf("nudges off: %q", got) + } + s.ext.ChiefNudge, s.ext.ChiefNudges = &ChiefNudges{Live: map[int64]bool{7: true}}, chiefnudge.NewPG(nil) + for owner, want := range map[int64]string{7: "live", 8: "shadow"} { + if got := s.attentionSummaryOf(owner, nil, false, nil, summaryNow).NudgeMode; got != want { + t.Errorf("owner %d: %q; want %q", owner, got, want) + } + } +} + +// TestTheSummaryDocumentIsTheDocumentedShape pins the WHOLE JSON of one +// summary — field names, types, order and the empty-array-not-null rule — as +// docs/attention.md shows it. A rename breaks every poller; this is where it +// shows. +func TestTheSummaryDocumentIsTheDocumentedShape(t *testing.T) { + s := New(nil, AuthConfig{PublicURL: "https://muster.example.test/"}, log.New(io.Discard, "", 0)) + empty, _ := json.Marshal(s.attentionSummaryOf(1, nil, true, nil, summaryNow)) + if want := `{"ts":1800000000,"nudge_mode":"off","partial":true,"hosts":[],"items":[]}`; string(empty) != want { + t.Fatalf("an empty summary:\n got %s\nwant %s", empty, want) + } + items := []attentionItem{{Key: "ask-5", Kind: attentionKindAsk, Source: "chief", Text: "Ship it?", Since: summaryNow.Add(-time.Minute)}} + hosts := []mirror.HostView{{Host: "host-a", Online: true, StatusReport: true, Windows: []mirror.Window{{Session: "a", ID: "@1", Claude: true, Status: mirror.StatusBusy}}}} + got, _ := json.Marshal(s.attentionSummaryOf(1, items, false, hosts, summaryNow)) + const want = `{"ts":1800000000,"nudge_mode":"off","partial":false,` + + `"hosts":[{"host":"host-a","online":true,"status_report":true,"busy":1,"idle":0}],` + + `"items":[{"key":"ask-5","kind":"ask","host":"","title":"chief","why":"Ship it?","since":1799999940,"age_secs":60,"offline":false,` + + `"path":"/attention/open/ask-5","url":"https://muster.example.test/attention/open/ask-5"}]}` + if string(got) != want { + t.Fatalf("a summary:\n got %s\nwant %s", got, want) + } +} + +// TestTheSummaryTakesOnlyAPersonalToken: no credential, the shared hook +// secret, and a session cookie are each refused; a personal token is admitted +// as its owner. Over the real mux (registerAll), so the route's wrapper is +// what is tested. +func TestTheSummaryTakesOnlyAPersonalToken(t *testing.T) { + f := newFakeAPITokens() + tok := f.add(t, 7) + s := New(nil, AuthConfig{HookToken: tokTestHook, UIPassword: "summary-operator-password"}, log.New(io.Discard, "", 0)) + s.UseExtensions(Extensions{APITokens: f, Users: stubUsers{}}) + mux := http.NewServeMux() + s.registerAll(mux) + get := func(set func(*http.Request)) *httptest.ResponseRecorder { + r := httptest.NewRequest(http.MethodGet, attentionSummaryPath, nil) + set(r) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, r) + return rec + } + for name, set := range map[string]func(*http.Request){ + "none": func(*http.Request) {}, + "hook": func(r *http.Request) { r.Header.Set("Authorization", "Bearer "+tokTestHook) }, + "session": func(r *http.Request) { r.Header.Set("Cookie", sessionCookieFor(s)) }, + } { + if rec := get(set); rec.Code != http.StatusUnauthorized { + t.Errorf("%s: %d %s; want 401", name, rec.Code, truncate(rec.Body.String(), 200)) + } + } + rec := get(func(r *http.Request) { r.Header.Set("Authorization", "Bearer "+tok) }) + var body attentionSummary + if err := json.Unmarshal(rec.Body.Bytes(), &body); rec.Code != 200 || err != nil || body.NudgeMode != "off" || body.Items == nil || body.Hosts == nil { + t.Fatalf("a personal token: %d %s (%v)", rec.Code, rec.Body, err) + } + if cc := rec.Header().Get("Cache-Control"); cc != "no-store" { + t.Errorf("Cache-Control %q; want no-store", cc) + } + if !reflect.DeepEqual(body.Items, []attentionSummaryItem{}) || strings.Contains(rec.Body.String(), "null") { + t.Errorf("an owner with nothing waiting: %s", rec.Body) + } +} + +// failingAttention is an attention store whose open asks cannot be read. +type failingAttention struct{ attention.Store } + +func (failingAttention) Open(context.Context) ([]attention.Ask, error) { + return nil, errors.New("connection reset") +} + +// TestASummaryThatCouldNotReadTheQueueSaysSo: when a source of the queue +// fails, the summary is still served (the rest of it is true) but says +// partial — a poller must not read the missing asks as "nothing waits". The +// control: the same token with a store that reads says partial false. +func TestASummaryThatCouldNotReadTheQueueSaysSo(t *testing.T) { + for _, c := range []struct { + store attention.Store + want bool + }{{failingAttention{}, true}, {emptyAttention{}, false}} { + f := newFakeAPITokens() + tok := f.add(t, 7) + s := New(nil, AuthConfig{HookToken: tokTestHook}, log.New(io.Discard, "", 0)) + s.UseExtensions(Extensions{APITokens: f, Users: stubUsers{}, Attention: c.store}) + mux := http.NewServeMux() + s.registerAll(mux) + r := httptest.NewRequest(http.MethodGet, attentionSummaryPath, nil) + r.Header.Set("Authorization", "Bearer "+tok) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, r) + var body attentionSummary + if err := json.Unmarshal(rec.Body.Bytes(), &body); rec.Code != http.StatusOK || err != nil || body.Partial != c.want { + t.Errorf("%T: %d %s; want 200 with partial %v", c.store, rec.Code, rec.Body, c.want) + } + } +} diff --git a/internal/api/flows_test.go b/internal/api/flows_test.go index 6ad0f437f..b776c263f 100644 --- a/internal/api/flows_test.go +++ b/internal/api/flows_test.go @@ -218,11 +218,11 @@ func TestFlowLifecycleOverHTTP(t *testing.T) { t.Fatal(err) } ts := apitokens.NewPG(pool) - aTok, _, err := ts.Create(asUser(a), "a", 0) + aTok, _, err := ts.Create(asUser(a), "a", 0, false) if err != nil { t.Fatal(err) } - bTok, _, err := ts.Create(asUser(b), "b", 0) + bTok, _, err := ts.Create(asUser(b), "b", 0, false) if err != nil { t.Fatal(err) } @@ -428,11 +428,11 @@ func TestFlowCancelOverTheAPI(t *testing.T) { return authz.WithPrincipal(ctx, authz.Principal{UserID: uid, Role: authz.RoleMember, Via: authz.ViaSession}) } ts := apitokens.NewPG(pool) - aTok, _, err := ts.Create(asUser(a), "a", 0) + aTok, _, err := ts.Create(asUser(a), "a", 0, false) if err != nil { t.Fatal(err) } - bTok, _, err := ts.Create(asUser(b), "b", 0) + bTok, _, err := ts.Create(asUser(b), "b", 0, false) if err != nil { t.Fatal(err) } diff --git a/internal/api/isolation_test.go b/internal/api/isolation_test.go index 007904268..c62e23899 100644 --- a/internal/api/isolation_test.go +++ b/internal/api/isolation_test.go @@ -185,6 +185,8 @@ var isolationClasses = map[string]routeClass{ "GET /api/doctor": classAdmin, "GET /api/tokens": classAPIToken, "DELETE /api/tokens/{id}": classAPIToken, + // The attention summary: the token owner's own queue and hosts. + "GET /api/attention/summary": classAPIToken, // muster-native Web Push (migration 0013). The key and the subscribe / // unsubscribe writes act on the CALLER's own devices (list); the account // page's remove names a device by id (param, A's is the object). A's device @@ -540,6 +542,9 @@ var wantCanaryForA = []string{ // A's canary-carrying question. (The tap on A's ask opens that page, by // its id — controlMarkers has its own control.) "GET /attention/asker", "GET /attention/asks/{id}", + // The attention summary through A's personal token: A's canary-carrying + // ask is on it (its why is the question). + "GET /api/attention/summary", } func TestEveryRouteIsClassifiedForIsolation(t *testing.T) { @@ -809,11 +814,11 @@ func newIsoWorld(t *testing.T) *isoWorld { ts := apitokens.NewPG(pool) w.apiTokens = map[int64]string{} for _, u := range []isoUser{w.a, w.b} { - tok, _, err := ts.Create(as2(u), u.name+" cli", 0) + tok, _, err := ts.Create(as2(u), u.name+" cli", 0, false) must(err, u.name+" api token") w.apiTokens[u.uid] = tok } - _, w.aToken2, err = ts.Create(ua, w.canary+" token", 0) + _, w.aToken2, err = ts.Create(ua, w.canary+" token", 0, false) must(err, "a second api token") w.aTokTask, err = ns.Create(ua, notes.Note{Title: w.canary + " token-control", Status: notes.StatusOpen}) must(err, "a token-control task") diff --git a/internal/api/nostore_ledger_test.go b/internal/api/nostore_ledger_test.go index 9a7b41745..02de41fb9 100644 --- a/internal/api/nostore_ledger_test.go +++ b/internal/api/nostore_ledger_test.go @@ -73,6 +73,7 @@ var noStoreLiterals = map[string]string{ "handleAttentionBadge": "the Asks badge", "handleAttentionItems": "the Asks list", "handleAttentionOpen": "an ask opened from a push", + "handleAttentionSummary": "the attention summary a status bar polls", "renderAskPage": "an ask page's live region (its options, suggestions and state)", "writeAskLive": "an ask's live answer", "handleFocusHistory": "an agent's focus history", diff --git a/internal/api/principal_test.go b/internal/api/principal_test.go index fd1b8dea0..f45b4e538 100644 --- a/internal/api/principal_test.go +++ b/internal/api/principal_test.go @@ -369,6 +369,8 @@ var routeVias = map[authz.Via][]string{ "GET /api/flows/jobs/{id}", "POST /api/flows/jobs", "POST /api/flows/jobs/{id}/cancel", + // The attention summary (attention_summary.go): a personal token ONLY. + "GET /api/attention/summary", }, // A browser-flow RUNNER token (0012) opens exactly the runner door. authz.ViaRunner: { diff --git a/internal/api/session_routes_pg_test.go b/internal/api/session_routes_pg_test.go index 95b9aff98..c07a44463 100644 --- a/internal/api/session_routes_pg_test.go +++ b/internal/api/session_routes_pg_test.go @@ -136,7 +136,7 @@ func newRouteFixture(t *testing.T) *routeFixture { g.chiefTok, g.workerTok = "route-chief-token-"+suffix, "route-worker-token-"+suffix g.chief = g.newAgent(t, g.owner, "rtchief-"+suffix, "chief", g.chiefTok) g.worker = g.newAgent(t, g.owner, "rtworker-"+suffix, "", g.workerTok) - tok, _, err := apitokens.NewPG(g.pool).Create(g.asOwner, "route-reply", time.Now().Unix()) + tok, _, err := apitokens.NewPG(g.pool).Create(g.asOwner, "route-reply", time.Now().Unix(), false) if err != nil { t.Fatal(err) } @@ -389,7 +389,7 @@ func TestTheAnswerIsRelayedPerTheAllowlist(t *testing.T) { // Another user's credential: the route is not theirs, so it is not found. stranger := f.newMember(t) - tok, _, err := apitokens.NewPG(f.pool).Create(f.as(stranger), "stranger", time.Now().Unix()) + tok, _, err := apitokens.NewPG(f.pool).Create(f.as(stranger), "stranger", time.Now().Unix(), false) if err != nil { t.Fatal(err) } diff --git a/internal/api/testdata/routes.golden b/internal/api/testdata/routes.golden index 22073efd7..c956836d2 100644 --- a/internal/api/testdata/routes.golden +++ b/internal/api/testdata/routes.golden @@ -62,6 +62,7 @@ GET /agents/{name}/ws GET /api/agents GET /api/agents/{name}/account-failovers GET /api/agents/{name}/messages +GET /api/attention/summary GET /api/device/config GET /api/directories GET /api/doctor diff --git a/internal/apitokens/apitokens.go b/internal/apitokens/apitokens.go index bcb9d4199..a7ac696ab 100644 --- a/internal/apitokens/apitokens.go +++ b/internal/apitokens/apitokens.go @@ -88,6 +88,11 @@ type Token struct { CreatedAt time.Time `json:"createdAt"` LastUsedAt *time.Time `json:"lastUsedAt"` RevokedAt *time.Time `json:"revokedAt"` + // ReadOnly: the token may only READ (migration 0068). Its scope is chosen + // when it is minted and never changes; a token minted before 0068 is full + // access. What "read" admits is decided at the door + // (internal/api.admitAPIToken), not here. + ReadOnly bool `json:"readOnly"` } // Revoked reports whether the token is revoked. @@ -133,13 +138,13 @@ type Store interface { // ErrUserDisabled when the owner is disabled. ctx must be able to read any // user's row (authz.System): the owner is what it learns. Authenticate(ctx context.Context, token string) (Token, error) - // Create mints a token named name for ctx's user, records it in - // audit_log, and returns the token ONCE with its record. + // Create mints a token named name for ctx's user, read-only when readOnly, + // records it in audit_log, and returns the token ONCE with its record. // sessionIssuedAt is the minting browser session's iat (unix seconds): a // session issued at or before the user's session cutoff is refused // ErrSessionEnded. Zero means the caller is not a GoTrue session (password // mode, a fixture) and there is no cutoff to compare. - Create(ctx context.Context, name string, sessionIssuedAt int64) (string, Token, error) + Create(ctx context.Context, name string, sessionIssuedAt int64, readOnly bool) (string, Token, error) // List returns ctx's user's tokens, newest first, revoked included. List(ctx context.Context) ([]Token, error) // Revoke revokes ctx's user's token id. found is false when the user has @@ -159,11 +164,11 @@ func NewPG(pool *pgxpool.Pool) *PGStore { return &PGStore{pool: pool, now: time. var _ Store = (*PGStore)(nil) -const cols = `id, user_id, name, created_at, last_used_at, revoked_at` +const cols = `id, user_id, name, created_at, last_used_at, revoked_at, read_only` func scan(row pgx.Row) (Token, error) { var t Token - err := row.Scan(&t.ID, &t.UserID, &t.Name, &t.CreatedAt, &t.LastUsedAt, &t.RevokedAt) + err := row.Scan(&t.ID, &t.UserID, &t.Name, &t.CreatedAt, &t.LastUsedAt, &t.RevokedAt, &t.ReadOnly) return t, err } @@ -185,11 +190,11 @@ func (s *PGStore) Authenticate(ctx context.Context, token string) (Token, error) } var t Token var disabled bool - err := s.pool.QueryRow(ctx, `SELECT t.id, t.user_id, t.name, t.created_at, t.last_used_at, t.revoked_at, + err := s.pool.QueryRow(ctx, `SELECT t.id, t.user_id, t.name, t.created_at, t.last_used_at, t.revoked_at, t.read_only, u.disabled_at IS NOT NULL FROM api_tokens t JOIN users u ON u.id = t.user_id WHERE t.token_hash = $1`, Hash(token)). - Scan(&t.ID, &t.UserID, &t.Name, &t.CreatedAt, &t.LastUsedAt, &t.RevokedAt, &disabled) + Scan(&t.ID, &t.UserID, &t.Name, &t.CreatedAt, &t.LastUsedAt, &t.RevokedAt, &t.ReadOnly, &disabled) if errors.Is(err, pgx.ErrNoRows) { return Token{}, ErrBadToken } @@ -251,7 +256,7 @@ func CleanName(name string) (string, error) { // with users.sessions_not_before by the session tier's own rule (at or before // the cutoff's second is refused): a session the reset ended cannot mint, and // the session the reset signs in can (TestAMintQueuedBehindAResetIsRefused). -func (s *PGStore) Create(ctx context.Context, name string, sessionIssuedAt int64) (string, Token, error) { +func (s *PGStore) Create(ctx context.Context, name string, sessionIssuedAt int64, readOnly bool) (string, Token, error) { p, err := user(ctx) if err != nil { return "", Token{}, err @@ -282,12 +287,12 @@ func (s *PGStore) Create(ctx context.Context, name string, sessionIssuedAt int64 return ErrSessionEnded } var err error - t, err = scan(tx.QueryRow(ctx, `INSERT INTO api_tokens (user_id, name, token_hash) - VALUES ($1, $2, $3) RETURNING `+cols, p.UserID, name, Hash(tok))) + t, err = scan(tx.QueryRow(ctx, `INSERT INTO api_tokens (user_id, name, token_hash, read_only) + VALUES ($1, $2, $3, $4) RETURNING `+cols, p.UserID, name, Hash(tok), readOnly)) if err != nil { return err } - return audit(ctx, tx, p, "api_token.create", t.ID, map[string]any{"name": t.Name}) + return audit(ctx, tx, p, "api_token.create", t.ID, map[string]any{"name": t.Name, "readOnly": t.ReadOnly}) }) if err != nil { return "", Token{}, err diff --git a/internal/apitokens/apitokens_test.go b/internal/apitokens/apitokens_test.go index ebe7063f3..85153c3aa 100644 --- a/internal/apitokens/apitokens_test.go +++ b/internal/apitokens/apitokens_test.go @@ -108,7 +108,7 @@ func TestAMalformedTokenIsRefusedWithoutADatabaseRead(t *testing.T) { func TestOnlyTheHashIsStoredAndTheTokenIsShownOnce(t *testing.T) { w := newWorld(t) - tok, rec, err := w.s.Create(as(w.a, authz.RoleMember), " laptop ", 0) + tok, rec, err := w.s.Create(as(w.a, authz.RoleMember), " laptop ", 0, false) if err != nil { t.Fatal(err) } @@ -145,11 +145,11 @@ func TestOnlyTheHashIsStoredAndTheTokenIsShownOnce(t *testing.T) { func TestAuthenticateResolvesTheOwnerAndRefusesRevokedAndDisabled(t *testing.T) { w := newWorld(t) - tokA, recA, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0) + tokA, recA, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, false) if err != nil { t.Fatal(err) } - tokB, _, err := w.s.Create(as(w.b, authz.RoleMember), "b", 0) + tokB, _, err := w.s.Create(as(w.b, authz.RoleMember), "b", 0, false) if err != nil { t.Fatal(err) } @@ -218,11 +218,11 @@ func TestAuthenticateResolvesTheOwnerAndRefusesRevokedAndDisabled(t *testing.T) // a token minted after the re-enable works. func TestDisablingAUserRevokesItsTokensForGood(t *testing.T) { w := newWorld(t) - tokA, _, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0) + tokA, _, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, false) if err != nil { t.Fatal(err) } - tokB, _, err := w.s.Create(as(w.b, authz.RoleMember), "b", 0) + tokB, _, err := w.s.Create(as(w.b, authz.RoleMember), "b", 0, false) if err != nil { t.Fatal(err) } @@ -246,7 +246,7 @@ func TestDisablingAUserRevokesItsTokensForGood(t *testing.T) { if got, err := w.s.Authenticate(sysCtx(), tokB); err != nil || got.UserID != w.b { t.Fatalf("CONTROL: B's token after A's disable: %+v %v", got, err) } - fresh, _, err := w.s.Create(as(w.a, authz.RoleMember), "after", 0) + fresh, _, err := w.s.Create(as(w.a, authz.RoleMember), "after", 0, false) if err != nil { t.Fatal(err) } @@ -259,7 +259,7 @@ func TestDisablingAUserRevokesItsTokensForGood(t *testing.T) { // "no such token", and A's token still works. func TestNobodyRevokesAnotherUsersToken(t *testing.T) { w := newWorld(t) - tokA, recA, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0) + tokA, recA, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, false) if err != nil { t.Fatal(err) } @@ -280,7 +280,7 @@ func TestNobodyRevokesAnotherUsersToken(t *testing.T) { // The positive control is A, who sees its own. func TestRowLevelSecurityKeepsTokensToTheirOwner(t *testing.T) { w := newWorld(t) - if _, _, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0); err != nil { + if _, _, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, false); err != nil { t.Fatal(err) } count := func(ctx context.Context) int { @@ -311,7 +311,7 @@ func TestRowLevelSecurityKeepsTokensToTheirOwner(t *testing.T) { func TestManagementNeedsAUserPrincipal(t *testing.T) { w := newWorld(t) for name, ctx := range map[string]context.Context{"none": context.Background(), "system": sysCtx()} { - if _, _, err := w.s.Create(ctx, "x", 0); !errors.Is(err, ErrNoUser) { + if _, _, err := w.s.Create(ctx, "x", 0, false); !errors.Is(err, ErrNoUser) { t.Errorf("%s Create: %v", name, err) } if _, err := w.s.List(ctx); !errors.Is(err, ErrNoUser) { @@ -321,10 +321,10 @@ func TestManagementNeedsAUserPrincipal(t *testing.T) { t.Errorf("%s Revoke: %v", name, err) } } - if _, _, err := w.s.Create(as(w.a, authz.RoleMember), strings.Repeat("é", MaxNameLen+1), 0); !errors.Is(err, ErrNameTooLong) { + if _, _, err := w.s.Create(as(w.a, authz.RoleMember), strings.Repeat("é", MaxNameLen+1), 0, false); !errors.Is(err, ErrNameTooLong) { t.Errorf("an over-long name: %v", err) } - if _, _, err := w.s.Create(as(w.a, authz.RoleMember), strings.Repeat("é", MaxNameLen), 0); err != nil { + if _, _, err := w.s.Create(as(w.a, authz.RoleMember), strings.Repeat("é", MaxNameLen), 0, false); err != nil { t.Errorf("a name of exactly %d characters: %v", MaxNameLen, err) } } @@ -333,7 +333,7 @@ func TestManagementNeedsAUserPrincipal(t *testing.T) { // within LastUsedGranularity does not rewrite it; a use after it does. func TestLastUsedIsStampedAndThrottled(t *testing.T) { w := newWorld(t) - tok, rec, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0) + tok, rec, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, false) if err != nil { t.Fatal(err) } @@ -437,7 +437,7 @@ func raceMintAgainstDisable(t *testing.T, w world, role authz.Role, lockTable st } mintc := make(chan minted, 1) go func() { - tok, _, err := w.s.Create(as(w.a, role), "racer", 0) + tok, _, err := w.s.Create(as(w.a, role), "racer", 0, false) mintc <- minted{tok, err} }() lw.until(2, "the mint to block") @@ -587,7 +587,7 @@ func TestAMintQueuedBehindAResetIsRefused(t *testing.T) { } mintc := make(chan minted, 1) go func() { - tok, _, err := w.s.Create(as(w.a, authz.RoleMember), "queued", oldSession) + tok, _, err := w.s.Create(as(w.a, authz.RoleMember), "queued", oldSession, false) mintc <- minted{tok, err} }() lw.until(2, "the mint to queue behind the reset") @@ -609,10 +609,10 @@ func TestAMintQueuedBehindAResetIsRefused(t *testing.T) { } // The boundary: a session stamped with the cutoff's own second is refused // (the session tier's rule); the next second's is the post-reset session. - if _, _, err := w.s.Create(as(w.a, authz.RoleMember), "same-second", r.cut.Unix()); !errors.Is(err, ErrSessionEnded) { + if _, _, err := w.s.Create(as(w.a, authz.RoleMember), "same-second", r.cut.Unix(), false); !errors.Is(err, ErrSessionEnded) { t.Fatalf("a session issued in the cutoff's own second: %v; want ErrSessionEnded", err) } - tok, _, err := w.s.Create(as(w.a, authz.RoleMember), "after-reset", r.cut.Unix()+1) + tok, _, err := w.s.Create(as(w.a, authz.RoleMember), "after-reset", r.cut.Unix()+1, false) if err != nil { t.Fatalf("the POST-reset session cannot mint: %v", err) } @@ -655,8 +655,8 @@ func stressCredentialWriters(t *testing.T, w world, rounds int, budget time.Dura target := users.Actor{UserID: w.a, Via: authz.ViaSession} asX, asY := as(w.a, authz.RoleAdmin), as(w.b, authz.RoleAdmin) ops := map[string]func(i int) error{ - "mint": func(int) error { _, _, err := w.s.Create(asX, "stress", 0); return err }, - "mint-by-another-admin": func(int) error { _, _, err := w.s.Create(asY, "stress", 0); return err }, + "mint": func(int) error { _, _, err := w.s.Create(asX, "stress", 0, false); return err }, + "mint-by-another-admin": func(int) error { _, _, err := w.s.Create(asY, "stress", 0, false); return err }, "revoke-one": func(int) error { list, err := w.s.List(asX) if err != nil { @@ -923,3 +923,48 @@ func TestCredentialWritersNeverDeadlock(t *testing.T) { } t.Logf("%d lockstep rounds, 0 deadlocks; successes per writer: %v", res.rounds, res.ok) } + +// TestAReadOnlyTokensScopeIsStoredAndReadBack (migration 0068): a token minted +// read-only says so in its record, in the list and in Authenticate's answer +// (the door reads THAT one); a token minted without is full access everywhere; +// and the audit row of each mint names its scope. +func TestAReadOnlyTokensScopeIsStoredAndReadBack(t *testing.T) { + w := newWorld(t) + ro, roRec, err := w.s.Create(as(w.a, authz.RoleMember), "bar", 0, true) + if err != nil { + t.Fatal(err) + } + full, fullRec, err := w.s.Create(as(w.a, authz.RoleMember), "cli", 0, false) + if err != nil { + t.Fatal(err) + } + if !roRec.ReadOnly || fullRec.ReadOnly { + t.Fatalf("records: read-only %+v, full %+v", roRec, fullRec) + } + for tok, want := range map[string]bool{ro: true, full: false} { + got, err := w.s.Authenticate(sysCtx(), tok) + if err != nil || got.ReadOnly != want { + t.Errorf("Authenticate: %+v %v; want readOnly %v", got, err, want) + } + } + list, err := w.s.List(as(w.a, authz.RoleMember)) + if err != nil || len(list) != 2 { + t.Fatalf("list = %+v, %v", list, err) + } + for _, tk := range list { + if tk.ReadOnly != (tk.ID == roRec.ID) { + t.Errorf("listed %+v: readOnly is not the minted scope", tk) + } + } + var details string + if err := w.pool.QueryRow(sysCtx(), `SELECT string_agg(detail->>'readOnly', ',' ORDER BY id) FROM audit_log + WHERE user_id = $1 AND action = 'api_token.create'`, w.a).Scan(&details); err != nil || details != "true,false" { + t.Fatalf("the mints' audit scopes = %q (%v); want \"true,false\"", details, err) + } + // A row written without the column (an older image's INSERT) is full access. + var dflt bool + if err := w.pool.QueryRow(sysCtx(), `SELECT column_default = 'false' AND is_nullable = 'NO' FROM information_schema.columns + WHERE table_name = 'api_tokens' AND column_name = 'read_only'`).Scan(&dflt); err != nil || !dflt { + t.Fatalf("api_tokens.read_only is not NOT NULL DEFAULT false: %v %v", dflt, err) + } +} diff --git a/internal/db/migrations/0068_api_token_read_only.sql b/internal/db/migrations/0068_api_token_read_only.sql new file mode 100644 index 000000000..aa09cc002 --- /dev/null +++ b/internal/db/migrations/0068_api_token_read_only.sql @@ -0,0 +1,24 @@ +-- muster 0068 — READ-ONLY personal API tokens (internal/apitokens). +-- +-- * api_tokens.read_only (default false): the token may only read. It is +-- chosen when the token is minted (/account/tokens) and never changes. +-- Every existing row is false, so every token minted before this one keeps +-- the full access it was minted with. +-- +-- What "read" admits is decided at the token door (internal/api +-- admitAPIToken): a read-only token is refused 403 on any method but GET and +-- HEAD, on every route a personal token reaches. +-- +-- The column inherits the table's ownership, grants and row-level security +-- (0009's user_isolation); no policy changes, nothing for db.RLSOn to re-apply. +-- +-- 🔴 Rollback: an image from before 0068 never reads read_only, so it would +-- admit a read-only token with FULL access. Revoke every live read-only token +-- first, then drop the column: +-- UPDATE public.api_tokens SET revoked_at = now() WHERE read_only AND revoked_at IS NULL; +-- ALTER TABLE public.api_tokens DROP COLUMN read_only; +-- DELETE FROM schema_migrations WHERE version = 68; + +SET LOCAL lock_timeout = '3s'; + +ALTER TABLE public.api_tokens ADD COLUMN IF NOT EXISTS read_only BOOLEAN NOT NULL DEFAULT false; diff --git a/tests/verb-ledger.sh b/tests/verb-ledger.sh index 0432f0859..e9b0badce 100755 --- a/tests/verb-ledger.sh +++ b/tests/verb-ledger.sh @@ -64,6 +64,7 @@ agent resolve agent task comment agent task get agent task status +attention summary chief ask chief replace doctor @@ -87,7 +88,7 @@ whoami" # still fails until someone moves this literal deliberately. (tree_test.go pins # the identical number for the in-process tree; they are two reads of one # decision.) -WANT_COUNT=23 +WANT_COUNT=24 # FORBIDDEN is the other half of the split: verbs that belong to the upstream # CLI this one was extracted from and must NEVER appear here. From 69c3b855749382c385b0bc3396e05773d70fd3ee Mon Sep 17 00:00:00 2001 From: Zachary Lowden Date: Sat, 31 Oct 2026 23:46:05 -0500 Subject: [PATCH 2/3] rework(attention-summary): summary-only token scope, no CLI verb, url only Round-0 answers on the PR. Q1: the read_only boolean becomes a token SCOPE. Migration 0068 is edited in place (unreleased): api_tokens.scope TEXT NOT NULL DEFAULT 'full', CHECK (scope IN ('full', 'summary')). A 'summary' token reaches GET (and so HEAD) /api/attention/summary and NOTHING else: every other route a personal token reaches, the hook-tier door included, answers 403 before its handler runs. The rule is one allowlist, tokenScopeRoutes (scope -> mux patterns, read from http.Request.Pattern), checked in the single admit path (admitAPIToken); only 'full' is unlimited, and an unknown or blank scope reaches nothing. GET /api/whoami is denied to a summary token on purpose: a bar does not need it, and the summary's own 200/401/403 already says whether the token works. Minting is a choice between "Full access" (the default) and "Bar summary only (can read the Attention summary and nothing else)"; the token page, /api/tokens (muster token ls) and whoami show the scope. An unknown scope at mint is a 400. Existing tokens stay full. D1: the `muster attention summary` verb and everything that only it used are gone (verb count back to 23). The bar GETs the endpoint over HTTPS. D2: the item's `path` field is dropped; `url` alone remains, absolute on MUSTER_PUBLIC_URL and "" when that is unset (documented). Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/muster/attention.go | 49 ---- cmd/muster/attention_test.go | 71 ----- cmd/muster/output_test.go | 1 - cmd/muster/root.go | 2 +- cmd/muster/token_seam_test.go | 4 +- cmd/muster/tree_test.go | 5 +- docs/attention.md | 11 +- docs/auth.md | 25 +- docs/cli.md | 1 - docs/schema.md | 2 +- internal/api/api_tokens.go | 82 ++++-- internal/api/api_tokens_readonly_test.go | 179 ------------ internal/api/api_tokens_scope_test.go | 272 ++++++++++++++++++ internal/api/api_tokens_test.go | 16 +- internal/api/attention.go | 3 +- internal/api/attention_summary.go | 16 +- internal/api/attention_summary_pg_test.go | 82 ++++-- internal/api/attention_summary_test.go | 24 +- internal/api/flows_test.go | 8 +- internal/api/isolation_test.go | 4 +- internal/api/principal_test.go | 2 +- internal/api/session_routes_pg_test.go | 4 +- internal/apitokens/apitokens.go | 57 +++- internal/apitokens/apitokens_test.go | 94 +++--- .../migrations/0068_api_token_read_only.sql | 24 -- .../db/migrations/0068_api_token_scope.sql | 28 ++ tests/verb-ledger.sh | 3 +- 27 files changed, 569 insertions(+), 500 deletions(-) delete mode 100644 cmd/muster/attention.go delete mode 100644 cmd/muster/attention_test.go delete mode 100644 internal/api/api_tokens_readonly_test.go create mode 100644 internal/api/api_tokens_scope_test.go delete mode 100644 internal/db/migrations/0068_api_token_read_only.sql create mode 100644 internal/db/migrations/0068_api_token_scope.sql diff --git a/cmd/muster/attention.go b/cmd/muster/attention.go deleted file mode 100644 index 200953b32..000000000 --- a/cmd/muster/attention.go +++ /dev/null @@ -1,49 +0,0 @@ -package main - -import ( - "net/http" - - "github.com/spf13/cobra" -) - -// The attention verb: `attention summary` over GET /api/attention/summary — -// what is waiting on you, your mirror hosts' busy/idle counts and your chief -// nudge mode, as one JSON document (docs/attention.md#the-summary-api). -// -// 🔴 IT WORKS ONLY WITH A PERSONAL TOKEN IN envToken, like `flow …`: the route -// (api.requireAPIToken) refuses the shared hook secret, because the summary is -// one person's queue and the hook secret names nobody. A read-only token is -// enough. A poller with its own token keeps it in its own file and passes -// --env-file, so that token never has to live in the default env file. - -const attentionSummaryRoute = "/api/attention/summary" - -func newAttentionCmd(a *app) *cobra.Command { - cmd := &cobra.Command{ - Use: "attention", - Short: "Read what is waiting on you (needs a personal API token; read-only is enough)", - } - cmd.AddCommand(newAttentionSummaryCmd(a)) - return cmd -} - -func newAttentionSummaryCmd(a *app) *cobra.Command { - return &cobra.Command{ - Use: "summary", - Short: "GET /api/attention/summary — what waits on you, your hosts, your nudge mode", - Long: `Print what is waiting on you — the same items as the Asks page — with each -item's title, why (the text its notification shows), age and link; each of your -mirror hosts with whether it is online and its busy and idle Claude windows; -and your chief nudge mode (off, shadow or live). The output is the server's -JSON document. "partial": true means part of the queue could not be read, so -the list may be short.`, - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { - body, err := a.call(cmd.Context(), request{method: http.MethodGet, path: attentionSummaryRoute, cred: credHook}) - if err != nil { - return err - } - return a.emit(body) - }, - } -} diff --git a/cmd/muster/attention_test.go b/cmd/muster/attention_test.go deleted file mode 100644 index 72fa2ef13..000000000 --- a/cmd/muster/attention_test.go +++ /dev/null @@ -1,71 +0,0 @@ -package main - -import ( - "encoding/json" - "net/http" - "os" - "path/filepath" - "strings" - "testing" -) - -// summaryBody is a summary as the server writes it (compact, one line). -const summaryBody = `{"ts":1800000000,"nudge_mode":"live","partial":false,` + - `"hosts":[{"host":"host-a","online":true,"status_report":true,"busy":2,"idle":1}],` + - `"items":[{"key":"ask-5","kind":"ask","host":"","title":"chief","why":"Ship it?","since":1799999940,"age_secs":60,"offline":false,"path":"/attention/open/ask-5","url":""}]}` - -// TestAttentionSummaryReadsItsTokenFromItsOwnEnvFile is the status bar's call: -// no credential flags, the URL and a personal token in a DEDICATED env file -// (--env-file), so the bar's token never has to live in the default one. The -// request is GET /api/attention/summary bearing that token; stdout is the -// server's document (indented), and the token is printed nowhere. -func TestAttentionSummaryReadsItsTokenFromItsOwnEnvFile(t *testing.T) { - h := newHarness(t) - h.json("GET /api/attention/summary", http.StatusOK, summaryBody) - const barToken = "mst_bar-token-from-its-own-file-never-printed" - envFile := filepath.Join(t.TempDir(), "bar.env") - if err := os.WriteFile(envFile, []byte("MUSTER_API_URL="+h.srv.URL+"\nMUSTER_TOKEN="+barToken+"\n"), 0o600); err != nil { - t.Fatal(err) - } - got := h.runCLINoCredFlags(nil, envFile, "attention", "summary") - if got.code != exitOK { - t.Fatalf("exit %d, stderr %q", got.code, got.stderr) - } - reqs := h.nonHealthRequests() - if len(reqs) != 1 || reqs[0].method != http.MethodGet || reqs[0].uri != "/api/attention/summary" || reqs[0].auth != "Bearer "+barToken { - t.Fatalf("requests %+v; want one GET /api/attention/summary bearing the env file's token", reqs) - } - if mustCompact(t, got.stdout) != mustCompact(t, summaryBody) { - t.Fatalf("stdout is not the server's document:\n%s", got.stdout) - } - if !strings.Contains(got.stdout, "\n \"hosts\": [") { - t.Errorf("stdout is not indented: %q", got.stdout) - } - if strings.Contains(got.stdout+got.stderr, barToken) { - t.Fatal("the token was printed") - } -} - -func mustCompact(t *testing.T, s string) string { - t.Helper() - var v any - if err := json.Unmarshal([]byte(s), &v); err != nil { - t.Fatal(err) - } - b, _ := json.Marshal(v) - return string(b) -} - -// TestAttentionSummaryAuthFailureExitsThree: a 401 (a hook secret, an unknown -// or revoked token) and a 403 are exit 3, the auth code a poller branches on -// to show "?" rather than "nothing waits"; nothing is written to stdout. -func TestAttentionSummaryAuthFailureExitsThree(t *testing.T) { - for _, code := range []int{http.StatusUnauthorized, http.StatusForbidden} { - h := newHarness(t) - h.json("GET /api/attention/summary", code, `{"error":"invalid or revoked API token"}`) - got := h.runCLI("attention", "summary") - if got.code != exitAuth || got.stdout != "" { - t.Errorf("HTTP %d: exit %d stdout %q; want exit %d and no stdout", code, got.code, got.stdout, exitAuth) - } - } -} diff --git a/cmd/muster/output_test.go b/cmd/muster/output_test.go index 0ed853e41..55ae1306b 100644 --- a/cmd/muster/output_test.go +++ b/cmd/muster/output_test.go @@ -181,7 +181,6 @@ func TestStdoutIsIndentedJSONForEveryVerb(t *testing.T) { h.json("POST /api/plugins/messages/6/retry", http.StatusAccepted, `{"id":6,"status":"queued","nested":{"a":1}}`) h.json("POST /api/chief/replace", http.StatusOK, `{"deleted":null,"created":true,"nested":{"a":1}}`) h.json("GET /api/doctor", http.StatusOK, `{"hook_token_armed":true,"push":true,"dictation":true,"mirror":{"enabled":true,"hosts":[{"host":"alpha","token":true,"online":true}]}}`) - h.json("GET /api/attention/summary", http.StatusOK, `{"ts":1,"nudge_mode":"off","partial":false,"hosts":[],"items":[],"nested":{"a":1}}`) got := h.runCLI(w.args...) if got.code != exitOK { diff --git a/cmd/muster/root.go b/cmd/muster/root.go index c5f93cac2..9905fbd2d 100644 --- a/cmd/muster/root.go +++ b/cmd/muster/root.go @@ -163,6 +163,6 @@ func newRootCmd(a *app) *cobra.Command { root.PersistentFlags().StringVar(&a.flagEnvFile, "env-file", defaultEnvFile, "KEY=VALUE config file") root.PersistentFlags().DurationVar(&a.flagTimeout, "timeout", rootDefaultTimeout, "per-request timeout") - root.AddCommand(newTaskCmd(a), newAgentCmd(a), newChiefCmd(a), newTokenCmd(a), newWhoAmICmd(a), newFlowCmd(a), newRouteCmd(a), newPluginsCmd(a), newDoctorCmd(a), newAttentionCmd(a)) + root.AddCommand(newTaskCmd(a), newAgentCmd(a), newChiefCmd(a), newTokenCmd(a), newWhoAmICmd(a), newFlowCmd(a), newRouteCmd(a), newPluginsCmd(a), newDoctorCmd(a)) return root } diff --git a/cmd/muster/token_seam_test.go b/cmd/muster/token_seam_test.go index 8eff371c1..1cbd26db3 100644 --- a/cmd/muster/token_seam_test.go +++ b/cmd/muster/token_seam_test.go @@ -53,11 +53,11 @@ func TestSeamPersonalTokensThroughTheRealServer(t *testing.T) { // is no CLI or API mint — so this test mints its two through the store, // as that page does. asUser := authz.WithPrincipal(ctx, authz.Principal{UserID: uid, Role: authz.RoleMember, Via: authz.ViaSession}) - first, _, err := apitokens.NewPG(pool).Create(asUser, "browser-minted", 0, false) + first, _, err := apitokens.NewPG(pool).Create(asUser, "browser-minted", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } - secondTok, secondRec, err := apitokens.NewPG(pool).Create(asUser, "laptop", 0, false) + secondTok, secondRec, err := apitokens.NewPG(pool).Create(asUser, "laptop", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } diff --git a/cmd/muster/tree_test.go b/cmd/muster/tree_test.go index ff29d17da..01f618c60 100644 --- a/cmd/muster/tree_test.go +++ b/cmd/muster/tree_test.go @@ -44,7 +44,7 @@ import ( // status/comment, agent ls/resolve/messages, agent task get/comment/status, and // chief ask. If a thirteenth is genuinely wanted, change this number, add its // ledger row, and say so in the commit — do not discover it in a diff. -const wantVerbCount = 24 +const wantVerbCount = 23 // minRunnableCommands is a hardcoded floor BELOW wantVerbCount. It exists so // that a tree wired to NOTHING cannot pass this file vacuously: a tree that lost @@ -101,8 +101,6 @@ var wiring = []struct { // serves plain http, so `server` would FAIL; it is skipped here, as are the // checks that need a sign-in. What this row pins is the route. {"doctor", []string{"doctor", "--skip", "server,manifest,sign-in,push,dictation"}, "GET /api/doctor"}, - // The 24th verb: the attention summary a status bar polls (attention.go). - {"attention summary", []string{"attention", "summary"}, "GET /api/attention/summary"}, } // runnableCommands walks the tree and returns the full path of every command @@ -212,7 +210,6 @@ func TestEveryCommandReachesItsRoute(t *testing.T) { h.json("POST /api/chief/replace", http.StatusOK, `{"deleted":null,"created":true,"agent":{"id":2,"name":"chief"}}`) h.json("GET /api/doctor", http.StatusOK, `{"auth_mode":"password","hook_token_armed":true,`+ `"push":true,"dictation":true,"mirror":{"enabled":true,"hosts":[{"host":"alpha","token":true,"online":true}]}}`) - h.json("GET /api/attention/summary", http.StatusOK, `{"ts":1,"nudge_mode":"off","partial":false,"hosts":[],"items":[]}`) got := h.runCLI(w.args...) if got.code != exitOK { diff --git a/docs/attention.md b/docs/attention.md index d94414779..24d21ceaa 100644 --- a/docs/attention.md +++ b/docs/attention.md @@ -605,13 +605,12 @@ reply draft decided outside muster is not cleared by this (it is not an ask). `GET /api/attention/summary` is what a status bar polls: everything waiting on you, your mirror hosts' busy and idle Claude windows, and your -[chief nudge](chief-nudges.md) mode, in one JSON document. From the CLI it is -`muster attention summary` ([cli.md](cli.md)), which prints the same document. +[chief nudge](chief-nudges.md) mode, in one JSON document, over HTTPS. - **Credential:** a [personal API token](auth.md) only (the hook token and a browser session are refused with 401). It reads as the token's owner, and - nothing of another user's is ever in it. A **read-only** token is enough, and - is the one to give a poller. + nothing of another user's is ever in it. Give a poller a **bar summary + only** token ([auth.md](auth.md)): it reaches this route and nothing else. - **No stream, no ETag.** Poll it; the body is small. It is never cached (`Cache-Control: no-store`). - **What "waiting" means:** exactly the items of the Asks page above — the @@ -629,7 +628,7 @@ you, your mirror hosts' busy and idle Claude windows, and your "items": [ {"key": "ask-5", "kind": "ask", "host": "", "title": "chief", "why": "Ship it?", "since": 1799999940, "age_secs": 60, "offline": false, - "path": "/attention/open/ask-5", "url": "https://muster.example.test/attention/open/ask-5"} + "url": "https://muster.example.test/attention/open/ask-5"} ] } ``` @@ -647,6 +646,6 @@ you, your mirror hosts' busy and idle Claude windows, and your | `items[].title`, `items[].why` | What its notification says: the source as a title, and the first line of the notification body. Secrets are redacted, a waiting session is `Waiting: · @ `, a reply draft is who and where only. Never Claude's last words, an ask's options or its suggested replies. | | `items[].since`, `items[].age_secs` | When it started waiting (Unix seconds) and `ts` minus that; `0` and `0` when unknown. | | `items[].offline` | A session item whose host dropped within the last 5 minutes, shown as last seen. | -| `items[].path`, `items[].url` | Where to act on it: the [tap route](push.md) `/attention/open/` for an ask or a session (resolved when opened), the item's own page for a draft or a task. `url` is `path` on `MUSTER_PUBLIC_URL`, `""` when that is not set. | +| `items[].url` | Where to act on it, on `MUSTER_PUBLIC_URL`: the [tap route](push.md) `/attention/open/` for an ask or a session (resolved when opened), the item's own page for a draft or a task. **`""` when `MUSTER_PUBLIC_URL` is not set**: the summary names no origin it was not told. | Fields may be added; none is renamed or removed. diff --git a/docs/auth.md b/docs/auth.md index f69fb927f..3727ca6bb 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -287,15 +287,22 @@ Two endpoints help with personal tokens: - `GET /api/tokens` and `DELETE /api/tokens/{id}` manage personal tokens. They accept a personal token only. -**Read-only tokens.** Tick **Read-only** when you mint a token and it may only -read: a `GET` or `HEAD` request is admitted as usual, and any other method -(`POST`, `PUT`, `PATCH`, `DELETE`) on any route a personal token reaches is -refused **403** before it runs. The check is by method, so it covers routes -added later. The token page and `muster token ls` (`"readOnly": true`) show -each token's scope, and `muster whoami` shows the presented token's. A token -minted before read-only tokens existed is full access, as is one minted with -the box left unticked. Give a poller (a status bar reading -[the attention summary](attention.md#the-summary-api)) a read-only token. +**Token scopes.** When you mint a token you choose its access: + +- **Full access** (the default): every route a personal token reaches, as + before scopes existed. Every token minted before them is full access. +- **Bar summary only**: [`GET /api/attention/summary`](attention.md#the-summary-api) + (and `HEAD` on it) and **nothing else**. Every other route a personal token + reaches answers **403** before it runs: the task, agent and flow APIs, the + token API, and `GET /api/whoami` too (the summary's own 200, 401 or 403 + says whether the token works). Give a status bar this kind. + +The allowed routes are one list in `internal/api/api_tokens.go` +(`tokenScopeRoutes`), checked where every personal token is admitted, so a +route added later is refused to a bar-summary token until someone adds it +there. The token page and `muster token ls` (`"scope": "full"` or +`"summary"`) show each token's scope, and `muster whoami` shows the +presented (full) token's. From the CLI these are `muster whoami`, `muster token ls` and `muster token revoke ` ([cli.md](cli.md)). diff --git a/docs/cli.md b/docs/cli.md index df31ac871..adce34598 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -18,7 +18,6 @@ The verb set is pinned in two places: | agents | `agent ls`, `agent messages`, `agent resolve`, `chief ask`, `chief replace` ([replacing the chief](chief.md#replacing-the-chief-on-another-kind)) | | browser flows | `flow enqueue`, `flow ls`, `flow status`, `flow cancel` | | credentials | `token ls`, `token revoke`, `whoami` | -| attention | `attention summary` (what is waiting on you, for a status bar: [the summary API](attention.md#the-summary-api)) | | session routing | `route reply` (a mirrored session's answer to a [routed message](plugins.md#routing-a-message-to-a-session)) | | plugins | `plugins retry ` ([retry an unconfirmed message](plugins.md#retrying-an-unconfirmed-message)) | | setup | `doctor` ([below](#muster-doctor)) | diff --git a/docs/schema.md b/docs/schema.md index 331bb0b51..1e7a09fee 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -94,7 +94,7 @@ buys only a word. | `0064` | `chief_nudges`: one row per [stall-nudge](chief-nudges.md) decision — the session (`session_kind` `mirror`/`agent`, `session_key`, its window, or its agent and `chat_session_id` — the one conversation of the agent that stalled), the idle streak it was taken on (`streak_key`, unique per owner and session: the sweep's claim), the decision (`nudge`, `ask`, `leave`, `invalid`), the nudge's message and the quoted owner message (`quote_id`, `quote_at`, `quote_text`) and the action it covers, the ask's question and options, the chief's reason, the `mode` (`shadow`/`live`; a shadow row can hold no ask and no queued message, by CHECK), the `outcome` (a live state, or `suppressed:`), the live ask and queued message it raised, and the owner's relayed answer. `send_queue.chief_nudge_id` marks a queued nudge or relay, gated by its decision rather than a sign-in (never together with `route_id`). Owned, under row-level security, and re-applied by `db.RLSOn`. Rollback: in the file's header. | | `0065` | `agent_focus.cwd`: the working directory the session's transcript tail last named when its focus was written, cleaned and absolute, `''` for unknown (`agent_focus_cwd_check`: empty, or starting with `/` and at most 512 characters). Read only by the fleet report's mail door, to decide whether a session is under `MUSTER_FLEET_REPORT_PATHS` ([focus](focus.md#the-working-directory)); never rendered or sent. The column inherits `agent_focus`'s ownership and row-level security; no policy change, not re-applied by `db.RLSOn`. Must apply after `0064`. Rollback: in the file's header. | | `0067` | `connectors.auth_scheme` gains `postgres`; `connectors.settings` (JSONB, default `{}`): a [postgres connector](connectors.md#postgres-connectors)'s non-secret settings — `database`, `sslmode`, `require_tls`, `statement_timeout_ms`, `max_rows`, `max_bytes` — and `{}` for every other scheme (`connectors_settings_scheme`); `connector_audit.kind` gains `query` (one row per agent query: the SQL's fingerprint and first 120 characters, rows, duration, outcome class; never a parameter value or the password). The columns inherit their tables' ownership and row-level security; not re-applied by `db.RLSOn`. Rollback: in the file's header (delete postgres connectors first). | -| `0068` | `api_tokens.read_only` (default `false`): a [read-only personal token](auth.md) may only make `GET` and `HEAD` requests, enforced at the token door. Every token minted before it is full access. Inherits the table's row-level security; not re-applied by `db.RLSOn`. 🔴 Rollback: an older image would admit a read-only token with full access, so revoke every live read-only token first (in the file's header). | +| `0068` | `api_tokens.scope` (default `full`, `api_tokens_scope_check`: `full` or `summary`): a [personal token's scope](auth.md) — `full` reaches every route a personal token reaches, `summary` only `GET /api/attention/summary`, enforced at the token door. Every token minted before it is `full`. Inherits the table's row-level security; not re-applied by `db.RLSOn`. 🔴 Rollback: an older image would admit a `summary` token with full access, so revoke every live non-`full` token first (in the file's header). | To change the schema, add a new numbered file. diff --git a/internal/api/api_tokens.go b/internal/api/api_tokens.go index e17974c64..de947faea 100644 --- a/internal/api/api_tokens.go +++ b/internal/api/api_tokens.go @@ -43,11 +43,14 @@ import ( // gotrue session tier's DisableSweepInterval, which bounds how long an already // OPEN stream survives a disable. No route a token can open streams. // -// 🔴 A READ-ONLY TOKEN (migration 0068) IS REFUSED AT THE SAME DOOR, BY METHOD. -// admitAPIToken lets it through only on readOnlyTokenMethods (GET, HEAD) and -// answers anything else 403 ReadOnlyTokenRefusal before the handler runs. It is -// an allowlist of METHODS, not of routes: a route added later is covered the -// day it is registered, and a write route is a write whatever its path. +// 🔴 A NARROWER-SCOPED TOKEN (migration 0068) IS REFUSED AT THE SAME DOOR, BY +// ROUTE. Every wrapper that takes a token goes through admitAPIToken, and it +// admits a token whose scope is not apitokens.ScopeFull only on the mux +// patterns tokenScopeRoutes lists for that scope — ONE table, here — and +// answers anything else 403 ScopedTokenRefusal before the handler runs. It is +// an ALLOWLIST: a route added later is refused to a narrower token the day it +// is registered, until someone writes it into the table. A scope the table +// does not name reaches nothing. // apiTokensPath is the token-management API (list, revoke; requireAPIToken). const apiTokensPath = "/api/tokens" @@ -138,18 +141,33 @@ func (s *Server) requireAPIToken(next http.HandlerFunc) http.HandlerFunc { // nothing about which. const APITokenRefusal = "invalid or revoked API token" -// ReadOnlyTokenRefusal is the 403 body a read-only token gets on any method but -// readOnlyTokenMethods. -const ReadOnlyTokenRefusal = "this API token is read-only: it may only make GET and HEAD requests (mint a full-access token at " + accountTokensPath + " to change anything)" +// ScopedTokenRefusal is the 403 body a narrower-scoped token gets on a route +// its scope does not reach. +const ScopedTokenRefusal = "this API token's scope does not reach this route: a bar-summary token can only read GET " + + attentionSummaryPath + " (mint a full-access token at " + accountTokensPath + " for anything else)" -// readOnlyTokenMethods is every method a read-only token may use. An -// allowlist: a method not named here (POST, PUT, PATCH, DELETE, and any other) -// is refused. -var readOnlyTokenMethods = map[string]bool{http.MethodGet: true, http.MethodHead: true} +// tokenScopeRoutes is every mux pattern each narrower token scope may reach: +// THE one allowlist (see the file header). The key is the pattern the route +// is registered with (http.Request.Pattern), so a GET pattern also admits +// HEAD — net/http serves HEAD through it — and nothing else. ScopeFull is not +// in it: a full token is not limited. +// +// ⚠ GET /api/whoami IS DELIBERATELY ABSENT. A bar token does not need to know +// who it is; the summary's own 200, 401 or 403 says whether the token works. +var tokenScopeRoutes = map[string]map[string]bool{ + apitokens.ScopeSummary: {"GET " + attentionSummaryPath: true}, +} + +// tokenScopeAdmits reports whether scope reaches the route r was matched to. +// An unknown scope reaches nothing, and a request with no matched pattern (a +// handler called directly) is refused to every scope but ScopeFull. +func tokenScopeAdmits(scope string, r *http.Request) bool { + return scope == apitokens.ScopeFull || tokenScopeRoutes[scope][r.Pattern] +} // admitAPIToken authenticates a personal token and admits the request as its -// owner, or refuses it 401 — or 403 for a read-only token on a method it may -// not use. See the file header. +// owner, or refuses it 401 — or 403 for a narrower-scoped token on a route its +// scope does not reach. See the file header. // // ⚠ NO FAILURE LIMITER, ON PURPOSE: a 256-bit token cannot be guessed, so a // limiter would only turn 401 into 429. The one gate before the lookup is @@ -176,8 +194,8 @@ func (s *Server) admitAPIToken(w http.ResponseWriter, r *http.Request, next http s.writeJSON(w, http.StatusServiceUnavailable, map[string]any{"error": "could not check the API token"}) return } - if t.ReadOnly && !readOnlyTokenMethods[r.Method] { - s.writeJSON(w, http.StatusForbidden, map[string]any{"error": ReadOnlyTokenRefusal}) + if !tokenScopeAdmits(t.Scope, r) { + s.writeJSON(w, http.StatusForbidden, map[string]any{"error": ScopedTokenRefusal}) return } ctx := context.WithValue(r.Context(), apiTokenCtxKey{}, t) @@ -201,7 +219,7 @@ func (s *Server) handleWhoAmI(w http.ResponseWriter, r *http.Request) { p := authz.MustFrom(r.Context()) out := map[string]any{"userId": p.UserID, "role": p.Role, "via": p.Via} if t, ok := apiTokenFromContext(r.Context()); ok { - out["token"] = map[string]any{"id": t.ID, "name": t.Name, "createdAt": t.CreatedAt, "readOnly": t.ReadOnly} + out["token"] = map[string]any{"id": t.ID, "name": t.Name, "createdAt": t.CreatedAt, "scope": t.Scope} } if actor, ok := serviceActorFromContext(r.Context()); ok { out["actor"] = actor @@ -256,10 +274,9 @@ func (s *Server) handleAccountTokenCreate(w http.ResponseWriter, r *http.Request s.renderAccountTokens(w, r, http.StatusInternalServerError, "", "The token could not be created.") return } - // The checkbox: absent (unchecked) is a full-access token, as before 0068. - readOnly := r.PostFormValue("read_only") != "" - tok, _, err := s.ext.APITokens.Create(r.Context(), r.PostFormValue("name"), iat, readOnly) - if errors.Is(err, apitokens.ErrNameTooLong) { + // The scope choice: absent is a full-access token, as before 0068. + tok, _, err := s.ext.APITokens.Create(r.Context(), r.PostFormValue("name"), iat, r.PostFormValue("scope")) + if errors.Is(err, apitokens.ErrNameTooLong) || errors.Is(err, apitokens.ErrBadScope) { s.renderAccountTokens(w, r, http.StatusBadRequest, "", err.Error()) return } @@ -299,6 +316,12 @@ func (s *Server) handleAccountTokenRevoke(w http.ResponseWriter, r *http.Request http.Redirect(w, r, accountTokensPath, http.StatusSeeOther) } +// The scopes as the token page names them. +const ( + scopeLabelFull = "Full access" + scopeLabelSummary = "Bar summary only" +) + // renderAccountTokens writes the token page. minted, when set, is the token // just created — the only time it is ever rendered. func (s *Server) renderAccountTokens(w http.ResponseWriter, r *http.Request, code int, minted, problem string) { @@ -329,8 +352,13 @@ func (s *Server) renderAccountTokens(w http.ResponseWriter, r *http.Request, cod - +
    +Access + + +
      @@ -343,9 +371,9 @@ Read-only: it can only read (GET), never change anything if name == "" { name = "(unnamed)" } - scope := "full access" - if t.ReadOnly { - scope = "read-only" + scope := scopeLabelFull + if t.Scope != apitokens.ScopeFull { + scope = scopeLabelSummary } used := "never used" if t.LastUsedAt != nil { @@ -353,7 +381,7 @@ Read-only: it can only read (GET), never change anything } b.WriteString(`
    • ` + html.EscapeString(name) + ` -` + scope + ` · created ` + t.CreatedAt.UTC().Format("2006-01-02 15:04") + ` UTC · ` + used + ` +` + scope + ` · created ` + t.CreatedAt.UTC().Format("2006-01-02 15:04") + ` UTC · ` + used + ` `) if t.Revoked() { b.WriteString(`revoked`) diff --git a/internal/api/api_tokens_readonly_test.go b/internal/api/api_tokens_readonly_test.go deleted file mode 100644 index 69d6705c9..000000000 --- a/internal/api/api_tokens_readonly_test.go +++ /dev/null @@ -1,179 +0,0 @@ -package api - -import ( - "context" - "encoding/json" - "io" - "log" - "net/http" - "net/http/httptest" - "net/url" - "strings" - "sync" - "testing" - - "github.com/ZacxDev/muster/internal/apitokens" -) - -// scopedTokens is fakeAPITokens with a read-only token beside the full one, -// and a List and Revoke that COUNT, so a test can tell "refused at the door" -// from "the handler ran and refused". -type scopedTokens struct { - *fakeAPITokens - mu sync.Mutex - revokes int - lists int -} - -func (s *scopedTokens) addReadOnly(t *testing.T, uid int64) string { - t.Helper() - tok := s.add(t, uid) - s.fakeAPITokens.mu.Lock() - defer s.fakeAPITokens.mu.Unlock() - rec := s.byTok[tok] - rec.ReadOnly = true - s.byTok[tok] = rec - return tok -} - -func (s *scopedTokens) List(context.Context) ([]apitokens.Token, error) { - s.mu.Lock() - defer s.mu.Unlock() - s.lists++ - return []apitokens.Token{}, nil -} - -func (s *scopedTokens) Revoke(context.Context, int64) (bool, bool, error) { - s.mu.Lock() - defer s.mu.Unlock() - s.revokes++ - return true, true, nil -} - -func (s *scopedTokens) counts() (int, int) { - s.mu.Lock() - defer s.mu.Unlock() - return s.lists, s.revokes -} - -// TestAReadOnlyTokenMayOnlyRead: a read-only token reaches GET routes (whoami, -// the token list, the attention summary) and is refused 403, with the -// read-only sentence, on every write — a token revoke (DELETE), a flow enqueue -// and cancel and a task create (POST), a task status (PATCH) — BEFORE the -// handler runs (the store's Revoke is never called). The same requests with -// the same owner's FULL token are not refused (the control: the 403 is the -// scope, not the route), and the full token's revoke reaches the store. -func TestAReadOnlyTokenMayOnlyRead(t *testing.T) { - f := &scopedTokens{fakeAPITokens: newFakeAPITokens()} - full := f.add(t, 7) - ro := f.addReadOnly(t, 7) - s := New(nil, AuthConfig{HookToken: tokTestHook}, log.New(io.Discard, "", 0)) - s.UseExtensions(Extensions{APITokens: f, Users: stubUsers{}}) - mux := http.NewServeMux() - s.registerAll(mux) - do := func(method, path, tok string) *httptest.ResponseRecorder { - r := httptest.NewRequest(method, path, strings.NewReader(`{}`)) - r.Header.Set("Authorization", "Bearer "+tok) - r.Header.Set("Content-Type", "application/json") - rec := httptest.NewRecorder() - mux.ServeHTTP(rec, r) - return rec - } - for _, path := range []string{"/api/whoami", apiTokensPath, attentionSummaryPath} { - if rec := do(http.MethodGet, path, ro); rec.Code != http.StatusOK { - t.Errorf("GET %s with a read-only token: %d %s; want 200", path, rec.Code, truncate(rec.Body.String(), 200)) - } - if rec := do(http.MethodHead, path, ro); rec.Code == http.StatusForbidden { - t.Errorf("HEAD %s with a read-only token: 403; HEAD is a read", path) - } - } - _, body := whoami(t, mux, ro, "") - if tk, _ := body["token"].(map[string]any); tk == nil || tk["readOnly"] != true { - t.Errorf("whoami does not say the token is read-only: %v", body) - } - writes := []struct{ method, path string }{ - {http.MethodDelete, apiTokensPath + "/1"}, - {http.MethodPost, "/api/flows/jobs"}, - {http.MethodPost, "/api/flows/jobs/1/cancel"}, - {http.MethodPost, "/api/tasks"}, - {http.MethodPatch, "/api/tasks/1/status"}, - } - for _, w := range writes { - rec := do(w.method, w.path, ro) - var out map[string]any - _ = json.Unmarshal(rec.Body.Bytes(), &out) - if rec.Code != http.StatusForbidden || out["error"] != ReadOnlyTokenRefusal { - t.Errorf("%s %s with a read-only token: %d %s; want 403 %q", w.method, w.path, rec.Code, truncate(rec.Body.String(), 200), ReadOnlyTokenRefusal) - } - } - if _, revokes := f.counts(); revokes != 0 { - t.Fatalf("a read-only token's DELETE reached the store's Revoke %d time(s); the door must refuse it first", revokes) - } - for _, w := range writes { - if rec := do(w.method, w.path, full); rec.Code == http.StatusForbidden { - t.Errorf("CONTROL: %s %s with the FULL token: 403 %s; a full token is unchanged", w.method, w.path, truncate(rec.Body.String(), 200)) - } - } - if _, revokes := f.counts(); revokes != 1 { - t.Fatalf("CONTROL: the full token's DELETE reached Revoke %d time(s); want 1", revokes) - } -} - -// mintScopeTokens records the scope each mint asked for, and lists one token -// of each scope. -type mintScopeTokens struct { - apitokens.Store - got *[]bool -} - -func (m mintScopeTokens) Create(_ context.Context, _ string, _ int64, readOnly bool) (string, apitokens.Token, error) { - *m.got = append(*m.got, readOnly) - tok, _ := apitokens.New() - return tok, apitokens.Token{ID: 1, ReadOnly: readOnly}, nil -} - -func (mintScopeTokens) List(context.Context) ([]apitokens.Token, error) { - return []apitokens.Token{{ID: 2, Name: "bar", ReadOnly: true}, {ID: 3, Name: "cli"}}, nil -} - -// TestTheMintPageOffersAReadOnlyToken: the page's form has a read-only -// checkbox, UNCHECKED by default (a mint that leaves it alone is full access, -// as before); a mint with it ticked asks the store for a read-only token, one -// without for a full one; and the list says each token's scope. -func TestTheMintPageOffersAReadOnlyToken(t *testing.T) { - var got []bool - s := New(nil, AuthConfig{HookToken: tokTestHook, UIPassword: "mint-scope-operator-pw"}, log.New(io.Discard, "", 0)) - s.UseExtensions(Extensions{APITokens: mintScopeTokens{got: &got}, Users: stubUsers{}}) - mux := http.NewServeMux() - s.registerAll(mux) - do := func(method, body string) string { - r := httptest.NewRequest(method, accountTokensPath, strings.NewReader(body)) - r.Host = "muster.test" - r.Header.Set("Cookie", sessionCookieFor(s)) - r.Header.Set("Origin", "http://muster.test") - if body != "" { - r.Header.Set("Content-Type", "application/x-www-form-urlencoded") - } - rec := httptest.NewRecorder() - mux.ServeHTTP(rec, r) - if rec.Code != http.StatusOK { - t.Fatalf("%s %s: %d", method, accountTokensPath, rec.Code) - } - return rec.Body.String() - } - page := do(http.MethodGet, "") - const box = `` - if !strings.Contains(page, box) { - t.Fatalf("the token page has no unchecked read-only checkbox %q:\n%s", box, truncate(page, 1200)) - } - for _, want := range []string{`data-token-scope="read-only">read-only · created`, `data-token-scope="full access">full access · created`} { - if !strings.Contains(page, want) { - t.Errorf("the token list does not say %q", want) - } - } - do(http.MethodPost, url.Values{"name": {"bar"}, "read_only": {"1"}}.Encode()) - do(http.MethodPost, url.Values{"name": {"cli"}}.Encode()) - if len(got) != 2 || !got[0] || got[1] { - t.Fatalf("the store was asked for scopes %v; want [read-only, full]", got) - } -} diff --git a/internal/api/api_tokens_scope_test.go b/internal/api/api_tokens_scope_test.go new file mode 100644 index 000000000..7b47577e9 --- /dev/null +++ b/internal/api/api_tokens_scope_test.go @@ -0,0 +1,272 @@ +package api + +import ( + "context" + "encoding/json" + "io" + "log" + "net/http" + "net/http/httptest" + "net/url" + "reflect" + "strings" + "sync" + "testing" + + "github.com/ZacxDev/muster/internal/apitokens" +) + +// scopedTokens is fakeAPITokens with tokens of a chosen scope beside the full +// one, and a List and Revoke that COUNT, so a test can tell "refused at the +// door" from "the handler ran and refused". +type scopedTokens struct { + *fakeAPITokens + mu sync.Mutex + revokes int + lists int +} + +func newScopedTokens() *scopedTokens { return &scopedTokens{fakeAPITokens: newFakeAPITokens()} } + +// addScoped adds a token of scope for uid (add, the embedded one, adds a full +// token; addScoped names the scope either way, so each test says it). +func (s *scopedTokens) addScoped(t *testing.T, uid int64, scope string) string { + t.Helper() + tok := s.add(t, uid) + s.fakeAPITokens.mu.Lock() + defer s.fakeAPITokens.mu.Unlock() + rec := s.byTok[tok] + rec.Scope = scope + s.byTok[tok] = rec + return tok +} + +func (s *scopedTokens) List(context.Context) ([]apitokens.Token, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.lists++ + return []apitokens.Token{}, nil +} + +func (s *scopedTokens) Revoke(context.Context, int64) (bool, bool, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.revokes++ + return true, true, nil +} + +func (s *scopedTokens) counts() (lists, revokes int) { + s.mu.Lock() + defer s.mu.Unlock() + return s.lists, s.revokes +} + +// scopeTestMux is the real route set (registerAll) over f. +func scopeTestMux(t *testing.T, f apitokens.Store) http.Handler { + t.Helper() + s := New(nil, AuthConfig{HookToken: tokTestHook}, log.New(io.Discard, "", 0)) + s.UseExtensions(Extensions{APITokens: f, Users: stubUsers{}}) + mux := http.NewServeMux() + s.registerAll(mux) + return mux +} + +func bearerDo(h http.Handler, method, path, tok string) *httptest.ResponseRecorder { + r := httptest.NewRequest(method, path, strings.NewReader(`{}`)) + r.Header.Set("Authorization", "Bearer "+tok) + r.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + h.ServeHTTP(rec, r) + return rec +} + +// bearerReach is bearerDo for a request the door may ADMIT into a handler this +// fixture leaves unwired (no agents store, say): the handler's nil-store panic +// is reported as -1, "the handler ran" — admitted, which is what the full +// token's control wants and what a refused token must never get. Without it, +// a broken door panics the test binary and hides every later result. +func bearerReach(h http.Handler, method, path, tok string) (code int, body string) { + defer func() { + if recover() != nil { + code, body = -1, "(the handler ran)" + } + }() + rec := bearerDo(h, method, path, tok) + return rec.Code, rec.Body.String() +} + +// summaryTokenRefused is a spread of routes a personal token reaches that a +// bar-summary token must NOT: reads on the hook-tier door (an agent's +// messages, the task list, a task), the token API's own reads (the list, +// whoami), the flow API, and writes of every method. +var summaryTokenRefused = []struct{ method, path string }{ + {http.MethodGet, "/api/agents/chief/messages"}, + {http.MethodGet, "/api/agents"}, + {http.MethodGet, "/api/tasks"}, + {http.MethodGet, "/api/tasks/1"}, + {http.MethodGet, "/api/flows/jobs"}, + {http.MethodGet, "/api/whoami"}, + {http.MethodGet, apiTokensPath}, + {http.MethodDelete, apiTokensPath + "/1"}, + {http.MethodPost, "/api/tasks"}, + {http.MethodPatch, "/api/tasks/1/status"}, + {http.MethodPost, "/api/flows/jobs"}, + {http.MethodPost, "/api/agents/chief/messages"}, +} + +// TestABarSummaryTokenReachesTheSummaryAndNothingElse: a summary-scoped token +// gets GET (and HEAD) /api/attention/summary, and is refused 403 with the +// scope sentence on every other route in summaryTokenRefused — GETs on the +// hook-tier door included — BEFORE the handler runs (the store's List and +// Revoke are never called). The same owner's FULL token on the same requests +// is not refused (the 403 is the scope, not the route), and its list and +// revoke reach the store. +func TestABarSummaryTokenReachesTheSummaryAndNothingElse(t *testing.T) { + f := newScopedTokens() + full := f.addScoped(t, 7, apitokens.ScopeFull) + bar := f.addScoped(t, 7, apitokens.ScopeSummary) + h := scopeTestMux(t, f) + if rec := bearerDo(h, http.MethodGet, attentionSummaryPath, bar); rec.Code != http.StatusOK { + t.Fatalf("GET %s with a summary token: %d %s; want 200", attentionSummaryPath, rec.Code, rec.Body) + } + if rec := bearerDo(h, http.MethodHead, attentionSummaryPath, bar); rec.Code != http.StatusOK { + t.Errorf("HEAD %s with a summary token: %d; want 200", attentionSummaryPath, rec.Code) + } + for _, c := range summaryTokenRefused { + code, body := bearerReach(h, c.method, c.path, bar) + var out map[string]any + _ = json.Unmarshal([]byte(body), &out) + if code != http.StatusForbidden || out["error"] != ScopedTokenRefusal { + t.Errorf("%s %s with a summary token: %d %s; want 403 %q", c.method, c.path, code, truncate(body, 200), ScopedTokenRefusal) + } + } + if lists, revokes := f.counts(); lists != 0 || revokes != 0 { + t.Fatalf("a summary token reached the store's List %d / Revoke %d time(s); the door must refuse it first", lists, revokes) + } + for _, c := range summaryTokenRefused { + if code, body := bearerReach(h, c.method, c.path, full); code == http.StatusForbidden || code == http.StatusUnauthorized { + t.Errorf("CONTROL: %s %s with the FULL token: %d %s; a full token is unchanged", c.method, c.path, code, truncate(body, 200)) + } + } + if lists, revokes := f.counts(); lists != 1 || revokes != 1 { + t.Fatalf("CONTROL: the full token reached List %d / Revoke %d time(s); want 1 and 1", lists, revokes) + } + if rec := bearerDo(h, http.MethodGet, attentionSummaryPath, full); rec.Code != http.StatusOK { + t.Fatalf("CONTROL: the full token on the summary: %d", rec.Code) + } +} + +// TestATokenOfAnUnknownScopeReachesNothing: a scope the allowlist does not +// name — a row from a newer image, a blank — is refused everywhere, the +// summary included. The door fails closed; only ScopeFull is unlimited. +func TestATokenOfAnUnknownScopeReachesNothing(t *testing.T) { + f := newScopedTokens() + for _, scope := range []string{"", "admin", "Summary"} { + tok := f.addScoped(t, 7, scope) + h := scopeTestMux(t, f) + for _, c := range append([]struct{ method, path string }{{http.MethodGet, attentionSummaryPath}}, summaryTokenRefused[:3]...) { + if code, _ := bearerReach(h, c.method, c.path, tok); code != http.StatusForbidden { + t.Errorf("scope %q: %s %s: %d; want 403", scope, c.method, c.path, code) + } + } + } +} + +// TestTheScopeAllowlistIsExactlyTheSummaryRoute pins the WHOLE allowlist: one +// narrower scope, one route, and that route registered under exactly that +// pattern (it is in routes.golden). A pattern that drifted from the route's +// registration would lock the bar out, and a second entry is a decision this +// line makes visible. +func TestTheScopeAllowlistIsExactlyTheSummaryRoute(t *testing.T) { + want := map[string]map[string]bool{"summary": {"GET /api/attention/summary": true}} + if !reflect.DeepEqual(tokenScopeRoutes, want) { + t.Fatalf("tokenScopeRoutes = %v; want %v", tokenScopeRoutes, want) + } + golden := map[string]bool{} + for _, r := range readGolden(t) { + golden[r] = true + } + for scope, routes := range tokenScopeRoutes { + for r := range routes { + if !golden[r] { + t.Errorf("scope %q allows %q, which no route is registered as", scope, r) + } + } + } +} + +// mintScopeTokens records the scope each mint asked for, and lists one token +// of each scope. +type mintScopeTokens struct { + apitokens.Store + got *[]string +} + +func (m mintScopeTokens) Create(_ context.Context, _ string, _ int64, scope string) (string, apitokens.Token, error) { + *m.got = append(*m.got, scope) + tok, _ := apitokens.New() + return tok, apitokens.Token{ID: 1, Scope: scope}, nil +} + +func (mintScopeTokens) List(context.Context) ([]apitokens.Token, error) { + return []apitokens.Token{{ID: 2, Name: "bar", Scope: apitokens.ScopeSummary}, {ID: 3, Name: "cli", Scope: apitokens.ScopeFull}}, nil +} + +// tokenPagePost posts form to the token page as the operator's session. +func tokenPageDo(t *testing.T, h http.Handler, s *Server, method, body string) *httptest.ResponseRecorder { + t.Helper() + r := httptest.NewRequest(method, accountTokensPath, strings.NewReader(body)) + r.Host = "muster.test" + r.Header.Set("Cookie", sessionCookieFor(s)) + r.Header.Set("Origin", "http://muster.test") + if body != "" { + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + } + rec := httptest.NewRecorder() + h.ServeHTTP(rec, r) + return rec +} + +// TestTheMintPageOffersABarSummaryToken: the form's access choice is "Full +// access" (CHECKED: a mint that leaves it alone is full, as before) or "Bar +// summary only"; each choice reaches the store as its scope; and the list +// names each token's scope. +func TestTheMintPageOffersABarSummaryToken(t *testing.T) { + var got []string + s := New(nil, AuthConfig{HookToken: tokTestHook, UIPassword: "mint-scope-operator-pw"}, log.New(io.Discard, "", 0)) + s.UseExtensions(Extensions{APITokens: mintScopeTokens{got: &got}, Users: stubUsers{}}) + mux := http.NewServeMux() + s.registerAll(mux) + page := tokenPageDo(t, mux, s, http.MethodGet, "").Body.String() + for _, want := range []string{ + `` + "\nFull access", + `` + "\nBar summary only (can read the Attention summary and nothing else)", + `data-token-scope="summary">Bar summary only · created`, + `data-token-scope="full">Full access · created`, + } { + if !strings.Contains(page, want) { + t.Errorf("the token page does not carry %q", want) + } + } + for _, form := range []url.Values{{"name": {"bar"}, "scope": {"summary"}}, {"name": {"cli"}, "scope": {"full"}}, {"name": {"old"}}} { + if rec := tokenPageDo(t, mux, s, http.MethodPost, form.Encode()); rec.Code != http.StatusOK { + t.Fatalf("mint %v: %d", form, rec.Code) + } + } + if strings.Join(got, ",") != "summary,full," { + t.Fatalf("the store was asked for scopes %q; want summary, full, and none (the store's full)", got) + } +} + +// TestAMintNamingAnUnknownScopeIsABadRequest: the store's ErrBadScope reaches +// the person as a 400 and no token. +func TestAMintNamingAnUnknownScopeIsABadRequest(t *testing.T) { + s := New(nil, AuthConfig{HookToken: tokTestHook, UIPassword: "mint-scope-operator-pw"}, log.New(io.Discard, "", 0)) + s.UseExtensions(Extensions{APITokens: refusingTokens{err: apitokens.ErrBadScope}, Users: stubUsers{}}) + mux := http.NewServeMux() + s.registerAll(mux) + rec := tokenPageDo(t, mux, s, http.MethodPost, url.Values{"name": {"x"}, "scope": {"admin"}}.Encode()) + if rec.Code != http.StatusBadRequest || strings.Contains(rec.Body.String(), "minted-token") { + t.Fatalf("a mint with scope admin: %d; want 400 and no token", rec.Code) + } +} diff --git a/internal/api/api_tokens_test.go b/internal/api/api_tokens_test.go index 265c28ab5..fa97d5077 100644 --- a/internal/api/api_tokens_test.go +++ b/internal/api/api_tokens_test.go @@ -47,7 +47,7 @@ func (f *fakeAPITokens) add(t *testing.T, uid int64) string { } f.mu.Lock() defer f.mu.Unlock() - f.byTok[tok] = apitokens.Token{ID: int64(len(f.byTok) + 1), UserID: uid, Name: "t"} + f.byTok[tok] = apitokens.Token{ID: int64(len(f.byTok) + 1), UserID: uid, Name: "t", Scope: apitokens.ScopeFull} return tok } @@ -200,7 +200,7 @@ type mintCountingTokens struct { mints int } -func (m *mintCountingTokens) Create(context.Context, string, int64, bool) (string, apitokens.Token, error) { +func (m *mintCountingTokens) Create(context.Context, string, int64, string) (string, apitokens.Token, error) { m.mints++ tok, _ := apitokens.New() return tok, apitokens.Token{ID: 99, UserID: 7}, nil @@ -351,7 +351,7 @@ func TestPersonalTokenLifecycleOverHTTP(t *testing.T) { if !strings.Contains(list, aTask.Title) || strings.Contains(list, bTask.Title) { t.Fatalf("A's token's task list: %s", truncate(list, 400)) } - bTok, _, err := apitokens.NewPG(pool).Create(asUser(b), "b", 0, false) + bTok, _, err := apitokens.NewPG(pool).Create(asUser(b), "b", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } @@ -491,7 +491,7 @@ func TestAPasswordResetRevokesTheUsersAPITokens(t *testing.T) { }) ts := apitokens.NewPG(w.pool) asUser := authz.WithPrincipal(context.Background(), authz.Principal{UserID: uid, Role: authz.RoleMember, Via: authz.ViaSession}) - secret, _, err := ts.Create(asUser, "before-reset", 0, false) + secret, _, err := ts.Create(asUser, "before-reset", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } @@ -537,7 +537,7 @@ func TestSignOutEverywhereSaysItKeepsAPITokens(t *testing.T) { _, _ = w.pool.Exec(authz.System(context.Background()), `DELETE FROM api_tokens WHERE user_id = 1`) }) ts := apitokens.NewPG(w.pool) - secret, _, err := ts.Create(authz.Operator(context.Background()), "kept", 0, false) + secret, _, err := ts.Create(authz.Operator(context.Background()), "kept", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } @@ -557,7 +557,7 @@ type refusingTokens struct { err error } -func (r refusingTokens) Create(context.Context, string, int64, bool) (string, apitokens.Token, error) { +func (r refusingTokens) Create(context.Context, string, int64, string) (string, apitokens.Token, error) { return "", apitokens.Token{}, r.err } func (refusingTokens) List(context.Context) ([]apitokens.Token, error) { return nil, nil } @@ -604,7 +604,7 @@ func TestAResetWhoseSignInFailsStillSaysTheTokensWereRevoked(t *testing.T) { }) ts := apitokens.NewPG(w.pool) secret, _, err := ts.Create(authz.WithPrincipal(context.Background(), - authz.Principal{UserID: uid, Role: authz.RoleMember, Via: authz.ViaSession}), "before", 0, false) + authz.Principal{UserID: uid, Role: authz.RoleMember, Via: authz.ViaSession}), "before", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } @@ -626,7 +626,7 @@ type iatCapturingTokens struct { got *[]int64 } -func (c iatCapturingTokens) Create(_ context.Context, _ string, iat int64, _ bool) (string, apitokens.Token, error) { +func (c iatCapturingTokens) Create(_ context.Context, _ string, iat int64, _ string) (string, apitokens.Token, error) { *c.got = append(*c.got, iat) tok, _ := apitokens.New() return tok, apitokens.Token{ID: 1}, nil diff --git a/internal/api/attention.go b/internal/api/attention.go index 8624a4583..ec230fc46 100644 --- a/internal/api/attention.go +++ b/internal/api/attention.go @@ -972,7 +972,8 @@ func (s *Server) registerAttentionRoutes(mux Mux) { mux.HandleFunc("GET "+askPagePrefix+"{id}", s.requireSession(s.handleAskPage)) mux.HandleFunc("GET "+ui.AttentionBadgePath, s.requireSession(s.handleAttentionBadge)) // What a status bar polls (attention_summary.go): a PERSONAL token only, so - // it reads as that token's owner; a read-only token is enough. + // it reads as that token's owner. It is the one route a bar-summary token + // (apitokens.ScopeSummary) reaches (tokenScopeRoutes). mux.HandleFunc("GET "+attentionSummaryPath, s.requireAPIToken(s.handleAttentionSummary)) mux.HandleFunc("POST "+attentionPath+"/asks/{id}/answer", s.requireOperatorWrite(s.handleAttentionAnswerForm)) mux.HandleFunc("POST "+attentionPath+"/asks/{id}/cancel", s.requireOperatorWrite(s.handleAttentionCancelForm)) diff --git a/internal/api/attention_summary.go b/internal/api/attention_summary.go index 3391fb6f8..9f9e09716 100644 --- a/internal/api/attention_summary.go +++ b/internal/api/attention_summary.go @@ -85,10 +85,9 @@ type attentionSummaryItem struct { Since int64 `json:"since"` AgeSecs int64 `json:"age_secs"` Offline bool `json:"offline"` - // Path is where the owner acts on it, on muster's origin; URL is it made - // absolute with MUSTER_PUBLIC_URL, "" when that is not set. - Path string `json:"path"` - URL string `json:"url"` + // URL is where the owner acts on it, absolute on MUSTER_PUBLIC_URL; "" + // when that is not set (the summary names no origin it was not told). + URL string `json:"url"` } func (s *Server) handleAttentionSummary(w http.ResponseWriter, r *http.Request) { @@ -155,12 +154,13 @@ func attentionSummaryHostOf(h mirror.HostView) attentionSummaryHost { // origin without a trailing slash, "" for none. func attentionSummaryItemOf(it attentionItem, base string, now time.Time) attentionSummaryItem { out := attentionSummaryItem{Key: it.Key, Kind: it.Kind, Host: it.host, Title: textplain.Title(it.Source), - Why: firstLine(attentionPushBody(it)), Offline: it.Offline, Path: it.Link} + Why: firstLine(attentionPushBody(it)), Offline: it.Offline} if out.Host == "" { out.Host = it.card.ref.Host // an ask from a mirrored session } + path := it.Link if attentionOpenKey.MatchString(it.Key) { - out.Path = attentionOpenURL(it.Key) + path = attentionOpenURL(it.Key) } if !it.Since.IsZero() { out.Since = it.Since.Unix() @@ -168,8 +168,8 @@ func attentionSummaryItemOf(it attentionItem, base string, now time.Time) attent out.AgeSecs = age } } - if base != "" && out.Path != "" { - out.URL = base + out.Path + if base != "" && path != "" { + out.URL = base + path } return out } diff --git a/internal/api/attention_summary_pg_test.go b/internal/api/attention_summary_pg_test.go index 99abfd9bf..daf6e8c41 100644 --- a/internal/api/attention_summary_pg_test.go +++ b/internal/api/attention_summary_pg_test.go @@ -75,12 +75,14 @@ func itemKeys(items []attentionItem) []string { // none of the operator's items or hosts. func TestTheSummaryIsTheOwnersQueueAndHosts(t *testing.T) { f := newSummaryFixture(t) + f.srv.auth.PublicURL = "https://muster.example.test/" + const base = "https://muster.example.test" op := asUser(1) - opTok, _, err := apitokens.NewPG(f.pool).Create(op, "bar", 0, false) + opTok, _, err := apitokens.NewPG(f.pool).Create(op, "bar", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } - memberTok, _, err := apitokens.NewPG(f.pool).Create(asUser(f.member), "member-bar", 0, false) + memberTok, _, err := apitokens.NewPG(f.pool).Create(asUser(f.member), "member-bar", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } @@ -155,18 +157,18 @@ func TestTheSummaryIsTheOwnersQueueAndHosts(t *testing.T) { byKey[it.Key] = it } sess := sessionKey("ws", "main", "@1") - for key, want := range map[string]struct{ kind, host, title, why, path string }{ - attentionAskKey(mirrored.ID): {"ask", "ws", "Gold:1", "Use «redacted» for the mirror?", attentionOpenPath + attentionAskKey(mirrored.ID)}, - attentionAskKey(agentAsk): {"ask", "", f.op.Name, "Which bucket?", attentionOpenPath + attentionAskKey(agentAsk)}, - sess: {"session", "ws", "main:1 @ ws", "Waiting: main:1 @ ws", attentionOpenPath + sess}, - draftKey(d.ID): {"plugin", "", "Attention inbox", "Reply draft: Dana (Attention inbox)", draftLink(d.ID)}, + for key, want := range map[string]struct{ kind, host, title, why, url string }{ + attentionAskKey(mirrored.ID): {"ask", "ws", "Gold:1", "Use «redacted» for the mirror?", base + attentionOpenPath + attentionAskKey(mirrored.ID)}, + attentionAskKey(agentAsk): {"ask", "", f.op.Name, "Which bucket?", base + attentionOpenPath + attentionAskKey(agentAsk)}, + sess: {"session", "ws", "main:1 @ ws", "Waiting: main:1 @ ws", base + attentionOpenPath + sess}, + draftKey(d.ID): {"plugin", "", "Attention inbox", "Reply draft: Dana (Attention inbox)", base + draftLink(d.ID)}, } { it, ok := byKey[key] if !ok { t.Errorf("%s is not in the summary: %s", key, raw) continue } - if it.Kind != want.kind || it.Host != want.host || it.Title != want.title || it.Why != want.why || it.Path != want.path { + if it.Kind != want.kind || it.Host != want.host || it.Title != want.title || it.Why != want.why || it.URL != want.url { t.Errorf("%s: %+v; want %+v", key, it, want) } } @@ -210,38 +212,58 @@ func TestTheSummaryIsTheOwnersQueueAndHosts(t *testing.T) { } } -// TestAReadOnlyTokenMintedOnThePageReadsAndCannotWrite drives the whole loop -// over the real stores: mint with the box ticked (the list says read-only), -// read the summary with it, and be refused 403 revoking a token — its own -// included — which stays live. -func TestAReadOnlyTokenMintedOnThePageReadsAndCannotWrite(t *testing.T) { +// TestABarSummaryTokenMintedOnThePageReadsTheSummaryAndNothingElse drives the +// whole loop over the real stores: mint with "Bar summary only" chosen (stored +// as scope summary, listed as such), read the summary with it, and be refused +// 403 on a hook-tier read (the task list), on whoami, and revoking itself — +// after which it is still live. The control: a full token minted on the same +// page reads the task list. +func TestABarSummaryTokenMintedOnThePageReadsTheSummaryAndNothingElse(t *testing.T) { f := newSummaryFixture(t) t.Cleanup(func() { - _, _ = f.pool.Exec(authz.System(context.Background()), `DELETE FROM api_tokens WHERE user_id = 1 AND name = 'bar-ro'`) + _, _ = f.pool.Exec(authz.System(context.Background()), `DELETE FROM api_tokens WHERE user_id = 1 AND name IN ('bar-sum', 'cli-full')`) }) - body, hdr := form(url.Values{"name": {"bar-ro"}, "read_only": {"1"}}) - rec := f.call(t, http.MethodPost, accountTokensPath, body, merge(f.operator(), hdr)) - m := mintedRE.FindStringSubmatch(rec.Body.String()) - if rec.Code != http.StatusOK || m == nil { - t.Fatalf("minting a read-only token: %d %s", rec.Code, truncate(rec.Body.String(), 400)) + mint := func(name, scope string) string { + t.Helper() + v := url.Values{"name": {name}} + if scope != "" { + v.Set("scope", scope) + } + body, hdr := form(v) + rec := f.call(t, http.MethodPost, accountTokensPath, body, merge(f.operator(), hdr)) + m := mintedRE.FindStringSubmatch(rec.Body.String()) + if rec.Code != http.StatusOK || m == nil { + t.Fatalf("minting %s: %d %s", name, rec.Code, truncate(rec.Body.String(), 400)) + } + return m[1] } - tok := m[1] - if page := f.page(t, accountTokensPath); !strings.Contains(page, `data-token-scope="read-only"`) { - t.Fatalf("the token page does not list the token as read-only") + tok := mint("bar-sum", "summary") + full := mint("cli-full", "") + if page := f.page(t, accountTokensPath); !strings.Contains(page, `data-token-scope="summary">Bar summary only`) { + t.Fatalf("the token page does not list the token as bar summary only") } var id int64 - if err := f.pool.QueryRow(authz.System(f.ctx), `SELECT id FROM api_tokens WHERE token_hash = $1 AND read_only`, apitokens.Hash(tok)).Scan(&id); err != nil { - t.Fatalf("the minted token is not stored read-only: %v", err) + if err := f.pool.QueryRow(authz.System(f.ctx), `SELECT id FROM api_tokens WHERE token_hash = $1 AND scope = 'summary'`, apitokens.Hash(tok)).Scan(&id); err != nil { + t.Fatalf("the minted token is not stored with scope summary: %v", err) + } + var fullScope string + if err := f.pool.QueryRow(authz.System(f.ctx), `SELECT scope FROM api_tokens WHERE token_hash = $1`, apitokens.Hash(full)).Scan(&fullScope); err != nil || fullScope != "full" { + t.Fatalf("a mint naming no scope stored %q (%v); want full", fullScope, err) } if code, _, raw := summaryAs(t, f, tok); code != http.StatusOK { - t.Fatalf("the summary with a read-only token: %d %s", code, raw) + t.Fatalf("the summary with a summary token: %d %s", code, raw) } auth := map[string]string{"Authorization": "Bearer " + tok} - if w := f.call(t, http.MethodDelete, apiTokensPath+"/"+strconv.FormatInt(id, 10), nil, auth); w.Code != http.StatusForbidden || - !strings.Contains(w.Body.String(), "read-only") { - t.Fatalf("a read-only token revoking itself: %d %s; want 403", w.Code, w.Body) + for _, c := range []struct{ method, path string }{{http.MethodGet, "/api/tasks"}, {http.MethodGet, "/api/whoami"}, + {http.MethodDelete, apiTokensPath + "/" + strconv.FormatInt(id, 10)}} { + if w := f.call(t, c.method, c.path, nil, auth); w.Code != http.StatusForbidden || !strings.Contains(w.Body.String(), "scope does not reach") { + t.Errorf("%s %s with a summary token: %d %s; want 403", c.method, c.path, w.Code, truncate(w.Body.String(), 200)) + } + } + if code, _, _ := summaryAs(t, f, tok); code != http.StatusOK { + t.Fatalf("the summary token after its refused revoke: %d; it must still be live", code) } - if w := f.call(t, http.MethodGet, "/api/whoami", nil, auth); w.Code != http.StatusOK { - t.Fatalf("the read-only token after its refused revoke: %d; it must still be live", w.Code) + if w := f.call(t, http.MethodGet, "/api/tasks", nil, map[string]string{"Authorization": "Bearer " + full}); w.Code != http.StatusOK { + t.Fatalf("CONTROL: the full token on the task list: %d", w.Code) } } diff --git a/internal/api/attention_summary_test.go b/internal/api/attention_summary_test.go index f5363f3b3..7d12d8dbf 100644 --- a/internal/api/attention_summary_test.go +++ b/internal/api/attention_summary_test.go @@ -27,8 +27,9 @@ var summaryNow = time.Unix(1_800_000_000, 0) // TestTheSummaryItemSaysWhatItsPushSays pins every field of one item of each // kind, WHOLE, as literals: its host, its title (the title rule), its why (the // first line of its push body — a secret redacted, a waiting session summary -// only, a draft who-and-where only), its age, and its link (the tap route for -// an ask's and a session's key, the item's own link otherwise). Every fixture +// only, a draft who-and-where only), its age, and its url (the tap route for +// an ask's and a session's key, the item's own link otherwise, on the public +// origin). Every fixture // carries a Detail, options or a message body that must NOT appear: the // summary says what the lock screen says and no more. func TestTheSummaryItemSaysWhatItsPushSays(t *testing.T) { @@ -48,35 +49,36 @@ func TestTheSummaryItemSaysWhatItsPushSays(t *testing.T) { }{ {"an ask from a mirrored session: its host, the question redacted, never its options", mirrorAsk, attentionSummaryItem{ Key: "ask-41", Kind: "ask", Host: "ws", Title: "Gold:1", Why: "Use «redacted» for the mirror? It is the only key.", - Since: 1_799_999_910, AgeSecs: 90, Path: "/attention/open/ask-41", URL: "https://muster.example.test/attention/open/ask-41"}}, + Since: 1_799_999_910, AgeSecs: 90, URL: "https://muster.example.test/attention/open/ask-41"}}, {"an agent's ask: no host, never its options or suggestions", agentAsk, attentionSummaryItem{ Key: "ask-42", Kind: "ask", Title: "deploy-bot", Why: "Deploy the importer?", - Since: 1_799_992_800, AgeSecs: 7200, Path: "/attention/open/ask-42", URL: "https://muster.example.test/attention/open/ask-42"}}, + Since: 1_799_992_800, AgeSecs: 7200, URL: "https://muster.example.test/attention/open/ask-42"}}, {"a session waiting at a prompt: summary only, never Claude's last words", attentionItem{Key: sessKey, Kind: attentionKindSession, Source: "**Fix** [auth](https://example.com/a) for C# @ host-b", Text: "Bash: curl -H 'Authorization: x' https://x.test", tool: "Bash", Detail: "Tests pass.\nPublishing now.", host: "host-b", Offline: true, Since: summaryNow.Add(-30 * time.Second), Link: "/mirror/view?host=host-b&session=main&window=%403", card: attentionCard{ref: ui.MirrorRef{Host: "host-b", Session: "main", Window: "@3"}}}, attentionSummaryItem{Key: sessKey, Kind: "session", Host: "host-b", Title: "Fix auth for C# @ host-b", Why: "Waiting: Bash · Fix auth for C# @ host-b", - Since: 1_799_999_970, AgeSecs: 30, Offline: true, Path: "/attention/open/" + sessKey, URL: "https://muster.example.test/attention/open/" + sessKey}}, + Since: 1_799_999_970, AgeSecs: 30, Offline: true, URL: "https://muster.example.test/attention/open/" + sessKey}}, {"a held question: its first question, never its choices", attentionItem{Key: sessKey, Kind: attentionKindQuestion, Source: "main:3 @ host-b", Text: "1. Which branch?\n2. Push it?", choices: []string{"main", "dev"}, host: "host-b", Since: summaryNow.Add(-5 * time.Second), Link: "/mirror/view?host=host-b&session=main&window=%403"}, attentionSummaryItem{Key: sessKey, Kind: "question", Host: "host-b", Title: "main:3 @ host-b", Why: "1. Which branch?", - Since: 1_799_999_995, AgeSecs: 5, Path: "/attention/open/" + sessKey, URL: "https://muster.example.test/attention/open/" + sessKey}}, + Since: 1_799_999_995, AgeSecs: 5, URL: "https://muster.example.test/attention/open/" + sessKey}}, {"a reply draft: who and where only, never the message or the reply", draft, attentionSummaryItem{ Key: "draft-3", Kind: "plugin", Title: "Discord DMs", Why: "Reply draft: Dana (Discord DMs)", - Since: 1_799_999_400, AgeSecs: 600, Path: "/attention?tab=drafts#draft-3", URL: "https://muster.example.test/attention?tab=drafts#draft-3"}}, + Since: 1_799_999_400, AgeSecs: 600, URL: "https://muster.example.test/attention?tab=drafts#draft-3"}}, {"a task nobody routed: its own link", attentionItem{Key: unroutedTaskKey(55), Kind: attentionKindTask, Source: "your chief has not routed it", Text: "Task #9: Rotate the build key", Since: summaryNow.Add(-24 * time.Hour), Link: canonicalTaskPath(9)}, attentionSummaryItem{Key: "task-55", Kind: "task", Title: "your chief has not routed it", Why: "Task #9: Rotate the build key", - Since: 1_799_913_600, AgeSecs: 86400, Path: "/tasks/9", URL: "https://muster.example.test/tasks/9"}}, + Since: 1_799_913_600, AgeSecs: 86400, URL: "https://muster.example.test/tasks/9"}}, } { if got := attentionSummaryItemOf(c.it, "https://muster.example.test", summaryNow); got != c.want { t.Errorf("%s:\n got %+v\nwant %+v", c.name, got, c.want) } } - // No public URL: the path alone, and no since is no age. - if got := attentionSummaryItemOf(attentionItem{Key: "ask-1", Kind: attentionKindAsk, Source: "x", Text: "q"}, "", summaryNow); got.URL != "" || got.Path != "/attention/open/ask-1" || got.Since != 0 || got.AgeSecs != 0 { + // No public URL: no url (the summary names no origin it was not told), + // and no since is no age. + if got := attentionSummaryItemOf(attentionItem{Key: "ask-1", Kind: attentionKindAsk, Source: "x", Text: "q"}, "", summaryNow); got.URL != "" || got.Since != 0 || got.AgeSecs != 0 { t.Errorf("no public URL, no since: %+v", got) } } @@ -160,7 +162,7 @@ func TestTheSummaryDocumentIsTheDocumentedShape(t *testing.T) { const want = `{"ts":1800000000,"nudge_mode":"off","partial":false,` + `"hosts":[{"host":"host-a","online":true,"status_report":true,"busy":1,"idle":0}],` + `"items":[{"key":"ask-5","kind":"ask","host":"","title":"chief","why":"Ship it?","since":1799999940,"age_secs":60,"offline":false,` + - `"path":"/attention/open/ask-5","url":"https://muster.example.test/attention/open/ask-5"}]}` + `"url":"https://muster.example.test/attention/open/ask-5"}]}` if string(got) != want { t.Fatalf("a summary:\n got %s\nwant %s", got, want) } diff --git a/internal/api/flows_test.go b/internal/api/flows_test.go index b776c263f..50c0e4143 100644 --- a/internal/api/flows_test.go +++ b/internal/api/flows_test.go @@ -218,11 +218,11 @@ func TestFlowLifecycleOverHTTP(t *testing.T) { t.Fatal(err) } ts := apitokens.NewPG(pool) - aTok, _, err := ts.Create(asUser(a), "a", 0, false) + aTok, _, err := ts.Create(asUser(a), "a", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } - bTok, _, err := ts.Create(asUser(b), "b", 0, false) + bTok, _, err := ts.Create(asUser(b), "b", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } @@ -428,11 +428,11 @@ func TestFlowCancelOverTheAPI(t *testing.T) { return authz.WithPrincipal(ctx, authz.Principal{UserID: uid, Role: authz.RoleMember, Via: authz.ViaSession}) } ts := apitokens.NewPG(pool) - aTok, _, err := ts.Create(asUser(a), "a", 0, false) + aTok, _, err := ts.Create(asUser(a), "a", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } - bTok, _, err := ts.Create(asUser(b), "b", 0, false) + bTok, _, err := ts.Create(asUser(b), "b", 0, apitokens.ScopeFull) if err != nil { t.Fatal(err) } diff --git a/internal/api/isolation_test.go b/internal/api/isolation_test.go index c62e23899..ff55d5fdf 100644 --- a/internal/api/isolation_test.go +++ b/internal/api/isolation_test.go @@ -814,11 +814,11 @@ func newIsoWorld(t *testing.T) *isoWorld { ts := apitokens.NewPG(pool) w.apiTokens = map[int64]string{} for _, u := range []isoUser{w.a, w.b} { - tok, _, err := ts.Create(as2(u), u.name+" cli", 0, false) + tok, _, err := ts.Create(as2(u), u.name+" cli", 0, apitokens.ScopeFull) must(err, u.name+" api token") w.apiTokens[u.uid] = tok } - _, w.aToken2, err = ts.Create(ua, w.canary+" token", 0, false) + _, w.aToken2, err = ts.Create(ua, w.canary+" token", 0, apitokens.ScopeFull) must(err, "a second api token") w.aTokTask, err = ns.Create(ua, notes.Note{Title: w.canary + " token-control", Status: notes.StatusOpen}) must(err, "a token-control task") diff --git a/internal/api/principal_test.go b/internal/api/principal_test.go index f45b4e538..801006c69 100644 --- a/internal/api/principal_test.go +++ b/internal/api/principal_test.go @@ -483,7 +483,7 @@ type principalAPITokens struct{ apitokens.Store } func (principalAPITokens) Authenticate(_ context.Context, tok string) (apitokens.Token, error) { if tok == pAPIToken { - return apitokens.Token{ID: 3, UserID: authz.OperatorUserID, Name: "principal-test"}, nil + return apitokens.Token{ID: 3, UserID: authz.OperatorUserID, Name: "principal-test", Scope: apitokens.ScopeFull}, nil } return apitokens.Token{}, apitokens.ErrBadToken } diff --git a/internal/api/session_routes_pg_test.go b/internal/api/session_routes_pg_test.go index c07a44463..0731f20c2 100644 --- a/internal/api/session_routes_pg_test.go +++ b/internal/api/session_routes_pg_test.go @@ -136,7 +136,7 @@ func newRouteFixture(t *testing.T) *routeFixture { g.chiefTok, g.workerTok = "route-chief-token-"+suffix, "route-worker-token-"+suffix g.chief = g.newAgent(t, g.owner, "rtchief-"+suffix, "chief", g.chiefTok) g.worker = g.newAgent(t, g.owner, "rtworker-"+suffix, "", g.workerTok) - tok, _, err := apitokens.NewPG(g.pool).Create(g.asOwner, "route-reply", time.Now().Unix(), false) + tok, _, err := apitokens.NewPG(g.pool).Create(g.asOwner, "route-reply", time.Now().Unix(), apitokens.ScopeFull) if err != nil { t.Fatal(err) } @@ -389,7 +389,7 @@ func TestTheAnswerIsRelayedPerTheAllowlist(t *testing.T) { // Another user's credential: the route is not theirs, so it is not found. stranger := f.newMember(t) - tok, _, err := apitokens.NewPG(f.pool).Create(f.as(stranger), "stranger", time.Now().Unix(), false) + tok, _, err := apitokens.NewPG(f.pool).Create(f.as(stranger), "stranger", time.Now().Unix(), apitokens.ScopeFull) if err != nil { t.Fatal(err) } diff --git a/internal/apitokens/apitokens.go b/internal/apitokens/apitokens.go index a7ac696ab..978b93ce0 100644 --- a/internal/apitokens/apitokens.go +++ b/internal/apitokens/apitokens.go @@ -55,6 +55,15 @@ const Prefix = "mst_" // encodedLen is the base64url (no padding) length of 32 bytes. const encodedLen = 43 +// The token scopes (api_tokens.scope, migration 0068). +const ( + // ScopeFull acts as the owner on every route a personal token reaches. + ScopeFull = "full" + // ScopeSummary reaches GET /api/attention/summary and nothing else: what a + // status bar polls (internal/api's tokenScopeRoutes). + ScopeSummary = "summary" +) + // MaxNameLen bounds a token's label, in characters. const MaxNameLen = 100 @@ -76,6 +85,8 @@ var ( // ErrNoUser: a management call with no user principal on its context (or // with the System principal, which is nobody's). ErrNoUser = errors.New("apitokens: no user principal on this context") + // ErrBadScope: the scope is not ScopeFull or ScopeSummary. + ErrBadScope = errors.New("apitokens: unknown token scope") // ErrNameTooLong: the label is longer than MaxNameLen characters. ErrNameTooLong = fmt.Errorf("apitokens: a token name is at most %d characters", MaxNameLen) ) @@ -88,11 +99,11 @@ type Token struct { CreatedAt time.Time `json:"createdAt"` LastUsedAt *time.Time `json:"lastUsedAt"` RevokedAt *time.Time `json:"revokedAt"` - // ReadOnly: the token may only READ (migration 0068). Its scope is chosen - // when it is minted and never changes; a token minted before 0068 is full - // access. What "read" admits is decided at the door - // (internal/api.admitAPIToken), not here. - ReadOnly bool `json:"readOnly"` + // Scope is what the token may reach (migration 0068): ScopeFull, or a + // narrower scope. It is chosen when the token is minted and never changes; + // a token minted before 0068 is ScopeFull. Which routes a scope reaches is + // decided at the door (internal/api.admitAPIToken), not here. + Scope string `json:"scope"` } // Revoked reports whether the token is revoked. @@ -138,13 +149,13 @@ type Store interface { // ErrUserDisabled when the owner is disabled. ctx must be able to read any // user's row (authz.System): the owner is what it learns. Authenticate(ctx context.Context, token string) (Token, error) - // Create mints a token named name for ctx's user, read-only when readOnly, + // Create mints a token named name, of scope (CleanScope), for ctx's user, // records it in audit_log, and returns the token ONCE with its record. // sessionIssuedAt is the minting browser session's iat (unix seconds): a // session issued at or before the user's session cutoff is refused // ErrSessionEnded. Zero means the caller is not a GoTrue session (password // mode, a fixture) and there is no cutoff to compare. - Create(ctx context.Context, name string, sessionIssuedAt int64, readOnly bool) (string, Token, error) + Create(ctx context.Context, name string, sessionIssuedAt int64, scope string) (string, Token, error) // List returns ctx's user's tokens, newest first, revoked included. List(ctx context.Context) ([]Token, error) // Revoke revokes ctx's user's token id. found is false when the user has @@ -164,11 +175,11 @@ func NewPG(pool *pgxpool.Pool) *PGStore { return &PGStore{pool: pool, now: time. var _ Store = (*PGStore)(nil) -const cols = `id, user_id, name, created_at, last_used_at, revoked_at, read_only` +const cols = `id, user_id, name, created_at, last_used_at, revoked_at, scope` func scan(row pgx.Row) (Token, error) { var t Token - err := row.Scan(&t.ID, &t.UserID, &t.Name, &t.CreatedAt, &t.LastUsedAt, &t.RevokedAt, &t.ReadOnly) + err := row.Scan(&t.ID, &t.UserID, &t.Name, &t.CreatedAt, &t.LastUsedAt, &t.RevokedAt, &t.Scope) return t, err } @@ -190,11 +201,11 @@ func (s *PGStore) Authenticate(ctx context.Context, token string) (Token, error) } var t Token var disabled bool - err := s.pool.QueryRow(ctx, `SELECT t.id, t.user_id, t.name, t.created_at, t.last_used_at, t.revoked_at, t.read_only, + err := s.pool.QueryRow(ctx, `SELECT t.id, t.user_id, t.name, t.created_at, t.last_used_at, t.revoked_at, t.scope, u.disabled_at IS NOT NULL FROM api_tokens t JOIN users u ON u.id = t.user_id WHERE t.token_hash = $1`, Hash(token)). - Scan(&t.ID, &t.UserID, &t.Name, &t.CreatedAt, &t.LastUsedAt, &t.RevokedAt, &t.ReadOnly, &disabled) + Scan(&t.ID, &t.UserID, &t.Name, &t.CreatedAt, &t.LastUsedAt, &t.RevokedAt, &t.Scope, &disabled) if errors.Is(err, pgx.ErrNoRows) { return Token{}, ErrBadToken } @@ -240,6 +251,19 @@ func CleanName(name string) (string, error) { return name, nil } +// CleanScope is scope as stored: "" is ScopeFull (a mint that names none is +// what every mint was before 0068); anything not a known scope is ErrBadScope. +// Exported so a handler can refuse a bad one with a 400 before minting. +func CleanScope(scope string) (string, error) { + switch scope { + case "", ScopeFull: + return ScopeFull, nil + case ScopeSummary: + return ScopeSummary, nil + } + return "", ErrBadScope +} + // Create implements Store. The token row and its audit row are one // transaction: a token is never live without its "created" record. // @@ -256,7 +280,7 @@ func CleanName(name string) (string, error) { // with users.sessions_not_before by the session tier's own rule (at or before // the cutoff's second is refused): a session the reset ended cannot mint, and // the session the reset signs in can (TestAMintQueuedBehindAResetIsRefused). -func (s *PGStore) Create(ctx context.Context, name string, sessionIssuedAt int64, readOnly bool) (string, Token, error) { +func (s *PGStore) Create(ctx context.Context, name string, sessionIssuedAt int64, scope string) (string, Token, error) { p, err := user(ctx) if err != nil { return "", Token{}, err @@ -264,6 +288,9 @@ func (s *PGStore) Create(ctx context.Context, name string, sessionIssuedAt int64 if name, err = CleanName(name); err != nil { return "", Token{}, err } + if scope, err = CleanScope(scope); err != nil { + return "", Token{}, err + } tok, err := New() if err != nil { return "", Token{}, err @@ -287,12 +314,12 @@ func (s *PGStore) Create(ctx context.Context, name string, sessionIssuedAt int64 return ErrSessionEnded } var err error - t, err = scan(tx.QueryRow(ctx, `INSERT INTO api_tokens (user_id, name, token_hash, read_only) - VALUES ($1, $2, $3, $4) RETURNING `+cols, p.UserID, name, Hash(tok), readOnly)) + t, err = scan(tx.QueryRow(ctx, `INSERT INTO api_tokens (user_id, name, token_hash, scope) + VALUES ($1, $2, $3, $4) RETURNING `+cols, p.UserID, name, Hash(tok), scope)) if err != nil { return err } - return audit(ctx, tx, p, "api_token.create", t.ID, map[string]any{"name": t.Name, "readOnly": t.ReadOnly}) + return audit(ctx, tx, p, "api_token.create", t.ID, map[string]any{"name": t.Name, "scope": t.Scope}) }) if err != nil { return "", Token{}, err diff --git a/internal/apitokens/apitokens_test.go b/internal/apitokens/apitokens_test.go index 85153c3aa..cf4403d25 100644 --- a/internal/apitokens/apitokens_test.go +++ b/internal/apitokens/apitokens_test.go @@ -108,7 +108,7 @@ func TestAMalformedTokenIsRefusedWithoutADatabaseRead(t *testing.T) { func TestOnlyTheHashIsStoredAndTheTokenIsShownOnce(t *testing.T) { w := newWorld(t) - tok, rec, err := w.s.Create(as(w.a, authz.RoleMember), " laptop ", 0, false) + tok, rec, err := w.s.Create(as(w.a, authz.RoleMember), " laptop ", 0, ScopeFull) if err != nil { t.Fatal(err) } @@ -145,11 +145,11 @@ func TestOnlyTheHashIsStoredAndTheTokenIsShownOnce(t *testing.T) { func TestAuthenticateResolvesTheOwnerAndRefusesRevokedAndDisabled(t *testing.T) { w := newWorld(t) - tokA, recA, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, false) + tokA, recA, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, ScopeFull) if err != nil { t.Fatal(err) } - tokB, _, err := w.s.Create(as(w.b, authz.RoleMember), "b", 0, false) + tokB, _, err := w.s.Create(as(w.b, authz.RoleMember), "b", 0, ScopeFull) if err != nil { t.Fatal(err) } @@ -218,11 +218,11 @@ func TestAuthenticateResolvesTheOwnerAndRefusesRevokedAndDisabled(t *testing.T) // a token minted after the re-enable works. func TestDisablingAUserRevokesItsTokensForGood(t *testing.T) { w := newWorld(t) - tokA, _, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, false) + tokA, _, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, ScopeFull) if err != nil { t.Fatal(err) } - tokB, _, err := w.s.Create(as(w.b, authz.RoleMember), "b", 0, false) + tokB, _, err := w.s.Create(as(w.b, authz.RoleMember), "b", 0, ScopeFull) if err != nil { t.Fatal(err) } @@ -246,7 +246,7 @@ func TestDisablingAUserRevokesItsTokensForGood(t *testing.T) { if got, err := w.s.Authenticate(sysCtx(), tokB); err != nil || got.UserID != w.b { t.Fatalf("CONTROL: B's token after A's disable: %+v %v", got, err) } - fresh, _, err := w.s.Create(as(w.a, authz.RoleMember), "after", 0, false) + fresh, _, err := w.s.Create(as(w.a, authz.RoleMember), "after", 0, ScopeFull) if err != nil { t.Fatal(err) } @@ -259,7 +259,7 @@ func TestDisablingAUserRevokesItsTokensForGood(t *testing.T) { // "no such token", and A's token still works. func TestNobodyRevokesAnotherUsersToken(t *testing.T) { w := newWorld(t) - tokA, recA, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, false) + tokA, recA, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, ScopeFull) if err != nil { t.Fatal(err) } @@ -280,7 +280,7 @@ func TestNobodyRevokesAnotherUsersToken(t *testing.T) { // The positive control is A, who sees its own. func TestRowLevelSecurityKeepsTokensToTheirOwner(t *testing.T) { w := newWorld(t) - if _, _, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, false); err != nil { + if _, _, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, ScopeFull); err != nil { t.Fatal(err) } count := func(ctx context.Context) int { @@ -311,7 +311,7 @@ func TestRowLevelSecurityKeepsTokensToTheirOwner(t *testing.T) { func TestManagementNeedsAUserPrincipal(t *testing.T) { w := newWorld(t) for name, ctx := range map[string]context.Context{"none": context.Background(), "system": sysCtx()} { - if _, _, err := w.s.Create(ctx, "x", 0, false); !errors.Is(err, ErrNoUser) { + if _, _, err := w.s.Create(ctx, "x", 0, ScopeFull); !errors.Is(err, ErrNoUser) { t.Errorf("%s Create: %v", name, err) } if _, err := w.s.List(ctx); !errors.Is(err, ErrNoUser) { @@ -321,10 +321,10 @@ func TestManagementNeedsAUserPrincipal(t *testing.T) { t.Errorf("%s Revoke: %v", name, err) } } - if _, _, err := w.s.Create(as(w.a, authz.RoleMember), strings.Repeat("é", MaxNameLen+1), 0, false); !errors.Is(err, ErrNameTooLong) { + if _, _, err := w.s.Create(as(w.a, authz.RoleMember), strings.Repeat("é", MaxNameLen+1), 0, ScopeFull); !errors.Is(err, ErrNameTooLong) { t.Errorf("an over-long name: %v", err) } - if _, _, err := w.s.Create(as(w.a, authz.RoleMember), strings.Repeat("é", MaxNameLen), 0, false); err != nil { + if _, _, err := w.s.Create(as(w.a, authz.RoleMember), strings.Repeat("é", MaxNameLen), 0, ScopeFull); err != nil { t.Errorf("a name of exactly %d characters: %v", MaxNameLen, err) } } @@ -333,7 +333,7 @@ func TestManagementNeedsAUserPrincipal(t *testing.T) { // within LastUsedGranularity does not rewrite it; a use after it does. func TestLastUsedIsStampedAndThrottled(t *testing.T) { w := newWorld(t) - tok, rec, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, false) + tok, rec, err := w.s.Create(as(w.a, authz.RoleMember), "a", 0, ScopeFull) if err != nil { t.Fatal(err) } @@ -437,7 +437,7 @@ func raceMintAgainstDisable(t *testing.T, w world, role authz.Role, lockTable st } mintc := make(chan minted, 1) go func() { - tok, _, err := w.s.Create(as(w.a, role), "racer", 0, false) + tok, _, err := w.s.Create(as(w.a, role), "racer", 0, ScopeFull) mintc <- minted{tok, err} }() lw.until(2, "the mint to block") @@ -587,7 +587,7 @@ func TestAMintQueuedBehindAResetIsRefused(t *testing.T) { } mintc := make(chan minted, 1) go func() { - tok, _, err := w.s.Create(as(w.a, authz.RoleMember), "queued", oldSession, false) + tok, _, err := w.s.Create(as(w.a, authz.RoleMember), "queued", oldSession, ScopeFull) mintc <- minted{tok, err} }() lw.until(2, "the mint to queue behind the reset") @@ -609,10 +609,10 @@ func TestAMintQueuedBehindAResetIsRefused(t *testing.T) { } // The boundary: a session stamped with the cutoff's own second is refused // (the session tier's rule); the next second's is the post-reset session. - if _, _, err := w.s.Create(as(w.a, authz.RoleMember), "same-second", r.cut.Unix(), false); !errors.Is(err, ErrSessionEnded) { + if _, _, err := w.s.Create(as(w.a, authz.RoleMember), "same-second", r.cut.Unix(), ScopeFull); !errors.Is(err, ErrSessionEnded) { t.Fatalf("a session issued in the cutoff's own second: %v; want ErrSessionEnded", err) } - tok, _, err := w.s.Create(as(w.a, authz.RoleMember), "after-reset", r.cut.Unix()+1, false) + tok, _, err := w.s.Create(as(w.a, authz.RoleMember), "after-reset", r.cut.Unix()+1, ScopeFull) if err != nil { t.Fatalf("the POST-reset session cannot mint: %v", err) } @@ -655,8 +655,8 @@ func stressCredentialWriters(t *testing.T, w world, rounds int, budget time.Dura target := users.Actor{UserID: w.a, Via: authz.ViaSession} asX, asY := as(w.a, authz.RoleAdmin), as(w.b, authz.RoleAdmin) ops := map[string]func(i int) error{ - "mint": func(int) error { _, _, err := w.s.Create(asX, "stress", 0, false); return err }, - "mint-by-another-admin": func(int) error { _, _, err := w.s.Create(asY, "stress", 0, false); return err }, + "mint": func(int) error { _, _, err := w.s.Create(asX, "stress", 0, ScopeFull); return err }, + "mint-by-another-admin": func(int) error { _, _, err := w.s.Create(asY, "stress", 0, ScopeFull); return err }, "revoke-one": func(int) error { list, err := w.s.List(asX) if err != nil { @@ -924,27 +924,29 @@ func TestCredentialWritersNeverDeadlock(t *testing.T) { t.Logf("%d lockstep rounds, 0 deadlocks; successes per writer: %v", res.rounds, res.ok) } -// TestAReadOnlyTokensScopeIsStoredAndReadBack (migration 0068): a token minted -// read-only says so in its record, in the list and in Authenticate's answer -// (the door reads THAT one); a token minted without is full access everywhere; -// and the audit row of each mint names its scope. -func TestAReadOnlyTokensScopeIsStoredAndReadBack(t *testing.T) { +// TestATokensScopeIsStoredAndReadBack (migration 0068): a token minted with +// the summary scope says so in its record, in the list and in Authenticate's +// answer (the door reads THAT one); a mint naming no scope is full access; an +// unknown scope is refused before anything is written, and the database's +// CHECK refuses one written around the store. Each mint's audit row names its +// scope. +func TestATokensScopeIsStoredAndReadBack(t *testing.T) { w := newWorld(t) - ro, roRec, err := w.s.Create(as(w.a, authz.RoleMember), "bar", 0, true) + sum, sumRec, err := w.s.Create(as(w.a, authz.RoleMember), "bar", 0, ScopeSummary) if err != nil { t.Fatal(err) } - full, fullRec, err := w.s.Create(as(w.a, authz.RoleMember), "cli", 0, false) + full, fullRec, err := w.s.Create(as(w.a, authz.RoleMember), "cli", 0, "") if err != nil { t.Fatal(err) } - if !roRec.ReadOnly || fullRec.ReadOnly { - t.Fatalf("records: read-only %+v, full %+v", roRec, fullRec) + if sumRec.Scope != ScopeSummary || fullRec.Scope != ScopeFull { + t.Fatalf("records: summary %+v, unnamed %+v", sumRec, fullRec) } - for tok, want := range map[string]bool{ro: true, full: false} { + for tok, want := range map[string]string{sum: ScopeSummary, full: ScopeFull} { got, err := w.s.Authenticate(sysCtx(), tok) - if err != nil || got.ReadOnly != want { - t.Errorf("Authenticate: %+v %v; want readOnly %v", got, err, want) + if err != nil || got.Scope != want { + t.Errorf("Authenticate: %+v %v; want scope %q", got, err, want) } } list, err := w.s.List(as(w.a, authz.RoleMember)) @@ -952,19 +954,29 @@ func TestAReadOnlyTokensScopeIsStoredAndReadBack(t *testing.T) { t.Fatalf("list = %+v, %v", list, err) } for _, tk := range list { - if tk.ReadOnly != (tk.ID == roRec.ID) { - t.Errorf("listed %+v: readOnly is not the minted scope", tk) + if want := map[bool]string{true: ScopeSummary, false: ScopeFull}[tk.ID == sumRec.ID]; tk.Scope != want { + t.Errorf("listed %+v: scope is not the minted %q", tk, want) } } + if _, _, err := w.s.Create(as(w.a, authz.RoleMember), "x", 0, "admin"); !errors.Is(err, ErrBadScope) { + t.Fatalf("an unknown scope: %v; want ErrBadScope", err) + } + var n int + if err := w.pool.QueryRow(sysCtx(), `SELECT count(*) FROM api_tokens WHERE user_id = $1`, w.a).Scan(&n); err != nil || n != 2 { + t.Fatalf("after the refused mint %d tokens (%v); want 2", n, err) + } + if _, err := w.pool.Exec(sysCtx(), `UPDATE api_tokens SET scope = 'admin' WHERE id = $1`, sumRec.ID); err == nil { + t.Fatal("the database accepted scope 'admin'; api_tokens_scope_check must refuse it") + } var details string - if err := w.pool.QueryRow(sysCtx(), `SELECT string_agg(detail->>'readOnly', ',' ORDER BY id) FROM audit_log - WHERE user_id = $1 AND action = 'api_token.create'`, w.a).Scan(&details); err != nil || details != "true,false" { - t.Fatalf("the mints' audit scopes = %q (%v); want \"true,false\"", details, err) - } - // A row written without the column (an older image's INSERT) is full access. - var dflt bool - if err := w.pool.QueryRow(sysCtx(), `SELECT column_default = 'false' AND is_nullable = 'NO' FROM information_schema.columns - WHERE table_name = 'api_tokens' AND column_name = 'read_only'`).Scan(&dflt); err != nil || !dflt { - t.Fatalf("api_tokens.read_only is not NOT NULL DEFAULT false: %v %v", dflt, err) + if err := w.pool.QueryRow(sysCtx(), `SELECT string_agg(detail->>'scope', ',' ORDER BY id) FROM audit_log + WHERE user_id = $1 AND action = 'api_token.create'`, w.a).Scan(&details); err != nil || details != "summary,full" { + t.Fatalf("the mints' audit scopes = %q (%v); want \"summary,full\"", details, err) + } + // A row written without the column (an older image's INSERT) is full. + var dflt string + if err := w.pool.QueryRow(sysCtx(), `SELECT column_default || '/' || is_nullable FROM information_schema.columns + WHERE table_name = 'api_tokens' AND column_name = 'scope'`).Scan(&dflt); err != nil || dflt != "'full'::text/NO" { + t.Fatalf("api_tokens.scope is %q (%v); want NOT NULL DEFAULT 'full'", dflt, err) } } diff --git a/internal/db/migrations/0068_api_token_read_only.sql b/internal/db/migrations/0068_api_token_read_only.sql deleted file mode 100644 index aa09cc002..000000000 --- a/internal/db/migrations/0068_api_token_read_only.sql +++ /dev/null @@ -1,24 +0,0 @@ --- muster 0068 — READ-ONLY personal API tokens (internal/apitokens). --- --- * api_tokens.read_only (default false): the token may only read. It is --- chosen when the token is minted (/account/tokens) and never changes. --- Every existing row is false, so every token minted before this one keeps --- the full access it was minted with. --- --- What "read" admits is decided at the token door (internal/api --- admitAPIToken): a read-only token is refused 403 on any method but GET and --- HEAD, on every route a personal token reaches. --- --- The column inherits the table's ownership, grants and row-level security --- (0009's user_isolation); no policy changes, nothing for db.RLSOn to re-apply. --- --- 🔴 Rollback: an image from before 0068 never reads read_only, so it would --- admit a read-only token with FULL access. Revoke every live read-only token --- first, then drop the column: --- UPDATE public.api_tokens SET revoked_at = now() WHERE read_only AND revoked_at IS NULL; --- ALTER TABLE public.api_tokens DROP COLUMN read_only; --- DELETE FROM schema_migrations WHERE version = 68; - -SET LOCAL lock_timeout = '3s'; - -ALTER TABLE public.api_tokens ADD COLUMN IF NOT EXISTS read_only BOOLEAN NOT NULL DEFAULT false; diff --git a/internal/db/migrations/0068_api_token_scope.sql b/internal/db/migrations/0068_api_token_scope.sql new file mode 100644 index 000000000..6f9bacbc8 --- /dev/null +++ b/internal/db/migrations/0068_api_token_scope.sql @@ -0,0 +1,28 @@ +-- muster 0068 — personal API token SCOPES (internal/apitokens). +-- +-- * api_tokens.scope (default 'full'; api_tokens_scope_check): what the +-- token may reach. 'full' is every route a personal token reaches, as +-- before; 'summary' is GET /api/attention/summary and nothing else (what a +-- status bar polls). It is chosen when the token is minted +-- (/account/tokens) and never changes. Every existing row is 'full', so +-- every token minted before this one keeps the access it was minted with. +-- +-- Which routes a scope reaches is decided at the token door (internal/api +-- admitAPIToken, tokenScopeRoutes): a narrower token is refused 403 on every +-- other route. +-- +-- The column inherits the table's ownership, grants and row-level security +-- (0009's user_isolation); no policy changes, nothing for db.RLSOn to re-apply. +-- +-- 🔴 Rollback: an image from before 0068 never reads scope, so it would admit +-- a 'summary' token with FULL access. Revoke every live narrower token first, +-- then drop the column: +-- UPDATE public.api_tokens SET revoked_at = now() WHERE scope <> 'full' AND revoked_at IS NULL; +-- ALTER TABLE public.api_tokens DROP COLUMN scope; +-- DELETE FROM schema_migrations WHERE version = 68; + +SET LOCAL lock_timeout = '3s'; + +ALTER TABLE public.api_tokens ADD COLUMN IF NOT EXISTS scope TEXT NOT NULL DEFAULT 'full'; +ALTER TABLE public.api_tokens DROP CONSTRAINT IF EXISTS api_tokens_scope_check; +ALTER TABLE public.api_tokens ADD CONSTRAINT api_tokens_scope_check CHECK (scope IN ('full', 'summary')); diff --git a/tests/verb-ledger.sh b/tests/verb-ledger.sh index e9b0badce..0432f0859 100755 --- a/tests/verb-ledger.sh +++ b/tests/verb-ledger.sh @@ -64,7 +64,6 @@ agent resolve agent task comment agent task get agent task status -attention summary chief ask chief replace doctor @@ -88,7 +87,7 @@ whoami" # still fails until someone moves this literal deliberately. (tree_test.go pins # the identical number for the in-process tree; they are two reads of one # decision.) -WANT_COUNT=24 +WANT_COUNT=23 # FORBIDDEN is the other half of the split: verbs that belong to the upstream # CLI this one was extracted from and must NEVER appear here. From 6cd348204f66cd413085ca93437777f33ec8c14e Mon Sep 17 00:00:00 2001 From: Zachary Lowden Date: Sun, 1 Nov 2026 00:10:27 -0500 Subject: [PATCH 3/3] fix(attention-summary): one hub read; 0068 rollout and rollback notes Round-1 audit findings (final round). - One Hosts() read per summary. The handler reads the hub once and hands the snapshot to attentionItemsIn -> waitingSessionsIn, so a flapping host cannot be online in hosts[] while its item says offline. attentionItems and waitingSessions keep their signatures and read the hub themselves. TestTheSummaryReadsTheHubOnce: red at 69c3b855 ("read the hub 2 times"), green here. - 0068 header + docs/schema.md: mint 'summary' tokens only after the rollout completes. A pre-0068 pod ignores scope, so a summary token minted mid-rollout (RollingUpdate, maxUnavailable 0) has full access on every request an old pod answers. - 0068 rollback: api_tokens is under FORCED row-level security, so the revoke must run with SET LOCAL muster.scope = 'all'. Measured against a throwaway Postgres, with counts taken as the superuser: as muster_app without the scope the UPDATE left 1 of 1 live summary tokens (and raised no error); with it, 0. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/schema.md | 2 +- internal/api/attention.go | 24 +++++++++++-- internal/api/attention_summary.go | 11 +++--- internal/api/attention_summary_test.go | 36 +++++++++++++++++++ .../db/migrations/0068_api_token_scope.sql | 16 ++++++++- 5 files changed, 81 insertions(+), 8 deletions(-) diff --git a/docs/schema.md b/docs/schema.md index 1e7a09fee..9663d57eb 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -94,7 +94,7 @@ buys only a word. | `0064` | `chief_nudges`: one row per [stall-nudge](chief-nudges.md) decision — the session (`session_kind` `mirror`/`agent`, `session_key`, its window, or its agent and `chat_session_id` — the one conversation of the agent that stalled), the idle streak it was taken on (`streak_key`, unique per owner and session: the sweep's claim), the decision (`nudge`, `ask`, `leave`, `invalid`), the nudge's message and the quoted owner message (`quote_id`, `quote_at`, `quote_text`) and the action it covers, the ask's question and options, the chief's reason, the `mode` (`shadow`/`live`; a shadow row can hold no ask and no queued message, by CHECK), the `outcome` (a live state, or `suppressed:`), the live ask and queued message it raised, and the owner's relayed answer. `send_queue.chief_nudge_id` marks a queued nudge or relay, gated by its decision rather than a sign-in (never together with `route_id`). Owned, under row-level security, and re-applied by `db.RLSOn`. Rollback: in the file's header. | | `0065` | `agent_focus.cwd`: the working directory the session's transcript tail last named when its focus was written, cleaned and absolute, `''` for unknown (`agent_focus_cwd_check`: empty, or starting with `/` and at most 512 characters). Read only by the fleet report's mail door, to decide whether a session is under `MUSTER_FLEET_REPORT_PATHS` ([focus](focus.md#the-working-directory)); never rendered or sent. The column inherits `agent_focus`'s ownership and row-level security; no policy change, not re-applied by `db.RLSOn`. Must apply after `0064`. Rollback: in the file's header. | | `0067` | `connectors.auth_scheme` gains `postgres`; `connectors.settings` (JSONB, default `{}`): a [postgres connector](connectors.md#postgres-connectors)'s non-secret settings — `database`, `sslmode`, `require_tls`, `statement_timeout_ms`, `max_rows`, `max_bytes` — and `{}` for every other scheme (`connectors_settings_scheme`); `connector_audit.kind` gains `query` (one row per agent query: the SQL's fingerprint and first 120 characters, rows, duration, outcome class; never a parameter value or the password). The columns inherit their tables' ownership and row-level security; not re-applied by `db.RLSOn`. Rollback: in the file's header (delete postgres connectors first). | -| `0068` | `api_tokens.scope` (default `full`, `api_tokens_scope_check`: `full` or `summary`): a [personal token's scope](auth.md) — `full` reaches every route a personal token reaches, `summary` only `GET /api/attention/summary`, enforced at the token door. Every token minted before it is `full`. Inherits the table's row-level security; not re-applied by `db.RLSOn`. 🔴 Rollback: an older image would admit a `summary` token with full access, so revoke every live non-`full` token first (in the file's header). | +| `0068` | `api_tokens.scope` (default `full`, `api_tokens_scope_check`: `full` or `summary`): a [personal token's scope](auth.md) — `full` reaches every route a personal token reaches, `summary` only `GET /api/attention/summary`, enforced at the token door. Every token minted before it is `full`. Inherits the table's row-level security; not re-applied by `db.RLSOn`. 🔴 **Mint `summary` tokens only after the rollout completes**: until the last pre-0068 pod is gone, one minted mid-rollout has full access on every request an old pod answers. 🔴 Rollback: an older image would admit a `summary` token with full access, so revoke every live non-`full` token first, in a transaction with `SET LOCAL muster.scope = 'all'` (the table is under forced row-level security, which binds its owner too, so without it the revoke silently matches nothing, as `muster_app` or as the owner); the steps are in the file's header. | To change the schema, add a new numbered file. diff --git a/internal/api/attention.go b/internal/api/attention.go index ec230fc46..2e04feea8 100644 --- a/internal/api/attention.go +++ b/internal/api/attention.go @@ -327,6 +327,17 @@ func attentionHostBelieved(h mirror.HostView, now time.Time) bool { // session reports StatusWaiting, in the hub's host order (Mirror.Hosts), for every host // attentionHostBelieved believes. func (s *Server) waitingSessions(ctx context.Context) []attentionItem { + if s.ext.Mirror == nil { + return nil + } + return s.waitingSessionsIn(ctx, s.ext.Mirror.Hosts(ctx)) +} + +// waitingSessionsIn is waitingSessions over hs, ctx's principal's hosts as +// one read of the hub: a caller that also shows the hosts (the attention +// summary) passes the SAME snapshot, so a host flapping between two reads +// cannot be online in one and offline in the other. +func (s *Server) waitingSessionsIn(ctx context.Context, hs []mirror.HostView) []attentionItem { if s.ext.Mirror == nil { return nil } @@ -343,7 +354,6 @@ func (s *Server) waitingSessions(ctx context.Context) []attentionItem { } } } - hs := s.ext.Mirror.Hosts(ctx) type wait struct { since int64 q *askq.Entry @@ -522,6 +532,16 @@ func (s *Server) pendingDraftItems(ctx context.Context) ([]attentionItem, error) // A source that cannot be read is left out and named in the error; the rest // are still returned. func (s *Server) attentionItems(ctx context.Context) ([]attentionItem, error) { + var hs []mirror.HostView + if s.ext.Mirror != nil { + hs = s.ext.Mirror.Hosts(ctx) + } + return s.attentionItemsIn(ctx, hs) +} + +// attentionItemsIn is attentionItems with its waiting sessions taken from hs, +// one read of ctx's principal's hosts (waitingSessionsIn). +func (s *Server) attentionItemsIn(ctx context.Context, hs []mirror.HostView) ([]attentionItem, error) { var out []attentionItem var errs []error if s.ext.Attention != nil { @@ -538,7 +558,7 @@ func (s *Server) attentionItems(ctx context.Context) ([]attentionItem, error) { out = append(out, askItem(a)) } } - out = append(out, s.waitingSessions(ctx)...) + out = append(out, s.waitingSessionsIn(ctx, hs)...) drafts, err := s.pendingDraftItems(ctx) if err != nil { errs = append(errs, fmt.Errorf("reply drafts: %w", err)) diff --git a/internal/api/attention_summary.go b/internal/api/attention_summary.go index 9f9e09716..6b6072af8 100644 --- a/internal/api/attention_summary.go +++ b/internal/api/attention_summary.go @@ -97,14 +97,17 @@ func (s *Server) handleAttentionSummary(w http.ResponseWriter, r *http.Request) s.writeJSON(w, http.StatusForbidden, map[string]any{"error": "the attention summary is a user's own; this credential names none"}) return } - items, err := s.attentionItems(ctx) - if err != nil { - s.logger.Printf("attention: summary: %v", err) - } + // ONE read of the hub, for the items' waiting sessions and the hosts both: + // a host that flaps between two reads would otherwise be offline in one + // and online in the other. var hosts []mirror.HostView if s.ext.Mirror != nil { hosts = s.ext.Mirror.Hosts(ctx) } + items, err := s.attentionItemsIn(ctx, hosts) + if err != nil { + s.logger.Printf("attention: summary: %v", err) + } w.Header().Set("Cache-Control", "no-store") s.writeJSON(w, http.StatusOK, s.attentionSummaryOf(owner, items, err != nil, hosts, s.now())) } diff --git a/internal/api/attention_summary_test.go b/internal/api/attention_summary_test.go index 7d12d8dbf..a2968c700 100644 --- a/internal/api/attention_summary_test.go +++ b/internal/api/attention_summary_test.go @@ -240,3 +240,39 @@ func TestASummaryThatCouldNotReadTheQueueSaysSo(t *testing.T) { } } } + +// hostReadCounter is fakeMirror counting its Hosts reads. +type hostReadCounter struct { + *fakeMirror + reads int +} + +func (c *hostReadCounter) Hosts(ctx context.Context) []mirror.HostView { + c.reads++ + return c.fakeMirror.Hosts(ctx) +} + +// TestTheSummaryReadsTheHubOnce: the items' waiting sessions and the hosts +// come from ONE Hosts read, so a host flapping between two reads cannot be +// online in hosts[] while its item says offline. The control: the summary is +// served, with the fake host and its waiting window both on it. +func TestTheSummaryReadsTheHubOnce(t *testing.T) { + f := newFakeAPITokens() + tok := f.add(t, 1) + m := &hostReadCounter{fakeMirror: &fakeMirror{online: true, statusReport: true, windows: attnWindows(mirror.StatusWaiting)}} + s := New(nil, AuthConfig{HookToken: tokTestHook}, log.New(io.Discard, "", 0)) + s.UseExtensions(Extensions{APITokens: f, Users: stubUsers{}, Mirror: m}) + mux := http.NewServeMux() + s.registerAll(mux) + r := httptest.NewRequest(http.MethodGet, attentionSummaryPath, nil) + r.Header.Set("Authorization", "Bearer "+tok) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, r) + var body attentionSummary + if err := json.Unmarshal(rec.Body.Bytes(), &body); rec.Code != 200 || err != nil || len(body.Hosts) != 1 || len(body.Items) != 1 { + t.Fatalf("CONTROL: the summary %d %s (%v); want one host and its waiting window", rec.Code, rec.Body, err) + } + if m.reads != 1 { + t.Fatalf("one summary read the hub %d times; want 1", m.reads) + } +} diff --git a/internal/db/migrations/0068_api_token_scope.sql b/internal/db/migrations/0068_api_token_scope.sql index 6f9bacbc8..1effd03f1 100644 --- a/internal/db/migrations/0068_api_token_scope.sql +++ b/internal/db/migrations/0068_api_token_scope.sql @@ -14,10 +14,24 @@ -- The column inherits the table's ownership, grants and row-level security -- (0009's user_isolation); no policy changes, nothing for db.RLSOn to re-apply. -- +-- 🔴 ROLLOUT: MINT 'summary' TOKENS ONLY AFTER THE ROLLOUT HAS COMPLETED. An +-- image from before 0068 never reads scope, so while old and new pods both +-- serve (a RollingUpdate with maxUnavailable 0 keeps an old pod up until the +-- new one is ready), a 'summary' token minted mid-rollout has FULL access on +-- every request an old pod answers, until the last old pod is gone. +-- -- 🔴 Rollback: an image from before 0068 never reads scope, so it would admit -- a 'summary' token with FULL access. Revoke every live narrower token first, --- then drop the column: +-- then drop the column. api_tokens is under FORCED row-level security, which +-- binds the table's OWNER too: as muster_app, or as the owner, without +-- muster.scope = 'all' the UPDATE matches NO rows and says nothing. Run it in +-- one transaction that sets the scope first (what the migration runner sets, +-- internal/db/scope.go), and check the count before committing: +-- BEGIN; +-- SET LOCAL muster.scope = 'all'; -- UPDATE public.api_tokens SET revoked_at = now() WHERE scope <> 'full' AND revoked_at IS NULL; +-- SELECT count(*) FROM public.api_tokens WHERE scope <> 'full' AND revoked_at IS NULL; -- must be 0 +-- COMMIT; -- ALTER TABLE public.api_tokens DROP COLUMN scope; -- DELETE FROM schema_migrations WHERE version = 68;