amadan

carlos / movement Public

Tasks 2 and 3: config that refuses, logging that cannot leak, copy that waits

Three gates, all of them proven to fail before they were made to pass.

The app now refuses to boot without an origin and an instance key, and
refuses to run without SMTP unless the origin is loopback — because
rastrillo's mail fallback logs the confirmation token instead of sending
it, into storage that cannot be grepped clean.

Only logging.go may reach slog, and its surface takes typed Reason codes:
no errors, no request strings. That is not fussiness. rastrillo/mail
formats failures as "send to <address>", so logging a send error beside a
request path carrying the stance would assemble the sensitive pair in one
line. The guard fired on app.go and handlers.go before they were routed
through it.

Copy lives in a table and templates carry none. The guard caught the
scaffold's own "this is a rastrillo app" on its first run. The lookup is
{{say}} rather than {{t}}, because ui.Funcs already registers T and two
lookups differing only in case is a mistake waiting to happen in a
template nobody type-checks.

make ci green, browser step included.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0175aLTHqLrYkGe1skMV1rvq
Paul Campbell 9785ecdefbab472848cc1601e1a70c12378339ef parent ad25639
14 files changed, +686 −25
  • cmd/movement/main.go +13 −9
  • internal/movement/app.go +19 −6
  • internal/movement/config.go +124 −0
  • internal/movement/config_test.go +89 −0
  • internal/movement/copy.go +124 −0
  • internal/movement/copy_test.go +57 −0
  • internal/movement/handlers.go +7 −5
  • internal/movement/logging.go +72 −0
  • internal/movement/render.go +10 −1
  • internal/movement/templates/index.html +3 −1
  • internal/movementtest/browser_test.go +4 −1
  • internal/movementtest/guards_test.go +147 −0
  • internal/movementtest/harness_test.go +12 −1
  • internal/movementtest/index_test.go +5 −1
