cairnobs/docs/agent-management-design.md

# Agent inventory, management, and remote config

Extends `/docs/agent-heartbeat-monitoring.md` (heartbeat + absence
alerting). This adds three things to the web UI: an inventory view of
every agent that's checked in, read-only visibility into each agent's
actual running config, and the ability to remotely edit a narrow,
deliberately-scoped subset of that config.

## Scope decision (confirmed before building)

"Agent management" could mean several different things with very
different risk profiles. Confirmed up front: this covers inventory,
config visibility, and remote config *editing* — explicitly **not**
remote lifecycle commands (restart/stop/uninstall). That's a real
command-and-control channel across every managed host and deserves its
own security design (signed commands, strict RBAC, full audit trail)
before it's built, not something to fold in as a side effect of a
config-editing feature.

## Why this is still pull, not push, on the wire

The agent's transport is unchanged and still push-only: it dials
*out* to `ingest` over mTLS; nothing in the platform ever reaches into
an agent. See `/docs/agent-heartbeat-monitoring.md`'s "Design: why this
is a heartbeat, not a true pull" section for the full reasoning (NAT/
dynamic-IP tolerance, no inbound port on any remote host). Remote config
editing extends that same posture: an operator's edit doesn't get
pushed to the agent at the moment it's saved. It sits in Postgres until
the agent's own next scheduled check-in asks "what should I be
running" and picks it up. The web UI's "pending" vs. "applied" badge
exists specifically to make that asynchrony visible rather than
implying something more immediate.

## Wire shape: `AgentControl.CheckIn`

New proto (`proto/sentry/agent/v1/agent_control.proto`), a second gRPC
service on the exact same mTLS channel/listener `LogIngest.PushBatch`
already uses — not a second protocol or connection the agent has to
maintain. Called on the agent's own heartbeat ticker (see
`agent/sentry-agent/src/main.rs`'s `heartbeat_ticker` arm),
independently of whether the heartbeat log record itself is enabled —
CheckIn keeps running even with `heartbeat.enabled = false`, since
that's an agent's only path to ever receive a remote override that
re-enables it.

- **Request**: host/service identity, a `ReportedConfig` snapshot of
  what the agent is actually running (version, source kind/detail,
  batch settings, heartbeat settings), and the version of the last
  override this agent successfully applied (empty if never).
