amadan

oficina / sheets Public

Keep Sheets' inline styles under rastrillo's new default CSP

rastrillo fb806a2's default CSP drops 'unsafe-inline' from style-src
(style-src 'self' 'unsafe-hashes' plus pow's honeypot hash). Standalone
Sheets served under it lost document presentation, as Astra found on #46:
- table cells lost their fill and text colours, which are custom
  properties on the cell's style attribute;
- the Format palettes' swatches lost their colours;
- the table, history and link views stopped scrolling sideways.
Home mode was masked: connect's FormPolicy keeps 'unsafe-inline'.

This keeps the old style policy explicitly rather than migrating. The
cell colours are per-document values, so they cannot be hashed or moved
into sheets.css. The vendored Jspreadsheet and jsuites write style
attributes of their own.
- sheets.ContentSecurityPolicy is rastrillo's default with style-src
  'self' 'unsafe-inline' and no other change.
- App.ServeOptions sets it, together with the router and error page.
  main now uses ServeOptions, and so do the tests.

Tests, all served through rastrillo.Handler, the production stack:
- TestDocumentStylesSurviveTheProductionCSP (browser): computed fill,
  ink, wrapper overflow and swatch --sheet-h in Chromium, standalone.
  It fails all four checks without the policy. make test-browser now
  runs it.
- TestProductionCSPIsTheDefaultBarStyles: every other directive equals
  rastrillo's default, read off a bare handler. style-src carries no
  hash or nonce, since either would void 'unsafe-inline'.
- TestStandaloneServesInlineStyles and TestTheConfirmDoorKeepsEveryPolicy:
  the confirm door carries both CSP headers, ours and auth's
  frame-ancestors 'none'. Both fail without the policy.
- TestHomeModeServesInlineStyles: connect's policy in Home mode.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 67c08dfc81b72636519d58af80cec5db24c9b2ed parent 5e7e873
5 files changed, +296 −12
  • Makefile +4 −2
  • cmd/sheets/main.go +2 −10
  • internal/sheets/serving.go +54 −0
  • internal/sheets/serving_browser_test.go +91 −0
  • internal/sheets/serving_test.go +145 −0

CI passed — run details

CI log 
amadan: .amadan/timeout: this job's budget is 15m0s (this runner allows up to 30m0s)
=== vet ===
CGO_ENABLED=0 go vet ./...
CGO_ENABLED=0 go vet -tags browser ./internal/sheets/
=== fmt ===
=== build ===
CGO_ENABLED=0 go build ./...
=== migrations ===
go tool rastrillo migration check
rastrillo migration check: models and migrations agree
=== go-test ===
CGO_ENABLED=0 go test -count=1 ./...
?   	amadan.net/oficina/sheets/cmd/sheets	[no test files]
ok  	amadan.net/oficina/sheets/internal/guardstest	0.144s
ok  	amadan.net/oficina/sheets/internal/sheets	5.403s
ok  	amadan.net/oficina/sheets/internal/sheets/calc	0.009s
ok  	amadan.net/oficina/sheets/internal/sheets/format	0.002s
?   	amadan.net/oficina/sheets/internal/sheets/keymailtest	[no test files]
ok  	amadan.net/oficina/sheets/internal/sheets/live	0.054s
ok  	amadan.net/oficina/sheets/internal/sheets/seal	0.002s
ok  	amadan.net/oficina/sheets/internal/sheets/sealtest	0.002s
ok  	amadan.net/oficina/sheets/internal/sheets/store	4.077s
ok  	amadan.net/oficina/sheets/internal/sheets/xlsx	0.384s
?   	amadan.net/oficina/sheets/tools/gencorpus	[no test files]
=== race ===
go test -race -count=1 ./internal/sheets/store/... ./internal/sheets/live/...
ok  	amadan.net/oficina/sheets/internal/sheets/store	49.200s
ok  	amadan.net/oficina/sheets/internal/sheets/live	1.063s
diff --git a/Makefile b/Makefile
index b55bd98..3a1b696 100644
--- a/Makefile
+++ b/Makefile
@@ -127,6 +127,8 @@ test-race:
# Chromium: on PATH, via RASTRILLO_CHROME, or in a Playwright cache.
# Deliberately NOT part of `test`: the gate CI runs on every push must
# not need a browser. Run it before shipping a change to the CSS or to
-# rastrillo's pin — TestNoZoomOnTap is the iOS zoom-on-tap check.
+# rastrillo's pin — TestNoZoomOnTap is the iOS zoom-on-tap check, and
+# TestDocumentStylesSurviveTheProductionCSP is the one browser check served
+# through rastrillo's production stack, so it sees the real CSP.
test-browser:
- CGO_ENABLED=0 go test -tags browser -count=1 -run 'OnTap' ./internal/sheets/
+ CGO_ENABLED=0 go test -tags browser -count=1 -run 'OnTap|DocumentStyles' ./internal/sheets/
diff --git a/cmd/sheets/main.go b/cmd/sheets/main.go
index 8aefff4..7f5488d 100644
--- a/cmd/sheets/main.go
+++ b/cmd/sheets/main.go
@@ -25,7 +25,6 @@ package main
import (
"context"
"log/slog"
- "net/http"
"os"
"amadan.net/rastrillo/rastrillo"
@@ -112,15 +111,8 @@ func main() {
os.Exit(1)
}
- // rastrillo.Options.Mux wants a *http.ServeMux; the app's own
- // router is a chi.Router (an http.Handler), so it mounts under one.
- mux := http.NewServeMux()
- mux.Handle("/", app.Router())
- opts.Mux = mux
- // A panic outside the router's signed-in group (where the app's own
- // recoverer renders the viewer's chrome) is the same error page, not
- // rastrillo's plain text (spec §3.3).
- opts.ErrorPage = app.ErrorPage
+ // The router, the error page and the CSP (sheets.ServeOptions).
+ opts = app.ServeOptions(opts)
// The app opened its own database handle above (db.Open already
// Pings eagerly, which satisfies Serve's boot-materialization duty
diff --git a/internal/sheets/serving.go b/internal/sheets/serving.go
new file mode 100644
index 0000000..6b5eb35
--- /dev/null
+++ b/internal/sheets/serving.go
@@ -0,0 +1,54 @@
+package sheets
+
+import (
+ "net/http"
+
+ "amadan.net/rastrillo/rastrillo"
+)
+
+// ContentSecurityPolicy is the policy Sheets serves under standalone.
+// It is rastrillo's default policy with one change: style-src keeps
+// 'unsafe-inline', which was rastrillo's own default until the bump to
+// fb806a2.
+//
+// The new default (style-src 'self' 'unsafe-hashes' plus pow's honeypot
+// hash) refuses every style attribute, and Sheets' documents are drawn
+// with them:
+//
+// - the scriptless table and public-link views paint each cell's fill
+// and text colour as custom properties on the cell (table.html,
+// tablepaint.go). The colours are the document's own, unbounded, so
+// they cannot be moved into sheets.css, and a per-document
+// stylesheet would be a new route and cache for every view;
+// - the Format menu's fill and text palettes draw each swatch's
+// colour the same way (sheet.html);
+// - the vendored Jspreadsheet and jsuites write style attributes of
+// their own (setAttribute("style", …) and style="…" in markup),
+// which this app does not patch: it is vendored as shipped.
+//
+// Hashing them is not possible (the values are per document), and the
+// other directives are unchanged, so scripts stay same-origin only and
+// framing, base and form targets stay locked. Home mode serves
+// connect.FormPolicy, which carries the same style-src.
+// TestProductionCSPIsTheDefaultBarStyles keeps every other directive in
+// step with rastrillo's default.
+const ContentSecurityPolicy = "default-src 'self'; style-src 'self' 'unsafe-inline'; " +
+ "img-src 'self' data:; frame-ancestors 'none'; base-uri 'self'; form-action 'self'"
+
+// ServeOptions completes opts with what Sheets serves: its router, its
+// error page and its CSP. main passes the result to rastrillo.Serve,
+// and the CSP tests pass it to rastrillo.Handler, so the policy they
+// check is the one production runs under.
+func (a *App) ServeOptions(opts rastrillo.Options) rastrillo.Options {
+ // rastrillo.Options.Mux wants a *http.ServeMux; the app's own
+ // router is a chi.Router (an http.Handler), so it mounts under one.
+ mux := http.NewServeMux()
+ mux.Handle("/", a.Router())
+ opts.Mux = mux
+ // A panic outside the router's signed-in group (where the app's own
+ // recoverer renders the viewer's chrome) is the same error page, not
+ // rastrillo's plain text (spec §3.3).
+ opts.ErrorPage = a.ErrorPage
+ opts.CSP = ContentSecurityPolicy
+ return opts
+}
diff --git a/internal/sheets/serving_browser_test.go b/internal/sheets/serving_browser_test.go
new file mode 100644
index 0000000..43ab172
--- /dev/null
+++ b/internal/sheets/serving_browser_test.go
@@ -0,0 +1,91 @@
+//go:build browser
+
+package sheets
+
+import (
+ "context"
+ "fmt"
+ "net/http"
+ "testing"
+ "time"
+
+ "amadan.net/rastrillo/rastrillo/harness"
+ "github.com/chromedp/cdproto/network"
+ "github.com/chromedp/chromedp"
+)
+
+// TestDocumentStylesSurviveTheProductionCSP: Sheets in Chromium behind
+// the production serving stack (productionHandler: rastrillo's security
+// headers and Sheets' CSP), standalone. A filled cell and an inked cell
+// in the scriptless table view paint differently from a plain one, the
+// table scrolls sideways in its wrapper, and the grid's Format palette
+// swatches carry their colours.
+//
+// Every one of those is a style attribute. Under rastrillo fb806a2's
+// default style-src the browser drops them all — the cell turns
+// transparent on black, the swatch loses --sheet-h, the wrapper stops
+// scrolling — while the other browser checks, which serve app.Router()
+// with no CSP at all, stay green (Astra on #46).
+func TestDocumentStylesSurviveTheProductionCSP(t *testing.T) {
+ app, cookies := signIn(t)
+ s, cells := newSheet(t, app, cookies, "Painted")
+ postCell(t, app, cookies, cells, `{"r":0,"c":0,"raw":"filled"}`)
+ postCell(t, app, cookies, cells, `{"r":0,"c":1,"raw":"inked"}`)
+ postCell(t, app, cookies, cells, `{"r":0,"c":2,"raw":"plain"}`)
+ postFormatForm(t, app, cookies, s, map[string]string{"r1": "0", "c1": "0", "r2": "0", "c2": "0", "fill": "red", "page": "1"})
+ postFormatForm(t, app, cookies, s, map[string]string{"r1": "0", "c1": "1", "r2": "0", "c2": "1", "ink": "blue", "page": "1"})
+
+ rig := harness.New(t, func(string) http.Handler { return productionHandler(t, app) })
+ run := func(what string, actions ...chromedp.Action) {
+ t.Helper()
+ ctx, cancel := context.WithTimeout(rig.Context(), 20*time.Second)
+ defer cancel()
+ if err := chromedp.Run(ctx, actions...); err != nil {
+ t.Fatalf("%s: %v", what, err)
+ }
+ }
+ var set []chromedp.Action
+ for _, c := range cookies {
+ set = append(set, network.SetCookie(c.Name, c.Value).WithURL(rig.Origin+"/").
+ WithHTTPOnly(c.HttpOnly).WithSecure(c.Secure).WithSameSite(network.CookieSameSiteLax))
+ }
+ run("sign in", set...)
+
+ var table struct {
+ FilledBg, PlainBg, InkedInk, PlainInk, Overflow string
+ }
+ run("table view",
+ chromedp.Navigate(fmt.Sprintf("%s/sheets/%d/table", rig.Origin, s.ID)),
+ chromedp.WaitVisible(`table[data-cell-table]`),
+ chromedp.Evaluate(`(() => {
+ const cell = (x) => [...document.querySelectorAll('table[data-cell-table] tbody td')].find(td => td.textContent.trim() === x);
+ const cs = (x) => getComputedStyle(cell(x));
+ return {
+ FilledBg: cs('filled').backgroundColor,
+ PlainBg: cs('plain').backgroundColor,
+ InkedInk: cs('inked').color,
+ PlainInk: cs('plain').color,
+ Overflow: getComputedStyle(document.querySelector('table[data-cell-table]').parentElement).overflowX,
+ };
+ })()`, &table),
+ )
+ if table.FilledBg == table.PlainBg {
+ t.Errorf("a red-filled cell paints %s, the same as a plain one: its style attribute was refused", table.FilledBg)
+ }
+ if table.InkedInk == table.PlainInk {
+ t.Errorf("a blue-inked cell paints %s text, the same as a plain one: its style attribute was refused", table.InkedInk)
+ }
+ if table.Overflow != "auto" {
+ t.Errorf("the table's wrapper has overflow-x %q, want auto: wide sheets cannot scroll", table.Overflow)
+ }
+
+ var swatch string
+ run("grid palette",
+ chromedp.Navigate(fmt.Sprintf("%s/sheets/%d", rig.Origin, s.ID)),
+ chromedp.WaitVisible(`td[data-x="0"][data-y="0"]`),
+ chromedp.Evaluate(`getComputedStyle(document.querySelector('[data-swatch][data-value="red"]')).getPropertyValue('--sheet-h').trim()`, &swatch),
+ )
+ if swatch == "" {
+ t.Error("the Fill palette's red swatch has no --sheet-h: its style attribute was refused")
+ }
+}
diff --git a/internal/sheets/serving_test.go b/internal/sheets/serving_test.go
new file mode 100644
index 0000000..946a7df
--- /dev/null
+++ b/internal/sheets/serving_test.go
@@ -0,0 +1,145 @@
+package sheets
+
+import (
+ "fmt"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+
+ "amadan.net/rastrillo/rastrillo"
+)
+
+// productionHandler is app behind rastrillo's serving stack, built from
+// the same ServeOptions main hands to rastrillo.Serve: security headers,
+// CSP and all. app.Router() alone carries no CSP, so a test that serves
+// only the router cannot see what a browser is told.
+func productionHandler(t *testing.T, app *App) http.Handler {
+ t.Helper()
+ h, closeFn, err := rastrillo.Handler(app.ServeOptions(rastrillo.Options{Logger: quietLogger()}))
+ if err != nil {
+ t.Fatalf("rastrillo.Handler: %v", err)
+ }
+ t.Cleanup(func() { _ = closeFn() })
+ return h
+}
+
+// directives splits a policy into directive name → source list.
+func directives(policy string) map[string]string {
+ out := map[string]string{}
+ for _, d := range strings.Split(policy, ";") {
+ name, value, _ := strings.Cut(strings.TrimSpace(d), " ")
+ if name != "" {
+ out[name] = strings.TrimSpace(value)
+ }
+ }
+ return out
+}
+
+// TestProductionCSPIsTheDefaultBarStyles: Sheets' policy is rastrillo's
+// default with style-src alone widened to 'unsafe-inline'
+// (ContentSecurityPolicy says why). Every other directive must match
+// the framework's, read off a handler built with no CSP of its own, so
+// a rastrillo bump that tightens anything else fails here rather than
+// being silently undone by our constant.
+func TestProductionCSPIsTheDefaultBarStyles(t *testing.T) {
+ bare, closeFn, err := rastrillo.Handler(rastrillo.Options{Mux: http.NewServeMux(), Logger: quietLogger()})
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer closeFn()
+ rec := httptest.NewRecorder()
+ bare.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/healthz", nil))
+ def := directives(rec.Header().Get("Content-Security-Policy"))
+ ours := directives(ContentSecurityPolicy)
+ if len(def) == 0 {
+ t.Fatal("rastrillo set no default CSP")
+ }
+ for name, value := range def {
+ if name == "style-src" {
+ continue
+ }
+ if ours[name] != value {
+ t.Errorf("%s: ours %q, rastrillo's default %q", name, ours[name], value)
+ }
+ }
+ for name := range ours {
+ if _, ok := def[name]; !ok {
+ t.Errorf("ours adds %s, which rastrillo's default does not have", name)
+ }
+ }
+ // A hash or nonce beside 'unsafe-inline' makes the browser ignore
+ // 'unsafe-inline' (CSP3), which is exactly how the styles broke.
+ if got := ours["style-src"]; got != "'self' 'unsafe-inline'" {
+ t.Errorf("style-src = %q, want 'self' 'unsafe-inline' and nothing else", got)
+ }
+}
+
+// TestStandaloneServesInlineStyles: through the production stack, a
+// formatted table page — whose cell colours are custom properties on a
+// style attribute — is served under a policy that lets that attribute
+// apply. On 5e7e873 the header was rastrillo's new default and the cell
+// lost its fill (Astra on #46).
+func TestStandaloneServesInlineStyles(t *testing.T) {
+ app, cookies := signIn(t)
+ s, cells := newSheet(t, app, cookies, "Painted")
+ postCell(t, app, cookies, cells, `{"r":0,"c":0,"raw":"heading"}`)
+ postFormatForm(t, app, cookies, s, map[string]string{
+ "r1": "0", "c1": "0", "r2": "0", "c2": "0", "fill": "green", "page": "1",
+ })
+ h := productionHandler(t, app)
+ req := httptest.NewRequest(http.MethodGet, fmt.Sprintf("/sheets/%d/table", s.ID), nil)
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, as(req, cookies))
+ if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `style="--sheet-h:`) {
+ t.Fatalf("table page = %d, or it paints no cell", rec.Code)
+ }
+ policies := rec.Header().Values("Content-Security-Policy")
+ if len(policies) != 1 || policies[0] != ContentSecurityPolicy {
+ t.Fatalf("CSP = %q, want exactly Sheets' policy", policies)
+ }
+}
+
+// TestTheConfirmDoorKeepsEveryPolicy: the confirm door through the
+// production stack carries Sheets' policy and auth's added
+// frame-ancestors 'none'. Browsers enforce every CSP header, so the
+// second one only narrows.
+func TestTheConfirmDoorKeepsEveryPolicy(t *testing.T) {
+ app, mail := appWithRecorder(t)
+ postSignin(t, app, signedInAddress)
+ link := mail.lastLink(t)
+ h := productionHandler(t, app)
+ action, _ := openLink(t, h, link)
+ if action != "/auth/verify" {
+ t.Fatalf("confirm action = %q", action)
+ }
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, strings.TrimPrefix(link, testOrigin), nil))
+ got := rec.Header().Values("Content-Security-Policy")
+ if len(got) != 2 || got[0] != ContentSecurityPolicy || got[1] != "frame-ancestors 'none'" {
+ t.Fatalf("confirm door CSP = %q, want Sheets' policy then frame-ancestors 'none'", got)
+ }
+ for header, want := range map[string]string{"X-Frame-Options": "DENY", "Referrer-Policy": "strict-origin", "Cache-Control": "no-store"} {
+ if v := rec.Header().Get(header); v != want {
+ t.Errorf("%s = %q, want %q", header, v, want)
+ }
+ }
+}
+
+// TestHomeModeServesInlineStyles: with Home configured, connect's
+// middleware replaces the policy with connect.FormPolicy. Through the
+// production stack its style-src must still admit the cells' style
+// attributes.
+func TestHomeModeServesInlineStyles(t *testing.T) {
+ f := newHomeFixture(t)
+ h := productionHandler(t, f.app)
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, testOrigin+"/signin/local", nil))
+ if rec.Code != http.StatusOK {
+ t.Fatalf("GET /signin/local = %d", rec.Code)
+ }
+ got := rec.Header().Values("Content-Security-Policy")
+ if len(got) != 1 || directives(got[0])["style-src"] != "'self' 'unsafe-inline'" {
+ t.Fatalf("Home-mode CSP = %q, want one policy whose style-src is 'self' 'unsafe-inline'", got)
+ }
+}