Produce the two files a crawler asks for, and refuse a sitemap that drifts [#69]

CHANGELOG.md

@@ -64,3 +64,10 @@ there is no earlier bundle to compare it against.
   it, packed so that two runs of one source produce one archive.
 - The bill of materials states the version of the thing it is about, so two
   archives are distinguishable by the document each one carries.
+- The build writes `robots.txt` and `sitemap.xml`. Both are asked for by clients
+  without any page linking them, and a bundle without them answers those
+  requests with the not-found page. The sitemap lists every page the build wrote
+  except the not-found one, at the address each is served at, and carries no
+  date, so two builds of one source still produce one set of bytes.
+- The gate refuses a bundle whose sitemap disagrees with the pages beside it, in
+  both directions: a page listed nowhere, and an address with no page behind it.

internal/gate/gate.go

@@ -21,6 +21,7 @@ import (
 	"github.com/Flowfin/site/internal/invariant"
 	"github.com/Flowfin/site/internal/link"
 	"github.com/Flowfin/site/internal/site"
+	"github.com/Flowfin/site/internal/sitemap"
 )
 
 // A leg is one thing the gate decides. Its report is the sentence printed
@@ -38,6 +39,7 @@ func legs() []leg {
 		{"test", testLeg},
 		{"build", buildLeg},
 		{"links", linksLeg},
+		{"sitemap", sitemapLeg},
 		{"invariants", invariantsLeg},
 	}
 }
@@ -209,6 +211,24 @@ func linksLeg(root string) (string, error) {
 	return strings.TrimSpace(lines[len(lines)-1]), nil
 }
 
+// sitemapLeg refuses a sitemap that disagrees with the pages beside it. It sits
+// after the links leg because both walk what the build produced and the links
+// leg answers the more basic question: a site whose pages point at files nobody
+// wrote is broken for a reader, where a sitemap that has drifted is broken only
+// for a crawler.
+func sitemapLeg(root string) (string, error) {
+	var log strings.Builder
+	if err := sitemap.Run(root, &log); err != nil {
+		lines := strings.Split(strings.TrimRight(log.String(), "\n"), "\n")
+		return "", fmt.Errorf("%v:\n%s", err, indent(lines))
+	}
+	// The last line the comparison wrote is the one that says what it
+	// covered, in its own words, so a run that examined nothing says so here
+	// rather than reporting a count assembled in this file.
+	lines := strings.Split(strings.TrimRight(log.String(), "\n"), "\n")
+	return strings.TrimSpace(lines[len(lines)-1]), nil
+}
+
 // invariantsLeg decides the rules that can be read off the tree and off the
 // output a build produces. The rows live in their own package rather than here,
 // so the same set is decided by this leg and by the workflow that reports it

internal/reproduce/reproduce_test.go

