Harden MSP tenant isolation: scope org-bound tokens away from default org, propagate webhook allowlist to all tenants

Two gaps found by exercising the MSP pilot path live on a throwaway
multi-tenant instance:

1. CheckAccess granted any authenticated principal access to the default
   org, so a token bound to a client org could read the provider's own
   default-org estate if it leaked from a client site. Org-bound tokens
   now fall through to the explicit binding check for the default org;
   authenticated users and legacy unbound tokens keep default-org access,
   and binding "default" explicitly still grants it.

2. The webhook private-target allowlist (instance-wide system setting)
   only ever reached the default org's notification manager on
   startup/reload, and only the request-context org on settings update.
   Tenant orgs' webhooks to private targets (per-client Gotify over VPN,
   the canonical MSP alert route) failed SSRF validation with no org-side
   remedy, and any allowlist died with a restart. Settings updates and
   reloads now fan out to every live tenant manager via the new
   MultiTenantMonitor.ForEachMonitor, and tenant monitors inherit the
   persisted allowlist and public URL at creation.

Both fixes verified live: org-bound token vs default org returns 403;
client-org webhooks to a private target succeed after restart and for
orgs created after the allowlist was saved. MSP.md validation checklist
gains the default-org probe and the allowlist guidance; MULTI_TENANT.md
documents the binding semantics. Contracts updated for api-contracts,
security-privacy, and monitoring with adjacency notes for
agent-lifecycle, storage-recovery, and performance-and-scalability.
This commit is contained in:
rcourtman 2026-06-10 11:37:39 +01:00
parent cbd0311055
commit 96001d134e
16 changed files with 327 additions and 17 deletions

View file

