Return the recipe source url when it is verifiably free of secrets
The dinner detail API now sends source.url when the stored recipe link
is https to a host with no userinfo, no query (not even a bare "?") and
no fragment (not even a bare "#"), and a port the client can parse;
otherwise source.url is null. The rule matches sourceUrl() in
web/src/dinners/detail.ts, so the recipe screen links "Recipe from
{site}" in a new tab for a safe link and shows plain text otherwise.
The site-name protection in pageSiteName is unchanged.
Task: c1a97243-3e41-42fc-a1a0-1ac684ccb463
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -58,15 +58,16 @@ type Ingredient struct {
|
||||
RawText string `json:"raw_text"`
|
||||
}
|
||||
|
||||
// Source is the site a recipe was fetched from. It has no url: the page's
|
||||
// link, pasted or offered as a candidate, may carry a personal token, and
|
||||
// no stored link can be shown to be free of one, so like a dinner's
|
||||
// recipe_url and a candidate's page it never leaves the server. SiteName
|
||||
// is the name the page itself gives its site, and null when it gives none
|
||||
// or the stored name could have come from a link (see pageSiteName), so a
|
||||
// client shows "Recipe from {site}" only when SiteName is not null.
|
||||
// Source is the site a recipe was fetched from. SiteName is the name the
|
||||
// page itself gives its site, and null when it gives none or the stored
|
||||
// name could have come from a link (see pageSiteName), so a client shows
|
||||
// "Recipe from {site}" only when SiteName is not null. URL is the page's
|
||||
// link only when it is verifiably free of secrets (see safeSourceURL), and
|
||||
// null otherwise: a link pasted or offered as a candidate may carry a
|
||||
// personal token, so a client links the credit only when URL is not null.
|
||||
type Source struct {
|
||||
SiteName *string `json:"site_name"`
|
||||
URL *string `json:"url"`
|
||||
}
|
||||
|
||||
// Get returns dinner id with its recipe and the reason it was suggested,
|
||||
@@ -149,7 +150,7 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
d.Source = &Source{SiteName: site}
|
||||
d.Source = &Source{SiteName: site, URL: safeSourceURL(rec.SourceUrl)}
|
||||
}
|
||||
ings, err := q.ListRecipeIngredients(ctx, recipeID)
|
||||
if err != nil {
|
||||
@@ -209,6 +210,28 @@ func pageSiteName(stored sql.NullString, sourceURL string) (*string, error) {
|
||||
return &name, nil
|
||||
}
|
||||
|
||||
// safeSourceURL is sourceURL when it is verifiably free of secrets — an
|
||||
// https link to a host, with no user or password, no query (not even a
|
||||
// bare "?") and no fragment (not even a bare "#") — else nil. Those are
|
||||
// the places a link carries a token; a link failing any check, or not
|
||||
// parsing, is withheld rather than trimmed, since a trimmed link is not the
|
||||
// page the recipe came from. The client (sourceUrl in
|
||||
// web/src/dinners/detail.ts) refuses any other link, so the port must also
|
||||
// be one it parses.
|
||||
func safeSourceURL(sourceURL string) *string {
|
||||
u, err := url.Parse(sourceURL)
|
||||
if err != nil || u.Scheme != "https" || u.Opaque != "" || u.Host == "" || u.User != nil ||
|
||||
u.RawQuery != "" || u.ForceQuery || u.Fragment != "" || strings.Contains(sourceURL, "#") {
|
||||
return nil
|
||||
}
|
||||
if port := u.Port(); port != "" {
|
||||
if n, err := strconv.ParseUint(port, 10, 16); err != nil || n == 0 {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
return &sourceURL
|
||||
}
|
||||
|
||||
// duplicatePosition is the error for two recipe items of kind at position.
|
||||
// Position is the only record of recipe order, and the id that breaks the
|
||||
// tie says nothing about it, so a recipe with a duplicate is refused rather
|
||||
|
||||
@@ -112,6 +112,45 @@ func TestPageSiteNameWithholdsHosts(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestSafeSourceURLOnlyWithoutSecrets: a recipe link is shown only when it
|
||||
// is https to a host with no user, password, query or fragment, each of
|
||||
// which can carry a token; any other link, or one that does not parse, is
|
||||
// withheld whole.
|
||||
func TestSafeSourceURLOnlyWithoutSecrets(t *testing.T) {
|
||||
t.Parallel()
|
||||
for _, link := range []string{
|
||||
"https://www.example.com/recipes/chicken-curry",
|
||||
"https://example.com",
|
||||
"https://example.com:8443/stew/",
|
||||
} {
|
||||
if got := dinners.SafeSourceURL(link); got == nil || *got != link {
|
||||
t.Errorf("SafeSourceURL(%q) = %v, want the link", link, deref(got))
|
||||
}
|
||||
}
|
||||
for _, link := range []string{
|
||||
"https://example.com/curry?access_token=query-token",
|
||||
"https://example.com/curry?",
|
||||
"https://user:password@example.com/curry",
|
||||
"https://token@example.com/curry",
|
||||
"https://@example.com/curry",
|
||||
"http://example.com/curry",
|
||||
"https://example.com/curry#token=fragment-token",
|
||||
"https://example.com/curry#",
|
||||
"ftp://example.com/curry",
|
||||
"https:example.com/curry",
|
||||
"https:///curry",
|
||||
"https://example.com:99999/curry",
|
||||
"https://example.com:0/curry",
|
||||
"//example.com/curry",
|
||||
"https://secret-token\x7f.example/%zz",
|
||||
"",
|
||||
} {
|
||||
if got := dinners.SafeSourceURL(link); got != nil {
|
||||
t.Errorf("SafeSourceURL(%q) = %q, want nil", link, *got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func deref(s *string) any {
|
||||
if s == nil {
|
||||
return nil
|
||||
|
||||
@@ -74,3 +74,6 @@ func (s *Service) RunGroundJob(ctx context.Context, p GroundJob) (string, error)
|
||||
|
||||
// PageSiteName is pageSiteName.
|
||||
var PageSiteName = pageSiteName
|
||||
|
||||
// SafeSourceURL is safeSourceURL.
|
||||
var SafeSourceURL = safeSourceURL
|
||||
|
||||
@@ -111,7 +111,7 @@ func TestGetDinnerGroundedSideQuest(t *testing.T) {
|
||||
{Name: str("onion"), Qty: str("1"), RawText: "1 onion"},
|
||||
},
|
||||
Steps: []string{"Fry with the garlic.", "Chop the onion."},
|
||||
Source: &dinners.Source{SiteName: str("Curry House")},
|
||||
Source: &dinners.Source{SiteName: str("Curry House"), URL: str(curryPage)},
|
||||
GroundingStatus: "grounded",
|
||||
WhyPicked: str(reason),
|
||||
}
|
||||
@@ -216,53 +216,84 @@ func TestGetDinnerNotFound(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestGetDinnerSourceHidesTokenURL: a recipe grounded in a pasted link that
|
||||
// carries a token in its path or query shows its site name, but never the
|
||||
// link or any part of it.
|
||||
func TestGetDinnerSourceHidesTokenURL(t *testing.T) {
|
||||
// TestGetDinnerSourceURLOnlyWhenSafe: a recipe grounded in a clean https
|
||||
// link shows it as source.url; one whose stored link carries a token in its
|
||||
// query or userinfo, is http, or has a fragment shows its site name but
|
||||
// source.url null and nothing of the link.
|
||||
func TestGetDinnerSourceURLOnlyWhenSafe(t *testing.T) {
|
||||
t.Parallel()
|
||||
e := newDinnersEnv(t)
|
||||
const (
|
||||
pathToken = "path-token-abc123"
|
||||
clean = "https://kitchen.example/recipes/chicken-curry"
|
||||
queryToken = "query-token-def456"
|
||||
pathPage = "https://secret-path.example/u/" + pathToken + "/chicken-curry"
|
||||
queryPage = "https://secret-query.example/curry?access_token=" + queryToken
|
||||
)
|
||||
e.pages.Bodies[pathPage] = recipestest.RecipePage("Chicken Curry", "Curry House", "https://a.example/curry.jpg", 40)
|
||||
e.pages.Bodies[clean] = recipestest.RecipePage("Chicken Curry", "Curry House", "https://a.example/curry.jpg", 40)
|
||||
e.pages.Bodies[queryPage] = recipestest.RecipePage("Weeknight Curry", "B Kitchen", "https://b.example/curry.jpg", 25)
|
||||
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)
|
||||
for _, tc := range []struct{ dish, page, site string }{
|
||||
{"Chicken curry", pathPage, "Curry House"},
|
||||
{"Weeknight curry", queryPage, "B Kitchen"},
|
||||
} {
|
||||
id := createDinner(t, e.h, `{"name":"`+tc.dish+`"}`) // runner not started: stays pending
|
||||
ground := func(dish, page string) (id string) {
|
||||
t.Helper()
|
||||
id = createDinner(t, e.h, `{"name":"`+dish+`"}`) // runner not started: stays pending
|
||||
e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25))
|
||||
if rec, _ := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground", `{"recipe_url":"`+tc.page+`"}`); rec.Code != http.StatusOK {
|
||||
t.Fatalf("POST ground %s = %d: %s", tc.page, rec.Code, rec.Body)
|
||||
if rec, _ := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground", `{"recipe_url":"`+page+`"}`); rec.Code != http.StatusOK {
|
||||
t.Fatalf("POST ground %s = %d: %s", page, rec.Code, rec.Body)
|
||||
}
|
||||
var stored string
|
||||
if err := e.st.Reader.QueryRow(`SELECT source_url FROM recipes WHERE id = (SELECT recipe_id FROM dinners WHERE id = ?)`, id).Scan(&stored); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if stored != tc.page {
|
||||
t.Fatalf("stored source_url = %q, want %q", stored, tc.page)
|
||||
if stored != page {
|
||||
t.Fatalf("stored source_url = %q, want %q", stored, page)
|
||||
}
|
||||
return id
|
||||
}
|
||||
source := func(what, id string) map[string]any {
|
||||
t.Helper()
|
||||
raw, _ := getDetail(t, e.h, id)
|
||||
src, ok := raw["source"].(map[string]any)
|
||||
if !ok {
|
||||
t.Fatalf("source of %s = %v, want an object", what, raw["source"])
|
||||
}
|
||||
return src
|
||||
}
|
||||
|
||||
cleanID := ground("Chicken curry", clean)
|
||||
if src := source("a clean link", cleanID); !reflect.DeepEqual(src, map[string]any{"site_name": "Curry House", "url": clean}) {
|
||||
t.Errorf("source of a clean link = %v, want site_name Curry House and url %s", src, clean)
|
||||
}
|
||||
|
||||
queryID := ground("Weeknight curry", queryPage)
|
||||
withheld := func(what, id, site string, secrets ...string) {
|
||||
t.Helper()
|
||||
rec, _ := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id, "")
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("GET dinner = %d: %s", rec.Code, rec.Body)
|
||||
t.Fatalf("GET %s = %d: %s", what, rec.Code, rec.Body)
|
||||
}
|
||||
for _, secret := range []string{tc.page, "secret-path.example", "secret-query.example", pathToken, queryToken, "access_token", `"url"`} {
|
||||
for _, secret := range secrets {
|
||||
if strings.Contains(rec.Body.String(), secret) {
|
||||
t.Errorf("detail of %s contains %q: %s", tc.dish, secret, rec.Body)
|
||||
t.Errorf("detail of %s contains %q: %s", what, secret, rec.Body)
|
||||
}
|
||||
}
|
||||
raw, _ := getDetail(t, e.h, id)
|
||||
if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, map[string]any{"site_name": tc.site}) {
|
||||
t.Errorf("source of %s = %v, want only site_name %q", tc.dish, raw["source"], tc.site)
|
||||
if src := source(what, id); !reflect.DeepEqual(src, map[string]any{"site_name": site, "url": nil}) {
|
||||
t.Errorf("source of %s = %v, want site_name %q and url null", what, src, site)
|
||||
}
|
||||
}
|
||||
withheld("a link with a query token", queryID, "B Kitchen", queryPage, "secret-query.example", queryToken, "access_token")
|
||||
|
||||
// The other unsafe shapes, stored as a link grounded elsewhere would
|
||||
// leave them.
|
||||
for _, tc := range []struct{ what, link string }{
|
||||
{"a link with a userinfo token", "https://user-token-ghi789@kitchen.example/recipes/chicken-curry"},
|
||||
{"a link with a userinfo password", "https://cook:password-jkl012@kitchen.example/recipes/chicken-curry"},
|
||||
{"an http link", "http://kitchen.example/recipes/chicken-curry"},
|
||||
{"a link with a fragment", "https://kitchen.example/recipes/chicken-curry#token=fragment-mno345"},
|
||||
} {
|
||||
if _, err := e.st.Writer.Exec(`UPDATE recipes SET source_url = ? WHERE id = (SELECT recipe_id FROM dinners WHERE id = ?)`, tc.link, cleanID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
withheld(tc.what, cleanID, "Curry House", tc.link, "kitchen.example", "-token-", "password", "fragment")
|
||||
}
|
||||
}
|
||||
|
||||
// TestGetDinnerSiteNameNeverFromLink: a page that names no site is stored
|
||||
@@ -270,14 +301,16 @@ func TestGetDinnerSourceHidesTokenURL(t *testing.T) {
|
||||
// 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) {
|
||||
t.Parallel()
|
||||
e := newDinnersEnv(t)
|
||||
const (
|
||||
bareToken = "private-token-abc123"
|
||||
namedToken = "named-token-def456"
|
||||
bare = "https://" + bareToken + ".example.com/recipe"
|
||||
named = "https://www." + namedToken + ".example.com/recipe"
|
||||
bare = "https://" + bareToken + ".example.com/recipe?print=1"
|
||||
named = "https://www." + namedToken + ".example.com/recipe?print=1"
|
||||
)
|
||||
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)
|
||||
@@ -315,8 +348,8 @@ 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}) {
|
||||
t.Errorf("source of %s = %v, want only site_name %v", what, raw["source"], want)
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -324,13 +357,13 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
|
||||
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", `"url"`}, nil)
|
||||
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)
|
||||
}
|
||||
check("a page naming its site", namedID, []string{named, namedToken, "example.com", `"url"`}, "Curry House")
|
||||
check("a page naming its site", namedID, []string{named, namedToken, "example.com"}, "Curry House")
|
||||
|
||||
const redirectHost = "redirect-token-ghi789.example.net"
|
||||
if _, err := e.st.Writer.Exec(`UPDATE recipes SET site_name = ? WHERE id = ?`, redirectHost, namedRecipe); err != nil {
|
||||
|
||||
Reference in New Issue
Block a user