Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,19 @@ exact tag (`ghcr.io/calnode/calnode:0.1.0`) if you need stability between upgrad
pool could satisfy are all excluded, so the explanation never appears attached to the
wrong cause. Three new/changed keys in all eight locales.

- **`FRAME_ANCESTORS`: embed the admin UI in your own console.** Space-separated origins
(`https://console.example.com 'self'`); when set, `/admin/` sends
`Content-Security-Policy: frame-ancestors <list>`. The public booking pages are
untouched and still deny framing outright — this is about the console, not the pages
that take card details.

Two deliberate refusals. An entry that is not `https://host[:port]` or `'self'` stops
the app booting rather than being ignored, because a browser drops a source list it
cannot parse, which would leave the admin UI *more* embeddable than the setting being
unset. And no `X-Frame-Options` is sent beside it: that header has no allow-list form,
so the only value it could carry is `SAMEORIGIN`, which browsers honour instead of the
CSP and would break the embedding this exists for.

### Fixed
- **Constraint violations are recognised by SQLite's error code rather than by its
English message.** Thirteen call sites asked `strings.Contains(err.Error(), "UNIQUE
Expand Down
1 change: 1 addition & 0 deletions DEPLOY.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ This guide covers a generic Docker deploy and a step-by-step **Railway** deploy
| `LITESTREAM_REPLICA_URL` | recommended | — | Enables continuous SQLite backup (see §6). |
| `COOKIE_SECURE` | no | https→true | Override cookie Secure flag; defaults from `BASE_URL` scheme. |
| `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. |
| `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. ⛔ **Same-site only in practice**: `calnode_session` is `SameSite=Lax`, so a cross-site parent can frame `/admin/` and still never be sent the cookie — it gets the login screen inside the frame. Use `'self'` or a host sharing `BASE_URL`'s registrable domain. |
| `LOG_LEVEL` | no | `info` | `debug`/`info`/`warn`/`error`. |

¹ 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**.
Expand Down
7 changes: 7 additions & 0 deletions cmd/calnode/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ func main() {
bi := buildinfo.Get()
logger.Info("starting calnode", "version", bi.Version, "commit", bi.Commit, "build_time", bi.BuildTime, "dirty", bi.Dirty)

// Config whose wrong value is worse than its absence is checked here rather than
// tolerated at request time — see (*config.Config).Validate.
if err := cfg.Validate(); err != nil {
logger.Error("invalid configuration", "error", err)
os.Exit(1)
}

if cfg.GoogleClientID != "" {
slog.Info("Google OAuth configured", "client_id_prefix", cfg.GoogleClientID[:20])
} else {
Expand Down
27 changes: 27 additions & 0 deletions docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,33 @@ as the desired state:
strict default and relaxes only when head code-injection is configured (broad
`https:` or the operator's `tracking_csp_allow`). Don't re-hardcode the CSP on the
`book`/`manage` handlers — route it through `publicCSP`.
9. **`FRAME_ANCESTORS` is the admin SPA's only, and it must stay that way.** Set it
(space-separated `https://host[:port]` / `'self'`) and the handler under `/admin/`
sends `Content-Security-Policy: frame-ancestors <list>` so an operator can embed the
console in their own tooling. `internal/server`'s `FrameAncestors` middleware wraps
`frontend.Handler()` and nothing else: the public booking pages keep
`frame-ancestors 'none'` + `X-Frame-Options: DENY` unconditionally, because they are
unauthenticated pages collecting names, emails and card details and clickjacking one
is worth more than framing a console nobody reaches without a session.
⚠️ **Unset sends no frame header at all, which is what `/admin/` has always sent** —
the SPA is framable by default. This setting deliberately does *not* add a default
deny, since an opt-in flag must not smuggle in a behaviour change;
`TestAdminSPA_sendsNoFrameHeadersWhenUnset` pins the current answer so changing it is
a decision. No `X-Frame-Options` is sent beside the CSP either: that header has no
allow-list form (`ALLOW-FROM` is dead), so the only value it could carry is
`SAMEORIGIN`, which browsers apply *instead of* the CSP and would break the embedding.
An entry that isn't `https://host[:port]` or `'self'` fails `config.Validate()` and the
process **refuses to start** — a browser drops a source list it cannot parse, so a
typo would otherwise leave `/admin/` more embeddable than with the setting unset.
Comment thread
pullfrog[bot] marked this conversation as resolved.
⛔ **The CSP is only half of what an embed needs, and the other half is not
configurable.** `calnode_session` is `SameSite=Lax` (`session.go`), so a browser does
not send it on a subresource request from a **cross-site** parent. A console on an
unrelated eTLD+1 may therefore frame `/admin/` after this setting and still get the
login screen inside the frame, forever — the header permits the embed and the cookie
declines to authenticate it. Working embeds are same-site: `'self'`, or a parent
sharing `BASE_URL`'s registrable domain. Relaxing that means `SameSite=None; Secure`
on the session cookie, which removes the CSRF protection the `Lax` default provides
for every other route, so it is deliberately not offered as a setting.

