Skip to content

Commit dc76391

Browse files
server: let an operator frame the admin console, and nothing else
Second of the features split out of #30. FRAME_ANCESTORS is a space-separated list of origins allowed to embed /admin/ in a frame; when set, the admin routes send `Content-Security-Policy: frame-ancestors <list>`. Unset, nothing is sent and /admin/ behaves exactly as it does today. Three deliberate refusals, each of which is the interesting half: ⛔ Scoped to the admin SPA, and it must stay that way. The public booking pages set `frame-ancestors 'none'` plus `X-Frame-Options: DENY` in their own handlers and this never reaches them. They are unauthenticated pages that collect names, emails and card details, and clickjacking one is worth more to an attacker than framing a console nobody can open without a session. ⛔ An entry that is not `https://host[:port]` or `'self'` stops the process booting rather than being dropped. A browser discards a source list it cannot parse, so a typo would leave the admin UI MORE embeddable than leaving the setting unset — the one failure mode a validation error must not have. ⛔ No X-Frame-Options is sent beside the CSP. That header has no allow-list form (its ALLOW-FROM was implemented by one browser and is dead), so the only value it could carry is SAMEORIGIN, which every browser that reads it applies INSTEAD of honouring frame-ancestors — breaking the embedding this exists to enable. With the list empty the wrapped handler sends no frame header at all rather than a default deny, because adding one would be a behaviour change smuggled in on an opt-in setting. TestAdminSPA_sendsNoFrameHeadersWhenUnset pins that. Mutation-checked rather than assumed: with the Set call replaced by a discard, TestAdminSPA_frameAncestorsWhenConfigured and ...OnSPAFallback both fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 42f486d commit dc76391

9 files changed

Lines changed: 283 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,19 @@ exact tag (`ghcr.io/calnode/calnode:0.1.0`) if you need stability between upgrad
3939
pool could satisfy are all excluded, so the explanation never appears attached to the
4040
wrong cause. Three new/changed keys in all eight locales.
4141

42+
- **`FRAME_ANCESTORS`: embed the admin UI in your own console.** Space-separated origins
43+
(`https://console.example.com 'self'`); when set, `/admin/` sends
44+
`Content-Security-Policy: frame-ancestors <list>`. The public booking pages are
45+
untouched and still deny framing outright — this is about the console, not the pages
46+
that take card details.
47+
48+
Two deliberate refusals. An entry that is not `https://host[:port]` or `'self'` stops
49+
the app booting rather than being ignored, because a browser drops a source list it
50+
cannot parse, which would leave the admin UI *more* embeddable than the setting being
51+
unset. And no `X-Frame-Options` is sent beside it: that header has no allow-list form,
52+
so the only value it could carry is `SAMEORIGIN`, which browsers honour instead of the
53+
CSP and would break the embedding this exists for.
54+
4255
### Fixed
4356
- **Constraint violations are recognised by SQLite's error code rather than by its
4457
English message.** Thirteen call sites asked `strings.Contains(err.Error(), "UNIQUE

DEPLOY.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ This guide covers a generic Docker deploy and a step-by-step **Railway** deploy
3636
| `LITESTREAM_REPLICA_URL` | recommended || Enables continuous SQLite backup (see §6). |
3737
| `COOKIE_SECURE` | no | https→true | Override cookie Secure flag; defaults from `BASE_URL` scheme. |
3838
| `TRUSTED_PROXY_CIDRS` | no || Comma-separated CIDRs (a bare address = one host) whose `X-Forwarded-For` is believed when keying per-IP rate limits, e.g. `10.0.0.0/8`. Include a fronting CDN's own ranges so the walk steps over its edge and lands on the visitor. Unset ⇒ the header is ignored and the limit keys on the TCP peer, so behind a CDN every visitor shares one bucket. **Only list networks you control**: anything in the list can name any client IP it likes. Single-value vendor headers (`CF-Connecting-IP`, `X-Real-IP`) are never read, from any peer. |
39+
| `FRAME_ANCESTORS` | no || **Space**-separated origins allowed to embed the **admin UI** in a frame, e.g. `https://console.example.com 'self'`. Each entry must be `https://host[:port]` or `'self'` — anything else and **the app refuses to start**, because browsers drop a policy they cannot parse. Does not affect the public booking pages, which always deny framing. |
3940
| `LOG_LEVEL` | no | `info` | `debug`/`info`/`warn`/`error`. |
4041

