carlos / platform Public

console: add the `member` verb — membership is authorization, and there was no tool to take it away

Authorization in the accounts model is "member of the account AND the account
claims the app", so a membership is access to EVERY app in that account. On the
flagship, the pre-accounts backfill swept a holder of ONE per-app grant into
ops, silently handing them promote/rollback over kass, keymail and the console.
Undoing it took hand-edited SQL against a live database, because grant/revoke
only ever spoke the language of the model that had just been replaced.

  console member -add <address> <sqid> [-role owner|member]
  console member -remove <address> <sqid>
  console member -list <sqid>

Box-side like grant/revoke: no session, no browser, no signed-in operator —
it is the tool you reach for when authorization itself is what is wrong.
-role defaults to member (the safe role is the one you get by forgetting the
flag). The last-owner guard is surfaced from the store rather than duplicated.
-add is idempotent and says which of add/re-role/nothing it did; removing a
non-member is a no-op that says so, so a cleanup script does not stop halfway.

Flags may follow positionals (parseInterspersed): Go's flag package stops at
the first non-flag word, which would have made `-role owner` at the end of the
line silently ignored — the one flag whose omission matters most.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude 2ab8080f11fbab4eb2aacb2fb58c186214e864b0 parent 5b4e8ff
5 files changed, +460 −22
  • cmd/console/main.go +12 −1
  • cmd/console/main_test.go +62 −18
  • cmd/console/member.go +183 −0
  • cmd/console/member_test.go +195 −0
  • internal/console/server.go +8 −3
