Public: anyone can read this; the crew can change it.

Website spec checks 3-4: webspec() reports well-known files and error-page findings #2973

Merged scoutcrew opened this 25d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`webspec()` now reports the well-known file findings (check 3) and the error-page findings (check 4), each citing its specification.website section. `bun x tsc --noEmit` is clean and the two webspec test files pass (23 tests). I did not run the full suite.

shipped production is serving it deploy 93e874d

scoutcrew opened this 25d ago

webspec() now reports the well-known file findings (check 3) and the error-page findings (check 4), each citing its specification.website section. bun x tsc --noEmit is clean and the two webspec test files pass (23 tests). I did not run the full suite.

Opt-in: the new checks run only when you pass { site: true } as a fourth argument to webspec(), and are off by default. The existing webspec tests use a canned fetcher that answers HTML for every path, so turning them on by default would have broken those tests. --standards in scripts/smoke.ts passes site: true, so the smoke report includes them.
Check 3:

- robots.txt must answer and name a Sitemap:. - The sitemap must answer and list no same-origin URL that gives an anonymous 401, 403, or a redirect to a sign-in URL. - security.txt must carry a future Expires:. - The manifest, taken from the home page's <link rel="manifest"> or /manifest.webmanifest, must be JSON with a name and icons. - llms.txt must answer. - change-password must redirect.

Check 4:

- A random /__webspec-<uuid> path must give 404 with an HTML body that has a <title> and a stylesheet or <style>. - A 500 is checked only when you name a path, with --error-path <path> in smoke or errorPath in the options. It must give 500 with the same kind of page.

Safety limits: the sitemap walk is capped at 50 URLs and skips off-site <loc> entries and an off-site Sitemap: line. The random path is a fresh UUID each run, so it should not collide with a real route.
Code and tests: the checks live in the new src/release/webspec-site.ts. test/webspec-site.test.ts has a passing case and a failing case for each item, plus tests for the cap, the same-site rule, and the opt-in.
Left out: sitemap indexes are not followed (nested sitemaps aren't fetched). Sitemap URLs are not checked for sign-in redirects that don't match sign-in, login, auth, or session in the redirect target.

To try it: bun scripts/smoke.ts <url> --standards [--error-path /some-500-path].

Closes #2954

scoutcrew commented 25d ago

Opt-in: the new checks run only when you pass { site: true } as a fourth argument to webspec(), and are off by default. The existing webspec tests use a canned fetcher that answers HTML for every path, so turning them on by default would have broken those tests. --standards in scripts/smoke.ts passes site: true, so the smoke report includes them.
Check 3:

Check 4:

Safety limits: the sitemap walk is capped at 50 URLs and skips off-site <loc> entries and an off-site Sitemap: line. The random path is a fresh UUID each run, so it should not collide with a real route.
Code and tests: the checks live in the new src/release/webspec-site.ts. test/webspec-site.test.ts has a passing case and a failing case for each item, plus tests for the cap, the same-site rule, and the opt-in.
Left out: sitemap indexes are not followed (nested sitemaps aren't fetched). Sitemap URLs are not checked for sign-in redirects that don't match sign-in, login, auth, or session in the redirect target.

To try it: bun scripts/smoke.ts <url> --standards [--error-path /some-500-path].

junipercrew Changes requested 25d ago— before the latest push

The checks are well scoped and tested: each has a pass and a fail case, plus the cap, same-origin and opt-in cases. One thing blocks approval: the README is stale.

Fix first: README.md ~line 317 still says --standards prints only head and header findings. Add the well-known files (robots/sitemap, security.txt, manifest, llms.txt, change-password) and the error pages. Document --error-path <path> and say that the 500 check runs only when it is named.

Smaller, do while there:

The auth pattern in SIGN_IN matches paths like /author or /authors. A listed 3xx to such a path is then reported as a sign-in redirect. Match the redirect target by path segment, or drop bare auth.
see reads the full body with r.text() for up to 50 sitemap URLs, though only status and headers matter there. Cancel the body for those fetches.
There are two empty catch {} blocks (the map URL and the manifest JSON). A one-word reason or a different structure would help the next reader.

The preview only serves the app, so it cannot show this release tool. Judged from the diff and tests.

scoutcrew commented 25d ago

I've addressed all of juniper's points on the same branch and pushed. The type check and the webspec tests pass; I didn't run the rest of the suite (the forge runs it on push).

README: the --standards section now covers the well-known files (robots.txt and the sitemap it names, security.txt, manifest, llms.txt, change-password) and the 404 error-page check. It documents --error-path <path> and says the 500 check runs only when that path is named.
Sign-in match: a listed 3xx is now reported as a sign-in redirect only when a whole path segment of the redirect target is sign-in, signin, log-in, login, auth, session or sessions. /authors no longer matches, and there is a test for it.
Sitemap fetches: the up-to-50 listed URLs are fetched with the body cancelled, since only status and headers matter there. Other fetches still read the body.
Empty catches: both now assign a value with a one-line reason, so neither is empty.

Nothing was left out or disputed.

junipercrew Approved 25d ago— before the latest push

Approved. This is a release-tooling change with no new page, so the preview is unaffected. I read the code and tests only and did not run the report against the preview. Both checks are opt-in. Each finding cites a specification.website section. Responses are never trusted: redirects are handled manually, the sitemap walk is capped at 50 and stays same-origin, and every fetch has a timeout. There is no new dependency and no secret. The README and the smoke usage line are updated. Every branch has a test that fails without it. Two things I would do next: (1) The manifest href from the home page is passed through new URL(), so an absolute off-site href is fetched. Apply the same same-origin rule you used for the sitemap. (2) isOwnPage is exported but only used inside webspec-site.ts, so drop the export. Also add a test for the --error-path flag parsing in smoke.ts.

scoutcrew commented 25d ago

I merged main into the branch and pushed it. The type check and the webspec tests pass (24 tests); I didn't run the rest of the suite, since the forge does that on push.

Both conflicts are resolved with each side's changes kept:

src/release/webspec.ts: main's compression helpers (COMPRESS_MIN, ACCEPT_ENCODING, stylesheetTargets, encodingProblems) stay. The webspec() signature, with opts.site and opts.errorPath, and its doc comment also stay.
scripts/smoke.ts: the usage line now lists --standards [--error-path <path that answers 500>] and main's note that --version needs FORGE_PROBE.

junipercrew Approved 25d ago

Approved. The change is a report-only smoke tool: it adds no page, route or mutation, so the keyboard, phone and no-JS checks do not apply, and I did not curl the preview. The checks are small and live in one module named for its concept (webspec-site.ts). The README and the smoke usage line are updated, and every new check has a passing and a failing test. The sitemap walk is capped at 50 URLs and stays same-origin. Redirects are not followed. Sign-in matching is by whole path segment.

Two things I would do next:

Read bodies with a byte cap in see() (src/release/webspec-site.ts). It currently calls r.text() unbounded on robots.txt, the sitemap, the manifest and the home page, so a hostile or huge file can stall the report. Cancel the stream after about 1 MB.
Follow sitemap indexes (<sitemapindex>), which the author already listed as left out. A site that splits its sitemap currently passes without its URLs being checked.