diff --git a/internal/dinners/detail.go b/internal/dinners/detail.go index dfaf779..0eb1f06 100644 --- a/internal/dinners/detail.go +++ b/internal/dinners/detail.go @@ -147,11 +147,11 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail) return fmt.Errorf("recipe %s: %w", recipeID, err) } if !IsManualRecipe(rec.SourceUrl) { - site, err := pageSiteName(rec.SiteName, rec.SourceUrl) + src, err := SourceOf(rec.SiteName, rec.SourceUrl) if err != nil { return err } - d.Source = &Source{SiteName: site, URL: safeSourceURL(rec.SourceUrl)} + d.Source = &src } ings, err := q.ListRecipeIngredients(ctx, recipeID) if err != nil { @@ -182,6 +182,19 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail) return nil } +// SourceOf is the Source of a recipe fetched from sourceURL whose stored +// site name is siteName: the name only when the page gave it (see +// pageSiteName) and the link only when it is free of secrets (see +// safeSourceURL). It is the one way a fetched recipe's source reaches a +// client. +func SourceOf(siteName sql.NullString, sourceURL string) (Source, error) { + site, err := pageSiteName(siteName, sourceURL) + if err != nil { + return Source{}, err + } + return Source{SiteName: site, URL: safeSourceURL(sourceURL)}, nil +} + // pageSiteName is the stored site name of a recipe fetched from sourceURL // when it is one the page gave, else nil. A page that names no site is // stored under recipes.HostSiteName of the URL it was served from: the diff --git a/internal/ideas/ideas.go b/internal/ideas/ideas.go index 41d1dc2..c95524c 100644 --- a/internal/ideas/ideas.go +++ b/internal/ideas/ideas.go @@ -11,6 +11,7 @@ package ideas import ( "context" + "database/sql" "encoding/json" "errors" "fmt" @@ -197,11 +198,13 @@ type Idea struct { // Difficulty is easy, medium or hard. Difficulty string `json:"difficulty"` // Quick says whether the idea is a Quick dinner (see IsQuick). - Quick bool `json:"quick"` - RecipeID string `json:"recipe_id"` - PhotoURL string `json:"photo_url"` - SourceURL string `json:"source_url"` - SiteName string `json:"site_name"` + Quick bool `json:"quick"` + RecipeID string `json:"recipe_id"` + PhotoURL string `json:"photo_url"` + // SourceURL and SiteName are the grounded recipe's source as + // dinners.SourceOf gives it: each null when it could leak a secret. + SourceURL *string `json:"source_url"` + SiteName *string `json:"site_name"` } // Result is the KindFromIngredients result, published as the job goes: @@ -312,9 +315,9 @@ func (s *Service) fill(ctx context.Context, p FromIngredientsJob, publish func(R for i, idea := range out.Ideas { reqs[i] = recipes.GroundRequest{DishName: idea.Name, Query: idea.SearchQuery} } - err = keepGrounded(ctx, s.grounder, reqs, skills.IdeasKept, &res.Ideas, &res.Dropped, func(i int, gr recipes.Grounded) (Idea, bool) { - idea := ideaOf(out.Ideas[i], gr) - return idea, !p.QuickOnly || idea.Quick + err = keepGrounded(ctx, s.grounder, reqs, skills.IdeasKept, &res.Ideas, &res.Dropped, func(i int, gr recipes.Grounded) (Idea, bool, error) { + idea, err := ideaOf(out.Ideas[i], gr) + return idea, !p.QuickOnly || idea.Quick, err }, func() error { return publish(res) }) if err != nil { return res, err @@ -333,9 +336,9 @@ func (s *Service) fill(ctx context.Context, p FromIngredientsJob, publish func(R // order (an idea that grounds sooner than one before it is shown first, // and the earlier one is put back in its place when it grounds) and // publish is called. An idea no recipe grounds, or that keep refuses, -// counts in *dropped. Any other grounding failure, or publish failing, is -// returned (see ground). -func keepGrounded[T any](ctx context.Context, g *recipes.Grounder, reqs []recipes.GroundRequest, want int, shown *[]T, dropped *int, keep func(i int, gr recipes.Grounded) (T, bool), publish func() error) error { +// counts in *dropped. Any other grounding failure, or keep or publish +// failing, is returned (see ground). +func keepGrounded[T any](ctx context.Context, g *recipes.Grounder, reqs []recipes.GroundRequest, want int, shown *[]T, dropped *int, keep func(i int, gr recipes.Grounded) (T, bool, error), publish func() error) error { // order[k] is the index in reqs of (*shown)[k], so each idea is put in // its place in reqs' order as it grounds. var order []int @@ -348,7 +351,10 @@ func keepGrounded[T any](ctx context.Context, g *recipes.Grounder, reqs []recipe *dropped++ return nil } - idea, ok := keep(first+i, *gr) + idea, ok, err := keep(first+i, *gr) + if err != nil { + return err + } if !ok { *dropped++ return nil @@ -418,7 +424,11 @@ func withoutIdeas[R shownResult[R]](stored json.RawMessage) (json.RawMessage, er } // ideaOf is idea as grounded by gr. -func ideaOf(idea skills.Idea, gr recipes.Grounded) Idea { +func ideaOf(idea skills.Idea, gr recipes.Grounded) (Idea, error) { + src, err := sourceOf(gr) + if err != nil { + return Idea{}, err + } return Idea{ Name: idea.Name, Uses: idea.Uses, @@ -430,9 +440,20 @@ func ideaOf(idea skills.Idea, gr recipes.Grounded) Idea { Quick: IsQuick(gr.Method, gr.MinutesTotal), RecipeID: gr.RecipeID, PhotoURL: dinners.PhotoURL(gr.PhotoID), - SourceURL: gr.SourceURL, - SiteName: gr.SiteName, + SourceURL: src.URL, + SiteName: src.SiteName, + }, nil +} + +// sourceOf is gr's recipe source as a client may see it (see +// dinners.SourceOf): a grounded recipe's site name is stored only when +// not empty. +func sourceOf(gr recipes.Grounded) (dinners.Source, error) { + src, err := dinners.SourceOf(sql.NullString{String: gr.SiteName, Valid: gr.SiteName != ""}, gr.SourceURL) + if err != nil { + return dinners.Source{}, fmt.Errorf("ideas: recipe %s: %w", gr.RecipeID, err) } + return src, nil } // QuickJob is the KindQuick payload: the kind of Quick dinner wanted, @@ -478,8 +499,9 @@ type QuickIdea struct { Difficulty string `json:"difficulty"` RecipeID string `json:"recipe_id"` PhotoURL string `json:"photo_url"` - SourceURL string `json:"source_url"` - SiteName string `json:"site_name"` + // SourceURL and SiteName are as in Idea. + SourceURL *string `json:"source_url"` + SiteName *string `json:"site_name"` } // QuickResult is the KindQuick result, published as the job goes: its @@ -564,8 +586,9 @@ func (s *Service) fillQuick(ctx context.Context, p QuickJob, publish func(QuickR for i, idea := range out.Ideas { reqs[i] = recipes.GroundRequest{DishName: idea.Name, Query: idea.SearchQuery} } - err = keepGrounded(ctx, s.grounder, reqs, skills.QuickIdeasKept, &res.Ideas, &res.Dropped, func(i int, gr recipes.Grounded) (QuickIdea, bool) { - return quickIdeaOf(out.Ideas[i], gr), OfQuickKind(p.Kind, gr.Method, gr.MinutesTotal) + err = keepGrounded(ctx, s.grounder, reqs, skills.QuickIdeasKept, &res.Ideas, &res.Dropped, func(i int, gr recipes.Grounded) (QuickIdea, bool, error) { + idea, err := quickIdeaOf(out.Ideas[i], gr) + return idea, OfQuickKind(p.Kind, gr.Method, gr.MinutesTotal), err }, func() error { return publish(res) }) if err != nil { return res, err @@ -578,7 +601,11 @@ func (s *Service) fillQuick(ctx context.Context, p QuickJob, publish func(QuickR } // quickIdeaOf is idea as grounded by gr. -func quickIdeaOf(idea skills.QuickIdea, gr recipes.Grounded) QuickIdea { +func quickIdeaOf(idea skills.QuickIdea, gr recipes.Grounded) (QuickIdea, error) { + src, err := sourceOf(gr) + if err != nil { + return QuickIdea{}, err + } return QuickIdea{ Name: idea.Name, Method: gr.Method, @@ -586,9 +613,9 @@ func quickIdeaOf(idea skills.QuickIdea, gr recipes.Grounded) QuickIdea { Difficulty: gr.Difficulty, RecipeID: gr.RecipeID, PhotoURL: dinners.PhotoURL(gr.PhotoID), - SourceURL: gr.SourceURL, - SiteName: gr.SiteName, - } + SourceURL: src.URL, + SiteName: src.SiteName, + }, nil } // Origins of a dinner made from an idea (the dinners.origin column). diff --git a/internal/ideas/quick_test.go b/internal/ideas/quick_test.go index 04bc7d2..d7acbbc 100644 --- a/internal/ideas/quick_test.go +++ b/internal/ideas/quick_test.go @@ -369,7 +369,7 @@ func TestStartQuickRunsTheJob(t *testing.T) { if res.Stage != StageDone || res.Kind != skills.QuickCrockpot || !reflect.DeepEqual(res.Exclude, []string{"Beef Stew"}) || len(res.Ideas) != 1 { t.Fatalf("result = %+v", res) } - if got := res.Ideas[0]; got.Name != "Pulled Pork" || got.Method != "crockpot" || got.SiteName != "R Kitchen" { + if got := res.Ideas[0]; got.Name != "Pulled Pork" || got.Method != "crockpot" || got.SiteName == nil || *got.SiteName != "R Kitchen" { t.Errorf("idea = %+v", got) } } diff --git a/internal/server/ideasource_test.go b/internal/server/ideasource_test.go new file mode 100644 index 0000000..a858bca --- /dev/null +++ b/internal/server/ideasource_test.go @@ -0,0 +1,124 @@ +package server + +import ( + "encoding/json" + "fmt" + "net/http" + "reflect" + "strings" + "testing" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes/recipestest" +) + +// 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). +type sourceCase struct { + dish, link, site string + // want is the idea's source_url and site_name; nil is null. + wantURL, wantSite any +} + +const ( + cleanSourceLink = "https://kitchen.example/recipes/chicken-curry" + queryToken = "query-token-abc123" + queryHost = "query-host-q1.example" + userinfoToken = "user-token-def456" + userinfoHost = "userinfo-host-u2.example" +) + +// 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. +var sourceCases = []sourceCase{ + {"Chicken Curry", cleanSourceLink, "Curry House", cleanSourceLink, "Curry House"}, + {"Beef Stew", "https://" + queryHost + "/stew?access_token=" + queryToken, "", nil, nil}, + {"Bean Chili", "https://" + userinfoToken + "@" + userinfoHost + "/chili", "", nil, nil}, +} + +// sourceSecrets are what no idea job response may contain. +var sourceSecrets = []string{queryToken, "access_token", queryHost, userinfoToken, userinfoHost} + +// sourceFakes makes the search for each case's dish find its page and +// scripts the normalize_recipe reply grounding it takes. +func sourceFakes(t *testing.T, minutes int) (*llm.Fake, *llm.Fake, *recipestest.Search, *recipestest.Pages, *recipestest.Images) { + t.Helper() + model, normalize, search, pages, images := newQuickFakes() + for i, c := range sourceCases { + photo := fmt.Sprintf("https://img.example/%d.jpg", i) + search.Results[c.dish] = []recipes.Candidate{{URL: c.link}} + pages.Bodies[c.link] = recipestest.RecipePage(c.dish, c.site, photo, minutes) + images.JPEGs[photo] = recipestest.JPEG(t, 400, 320, uint8(i+1)) + normalize.QueueGenerate(recipestest.NormalizedReply("stovetop", minutes)) + } + return model, normalize, search, pages, images +} + +// checkIdeaSources checks the finished job id: each idea shows its case's +// safe source, and the response holds nothing of a token-bearing link. +func checkIdeaSources(t *testing.T, h http.Handler, id string) { + t.Helper() + job := waitJob(t, h, id) + if job["status"] != "succeeded" { + t.Fatalf("GET job = %v", job) + } + rec, _ := callDinners(t, h, http.MethodGet, "/api/v1/jobs/"+id, "") + for _, secret := range sourceSecrets { + if strings.Contains(rec.Body.String(), secret) { + t.Errorf("job response contains %q: %s", secret, rec.Body) + } + } + list := job["result"].(map[string]any)["ideas"].([]any) + if len(list) != len(sourceCases) { + t.Fatalf("ideas = %v, want %d", list, len(sourceCases)) + } + for i, c := range sourceCases { + idea := list[i].(map[string]any) + if idea["name"] != c.dish { + t.Fatalf("idea %d = %v, want %s", i, idea, c.dish) + } + for key, want := range map[string]any{"source_url": c.wantURL, "site_name": c.wantSite} { + got, ok := idea[key] + if !ok || !reflect.DeepEqual(got, want) { + t.Errorf("%s %s = %v (present %t), want %v", c.dish, key, got, ok, want) + } + } + if idea["recipe_id"] == "" || idea["photo_url"] == "" { + t.Errorf("idea %s = %v, want a recipe and photo", c.dish, idea) + } + } +} + +// An ideas_from_ingredients job shows a clean link and its page's name, +// and neither a token-bearing link nor a name taken from its host. +func TestIdeasJobShowsOnlySafeSources(t *testing.T) { + t.Parallel() + model, normalize, search, pages, images := sourceFakes(t, 25) + list := make([]string, len(sourceCases)) + for i, c := range sourceCases { + list[i] = fmt.Sprintf(`{"name":%q,"uses":["rice"],"missing":[],"est_minutes":25,"method":"stovetop","difficulty":"easy","search_query":%q}`, c.dish, c.dish) + } + js := `{"ideas":[` + strings.Join(list, ",") + `]}` + model.QueueGenerate(llm.GenerateResult{Response: llm.Response{Text: js, JSON: json.RawMessage(js)}}) + h, _, _ := newIdeasRouter(t, model, normalize, search, pages, images, true) + checkIdeaSources(t, h, startIdeasJob(t, h)) +} + +// A quick_ideas job shows sources as an ideas_from_ingredients job does. +func TestQuickJobShowsOnlySafeSources(t *testing.T) { + t.Parallel() + model, normalize, search, pages, images := sourceFakes(t, 25) + ideas := make([][3]any, len(sourceCases)) + for i, c := range sourceCases { + ideas[i] = [3]any{c.dish, "stovetop", 20} + } + quickReply(model, ideas...) + h, _, _ := newIdeasRouter(t, model, normalize, search, pages, images, true) + rec, body := callDinners(t, h, http.MethodPost, quickPath, `{"kind": "under30", "exclude": []}`) + if rec.Code != http.StatusAccepted { + t.Fatalf("POST = %d: %s", rec.Code, rec.Body) + } + checkIdeaSources(t, h, body["jobId"].(string)) +} diff --git a/web/src/ideas/CookFromPantryPage.test.tsx b/web/src/ideas/CookFromPantryPage.test.tsx index 6733bf8..f17c70d 100644 --- a/web/src/ideas/CookFromPantryPage.test.tsx +++ b/web/src/ideas/CookFromPantryPage.test.tsx @@ -33,7 +33,9 @@ function idea(name: string, summary: string, minutes: number | null = 30) { // The ideas of mock 1i. const CURRY = idea('Coconut chicken curry', 'Uses all 5 · missing: 1 lime') -const BOWLS = idea('Garlic spinach rice bowls', 'Uses 4 of 5 · nothing missing', 25) +// BOWLS's source is withheld, as the server sends one whose link could +// carry a secret: the card shows all the same. +const BOWLS = { ...idea('Garlic spinach rice bowls', 'Uses 4 of 5 · nothing missing', 25), source_url: null, site_name: null } const ONE_PAN = idea('One-pan chicken & rice', 'Uses 3 of 5 · missing: onion') type JobBody = { status: string; result: unknown; error: string | null } diff --git a/web/src/ideas/QuickDinnersPage.test.tsx b/web/src/ideas/QuickDinnersPage.test.tsx index fe41e8c..90fbe39 100644 --- a/web/src/ideas/QuickDinnersPage.test.tsx +++ b/web/src/ideas/QuickDinnersPage.test.tsx @@ -30,7 +30,9 @@ function idea(name: string, method: string, minutes: number | null, difficulty: // The ideas of mock 1j, and more for later pages and the Crock pot tab. const ORZO = idea('Lemon garlic shrimp orzo', 'stovetop', 25, 'medium') -const QUESADILLAS = idea('Chicken quesadillas', 'stovetop', 20, 'easy') +// QUESADILLAS's source is withheld, as the server sends one whose link +// could carry a secret: the card shows all the same. +const QUESADILLAS = { ...idea('Chicken quesadillas', 'stovetop', 20, 'easy'), source_url: null, site_name: null } const BROCCOLI = idea('Beef & broccoli', 'stovetop', 30, 'easy') const GNOCCHI = idea('Pesto gnocchi & peas', 'stovetop', 15, 'easy') const CHILI = idea('Crock pot chili', 'crockpot', 360, 'easy')