- **Response**: `has_override` plus, when true, a `DesiredOverride` —
  every field optional (unset = "no change to this field, keep local
  config"), each independently overridable.

## What's remotely editable, and what deliberately isn't

Editable: `batch.max_size`, `batch.flush_interval_ms`,
`heartbeat.enabled`, `heartbeat.interval`, and — only when the agent's
local source is journald — the unit filter.

**Never editable, permanently**: TLS material and the ingest endpoint.
`ReportedConfig` doesn't even carry these fields, and
`DesiredOverride` has no fields for them at all — this isn't a
validation rule that could be relaxed later, it's a shape decision.
Two reasons, both serious enough that this needed to be a design
boundary rather than a judgment call per edit:

1. **A bad edit could permanently strand an agent.** Point an agent's
   `ingest.endpoint` at an address that doesn't exist, or corrupt its
   TLS config, and it can never call `CheckIn` again to receive a
   correction — the one channel capable of fixing the mistake would be
   exactly what broke. Every other editable field is safe by
   construction: even a bad heartbeat interval or an over-aggressive
   batch size degrades the agent's behavior without ever cutting off
   its ability to receive the next correction.
2. **A compromised web session/API credential must not be able to
   redirect where an agent's logs go.** If `ingest.endpoint` were
   editable, an attacker with write access to this feature could point
   agents at an address they control and exfiltrate log data. Keeping
   connection details local-file-only means this feature's blast
   radius is "an agent's operational tuning gets messed with," never
   "an agent's data goes somewhere else."

`validateOverride` (`api/agents/handler.go`) additionally floors
`batch_max_size >= 1`, `batch_flush_interval_ms >= 100`, and
`heartbeat_interval_ms >= 5000` — the same kind of real, found-by-
building-it floor as alerting's `eval_interval_seconds >= 30`, here to
stop a fat-fingered edit from telling an agent to flush constantly or
heartbeat constantly.

## Merge semantics: an override is a live layer, not a rewrite

The agent's local `agent.toml` is never rewritten. A remote override
lives only in the running process's memory
(`agent/sentry-agent/src/main.rs`'s `apply_override`) and is re-applied
fresh on every check-in that returns one — a restarted agent boots from
`agent.toml` alone and re-syncs whatever override is still set on its
next successful check-in. This was a deliberate simplicity choice over
persisting the override to disk: it avoids needing filesystem write
access on every managed host (not guaranteed, e.g. read-only base
images) and avoids a whole "reconcile a locally-cached override against
a freshly-fetched one at startup" state machine. The cost is that an
agent's effective config isn't fully recoverable from `agent.toml`
alone while an override is active — acceptable, since the web UI's
`GET /agents/{host}` is the source of truth for "what is this agent
actually running" regardless.

Applying an override that changes `batch_max_size`/
`batch_flush_interval_ms` rebuilds the `Batcher`/flush ticker outright.
Whatever was buffered under the old settings is flushed first
(`Batcher::flush_all`, new) rather than dropped — building this exposed
a real, independent, pre-existing bug: agent shutdown was calling
`poll_timeout()`, which only drains when `flush_interval` has already
elapsed, meaning records buffered more recently than that were silently
lost on every graceful shutdown that happened to land between flushes.
Fixed alongside this feature (`flush_all()` now used at both shutdown
and hot-reload) since it's the exact same correctness property in both
places.

Changing the journald unit filter is the one override that can't just
swap a struct field — there's no way to change what
`source::journald::run` is tailing without restarting that task. Applying
it aborts the current source task and respawns a fresh one with the new
filter, swapping the channel `main.rs` reads from. Source *kind*
(journald vs. file vs. eventlog vs. etw) is never remotely switchable —
only narrowing/widening the filter within whatever source the host is
already configured for.

## Data model

`metadata/migrations/0037_create_agents.sql`: one `agents` table, one
row per `(tenant_id, host)`, in the same `sentry_metadata` Postgres
dashboards/alert_rules already live in — not a new database, matching
this project's established "shared schema, different services own
different tables" shape. `tenant_id` defaults to `'default'` for
single-tenant deployments with no `TenantResolver` configured on
ingest, same as every other tenant-scoped table since Phase 3.

Two services write to it, cleanly split by concern:

- **`ingest/internal/agentregistry`** (new) upserts on every `CheckIn`:
  `last_seen_at`, the `reported_*` columns, and
  `applied_override_version` (echoed from the agent). Reads back
  whatever `desired_override`/`desired_override_version` is currently
  stored to answer the RPC. Gated on `AGENT_REGISTRY_POSTGRES_ADDR`
  being set — nil/off by default, same "off unless configured" shape as
  `TenantResolver`; a deployment that hasn't opted in still accepts
  `CheckIn` calls (agents never see an error), it just doesn't record
  anything or ever return an override.
- **`api/agents`** (new) is the web-facing read/write side: `GET
  /agents`, `GET /agents/{host}`, `PUT /agents/{host}/config` (replaces
  the whole stored override — the web UI's edit form always reads the
  current override first and submits the complete merged set, same
  "PUT replaces the resource" convention every other edit form in this
  codebase already uses), `DELETE /agents/{host}/config` (clears it,
  reverting the agent to its local `agent.toml`).

`ConfigOverride`'s JSON shape is duplicated three times — Go structs in
`ingest/internal/agentregistry` and `api/agents`, a proto message for
the wire — deliberately, matching this codebase's established
convention for shapes shared across module boundaries (see
`grpcserver.TenantIDHeaderKey`, `enterprise/internal/apiconfig.AIConfig`)
rather than coupling independently deployable services' builds
together. Keep the three in sync by hand.

## RBAC

Viewing inventory is `RoleViewer` (same bar as viewing a dashboard);
editing an agent's remote config is `RoleEditor` — treated as an
operational-tuning action matching alert rules/notification targets,
not an admin-only capability like user/role management.

## Verified live

See the runbook entry (task follow-up) for the full walkthrough: a real
agent binary, its heartbeat/CheckIn cadence pointed at a live
`ingest` with `AGENT_REGISTRY_POSTGRES_ADDR` configured, confirming (a)
the agent appears in `GET /agents` after its first check-in, (b) an
edit made via `PUT /agents/{host}/config` shows `pending: true`
immediately and `pending: false` after the agent's next check-in, and
(c) the edited setting (heartbeat interval) visibly takes effect in the
agent's own behavior — confirmed by the change in cadence of new
heartbeat rows landing in ClickHouse.