Fix security around command parsing

internal/core/console.go

@@ -21,6 +21,9 @@ import (
 
 const (
 	consoleIdleTimeout = 5 * time.Minute
+	// maxCommandLen bounds a single CLI command; MeshCore commands are short and
+	// a LoRa frame is tiny, so anything longer is malformed/abusive.
+	maxCommandLen = 200
 	// sendInterval is the minimum spacing between our LoRa transmissions, so a
 	// user can't flood the shared mesh through their modem.
 	sendInterval = time.Second
@@ -35,26 +38,81 @@ func commandPrefix(template string) string {
 	return strings.TrimSpace(template)
 }
 
-// resolveCommand maps typed CLI text to its catalog command by longest matching
-// literal prefix, so "set tx 20" resolves to set.tx (not a bare "set").
+// arityVariadic marks a command that accepts any number of arguments (e.g. a
+// rest-of-line free-text arg like "set name <text>", or "region def …").
+const arityVariadic = -1
+
+// resolveCommand maps typed CLI text to the exact catalog command the firmware
+// would run, matched by command TOKEN and argument count (arity). It returns nil
+// when no command matches — callers MUST treat nil as "deny", never as "send
+// anyway".
+//
+// Security model: authorization is by the exact (token, arity) tuple, never a
+// loose prefix. A command's token is the literal words before its first "<arg>"
+// (e.g. "set tx", "region put", "setperm"); arity is the count of remaining
+// whitespace tokens, or -1 for variadic. The firmware (a) runs exactly one
+// command per message — no ';'/newline chaining — and (b) tokenizes on the same
+// whitespace and overloads commands by arg count (e.g. setperm/2 sets a
+// permission, setperm/1 removes it). Matching the same way means an authorized
+// command can never be re-interpreted by the device as a different, ungranted
+// one. The longest matching token wins ("region put" over "region"), so a
+// shorter command can't shadow a more specific one. Args themselves are not
+// constrained — granting a command grants all of its argument values.
 func resolveCommand(typed string, catalog []*store.Command) *store.Command {
-	typed = strings.TrimSpace(typed)
+	fields := strings.Fields(typed)
+	if len(fields) == 0 {
+		return nil
+	}
 	var best *store.Command
-	bestLen := -1
+	bestTokenLen := -1
 	for _, c := range catalog {
-		prefix := commandPrefix(c.Template)
-		if prefix == "" {
+		token := strings.Fields(commandPrefix(c.Template))
+		if len(token) == 0 || len(token) > len(fields) {
 			continue
 		}
-		if typed == prefix || strings.HasPrefix(typed, prefix+" ") {
-			if len(prefix) > bestLen {
-				best, bestLen = c, len(prefix)
-			}
+		if !equalWords(fields[:len(token)], token) {
+			continue
+		}
+		argc := len(fields) - len(token)
+		if c.Arity != arityVariadic && argc != c.Arity {
+			continue
+		}
+		if len(token) > bestTokenLen {
+			best, bestTokenLen = c, len(token)
 		}
 	}
 	return best
 }
 
+func equalWords(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}
+
+// validCommandText reports whether s is a single line of printable text — the
+// only shape a CLI command can legitimately take. Rejecting control characters
+// (newlines especially) is defense in depth: the firmware runs one command per
+// message today, but we never want to forward bytes that could split or be
+// re-interpreted, and it keeps the command log/echo clean.
+func validCommandText(s string) bool {
+	if strings.TrimSpace(s) == "" || len(s) > maxCommandLen {
+		return false
+	}
+	for _, r := range s {
+		if r < 0x20 || r == 0x7f {
+			return false
+		}
+	}
+	return true
+}
+
 // allowedCommands returns the catalog commands the user may run on the repeater:
 // all of them for the owner, else the share-granted subset.
 func (s *Handlers) allowedCommands(ctx context.Context, rep *store.Repeater, userID int64, catalog []*store.Command) []*store.Command {
@@ -233,6 +291,10 @@ func (s *Handlers) wsConsole(w http.ResponseWriter, r *http.Request) {
 
 	runCommand := func(text string) {
 		idle.Reset(consoleIdleTimeout)
+		if !validCommandText(text) {
+			_ = bridge.Status("denied", "Invalid command.")
+			return
+		}
 		cmd := resolveCommand(text, catalog)
 		if cmd == nil {
 			_ = bridge.Status("denied", "Unknown command: "+text)

internal/core/console_resolve_test.go

@@ -0,0 +1,213 @@
+package core
+
+import (
+	"context"
+	"fmt"
+	"net/url"
+	"os"
+	"regexp"
+	"strings"
+	"testing"
+
+	"github.com/jleight/meshtender/internal/store"
+)
+
+// cmd builds a catalog entry; resolveCommand only reads Template and Arity, and
+// Key is carried for readable assertions.
+func cmd(key, template string, arity int) *store.Command {
+	return &store.Command{Key: key, Template: template, Arity: arity}
+}
+
+// resolveTestCatalog mirrors the real catalog's tricky cases: overloaded tokens
+// (setperm, region put), prefix-related tokens (region vs region get/put),
+// dotted sub-keys (set flood.max vs set flood.max.advert), comma-arg commands
+// (set radio = one token), and variadic free-text (set name).
+func resolveTestCatalog() []*store.Command {
+	return []*store.Command{
+		cmd("ver", "ver", 0),
+		cmd("advert", "advert", 0),
+		cmd("advert.zerohop", "advert.zerohop", 0),
+		cmd("erase", "erase", 0),
+		cmd("setperm.set", "setperm <pubkey> <level>", 2),
+		cmd("setperm.remove", "setperm <pubkey>", 1),
+		cmd("set.tx", "set tx <0-22>", 1),
+		cmd("set.radio", "set radio <f,bw,sf,cr>", 1),
+		cmd("set.name", "set name <text>", arityVariadic),
+		cmd("set.flood_max", "set flood.max <n>", 1),
+		cmd("set.flood_max_advert", "set flood.max.advert <n>", 1),
+		cmd("region", "region", 0),
+		cmd("region.get", "region get <name>", 1),
+		cmd("region.put_root", "region put <name>", 1),
+		cmd("region.put_sub", "region put <name> <parent>", 2),
+		cmd("region.def", "region def <tokens>", arityVariadic),
+		cmd("region.home_get", "region home", 0),
+		cmd("region.home_set", "region home <name>", 1),
+	}
+}
+
+func TestResolveCommand(t *testing.T) {
+	cat := resolveTestCatalog()
+	cases := []struct {
+		typed, want string // want == "" means must be denied (nil)
+	}{
+		// Exact no-arg commands; extra tokens must NOT be swallowed.
+		{"ver", "ver"},
+		{"ver extra", ""},
+		{"advert", "advert"},
+		{"advert.zerohop", "advert.zerohop"},
+		{"advert evil", ""}, // no args allowed on a 0-arity command
+		{"erase", "erase"},
+
+		// setperm overloaded by arity: 2 = set, 1 = remove, anything else denied.
+		{"setperm KEY 3", "setperm.set"},
+		{"setperm KEY", "setperm.remove"},
+		{"setperm KEY 3 junk", ""},
+		{"setperm", ""},
+
+		// Single-arg vs missing/extra args.
+		{"set tx 20", "set.tx"},
+		{"set tx", ""},
+		{"set tx 20 30", ""},
+
+		// Comma-arg radio is ONE whitespace token (arity 1); space form is denied
+		// (the firmware parses it comma-separated, so the space form is invalid).
+		{"set radio 868,250,11,5", "set.radio"},
+		{"set radio 868 250 11 5", ""},
+
+		// Variadic free-text takes any number of trailing tokens.
+		{"set name My Cool Repeater", "set.name"},
+		{"set name", "set.name"},
+
+		// Dotted sub-keys are distinct words — no prefix collision.
+		{"set flood.max 5", "set.flood_max"},
+		{"set flood.max.advert 5", "set.flood_max_advert"},
+
+		// region: bare is a 0-arg read; "region <unknown>" must be DENIED rather
+		// than falling through to the bare read (the catch-all bug this prevents).
+		{"region", "region"},
+		{"region foo", ""},
+		{"region get EU", "region.get"},
+		{"region get", ""},
+
+		// Longest token wins: "region put" over "region"; arity picks root vs sub.
+		{"region put EU", "region.put_root"},
+		{"region put EU NA", "region.put_sub"},
+		{"region put EU NA X", ""},
+		{"region def A B C", "region.def"},
+
+		// Optional-arg get/set split by arity.
+		{"region home", "region.home_get"},
+		{"region home EU", "region.home_set"},
+
+		// Whitespace is normalized.
+		{"setperm   KEY   3", "setperm.set"},
+		{"  set tx 20  ", "set.tx"},
+
+		// Total nonsense.
+		{"totally bogus", ""},
+		{"", ""},
+	}
+	for _, tc := range cases {
+		got := resolveCommand(tc.typed, cat)
+		gotKey := ""
+		if got != nil {
+			gotKey = got.Key
+		}
+		if gotKey != tc.want {
+			t.Errorf("resolveCommand(%q) = %q, want %q", tc.typed, gotKey, tc.want)
+		}
+	}
+}
+
+func TestValidCommandText(t *testing.T) {
+	ok := []string{"ver", "set tx 20", "set name My Repeater", "setperm abc 3"}
+	for _, s := range ok {
+		if !validCommandText(s) {
+			t.Errorf("validCommandText(%q) = false, want true", s)
+		}
+	}
+	bad := []string{
+		"",
+		"   ",
+		"ver\nreboot",     // newline (chaining attempt)
+		"ver\rreboot",     // carriage return
+		"set name a\x00b", // null
+		"cmd\x7f",         // delete
+		strings.Repeat("a", maxCommandLen+1), // too long
+	}
+	for _, s := range bad {
+		if validCommandText(s) {
+			t.Errorf("validCommandText(%q) = true, want false", s)
+		}
+	}
+}
+
+// TestResolveCommandRealCatalog enforces, against the actual seeded catalog, the
+// two invariants the console parser's safety depends on:
+//  1. No two commands share a (token, arity) — resolution is never ambiguous.
+//  2. Every fixed-arity command's arity equals its template's "<arg>" count, and
+//     each command's own template round-trips back to itself through the parser.
+//
+// Requires the *_test database (it runs migrations).
+func TestResolveCommandRealCatalog(t *testing.T) {
+	dsn := os.Getenv("MESHTENDER_TEST_DATABASE_URL")
+	if dsn == "" {
+		t.Skip("set MESHTENDER_TEST_DATABASE_URL to run this integration test")
+	}
+	if u, err := url.Parse(dsn); err != nil || !strings.HasSuffix(strings.TrimPrefix(u.Path, "/"), "_test") {
+		t.Fatalf("refusing to run: test DB name must end in _test (got %q)", dsn)
+	}
+	ctx := context.Background()
+	st, err := store.New(ctx, dsn)
+	if err != nil {
+		t.Fatalf("store: %v", err)
+	}
+	defer st.Close()
+	if err := st.Migrate(ctx); err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+	cat, err := st.ListCommands(ctx)
+	if err != nil {
+		t.Fatalf("list commands: %v", err)
+	}
+	if len(cat) == 0 {
+		t.Fatal("empty catalog")
+	}
+
+	argGroup := regexp.MustCompile(`<[^>]*>`)
+	seen := map[string]string{} // (token|arity) -> key
+	for _, c := range cat {
+		token := commandPrefix(c.Template)
+		groups := len(argGroup.FindAllString(c.Template, -1))
+
+		// (1) uniqueness of (token, arity)
+		k := fmt.Sprintf("%q|%d", token, c.Arity)
+		if other, dup := seen[k]; dup {
+			t.Errorf("commands %q and %q share token %q + arity %d (ambiguous)", other, c.Key, token, c.Arity)
+		}
+		seen[k] = c.Key
+
+		// (2) fixed arity must match the template's arg-group count
+		if c.Arity >= 0 && groups != c.Arity {
+			t.Errorf("command %q: arity %d but template %q has %d <arg> groups", c.Key, c.Arity, c.Template, groups)
+		}
+
+		// (3) the command's own template round-trips back to itself
+		n := c.Arity
+		if n < 0 {
+			n = 1 // variadic: one sample arg
+		}
+		sample := token
+		for i := 0; i < n; i++ {
+			sample += " x"
+		}
+		got := resolveCommand(sample, cat)
+		if got == nil || got.Key != c.Key {
+			gk := "<nil>"
+			if got != nil {
+				gk = got.Key
+			}
+			t.Errorf("round-trip: %q (for %q) resolved to %q", sample, c.Key, gk)
+		}
+	}
+}

internal/store/commands.go

@@ -9,23 +9,28 @@ import (
 
 // Command is a row in the firmware command catalog.
 type Command struct {
-	ID                 int64
-	Key                string
-	Template           string
-	Category           string
-	Args               string
+	ID       int64
+	Key      string
+	Template string
+	Category string
+	Args     string
+	// Arity is the exact number of whitespace-separated argument tokens the
+	// command takes (after its command token). -1 means variadic / rest-of-line
+	// (e.g. "set name <text>"). The console parser authorizes by (token, arity).
+	Arity              int
+	Description        string
 	Risky              bool
 	InShareDefault     bool
 	InOrgMemberDefault bool
 	InOrgAdminDefault  bool
 }
 
-const commandCols = `id, key, template, category, args, risky,
+const commandCols = `id, key, template, category, args, arity, description, risky,
 	in_share_default, in_org_member_default, in_org_admin_default`
 
 func scanCommand(row pgx.Row) (*Command, error) {
 	var c Command
-	err := row.Scan(&c.ID, &c.Key, &c.Template, &c.Category, &c.Args, &c.Risky,
+	err := row.Scan(&c.ID, &c.Key, &c.Template, &c.Category, &c.Args, &c.Arity, &c.Description, &c.Risky,
 		&c.InShareDefault, &c.InOrgMemberDefault, &c.InOrgAdminDefault)
 	if err != nil {
 		return nil, err

internal/store/migrations/0017_more_commands.sql

@@ -0,0 +1,227 @@
+-- +goose Up
+-- Two things here:
+--   1. Add `arity` + `description` to the command catalog. `arity` is the exact
+--      number of whitespace-separated argument tokens a command takes (-1 =
+--      variadic / rest-of-line, e.g. "set name <text>"). The console parser
+--      authorizes by the exact (command-token, arity) tuple — this is the
+--      security boundary, so the arities below must match how the firmware
+--      tokenizes (CommonCLI.cpp: whitespace, and commas WITHIN a single token
+--      for "set radio"/"tempradio" via parseTextParts(',')).
+--   2. Add the commands the 0003 seed missed — setperm and the shared CommonCLI
+--      get/set and region subcommands — sourced from docs.meshcore.io and the
+--      firmware. Commands the firmware overloads by argument count (setperm,
+--      region put/home/default) become separate rows per arity.
+--
+-- Levels for setperm on a repeater: 0 = guest, 3 = admin (1/2 are room-server
+-- only); `setperm <pubkey>` with no level removes the entry.
+--
+-- Omitted as board/build-specific: bridge.* (RS232/ESP-NOW), pwrmgt.*/
+-- bootloader.ver (NRF52 only).
+
+ALTER TABLE command_catalog ADD COLUMN arity       INTEGER NOT NULL DEFAULT 0;
+ALTER TABLE command_catalog ADD COLUMN description TEXT    NOT NULL DEFAULT '';
+
+-- Backfill arity + description for the 0003 seed (region and tempradio handled
+-- separately below since their template/category also change).
+UPDATE command_catalog c SET arity = v.arity, description = v.descr
+FROM (VALUES
+  ('ver',                  0, 'Returns the firmware version and build date'),
+  ('board',                0, 'Shows the board/hardware model'),
+  ('clock',                0, 'Shows the current UTC time'),
+  ('neighbors',            0, 'Lists recently seen neighbor nodes'),
+  ('stats-packets',        0, 'Shows packet counters (serial only)'),
+  ('stats-radio',          0, 'Shows radio stats: noise floor, RSSI, SNR, airtime (serial only)'),
+  ('stats-core',           0, 'Shows core stats: battery, uptime, queues (serial only)'),
+  ('sensor.list',         -1, 'Lists available sensors and values (optional start index)'),
+  ('sensor.get',           1, 'Reads a sensor value by key'),
+  ('advert',               0, 'Sends a flood advertisement'),
+  ('advert.zerohop',       0, 'Sends a zero-hop (local) advertisement'),
+  ('clock.sync',           0, 'Syncs the clock to the sender''s time'),
+  ('time',                 1, 'Sets the clock to a Unix epoch time'),
+  ('reboot',               0, 'Reboots the node'),
+  ('clkreboot',            0, 'Resets the clock and reboots'),
+  ('clear-stats',          0, 'Resets all statistics counters'),
+  ('neighbor.remove',      1, 'Removes neighbors matching a public-key prefix'),
+  ('powersaving.on',       0, 'Enables power saving'),
+  ('powersaving.off',      0, 'Disables power saving'),
+  ('log.start',            0, 'Starts capturing the RX packet log'),
+  ('log.stop',             0, 'Stops capturing the RX packet log'),
+  ('log.erase',            0, 'Erases the captured packet log'),
+  ('gps.on',               0, 'Enables GPS'),
+  ('gps.off',              0, 'Disables GPS'),
+  ('gps.sync',             0, 'Syncs the clock from GPS'),
+  ('gps.setloc',           0, 'Sets the node location from the current GPS fix'),
+  ('gps.advert',           0, 'Reads the GPS location advert policy'),
+  ('sensor.set',           2, 'Sets a sensor value'),
+  ('get.tx',               0, 'Reads the TX power (dBm)'),
+  ('get.freq',             0, 'Reads the frequency (MHz)'),
+  ('get.radio',            0, 'Reads the radio params (freq, bw, sf, cr)'),
+  ('get.name',             0, 'Reads the node name'),
+  ('get.repeat',           0, 'Reads the repeat (forwarding) state'),
+  ('get.role',             0, 'Reads the node role'),
+  ('get.public_key',       0, 'Reads the node public key'),
+  ('get.lat',              0, 'Reads the latitude'),
+  ('get.lon',              0, 'Reads the longitude'),
+  ('get.advert_interval',  0, 'Reads the zero-hop advert interval'),
+  ('set.tx',               1, 'Sets the TX power (dBm)'),
+  ('set.freq',             1, 'Sets the frequency (MHz); requires reboot'),
+  ('set.radio',            1, 'Sets radio params freq,bw,sf,cr (comma-separated); requires reboot'),
+  ('set.name',            -1, 'Sets the node name'),
+  ('set.repeat',           1, 'Enables/disables packet repeating (on/off)'),
+  ('set.lat',              1, 'Sets the latitude (degrees)'),
+  ('set.lon',              1, 'Sets the longitude (degrees)'),
+  ('set.rxdelay',          1, 'Sets the RX delay base (ms)'),
+  ('set.txdelay',          1, 'Sets the flood TX delay factor'),
+  ('set.direct_txdelay',   1, 'Sets the direct TX delay factor'),
+  ('set.advert_interval',  1, 'Sets the zero-hop advert interval (minutes)'),
+  ('set.flood_max',        1, 'Sets the max flood hop count'),
+  ('set.flood_max_advert', 1, 'Sets the max advert flood hop count'),
+  ('set.flood_max_unscoped',1,'Sets the max unscoped flood hop count'),
+  ('set.role',             1, 'Changes the node role (changes node type)'),
+  ('set.prv_key',          1, 'Sets the private key (changes identity); requires reboot'),
+  ('set.public_key',       1, 'Sets the public key (changes identity)'),
+  ('password',            -1, 'Changes the admin password'),
+  ('guest.password',      -1, 'Sets the guest/public password'),
+  ('erase',                0, 'Factory-resets and wipes the node (serial only)'),
+  ('start.ota',            0, 'Starts an over-the-air firmware update'),
+  ('poweroff',             0, 'Powers the node off'),
+  ('shutdown',             0, 'Powers the node off')
+) AS v(key, arity, descr)
+WHERE c.key = v.key;
+
+-- tempradio was missing its 5th comma field (timeout minutes); it's one
+-- whitespace token (comma-separated), so arity 1.
+UPDATE command_catalog
+   SET template = 'tempradio <f,bw,sf,cr,mins>', arity = 1,
+       description = 'Temporarily applies radio params until reboot (comma-separated)'
+ WHERE key = 'tempradio';
+
+-- The bare `region` command lists regions/flood policy — a read, no args.
+UPDATE command_catalog
+   SET template = 'region', category = 'get', args = 'lists regions/transports', arity = 0,
+       description = 'Lists configured regions and flood policy',
+       in_share_default = TRUE, in_org_member_default = FALSE, in_org_admin_default = TRUE
+ WHERE key = 'region';
+
+INSERT INTO command_catalog (key, template, category, args, arity, description, risky, in_share_default, in_org_member_default, in_org_admin_default) VALUES
+  -- Access control & discovery
+  ('setperm.set',    'setperm <pubkey> <level>', 'config', '0 guest, 3 admin (1,2 room servers only)', 2, 'Adds or updates an ACL entry for a public key', FALSE, FALSE, FALSE, TRUE),
+  ('setperm.remove', 'setperm <pubkey>',         'config', 'pubkey',                                   1, 'Removes the ACL entry for a public key',        FALSE, FALSE, FALSE, TRUE),
+  ('get.acl',        'get acl',                  'diag',   'serial only',                               0, 'Lists the access-control list',                 FALSE, FALSE, FALSE, TRUE),
+  ('discover.neighbors','discover.neighbors',    'diag',   '',                                          0, 'Broadcasts a zero-hop neighbor discovery',      FALSE, TRUE,  TRUE,  TRUE),
+
+  -- Config set (owner/admin tier)
+  ('set.dutycycle',          'set dutycycle <percent>',      'config', '1-100%',           1, 'Sets the duty-cycle limit',                         FALSE, FALSE, FALSE, TRUE),
+  ('set.airtime_factor',     'set af <factor>',              'config', 'deprecated',       1, 'Sets the airtime factor (deprecated)',              FALSE, FALSE, FALSE, TRUE),
+  ('set.int_thresh',         'set int.thresh <n>',           'config', '',                 1, 'Sets the local interference threshold',             FALSE, FALSE, FALSE, TRUE),
+  ('set.cad',                'set cad <on|off>',             'config', '',                 1, 'Enables/disables channel-activity detection',       FALSE, FALSE, FALSE, TRUE),
+  ('set.agc_reset_interval', 'set agc.reset.interval <sec>', 'config', 'multiple of 4',    1, 'Sets the AGC reset interval (seconds)',             FALSE, FALSE, FALSE, TRUE),
+  ('set.multi_acks',         'set multi.acks <0|1>',         'config', '',                 1, 'Enables/disables multi-ACKs',                       FALSE, FALSE, FALSE, TRUE),
+  ('set.allow_read_only',    'set allow.read.only <on|off>', 'config', '',                 1, 'Sets read-only access permission',                  FALSE, FALSE, FALSE, TRUE),
+  ('set.flood_advert_interval','set flood.advert.interval <hrs>','config','3-168 hours',   1, 'Sets the flood advert interval',                    FALSE, FALSE, FALSE, TRUE),
+  ('set.owner_info',         'set owner.info <text>',        'config', '| for newlines',  -1, 'Sets the owner info text',                          FALSE, FALSE, FALSE, TRUE),
+  ('set.path_hash_mode',     'set path.hash.mode <mode>',    'config', '',                 1, 'Sets the advert path-hash size mode',               FALSE, FALSE, FALSE, TRUE),
+  ('set.loop_detect',        'set loop.detect <mode>',       'config', 'off|minimal|moderate|strict', 1, 'Sets the loop-detection level',           FALSE, FALSE, FALSE, TRUE),
+  ('set.adc_multiplier',     'set adc.multiplier <m>',       'config', '0.0-10.0',         1, 'Sets the battery ADC multiplier',                   FALSE, FALSE, FALSE, TRUE),
+  ('powersaving.status',     'powersaving',                  'config', '',                 0, 'Reads the power-saving state',                      FALSE, TRUE,  FALSE, TRUE),
+  ('gps.advert.set',         'gps advert <policy>',          'config', 'none|share|prefs', 1, 'Sets the GPS location advert policy',               FALSE, FALSE, FALSE, TRUE),
+
+  -- Radio set (owner/admin tier)
+  ('set.radio_rxgain',     'set radio.rxgain <on|off>',     'radio', 'RX boosted gain', 1, 'Sets RX boosted gain',         FALSE, FALSE, FALSE, TRUE),
+  ('set.radio_fem_rxgain', 'set radio.fem.rxgain <on|off>', 'radio', 'LoRa FEM RX gain', 1, 'Sets the LoRa FEM RX gain',  FALSE, FALSE, FALSE, TRUE),
+
+  -- Reads (safe to share); guest password reveals a secret, so admin-only
+  ('get.dutycycle',          'get dutycycle',           'get', '', 0, 'Reads the duty-cycle limit',          FALSE, TRUE,  FALSE, TRUE),
+  ('get.airtime_factor',     'get af',                  'get', '', 0, 'Reads the airtime factor',            FALSE, TRUE,  FALSE, TRUE),
+  ('get.int_thresh',         'get int.thresh',          'get', '', 0, 'Reads the interference threshold',    FALSE, TRUE,  FALSE, TRUE),
+  ('get.cad',                'get cad',                 'get', '', 0, 'Reads the CAD state',                 FALSE, TRUE,  FALSE, TRUE),
+  ('get.agc_reset_interval', 'get agc.reset.interval',  'get', '', 0, 'Reads the AGC reset interval',        FALSE, TRUE,  FALSE, TRUE),
+  ('get.multi_acks',         'get multi.acks',          'get', '', 0, 'Reads the multi-ACKs state',          FALSE, TRUE,  FALSE, TRUE),
+  ('get.allow_read_only',    'get allow.read.only',     'get', '', 0, 'Reads the read-only access flag',     FALSE, TRUE,  FALSE, TRUE),
+  ('get.flood_advert_interval','get flood.advert.interval','get','', 0, 'Reads the flood advert interval',   FALSE, TRUE,  FALSE, TRUE),
+  ('get.guest_password',     'get guest.password',      'get', 'reveals the guest password', 0, 'Reads the guest password', FALSE, FALSE, FALSE, TRUE),
+  ('get.rxdelay',            'get rxdelay',             'get', '', 0, 'Reads the RX delay base',             FALSE, TRUE,  FALSE, TRUE),
+  ('get.txdelay',            'get txdelay',             'get', '', 0, 'Reads the flood TX delay factor',     FALSE, TRUE,  FALSE, TRUE),
+  ('get.direct_txdelay',     'get direct.txdelay',      'get', '', 0, 'Reads the direct TX delay factor',    FALSE, TRUE,  FALSE, TRUE),
+  ('get.flood_max',          'get flood.max',           'get', '', 0, 'Reads the max flood hop count',       FALSE, TRUE,  FALSE, TRUE),
+  ('get.flood_max_advert',   'get flood.max.advert',    'get', '', 0, 'Reads the max advert hop count',      FALSE, TRUE,  FALSE, TRUE),
+  ('get.flood_max_unscoped', 'get flood.max.unscoped',  'get', '', 0, 'Reads the max unscoped hop count',    FALSE, TRUE,  FALSE, TRUE),
+  ('get.owner_info',         'get owner.info',          'get', '', 0, 'Reads the owner info text',           FALSE, TRUE,  FALSE, TRUE),
+  ('get.path_hash_mode',     'get path.hash.mode',      'get', '', 0, 'Reads the path-hash size mode',       FALSE, TRUE,  FALSE, TRUE),
+  ('get.loop_detect',        'get loop.detect',         'get', '', 0, 'Reads the loop-detection level',      FALSE, TRUE,  FALSE, TRUE),
+  ('get.radio_rxgain',       'get radio.rxgain',        'get', '', 0, 'Reads the RX boosted gain state',     FALSE, TRUE,  FALSE, TRUE),
+  ('get.radio_fem_rxgain',   'get radio.fem.rxgain',    'get', '', 0, 'Reads the FEM RX gain state',         FALSE, TRUE,  FALSE, TRUE),
+  ('get.adc_multiplier',     'get adc.multiplier',      'get', '', 0, 'Reads the battery ADC multiplier',    FALSE, TRUE,  FALSE, TRUE),
+
+  -- Region subcommands. Reads (get/list/home/default with no arg) are shareable;
+  -- the rest mutate region/flood policy, so they're owner/admin-tier.
+  ('region.get',         'region get <name>',            'get',    'region details',          1, 'Shows details for a named region',          FALSE, TRUE,  FALSE, TRUE),
+  ('region.list',        'region list <allowed|denied>', 'get',    'serial only',             1, 'Lists regions by flood policy',             FALSE, TRUE,  FALSE, TRUE),
+  ('region.def',         'region def <tokens>',          'config', 'parent/jump syntax',     -1, 'Defines a region hierarchy in one line',    FALSE, FALSE, FALSE, TRUE),
+  ('region.load',        'region load',                  'config', 'interactive or args',    -1, 'Bulk-loads a region hierarchy',             FALSE, FALSE, FALSE, TRUE),
+  ('region.save',        'region save',                  'config', '',                        0, 'Persists region changes to storage',        FALSE, FALSE, FALSE, TRUE),
+  ('region.allowf',      'region allowf <name>',         'config', 'or * wildcard',           1, 'Enables flooding for a region',             FALSE, FALSE, FALSE, TRUE),
+  ('region.denyf',       'region denyf <name>',          'config', 'or * wildcard',           1, 'Disables flooding for a region',            FALSE, FALSE, FALSE, TRUE),
+  ('region.put_root',    'region put <name>',            'config', '',                        1, 'Creates a root region',                     FALSE, FALSE, FALSE, TRUE),
+  ('region.put_sub',     'region put <name> <parent>',   'config', '',                        2, 'Creates a subregion under a parent',        FALSE, FALSE, FALSE, TRUE),
+  ('region.remove',      'region remove <name>',         'config', 'must be empty',           1, 'Removes a named region',                    FALSE, FALSE, FALSE, TRUE),
+  ('region.home_get',    'region home',                  'get',    '',                        0, 'Reads the home region',                     FALSE, TRUE,  FALSE, TRUE),
+  ('region.home_set',    'region home <name>',           'config', '',                        1, 'Sets the home region',                      FALSE, FALSE, FALSE, TRUE),
+  ('region.default_get', 'region default',               'get',    '',                        0, 'Reads the default scope region',            FALSE, TRUE,  FALSE, TRUE),
+  ('region.default_set', 'region default <name>',        'config', 'or null',                 1, 'Sets the default scope region',             FALSE, FALSE, FALSE, TRUE);
+
+-- Reclassify risky / share / member / admin defaults across the WHOLE catalog
+-- (authoritative — overrides the per-row flags set above and in 0003).
+--   risky  = identity changes, the admin password, wipe, OTA, or power-off —
+--            things that brick the device or lock the owner out. NOTE: setperm is
+--            NOT risky — the owner can always re-authenticate with the password
+--            and re-grant access, so the password is the gated secret, not the
+--            ACL. Radio settings aren't risky either (a nearby companion fixes them).
+--   member = small debug set: read stats/info, send adverts.
+--   admin  = mostly trusted: read any non-secret info and set any non-secret
+--            config, INCLUDING radio (e.g. retune a shared repeater), ACL changes
+--            (setperm), and clkreboot (the only way to wind the clock back).
+--   share  = one-off, highly trusted: everything that isn't risky (a superset of
+--            admin — also gets the secret commands admins don't).
+--
+-- Baseline: non-risky, granted to one-off shares and org admins, not org members.
+UPDATE command_catalog
+   SET risky = FALSE, in_share_default = TRUE,
+       in_org_member_default = FALSE, in_org_admin_default = TRUE;
+-- Risky: owner-only — excluded from every default tier.
+UPDATE command_catalog
+   SET risky = TRUE, in_share_default = FALSE, in_org_member_default = FALSE, in_org_admin_default = FALSE
+ WHERE key IN ('set.role','set.prv_key','set.public_key','password','erase','start.ota','poweroff','shutdown');
+-- Org-member debug set: version/board, clock, neighbors, stats, adverts, discovery.
+UPDATE command_catalog
+   SET in_org_member_default = TRUE
+ WHERE key IN ('ver','board','clock','neighbors','stats-packets','stats-radio','stats-core','advert','advert.zerohop','discover.neighbors');
+-- Secret reads/writes: fine for a trusted one-off share, but kept out of the
+-- broad org-admin default (so share ends up a strict superset of admin).
+UPDATE command_catalog
+   SET in_org_admin_default = FALSE
+ WHERE key IN ('guest.password','get.guest_password');
+
+-- +goose Down
+DELETE FROM command_catalog WHERE key IN (
+  'setperm.set', 'setperm.remove', 'get.acl', 'discover.neighbors',
+  'set.dutycycle', 'set.airtime_factor', 'set.int_thresh', 'set.cad', 'set.agc_reset_interval',
+  'set.multi_acks', 'set.allow_read_only', 'set.flood_advert_interval', 'set.owner_info',
+  'set.path_hash_mode', 'set.loop_detect', 'set.adc_multiplier', 'powersaving.status', 'gps.advert.set',
+  'set.radio_rxgain', 'set.radio_fem_rxgain',
+  'get.dutycycle', 'get.airtime_factor', 'get.int_thresh', 'get.cad', 'get.agc_reset_interval',
+  'get.multi_acks', 'get.allow_read_only', 'get.flood_advert_interval', 'get.guest_password',
+  'get.rxdelay', 'get.txdelay', 'get.direct_txdelay', 'get.flood_max', 'get.flood_max_advert',
+  'get.flood_max_unscoped', 'get.owner_info', 'get.path_hash_mode', 'get.loop_detect',
+  'get.radio_rxgain', 'get.radio_fem_rxgain', 'get.adc_multiplier',
+  'region.get', 'region.list', 'region.def', 'region.load', 'region.save', 'region.allowf',
+  'region.denyf', 'region.put_root', 'region.put_sub', 'region.remove',
+  'region.home_get', 'region.home_set', 'region.default_get', 'region.default_set'
+);
+-- Restore the bare `region` and `tempradio` rows to their 0003 seed state.
+UPDATE command_catalog SET template = 'region <...>', category = 'config', args = 'region map ops',
+       in_share_default = FALSE, in_org_member_default = FALSE, in_org_admin_default = TRUE
+ WHERE key = 'region';
+UPDATE command_catalog SET template = 'tempradio <f> <bw> <sf> <cr>' WHERE key = 'tempradio';
+ALTER TABLE command_catalog DROP COLUMN description;
+ALTER TABLE command_catalog DROP COLUMN arity;