9 files changed, 224 insertions(+), 12 deletions(-)

M assets/assets.go
M assets/assets_test.go
M bearer/status.go
M bearer/status_test.go
M instconf/instconf.go
M logging/logging.go
M logging/logging_test.go
M pages/error.go
A pages/error_test.go

@@ 43,6 43,7 @@ import (
 	"fmt"
 	"io/fs"
 	"net/http"
+	"os"
 	"path"
 	"regexp"
 	"strings"


@@ 313,3 314,25 @@ func (w *writer) stamp() {
 	w.Header().Set("Cache-Control", w.cacheControl)
 	w.Header().Del("Vary")
 }
+
+// DirFS is os.DirFS for a configured directory, and an empty filesystem for an
+// unconfigured one.
+//
+// os.DirFS("") does not mean "this build ships no assets". It resolves every
+// name against the filesystem root, so one unset config key turns a static
+// handler into a reader of the host — reachable, on this instance, by leaving
+// a single line out of config.ini. The guard is four lines that nobody writes
+// until they have seen it happen, which is the argument for it living here.
+func DirFS(dir string) fs.FS {
+	if dir == "" {
+		return emptyFS{}
+	}
+	return os.DirFS(dir)
+}
+
+// emptyFS is a filesystem in which nothing exists.
+type emptyFS struct{}
+
+func (emptyFS) Open(name string) (fs.File, error) {
+	return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist}
+}

@@ 290,3 290,24 @@ func TestNormalizePrefixIsWhatKeepsTheHrefAndTheMountTogether(t *testing.T) {
 		assert.Equal(t, want, assets.NormalizePrefix(given), given)
 	}
 }
+
+// TestDirFSRefusesToServeTheFilesystemRoot pins the one-key hole: os.DirFS("")
+// resolves every name against /, so an unset static directory would turn the
+// handler into a reader of the host.
+func TestDirFSRefusesToServeTheFilesystemRoot(t *testing.T) {
+	empty := assets.DirFS("")
+	_, err := fs.ReadFile(empty, "etc/passwd")
+	require.Error(t, err)
+	assert.ErrorIs(t, err, fs.ErrNotExist)
+
+	names, err := fs.Glob(empty, "*")
+	require.NoError(t, err)
+	assert.Empty(t, names, "nothing exists in it, so a glob finds nothing")
+
+	// A configured directory behaves as os.DirFS does.
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(dir+"/main.min.0badc0de.css", []byte("body{}"), 0o644))
+	got, err := fs.ReadFile(assets.DirFS(dir), "main.min.0badc0de.css")
+	require.NoError(t, err)
+	assert.Equal(t, "body{}", string(got))
+}

@@ 48,6 48,35 @@ func StatusFor(err error) int {
 	}
 }
 
+// IsRefusal reports whether err is one this package decided — that is, whether
+// StatusFor's answer means anything for it.
+//
+// A service's own resolver returns more than bearer's vocabulary: the Postgres
+// lookup it had to make, a context that expired, a bug. Handing those to
+// StatusFor would answer 401 through its default arm, which is right for a
+// credential and wrong for a database that did not answer — a caller told its
+// token is bad re-mints a token that was never the problem. So a service that
+// wraps this table guards the delegation:
+//
+//	if bearer.IsRefusal(err) {
+//	    http.Error(w, msg, bearer.StatusFor(err))
+//	    return
+//	}
+//	// anything else is ours, not the caller's
+//
+// Without this, each service spells out the sentinel list again, which is the
+// five-line copy this package exists to stop.
+func IsRefusal(err error) bool {
+	for _, sentinel := range []error{
+		ErrInvalid, ErrNotOurs, ErrForbidden, ErrRevoked, ErrUnavailable,
+	} {
+		if errors.Is(err, sentinel) {
+			return true
+		}
+	}
+	return false
+}
+
 // Challenge is the WWW-Authenticate value a 401 carries: the scheme, and the
 // service's own config section as the realm.
 //

@@ 1,6 1,7 @@
 package bearer
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"net/http"


@@ 31,6 32,20 @@ func TestStatusForKeepsTheUnreachableDaemonOutOfThe401(t *testing.T) {
 		"an unrecognised failure refuses the request rather than declaring the service unwell")
 }
 
