M web/handlers.go => web/handlers.go +14 -2
@@ 100,14 100,26 @@ func httpStatusFor(err error) int {
}
// fail renders the chrome error page for err, logging 5xx causes.
+//
+// Only a 400 carries the error's own text, and it is the one class that should:
+// "invalid git ref" tells the viewer what to change about what they typed.
+// Every other status takes the instance's standard sentence — a 500 because an
+// error from below names paths and queries, a 404 because the standard sentence
+// is exactly the one a repository that never existed produces, which is what
+// keeps a repository the viewer may not see indistinguishable from an absent
+// one.
func (s *Server) fail(w http.ResponseWriter, r *http.Request, err error) {
status := httpStatusFor(err)
if status >= 500 {
logrus.WithError(err).WithField("path", r.URL.Path).Error("web: request failed")
- s.renderError(w, r, status, "an internal error occurred")
+ s.renderError(w, r, status, "")
return
}
- s.renderError(w, r, status, err.Error())
+ if status == http.StatusBadRequest {
+ s.renderError(w, r, status, err.Error())
+ return
+ }
+ s.renderError(w, r, status, "")
}
// resolve authorizes and opens a repository, returning the git handle and the
M web/router.go => web/router.go +45 -29
@@ 2,26 2,57 @@ package web
import (
"net/http"
- "path"
"strings"
"github.com/go-chi/chi/v5"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/assets"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/csrf"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/middleware"
)
-// Register mounts every compare.sr.ht route onto r. The caller is responsible
-// for installing the middleware documented on the package (config + authz at a
-// minimum); Register adds no middleware of its own.
+// Register mounts every compare.sr.ht route onto r, inside a group carrying the
+// three middlewares that need this Server. The chain documented on the package
+// (config + authz at a minimum) is still the caller's, and stays outside this
+// group.
+//
+// - PrivateCache, because every page here is a page to its owner and a 404 to
+// everybody else, at a URL that says nothing about the viewer. The static
+// route opts back out per asset, from inside assets.Handler, and only once
+// the bytes are committed.
+// - RecoverPanics, so a panicking handler produces this service's own error
+// page instead of a dropped connection — and, when the response has already
+// started, a dropped connection instead of an error page appended to half a
+// rendered diff.
+// - csrf.Require, which guards a future rather than a present: every route
+// below is a GET, so nothing is refused by it today. It is installed anyway
+// because the day somebody adds the first POST is exactly the day nobody
+// remembers to add the check, and these services have no CSRF token to fall
+// back on — the session cookie is meta.sr.ht's, set on the parent domain,
+// with a SameSite no individual service can choose.
+//
+// The router's own 404 is set here too, so a mistyped URL lands on a page with a
+// nav rather than on chi's plain-text dead end.
func (s *Server) Register(r chi.Router) {
- r.Get("/", s.handleIndex)
- r.Get("/jump", s.handleJump)
- r.Get("/healthz", s.handleHealthz)
- r.Get("/static/*", s.handleStatic)
-
- r.Get("/~{owner}/{repo}", s.handleRepo)
- // A single wildcard route serves both the form target (empty wildcard ⇒
- // redirect to the canonical URL) and the compare view itself.
- r.Get("/~{owner}/{repo}/compare/*", s.handleCompare)
- r.Get("/~{owner}/{repo}/commit/{rev}", s.handleCommit)
+ r.Group(func(r chi.Router) {
+ r.Use(middleware.PrivateCache)
+ r.Use(middleware.RecoverPanics(func(w http.ResponseWriter, r *http.Request, _ any) {
+ s.renderError(w, r, http.StatusInternalServerError, "")
+ }))
+ r.Use(csrf.Require(s.chromeSvc.SelfOrigin(), nil))
+
+ r.NotFound(s.handleNotFound)
+
+ r.Get("/", s.handleIndex)
+ r.Get("/jump", s.handleJump)
+ r.Get("/healthz", s.handleHealthz)
+ r.Handle(assets.DefaultPrefix+"*", s.static)
+
+ r.Get("/~{owner}/{repo}", s.handleRepo)
+ // A single wildcard route serves both the form target (empty wildcard ⇒
+ // redirect to the canonical URL) and the compare view itself.
+ r.Get("/~{owner}/{repo}/compare/*", s.handleCompare)
+ r.Get("/~{owner}/{repo}/commit/{rev}", s.handleCommit)
+ })
}
// handleHealthz is a dependency-free liveness probe.
@@ 30,21 61,6 @@ func (s *Server) handleHealthz(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write([]byte("ok\n"))
}
-// handleStatic serves the embedded assets, tagging the content-addressed
-// stylesheet as immutable and forcing a JS content type for the bundle.
-func (s *Server) handleStatic(w http.ResponseWriter, r *http.Request) {
- name := path.Base(r.URL.Path)
- if strings.HasSuffix(name, ".js") {
- w.Header().Set("Content-Type", "application/javascript; charset=utf-8")
- }
- if hashedCSSRe.MatchString(name) || hashedBundleRe.MatchString(name) {
- w.Header().Set("Cache-Control", "public, max-age=31536000, immutable")
- } else {
- w.Header().Set("Cache-Control", "public, max-age=3600")
- }
- s.staticFileServer.ServeHTTP(w, r)
-}
-
// handleJump powers the owner/repo jump form: it redirects to the canonical repo
// URL. A leading "~" on the owner is tolerated.
func (s *Server) handleJump(w http.ResponseWriter, r *http.Request) {
M web/server.go => web/server.go +64 -69
@@ 19,16 19,20 @@
// switcher or re-derives a login URL: the copy that used to live in web/chrome.go
// is exactly what ecore exists to have deleted.
//
-// Two things about the chrome remain this service's own, because they are about
-// what compare renders and not about the instance: the vendored bundle's href
-// (BundleHref below), and the full-bleed ContainerClass the two diff views set —
-// a side-by-side diff in a centered "container" is a column of code half the
-// window wide.
+// One thing about the chrome remains this service's own, because it is about
+// what compare renders and not about the instance: the full-bleed
+// ContainerClass the two diff views set — a side-by-side diff in a centered
+// "container" is a column of code half the window wide. The vendored bundle's
+// href is no longer among them: it is a hashed build artefact like the
+// stylesheet, so it lives in chrome.Service.Assets, which is the slot ecore
+// grew once three services had each added their own field for it.
//
// # What the cmd layer must wire
//
-// Register only installs routes; it assumes the following middleware is already
-// applied to the router it is handed, in this order (outermost first):
+// Register installs the middleware that needs this Server — the private cache
+// policy, panic recovery through this package's error page, and the same-origin
+// guard — and assumes the following is already applied to the router it is
+// handed, in this order (outermost first):
//
// chi middleware.RealIP
// chi middleware.Recoverer
@@ 46,11 50,12 @@ import (
"fmt"
"io/fs"
"net/http"
- "path"
- "regexp"
+ "github.com/sirupsen/logrus"
"github.com/vaughan0/go-ini"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/assets"
"sourcecraft.dev/bigbes/sr-ht-ecore/chrome"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/pages"
"sourcecraft.dev/bigbes/sr-ht-compare/authz"
)
@@ 62,15 67,12 @@ import (
// instance's navigation and fail to recognise itself in it.
const configSection = "compare.sr.ht"
-// hashedCSSRe matches the content-addressed stylesheet name so it can be served
-// with an immutable cache lifetime (the hash changes whenever the bytes do).
-var hashedCSSRe = regexp.MustCompile(`^main\.min\.[0-9a-f]{6,}\.css$`)
-
-// hashedBundleRe matches the content-addressed frontend bundle. Like the
-// stylesheet it carries a content hash in its name so a deploy busts the browser
-// cache (a stale bundle.js is why the file tree can render blank after an
-// upgrade); a matching name is served immutable.
-var hashedBundleRe = regexp.MustCompile(`^bundle\.[0-9a-f]{6,}\.js$`)
+// bundleAsset is the key compare's layout reads its front-end bundle's href
+// under, in chrome.Service.Assets. It is spelled once here and once in the
+// "scripts" block of the two diff pages; a third spelling would render no
+// script tag at all rather than fail, which is why the two that exist are a
+// constant and a template guarded on emptiness.
+const bundleAsset = "bundle.js"
// Server holds the immutable configuration a request handler needs. It is built
// once at startup and is safe for concurrent use.
@@ 84,18 86,31 @@ type Server struct {
// job until this service stopped carrying its own copy).
chromeSvc *chrome.Service
- bundleHref string
+ // pages is one parsed template set per page, discovered from the embedded
+ // tree by ecore rather than listed here. A page that defines no "content"
+ // never gets this far: pages.Load refuses it at startup, where the
+ // alternative was the chrome around a hole served with a 200.
+ pages pages.Set
- staticFileServer http.Handler
+ // static serves the embedded asset tree with the cache policy the hashed
+ // name implies, and answers everything that is not a file — a directory
+ // above all — through this service's own 404 page.
+ static http.Handler
}
// New assembles a Server from the shared SourceHut config. It reads
// [git.sr.ht] repos, [meta.sr.ht] origin and [compare.sr.ht] origin (all
// required), hands the whole file to chrome.NewService — the switcher is a
// question about every [*.sr.ht] section the instance defines, not about our own
-// keys — and resolves the hashed stylesheet and bundle names by globbing the
-// embedded static FS. A missing required key is a clear error, not a panic, so
-// the cmd layer can fail startup loudly.
+// keys — resolves the hashed stylesheet and bundle through ecore's assets, and
+// parses the page templates through ecore's pages. A missing required key is a
+// clear error, not a panic, so the cmd layer can fail startup loudly.
+//
+// A missing build artefact is not one of those errors. assets.Resolve answers
+// "" for a stylesheet or a bundle this binary was built without, because a
+// checkout that has not run `make css` must still be runnable; the layout
+// guards both hrefs on emptiness so a bare page is what such a build serves,
+// rather than a <link href=""> that re-requests the page it is on.
func New(conf ini.File, authorizer authz.Authorizer) (*Server, error) {
reposRoot, ok := conf.Get("git.sr.ht", "repos")
if !ok || reposRoot == "" {
@@ 113,15 128,24 @@ func New(conf ini.File, authorizer authz.Authorizer) (*Server, error) {
return nil, fmt.Errorf("web: [%s] origin is required", configSection)
}
- cssHref, err := resolveCSSHref()
+ cssHref, err := assets.Resolve(staticFS, cssGlob, assets.DefaultPrefix)
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("web: resolve the stylesheet: %w", err)
+ }
+ bundleHref, err := assets.Resolve(staticFS, bundleGlob, assets.DefaultPrefix)
+ if err != nil {
+ return nil, fmt.Errorf("web: resolve the bundle: %w", err)
+ }
+ if cssHref == "" || bundleHref == "" {
+ logrus.WithFields(logrus.Fields{"css": cssHref, "bundle": bundleHref}).
+ Warn("web: built without a front-end artefact; run `make` before `go build`")
}
chromeSvc.StyleHref = cssHref
+ chromeSvc.Assets = map[string]string{bundleAsset: bundleHref}
- bundleHref, err := resolveBundleHref()
+ set, err := pages.Load(tmplFS, pages.Options{Funcs: funcMap})
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("web: load the page templates: %w", err)
}
staticSub, err := fs.Sub(staticFS, "static")
@@ 129,13 153,18 @@ func New(conf ini.File, authorizer authz.Authorizer) (*Server, error) {
return nil, fmt.Errorf("web: sub static FS: %w", err)
}
- return &Server{
- authorizer: authorizer,
- reposRoot: reposRoot,
- chromeSvc: chromeSvc,
- bundleHref: bundleHref,
- staticFileServer: http.StripPrefix("/static/", http.FileServer(http.FS(staticSub))),
- }, nil
+ s := &Server{
+ authorizer: authorizer,
+ reposRoot: reposRoot,
+ chromeSvc: chromeSvc,
+ pages: set,
+ }
+ // Built after the Server exists because the not-found arm is this service's
+ // own error page: an asset URL typed by hand lands on a page with a nav to
+ // get out of, and a directory — /static/, which the file server alone would
+ // answer with a listing of every artefact in the binary — lands there too.
+ s.static = assets.Handler(staticSub, assets.DefaultPrefix, http.HandlerFunc(s.handleNotFound))
+ return s, nil
}
// viewData is the root value every template is executed against.
@@ 148,11 177,6 @@ func New(conf ini.File, authorizer authz.Authorizer) (*Server, error) {
type viewData struct {
chrome.Page
- // BundleHref is the hashed front-end bundle's URL. It is chrome — every page
- // may load it — but it is this service's alone: no other service on the
- // instance ships a diff renderer, so it stays here and not in ecore's Page.
- BundleHref string
-
// Data is the page's own payload.
Data any
}
@@ 163,34 187,5 @@ type viewData struct {
// a viewer whose cookie is missing, expired or unreadable — so the nav offers
// login to exactly the viewers the handlers treat as anonymous.
func (s *Server) view(r *http.Request, title string) viewData {
- return viewData{
- Page: s.chromeSvc.Page(r, title, authz.ForContext(r.Context())),
- BundleHref: s.bundleHref,
- }
-}
-
-// resolveCSSHref globs the embedded static FS for the content-addressed
-// stylesheet and returns its site-absolute URL.
-func resolveCSSHref() (string, error) {
- matches, err := fs.Glob(staticFS, "static/main.min.*.css")
- if err != nil {
- return "", fmt.Errorf("web: glob stylesheet: %w", err)
- }
- if len(matches) == 0 {
- return "", fmt.Errorf("web: no main.min.*.css in embedded static assets")
- }
- return "/static/" + path.Base(matches[0]), nil
-}
-
-// resolveBundleHref globs the embedded static FS for the content-addressed
-// frontend bundle and returns its site-absolute URL.
-func resolveBundleHref() (string, error) {
- matches, err := fs.Glob(staticFS, "static/bundle.*.js")
- if err != nil {
- return "", fmt.Errorf("web: glob bundle: %w", err)
- }
- if len(matches) == 0 {
- return "", fmt.Errorf("web: no bundle.*.js in embedded static assets")
- }
- return "/static/" + path.Base(matches[0]), nil
+ return viewData{Page: s.chromeSvc.Page(r, title, authz.ForContext(r.Context()))}
}
M web/templates.go => web/templates.go +43 -59
@@ 1,19 1,19 @@
package web
import (
- "bytes"
"embed"
"html/template"
"net/http"
- "time"
"github.com/sirupsen/logrus"
"sourcecraft.dev/bigbes/sr-ht-ecore/chrome"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/pages"
)
-// tmplFS holds the page templates. Each page is parsed together with the shared
-// layout into its own template set so that per-page "content"/"scripts" defines
-// do not collide across pages.
+// tmplFS holds the page templates. pages.Load discovers the pages in it and
+// parses each one into its own set together with the shared layout, so that
+// per-page "content"/"scripts" defines do not collide across pages — and so
+// that adding templates/whatever.html is the whole registration of a page.
//
//go:embed templates/*.html
var tmplFS embed.FS
@@ 24,6 24,14 @@ var tmplFS embed.FS
//go:embed static
var staticFS embed.FS
+// The globs that find this build's content-addressed artefacts in staticFS.
+// `make css` and the bundle build each write exactly one file, removing the
+// previous build's first, so a glob here has at most one match to pick.
+const (
+ cssGlob = "static/main.min.*.css"
+ bundleGlob = "static/bundle.*.js"
+)
+
// funcMap holds the template helpers shared by every page.
//
// It starts from chrome.Funcs — so the shared partials find the helpers they
@@ 31,14 39,13 @@ var staticFS embed.FS
// rule rather than this service's copy of it — and adds compare's own on top.
// Adding after is deliberate: a name may then be shadowed on purpose rather than
// by accident of map ordering.
+// The commit timestamps this service used to format with a "date" helper of its
+// own now go through chrome's "reltime" and "abstime": a listing says "3 days
+// ago" and hovers to the exact UTC stamp, which is the one spelling the whole
+// instance shows.
var funcMap = func() template.FuncMap {
m := chrome.Funcs()
- // date formats a commit timestamp for display.
- m["date"] = func(t time.Time) string {
- return t.UTC().Format("2006-01-02 15:04 MST")
- }
-
// statusClass maps a git file-change status letter to a CSS modifier used by
// the .diff-status badge (see the diff-status rules in layout.html). Anything
// unrecognized falls back to the neutral "o".
@@ 82,59 89,36 @@ var funcMap = func() template.FuncMap {
return m
}()
-// pageNames are the content templates; each is parsed with layout.html.
-var pageNames = []string{"index", "repo", "compare", "commit", "error"}
-
-// pages maps a page name to its parsed template set: the shared chrome partials
-// of sr-ht-ecore, the layout, and that one page. The partials are attached to
-// every set rather than to a shared one, for the same reason the layout is —
-// each page defines its own "content", and one set would let the last parsed win.
-var pages = func() map[string]*template.Template {
- m := make(map[string]*template.Template, len(pageNames))
- for _, name := range pageNames {
- t := chrome.MustAttach(template.New("layout.html").Funcs(funcMap))
- t = template.Must(t.ParseFS(tmplFS, "templates/layout.html", "templates/"+name+".html"))
- m[name] = t
- }
- return m
-}()
-
-// render executes a page into a buffer first, so a template error yields a clean
-// 500 rather than a half-written response. On success it writes the status and
-// the buffered HTML.
+// render writes one page, and logs whatever pages.Render gives back.
+//
+// The log line is the whole of what a caller may do with that error: Render has
+// already answered the response — a 500 carrying a fixed string when the
+// template failed — so handing it to fail would write a second response over a
+// committed one, or, when the failure is in the error page itself, recurse
+// through the page that just broke. That is why this returns nothing.
func (s *Server) render(w http.ResponseWriter, status int, page string, vd viewData) {
- t, ok := pages[page]
- if !ok {
- logrus.WithField("page", page).Error("web: unknown template page")
- http.Error(w, "internal server error", http.StatusInternalServerError)
- return
- }
- var buf bytes.Buffer
- if err := t.ExecuteTemplate(&buf, "layout.html", vd); err != nil {
- logrus.WithError(err).WithField("page", page).Error("web: template execution failed")
- http.Error(w, "internal server error", http.StatusInternalServerError)
- return
+ if err := s.pages.Render(w, status, page, vd); err != nil {
+ logrus.WithError(err).WithField("page", page).Error("web: render")
}
- w.Header().Set("Content-Type", "text/html; charset=utf-8")
- w.WriteHeader(status)
- _, _ = buf.WriteTo(w)
-}
-
-// errorData is the payload of the error page.
-type errorData struct {
- Status int
- StatusText string
- Message string
}
-// renderError renders the chrome-wrapped error page. It never recurses into
-// render on failure (render falls back to http.Error itself).
+// renderError renders the chrome-wrapped error page for a status.
+//
+// An empty message takes the instance's standard sentence for that status, so
+// the 404 a hidden repository produces reads exactly like the 404 of a
+// repository that never existed — which is the point of answering 404 rather
+// than 403 in the first place. A message is only ever this service's own words
+// about what the viewer typed (a malformed compare spec), never an error from
+// below.
func (s *Server) renderError(w http.ResponseWriter, r *http.Request, status int, message string) {
vd := s.view(r, http.StatusText(status))
- vd.Data = errorData{
- Status: status,
- StatusText: http.StatusText(status),
- Message: message,
- }
- s.render(w, status, "error", vd)
+ vd.Data = pages.Error(status, message)
+ s.render(w, status, pages.ErrorPage, vd)
+}
+
+// handleNotFound is the 404 for a route the router does not have and for an
+// asset path that is not a file. It is a http.HandlerFunc so it can be handed
+// to chi's NotFound and to assets.Handler, which both want one.
+func (s *Server) handleNotFound(w http.ResponseWriter, r *http.Request) {
+ s.renderError(w, r, http.StatusNotFound, "")
}
M web/templates/commit.html => web/templates/commit.html +6 -2
@@ 11,7 11,7 @@
<p><strong>{{$c.Subject}}</strong></p>
{{if $c.Body}}<pre class="commit-body">{{$c.Body}}</pre>{{end}}
<p class="text-muted">
- {{$c.AuthorName}} <{{$c.AuthorEmail}}> — {{date $c.Date}}
+ {{$c.AuthorName}} <{{$c.AuthorEmail}}> — {{abstime $c.Date}}
</p>
<p class="text-muted">
Commit <code>{{$c.SHA}}</code> —
@@ 77,6 77,10 @@
<script id="compare-data" type="application/json">{{.Data.JSON}}</script>
{{end}}
+{{/* The bundle's href comes from the chrome's Assets, and is guarded on
+ emptiness for the reason StyleHref is: a build without the artefact must
+ render a page that says so by being inert, not a <script src=""> that
+ re-requests the document it is in. */}}
{{define "scripts"}}
-<script type="module" src="{{.BundleHref}}"></script>
+{{with index .Assets "bundle.js"}}<script type="module" src="{{.}}"></script>{{end}}
{{end}}
M web/templates/compare.html => web/templates/compare.html +3 -2
@@ 35,7 35,7 @@
<li>
<a href="/~{{$owner}}/{{$repo}}/commit/{{.SHA}}"><code>{{.ShortSHA}}</code></a>
{{.Subject}}
- <span class="text-muted">— {{.AuthorName}}, {{date .Date}}</span>
+ <span class="text-muted" title="{{abstime .Date}}">— {{.AuthorName}}, {{reltime .Date}}</span>
</li>
{{end}}
</ul>
@@ 79,6 79,7 @@
<script id="compare-data" type="application/json">{{.Data.JSON}}</script>
{{end}}
+{{/* See commit.html: the bundle is a chrome asset, guarded on emptiness. */}}
{{define "scripts"}}
-<script type="module" src="{{.BundleHref}}"></script>
+{{with index .Assets "bundle.js"}}<script type="module" src="{{.}}"></script>{{end}}
{{end}}
M web/templates/layout.html => web/templates/layout.html +7 -3
@@ 9,7 9,9 @@
own, and nobody else's), and three seams: "head" and "scripts" are blocks, so
a page that needs neither still renders; "content" is a {{template}} and must
stay one — a block would give every page an empty default, which is the chrome
- around a hole served 200 for the page that forgot to define it.
+ around a hole served 200 for the page that forgot to define it. ecore's
+ pages.Load refuses such a page at startup, and it can go on doing so only
+ while this line stays a {{template}}.
*/}}
<!doctype html>
<html lang="en">
@@ 19,8 21,10 @@
<title>{{.Title}}</title>
<link rel="icon" type="image/svg+xml" href="/static/logo.svg">
{{/* Guarded rather than emitted empty: <link href=""> re-requests the page
- it is on, which is a page load per page load. New refuses to start
- without a stylesheet, so in a deployed binary this is always taken. */}}
+ it is on, which is a page load per page load. assets.Resolve answers ""
+ for a binary built without `make css`, which is a checkout run from
+ source; a deployed build always has the artefact and always takes
+ this. */}}
{{if .StyleHref}}<link rel="stylesheet" href="{{.StyleHref}}">{{end}}
<style>
/* Semantic status badge for the changed-files table. The default table
M web/templates/repo.html => web/templates/repo.html +1 -1
@@ 60,7 60,7 @@
<li>
<a href="/~{{$owner}}/{{$repo}}/commit/{{.SHA}}"><code>{{.ShortSHA}}</code></a>
{{.Subject}}
- <span class="text-muted">— {{.AuthorName}}, {{date .Date}}</span>
+ <span class="text-muted" title="{{abstime .Date}}">— {{.AuthorName}}, {{reltime .Date}}</span>
</li>
{{end}}
</ul>
M web/web_test.go => web/web_test.go +141 -39
@@ 2,8 2,6 @@ package web
import (
"context"
- "crypto/rand"
- "encoding/base64"
"encoding/json"
"errors"
"net/http"
@@ 14,37 12,26 @@ import (
"strings"
"testing"
- "github.com/fernet/fernet-go"
"github.com/go-chi/chi/v5"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
- "github.com/vaughan0/go-ini"
"sourcecraft.dev/bigbes/sr-ht-core/config"
"sourcecraft.dev/bigbes/sr-ht-core/crypto"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/assets"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/csrf"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/ecoretest"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/pages"
"sourcecraft.dev/bigbes/sr-ht-compare/authz"
"sourcecraft.dev/bigbes/sr-ht-compare/core"
"sourcecraft.dev/bigbes/sr-ht-compare/gitx"
)
-// testConf carries the crypto keys established in TestMain so tests can seal
-// unified-login cookies.
-var testConf ini.File
-
+// TestMain seeds core-go's process-global crypto with ecore's fixed test
+// keyset, which is what lets login below seal a unified-login cookie the authz
+// middleware can open again.
func TestMain(m *testing.M) {
- var fk fernet.Key
- if err := fk.Generate(); err != nil {
- panic("generate fernet key: " + err.Error())
- }
- seed := make([]byte, 32)
- if _, err := rand.Read(seed); err != nil {
- panic("generate webhook seed: " + err.Error())
- }
- testConf = ini.File{
- "sr.ht": ini.Section{"network-key": fk.Encode()},
- "webhooks": ini.Section{"private-key": base64.StdEncoding.EncodeToString(seed)},
- }
- crypto.InitCrypto(testConf)
+ ecoretest.InitCrypto()
os.Exit(m.Run())
}
@@ 144,19 131,13 @@ func runGit(t *testing.T, dir string, args ...string) string {
// middleware the cmd layer installs, and returns the handler.
func testServer(t *testing.T, root string, az authz.Authorizer) http.Handler {
t.Helper()
- conf := ini.File{
- "sr.ht": ini.Section{
- "network-key": testConf.Section("sr.ht")["network-key"],
- "site-name": "sourcehut",
- "environment": "development",
- },
- "webhooks": ini.Section{"private-key": testConf.Section("webhooks")["private-key"]},
- "compare.sr.ht": ini.Section{"origin": "https://compare.example"},
- "meta.sr.ht": ini.Section{"origin": "https://meta.example"},
- "git.sr.ht": ini.Section{"origin": "https://git.example", "repos": root},
- "todo.sr.ht": ini.Section{"origin": "https://todo.example"},
- "hub.sr.ht": ini.Section{"origin": "https://hub.example"},
- }
+ // The synthetic instance of ecoretest, with the two keys this service adds
+ // to it: the bare-repository root gitx reads, and a non-production
+ // environment so the banner is on the page TestChromeIsRendered inspects.
+ conf := ecoretest.Config(configSection,
+ ecoretest.Set("git.sr.ht", "repos", root),
+ ecoretest.Set("sr.ht", "environment", "development"),
+ )
srv, err := New(conf, az)
require.NoError(t, err, "New")
@@ 402,6 383,121 @@ func TestStaticBundleAndCSS(t *testing.T) {
require.Equalf(t, http.StatusOK, rec.Code, "%s", css)
assert.Contains(t, rec.Header().Get("Cache-Control"), "immutable",
"a hashed stylesheet must be cacheable forever")
+ assert.Empty(t, rec.Header().Get("Vary"),
+ "an asset served identically to everybody must not be declared to vary on Cookie")
+
+ // An asset whose name carries no hash cannot be immutable: the next deploy
+ // serves different bytes at the same URL.
+ rec = get(t, h, "/static/logo.svg", "")
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.Equal(t, "public, max-age=3600", rec.Header().Get("Cache-Control"))
+}
+
+// TestStaticListingRefused pins what the hand-rolled static route published
+// before this service adopted assets.Handler: an http.FileServer answers a
+// directory with a listing, so /static/ was the whole inventory of the binary —
+// every vendored artefact and the hashed names that fingerprint the build — as
+// a public, hour-cacheable page.
+func TestStaticListingRefused(t *testing.T) {
+ root, _ := gitFixture(t)
+ h := testServer(t, root, demoAuthorizer())
+
+ for _, target := range []string{"/static/", "/static/nothing-here.css"} {
+ t.Run(target, func(t *testing.T) {
+ rec := get(t, h, target, "")
+ assert.Equal(t, http.StatusNotFound, rec.Code)
+ // A Go file-server listing is a list of <a href="name">name</a>; the
+ // stylesheet's own <link> in the chrome is not, which is why the
+ // entry and not the name is what this looks for.
+ assert.NotContains(t, rec.Body.String(), `<a href="logo.svg">`,
+ "the response is a directory listing")
+ // And it is a page, not net/http's plain-text dead end.
+ assert.Contains(t, rec.Body.String(), "navbar-brand")
+ })
+ }
+}
+
+// TestPagesAreNotCacheable pins the policy every page behind the login cookie
+// carries: these URLs say nothing about the viewer, so a cache with no
+// instruction would hand one account's page to the next request.
+func TestPagesAreNotCacheable(t *testing.T) {
+ root, _ := gitFixture(t)
+ h := testServer(t, root, demoAuthorizer())
+
+ rec := get(t, h, "/", "bigbes")
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.Equal(t, "private, no-store", rec.Header().Get("Cache-Control"))
+ assert.Equal(t, "Cookie, Authorization", rec.Header().Get("Vary"))
+}
+
+// TestNotFoundIsAPage checks that a URL the router does not have lands on the
+// shared error page rather than chi's plain-text 404, and that it says the
+// instance's standard sentence — the same one a repository the viewer may not
+// see produces, which is what keeps the two indistinguishable.
+func TestNotFoundIsAPage(t *testing.T) {
+ root, _ := gitFixture(t)
+ h := testServer(t, root, demoAuthorizer())
+
+ for name, target := range map[string]string{
+ "unrouted": "/no/such/page",
+ "hidden repo": "/~alice/nope",
+ } {
+ t.Run(name, func(t *testing.T) {
+ rec := get(t, h, target, "")
+ require.Equal(t, http.StatusNotFound, rec.Code)
+ assert.Contains(t, rec.Body.String(), pages.NotFoundMessage)
+ assert.Contains(t, rec.Body.String(), "navbar-brand", "the 404 has no chrome")
+ })
+ }
+}
+
+// TestBadSpecKeepsItsOwnMessage is the other half of that rule: a 400 describes
+// something the viewer just typed, so it must not be replaced by a house phrase
+// that sends them back to the form with nothing to change.
+func TestBadSpecKeepsItsOwnMessage(t *testing.T) {
+ root, _ := gitFixture(t)
+ h := testServer(t, root, demoAuthorizer())
+
+ rec := get(t, h, "/~alice/demo/compare/..bad", "")
+ require.Equal(t, http.StatusBadRequest, rec.Code)
+ assert.Contains(t, rec.Body.String(), core.ErrBadRef.Error())
+}
+
+// TestUnsafeMethodRefused pins the same-origin guard. compare serves no POST
+// today, so what this asserts is that the guard is installed at all — the day a
+// form arrives it is already covered.
+//
+// The probe is a path with no route, because that is the one unsafe request
+// this router hands to the guard: chi resolves a method before a group's
+// middleware runs, so a POST to a GET-only path is its own 405, while a POST to
+// nothing at all reaches the not-found handler through the whole chain. The
+// consequence to know about is in the first arm — such a request is now refused
+// 403 rather than answered 404.
+func TestUnsafeMethodRefused(t *testing.T) {
+ root, _ := gitFixture(t)
+ h := testServer(t, root, demoAuthorizer())
+
+ post := func(origin string) *httptest.ResponseRecorder {
+ req := httptest.NewRequest(http.MethodPost, "/no/such/page", nil)
+ if origin != "" {
+ req.Header.Set("Origin", origin)
+ }
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ return rec
+ }
+
+ rec := post("")
+ assert.Equal(t, http.StatusForbidden, rec.Code,
+ "a request carrying neither Origin nor Referer cannot be shown to have come from us")
+ assert.Contains(t, rec.Body.String(), csrf.Message)
+
+ rec = post("https://evil.example")
+ assert.Equal(t, http.StatusForbidden, rec.Code, "a cross-origin POST must be refused")
+
+ rec = post(ecoretest.Origin(configSection))
+ assert.Equal(t, http.StatusNotFound, rec.Code,
+ "a same-origin POST passes the guard and meets the router's own answer")
}
// TestCompareJSONNoScriptBreakout verifies a file path containing "</script>"
@@ 422,19 518,25 @@ func TestCompareJSONNoScriptBreakout(t *testing.T) {
assert.Containsf(t, s, "\\u003c/script", "expected an escaped \\u003c/script, got: %s", s)
}
+// cssName resolves the content-hashed stylesheet filename (main.min.<hash>.css)
+// out of the embedded tree, through the same call the server resolves it with.
func cssName(t *testing.T) string {
t.Helper()
- href, err := resolveCSSHref()
- require.NoError(t, err)
- return strings.TrimPrefix(href, "/static/")
+ return assetName(t, cssGlob)
}
// bundleName resolves the content-hashed frontend bundle filename (bundle.<hash>.js).
func bundleName(t *testing.T) string {
t.Helper()
- href, err := resolveBundleHref()
+ return assetName(t, bundleGlob)
+}
+
+func assetName(t *testing.T, glob string) string {
+ t.Helper()
+ href, err := assets.Resolve(staticFS, glob, assets.DefaultPrefix)
require.NoError(t, err)
- return strings.TrimPrefix(href, "/static/")
+ require.NotEmptyf(t, href, "no artefact matching %s in the embedded tree", glob)
+ return strings.TrimPrefix(href, assets.DefaultPrefix)
}
// extractCompareData pulls and decodes the embedded JSON payload from a page.