diff --git a/go.mod b/go.mod
index 1179c310eb9e2ec17ca0e755736d8cb62b819ab9..88ca842330fec0b248daf17022371596120b816a 100644
--- a/go.mod
+++ b/go.mod
@@ -22,7 +22,7 @@ github.com/yuin/goldmark v1.8.2
go.bigb.es/auxilia v0.5.0
gopkg.in/yaml.v3 v3.0.1
sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260718185800-dd418a200152
- sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808143603-174115990895
+ sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808192241-9377c43a02ca
)
require (
diff --git a/go.sum b/go.sum
index 6bad8c61703d683b5f8faad06ecefaf49b766e35..abaaedf704a182e187deab124851d27940096d2e 100644
--- a/go.sum
+++ b/go.sum
@@ -421,5 +421,5 @@ modernc.org/sqlite v1.38.2 h1:Aclu7+tgjgcQVShZqim41Bbw9Cho0y/7WzYptXqkEek=
modernc.org/sqlite v1.38.2/go.mod h1:cPTJYSlgg3Sfg046yBShXENNtPrWrDX8bsbAQBzgQ5E=
sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260718185800-dd418a200152 h1:9kQC+tDO2CO8avlKadb9Z0if4a6vJuEK80+4zcb6/fU=
sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260718185800-dd418a200152/go.mod h1:Mu1Vx39ws/OTKWGoVERXvkdRSPLBdhuFTYv0ftVV31c=
-sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808143603-174115990895 h1:OGZrtBtMoXhyZGXrPqMzmrNQnStoCLBVuegGo7yF1Us=
-sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808143603-174115990895/go.mod h1:KeoZjm+/nnsdtc1WxB7X/0EeC+Rggt2OkwJDEc6XWnw=
+sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808192241-9377c43a02ca h1:LCfxvF1VJl7djl7noAeXObfuY1csJr2OOFExwU4Y/N4=
+sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808192241-9377c43a02ca/go.mod h1:KeoZjm+/nnsdtc1WxB7X/0EeC+Rggt2OkwJDEc6XWnw=
diff --git a/scss/main.scss b/scss/main.scss
index 9fad8f309f5735f2ad9ec225a80dc123c2fd8e05..32138a3f2c4802fd30b6ac486d338a258cfff5b0 100644
--- a/scss/main.scss
+++ b/scss/main.scss
@@ -518,7 +518,7 @@ // would click to get their text back goes with it. An empty composer is
// chrome and this rule may hide it; one holding a draft is content, exactly
// as a posted thread is, and stays drawn until it is posted or emptied.
//
- // DO NOT "tidy this up" by deleting the composer from threads.html, and do
+ // DO NOT "tidy this up" by deleting the composer from _threads.html, and do
// not drop the `.ph-js` qualifier: with JavaScript off no table is marked,
// this rule matches nothing, and every block keeps its visible composer,
// which is the only reason hiding it here is honest.
diff --git a/web/comments.go b/web/comments.go
index 2587a1fcd56663a53ca0f03269431a8f6172c02c..898c4dd491cacff9929373d5d4934b94b1ccf029 100644
--- a/web/comments.go
+++ b/web/comments.go
@@ -327,10 +327,14 @@ }
s.backToThread(w, r, p, threadID)
}
-// commentPost is the prologue every comment POST shares: the cross-site guard,
-// read authority, a parsed form, and the proposal the URL names — refusing one
-// that belongs to another space for the same reason the review page does, that
-// the id is global but the link names its space.
+// commentPost is the prologue every comment POST shares: read authority, a
+// parsed form, and the proposal the URL names — refusing one that belongs to
+// another space for the same reason the review page does, that the id is global
+// but the link names its space.
+//
+// The cross-site guard used to open this list and is gone from it: it is
+// csrf.Require on the router now (Handler), where it also covers the POST
+// nobody has written yet.
//
// The one authority it checks is the read ACL — these handlers go on to read the
// proposal branch to place an anchor and to list a proposal's threads, and that
@@ -339,10 +343,6 @@ // restated: opening and resolving are the owner's alone and replying is not, the
// service knows both rules, and a second copy here would be a second place for
// them to be wrong.
func (s *Server) commentPost(w http.ResponseWriter, r *http.Request) (service.Proposal, bool) {
- if !s.sameOrigin(r) {
- s.renderError(w, r, http.StatusForbidden, "this request did not originate from this site")
- return service.Proposal{}, false
- }
if !mayRead(r) {
s.renderError(w, r, http.StatusForbidden, "you may not read this proposal")
return service.Proposal{}, false
diff --git a/web/grant_test.go b/web/grant_test.go
index 4e050c5821abe634b7fc02aed9f021117f29b3e8..18a086d5605884784e95d23d40ecbaf78f9b57db 100644
--- a/web/grant_test.go
+++ b/web/grant_test.go
@@ -8,8 +8,8 @@
"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-ecore/ecoretest"
"sourcecraft.dev/bigbes/sr-ht-ecore/grants"
"sourcecraft.dev/bigbes/sr-ht-spec/authn"
@@ -29,18 +29,7 @@ // resolver's business, and this file is about what the handlers do with it.
func grantRouter(t *testing.T, p authn.Principal) http.Handler {
t.Helper()
srv, err := New(Options{
- Conf: ini.File{
- "sr.ht": ini.Section{
- "network-key": testConf.Section("sr.ht")["network-key"],
- "site-name": "sourcehut",
- "environment": "development",
- "owner-name": "bigbes",
- },
- "webhooks": ini.Section{"private-key": testConf.Section("webhooks")["private-key"]},
- "spec.sr.ht": ini.Section{"origin": "https://spec.example"},
- "meta.sr.ht": ini.Section{"origin": "https://meta.example"},
- "tokens.sr.ht": ini.Section{"origin": "https://tokens.example"},
- },
+ Conf: ecoretest.Config(authn.ConfigSection),
Reader: newFakeReader(),
Searcher: &fakeSearcher{},
Resolver: testResolver(t),
@@ -122,7 +111,7 @@
rec := httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/~bigbes/rfcs/specs/0007-storage", nil))
assert.Equal(t, http.StatusFound, rec.Code)
- assert.Contains(t, rec.Header().Get("Location"), "meta.example")
+ assert.Contains(t, rec.Header().Get("Location"), ecoretest.Origin("meta.sr.ht"))
rec = httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/~bigbes/rfcs/specs/0007-storage.md", nil))
diff --git a/web/handlers.go b/web/handlers.go
index 26a68db9b61300d17178b09cee819d113da1c4c3..7240071dca791e8e733f75e08fdcfbfd54255318 100644
--- a/web/handlers.go
+++ b/web/handlers.go
@@ -139,11 +139,18 @@ }
// fail renders the chrome error page for err, logging 5xx causes and telling
// the viewer nothing about them.
+//
+// The 5xx message is left empty so the page takes ecore's shared sentence for
+// the status: a bug here and a panic recovered by the router are the same event
+// to a viewer, and two house phrases for it would only say that they came out
+// of different code. Below 500 the error's own text is the message — those name
+// a revision that does not parse or a document that is not there, which is what
+// the viewer needs.
func (s *Server) fail(w http.ResponseWriter, r *http.Request, err error) {
status := httpStatusFor(err)
if status >= 500 {
log.Printf("web: %s: %v", r.URL.Path, err)
- s.renderError(w, r, status, "an internal error occurred")
+ s.renderError(w, r, status, "")
return
}
s.renderError(w, r, status, err.Error())
@@ -208,7 +215,9 @@ })
}
}
vd.Data = data
- s.render(w, http.StatusOK, "index", vd)
+ if err := s.pages.Render(w, http.StatusOK, "index", vd); err != nil {
+ log.Printf("web: render the landing page for %s: %v", r.URL.Path, err)
+ }
}
// ---- space ----------------------------------------------------------------
@@ -260,7 +269,9 @@ RevQuery: revQuery(rev),
Count: len(snap.Archive.All()),
Items: flattenTree(snap, revQuery(rev)),
}
- s.render(w, http.StatusOK, "space", vd)
+ if err := s.pages.Render(w, http.StatusOK, "space", vd); err != nil {
+ log.Printf("web: render the space page for %s: %v", r.URL.Path, err)
+ }
}
// flattenTree walks the archive's `parent:` hierarchy into an ordered, depth-
@@ -518,7 +529,9 @@ }
vd := s.view(r, page.Title+" — "+ref.String())
vd.Data = data
- s.render(w, http.StatusOK, "document", vd)
+ if err := s.pages.Render(w, http.StatusOK, "document", vd); err != nil {
+ log.Printf("web: render the document page for %s: %v", r.URL.Path, err)
+ }
}
// pinned wraps the archive's resolver so that every site-internal link a
@@ -633,7 +646,9 @@
vd := s.view(r, "")
vd.Title = "search — " + vd.SiteName + " " + vd.SiteLabel
vd.Data = data
- s.render(w, http.StatusOK, "search", vd)
+ if err := s.pages.Render(w, http.StatusOK, "search", vd); err != nil {
+ log.Printf("web: render the search page for %s: %v", r.URL.Path, err)
+ }
}
// hitHref turns a hit into the pinned URL the design specifies for it:
diff --git a/web/inbox.go b/web/inbox.go
index e6bd748c0267e3831c2150e26a75add1576538c9..f67a387522dae6e95bdd4d8ee35bc84a8d3242cf 100644
--- a/web/inbox.go
+++ b/web/inbox.go
@@ -1,6 +1,7 @@
package web
import (
+ "log"
"net/http"
"strconv"
"time"
@@ -70,23 +71,20 @@ Open: proposalRows(open),
Digest: digestRows,
NewCount: newCount,
}
- s.render(w, http.StatusOK, "inbox", vd)
+ if err := s.pages.Render(w, http.StatusOK, "inbox", vd); err != nil {
+ log.Printf("web: render the inbox page for %s: %v", r.URL.Path, err)
+ }
}
// handleInboxSeen advances the owner's digest mark to now, then redirects back
// to the queue so a reload does not re-submit. It is the one write the review
// queue makes; keeping it a POST is what lets handleInbox stay a pure read.
//
-// Only the owner may move their own mark, and the cross-site guard is the same
-// one approve/reject use — the CSRF defense a form post needs when the session
-// cookie is meta's and this service cannot set its SameSite.
+// Only the owner may move their own mark. The cross-site guard this handler
+// used to call for itself is csrf.Require on the router now (Handler).
func (s *Server) handleInboxSeen(w http.ResponseWriter, r *http.Request) {
if !authn.PrincipalFromContext(r.Context()).IsOwner() {
s.renderError(w, r, http.StatusForbidden, "only the instance owner may mark the digest seen")
- return
- }
- if !s.sameOrigin(r) {
- s.renderError(w, r, http.StatusForbidden, "this request did not originate from this site")
return
}
if err := s.reader.MarkDigestSeen(r.Context(), time.Now()); err != nil {
diff --git a/web/proposal.go b/web/proposal.go
index dfbb7c2ca7875c101b9cb9ea0bb3298b2c959023..625a0f7ea014ddc8e159ec1a35c2e6c26e87cdfe 100644
--- a/web/proposal.go
+++ b/web/proposal.go
@@ -3,8 +3,8 @@
import (
"context"
"fmt"
+ "log"
"net/http"
- "net/url"
"strconv"
"github.com/go-chi/chi/v5"
@@ -155,6 +155,11 @@ lost = append(lost, ts...)
}
vd := s.view(r, fmt.Sprintf("Proposal #%d — %s", p.ID, p.Title))
+ // The review page is two prose columns side by side, and the centred
+ // container gives them about half the width they need — every line wraps
+ // twice and the diff stops reading as a diff. This is the page that pays for
+ // full bleed, so it takes it; the rest of the surface stays centred.
+ vd.ContainerClass = "container-fluid"
vd.Data = proposalData{
Proposal: p,
SpaceHref: "/" + ref.String(),
@@ -166,7 +171,9 @@ Rejected: p.State == core.StateRejected,
Lost: lostPanels(lost, controls),
Unresolved: unresolvedThreads(threads),
}
- s.render(w, http.StatusOK, "proposal", vd)
+ if err := s.pages.Render(w, http.StatusOK, "proposal", vd); err != nil {
+ log.Printf("web: render the proposal page for %s: %v", r.URL.Path, err)
+ }
}
// handleProposalApprove merges a proposal on the owner's approval, then redirects
@@ -181,22 +188,18 @@ s.actOnProposal(w, r, s.reader.Reject)
}
// actOnProposal is the shared approve/reject path: the owner-only gate, the
-// cross-site guard, the action, and the post-redirect-get back to the page.
+// action, and the post-redirect-get back to the page.
//
// Only the owner may approve or reject — that is the one authority the whole
// authorization model turns on, and an agent, though authenticated, has it no
-// more than an anonymous viewer. The cross-site guard refuses a state change
-// whose Origin is not this instance, which is the CSRF defense a form post needs
-// when the session cookie is meta's and this service cannot set its SameSite.
+// more than an anonymous viewer. The cross-site guard is not here any more: it
+// is csrf.Require on the router (Handler), so it holds for every mutation this
+// service serves and not only for the ones whose handler remembered to ask.
func (s *Server) actOnProposal(w http.ResponseWriter, r *http.Request,
act func(context.Context, core.SpaceRef, int) (service.Proposal, error)) {
if !authn.PrincipalFromContext(r.Context()).IsOwner() {
s.renderError(w, r, http.StatusForbidden, "only the instance owner may approve or reject a proposal")
- return
- }
- if !s.sameOrigin(r) {
- s.renderError(w, r, http.StatusForbidden, "this request did not originate from this site")
return
}
ref, err := spaceRefFrom(r)
@@ -214,30 +217,6 @@ s.fail(w, r, err)
return
}
http.Redirect(w, r, fmt.Sprintf("/%s/p/%d", ref, id), http.StatusSeeOther)
-}
-
-// sameOrigin reports whether a state-changing request came from this site. It
-// checks the Origin header — which browsers send on every form POST — and falls
-// back to Referer, refusing a request that carries neither. A cross-site forgery
-// carries the attacker's origin and fails; this instance's own form carries its
-// own and passes.
-func (s *Server) sameOrigin(r *http.Request) bool {
- claimed := r.Header.Get("Origin")
- if claimed == "" {
- claimed = r.Header.Get("Referer")
- }
- if claimed == "" {
- return false
- }
- got, err := url.Parse(claimed)
- if err != nil || got.Host == "" {
- return false
- }
- want, err := url.Parse(s.chromeSvc.SelfOrigin())
- if err != nil {
- return false
- }
- return got.Scheme == want.Scheme && got.Host == want.Host
}
// stateBadge maps a proposal state onto the Bootstrap badge class the template
diff --git a/web/proposal_test.go b/web/proposal_test.go
index 471991da9ec0134fdfb587d6b1dc618922603869..5fce1fbda0fd14af1a2d117ffc320053989d1902 100644
--- a/web/proposal_test.go
+++ b/web/proposal_test.go
@@ -64,6 +64,11 @@ }
if !strings.Contains(body, "specs/0007-storage.md") {
t.Errorf("page does not name the changed document")
}
+ // The review page is the one full-bleed page of this surface: two prose
+ // columns side by side do not fit the centred container.
+ if !strings.Contains(body, `class="container-fluid"`) {
+ t.Errorf("the review page is not full width; body:\n%s", body)
+ }
}
// TestProposalPageHidesControlsWhenMerged proves a terminal proposal shows no
diff --git a/web/router.go b/web/router.go
index 52a75196877858ddbeb8784bca86dcf0085c3f7b..9091803dfa873228a495f01cdc36dd75124bc856 100644
--- a/web/router.go
+++ b/web/router.go
@@ -3,32 +3,60 @@
import (
"net/http"
"net/url"
- "path"
"strings"
"github.com/go-chi/chi/v5"
- "github.com/go-chi/chi/v5/middleware"
+
+ "sourcecraft.dev/bigbes/sr-ht-ecore/assets"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/csrf"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/middleware"
)
// Handler returns a router with everything this package needs already
-// installed: panic recovery and the authn principal middleware, then the
-// routes. The daemon mounts it at "/".
+// installed: panic recovery, the private cache policy, the authn principal
+// middleware and the same-origin guard, then the routes. The daemon mounts it
+// at "/".
+//
+// The order is the one the shared packages ask for. RecoverPanics is outermost
+// so it covers the later middleware as well as the handlers, and it is
+// sr-ht-ecore's rather than chi's or our own for one behaviour: a panic that
+// arrives *after* the response has started aborts the connection instead of
+// appending an error page to a truncated one. PrivateCache sits inside it so
+// that every answer — including the two refusals below — carries the same
+// private, no-store a page rendered behind a login cookie needs.
+//
+// csrf.Require goes last of the four, and on the router rather than on the
+// routes, which is the whole point of the change: the guard used to be a
+// predicate that three handlers remembered to call, so a form added later went
+// out unprotected by default. Here it covers the routes that are not written
+// yet, and it runs before routing — a mutation aimed at an address this surface
+// does not serve is refused rather than 404'd, which is the right way round,
+// since an unrouted POST answering differently from a routed one would be a way
+// to enumerate them without ever passing the check. It sits after the resolver
+// so the refusal page names the viewer the way every other page does.
//
// A caller that owns its own middleware stack — and has already applied
-// authn.Resolver.Middleware to it — uses Register instead. Installing the
-// principal middleware twice is harmless but pointless: it is idempotent.
+// authn.Resolver.Middleware to it — uses Register instead, and owes its router
+// this guard: Register installs no middleware of its own.
func (s *Server) Handler() http.Handler {
r := chi.NewRouter()
- r.Use(middleware.Recoverer)
+ r.Use(middleware.RecoverPanics(func(w http.ResponseWriter, r *http.Request, _ any) {
+ s.renderError(w, r, http.StatusInternalServerError, "")
+ }))
+ r.Use(middleware.PrivateCache)
r.Use(s.resolver.Middleware())
+ r.Use(csrf.Require(s.chromeSvc.SelfOrigin(), func(w http.ResponseWriter, r *http.Request) {
+ s.renderError(w, r, http.StatusForbidden, csrf.Message)
+ }))
s.Register(r)
return r
}
// Register mounts every spec.sr.ht read-plane route onto r. It installs no
// middleware of its own; the router it is handed must already resolve a
-// principal into the request context (authn.Resolver.Middleware), or every
-// viewer looks anonymous.
+// principal into the request context (authn.Resolver.Middleware) and must
+// already carry csrf.Require, or every viewer looks anonymous and every form is
+// forgeable. Handler does both.
//
// The document route is a single wildcard because the format selector lives in
// the *extension* and the document's address does not have one: ".md" and
@@ -38,7 +66,7 @@ // "notes/2026.json" still means "the JSON of notes/2026".
func (s *Server) Register(r chi.Router) {
r.Get("/", s.handleIndex)
r.Get("/healthz", s.handleHealthz)
- r.Get("/static/*", s.handleStatic)
+ r.Mount(assets.DefaultPrefix, s.static)
r.Get("/search", s.handleSearch)
r.Get("/inbox", s.handleInbox)
r.Post("/inbox/seen", s.handleInboxSeen)
@@ -68,19 +96,6 @@ // handleHealthz is a dependency-free liveness probe.
func (s *Server) handleHealthz(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
_, _ = w.Write([]byte("ok\n"))
-}
-
-// handleStatic serves the embedded assets, tagging the content-addressed
-// stylesheet as immutable: its name changes whenever its bytes do, so a browser
-// may keep it forever and a deploy still busts the cache.
-func (s *Server) handleStatic(w http.ResponseWriter, r *http.Request) {
- name := path.Base(r.URL.Path)
- if hashedCSSRe.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)
}
// unescapePath decodes a chi wildcard back into a tree path.
diff --git a/web/server.go b/web/server.go
index 52d52208c3dec045f3961f38bacbb3a8b1955769..3676829236a629199116ce70a705ccd5af4476ad 100644
--- a/web/server.go
+++ b/web/server.go
@@ -3,7 +3,7 @@ // tree, a rendered document with its metadata and backlinks, the proposal
// review page and keyword search — all served from one chi router the daemon
// mounts.
//
-// # The chrome is not ours
+// # The web tier is not ours
//
// The brand, the service switcher, the login block and the environment banner
// come from sourcecraft.dev/bigbes/sr-ht-ecore/chrome, which every custom
@@ -14,6 +14,25 @@ // here rebuilds the switcher or re-derives a login URL: this service's copy of
// that code — inherited from compare.sr.ht, which had inherited it from
// somewhere else — is what ecore exists to have deleted.
//
+// Four more of ecore's packages carry what used to be local copies of the same
+// idea, and the pattern is the same every time — the rule lives in one place
+// and this package supplies only what is genuinely spec.sr.ht's:
+//
+// - pages discovers the page templates, refuses at startup a page that
+// defines no "content", renders into a buffer before touching the response
+// and ships the shared error body. What stays here is renderError, which
+// wraps that body in this service's view struct, and fail, which maps this
+// service's own sentinels onto statuses.
+// - assets finds the hashed stylesheet and serves the static tree with the
+// cache policy each name implies.
+// - csrf is the same-origin guard, installed on the router rather than called
+// by three handlers — see [Server.Handler].
+// - middleware is the private-cache policy and the panic guard.
+//
+// pages.Render answers the response itself and returns an error only for the
+// log. It must never be handed to fail: that would either write a second
+// response over a committed one or recurse through the page that just broke.
+//
// # URL grammar
//
// The design pins this, so it is spelled out here rather than left to the
@@ -48,7 +67,8 @@ //
// [Server.Handler] returns a router with everything this package needs already
// installed, so the daemon can mount it at "/". A caller that owns its own
// router and middleware stack uses [Server.Register] instead; it installs
-// routes only, and assumes authn.Resolver.Middleware is already applied.
+// routes only, and assumes both authn.Resolver.Middleware and csrf.Require are
+// already applied — the second is a security rule, not a convenience.
//
// # Assets are embedded
//
@@ -65,20 +85,16 @@ "fmt"
"io/fs"
"log"
"net/http"
- "path"
- "regexp"
"github.com/vaughan0/go-ini"
"sourcecraft.dev/bigbes/sr-ht-core/config"
+ "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-spec/authn"
"sourcecraft.dev/bigbes/sr-ht-spec/doc"
)
-
-// 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$`)
// tokensSection is tokens.sr.ht's config section, spelled the way the instance's
// config.ini spells it. service.TokensSection is the same string read for the
@@ -119,11 +135,17 @@
// chromeSvc is the shared page frame of sr-ht-ecore: the brand, the service
// switcher, the login block and the environment banner, built once from
// config.ini and asked for a per-request Page in view (view.go). It is also
- // this package's only reader of our own and meta's origins — sameOrigin and
- // the login redirect ask it rather than keeping a second copy that could
+ // this package's only reader of our own and meta's origins — the CSRF guard
+ // and the login redirect ask it rather than keeping a second copy that could
// disagree with the links on the page.
chromeSvc *chrome.Service
+ // pages is the page set of sr-ht-ecore: one template set per file in
+ // templates/, discovered at startup. Adding a page is adding a file — there
+ // is no list here to forget to edit — and a page that defines no "content"
+ // fails New rather than serving the chrome around a hole.
+ pages pages.Set
+
// tokensOrigin is [tokens.sr.ht] origin in its *external* form. The only
// thing this package does with it is redirect a browser there, and a browser
// cannot reach the internal origin the bearer validator uses. Empty when the
@@ -131,7 +153,9 @@ // instance config has no such section, which handleTokens answers rather
// than papers over with a redirect to nowhere.
tokensOrigin string
- staticFileServer http.Handler
+ // static serves the embedded asset tree with the cache policy each name
+ // implies — see sr-ht-ecore/assets.
+ static http.Handler
}
// New assembles a Server from the shared SourceHut config.
@@ -167,9 +191,12 @@ if chromeSvc.MetaOrigin() == "" {
return nil, fmt.Errorf("web: [meta.sr.ht] origin is required")
}
- cssHref, err := resolveCSSHref()
+ // The error is a malformed glob — a mistake in this line — and not a missing
+ // stylesheet, which resolves to "" and is a warning: a service that will not
+ // boot without a build artefact cannot be run from a checkout.
+ cssHref, err := assets.Resolve(staticFS, "static/main.min.*.css", assets.DefaultPrefix)
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("web: %w", err)
}
if cssHref == "" {
log.Printf("web: no main.min.*.css embedded in this binary — pages will " +
@@ -179,36 +206,34 @@ // chrome.Page renders a bare page for an empty StyleHref rather than an
// empty , so an unstyled build stays a presentation failure.
chromeSvc.StyleHref = cssHref
+ // A page that defines no "content" is refused here rather than serving a
+ // 200 around a hole, so this error is a startup failure and not a warning.
+ set, err := pages.Load(tmplFS, pages.Options{Funcs: funcMap})
+ if err != nil {
+ return nil, fmt.Errorf("web: %w", err)
+ }
+
staticSub, err := fs.Sub(staticFS, "static")
if err != nil {
return nil, fmt.Errorf("web: sub static FS: %w", err)
}
- return &Server{
- reader: opts.Reader,
- searcher: opts.Searcher,
- resolver: opts.Resolver,
- renderer: doc.NewRenderer(),
- chromeSvc: chromeSvc,
- tokensOrigin: config.GetOrigin(opts.Conf, tokensSection, true),
- staticFileServer: http.StripPrefix("/static/", http.FileServer(http.FS(staticSub))),
- }, nil
-}
-
-// resolveCSSHref globs the embedded static FS for the content-addressed
-// stylesheet and returns its site-absolute URL, or "" when the binary was built
-// without one.
-//
-// Absence is reported rather than substituted: there is no placeholder href to
-// invent, and a link to a stylesheet that is not there would 404 on every page
-// load instead of saying what is wrong once, at startup.
-func resolveCSSHref() (string, error) {
- matches, err := fs.Glob(staticFS, "static/main.min.*.css")
- if err != nil {
- return "", fmt.Errorf("web: glob stylesheet: %w", err)
+ s := &Server{
+ reader: opts.Reader,
+ searcher: opts.Searcher,
+ resolver: opts.Resolver,
+ renderer: doc.NewRenderer(),
+ chromeSvc: chromeSvc,
+ pages: set,
+ tokensOrigin: config.GetOrigin(opts.Conf, tokensSection, true),
}
- if len(matches) == 0 {
- return "", nil
- }
- return "/static/" + path.Base(matches[0]), nil
+ // The 404 of the asset tree is this service's own page and not net/http's
+ // plaintext one: an asset URL typed by hand is a dead end without a nav to
+ // get out of. It is wired after the Server exists because it renders through
+ // it.
+ s.static = assets.Handler(staticSub, assets.DefaultPrefix,
+ http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ s.renderError(w, r, http.StatusNotFound, "")
+ }))
+ return s, nil
}
diff --git a/web/templates.go b/web/templates.go
index 1e9bff9b572eca55afeeff4dc4926a3c93d5d922..9ef8bbd911d06ea4bad7d8eeb4d5e4fa2434c956 100644
--- a/web/templates.go
+++ b/web/templates.go
@@ -1,7 +1,6 @@
package web
import (
- "bytes"
"embed"
"html/template"
"log"
@@ -9,11 +8,17 @@ "net/http"
"strings"
"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" defines do not
-// collide across pages.
+// tmplFS holds the page templates. sr-ht-ecore's pages.Load discovers them:
+// layout.html is the chrome every page is executed through, a file whose name
+// starts with "_" is a partial parsed into every set, and everything else is a
+// page that must define "content".
+//
+// The glob and not the bare directory, because Go's embed excludes names
+// starting with '_' when it walks a directory and _threads.html is exactly such
+// a name.
//
//go:embed templates/*.html
var tmplFS embed.FS
@@ -25,106 +30,59 @@ //
//go:embed static
var staticFS embed.FS
-// funcMap holds the template helpers available to every page.
+// funcMap holds this service's own template helpers.
//
-// It starts from chrome.Funcs — the generic helpers every custom service on
-// this instance was carrying its own copy of, `shortsha` among them — and adds
-// this service's own on top, after, so that a name may be shadowed
+// It carries only what is ours: pages.Load merges it over chrome.Funcs — the
+// generic helpers every custom service on this instance was carrying its own
+// copy of, `shortsha` among them — in that order, so a name may be shadowed
// deliberately rather than by accident of map ordering. Nothing shadows one
// today, and a helper that diverged from the shared spelling of the same name
// would be the drift ecore exists to prevent.
-var funcMap = func() template.FuncMap {
- m := chrome.Funcs()
-
+var funcMap = template.FuncMap{
// indent renders a tree depth as non-breaking space, so the space view's
// hierarchy reads as a hierarchy without a nested-list template recursion.
// A negative depth (a level-1 heading, once decremented) indents nothing.
- m["indent"] = func(depth int) template.HTML {
+ "indent": func(depth int) template.HTML {
if depth <= 0 {
return ""
}
return template.HTML(strings.Repeat(" ", depth))
- }
+ },
// dec turns a 1-based heading level into a 0-based indent depth.
- m["dec"] = func(n int) int { return n - 1 }
+ "dec": func(n int) int { return n - 1 },
+}
- return m
-}()
-
-// pageNames are the content templates; each is parsed with layout.html.
-var pageNames = []string{"index", "space", "document", "search", "error", "proposal", "inbox"}
-
-// pages maps a page name to its parsed template set (layout + the shared
-// chrome partials + local partials + that page). threads.html is parsed into
-// every set rather than only into the proposal page's: it defines review-thread
-// markup and nothing else, and a partial that only some sets know about is a
-// lookup that fails on the page that later needs it. ecore's "srht-nav" and
-// "srht-env-banner" are attached to every set for the same reason, and through
-// MustAttach because a set that cannot draw the chrome is not a page this
-// binary should start serving.
-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/threads.html", "templates/"+name+".html"))
- m[name] = t
- }
- return m
-}()
-
-// blockThreadsTmpl is the per-block comment markup, taken out of the proposal
-// page's own set so the diff renderer — which builds its HTML in Go and cannot
-// reach a page template through the usual {{template}} call — and the page
-// itself cannot drift into two spellings of a thread.
+// blockThreadsTmpl is the per-block comment markup, parsed on its own so the
+// diff renderer — which builds its HTML in Go and cannot reach a page template
+// through the usual {{template}} call — and the proposal page cannot drift into
+// two spellings of a thread. It is the same file the page set parses as a
+// partial, so there is one spelling and not two.
//
// A missing define is a build-time mistake in this package, so it panics at
// init the way template.Must does, rather than yielding a page with the
// comments silently absent.
var blockThreadsTmpl = func() *template.Template {
- t := pages["proposal"].Lookup("blockthreads")
- if t == nil {
- panic(`web: templates/threads.html does not define "blockthreads"`)
+ t := template.Must(template.New("_threads.html").
+ Funcs(chrome.Funcs()).Funcs(funcMap).
+ ParseFS(tmplFS, "templates/_threads.html"))
+ blocks := t.Lookup("blockthreads")
+ if blocks == nil {
+ panic(`web: templates/_threads.html does not define "blockthreads"`)
}
- return t
+ return blocks
}()
-// 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.
-func (s *Server) render(w http.ResponseWriter, status int, page string, vd viewData) {
- t, ok := pages[page]
- if !ok {
- log.Printf("web: unknown template page %q", page)
- http.Error(w, "internal server error", http.StatusInternalServerError)
- return
- }
- var buf bytes.Buffer
- if err := t.ExecuteTemplate(&buf, "layout.html", vd); err != nil {
- log.Printf("web: executing template %q: %v", page, err)
- http.Error(w, "internal server error", http.StatusInternalServerError)
- return
- }
- 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: this service's view struct
+// around ecore's shared error body.
+//
+// An empty message takes the standard sentence for the status, so a refusal
+// with nothing of its own to add says what every other service on this instance
+// says. It never recurses on failure — pages.Render answers the response itself
+// and hands back only a line for the log.
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,
+ vd.Data = pages.Error(status, message)
+ if err := s.pages.Render(w, status, pages.ErrorPage, vd); err != nil {
+ log.Printf("web: render the %d page for %s %s: %v", status, r.Method, r.URL.Path, err)
}
- s.render(w, status, "error", vd)
}
diff --git a/web/templates/error.html b/web/templates/error.html
deleted file mode 100644
index 52baa57535cda3a941aa121a6ea817cc23b11aed..0000000000000000000000000000000000000000
--- a/web/templates/error.html
+++ /dev/null
@@ -1,9 +0,0 @@
-{{define "content"}}
-
-{{end}}
diff --git a/web/templates/threads.html b/web/templates/_threads.html
rename from web/templates/threads.html
rename to web/templates/_threads.html
diff --git a/web/tokens_test.go b/web/tokens_test.go
index 15f9ac975fc2f4d48147418a3400fcffbcb991db..706de7636f9036a22effad4ac907825df4c16dd3 100644
--- a/web/tokens_test.go
+++ b/web/tokens_test.go
@@ -7,7 +7,10 @@ "testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
- "github.com/vaughan0/go-ini"
+
+ "sourcecraft.dev/bigbes/sr-ht-ecore/ecoretest"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/authn"
)
// /tokens is a signpost now. spec.sr.ht mints no credential of its own, so the
@@ -24,14 +27,16 @@ rec := get(t, h, "/tokens", user)
assert.Equal(t, http.StatusSeeOther, rec.Code, "body: %s", rec.Body)
// The external origin, because this is for a browser, and the page
// SPEC ch. 7 puts the token UI on.
- assert.Equal(t, "https://tokens.example/tokens", rec.Header().Get("Location"))
+ assert.Equal(t, ecoretest.Origin("tokens.sr.ht")+"/tokens", rec.Header().Get("Location"))
})
}
}
// The POST routes went with the table behind them: nothing here mints or
-// revokes any more, and a form posted at the old address must not 404 into
-// something that looks like it might have worked.
+// revokes any more, and a form posted at the old address must not answer as
+// though it might have worked. It is now csrf.Require that refuses them — the
+// guard runs before routing, so a POST to an address this surface does not
+// serve is refused rather than 404'd.
func TestTokensAcceptsNoWrites(t *testing.T) {
h, _ := testServer(t)
@@ -51,15 +56,7 @@ // An instance with no [tokens.sr.ht] section has nowhere to send anybody, and
// says so instead of redirecting to a URL built out of an empty string.
func TestTokensWithoutTheSectionSaysSo(t *testing.T) {
srv, err := New(Options{
- Conf: ini.File{
- "sr.ht": ini.Section{
- "network-key": testConf.Section("sr.ht")["network-key"],
- "owner-name": "bigbes",
- },
- "webhooks": ini.Section{"private-key": testConf.Section("webhooks")["private-key"]},
- "spec.sr.ht": ini.Section{"origin": "https://spec.example"},
- "meta.sr.ht": ini.Section{"origin": "https://meta.example"},
- },
+ Conf: ecoretest.Config(authn.ConfigSection, ecoretest.Delete("tokens.sr.ht")),
Reader: newFakeReader(),
Searcher: &fakeSearcher{},
Resolver: testResolver(t),
diff --git a/web/view.go b/web/view.go
index 6943632da35543b172e8107d1498a39bbf610907..946ec41615ec32fbe6fa4b565b2f2e567d44814a 100644
--- a/web/view.go
+++ b/web/view.go
@@ -43,10 +43,12 @@ // loginRedirect sends a viewer with no read authority to meta.sr.ht's login,
// with return_to pointing back at what they asked for. There is no login flow
// of our own — identity is the shared unified-login cookie and nothing else.
//
-// The URL comes from a throwaway chrome.Page rather than from a second
-// hand-rolled concatenation of the meta origin and an escaped return_to: the
-// link in the nav and the redirect a gate issues must be the same URL, and the
-// cheapest way to guarantee that is to have exactly one place that builds it.
+// The URL comes from the chrome rather than from a second hand-rolled
+// concatenation of the meta origin and an escaped return_to: the link in the
+// nav and the redirect a gate issues must be the same URL, and the cheapest way
+// to guarantee that is to have exactly one place that builds it. LoginURLFor is
+// that place — a redirect wants the one field, not a whole page built to be
+// thrown away.
func (s *Server) loginRedirect(w http.ResponseWriter, r *http.Request) {
- http.Redirect(w, r, s.chromeSvc.Page(r, "", "").LoginURL, http.StatusFound)
+ http.Redirect(w, r, s.chromeSvc.LoginURLFor(r), http.StatusFound)
}
diff --git a/web/web_test.go b/web/web_test.go
index 7328699c77286b07f6f1776f68e56261bb308e47..33107cb8c3fc1220a4bd694cb8ee186606566a23 100644
--- a/web/web_test.go
+++ b/web/web_test.go
@@ -2,9 +2,7 @@ package web
import (
"context"
- "crypto/rand"
"crypto/sha1"
- "encoding/base64"
"encoding/hex"
"encoding/json"
"fmt"
@@ -16,13 +14,13 @@ "strings"
"testing"
"time"
- "github.com/fernet/fernet-go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
- "github.com/vaughan0/go-ini"
"sourcecraft.dev/bigbes/sr-ht-core/auth"
"sourcecraft.dev/bigbes/sr-ht-core/crypto"
"sourcecraft.dev/bigbes/sr-ht-ecore/bearer"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/csrf"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/ecoretest"
"sourcecraft.dev/bigbes/sr-ht-spec/authn"
"sourcecraft.dev/bigbes/sr-ht-spec/core"
@@ -30,24 +28,13 @@ "sourcecraft.dev/bigbes/sr-ht-spec/search"
"sourcecraft.dev/bigbes/sr-ht-spec/service"
)
-// testConf carries the crypto keys established in TestMain so tests can seal
-// unified-login cookies the way meta.sr.ht does.
-var testConf ini.File
-
+// TestMain installs sr-ht-ecore's fixed test keyset, which is what lets these
+// tests seal a unified-login cookie the way meta.sr.ht does with no meta and no
+// network. The keys are constants there, so it is idempotent: two packages of
+// this service can both call it without the second rotating what the first
+// sealed with.
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()
// The agent credential is a signed tokens.sr.ht working token now, so it
// cannot be a constant: it is minted here, once the signing key exists.
agentTk = agentToken("spec:read")
@@ -467,28 +454,12 @@ // testServerWith is testServer with a caller-supplied reader, so the review-page
// tests can seed proposals into it and still get the same middleware stack.
func testServerWith(t *testing.T, reader *fakeReader) (http.Handler, *fakeReader, *fakeSearcher) {
t.Helper()
- conf := ini.File{
- "sr.ht": ini.Section{
- "network-key": testConf.Section("sr.ht")["network-key"],
- "site-name": "sourcehut",
- "environment": "development",
- "owner-name": "bigbes",
- },
- "webhooks": ini.Section{"private-key": testConf.Section("webhooks")["private-key"]},
- "spec.sr.ht": ini.Section{"origin": "https://spec.example"},
- "meta.sr.ht": ini.Section{"origin": "https://meta.example"},
- "git.sr.ht": ini.Section{"origin": "https://git.example"},
- // The rest of the instance's services. Which of them the switcher shows
- // and in what order is sr-ht-ecore's decision and is tested there; they
- // are here so that these pages render against a realistic config.
- "todo.sr.ht": ini.Section{"origin": "https://todo.example"},
- "paste.sr.ht": ini.Section{"origin": "https://paste.example"},
- "pages.sr.ht": ini.Section{"origin": "https://pages.example"},
- "hub.sr.ht": ini.Section{"origin": "https://hub.example"},
- // /tokens redirects here, and the external origin is the one a browser
- // can reach.
- "tokens.sr.ht": ini.Section{"origin": "https://tokens.example"},
- }
+ // The synthetic instance config of sr-ht-ecore: this service's own section
+ // with an origin, meta's, tokens' (which /tokens redirects to), and the rest
+ // of the instance so that these pages render against a realistic switcher.
+ // Which entries the switcher shows and in what order is ecore's decision and
+ // is tested there.
+ conf := ecoretest.Config(authn.ConfigSection)
resolver := testResolver(t)
searcher := &fakeSearcher{}
srv, err := New(Options{
@@ -893,31 +864,43 @@ t.Fatalf("healthz = %d %q", rec.Code, rec.Body.String())
}
}
+// The same-origin guard is router-wide (Handler) rather than a line three
+// handlers remember, and this is what that buys: it runs before routing, so a
+// mutation aimed at an address this surface does not serve is refused too, and
+// a form added tomorrow is protected by having been registered.
+func TestTheGuardCoversRoutesThisSurfaceDoesNotServe(t *testing.T) {
+ h, _ := testServer(t)
+
+ rec := post(t, h, "/no/such/route", "bigbes", "")
+ assert.Equal(t, http.StatusForbidden, rec.Code)
+ assert.Contains(t, rec.Body.String(), csrf.Message)
+
+ // With this site's own Origin the guard passes and the router answers for
+ // itself — the refusal above is the guard's and not the router's.
+ rec = post(t, h, "/no/such/route", "bigbes", ecoretest.Origin(authn.ConfigSection))
+ assert.Equal(t, http.StatusNotFound, rec.Code)
+}
+
+// The asset tree is mounted and served publicly. Which lifetime a name earns is
+// sr-ht-ecore/assets' rule and is tested there; what this asserts is that this
+// service's static mount reaches the embedded tree at all, and that the page
+// policy (private, no-store) does not follow an asset out.
func TestStaticLogoIsServed(t *testing.T) {
h, _ := testServer(t)
rec := get(t, h, "/static/logo.svg", "")
- if rec.Code != http.StatusOK {
- t.Fatalf("status = %d", rec.Code)
- }
- if cc := rec.Header().Get("Cache-Control"); !strings.Contains(cc, "max-age") {
- t.Fatalf("cache-control = %q", cc)
- }
+ assert.Equal(t, http.StatusOK, rec.Code)
+ assert.Contains(t, rec.Header().Get("Cache-Control"), "max-age")
+ assert.Empty(t, rec.Header().Get("Vary"), "an asset is the same bytes for everybody")
}
-// TestHashedCSSIsImmutable checks the cache policy without depending on a built
-// stylesheet: `make css` needs sassc and the shared sourcehut partials, neither
-// of which a test may assume.
-func TestHashedCSSIsImmutable(t *testing.T) {
- for _, name := range []string{"main.min.79713f25.css", "main.min.abc123.css"} {
- if !hashedCSSRe.MatchString(name) {
- t.Fatalf("%s should be recognised as a hashed stylesheet", name)
- }
- }
- for _, name := range []string{"main.css", "main.min.css", "logo.svg"} {
- if hashedCSSRe.MatchString(name) {
- t.Fatalf("%s should not be recognised as a hashed stylesheet", name)
- }
- }
+// An asset name that is not in the tree gets this service's own 404 page rather
+// than net/http's plaintext one — a URL typed by hand is a dead end without a
+// nav to get out of.
+func TestMissingAssetGetsTheChromePage(t *testing.T) {
+ h, _ := testServer(t)
+ rec := get(t, h, "/static/nope.css", "")
+ assert.Equal(t, http.StatusNotFound, rec.Code)
+ assert.Contains(t, rec.Body.String(), "navbar-brand", "the 404 carries the chrome")
}
// ---- unit-level grammar ---------------------------------------------------