diff --git a/internal/dinners/detail.go b/internal/dinners/detail.go index dfaf779..6ccb079 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,7 +182,22 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail) return nil } -// pageSiteName is the stored site name of a recipe fetched from sourceURL +// 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. A sourceURL that does not parse is an error, whether or not a +// site name is stored. +func SourceOf(siteName sql.NullString, sourceURL string) (Source, error) { + link, err := url.Parse(sourceURL) + if err != nil { + // Not wrapped: url.Parse's error quotes the link. + return Source{}, errors.New("source url does not parse") + } + return Source{SiteName: pageSiteName(siteName, link), URL: safeSourceURL(sourceURL)}, nil +} + +// pageSiteName is the stored site name of a recipe fetched from link // 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 // pasted link's host, which may hold a personal token (a private @@ -192,23 +207,18 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail) // is treated as one and withheld. A site that names itself as a domain, // such as "Food.com", loses its name with them: a credit that does not // name the site is better than one that leaks a link. -func pageSiteName(stored sql.NullString, sourceURL string) (*string, error) { +func pageSiteName(stored sql.NullString, link *url.URL) *string { if !stored.Valid { - return nil, nil - } - link, err := url.Parse(sourceURL) - if err != nil { - // Not wrapped: url.Parse's error quotes the link. - return nil, errors.New("source url does not parse") + return nil } name := stored.String if strings.EqualFold(name, recipes.HostSiteName(link)) || net.ParseIP(name) != nil { - return nil, nil + return nil } if u, err := url.Parse("https://" + name + "/"); err == nil && u.Host == name && strings.Contains(name, ".") { - return nil, nil + return nil } - return &name, nil + return &name } // safeSourceURL is sourceURL when it is verifiably free of secrets — an diff --git a/internal/dinners/dinners_test.go b/internal/dinners/dinners_test.go index dbd8bce..14e0ff6 100644 --- a/internal/dinners/dinners_test.go +++ b/internal/dinners/dinners_test.go @@ -99,16 +99,19 @@ func TestPageSiteNameWithholdsHosts(t *testing.T) { {"Bon Appétit", true, ptr("Bon Appétit")}, {"Mr. Food Test Kitchen", true, ptr("Mr. Food Test Kitchen")}, } { - got, err := dinners.PageSiteName(sql.NullString{String: tc.stored, Valid: tc.valid}, link) + src, err := dinners.SourceOf(sql.NullString{String: tc.stored, Valid: tc.valid}, link) if err != nil { - t.Fatalf("PageSiteName(%q): %v", tc.stored, err) + t.Fatalf("SourceOf(%q): %v", tc.stored, err) } - if (got == nil) != (tc.want == nil) || (got != nil && *got != *tc.want) { - t.Errorf("PageSiteName(%q) = %v, want %v", tc.stored, deref(got), deref(tc.want)) + if got := src.SiteName; (got == nil) != (tc.want == nil) || (got != nil && *got != *tc.want) { + t.Errorf("SourceOf(%q) site name = %v, want %v", tc.stored, deref(got), deref(tc.want)) } } - if _, err := dinners.PageSiteName(sql.NullString{String: "Delish", Valid: true}, "https://secret-token\x7f.example/%zz"); err == nil || strings.Contains(err.Error(), "secret-token") { - t.Errorf("PageSiteName of an unparseable link: err = %v, want one not quoting the link", err) + // A link that does not parse is an error with or without a site name. + for _, site := range []sql.NullString{{String: "Delish", Valid: true}, {}} { + if _, err := dinners.SourceOf(site, "https://secret-token\x7f.example/%zz"); err == nil || strings.Contains(err.Error(), "secret-token") { + t.Errorf("SourceOf(%v) of an unparseable link: err = %v, want one not quoting the link", site, err) + } } } diff --git a/internal/dinners/export_test.go b/internal/dinners/export_test.go index 1d62211..0f4a731 100644 --- a/internal/dinners/export_test.go +++ b/internal/dinners/export_test.go @@ -72,8 +72,5 @@ func (s *Service) RunGroundJob(ctx context.Context, p GroundJob) (string, error) return r.GroundingStatus, err } -// PageSiteName is pageSiteName. -var PageSiteName = pageSiteName - // SafeSourceURL is safeSourceURL. var SafeSourceURL = safeSourceURL diff --git a/internal/ideas/ideas.go b/internal/ideas/ideas.go index 41d1dc2..557b512 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" @@ -123,18 +124,18 @@ func NewService(st *store.Store, grounder *recipes.Grounder, generate Generator, } s := &Service{st: st, grounder: grounder, runner: runner, generate: generate, quick: quick, logger: logger} var err error - if s.job, err = register(runner, KindFromIngredients, s.run, withoutIdeas[Result]); err != nil { + if s.job, err = register(runner, KindFromIngredients, s.run, shownJobResult[Result]); err != nil { return nil, err } - if s.quickJob, err = register(runner, KindQuick, s.runQuick, withoutIdeas[QuickResult]); err != nil { + if s.quickJob, err = register(runner, KindQuick, s.runQuick, shownJobResult[QuickResult]); err != nil { return nil, err } return s, nil } -// register registers the service-only job name, run by run, whose failed -// jobs show their result through failed. -func register[P any, R jobs.Public](runner *jobs.Runner, name string, run jobs.Handler[P, R], failed func(json.RawMessage) (json.RawMessage, error)) (*jobs.Kind[P], error) { +// register registers the service-only job name, run by run, whose jobs +// show their result through show. +func register[P any, R jobs.Public](runner *jobs.Runner, name string, run jobs.Handler[P, R], show func(jobs.Status, json.RawMessage) (json.RawMessage, error)) (*jobs.Kind[P], error) { kind, err := jobs.Register(runner, name, run, Reasons...) if err != nil { return nil, err @@ -142,7 +143,7 @@ func register[P any, R jobs.Public](runner *jobs.Runner, name string, run jobs.H if err := kind.ServiceOnly(); err != nil { return nil, err } - if err := kind.FailedResult(failed); err != nil { + if err := kind.ShownResult(show); err != nil { return nil, err } return kind, nil @@ -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: @@ -251,6 +254,9 @@ type shownResult[R any] interface { shown() int // cleared is the result showing no ideas. cleared() R + // withSafeSources is the result with each idea's source as + // shownSource gives it. + withSafeSources() (R, error) } func (r Result) shown() int { return len(r.Ideas) } @@ -260,11 +266,25 @@ func (r Result) cleared() Result { return r } +func (r Result) withSafeSources() (Result, error) { + ideas := make([]Idea, len(r.Ideas)) + for i, idea := range r.Ideas { + src, err := shownSource(idea.SourceURL, idea.SiteName) + if err != nil { + return Result{}, fmt.Errorf("ideas: idea %d: %w", i, err) + } + idea.SourceURL, idea.SiteName = src.URL, src.SiteName + ideas[i] = idea + } + r.Ideas = ideas + return r, nil +} + // finish ends a job whose work returned res and err. A job that failed // has its result published again with no ideas, to tidy the stored // result. That write is not what keeps a failed job from showing ideas, // since it may fail and a job interrupted by restart never makes it: -// withoutIdeas, applied whenever a failed job is read, is. +// shownJobResult, applied whenever a failed job is read, is. func finish[R shownResult[R]](ctx context.Context, logger *slog.Logger, what string, res R, err error, publish func(R) error) (R, error) { var zero R if err == nil { @@ -312,9 +332,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 +353,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 +368,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 @@ -406,19 +429,32 @@ func ground(ctx context.Context, g *recipes.Grounder, reqs []recipes.GroundReque return failed } -// withoutIdeas is a failed job's stored result as the job shows it: with no -// ideas, however the job failed, so a failed job never shows ideas kept -// before the failure. -func withoutIdeas[R shownResult[R]](stored json.RawMessage) (json.RawMessage, error) { +// shownJobResult is a job's stored result as the job shows it. A failed +// job's shows no ideas, however the job failed, so a failed job never +// shows ideas kept before the failure. Any other job's shows each idea's +// source as shownSource gives it, so a result stored before sources were +// made safe never shows a link or name that could leak a secret. +func shownJobResult[R shownResult[R]](status jobs.Status, stored json.RawMessage) (json.RawMessage, error) { var res R if err := json.Unmarshal(stored, &res); err != nil { - return nil, fmt.Errorf("ideas: decode failed job's result: %w", err) + return nil, fmt.Errorf("ideas: decode %s job's result: %w", status, err) } - return res.cleared().PublicJSON() + if status == jobs.StatusFailed { + return res.cleared().PublicJSON() + } + safe, err := res.withSafeSources() + if err != nil { + return nil, err + } + return safe.PublicJSON() } // 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 +466,38 @@ 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 +} + +// shownSource is the source of an idea as stored in a job's result, with +// link sourceURL and site name siteName, as a client may see it (see +// dinners.SourceOf). A result stored now already holds it, and one stored +// before sources were made safe holds the grounded recipe's own, with "" +// for no site name, as sourceOf reads it; a null link, which only a result +// stored now holds, is checked as "". +func shownSource(sourceURL, siteName *string) (dinners.Source, error) { + var link string + if sourceURL != nil { + link = *sourceURL + } + var site sql.NullString + if siteName != nil { + site = sql.NullString{String: *siteName, Valid: *siteName != ""} + } + return dinners.SourceOf(site, link) } // QuickJob is the KindQuick payload: the kind of Quick dinner wanted, @@ -478,8 +543,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 @@ -504,6 +570,20 @@ func (r QuickResult) cleared() QuickResult { return r } +func (r QuickResult) withSafeSources() (QuickResult, error) { + ideas := make([]QuickIdea, len(r.Ideas)) + for i, idea := range r.Ideas { + src, err := shownSource(idea.SourceURL, idea.SiteName) + if err != nil { + return QuickResult{}, fmt.Errorf("ideas: idea %d: %w", i, err) + } + idea.SourceURL, idea.SiteName = src.URL, src.SiteName + ideas[i] = idea + } + r.Ideas = ideas + return r, nil +} + // OfQuickKind reports whether a recipe cooked by method in minutes (nil // when unknown) is a Quick dinner of kind: for skills.QuickCrockpot, // cooked in a crock pot (normalize_recipe writes a slow cooker as @@ -564,8 +644,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 +659,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 +671,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/jobs/jobs.go b/internal/jobs/jobs.go index e6b6561..34856cf 100644 --- a/internal/jobs/jobs.go +++ b/internal/jobs/jobs.go @@ -136,13 +136,13 @@ type runFunc func(ctx context.Context, payload string, publish func(json.RawMess // handler is a registered kind: run runs a job, check reports whether a // payload decodes into the kind's payload type, serviceOnly is set when -// Runner.Enqueue refuses the kind, and failedResult, when set, is what Get -// shows of a failed job's stored result (see Kind.FailedResult). +// Runner.Enqueue refuses the kind, and shownResult, when set, is what Get +// shows of a job's stored result (see Kind.ShownResult). type handler struct { - run runFunc - check func(payload []byte) bool - serviceOnly bool - failedResult func(stored json.RawMessage) (json.RawMessage, error) + run runFunc + check func(payload []byte) bool + serviceOnly bool + shownResult func(status Status, stored json.RawMessage) (json.RawMessage, error) } // Runner owns the job handlers and the worker pool. @@ -309,23 +309,24 @@ func (k *Kind[P]) ServiceOnly() error { return nil } -// FailedResult makes Get return, for a failed job of the kind with a -// stored result, show's view of that result instead of the result itself. -// It is applied on every read, so it holds however the job failed: by its -// handler, as interrupted by restart, or after a write meant to tidy the -// result failed. A show error fails Get. A kind that never calls it shows a -// failed job's last result as stored. It must be called before Start. -func (k *Kind[P]) FailedResult(show func(stored json.RawMessage) (json.RawMessage, error)) error { +// ShownResult makes Get return, for a job of the kind with a stored +// result, show's view of that result, given the job's status, instead of +// the result itself. It is applied on every read, so it holds for every +// stored result: however a job failed (by its handler, as interrupted by +// restart, or after a write meant to tidy the result failed) and whatever +// version of the kind stored it. A show error fails Get. A kind that never +// calls it shows a job's result as stored. It must be called before Start. +func (k *Kind[P]) ShownResult(show func(status Status, stored json.RawMessage) (json.RawMessage, error)) error { if show == nil { - return fmt.Errorf("jobs: failed result %q: nil func", k.name) + return fmt.Errorf("jobs: shown result %q: nil func", k.name) } k.r.mu.Lock() defer k.r.mu.Unlock() if k.r.started { - return fmt.Errorf("jobs: failed result %q: runner already started", k.name) + return fmt.Errorf("jobs: shown result %q: runner already started", k.name) } h := k.r.handlers[k.name] - h.failedResult = show + h.shownResult = show k.r.handlers[k.name] = h return nil } @@ -425,8 +426,8 @@ func (r *Runner) Wait() { r.wg.Wait() } -// Get returns the job with id, or ErrNotFound. A failed job's result is -// shown through its kind's FailedResult, when the kind set one. +// Get returns the job with id, or ErrNotFound. A job's result is shown +// through its kind's ShownResult, when the kind set one. func (r *Runner) Get(ctx context.Context, id string) (Job, error) { row, err := db.New(r.st.Reader).GetJob(ctx, id) if errors.Is(err, sql.ErrNoRows) { @@ -442,17 +443,17 @@ func (r *Runner) Get(ctx context.Context, id string) (Job, error) { if row.Error.Valid { job.Error = &row.Error.String } - if job.Status == StatusFailed && job.Result != nil { + if job.Result != nil { r.mu.Lock() - show := r.handlers[job.Kind].failedResult + show := r.handlers[job.Kind].shownResult r.mu.Unlock() if show != nil { - shown, err := show(job.Result) + shown, err := show(job.Status, job.Result) if err != nil { - return Job{}, fmt.Errorf("jobs: show failed %s result: %w", job.Kind, err) + return Job{}, fmt.Errorf("jobs: show %s %s result: %w", job.Status, job.Kind, err) } if !json.Valid(shown) { - return Job{}, fmt.Errorf("jobs: show failed %s result: not one JSON value", job.Kind) + return Job{}, fmt.Errorf("jobs: show %s %s result: not one JSON value", job.Status, job.Kind) } job.Result = shown } diff --git a/internal/jobs/jobs_test.go b/internal/jobs/jobs_test.go index 79cb103..837f497 100644 --- a/internal/jobs/jobs_test.go +++ b/internal/jobs/jobs_test.go @@ -773,7 +773,7 @@ func TestServiceOnlyKind(t *testing.T) { } } -func TestFailedResultAppliesOnlyToItsKindsFailedJobs(t *testing.T) { +func TestShownResultAppliesOnlyToItsKindsJobs(t *testing.T) { t.Parallel() st := migratedStore(t) r := newRunner(t, st, 1) @@ -788,17 +788,24 @@ func TestFailedResultAppliesOnlyToItsKindsFailedJobs(t *testing.T) { } hidden := register(t, r, "hidden", h, "stopped") plain := register(t, r, "plain", h, "stopped") - if err := hidden.FailedResult(func(json.RawMessage) (json.RawMessage, error) { - return json.RawMessage(`{"done":0}`), nil + if err := hidden.ShownResult(func(status Status, stored json.RawMessage) (json.RawMessage, error) { + if status == StatusFailed { + return json.RawMessage(`{"done":0}`), nil + } + var res countResult + if err := json.Unmarshal(stored, &res); err != nil { + return nil, err + } + return json.Marshal(countResult{Done: res.Done * 10}) }); err != nil { t.Fatal(err) } - if err := hidden.FailedResult(nil); err == nil { - t.Error("FailedResult(nil) succeeded, want an error") + if err := hidden.ShownResult(nil); err == nil { + t.Error("ShownResult(nil) succeeded, want an error") } start(t, r) - if err := hidden.FailedResult(func(s json.RawMessage) (json.RawMessage, error) { return s, nil }); err == nil { - t.Error("FailedResult after Start succeeded, want an error") + if err := hidden.ShownResult(func(_ Status, s json.RawMessage) (json.RawMessage, error) { return s, nil }); err == nil { + t.Error("ShownResult after Start succeeded, want an error") } for _, tc := range []struct { @@ -806,9 +813,10 @@ func TestFailedResultAppliesOnlyToItsKindsFailedJobs(t *testing.T) { steps int want string }{ - {hidden, 0, `{"done":0}`}, // failed: shown through FailedResult - {hidden, 3, `{"done":3}`}, // succeeded: as stored - {plain, 0, `{"done":5}`}, // failed, kind without FailedResult: as stored + {hidden, 0, `{"done":0}`}, // failed: shown through ShownResult + {hidden, 3, `{"done":30}`}, // succeeded: shown through ShownResult + {plain, 0, `{"done":5}`}, // failed, kind without ShownResult: as stored + {plain, 3, `{"done":3}`}, // succeeded, kind without ShownResult: as stored } { job := waitFor(t, r, enqueue(t, tc.kind, countPayload{Steps: tc.steps}), finished) if string(job.Result) != tc.want { diff --git a/internal/server/ideasource_test.go b/internal/server/ideasource_test.go new file mode 100644 index 0000000..2aae059 --- /dev/null +++ b/internal/server/ideasource_test.go @@ -0,0 +1,197 @@ +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/ideas" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" + "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)) +} + +// legacyIdea is an idea as the result builders stored it before sources +// were made safe: the grounded recipe's raw link and stored site name, +// which is the link's host when its page named no site. +func legacyIdea(name, link, site string) map[string]any { + return map[string]any{ + "name": name, "method": "stovetop", "minutes_total": 25, "difficulty": "easy", + "recipe_id": "01JLEGACY" + strings.ToUpper(strings.ReplaceAll(name, " ", "")), "photo_url": "/api/v1/photos/legacy", + "source_url": link, "site_name": site, + } +} + +// 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. +func TestJobServesLegacyIdeaResultsWithOnlySafeSources(t *testing.T) { + t.Parallel() + model, normalize, search, pages, images := newQuickFakes() + h, _, st := newIdeasRouter(t, model, normalize, search, pages, images, false) + legacy := []any{ + legacyIdea("Chicken Curry", cleanSourceLink, "Curry House"), + legacyIdea("Beef Stew", "https://"+queryHost+"/stew?access_token="+queryToken, queryHost), + legacyIdea("Bean Chili", "https://"+userinfoToken+"@"+userinfoHost+"/chili", userinfoHost), + } + for kind, result := range map[string]map[string]any{ + ideas.KindFromIngredients: {"stage": "done", "ingredients": []string{"rice"}, "quick_only": false, "ideas": legacy, "dropped": 0}, + ideas.KindQuick: {"stage": "done", "kind": "under30", "exclude": []string{}, "ideas": legacy, "dropped": 0}, + } { + js, err := json.Marshal(result) + if err != nil { + t.Fatal(err) + } + id := "legacy-" + kind + if _, err := st.Writer.ExecContext(t.Context(), + `INSERT INTO jobs (id, kind, status, payload, result, created_at, updated_at) VALUES (?, ?, 'succeeded', '{}', ?, '2026-10-01T00:00:00Z', '2026-10-01T00:00:00Z')`, + id, kind, string(js)); err != nil { + t.Fatal(err) + } + checkIdeaSources(t, h, id) + } +} + +// An idea job whose grounded recipe's stored link does not parse fails, +// even when no site name is stored, rather than show the idea without a +// source. +func TestIdeaJobFailsOnUnparseableSourceWithoutSiteName(t *testing.T) { + t.Parallel() + const badLink = "https://kitchen.example/%zz" + model, normalize, search, pages, images := sourceFakes(t, 25) + idea := fmt.Sprintf(`{"name":%q,"uses":["rice"],"missing":[],"est_minutes":25,"method":"stovetop","difficulty":"easy","search_query":%q}`, sourceCases[0].dish, sourceCases[0].dish) + js := `{"ideas":[` + idea + `]}` + for range 2 { + model.QueueGenerate(llm.GenerateResult{Response: llm.Response{Text: js, JSON: json.RawMessage(js)}}) + } + h, _, st := newIdeasRouter(t, model, normalize, search, pages, images, true) + if job := waitJob(t, h, startIdeasJob(t, h)); job["status"] != "succeeded" { + t.Fatalf("grounding job = %v", job) + } + // The recipe is now stored under badLink with no site name, and the + // search finds it there. + res, err := st.Writer.ExecContext(t.Context(), `UPDATE recipes SET source_url = ?, site_name = NULL WHERE source_url = ?`, badLink, cleanSourceLink) + if err != nil { + t.Fatal(err) + } + if n, err := res.RowsAffected(); err != nil || n != 1 { + t.Fatalf("updated %d recipes (%v), want 1", n, err) + } + search.Results[sourceCases[0].dish] = []recipes.Candidate{{URL: badLink}} + job := waitJob(t, h, startIdeasJob(t, h)) + assertFailedWithoutIdeas(t, job, jobs.CodeInternal) +} 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')