diff --git a/authn/provenance.go b/authn/provenance.go index 38735a9b6dfca9ab08b3794c57ceee9f714a40a2..66b9c4fa6ce5c5f5a3cb37d8c0c157dec0c07daf 100644 --- a/authn/provenance.go +++ b/authn/provenance.go @@ -2,11 +2,11 @@ package authn import ( "fmt" - "net/url" "strings" "unicode/utf8" "github.com/vaughan0/go-ini" + "sourcecraft.dev/bigbes/sr-ht-ecore/instconf" "sourcecraft.dev/bigbes/sr-ht-spec/core" ) @@ -87,6 +87,13 @@ // worked example shows agent@srht.bigb.es (the bare cookie domain) rather than // agent@spec.srht.bigb.es; the design never says where that address comes from, // and deriving it from our own origin is the only rule that needs no operator // input. A caller that wants the bare domain sets Instance.AgentEmail directly. +// +// The host is instconf.OriginHost and deliberately not instconf.OriginAuthority, +// whose doc names a synthesized email domain among its callers: the design pins +// this address at "agent@", and a port in the +// domain half — agent@localhost:5091 on a development instance — is not a +// mailbox. The port distinguishes two endpoints, which is what an audience needs +// and what an address nobody may mail does not. func InstanceFromConfig(conf ini.File) (Instance, error) { ownerName, ok := conf.Get("sr.ht", "owner-name") if !ok { @@ -96,19 +103,18 @@ ownerEmail, ok := conf.Get("sr.ht", "owner-email") if !ok { return Instance{}, fmt.Errorf("%w: [sr.ht] owner-email", ErrMissingConfig) } - origin, ok := conf.Get(ConfigSection, "origin") - if !ok { + origin := instconf.ExternalOrigin(conf, ConfigSection) + if origin == "" { return Instance{}, fmt.Errorf("%w: [%s] origin", ErrMissingConfig, ConfigSection) } - u, err := url.Parse(origin) - if err != nil { - return Instance{}, fmt.Errorf("%w: [%s] origin %q is not a URL: %v", - ErrMissingConfig, ConfigSection, origin, err) - } - host := u.Hostname() + // "" covers both halves of what this used to report separately — an origin + // that does not parse as a URL and one that parses to no host, such as a + // scheme-less "spec.srht.bigb.es", which is a path. Neither can name the + // domain of a mailbox, and the operator's fix is the same line either way. + host := instconf.OriginHost(origin) if host == "" { - return Instance{}, fmt.Errorf("%w: [%s] origin %q has no host", + return Instance{}, fmt.Errorf("%w: [%s] origin %q names no host", ErrMissingConfig, ConfigSection, origin) } diff --git a/mcpsrv/mcpsrv.go b/mcpsrv/mcpsrv.go index f59e9c3185dff543c52562d851c1b1e95c7fa878..132a4774bbf160124f655cfa92843f96424d2cd7 100644 --- a/mcpsrv/mcpsrv.go +++ b/mcpsrv/mcpsrv.go @@ -59,10 +59,10 @@ "errors" "log/slog" "net" "net/http" - "net/url" "strings" "github.com/modelcontextprotocol/go-sdk/mcp" + "sourcecraft.dev/bigbes/sr-ht-ecore/instconf" "sourcecraft.dev/bigbes/sr-ht-spec/authn" "sourcecraft.dev/bigbes/sr-ht-spec/search" @@ -251,7 +251,13 @@ // An empty or unparseable origin leaves the endpoint unguarded, so it says so // loudly. A misconfigured origin must not quietly become the difference between // protected and open — that is the class of failure nobody discovers. func allowHosts(next http.Handler, origin string) http.Handler { - want := originHost(origin) + // instconf.OriginHost and not a local parse: this is the reading of an + // origin, and it is the same one authn's mailbox derivation and service's + // config validation make. An origin nobody can extract a host from answers + // "" here — never a guess such as "localhost", which would silently make + // every malformed origin agree with a local client — and "" is what turns + // the guard off, loudly, below. + want := instconf.OriginHost(origin) if want == "" { slog.Warn("mcpsrv: no usable origin configured; Host validation on /mcp is DISABLED") return next @@ -326,18 +332,6 @@ return true default: return false } -} - -// originHost extracts the hostname from a configured origin URL. -func originHost(origin string) string { - if origin == "" { - return "" - } - u, err := url.Parse(origin) - if err != nil { - return "" - } - return u.Hostname() } // clampLimit applies the hit-count policy: unset defers to search's own diff --git a/service/bearer.go b/service/bearer.go index 8b508cd95f426db0b097517c1e28f269a8d08a16..541810af3ed187b442fb55f76ecc8b244e64f563 100644 --- a/service/bearer.go +++ b/service/bearer.go @@ -11,6 +11,7 @@ "sourcecraft.dev/bigbes/sr-ht-core/auth" "sourcecraft.dev/bigbes/sr-ht-core/config" "sourcecraft.dev/bigbes/sr-ht-core/database" "sourcecraft.dev/bigbes/sr-ht-ecore/bearer" + "sourcecraft.dev/bigbes/sr-ht-ecore/instconf" "sourcecraft.dev/bigbes/sr-ht-spec/authn" "sourcecraft.dev/bigbes/sr-ht-spec/db" @@ -59,11 +60,12 @@ // instance — over HTTP and over `git push` alike — so the absence fails startup, // where an operator is looking, instead of surfacing one request at a time as an // unexplained 503. // -// The origin is read in its internal form (GetOrigin's external=false), so the -// revocation check of SPEC ch. 6 step 4 crosses the docker network directly -// instead of going out through the reverse proxy and back in. +// The origin is read in its internal form — instconf.InternalOrigin, which is +// named rather than a bool, so that the reading cannot be flipped invisibly — +// and so the revocation check of SPEC ch. 6 step 4 crosses the docker network +// directly instead of going out through the reverse proxy and back in. func instancePlane(conf ini.File, q db.Querier) (authn.ResolverOption, error) { - origin := config.GetOrigin(conf, TokensSection, false) + origin := instconf.InternalOrigin(conf, TokensSection) if origin == "" { return nil, fmt.Errorf( "service: no [%s] origin in config.ini; spec.sr.ht issues no agent credential of "+ diff --git a/service/service.go b/service/service.go index 44f7d19d81d03060032b25ff37353de966ac1fb9..4c035d2baba4b3c85e68b01ba284f78ad4ca92f4 100644 --- a/service/service.go +++ b/service/service.go @@ -9,6 +9,7 @@ "strings" "time" "github.com/vaughan0/go-ini" + "sourcecraft.dev/bigbes/sr-ht-ecore/instconf" "sourcecraft.dev/bigbes/sr-ht-spec/authn" "sourcecraft.dev/bigbes/sr-ht-spec/db" @@ -135,14 +136,16 @@ return v } cfg := Config{ - Repos: get(ConfigSection, "repos"), - Cache: get(ConfigSection, "cache"), - Origin: get(ConfigSection, "origin"), + Repos: get(ConfigSection, "repos"), + Cache: get(ConfigSection, "cache"), + // One canonical spelling of the origin, so a proposal URL built from it + // never grows a double slash and never differs between two callers — + // instconf's, which strips every trailing slash rather than the one + // TrimSuffix took, and which is the same spelling authn derives the agent + // mailbox from and mcpsrv compares a Host header against. + Origin: instconf.CanonicalOrigin(get(ConfigSection, "origin")), ConnectionString: get(ConfigSection, "connection-string"), } - // One canonical spelling of the origin, so a proposal URL built from it - // never grows a double slash and never differs between two callers. - cfg.Origin = strings.TrimSuffix(cfg.Origin, "/") // Read for their presence only; authn.InstanceFromConfig is what turns them // into identities, and it must not be reached with a key missing or it @@ -184,13 +187,16 @@ } requireAbs("repos", c.Repos) requireAbs("cache", c.Cache) - switch u, err := url.Parse(c.Origin); { + // instconf.OriginHost answers "" for both an origin that does not parse and + // one that parses to no host, which are one problem to the operator and one + // message here. The scheme is still read locally: instconf deliberately + // exposes the host and the authority and no scheme accessor, and "an origin + // this service will redirect a browser to must be http or https" is + // spec.sr.ht's own rule rather than the instance's. + switch u, _ := url.Parse(c.Origin); { case c.Origin == "": problems = append(problems, fmt.Sprintf("[%s] origin is empty", ConfigSection)) - case err != nil: - problems = append(problems, fmt.Sprintf("[%s] origin %q is not a URL: %v", - ConfigSection, c.Origin, err)) - case u.Hostname() == "": + case instconf.OriginHost(c.Origin) == "": problems = append(problems, fmt.Sprintf("[%s] origin %q has no host", ConfigSection, c.Origin)) case u.Scheme != "http" && u.Scheme != "https": problems = append(problems, fmt.Sprintf("[%s] origin %q must be http or https", diff --git a/web/server.go b/web/server.go index af0ba42e45dba809b9b0b8b3794a40938f8efde3..0dbe43734a1fed8de7171283f6360cf85e917292 100644 --- a/web/server.go +++ b/web/server.go @@ -87,9 +87,9 @@ "log/slog" "net/http" "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/instconf" "sourcecraft.dev/bigbes/sr-ht-ecore/pages" "sourcecraft.dev/bigbes/sr-ht-spec/authn" @@ -146,9 +146,12 @@ // 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 + // tokensOrigin is [tokens.sr.ht] origin in its *external* form — + // instconf.ExternalOrigin, named rather than a bool, because the wrong + // reading here sends a browser to an address only the daemon can reach and + // nothing at the call site would say so. 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 // instance config has no such section, which handleTokens answers rather // than papers over with a redirect to nowhere. tokensOrigin string @@ -225,7 +228,7 @@ resolver: opts.Resolver, renderer: doc.NewRenderer(), chromeSvc: chromeSvc, pages: set, - tokensOrigin: config.GetOrigin(opts.Conf, tokensSection, true), + tokensOrigin: instconf.ExternalOrigin(opts.Conf, tokensSection), } // 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