Enforce name limits server-side and reject hidden runes in model names
Security review follow-ups. The 200-character registry-name cap the UI forms promise now holds in domain.Validate for categories, tags, merchants and instruments, so a non-browser client cannot persist an unbounded name that every subsequent state response would carry. And a model-supplied merchant or taxonomy name containing control or format code points — bidi overrides, zero-width characters — is dropped like an identifier-shaped one: React escaping already prevented injection, but such names could visually spoof or reorder the review UI the operator approves from.
This commit is contained in:
@@ -12,6 +12,7 @@ import (
|
|||||||
"strings"
|
"strings"
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
"time"
|
"time"
|
||||||
|
"unicode"
|
||||||
"unicode/utf8"
|
"unicode/utf8"
|
||||||
|
|
||||||
"finance-duck/internal/domain"
|
"finance-duck/internal/domain"
|
||||||
@@ -251,6 +252,13 @@ func (c *Client) Classify(ctx context.Context, facts domain.Facts, data domain.D
|
|||||||
return result, nil
|
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,
|
// resolveAnswer maps one schema-valid provider answer onto enrichment,
|
||||||
// revalidating every id against the local registry. proposed collects newly
|
// revalidating every id against the local registry. proposed collects newly
|
||||||
// minted merchants by normalized name so several rows resolved against the
|
// 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 {
|
if answer.NewMerchant != nil {
|
||||||
name := strings.Join(strings.Fields(*answer.NewMerchant), " ")
|
name := strings.Join(strings.Fields(*answer.NewMerchant), " ")
|
||||||
// An identifier-shaped or oversized name is dropped, never stored, but
|
// An identifier-shaped, oversized or hidden-rune name is dropped,
|
||||||
// the row keeps its independently enum-validated category and tags: a
|
// never stored, but the row keeps its independently enum-validated
|
||||||
// legitimate payee whose spelling trips the redactor (observed in the
|
// category and tags: a legitimate payee whose spelling trips the
|
||||||
// field) must not lose its whole classification.
|
// redactor (observed in the field) must not lose its whole
|
||||||
if !utf8.ValidString(name) || utf8.RuneCountInString(name) > 100 || normalize(name) == "" || normalize(clean(name)) != normalize(name) {
|
// classification.
|
||||||
|
if !utf8.ValidString(name) || utf8.RuneCountInString(name) > 100 || normalize(name) == "" || normalize(clean(name)) != normalize(name) || hasHiddenRunes(name) {
|
||||||
// no merchant
|
// no merchant
|
||||||
} else if existing := duplicateMerchant(name, data.Merchants); existing != nil {
|
} else if existing := duplicateMerchant(name, data.Merchants); existing != nil {
|
||||||
e.MerchantID = existing.ID
|
e.MerchantID = existing.ID
|
||||||
|
|||||||
@@ -343,7 +343,7 @@ func TestTransactionAmountAndCounterpartyAreSent(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestUnsafeMerchantProposalDroppedWithoutLosingClassification(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) {
|
t.Run(name, func(t *testing.T) {
|
||||||
f, d := fixture()
|
f, d := fixture()
|
||||||
f.Counterparty = "Alice Privateperson"
|
f.Counterparty = "Alice Privateperson"
|
||||||
|
|||||||
@@ -83,7 +83,7 @@ func normalizedProposalName(value string, max int) (string, error) {
|
|||||||
if !utf8.ValidString(value) || value == "" || utf8.RuneCountInString(value) > max {
|
if !utf8.ValidString(value) || value == "" || utf8.RuneCountInString(value) > max {
|
||||||
return "", errors.New("proposal name is blank, invalid UTF-8 or too long")
|
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 "", errors.New("proposal name is identifier-shaped")
|
||||||
}
|
}
|
||||||
return value, nil
|
return value, nil
|
||||||
|
|||||||
@@ -250,6 +250,11 @@ func validHint(s string) bool {
|
|||||||
return utf8.ValidString(s) && utf8.RuneCountInString(s) <= 200
|
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
|
// ValidISIN reports a syntactically valid ISIN: two country letters, nine
|
||||||
// alphanumerics and a check digit.
|
// alphanumerics and a check digit.
|
||||||
func ValidISIN(s string) bool { return isinPattern.MatchString(s) }
|
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 {
|
if err := register(c.ID, "category"); err != nil {
|
||||||
return err
|
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)
|
return fmt.Errorf("category %q: invalid name, hint or kind", c.ID)
|
||||||
}
|
}
|
||||||
categories[c.ID] = c
|
categories[c.ID] = c
|
||||||
@@ -330,7 +335,7 @@ func Validate(d Dataset) error {
|
|||||||
if err := register(t.ID, "tag"); err != nil {
|
if err := register(t.ID, "tag"); err != nil {
|
||||||
return err
|
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)
|
return fmt.Errorf("tag %q: name or hint invalid", t.ID)
|
||||||
}
|
}
|
||||||
tags[t.ID] = true
|
tags[t.ID] = true
|
||||||
@@ -339,8 +344,8 @@ func Validate(d Dataset) error {
|
|||||||
if err := register(m.ID, "merchant"); err != nil {
|
if err := register(m.ID, "merchant"); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
if !nonblank(m.Name) {
|
if !validName(m.Name) {
|
||||||
return fmt.Errorf("merchant %q: name required", m.ID)
|
return fmt.Errorf("merchant %q: valid name of at most 200 characters required", m.ID)
|
||||||
}
|
}
|
||||||
if m.DefaultCategoryID != "" {
|
if m.DefaultCategoryID != "" {
|
||||||
if _, ok := categories[m.DefaultCategoryID]; !ok || children[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 {
|
if other, ok := isins[v.ISIN]; ok {
|
||||||
return fmt.Errorf("instrument %q: ISIN %s already held by %q", v.ID, v.ISIN, other)
|
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)
|
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
|
// A quote without its day cannot be judged stale, and a day without a
|
||||||
|
|||||||
@@ -75,6 +75,8 @@ func TestDomainRejectsBrokenReferencesAndTaxonomy(t *testing.T) {
|
|||||||
{"duplicate identity", func(d *Dataset) { d.Tags[0].ID = "acc_main" }},
|
{"duplicate identity", func(d *Dataset) { d.Tags[0].ID = "acc_main" }},
|
||||||
{"invalid provenance date", func(d *Dataset) { d.Transactions[0].Enrichment.Classification.Timestamp = "yesterday" }},
|
{"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" }},
|
{"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 {
|
for _, tc := range cases {
|
||||||
t.Run(tc.name, func(t *testing.T) {
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
|||||||
Reference in New Issue
Block a user