diff --git a/CHANGELOG.md b/CHANGELOG.md index 65467b8..cd7c1e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ version headings refer to releases, not merely to merged pull requests. ### Added +- Two optional fields on a profile's operation, for KIFF's "Did this happen?" + ask: `record_url`, where the owner looks at the vendor's record of the + resource (an https address with the resource's id path-escaped into + `{resource}`, reviewed catalog only), and `label_read`, a read of the same + profile that gives the resource's own name (for an operation with a + `resource`). A profile that names neither is unchanged. + - A call whose outcome comes later (the slow-tools design): a write's profile may say `completes: {event, key, event_key, timeout_s, words}`. The relay then refuses the call while the taf doesn't listen for that diff --git a/docs/connections.md b/docs/connections.md index c1bb07b..162cf8b 100644 --- a/docs/connections.md +++ b/docs/connections.md @@ -377,7 +377,12 @@ revision so a replay cannot adopt a later profile or send the write again. Profiles can require arguments before any vendor call. A marker must name a string field in the vendor schema; planque fills that field from the permit's -operation id and refuses a caller-supplied value there. Agent proposals +operation id and refuses a caller-supplied value there. An operation may say +where the owner looks at the vendor's own record of its resource +(`record_url`, an https address with the resource's id path-escaped into +`{resource}`; reviewed catalog only, as `no_effect` is) and which read of the +profile gives the resource's own name (`label_read`, for an operation with a +`resource`): KIFF shows both when it asks the owner "Did this happen?". Agent proposals cannot claim reviewed catalog status or successful test calls. Profiles needing a nominal-currency lookup definition are refused until that definition is supported. Version-header pins use the reviewed configuration described below. diff --git a/internal/profile/profile.go b/internal/profile/profile.go index 6788372..54944ab 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "net/url" "regexp" "slices" "strings" @@ -49,6 +50,9 @@ type Profile struct { Operations []Operation `json:"operations"` } +// RecordPlaceholder is what a record_url carries where the resource's id goes. +const RecordPlaceholder = "{resource}" + // Pin is how the profile is pinned to a vendor version. type Pin struct { // Kind is "header" (a version header, such as Stripe-Version), @@ -81,6 +85,17 @@ type Operation struct { NoEffect *NoEffect `json:"no_effect,omitempty"` // RequiredArguments are arguments planque requires before sending. RequiredArguments []string `json:"required_arguments,omitempty"` + // RecordURL is where the owner looks at the vendor's record of this + // operation's resource ("https://dashboard.stripe.com/payments/{resource}"): + // an https address at the vendor, with the resource's id path-escaped + // into {resource}. Reviewed catalog only: a drafted profile carries none, + // so no link reaches the owner that no reviewer saw. KIFF shows it + // when it asks "Did this happen?". + RecordURL string `json:"record_url,omitempty"` + // LabelRead is a read of the same profile that gives the resource's own + // name (a customer's name, an invoice's number), for the words KIFF + // asks with; it needs a resource. + LabelRead *Read `json:"label_read,omitempty"` // Completes says the write's outcome comes later, as one of the app's // events: the run that made the call waits for it (kiff/planque#86's // slow tools). Only a write. @@ -394,6 +409,25 @@ func (p Profile) validateOp(o Operation, byTool map[string]Operation) error { return fmt.Errorf("required_arguments: %q isn't an argument name", a) } } + if u := o.RecordURL; u != "" { + if p.Source != "catalog" { + return errors.New("record_url comes only from a catalog entry merged by review (reviewed catalog only)") + } + if err := checkRecordURL(u); err != nil { + return fmt.Errorf("record_url: %w", err) + } + } + if l := o.LabelRead; l != nil { + if o.Resource == nil { + return errors.New("label_read names the resource's own name, so the operation needs a resource") + } + if err := readOf(l.Tool, l.Args, byTool); err != nil { + return fmt.Errorf("label_read: %w", err) + } + if !pathExpr.MatchString(l.Path) { + return errors.New("label_read.path is a dotted path") + } + } if c := o.Completes; c != nil { switch { case !eventSlug.MatchString(c.Event): @@ -411,6 +445,46 @@ func (p Profile) validateOp(o Operation, byTool map[string]Operation) error { return nil } +// checkRecordURL accepts an https address at a host, with no credentials in +// it and at most one {resource} placeholder, which is the only brace in it +// and sits in the path: never in the query or the fragment, where an id's +// "&" or "=" would add a parameter the vendor's page may act on. +func checkRecordURL(u string) error { + if len(u) > 512 || !strings.HasPrefix(u, "https://") { + return errors.New("an https address at the vendor, at most 512 characters") + } + if strings.Count(u, RecordPlaceholder) > 1 || strings.Count(u, "{") != strings.Count(u, RecordPlaceholder) || strings.Count(u, "}") != strings.Count(u, RecordPlaceholder) { + return errors.New("{resource} is the only placeholder, at most once") + } + parsed, err := url.Parse(strings.ReplaceAll(u, RecordPlaceholder, "x")) + if err != nil || parsed.Scheme != "https" || parsed.Host == "" || parsed.User != nil { + return errors.New("an https address at a host, with no credentials in it") + } + // Where the placeholder sits, by position: after the host, before any "?" or "#". + if at := strings.Index(u, RecordPlaceholder); at >= 0 { + host := len("https://") + len(parsed.Host) + stop := strings.IndexAny(u, "?#") + if at < host || (stop >= 0 && at > stop) { + return errors.New("{resource} goes in the address's path, never its host, query or fragment") + } + } + return nil +} + +// RecordURL fills template with the resource's id, path-escaped, so an id +// holding "/", "?", "#" or "@" stays inside the vendor's host and path. An +// empty id, or "." or "..", which a browser reads as a step up the path, +// gives no link: KIFF then shows the vendor's name alone. +func RecordURL(template, resource string) string { + if !strings.Contains(template, RecordPlaceholder) { + return template + } + if resource == "" || resource == "." || resource == ".." { + return "" + } + return strings.ReplaceAll(template, RecordPlaceholder, url.PathEscape(resource)) +} + func validateAmount(a Amount, byTool map[string]Operation) error { switch a.From { case "argument": diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go index 775fd8a..49c76e4 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -54,3 +54,89 @@ func TestProfileRefusesUnknownAndTrailingFields(t *testing.T) { } } } + +// An operation may say where the owner looks at the vendor's record of its +// resource (record_url, reviewed catalog only) and which read gives the +// resource's own name (label_read); the id is path-escaped into the address. +func TestRecordURLAndLabelRead(t *testing.T) { + raw, _ := os.ReadFile("testdata/stripe-refunds.json") + with := func(changes map[string]any, source string) ([]byte, error) { + var p map[string]any + if err := json.Unmarshal(raw, &p); err != nil { + t.Fatal(err) + } + if source != "" { + p["source"] = source + delete(p, "catalog_entry") + } + ops := p["operations"].([]any) + for _, o := range ops { + op := o.(map[string]any) + if op["tool"] == "create_refund" { + for k, v := range changes { + op[k] = v + } + } + } + out, _ := json.Marshal(p) + _, err := Parse(out) + return out, err + } + good := map[string]any{"record_url": "https://dashboard.stripe.com/payments/{resource}", "label_read": map[string]any{"tool": "get_payment_intent", "args": map[string]string{"payment_intent": "$args.payment_intent"}, "path": "description"}} + if _, err := with(good, ""); err != nil { + t.Fatalf("a record_url and a label_read on a catalog profile: %v", err) + } + for name, c := range map[string]struct { + changes map[string]any + source string + want string + }{ + "a drafted profile's record_url": {map[string]any{"record_url": "https://dashboard.stripe.com/payments/{resource}"}, "drafted", "reviewed catalog only"}, + "http": {map[string]any{"record_url": "http://dashboard.stripe.com/payments/{resource}"}, "", "https"}, + "credentials in it": {map[string]any{"record_url": "https://user:pw@dashboard.stripe.com/{resource}"}, "", "no credentials"}, + "two placeholders": {map[string]any{"record_url": "https://dashboard.stripe.com/{resource}/{resource}"}, "", "at most once"}, + "another brace": {map[string]any{"record_url": "https://dashboard.stripe.com/{account}/payments"}, "", "only placeholder"}, + "the placeholder as the host": {map[string]any{"record_url": "https://{resource}/payments"}, "", "path"}, + "the placeholder in the query": {map[string]any{"record_url": "https://dash.example.com/resources/view?id={resource}"}, "", "path"}, + "the placeholder in the fragment": {map[string]any{"record_url": "https://dash.example.com/resource#{resource}"}, "", "path"}, + "a label_read that is a write": {map[string]any{"label_read": map[string]any{"tool": "create_refund", "path": "id"}}, "", "is a write"}, + "a label_read's bad path": {map[string]any{"label_read": map[string]any{"tool": "get_payment_intent", "path": "../x"}}, "", "dotted path"}, + } { + if _, err := with(c.changes, c.source); err == nil || !strings.Contains(err.Error(), c.want) { + t.Errorf("%s: %v, want %q", name, err, c.want) + } + } + // A label_read needs a resource: list_contacts in another vector has none. + raw2, _ := os.ReadFile("testdata/holded-invoices.json") + var p map[string]any + _ = json.Unmarshal(raw2, &p) + for _, o := range p["operations"].([]any) { + if op := o.(map[string]any); op["tool"] == "list_contacts" { + op["label_read"] = map[string]any{"tool": "list_contacts", "path": "name"} + } + } + out, _ := json.Marshal(p) + if _, err := Parse(out); err == nil || !strings.Contains(err.Error(), "needs a resource") { + t.Errorf("a label_read on an operation without a resource: %v", err) + } + // The id is path-escaped into the address: it can't leave the vendor's host or path. + for id, want := range map[string]string{ + "pi_3Abc": "https://dashboard.stripe.com/payments/pi_3Abc", + "../../admin": "https://dashboard.stripe.com/payments/..%2F..%2Fadmin", + "x?y=1#z": "https://dashboard.stripe.com/payments/x%3Fy=1%23z", + "a@evil.example/b": "https://dashboard.stripe.com/payments/a@evil.example%2Fb", + } { + if got := RecordURL("https://dashboard.stripe.com/payments/{resource}", id); got != want { + t.Errorf("%q: %s, want %s", id, got, want) + } + } + if got := RecordURL("https://dashboard.stripe.com/payments", "pi_1"); got != "https://dashboard.stripe.com/payments" { + t.Errorf("no placeholder: %s", got) + } + // An id a browser would read as a step up the path, or none, gives no link. + for _, id := range []string{"", ".", ".."} { + if got := RecordURL("https://dashboard.stripe.com/payments/{resource}", id); got != "" { + t.Errorf("%q: %s, want no link", id, got) + } + } +}