Source attribution links when a URL is verifiably safe (Feature 15) (#154)

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

internal/dinners/detail.go

@@ -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,69 @@ 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) parses the link with the browser's URL parser
+// and refuses the whole recipe when it fails, so the host and port must
+// also be ones it parses: see browserHost.
+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
+	}
+	// url.Parse decodes a percent-encoded host; the link must spell its
+	// host out as it is checked.
+	authority, _, _ := strings.Cut(sourceURL[len(u.Scheme)+len("://"):], "/")
+	if authority != u.Host || !browserHost(u) {
+		return nil
+	}
+	if port := u.Port(); port != "" {
+		if n, err := strconv.ParseUint(port, 10, 16); err != nil || n == 0 {
+			return nil
+		}
+	}
+	return &sourceURL
+}
+
+// browserHost says whether u's host is one the browser's URL parser (the
+// WHATWG URL Standard) accepts as it stands, where url.Parse accepts more,
+// such as "[bad]". The rule is narrower than the browser's: a bracketed
+// host is an IPv6 address without a zone; any other is ASCII letters,
+// digits and hyphens in dot-separated labels, none empty but a trailing
+// one, none punycode ("xn--"), since the browser refuses invalid punycode
+// and checking it is not worth a decoder; and a host whose last label is
+// a number, which the browser reads as an IPv4 address, must be a dotted
+// quad. A link to any other host is withheld.
+func browserHost(u *url.URL) bool {
+	host := u.Hostname()
+	if strings.HasPrefix(u.Host, "[") {
+		ip := net.ParseIP(host)
+		return ip != nil && strings.Contains(host, ":")
+	}
+	labels := strings.Split(strings.TrimSuffix(host, "."), ".")
+	for _, label := range labels {
+		if label == "" || len(label) >= 4 && strings.EqualFold(label[:4], "xn--") {
+			return false
+		}
+		for _, c := range label {
+			if !('a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' || c == '-') {
+				return false
+			}
+		}
+	}
+	last := labels[len(labels)-1]
+	if strings.Trim(last, "0123456789") == "" || len(last) >= 2 && strings.EqualFold(last[:2], "0x") {
+		ip := net.ParseIP(host)
+		return ip != nil && ip.To4() != nil && !strings.Contains(host, ":")
+	}
+	return true
+}
+
 // 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

internal/dinners/dinners_test.go

@@ -112,6 +112,63 @@ 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/",
+		"https://[2001:db8::1]/curry",
+		"https://1.2.3.4/curry",
+		"https://Kitchen-2.Example.com./curry",
+	} {
+		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",
+		"",
+		// Hosts url.Parse accepts but the browser's URL parser refuses
+		// (or reads as another host), which would fail the whole recipe
+		// screen.
+		"https://[bad]/curry",
+		"https://[fe80::1%25en0]/curry",
+		"https://[1.2.3.4]/curry",
+		"https://1.2.3.999/curry",
+		"https://1.2.3.4.5/curry",
+		"https://999/curry",
+		"https://example.0x/curry",
+		"https://xn--a/curry",
+		"https://ex%61mple.com/curry",
+		"https://ex%2fample.com/curry",
+		"https://a..b/curry",
+		"https://café.example/curry",
+	} {
+		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

internal/dinners/export_test.go

@@ -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