fix(security): PAT org binding, subscribe throttle, invite lookup, SNMP secret

server/internal/app/pat_binding_test.go

@@ -0,0 +1,216 @@
+package app
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/fclairamb/solidping/server/internal/config"
+	"github.com/fclairamb/solidping/server/internal/db/models"
+	"github.com/fclairamb/solidping/server/internal/handlers/auth"
+	"github.com/fclairamb/solidping/server/internal/handlers/base"
+	"github.com/fclairamb/solidping/server/internal/utils/passwords"
+)
+
+const patBindingPassword = "alicepass1234"
+
+// patBindingEnv is the real server with one user, admin of org A and org B,
+// a PAT created in org A and a session from a password login (spec
+// 2026-10-28-01).
+type patBindingEnv struct {
+	*impersonationEnv
+
+	user    *models.User
+	pat     string
+	session string
+}
+
+func newPATBindingEnv(t *testing.T) *patBindingEnv {
+	t.Helper()
+	r := require.New(t)
+	ctx := context.Background()
+
+	cfg := &config.Config{}
+	cfg.Database.Type = dbTypeSQLiteMemory
+	cfg.Server.BaseURL = "http://localhost"
+	cfg.Auth.JWTSecret = "pat-binding-secret"
+	cfg.Auth.AccessTokenExpiry = time.Hour
+	cfg.Auth.RefreshTokenExpiry = 24 * time.Hour
+	cfg.Auth.WebAuthn.Enabled = true
+
+	server, err := NewServer(ctx, cfg)
+	r.NoError(err)
+	t.Cleanup(func() { _ = server.dbService.Close() })
+
+	r.NoError(server.Initialize(ctx))
+	r.NoError(server.InitializeSystemConfig(ctx, cfg))
+	server.SetupRoutes(ctx)
+
+	ts := httptest.NewServer(server.Handler())
+	t.Cleanup(ts.Close)
+
+	env := &patBindingEnv{impersonationEnv: &impersonationEnv{t: t, server: server, ts: ts}}
+
+	env.orgA = models.NewOrganization("pata", "Acme A")
+	r.NoError(server.dbService.CreateOrganization(ctx, env.orgA))
+	env.orgB = models.NewOrganization("patb", "Acme B")
+	r.NoError(server.dbService.CreateOrganization(ctx, env.orgB))
+	r.NoError(server.dbService.CreateCheck(ctx, models.NewCheck(env.orgA.UID, "a-check", "http")))
+	r.NoError(server.dbService.CreateCheck(ctx, models.NewCheck(env.orgB.UID, "b-check", "http")))
+
+	hash, err := passwords.Hash(patBindingPassword)
+	r.NoError(err)
+
+	env.user = models.NewUser("alice@acme.com")
+	env.user.PasswordHash = &hash
+	r.NoError(server.dbService.CreateUser(ctx, env.user))
+
+	now := time.Now()
+	for _, org := range []*models.Organization{env.orgA, env.orgB} {
+		member := models.NewOrganizationMember(org.UID, env.user.UID, models.MemberRoleAdmin)
+		member.JoinedAt = &now
+		r.NoError(server.dbService.CreateOrganizationMember(ctx, member))
+	}
+
+	created, err := server.authService.CreatePAT(ctx, env.orgA.Slug, env.user.UID, auth.CreateTokenRequest{Name: "ci"})
+	r.NoError(err)
+	env.pat = created.Token
+
+	status, body := env.do(http.MethodPost, "/api/v1/auth/login", "",
+		`{"org":"pata","email":"alice@acme.com","password":"`+patBindingPassword+`"}`)
+	r.Equal(http.StatusOK, status, body)
+	env.session, _ = body["accessToken"].(string)
+	r.NotEmpty(env.session)
+
+	return env
+}
+
+func (e *patBindingEnv) tokenCount(tokenType models.TokenType) int {
+	e.t.Helper()
+
+	tokens, err := e.server.dbService.ListUserTokensByType(context.Background(), e.user.UID, tokenType)
+	require.NoError(e.t, err)
+
+	return len(tokens)
+}
+
+func (e *patBindingEnv) twoFACounterExists() bool {
+	e.t.Helper()
+
+	entry, err := e.server.dbService.GetStateEntry(context.Background(), nil, "twofa_attempt_count:"+e.user.UID)
+
+	return err == nil && entry != nil
+}
+
+// TestPATCannotLeaveItsOrg is the regression for finding X1: every route that
+// would let a PAT step out of its org binding answers 403 PAT_FORBIDDEN, and
+// the same calls made from a password session still go through.
+//
+//nolint:paralleltest,tparallel // sub-tests share one server and its token rows
+func TestPATCannotLeaveItsOrg(t *testing.T) {
+	t.Parallel()
+	env := newPATBindingEnv(t)
+
+	forbidden := []struct {
+		name, method, path, body string
+	}{
+		{"switch org", http.MethodPost, "/api/v1/auth/switch-org", `{"org":"patb"}`},
+		{"org creation", http.MethodPost, "/api/v1/orgs", `{"slug":"patc","name":"Acme C"}`},
+		{"accept invite", http.MethodPost, "/api/v1/auth/accept-invite", `{"token":"x"}`},
+		{"change password", http.MethodPost, "/api/v1/auth/change-password", `{"newPassword":"hijack-1234567"}`},
+		{"2FA setup", http.MethodPost, "/api/v1/auth/2fa/setup", `{}`},
+		{"2FA confirm", http.MethodPost, "/api/v1/auth/2fa/confirm", `{"code":"123456"}`},
+		{"2FA removal", http.MethodDelete, "/api/v1/auth/2fa", `{"code":"123456"}`},
+		{"passkey add (begin)", http.MethodPost, "/api/v1/auth/passkeys/register/begin", `{}`},
+		{"passkey add (finish)", http.MethodPost, "/api/v1/auth/passkeys/register/finish", `{}`},
+		{"passkey removal", http.MethodDelete, "/api/v1/auth/passkeys/abc", ``},
+		{
+			"device consent", http.MethodPost, "/api/v1/auth/device/consent",
+			`{"userCode":"ABCD-EFGH","org":"patb","approve":true}`,
+		},
+		{"discord link", http.MethodPost, "/api/v1/orgs/pata/users/me/discord/link-start", `{}`},
+		{"discord connect", http.MethodPost, "/api/v1/orgs/pata/users/me/discord/connect", `{}`},
+		{"telegram link", http.MethodPost, "/api/v1/orgs/pata/users/me/telegram/link", `{}`},
+	}
+
+	refreshBefore := env.tokenCount(models.TokenTypeRefresh)
+	patBefore := env.tokenCount(models.TokenTypePAT)
+
+	for _, call := range forbidden {
+		t.Run(call.name, func(t *testing.T) {
+			r := require.New(t)
+
+			status, body := env.do(call.method, call.path, env.pat, call.body)
+			r.Equal(http.StatusForbidden, status, body)
+			r.Equal(string(base.ErrorCodePATForbidden), body["code"])
+		})
+	}
+
+	r := require.New(t)
+	r.Equal(refreshBefore, env.tokenCount(models.TokenTypeRefresh), "no session was minted")
+	r.Equal(patBefore, env.tokenCount(models.TokenTypePAT), "no PAT was minted")
+	r.False(env.twoFACounterExists(), "DELETE /auth/2fa spent no 2FA attempt")
+
+	// The PAT still reads its own org, and only its own org.
+	status, body := env.do(http.MethodGet, "/api/v1/orgs/pata/checks", env.pat, "")
+	r.Equal(http.StatusOK, status, body)
+	status, _ = env.do(http.MethodGet, "/api/v1/orgs/patb/checks", env.pat, "")
+	r.Equal(http.StatusForbidden, status)
+
+	// The PAT still creates a PAT in its own org.
+	status, body = env.do(http.MethodPost, "/api/v1/orgs/pata/tokens", env.pat, `{"name":"second"}`)
+	r.Equal(http.StatusCreated, status, body)
+
+	// The PAT still edits the profile, but not the sign-in email.
+	status, body = env.do(http.MethodPatch, "/api/v1/auth/me", env.pat, `{"name":"Alice"}`)
+	r.Equal(http.StatusOK, status, body)
+	status, body = env.do(http.MethodPatch, "/api/v1/auth/me", env.pat,
+		`{"email":"mallory@acme.com","currentPassword":"`+patBindingPassword+`"}`)
+	r.Equal(http.StatusForbidden, status, body)
+	r.Equal(string(base.ErrorCodePATForbidden), body["code"])
+
+	// Positive controls: the password session does what the PAT may not.
+	status, body = env.do(http.MethodPost, "/api/v1/auth/passkeys/register/begin", env.session, `{}`)
+	r.Equal(http.StatusOK, status, body)
+
+	status, body = env.do(http.MethodPost, "/api/v1/auth/switch-org", env.session, `{"org":"patb"}`)
+	r.Equal(http.StatusOK, status, body)
+	r.Equal(refreshBefore+1, env.tokenCount(models.TokenTypeRefresh))
+	switched, _ := body["accessToken"].(string)
+	status, _ = env.do(http.MethodGet, "/api/v1/orgs/patb/checks", switched, "")
+	r.Equal(http.StatusOK, status)
+
+	// DELETE /auth/2fa from a session still spends the shared login budget
+	// (spec 2026-10-27-02): 2FA is off, so the call is refused, but counted.
+	status, _ = env.do(http.MethodDelete, "/api/v1/auth/2fa", env.session, `{"code":"123456"}`)
+	r.NotEqual(http.StatusForbidden, status)
+	r.True(env.twoFACounterExists())
+}
+
+// TestPATDenylistNamesRealRoutes keeps the denylist honest: every entry must
+// be a registered route, or a renamed endpoint would silently fall out of the
+// guard.
+func TestPATDenylistNamesRealRoutes(t *testing.T) {
+	t.Parallel()
+	r := require.New(t)
+	env := newImpersonationEnv(t, true)
+
+	registered := map[string]bool{}
+	r.NoError(env.server.router.Walk(func(method, pattern string) error {
+		registered[method+" "+pattern] = true
+
+		return nil
+	}))
+
+	denylist := auth.PATForbiddenRoutes()
+	r.NotEmpty(denylist)
+
+	for _, entry := range denylist {
+		r.Truef(registered[entry[0]+" "+entry[1]],
+			"PAT denylist entry %s %s is not a registered route", entry[0], entry[1])
+	}
+}

