From c6eda96586f91f96c1128d2d6e366e13679f421d Mon Sep 17 00:00:00 2001 From: kiff-admin <352663180+kiff-admin@users.noreply.github.com> Date: Thu, 5 Nov 2026 12:28:52 +0000 Subject: [PATCH 1/2] A profile's operation may say where the vendor's record is, and which read names the resource (kiff/tafs#498) Two optional fields for KIFF's "Did this happen?" ask: record_url, an https address at the vendor with the resource's id path-escaped into {resource}, reviewed catalog only, so no link reaches the owner that no reviewer saw; 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. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 7 +++ docs/connections.md | 7 ++- internal/profile/profile.go | 59 ++++++++++++++++++++++++ internal/profile/profile_test.go | 78 ++++++++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 1 deletion(-) 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..90c0d7e 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,31 @@ 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. +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") + } + parsed, err := url.Parse(strings.ReplaceAll(u, RecordPlaceholder, "resource")) + 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") + } + 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") + } + if strings.Contains(u, RecordPlaceholder) && !strings.Contains(parsed.EscapedPath(), "resource") { + return errors.New("{resource} goes in the address's path") + } + 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. +func RecordURL(template, resource string) string { + 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..2aedce6 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -54,3 +54,81 @@ 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"}, + "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) + } +} From c18197003125407df190f6831df827857ebb3a28 Mon Sep 17 00:00:00 2001 From: kiff-admin <352663180+kiff-admin@users.noreply.github.com> Date: Thu, 5 Nov 2026 12:31:49 +0000 Subject: [PATCH 2/2] record_url: the placeholder by position, and no link for an id that steps up The placeholder's place is checked by where it sits (after the host, before any "?" or "#"), not by a word in the path, so it never lands in the query or the fragment; an empty id, "." or ".." gives no link, so a browser never reads a step up the vendor's path. Co-Authored-By: Claude Fable 5.1 --- internal/profile/profile.go | 31 +++++++++++++++++++++++-------- internal/profile/profile_test.go | 24 ++++++++++++++++-------- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/internal/profile/profile.go b/internal/profile/profile.go index 90c0d7e..54944ab 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -446,27 +446,42 @@ func (p Profile) validateOp(o Operation, byTool map[string]Operation) error { } // 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. +// 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") } - parsed, err := url.Parse(strings.ReplaceAll(u, RecordPlaceholder, "resource")) - 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") - } 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") } - if strings.Contains(u, RecordPlaceholder) && !strings.Contains(parsed.EscapedPath(), "resource") { - return errors.New("{resource} goes in the address's path") + 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. +// 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)) } diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go index 2aedce6..49c76e4 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -91,14 +91,16 @@ func TestRecordURLAndLabelRead(t *testing.T) { 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"}, - "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"}, + "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) @@ -131,4 +133,10 @@ func TestRecordURLAndLabelRead(t *testing.T) { 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) + } + } }