Use one identifier per account and mask secrets in logs

signal-cli tags receive events of numbered accounts with the phone
number, so a websocket opened with the ACI of such an account never
received anything. The same mismatch split the trust mode settings
between the number and the ACI. Map the ACI of a numbered account to
its phone number (from accounts.json) for the websocket filter and the
trust mode settings.

Accept an ACI the same way signal-cli does: with or without dashes and
whitespace, so the Account Key shown by Signal Android can be used
as is. Phone numbers now need the leading +, which signal-cli requires
anyway.

Mask PINs and recovery credentials in the debug logs of both the CLI
and the JSON-RPC client.

src/api/api.go

@@ -724,7 +724,8 @@ func (a *Api) Receive(c *gin.Context) {
 		}
 		defer ws.Close()
 		var stop = make(chan struct{})
-		go a.handleSignalReceive(ws, number, stop)
+		// receive events carry the phone number of numbered accounts, so filter by the canonical account
+		go a.handleSignalReceive(ws, a.signalClient.CanonicalAccount(number), stop)
 		go a.wsPing(ws, stop)
 		wsPong(ws, stop)
 	} else {

src/client/cli.go

@@ -56,6 +56,18 @@ func isAccountSelector(candidate string) bool {
 	return utils.IsPhoneNumber(candidate)
 }
 
+// redactSecretArgs returns a copy of args with secret values masked, so they don't end up in the logs.
+func redactSecretArgs(args []string) []string {
+	redacted := append([]string{}, args...)
+	for i := 1; i < len(redacted); i++ {
+		switch redacted[i-1] {
+		case "--recovery-key", "--totp", "--pin", "setPin":
+			redacted[i] = "***"
+		}
+	}
+	return redacted
+}
+
 func (s *CliClient) Execute(wait bool, args []string, stdin string) (string, error) {
 	containerId, err := getContainerId()
 
@@ -100,7 +112,7 @@ func (s *CliClient) Execute(wait bool, args []string, stdin string) (string, err
 	if stdin != "" {
 		fullCmd += "echo '" + stdin + "' | "
 	}
-	fullCmd += signalCliBinary + " " + strings.Join(args, " ")
+	fullCmd += signalCliBinary + " " + strings.Join(redactSecretArgs(args), " ")
 
 	log.Debug("*) su signal-api")
 	log.Debug("*) ", fullCmd)

src/client/client.go

@@ -436,7 +436,7 @@ func (s *SignalClient) GetSignalCliMode() SignalCliMode {
 }
 
 func (s *SignalClient) Init(maxRetries int) error {
-	s.signalCliApiConfig = utils.NewSignalCliApiConfig()
+	s.signalCliApiConfig = utils.NewSignalCliApiConfig(s.signalCliConfig)
 	err := s.signalCliApiConfig.Load(s.signalCliApiConfigPath)
 	if err != nil {
 		return err
@@ -1093,6 +1093,11 @@ func (s *SignalClient) Receive(number string, timeout int64, ignoreAttachments b
 	}
 }
 
+// CanonicalAccount maps the ACI of a numbered account to its phone number (see utils.CanonicalAccount).
+func (s *SignalClient) CanonicalAccount(account string) string {
+	return utils.CanonicalAccount(s.signalCliConfig, account)
+}
+
 func (s *SignalClient) GetReceiveChannel(number string) (chan JsonRpc2ReceivedMessage, string, error) {
 	jsonRpc2Client, err := s.getJsonRpc2Client()
 	if err != nil {

src/client/jsonrpc2.go

@@ -15,6 +15,7 @@ import (
 	"github.com/bbernhard/signal-cli-rest-api/utils"
 	uuid "github.com/gofrs/uuid"
 	log "github.com/sirupsen/logrus"
+	"github.com/tidwall/gjson"
 	"github.com/tidwall/sjson"
 )
 
@@ -116,6 +117,16 @@ func (r *JsonRpc2Client) Dial(address string, maxRetries int) error {
 	return nil
 }
 
+// redactSecretParams returns a copy of a JSON-RPC request with secret params masked, so they don't end up in the logs.
+func redactSecretParams(request []byte) []byte {
+	for _, key := range []string{"recoveryKey", "totp", "pin"} {
+		if gjson.GetBytes(request, "params."+key).Exists() {
+			request, _ = sjson.SetBytes(request, "params."+key, "***")
+		}
+	}
+	return request
+}
+
 func (r *JsonRpc2Client) getRaw(command string, account *string, args interface{}) (string, error) {
 	type Request struct {
 		JsonRpc string      `json:"jsonrpc"`
@@ -163,7 +174,7 @@ func (r *JsonRpc2Client) getRaw(command string, account *string, args interface{
 		}
 	}
 
-	log.Debug("json-rpc command: ", string(fullCommandBytes))
+	log.Debug("json-rpc command: ", string(redactSecretParams(fullCommandBytes)))
 
 	_, err = r.conn.Write([]byte(string(fullCommandBytes) + "\n"))
 	if err != nil {

src/client/redact_test.go

@@ -0,0 +1,37 @@
+package client
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+)
+
+func TestRedactSecretArgs(t *testing.T) {
+	args := []string{"-a", "+12025550123", "register", "--recovery-key", "KEY", "--totp", "123456"}
+	want := []string{"-a", "+12025550123", "register", "--recovery-key", "***", "--totp", "***"}
+	if got := redactSecretArgs(args); !reflect.DeepEqual(got, want) {
+		t.Fatalf("got %q, want %q", got, want)
+	}
+	if args[4] != "KEY" {
+		t.Fatal("redactSecretArgs modified the arguments that are passed to signal-cli")
+	}
+	if got := redactSecretArgs([]string{"-a", "+12025550123", "setPin", "1234"}); got[3] != "***" {
+		t.Fatalf("setPin pin not redacted: %q", got)
+	}
+}
+
+func TestRedactSecretParams(t *testing.T) {
+	request := []byte(`{"jsonrpc":"2.0","method":"register","params":{"account":"+12025550123","recoveryKey":"KEY","totp":"123456"}}`)
+	original := string(request)
+
+	got := string(redactSecretParams(request))
+	if strings.Contains(got, "KEY") || strings.Contains(got, "123456") || !strings.Contains(got, `"recoveryKey":"***"`) {
+		t.Fatalf("secrets not redacted: %s", got)
+	}
+	if string(request) != original {
+		t.Fatal("redactSecretParams modified the request that is sent to signal-cli")
+	}
+	if strings.Contains(got, `"pin"`) {
+		t.Fatalf("absent param was added: %s", got)
+	}
+}

src/go.mod

@@ -17,6 +17,7 @@ require (
 	github.com/swaggo/files v1.0.1
 	github.com/swaggo/gin-swagger v1.6.0
 	github.com/swaggo/swag v1.16.4
+	github.com/tidwall/gjson v1.14.4
 	github.com/tidwall/sjson v1.2.5
 	github.com/yuin/gopher-lua v1.1.2
 	gopkg.in/yaml.v2 v2.4.0
@@ -51,7 +52,6 @@ require (
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/pelletier/go-toml/v2 v2.2.3 // indirect
-	github.com/tidwall/gjson v1.14.4 // indirect
 	github.com/tidwall/match v1.1.1 // indirect
 	github.com/tidwall/pretty v1.2.1 // indirect
 	github.com/twitchyliquid64/golang-asm v0.15.1 // indirect

src/utils/account_identifier.go

@@ -1,7 +1,10 @@
 package utils
 
 import (
+	"encoding/json"
 	"errors"
+	"os"
+	"path/filepath"
 	"regexp"
 	"strings"
 )
@@ -16,21 +19,53 @@ const (
 )
 
 var (
-	e164AccountPattern = regexp.MustCompile(`^\+?[1-9]\d{1,14}$`)
-	aciAccountPattern  = regexp.MustCompile(`(?i)^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)
+	e164AccountPattern = regexp.MustCompile(`^\+[1-9]\d{1,14}$`)
+	aciAccountPattern  = regexp.MustCompile(`^[0-9a-f]{32}$`)
 )
 
-// ParseAccountIdentifier accepts an E.164 phone number or an ACI UUID.
-// ACI values are returned in lowercase, which is the form signal-cli stores.
+// ParseAccountIdentifier accepts an E.164 phone number (signal-cli requires the leading +) or an ACI.
+// Like signal-cli, an ACI may be given with or without dashes and whitespace (e.g. the Account Key
+// shown by Signal Android). It is returned as a lowercase UUID with dashes, the form signal-cli stores.
 func ParseAccountIdentifier(s string) (IdentifierType, string, error) {
 	s = strings.TrimSpace(s)
 	if e164AccountPattern.MatchString(s) {
 		return IdentifierE164, s, nil
 	}
-	if aciAccountPattern.MatchString(s) {
-		return IdentifierACI, strings.ToLower(s), nil
+	compact := strings.ToLower(strings.ReplaceAll(strings.Join(strings.Fields(s), ""), "-", ""))
+	if aciAccountPattern.MatchString(compact) {
+		return IdentifierACI, compact[:8] + "-" + compact[8:12] + "-" + compact[12:16] + "-" + compact[16:20] + "-" + compact[20:], nil
 	}
-	return 0, "", errors.New("Couldn't process request - invalid account identifier (expected an E.164 phone number or an ACI UUID)")
+	return 0, "", errors.New("Couldn't process request - invalid account identifier (expected an E.164 phone number with leading + or an ACI)")
+}
+
+// CanonicalAccount returns the phone number of the local account whose ACI is id, otherwise id unchanged.
+// signal-cli reports numbered accounts by phone number (e.g. in receive events), so mapping the ACI
+// to that number gives every account a single key in the API's own bookkeeping.
+func CanonicalAccount(signalCliConfigDir string, id string) string {
+	if typ, _, err := ParseAccountIdentifier(id); err != nil || typ != IdentifierACI {
+		return id
+	}
+
+	data, err := os.ReadFile(filepath.Join(signalCliConfigDir, "data", "accounts.json"))
+	if err != nil {
+		return id
+	}
+	var stored struct {
+		Accounts []struct {
+			Number string `json:"number"`
+			Uuid   string `json:"uuid"`
+		} `json:"accounts"`
+	}
+	if json.Unmarshal(data, &stored) != nil {
+		return id
+	}
+
+	for _, account := range stored.Accounts {
+		if account.Number != "" && strings.EqualFold(account.Uuid, id) {
+			return account.Number
+		}
+	}
+	return id
 }
 
 // SignalCliAccountSelector returns the global signal-cli account flag and value.

src/utils/account_identifier_test.go

@@ -1,9 +1,13 @@
 package utils
 
-import "testing"
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
 
 func TestParseAccountIdentifierE164(t *testing.T) {
-	cases := []string{"+12025550123", "+12", "12025550123"}
+	cases := []string{"+12025550123", "+12"}
 	for _, raw := range cases {
 		typ, value, err := ParseAccountIdentifier(raw)
 		if err != nil {
@@ -50,6 +54,62 @@ func TestParseAccountIdentifierACI(t *testing.T) {
 	}
 }
 
+// signal-cli also accepts the Account Key form: 32 hex characters, optionally grouped by whitespace.
+func TestParseAccountIdentifierACIWithoutDashes(t *testing.T) {
+	want := "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"
+	for _, raw := range []string{"AAAAAAAABBBB4CCC8DDDEEEEEEEEEEEE", "aaaa aaaa bbbb 4ccc 8ddd eeee eeee eeee"} {
+		typ, value, err := ParseAccountIdentifier(raw)
+		if err != nil {
+			t.Fatalf("%q: %v", raw, err)
+		}
+		if typ != IdentifierACI || value != want {
+			t.Fatalf("%q: type=%v value=%q, want %q", raw, typ, value, want)
+		}
+	}
+}
+
+// writeAccountsJson creates a signal-cli config dir with one numbered and one numberless account.
+func writeAccountsJson(t *testing.T) string {
+	dir := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(dir, "data"), 0700); err != nil {
+		t.Fatal(err)
+	}
+	accounts := `{"accounts":[
+		{"path":"1","environment":"LIVE","number":"+12025550123","uuid":"aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"},
+		{"path":"2","environment":"LIVE","number":null,"uuid":"11111111-1111-4111-8111-111111111111"}
+	],"version":2}`
+	if err := os.WriteFile(filepath.Join(dir, "data", "accounts.json"), []byte(accounts), 0600); err != nil {
+		t.Fatal(err)
+	}
+	return dir
+}
+
+func TestCanonicalAccount(t *testing.T) {
+	dir := writeAccountsJson(t)
+	cases := map[string]string{
+		"aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee": "+12025550123",                         // ACI of a numbered account
+		"11111111-1111-4111-8111-111111111111": "11111111-1111-4111-8111-111111111111", // numberless account
+		"22222222-2222-4222-8222-222222222222": "22222222-2222-4222-8222-222222222222", // unknown account
+		"+12025550123":                         "+12025550123",
+	}
+	for id, want := range cases {
+		if got := CanonicalAccount(dir, id); got != want {
+			t.Fatalf("%q: got %q, want %q", id, got, want)
+		}
+	}
+	if got := CanonicalAccount(t.TempDir(), "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"); got != "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee" {
+		t.Fatalf("missing accounts.json: got %q", got)
+	}
+}
+
+func TestTrustModeSharedByNumberAndACI(t *testing.T) {
+	config := NewSignalCliApiConfig(writeAccountsJson(t))
+	config.SetTrustModeForNumber("aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee", AlwaysTrust)
+	if trustMode, err := config.GetTrustModeForNumber("+12025550123"); err != nil || trustMode != AlwaysTrust {
+		t.Fatalf("got %v %v, want AlwaysTrust", trustMode, err)
+	}
+}
+
 func TestParseAccountIdentifierInvalid(t *testing.T) {
 	cases := []string{
 		"",
@@ -58,9 +118,9 @@ func TestParseAccountIdentifierInvalid(t *testing.T) {
 		"+1",
 		"+1234567890123456",
 		"+12 345",
+		"12025550123",
 		"not-an-account",
 		"11111111-1111-4111-8111-11111111111",
-		"11111111111141118111111111111111",
 		"GGGGGGGG-1111-4111-8111-111111111111",
 	}
 	for _, raw := range cases {

src/utils/api_config.go

@@ -85,12 +85,14 @@ type SignalCliApiConfigEntries struct {
 }
 
 type SignalCliApiConfig struct {
-	config SignalCliApiConfigEntries
-	path   string
+	config             SignalCliApiConfigEntries
+	path               string
+	signalCliConfigDir string
 }
 
-func NewSignalCliApiConfig() *SignalCliApiConfig {
-	return &SignalCliApiConfig{}
+// NewSignalCliApiConfig creates the API config. signalCliConfigDir is used to key accounts canonically (see CanonicalAccount).
+func NewSignalCliApiConfig(signalCliConfigDir string) *SignalCliApiConfig {
+	return &SignalCliApiConfig{signalCliConfigDir: signalCliConfigDir}
 }
 
 func (c *SignalCliApiConfig) Load(path string) error {
@@ -110,7 +112,10 @@ func (c *SignalCliApiConfig) Load(path string) error {
 	return nil
 }
 
+// GetTrustModeForNumber and SetTrustModeForNumber key by CanonicalAccount, so the setting
+// applies whether a numbered account is addressed by its phone number or its ACI.
 func (c *SignalCliApiConfig) GetTrustModeForNumber(number string) (SignalCliTrustMode, error) {
+	number = CanonicalAccount(c.signalCliConfigDir, number)
 	if val, ok := c.config.Entries[number]; ok {
 		return val.TrustMode, nil
 	}
@@ -119,6 +124,7 @@ func (c *SignalCliApiConfig) GetTrustModeForNumber(number string) (SignalCliTrus
 }
 
 func (c *SignalCliApiConfig) SetTrustModeForNumber(number string, trustMode SignalCliTrustMode) {
+	number = CanonicalAccount(c.signalCliConfigDir, number)
 	if c.config.Entries == nil {
 		c.config.Entries = make(map[string]SignalCliApiConfigEntry)
 	}