Review hardening on e2f5f1f. None of these were live bypasses; all were
one bad row or one new caller away from becoming one.
- Replace the empty-orgID sentinel with explicit scheduler entry points.
The sentinel meant "skip the org check" and was keyed on a value read
from a DB record on the agent path, so a server doc with a blank org_id
would silently disable isolation. The exported agent-facing functions
now reject an empty orgID outright.
- ValidateAgentToken errors when the resolved server has no org.
- UpdateMonitor's runner and channel_ids type assertions were fail-open:
a wrong-typed value skipped validation while the $set still ran. Now a
hard error.
- Normalise an empty runner on update to the server runner, matching
create. Previously it matched no runner at all, so the monitor silently
stopped being checked and stopped alerting.
- IngestResult returns an error for an unknown monitor, so probing an
unknown ID looks the same as probing a foreign one.
Two instances of the branch's recurring bug class remained in the agent
path: a client-supplied ID accepted as data, then consumed by an
unscoped query.
- ListMonitorsForRunner filtered on `runner` alone, and `runner` is set
by the client on monitor create/update. Org A could point a monitor at
org B's server_id and org B's agent would fetch and execute the check.
Now org-filtered, and `runner` is validated against the caller's org on
create and update.
- IngestResult resolved the monitor via the unfiltered getMonitorByID
using a monitor_id from the agent's request body, letting org A's agent
write state and incidents into org B's monitor and fire its channels.
Now rejects on org mismatch and on a monitor not assigned to the
reporting agent.
The in-process scheduler passes an empty orgID as a documented sentinel
for the cross-org server-run sweep.
Install script still emits a single shared GRPC_HOST; the agent path
resolves org from the server record, never from a hostname.
Review follow-ups on e5363a6:
- MigrateSettingsOrg no longer guesses via the "default" slug. One org
means stamp that org; zero orgs means synthesise Default; more than
one means leave it alone and log, since guessing would hand one org
another's SMTP config and ESO token.
- EnsureSettingsIndexes failure is now fatal. Without the unique index
on org_id, GetSettings returns an arbitrary duplicate; without the one
on the token hash, ResolveSecretsReadToken picks an arbitrary org.
- Name the token-hash index explicitly so it stops colliding with the
legacy name DropOne targets, and exclude the empty string from the
partial filter.
- Log retention: distinguish a missing run doc from a Mongo error, so a
transient failure skips the directory rather than purging it at the
30-day default.
- Offline sweep: fresh context per org, log-and-continue on a per-org
error, plus a final pass for servers whose org no longer exists.
- ESO handler 401s on an empty token-derived org rather than querying
org_id "".
The settings collection was a single global document, so every org
shared one SMTP config, alert config, retention policy and ESO read
token. GetSecretGroupDecryptedAny then flattened every org's secrets
for a group into one map, meaning any tenant's token read every
tenant's secrets.
- settings gains org_id; GetSettings/SaveSettings/RotateSecretsReadToken/
GetWorkflowLogRetentionDays all take orgID
- VerifySecretsReadToken replaced by ResolveSecretsReadToken, which
resolves the org from the presented token's hash; the ESO endpoint
derives its org from the token rather than a session, since it is
called machine-to-machine
- GetSecretGroupDecryptedAny deleted in favour of the org-scoped variant
- settings and token-rotation routes now require owner/admin
- offline sweep and log retention resolve org per server / per run
- migration 0002 stamps the legacy settings doc with the default org
Note: /api/settings now 403s for members; the web settings page needs a
matching role check.
Review of the org-scoping pass found that org_id on a query filter
protects the row you look up, but does nothing when a handler accepts a
foreign resource ID as data and a downstream unscoped query consumes it.
- AssignKey: verify key and server both belong to the org
- BuildAuthorizedKeys: resolve server first, scope assignments and keys
to that server's org (was honouring foreign assignment rows)
- Workflows: validate TargetServerIDs on create/update and re-check at
trigger time
- Monitor incidents/uptime handlers: gate on org-scoped GetMonitor
- GetChannels: take orgID; validate channel_ids on monitor create/update
- Secret and default-step unique indexes: scope to org_id so a second
org no longer hits E11000
- DeleteServer/DeleteMonitor: scope cascading deletes
Threads org_id through every admin-facing service function (servers, keys,
assignments, secrets, workflows/steps/runs, monitors, channels, audit),
adds RequireRole middleware, and wires /api/org user + OIDC management
routes. Agent/scheduler paths keep unique-key signatures and resolve org
from the loaded record; internal-only helpers (getServerByID,
getRunByID, getMonitorByID) preserve those call sites.
mrhid6
domrichardson