diff --git a/cmd/movement/main.go b/cmd/movement/main.go
index 6954295..9325abc 100644
--- a/cmd/movement/main.go
+++ b/cmd/movement/main.go
@@ -33,17 +33,21 @@ func main() {
}
defer d.Close()
- // Origin decides the CSRF same-origin check (and, when the app
- // grows accounts, the Secure/__Host- cookie attributes) — so
- // defaulting it silently in production would be a real bug. Loud
- // on purpose, harmless in local dev.
- origin := os.Getenv("MOVEMENT_ORIGIN")
- if origin == "" {
- origin = "http://localhost:8080"
- logger.Warn("MOVEMENT_ORIGIN not set; defaulting", "origin", origin)
+ // The scaffold defaulted a missing origin to localhost and warned. This
+ // app refuses instead. The origin is the base of every emailed link, the
+ // CSRF yardstick and the cookie's host, so a default that boots happily
+ // is a canary quietly mailing people into production. LoadEnv also
+ // refuses an unset instance key, and refuses to run without SMTP unless
+ // the origin is loopback — because rastrillo's mail fallback logs the
+ // confirmation token rather than sending it.
+ env, err := movement.LoadEnv()
+ if err != nil {
+ logger.Error("configuration", "err", err)
+ os.Exit(1)
}
+ movement.SetLogger(logger)
- mux, err := movement.App(d, origin, logger)
+ mux, err := movement.App(d, env)
if err != nil {
logger.Error("build app", "err", err)
os.Exit(1)
diff --git a/internal/movement/app.go b/internal/movement/app.go
index 3ea0291..6adfdc6 100644
--- a/internal/movement/app.go
+++ b/internal/movement/app.go
@@ -2,7 +2,6 @@ package movement
import (
"context"
- "log/slog"
"net/http"
"github.com/go-chi/chi/v5"
@@ -24,7 +23,18 @@ import (
// rastrillo repo is the worked example to copy). Adding a subsystem
// also means adding its Schema to migrations.go's BootSchema — see
// the comment there.
-func App(d *db.DB, origin string, logger *slog.Logger) (*http.ServeMux, error) {
+// App takes the whole Env rather than a bare origin, because the origin is
+// no longer the only thing wiring depends on: the instance key seals
+// challenges, the allowlist gates admin, and the origin decides whether
+// unreviewed copy is allowed to boot.
+func App(d *db.DB, env Env) (*http.ServeMux, error) {
+ // Unreviewed copy can be reviewed on a canary; it must never answer on
+ // the canonical host. The check is here, at boot, against the origin —
+ // not at build time — because there is only one build and the canary
+ // serves the same binary production would.
+ if err := CheckPlaceholders(env.Origin, CanonicalHost); err != nil {
+ return nil, err
+ }
// Apply BootSchema, not Schema: BootSchema is everything this
// app needs at boot (its own migrations plus any subsystem's),
// while Schema (migrations.go) stays just this app's own so the
@@ -38,16 +48,19 @@ func App(d *db.DB, origin string, logger *slog.Logger) (*http.ServeMux, error) {
// until something does, a fleet-wide adoption — every database
// stamped, no DDL run, exactly once, never again — produces no
// output at all, which is the last event you want to be silent.
- logger.Info("migrate", "applied", migrated.Applied,
- "skipped", migrated.Skipped, "adopted", migrated.Adopted)
+ Migrated(migrated.Applied, migrated.Skipped, migrated.Adopted)
+
+ if len(Placeholders()) > 0 {
+ Warn("this build carries unreviewed placeholder copy")
+ }
- a := &app{db: d, logger: logger}
+ a := &app{db: d, env: env}
r := chi.NewRouter()
// App-wide, from day one: every state-changing route added below
// is born covered. Origin-checking, not tokens — nothing to mint
// or forget in a form.
- r.Use(csrf.Protect(origin))
+ r.Use(csrf.Protect(env.Origin))
r.Get("/", a.index)
mux := http.NewServeMux()
diff --git a/internal/movement/config.go b/internal/movement/config.go
new file mode 100644
index 0000000..b6ce1e2
--- /dev/null
+++ b/internal/movement/config.go
@@ -0,0 +1,124 @@
+package movement
+
+import (
+ "errors"
+ "fmt"
+ "net/url"
+ "os"
+ "strings"
+)
+
+// CanonicalHost is the one host this app is meant to serve in production.
+// It gates the placeholder-copy refusal (copy.go): a canary runs at
+// <canary>.movement.bdj.oncarlos.com and must be able to boot with
+// unreviewed copy, because the shipping cadence reviews branches on a
+// deployed canary and there is only one build.
+const CanonicalHost = "movement.bdj.oncarlos.com"
+
+// Env is the configuration this app refuses to start without.
+type Env struct {
+ Origin string
+ InstanceKey string
+ Admins []string
+}
+
+// loopback reports whether origin is a local development origin.
+//
+// Only the mail refusal below is relaxed for loopback, and the check is
+// deliberately narrow: an instance claiming a loopback origin in production
+// is a misconfiguration worth seeing loudly rather than quietly excusing.
+func loopback(origin string) bool {
+ u, err := url.Parse(origin)
+ if err != nil {
+ return false
+ }
+ switch u.Hostname() {
+ case "localhost", "127.0.0.1", "::1":
+ return true
+ }
+ return false
+}
+
+// LoadEnv reads configuration and refuses rather than guessing.
+//
+// A missing origin refuses here; a wrong one boots happily and mails links
+// to the wrong host, which is the failure that costs an afternoon. It is
+// also why the canary story matters: a hard-coded production origin would
+// have a canary mailing people into production.
+//
+// auth.New refuses an empty InstanceKey on its own, but it does so at the
+// wiring call with a framework message. Refusing here, naming the variable
+// an operator has to set, is the difference between a five-minute fix and a
+// confusing one.
+func LoadEnv() (Env, error) {
+ env := Env{
+ Origin: strings.TrimSpace(os.Getenv("MOVEMENT_ORIGIN")),
+ InstanceKey: strings.TrimSpace(os.Getenv("MOVEMENT_INSTANCE_KEY")),
+ Admins: parseAdmins(os.Getenv("MOVEMENT_ADMINS")),
+ }
+
+ var missing []string
+ if env.Origin == "" {
+ missing = append(missing, "MOVEMENT_ORIGIN")
+ }
+ if env.InstanceKey == "" {
+ missing = append(missing, "MOVEMENT_INSTANCE_KEY")
+ }
+ if len(missing) > 0 {
+ return Env{}, fmt.Errorf("movement: set %s", strings.Join(missing, ", "))
+ }
+ if _, err := url.Parse(env.Origin); err != nil {
+ return Env{}, errors.New("movement: MOVEMENT_ORIGIN is not a URL")
+ }
+
+ // mail.FromEnv falls back to mail.Logged when SMTP is unconfigured, and
+ // Logged writes recipient, subject and body — which here means the magic
+ // link and the confirmation token — into carlos logs. That is exactly
+ // the storage the logging rule exists to keep clean.
+ //
+ // The check is on the environment rather than on the returned Sender
+ // because loggedSender is unexported: the fallback cannot be detected
+ // from the value, so it has to be predicted from its cause.
+ if os.Getenv("CARLOS_SMTP_HOST") == "" || os.Getenv("CARLOS_SMTP_FROM") == "" {
+ if !loopback(env.Origin) {
+ return Env{}, errors.New(
+ "movement: no SMTP configured and origin is not loopback; " +
+ "run carlos email enable -app movement")
+ }
+ }
+ return env, nil
+}
+
+// parseAdmins splits and normalises the allowlist. An empty result is
+// meaningful: every admin route answers 404, which is the fail-closed state.
+func parseAdmins(raw string) []string {
+ var out []string
+ for _, a := range strings.Split(raw, ",") {
+ if a = strings.ToLower(strings.TrimSpace(a)); a != "" {
+ out = append(out, a)
+ }
+ }
+ return out
+}
+
+// IsAdmin reports whether address is on the allowlist. An empty allowlist
+// admits nobody.
+//
+// This is consulted on every admin request, not once at sign-in. auth's own
+// Authorize runs only when a magic link is redeemed, so without a
+// per-request check a removed admin would keep access for the life of their
+// session. The list itself is read at boot: a process's environment is fixed
+// at exec, so nothing re-reads it — what makes removal take effect quickly is
+// `carlos env set` converging and bouncing the instance.
+func (e Env) IsAdmin(address string) bool {
+ address = strings.ToLower(strings.TrimSpace(address))
+ if address == "" {
+ return false
+ }
+ for _, a := range e.Admins {
+ if a == address {
+ return true
+ }
+ }
+ return false
+}
diff --git a/internal/movement/config_test.go b/internal/movement/config_test.go
new file mode 100644
index 0000000..cb81dc7
--- /dev/null
+++ b/internal/movement/config_test.go
@@ -0,0 +1,89 @@
+package movement
+
+import (
+ "strings"
+ "testing"
+)
+
+// setEnv installs a complete, valid environment and lets a caller override
+// pieces of it, so each test says only what it is actually about.
+func setEnv(t *testing.T, over map[string]string) {
+ t.Helper()
+ base := map[string]string{
+ "MOVEMENT_ORIGIN": "https://" + CanonicalHost,
+ "MOVEMENT_INSTANCE_KEY": strings.Repeat("a", 64),
+ "MOVEMENT_ADMINS": "",
+ "CARLOS_SMTP_HOST": "smtp.example.com",
+ "CARLOS_SMTP_FROM": "movement@example.com",
+ }
+ for k, v := range over {
+ base[k] = v
+ }
+ for k, v := range base {
+ t.Setenv(k, v)
+ }
+}
+
+func TestLoadEnvRefusesMissingOriginAndKey(t *testing.T) {
+ setEnv(t, map[string]string{"MOVEMENT_ORIGIN": "", "MOVEMENT_INSTANCE_KEY": ""})
+
+ _, err := LoadEnv()
+ if err == nil {
+ t.Fatal("expected a refusal with no origin and no instance key")
+ }
+ // Both names, so an operator fixes it in one pass rather than two.
+ for _, want := range []string{"MOVEMENT_ORIGIN", "MOVEMENT_INSTANCE_KEY"} {
+ if !strings.Contains(err.Error(), want) {
+ t.Errorf("refusal should name %s, got: %v", want, err)
+ }
+ }
+}
+
+func TestLoadEnvRefusesUnconfiguredMailOnPublicOrigin(t *testing.T) {
+ setEnv(t, map[string]string{"CARLOS_SMTP_HOST": "", "CARLOS_SMTP_FROM": ""})
+
+ // mail.Logged would write the confirmation token and the address into
+ // carlos logs, which cannot be grepped clean afterwards.
+ if _, err := LoadEnv(); err == nil {
+ t.Fatal("expected a refusal: the mail.Logged fallback leaks into carlos logs")
+ }
+}
+
+func TestLoadEnvAllowsUnconfiguredMailOnLoopback(t *testing.T) {
+ setEnv(t, map[string]string{
+ "MOVEMENT_ORIGIN": "http://127.0.0.1:8080",
+ "CARLOS_SMTP_HOST": "",
+ "CARLOS_SMTP_FROM": "",
+ })
+
+ if _, err := LoadEnv(); err != nil {
+ t.Fatalf("loopback development should warn, not refuse: %v", err)
+ }
+}
+
+func TestAdminAllowlistIsEmptyByDefaultAndCaseInsensitive(t *testing.T) {
+ setEnv(t, map[string]string{"MOVEMENT_ADMINS": ""})
+ env, err := LoadEnv()
+ if err != nil {
+ t.Fatal(err)
+ }
+ // The fail-closed state: nobody is an admin, and the routes 404.
+ if env.IsAdmin("paul@keymail.dev") {
+ t.Fatal("an empty allowlist must admit nobody")
+ }
+
+ setEnv(t, map[string]string{"MOVEMENT_ADMINS": " Paul@Keymail.dev , other@example.com "})
+ env, err = LoadEnv()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !env.IsAdmin("PAUL@keymail.DEV") {
+ t.Fatal("the allowlist must match regardless of case or surrounding space")
+ }
+ if env.IsAdmin("stranger@example.com") {
+ t.Fatal("a stranger must not be admitted")
+ }
+ if env.IsAdmin("") {
+ t.Fatal("an empty address must never be admitted")
+ }
+}
diff --git a/internal/movement/copy.go b/internal/movement/copy.go
new file mode 100644
index 0000000..7228db2
--- /dev/null
+++ b/internal/movement/copy.go
@@ -0,0 +1,124 @@
+package movement
+
+import (
+ "fmt"
+ "net/url"
+ "sort"
+)
+
+// This app is its copy. The words are the mechanism that sorts people into
+// buckets, not decoration on top of one — so they go through the copy-review
+// gate before they reach a reader, and until they have, they live here as
+// marked placeholders.
+//
+// Holding them in a table rather than in markup is what makes the whole
+// engine buildable and canary-reviewable before copy review has happened. It
+// also means unreviewed words cannot reach production by being quietly typed
+// into a template: CheckPlaceholders refuses the canonical host while any
+// remain.
+
+type entry struct {
+ text string
+ // placeholder marks copy that has not been through review. Plan 2
+ // replaces the text and clears the flag, one id at a time.
+ placeholder bool
+}
+
+var table = map[string]entry{
+ // The landing page and the three choices. Slugs and labels are settled
+ // at copy review; the third label especially, because "loves AI, hasn't
+ // considered the ethics" describes a real group that will never
+ // self-select under that description.
+ "index.headline": {"PLACEHOLDER headline", true},
+ "index.lede": {"PLACEHOLDER lede", true},
+ "index.link": {"PLACEHOLDER link to carlosframework.com", true},
+ "stance.a.label": {"PLACEHOLDER stance A label", true},
+ "stance.b.label": {"PLACEHOLDER stance B label", true},
+ "stance.c.label": {"PLACEHOLDER stance C label", true},
+ "stance.a.message": {"PLACEHOLDER stance A message", true},
+ "stance.b.message": {"PLACEHOLDER stance B message", true},
+ "stance.c.message": {"PLACEHOLDER stance C message", true},
+
+ // The form.
+ "form.email": {"PLACEHOLDER email label", true},
+ "form.background": {"PLACEHOLDER background label", true},
+ "form.submit": {"PLACEHOLDER submit", true},
+ "form.working": {"PLACEHOLDER working", true},
+ "form.nojs": {"PLACEHOLDER javascript is required to sign up", true},
+ "form.bademail": {"PLACEHOLDER that address does not look right", true},
+
+ // The one page every outcome renders. Its wording is load-bearing: it
+ // is shown to an unknown address, a pending one and a confirmed one
+ // alike, so it cannot say anything that would only be true of one.
+ "sent.title": {"PLACEHOLDER check your inbox", true},
+ "sent.body": {"PLACEHOLDER check your inbox body", true},
+
+ // Confirmation.
+ "confirm.title": {"PLACEHOLDER confirm", true},
+ "confirm.button": {"PLACEHOLDER confirm button", true},
+ "confirm.done": {"PLACEHOLDER you are on the list", true},
+ "confirm.dead": {"PLACEHOLDER that link is no longer valid", true},
+
+ // Mail.
+ "mail.confirm.subject": {"PLACEHOLDER confirmation subject", true},
+ "mail.confirm.body": {"PLACEHOLDER confirmation body", true},
+
+ // Admin.
+ "signin.title": {"PLACEHOLDER sign in", true},
+ "signin.sent": {"PLACEHOLDER check your inbox for a link", true},
+ "admin.title": {"PLACEHOLDER admin", true},
+
+ // Not a placeholder: this one exists to say that the others are.
+ "banner.placeholder": {"This build carries unreviewed placeholder copy.", false},
+}
+
+// T resolves a copy id.
+//
+// It panics on an unknown id rather than rendering empty. A missing string
+// should fail in a test, loudly, rather than ship as a blank space nobody
+// notices until a visitor does.
+func T(id string) string {
+ e, ok := table[id]
+ if !ok {
+ panic("movement: unknown copy id " + id)
+ }
+ return e.text
+}
+
+// Placeholders lists the ids still awaiting copy review, sorted so the
+// output is stable enough to diff between builds.
+func Placeholders() []string {
+ ids := make([]string, 0, len(table))
+ for id, e := range table {
+ if e.placeholder {
+ ids = append(ids, id)
+ }
+ }
+ sort.Strings(ids)
+ return ids
+}
+
+// CheckPlaceholders refuses the canonical host while any copy is unreviewed,
+// and allows everywhere else.
+//
+// The asymmetry is the point. The shipping cadence reviews a branch on a
+// deployed canary, and a canary serves the same binary production would — so
+// a build-time refusal would make plan 1 impossible to review, while no
+// refusal at all would let unreviewed words reach readers. Checking the
+// origin at boot is what separates the two.
+func CheckPlaceholders(origin, canonicalHost string) error {
+ left := Placeholders()
+ if len(left) == 0 {
+ return nil
+ }
+ u, err := url.Parse(origin)
+ if err != nil {
+ return fmt.Errorf("movement: cannot parse origin %q", origin)
+ }
+ if u.Hostname() == canonicalHost {
+ return fmt.Errorf(
+ "movement: %d copy strings have not been through review; refusing to serve %s",
+ len(left), canonicalHost)
+ }
+ return nil
+}
diff --git a/internal/movement/copy_test.go b/internal/movement/copy_test.go
new file mode 100644
index 0000000..5c33a6b
--- /dev/null
+++ b/internal/movement/copy_test.go
@@ -0,0 +1,57 @@
+package movement
+
+import "testing"
+
+func TestPlaceholdersRefuseTheCanonicalHostOnly(t *testing.T) {
+ if len(Placeholders()) == 0 {
+ t.Skip("copy review has landed; this gate has done its job")
+ }
+
+ if err := CheckPlaceholders("https://"+CanonicalHost, CanonicalHost); err == nil {
+ t.Error("unreviewed copy must not be allowed to serve the canonical host")
+ }
+
+ // A canary serves the same binary production would, and the shipping
+ // cadence reviews branches on a deployed canary. If placeholders refused
+ // here too, the engine could never be reviewed before copy review — which
+ // is the whole reason the copy table exists.
+ for _, origin := range []string{
+ "https://c1." + CanonicalHost,
+ "http://127.0.0.1:8080",
+ "http://app.test",
+ } {
+ if err := CheckPlaceholders(origin, CanonicalHost); err != nil {
+ t.Errorf("%s must boot with placeholders: %v", origin, err)
+ }
+ }
+}
+
+func TestUnknownCopyIDPanics(t *testing.T) {
+ defer func() {
+ if recover() == nil {
+ t.Fatal("an unknown copy id must panic; rendering empty ships a blank page")
+ }
+ }()
+ T("no.such.id")
+}
+
+// Every id the templates and handlers ask for has to exist, or T panics at
+// render time in front of a visitor rather than here.
+func TestEveryReferencedCopyIDResolves(t *testing.T) {
+ for _, id := range []string{
+ "index.headline", "index.lede",
+ "sent.title", "sent.body",
+ "confirm.title", "confirm.button", "confirm.done", "confirm.dead",
+ "form.email", "form.background", "form.submit", "form.nojs",
+ "mail.confirm.subject", "mail.confirm.body",
+ } {
+ func() {
+ defer func() {
+ if r := recover(); r != nil {
+ t.Errorf("copy id %q is referenced but missing", id)
+ }
+ }()
+ _ = T(id)
+ }()
+ }
+}
diff --git a/internal/movement/handlers.go b/internal/movement/handlers.go
index cc8871a..76200c2 100644
--- a/internal/movement/handlers.go
+++ b/internal/movement/handlers.go
@@ -1,17 +1,19 @@
package movement
import (
- "log/slog"
"net/http"
"github.com/carlosframework/rastrillo/db"
)
-// app holds what every handler needs. When the app grows accounts,
-// this is where the sessions handle lands too.
+// app holds what every handler needs.
+//
+// There is no logger field, and that is deliberate: a handler holds the
+// request, so a logger within reach of one is a logger within reach of an
+// address and a stance. Handlers call logging.go's typed helpers instead.
type app struct {
- db *db.DB
- logger *slog.Logger
+ db *db.DB
+ env Env
}
func (a *app) index(w http.ResponseWriter, r *http.Request) {
diff --git a/internal/movement/logging.go b/internal/movement/logging.go
new file mode 100644
index 0000000..b6faf1e
--- /dev/null
+++ b/internal/movement/logging.go
@@ -0,0 +1,72 @@
+package movement
+
+import "log/slog"
+
+// This file is the only place in internal/movement that imports log/slog,
+// and a guard test in internal/movementtest enforces that.
+//
+// The reason is narrow and specific. An email address on its own is
+// ordinary and a stance on its own is anonymous, but the two together are a
+// political position attached to a named person — and `carlos logs` is
+// platform-held storage that cannot be grepped clean afterwards. The pair is
+// easy to assemble by accident: rastrillo/mail formats its own failures as
+// "send to <address>", so logging a send error verbatim next to a request
+// path carrying the stance would do it in one line.
+//
+// So the log surface takes a fixed vocabulary and nothing else. No error
+// values, no strings drawn from a request. Anything a caller might want to
+// say about a submission has to already be one of the Reason constants
+// below, which means adding a new one is a deliberate act rather than a
+// convenient interpolation.
+
+// Reason is why a submission was refused. The set is closed on purpose.
+type Reason string
+
+const (
+ ReasonSealInvalid Reason = "seal_invalid"
+ ReasonNonceSpent Reason = "nonce_spent"
+ ReasonTooFast Reason = "too_fast"
+ ReasonTooOld Reason = "too_old"
+ ReasonPowShort Reason = "pow_short"
+ ReasonHoneypot Reason = "honeypot"
+ ReasonBounds Reason = "bounds"
+ ReasonMailFailed Reason = "mail_failed"
+ ReasonMailBudget Reason = "mail_budget"
+)
+
+var logger = slog.Default()
+
+// SetLogger installs the process logger. Called once, from App.
+func SetLogger(l *slog.Logger) {
+ if l != nil {
+ logger = l
+ }
+}
+
+// Rejected records why a submission was refused. It deliberately takes no
+// address, no stance and no error — only the code.
+func Rejected(r Reason) {
+ logger.Info("signup rejected", "reason", string(r))
+}
+
+// Migrated is the one boot line about schema, and it exists because a
+// fleet-wide adoption — every database stamped, no DDL run, exactly once —
+// otherwise produces no output at all, which is the last event you want to
+// be silent.
+//
+// Typed rather than variadic so this file's rule needs no exception: there
+// is no way to hand it a string that came from a request. Migration names
+// are compile-time constants, so they are safe.
+func Migrated(applied []string, skipped int, adopted bool) {
+ logger.Info("migrate", "applied", applied, "skipped", skipped, "adopted", adopted)
+}
+
+// Warn carries a fixed message. Callers pass literals, never request data.
+func Warn(msg string) {
+ logger.Warn(msg)
+}
+
+// Info carries a fixed message, same rule as Warn.
+func Info(msg string) {
+ logger.Info(msg)
+}
diff --git a/internal/movement/render.go b/internal/movement/render.go
index 2eb9630..7655cfa 100644
--- a/internal/movement/render.go
+++ b/internal/movement/render.go
@@ -40,7 +40,16 @@ func init() {
// switching delivery later needs no template edit.
template.Must(template.New("layout").
Funcs(ui.Funcs(ui.WithIcons(icons.Icon, icons.Assets))).
- Funcs(template.FuncMap{"asset": assets.Path}).
+ // say is how every user-facing string reaches a page. No
+ // template writes a literal sentence: a guard test in
+ // movementtest fails the build if one does, so that
+ // unreviewed copy cannot arrive by being typed into markup.
+ //
+ // Named "say" rather than "t" on purpose — ui.Funcs already
+ // registers T for the framework's own strings, and two
+ // lookups differing only in case is a mistake waiting to be
+ // made in a template nobody type-checks.
+ Funcs(template.FuncMap{"asset": assets.Path, "say": T}).
ParseFS(ui.Templates(), "*.html")).
// Then the app's own: the shell and this page's body.
ParseFS(appFS, "templates/layout.html", "templates/"+name+".html"))
diff --git a/internal/movement/templates/index.html b/internal/movement/templates/index.html
index dd9abab..c354740 100644
--- a/internal/movement/templates/index.html
+++ b/internal/movement/templates/index.html
@@ -1,3 +1,5 @@
+{{define "title"}}{{say "index.headline"}}{{end}}
{{define "content"}}
-<h1>Hello, World — this is a rastrillo app.</h1>
+<h1 class="rst-page-title">{{say "index.headline"}}</h1>
+<p>{{say "index.lede"}}</p>
{{end}}
diff --git a/internal/movementtest/browser_test.go b/internal/movementtest/browser_test.go
index 89d8a29..17830d6 100644
--- a/internal/movementtest/browser_test.go
+++ b/internal/movementtest/browser_test.go
@@ -42,7 +42,10 @@ func TestBrowserWalk(t *testing.T) {
t.Fatalf("db.Open: %v", err)
}
t.Cleanup(func() { d.Close() })
- mux, err := movement.App(d, origin, logger)
+ movement.SetLogger(logger)
+ env := testEnv()
+ env.Origin = origin // the rig picks its own localhost port
+ mux, err := movement.App(d, env)
if err != nil {
t.Fatalf("App: %v", err)
}
diff --git a/internal/movementtest/guards_test.go b/internal/movementtest/guards_test.go
new file mode 100644
index 0000000..c3dcc48
--- /dev/null
+++ b/internal/movementtest/guards_test.go
@@ -0,0 +1,147 @@
+package movementtest
+
+import (
+ "bytes"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+// The rules in this file are the ones that would otherwise depend on
+// somebody remembering them. AGENTS.md says: if you find yourself relying on
+// remembering a rule, make it a test instead.
+
+// TestOnlyLoggingFileImportsSlog keeps the sensitive pair unreachable from a
+// log call.
+//
+// Email and stance together are a political position attached to a named
+// person, and carlos logs is platform-held storage that cannot be grepped
+// clean afterwards. Pattern-matching for "a call that takes both an address
+// and a stance" is not decidable over arbitrary Go — wrappers, structs, a %v
+// of a Signup — so the boundary is drawn one level up: only logging.go may
+// reach the logger at all, and its surface takes typed reason codes rather
+// than strings or errors.
+//
+// cmd/ is out of scope. It logs its own startup failures before any request
+// exists, and it has no access to a Signup.
+func TestOnlyLoggingFileImportsSlog(t *testing.T) {
+ const pkg = "../movement"
+
+ err := filepath.Walk(pkg, func(path string, info os.FileInfo, err error) error {
+ if err != nil {
+ return err
+ }
+ if info.IsDir() || !strings.HasSuffix(path, ".go") {
+ return nil
+ }
+ if filepath.Base(path) == "logging.go" {
+ return nil
+ }
+ src, readErr := os.ReadFile(path)
+ if readErr != nil {
+ return readErr
+ }
+ if bytes.Contains(src, []byte(`"log/slog"`)) {
+ t.Errorf("%s imports log/slog directly.\n"+
+ "Route it through logging.go: the log surface takes typed Reason "+
+ "codes so an address and a stance cannot meet in one line.", path)
+ }
+ return nil
+ })
+ if err != nil {
+ t.Fatal(err)
+ }
+}
+
+// TestTemplatesCarryNoLiteralCopy enforces the copy table.
+//
+// Every user-facing string resolves through internal/copy, so that the whole
+// engine can be built and reviewed on a canary before copy review has
+// happened — and so that unreviewed words cannot reach production by being
+// quietly typed into markup. Markup, attribute values and template actions
+// are fine; sentences are not.
+func TestTemplatesCarryNoLiteralCopy(t *testing.T) {
+ dir := filepath.Join("..", "movement", "templates")
+ entries, err := os.ReadDir(dir)
+ if err != nil {
+ t.Skipf("no templates yet: %v", err)
+ }
+
+ for _, e := range entries {
+ if e.IsDir() || !strings.HasSuffix(e.Name(), ".html") {
+ continue
+ }
+ src, err := os.ReadFile(filepath.Join(dir, e.Name()))
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, run := range prose(string(src)) {
+ t.Errorf("%s contains literal copy: %q\n"+
+ "Move it into internal/copy and render it with {{t \"some.id\"}}.",
+ e.Name(), run)
+ }
+ }
+}
+
+// prose returns runs of three or more consecutive words left after template
+// actions, tags, comments and entities are removed. Three is the threshold
+// where a string stops looking like a class name and starts looking like a
+// sentence somebody wrote for a reader.
+func prose(src string) []string {
+ src = strip(src, "{{", "}}")
+ src = strip(src, "<!--", "-->")
+ src = strip(src, "<", ">")
+
+ var out []string
+ for _, line := range strings.Split(src, "\n") {
+ words := 0
+ var run []string
+ flush := func() {
+ if words >= 3 {
+ out = append(out, strings.Join(run, " "))
+ }
+ words, run = 0, nil
+ }
+ for _, f := range strings.Fields(line) {
+ f = strings.Trim(f, ".,:;!?\"'()")
+ if f != "" && isWord(f) {
+ words++
+ run = append(run, f)
+ continue
+ }
+ flush()
+ }
+ flush()
+ }
+ return out
+}
+
+func isWord(s string) bool {
+ for _, r := range s {
+ if (r < 'a' || r > 'z') && (r < 'A' || r > 'Z') && r != '-' && r != '\'' {
+ return false
+ }
+ }
+ return true
+}
+
+// strip removes every open..close span, including unterminated trailing ones.
+func strip(s, open, close string) string {
+ var b strings.Builder
+ for {
+ i := strings.Index(s, open)
+ if i < 0 {
+ b.WriteString(s)
+ return b.String()
+ }
+ b.WriteString(s[:i])
+ b.WriteString(" ")
+ rest := s[i+len(open):]
+ j := strings.Index(rest, close)
+ if j < 0 {
+ return b.String()
+ }
+ s = rest[j+len(close):]
+ }
+}
diff --git a/internal/movementtest/harness_test.go b/internal/movementtest/harness_test.go
index 2ebc1a5..96c5494 100644
--- a/internal/movementtest/harness_test.go
+++ b/internal/movementtest/harness_test.go
@@ -35,13 +35,24 @@ func newApp(t *testing.T) http.Handler {
t.Fatalf("db.Open: %v", err)
}
t.Cleanup(func() { d.Close() })
- mux, err := movement.App(d, testOrigin, logger)
+ movement.SetLogger(logger)
+ mux, err := movement.App(d, testEnv())
if err != nil {
t.Fatalf("App: %v", err)
}
return mux
}
+// testEnv is the configuration a test runs against. It is not the canonical
+// host, which matters: placeholder copy refuses to boot there, and every
+// test in this package runs against a build that still carries placeholders.
+func testEnv() movement.Env {
+ return movement.Env{
+ Origin: testOrigin,
+ InstanceKey: strings.Repeat("k", 64),
+ }
+}
+
func get(t *testing.T, h http.Handler, target string) *httptest.ResponseRecorder {
t.Helper()
rec := httptest.NewRecorder()
diff --git a/internal/movementtest/index_test.go b/internal/movementtest/index_test.go
index d7645f7..8fff9e6 100644
--- a/internal/movementtest/index_test.go
+++ b/internal/movementtest/index_test.go
@@ -19,7 +19,11 @@ func TestIndexRenders(t *testing.T) {
if rec.Code != http.StatusOK {
t.Fatalf("GET /: status %d, want 200", rec.Code)
}
- if !strings.Contains(rec.Body.String(), "this is a rastrillo app") {
+ // The heading comes from the copy table, not from the template, so this
+ // asserts the wiring rather than the words. When copy review replaces
+ // the placeholder, this test does not need to change — which is the
+ // point of holding copy separately.
+ if !strings.Contains(rec.Body.String(), movement.T("index.headline")) {
t.Errorf("GET /: page is missing its heading:\n%s", rec.Body.String())
}
}