server/internal/app/server.go

@@ -2137,7 +2137,8 @@ func (s *Server) SetupRoutes(ctx context.Context) {
 	// shared by the authed admin routes (below) and the public routes (further
 	// down, outside RequireAuth).
 	statusSubscribersService := statussubscribers.NewService(s.dbService, s.services.Credentials,
-		statussubscribers.WithEgressGuard(s.services.EgressGuard))
+		statussubscribers.WithEgressGuard(s.services.EgressGuard),
+		statussubscribers.WithConfirmBudget(s.config.StatusSubscribers))
 	statusSubscribersHandler := statussubscribers.NewHandler(
 		statusSubscribersService, s.dbService, s.services.EmailSender, s.services.EmailFormatter, s.config)
 	// Authed admin: list (count + redactable addresses) and remove.
@@ -2250,9 +2251,16 @@ func (s *Server) SetupRoutes(ctx context.Context) {
 	api.GET("/public/custom-domains/allowed", statusPagesHandler.CustomDomainAllowed)
 
 	// Public status-page subscription endpoints (no authentication). The
-	// subscribe endpoint inherits the global per-IP rate limit on /api/v1/;
-	// double opt-in is the primary anti-abuse control. Confirm/unsubscribe are
-	// single-purpose token links that render an HTML landing page.
+	// subscribe endpoint inherits the global per-IP rate limit on /api/v1/,
+	// which is far too loose to protect a third-party mailbox on its own, so
+	// the confirm mail is budgeted per recipient address (across all pages),
+	// per page and per trusted client IP (status_subscribers.* config); over
+	// budget it still answers 202 and sends nothing. A live confirmed
+	// subscription is never touched by a repeat subscribe (no mail, same 202),
+	// so the endpoint cannot cancel a subscriber nor reveal one. Confirm links
+	// expire after 7 days and the state-cleanup job reaps the unconfirmed rows
+	// (spec 2026-10-28-02). Confirm/unsubscribe are single-purpose token links
+	// that render an HTML landing page.
 	// Email-only, public. Its operator-side sibling deliberately lives at
 	// `.../subscribers/endpoints` — see the comment there for why they cannot
 	// share this pattern.

server/internal/checkers/checksnmp/config/secret_fields.go

@@ -3,5 +3,5 @@ package config
 // SecretFields declares which top-level config keys carry secrets and must
 // be encrypted at rest. Implements credentials.SecretFielder.
 func (c *SNMPConfig) SecretFields() []string {
-	return []string{"authPassword", "privPassword"}
+	return []string{"community", "authPassword", "privPassword"}
 }

server/internal/checkers/checksnmp/config/secret_fields_test.go

@@ -0,0 +1,15 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSecretFieldsIncludeCommunity(t *testing.T) {
+	t.Parallel()
+
+	require.ElementsMatch(t,
+		[]string{"community", "authPassword", "privPassword"},
+		(&SNMPConfig{}).SecretFields())
+}

server/internal/checkers/schemas/snmp.json

@@ -12,7 +12,9 @@
       "type": "string"
     },
     "community": {
-      "type": "string"
+      "type": "string",
+      "format": "solidping-secret-ref",
+      "description": "Stored encrypted. Prefer a `${env:VAR}` or `${param:key}` reference over an inlined credential in a config-as-code manifest."
     },
     "oid": {
       "type": "string"
@@ -58,6 +60,7 @@
   "x-solidping-notes": [],
   "x-solidping-secret-fields": [
     "authPassword",
+    "community",
     "privPassword"
   ],
   "x-solidping-validation": {

server/internal/config/config.go

@@ -639,6 +639,9 @@ type Config struct {
 	UserAgent    string             `koanf:"useragent"` // Identity string for protocol checks (SP_USERAGENT)
 	LogLevel     slog.Level         `koanf:"-"`         // Logging level (SP_LOG_LEVEL, LOG_LEVEL as fallback)
 	LogFormat    LogFormat          `koanf:"-"`         // Logging output format (parsed from SP_LOG_FORMAT env var)
+
+	// StatusSubscribers budgets the public status-page subscribe confirm mail.
+	StatusSubscribers StatusSubscribersConfig `koanf:"status_subscribers"`
 }
 
 // ACMEConfig turns on in-server TLS: certmagic obtains and renews Let's Encrypt
@@ -2177,6 +2180,8 @@ func Load() (*Config, error) {
 		// In-server ACME is opt-in; the listen addresses are pre-filled so
 		// enabling it is a one-flag change on a host with :80/:443 free.
 		Heartbeat: DefaultHeartbeatConfig(),
+		// Confirm-mail budgets of the public subscribe endpoint (spec 2026-10-28-02).
+		StatusSubscribers: DefaultStatusSubscribersConfig(),
 		ACME: ACMEConfig{
 			Enabled:     false,
 			ListenHTTP:  DefaultACMEListenHTTP,
@@ -2470,6 +2475,7 @@ func Load() (*Config, error) {
 	applyWorkerPoolEnv(&cfg)
 	applyACMEEnv(&cfg.ACME)
 	applyHeartbeatEnv(&cfg.Heartbeat)
+	applyStatusSubscribersEnv(&cfg.StatusSubscribers)
 	applyEncryptionEnv(&cfg.Encryption)
 	applyAuditEnv(&cfg.Audit)
 	applyDemoEnv(&cfg.Demo, cfg.RunMode)

server/internal/config/envvars.go

@@ -178,6 +178,10 @@ func manualReaderServerEnvVars() []string {
 		// env provider, so Load reads these by hand (see envInt calls).
 		"SP_ENTITLEMENTS_SMS_RUNAWAY_PER_HOUR",
 		"SP_ENTITLEMENTS_CALL_RUNAWAY_PER_HOUR",
+		// applyStatusSubscribersEnv — snake_case segments (spec 2026-10-28-02).
+		EnvStatusSubscriberConfirmPerRecipient,
+		EnvStatusSubscriberConfirmPerPage,
+		EnvStatusSubscriberConfirmPerIP,
 		"SP_SHUTDOWN_TIMEOUT",
 		"SP_SERVER_MAX_REQUEST_DURATION",
 		"SP_DB_RESET",

server/internal/config/status_subscribers.go

@@ -0,0 +1,68 @@
+package config
+
+import (
+	"os"
+	"strconv"
+)
+
+// Defaults for StatusSubscribersConfig (spec 2026-10-28-02). Exported so the
+// subscribers service and its tests cannot drift from the documented values.
+const (
+	// DefaultStatusSubscriberConfirmPerRecipientPerHour bounds the confirm
+	// mails one address receives, across every status page of the instance.
+	DefaultStatusSubscriberConfirmPerRecipientPerHour = 3
+	// DefaultStatusSubscriberConfirmPerPagePerHour bounds the confirm mails
+	// one status page sends.
+	DefaultStatusSubscriberConfirmPerPagePerHour = 100
+	// DefaultStatusSubscriberConfirmPerIPPerHour bounds the confirm mails one
+	// client IP (trusted-proxy resolution) can trigger.
+	DefaultStatusSubscriberConfirmPerIPPerHour = 20
+)
+
+// Env names of the confirm-mail budgets. Their key segments are snake_case,
+// which koanf's env provider collapses to dots, so applyStatusSubscribersEnv
+// reads them by hand.
+const (
+	EnvStatusSubscriberConfirmPerRecipient = "SP_STATUS_SUBSCRIBERS_CONFIRM_PER_RECIPIENT_PER_HOUR"
+	EnvStatusSubscriberConfirmPerPage      = "SP_STATUS_SUBSCRIBERS_CONFIRM_PER_PAGE_PER_HOUR"
+	EnvStatusSubscriberConfirmPerIP        = "SP_STATUS_SUBSCRIBERS_CONFIRM_PER_IP_PER_HOUR"
+)
+
+// StatusSubscribersConfig budgets the double opt-in mail of the public
+// status-page subscribe endpoint. Every budget is a count per sliding hour; 0
+// disables that budget. Over budget the endpoint still answers 202 but sends
+// nothing, so it is neither a mail cannon nor an oracle.
+type StatusSubscribersConfig struct {
+	ConfirmPerRecipientPerHour int `koanf:"confirm_per_recipient_per_hour"`
+	ConfirmPerPagePerHour      int `koanf:"confirm_per_page_per_hour"`
+	ConfirmPerIPPerHour        int `koanf:"confirm_per_ip_per_hour"`
+}
+
+// DefaultStatusSubscribersConfig returns the shipped budgets: 3 per recipient,
+// 100 per page and 20 per client IP, each per hour.
+func DefaultStatusSubscribersConfig() StatusSubscribersConfig {
+	return StatusSubscribersConfig{
+		ConfirmPerRecipientPerHour: DefaultStatusSubscriberConfirmPerRecipientPerHour,
+		ConfirmPerPagePerHour:      DefaultStatusSubscriberConfirmPerPagePerHour,
+		ConfirmPerIPPerHour:        DefaultStatusSubscriberConfirmPerIPPerHour,
+	}
+}
+
+// applyStatusSubscribersEnv reads the SP_STATUS_SUBSCRIBERS_* budgets. A
+// non-negative integer applies (0 disables the budget); anything else is
+// ignored and the configured value stays.
+func applyStatusSubscribersEnv(cfg *StatusSubscribersConfig) {
+	read := func(raw string, dst *int) {
+		if raw == "" {
+			return
+		}
+
+		if n, err := strconv.Atoi(raw); err == nil && n >= 0 {
+			*dst = n
+		}
+	}
+
+	read(os.Getenv("SP_STATUS_SUBSCRIBERS_CONFIRM_PER_RECIPIENT_PER_HOUR"), &cfg.ConfirmPerRecipientPerHour)
+	read(os.Getenv("SP_STATUS_SUBSCRIBERS_CONFIRM_PER_PAGE_PER_HOUR"), &cfg.ConfirmPerPagePerHour)
+	read(os.Getenv("SP_STATUS_SUBSCRIBERS_CONFIRM_PER_IP_PER_HOUR"), &cfg.ConfirmPerIPPerHour)
+}