4142
¹ Email is optional to boot, but bookings won't send confirmations until SMTP is configured (env **or** the admin UI). Precedence is **env var > DB setting > default**.

cmd/calnode/main.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ func main() {
5353
bi := buildinfo.Get()
5454
logger.Info("starting calnode", "version", bi.Version, "commit", bi.Commit, "build_time", bi.BuildTime, "dirty", bi.Dirty)
5555

56+
// Config whose wrong value is worse than its absence is checked here rather than
57+
// tolerated at request time — see (*config.Config).Validate.
58+
if err := cfg.Validate(); err != nil {
59+
logger.Error("invalid configuration", "error", err)
60+
os.Exit(1)
61+
}
62+
5663
if cfg.GoogleClientID != "" {
5764
slog.Info("Google OAuth configured", "client_id_prefix", cfg.GoogleClientID[:20])
5865
} else {

docs/ARCHITECTURE.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,24 @@ as the desired state:
783783
strict default and relaxes only when head code-injection is configured (broad
784784
`https:` or the operator's `tracking_csp_allow`). Don't re-hardcode the CSP on the
785785
`book`/`manage` handlers — route it through `publicCSP`.
786+
9. **`FRAME_ANCESTORS` is the admin SPA's only, and it must stay that way.** Set it
787+
(space-separated `https://host[:port]` / `'self'`) and the handler under `/admin/`
788+
sends `Content-Security-Policy: frame-ancestors <list>` so an operator can embed the
789+
console in their own tooling. `internal/server`'s `FrameAncestors` middleware wraps
790+
`frontend.Handler()` and nothing else: the public booking pages keep
791+
`frame-ancestors 'none'` + `X-Frame-Options: DENY` unconditionally, because they are
792+
unauthenticated pages collecting names, emails and card details and clickjacking one
793+
is worth more than framing a console nobody reaches without a session.
794+
⚠️ **Unset sends no frame header at all, which is what `/admin/` has always sent**
795+
the SPA is framable by default. This setting deliberately does *not* add a default
796+
deny, since an opt-in flag must not smuggle in a behaviour change;
797+
`TestAdminSPA_sendsNoFrameHeadersWhenUnset` pins the current answer so changing it is
798+
a decision. No `X-Frame-Options` is sent beside the CSP either: that header has no
799+
allow-list form (`ALLOW-FROM` is dead), so the only value it could carry is
800+
`SAMEORIGIN`, which browsers apply *instead of* the CSP and would break the embedding.
801+
An entry that isn't `https://host[:port]` or `'self'` fails `config.Validate()` and the
802+
process **refuses to start** — a browser drops a source list it cannot parse, so a
803+
typo would otherwise leave `/admin/` more embeddable than with the setting unset.
786804

787805
---
788806

internal/config/config.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package config
22

33
import (
4+
"fmt"
45
"log/slog"
6+
"net/url"
57
"os"
68
"strconv"
79
"strings"
@@ -67,6 +69,19 @@ type Config struct {
6769
// walk steps over its edge address and lands on the visitor.
6870
TrustedProxyCIDRs []string
6971

72+
// FrameAncestors lists the origins allowed to embed the admin SPA in a frame, as a
73+
// Content-Security-Policy frame-ancestors source list. Space-separated, matching the
74+
// CSP syntax it becomes. Empty (the default) ⇒ nothing is sent and /admin/ behaves
75+
// exactly as it did. Each entry must be `https://host[:port]` or `'self'`; anything
76+
// else fails Validate and the process refuses to start, because a directive the
77+
// browser cannot parse is a directive that silently allows everything.
78+
//
79+
// Only the admin SPA is affected. The public booking pages keep their
80+
// `frame-ancestors 'none'` + `X-Frame-Options: DENY` unconditionally — those are
81+
// unauthenticated pages that take payment details, and no operator convenience is
82+
// worth making them embeddable.
83+
FrameAncestors []string
84+
7085
// DemoMode turns this instance into a public, self-resetting demo: seeds sample
7186
// data on every boot (there's no persistent volume, so every boot is a fresh DB),
7287
// disables calendar/Zoom connect, serves a disallow-all robots.txt, and exposes
@@ -106,6 +121,9 @@ func Load() *Config {
106121
EmbedAllowedOrigins: splitCSV(getEnv("EMBED_ALLOWED_ORIGINS", "")),
107122
DataDir: getEnv("DATA_DIR", "data"),
108123
TrustedProxyCIDRs: splitCSV(getEnv("TRUSTED_PROXY_CIDRS", "")),
124+
// Space-separated, not comma: the value goes into a CSP source list verbatim, so
125+
// it reads the same in the env var as it does in the header.
126+
FrameAncestors: strings.Fields(getEnv("FRAME_ANCESTORS", "")),
109127
}
110128

111129
cfg.EncryptionKey = os.Getenv("CALNODE_ENCRYPTION_KEY")
@@ -121,6 +139,60 @@ func Load() *Config {
121139
return cfg
122140
}
123141

142+
// Validate reports the configuration errors an operator has to fix before the process
143+
// can safely serve traffic. Called from main after Load; a non-nil error is fatal.
144+
//
145+
// It holds the settings whose wrong value is worse than their absence. A malformed CSP
146+
// directive is the example: browsers drop a source list they cannot parse, so the admin
147+
// UI would end up MORE embeddable than with the setting unset, and nothing in the
148+
// response would say so.
149+
func (c *Config) Validate() error {
150+
for _, origin := range c.FrameAncestors {
151+
if err := validFrameAncestor(origin); err != nil {
152+
return fmt.Errorf("FRAME_ANCESTORS: %w", err)
153+
}
154+
}
155+
return nil
156+
}
157+
158+
// validFrameAncestor accepts 'self' or an https origin with no path, credentials, query
159+
// or fragment.
160+
//
161+
// Wildcards are deliberately refused even though CSP allows them. `https://*.example.com`
162+
// trusts every host any subdomain of that name ever points at, including one taken over
163+
// later; an operator who needs two hosts can name two hosts. Plain http is refused for
164+
// the same reason the admin session cookie is Secure — the framing page would be able to
165+
// read nothing, but its own compromise becomes a foothold.
166+
func validFrameAncestor(origin string) error {
167+
if origin == "'self'" {
168+
return nil
169+
}
170+
if strings.HasPrefix(origin, "'") {
171+
// 'none', 'unsafe-inline' and friends are keywords this setting has no use for:
172+
// 'none' is not "unset" (see FrameAncestors) and the rest are not source
173+
// expressions at all. Refusing them keeps the accepted grammar one line long.
174+
return fmt.Errorf("%q is not a supported keyword; use 'self' or an https:// origin", origin)
175+
}
176+
u, err := url.Parse(origin)
177+
switch {
178+
case err != nil:
179+
return fmt.Errorf("%q is not a URL: %w", origin, err)
180+
case u.Scheme != "https":
181+
return fmt.Errorf("%q must use https://", origin)
182+
case u.Host == "":
183+
return fmt.Errorf("%q has no host", origin)
184+
case strings.Contains(u.Host, "*"):
185+
return fmt.Errorf("%q must name one host, not a wildcard", origin)
186+
case u.User != nil:
187+
return fmt.Errorf("%q must not carry credentials", origin)
188+
case u.Path != "" && u.Path != "/":
189+
return fmt.Errorf("%q must be an origin, with no path", origin)
190+
case u.RawQuery != "" || u.Fragment != "":
191+
return fmt.Errorf("%q must be an origin, with no query or fragment", origin)
192+
}
193+
return nil
194+
}
195+
124196
func parseLogLevel(s string) slog.Level {
125197
switch s {
126198
case "debug":

internal/config/config_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,3 +134,77 @@ func TestLoad_dataDir(t *testing.T) {
134134
t.Errorf("DataDir = %q; want /var/lib/calnode", cfg.DataDir)
135135
}
136136
}
137+
138+
// ---------------------------------------------------------------------------
139+
// FRAME_ANCESTORS
140+
// ---------------------------------------------------------------------------
141+
142+
func TestLoad_frameAncestorsIsSpaceSeparated(t *testing.T) {
143+
t.Setenv("FRAME_ANCESTORS", " https://console.example.test 'self' ")
144+
145+
cfg := config.Load()
146+
147+
if len(cfg.FrameAncestors) != 2 {
148+
t.Fatalf("FrameAncestors = %#v; want 2 entries", cfg.FrameAncestors)
149+
}
150+
if cfg.FrameAncestors[0] != "https://console.example.test" || cfg.FrameAncestors[1] != "'self'" {
151+
t.Errorf("FrameAncestors = %#v; want the two sources unchanged", cfg.FrameAncestors)
152+
}
153+
if err := cfg.Validate(); err != nil {
154+
t.Errorf("Validate() = %v; want nil", err)
155+
}
156+
}
157+
158+
func TestLoad_frameAncestorsDefaultsToEmpty(t *testing.T) {
159+
os.Unsetenv("FRAME_ANCESTORS")
160+
161+
cfg := config.Load()
162+
163+
if len(cfg.FrameAncestors) != 0 {
164+
t.Errorf("FrameAncestors = %#v; want empty", cfg.FrameAncestors)
165+
}
166+
if err := cfg.Validate(); err != nil {
167+
t.Errorf("Validate() = %v; want nil", err)
168+
}
169+
}
170+
171+
// A directive the browser cannot parse is dropped whole, which would leave the admin SPA
172+
// more embeddable than with the setting unset. Refusing to start is the only outcome that
173+
// cannot be missed.
174+
func TestValidate_rejectsBadFrameAncestors(t *testing.T) {
175+
cases := map[string]string{
176+
"plain http": "http://console.example.test",
177+
"no scheme": "console.example.test",
178+
"wildcard host": "https://*.example.test",
179+
"with a path": "https://console.example.test/admin",
180+
"with a query": "https://console.example.test?x=1",
181+
"credentials": "https://user:pw@console.example.test",
182+
"none keyword": "'none'",
183+
"unsafe keyword": "'unsafe-inline'",
184+
"scheme only": "https://",
185+
}
186+
for name, value := range cases {
187+
t.Run(name, func(t *testing.T) {
188+
t.Setenv("FRAME_ANCESTORS", value)
189+
if err := config.Load().Validate(); err == nil {
190+
t.Errorf("Validate() = nil for %q; want an error", value)
191+
}
192+
})
193+
}
194+
}
195+
196+
// One bad entry beside a good one still fails: a half-applied source list is a policy
197+
// nobody wrote.
198+
func TestValidate_rejectsAListWithOneBadEntry(t *testing.T) {
199+
t.Setenv("FRAME_ANCESTORS", "https://good.example.test http://bad.example.test")
200+
if err := config.Load().Validate(); err == nil {
201+
t.Error("Validate() = nil; want an error naming the http entry")
202+
}
203+
}
204+
205+
func TestValidate_acceptsAPortAndATrailingSlash(t *testing.T) {
206+
t.Setenv("FRAME_ANCESTORS", "https://console.example.test:8443 https://other.example.test/")
207+
if err := config.Load().Validate(); err != nil {
208+
t.Errorf("Validate() = %v; want nil", err)
209+
}
210+
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package server
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
8+
"github.com/calnode/calnode/frontend"
9+
)
10+
11+
// serveAdminSPA runs a request through the admin SPA handler wrapped exactly as New
12+
// wraps it, and returns the response.
13+
func serveAdminSPA(origins []string, path string) *httptest.ResponseRecorder {
14+
h := FrameAncestors(origins)(frontend.Handler())
15+
rec := httptest.NewRecorder()
16+
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
17+
return rec
18+
}
19+
20+
// ⚠️ This pins what /admin/ sends TODAY, which is no frame header at all: the SPA is
21+
// framable by any site unless an operator says otherwise. FRAME_ANCESTORS deliberately
22+
// does not change that when unset — an opt-in setting must not smuggle in a default deny
23+
// — so this test exists to make the next person's change to it deliberate rather than
24+
// incidental. The public booking pages are the ones that carry an unconditional DENY.
25+
func TestAdminSPA_sendsNoFrameHeadersWhenUnset(t *testing.T) {
26+
for _, path := range []string{"/", "/bookings", "/favicon.svg"} {
27+
t.Run(path, func(t *testing.T) {
28+
rec := serveAdminSPA(nil, path)
29+
if got := rec.Header().Get("Content-Security-Policy"); got != "" {
30+
t.Errorf("Content-Security-Policy = %q; want it absent", got)
31+
}
32+
if got := rec.Header().Get("X-Frame-Options"); got != "" {
33+
t.Errorf("X-Frame-Options = %q; want it absent", got)
34+
}
35+
})
36+
}
37+
}
38+
39+
func TestAdminSPA_frameAncestorsWhenConfigured(t *testing.T) {
40+
rec := serveAdminSPA([]string{"https://console.example.test", "'self'"}, "/")
41+
42+
want := "frame-ancestors https://console.example.test 'self'"
43+
if got := rec.Header().Get("Content-Security-Policy"); got != want {
44+
t.Errorf("Content-Security-Policy = %q; want %q", got, want)
45+
}
46+
// X-Frame-Options has no allow-list form, and SAMEORIGIN would be honoured instead of
47+
// the CSP by the browsers that read it, breaking the embedding this enables.
48+
if got := rec.Header().Get("X-Frame-Options"); got != "" {
49+
t.Errorf("X-Frame-Options = %q; want it absent alongside frame-ancestors", got)
50+
}
51+
}
52+
53+
// The SPA fallback route (any client-side path) carries it too, not just the shell.
54+
func TestAdminSPA_frameAncestorsOnSPAFallback(t *testing.T) {
55+
rec := serveAdminSPA([]string{"'self'"}, "/settings/video")
56+
if got := rec.Header().Get("Content-Security-Policy"); got != "frame-ancestors 'self'" {
57+
t.Errorf("Content-Security-Policy = %q; want frame-ancestors 'self'", got)
58+
}
59+
}

internal/server/middleware.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,41 @@ func (rl *rateLimiter) cleanup() {
240240
}
241241
}
242242

243+
// FrameAncestors returns middleware that sets `Content-Security-Policy:
244+
// frame-ancestors <origins>` on the responses it wraps, so an operator can embed the
245+
// admin SPA in their own console. An empty list is a pass-through.
246+
//
247+
// ⛔ Scoped to the admin SPA on purpose, and it must stay that way. The public booking
248+
// pages set `frame-ancestors 'none'` plus `X-Frame-Options: DENY` in their own handlers
249+
// (book.go, manage_handler.go, tracking_settings.go's publicCSP) and this must never
250+
// reach them: they are unauthenticated pages that collect names, emails and card
251+
// details, and clickjacking one is worth more to an attacker than framing an admin UI
252+
// nobody can reach without a session.
253+
//
254+
// No `X-Frame-Options` is set beside the CSP. That header has no allow-list form — its
255+
// `ALLOW-FROM` was only ever implemented by one browser and is dead — so the only value
256+
// it could carry here is `SAMEORIGIN`, which every browser that reads it would apply
257+
// INSTEAD of honouring the CSP, breaking the embedding this exists to enable. Every
258+
// browser that can frame anything today supports frame-ancestors.
259+
//
260+
// ⚠️ With the list empty the wrapped handler sends no frame header at all, which is what
261+
// /admin/ has always sent: this middleware does not add a default deny, because that
262+
// would be a behaviour change smuggled in on an opt-in setting. See
263+
// TestAdminSPA_sendsNoFrameHeadersWhenUnset, which pins it.
264+
func FrameAncestors(origins []string) func(http.Handler) http.Handler {
265+
if len(origins) == 0 {
266+
return func(next http.Handler) http.Handler { return next }
267+
}
268+
policy := "frame-ancestors " + strings.Join(origins, " ")
269+
return func(next http.Handler) http.Handler {
270+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
271+
// Set before next writes: a header added after the first Write is dropped.
272+
w.Header().Set("Content-Security-Policy", policy)
273+
next.ServeHTTP(w, r)
274+
})
275+
}
276+
}
277+
243278
// remoteIP returns the IP a per-IP limit keys on.
244279
//
245280
// By default that is the TCP-level remote address, stripped of its port, and the

internal/server/server.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,10 @@ func New(ctx context.Context, cfg *config.Config, db *sql.DB, logger *slog.Logge
533533
mux.Handle("GET /favicon.ico", favicon)
534534

535535
// Admin SPA — served at /admin/* with SPA fallback for client-side routing.
536-
adminSPA := frontend.Handler()
536+
// FrameAncestors is applied here and nowhere else: FRAME_ANCESTORS is about embedding
537+
// the admin console, and the public pages' own DENY must not be reachable from a
538+
// config flag.
539+
adminSPA := FrameAncestors(cfg.FrameAncestors)(frontend.Handler())
537540
mux.Handle("GET /admin", http.RedirectHandler("/admin/", http.StatusMovedPermanently))
538541
mux.Handle("/admin/", http.StripPrefix("/admin", adminSPA))
539542

0 commit comments

Comments
 (0)