@@ 0,0 1,307 @@
+// Package instconf is one reading of the origins in a self-hosted SourceHut
+// instance's shared config.ini — the "where does this service live" strings
+// that every custom service on the instance has to agree about.
+//
+// It exists because they did not agree. Five services and this library each
+// grew their own copy of the same three lines, and the copies drifted in ways
+// that are invisible until something breaks far from the config: one service
+// canonicalized an origin with strings.TrimRight(o, "/") and another with
+// strings.TrimSuffix(o, "/"), so a config.ini carrying "https://x//" produced
+// two different origins in two daemons that must produce the same string when
+// one of them checks a browser-supplied Origin header against it. One repo held
+// two origin-to-host extractors that disagreed about a malformed origin — one
+// returned an error, the other quietly answered "localhost". A third copy feeds
+// the DNS-rebinding guard on an MCP endpoint, where an empty host means the
+// guard turns itself off. That is not a place for a fourth spelling.
+//
+// So: one canonicalization, one host extraction, one internal/external
+// distinction, used by everyone.
+//
+// The origin vocabulary this package reads is upstream sourcehut's, and the
+// accessors correspond to core-go's config.GetOrigin and config.GetAPI:
+//
+// - [ExternalOrigin] — [<svc>] origin, the address a browser is sent to.
+// - [InternalOrigin] — [<svc>] internal-origin, falling back to origin: the
+// address a daemon dials from inside the instance's network.
+// - [InternalAPIOrigin] — the four-key ladder api-internal-origin,
+// internal-origin, api-origin, origin, for a service calling a sibling's
+// GraphQL API.
+//
+// Two named accessors rather than one taking an external bool, deliberately.
+// The donors spelled it config.GetOrigin(conf, svc, true) and
+// config.GetOrigin(conf, svc, false) in different files, each with a comment
+// explaining which was which, and a flipped flag is invisible at the call site:
+// it shows up as a browser redirected to an address only the daemon can reach,
+// or as a daemon dialing out through the public load balancer.
+//
+// Everything returned is canonical — whitespace-trimmed and stripped of every
+// trailing slash — so a URL built by joining an origin with a path never grows
+// a double slash and two callers never hold two spellings of the same address.
+// An absent key, an absent section and a present-but-blank value all read as
+// "", which the caller is expected to treat as "not configured": see [Require]
+// for reporting that in one pass rather than one restart per missing key.
+package instconf
+
+import (
+ "errors"
+ "fmt"
+ "net/url"
+ "strings"
+
+ "github.com/vaughan0/go-ini"
+)
+
+// ErrIncompleteConfig is the sentinel behind every error [Require] returns, so
+// a daemon can tell "the operator has not finished configuring me" apart from
+// the failures that come later.
+var ErrIncompleteConfig = errors.New("incomplete configuration")
+
+// apiOriginKeys is the internal API-origin ladder, in preference order. It is
+// upstream core-go's own candidate list for config.GetAPI with external=false,
+// restated here because config.GetAPI panics when none of them is set, which is
+// no way to tell an operator about a missing config key.
+var apiOriginKeys = []string{
+ "api-internal-origin",
+ "internal-origin",
+ "api-origin",
+ "origin",
+}
+
+// APIOriginKeys returns the key names [InternalAPIOrigin] consults, in
+// preference order. Mostly useful for [NeedAny], so that a startup check and
+// the lookup itself cannot drift apart.
+func APIOriginKeys() []string {
+ return append([]string(nil), apiOriginKeys...)
+}
+
+// CanonicalOrigin returns the one spelling of an origin: surrounding whitespace
+// removed, then every trailing slash removed. "" stays "".
+//
+// Both trims matter, and both come from a donor that had been bitten:
+//
+// - TrimSpace, because a value can reach this function from somewhere other
+// than the ini parser (a test fixture, a flag, a config assembled in code),
+// and " https://x" does not even parse as a URL.
+// - TrimRight over TrimSuffix, because TrimSuffix removes one slash and
+// leaves "https://x/" from "https://x//". The donors used one each. This is
+// the stricter reading, and it is the one the comparison callers need: an
+// Origin header from a browser never carries a trailing slash, so an origin
+// that kept one silently fails every equality check made against it.
+//
+// Only trailing slashes are touched. The scheme, host, port and any path
+// prefix are left exactly as configured — an instance that serves a service
+// under https://example.org/git means it.
+func CanonicalOrigin(origin string) string {
+ return strings.TrimRight(strings.TrimSpace(origin), "/")
+}
+
+// ExternalOrigin returns the canonical external origin of a service: the
+// address a browser is sent to, from [<section>] origin.
+//
+// This is the origin that belongs in a page, a redirect, a webhook payload or
+// an email — anything a user's own client will follow. It returns "" when the
+// section or the key is missing, or the value is blank.
+func ExternalOrigin(conf ini.File, section string) string {
+ return CanonicalOrigin(lookup(conf, section, "origin"))
+}
+
+// InternalOrigin returns the canonical origin a daemon should dial to reach a
+// service from inside the instance: [<section>] internal-origin when set,
+// otherwise [<section>] origin.
+//
+// It is the address of the same service, not the same address: an instance may
+// route service-to-service traffic over a private network, and on such an
+// instance the external origin resolves to a load balancer the daemon cannot
+// or should not use. Never put this string in front of a user.
+func InternalOrigin(conf ini.File, section string) string {
+ if v := lookup(conf, section, "internal-origin"); v != "" {
+ return CanonicalOrigin(v)
+ }
+ return CanonicalOrigin(lookup(conf, section, "origin"))
+}
+
+// InternalAPIOrigin returns the canonical origin at which a service's GraphQL
+// API can be reached from inside the instance, and whether one is configured at
+// all. The returned origin does not include the /query path.
+//
+// It walks [APIOriginKeys] in order, taking the first key that is present and
+// non-blank. The ladder exists because an instance may put the API behind a
+// different address than the web UI, and may or may not have a separate
+// internal route to either; every service that calls a sibling's API has to
+// walk it, because core-go's config.GetAPI panics when it reaches the end of
+// the ladder with nothing.
+//
+// The bool is the point: it turns that panic into a decision the caller makes
+// at startup, alongside every other missing key, rather than a stack trace on
+// the first authorization request.
+func InternalAPIOrigin(conf ini.File, section string) (string, bool) {
+ for _, key := range apiOriginKeys {
+ if v := lookup(conf, section, key); v != "" {
+ return CanonicalOrigin(v), true
+ }
+ }
+ return "", false
+}
+
+// OriginHost returns the host name of an origin, without any port: the string
+// to compare a request's Host header against. It returns "" when the origin is
+// blank, does not parse as a URL, or parses to no host at all — which is what a
+// scheme-less value such as "example.org" does, since a URL without a scheme is
+// a path.
+//
+// "" means "this origin names no host", and callers on a security path must
+// treat it as a configuration error rather than as permission to skip a check.
+// One donor uses this value for the DNS-rebinding guard on an MCP endpoint and
+// disables the guard when it comes back empty (loudly, at warn level, which is
+// the only reason that is defensible). Another donor's copy answered "localhost"
+// for anything it could not parse, which is worse: it is a guess that looks like
+// an answer, and it silently makes every malformed origin agree with a local
+// client.
+//
+// The host is returned exactly as configured, case included, because it is also
+// what identifies the endpoint elsewhere. Host names are case-insensitive, so
+// compare with strings.EqualFold and not ==.
+//
+// A host name is not an origin, so the two live under two names: use
+// [CanonicalOrigin] when what is wanted back is an address to fetch.
+func OriginHost(origin string) string {
+ u, err := url.Parse(CanonicalOrigin(origin))
+ if err != nil {
+ return ""
+ }
+ return u.Hostname()
+}
+
+// OriginAuthority returns the authority of an origin — host[:port], with an
+// IPv6 host still bracketed — or "" under exactly the conditions [OriginHost]
+// returns "".
+//
+// It is the other half of a disagreement between two copies in one repo: a
+// Host-header check wants the name alone, because it compares against a request
+// Host whose port it has already stripped, while a JWT audience, a sealed URL
+// or a synthesized email domain wants the authority that actually identifies
+// the endpoint — https://x:8080 and https://x:9090 are two audiences. Neither
+// is the general answer, so both are here and the caller names which one it
+// means.
+func OriginAuthority(origin string) string {
+ u, err := url.Parse(CanonicalOrigin(origin))
+ if err != nil {
+ return ""
+ }
+ if u.Hostname() == "" {
+ return ""
+ }
+ return u.Host
+}
+
+// Key is one configuration requirement: a section, and the key names that
+// satisfy it. More than one name means an alternation — any of them will do —
+// which is how the internal API-origin ladder is expressed. Build one with
+// [Need] or [NeedAny].
+type Key struct {
+ Section string
+ Names []string
+}
+
+// Need is a requirement for one named key in a section.
+func Need(section, name string) Key {
+ return Key{Section: section, Names: []string{name}}
+}
+
+// NeedAny is a requirement satisfied by any one of several keys in a section,
+// e.g. NeedAny("git.sr.ht", instconf.APIOriginKeys()...).
+func NeedAny(section string, names ...string) Key {
+ return Key{Section: section, Names: names}
+}
+
+// String renders the requirement the way an operator has to read it back into
+// config.ini: "[git.sr.ht] origin", or "[git.sr.ht] one of api-internal-origin,
+// internal-origin, api-origin, origin".
+func (k Key) String() string {
+ switch len(k.Names) {
+ case 0:
+ return fmt.Sprintf("[%s] <no key>", k.Section)
+ case 1:
+ return fmt.Sprintf("[%s] %s", k.Section, k.Names[0])
+ default:
+ return fmt.Sprintf("[%s] one of %s", k.Section, strings.Join(k.Names, ", "))
+ }
+}
+
+// satisfied reports whether the config has a non-blank value for any of the
+// key's names. A requirement with no names is unsatisfiable by construction,
+// which surfaces the programming mistake instead of silently passing.
+func (k Key) satisfied(conf ini.File) bool {
+ for _, name := range k.Names {
+ if lookup(conf, k.Section, name) != "" {
+ return true
+ }
+ }
+ return false
+}
+
+// MissingKeysError reports every configuration key a caller required and did
+// not find. It wraps [ErrIncompleteConfig].
+type MissingKeysError struct {
+ Keys []Key
+}
+
+// Error lists every missing key on one line.
+func (e *MissingKeysError) Error() string {
+ return fmt.Sprintf("%s; missing required keys: %s",
+ ErrIncompleteConfig, strings.Join(e.Strings(), ", "))
+}
+
+// Unwrap makes errors.Is(err, ErrIncompleteConfig) work.
+func (e *MissingKeysError) Unwrap() error { return ErrIncompleteConfig }
+
+// Strings renders the missing keys one per element, for a structured log
+// attribute that keeps them a list — a slog handler will then render them as a
+// list, and the operator gets every gap at once instead of one per line.
+func (e *MissingKeysError) Strings() []string {
+ out := make([]string, 0, len(e.Keys))
+ for _, k := range e.Keys {
+ out = append(out, k.String())
+ }
+ return out
+}
+
+// Require checks that every given key is present and non-blank, and reports
+// *all* the missing ones in a single [*MissingKeysError]. It returns nil when
+// nothing is missing.
+//
+// Reporting all of them is the entire point, and it is the reason this is a
+// function rather than five ifs at each daemon's startup: a daemon that fatals
+// on the first gap it finds costs the operator one restart per missing key, and
+// an operator editing config.ini wants the whole list in front of them. It also
+// wants to run before anything reaches core-go's config.GetAPI or
+// crypto.InitCrypto, both of which panic on missing configuration with a
+// message aimed at a programmer.
+//
+// A present-but-blank value counts as missing: "origin =" in config.ini is a
+// half-finished edit, not a configured empty origin.
+func Require(conf ini.File, keys ...Key) error {
+ var missing []Key
+ for _, k := range keys {
+ if !k.satisfied(conf) {
+ missing = append(missing, k)
+ }
+ }
+ if len(missing) == 0 {
+ return nil
+ }
+ return &MissingKeysError{Keys: missing}
+}
+
+// lookup reads one key, treating a present-but-blank value as absent and
+// trimming what it returns. The ini parser already trims, but a File can be
+// built in code — every test fixture in this repo does — and a lookup that
+// depends on which of the two produced the map is exactly the kind of
+// disagreement this package exists to remove.
+func lookup(conf ini.File, section, key string) string {
+ v, ok := conf.Get(section, key)
+ if !ok {
+ return ""
+ }
+ return strings.TrimSpace(v)
+}
@@ 0,0 1,296 @@
+package instconf_test
+
+import (
+ "errors"
+ "strings"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+ "github.com/vaughan0/go-ini"
+
+ "sourcecraft.dev/bigbes/sr-ht-ecore/instconf"
+)
+
+// parse builds a File the way a daemon gets one, so the tests exercise the
+// parser's own trimming rather than a map the test wrote by hand.
+func parse(t *testing.T, text string) ini.File {
+ t.Helper()
+ conf, err := ini.Load(strings.NewReader(text))
+ require.NoError(t, err)
+ return conf
+}
+
+func TestCanonicalOriginTrimsTrailingSlashesAndSpace(t *testing.T) {
+ for name, tc := range map[string]struct{ in, want string }{
+ "empty stays empty": {"", ""},
+ "blank stays empty": {" ", ""},
+ "no slash untouched": {"https://x.example.org", "https://x.example.org"},
+ "one trailing slash": {"https://x.example.org/", "https://x.example.org"},
+ "several slashes": {"https://x.example.org///", "https://x.example.org"},
+ "trailing space": {"https://x.example.org ", "https://x.example.org"},
+ "leading space": {" https://x.example.org", "https://x.example.org"},
+ "space then slashes": {" https://x.example.org// ", "https://x.example.org"},
+ "port kept": {"https://x.example.org:8443/", "https://x.example.org:8443"},
+ "path prefix kept": {"https://example.org/git/", "https://example.org/git"},
+ "scheme-less untouched": {"x.example.org/", "x.example.org"},
+ } {
+ t.Run(name, func(t *testing.T) {
+ assert.Equal(t, tc.want, instconf.CanonicalOrigin(tc.in))
+ })
+ }
+}
+
+// The donors disagreed here: one canonicalized with strings.TrimSuffix, which
+// leaves "https://x/" from "https://x//", and one with strings.TrimRight, which
+// does not. This pins the strict reading, because the whole point is that the
+// string compares equal to a browser's Origin header.
+func TestCanonicalOriginBeatsTrimSuffix(t *testing.T) {
+ const doubled = "https://x.example.org//"
+ assert.Equal(t, "https://x.example.org/", strings.TrimSuffix(doubled, "/"),
+ "sanity: TrimSuffix is the behaviour being overruled")
+ assert.Equal(t, "https://x.example.org", instconf.CanonicalOrigin(doubled))
+}
+
+func TestExternalOriginReadsOriginOnly(t *testing.T) {
+ conf := parse(t, `
+[git.sr.ht]
+origin=https://git.example.org/
+internal-origin=http://git.internal:5001
+`)
+ assert.Equal(t, "https://git.example.org", instconf.ExternalOrigin(conf, "git.sr.ht"),
+ "internal-origin must never reach a browser")
+}
+
+func TestExternalOriginMissing(t *testing.T) {
+ conf := parse(t, `
+[git.sr.ht]
+internal-origin=http://git.internal:5001
+
+[meta.sr.ht]
+origin=
+`)
+ assert.Empty(t, instconf.ExternalOrigin(conf, "git.sr.ht"), "key absent")
+ assert.Empty(t, instconf.ExternalOrigin(conf, "meta.sr.ht"), "key present but blank")
+ assert.Empty(t, instconf.ExternalOrigin(conf, "nope.sr.ht"), "section absent")
+ assert.Empty(t, instconf.ExternalOrigin(nil, "git.sr.ht"), "no config at all")
+}
+
+func TestInternalOriginPrefersInternal(t *testing.T) {
+ conf := parse(t, `
+[git.sr.ht]
+origin=https://git.example.org
+internal-origin=http://git.internal:5001/
+`)
+ assert.Equal(t, "http://git.internal:5001", instconf.InternalOrigin(conf, "git.sr.ht"))
+}
+
+func TestInternalOriginFallsBackToOrigin(t *testing.T) {
+ conf := parse(t, `
+[git.sr.ht]
+origin=https://git.example.org/
+
+[meta.sr.ht]
+origin=https://meta.example.org
+internal-origin=
+`)
+ assert.Equal(t, "https://git.example.org", instconf.InternalOrigin(conf, "git.sr.ht"),
+ "internal-origin absent")
+ assert.Equal(t, "https://meta.example.org", instconf.InternalOrigin(conf, "meta.sr.ht"),
+ "internal-origin present but blank must not shadow origin")
+ assert.Empty(t, instconf.InternalOrigin(conf, "nope.sr.ht"))
+}
+
+func TestInternalAPIOriginLadder(t *testing.T) {
+ full := `
+[git.sr.ht]
+api-internal-origin=http://git.internal:5101/
+internal-origin=http://git.internal:5001
+api-origin=https://api.git.example.org
+origin=https://git.example.org
+`
+ for name, tc := range map[string]struct {
+ drop []string
+ want string
+ }{
+ "api-internal-origin wins": {nil, "http://git.internal:5101"},
+ "then internal-origin": {
+ []string{"api-internal-origin"}, "http://git.internal:5001"},
+ "then api-origin": {
+ []string{"api-internal-origin", "internal-origin"}, "https://api.git.example.org"},
+ "then origin": {
+ []string{"api-internal-origin", "internal-origin", "api-origin"},
+ "https://git.example.org"},
+ } {
+ t.Run(name, func(t *testing.T) {
+ text := full
+ for _, k := range tc.drop {
+ text = dropKey(t, text, k)
+ }
+ got, ok := instconf.InternalAPIOrigin(parse(t, text), "git.sr.ht")
+ require.True(t, ok)
+ assert.Equal(t, tc.want, got)
+ })
+ }
+}
+
+func TestInternalAPIOriginNotConfigured(t *testing.T) {
+ conf := parse(t, `
+[git.sr.ht]
+repos=/var/lib/git
+
+[meta.sr.ht]
+origin=
+`)
+ got, ok := instconf.InternalAPIOrigin(conf, "git.sr.ht")
+ assert.False(t, ok, "no candidate key at all")
+ assert.Empty(t, got)
+
+ got, ok = instconf.InternalAPIOrigin(conf, "meta.sr.ht")
+ assert.False(t, ok, "a blank origin is not a configured API origin")
+ assert.Empty(t, got)
+
+ got, ok = instconf.InternalAPIOrigin(conf, "nope.sr.ht")
+ assert.False(t, ok, "section absent")
+ assert.Empty(t, got)
+}
+
+func TestAPIOriginKeysMatchesTheLadder(t *testing.T) {
+ want := []string{"api-internal-origin", "internal-origin", "api-origin", "origin"}
+ assert.Equal(t, want, instconf.APIOriginKeys())
+
+ keys := instconf.APIOriginKeys()
+ keys[0] = "clobbered"
+ assert.Equal(t, want, instconf.APIOriginKeys(), "the exported ladder must not be writable")
+}
+
+func TestOriginHostAndAuthority(t *testing.T) {
+ for name, tc := range map[string]struct{ in, host, authority string }{
+ "plain": {"https://git.example.org", "git.example.org", "git.example.org"},
+ "trailing slash": {"https://git.example.org/", "git.example.org", "git.example.org"},
+ "several slashes": {"https://git.example.org//", "git.example.org", "git.example.org"},
+ "surrounding space": {" https://git.example.org ", "git.example.org", "git.example.org"},
+ "with port": {"https://git.example.org:8443", "git.example.org", "git.example.org:8443"},
+ "path prefix": {"https://example.org/git", "example.org", "example.org"},
+ "ipv6 with port": {"http://[::1]:8080", "::1", "[::1]:8080"},
+ "scheme relative": {"//git.example.org", "git.example.org", "git.example.org"},
+ "case as configured": {"https://Git.Example.ORG", "Git.Example.ORG", "Git.Example.ORG"},
+
+ // Everything below has no host to report. "" is the answer in every
+ // case; a caller on a security path must not read it as "allow".
+ "empty": {"", "", ""},
+ "blank": {" ", "", ""},
+ "scheme-less": {"git.example.org", "", ""},
+ "bare word": {"not-a-url", "", ""},
+ "no scheme": {"://git.example.org", "", ""},
+ "space inside": {"https://git example.org", "", ""},
+ "bad escape": {"https://example.org/%zz", "", ""},
+ "unclosed ipv6": {"http://[::1", "", ""},
+ } {
+ t.Run(name, func(t *testing.T) {
+ assert.Equal(t, tc.host, instconf.OriginHost(tc.in), "OriginHost")
+ assert.Equal(t, tc.authority, instconf.OriginAuthority(tc.in), "OriginAuthority")
+ })
+ }
+}
+
+// The donor that answered "localhost" for an unparseable origin made every
+// malformed config agree with a local client. This package refuses to guess.
+func TestOriginHostNeverInventsLocalhost(t *testing.T) {
+ for _, bad := range []string{"", " ", "not-a-url", "://x", "http://[::1"} {
+ assert.Empty(t, instconf.OriginHost(bad), "origin %q", bad)
+ assert.Empty(t, instconf.OriginAuthority(bad), "origin %q", bad)
+ }
+}
+
+func TestRequireSatisfied(t *testing.T) {
+ conf := parse(t, `
+[sr.ht]
+network-key=abc
+
+[webhooks]
+private-key=def
+
+[git.sr.ht]
+internal-origin=http://git.internal:5001
+
+[compare.sr.ht]
+origin=https://compare.example.org
+`)
+ err := instconf.Require(conf,
+ instconf.Need("sr.ht", "network-key"),
+ instconf.Need("webhooks", "private-key"),
+ instconf.Need("compare.sr.ht", "origin"),
+ instconf.NeedAny("git.sr.ht", instconf.APIOriginKeys()...),
+ )
+ assert.NoError(t, err)
+ assert.NoError(t, instconf.Require(conf), "no requirements is not a failure")
+}
+
+func TestRequireReportsEveryMissingKeyAtOnce(t *testing.T) {
+ conf := parse(t, `
+[sr.ht]
+network-key=abc
+
+[meta.sr.ht]
+origin=
+
+[git.sr.ht]
+repos=/var/lib/git
+`)
+ err := instconf.Require(conf,
+ instconf.Need("sr.ht", "network-key"), // present
+ instconf.Need("webhooks", "private-key"), // section absent
+ instconf.Need("meta.sr.ht", "origin"), // present but blank
+ instconf.Need("compare.sr.ht", "origin"), // key absent
+ instconf.NeedAny("git.sr.ht", instconf.APIOriginKeys()...), // ladder empty
+ )
+ require.Error(t, err)
+ require.ErrorIs(t, err, instconf.ErrIncompleteConfig)
+
+ var missing *instconf.MissingKeysError
+ require.ErrorAs(t, err, &missing)
+ assert.Equal(t, []string{
+ "[webhooks] private-key",
+ "[meta.sr.ht] origin",
+ "[compare.sr.ht] origin",
+ "[git.sr.ht] one of api-internal-origin, internal-origin, api-origin, origin",
+ }, missing.Strings(), "every gap in one pass, in the order asked")
+
+ msg := err.Error()
+ assert.Contains(t, msg, "incomplete configuration")
+ assert.Contains(t, msg, "[compare.sr.ht] origin")
+ assert.NotContains(t, msg, "network-key", "a satisfied key must not be reported")
+}
+
+func TestRequireNamelessKeyIsUnsatisfiable(t *testing.T) {
+ conf := parse(t, "[sr.ht]\nnetwork-key=abc\n")
+ err := instconf.Require(conf, instconf.NeedAny("sr.ht"))
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "[sr.ht] <no key>")
+}
+
+// dropKey removes an "key=..." line from an ini fixture.
+func dropKey(t *testing.T, text, key string) string {
+ t.Helper()
+ var kept []string
+ var dropped bool
+ for _, line := range strings.Split(text, "\n") {
+ if strings.HasPrefix(line, key+"=") {
+ dropped = true
+ continue
+ }
+ kept = append(kept, line)
+ }
+ require.True(t, dropped, "fixture has no %q line", key)
+ return strings.Join(kept, "\n")
+}
+
+// The sentinel is what a daemon matches on to tell "not configured yet" apart
+// from the failures that come after startup, so check it with errors.Is
+// directly rather than only through testify.
+func TestErrIncompleteConfigIsTheSentinel(t *testing.T) {
+ err := instconf.Require(nil, instconf.Need("sr.ht", "network-key"))
+ require.Error(t, err)
+ assert.True(t, errors.Is(err, instconf.ErrIncompleteConfig))
+}