Record source-attribution provenance instead of inferring it from the name shape (Maestro 966066e3) (#202)

Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com>

internal/server/dinnerdetail_test.go

@@ -3,6 +3,7 @@ package server
 import (
 	"database/sql"
 	"encoding/json"
+	"io/fs"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -14,6 +15,7 @@ import (
 	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners"
 	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes/recipestest"
 	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations"
 )
 
 // getDetail answers GET /dinners/{id} for id, failing unless it is 200, and
@@ -328,14 +330,33 @@ func TestGetDinnerSourceURLOnlyWhenSafe(t *testing.T) {
 	}
 }
 
-// TestGetDinnerSiteNameNeverFromLink: a page that names no site is stored
-// under its host, so a recipe grounded in a pasted link whose host carries a
-// token has site_name null and nothing from the link in its detail; a page
-// that names its site keeps that name, even on a token-bearing host; and a
-// stored name shaped like another host, as a redirect leaves, is withheld.
-// The links carry a query, so source.url is withheld too and nothing of the
-// link may appear.
-func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
+// siteNameMigration is migration 0020, which clears the host-shaped site
+// names stored before extraction stopped storing hosts.
+const siteNameMigration = "0020_site_name_from_page.sql"
+
+// applySiteNameMigration runs siteNameMigration again over rows a test
+// stored as the code before it did, as upgrading a database holding them
+// does.
+func applySiteNameMigration(t *testing.T, st *store.Store) {
+	t.Helper()
+	body, err := fs.ReadFile(migrations.FS, siteNameMigration)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := st.Writer.ExecContext(t.Context(), string(body)); err != nil {
+		t.Fatalf("%s: %v", siteNameMigration, err)
+	}
+}
+
+// TestGetDinnerSiteNameProvenance: extraction stores only a site name the
+// page gave, never the URL's host, and the detail applies one rule to what
+// is stored. A name from page metadata is shown even over a token-bearing
+// link; a page naming no site stores no name, and is named by its link's
+// host only when the link is shown — so over a link with a token nothing
+// of the link reaches the client. A name stored before extraction stopped
+// storing hosts is read by the same rule once migration 0020 has cleared
+// it if it could be a host.
+func TestGetDinnerSiteNameProvenance(t *testing.T) {
 	t.Parallel()
 	e := newDinnersEnv(t)
 	const (
@@ -343,11 +364,14 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
 		namedToken = "named-token-def456"
 		bare       = "https://" + bareToken + ".example.com/recipe?print=1"
 		named      = "https://www." + namedToken + ".example.com/recipe?print=1"
+		ordinary   = "https://www.kitchen.example/recipes/curry"
 	)
 	e.pages.Bodies[bare] = recipestest.RecipePage("Chicken Curry", "", "https://a.example/curry.jpg", 40)
 	e.pages.Bodies[named] = recipestest.RecipePage("Weeknight Curry", "Curry House", "https://b.example/curry.jpg", 25)
+	e.pages.Bodies[ordinary] = recipestest.RecipePage("Green Curry", "", "https://c.example/curry.jpg", 30)
 	e.images.JPEGs["https://a.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 1)
 	e.images.JPEGs["https://b.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 2)
+	e.images.JPEGs["https://c.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 3)
 	ground := func(dish, page string) (id, recipeID string) {
 		t.Helper()
 		id = createDinner(t, e.h, `{"name":"`+dish+`"}`) // runner not started: stays pending
@@ -360,15 +384,18 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
 		}
 		return id, recipeID
 	}
-	storedSite := func(recipeID string) string {
+	storedSite := func(recipeID string) any {
 		t.Helper()
-		var site string
+		var site sql.NullString
 		if err := e.st.Reader.QueryRow(`SELECT site_name FROM recipes WHERE id = ?`, recipeID).Scan(&site); err != nil {
 			t.Fatal(err)
 		}
-		return site
+		if !site.Valid {
+			return nil
+		}
+		return site.String
 	}
-	check := func(what, id string, leaks []string, want any) {
+	check := func(what, id string, leaks []string, want map[string]any) {
 		t.Helper()
 		rec, _ := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id, "")
 		if rec.Code != http.StatusOK {
@@ -380,28 +407,48 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
 			}
 		}
 		raw, _ := getDetail(t, e.h, id)
-		if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, map[string]any{"site_name": want, "url": nil}) {
-			t.Errorf("source of %s = %v, want site_name %v and url null", what, raw["source"], want)
+		if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, want) {
+			t.Errorf("source of %s = %v, want %v", what, raw["source"], want)
 		}
 	}
 
-	bareID, bareRecipe := ground("Chicken curry", bare)
-	if got := storedSite(bareRecipe); got != bareToken+".example.com" {
-		t.Fatalf("stored site_name = %q, want the link's host", got)
-	}
-	check("a page naming no site", bareID, []string{bare, bareToken, "example.com"}, nil)
-
 	namedID, namedRecipe := ground("Weeknight curry", named)
 	if got := storedSite(namedRecipe); got != "Curry House" {
-		t.Fatalf("stored site_name = %q, want the page's", got)
+		t.Fatalf("stored site_name = %v, want the page's", got)
 	}
-	check("a page naming its site", namedID, []string{named, namedToken, "example.com"}, "Curry House")
+	check("a metadata-derived name over a token-bearing link", namedID, []string{named, namedToken, "example.com"},
+		map[string]any{"site_name": "Curry House", "url": nil})
 
-	const redirectHost = "redirect-token-ghi789.example.net"
-	if _, err := e.st.Writer.Exec(`UPDATE recipes SET site_name = ? WHERE id = ?`, redirectHost, namedRecipe); err != nil {
-		t.Fatal(err)
+	bareID, bareRecipe := ground("Chicken curry", bare)
+	if got := storedSite(bareRecipe); got != nil {
+		t.Fatalf("stored site_name = %v, want null: the page names no site", got)
 	}
-	check("a page named by a redirect's host", namedID, []string{redirectHost, "redirect-token", namedToken}, nil)
+	check("a hostname-derived name over a token-bearing link", bareID, []string{bare, bareToken, "example.com"},
+		map[string]any{"site_name": nil, "url": nil})
+
+	ordinaryID, ordinaryRecipe := ground("Green curry", ordinary)
+	if got := storedSite(ordinaryRecipe); got != nil {
+		t.Fatalf("stored site_name = %v, want null: the page names no site", got)
+	}
+	check("a hostname-derived name over an ordinary link", ordinaryID, nil,
+		map[string]any{"site_name": "kitchen.example", "url": ordinary})
+
+	// Rows as extraction stored them before it stopped storing hosts: the
+	// token-bearing link's own host, and a redirect's host under a page
+	// that named its site, are cleared by 0020; a page's own name stays.
+	const redirectHost = "redirect-token-ghi789.example.net"
+	for recipeID, site := range map[string]string{bareRecipe: bareToken + ".example.com", namedRecipe: redirectHost, ordinaryRecipe: "Green Kitchen"} {
+		if _, err := e.st.Writer.Exec(`UPDATE recipes SET site_name = ? WHERE id = ?`, site, recipeID); err != nil {
+			t.Fatal(err)
+		}
+	}
+	applySiteNameMigration(t, e.st)
+	check("a row stored before, named by its token-bearing link's host", bareID, []string{bare, bareToken, "example.com"},
+		map[string]any{"site_name": nil, "url": nil})
+	check("a row stored before, named by a redirect's host", namedID, []string{redirectHost, "redirect-token", namedToken},
+		map[string]any{"site_name": nil, "url": nil})
+	check("a row stored before, named by its page", ordinaryID, nil,
+		map[string]any{"site_name": "Green Kitchen", "url": ordinary})
 }
 
 // TestGetDinnerWhyPickedIsCreatingSuggestion: why_picked is the reason of

internal/server/dinners.go

@@ -72,11 +72,11 @@ func getDinners(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
 
 // getDinner answers GET /dinners/{id} with the dinner, its recipe and the
 // reason it was suggested. An archived dinner is 404, like an unknown one,
-// not a 200 with its status: it has no detail to show. source never has the
-// recipe page's link, and source.site_name is null unless the page itself
-// named its site — a name that could be a host, the link's or a redirect's,
-// is withheld (dinners.Source) — so the detail screen shows "Recipe from
-// {site}" only when site_name is not null.
+// not a 200 with its status: it has no detail to show. source is as
+// dinners.SourceOf gives it: the recipe page's link only when it is free of
+// secrets, and site_name the name the page gave its site, else the shown
+// link's host, else null — so the detail screen shows "Recipe from {site}"
+// only when site_name is not null.
 func getDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		d, err := svc.Get(r.Context(), chi.URLParam(r, "id"))
@@ -134,7 +134,8 @@ func patchDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
 }
 
 // getDinnerCandidates answers GET /dinners/{id}/recipe-candidates with up
-// to dinners.MaxCandidates recipe pages for the dinner's name.
+// to dinners.MaxCandidates recipe pages for the dinner's name, each
+// site_name as dinners.SourceOf gives it.
 func getDinnerCandidates(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		list, err := svc.Candidates(r.Context(), chi.URLParam(r, "id"))

internal/server/grounding_test.go

@@ -184,6 +184,51 @@ func TestGetRecipeCandidates(t *testing.T) {
 	}
 }
 
+// TestGetRecipeCandidatesSiteNames: a candidate's site_name follows the
+// recipe detail's rule (dinners.SourceOf), not a hostname fallback of its
+// own: a name the page gave is offered as it stands, even over a link with
+// a token; a page naming no site is named by its link's host only when
+// the link is free of secrets, and is null over a token-bearing link, so
+// nothing of that link reaches the client.
+func TestGetRecipeCandidatesSiteNames(t *testing.T) {
+	t.Parallel()
+	e := newDinnersEnv(t)
+	const (
+		token    = "offer-token-abc123"
+		named    = "https://named.example/curry?access_token=" + token
+		bare     = "https://" + token + ".example.com/curry?print=1"
+		ordinary = "https://www.kitchen.example/recipes/curry"
+	)
+	e.search.Results["Chicken curry"] = []recipes.Candidate{{URL: named}, {URL: bare}, {URL: ordinary}}
+	e.pages.Bodies[named] = recipestest.RecipePage("Named Curry", "Curry House", "https://a.example/curry.jpg", 40)
+	e.pages.Bodies[bare] = recipestest.RecipePage("Bare Curry", "", "https://b.example/curry.jpg", 30)
+	e.pages.Bodies[ordinary] = recipestest.RecipePage("Ordinary Curry", "", "https://c.example/curry.jpg", 25)
+	id := createDinner(t, e.h, `{"name":"Chicken curry"}`) // runner not started: stays pending
+
+	rec, body := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id+"/recipe-candidates", "")
+	if rec.Code != http.StatusOK {
+		t.Fatalf("GET = %d: %s", rec.Code, rec.Body)
+	}
+	for _, leak := range []string{token, "access_token", "example.com"} {
+		if strings.Contains(rec.Body.String(), leak) {
+			t.Errorf("candidates contain %q: %s", leak, rec.Body)
+		}
+	}
+	want := map[string]any{"Named Curry": "Curry House", "Bare Curry": nil, "Ordinary Curry": "kitchen.example"}
+	list, _ := body["candidates"].([]any)
+	if len(list) != len(want) {
+		t.Fatalf("candidates = %v, want %d", body, len(want))
+	}
+	for _, c := range list {
+		c := c.(map[string]any)
+		title, _ := c["title"].(string)
+		site, ok := c["site_name"]
+		if w, known := want[title]; !known || !ok || site != w {
+			t.Errorf("candidate %q site_name = %#v (present %v), want %#v", title, site, ok, w)
+		}
+	}
+}
+
 func TestGetRecipeCandidatesSearchFails(t *testing.T) {
 	t.Parallel()
 	e := newDinnersEnv(t)

internal/server/ideasource_test.go

@@ -16,7 +16,7 @@ import (
 )
 
 // sourceCase is a recipe page an idea grounds in: its link and the site
-// name its page gives (empty for none, so the host is stored instead).
+// name its page gives (empty for none, so no name is stored).
 type sourceCase struct {
 	dish, link, site string
 	// want is the idea's source_url and site_name; nil is null.
@@ -33,7 +33,7 @@ const (
 
 // sourceCases are a clean link, credited by its page's name, and links
 // carrying a token in the query and the userinfo whose pages name no site,
-// so the stored name is the link's host.
+// so no name is stored and, the links being unsafe, none is shown.
 var sourceCases = []sourceCase{
 	{"Chicken Curry", cleanSourceLink, "Curry House", cleanSourceLink, "Curry House"},
 	{"Beef Stew", "https://" + queryHost + "/stew?access_token=" + queryToken, "", nil, nil},
@@ -137,8 +137,9 @@ func legacyIdea(name, link, site string) map[string]any {
 }
 
 // A job of either idea kind whose result was stored before sources were
-// made safe is served with the sources a job stored now holds: neither a
-// token-bearing link nor a name taken from its host.
+// made safe, and upgraded by migration 0020, is served with the sources a
+// job stored now holds: neither a token-bearing link nor a name taken from
+// its host.
 func TestJobServesLegacyIdeaResultsWithOnlySafeSources(t *testing.T) {
 	t.Parallel()
 	model, normalize, search, pages, images := newQuickFakes()
@@ -162,7 +163,10 @@ func TestJobServesLegacyIdeaResultsWithOnlySafeSources(t *testing.T) {
 			id, kind, string(js)); err != nil {
 			t.Fatal(err)
 		}
-		checkIdeaSources(t, h, id)
+	}
+	applySiteNameMigration(t, st)
+	for kind := range map[string]bool{ideas.KindFromIngredients: true, ideas.KindQuick: true} {
+		checkIdeaSources(t, h, "legacy-"+kind)
 	}
 }