@@ -54,12 +54,18 @@ func write(t *testing.T, root, name, body string) {
 // The neighbour, and it is the real build rather than a stand-in: a tree whose
 // build reads only what is committed produces the same bytes twice, and the run
 // says how many files it compared rather than passing silently.
+//
+// The count is the page and the two files a crawler asks for. It is written out
+// rather than derived, because those two are the ones a build could most easily
+// make unreproducible: a sitemap may state when each page last changed, and one
+// carrying today's date would pass every other check in this tree and red only
+// here.
 func TestTwoBuildsOfTheRealGeneratorAgree(t *testing.T) {
 	var log bytes.Buffer
 	if err := Run(tree(t), &log); err != nil {
 		t.Fatalf("Run refused two builds of one source: %v\n%s", err, log.String())
 	}
-	if !strings.Contains(log.String(), "1 file(s), identical in both builds") {
+	if !strings.Contains(log.String(), "3 file(s), identical in both builds") {
 		t.Errorf("the run did not say what it compared; it said:\n%s", log.String())
 	}
 }

internal/site/crawler.go

@@ -0,0 +1,146 @@
+// The files a crawler asks for without any page linking them, and what the
+// build writes into them.
+//
+// A crawler asks for the exclusion file before it asks for anything else, and it
+// asks again on every site it has not seen answer. Where the repository provides
+// none, the host answers with the not-found page: a request the reader's host
+// serves for nothing, and an answer that carries none of what was asked for.
+// Producing the file is what turns that into one short answer.
+//
+// The sitemap is generated rather than written down. Twelve of the addresses
+// this site will have come from a file somebody else edits, so a hand-written
+// list is wrong the day a row is added and nothing about the wrong list looks
+// wrong. What the generated one lists is what this build just wrote.
+//
+// Neither file carries a date. A sitemap may state when each page last changed,
+// and a build that wrote today's date into one would produce different bytes
+// from the same source on two days, against a check that exists to compare
+// exactly that. What it costs is that a crawler is told which addresses exist
+// and not which of them moved, and the second half is worth less here than a
+// build somebody can reproduce.
+package site
+
+import (
+	"encoding/xml"
+	"fmt"
+	"io"
+	"os"
+	"path"
+	"path/filepath"
+	"sort"
+	"strings"
+)
+
+// RobotsPath and SitemapPath are where the two files land. Neither name is this
+// repository's choice: the first is the one address a crawler asks for before it
+// has read anything, and the second is found only because the first names it.
+const (
+	RobotsPath  = "robots.txt"
+	SitemapPath = "sitemap.xml"
+)
+
+// SitemapAddresses is which of the produced files a sitemap lists, and at which
+// address. It takes paths relative to the output directory.
+//
+// It is exported because two readers need the same answer: the writer below,
+// which turns it into the file, and the leg that walks the output afterwards and
+// compares the file against it. Stating the rule in both places would state it
+// twice, and the second statement would agree with the first on the day it was
+// written and never be checked against it again.
+//
+// The not-found page is the one page left out, and it is left out rather than
+// forgotten. A sitemap is a list of addresses a crawler is invited to fetch and
+// index, and that page is served in answer to addresses that are not its own, so
+// listing it asks for an index entry that sends a reader to an error page under
+// an address the site says it has. Every other produced page is listed, and
+// anything that is not a page is not, because what a sitemap carries is the
+// addresses a reader can be sent to.
+//
+// The addresses are sorted, so the file the build writes is a property of which
+// pages exist rather than of the order the writers happen to run in.
+func SitemapAddresses(produced []string) []string {
+	var addresses []string
+	for _, name := range produced {
+		name = strings.TrimPrefix(path.Clean(filepath.ToSlash(name)), "/")
+		if !strings.HasSuffix(name, ".html") || name == NotFoundPath {
+			continue
+		}
+		addresses = append(addresses, Origin+addressOf(name))
+	}
+	sort.Strings(addresses)
+	return addresses
+}
+
+// writeSitemap writes the list of addresses this build produced, and reports
+// what it wrote.
+//
+// A build that produced no page writes no sitemap and says so, rather than
+// writing a list of nothing. An empty list is a file that says this site has no
+// pages, which is a statement about the site rather than a statement that the
+// build had nothing to say.
+func writeSitemap(out, label string, written []string, log io.Writer) ([]string, error) {
+	addresses := SitemapAddresses(relativeTo(label, written))
+	if len(addresses) == 0 {
+		fmt.Fprintf(log, "the build produced no page, so no %s was written\n", SitemapPath)
+		return nil, nil
+	}
+
+	var body strings.Builder
+	body.WriteString(xml.Header)
+	body.WriteString(`<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">` + "\n")
+	for _, a := range addresses {
+		var escaped strings.Builder
+		if err := xml.EscapeText(&escaped, []byte(a)); err != nil {
+			return nil, fmt.Errorf("writing %s: %w", SitemapPath, err)
+		}
+		fmt.Fprintf(&body, "  <url><loc>%s</loc></url>\n", escaped.String())
+	}
+	body.WriteString("</urlset>\n")
+
+	name := filepath.Join(out, filepath.FromSlash(SitemapPath))
+	if err := os.WriteFile(name, []byte(body.String()), 0o644); err != nil {
+		return nil, fmt.Errorf("writing %s: %w", SitemapPath, err)
+	}
+	slashed := path.Join(label, SitemapPath)
+	fmt.Fprintf(log, "wrote %s (%d bytes, %d address(es) listed)\n", slashed, body.Len(), len(addresses))
+	return []string{slashed}, nil
+}
+
+// writeRobots writes the exclusion file. Nothing on this site is kept out of an
+// index, so the file says that plainly and spends the rest of itself naming
+// where the list of addresses is.
+//
+// It names the sitemap only where one was written. A robots file pointing at an
+// address the build did not produce sends the one client that reads it to the
+// not-found page, which is the failure this pair exists to remove, arriving from
+// the file that was supposed to remove it.
+func writeRobots(out, label string, sitemap []string, log io.Writer) ([]string, error) {
+	var body strings.Builder
+	body.WriteString("# Nothing on this site is kept out of an index. This file exists so that a\n")
+	body.WriteString("# crawler asking for it is answered rather than served the not-found page.\n")
+	body.WriteString("User-agent: *\n")
+	body.WriteString("Disallow:\n")
+	if len(sitemap) > 0 {
+		fmt.Fprintf(&body, "\nSitemap: %s/%s\n", Origin, SitemapPath)
+	}
+
+	name := filepath.Join(out, filepath.FromSlash(RobotsPath))
+	if err := os.WriteFile(name, []byte(body.String()), 0o644); err != nil {
+		return nil, fmt.Errorf("writing %s: %w", RobotsPath, err)
+	}
+	slashed := path.Join(label, RobotsPath)
+	fmt.Fprintf(log, "wrote %s (%d bytes, %d sitemap(s) named)\n", slashed, body.Len(), len(sitemap))
+	return []string{slashed}, nil
+}
+
+// relativeTo strips the output label off the paths the writers report, so what
+// this file works in is the addresses the host will serve rather than wherever
+// the run happened to render.
+func relativeTo(label string, written []string) []string {
+	prefix := filepath.ToSlash(label) + "/"
+	out := make([]string, 0, len(written))
+	for _, w := range written {
+		out = append(out, strings.TrimPrefix(filepath.ToSlash(w), prefix))
+	}
+	return out
+}

internal/site/crawler_test.go

@@ -0,0 +1,142 @@
+// The suite over the two files a crawler asks for.
+//
+// The cases about which addresses belong in a sitemap drive the rule directly
+// rather than through a build, because the rule is what the leg over the output
+// reads and a case that went through a build would be testing the build.
+package site
+
+import (
+	"io"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// The not-found page is produced, is a page, and is the one page a sitemap must
+// not carry. Listing it asks a crawler to index the document a host serves for
+// addresses this site does not have, under an address the site says it has.
+func TestSitemapLeavesOutTheNotFoundPage(t *testing.T) {
+	produced := []string{"index.html", "privacy/index.html", NotFoundPath}
+
+	got := SitemapAddresses(produced)
+
+	for _, a := range got {
+		if strings.HasSuffix(a, "/"+NotFoundPath) {
+			t.Fatalf("the sitemap lists the not-found page: %v", got)
+		}
+	}
+	if len(got) != 2 {
+		t.Fatalf("SitemapAddresses(%v) = %v, want the two pages that are not the not-found one", produced, got)
+	}
+}
+
+// A directory address is served by the index document inside it, and the
+// sitemap states the address rather than the file. A crawler told to fetch the
+// file would index a second address for a page that already has one.
+func TestSitemapStatesTheAddressAndNotTheFile(t *testing.T) {
+	got := SitemapAddresses([]string{"index.html", "privacy/index.html"})
+
+	want := []string{Origin + "/", Origin + "/privacy/"}
+	if len(got) != len(want) {
+		t.Fatalf("SitemapAddresses gave %v, want %v", got, want)
+	}
+	for i := range want {
+		if got[i] != want[i] {
+			t.Errorf("entry %d is %s, want %s", i, got[i], want[i])
+		}
+	}
+}
+
+// What a sitemap carries is addresses a reader can be sent to. The reporting
+// route, the exclusion file and the sitemap itself are all produced and none of
+// them is a page.
+func TestSitemapCarriesOnlyPages(t *testing.T) {
+	produced := []string{
+		"index.html",
+		".well-known/security.txt",
+		RobotsPath,
+		SitemapPath,
+		"nested/style.css",
+	}
+
+	got := SitemapAddresses(produced)
+
+	if len(got) != 1 || got[0] != Origin+"/" {
+		t.Fatalf("SitemapAddresses(%v) = %v, want the one page", produced, got)
+	}
+}
+
+// The order of the file is a property of which pages exist. Two runs over the
+// same tree produce the same bytes, which is what the check that builds twice
+// compares, and the order the writers happen to run in is not part of it.
+func TestSitemapIsSortedWhateverOrderThePagesArrivedIn(t *testing.T) {
+	first := SitemapAddresses([]string{"privacy/index.html", "index.html", "legal/index.html"})
+	second := SitemapAddresses([]string{"index.html", "legal/index.html", "privacy/index.html"})
+
+	if len(first) != len(second) {
+		t.Fatalf("the two orders gave %v and %v", first, second)
+	}
+	for i := range first {
+		if first[i] != second[i] {
+			t.Fatalf("the two orders gave %v and %v", first, second)
+		}
+	}
+	if first[0] != Origin+"/" {
+		t.Errorf("the list opens with %s, and sorted it opens with the site root", first[0])
+	}
+}
+
+// A build that produced no page writes no sitemap and says so. An empty list is
+// a file stating that this site has no pages, which is a claim rather than a
+// report that there was nothing to write.
+func TestNoPageMeansNoSitemapAndASaidReason(t *testing.T) {
+	out := t.TempDir()
+
+	var log strings.Builder
+	written, err := writeSitemap(out, "dist", []string{"dist/" + RobotsPath}, &log)
+	if err != nil {
+		t.Fatalf("writeSitemap: %v", err)
+	}
+	if len(written) != 0 {
+		t.Errorf("writeSitemap reported %v out of a build with no page", written)
+	}
+	if !strings.Contains(log.String(), "produced no page") {
+		t.Errorf("the run does not say why nothing was written:\n%s", log.String())
+	}
+}
+
+// The exclusion file names the sitemap only where one was written. A robots file
+// pointing at an address the build did not produce sends the one client that
+// reads it to the not-found page, which is the failure this pair exists to
+// remove.
+func TestRobotsNamesNoSitemapThatWasNotWritten(t *testing.T) {
+	out := t.TempDir()
+
+	if _, err := writeRobots(out, "dist", nil, io.Discard); err != nil {
+		t.Fatalf("writeRobots: %v", err)
+	}
+	got := read(t, filepath.Join(out, RobotsPath))
+
+	if strings.Contains(got, "Sitemap:") {
+		t.Errorf("the file names a sitemap the build did not write:\n%s", got)
+	}
+	if !strings.Contains(got, "User-agent: *") {
+		t.Errorf("the file excludes nobody and does not say so:\n%s", got)
+	}
+}
+
+// With a sitemap beside it the file names it, at the address a crawler will ask
+// for rather than at the path the build wrote.
+func TestRobotsNamesTheSitemapThatWasWritten(t *testing.T) {
+	out := t.TempDir()
+
+	if _, err := writeRobots(out, "dist", []string{"dist/" + SitemapPath}, io.Discard); err != nil {
+		t.Fatalf("writeRobots: %v", err)
+	}
+	got := read(t, filepath.Join(out, RobotsPath))
+
+	want := "Sitemap: " + Origin + "/" + SitemapPath
+	if !strings.Contains(got, want) {
+		t.Errorf("the file does not carry %q; it is:\n%s", want, got)
+	}
+}

internal/site/privacy_test.go

@@ -277,7 +277,7 @@ func TestBuildWritesThePrivacyPageAndSaysWhatIsOnIt(t *testing.T) {
 	if err != nil {
 		t.Fatalf("the build refused: %v\n%s", err, log.String())
 	}
-	if len(written) != 2 || written[1] != "dist/privacy/index.html" {
+	if !wrote(written, "dist/privacy/index.html") {
 		t.Fatalf("the build wrote %q", written)
 	}
 	if !strings.Contains(log.String(), "1 checked, 0 promised, 1 residual") {
@@ -327,8 +327,8 @@ func TestBuildSaysWhenThereIsNoPrivacyProse(t *testing.T) {
 	if err != nil {
 		t.Fatalf("the build refused: %v\n%s", err, log.String())
 	}
-	if len(written) != 1 {
-		t.Fatalf("the build wrote %q", written)
+	if wrote(written, "dist/privacy/index.html") {
+		t.Fatalf("the build wrote a privacy page out of a tree that carries no prose for one: %q", written)
 	}
 	if !strings.Contains(log.String(), "no content/privacy.txt in the tree") {
 		t.Errorf("the run passed over the absence:\n%s", log.String())