+// TestIsRefusalSeparatesOurVocabularyFromTheServices is the guard a resolver
+// needs: its own failures must not be answered as a bad credential.
+func TestIsRefusalSeparatesOurVocabularyFromTheServices(t *testing.T) {
+	for _, err := range []error{ErrInvalid, ErrNotOurs, ErrForbidden, ErrRevoked, ErrUnavailable} {
+		assert.True(t, IsRefusal(err), "%v", err)
+		assert.True(t, IsRefusal(fmt.Errorf("validate the token: %w", err)), "wrapped %v", err)
+	}
+
+	assert.False(t, IsRefusal(nil))
+	assert.False(t, IsRefusal(errors.New("dial tcp 127.0.0.1:5432: connection refused")),
+		"a database that did not answer is not a refused credential")
+	assert.False(t, IsRefusal(context.DeadlineExceeded))
+}
+
 func TestChallengeNamesTheServiceAndQuotesIt(t *testing.T) {
 	assert.Equal(t, `Bearer realm="bench.sr.ht"`, Challenge("bench.sr.ht"))
 	assert.Equal(t, `Bearer realm="dolt.sr.ht"`, Challenge("dolt.sr.ht"))

@@ 178,11 178,13 @@ func OriginHost(origin string) string {
 //
 // 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.
+// Host whose port it has already stripped, while a JWT audience or a sealed URL
+// 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.
+//
+// A synthesized email domain is NOT one of these: an address is built from
+// [OriginHost], because agent@localhost:5091 is not a mailbox.
 func OriginAuthority(origin string) string {
 	u, err := url.Parse(CanonicalOrigin(origin))
 	if err != nil {


@@ 197,10 199,15 @@ func OriginAuthority(origin string) string {
 // 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].
+// [Need] or [NeedAny], and add the reason with [Key.Because].
 type Key struct {
 	Section string
 	Names   []string
+	// Why is what the operator is told the key is for, e.g. "crypto.InitCrypto
+	// exits without it". It is optional, and it is the difference between a
+	// list of names and a message somebody can act on: three services kept
+	// their own hand-written checker rather than lose this sentence.
+	Why string
 }
 
 // Need is a requirement for one named key in a section.


@@ 214,18 221,31 @@ func NeedAny(section string, names ...string) Key {
 	return Key{Section: section, Names: names}
 }
 
+// Because attaches the reason the key is required, for the operator reading the
+// refusal: Need("sr.ht", "network-key").Because("crypto.InitCrypto exits
+// without it").
+func (k Key) Because(why string) Key {
+	k.Why = why
+	return k
+}
+
 // 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 {
+	var named string
 	switch len(k.Names) {
 	case 0:
-		return fmt.Sprintf("[%s] <no key>", k.Section)
+		named = fmt.Sprintf("[%s] <no key>", k.Section)
 	case 1:
-		return fmt.Sprintf("[%s] %s", k.Section, k.Names[0])
+		named = fmt.Sprintf("[%s] %s", k.Section, k.Names[0])
 	default:
-		return fmt.Sprintf("[%s] one of %s", k.Section, strings.Join(k.Names, ", "))
+		named = fmt.Sprintf("[%s] one of %s", k.Section, strings.Join(k.Names, ", "))
+	}
+	if k.Why != "" {
+		return named + " — " + k.Why
 	}
+	return named
 }
 
 // satisfied reports whether the config has a non-blank value for any of the

@@ 223,8 223,25 @@ type Options struct {
 // loading config calls Defaults(nil, "") — -d and $LOG_LEVEL still resolve, and
 // the config file has nothing to say yet.
 func Defaults(conf ini.File, section string) Options {
+	return defaults(resolveLevel(conf, section, true))
+}
+
+// DefaultsWithoutDebugFlag is Defaults for a binary whose -d is not the
+// daemon's.
+//
+// A migration CLI on this instance passes -d to brant, where it means
+// --dialect and takes a value: `coversrht-migrate -d postgres` would otherwise
+// arrive here as a request for debug logging, silently, because the probe sees
+// the flag and never the value. $LOG_LEVEL and the config key still resolve —
+// only the argument scan is dropped, which is the one source that cannot tell
+// the two meanings apart.
+func DefaultsWithoutDebugFlag(conf ini.File, section string) Options {
+	return defaults(resolveLevel(conf, section, false))
+}
+
+func defaults(level slog.Level) Options {
 	return Options{
-		Level:           resolveLevel(conf, section),
+		Level:           level,
 		AddSource:       true,
 		Color:           ColorEnabled(os.Stderr),
 		TimeFormat:      TimeFormat,


@@ 235,8 252,9 @@ func Defaults(conf ini.File, section string) Options {
 }
 
 // resolveLevel walks the three sources of verbosity in order of authority.
-func resolveLevel(conf ini.File, section string) slog.Level {
-	if DebugRequested(os.Args[1:]) {
+// debugFlag is false for a binary whose -d belongs to something else.
+func resolveLevel(conf ini.File, section string, debugFlag bool) slog.Level {
+	if debugFlag && DebugRequested(os.Args[1:]) {
 		return slog.LevelDebug
 	}
 	if level, ok := ParseLevel(os.Getenv(LevelEnv)); ok {

@@ 357,3 357,22 @@ func TestInstallSetsTheDefault(t *testing.T) {
 func TestInstallRejectsANilHandler(t *testing.T) {
 	assert.Panics(t, func() { Install(nil) })
 }
+
+// TestDefaultsWithoutDebugFlagIgnoresTheArgument covers the migration CLI whose
+// -d is brant's --dialect and takes a value: `-d postgres` must not arrive here
+// as a request for debug logging.
+func TestDefaultsWithoutDebugFlagIgnoresTheArgument(t *testing.T) {
+	saved := os.Args
+	os.Args = []string{"coversrht-migrate", "-d", "postgres"}
+	t.Cleanup(func() { os.Args = saved })
+	t.Setenv(LevelEnv, "")
+
+	assert.Equal(t, slog.LevelDebug, Defaults(nil, "").Level,
+		"the daemon's probe still sees -d")
+	assert.Equal(t, slog.LevelInfo, DefaultsWithoutDebugFlag(nil, "").Level,
+		"the CLI's -d is not ours")
+
+	// The other two sources keep working for the CLI.
+	t.Setenv(LevelEnv, "warn")
+	assert.Equal(t, slog.LevelWarn, DefaultsWithoutDebugFlag(nil, "").Level)
+}

@@ 55,6 55,46 @@ const (
 
 // Message is the standard message for a status, or "" for a status that has
 // none — a 400 above all, whose message is the caller's own text.
+// The machine-facing halves of the same table. A REST surface answers a caller
+// that parses, not a person that reads, and every service on the instance keeps
+// its 404 body byte-identical on purpose: two spellings of "not found" are two
+// facts a client can accidentally distinguish, which is exactly what the shared
+// status was chosen to prevent.
+//
+// They live beside the page sentences rather than in a second package because
+// they are one table read two ways — and because the service that tried to
+// reuse RenderRefusals on its REST surface could not, for want of these.
+const (
+	APINotFoundMessage     = "not found"
+	APIUnauthorizedMessage = "unauthorized"
+	APIForbiddenMessage    = "forbidden"
+	APIMethodMessage       = "method not allowed"
+	APIInternalMessage     = "internal server error"
+	APIUnavailableMessage  = "service unavailable"
+)
+
+// APIMessage is Message for a machine-facing surface: the same statuses, in the
+// register a JSON client expects. An unmapped status returns "", which the
+// caller renders as it likes — usually http.StatusText.
+func APIMessage(status int) string {
+	switch status {
+	case http.StatusUnauthorized:
+		return APIUnauthorizedMessage
+	case http.StatusForbidden:
+		return APIForbiddenMessage
+	case http.StatusNotFound:
+		return APINotFoundMessage
+	case http.StatusMethodNotAllowed:
+		return APIMethodMessage
+	case http.StatusInternalServerError:
+		return APIInternalMessage
+	case http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout:
+		return APIUnavailableMessage
+	default:
+		return ""
+	}
+}
+
 func Message(status int) string {
 	switch status {
 	case http.StatusUnauthorized:

@@ 0,0 1,27 @@
+package pages
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestAPIMessageMirrorsMessage keeps the two registers on one table: a status
+// that has a sentence for a reader must have a word for a parser, or a REST
+// surface silently falls back to its own spelling.
+func TestAPIMessageMirrorsMessage(t *testing.T) {
+	for _, status := range []int{
+		http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound,
+		http.StatusMethodNotAllowed, http.StatusInternalServerError,
+		http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout,
+	} {
+		assert.NotEmpty(t, Message(status), "page sentence for %d", status)
+		assert.NotEmpty(t, APIMessage(status), "machine word for %d", status)
+		assert.NotEqual(t, Message(status), APIMessage(status),
+			"%d: a parser and a reader are not the same audience", status)
+	}
+
+	assert.Empty(t, APIMessage(http.StatusTeapot))
+	assert.Equal(t, "not found", APIMessage(http.StatusNotFound))
+}