From 88fb9de92f3d1a15e257c5d0173c8d4720922062 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Mon, 21 Sep 2026 12:24:32 -0400 Subject: [PATCH 1/2] feat: declare client surfaces in the solution manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A solution can offer something inside a client — the wiki offers citing inside a Word document — and until now the add-in carried a table of who offers what. Manifest gains Surfaces, served beside nav and dashboard, so a client discovers the offering from the solution's own declaration. The runtime validates what a client cannot recover from: a well-formed, unique id, a known client kind, and a module path on the solution's own origin. applies and events are carried verbatim, like dashboard — what a tag selects and what a namespace names are the client's to interpret. Co-Authored-By: Claude Opus 5 (1M context) --- solution.go | 109 +++++++++++++++++++++++++++++++++ solution_test.go | 154 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 263 insertions(+) diff --git a/solution.go b/solution.go index 2324fc6..f074203 100644 --- a/solution.go +++ b/solution.go @@ -50,6 +50,85 @@ type Manifest struct { // validation are owned by the host, so this stays an opaque declaration to // keep the runtime host-agnostic. Dashboard any + // Surfaces are what this solution offers inside a client application — a + // Word add-in, a Slack app — rather than as a page the host renders. The + // host's registry projects them so a client can discover, without a table + // of its own, which solutions offer it something. + Surfaces []Surface +} + +// Surface is one offering a solution makes inside a client application. +type Surface struct { + ID string // unique within the solution, e.g. "footnote" + Client string // client kind: word, powerpoint, excel, slack + Title string + Description string + // Module is the path, on this solution's own origin, of a self-contained ES + // module the client loads. Same-origin: the client resolves it against the + // solution it is loading, so a full URL here would let a manifest point a + // client's module loader at a third party. + Module string + // Contract is the surface contract major this module is built against + // (obin-ai/obin-word docs/SURFACE.md), so a client refuses a surface it + // cannot run rather than loading it and failing inside. + Contract int + // Applies is "always" or a {"tagged": [...]} selector, and Events are the + // journal namespaces the surface reconciles on. Both are carried verbatim: + // what a tag selects and what a namespace names are the client's to + // interpret, not this runtime's. + Applies any + Events []string +} + +// surfaceClients are the client kinds a surface may declare. A client loads +// only what names it, so an unknown kind is a surface nothing will ever load — +// caught at boot rather than showing up as a solution that simply never +// appears in the add-in. +var surfaceClients = map[string]bool{"word": true, "powerpoint": true, "excel": true, "slack": true} + +// validateSurfaces refuses a surface declaration no client could use. The +// author writes these in code, so every failure here is a mistake a boot should +// name rather than a condition the runtime can recover from. +func (m Manifest) validateSurfaces() error { + seen := make(map[string]bool, len(m.Surfaces)) + for i, surface := range m.Surfaces { + switch { + case surface.ID == "": + return fmt.Errorf("surface %d has no id", i) + case !isSurfaceID(surface.ID): + return fmt.Errorf("surface %q: id must be lowercase letters, digits and dashes", surface.ID) + case seen[surface.ID]: + return fmt.Errorf("surface %q declared twice: a client addresses a surface by id, so two cannot share one", surface.ID) + case !surfaceClients[surface.Client]: + return fmt.Errorf("surface %q: unknown client kind %q", surface.ID, surface.Client) + case !isSameOriginPath(surface.Module): + return fmt.Errorf("surface %q: module %q must be a path on this solution's own origin, e.g. %q", + surface.ID, surface.Module, "/surfaces/"+surface.Client+"/"+surface.ID+".js") + } + seen[surface.ID] = true + } + return nil +} + +func isSurfaceID(id string) bool { + for _, r := range id { + if !(r >= 'a' && r <= 'z' || r >= '0' && r <= '9' || r == '-') { + return false + } + } + return true +} + +// isSameOriginPath reports whether p addresses this solution's own origin: a +// rooted path and nothing else. "//host/x.js" is rejected explicitly — it is a +// rooted path to read but a protocol-relative URL to a browser, which is the +// one form that looks same-origin here and is not one once loaded. +func isSameOriginPath(p string) bool { + if !strings.HasPrefix(p, "/") || strings.HasPrefix(p, "//") { + return false + } + u, err := url.Parse(p) + return err == nil && u.Scheme == "" && u.Host == "" } // Handler is a solution endpoint. It receives a Gateway bound to the caller's @@ -473,6 +552,12 @@ func (s *Server) HandleRequest(path string, handler RequestHandler) *Server { // Serve reads env config, self-registers, and blocks serving the solution. func (s *Server) Serve() error { ctx := context.Background() + // Before anything the environment owns: a surface the author declared wrong + // is wrong in every environment, and saying so first keeps that mistake from + // reading as one more unresolved address. + if err := s.manifest.validateSurfaces(); err != nil { + return fmt.Errorf("solution %q: %w", s.manifest.ID, err) + } // The SDK owns environment resolution: load Codefly's injected carriers so // endpoint and workspace-secret lookups resolve from them (falling back to // the local native workspace map when not running under the runtime). @@ -644,6 +729,30 @@ func (s *Server) manifestMap() map[string]any { if s.manifest.Dashboard != nil { m["dashboard"] = s.manifest.Dashboard } + if len(s.manifest.Surfaces) > 0 { + surfaces := make([]any, 0, len(s.manifest.Surfaces)) + for _, surface := range s.manifest.Surfaces { + entry := map[string]any{ + "id": surface.ID, + "client": surface.Client, + "title": surface.Title, + "description": surface.Description, + "module": surface.Module, + "contract": surface.Contract, + } + // Same reasoning as the dashboard slot above: the host defaults what + // the solution left unsaid, and an explicit null would instead be a + // declaration that no selector and no namespace apply. + if surface.Applies != nil { + entry["applies"] = surface.Applies + } + if len(surface.Events) > 0 { + entry["events"] = surface.Events + } + surfaces = append(surfaces, entry) + } + m["surfaces"] = surfaces + } return m } diff --git a/solution_test.go b/solution_test.go index b114b70..47826ac 100644 --- a/solution_test.go +++ b/solution_test.go @@ -225,6 +225,160 @@ func TestManifestOmitsAbsentDashboard(t *testing.T) { } } +// wordFootnote is the surface shape the wiki declares for a Word document: the +// whole set of slots a client reads, so a test can assert on all of them. +func wordFootnote() Surface { + return Surface{ + ID: "footnote", + Client: "word", + Title: "Footnote", + Description: "Cite a claim from the wiki.", + Module: "/surfaces/word/footnote.js", + Contract: 1, + Applies: "always", + Events: []string{"documents.entry.*"}, + } +} + +// A client discovers what a solution offers it from the served manifest alone, +// so every declared slot must survive the trip through JSON — including the +// applies selector, which the runtime carries without interpreting. +func TestServedManifestCarriesDeclaredSurfaces(t *testing.T) { + s := &Server{manifest: Manifest{ID: "wiki", Surfaces: []Surface{wordFootnote()}}} + + var got map[string]any + body, err := json.Marshal(s.manifestMap()) + if err != nil { + t.Fatalf("marshal manifest: %v", err) + } + if err := json.Unmarshal(body, &got); err != nil { + t.Fatalf("unmarshal manifest: %v", err) + } + want := []any{map[string]any{ + "id": "footnote", + "client": "word", + "title": "Footnote", + "description": "Cite a claim from the wiki.", + "module": "/surfaces/word/footnote.js", + "contract": float64(1), + "applies": "always", + "events": []any{"documents.entry.*"}, + }} + if !reflect.DeepEqual(got["surfaces"], want) { + t.Errorf("served surfaces = %#v, want %#v", got["surfaces"], want) + } +} + +// A tagged selector is a structure, not a keyword, and the runtime must hand it +// to the client exactly as declared rather than flattening it. +func TestServedSurfaceCarriesTaggedAppliesVerbatim(t *testing.T) { + applies := map[string]any{"tagged": []any{"legal", "finance"}} + surface := wordFootnote() + surface.Applies = applies + s := &Server{manifest: Manifest{ID: "wiki", Surfaces: []Surface{surface}}} + + var got map[string]any + body, _ := json.Marshal(s.manifestMap()) + if err := json.Unmarshal(body, &got); err != nil { + t.Fatalf("unmarshal manifest: %v", err) + } + entry := got["surfaces"].([]any)[0].(map[string]any) + if !reflect.DeepEqual(entry["applies"], applies) { + t.Errorf("served applies = %#v, want %#v", entry["applies"], applies) + } +} + +// A solution that offers nothing inside a client must not gain the key, and one +// that declares no selector or namespaces must not gain those slots: the host +// defaults what a solution left unsaid, and a null says something else. +func TestManifestOmitsUndeclaredSurfaceSlots(t *testing.T) { + bare := &Server{manifest: Manifest{ID: "lastlogin-go"}} + if _, ok := bare.manifestMap()["surfaces"]; ok { + t.Errorf("emitted a surfaces key with no surface declared") + } + + surface := wordFootnote() + surface.Applies = nil + surface.Events = nil + s := &Server{manifest: Manifest{ID: "wiki", Surfaces: []Surface{surface}}} + entry := s.manifestMap()["surfaces"].([]any)[0].(map[string]any) + if _, ok := entry["applies"]; ok { + t.Errorf("emitted an applies key with no selector declared") + } + if _, ok := entry["events"]; ok { + t.Errorf("emitted an events key with no namespaces declared") + } +} + +// A surface no client could load must fail the boot, not register quietly and +// show up as a solution that simply never appears in the add-in. +func TestSurfaceValidationRejectsUnusableDeclarations(t *testing.T) { + cases := []struct { + name string + surfaces []Surface + want string + }{ + {"no id", []Surface{func() Surface { s := wordFootnote(); s.ID = ""; return s }()}, "no id"}, + {"id with uppercase", []Surface{func() Surface { s := wordFootnote(); s.ID = "Footnote"; return s }()}, "id must be"}, + {"id with a slash", []Surface{func() Surface { s := wordFootnote(); s.ID = "word/footnote"; return s }()}, "id must be"}, + {"duplicate id", []Surface{wordFootnote(), wordFootnote()}, "declared twice"}, + {"unknown client", []Surface{func() Surface { s := wordFootnote(); s.Client = "notion"; return s }()}, "unknown client kind"}, + {"no client", []Surface{func() Surface { s := wordFootnote(); s.Client = ""; return s }()}, "unknown client kind"}, + {"absolute module URL", []Surface{func() Surface { + s := wordFootnote() + s.Module = "https://evil.example/footnote.js" + return s + }()}, "own origin"}, + {"protocol-relative module", []Surface{func() Surface { + s := wordFootnote() + s.Module = "//evil.example/footnote.js" + return s + }()}, "own origin"}, + {"relative module", []Surface{func() Surface { s := wordFootnote(); s.Module = "surfaces/footnote.js"; return s }()}, "own origin"}, + {"no module", []Surface{func() Surface { s := wordFootnote(); s.Module = ""; return s }()}, "own origin"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := Manifest{ID: "wiki", Surfaces: tc.surfaces}.validateSurfaces() + if err == nil { + t.Fatalf("validateSurfaces accepted %s", tc.name) + } + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("error %q does not say %q", err, tc.want) + } + }) + } +} + +// Two surfaces for two clients are the normal case — the wiki offers one inside +// Word and one inside Slack — and must not read as a duplicate. +func TestSurfaceValidationAcceptsSeveralClients(t *testing.T) { + slack := wordFootnote() + slack.ID = "ask" + slack.Client = "slack" + slack.Module = "/surfaces/slack/ask.js" + if err := (Manifest{ID: "wiki", Surfaces: []Surface{wordFootnote(), slack}}).validateSurfaces(); err != nil { + t.Errorf("validateSurfaces refused a valid pair: %v", err) + } +} + +// Serve must refuse a manifest whose surface no client could load, and name the +// surface — the mistake is in the author's code, and it is the same mistake in +// every environment, so it must not wait on the environment resolving first. +func TestServeRejectsUnusableSurface(t *testing.T) { + surface := wordFootnote() + surface.Module = "https://evil.example/footnote.js" + + s := New(Manifest{ID: "wiki", Title: "Wiki", Surfaces: []Surface{surface}}) + err := s.Serve() + if err == nil { + t.Fatal("Serve returned nil for an off-origin surface module; expected a boot error and no bind") + } + if !strings.Contains(err.Error(), "footnote") { + t.Errorf("boot error should name the surface, got: %v", err) + } +} + // The host registration payload (the heartbeat body) must carry the declared // data-graph verbatim, not just the GET manifest — that is the surface the host // self-registration path reads. From 64def6eafde56508672d580a43780501d855d322 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Mon, 21 Sep 2026 19:41:58 -0400 Subject: [PATCH 2/2] fix: close the surface validations that let an unusable surface through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The module check did not hold. A browser resolves "/\host/x.js" by reading the backslash as a slash and what follows as an authority, so it loads https://host/x.js, while net/url reads that backslash as an ordinary path byte and reports no host — the check asked net/url and so accepted it. ASCII tab and newline are removed by that same parser before it parses, turning "//host/x.js" protocol-relative on arrival; those were refused only because net/url happens to reject control characters. Both are now refused by inspecting the bytes, because what makes a path rooted is the parser that will resolve it, not the one validating it. The check catches an author's mistake, which is all a validation here can do: a solution serves its own manifest, so the host's registry and the client's loader still own that boundary. A zero Contract was emitted as "contract": 0. A client runs a surface only when it recognises the major it declares, so that shipped a surface every client refuses, from a boot that looked healthy and logged nothing — the silent no-op the rest of this runtime's validation exists to prevent. An empty Title had the same shape one step further out: a client labels the surface with it and renders a blank control. Both are now refused at boot. Surface ids were unique per solution, which refused the ordinary case of one offering in two clients — "footnote" in Word and in PowerPoint. Two clients never see each other's surfaces, so only a collision within one client is ambiguous; the key is now (client, id). An undeclared Applies was omitted, leaving every client to invent the same default for a surface that is otherwise fully declared. It is now served as "always". Description and Events keep being omitted when empty: absent, those say something a client can act on. Co-Authored-By: Claude Opus 5 (1M context) --- solution.go | 104 +++++++++++++++++++++++++++++++++++------------ solution_test.go | 84 ++++++++++++++++++++++++++++++-------- 2 files changed, 145 insertions(+), 43 deletions(-) diff --git a/solution.go b/solution.go index f074203..c18c341 100644 --- a/solution.go +++ b/solution.go @@ -59,14 +59,20 @@ type Manifest struct { // Surface is one offering a solution makes inside a client application. type Surface struct { - ID string // unique within the solution, e.g. "footnote" - Client string // client kind: word, powerpoint, excel, slack - Title string + ID string // unique per client, e.g. "footnote" + Client string // client kind: word, powerpoint, excel, slack + Title string // what the client labels the surface with + // Description is the longer explanation a client may show beside the title. + // Optional, and omitted from the manifest when empty so a client renders no + // description rather than an empty one. Description string // Module is the path, on this solution's own origin, of a self-contained ES - // module the client loads. Same-origin: the client resolves it against the - // solution it is loading, so a full URL here would let a manifest point a - // client's module loader at a third party. + // module the client loads, resolved against the origin the client loaded + // this solution from. The check on it catches an author reaching off that + // origin by mistake; it is not what keeps a hostile solution from doing so + // deliberately, since a solution serves its own manifest and need not use + // this runtime at all. The host's registry and the client's loader own that + // boundary and have to enforce it themselves. Module string // Contract is the surface contract major this module is built against // (obin-ai/obin-word docs/SURFACE.md), so a client refuses a surface it @@ -80,6 +86,12 @@ type Surface struct { Events []string } +// surfaceAppliesAlways is what an undeclared Applies is served as. A surface is +// present in the manifest either way, so leaving the slot out would not say +// "no surface" the way an absent dashboard does — it would ask every client to +// invent the same default for a surface that is otherwise fully declared. +const surfaceAppliesAlways = "always" + // surfaceClients are the client kinds a surface may declare. A client loads // only what names it, so an unknown kind is a surface nothing will ever load — // caught at boot rather than showing up as a solution that simply never @@ -90,29 +102,53 @@ var surfaceClients = map[string]bool{"word": true, "powerpoint": true, "excel": // author writes these in code, so every failure here is a mistake a boot should // name rather than a condition the runtime can recover from. func (m Manifest) validateSurfaces() error { + // Keyed by client and id together: two clients never see each other's + // surfaces, so the same offering carries the same id in Word and in + // PowerPoint, and only a collision within one client is ambiguous. The id + // and client checks below run first, so neither half of the key can contain + // the separator and no two distinct pairs can collide into one string. seen := make(map[string]bool, len(m.Surfaces)) for i, surface := range m.Surfaces { switch { case surface.ID == "": return fmt.Errorf("surface %d has no id", i) case !isSurfaceID(surface.ID): - return fmt.Errorf("surface %q: id must be lowercase letters, digits and dashes", surface.ID) - case seen[surface.ID]: - return fmt.Errorf("surface %q declared twice: a client addresses a surface by id, so two cannot share one", surface.ID) + return fmt.Errorf("surface %q: id must be lowercase letters and digits, separated by dashes", surface.ID) + case surface.Title == "": + return fmt.Errorf("surface %q has no title: a client labels the surface with it, and renders a blank control without one", surface.ID) case !surfaceClients[surface.Client]: return fmt.Errorf("surface %q: unknown client kind %q", surface.ID, surface.Client) + case seen[surface.Client+"/"+surface.ID]: + return fmt.Errorf("surface %q declared twice for client %q: a client addresses a surface by id, so two of its own cannot share one", + surface.ID, surface.Client) + case surface.Contract < 1: + // A client runs a surface only when it recognises the contract + // major it declares, so the zero an author leaves behind is not a + // lenient default: it is a surface every client refuses, in a boot + // that otherwise looks entirely healthy. + return fmt.Errorf("surface %q: contract %d is not a surface contract major; declare the major this module is built against (>= 1)", + surface.ID, surface.Contract) case !isSameOriginPath(surface.Module): return fmt.Errorf("surface %q: module %q must be a path on this solution's own origin, e.g. %q", surface.ID, surface.Module, "/surfaces/"+surface.Client+"/"+surface.ID+".js") } - seen[surface.ID] = true + seen[surface.Client+"/"+surface.ID] = true } return nil } +// isSurfaceID accepts lowercase letters and digits separated by dashes. A +// leading or trailing dash is refused so that an id is never a prefix game, and +// so the empty string cannot pass by having no characters to object to. func isSurfaceID(id string) bool { - for _, r := range id { - if !(r >= 'a' && r <= 'z' || r >= '0' && r <= '9' || r == '-') { + if id == "" { + return false + } + for i, r := range id { + if r >= 'a' && r <= 'z' || r >= '0' && r <= '9' { + continue + } + if r != '-' || i == 0 || i == len(id)-1 { return false } } @@ -120,13 +156,26 @@ func isSurfaceID(id string) bool { } // isSameOriginPath reports whether p addresses this solution's own origin: a -// rooted path and nothing else. "//host/x.js" is rejected explicitly — it is a -// rooted path to read but a protocol-relative URL to a browser, which is the -// one form that looks same-origin here and is not one once loaded. +// rooted path and nothing else. +// +// Whether a path is rooted is decided by the parser that will resolve it — the +// client's, not Go's — and the two disagree on three inputs. A browser follows +// WHATWG URL, where a backslash beside the leading slash is read as a slash and +// what follows it as an authority, so "/\host/x.js" resolves to https://host/x.js +// while net/url reads that backslash as an ordinary path byte and reports no +// host at all; and where ASCII tab and newline are removed from the input before +// it is parsed, so "//host/x.js" becomes protocol-relative on arrival. None +// of the three can be seen in net/url's answer, so they are refused here by +// inspecting the bytes rather than by asking net/url what it made of them. func isSameOriginPath(p string) bool { if !strings.HasPrefix(p, "/") || strings.HasPrefix(p, "//") { return false } + for _, r := range p { + if r == '\\' || unicode.IsControl(r) { + return false + } + } u, err := url.Parse(p) return err == nil && u.Scheme == "" && u.Host == "" } @@ -733,18 +782,21 @@ func (s *Server) manifestMap() map[string]any { surfaces := make([]any, 0, len(s.manifest.Surfaces)) for _, surface := range s.manifest.Surfaces { entry := map[string]any{ - "id": surface.ID, - "client": surface.Client, - "title": surface.Title, - "description": surface.Description, - "module": surface.Module, - "contract": surface.Contract, + "id": surface.ID, + "client": surface.Client, + "title": surface.Title, + "module": surface.Module, + "contract": surface.Contract, + "applies": surface.Applies, + } + if surface.Applies == nil { + entry["applies"] = surfaceAppliesAlways } - // Same reasoning as the dashboard slot above: the host defaults what - // the solution left unsaid, and an explicit null would instead be a - // declaration that no selector and no namespace apply. - if surface.Applies != nil { - entry["applies"] = surface.Applies + // Absent, these two say something a client can act on — no + // description to show, nothing to reconcile on — where a null would + // only be a slot the client still has to interpret. + if surface.Description != "" { + entry["description"] = surface.Description } if len(surface.Events) > 0 { entry["events"] = surface.Events diff --git a/solution_test.go b/solution_test.go index 47826ac..91ab79d 100644 --- a/solution_test.go +++ b/solution_test.go @@ -288,22 +288,32 @@ func TestServedSurfaceCarriesTaggedAppliesVerbatim(t *testing.T) { } } -// A solution that offers nothing inside a client must not gain the key, and one -// that declares no selector or namespaces must not gain those slots: the host -// defaults what a solution left unsaid, and a null says something else. -func TestManifestOmitsUndeclaredSurfaceSlots(t *testing.T) { +// A solution that offers nothing inside a client must not gain the key at all. +func TestManifestOmitsAbsentSurfaces(t *testing.T) { bare := &Server{manifest: Manifest{ID: "lastlogin-go"}} if _, ok := bare.manifestMap()["surfaces"]; ok { t.Errorf("emitted a surfaces key with no surface declared") } +} +// A surface is in the manifest whether or not it declares a selector, so an +// absent applies is not the absence of the surface — it is the default the +// contract names, and every client would otherwise have to invent it alike. +// Nothing to show and nothing to reconcile on are different: those slots say +// what they mean by being absent. +func TestManifestDefaultsAppliesAndOmitsEmptySurfaceSlots(t *testing.T) { surface := wordFootnote() surface.Applies = nil surface.Events = nil + surface.Description = "" s := &Server{manifest: Manifest{ID: "wiki", Surfaces: []Surface{surface}}} + entry := s.manifestMap()["surfaces"].([]any)[0].(map[string]any) - if _, ok := entry["applies"]; ok { - t.Errorf("emitted an applies key with no selector declared") + if got := entry["applies"]; got != "always" { + t.Errorf("applies with no selector declared = %#v, want %q", got, "always") + } + if _, ok := entry["description"]; ok { + t.Errorf("emitted a description key with no description declared") } if _, ok := entry["events"]; ok { t.Errorf("emitted an events key with no namespaces declared") @@ -321,9 +331,15 @@ func TestSurfaceValidationRejectsUnusableDeclarations(t *testing.T) { {"no id", []Surface{func() Surface { s := wordFootnote(); s.ID = ""; return s }()}, "no id"}, {"id with uppercase", []Surface{func() Surface { s := wordFootnote(); s.ID = "Footnote"; return s }()}, "id must be"}, {"id with a slash", []Surface{func() Surface { s := wordFootnote(); s.ID = "word/footnote"; return s }()}, "id must be"}, - {"duplicate id", []Surface{wordFootnote(), wordFootnote()}, "declared twice"}, + {"id with a leading dash", []Surface{func() Surface { s := wordFootnote(); s.ID = "-footnote"; return s }()}, "id must be"}, + {"id with a trailing dash", []Surface{func() Surface { s := wordFootnote(); s.ID = "footnote-"; return s }()}, "id must be"}, + {"id of one dash", []Surface{func() Surface { s := wordFootnote(); s.ID = "-"; return s }()}, "id must be"}, + {"duplicate id in one client", []Surface{wordFootnote(), wordFootnote()}, "declared twice"}, + {"no title", []Surface{func() Surface { s := wordFootnote(); s.Title = ""; return s }()}, "no title"}, {"unknown client", []Surface{func() Surface { s := wordFootnote(); s.Client = "notion"; return s }()}, "unknown client kind"}, {"no client", []Surface{func() Surface { s := wordFootnote(); s.Client = ""; return s }()}, "unknown client kind"}, + {"undeclared contract", []Surface{func() Surface { s := wordFootnote(); s.Contract = 0; return s }()}, "contract"}, + {"negative contract", []Surface{func() Surface { s := wordFootnote(); s.Contract = -1; return s }()}, "contract"}, {"absolute module URL", []Surface{func() Surface { s := wordFootnote() s.Module = "https://evil.example/footnote.js" @@ -334,6 +350,29 @@ func TestSurfaceValidationRejectsUnusableDeclarations(t *testing.T) { s.Module = "//evil.example/footnote.js" return s }()}, "own origin"}, + // A browser reads the backslash beside the leading slash as a slash and + // what follows as a host, so each of these loads third-party code even + // though net/url reports no host in any of them. + {"backslash authority module", []Surface{func() Surface { + s := wordFootnote() + s.Module = `/\evil.example/footnote.js` + return s + }()}, "own origin"}, + {"backslash-slash authority module", []Surface{func() Surface { + s := wordFootnote() + s.Module = `/\/evil.example/footnote.js` + return s + }()}, "own origin"}, + {"tab-smuggled authority module", []Surface{func() Surface { + s := wordFootnote() + s.Module = "/\t/evil.example/footnote.js" + return s + }()}, "own origin"}, + {"newline-smuggled authority module", []Surface{func() Surface { + s := wordFootnote() + s.Module = "/\n//evil.example/footnote.js" + return s + }()}, "own origin"}, {"relative module", []Surface{func() Surface { s := wordFootnote(); s.Module = "surfaces/footnote.js"; return s }()}, "own origin"}, {"no module", []Surface{func() Surface { s := wordFootnote(); s.Module = ""; return s }()}, "own origin"}, } @@ -350,15 +389,26 @@ func TestSurfaceValidationRejectsUnusableDeclarations(t *testing.T) { } } -// Two surfaces for two clients are the normal case — the wiki offers one inside -// Word and one inside Slack — and must not read as a duplicate. -func TestSurfaceValidationAcceptsSeveralClients(t *testing.T) { - slack := wordFootnote() - slack.ID = "ask" - slack.Client = "slack" - slack.Module = "/surfaces/slack/ask.js" - if err := (Manifest{ID: "wiki", Surfaces: []Surface{wordFootnote(), slack}}).validateSurfaces(); err != nil { - t.Errorf("validateSurfaces refused a valid pair: %v", err) +// The same offering in two clients carries the same id — the wiki's footnote is +// "footnote" in Word and in PowerPoint — and neither client can see the other's, +// so that is not a collision. Only two surfaces of one client are ambiguous. +func TestSurfaceIDsAreUniquePerClientNotPerSolution(t *testing.T) { + powerpoint := wordFootnote() + powerpoint.Client = "powerpoint" + powerpoint.Module = "/surfaces/powerpoint/footnote.js" + if err := (Manifest{ID: "wiki", Surfaces: []Surface{wordFootnote(), powerpoint}}).validateSurfaces(); err != nil { + t.Errorf("validateSurfaces refused one id shared across two clients: %v", err) + } + + second := wordFootnote() + second.Title = "Footnote, again" + second.Module = "/surfaces/word/footnote-2.js" + err := (Manifest{ID: "wiki", Surfaces: []Surface{wordFootnote(), second}}).validateSurfaces() + if err == nil { + t.Fatal("validateSurfaces accepted two surfaces sharing an id within one client") + } + if !strings.Contains(err.Error(), "declared twice") { + t.Errorf("error %q does not say the id was declared twice", err) } } @@ -367,7 +417,7 @@ func TestSurfaceValidationAcceptsSeveralClients(t *testing.T) { // every environment, so it must not wait on the environment resolving first. func TestServeRejectsUnusableSurface(t *testing.T) { surface := wordFootnote() - surface.Module = "https://evil.example/footnote.js" + surface.Module = `/\evil.example/footnote.js` s := New(Manifest{ID: "wiki", Title: "Wiki", Surfaces: []Surface{surface}}) err := s.Serve()