diff --git a/cmd/console/main.go b/cmd/console/main.go
index ed0e2280a..bcd5db3d3 100644
--- a/cmd/console/main.go
+++ b/cmd/console/main.go
@@ -13,6 +13,13 @@
// console grant -operator <address> make address a fleet operator
// console revoke -operator <address> remove that operator role
// console owners list every owner and their apps
+// console member -add <address> <account-sqid> [-role owner|member]
+// console member -remove <address> <account-sqid>
+// console member -list <account-sqid>
+// membership IS authorization in the
+// accounts model — a member can deploy
+// every app in the account — so adding
+// and REMOVING one is box-side authority
// console migrate-app -app <a> -to <sqid>
// re-home an app onto another account —
// the console half of
@@ -79,6 +86,8 @@ func main() {
if err != nil {
log.Fatal(err)
}
+ case "member":
+ runMember(os.Args[2:])
case "migrate-app":
migrateApp(os.Args[2:])
case "owners":
@@ -101,7 +110,9 @@ func main() {
}
func usage() {
- log.Fatal("usage: console serve | grant [-operator] <address> [<app>] | revoke [-operator] <address> [<app>] | owners | migrate-app -app <app> -to <sqid> [-from <sqid>]")
+ log.Fatal("usage: console serve | grant [-operator] <address> [<app>] | revoke [-operator] <address> [<app>] | owners | " +
+ "member -add|-remove <address> <account-sqid> [-role owner|member] | member -list <account-sqid> | " +
+ "migrate-app -app <app> -to <sqid> [-from <sqid>]")
}
// migrateApp completes the second half of `carlos accounts migrate`: that
diff --git a/cmd/console/main_test.go b/cmd/console/main_test.go
index 706d5d22c..fb1c6ae87 100644
--- a/cmd/console/main_test.go
+++ b/cmd/console/main_test.go
@@ -5,38 +5,82 @@ import (
"os/exec"
"path/filepath"
"strings"
+ "sync"
"testing"
"time"
"github.com/carlosframework/platform/internal/console"
)
+// buildConsole compiles the console binary ONCE per test run and hands every
+// test the same path: these are end-to-end verb tests (the flag parsing and
+// the exit codes are half of what is under test, and neither is reachable from
+// a unit test), and a `go build` per test is the whole cost of the package.
+var (
+ buildOnce sync.Once
+ consoleBin string
+ buildDir string
+ buildErr error
+)
+
+func buildConsole(t *testing.T) string {
+ t.Helper()
+ buildOnce.Do(func() {
+ goBin, err := exec.LookPath("go")
+ if err != nil {
+ buildErr = err
+ return
+ }
+ buildDir, buildErr = os.MkdirTemp("", "console-bin")
+ if buildErr != nil {
+ return
+ }
+ consoleBin = filepath.Join(buildDir, "console")
+ build := exec.Command(goBin, "build", "-o", consoleBin, ".")
+ build.Env = os.Environ()
+ if out, err := build.CombinedOutput(); err != nil {
+ buildErr = err
+ t.Logf("build console: %s", out)
+ }
+ })
+ if buildErr != nil {
+ t.Skipf("cannot build the console binary: %v", buildErr)
+ }
+ return consoleBin
+}
+
+// runner returns a "run these args against the built binary" helper with env
+// pinned — combined output plus the error, because a verb's refusal is as
+// much under test as its success.
+func runner(t *testing.T, bin string, env ...string) func(args ...string) (string, error) {
+ full := append(os.Environ(), env...)
+ return func(args ...string) (string, error) {
+ t.Helper()
+ cmd := exec.Command(bin, args...)
+ cmd.Env = full
+ out, err := cmd.CombinedOutput()
+ return string(out), err
+ }
+}
+
+func TestMain(m *testing.M) {
+ code := m.Run()
+ if buildDir != "" {
+ os.RemoveAll(buildDir)
+ }
+ os.Exit(code)
+}
+
// migrate-app is the CONSOLE half of `carlos accounts migrate`. It lives here
// rather than in the carlos binary on purpose: the console's app→account row
// is console state, and the carlos CLI has no business opening the console's
// database — box-side authority changes go through this binary, exactly like
// `console grant`.
func TestMigrateAppVerb(t *testing.T) {
- goBin, err := exec.LookPath("go")
- if err != nil {
- t.Skip("go toolchain not found")
- }
+ bin := buildConsole(t)
tmp := t.TempDir()
- bin := filepath.Join(tmp, "console")
- build := exec.Command(goBin, "build", "-o", bin, ".")
- build.Env = os.Environ()
- if out, err := build.CombinedOutput(); err != nil {
- t.Fatalf("build console: %v\n%s", err, out)
- }
dbPath := filepath.Join(tmp, "console.db")
- env := append(os.Environ(), "CONSOLE_DB="+dbPath)
- run := func(args ...string) (string, error) {
- t.Helper()
- cmd := exec.Command(bin, args...)
- cmd.Env = env
- out, err := cmd.CombinedOutput()
- return string(out), err
- }
+ run := runner(t, bin, "CONSOLE_DB="+dbPath)
// A console in the shape a deployment is in before the migration: ops
// owns kass.
diff --git a/cmd/console/member.go b/cmd/console/member.go
new file mode 100644
index 000000000..03aed9239
--- /dev/null
+++ b/cmd/console/member.go
@@ -0,0 +1,183 @@
+package main
+
+import (
+ "flag"
+ "fmt"
+ "log"
+ "os"
+ "time"
+
+ "github.com/carlosframework/platform/internal/console"
+)
+
+// memberUsage is spelled out rather than left to flag.PrintDefaults because
+// the ORDER matters to a reader: mode first, then who, then where.
+const memberUsage = `usage: console member -add <address> <account-sqid> [-role owner|member]
+ console member -remove <address> <account-sqid>
+ console member -list <account-sqid>`
+
+// runMember is the box-side membership tool — `console grant`'s replacement in
+// the accounts world, and the answer to a gap that bit production.
+//
+// Why it has to exist: authorization is "you are a member of the account AND
+// the account claims the app". A membership is therefore access to EVERY app
+// in that account, not to one app. When the pre-accounts backfill swept
+// per-app grant holders into ops, someone holding a single grant for a toy app
+// silently acquired promote/rollback over kass, keymail and the console. The
+// only fix available at the time was hand-edited SQL against a live database,
+// because grant/revoke still spoke the language of the model that had just
+// been replaced. This verb is that missing authority.
+//
+// Box-side exactly like grant/revoke: it needs no session, no browser and no
+// signed-in operator, because it is the tool you reach for when the console's
+// own authorization is what is wrong.
+func runMember(args []string) {
+ fs := flag.NewFlagSet("member", flag.ExitOnError)
+ add := fs.Bool("add", false, "add (or re-role) a member")
+ remove := fs.Bool("remove", false, "remove a member")
+ list := fs.Bool("list", false, "list an account's members")
+ // Defaulting to member, never owner: the safe role is the one you get by
+ // forgetting the flag. An owner can invite, remove members and delete the
+ // account, so it is always a deliberate word on the command line.
+ role := fs.String("role", console.RoleMember, "role for -add: owner or member")
+ pos := parseInterspersed(fs, args)
+
+ modes := 0
+ for _, on := range []bool{*add, *remove, *list} {
+ if on {
+ modes++
+ }
+ }
+ if modes != 1 {
+ log.Fatal(memberUsage)
+ }
+ if *role != console.RoleOwner && *role != console.RoleMember {
+ log.Fatalf("unknown role %q — there are exactly two: owner or member", *role)
+ }
+
+ var address, sqid string
+ switch {
+ case *list:
+ if len(pos) != 1 {
+ log.Fatal(memberUsage)
+ }
+ sqid = pos[0]
+ default:
+ if len(pos) != 2 {
+ log.Fatal(memberUsage)
+ }
+ address, sqid = pos[0], pos[1]
+ // Same shape check the sign-in form makes: a membership for an address
+ // keymail could never authenticate is a grant nobody can claim, and a
+ // silent typo here is a member who "was added" and cannot get in.
+ if !console.ValidAddress(address) {
+ log.Fatalf("%q is not a keymail address — expected <user>@<host>, e.g. paul@keymail.dev", address)
+ }
+ }
+
+ st := openConsoleDB()
+ defer st.Close()
+
+ acct, err := st.AccountBySqid(sqid)
+ if err != nil {
+ log.Fatal(err)
+ }
+ if acct == nil {
+ log.Fatalf("no account %q: nothing with that sqid in %s — create one with: "+
+ "carlos accounts create -name <name>", sqid, envOr("CONSOLE_DB", "/data/console/console.db"))
+ }
+
+ switch {
+ case *list:
+ memberList(st, acct)
+ case *add:
+ memberAdd(st, acct, address, *role)
+ case *remove:
+ memberRemove(st, acct, address)
+ }
+}
+
+func memberList(st *console.Store, acct *console.Account) {
+ members, err := st.Members(acct.ID)
+ if err != nil {
+ log.Fatal(err)
+ }
+ if len(members) == 0 {
+ // Not an error: an account materialized from the bucket legitimately
+ // has no members here yet, and saying so is how the operator learns
+ // this verb is what gives it one.
+ fmt.Printf("%s (%s) has no members\n", acct.Sqid, acct.Name)
+ return
+ }
+ for _, m := range members {
+ fmt.Printf("%s\t%s\n", m.Name, m.Role) // Members carries the address in Name
+ }
+}
+
+// memberAdd is idempotent and SAYS which of the three things it did. An
+// operator repeating a command has to be able to tell "already done" from
+// "just done" — silence reads as success either way, and that is how a
+// membership gets added twice with two different roles.
+func memberAdd(st *console.Store, acct *console.Account, address, role string) {
+ current, err := st.Role(acct.ID, address)
+ if err != nil {
+ log.Fatal(err)
+ }
+ if current == role {
+ fmt.Printf("%s is already a %s of %s (%s) — nothing to do\n", address, role, acct.Sqid, acct.Name)
+ return
+ }
+ // The last-owner guard lives in the store, inside the transaction that
+ // does the write — the count and the write cannot be separated by a
+ // concurrent second demotion. Surfaced here verbatim rather than
+ // re-checked: two copies of a safety rule is one copy too many.
+ if err := st.AddMember(acct.ID, address, role, time.Now()); err != nil {
+ log.Fatal(err)
+ }
+ if current == "" {
+ fmt.Printf("added %s to %s (%s) as %s\n", address, acct.Sqid, acct.Name, role)
+ return
+ }
+ fmt.Printf("changed %s in %s (%s) from %s to %s\n", address, acct.Sqid, acct.Name, current, role)
+}
+
+// memberRemove is the verb the flagship needed and did not have. Removing
+// somebody who is not a member is a no-op that says so rather than an error:
+// the operator's intent — "they must not be in this account" — is already
+// satisfied, and failing would make a cleanup script stop halfway.
+func memberRemove(st *console.Store, acct *console.Account, address string) {
+ current, err := st.Role(acct.ID, address)
+ if err != nil {
+ log.Fatal(err)
+ }
+ if current == "" {
+ fmt.Printf("%s is not a member of %s (%s) — nothing to do\n", address, acct.Sqid, acct.Name)
+ return
+ }
+ if err := st.RemoveMember(acct.ID, address); err != nil {
+ log.Fatal(err)
+ }
+ fmt.Printf("removed %s (%s) from %s (%s)\n", address, current, acct.Sqid, acct.Name)
+}
+
+// parseInterspersed parses flags that may appear AFTER positional arguments.
+// Go's flag package stops at the first non-flag word, which would make
+// `console member -add alice@keymail.dev bab -role owner` silently ignore the
+// role — the one flag whose omission matters most. Re-parsing what is left
+// after each positional accepts both orders instead of quietly doing the
+// wrong thing.
+func parseInterspersed(fs *flag.FlagSet, args []string) []string {
+ var pos []string
+ rest := args
+ for {
+ if err := fs.Parse(rest); err != nil {
+ os.Exit(2) // ExitOnError already printed; unreachable in practice
+ }
+ rest = fs.Args()
+ if len(rest) == 0 {
+ return pos
+ }
+ pos = append(pos, rest[0])
+ rest = rest[1:]
+ }
+}
diff --git a/cmd/console/member_test.go b/cmd/console/member_test.go
new file mode 100644
index 000000000..b5a46d7b9
--- /dev/null
+++ b/cmd/console/member_test.go
@@ -0,0 +1,195 @@
+package main_test
+
+import (
+ "path/filepath"
+ "strings"
+ "testing"
+ "time"
+
+ "github.com/carlosframework/platform/internal/console"
+)
+
+// `console member` is the box-side membership tool, and it exists because
+// there was none. Authorization is "member of the account AND the account has
+// the app", so a membership is access to EVERY app in the account — on the
+// flagship, a backfilled membership handed someone who held one grant for a
+// toy app the right to promote and roll back kass, keymail and the console
+// itself. Removing it took hand-edited SQL, because grant/revoke only ever
+// spoke the pre-accounts language. This verb is the missing half.
+func TestMemberVerb(t *testing.T) {
+ bin := buildConsole(t)
+ tmp := t.TempDir()
+ dbPath := filepath.Join(tmp, "console.db")
+ run := runner(t, bin, "CONSOLE_DB="+dbPath)
+
+ // Two accounts: ops with two owners (so one can be removed), and a
+ // single-owner account to prove the last-owner guard.
+ st, err := console.Open(dbPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ now := time.Now()
+ if err := st.CreateAccount(console.OpsAccountID, "bab", "ops", "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ if err := st.CreateAccount(42, "cdef", "Kass", "solo@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ st.Close()
+
+ // -list shows who is in an account and with what role.
+ out, err := run("member", "-list", "bab")
+ if err != nil {
+ t.Fatalf("member -list: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "paul@keymail.dev") || !strings.Contains(out, "owner") {
+ t.Errorf("member -list does not show the owner: %s", out)
+ }
+
+ // -add defaults to member: the safe role. An owner is a deliberate flag.
+ if out, err := run("member", "-add", "dev@keymail.dev", "bab"); err != nil {
+ t.Fatalf("member -add: %v\n%s", err, out)
+ } else if !strings.Contains(out, "member") {
+ t.Errorf("member -add does not report the role it used: %s", out)
+ }
+ if role := roleOf(t, dbPath, console.OpsAccountID, "dev@keymail.dev"); role != console.RoleMember {
+ t.Fatalf("role after -add = %q, want member", role)
+ }
+
+ // Idempotent, and it SAYS so — an operator repeating a command must be
+ // able to tell "already done" from "just done".
+ out, err = run("member", "-add", "dev@keymail.dev", "bab")
+ if err != nil {
+ t.Fatalf("re-adding must be a no-op: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "already") {
+ t.Errorf("re-add output does not say it was already done: %s", out)
+ }
+
+ // Adding an existing member with a different role RE-ROLES them, and says
+ // that is what happened rather than silently doing nothing.
+ out, err = run("member", "-add", "dev@keymail.dev", "bab", "-role", "owner")
+ if err != nil {
+ t.Fatalf("re-role: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "owner") || strings.Contains(out, "already") {
+ t.Errorf("re-role output does not report the change: %s", out)
+ }
+ if role := roleOf(t, dbPath, console.OpsAccountID, "dev@keymail.dev"); role != console.RoleOwner {
+ t.Fatalf("role after re-role = %q, want owner", role)
+ }
+
+ // The removal the flagship needed and did not have.
+ if out, err := run("member", "-remove", "dev@keymail.dev", "bab"); err != nil {
+ t.Fatalf("member -remove: %v\n%s", err, out)
+ }
+ if role := roleOf(t, dbPath, console.OpsAccountID, "dev@keymail.dev"); role != "" {
+ t.Fatalf("role after -remove = %q, want none", role)
+ }
+
+ // Removing a non-member is a no-op that says so, not an error: the
+ // operator's intent ("they must not be in this account") is satisfied.
+ out, err = run("member", "-remove", "dev@keymail.dev", "bab")
+ if err != nil {
+ t.Fatalf("removing a non-member must not fail: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "not a member") {
+ t.Errorf("no-op removal does not say why: %s", out)
+ }
+}
+
+// An account with nobody who can administer it is unrecoverable: no one left
+// to invite, rename or delete. The store guard already refuses it; the verb's
+// job is to surface that refusal instead of swallowing it.
+func TestMemberVerbRefusesToStrandAnAccount(t *testing.T) {
+ bin := buildConsole(t)
+ tmp := t.TempDir()
+ dbPath := filepath.Join(tmp, "console.db")
+ run := runner(t, bin, "CONSOLE_DB="+dbPath)
+
+ st, err := console.Open(dbPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := st.CreateAccount(42, "cdef", "Kass", "solo@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ st.Close()
+
+ out, err := run("member", "-remove", "solo@keymail.dev", "cdef")
+ if err == nil {
+ t.Errorf("removing the last owner succeeded: %s", out)
+ }
+ if !strings.Contains(out, "last owner") {
+ t.Errorf("refusal does not explain itself: %s", out)
+ }
+ out, err = run("member", "-add", "solo@keymail.dev", "cdef", "-role", "member")
+ if err == nil {
+ t.Errorf("demoting the last owner succeeded: %s", out)
+ }
+ if !strings.Contains(out, "last owner") {
+ t.Errorf("demotion refusal does not explain itself: %s", out)
+ }
+ if role := roleOf(t, dbPath, 42, "solo@keymail.dev"); role != console.RoleOwner {
+ t.Errorf("the last owner survived as %q, want owner", role)
+ }
+}
+
+// A typo must name what it looked for, not just fail. These commands get run
+// at 2am against a production box.
+func TestMemberVerbRejectsBadArguments(t *testing.T) {
+ bin := buildConsole(t)
+ tmp := t.TempDir()
+ dbPath := filepath.Join(tmp, "console.db")
+ run := runner(t, bin, "CONSOLE_DB="+dbPath)
+
+ st, err := console.Open(dbPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := st.CreateAccount(console.OpsAccountID, "bab", "ops", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ st.Close()
+
+ for _, tc := range []struct {
+ name string
+ args []string
+ want []string
+ }{
+ {"unknown account", []string{"member", "-add", "dev@keymail.dev", "nosuch"},
+ []string{"nosuch", "carlos accounts create"}},
+ {"malformed address", []string{"member", "-add", "dev", "bab"},
+ []string{"dev", "@"}},
+ {"unknown role", []string{"member", "-add", "dev@keymail.dev", "bab", "-role", "admin"},
+ []string{"admin", "owner", "member"}},
+ {"no mode", []string{"member", "dev@keymail.dev", "bab"},
+ []string{"-add", "-remove", "-list"}},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ out, err := run(tc.args...)
+ if err == nil {
+ t.Fatalf("%v succeeded: %s", tc.args, out)
+ }
+ for _, want := range tc.want {
+ if !strings.Contains(out, want) {
+ t.Errorf("error does not mention %q: %s", want, out)
+ }
+ }
+ })
+ }
+}
+
+func roleOf(t *testing.T, dbPath string, accountID int64, address string) string {
+ t.Helper()
+ st, err := console.Open(dbPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer st.Close()
+ role, err := st.Role(accountID, address)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return role
+}
diff --git a/internal/console/server.go b/internal/console/server.go
index be121fcef..6d1045044 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -496,7 +496,7 @@ func (s *Server) handleInvite(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "/a/"+url.PathEscape(acct.Sqid)+"/", http.StatusSeeOther)
}
to := strings.ToLower(strings.TrimSpace(r.FormValue("address")))
- if !validAddress(to) {
+ if !ValidAddress(to) {
back("err", "That doesn't look like a keymail address.")
return
}
@@ -607,7 +607,7 @@ func (s *Server) handleContinue(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "/auth/signin", http.StatusSeeOther)
return
}
- if !validAddress(addr) {
+ if !ValidAddress(addr) {
p := page{Title: "Sign in", Error: "That doesn't look like a keymail address — expected something like you@keymail.dev."}
s.render(w, http.StatusOK, "home", p)
return
@@ -638,7 +638,12 @@ func (s *Server) handleContinue(w http.ResponseWriter, r *http.Request) {
s.render(w, http.StatusOK, "check", p)
}
-func validAddress(a string) bool {
+// ValidAddress is the console's one shape check on a keymail address:
+// <local>@<host>, no whitespace. Exported because the box-side verbs
+// (`console member`) must reject a typo with the SAME rule the web surface
+// uses — a membership created from the command line for an address the sign-in
+// form would refuse is a grant nobody can ever claim.
+func ValidAddress(a string) bool {
i := strings.LastIndex(a, "@")
return i > 0 && i < len(a)-1 && !strings.ContainsAny(a, " \t\r\n")
}