@ -88,14 +88,23 @@ the tunnels and keep the management port out of them.
```
4. **Cross-tenant isolation (shared-process mode only).** A token bound to one
organization must get `403` when targeting another:
organization must get `403` when targeting another organization AND when
targeting the default org (a leaked client-site token must not read the
provider's own estate):
```bash
curl -sk -o /dev/null -w '%{http_code}\n' \
-H "X-API-Token: <org-A token>" -H "X-Pulse-Org-ID: org-b" \
https://pulse.internal:7655/api/alerts/active # 403
curl -sk -o /dev/null -w '%{http_code}\n' \
-H "X-API-Token: <org-A token>" -H "X-Pulse-Org-ID: default" \
https://pulse.internal:7655/api/alerts/active # 403
```
Keep your own monitoring estate in its own organization too, rather than
in the default org, so every boundary in the instance is an explicit org
boundary.
## Per-client alert routing
Configure notification destinations inside each client's scope — the client
@ -103,6 +112,12 @@ runtime in the provider-hosted model, or the organization in shared-process
mode. A per-client Gotify server, Slack channel, or PSA endpoint only ever
sees that client's alerts.
Webhook targets on private IPs (a Gotify server reached over a VPN tunnel,
for example) are blocked by default for SSRF safety. Allow them once in
**Settings → System → Network → Webhook Security**; the allowlist is
instance-wide and applies to every organization, including ones created
later.
Alert webhook payloads carry the firing tenant's identity (`{{.TenantID}}`,
`{{.TenantName}}`), so a single central PSA endpoint can also route by client.
For ticket bridges (ConnectWise and similar), use the delivery contract —

View file

@ -129,7 +129,7 @@ Use this for one company operating many internal sites, teams, departments, or e
To onboard an internal estate:
1. **Create an organization for the estate** (see [Creating an Organization](#creating-an-organization)).
2. **Create an org-bound API token** with the `agent:report` scope, bound to that estate's organization (`orgId`). A token bound to a single organization automatically routes every agent that uses it into that organization, with no extra header required.
2. **Create an org-bound API token** with the `agent:report` scope, bound to that estate's organization (`orgId`). A token bound to a single organization automatically routes every agent that uses it into that organization, with no extra header required. Binding also scopes the token: an org-bound token cannot access other organizations, including the default org (bind `default` explicitly if a token genuinely needs it). Legacy unbound tokens keep their default-org access.
3. **Install the estate's agents** (Proxmox host, Docker, Kubernetes) using that token. Their telemetry lands in the selected organization.
4. **(Optional) Alias node names per estate.** If two estates both use the default `pve` hostname and you want them visually distinct, set `--hostname` (or the `PULSE_HOSTNAME` environment variable) on the agent, for example `--hostname "acme-pve1"`. See [UNIFIED_AGENT.md](UNIFIED_AGENT.md).
5. **(Optional) Isolate agent check-in on its own port.** When remote nodes reach the central server across the internet, enable [Split-Port Agent Ingest](CONFIGURATION.md#split-port-agent-ingest-network-isolation) so agents connect on a dedicated, firewalled port that exposes only `/api/agents/*` and never the web UI or management API.

View file

@ -1204,6 +1204,13 @@ profile and assignment columns, but embedded table framing must route through
## Current State
Default-org token scoping and notification-settings fan-out on shared
`internal/api/` handlers are likewise adjacent only: org-bound token denial
for the default org and instance-wide webhook allowlist propagation are
api-contract/security owned and change no install, enrollment, setup-token,
or fleet command authority. Agent tokens keep routing agents into their bound
org exactly as before.
Notification webhook management changes on shared `internal/api/` handlers are
likewise adjacent only: the webhook `signingSecret` payload field, its masked
list representation, and masked-echo preservation on update are

View file

@ -2024,6 +2024,18 @@ the canonical monitored-system blocked payload.
## Current State
That same shared API boundary now also owns the default-org token scoping
rule and instance-wide notification settings propagation. A token explicitly
bound to one or more organizations is scoped to those organizations only and
must not implicitly reach the default org's data; authenticated users and
legacy unbound tokens retain default-org access, and binding `default`
explicitly still grants it (pinned by
`TestContract_OrgBoundTokenIsScopedAwayFromDefaultOrg`). System settings that
shape notification transport (webhook private-target allowlist, public URL)
are stored instance-wide and must be applied to every live tenant monitor's
notification manager on update and reload, and inherited by tenant monitors at
creation, instead of reaching only the request's org or the default monitor.
That same notifications boundary also carries the webhook signing-secret
payload contract: webhook management payloads may include an optional
`signingSecret` that enables HMAC-signed deliveries, the list API must mask a

View file

@ -314,6 +314,14 @@ truth for live infrastructure data.
## Current State
That same reloadable multi-tenant monitor boundary also owns instance-wide
notification settings fan-out. `ForEachMonitor` visits every live tenant
monitor so callers can propagate the webhook security allowlist and public
URL to each org's notification manager, and tenant monitors inherit those
persisted settings at creation through the router's monitor initializer, so
an org created after the settings were saved (or after a restart) observes
the same allowlist as the default org.
This subsystem now sits under the dedicated core monitoring runtime lane so
discovery, metrics-history correctness, and platform-specific runtime coverage
can be governed as first-class product work instead of staying diluted inside

View file

@ -627,6 +627,11 @@ shell clickable behind another overlay.
## Current State
Default-org token scoping and notification-settings fan-out on shared
`internal/api/router.go` are likewise adjacent only: the tenant-monitor
settings inheritance hook runs once per tenant-monitor creation and changes
no chart transport, downsampling, or polling behavior.
The bars / sparklines toggle and its sparkline-range picker on the
WorkloadsSurface support page-level ownership through four optional
overrides exposed by

View file

@ -229,6 +229,17 @@ controls as normal product settings.
## Current State
The multi-tenant authorization boundary now also owns default-org token
scoping. An org-bound API token is a client-scoped credential: it must be
denied implicit access to the default org so a token that leaks from a client
site cannot read the provider's own estate, while authenticated users and
legacy unbound tokens keep default-org access for compatibility. The webhook
SSRF allowlist is the related instance-wide security setting: it must
propagate to every tenant org's notification manager (update, reload, and
tenant-monitor creation), because an allowlist that only the default org
observes silently denies legitimate per-client private webhook targets and
invites per-org security drift.
This subsystem now gives `L14` an explicit governed home for privacy guidance
and telemetry disclosures instead of leaving those trust surfaces as lane-level
evidence with no subsystem ownership.

View file

@ -1180,6 +1180,11 @@ recovery scope, or a storage/recovery-owned secret source.
## Current State
Default-org token scoping and notification-settings fan-out on shared
`internal/api/` handlers are likewise adjacent only: they are
api-contract/security owned and create no storage, recovery-point, or
backup-surface semantics.
Notification webhook management changes on shared `internal/api/` handlers are
likewise adjacent only: the webhook `signingSecret` payload field and its
masking semantics are notifications/API-contract owned and create no storage,

View file

@ -142,11 +142,27 @@ type AuthorizationResult struct {
// CheckAccess performs a comprehensive authorization check for a request.
func (c *DefaultAuthorizationChecker) CheckAccess(token *config.APITokenRecord, userID, orgID string) AuthorizationResult {
// The default organization is always accessible to any authenticated principal.
if orgID == "default" && (token != nil || userID != "") {
return AuthorizationResult{
Allowed: true,
Reason: "Default organization is accessible to all authenticated users",
// The default organization stays accessible to authenticated users and to
// legacy unbound tokens. A token explicitly bound to organizations is
// scoped to those organizations only — binding expresses intent to limit,
// so it must not implicitly reach the default org's data (bind "default"
// explicitly to grant that). Without this, a client-bound MSP token that
// leaks from a client site could read the provider's own default-org
// estate.
if orgID == "default" {
if token != nil {
if token.IsLegacyToken() {
return AuthorizationResult{
Allowed: true,
Reason: "Default organization is accessible to unbound tokens",
}
}
// Org-bound tokens fall through to the explicit binding check.
} else if userID != "" {
return AuthorizationResult{
Allowed: true,
Reason: "Default organization is accessible to all authenticated users",
}
}
}

View file

@ -144,4 +144,29 @@ func TestDefaultAuthorizationChecker_CheckAccess(t *testing.T) {
assert.False(t, res.Allowed)
assert.Equal(t, "No authentication context provided", res.Reason)
})
t.Run("default org access by principal kind", func(t *testing.T) {
// Users keep implicit default-org access.
res := checker.CheckAccess(nil, "user1", "default")
assert.True(t, res.Allowed)
// Legacy unbound tokens keep implicit default-org access.
res = checker.CheckAccess(&config.APITokenRecord{}, "", "default")
assert.True(t, res.Allowed)
// A token bound to a client org must NOT implicitly reach the
// default org — binding expresses intent to limit. This is the MSP
// isolation boundary: a leaked client-site token cannot read the
// provider's own estate.
res = checker.CheckAccess(&config.APITokenRecord{OrgID: "client-acme"}, "", "default")
assert.False(t, res.Allowed)
res = checker.CheckAccess(&config.APITokenRecord{OrgIDs: []string{"client-acme", "client-beta"}}, "", "default")
assert.False(t, res.Allowed)
// Binding "default" explicitly still grants it.
res = checker.CheckAccess(&config.APITokenRecord{OrgID: "default"}, "", "default")
assert.True(t, res.Allowed)
res = checker.CheckAccess(&config.APITokenRecord{OrgIDs: []string{"default", "client-acme"}}, "", "default")
assert.True(t, res.Allowed)
})
}

View file

@ -15480,3 +15480,28 @@ func TestContract_WebhookSigningSecretMaskedAndPreserved(t *testing.T) {
}
mockManager.AssertExpectations(t)
}
// TestContract_OrgBoundTokenIsScopedAwayFromDefaultOrg pins the MSP tenant
// isolation boundary: a token explicitly bound to client organizations must
// not implicitly reach the default org's data (a leaked client-site token
// must not read the provider's own estate), while users and legacy unbound
// tokens retain default-org access for backward compatibility.
func TestContract_OrgBoundTokenIsScopedAwayFromDefaultOrg(t *testing.T) {
checker := NewAuthorizationChecker(nil)
if res := checker.CheckAccess(&config.APITokenRecord{OrgID: "client-acme"}, "", "default"); res.Allowed {
t.Fatal("org-bound token must not implicitly access the default org")
}
if res := checker.CheckAccess(&config.APITokenRecord{OrgIDs: []string{"client-a", "client-b"}}, "", "default"); res.Allowed {
t.Fatal("multi-org-bound token must not implicitly access the default org")
}
if res := checker.CheckAccess(&config.APITokenRecord{OrgID: "default"}, "", "default"); !res.Allowed {
t.Fatal("token explicitly bound to default must access the default org")
}
if res := checker.CheckAccess(&config.APITokenRecord{}, "", "default"); !res.Allowed {
t.Fatal("legacy unbound token must retain default-org access")
}
if res := checker.CheckAccess(nil, "admin", "default"); !res.Allowed {
t.Fatal("authenticated users must retain default-org access")
}
}

View file

@ -1421,6 +1421,27 @@ func (r *Router) configureMonitorDependencies(m *monitoring.Monitor) {
m.SetResourceStore(adapter)
}
// Tenant monitors must inherit the persisted instance-wide notification
// settings. System settings are stored globally, so without this a tenant
// monitor created after the allowlist was saved (or after a restart)
// silently falls back to deny-all-private and per-client private webhook
// targets (e.g. MSP Gotify endpoints reached over VPN) fail SSRF
// validation with no org-side way to allow them.
if r.persistence != nil {
if settings, err := r.persistence.LoadSystemSettings(); err == nil && settings != nil {
if nm := m.GetNotificationManager(); nm != nil {
if settings.WebhookAllowedPrivateCIDRs != "" {
if err := nm.UpdateAllowedPrivateCIDRs(settings.WebhookAllowedPrivateCIDRs); err != nil {
log.Error().Err(err).Msg("Failed to apply webhook allowed private CIDRs to tenant monitor")
}
}
if settings.PublicURL != "" {
nm.SetPublicURL(settings.PublicURL)
}
}
}
}
if len(r.monitorSupplementalRecords) == 0 {
return
}
@ -3737,7 +3758,9 @@ func (r *Router) reloadSystemSettings() {
r.config.HideLocalLogin = systemSettings.HideLocalLogin
}
// Update webhook allowed private CIDRs in notification manager
// Update webhook allowed private CIDRs in notification managers.
// The setting is instance-wide, so every tenant monitor's manager
// must observe it, not just the default org's.
if r.monitor != nil {
if nm := r.monitor.GetNotificationManager(); nm != nil {
if err := nm.UpdateAllowedPrivateCIDRs(systemSettings.WebhookAllowedPrivateCIDRs); err != nil {
@ -3745,6 +3768,15 @@ func (r *Router) reloadSystemSettings() {
}
}
}
if r.mtMonitor != nil {
r.mtMonitor.ForEachMonitor(func(m *monitoring.Monitor) {
if nm := m.GetNotificationManager(); nm != nil {
if err := nm.UpdateAllowedPrivateCIDRs(systemSettings.WebhookAllowedPrivateCIDRs); err != nil {
log.Error().Err(err).Msg("Failed to update webhook allowed private CIDRs on tenant monitor during settings reload")
}
}
})
}
} else {
// On error, use safe defaults
r.cachedAllowEmbedding = false

View file

@ -17,6 +17,7 @@ import (
"github.com/gorilla/websocket"
"github.com/rcourtman/pulse-go-rewrite/internal/agentexec"
"github.com/rcourtman/pulse-go-rewrite/internal/config"
"github.com/rcourtman/pulse-go-rewrite/internal/models"
"github.com/rcourtman/pulse-go-rewrite/internal/monitoring"
"github.com/rcourtman/pulse-go-rewrite/internal/servicediscovery"
pulsews "github.com/rcourtman/pulse-go-rewrite/internal/websocket"
@ -4767,3 +4768,73 @@ func TestRequirePermissionUsesContextUsername(t *testing.T) {
t.Fatalf("RBAC subject must come from context, got %q (header was %q)", observedSubject, "evil-spoofed-user")
}
}
// Instance-wide webhook security settings must reach every tenant org's
// notification manager: the allowlist is stored globally, so applying it only
// to the request's org (or only to the default org on reload) leaves other
// tenants' webhook targets failing SSRF validation with no org-side remedy.
func TestSystemSettingsWebhookAllowlistPropagatesToAllTenantManagers(t *testing.T) {
t.Setenv("PULSE_MULTI_TENANT_ENABLED", "true")
defer SetMultiTenantEnabled(false)
SetMultiTenantEnabled(true)
dataDir := t.TempDir()
tokenVal := "allowlist-propagation-token-123.12345678"
cfg := &config.Config{
DataPath: dataDir,
APITokens: []config.APITokenRecord{
{ID: "tok-allowlist", Hash: auth.HashAPIToken(tokenVal), Name: "Allowlist Test Token"},
},
}
persistence := config.NewConfigPersistence(dataDir)
mtp := config.NewMultiTenantPersistence(dataDir)
mtm := monitoring.NewMultiTenantMonitor(cfg, mtp, nil)
t.Cleanup(mtm.Stop)
for _, org := range []struct{ id, name string }{
{"org-a", "Org A"},
{"org-b", "Org B"},
} {
if err := mtp.SaveOrganization(&models.Organization{ID: org.id, DisplayName: org.name}); err != nil {
t.Fatalf("SaveOrganization(%s): %v", org.id, err)
}
if _, err := mtm.GetMonitor(org.id); err != nil {
t.Fatalf("GetMonitor(%s): %v", org.id, err)
}
}
h := NewSystemSettingsHandler(cfg, persistence, nil, mtm, nil, func() {}, nil)
// Both org managers must reject the private target before the update.
for _, orgID := range []string{"org-a", "org-b"} {
m, err := mtm.GetMonitor(orgID)
if err != nil {
t.Fatalf("GetMonitor(%s): %v", orgID, err)
}
if err := m.GetNotificationManager().ValidateWebhookURL("http://192.0.2.10:9999/hook"); err == nil {
t.Fatalf("%s: expected private webhook target to be rejected before allowlist update", orgID)
}
}
body := bytes.NewBufferString(`{"webhookAllowedPrivateCIDRs":"192.0.2.0/24"}`)
req := httptest.NewRequest(http.MethodPost, "/api/system/settings/update", body)
req.Header.Set("Content-Type", "application/json")
req.Header.Set("X-API-Token", tokenVal)
rec := httptest.NewRecorder()
h.HandleUpdateSystemSettings(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf("HandleUpdateSystemSettings responded %d: %s", rec.Code, rec.Body.String())
}
// Every live tenant manager must observe the new allowlist, regardless of
// which org the admin's request context pointed at.
for _, orgID := range []string{"org-a", "org-b"} {
m, err := mtm.GetMonitor(orgID)
if err != nil {
t.Fatalf("GetMonitor(%s): %v", orgID, err)
}
if err := m.GetNotificationManager().ValidateWebhookURL("http://192.0.2.10:9999/hook"); err != nil {
t.Fatalf("%s: expected private webhook target to be allowed after allowlist update, got %v", orgID, err)
}
}
}

View file

@ -134,6 +134,33 @@ func (h *SystemSettingsHandler) SetMultiTenantMonitor(mtm *monitoring.MultiTenan
}
}
// forEachNotificationManager applies fn to the notification manager of every
// live tenant monitor when multi-tenant iteration is available, falling back
// to the request's monitor otherwise. Instance-wide notification settings
// (webhook security allowlist, public URL) must reach every org's manager.
func (h *SystemSettingsHandler) forEachNotificationManager(r *http.Request, fn func(*notifications.NotificationManager)) {
h.stateMu.RLock()
mtMonitor := h.mtMonitor
h.stateMu.RUnlock()
type monitorRanger interface {
ForEachMonitor(func(*monitoring.Monitor))
}
if ranger, ok := mtMonitor.(monitorRanger); ok && ranger != nil {
ranger.ForEachMonitor(func(m *monitoring.Monitor) {
if nm := m.GetNotificationManager(); nm != nil {
fn(nm)
}
})
return
}
if monitor := h.getMonitor(r.Context()); monitor != nil {
if nm := monitor.GetNotificationManager(); nm != nil {
fn(nm)
}
}
}
func (h *SystemSettingsHandler) getMonitor(ctx context.Context) SystemSettingsMonitor {
h.stateMu.RLock()
mtMonitor := h.mtMonitor
@ -1045,18 +1072,19 @@ func (h *SystemSettingsHandler) HandleUpdateSystemSettings(w http.ResponseWriter
h.getMonitor(r.Context()).DisableTemperatureMonitoring()
}
}
if cidrUpdateRequested && h.getMonitor(r.Context()) != nil {
if nm := h.getMonitor(r.Context()).GetNotificationManager(); nm != nil {
// Webhook security allowlist and public URL are instance-wide settings:
// apply them to every live tenant monitor's notification manager, not
// just the org the admin happened to have selected.
if cidrUpdateRequested {
h.forEachNotificationManager(r, func(nm *notifications.NotificationManager) {
nm.ApplyAllowedPrivateCIDRs(settings.WebhookAllowedPrivateCIDRs, parsedCIDRNets)
}
})
}
if _, ok := rawRequest["publicURL"]; ok {
if h.getMonitor(r.Context()) != nil {
if nm := h.getMonitor(r.Context()).GetNotificationManager(); nm != nil {
nm.SetPublicURL(settings.PublicURL)
log.Info().Str("publicURL", settings.PublicURL).Msg("Updated notification public URL from settings")
}
}
h.forEachNotificationManager(r, func(nm *notifications.NotificationManager) {
nm.SetPublicURL(settings.PublicURL)
})
log.Info().Str("publicURL", settings.PublicURL).Msg("Updated notification public URL from settings")
}
// Reload cached system settings after successful save

View file

@ -2281,3 +2281,33 @@ func TestTenantMonitorWiresOrgIdentityIntoNotifications(t *testing.T) {
t.Fatalf("TenantIdentity() name after rename = %q, want %q", tenantName, "Acme Corp Renamed")
}
}
// Instance-wide notification settings (webhook security allowlist, public
// URL) propagate through ForEachMonitor; it must visit every live tenant
// monitor so no org's manager is left observing stale security settings.
func TestForEachMonitorVisitsAllTenantMonitors(t *testing.T) {
mtm := &MultiTenantMonitor{
monitors: map[string]*Monitor{
"default": {},
"org-a": {},
"org-b": {},
"nil-org": nil,
},
}
visited := 0
mtm.ForEachMonitor(func(m *Monitor) {
if m == nil {
t.Fatal("ForEachMonitor must skip nil monitors")
}
visited++
})
if visited != 3 {
t.Fatalf("ForEachMonitor visited %d monitors, want 3", visited)
}
// Nil receiver and nil callback are safe no-ops.
var nilMTM *MultiTenantMonitor
nilMTM.ForEachMonitor(func(*Monitor) { t.Fatal("nil receiver must not invoke callback") })
mtm.ForEachMonitor(nil)
}

View file

@ -84,6 +84,26 @@ func (mtm *MultiTenantMonitor) SetMonitorInitializer(initializer func(*Monitor))
}
}
// ForEachMonitor runs fn against every live tenant monitor. Use it to
// propagate instance-wide runtime settings (e.g. webhook security allowlist)
// that every org's monitor must observe.
func (mtm *MultiTenantMonitor) ForEachMonitor(fn func(*Monitor)) {
if mtm == nil || fn == nil {
return
}
mtm.mu.RLock()
monitors := make([]*Monitor, 0, len(mtm.monitors))
for _, monitor := range mtm.monitors {
if monitor != nil {
monitors = append(monitors, monitor)
}
}
mtm.mu.RUnlock()
for _, monitor := range monitors {
fn(monitor)
}
}
// GetMonitor returns the monitor instance for a specific organization.
// It lazily initializes the monitor if it doesn't exist.
func (mtm *MultiTenantMonitor) GetMonitor(orgID string) (*Monitor, error) {