diff --git a/internal/classification/client.go b/internal/classification/client.go index cc7fed1..de1cdc9 100644 --- a/internal/classification/client.go +++ b/internal/classification/client.go @@ -12,6 +12,7 @@ import ( "strings" "sync/atomic" "time" + "unicode" "unicode/utf8" "finance-duck/internal/domain" @@ -251,6 +252,13 @@ func (c *Client) Classify(ctx context.Context, facts domain.Facts, data domain.D return result, nil } +// hasHiddenRunes reports control or format code points — bidi overrides, +// zero-width characters — that would let model-supplied text spoof or +// reorder review UI. Legitimate payee names never need them. +func hasHiddenRunes(s string) bool { + return strings.ContainsFunc(s, func(r rune) bool { return unicode.IsControl(r) || unicode.Is(unicode.Cf, r) }) +} + // resolveAnswer maps one schema-valid provider answer onto enrichment, // revalidating every id against the local registry. proposed collects newly // minted merchants by normalized name so several rows resolved against the @@ -280,11 +288,12 @@ func resolveAnswer(answer answer, facts domain.Facts, data domain.Dataset, candi } if answer.NewMerchant != nil { name := strings.Join(strings.Fields(*answer.NewMerchant), " ") - // An identifier-shaped or oversized name is dropped, never stored, but - // the row keeps its independently enum-validated category and tags: a - // legitimate payee whose spelling trips the redactor (observed in the - // field) must not lose its whole classification. - if !utf8.ValidString(name) || utf8.RuneCountInString(name) > 100 || normalize(name) == "" || normalize(clean(name)) != normalize(name) { + // An identifier-shaped, oversized or hidden-rune name is dropped, + // never stored, but the row keeps its independently enum-validated + // category and tags: a legitimate payee whose spelling trips the + // redactor (observed in the field) must not lose its whole + // classification. + if !utf8.ValidString(name) || utf8.RuneCountInString(name) > 100 || normalize(name) == "" || normalize(clean(name)) != normalize(name) || hasHiddenRunes(name) { // no merchant } else if existing := duplicateMerchant(name, data.Merchants); existing != nil { e.MerchantID = existing.ID diff --git a/internal/classification/client_test.go b/internal/classification/client_test.go index 3139312..923c6bb 100644 --- a/internal/classification/client_test.go +++ b/internal/classification/client_test.go @@ -343,7 +343,7 @@ func TestTransactionAmountAndCounterpartyAreSent(t *testing.T) { } func TestUnsafeMerchantProposalDroppedWithoutLosingClassification(t *testing.T) { - for _, name := range []string{"Alice Privateperson", "DE89370400440532013000", "Bank 123456789", "reference secretpayment", strings.Repeat("x", 101)} { + for _, name := range []string{"Alice Privateperson", "DE89370400440532013000", "Bank 123456789", "reference secretpayment", strings.Repeat("x", 101), "Rent \u202Edeifirev \u2713", "zero\u200Bwidth"} { t.Run(name, func(t *testing.T) { f, d := fixture() f.Counterparty = "Alice Privateperson" diff --git a/internal/classification/propose.go b/internal/classification/propose.go index eceb419..9ebe9a2 100644 --- a/internal/classification/propose.go +++ b/internal/classification/propose.go @@ -83,7 +83,7 @@ func normalizedProposalName(value string, max int) (string, error) { if !utf8.ValidString(value) || value == "" || utf8.RuneCountInString(value) > max { return "", errors.New("proposal name is blank, invalid UTF-8 or too long") } - if strings.ContainsAny(value, "{}[]()<>/\\") || strings.Contains(value, "___") { + if strings.ContainsAny(value, "{}[]()<>/\\") || strings.Contains(value, "___") || hasHiddenRunes(value) { return "", errors.New("proposal name is identifier-shaped") } return value, nil diff --git a/internal/domain/domain.go b/internal/domain/domain.go index e3f3816..f999e8d 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -250,6 +250,11 @@ func validHint(s string) bool { return utf8.ValidString(s) && utf8.RuneCountInString(s) <= 200 } +// validName bounds registry display names at the 200 runes every UI form +// already enforces, so no client can persist an unbounded name that every +// later state response would carry. +func validName(s string) bool { return nonblank(s) && utf8.RuneCountInString(s) <= 200 } + // ValidISIN reports a syntactically valid ISIN: two country letters, nine // alphanumerics and a check digit. func ValidISIN(s string) bool { return isinPattern.MatchString(s) } @@ -292,7 +297,7 @@ func Validate(d Dataset) error { if err := register(c.ID, "category"); err != nil { return err } - if !nonblank(c.Name) || !validHint(c.Hint) || (c.Kind != "expense" && c.Kind != "income") { + if !validName(c.Name) || !validHint(c.Hint) || (c.Kind != "expense" && c.Kind != "income") { return fmt.Errorf("category %q: invalid name, hint or kind", c.ID) } categories[c.ID] = c @@ -330,7 +335,7 @@ func Validate(d Dataset) error { if err := register(t.ID, "tag"); err != nil { return err } - if !nonblank(t.Name) || !validHint(t.Hint) { + if !validName(t.Name) || !validHint(t.Hint) { return fmt.Errorf("tag %q: name or hint invalid", t.ID) } tags[t.ID] = true @@ -339,8 +344,8 @@ func Validate(d Dataset) error { if err := register(m.ID, "merchant"); err != nil { return err } - if !nonblank(m.Name) { - return fmt.Errorf("merchant %q: name required", m.ID) + if !validName(m.Name) { + return fmt.Errorf("merchant %q: valid name of at most 200 characters required", m.ID) } if m.DefaultCategoryID != "" { if _, ok := categories[m.DefaultCategoryID]; !ok || children[m.DefaultCategoryID] { @@ -375,7 +380,7 @@ func Validate(d Dataset) error { if other, ok := isins[v.ISIN]; ok { return fmt.Errorf("instrument %q: ISIN %s already held by %q", v.ID, v.ISIN, other) } - if !nonblank(v.Name) || !currencyPattern.MatchString(v.Currency) || !validText(v.Symbol) { + if !validName(v.Name) || !currencyPattern.MatchString(v.Currency) || !validText(v.Symbol) { return fmt.Errorf("instrument %q: valid UTF-8 name and symbol and three-letter uppercase currency required", v.ID) } // A quote without its day cannot be judged stale, and a day without a diff --git a/internal/domain/domain_test.go b/internal/domain/domain_test.go index 3caf554..f7177a4 100644 --- a/internal/domain/domain_test.go +++ b/internal/domain/domain_test.go @@ -75,6 +75,8 @@ func TestDomainRejectsBrokenReferencesAndTaxonomy(t *testing.T) { {"duplicate identity", func(d *Dataset) { d.Tags[0].ID = "acc_main" }}, {"invalid provenance date", func(d *Dataset) { d.Transactions[0].Enrichment.Classification.Timestamp = "yesterday" }}, {"nonleaf merchant default", func(d *Dataset) { d.Merchants[0].DefaultCategoryID = "cat_food" }}, + {"oversized tag name", func(d *Dataset) { d.Tags[0].Name = strings.Repeat("x", 201) }}, + {"oversized category name", func(d *Dataset) { d.Categories[2].Name = strings.Repeat("x", 201) }}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) {