Withhold unsafe recipe sources from idea job results (BASE-01) (#170)

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

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

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)
+		}
 	}
 }
 

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