---

Expand Down
81 changes: 81 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package config

import (
"fmt"
"log/slog"
"net/url"
"os"
"strconv"
"strings"
Expand Down Expand Up @@ -67,6 +69,28 @@ type Config struct {
// walk steps over its edge address and lands on the visitor.
TrustedProxyCIDRs []string

// FrameAncestors lists the origins allowed to embed the admin SPA in a frame, as a
// Content-Security-Policy frame-ancestors source list. Space-separated, matching the
// CSP syntax it becomes. Empty (the default) ⇒ nothing is sent and /admin/ behaves
// exactly as it did. Each entry must be `https://host[:port]` or `'self'`; anything
// else fails Validate and the process refuses to start, because a directive the
// browser cannot parse is a directive that silently allows everything.
//
// Only the admin SPA is affected. The public booking pages keep their
// `frame-ancestors 'none'` + `X-Frame-Options: DENY` unconditionally — those are
// unauthenticated pages that take payment details, and no operator convenience is
// worth making them embeddable.
//
// ⛔ Permitting the frame is not the same as making it work, and this setting is only
// the first half. calnode_session is SameSite=Lax, so a browser withholds it from a
// subresource request made by a CROSS-SITE parent: an operator who lists a console on
// an unrelated registrable domain gets the frame they asked for and a login screen
// inside it, with nothing in the response explaining why. Same-site parents ('self',
// or a host under BASE_URL's registrable domain) are the working case. The fix would
// be SameSite=None on the session cookie, which withdraws the CSRF protection Lax
// gives every other route, so it is not on offer here.
FrameAncestors []string
Comment thread
pullfrog[bot] marked this conversation as resolved.

// DemoMode turns this instance into a public, self-resetting demo: seeds sample
// data on every boot (there's no persistent volume, so every boot is a fresh DB),
// disables calendar/Zoom connect, serves a disallow-all robots.txt, and exposes
Expand Down Expand Up @@ -106,6 +130,9 @@ func Load() *Config {
EmbedAllowedOrigins: splitCSV(getEnv("EMBED_ALLOWED_ORIGINS", "")),
DataDir: getEnv("DATA_DIR", "data"),
TrustedProxyCIDRs: splitCSV(getEnv("TRUSTED_PROXY_CIDRS", "")),
// Space-separated, not comma: the value goes into a CSP source list verbatim, so
// it reads the same in the env var as it does in the header.
FrameAncestors: strings.Fields(getEnv("FRAME_ANCESTORS", "")),
}

cfg.EncryptionKey = os.Getenv("CALNODE_ENCRYPTION_KEY")
Expand All @@ -121,6 +148,60 @@ func Load() *Config {
return cfg
}

// Validate reports the configuration errors an operator has to fix before the process
// can safely serve traffic. Called from main after Load; a non-nil error is fatal.
//
// It holds the settings whose wrong value is worse than their absence. A malformed CSP
// directive is the example: browsers drop a source list they cannot parse, so the admin
// UI would end up MORE embeddable than with the setting unset, and nothing in the
// response would say so.
func (c *Config) Validate() error {
for _, origin := range c.FrameAncestors {
if err := validFrameAncestor(origin); err != nil {
return fmt.Errorf("FRAME_ANCESTORS: %w", err)
}
}
return nil
}

// validFrameAncestor accepts 'self' or an https origin with no path, credentials, query
// or fragment.
//
// Wildcards are deliberately refused even though CSP allows them. `https://*.example.com`
// trusts every host any subdomain of that name ever points at, including one taken over
// later; an operator who needs two hosts can name two hosts. Plain http is refused for
// the same reason the admin session cookie is Secure — the framing page would be able to
// read nothing, but its own compromise becomes a foothold.
func validFrameAncestor(origin string) error {
if origin == "'self'" {
return nil
}
if strings.HasPrefix(origin, "'") {
// 'none', 'unsafe-inline' and friends are keywords this setting has no use for:
// 'none' is not "unset" (see FrameAncestors) and the rest are not source
// expressions at all. Refusing them keeps the accepted grammar one line long.
return fmt.Errorf("%q is not a supported keyword; use 'self' or an https:// origin", origin)
}
u, err := url.Parse(origin)
switch {
case err != nil:
return fmt.Errorf("%q is not a URL: %w", origin, err)
case u.Scheme != "https":
return fmt.Errorf("%q must use https://", origin)
case u.Host == "":
return fmt.Errorf("%q has no host", origin)
case strings.Contains(u.Host, "*"):
return fmt.Errorf("%q must name one host, not a wildcard", origin)
case u.User != nil:
return fmt.Errorf("%q must not carry credentials", origin)
case u.Path != "" && u.Path != "/":
return fmt.Errorf("%q must be an origin, with no path", origin)
case u.RawQuery != "" || u.Fragment != "":
return fmt.Errorf("%q must be an origin, with no query or fragment", origin)
}
return nil
}

func parseLogLevel(s string) slog.Level {
switch s {
case "debug":
Expand Down
74 changes: 74 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,77 @@ func TestLoad_dataDir(t *testing.T) {
t.Errorf("DataDir = %q; want /var/lib/calnode", cfg.DataDir)
}
}

// ---------------------------------------------------------------------------
// FRAME_ANCESTORS
// ---------------------------------------------------------------------------

func TestLoad_frameAncestorsIsSpaceSeparated(t *testing.T) {
t.Setenv("FRAME_ANCESTORS", " https://console.example.test 'self' ")

cfg := config.Load()

if len(cfg.FrameAncestors) != 2 {
t.Fatalf("FrameAncestors = %#v; want 2 entries", cfg.FrameAncestors)
}
if cfg.FrameAncestors[0] != "https://console.example.test" || cfg.FrameAncestors[1] != "'self'" {
t.Errorf("FrameAncestors = %#v; want the two sources unchanged", cfg.FrameAncestors)
}
if err := cfg.Validate(); err != nil {
t.Errorf("Validate() = %v; want nil", err)
}
}

func TestLoad_frameAncestorsDefaultsToEmpty(t *testing.T) {
os.Unsetenv("FRAME_ANCESTORS")

cfg := config.Load()

if len(cfg.FrameAncestors) != 0 {
t.Errorf("FrameAncestors = %#v; want empty", cfg.FrameAncestors)
}
if err := cfg.Validate(); err != nil {
t.Errorf("Validate() = %v; want nil", err)
}
}

// A directive the browser cannot parse is dropped whole, which would leave the admin SPA
// more embeddable than with the setting unset. Refusing to start is the only outcome that
// cannot be missed.
func TestValidate_rejectsBadFrameAncestors(t *testing.T) {
cases := map[string]string{
"plain http": "http://console.example.test",
"no scheme": "console.example.test",
"wildcard host": "https://*.example.test",
"with a path": "https://console.example.test/admin",
"with a query": "https://console.example.test?x=1",
"credentials": "https://user:pw@console.example.test",
"none keyword": "'none'",
"unsafe keyword": "'unsafe-inline'",
"scheme only": "https://",
}
for name, value := range cases {
t.Run(name, func(t *testing.T) {
t.Setenv("FRAME_ANCESTORS", value)
if err := config.Load().Validate(); err == nil {
t.Errorf("Validate() = nil for %q; want an error", value)
}
})
}
}

// One bad entry beside a good one still fails: a half-applied source list is a policy
// nobody wrote.
func TestValidate_rejectsAListWithOneBadEntry(t *testing.T) {
t.Setenv("FRAME_ANCESTORS", "https://good.example.test http://bad.example.test")
if err := config.Load().Validate(); err == nil {
t.Error("Validate() = nil; want an error naming the http entry")
}
}

func TestValidate_acceptsAPortAndATrailingSlash(t *testing.T) {
t.Setenv("FRAME_ANCESTORS", "https://console.example.test:8443 https://other.example.test/")
if err := config.Load().Validate(); err != nil {
t.Errorf("Validate() = %v; want nil", err)
}
}
59 changes: 59 additions & 0 deletions internal/server/frameancestors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package server

import (
"net/http"
"net/http/httptest"
"testing"

"github.com/calnode/calnode/frontend"
)

// serveAdminSPA runs a request through the admin SPA handler wrapped exactly as New
// wraps it, and returns the response.
func serveAdminSPA(origins []string, path string) *httptest.ResponseRecorder {
h := FrameAncestors(origins)(frontend.Handler())
rec := httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
return rec
}

// ⚠️ This pins what /admin/ sends TODAY, which is no frame header at all: the SPA is
// framable by any site unless an operator says otherwise. FRAME_ANCESTORS deliberately
// does not change that when unset — an opt-in setting must not smuggle in a default deny
// — so this test exists to make the next person's change to it deliberate rather than
// incidental. The public booking pages are the ones that carry an unconditional DENY.
func TestAdminSPA_sendsNoFrameHeadersWhenUnset(t *testing.T) {
for _, path := range []string{"/", "/bookings", "/favicon.svg"} {
t.Run(path, func(t *testing.T) {
rec := serveAdminSPA(nil, path)
if got := rec.Header().Get("Content-Security-Policy"); got != "" {
t.Errorf("Content-Security-Policy = %q; want it absent", got)
}
if got := rec.Header().Get("X-Frame-Options"); got != "" {
t.Errorf("X-Frame-Options = %q; want it absent", got)
}
})
}
}

func TestAdminSPA_frameAncestorsWhenConfigured(t *testing.T) {
rec := serveAdminSPA([]string{"https://console.example.test", "'self'"}, "/")

want := "frame-ancestors https://console.example.test 'self'"
if got := rec.Header().Get("Content-Security-Policy"); got != want {
t.Errorf("Content-Security-Policy = %q; want %q", got, want)
}
// X-Frame-Options has no allow-list form, and SAMEORIGIN would be honoured instead of
// the CSP by the browsers that read it, breaking the embedding this enables.
if got := rec.Header().Get("X-Frame-Options"); got != "" {
t.Errorf("X-Frame-Options = %q; want it absent alongside frame-ancestors", got)
}
}

// The SPA fallback route (any client-side path) carries it too, not just the shell.
func TestAdminSPA_frameAncestorsOnSPAFallback(t *testing.T) {
rec := serveAdminSPA([]string{"'self'"}, "/settings/video")
if got := rec.Header().Get("Content-Security-Policy"); got != "frame-ancestors 'self'" {
t.Errorf("Content-Security-Policy = %q; want frame-ancestors 'self'", got)
}
}
35 changes: 35 additions & 0 deletions internal/server/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,41 @@ func (rl *rateLimiter) cleanup() {
}
}

// FrameAncestors returns middleware that sets `Content-Security-Policy:
// frame-ancestors <origins>` on the responses it wraps, so an operator can embed the
// admin SPA in their own console. An empty list is a pass-through.
//
// ⛔ Scoped to the admin SPA on purpose, and it must stay that way. The public booking
// pages set `frame-ancestors 'none'` plus `X-Frame-Options: DENY` in their own handlers
// (book.go, manage_handler.go, tracking_settings.go's publicCSP) and this must never
// reach them: they are unauthenticated pages that collect names, emails and card
// details, and clickjacking one is worth more to an attacker than framing an admin UI
// nobody can reach without a session.
//
// No `X-Frame-Options` is set beside the CSP. That header has no allow-list form — its
// `ALLOW-FROM` was only ever implemented by one browser and is dead — so the only value
// it could carry here is `SAMEORIGIN`, which every browser that reads it would apply
// INSTEAD of honouring the CSP, breaking the embedding this exists to enable. Every
// browser that can frame anything today supports frame-ancestors.
//
// ⚠️ With the list empty the wrapped handler sends no frame header at all, which is what
// /admin/ has always sent: this middleware does not add a default deny, because that
// would be a behaviour change smuggled in on an opt-in setting. See
// TestAdminSPA_sendsNoFrameHeadersWhenUnset, which pins it.
func FrameAncestors(origins []string) func(http.Handler) http.Handler {
if len(origins) == 0 {
return func(next http.Handler) http.Handler { return next }
}
policy := "frame-ancestors " + strings.Join(origins, " ")
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Set before next writes: a header added after the first Write is dropped.
w.Header().Set("Content-Security-Policy", policy)
next.ServeHTTP(w, r)
})
}
}

// remoteIP returns the IP a per-IP limit keys on.
//
// By default that is the TCP-level remote address, stripped of its port, and the
Expand Down
5 changes: 4 additions & 1 deletion internal/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,10 @@ func New(ctx context.Context, cfg *config.Config, db *sql.DB, logger *slog.Logge
mux.Handle("GET /favicon.ico", favicon)

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

Expand Down
Loading